-
Notifications
You must be signed in to change notification settings - Fork 675
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
ACP 118 reference implementation #3218
Conversation
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.
Makes sense to me, would defer to those with more context of the codebase to comment on where the definitions live within the files/packages.
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.
looks good to me, but would be happy to have others review this as well.
After adding the couple requested comments - lgtm |
Why this should be merged
SignatureRequest
andSignatureResponse
AppRequest
/AppResponse
message types as defined in ACP-118Defines the canonical handler ID forSignatureRequests
as1
, as specified in ACP-118: Specify SignatureRequest handler ID avalanche-foundation/ACPs#129SignatureRequests
as2
, as specified in ACP 118 - update handler ID and rename field avalanche-foundation/ACPs#132p2p
packageDoes not go so far as to add a new package under
network/p2p
with message parsing, message marshalling, default handlers, etc. ala the gossip package.How this works
See above
How this was tested
CI