feat: support parameter #967
Conversation
Signed-off-by: Bassam Riman <bassam.riman@iohk.io>
CryptoKnightIOG
requested review from
bvoiturier,
yshyn-iohk,
patlo-iog and
mineme0110
as code owners
There was a problem hiding this comment.
Good job @CryptoKnightIOG. I left a couple of comments... 😉
| } | ||
| } | ||
|
|
||
| case class AudienceParameter(aud: String) extends VcVerificationParameter |
There was a problem hiding this comment.
Do we really want to keep both the enum part and the related xxxParameter(xxx) part? It seems like there is always a 1:1 mapping between them. Keeping both allows the user to mix unrelated types together (e.g. SubjectCheck with AudienceParameter). Why don't we just have AudienceCheck(aud: String) instead?
There was a problem hiding this comment.
You are right, will fix it in follow up
| final case class VcVerificationRequest( | ||
| credential: String, | ||
| verifications: List[VcVerification] | ||
| verification: VcVerification, |
There was a problem hiding this comment.
Why not revert to what you did before, that is, passing a list of verifications to the service verify method? This allows one to make a single service call to check, e.g. 10 properties of a VC, instead of making 10 separate calls. I find it more efficient and elegant...
There was a problem hiding this comment.
I am not passing a list of VcVerificationRequest in a single call. It simplifies the logic.
| case SemanticCheckOfClaims | ||
| } | ||
|
|
||
| object VcVerification { |
There was a problem hiding this comment.
It's up to you @CryptoKnightIOG, but if you want to reduce the conversion logic, you can also expose the VcVerification from the service layer right away. It seems they have the exact same structure, right? We do that a lot in other parts of the codebase...
There was a problem hiding this comment.
Will fix in follow up
CryptoKnightIOG
merged commit 4be27ab
into
ATL-6785-6789-Semantic-Subject-Verification
https://input-output.atlassian.net/browse/ATL-6913