-
Notifications
You must be signed in to change notification settings - Fork 671
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
cache signatures only in acp118 handler #3474
base: master
Are you sure you want to change the base?
Conversation
network/p2p/acp118/handler_test.go
Outdated
if tt.cacher.Len() > 0 { | ||
// Ensure the cache is populated with correct signature | ||
sig, ok := tt.cacher.Get(unsignedMessage.ID()) | ||
require.True(ok) | ||
require.Equal(sig, response.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.
Rather than checking the length (which seems fairly fragile) we should just assert that the signature is as expected if the cacher returns the 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.
done
network/p2p/acp118/handler.go
Outdated
return responseBytesFromSignature(signature) | ||
} | ||
|
||
func responseBytesFromSignature(signature []byte) ([]byte, *common.AppError) { |
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.
func responseBytesFromSignature(signature []byte) ([]byte, *common.AppError) { | |
func signatureToResponse(signature []byte) ([]byte, *common.AppError) { |
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
Why this should be merged
Cache might be needed to shared with other components. Currently we cache whole proto signature response, a plain signature caching can be more straightforward to be returned from shared cache for other components.
For instance Subnet-EVM can use this signature cache in API calls and in legacy p2p handler.
How this works
Caches and returns direct signature bytes rather than proto
SignatureResponse
How this was tested
Added UT