diff --git a/cmd/keeper/main.go b/cmd/keeper/main.go index df6881acbf..14316e6659 100644 --- a/cmd/keeper/main.go +++ b/cmd/keeper/main.go @@ -53,17 +53,8 @@ func main() { } vmConfig := vm.Config{} - crossStateRoot, crossReceiptRoot, err := core.ExecuteStateless(context.Background(), chainConfig, vmConfig, payload.Block, payload.Witness) - if err != nil { + if err := core.ExecuteStateless(context.Background(), chainConfig, vmConfig, payload.Block, payload.Witness); err != nil { fmt.Fprintf(os.Stderr, "stateless self-validation failed: %v\n", err) os.Exit(10) } - if crossStateRoot != payload.Block.Root() { - fmt.Fprintf(os.Stderr, "stateless self-validation root mismatch (cross: %x local: %x)\n", crossStateRoot, payload.Block.Root()) - os.Exit(11) - } - if crossReceiptRoot != payload.Block.ReceiptHash() { - fmt.Fprintf(os.Stderr, "stateless self-validation receipt root mismatch (cross: %x local: %x)\n", crossReceiptRoot, payload.Block.ReceiptHash()) - os.Exit(12) - } } diff --git a/core/block_validator.go b/core/block_validator.go index 63047d0941..689b6ccb09 100644 --- a/core/block_validator.go +++ b/core/block_validator.go @@ -19,7 +19,7 @@ package core import ( "errors" "fmt" - "github.com/ethereum/go-ethereum/consensus" + "github.com/ethereum/go-ethereum/core/state" "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/params" @@ -32,16 +32,11 @@ import ( // BlockValidator implements Validator. type BlockValidator struct { config *params.ChainConfig // Chain configuration options - bc *BlockChain // Canonical block chain } // NewBlockValidator returns a new block validator which is safe for re-use -func NewBlockValidator(config *params.ChainConfig, blockchain *BlockChain) *BlockValidator { - validator := &BlockValidator{ - config: config, - bc: blockchain, - } - return validator +func NewBlockValidator(config *params.ChainConfig) *BlockValidator { + return &BlockValidator{config: config} } // ValidateBody validates the given block's uncles and verifies the block @@ -52,17 +47,9 @@ func (v *BlockValidator) ValidateBody(block *types.Block) error { if v.config.IsOsaka(block.Number(), block.Time()) && block.ConsensusSize() > params.MaxBlockSize { return ErrBlockOversized } - // Check whether the block is already imported. - if v.bc.HasBlockAndState(block.Hash(), block.NumberU64()) { - return ErrKnownBlock - } - // Header validity is known at this point. Here we verify that uncles, transactions // and withdrawals given in the block body match the header. header := block.Header() - if err := v.bc.engine.VerifyUncles(v.bc, block); err != nil { - return err - } if hash := types.CalcUncleHash(block.Uncles()); hash != header.UncleHash { return fmt.Errorf("uncle root hash mismatch (header value %x, calculated %x)", header.UncleHash, hash) } @@ -133,20 +120,12 @@ func (v *BlockValidator) ValidateBody(block *types.Block) error { return fmt.Errorf("access list hash in block header not allowed when experimental.bal is set") } } - - // Ancestor block must be known. - if !v.bc.HasBlockAndState(block.ParentHash(), block.NumberU64()-1) { - if !v.bc.HasBlock(block.ParentHash(), block.NumberU64()-1) { - return consensus.ErrUnknownAncestor - } - return consensus.ErrPrunedAncestor - } return nil } // ValidateState validates the various changes that happen after a state transition, // such as amount of used gas, the receipt roots and the state root itself. -func (v *BlockValidator) ValidateState(block *types.Block, stateTransition state.BlockStateTransition, res *ProcessResult, stateless bool) error { +func (v *BlockValidator) ValidateState(block *types.Block, stateTransition state.BlockStateTransition, res *ProcessResult) error { if res == nil { return errors.New("nil ProcessResult value") } @@ -164,10 +143,16 @@ func (v *BlockValidator) ValidateState(block *types.Block, stateTransition state if rbloom != header.Bloom { return fmt.Errorf("invalid bloom (remote: %x local: %x)", header.Bloom, rbloom) } - // In stateless mode, return early because the receipt and state root are not - // provided through the witness, rather the cross validator needs to return it. - if stateless { - return nil + // Validate the computed BAL hash matches the header and doesn't exceed gas limit. + if v.config.IsAmsterdam(header.Number, header.Time) && header.BlockAccessListHash != nil { + computedBAL := res.AccessList.ToEncodingObj() + computedHash := computedBAL.Hash() + if *header.BlockAccessListHash != computedHash { + return fmt.Errorf("block access list hash mismatch (header %x, computed %x)", *header.BlockAccessListHash, computedHash) + } + if err := computedBAL.ValidateGasLimit(header.GasLimit); err != nil { + return err + } } // The receipt Trie's root (R = (Tr [[H1, R1], ... [Hn, Rn]])) receiptSha := types.DeriveSha(res.Receipts, trie.NewStackTrie(nil)) diff --git a/core/blockchain.go b/core/blockchain.go index 6345e05609..dc4c02a8ec 100644 --- a/core/blockchain.go +++ b/core/blockchain.go @@ -451,7 +451,7 @@ func NewBlockChain(db ethdb.Database, genesis *Genesis, engine consensus.Engine, } bc.flushInterval.Store(int64(cfg.TrieTimeLimit)) bc.statedb = state.NewDatabase(bc.triedb, nil) - bc.validator = NewBlockValidator(chainConfig, bc) + bc.validator = NewBlockValidator(chainConfig) bc.prefetcher = newStatePrefetcher(chainConfig, bc.hc) bc.processor = NewStateProcessor(bc.hc) bc.parallelProcessor = NewParallelStateProcessor(bc.hc, &cfg.VmConfig) @@ -641,7 +641,7 @@ func (bc *BlockChain) processBlockWithAccessList(parentRoot common.Hash, block * return nil, err } - if err := bc.validator.ValidateState(block, stateTransition, res.ProcessResult, false); err != nil { + if err := bc.validator.ValidateState(block, stateTransition, res.ProcessResult); err != nil { return nil, err } @@ -2012,7 +2012,7 @@ func (bc *BlockChain) insertChain(ctx context.Context, chain types.Blocks, setHe defer close(abort) // Peek the error for the first block to decide the directing import logic - it := newInsertIterator(chain, results, bc.validator) + it := newInsertIterator(chain, results, bc.validator, bc) block, err := it.next() // Left-trim all the known blocks that don't need to build snapshot @@ -2400,7 +2400,7 @@ func (bc *BlockChain) ProcessBlock(ctx context.Context, parentRoot common.Hash, vstart := time.Now() _, _, spanEnd = telemetry.StartSpan(ctx, "bc.validator.ValidateState") - err = bc.validator.ValidateState(block, statedb, res, false) + err = bc.validator.ValidateState(block, statedb, res) spanEnd(&err) if err != nil { bc.reportBadBlock(block, res, err) @@ -2408,30 +2408,11 @@ func (bc *BlockChain) ProcessBlock(ctx context.Context, parentRoot common.Hash, } vtime = time.Since(vstart) - if isAmsterdam { + if isAmsterdam && block.AccessList() == nil { + // Attach the computed access list to the block so it gets persisted + // when the block is written to disk. computedAccessList := res.AccessList.ToEncodingObj() - computedAccessListHash := computedAccessList.Hash() - - if *block.Header().BlockAccessListHash != computedAccessListHash { - //fmt.Printf("remote:\n%s\nlocal:\n%s\n", block.Body().AccessList.JSONString(), computedAccessList.JSONString()) - err := fmt.Errorf("block header access list hash mismatch with computed (header=%x computed=%x)", *block.Header().BlockAccessListHash, computedAccessListHash) - bc.reportBadBlock(block, res, err) - return nil, err - } - // EIP-7928: Validate BAL items do not exceed block gas limit - if err := computedAccessList.ValidateGasLimit(block.Header().GasLimit); err != nil { - bc.reportBadBlock(block, res, err) - return nil, err - } - if block.AccessList() == nil { - // attach the computed access list to the block so it gets persisted - // when the block is written to disk - block = block.WithAccessList(computedAccessList) - } else if block.AccessList().Hash() != computedAccessListHash { - err := fmt.Errorf("block access list hash mismatch (remote=%x computed=%x)", block.AccessList().Hash(), computedAccessListHash) - bc.reportBadBlock(block, res, err) - return nil, err - } + block = block.WithAccessList(computedAccessList) } // If witnesses was generated and stateless self-validation requested, do @@ -2443,24 +2424,10 @@ func (bc *BlockChain) ProcessBlock(ctx context.Context, parentRoot common.Hash, if witness := statedb.Witness(); witness != nil && config.StatelessSelfValidation { log.Warn("Running stateless self-validation", "block", block.Number(), "hash", block.Hash()) - // Remove critical computed fields from the block to force true recalculation - context := block.Header() - context.Root = common.Hash{} - context.ReceiptHash = common.Hash{} - - task := types.NewBlockWithHeader(context).WithBody(*block.Body()) - // Run the stateless self-cross-validation - crossStateRoot, crossReceiptRoot, err := ExecuteStateless(ctx, bc.chainConfig, bc.cfg.VmConfig, task, witness) - if err != nil { + if err := ExecuteStateless(ctx, bc.chainConfig, bc.cfg.VmConfig, block, witness); err != nil { return nil, fmt.Errorf("stateless self-validation failed: %v", err) } - if crossStateRoot != block.Root() { - return nil, fmt.Errorf("stateless self-validation root mismatch (cross: %x local: %x)", crossStateRoot, block.Root()) - } - if crossReceiptRoot != block.ReceiptHash() { - return nil, fmt.Errorf("stateless self-validation receipt root mismatch (cross: %x local: %x)", crossReceiptRoot, block.ReceiptHash()) - } } var ( diff --git a/core/blockchain_insert.go b/core/blockchain_insert.go index 07a250a1bb..6d2d3bf261 100644 --- a/core/blockchain_insert.go +++ b/core/blockchain_insert.go @@ -21,6 +21,7 @@ import ( "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/mclock" + "github.com/ethereum/go-ethereum/consensus" "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/log" ) @@ -95,19 +96,21 @@ type insertIterator struct { results <-chan error // Verification result sink from the consensus engine errors []error // Header verification errors for the blocks - index int // Current offset of the iterator - validator Validator // Validator to run if verification succeeds + index int // Current offset of the iterator + validator Validator // Validator to run if verification succeeds + bc *BlockChain // Blockchain for admission checks } // newInsertIterator creates a new iterator based on the given blocks, which are // assumed to be a contiguous chain. -func newInsertIterator(chain types.Blocks, results <-chan error, validator Validator) *insertIterator { +func newInsertIterator(chain types.Blocks, results <-chan error, validator Validator, bc *BlockChain) *insertIterator { return &insertIterator{ chain: chain, results: results, errors: make([]error, 0, len(chain)), index: -1, validator: validator, + bc: bc, } } @@ -127,8 +130,21 @@ func (it *insertIterator) next() (*types.Block, error) { if it.errors[it.index] != nil { return it.chain[it.index], it.errors[it.index] } - // Block header valid, run body validation and return - return it.chain[it.index], it.validator.ValidateBody(it.chain[it.index]) + // Block header valid, run admission checks and body validation + block := it.chain[it.index] + if it.bc.HasBlockAndState(block.Hash(), block.NumberU64()) { + return block, ErrKnownBlock + } + if err := it.bc.engine.VerifyUncles(it.bc, block); err != nil { + return block, err + } + if !it.bc.HasBlockAndState(block.ParentHash(), block.NumberU64()-1) { + if !it.bc.HasBlock(block.ParentHash(), block.NumberU64()-1) { + return block, consensus.ErrUnknownAncestor + } + return block, consensus.ErrPrunedAncestor + } + return block, it.validator.ValidateBody(block) } // previous returns the previous header that was being processed, or nil. diff --git a/core/blockchain_test.go b/core/blockchain_test.go index 30a0685f6d..271f92d55a 100644 --- a/core/blockchain_test.go +++ b/core/blockchain_test.go @@ -166,7 +166,7 @@ func testBlockChainImport(chain types.Blocks, blockchain *BlockChain) error { blockchain.reportBadBlock(block, res, err) return err } - err = blockchain.validator.ValidateState(block, statedb, res, true) + err = blockchain.validator.ValidateState(block, statedb, res) if err != nil { blockchain.reportBadBlock(block, res, err) return err diff --git a/core/stateless.go b/core/stateless.go index 14cd4d4467..e35af4a103 100644 --- a/core/stateless.go +++ b/core/stateless.go @@ -27,60 +27,56 @@ import ( "github.com/ethereum/go-ethereum/core/stateless" "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/core/vm" - "github.com/ethereum/go-ethereum/log" "github.com/ethereum/go-ethereum/params" - "github.com/ethereum/go-ethereum/trie" "github.com/ethereum/go-ethereum/triedb" ) -// ExecuteStateless runs a stateless execution based on a witness, verifies -// everything it can locally and returns the state root and receipt root, that -// need the other side to explicitly check. +// ExecuteStateless runs a stateless execution based on a witness, fully +// validating the block including header, body, state root and receipt root. // // This method is a bit of a sore thumb here, but: // - It cannot be placed in core/stateless, because state.New prodces a circular dep // - It cannot be placed outside of core, because it needs to construct a dud headerchain // // TODO(karalabe): Would be nice to resolve both issues above somehow and move it. -func ExecuteStateless(ctx context.Context, config *params.ChainConfig, vmconfig vm.Config, block *types.Block, witness *stateless.Witness) (common.Hash, common.Hash, error) { - // Sanity check if the supplied block accidentally contains a set root or - // receipt hash. If so, be very loud, but still continue. - if block.Root() != (common.Hash{}) { - log.Error("stateless runner received state root it's expected to calculate (faulty consensus client)", "block", block.Number()) - } - if block.ReceiptHash() != (common.Hash{}) { - log.Error("stateless runner received receipt root it's expected to calculate (faulty consensus client)", "block", block.Number()) - } +func ExecuteStateless(ctx context.Context, config *params.ChainConfig, vmconfig vm.Config, block *types.Block, witness *stateless.Witness) error { // Create and populate the state database to serve as the stateless backend memdb := witness.MakeHashDB() db, err := state.New(witness.Root(), state.NewDatabase(triedb.NewDatabase(memdb, triedb.HashDefaults), nil)) if err != nil { - return common.Hash{}, common.Hash{}, err + return err } // Create a blockchain that is idle, but can be used to access headers through + engine := beacon.New(ethash.NewFaker()) chain := &HeaderChain{ config: config, chainDb: memdb, headerCache: lru.NewCache[common.Hash, *types.Header](256), - engine: beacon.New(ethash.NewFaker()), + engine: engine, + } + // Verify the block header against the parent header from the witness + if err := engine.VerifyHeader(chain, block.Header()); err != nil { + return err } processor := NewStateProcessor(chain) - validator := NewBlockValidator(config, nil) // No chain, we only validate the state, not the block + validator := NewBlockValidator(config) + + // Verify the block body (transactions, withdrawals, blob gas, BAL) against the header + if err := validator.ValidateBody(block); err != nil { + return err + } if config.IsAmsterdam(block.Number(), block.Time()) { db = db.WithReader(state.NewReaderWithTracker(db.Reader())) } - // Run the stateless blocks processing and self-validate certain fields + // Run the stateless blocks processing and self-validate all fields res, err := processor.Process(ctx, block, db, vmconfig) if err != nil { - return common.Hash{}, common.Hash{}, err + return err } - if err = validator.ValidateState(block, db, res, true); err != nil { - return common.Hash{}, common.Hash{}, err + if err = validator.ValidateState(block, db, res); err != nil { + return err } - // Almost everything validated, but receipt and state root needs to be returned - receiptRoot := types.DeriveSha(res.Receipts, trie.NewStackTrie(nil)) - stateRoot := db.IntermediateRoot(config.IsEIP158(block.Number())) - return stateRoot, receiptRoot, nil + return nil } diff --git a/core/types.go b/core/types.go index f881c9ed8f..818d40afa2 100644 --- a/core/types.go +++ b/core/types.go @@ -34,7 +34,7 @@ type Validator interface { ValidateBody(block *types.Block) error // ValidateState validates the given statedb and optionally the process result. - ValidateState(block *types.Block, state state.BlockStateTransition, res *ProcessResult, stateless bool) error + ValidateState(block *types.Block, state state.BlockStateTransition, res *ProcessResult) error } // Prefetcher is an interface for pre-caching transaction signatures and state. diff --git a/eth/catalyst/witness.go b/eth/catalyst/witness.go index 14ca29e079..f6a51bfd77 100644 --- a/eth/catalyst/witness.go +++ b/eth/catalyst/witness.go @@ -284,11 +284,10 @@ func (api *ConsensusAPI) executeStatelessPayload(params engine.ExecutableData, v api.lastNewPayloadUpdate.Store(time.Now().Unix()) log.Trace("Executing block statelessly", "number", block.Number(), "hash", params.BlockHash) - stateRoot, receiptRoot, err := core.ExecuteStateless(context.Background(), api.config(), vm.Config{}, block, witness) - if err != nil { + if err := core.ExecuteStateless(context.Background(), api.config(), vm.Config{}, block, witness); err != nil { log.Warn("ExecuteStatelessPayload: execution failed", "err", err) errorMsg := err.Error() return engine.StatelessPayloadStatusV1{Status: engine.INVALID, ValidationError: &errorMsg}, nil } - return engine.StatelessPayloadStatusV1{Status: engine.VALID, StateRoot: stateRoot, ReceiptsRoot: receiptRoot}, nil + return engine.StatelessPayloadStatusV1{Status: engine.VALID, StateRoot: block.Root(), ReceiptsRoot: block.ReceiptHash()}, nil } diff --git a/tests/block_test.go b/tests/block_test.go index 9e314e763e..eca36a74f6 100644 --- a/tests/block_test.go +++ b/tests/block_test.go @@ -879,22 +879,31 @@ func TestExecutionSpecZkevmBlocktests(t *testing.T) { bt.walk(t, executionSpecZkevmBlockchainTestDir, func(t *testing.T, name string, test *BlockTest) { execBlockTest(t, bt, test, true) - // Validate execution witnesses for blocks that have them - // TODO: Execution specs don't emit a witness when block is valid right now + // Validate execution witnesses for each block for _, b := range test.json.Blocks { - if b.ExecutionWitness == nil || b.BlockHeader == nil { + shouldFail := b.ExpectException != "" + + if b.ExecutionWitness == nil { + if !shouldFail { + t.Errorf("valid block missing execution witness") + } continue } block, err := b.decode() if err != nil { - t.Fatalf("failed to decode block for witness validation: %v", err) + if !shouldFail { + t.Errorf("failed to decode valid block: %v", err) + } + continue } - if err := test.validateExecutionWitness(block, b.ExecutionWitness); err != nil { + err = test.validateExecutionWitness(block, b.ExecutionWitness) + if shouldFail && err == nil { + t.Errorf("stateless execution succeeded for invalid block (exception: %s)", b.ExpectException) + } + if !shouldFail && err != nil { t.Errorf("execution witness validation failed: %v", err) } - // Validate that the SSZ-encoded statelessInputBytes witness matches the JSON executionWitness - // TODO: long term, we will only have statelessInputBytes and we will have no redundancy if b.StatelessInputBytes != nil { if err := validateStatelessInputWitness([]byte(*b.StatelessInputBytes), b.ExecutionWitness); err != nil { t.Errorf("stateless input witness mismatch: %v", err) diff --git a/tests/block_test_util.go b/tests/block_test_util.go index 388d1f7a5f..4fd3451d90 100644 --- a/tests/block_test_util.go +++ b/tests/block_test_util.go @@ -522,26 +522,7 @@ func (t *BlockTest) validateExecutionWitness(block *types.Block, ew *btExecution if err != nil { return fmt.Errorf("failed to parse execution witness: %v", err) } - // ExecuteStateless expects the block header to have zeroed root and receipt hash - // so it can compute them from the witness. Make a copy with those fields cleared. - header := types.CopyHeader(block.Header()) - expectedRoot := header.Root - expectedReceiptHash := header.ReceiptHash - header.Root = common.Hash{} - header.ReceiptHash = common.Hash{} - - witnessBlock := types.NewBlockWithHeader(header).WithBody(*block.Body()) - stateRoot, receiptRoot, err := core.ExecuteStateless(context.TODO(), config, vm.Config{}, witnessBlock, witness) - if err != nil { - return fmt.Errorf("stateless execution failed: %v", err) - } - if stateRoot != expectedRoot { - return fmt.Errorf("execution witness state root mismatch: computed %x, expected %x", stateRoot, expectedRoot) - } - if receiptRoot != expectedReceiptHash { - return fmt.Errorf("execution witness receipt root mismatch: computed %x, expected %x", receiptRoot, expectedReceiptHash) - } - return nil + return core.ExecuteStateless(context.TODO(), config, vm.Config{}, block, witness) } // validateStatelessInputWitness checks that the execution witness encoded in the