From 1d8d87976495d1bb380f2071f1f81b38f4d8a7f8 Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Thu, 21 Mar 2024 07:55:23 +0100 Subject: [PATCH] all: fix tests --- beacon/engine/types.go | 5 +++- consensus/beacon/consensus.go | 2 +- core/chain_makers.go | 13 ++++---- core/types/block.go | 12 ++++---- eth/catalyst/api.go | 43 +++++++++++++++------------ eth/catalyst/simulated_beacon_test.go | 2 +- eth/protocols/eth/handler_test.go | 4 +-- miner/payload_building.go | 36 +++++++++++----------- miner/worker.go | 25 +++++++++------- rpc/subscription_test.go | 2 +- 10 files changed, 81 insertions(+), 63 deletions(-) diff --git a/beacon/engine/types.go b/beacon/engine/types.go index 564e001333..d4f169d2bd 100644 --- a/beacon/engine/types.go +++ b/beacon/engine/types.go @@ -261,7 +261,10 @@ func ExecutableDataToBlock(params ExecutableData, versionedHashes []common.Hash, ParentBeaconRoot: beaconRoot, InclusionListSummaryRoot: inclusionRoot, } - block := types.NewBlockWithHeader(header).WithBody(txs, nil /* uncles */).WithWithdrawals(params.Withdrawals).WithInclusionList(params.InclusionListSummary.Summary) + block := types.NewBlockWithHeader(header).WithBody(txs, nil /* uncles */).WithWithdrawals(params.Withdrawals) + if inclusionRoot != nil { + block = block.WithInclusionList(params.InclusionListSummary.Summary) + } if block.Hash() != params.BlockHash { return nil, fmt.Errorf("blockhash mismatch, want %x, got %x", params.BlockHash, block.Hash()) } diff --git a/consensus/beacon/consensus.go b/consensus/beacon/consensus.go index 9ffed438a8..cf176934dd 100644 --- a/consensus/beacon/consensus.go +++ b/consensus/beacon/consensus.go @@ -387,7 +387,7 @@ func (beacon *Beacon) FinalizeAndAssemble(chain consensus.ChainHeaderReader, hea header.Root = state.IntermediateRoot(true) // Assemble and return the final block. - return types.NewBlockWithWithdrawals(header, body.Transactions, body.Uncles, receipts, body.Withdrawals, trie.NewStackTrie(nil)), nil + return types.NewBlockWithWithdrawals(header, body.Transactions, body.Uncles, receipts, body.Withdrawals, trie.NewStackTrie(nil)).WithInclusionList(body.InclusionListSummary), nil } // Seal generates a new sealing request for the given input block and pushes diff --git a/core/chain_makers.go b/core/chain_makers.go index 1c42ab0c9a..288864de75 100644 --- a/core/chain_makers.go +++ b/core/chain_makers.go @@ -44,11 +44,12 @@ type BlockGen struct { header *types.Header statedb *state.StateDB - gasPool *GasPool - txs []*types.Transaction - receipts []*types.Receipt - uncles []*types.Header - withdrawals []*types.Withdrawal + gasPool *GasPool + txs []*types.Transaction + receipts []*types.Receipt + uncles []*types.Header + withdrawals []*types.Withdrawal + inclusionListSummary []*types.InclusionListEntry engine consensus.Engine } @@ -345,7 +346,7 @@ func GenerateChain(config *params.ChainConfig, parent *types.Block, engine conse gen(i, b) } - body := types.Body{Transactions: b.txs, Uncles: b.uncles, Withdrawals: b.withdrawals} + body := types.Body{Transactions: b.txs, Uncles: b.uncles, Withdrawals: b.withdrawals, InclusionListSummary: b.inclusionListSummary} block, err := b.engine.FinalizeAndAssemble(cm, b.header, statedb, &body, b.receipts) if err != nil { panic(err) diff --git a/core/types/block.go b/core/types/block.go index 8a89c9f37c..0e932c2ef0 100644 --- a/core/types/block.go +++ b/core/types/block.go @@ -320,7 +320,7 @@ func (b *Block) DecodeRLP(s *rlp.Stream) error { if err := s.Decode(&eb); err != nil { return err } - b.header, b.uncles, b.transactions, b.withdrawals = eb.Header, eb.Uncles, eb.Txs, eb.Withdrawals + b.header, b.uncles, b.transactions, b.withdrawals, b.inclusionListSummary = eb.Header, eb.Uncles, eb.Txs, eb.Withdrawals, eb.InclusionListSummary b.size.Store(rlp.ListSize(size)) return nil } @@ -328,10 +328,11 @@ func (b *Block) DecodeRLP(s *rlp.Stream) error { // EncodeRLP serializes a block as RLP. func (b *Block) EncodeRLP(w io.Writer) error { return rlp.Encode(w, &extblock{ - Header: b.header, - Txs: b.transactions, - Uncles: b.uncles, - Withdrawals: b.withdrawals, + Header: b.header, + Txs: b.transactions, + Uncles: b.uncles, + Withdrawals: b.withdrawals, + InclusionListSummary: b.inclusionListSummary, }) } @@ -495,6 +496,7 @@ func (b *Block) WithInclusionList(summary []*InclusionListEntry) *Block { header: b.header, transactions: b.transactions, uncles: b.uncles, + withdrawals: b.withdrawals, } if summary != nil { block.inclusionListSummary = make([]*InclusionListEntry, len(summary)) diff --git a/eth/catalyst/api.go b/eth/catalyst/api.go index 8edecb18a1..4a01becd7a 100644 --- a/eth/catalyst/api.go +++ b/eth/catalyst/api.go @@ -137,7 +137,8 @@ type ConsensusAPI struct { forkchoiceLock sync.Mutex // Lock for the forkChoiceUpdated method newPayloadLock sync.Mutex // Lock for the NewPayload method - coolMapThatIsNotAMemLeak map[common.Hash][]*types.Transaction + coolMapThatIsNotAMemLeak map[common.Hash][]*types.Transaction + coolMapThatIsNotAMemLeak2 map[common.Hash]*types.InclusionListSummary } // NewConsensusAPI creates a new consensus api for the given backend. @@ -154,12 +155,13 @@ func newConsensusAPIWithoutHeartbeat(eth *eth.Ethereum) *ConsensusAPI { log.Warn("Engine API started but chain not configured for merge yet") } api := &ConsensusAPI{ - eth: eth, - remoteBlocks: newHeaderQueue(), - localBlocks: newPayloadQueue(), - invalidBlocksHits: make(map[common.Hash]int), - invalidTipsets: make(map[common.Hash]*types.Header), - coolMapThatIsNotAMemLeak: make(map[common.Hash][]*types.Transaction), + eth: eth, + remoteBlocks: newHeaderQueue(), + localBlocks: newPayloadQueue(), + invalidBlocksHits: make(map[common.Hash]int), + invalidTipsets: make(map[common.Hash]*types.Header), + coolMapThatIsNotAMemLeak: make(map[common.Hash][]*types.Transaction), + coolMapThatIsNotAMemLeak2: make(map[common.Hash]*types.InclusionListSummary), } eth.Downloader().SetBadBlockCallback(api.setInvalidAncestor) return api @@ -346,6 +348,7 @@ func (api *ConsensusAPI) forkchoiceUpdated(update engine.ForkchoiceStateV1, payl api.eth.BlockChain().SetFinalized(finalBlock.Header()) // Clear the inclusionList for that block delete(api.coolMapThatIsNotAMemLeak, update.FinalizedBlockHash) + delete(api.coolMapThatIsNotAMemLeak2, update.FinalizedBlockHash) } // Check if the safe block hash is in our canonical tree, if not something is wrong if update.SafeBlockHash != (common.Hash{}) { @@ -366,14 +369,15 @@ func (api *ConsensusAPI) forkchoiceUpdated(update engine.ForkchoiceStateV1, payl // will replace it arbitrarily many times in between. if payloadAttributes != nil { args := &miner.BuildPayloadArgs{ - Parent: update.HeadBlockHash, - Timestamp: payloadAttributes.Timestamp, - FeeRecipient: payloadAttributes.SuggestedFeeRecipient, - Random: payloadAttributes.Random, - Withdrawals: payloadAttributes.Withdrawals, - BeaconRoot: payloadAttributes.BeaconRoot, - Version: payloadVersion, - InclusionList: api.coolMapThatIsNotAMemLeak[update.HeadBlockHash], + Parent: update.HeadBlockHash, + Timestamp: payloadAttributes.Timestamp, + FeeRecipient: payloadAttributes.SuggestedFeeRecipient, + Random: payloadAttributes.Random, + Withdrawals: payloadAttributes.Withdrawals, + BeaconRoot: payloadAttributes.BeaconRoot, + Version: payloadVersion, + InclusionList: api.coolMapThatIsNotAMemLeak[update.HeadBlockHash], + InclusionListSummary: api.coolMapThatIsNotAMemLeak2[update.HeadBlockHash], } id := args.Id() // If we already are busy generating this work, then we do not need @@ -904,8 +908,8 @@ func getBody(block *types.Block) *engine.ExecutionPayloadBodyV1 { } } -func (api *ConsensusAPI) NewInclusionListV1(addresses []common.Address, transactions [][]byte, parentBlockHash common.Hash) (*engine.InclusionListStatusV1, error) { - if len(addresses) != len(transactions) { +func (api *ConsensusAPI) NewInclusionListV1(inclusionListSummary *types.InclusionListSummary, transactions [][]byte, parentBlockHash common.Hash) (*engine.InclusionListStatusV1, error) { + if len(inclusionListSummary.Summary) != len(transactions) { return inclusionListError("number of transactions do not match addresses"), nil } txs, err := engine.DecodeTransactions(transactions) @@ -919,8 +923,8 @@ func (api *ConsensusAPI) NewInclusionListV1(addresses []common.Address, transact if err != nil { return inclusionListError(fmt.Sprintf("Invalid signer at index %v, error: %v", index, err)), nil } - if addresses[index] != sender { - return inclusionListError(fmt.Sprintf("Invalid sender at index %v, got %v want %v", index, sender, addresses[index])), nil + if inclusionListSummary.Summary[index].Address != sender { + return inclusionListError(fmt.Sprintf("Invalid sender at index %v, got %v want %v", index, sender, inclusionListSummary.Summary[index].Address)), nil } maxGas += tx.Gas() } @@ -935,6 +939,7 @@ func (api *ConsensusAPI) NewInclusionListV1(addresses []common.Address, transact return inclusionListError(err.Error()), nil } api.coolMapThatIsNotAMemLeak[parentBlockHash] = txs + api.coolMapThatIsNotAMemLeak2[parentBlockHash] = inclusionListSummary return &engine.InclusionListStatusV1{Status: engine.VALID}, nil } diff --git a/eth/catalyst/simulated_beacon_test.go b/eth/catalyst/simulated_beacon_test.go index df682b49d9..34f0369651 100644 --- a/eth/catalyst/simulated_beacon_test.go +++ b/eth/catalyst/simulated_beacon_test.go @@ -136,7 +136,7 @@ func TestSimulatedBeaconSendWithdrawals(t *testing.T) { return } case <-timer.C: - t.Fatal("timed out without including all withdrawals/txs") + t.Fatalf("timed out without including all withdrawals/txs, want %v got %v", len(withdrawals), len(includedWithdrawals)) } } } diff --git a/eth/protocols/eth/handler_test.go b/eth/protocols/eth/handler_test.go index fdf551ef21..9a645f9b53 100644 --- a/eth/protocols/eth/handler_test.go +++ b/eth/protocols/eth/handler_test.go @@ -400,7 +400,7 @@ func testGetBlockBodies(t *testing.T, protocol uint) { block := backend.chain.GetBlockByNumber(uint64(num)) hashes = append(hashes, block.Hash()) if len(bodies) < tt.expected { - bodies = append(bodies, &BlockBody{Transactions: block.Transactions(), Uncles: block.Uncles(), Withdrawals: block.Withdrawals()}) + bodies = append(bodies, &BlockBody{Transactions: block.Transactions(), Uncles: block.Uncles(), Withdrawals: block.Withdrawals(), InclusionListSummary: block.InclusionListSummary()}) } break } @@ -410,7 +410,7 @@ func testGetBlockBodies(t *testing.T, protocol uint) { hashes = append(hashes, hash) if tt.available[j] && len(bodies) < tt.expected { block := backend.chain.GetBlockByHash(hash) - bodies = append(bodies, &BlockBody{Transactions: block.Transactions(), Uncles: block.Uncles(), Withdrawals: block.Withdrawals()}) + bodies = append(bodies, &BlockBody{Transactions: block.Transactions(), Uncles: block.Uncles(), Withdrawals: block.Withdrawals(), InclusionListSummary: block.InclusionListSummary()}) } } diff --git a/miner/payload_building.go b/miner/payload_building.go index b42f3dff59..5961a9b252 100644 --- a/miner/payload_building.go +++ b/miner/payload_building.go @@ -35,14 +35,15 @@ import ( // Check engine-api specification for more details. // https://github.com/ethereum/execution-apis/blob/main/src/engine/cancun.md#payloadattributesv3 type BuildPayloadArgs struct { - Parent common.Hash // The parent block to build payload on top - Timestamp uint64 // The provided timestamp of generated payload - FeeRecipient common.Address // The provided recipient address for collecting transaction fee - Random common.Hash // The provided randomness value - Withdrawals types.Withdrawals // The provided withdrawals - BeaconRoot *common.Hash // The provided beaconRoot (Cancun) - Version engine.PayloadVersion // Versioning byte for payload id calculation. - InclusionList []*types.Transaction // Mandatory Inclusion List + Parent common.Hash // The parent block to build payload on top + Timestamp uint64 // The provided timestamp of generated payload + FeeRecipient common.Address // The provided recipient address for collecting transaction fee + Random common.Hash // The provided randomness value + Withdrawals types.Withdrawals // The provided withdrawals + BeaconRoot *common.Hash // The provided beaconRoot (Cancun) + Version engine.PayloadVersion // Versioning byte for payload id calculation. + InclusionList []*types.Transaction // Mandatory Inclusion List + InclusionListSummary *types.InclusionListSummary // Inclusion List Summary } // Id computes an 8-byte identifier by hashing the components of the payload arguments. @@ -182,15 +183,16 @@ func (miner *Miner) buildPayload(args *BuildPayloadArgs) (*Payload, error) { // enough to run. The empty payload can at least make sure there is something // to deliver for not missing slot. emptyParams := &generateParams{ - timestamp: args.Timestamp, - forceTime: true, - parentHash: args.Parent, - coinbase: args.FeeRecipient, - random: args.Random, - withdrawals: args.Withdrawals, - beaconRoot: args.BeaconRoot, - noTxs: true, - inclusionList: args.InclusionList, + timestamp: args.Timestamp, + forceTime: true, + parentHash: args.Parent, + coinbase: args.FeeRecipient, + random: args.Random, + withdrawals: args.Withdrawals, + beaconRoot: args.BeaconRoot, + noTxs: true, + inclusionList: args.InclusionList, + inclusionListSummary: args.InclusionListSummary, } empty := miner.generateWork(emptyParams) if empty.err != nil { diff --git a/miner/worker.go b/miner/worker.go index 31b563917f..91a8e490e5 100644 --- a/miner/worker.go +++ b/miner/worker.go @@ -77,15 +77,16 @@ type newPayloadResult struct { // generateParams wraps various of settings for generating sealing task. type generateParams struct { - timestamp uint64 // The timestamp for sealing task - forceTime bool // Flag whether the given timestamp is immutable or not - parentHash common.Hash // Parent block hash, empty means the latest chain head - coinbase common.Address // The fee recipient address for including transaction - random common.Hash // The randomness generated by beacon chain, empty before the merge - withdrawals types.Withdrawals // List of withdrawals to include in block (shanghai field) - beaconRoot *common.Hash // The beacon root (cancun field). - noTxs bool // Flag whether an empty block without any transaction is expected - inclusionList []*types.Transaction // Mandatory transactions to include + timestamp uint64 // The timestamp for sealing task + forceTime bool // Flag whether the given timestamp is immutable or not + parentHash common.Hash // Parent block hash, empty means the latest chain head + coinbase common.Address // The fee recipient address for including transaction + random common.Hash // The randomness generated by beacon chain, empty before the merge + withdrawals types.Withdrawals // List of withdrawals to include in block (shanghai field) + beaconRoot *common.Hash // The beacon root (cancun field). + noTxs bool // Flag whether an empty block without any transaction is expected + inclusionListSummary *types.InclusionListSummary // Inclusion List summary + inclusionList []*types.Transaction // Mandatory transactions to include } // generateWork generates a sealing block based on the given parameters. @@ -119,7 +120,11 @@ func (miner *Miner) generateWork(params *generateParams) *newPayloadResult { log.Warn("Block building is interrupted", "allowance", common.PrettyDuration(miner.config.Recommit)) } } - body := types.Body{Transactions: work.txs, Withdrawals: params.withdrawals} + var summary []*types.InclusionListEntry + if params.inclusionListSummary != nil { + summary = params.inclusionListSummary.Summary + } + body := types.Body{Transactions: work.txs, Withdrawals: params.withdrawals, InclusionListSummary: summary} block, err := miner.engine.FinalizeAndAssemble(miner.chain, work.header, work.state, &body, work.receipts) if err != nil { return &newPayloadResult{err: err} diff --git a/rpc/subscription_test.go b/rpc/subscription_test.go index 3a131c8e6b..a5b7fc320b 100644 --- a/rpc/subscription_test.go +++ b/rpc/subscription_test.go @@ -273,7 +273,7 @@ func TestNotify(t *testing.T) { } notifier.Notify(id, msg) have := strings.TrimSpace(out.String()) - want := `{"jsonrpc":"2.0","method":"_subscription","params":{"subscription":"test","result":{"parentHash":"0x0000000000000000000000000000000000000000000000000000000000000001","sha3Uncles":"0x0000000000000000000000000000000000000000000000000000000000000000","miner":"0x0000000000000000000000000000000000000000","stateRoot":"0x0000000000000000000000000000000000000000000000000000000000000000","transactionsRoot":"0x0000000000000000000000000000000000000000000000000000000000000000","receiptsRoot":"0x0000000000000000000000000000000000000000000000000000000000000000","logsBloom":"0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000","difficulty":null,"number":"0x64","gasLimit":"0x0","gasUsed":"0x0","timestamp":"0x0","extraData":"0x","mixHash":"0x0000000000000000000000000000000000000000000000000000000000000000","nonce":"0x0000000000000000","baseFeePerGas":null,"withdrawalsRoot":null,"blobGasUsed":null,"excessBlobGas":null,"parentBeaconBlockRoot":null,"hash":"0xe5fb877dde471b45b9742bb4bb4b3d74a761e2fb7cb849a3d2b687eed90fb604"}}}` + want := `{"jsonrpc":"2.0","method":"_subscription","params":{"subscription":"test","result":{"parentHash":"0x0000000000000000000000000000000000000000000000000000000000000001","sha3Uncles":"0x0000000000000000000000000000000000000000000000000000000000000000","miner":"0x0000000000000000000000000000000000000000","stateRoot":"0x0000000000000000000000000000000000000000000000000000000000000000","transactionsRoot":"0x0000000000000000000000000000000000000000000000000000000000000000","receiptsRoot":"0x0000000000000000000000000000000000000000000000000000000000000000","logsBloom":"0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000","difficulty":null,"number":"0x64","gasLimit":"0x0","gasUsed":"0x0","timestamp":"0x0","extraData":"0x","mixHash":"0x0000000000000000000000000000000000000000000000000000000000000000","nonce":"0x0000000000000000","baseFeePerGas":null,"withdrawalsRoot":null,"blobGasUsed":null,"excessBlobGas":null,"parentBeaconBlockRoot":null,"inclusionListSummaryRoot":null,"hash":"0xe5fb877dde471b45b9742bb4bb4b3d74a761e2fb7cb849a3d2b687eed90fb604"}}}` if have != want { t.Errorf("have:\n%v\nwant:\n%v\n", have, want) }