consensus, core, eth/downloader: add 4844 excessDataGas validations

consensus/beacon/consensus.go

@@ -257,7 +257,7 @@ func (beacon *Beacon) verifyHeader(chain consensus.ChainHeaderReader, header, pa
 		return consensus.ErrInvalidNumber
 	}
 	// Verify the header's EIP-1559 attributes.
-	if err := misc.VerifyEip1559Header(chain.Config(), parent, header); err != nil {
+	if err := misc.VerifyEIP1559Header(chain.Config(), parent, header); err != nil {
 		return err
 	}
 	// Verify existence / non-existence of withdrawalsHash.
@@ -270,12 +270,14 @@ func (beacon *Beacon) verifyHeader(chain consensus.ChainHeaderReader, header, pa
 	}
 	// Verify the existence / non-existence of excessDataGas
 	cancun := chain.Config().IsCancun(header.Number, header.Time)
-	if cancun && header.ExcessDataGas == nil {
-		return errors.New("missing excessDataGas")
-	}
 	if !cancun && header.ExcessDataGas != nil {
 		return fmt.Errorf("invalid excessDataGas: have %d, expected nil", header.ExcessDataGas)
 	}
+	if cancun {
+		if err := misc.VerifyEIP4844Header(parent, header); err != nil {
+			return err
+		}
+	}
 	return nil
 }
 

consensus/clique/clique.go

@@ -343,7 +343,7 @@ func (c *Clique) verifyCascadingFields(chain consensus.ChainHeaderReader, header
 		if err := misc.VerifyGaslimit(parent.GasLimit, header.GasLimit); err != nil {
 			return err
 		}
-	} else if err := misc.VerifyEip1559Header(chain.Config(), parent, header); err != nil {
+	} else if err := misc.VerifyEIP1559Header(chain.Config(), parent, header); err != nil {
 		// Verify the header's EIP-1559 attributes.
 		return err
 	}

consensus/ethash/consensus.go

@@ -254,7 +254,7 @@ func (ethash *Ethash) verifyHeader(chain consensus.ChainHeaderReader, header, pa
 		if err := misc.VerifyGaslimit(parent.GasLimit, header.GasLimit); err != nil {
 			return err
 		}
-	} else if err := misc.VerifyEip1559Header(chain.Config(), parent, header); err != nil {
+	} else if err := misc.VerifyEIP1559Header(chain.Config(), parent, header); err != nil {
 		// Verify the header's EIP-1559 attributes.
 		return err
 	}

consensus/misc/eip1559.go

@@ -27,10 +27,10 @@ import (
 	"github.com/ethereum/go-ethereum/params"
 )
 
-// VerifyEip1559Header verifies some header attributes which were changed in EIP-1559,
+// VerifyEIP1559Header verifies some header attributes which were changed in EIP-1559,
 // - gas limit check
 // - basefee check
-func VerifyEip1559Header(config *params.ChainConfig, parent, header *types.Header) error {
+func VerifyEIP1559Header(config *params.ChainConfig, parent, header *types.Header) error {
 	// Verify that the gas limit remains within allowed bounds
 	parentGasLimit := parent.GasLimit
 	if !config.IsLondon(parent.Number) {

consensus/misc/eip1559_test.go

@@ -95,7 +95,7 @@ func TestBlockGasLimits(t *testing.T) {
 			BaseFee:  initial,
 			Number:   big.NewInt(tc.pNum + 1),
 		}
-		err := VerifyEip1559Header(config(), parent, header)
+		err := VerifyEIP1559Header(config(), parent, header)
 		if tc.ok && err != nil {
 			t.Errorf("test %d: Expected valid header: %s", i, err)
 		}

consensus/misc/eip4844.go

@@ -17,8 +17,11 @@
 package misc
 
 import (
+	"errors"
+	"fmt"
 	"math/big"
 
+	"github.com/ethereum/go-ethereum/core/types"
 	"github.com/ethereum/go-ethereum/params"
 )
 
@@ -27,7 +30,59 @@ var (
 	dataGaspriceUpdateFraction = big.NewInt(params.BlobTxDataGaspriceUpdateFraction)
 )
 
+// VerifyEIP4844Header verifies the presence of the excessDataGas field and that
+// if the current block contains no transactions, the excessDataGas is updated
+// accordingly.
+//
+// We cannot verify excessDataGas if there *are* transactions included as that
+// would require the block body. However, it is nonetheless useful to verify the
+// header in case of an empty body since certain code might skip body validations
+// with no included transactions (e.g. snap sync).
+func VerifyEIP4844Header(parent, header *types.Header) error {
+	// Verify the header is not malformed
+	if header.ExcessDataGas == nil {
+		return errors.New("header is missing excessDataGas")
+	}
+	// Verify the excessDataGas is correct based on the parent header iff the
+	// transaction list is empty. For non-empty blocks, validation needs to be
+	// done later.
+	if header.TxHash == types.EmptyTxsHash {
+		expectedExcessDataGas := CalcExcessDataGas(parent.ExcessDataGas, 0)
+		if header.ExcessDataGas.Cmp(expectedExcessDataGas) != 0 {
+			return fmt.Errorf("invalid excessDataGas: have %s, want %s, parentExcessDataGas %s, blob txs %d",
+				header.ExcessDataGas, expectedExcessDataGas, parent.ExcessDataGas, 0)
+		}
+	}
+	return nil
+}
+
+// CalcExcessDataGas calculates the excess data gas after applying the set of
+// blobs on top of the paren't excess data gas.
+//
+// Note, the excessDataGas is akin to gasUsed, in that it's calculated post-
+// execution of the blob transactions not before. Hence, the blob fee used to
+// pay for the blobs are actually derived from the parent data gas.
+func CalcExcessDataGas(parentExcessDataGas *big.Int, blobs int) *big.Int {
+	excessDataGas := new(big.Int)
+	if parentExcessDataGas != nil {
+		excessDataGas.Set(parentExcessDataGas)
+	}
+	consumed := big.NewInt(params.BlobTxDataGasPerBlob)
+	consumed.Mul(consumed, big.NewInt(int64(blobs)))
+	excessDataGas.Add(excessDataGas, consumed)
+
+	targetGas := big.NewInt(params.BlobTxTargetDataGasPerBlock)
+	if excessDataGas.Cmp(targetGas) < 0 {
+		return excessDataGas.SetUint64(0)
+	}
+	return excessDataGas.Sub(excessDataGas, targetGas)
+}
+
 // CalcBlobFee calculates the blobfee from the header's excess data gas field.
+//
+// Note, the blob fee used to pay for blob transactions should be derived from
+// the parent block's excess data gas, since it is a post-execution field akin
+// to gas used.
 func CalcBlobFee(excessDataGas *big.Int) *big.Int {
 	// If this block does not yet have EIP-4844 enabled, return the starting fee
 	if excessDataGas == nil {

consensus/misc/eip4844_test.go

@@ -24,6 +24,44 @@ import (
 	"github.com/ethereum/go-ethereum/params"
 )
 
+func TestCalcExcessDataGas(t *testing.T) {
+	var tests = []struct {
+		parent int64
+		blobs  int
+		want   int64
+	}{
+		// The excess data gas should not increase from zero if the used blob
+		// slots are below - or equal - to the target.
+		{0, 0, 0},
+		{0, 1, 0},
+		{0, params.BlobTxTargetDataGasPerBlock / params.BlobTxDataGasPerBlob, 0},
+
+		// If the target data gas is exceeded, the excessDataGas should increase
+		// by however much it was overshot
+		{0, (params.BlobTxTargetDataGasPerBlock / params.BlobTxDataGasPerBlob) + 1, params.BlobTxDataGasPerBlob},
+		{1, (params.BlobTxTargetDataGasPerBlock / params.BlobTxDataGasPerBlob) + 1, params.BlobTxDataGasPerBlob + 1},
+		{1, (params.BlobTxTargetDataGasPerBlock / params.BlobTxDataGasPerBlob) + 2, 2*params.BlobTxDataGasPerBlob + 1},
+
+		// The excess data gas should decrease by however much the target was
+		// under-shot, capped at zero.
+		{params.BlobTxTargetDataGasPerBlock, params.BlobTxTargetDataGasPerBlock / params.BlobTxDataGasPerBlob, params.BlobTxTargetDataGasPerBlock},
+		{params.BlobTxTargetDataGasPerBlock, (params.BlobTxTargetDataGasPerBlock / params.BlobTxDataGasPerBlob) - 1, params.BlobTxDataGasPerBlob},
+		{params.BlobTxTargetDataGasPerBlock, (params.BlobTxTargetDataGasPerBlock / params.BlobTxDataGasPerBlob) - 2, 0},
+		{params.BlobTxDataGasPerBlob - 1, (params.BlobTxTargetDataGasPerBlock / params.BlobTxDataGasPerBlob) - 1, 0},
+	}
+	for _, tt := range tests {
+		result := CalcExcessDataGas(big.NewInt(tt.parent), tt.blobs)
+		if result.Int64() != tt.want {
+			t.Errorf("excess data gas mismatch: have %v, want %v", result, tt.want)
+		}
+	}
+	// Test nil value for parent
+	result := CalcExcessDataGas(nil, (params.BlobTxTargetDataGasPerBlock/params.BlobTxDataGasPerBlob)+1)
+	if result.Int64() != params.BlobTxDataGasPerBlob {
+		t.Errorf("nil parent excess data gas mismatch: have %v, want %v", result, params.BlobTxDataGasPerBlob)
+	}
+}
+
 func TestCalcBlobFee(t *testing.T) {
 	tests := []struct {
 		excessDataGas int64

core/block_validator.go

@@ -21,6 +21,7 @@ import (
 	"fmt"
 
 	"github.com/ethereum/go-ethereum/consensus"
+	"github.com/ethereum/go-ethereum/consensus/misc"
 	"github.com/ethereum/go-ethereum/core/state"
 	"github.com/ethereum/go-ethereum/core/types"
 	"github.com/ethereum/go-ethereum/params"
@@ -50,12 +51,16 @@ func NewBlockValidator(config *params.ChainConfig, blockchain *BlockChain, engin
 // ValidateBody validates the given block's uncles and verifies the block
 // header's transaction and uncle roots. The headers are assumed to be already
 // validated at this point.
-func (v *BlockValidator) ValidateBody(block *types.Block) error {
+//
+// Note, the parent's presence necessity was introduced in Cancun where the
+// blob excess data gas field is the current block depends on the value in
+// the parent block but needs the transaction list too to validate. If the
+// parent is nil but needed, the validator will attept to load it from disk.
+func (v *BlockValidator) ValidateBody(parent *types.Header, block *types.Block) error {
 	// 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()
@@ -78,10 +83,29 @@ func (v *BlockValidator) ValidateBody(block *types.Block) error {
 			return fmt.Errorf("withdrawals root hash mismatch (header value %x, calculated %x)", *header.WithdrawalsHash, hash)
 		}
 	} else if block.Withdrawals() != nil {
-		// Withdrawals are not allowed prior to shanghai fork
+		// Withdrawals are not allowed prior to Shanghai fork
 		return errors.New("withdrawals present in block body")
 	}
-
+	// Blob transactions may be present after the Cancun fork, validate against
+	// the excessDataGas field. Note, the presence of the field itself must have
+	// already been validated when verifying the header, we only check its value.
+	if header.ExcessDataGas != nil {
+		var blobs int
+		for _, tx := range block.Transactions() {
+			blobs += len(tx.BlobHashes())
+		}
+		if blobs > params.BlobTxMaxDataGasPerBlock/params.BlobTxDataGasPerBlob {
+			return fmt.Errorf("exceeded block capacity (permitted %d, included %d)", params.BlobTxMaxDataGasPerBlock/params.BlobTxDataGasPerBlob, blobs)
+		}
+		if parent == nil {
+			if parent = v.bc.GetHeader(block.ParentHash(), block.NumberU64()-1); parent == nil {
+				return consensus.ErrUnknownAncestor
+			}
+		}
+		if excessDataGas := misc.CalcExcessDataGas(parent.ExcessDataGas, blobs); header.ExcessDataGas.Cmp(excessDataGas) != 0 {
+			return fmt.Errorf("excess data gas mismatch (header value %v, calculated %v)", header.ExcessDataGas, excessDataGas)
+		}
+	}
 	if !v.bc.HasBlockAndState(block.ParentHash(), block.NumberU64()-1) {
 		if !v.bc.HasBlock(block.ParentHash(), block.NumberU64()-1) {
 			return consensus.ErrUnknownAncestor

core/blockchain_insert.go

@@ -121,7 +121,11 @@ func (it *insertIterator) next() (*types.Block, error) {
 		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])
+	var parent *types.Header
+	if it.index > 0 {
+		parent = it.chain[it.index-1].Header()
+	}
+	return it.chain[it.index], it.validator.ValidateBody(parent, it.chain[it.index])
 }
 
 // peek returns the next block in the iterator, along with any potential validation

core/blockchain_test.go

@@ -150,7 +150,7 @@ func testBlockChainImport(chain types.Blocks, blockchain *BlockChain) error {
 		// Try and process the block
 		err := blockchain.engine.VerifyHeader(blockchain, block.Header())
 		if err == nil {
-			err = blockchain.validator.ValidateBody(block)
+			err = blockchain.validator.ValidateBody(nil, block)
 		}
 		if err != nil {
 			if err == ErrKnownBlock {

core/types.go

@@ -28,8 +28,9 @@ import (
 // is only responsible for validating block contents, as the header validation is
 // done by the specific consensus engines.
 type Validator interface {
-	// ValidateBody validates the given block's content.
-	ValidateBody(block *types.Block) error
+	// ValidateBody validates the given block's content. If the parent is nil,
+	// but it is required, the validator should look it up on chain.
+	ValidateBody(parent *types.Header, block *types.Block) error
 
 	// ValidateState validates the given statedb and optionally the receipts and
 	// gas used.

core/types/block.go

@@ -152,10 +152,10 @@ func (h *Header) SanityCheck() error {
 // EmptyBody returns true if there is no additional 'body' to complete the header
 // that is: no transactions, no uncles and no withdrawals.
 func (h *Header) EmptyBody() bool {
-	if h.WithdrawalsHash == nil {
-		return h.TxHash == EmptyTxsHash && h.UncleHash == EmptyUncleHash
+	if h.WithdrawalsHash != nil {
+		return h.TxHash == EmptyTxsHash && *h.WithdrawalsHash == EmptyWithdrawalsHash
 	}
-	return h.TxHash == EmptyTxsHash && h.UncleHash == EmptyUncleHash && *h.WithdrawalsHash == EmptyWithdrawalsHash
+	return h.TxHash == EmptyTxsHash && h.UncleHash == EmptyUncleHash
 }
 
 // EmptyReceipts returns true if there are no receipts for this header/block.
@@ -353,6 +353,13 @@ func (b *Block) Withdrawals() Withdrawals {
 	return b.withdrawals
 }
 
+func (b *Block) ExcessDataGas() *big.Int {
+	if b.header.ExcessDataGas == nil {
+		return nil
+	}
+	return new(big.Int).Set(b.header.ExcessDataGas)
+}
+
 func (b *Block) Header() *Header { return CopyHeader(b.header) }
 
 // Body returns the non-header content of the block.

eth/downloader/downloader.go

@@ -147,10 +147,10 @@ type Downloader struct {
 	quitLock sync.Mutex    // Lock to prevent double closes
 
 	// Testing hooks
-	syncInitHook     func(uint64, uint64)  // Method to call upon initiating a new sync run
-	bodyFetchHook    func([]*types.Header) // Method to call upon starting a block body fetch
-	receiptFetchHook func([]*types.Header) // Method to call upon starting a receipt fetch
-	chainInsertHook  func([]*fetchResult)  // Method to call upon inserting a chain of blocks (possibly in multiple invocations)
+	syncInitHook     func(uint64, uint64) // Method to call upon initiating a new sync run
+	bodyFetchHook    func([]*fetchTask)   // Method to call upon starting a block body fetch
+	receiptFetchHook func([]*fetchTask)   // Method to call upon starting a receipt fetch
+	chainInsertHook  func([]*fetchResult) // Method to call upon inserting a chain of blocks (possibly in multiple invocations)
 
 	// Progress reporting metrics
 	syncStartBlock uint64    // Head snap block when Geth was started
@@ -166,6 +166,9 @@ type LightChain interface {
 	// GetHeaderByHash retrieves a header from the local chain.
 	GetHeaderByHash(common.Hash) *types.Header
 
+	// GetHeaderByNumber retrieves a header from the local canonical chain.
+	GetHeaderByNumber(uint64) *types.Header
+
 	// CurrentHeader retrieves the head header from the local chain.
 	CurrentHeader() *types.Header
 
@@ -614,6 +617,11 @@ func (d *Downloader) syncWithPeer(p *peerConnection, hash common.Hash, td, ttd *
 			}
 		}
 	}
+	parent := d.lightchain.GetHeaderByNumber(origin)
+	if parent == nil {
+		log.Error("Failed to retrieve just found common ancestor")
+		return fmt.Errorf("missing common ancestor #%d", origin)
+	}
 	// Initiate the sync using a concurrent header and content retrieval algorithm
 	d.queue.Prepare(origin+1, mode)
 	if d.syncInitHook != nil {
@@ -631,7 +639,7 @@ func (d *Downloader) syncWithPeer(p *peerConnection, hash common.Hash, td, ttd *
 		headerFetcher, // Headers are always retrieved
 		func() error { return d.fetchBodies(origin+1, beaconMode) },   // Bodies are retrieved during normal and snap sync
 		func() error { return d.fetchReceipts(origin+1, beaconMode) }, // Receipts are retrieved during snap sync
-		func() error { return d.processHeaders(origin+1, td, ttd, beaconMode) },
+		func() error { return d.processHeaders(parent, td, ttd, beaconMode) },
 	}
 	if mode == SnapSync {
 		d.pivotLock.Lock()
@@ -1272,7 +1280,7 @@ func (d *Downloader) fetchReceipts(from uint64, beaconMode bool) error {
 // processHeaders takes batches of retrieved headers from an input channel and
 // keeps processing and scheduling them into the header chain and downloader's
 // queue until the stream ends or a failure occurs.
-func (d *Downloader) processHeaders(origin uint64, td, ttd *big.Int, beaconMode bool) error {
+func (d *Downloader) processHeaders(parent *types.Header, td, ttd *big.Int, beaconMode bool) error {
 	// Keep a count of uncertain headers to roll back
 	var (
 		rollback    uint64 // Zero means no rollback (fine as you can't unroll the genesis)
@@ -1454,20 +1462,20 @@ func (d *Downloader) processHeaders(origin uint64, td, ttd *big.Int, beaconMode
 						}
 					}
 					// Otherwise insert the headers for content retrieval
-					inserts := d.queue.Schedule(chunkHeaders, chunkHashes, origin)
+					inserts := d.queue.Schedule(parent, chunkHeaders, chunkHashes)
 					if len(inserts) != len(chunkHeaders) {
 						rollbackErr = fmt.Errorf("stale headers: len inserts %v len(chunk) %v", len(inserts), len(chunkHeaders))
 						return fmt.Errorf("%w: stale headers", errBadPeer)
 					}
 				}
+				parent = headers[limit-1]
 				headers = headers[limit:]
 				hashes = hashes[limit:]
-				origin += uint64(limit)
 			}
 			// Update the highest block number we know if a higher one is found.
 			d.syncStatsLock.Lock()
-			if d.syncStatsChainHeight < origin {
-				d.syncStatsChainHeight = origin - 1
+			if d.syncStatsChainHeight <= parent.Number.Uint64() {
+				d.syncStatsChainHeight = parent.Number.Uint64()
 			}
 			d.syncStatsLock.Unlock()
 

eth/downloader/downloader_test.go

@@ -787,10 +787,10 @@ func testEmptyShortCircuit(t *testing.T, protocol uint, mode SyncMode) {
 
 	// Instrument the downloader to signal body requests
 	var bodiesHave, receiptsHave atomic.Int32
-	tester.downloader.bodyFetchHook = func(headers []*types.Header) {
+	tester.downloader.bodyFetchHook = func(headers []*fetchTask) {
 		bodiesHave.Add(int32(len(headers)))
 	}
-	tester.downloader.receiptFetchHook = func(headers []*types.Header) {
+	tester.downloader.receiptFetchHook = func(headers []*fetchTask) {
 		receiptsHave.Add(int32(len(headers)))
 	}
 	// Synchronise with the peer and make sure all blocks were retrieved