diff --git a/db/rawdb/accessors_chain.go b/db/rawdb/accessors_chain.go index 6b67a46cd0f..5c1c76fee6d 100644 --- a/db/rawdb/accessors_chain.go +++ b/db/rawdb/accessors_chain.go @@ -812,7 +812,7 @@ func ReadBlock(tx kv.Getter, hash common.Hash, number uint64) *types.Block { block := types.NewBlockFromStorage(hash, header, body.Transactions, body.Uncles, body.Withdrawals) // Carry the BAL sidecar (secondary storage) so a block reconstructed from the // DB carries its BAL like its header/body. Only Amsterdam+ blocks have one. - if header.BlockAccessListHash != nil { + if header.HasBAL() { if bal, err := ReadBlockAccessListBytes(tx, hash, number); err == nil && len(bal) > 0 { block.SetBlockAccessList(bal) } diff --git a/db/rawdb/accessors_chain_test.go b/db/rawdb/accessors_chain_test.go index 99e333d44a5..b7e4bd27518 100644 --- a/db/rawdb/accessors_chain_test.go +++ b/db/rawdb/accessors_chain_test.go @@ -1154,6 +1154,40 @@ func TestBlockWithdrawalsStorage(t *testing.T) { require.Nil(entry) } +func TestReadBlockLoadsEmptyBlockAccessList(t *testing.T) { + t.Parallel() + _, tx := memdb.NewTestTx(t) + defer tx.Rollback() + + emptyBALHash := empty.BlockAccessListHash + withdrawalsHash := empty.RootHash + blobGas := uint64(0) + parentBeaconBlockRoot := common.Hash{} + requestsHash := common.Hash{} + block := types.NewBlockWithHeader(&types.Header{ + Number: *uint256.NewInt(1), + Extra: []byte("test block"), + UncleHash: empty.UncleHash, + TxHash: empty.RootHash, + ReceiptHash: empty.RootHash, + BaseFee: uint256.NewInt(1), + WithdrawalsHash: &withdrawalsHash, + BlobGasUsed: &blobGas, + ExcessBlobGas: &blobGas, + ParentBeaconBlockRoot: &parentBeaconBlockRoot, + RequestsHash: &requestsHash, + BlockAccessListHash: &emptyBALHash, + }) + require.NoError(t, rawdb.WriteBlock(tx, block)) + emptyBALBytes, err := types.EncodeBlockAccessListBytes(nil) + require.NoError(t, err) + require.NoError(t, rawdb.WriteBlockAccessListBytes(tx, block.Hash(), block.NumberU64(), emptyBALBytes)) + + readBlock := rawdb.ReadBlock(tx, block.Hash(), block.NumberU64()) + require.NotNil(t, readBlock) + require.Equal(t, emptyBALBytes, readBlock.BlockAccessList()) +} + func TestBlockAccessListStorage(t *testing.T) { t.Parallel() _, tx := memdb.NewTestTx(t) diff --git a/execution/execmodule/exec_module_test.go b/execution/execmodule/exec_module_test.go index 1f40c01f656..d1ce2ad3d9b 100644 --- a/execution/execmodule/exec_module_test.go +++ b/execution/execmodule/exec_module_test.go @@ -111,6 +111,35 @@ func (p *rewindingTxnProvider) ProvideTxns(ctx context.Context, opts ...txnprovi return p.pool.ProvideTxns(ctx, opts...) } +type observingTxnProvider struct { + txnprovider.TxnProvider + txnCounts chan int +} + +func (p *observingTxnProvider) ProvideTxns(ctx context.Context, opts ...txnprovider.ProvideOption) ([]types.Transaction, error) { + txns, err := p.TxnProvider.ProvideTxns(ctx, opts...) + if err == nil { + p.txnCounts <- len(txns) + } + return txns, err +} + +func waitForProvidedTxnCount(t *testing.T, txnCounts <-chan int, want int) { + t.Helper() + timer := time.NewTimer(10 * time.Second) + defer timer.Stop() + for { + select { + case got := <-txnCounts: + if got == want { + return + } + case <-timer.C: + t.Fatalf("transaction provider did not return %d transactions", want) + } + } +} + func txPoolHead(block *types.Block) *remoteproto.StateChangeBatch { return &remoteproto.StateChangeBatch{ PendingBlockBaseFee: block.BaseFee().Uint64(), @@ -792,6 +821,10 @@ func TestAssembleBlockWithFreshlyAddedTxns(t *testing.T) { require.NoError(t, err) baseFee := chainPack.TopBlock.BaseFee().Uint64() addTwoTxnsToPool(ctx, 1, t, m, txpool, baseFee) + provider := &observingTxnProvider{ + TxnProvider: m.TxPool, + txnCounts: make(chan int, 16), + } var parentBeaconBlockRoot common.Hash _, err = rand.Read(parentBeaconBlockRoot[:]) @@ -803,14 +836,16 @@ func TestAssembleBlockWithFreshlyAddedTxns(t *testing.T) { SuggestedFeeRecipient: common.Address{1}, Withdrawals: make([]*types.Withdrawal, 0), ParentBeaconBlockRoot: &parentBeaconBlockRoot, + CustomTxnProvider: provider, }) require.NoError(t, err) - // Add new transactions with a delay - time.Sleep(300 * time.Millisecond) + waitForProvidedTxnCount(t, provider.txnCounts, 2) + waitForProvidedTxnCount(t, provider.txnCounts, 0) addTwoTxnsToPool(ctx, 3, t, m, txpool, baseFee) + waitForProvidedTxnCount(t, provider.txnCounts, 2) + waitForProvidedTxnCount(t, provider.txnCounts, 0) - // The block should have all four transactions block, err := getAssembledBlock(ctx, exec, payloadId) require.NoError(t, err) require.Equal(t, uint64(2), block.NumberU64()) diff --git a/execution/p2p/bbd.go b/execution/p2p/bbd.go index 150d4d69e68..2a34416758b 100644 --- a/execution/p2p/bbd.go +++ b/execution/p2p/bbd.go @@ -668,7 +668,7 @@ func (bbd *BackwardBlockDownloader) downloadBlocksForHeaders( func balRequestsForHeaders(headers []*types.Header) []BALRequest { reqs := make([]BALRequest, 0, len(headers)) for _, header := range headers { - if !header.HasBAL() { + if !header.HasNonEmptyBAL() { continue } reqs = append(reqs, BALRequest{ diff --git a/execution/stagedsync/exec3.go b/execution/stagedsync/exec3.go index 88b1ccb1cad..51ccf819c5d 100644 --- a/execution/stagedsync/exec3.go +++ b/execution/stagedsync/exec3.go @@ -528,6 +528,14 @@ func (te *txExecutor) onBlockStart(ctx context.Context, blockNum uint64, blockHa } } +func blockAccessListBytes(blockTx kv.Getter, block *types.Block, blockNum uint64) ([]byte, error) { + data := block.BlockAccessList() + if len(data) == 0 && block.HeaderNoCopy().HasNonEmptyBAL() { + return rawdb.ReadBlockAccessListBytes(blockTx, block.Hash(), blockNum) + } + return data, nil +} + func (te *txExecutor) executeBlocks(ctx context.Context, startBlockNum uint64, maxBlockNum uint64, blockLimit uint64, initialTxNum uint64, inputTxNum uint64, readAhead chan uint64, initialCycle bool, applyResults chan applyResult, blockRequests chan *blockRequest, commitResults chan applyResult) error { if te.execLoopGroup == nil { return errors.New("no exec group") @@ -619,12 +627,9 @@ func (te *txExecutor) executeBlocks(ctx context.Context, startBlockNum uint64, m // db.View() as it can deadlock with the stageloop's RW transaction when // BlockOverlay is active. ProcessBAL still computes+validates the BAL // from the write-set as the ultimate fallback. - data := b.BlockAccessList() - if len(data) == 0 { - data, err = rawdb.ReadBlockAccessListBytes(blockTx, b.Hash(), blockNum) - if err != nil { - return err - } + data, err := blockAccessListBytes(blockTx, b, blockNum) + if err != nil { + return err } if len(data) > 0 && !dbg.IgnoreBAL { dbBAL, err = types.DecodeBlockAccessListBytes(data) diff --git a/execution/stagedsync/exec3_bal_test.go b/execution/stagedsync/exec3_bal_test.go new file mode 100644 index 00000000000..bab04ca8469 --- /dev/null +++ b/execution/stagedsync/exec3_bal_test.go @@ -0,0 +1,56 @@ +package stagedsync + +import ( + "bytes" + "testing" + + "github.com/erigontech/erigon/common" + "github.com/erigontech/erigon/common/empty" + "github.com/erigontech/erigon/db/kv" + "github.com/erigontech/erigon/execution/types" +) + +type countingBlockAccessListGetter struct { + kv.Getter + data []byte + calls int +} + +func (g *countingBlockAccessListGetter) GetOne(string, []byte) ([]byte, error) { + g.calls++ + return g.data, nil +} + +func TestBlockAccessListBytes(t *testing.T) { + nonEmptyBALHash := common.Hash{1} + storedBAL := []byte{1, 2, 3} + tests := []struct { + name string + hash *common.Hash + storedBAL []byte + wantBAL []byte + wantReads int + }{ + {name: "missing commitment"}, + {name: "empty commitment", hash: &empty.BlockAccessListHash}, + {name: "non-empty commitment", hash: &nonEmptyBALHash, storedBAL: storedBAL, wantBAL: storedBAL, wantReads: 1}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + getter := &countingBlockAccessListGetter{data: test.storedBAL} + block := types.NewBlockFromStorage(common.Hash{}, &types.Header{BlockAccessListHash: test.hash}, nil, nil, nil) + + got, err := blockAccessListBytes(getter, block, 1) + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(got, test.wantBAL) { + t.Fatalf("block access list = %x, want %x", got, test.wantBAL) + } + if getter.calls != test.wantReads { + t.Fatalf("DB reads = %d, want %d", getter.calls, test.wantReads) + } + }) + } +} diff --git a/execution/types/block.go b/execution/types/block.go index e0b5d631d2d..1f4301303c2 100644 --- a/execution/types/block.go +++ b/execution/types/block.go @@ -606,9 +606,14 @@ func (h *Header) Size() common.StorageSize { return s } -// HasBAL reports whether the header commits to a non-empty EIP-7928 block access list. +// HasBAL reports whether the header contains an EIP-7928 block access list commitment. func (h *Header) HasBAL() bool { - return h.BlockAccessListHash != nil && *h.BlockAccessListHash != empty.BlockAccessListHash + return h.BlockAccessListHash != nil +} + +// HasNonEmptyBAL reports whether the commitment is for a non-empty block access list. +func (h *Header) HasNonEmptyBAL() bool { + return h.HasBAL() && *h.BlockAccessListHash != empty.BlockAccessListHash } // SanityCheck checks a few basic things -- these checks are way beyond what diff --git a/execution/types/block_access_list_test.go b/execution/types/block_access_list_test.go index 1808ee6c3f0..a9fa01f05c2 100644 --- a/execution/types/block_access_list_test.go +++ b/execution/types/block_access_list_test.go @@ -10,6 +10,7 @@ import ( "github.com/holiman/uint256" "github.com/erigontech/erigon/common" + "github.com/erigontech/erigon/common/empty" "github.com/erigontech/erigon/execution/rlp" "github.com/erigontech/erigon/execution/types/accounts" ) @@ -182,6 +183,50 @@ func TestBlockAccessListHashEmpty(t *testing.T) { } } +func TestHeaderHasBAL(t *testing.T) { + nonEmptyBALHash := common.Hash{1} + tests := []struct { + name string + hash *common.Hash + want bool + }{ + {name: "missing", want: false}, + {name: "empty", hash: &empty.BlockAccessListHash, want: true}, + {name: "non-empty", hash: &nonEmptyBALHash, want: true}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + header := Header{BlockAccessListHash: test.hash} + if got := header.HasBAL(); got != test.want { + t.Fatalf("HasBAL() = %t, want %t", got, test.want) + } + }) + } +} + +func TestHeaderHasNonEmptyBAL(t *testing.T) { + nonEmptyBALHash := common.Hash{1} + tests := []struct { + name string + hash *common.Hash + want bool + }{ + {name: "missing", want: false}, + {name: "empty", hash: &empty.BlockAccessListHash, want: false}, + {name: "non-empty", hash: &nonEmptyBALHash, want: true}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + header := Header{BlockAccessListHash: test.hash} + if got := header.HasNonEmptyBAL(); got != test.want { + t.Fatalf("HasNonEmptyBAL() = %t, want %t", got, test.want) + } + }) + } +} + // TestBlockAccessListEmptyRoundTrip verifies that an empty BAL encodes to the // canonical empty RLP list (0xc0) and decodes back to a non-nil empty slice. // EIP-7928 requires: "When no state changes are present, this field is the