Open
Conversation
aldigjo
force-pushed
the
submit_verifications
branch
from
b35afec to
3a2eeb1
Compare
| return CheckVerification200JSONResponse{union: json.RawMessage(fmt.Sprintf(`{"query": %v}`, query))}, nil | ||
| } | ||
|
|
||
| return CheckVerification404JSONResponse{N404JSONResponse{Message: "Verification query not found"}}, nil |
Contributor
There was a problem hiding this comment.
default response should be 500. To return error 404 we'd check the error from the repository /service layer
|
|
||
| // VerificationService interface | ||
| type VerificationService interface { | ||
| CheckVerification(ctx context.Context, issuerID w3c.DID, verificationQueryID uuid.UUID) (*domain.VerificationResponse, *domain.VerificationQuery, error) |
Contributor
There was a problem hiding this comment.
methods inside services should not contain the service name. Rename this to Check
| // VerificationService interface | ||
| type VerificationService interface { | ||
| CheckVerification(ctx context.Context, issuerID w3c.DID, verificationQueryID uuid.UUID) (*domain.VerificationResponse, *domain.VerificationQuery, error) | ||
| SubmitVerificationResponse(ctx context.Context, verificationQueryID uuid.UUID, issuerID w3c.DID, token string, serverURL string) (*domain.VerificationResponse, error) |
Contributor
There was a problem hiding this comment.
methods inside services should not contain the service name. Rename this to SubmitResponse
Contributor
|
Please, run golangci-lint (or make lint) and fix linter warnings. |
x1m3
reviewed
Dec 16, 2024
| } | ||
|
|
||
| nullifierSessionID := new(big.Int) | ||
| if _, ok := nullifierSessionID.SetString(val.(string), defaultBigIntBase); !ok { |
Contributor
There was a problem hiding this comment.
We should check that this conversion to string is valid. If not we will have a core dump.
Something like this.
var sVal string
if sVal, ok = val.(string); ok {
return nil, errors.New("nullifierSessionID is not a string")
}
Working with map[string]interfaces needs a lot of boilerplate
aldigjo
force-pushed
the
submit_verifications
branch
from
392cf6f to
fdb7307
Compare
aldigjo
force-pushed
the
submit_verifications
branch
from
54fd89b to
eb0b06c
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adding verification endpoints