Feature/run 79 verify presentation api #277
Conversation
squirer
changed the base branch from
main
to
feature/RUN-36-credential-verification-service
| }; | ||
| Ok(Json(verify_presentation_response)) | ||
| } | ||
| Err(e) => Err(ServerError::PresentationVerifificationFailed(e)), |
There was a problem hiding this comment.
Can we similar to the other endpoint, check if there is an account nonce mismatch. If that is the case, refresh the nonce in the state and re-submit the tx.
|
|
||
| match presentation_verification_data_result { | ||
| Ok(presentation_verification_data) => { | ||
| let result = match presentation_verification_data.verification_result { |
There was a problem hiding this comment.
The account nonce needs to be updated in the state. Otherwise the next tx sent by the service will fail.
| tracing::error!("Presentaion Verification Failed: {error}."); | ||
| ( | ||
| StatusCode::BAD_REQUEST, | ||
| Json("Presentation failed its verification.".to_string()), |
There was a problem hiding this comment.
I think it would be good to let the user know why the verification failed in a bad_request case but not in the internal_error case.
| } | ||
| // TODO - here needs to be expanded with some code reasoning probably. Blanket 400 for now | ||
| ServerError::PresentationVerifificationFailed(error) => { | ||
| tracing::error!("Presentaion Verification Failed: {error}."); |
There was a problem hiding this comment.
If it is bad request, then nothing is wrong with our service, so I would only warn in that case.
| tracing::error!("Presentaion Verification Failed: {error}."); | |
| tracing::warn!("Presentaion Verification Failed: {error}."); |
As you TODO already mentions, we should distinguish between some internal_errors and bad_request errors here.
There was a problem hiding this comment.
Im not sure I agree with the warning level here, maybe I need more convincing...If we reach a case where there is a VerifyError occurring for some presentation that is being verified, it feels more like an error than a warning and the reason I say this is because the VerifyError enum lists out a bunch of cases that should for the most part never happen right?
Like in the happy path flow if the user has created and proven their Verification Request and created a valid Verifiable Presentation, then getting to this point is likely something that should be announced as an ERROR log in which case something needs a bit further investigating/tuning.
This is my gut feeling, and I set BAD_REQUEST as I think we need to define the error structure we want with codes and descriptions before handling case by case.
Co-authored-by: Doris Benda <[email protected]>
Co-authored-by: Doris Benda <[email protected]>
Co-authored-by: Doris Benda <[email protected]>
Co-authored-by: Doris Benda <[email protected]>
Purpose
Create the Verify presentation API initial flow
Changes
_Describe the changes that were needed.
Checklist
hard-to-understand areas.