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

Check builder's payload block number #13224

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

Check builder's payload block number #13224

wants to merge 2 commits into from

Conversation

potuz
Copy link
Contributor

@potuz potuz commented Nov 24, 2023

Before checking for withdrawals roots that involves hashing, perform a quick check that the builder actually returned a block for the right block number. Previous version of this PR would actually check the parent hash, but fixing tests is more involved.

@potuz potuz added Ready For Review A pull request ready for code review OK to merge labels Nov 24, 2023
@potuz potuz requested a review from a team as a code owner November 24, 2023 13:39
@potuz potuz changed the title Check builder's payload parent hash Check builder's payload block number Nov 24, 2023
@potuz potuz removed the OK to merge label Nov 24, 2023
@@ -71,6 +71,11 @@ func setExecutionData(ctx context.Context, blk interfaces.SignedBeaconBlock, loc
log.WithError(err).Warn("Proposer: failed to get builder payload value") // Default to local if can't get builder value.
return setLocalExecution(blk, localPayload)
}
// Check that the builder has built the right blocknumber
if builderPayload.BlockNumber() != localPayload.BlockNumber() {
log.Warn("Proposer: builder's payload has the wrong block number, using local block.") // Default to local if can't get builder value.
Copy link
Member

Choose a reason for hiding this comment

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

Makes sense to me. Can we also log the numbers?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready For Review A pull request ready for code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants