-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Signature / proof verification #84
Conversation
Refactor VerifyProof
publicKey.X = rawSlotInts[2] // Ax should be in indexSlotA | ||
publicKey.Y = rawSlotInts[3] // Ay should be in indexSlotB | ||
|
||
sig, err := bjjSignatureFromHexString(proof.Signature) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can use already exists bjj.SignatureComp. This method has MarshalText and UnmarshalText that encode/decode from/to hex.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MarshalText/UnmarshalText works with bytes, inside bjjSignatureFromHexString
we just converting to byres and call Decompress
verifiable/status_direct.go
Outdated
httpResp.StatusCode) | ||
} | ||
|
||
respData, err := io.ReadAll(io.LimitReader(httpResp.Body, limitReaderBytes)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this case, you can read only half of the body. Since LinitRead sets only how many bytes should be read and ReadAll reads these bytes.
You should limit read operation and return an error if the real body of the buffer bigger then limit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
verifiable/status_direct.go
Outdated
|
||
statusOK := httpResp.StatusCode >= 200 && httpResp.StatusCode < 300 | ||
if !statusOK { | ||
return out, fmt.Errorf("unexpected status code: %v", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use %d for int
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
delete(r.resolvers, resolverType) | ||
} | ||
|
||
var DefaultCredentialStatusResolverRegistry = &CredentialStatusResolverRegistry{} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why need this empty default resolver?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe for the same reasons we need http.DefaultClient
. You can create your own copy of the registry and use different registries for different purposes. However, in 99% of cases, you only need one registry per application. Therefore, it's simply a convenient way to use global functions like GetStatusResolver
, RegisterStatusResolver
that operate on the default registry and do not compel you to create your own instance of this registry.
verifiable/proof.go
Outdated
@@ -22,6 +22,11 @@ type IssuerData struct { | |||
CredentialStatus interface{} `json:"credentialStatus,omitempty"` | |||
} | |||
|
|||
func (id *IssuerData) authClaim() (*core.Claim, error) { | |||
var claim core.Claim | |||
return &claim, claim.FromHex(id.AuthCoreClaim) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's hard to read. Split to separate lines.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
separated
resp *http.Response | ||
httpClient *http.Client | ||
) | ||
if r.customHTTPClient != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can simplify
httpClient = http.DefaultClient
if r.customHTTPClient != nil {
httpClient = r.customHTTPClient
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
verifiable/did_resolver.go
Outdated
|
||
defer func() { | ||
err2 := resp.Body.Close() | ||
if err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if err2 is nil, but err == nil, you will return empty error
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
No description provided.