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

eth/catalyst: fix engine API #28135

Merged
merged 1 commit into from
Sep 17, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions eth/catalyst/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -207,21 +207,21 @@ func (api *ConsensusAPI) verifyPayloadAttributes(attr *engine.PayloadAttributes)
c := api.eth.BlockChain().Config()

// Verify withdrawals attribute for Shanghai.
if err := checkAttribute(c.IsShanghai, attr.Withdrawals != nil, attr.Timestamp); err != nil {
if err := checkAttribute(c.IsShanghai, attr.Withdrawals != nil, c.LondonBlock, attr.Timestamp); err != nil {
return fmt.Errorf("invalid withdrawals: %w", err)
}
// Verify beacon root attribute for Cancun.
if err := checkAttribute(c.IsCancun, attr.BeaconRoot != nil, attr.Timestamp); err != nil {
if err := checkAttribute(c.IsCancun, attr.BeaconRoot != nil, c.LondonBlock, attr.Timestamp); err != nil {
return fmt.Errorf("invalid parent beacon block root: %w", err)
}
return nil
}

func checkAttribute(active func(*big.Int, uint64) bool, exists bool, time uint64) error {
if active(common.Big0, time) && !exists {
func checkAttribute(active func(*big.Int, uint64) bool, exists bool, block *big.Int, time uint64) error {
Copy link
Member Author

@rjl493456442 rjl493456442 Sep 17, 2023

Choose a reason for hiding this comment

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

The london block is used to align with line https://github.com/ethereum/go-ethereum/pull/28135/files#diff-96840e4882901b94cef9f51a992a44e64df659335ff13dd011be8c1ffa097135R179

Although I don't think london block is correct here. The block number to generate is the one should pass, IIUC.

Copy link
Member

Choose a reason for hiding this comment

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

The original code passed the weird london block because in merge mode we know that we already passed london. So instead of complicating everything with passing a block number, it was a hacky way to hard code "always london".

if active(block, time) && !exists {
return errors.New("fork active, missing expected attribute")
}
if !active(common.Big0, time) && exists {
if !active(block, time) && exists {
return errors.New("fork inactive, unexpected attribute set")
}
return nil
Expand Down