Skip to content

MSM: Enforce P-chain height at least as high as the parent's - #670

Open
yacovm wants to merge 4 commits into
mainfrom
656
Open

yacovm wants to merge 4 commits into
mainfrom
656

Conversation

@yacovm

@yacovm yacovm commented Oct 27, 2026 •

Copy link
Copy Markdown
Collaborator

This commit makes the MSM carry over the P-chain height of the parent in case it's higher than the sampled P-chain height, to make sure the P-chain is non-decreasing.

Comment thread msm/msm.go Outdated

blockBuildingDecider := sm.createBlockBuildingDecider(currentValidatorSet)
// Overwrite the GetPChainHeight function to ensure that we always propose a block with a P-chain height
// that is at least as high as the parent block's P-chain height.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

i dont get how we can verify a block that has a higher pchain height than ours? we have verifyPChainHeight in the msm, that rejects heights too big. Therefore wouldn't we not build on top of a block with a higher pchain?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Recall that we have two P-chain height functions - one for proposing and one for verifying. The one for proposing may return a slightly stale P-chain height. The one for verifying returns the latest height.
So, when we build a block ourselves, we use the one for proposing and it may return a slightly stale height. But the parent block might have been built with a higher P-chain height that may still be lower than the P-chain height returned by the function we use for verifying. It's all legal.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

i see because we have GetMinimumPChainHeight and GetCurrentPChainHeight

Comment thread msm/msm.go Outdated
blockBuildingDecider := sm.createBlockBuildingDecider(currentValidatorSet)
// Overwrite the GetPChainHeight function to ensure that we always propose a block with a P-chain height
// that is at least as high as the parent block's P-chain height.
blockBuildingDecider.getPChainHeight = func() uint64 {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

can we just set the getPChainHeight field in the constructor? rather than having to do it after

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

which constructor? can you be more explicit? I don't understand

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

im referring to the createBlockBuildingDecider constructor. We can just pass in the another variable to initialize it.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

done

Comment thread msm/msm.go
icmEpochInfo := computeICMEpochInfo(parentBlock, sm.ComputeICMEpoch, now)
pChainHeight := sm.GetPChainHeightForProposing()
return wrapBlock(nil, newSimplexEpochInfo, pChainHeight, simplexMetadata, simplexBlacklist, now, icmEpochInfo, nil), nil
if parentBlock.Metadata.PChainHeight > pChainHeight {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

if we have it in the constructor above, we can just call blockBuildingDecider.getPChainHeight here

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

done 6021aa7

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

done

Comment thread instance_test.go Resolved
Comment thread instance_test.go Resolved
Signed-off-by: Yacov Manevich <yacov.manevich@avalabs.org>
Signed-off-by: Yacov Manevich <yacov.manevich@avalabs.org>
Signed-off-by: Yacov Manevich <yacov.manevich@avalabs.org>
Signed-off-by: Yacov Manevich <yacov.manevich@avalabs.org>
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.

Builder ignores parent's P-chain height, so a leader's maximal height stalls honest proposals

2 participants