Skip to content

eth/catalyst: fix races #26950

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Mar 22, 2023
Merged

eth/catalyst: fix races #26950

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4c080bb
eth/catalyst: fix races
MariusVanDerWijden Mar 22, 2023
146af0a
eth/catalyst: fix race
MariusVanDerWijden Mar 22, 2023
fd90169
eth/catalyst: reduce diff
MariusVanDerWijden Mar 22, 2023
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
36 changes: 27 additions & 9 deletions eth/catalyst/api_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -879,15 +879,10 @@ func TestNewPayloadOnInvalidTerminalBlock(t *testing.T) {
genesis, preMergeBlocks := generateMergeChain(100, false)
n, ethservice := startEthService(t, genesis, preMergeBlocks)
defer n.Close()

ethservice.BlockChain().Config().TerminalTotalDifficulty = preMergeBlocks[0].Difficulty() //.Sub(genesis.Config.TerminalTotalDifficulty, preMergeBlocks[len(preMergeBlocks)-1].Difficulty())

var (
api = NewConsensusAPI(ethservice)
parent = preMergeBlocks[len(preMergeBlocks)-1]
)
api := NewConsensusAPI(ethservice)

// Test parent already post TTD in FCU
parent := preMergeBlocks[len(preMergeBlocks)-2]
fcState := engine.ForkchoiceStateV1{
HeadBlockHash: parent.Hash(),
SafeBlockHash: common.Hash{},
Expand All @@ -913,6 +908,28 @@ func TestNewPayloadOnInvalidTerminalBlock(t *testing.T) {
t.Fatalf("error preparing payload, err=%v", err)
}
data := *payload.Resolve().ExecutionPayload
// We need to recompute the blockhash, since the miner computes a wrong (correct) blockhash
txs, _ := decodeTransactions(data.Transactions)
header := &types.Header{
ParentHash: data.ParentHash,
UncleHash: types.EmptyUncleHash,
Coinbase: data.FeeRecipient,
Root: data.StateRoot,
TxHash: types.DeriveSha(types.Transactions(txs), trie.NewStackTrie(nil)),
ReceiptHash: data.ReceiptsRoot,
Bloom: types.BytesToBloom(data.LogsBloom),
Difficulty: common.Big0,
Number: new(big.Int).SetUint64(data.Number),
GasLimit: data.GasLimit,
GasUsed: data.GasUsed,
Time: data.Timestamp,
BaseFee: data.BaseFeePerGas,
Extra: data.ExtraData,
MixDigest: data.Random,
}
block := types.NewBlockWithHeader(header).WithBody(txs, nil /* uncles */)
data.BlockHash = block.Hash()
// Send the new payload
resp2, err := api.NewPayloadV1(data)
if err != nil {
t.Fatalf("error sending NewPayload, err=%v", err)
Expand Down Expand Up @@ -1240,9 +1257,10 @@ func TestNilWithdrawals(t *testing.T) {

func setupBodies(t *testing.T) (*node.Node, *eth.Ethereum, []*types.Block) {
genesis, blocks := generateMergeChain(10, true)
n, ethservice := startEthService(t, genesis, blocks)
// enable shanghai on the last block
ethservice.BlockChain().Config().ShanghaiTime = &blocks[len(blocks)-1].Header().Time
time := blocks[len(blocks)-1].Header().Time + 1
Copy link
Contributor

Choose a reason for hiding this comment

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

This does something more than just fix a races, it also bumps the time by 1 second. So it also either fixes or introduces a bug... ?

Copy link
Member Author

Choose a reason for hiding this comment

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

It fixes a bug. Previously shanghai was enabled before the last block, now its enabled afterwards. This means previously we would import the block on pre-shanghai rules, then afterwards set the chain config so the last block is post-shanghai. Basically we imported the block and modified the rules afterwards.

genesis.Config.ShanghaiTime = &time
n, ethservice := startEthService(t, genesis, blocks)

var (
parent = ethservice.BlockChain().CurrentBlock()
Expand Down