-
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
Parsing acp-77 warp payloads #3270
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.
Is there follow-on work expected that would exercise these new payloads? Even at the risk of a larger review burden, I think it's generally preferable for new code to be exercised in CI before merge.
vms/platformvm/warp/messages/subnet_validator_registration_message.go
Outdated
Show resolved
Hide resolved
I added unit tests for the new package. Until a non-dummy signature request handler is added, I'm not sure how meaningful an E2E or integration test would be. In the meantime, we have incorporated these types and the dummy handler in our E2E testing on the staking contract side. The relevant bit of that testing is that we're able to construct signed Warp messages that contain payloads of these types, and verify them in the smart contract using the Warp precompile. |
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.
LGTM
Why this should be merged
Adds type definitions for the Warp message payloads that the P-Chain signs to implement subnet-only validators, as specified in ACP-77
How this works
Defines the Warp message payload types and registers them with the codec.
How this was tested
CI