Skip to content
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

Add P-Chain ctx into inner block and verify within snow pkg #1897

Merged
merged 7 commits into from
Jan 31, 2025

Conversation

aaronbuchwald
Copy link
Collaborator

This PR adds the P-Chain context into the inner block.

This embeds it directly into the inner block, so that it's always available to the inner block. By adding this into the inner block interface, we can verify it against the consensus context in the snow package, so that the inner VM/chain does not need to.

@aaronbuchwald aaronbuchwald marked this pull request as ready for review January 30, 2025 15:30
@aaronbuchwald
Copy link
Collaborator Author

b4ec9ce fixes a bug where the block implements block.WithVerifyContext, but the VM did not implement block.BuildBlockWithContextChainVM so we'd build a block without the context and verify it with the P-Chain context.

Needed to add the covariant version of BuildBlockWithContext to implement the interface correctly and fixes the issue.

@aaronbuchwald
Copy link
Collaborator Author

ref on ProposerVM: https://github.com/ava-labs/avalanchego/tree/master/vms/proposervm#snowman-congestion-control-for-snowman-vms

We need the ProposerVM block context to verify chunk signatures for DSMR and for ICM signature verification.

@@ -20,6 +21,8 @@ type StatelessBlock struct {
Tmstmp int64 `json:"timestamp"`
Hght uint64 `json:"height"`

BlockContext *block.Context `json:"pChainHeight"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we change the struct tag from json:"pChainHeight" to json:"blockContext"?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same thought here. The inner BlockContext object would already have a json field name PChainHeight.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ya good callout

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It only contains P-Chain height, but this would be better

@aaronbuchwald aaronbuchwald merged commit 14ea750 into main Jan 31, 2025
17 checks passed
@aaronbuchwald aaronbuchwald deleted the pchain-ctx branch January 31, 2025 16:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants