From 16dc7413dd3c874eedb326e7ff08649fd63ea80d Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Tue, 1 Aug 2023 20:10:32 +0200 Subject: [PATCH 01/22] core/types: remove blob methods from TxData interface Only blob transactions can have blobs, so there is no need to have these methods on every single tx type. --- core/types/transaction.go | 40 +++++++++++++++++++++++------------- core/types/tx_access_list.go | 25 ++++++++++------------ core/types/tx_blob.go | 27 ++++++++++++------------ core/types/tx_dynamic_fee.go | 25 ++++++++++------------ core/types/tx_legacy.go | 25 ++++++++++------------ 5 files changed, 72 insertions(+), 70 deletions(-) diff --git a/core/types/transaction.go b/core/types/transaction.go index e774809c29d9..8583c7a86017 100644 --- a/core/types/transaction.go +++ b/core/types/transaction.go @@ -82,9 +82,6 @@ type TxData interface { value() *big.Int nonce() uint64 to() *common.Address - blobGas() uint64 - blobGasFeeCap() *big.Int - blobHashes() []common.Hash rawSignatureValues() (v, r, s *big.Int) setSignatureValues(chainID, v, r, s *big.Int) @@ -288,15 +285,6 @@ func (tx *Transaction) GasTipCap() *big.Int { return new(big.Int).Set(tx.inner.g // GasFeeCap returns the fee cap per gas of the transaction. func (tx *Transaction) GasFeeCap() *big.Int { return new(big.Int).Set(tx.inner.gasFeeCap()) } -// BlobGas returns the blob gas limit of the transaction for blob transactions, 0 otherwise. -func (tx *Transaction) BlobGas() uint64 { return tx.inner.blobGas() } - -// BlobGasFeeCap returns the blob gas fee cap per blob gas of the transaction for blob transactions, nil otherwise. -func (tx *Transaction) BlobGasFeeCap() *big.Int { return tx.inner.blobGasFeeCap() } - -// BlobHashes returns the hases of the blob commitments for blob transactions, nil otherwise. -func (tx *Transaction) BlobHashes() []common.Hash { return tx.inner.blobHashes() } - // Value returns the ether amount of the transaction. func (tx *Transaction) Value() *big.Int { return new(big.Int).Set(tx.inner.value()) } @@ -383,14 +371,38 @@ func (tx *Transaction) EffectiveGasTipIntCmp(other *big.Int, baseFee *big.Int) i return tx.EffectiveGasTipValue(baseFee).Cmp(other) } +// BlobGas returns the blob gas limit of the transaction for blob transactions, 0 otherwise. +func (tx *Transaction) BlobGas() uint64 { + if blobtx, ok := tx.inner.(*BlobTx); ok { + return blobtx.blobGas() + } + return 0 +} + +// BlobGasFeeCap returns the blob gas fee cap per blob gas of the transaction for blob transactions, nil otherwise. +func (tx *Transaction) BlobGasFeeCap() *big.Int { + if blobtx, ok := tx.inner.(*BlobTx); ok { + return blobtx.BlobFeeCap.ToBig() + } + return nil +} + +// BlobHashes returns the hases of the blob commitments for blob transactions, nil otherwise. +func (tx *Transaction) BlobHashes() []common.Hash { + if blobtx, ok := tx.inner.(*BlobTx); ok { + return blobtx.BlobHashes + } + return nil +} + // BlobGasFeeCapCmp compares the blob fee cap of two transactions. func (tx *Transaction) BlobGasFeeCapCmp(other *Transaction) int { - return tx.inner.blobGasFeeCap().Cmp(other.inner.blobGasFeeCap()) + return tx.BlobGasFeeCap().Cmp(other.BlobGasFeeCap()) } // BlobGasFeeCapIntCmp compares the blob fee cap of the transaction against the given blob fee cap. func (tx *Transaction) BlobGasFeeCapIntCmp(other *big.Int) int { - return tx.inner.blobGasFeeCap().Cmp(other) + return tx.BlobGasFeeCap().Cmp(other) } // SetTime sets the decoding time of a transaction. This is used by tests to set diff --git a/core/types/tx_access_list.go b/core/types/tx_access_list.go index 7ce9da73ecbe..e7d6efffbbbb 100644 --- a/core/types/tx_access_list.go +++ b/core/types/tx_access_list.go @@ -94,20 +94,17 @@ func (tx *AccessListTx) copy() TxData { } // accessors for innerTx. -func (tx *AccessListTx) txType() byte { return AccessListTxType } -func (tx *AccessListTx) chainID() *big.Int { return tx.ChainID } -func (tx *AccessListTx) accessList() AccessList { return tx.AccessList } -func (tx *AccessListTx) data() []byte { return tx.Data } -func (tx *AccessListTx) gas() uint64 { return tx.Gas } -func (tx *AccessListTx) gasPrice() *big.Int { return tx.GasPrice } -func (tx *AccessListTx) gasTipCap() *big.Int { return tx.GasPrice } -func (tx *AccessListTx) gasFeeCap() *big.Int { return tx.GasPrice } -func (tx *AccessListTx) value() *big.Int { return tx.Value } -func (tx *AccessListTx) nonce() uint64 { return tx.Nonce } -func (tx *AccessListTx) to() *common.Address { return tx.To } -func (tx *AccessListTx) blobGas() uint64 { return 0 } -func (tx *AccessListTx) blobGasFeeCap() *big.Int { return nil } -func (tx *AccessListTx) blobHashes() []common.Hash { return nil } +func (tx *AccessListTx) txType() byte { return AccessListTxType } +func (tx *AccessListTx) chainID() *big.Int { return tx.ChainID } +func (tx *AccessListTx) accessList() AccessList { return tx.AccessList } +func (tx *AccessListTx) data() []byte { return tx.Data } +func (tx *AccessListTx) gas() uint64 { return tx.Gas } +func (tx *AccessListTx) gasPrice() *big.Int { return tx.GasPrice } +func (tx *AccessListTx) gasTipCap() *big.Int { return tx.GasPrice } +func (tx *AccessListTx) gasFeeCap() *big.Int { return tx.GasPrice } +func (tx *AccessListTx) value() *big.Int { return tx.Value } +func (tx *AccessListTx) nonce() uint64 { return tx.Nonce } +func (tx *AccessListTx) to() *common.Address { return tx.To } func (tx *AccessListTx) effectiveGasPrice(dst *big.Int, baseFee *big.Int) *big.Int { return dst.Set(tx.GasPrice) diff --git a/core/types/tx_blob.go b/core/types/tx_blob.go index a08121bf1416..0f19163baf53 100644 --- a/core/types/tx_blob.go +++ b/core/types/tx_blob.go @@ -94,20 +94,19 @@ func (tx *BlobTx) copy() TxData { } // accessors for innerTx. -func (tx *BlobTx) txType() byte { return BlobTxType } -func (tx *BlobTx) chainID() *big.Int { return tx.ChainID.ToBig() } -func (tx *BlobTx) accessList() AccessList { return tx.AccessList } -func (tx *BlobTx) data() []byte { return tx.Data } -func (tx *BlobTx) gas() uint64 { return tx.Gas } -func (tx *BlobTx) gasFeeCap() *big.Int { return tx.GasFeeCap.ToBig() } -func (tx *BlobTx) gasTipCap() *big.Int { return tx.GasTipCap.ToBig() } -func (tx *BlobTx) gasPrice() *big.Int { return tx.GasFeeCap.ToBig() } -func (tx *BlobTx) value() *big.Int { return tx.Value.ToBig() } -func (tx *BlobTx) nonce() uint64 { return tx.Nonce } -func (tx *BlobTx) to() *common.Address { tmp := tx.To; return &tmp } -func (tx *BlobTx) blobGas() uint64 { return params.BlobTxBlobGasPerBlob * uint64(len(tx.BlobHashes)) } -func (tx *BlobTx) blobGasFeeCap() *big.Int { return tx.BlobFeeCap.ToBig() } -func (tx *BlobTx) blobHashes() []common.Hash { return tx.BlobHashes } +func (tx *BlobTx) txType() byte { return BlobTxType } +func (tx *BlobTx) chainID() *big.Int { return tx.ChainID.ToBig() } +func (tx *BlobTx) accessList() AccessList { return tx.AccessList } +func (tx *BlobTx) data() []byte { return tx.Data } +func (tx *BlobTx) gas() uint64 { return tx.Gas } +func (tx *BlobTx) gasFeeCap() *big.Int { return tx.GasFeeCap.ToBig() } +func (tx *BlobTx) gasTipCap() *big.Int { return tx.GasTipCap.ToBig() } +func (tx *BlobTx) gasPrice() *big.Int { return tx.GasFeeCap.ToBig() } +func (tx *BlobTx) value() *big.Int { return tx.Value.ToBig() } +func (tx *BlobTx) nonce() uint64 { return tx.Nonce } +func (tx *BlobTx) to() *common.Address { tmp := tx.To; return &tmp } +func (tx *BlobTx) blobGas() uint64 { return params.BlobTxBlobGasPerBlob * uint64(len(tx.BlobHashes)) } +func (tx *BlobTx) blobGasFeeCap() *big.Int { return tx.BlobFeeCap.ToBig() } func (tx *BlobTx) effectiveGasPrice(dst *big.Int, baseFee *big.Int) *big.Int { if baseFee == nil { diff --git a/core/types/tx_dynamic_fee.go b/core/types/tx_dynamic_fee.go index 47b870abbf5c..7467c72e8ff5 100644 --- a/core/types/tx_dynamic_fee.go +++ b/core/types/tx_dynamic_fee.go @@ -83,20 +83,17 @@ func (tx *DynamicFeeTx) copy() TxData { } // accessors for innerTx. -func (tx *DynamicFeeTx) txType() byte { return DynamicFeeTxType } -func (tx *DynamicFeeTx) chainID() *big.Int { return tx.ChainID } -func (tx *DynamicFeeTx) accessList() AccessList { return tx.AccessList } -func (tx *DynamicFeeTx) data() []byte { return tx.Data } -func (tx *DynamicFeeTx) gas() uint64 { return tx.Gas } -func (tx *DynamicFeeTx) gasFeeCap() *big.Int { return tx.GasFeeCap } -func (tx *DynamicFeeTx) gasTipCap() *big.Int { return tx.GasTipCap } -func (tx *DynamicFeeTx) gasPrice() *big.Int { return tx.GasFeeCap } -func (tx *DynamicFeeTx) value() *big.Int { return tx.Value } -func (tx *DynamicFeeTx) nonce() uint64 { return tx.Nonce } -func (tx *DynamicFeeTx) to() *common.Address { return tx.To } -func (tx *DynamicFeeTx) blobGas() uint64 { return 0 } -func (tx *DynamicFeeTx) blobGasFeeCap() *big.Int { return nil } -func (tx *DynamicFeeTx) blobHashes() []common.Hash { return nil } +func (tx *DynamicFeeTx) txType() byte { return DynamicFeeTxType } +func (tx *DynamicFeeTx) chainID() *big.Int { return tx.ChainID } +func (tx *DynamicFeeTx) accessList() AccessList { return tx.AccessList } +func (tx *DynamicFeeTx) data() []byte { return tx.Data } +func (tx *DynamicFeeTx) gas() uint64 { return tx.Gas } +func (tx *DynamicFeeTx) gasFeeCap() *big.Int { return tx.GasFeeCap } +func (tx *DynamicFeeTx) gasTipCap() *big.Int { return tx.GasTipCap } +func (tx *DynamicFeeTx) gasPrice() *big.Int { return tx.GasFeeCap } +func (tx *DynamicFeeTx) value() *big.Int { return tx.Value } +func (tx *DynamicFeeTx) nonce() uint64 { return tx.Nonce } +func (tx *DynamicFeeTx) to() *common.Address { return tx.To } func (tx *DynamicFeeTx) effectiveGasPrice(dst *big.Int, baseFee *big.Int) *big.Int { if baseFee == nil { diff --git a/core/types/tx_legacy.go b/core/types/tx_legacy.go index 902e70cf94b9..aefce2436abe 100644 --- a/core/types/tx_legacy.go +++ b/core/types/tx_legacy.go @@ -91,20 +91,17 @@ func (tx *LegacyTx) copy() TxData { } // accessors for innerTx. -func (tx *LegacyTx) txType() byte { return LegacyTxType } -func (tx *LegacyTx) chainID() *big.Int { return deriveChainId(tx.V) } -func (tx *LegacyTx) accessList() AccessList { return nil } -func (tx *LegacyTx) data() []byte { return tx.Data } -func (tx *LegacyTx) gas() uint64 { return tx.Gas } -func (tx *LegacyTx) gasPrice() *big.Int { return tx.GasPrice } -func (tx *LegacyTx) gasTipCap() *big.Int { return tx.GasPrice } -func (tx *LegacyTx) gasFeeCap() *big.Int { return tx.GasPrice } -func (tx *LegacyTx) value() *big.Int { return tx.Value } -func (tx *LegacyTx) nonce() uint64 { return tx.Nonce } -func (tx *LegacyTx) to() *common.Address { return tx.To } -func (tx *LegacyTx) blobGas() uint64 { return 0 } -func (tx *LegacyTx) blobGasFeeCap() *big.Int { return nil } -func (tx *LegacyTx) blobHashes() []common.Hash { return nil } +func (tx *LegacyTx) txType() byte { return LegacyTxType } +func (tx *LegacyTx) chainID() *big.Int { return deriveChainId(tx.V) } +func (tx *LegacyTx) accessList() AccessList { return nil } +func (tx *LegacyTx) data() []byte { return tx.Data } +func (tx *LegacyTx) gas() uint64 { return tx.Gas } +func (tx *LegacyTx) gasPrice() *big.Int { return tx.GasPrice } +func (tx *LegacyTx) gasTipCap() *big.Int { return tx.GasPrice } +func (tx *LegacyTx) gasFeeCap() *big.Int { return tx.GasPrice } +func (tx *LegacyTx) value() *big.Int { return tx.Value } +func (tx *LegacyTx) nonce() uint64 { return tx.Nonce } +func (tx *LegacyTx) to() *common.Address { return tx.To } func (tx *LegacyTx) effectiveGasPrice(dst *big.Int, baseFee *big.Int) *big.Int { return dst.Set(tx.GasPrice) From 13b7f12e799c0a6ddbb2f78f91130a6121660a4b Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Wed, 2 Aug 2023 21:25:55 +0200 Subject: [PATCH 02/22] core/types: optional blob sidecar in BlobTx --- core/types/transaction.go | 28 +++++++++----- core/types/tx_access_list.go | 10 +++++ core/types/tx_blob.go | 75 ++++++++++++++++++++++++++++++++++++ core/types/tx_dynamic_fee.go | 10 +++++ core/types/tx_legacy.go | 9 +++++ 5 files changed, 122 insertions(+), 10 deletions(-) diff --git a/core/types/transaction.go b/core/types/transaction.go index 8583c7a86017..da37d1a8c4f5 100644 --- a/core/types/transaction.go +++ b/core/types/transaction.go @@ -93,6 +93,9 @@ type TxData interface { // copy of the computed value, i.e. callers are allowed to mutate the result. // Method implementations can use 'dst' to store the result. effectiveGasPrice(dst *big.Int, baseFee *big.Int) *big.Int + + encode(b *bytes.Buffer) error + decode(b []byte) error } // EncodeRLP implements rlp.Encoder @@ -113,7 +116,7 @@ func (tx *Transaction) EncodeRLP(w io.Writer) error { // encodeTyped writes the canonical encoding of a typed transaction to w. func (tx *Transaction) encodeTyped(w *bytes.Buffer) error { w.WriteByte(tx.Type()) - return rlp.Encode(w, tx.inner) + return tx.inner.encode(w) } // MarshalBinary returns the canonical encoding of the transaction. @@ -183,22 +186,19 @@ func (tx *Transaction) decodeTyped(b []byte) (TxData, error) { if len(b) <= 1 { return nil, errShortTypedTx } + var inner TxData switch b[0] { case AccessListTxType: - var inner AccessListTx - err := rlp.DecodeBytes(b[1:], &inner) - return &inner, err + inner = new(AccessListTx) case DynamicFeeTxType: - var inner DynamicFeeTx - err := rlp.DecodeBytes(b[1:], &inner) - return &inner, err + inner = new(DynamicFeeTx) case BlobTxType: - var inner BlobTx - err := rlp.DecodeBytes(b[1:], &inner) - return &inner, err + inner = new(BlobTx) default: return nil, ErrTxTypeNotSupported } + err := inner.decode(b[1:]) + return inner, err } // setDecoded sets the inner transaction and size after decoding. @@ -395,6 +395,14 @@ func (tx *Transaction) BlobHashes() []common.Hash { return nil } +// BlobSidecar returns the sidecar of a blob transaction, nil otherwise. +func (tx *Transaction) BlobSidecar() *BlobSidecar { + if blobtx, ok := tx.inner.(*BlobTx); ok { + return blobtx.Sidecar + } + return nil +} + // BlobGasFeeCapCmp compares the blob fee cap of two transactions. func (tx *Transaction) BlobGasFeeCapCmp(other *Transaction) int { return tx.BlobGasFeeCap().Cmp(other.BlobGasFeeCap()) diff --git a/core/types/tx_access_list.go b/core/types/tx_access_list.go index e7d6efffbbbb..730a77b75286 100644 --- a/core/types/tx_access_list.go +++ b/core/types/tx_access_list.go @@ -17,9 +17,11 @@ package types import ( + "bytes" "math/big" "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/rlp" ) //go:generate go run github.com/fjl/gencodec -type AccessTuple -out gen_access_tuple.go @@ -117,3 +119,11 @@ func (tx *AccessListTx) rawSignatureValues() (v, r, s *big.Int) { func (tx *AccessListTx) setSignatureValues(chainID, v, r, s *big.Int) { tx.ChainID, tx.V, tx.R, tx.S = chainID, v, r, s } + +func (tx *AccessListTx) encode(b *bytes.Buffer) error { + return rlp.Encode(b, tx) +} + +func (tx *AccessListTx) decode(input []byte) error { + return rlp.DecodeBytes(input, tx) +} diff --git a/core/types/tx_blob.go b/core/types/tx_blob.go index 0f19163baf53..e0488ffa6747 100644 --- a/core/types/tx_blob.go +++ b/core/types/tx_blob.go @@ -17,10 +17,13 @@ package types import ( + "bytes" "math/big" "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/crypto/kzg4844" "github.com/ethereum/go-ethereum/params" + "github.com/ethereum/go-ethereum/rlp" "github.com/holiman/uint256" ) @@ -38,12 +41,31 @@ type BlobTx struct { BlobFeeCap *uint256.Int // a.k.a. maxFeePerBlobGas BlobHashes []common.Hash + // A blob transaction can optionally contain blobs. This field must be set when BlobTx + // is used to create a transaction for sigining. + Sidecar *BlobSidecar `rlp:"-"` + // Signature values V *uint256.Int `json:"v" gencodec:"required"` R *uint256.Int `json:"r" gencodec:"required"` S *uint256.Int `json:"s" gencodec:"required"` } +// BlobSidecar contains the blobs of a blob transaction. +type BlobSidecar struct { + Blobs []kzg4844.Blob // Blobs needed by the blob pool + Commitments []kzg4844.Commitment // Commitments needed by the blob pool + Proofs []kzg4844.Proof // Proofs needed by the blob pool +} + +// blobTxWithBlobs is used for encoding of transactions when blobs are present. +type blobTxWithBlobs struct { + BlobTx *BlobTx + Blobs []kzg4844.Blob + Commitments []kzg4844.Commitment + Proofs []kzg4844.Proof +} + // copy creates a deep copy of the transaction data and initializes all fields. func (tx *BlobTx) copy() TxData { cpy := &BlobTx{ @@ -90,6 +112,13 @@ func (tx *BlobTx) copy() TxData { if tx.S != nil { cpy.S.Set(tx.S) } + if tx.Sidecar != nil { + cpy.Sidecar = &BlobSidecar{ + Blobs: append([]kzg4844.Blob(nil), tx.Sidecar.Blobs...), + Commitments: append([]kzg4844.Commitment(nil), tx.Sidecar.Commitments...), + Proofs: append([]kzg4844.Proof(nil), tx.Sidecar.Proofs...), + } + } return cpy } @@ -129,3 +158,49 @@ func (tx *BlobTx) setSignatureValues(chainID, v, r, s *big.Int) { tx.R.SetFromBig(r) tx.S.SetFromBig(s) } + +func (tx *BlobTx) encode(b *bytes.Buffer) error { + if tx.Sidecar == nil { + return rlp.Encode(b, tx) + } + inner := &blobTxWithBlobs{ + BlobTx: tx, + Blobs: tx.Sidecar.Blobs, + Commitments: tx.Sidecar.Commitments, + Proofs: tx.Sidecar.Proofs, + } + return rlp.Encode(b, inner) +} + +func (tx *BlobTx) decode(input []byte) error { + // Here we need to support two formats: the network protocol encoding of the tx (with + // blobs) or the canonical encoding without blobs. + // + // The two encodings can be distinguished by checking whether the first element of the + // input list is itself a list. + + outerList, _, err := rlp.SplitList(input) + if err != nil { + return err + } + firstElemKind, _, _, err := rlp.Split(outerList) + if err != nil { + return err + } + + if firstElemKind != rlp.List { + return rlp.DecodeBytes(input, tx) + } + // It's a tx with blobs. + var inner blobTxWithBlobs + if err := rlp.DecodeBytes(input, &inner); err != nil { + return err + } + *tx = *inner.BlobTx + tx.Sidecar = &BlobSidecar{ + Blobs: inner.Blobs, + Commitments: inner.Commitments, + Proofs: inner.Proofs, + } + return nil +} diff --git a/core/types/tx_dynamic_fee.go b/core/types/tx_dynamic_fee.go index 7467c72e8ff5..8b5b514fdec5 100644 --- a/core/types/tx_dynamic_fee.go +++ b/core/types/tx_dynamic_fee.go @@ -17,9 +17,11 @@ package types import ( + "bytes" "math/big" "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/rlp" ) // DynamicFeeTx represents an EIP-1559 transaction. @@ -113,3 +115,11 @@ func (tx *DynamicFeeTx) rawSignatureValues() (v, r, s *big.Int) { func (tx *DynamicFeeTx) setSignatureValues(chainID, v, r, s *big.Int) { tx.ChainID, tx.V, tx.R, tx.S = chainID, v, r, s } + +func (tx *DynamicFeeTx) encode(b *bytes.Buffer) error { + return rlp.Encode(b, tx) +} + +func (tx *DynamicFeeTx) decode(input []byte) error { + return rlp.DecodeBytes(input, tx) +} diff --git a/core/types/tx_legacy.go b/core/types/tx_legacy.go index aefce2436abe..71025b78fc06 100644 --- a/core/types/tx_legacy.go +++ b/core/types/tx_legacy.go @@ -17,6 +17,7 @@ package types import ( + "bytes" "math/big" "github.com/ethereum/go-ethereum/common" @@ -114,3 +115,11 @@ func (tx *LegacyTx) rawSignatureValues() (v, r, s *big.Int) { func (tx *LegacyTx) setSignatureValues(chainID, v, r, s *big.Int) { tx.V, tx.R, tx.S = v, r, s } + +func (tx *LegacyTx) encode(*bytes.Buffer) error { + panic("encode called on LegacyTx") +} + +func (tx *LegacyTx) decode([]byte) error { + panic("decode called on LegacyTx)") +} From d2c86cdcef37c2ad7cad6a8d9dfa1b88156f2e4c Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Wed, 2 Aug 2023 21:30:19 +0200 Subject: [PATCH 03/22] core: verify absence of sidecar when importing blocks --- core/block_validator.go | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/core/block_validator.go b/core/block_validator.go index 3c9ac3dc49d5..c3c031b4ba9d 100644 --- a/core/block_validator.go +++ b/core/block_validator.go @@ -68,6 +68,7 @@ func (v *BlockValidator) ValidateBody(block *types.Block) error { if hash := types.DeriveSha(block.Transactions(), trie.NewStackTrie(nil)); hash != header.TxHash { return fmt.Errorf("transaction root hash mismatch (header value %x, calculated %x)", header.TxHash, hash) } + // Withdrawals are present after the Shanghai fork. if header.WithdrawalsHash != nil { // Withdrawals list must be present in body after Shanghai. @@ -81,14 +82,23 @@ func (v *BlockValidator) ValidateBody(block *types.Block) error { // 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. var blobs int - for _, tx := range block.Transactions() { + for i, tx := range block.Transactions() { // Count the number of blobs to validate against the header's blobGasUsed blobs += len(tx.BlobHashes()) + + // If the tx is a blob tx, it must have a sidecar attached to be valid in a block. + if tx.Type() == types.BlobTxType && tx.BlobSidecar() != nil { + return fmt.Errorf("unexpected blob sidecar in transaction at index %d", i) + } + // The individual checks for blob validity (version-check + not empty) - // happens in the state_transition check. + // happens in StateTransition. } + + // Check blob gas usage. if header.BlobGasUsed != nil { if want := *header.BlobGasUsed / params.BlobTxBlobGasPerBlob; uint64(blobs) != want { // div because the header is surely good vs the body might be bloated return fmt.Errorf("blob gas used mismatch (header %v, calculated %v)", *header.BlobGasUsed, blobs*params.BlobTxBlobGasPerBlob) @@ -98,6 +108,8 @@ func (v *BlockValidator) ValidateBody(block *types.Block) error { return errors.New("data blobs present in block body") } } + + // 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 From 10b14f478df39e65ab0596e0453d86cf4b103383 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Thu, 3 Aug 2023 10:20:11 +0200 Subject: [PATCH 04/22] core/txpool: new blob tx type integration --- core/txpool/blobpool/blobpool.go | 69 +++++++++++---------------- core/txpool/blobpool/blobpool_test.go | 51 ++++++++++---------- core/txpool/blobpool/limbo.go | 36 ++++++-------- core/txpool/legacypool/legacypool.go | 37 ++++++-------- core/txpool/subpool.go | 23 +++------ core/txpool/txpool.go | 8 ++-- core/txpool/validation.go | 67 +++++++++++++++----------- 7 files changed, 133 insertions(+), 158 deletions(-) diff --git a/core/txpool/blobpool/blobpool.go b/core/txpool/blobpool/blobpool.go index c0dd6e8acc90..ea6d71d4629f 100644 --- a/core/txpool/blobpool/blobpool.go +++ b/core/txpool/blobpool/blobpool.go @@ -35,7 +35,6 @@ import ( "github.com/ethereum/go-ethereum/core/state" "github.com/ethereum/go-ethereum/core/txpool" "github.com/ethereum/go-ethereum/core/types" - "github.com/ethereum/go-ethereum/crypto/kzg4844" "github.com/ethereum/go-ethereum/event" "github.com/ethereum/go-ethereum/log" "github.com/ethereum/go-ethereum/metrics" @@ -83,16 +82,6 @@ const ( limboedTransactionStore = "limbo" ) -// blobTx is a wrapper around types.BlobTx which also contains the literal blob -// data along with all the transaction metadata. -type blobTx struct { - Tx *types.Transaction - - Blobs []kzg4844.Blob - Commits []kzg4844.Commitment - Proofs []kzg4844.Proof -} - // blobTxMeta is the minimal subset of types.BlobTx necessary to validate and // schedule the blob transactions into the following blocks. Only ever add the // bare minimum needed fields to keep the size down (and thus number of entries @@ -455,22 +444,22 @@ func (p *BlobPool) Close() error { // parseTransaction is a callback method on pool creation that gets called for // each transaction on disk to create the in-memory metadata index. func (p *BlobPool) parseTransaction(id uint64, size uint32, blob []byte) error { - item := new(blobTx) - if err := rlp.DecodeBytes(blob, item); err != nil { + tx := new(types.Transaction) + if err := rlp.DecodeBytes(blob, tx); err != nil { // This path is impossible unless the disk data representation changes // across restarts. For that ever unprobable case, recover gracefully // by ignoring this data entry. log.Error("Failed to decode blob pool entry", "id", id, "err", err) return err } - meta := newBlobTxMeta(id, size, item.Tx) + meta := newBlobTxMeta(id, size, tx) - sender, err := p.signer.Sender(item.Tx) + sender, err := p.signer.Sender(tx) if err != nil { // This path is impossible unless the signature validity changes across // restarts. For that ever unprobable case, recover gracefully by ignoring // this data entry. - log.Error("Failed to recover blob tx sender", "id", id, "hash", item.Tx.Hash(), "err", err) + log.Error("Failed to recover blob tx sender", "id", id, "hash", tx.Hash(), "err", err) return err } if _, ok := p.index[sender]; !ok { @@ -718,17 +707,17 @@ func (p *BlobPool) offload(addr common.Address, nonce uint64, id uint64, inclusi log.Error("Blobs missing for included transaction", "from", addr, "nonce", nonce, "id", id, "err", err) return } - item := new(blobTx) - if err = rlp.DecodeBytes(data, item); err != nil { + var tx types.Transaction + if err = rlp.DecodeBytes(data, tx); err != nil { log.Error("Blobs corrupted for included transaction", "from", addr, "nonce", nonce, "id", id, "err", err) return } - block, ok := inclusions[item.Tx.Hash()] + block, ok := inclusions[tx.Hash()] if !ok { log.Warn("Blob transaction swapped out by signer", "from", addr, "nonce", nonce, "id", id) return } - if err := p.limbo.push(item.Tx.Hash(), block, item.Blobs, item.Commits, item.Proofs); err != nil { + if err := p.limbo.push(tx.Hash(), block, &tx); err != nil { log.Warn("Failed to offload blob tx into limbo", "err", err) return } @@ -760,7 +749,7 @@ func (p *BlobPool) Reset(oldHead, newHead *types.Header) { for addr, txs := range reinject { // Blindly push all the lost transactions back into the pool for _, tx := range txs { - p.reinject(addr, tx) + p.reinject(addr, tx.Hash()) } // Recheck the account's pooled transactions to drop included and // invalidated one @@ -920,16 +909,19 @@ func (p *BlobPool) reorg(oldHead, newHead *types.Header) (map[common.Address][]* // Note, the method will not initialize the eviction cache values as those will // be done once for all transactions belonging to an account after all individual // transactions are injected back into the pool. -func (p *BlobPool) reinject(addr common.Address, tx *types.Transaction) { +func (p *BlobPool) reinject(addr common.Address, txhash common.Hash) { // Retrieve the associated blob from the limbo. Without the blobs, we cannot // add the transaction back into the pool as it is not mineable. - blobs, commits, proofs, err := p.limbo.pull(tx.Hash()) + tx, err := p.limbo.pull(txhash) if err != nil { log.Error("Blobs unavailable, dropping reorged tx", "err", err) return } - // Serialize the transaction back into the primary datastore - blob, err := rlp.EncodeToBytes(&blobTx{Tx: tx, Blobs: blobs, Commits: commits, Proofs: proofs}) + // TODO: seems like an easy optimization here would be getting the serialized tx + // from limbo instead of re-serializing it here. + + // Serialize the transaction back into the primary datastore. + blob, err := rlp.EncodeToBytes(tx) if err != nil { log.Error("Failed to encode transaction for storage", "hash", tx.Hash(), "err", err) return @@ -939,9 +931,9 @@ func (p *BlobPool) reinject(addr common.Address, tx *types.Transaction) { log.Error("Failed to write transaction into storage", "hash", tx.Hash(), "err", err) return } + // Update the indixes and metrics meta := newBlobTxMeta(id, p.store.Size(id), tx) - if _, ok := p.index[addr]; !ok { if err := p.reserve(addr, true); err != nil { log.Warn("Failed to reserve account for blob pool", "tx", tx.Hash(), "from", addr, "err", err) @@ -1023,7 +1015,7 @@ func (p *BlobPool) SetGasTip(tip *big.Int) { // validateTx checks whether a transaction is valid according to the consensus // rules and adheres to some heuristic limits of the local node (price and size). -func (p *BlobPool) validateTx(tx *types.Transaction, blobs []kzg4844.Blob, commits []kzg4844.Commitment, proofs []kzg4844.Proof) error { +func (p *BlobPool) validateTx(tx *types.Transaction) error { // Ensure the transaction adheres to basic pool filters (type, size, tip) and // consensus rules baseOpts := &txpool.ValidationOptions{ @@ -1032,7 +1024,7 @@ func (p *BlobPool) validateTx(tx *types.Transaction, blobs []kzg4844.Blob, commi MaxSize: txMaxSize, MinTip: p.gasTip.ToBig(), } - if err := txpool.ValidateTransaction(tx, blobs, commits, proofs, p.head, p.signer, baseOpts); err != nil { + if err := txpool.ValidateTransaction(tx, p.head, p.signer, baseOpts); err != nil { return err } // Ensure the transaction adheres to the stateful pool filters (nonce, balance) @@ -1117,7 +1109,7 @@ func (p *BlobPool) Has(hash common.Hash) bool { } // Get returns a transaction if it is contained in the pool, or nil otherwise. -func (p *BlobPool) Get(hash common.Hash) *txpool.Transaction { +func (p *BlobPool) Get(hash common.Hash) *types.Transaction { // Track the amount of time waiting to retrieve a fully resolved blob tx from // the pool and the amount of time actually spent on pulling the data from disk. getStart := time.Now() @@ -1139,32 +1131,27 @@ func (p *BlobPool) Get(hash common.Hash) *txpool.Transaction { log.Error("Tracked blob transaction missing from store", "hash", hash, "id", id, "err", err) return nil } - item := new(blobTx) + item := new(types.Transaction) if err = rlp.DecodeBytes(data, item); err != nil { log.Error("Blobs corrupted for traced transaction", "hash", hash, "id", id, "err", err) return nil } - return &txpool.Transaction{ - Tx: item.Tx, - BlobTxBlobs: item.Blobs, - BlobTxCommits: item.Commits, - BlobTxProofs: item.Proofs, - } + return item } // Add inserts a set of blob transactions into the pool if they pass validation (both // consensus validity and pool restictions). -func (p *BlobPool) Add(txs []*txpool.Transaction, local bool, sync bool) []error { +func (p *BlobPool) Add(txs []*types.Transaction, local bool, sync bool) []error { errs := make([]error, len(txs)) for i, tx := range txs { - errs[i] = p.add(tx.Tx, tx.BlobTxBlobs, tx.BlobTxCommits, tx.BlobTxProofs) + errs[i] = p.add(tx) } return errs } // Add inserts a new blob transaction into the pool if it passes validation (both // consensus validity and pool restictions). -func (p *BlobPool) add(tx *types.Transaction, blobs []kzg4844.Blob, commits []kzg4844.Commitment, proofs []kzg4844.Proof) (err error) { +func (p *BlobPool) add(tx *types.Transaction) (err error) { // The blob pool blocks on adding a transaction. This is because blob txs are // only even pulled form the network, so this method will act as the overload // protection for fetches. @@ -1178,7 +1165,7 @@ func (p *BlobPool) add(tx *types.Transaction, blobs []kzg4844.Blob, commits []kz }(time.Now()) // Ensure the transaction is valid from all perspectives - if err := p.validateTx(tx, blobs, commits, proofs); err != nil { + if err := p.validateTx(tx); err != nil { log.Trace("Transaction validation failed", "hash", tx.Hash(), "err", err) return err } @@ -1203,7 +1190,7 @@ func (p *BlobPool) add(tx *types.Transaction, blobs []kzg4844.Blob, commits []kz } // Transaction permitted into the pool from a nonce and cost perspective, // insert it into the database and update the indices - blob, err := rlp.EncodeToBytes(&blobTx{Tx: tx, Blobs: blobs, Commits: commits, Proofs: proofs}) + blob, err := rlp.EncodeToBytes(tx) if err != nil { log.Error("Failed to encode transaction for storage", "hash", tx.Hash(), "err", err) return err diff --git a/core/txpool/blobpool/blobpool_test.go b/core/txpool/blobpool/blobpool_test.go index 78a5039b5b13..2af5ab43870c 100644 --- a/core/txpool/blobpool/blobpool_test.go +++ b/core/txpool/blobpool/blobpool_test.go @@ -193,8 +193,8 @@ func makeAddressReserver() txpool.AddressReserver { // with a valid key, only setting the interesting fields from the perspective of // the blob pool. func makeTx(nonce uint64, gasTipCap uint64, gasFeeCap uint64, blobFeeCap uint64, key *ecdsa.PrivateKey) *types.Transaction { - tx, _ := types.SignNewTx(key, types.LatestSigner(testChainConfig), makeUnsignedTx(nonce, gasTipCap, gasFeeCap, blobFeeCap)) - return tx + blobtx := makeUnsignedTx(nonce, gasTipCap, gasFeeCap, blobFeeCap) + return types.MustSignNewTx(key, types.LatestSigner(testChainConfig), blobtx) } // makeUnsignedTx is a utility method to construct a random blob tranasaction @@ -209,6 +209,11 @@ func makeUnsignedTx(nonce uint64, gasTipCap uint64, gasFeeCap uint64, blobFeeCap BlobFeeCap: uint256.NewInt(blobFeeCap), BlobHashes: []common.Hash{emptyBlobVHash}, Value: uint256.NewInt(100), + Sidecar: &types.BlobSidecar{ + Blobs: []kzg4844.Blob{emptyBlob}, + Commitments: []kzg4844.Commitment{emptyBlobCommit}, + Proofs: []kzg4844.Proof{emptyBlobProof}, + }, } } @@ -341,7 +346,7 @@ func TestOpenDrops(t *testing.T) { R: new(uint256.Int), S: new(uint256.Int), }) - blob, _ := rlp.EncodeToBytes(&blobTx{Tx: tx}) + blob, _ := rlp.EncodeToBytes(tx) badsig, _ := store.Put(blob) // Insert a sequence of transactions with a nonce gap in between to verify @@ -354,7 +359,7 @@ func TestOpenDrops(t *testing.T) { ) for _, nonce := range []uint64{0, 1, 3, 4, 6, 7} { // first gap at #2, another at #5 tx := makeTx(nonce, 1, 1, 1, gapper) - blob, _ := rlp.EncodeToBytes(&blobTx{Tx: tx}) + blob, _ := rlp.EncodeToBytes(tx) id, _ := store.Put(blob) if nonce < 2 { @@ -371,7 +376,7 @@ func TestOpenDrops(t *testing.T) { ) for _, nonce := range []uint64{1, 2, 3} { // first gap at #0, all set dangling tx := makeTx(nonce, 1, 1, 1, dangler) - blob, _ := rlp.EncodeToBytes(&blobTx{Tx: tx}) + blob, _ := rlp.EncodeToBytes(tx) id, _ := store.Put(blob) dangling[id] = struct{}{} @@ -384,7 +389,7 @@ func TestOpenDrops(t *testing.T) { ) for _, nonce := range []uint64{0, 1, 2} { // account nonce at 3, all set filled tx := makeTx(nonce, 1, 1, 1, filler) - blob, _ := rlp.EncodeToBytes(&blobTx{Tx: tx}) + blob, _ := rlp.EncodeToBytes(tx) id, _ := store.Put(blob) filled[id] = struct{}{} @@ -397,7 +402,7 @@ func TestOpenDrops(t *testing.T) { ) for _, nonce := range []uint64{0, 1, 2, 3} { // account nonce at 2, half filled tx := makeTx(nonce, 1, 1, 1, overlapper) - blob, _ := rlp.EncodeToBytes(&blobTx{Tx: tx}) + blob, _ := rlp.EncodeToBytes(tx) id, _ := store.Put(blob) if nonce >= 2 { @@ -419,7 +424,7 @@ func TestOpenDrops(t *testing.T) { } else { tx = makeTx(uint64(i), 1, 1, 1, underpayer) } - blob, _ := rlp.EncodeToBytes(&blobTx{Tx: tx}) + blob, _ := rlp.EncodeToBytes(tx) id, _ := store.Put(blob) underpaid[id] = struct{}{} @@ -438,7 +443,7 @@ func TestOpenDrops(t *testing.T) { } else { tx = makeTx(uint64(i), 1, 1, 1, outpricer) } - blob, _ := rlp.EncodeToBytes(&blobTx{Tx: tx}) + blob, _ := rlp.EncodeToBytes(tx) id, _ := store.Put(blob) if i < 2 { @@ -460,7 +465,7 @@ func TestOpenDrops(t *testing.T) { } else { tx = makeTx(nonce, 1, 1, 1, exceeder) } - blob, _ := rlp.EncodeToBytes(&blobTx{Tx: tx}) + blob, _ := rlp.EncodeToBytes(tx) id, _ := store.Put(blob) exceeded[id] = struct{}{} @@ -478,7 +483,7 @@ func TestOpenDrops(t *testing.T) { } else { tx = makeTx(nonce, 1, 1, 1, overdrafter) } - blob, _ := rlp.EncodeToBytes(&blobTx{Tx: tx}) + blob, _ := rlp.EncodeToBytes(tx) id, _ := store.Put(blob) if nonce < 1 { @@ -494,7 +499,7 @@ func TestOpenDrops(t *testing.T) { overcapped = make(map[uint64]struct{}) ) for nonce := uint64(0); nonce < maxTxsPerAccount+3; nonce++ { - blob, _ := rlp.EncodeToBytes(&blobTx{Tx: makeTx(nonce, 1, 1, 1, overcapper)}) + blob, _ := rlp.EncodeToBytes(makeTx(nonce, 1, 1, 1, overcapper)) id, _ := store.Put(blob) if nonce < maxTxsPerAccount { @@ -625,7 +630,7 @@ func TestOpenIndex(t *testing.T) { ) for _, i := range []int{5, 3, 4, 2, 0, 1} { // Randomize the tx insertion order to force sorting on load tx := makeTx(uint64(i), txExecTipCaps[i], txExecFeeCaps[i], txBlobFeeCaps[i], key) - blob, _ := rlp.EncodeToBytes(&blobTx{Tx: tx}) + blob, _ := rlp.EncodeToBytes(tx) store.Put(blob) } store.Close() @@ -718,9 +723,9 @@ func TestOpenHeap(t *testing.T) { tx2 = makeTx(0, 1, 800, 70, key2) tx3 = makeTx(0, 1, 1500, 110, key3) - blob1, _ = rlp.EncodeToBytes(&blobTx{Tx: tx1}) - blob2, _ = rlp.EncodeToBytes(&blobTx{Tx: tx2}) - blob3, _ = rlp.EncodeToBytes(&blobTx{Tx: tx3}) + blob1, _ = rlp.EncodeToBytes(tx1) + blob2, _ = rlp.EncodeToBytes(tx2) + blob3, _ = rlp.EncodeToBytes(tx3) heapOrder = []common.Address{addr2, addr1, addr3} heapIndex = map[common.Address]int{addr2: 0, addr1: 1, addr3: 2} @@ -794,9 +799,9 @@ func TestOpenCap(t *testing.T) { tx2 = makeTx(0, 1, 800, 70, key2) tx3 = makeTx(0, 1, 1500, 110, key3) - blob1, _ = rlp.EncodeToBytes(&blobTx{Tx: tx1, Blobs: []kzg4844.Blob{emptyBlob}, Commits: []kzg4844.Commitment{emptyBlobCommit}, Proofs: []kzg4844.Proof{emptyBlobProof}}) - blob2, _ = rlp.EncodeToBytes(&blobTx{Tx: tx2, Blobs: []kzg4844.Blob{emptyBlob}, Commits: []kzg4844.Commitment{emptyBlobCommit}, Proofs: []kzg4844.Proof{emptyBlobProof}}) - blob3, _ = rlp.EncodeToBytes(&blobTx{Tx: tx3, Blobs: []kzg4844.Blob{emptyBlob}, Commits: []kzg4844.Commitment{emptyBlobCommit}, Proofs: []kzg4844.Proof{emptyBlobProof}}) + blob1, _ = rlp.EncodeToBytes(tx1) + blob2, _ = rlp.EncodeToBytes(tx2) + blob3, _ = rlp.EncodeToBytes(tx3) keep = []common.Address{addr1, addr3} drop = []common.Address{addr2} @@ -1210,10 +1215,8 @@ func TestAdd(t *testing.T) { // Sign the seed transactions and store them in the data store for _, tx := range seed.txs { - var ( - signed, _ = types.SignNewTx(keys[acc], types.LatestSigner(testChainConfig), tx) - blob, _ = rlp.EncodeToBytes(&blobTx{Tx: signed, Blobs: []kzg4844.Blob{emptyBlob}, Commits: []kzg4844.Commitment{emptyBlobCommit}, Proofs: []kzg4844.Proof{emptyBlobProof}}) - ) + signed := types.MustSignNewTx(keys[acc], types.LatestSigner(testChainConfig), tx) + blob, _ := rlp.EncodeToBytes(signed) store.Put(blob) } } @@ -1236,7 +1239,7 @@ func TestAdd(t *testing.T) { // Add each transaction one by one, verifying the pool internals in between for j, add := range tt.adds { signed, _ := types.SignNewTx(keys[add.from], types.LatestSigner(testChainConfig), add.tx) - if err := pool.add(signed, []kzg4844.Blob{emptyBlob}, []kzg4844.Commitment{emptyBlobCommit}, []kzg4844.Proof{emptyBlobProof}); !errors.Is(err, add.err) { + if err := pool.add(signed); !errors.Is(err, add.err) { t.Errorf("test %d, tx %d: adding transaction error mismatch: have %v, want %v", i, j, err, add.err) } verifyPoolInternals(t, pool) diff --git a/core/txpool/blobpool/limbo.go b/core/txpool/blobpool/limbo.go index 4cb5042c2bb5..dbcf0d76e6cc 100644 --- a/core/txpool/blobpool/limbo.go +++ b/core/txpool/blobpool/limbo.go @@ -21,7 +21,6 @@ import ( "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core/types" - "github.com/ethereum/go-ethereum/crypto/kzg4844" "github.com/ethereum/go-ethereum/log" "github.com/ethereum/go-ethereum/rlp" "github.com/holiman/billy" @@ -33,10 +32,7 @@ import ( type limboBlob struct { Owner common.Hash // Owner transaction's hash to support resurrecting reorged txs Block uint64 // Block in which the blob transaction was included - - Blobs []kzg4844.Blob // The opaque blobs originally part of the transaction - Commits []kzg4844.Commitment // The commitments for the original blobs - Proofs []kzg4844.Proof // The proofs verifying the commitments + Tx *types.Transaction } // limbo is a light, indexed database to temporarily store recently included @@ -139,14 +135,14 @@ func (l *limbo) finalize(final *types.Header) { // push stores a new blob transaction into the limbo, waiting until finality for // it to be automatically evicted. -func (l *limbo) push(tx common.Hash, block uint64, blobs []kzg4844.Blob, commits []kzg4844.Commitment, proofs []kzg4844.Proof) error { +func (l *limbo) push(txhash common.Hash, block uint64, tx *types.Transaction) error { // If the blobs are already tracked by the limbo, consider it a programming // error. There's not much to do against it, but be loud. - if _, ok := l.index[tx]; ok { + if _, ok := l.index[txhash]; ok { log.Error("Limbo cannot push already tracked blobs", "tx", tx) return errors.New("already tracked blob transaction") } - if err := l.setAndIndex(tx, block, blobs, commits, proofs); err != nil { + if err := l.setAndIndex(txhash, block, tx); err != nil { log.Error("Failed to set and index liboed blobs", "tx", tx, "err", err) return err } @@ -156,21 +152,21 @@ func (l *limbo) push(tx common.Hash, block uint64, blobs []kzg4844.Blob, commits // pull retrieves a previously pushed set of blobs back from the limbo, removing // it at the same time. This method should be used when a previously included blob // transaction gets reorged out. -func (l *limbo) pull(tx common.Hash) ([]kzg4844.Blob, []kzg4844.Commitment, []kzg4844.Proof, error) { +func (l *limbo) pull(tx common.Hash) (*types.Transaction, error) { // If the blobs are not tracked by the limbo, there's not much to do. This // can happen for example if a blob transaction is mined without pushing it // into the network first. id, ok := l.index[tx] if !ok { log.Trace("Limbo cannot pull non-tracked blobs", "tx", tx) - return nil, nil, nil, errors.New("unseen blob transaction") + return nil, errors.New("unseen blob transaction") } item, err := l.getAndDrop(id) if err != nil { log.Error("Failed to get and drop limboed blobs", "tx", tx, "id", id, "err", err) - return nil, nil, nil, err + return nil, err } - return item.Blobs, item.Commits, item.Proofs, nil + return item.Tx, nil } // update changes the block number under which a blob transaction is tracked. This @@ -202,7 +198,7 @@ func (l *limbo) update(tx common.Hash, block uint64) { log.Error("Failed to get and drop limboed blobs", "tx", tx, "id", id, "err", err) return } - if err := l.setAndIndex(tx, block, item.Blobs, item.Commits, item.Proofs); err != nil { + if err := l.setAndIndex(tx, block, item.Tx); err != nil { log.Error("Failed to set and index limboed blobs", "tx", tx, "err", err) return } @@ -233,13 +229,11 @@ func (l *limbo) getAndDrop(id uint64) (*limboBlob, error) { // setAndIndex assembles a limbo blob database entry and stores it, also updating // the in-memory indices. -func (l *limbo) setAndIndex(tx common.Hash, block uint64, blobs []kzg4844.Blob, commits []kzg4844.Commitment, proofs []kzg4844.Proof) error { +func (l *limbo) setAndIndex(txhash common.Hash, block uint64, tx *types.Transaction) error { item := &limboBlob{ - Owner: tx, - Block: block, - Blobs: blobs, - Commits: commits, - Proofs: proofs, + Owner: txhash, + Block: block, + Tx: tx, } data, err := rlp.EncodeToBytes(item) if err != nil { @@ -249,10 +243,10 @@ func (l *limbo) setAndIndex(tx common.Hash, block uint64, blobs []kzg4844.Blob, if err != nil { return err } - l.index[tx] = id + l.index[txhash] = id if _, ok := l.groups[block]; !ok { l.groups[block] = make(map[uint64]common.Hash) } - l.groups[block][id] = tx + l.groups[block][id] = txhash return nil } diff --git a/core/txpool/legacypool/legacypool.go b/core/txpool/legacypool/legacypool.go index b1ae8bac8140..00e326c4b87f 100644 --- a/core/txpool/legacypool/legacypool.go +++ b/core/txpool/legacypool/legacypool.go @@ -535,7 +535,7 @@ func (pool *LegacyPool) Pending(enforceTips bool) map[common.Address][]*txpool.L lazies[i] = &txpool.LazyTransaction{ Pool: pool, Hash: txs[i].Hash(), - Tx: &txpool.Transaction{Tx: txs[i]}, + Tx: txs[i], Time: txs[i].Time(), GasFeeCap: txs[i].GasFeeCap(), GasTipCap: txs[i].GasTipCap(), @@ -588,7 +588,7 @@ func (pool *LegacyPool) validateTxBasics(tx *types.Transaction, local bool) erro if local { opts.MinTip = new(big.Int) } - if err := txpool.ValidateTransaction(tx, nil, nil, nil, pool.currentHead.Load(), pool.signer, opts); err != nil { + if err := txpool.ValidateTransaction(tx, pool.currentHead.Load(), pool.signer, opts); err != nil { return err } return nil @@ -900,26 +900,13 @@ func (pool *LegacyPool) promoteTx(addr common.Address, hash common.Hash, tx *typ return true } -// Add enqueues a batch of transactions into the pool if they are valid. Depending -// on the local flag, full pricing contraints will or will not be applied. -// -// If sync is set, the method will block until all internal maintenance related -// to the add is finished. Only use this during tests for determinism! -func (pool *LegacyPool) Add(txs []*txpool.Transaction, local bool, sync bool) []error { - unwrapped := make([]*types.Transaction, len(txs)) - for i, tx := range txs { - unwrapped[i] = tx.Tx - } - return pool.addTxs(unwrapped, local, sync) -} - // addLocals enqueues a batch of transactions into the pool if they are valid, marking the // senders as a local ones, ensuring they go around the local pricing constraints. // // This method is used to add transactions from the RPC API and performs synchronous pool // reorganization and event propagation. func (pool *LegacyPool) addLocals(txs []*types.Transaction) []error { - return pool.addTxs(txs, !pool.config.NoLocals, true) + return pool.Add(txs, !pool.config.NoLocals, true) } // addLocal enqueues a single local transaction into the pool if it is valid. This is @@ -935,7 +922,7 @@ func (pool *LegacyPool) addLocal(tx *types.Transaction) error { // This method is used to add transactions from the p2p network and does not wait for pool // reorganization and internal event propagation. func (pool *LegacyPool) addRemotes(txs []*types.Transaction) []error { - return pool.addTxs(txs, false, false) + return pool.Add(txs, false, false) } // addRemote enqueues a single transaction into the pool if it is valid. This is a convenience @@ -947,16 +934,20 @@ func (pool *LegacyPool) addRemote(tx *types.Transaction) error { // addRemotesSync is like addRemotes, but waits for pool reorganization. Tests use this method. func (pool *LegacyPool) addRemotesSync(txs []*types.Transaction) []error { - return pool.addTxs(txs, false, true) + return pool.Add(txs, false, true) } // This is like addRemotes with a single transaction, but waits for pool reorganization. Tests use this method. func (pool *LegacyPool) addRemoteSync(tx *types.Transaction) error { - return pool.addTxs([]*types.Transaction{tx}, false, true)[0] + return pool.Add([]*types.Transaction{tx}, false, true)[0] } -// addTxs attempts to queue a batch of transactions if they are valid. -func (pool *LegacyPool) addTxs(txs []*types.Transaction, local, sync bool) []error { +// Add enqueues a batch of transactions into the pool if they are valid. Depending +// on the local flag, full pricing contraints will or will not be applied. +// +// If sync is set, the method will block until all internal maintenance related +// to the add is finished. Only use this during tests for determinism! +func (pool *LegacyPool) Add(txs []*types.Transaction, local, sync bool) []error { // Filter out known ones without obtaining the pool lock or recovering signatures var ( errs = make([]error, len(txs)) @@ -1042,12 +1033,12 @@ func (pool *LegacyPool) Status(hash common.Hash) txpool.TxStatus { } // Get returns a transaction if it is contained in the pool and nil otherwise. -func (pool *LegacyPool) Get(hash common.Hash) *txpool.Transaction { +func (pool *LegacyPool) Get(hash common.Hash) *types.Transaction { tx := pool.get(hash) if tx == nil { return nil } - return &txpool.Transaction{Tx: tx} + return tx } // get returns a transaction if it is contained in the pool and nil otherwise. diff --git a/core/txpool/subpool.go b/core/txpool/subpool.go index 70c0918e140c..85312c431807 100644 --- a/core/txpool/subpool.go +++ b/core/txpool/subpool.go @@ -23,27 +23,16 @@ import ( "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core" "github.com/ethereum/go-ethereum/core/types" - "github.com/ethereum/go-ethereum/crypto/kzg4844" "github.com/ethereum/go-ethereum/event" ) -// Transaction is a helper struct to group together a canonical transaction with -// satellite data items that are needed by the pool but are not part of the chain. -type Transaction struct { - Tx *types.Transaction // Canonical transaction - - BlobTxBlobs []kzg4844.Blob // Blobs needed by the blob pool - BlobTxCommits []kzg4844.Commitment // Commitments needed by the blob pool - BlobTxProofs []kzg4844.Proof // Proofs needed by the blob pool -} - // LazyTransaction contains a small subset of the transaction properties that is // enough for the miner and other APIs to handle large batches of transactions; // and supports pulling up the entire transaction when really needed. type LazyTransaction struct { - Pool SubPool // Transaction subpool to pull the real transaction up - Hash common.Hash // Transaction hash to pull up if needed - Tx *Transaction // Transaction if already resolved + Pool SubPool // Transaction subpool to pull the real transaction up + Hash common.Hash // Transaction hash to pull up if needed + Tx *types.Transaction // Transaction if already resolved Time time.Time // Time when the transaction was first seen GasFeeCap *big.Int // Maximum fee per gas the transaction may consume @@ -52,7 +41,7 @@ type LazyTransaction struct { // Resolve retrieves the full transaction belonging to a lazy handle if it is still // maintained by the transaction pool. -func (ltx *LazyTransaction) Resolve() *Transaction { +func (ltx *LazyTransaction) Resolve() *types.Transaction { if ltx.Tx == nil { ltx.Tx = ltx.Pool.Get(ltx.Hash) } @@ -99,12 +88,12 @@ type SubPool interface { Has(hash common.Hash) bool // Get returns a transaction if it is contained in the pool, or nil otherwise. - Get(hash common.Hash) *Transaction + Get(hash common.Hash) *types.Transaction // Add enqueues a batch of transactions into the pool if they are valid. Due // to the large transaction churn, add may postpone fully integrating the tx // to a later point to batch multiple ones together. - Add(txs []*Transaction, local bool, sync bool) []error + Add(txs []*types.Transaction, local bool, sync bool) []error // Pending retrieves all currently processable transactions, grouped by origin // account and sorted by nonce. diff --git a/core/txpool/txpool.go b/core/txpool/txpool.go index b0e91fee6c44..e40b41405454 100644 --- a/core/txpool/txpool.go +++ b/core/txpool/txpool.go @@ -249,7 +249,7 @@ func (p *TxPool) Has(hash common.Hash) bool { } // Get returns a transaction if it is contained in the pool, or nil otherwise. -func (p *TxPool) Get(hash common.Hash) *Transaction { +func (p *TxPool) Get(hash common.Hash) *types.Transaction { for _, subpool := range p.subpools { if tx := subpool.Get(hash); tx != nil { return tx @@ -261,14 +261,14 @@ func (p *TxPool) Get(hash common.Hash) *Transaction { // Add enqueues a batch of transactions into the pool if they are valid. Due // to the large transaction churn, add may postpone fully integrating the tx // to a later point to batch multiple ones together. -func (p *TxPool) Add(txs []*Transaction, local bool, sync bool) []error { +func (p *TxPool) Add(txs []*types.Transaction, local bool, sync bool) []error { // Split the input transactions between the subpools. It shouldn't really // happen that we receive merged batches, but better graceful than strange // errors. // // We also need to track how the transactions were split across the subpools, // so we can piece back the returned errors into the original order. - txsets := make([][]*Transaction, len(p.subpools)) + txsets := make([][]*types.Transaction, len(p.subpools)) splits := make([]int, len(txs)) for i, tx := range txs { @@ -277,7 +277,7 @@ func (p *TxPool) Add(txs []*Transaction, local bool, sync bool) []error { // Try to find a subpool that accepts the transaction for j, subpool := range p.subpools { - if subpool.Filter(tx.Tx) { + if subpool.Filter(tx) { txsets[j] = append(txsets[j], tx) splits[i] = j break diff --git a/core/txpool/validation.go b/core/txpool/validation.go index e1c0f314c789..722411b8a2a8 100644 --- a/core/txpool/validation.go +++ b/core/txpool/validation.go @@ -46,7 +46,7 @@ type ValidationOptions struct { // // This check is public to allow different transaction pools to check the basic // rules without duplicating code and running the risk of missed updates. -func ValidateTransaction(tx *types.Transaction, blobs []kzg4844.Blob, commits []kzg4844.Commitment, proofs []kzg4844.Proof, head *types.Header, signer types.Signer, opts *ValidationOptions) error { +func ValidateTransaction(tx *types.Transaction, head *types.Header, signer types.Signer, opts *ValidationOptions) error { // Ensure transactions not implemented by the calling pool are rejected if opts.Accept&(1< params.BlobTxMaxBlobGasPerBlock/params.BlobTxBlobGasPerBlob { return fmt.Errorf("too many blobs in transaction: have %d, permitted %d", len(hashes), params.BlobTxMaxBlobGasPerBlock/params.BlobTxBlobGasPerBlob) } - if len(blobs) != len(hashes) { - return fmt.Errorf("invalid number of %d blobs compared to %d blob hashes", len(blobs), len(hashes)) - } - if len(commits) != len(hashes) { - return fmt.Errorf("invalid number of %d blob commitments compared to %d blob hashes", len(commits), len(hashes)) + if err := validateBlobSidecar(hashes, sidecar); err != nil { + return err } - if len(proofs) != len(hashes) { - return fmt.Errorf("invalid number of %d blob proofs compared to %d blob hashes", len(proofs), len(hashes)) - } - // Blob quantities match up, validate that the provers match with the - // transaction hash before getting to the cryptography - hasher := sha256.New() - for i, want := range hashes { - hasher.Write(commits[i][:]) - hash := hasher.Sum(nil) - hasher.Reset() + } + return nil +} - var vhash common.Hash - vhash[0] = params.BlobTxHashVersion - copy(vhash[1:], hash[1:]) +func validateBlobSidecar(hashes []common.Hash, sidecar *types.BlobSidecar) error { + if len(sidecar.Blobs) != len(hashes) { + return fmt.Errorf("invalid number of %d blobs compared to %d blob hashes", len(sidecar.Blobs), len(hashes)) + } + if len(sidecar.Commitments) != len(hashes) { + return fmt.Errorf("invalid number of %d blob commitments compared to %d blob hashes", len(sidecar.Commitments), len(hashes)) + } + if len(sidecar.Proofs) != len(hashes) { + return fmt.Errorf("invalid number of %d blob proofs compared to %d blob hashes", len(sidecar.Proofs), len(hashes)) + } + // Blob quantities match up, validate that the provers match with the + // transaction hash before getting to the cryptography + hasher := sha256.New() + for i, want := range hashes { + hasher.Write(sidecar.Commitments[i][:]) + hash := hasher.Sum(nil) + hasher.Reset() - if vhash != want { - return fmt.Errorf("blob %d: computed hash %#x mismatches transaction one %#x", i, vhash, want) - } + var vhash common.Hash + vhash[0] = params.BlobTxHashVersion + copy(vhash[1:], hash[1:]) + + if vhash != want { + return fmt.Errorf("blob %d: computed hash %#x mismatches transaction one %#x", i, vhash, want) } - // Blob commitments match with the hashes in the transaction, verify the - // blobs themselves via KZG - for i := range blobs { - if err := kzg4844.VerifyBlobProof(blobs[i], commits[i], proofs[i]); err != nil { - return fmt.Errorf("invalid blob %d: %v", i, err) - } + } + // Blob commitments match with the hashes in the transaction, verify the + // blobs themselves via KZG + for i := range sidecar.Blobs { + if err := kzg4844.VerifyBlobProof(sidecar.Blobs[i], sidecar.Commitments[i], sidecar.Proofs[i]); err != nil { + return fmt.Errorf("invalid blob %d: %v", i, err) } } return nil From bd582b54ade38e583a9dacb486a014659bf84c98 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Thu, 3 Aug 2023 10:25:41 +0200 Subject: [PATCH 05/22] core: fix comment --- core/block_validator.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/block_validator.go b/core/block_validator.go index c3c031b4ba9d..7fd848a73f65 100644 --- a/core/block_validator.go +++ b/core/block_validator.go @@ -89,7 +89,7 @@ func (v *BlockValidator) ValidateBody(block *types.Block) error { // Count the number of blobs to validate against the header's blobGasUsed blobs += len(tx.BlobHashes()) - // If the tx is a blob tx, it must have a sidecar attached to be valid in a block. + // If the tx is a blob tx, it must NOT have a sidecar attached to be valid in a block. if tx.Type() == types.BlobTxType && tx.BlobSidecar() != nil { return fmt.Errorf("unexpected blob sidecar in transaction at index %d", i) } From 014d70af6016815d70acbce45c94c6528ab0cea7 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Thu, 3 Aug 2023 10:26:47 +0200 Subject: [PATCH 06/22] core/types: remove some receiver names in TxData --- core/types/transaction.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/types/transaction.go b/core/types/transaction.go index da37d1a8c4f5..67f49a90e05e 100644 --- a/core/types/transaction.go +++ b/core/types/transaction.go @@ -94,8 +94,8 @@ type TxData interface { // Method implementations can use 'dst' to store the result. effectiveGasPrice(dst *big.Int, baseFee *big.Int) *big.Int - encode(b *bytes.Buffer) error - decode(b []byte) error + encode(*bytes.Buffer) error + decode([]byte) error } // EncodeRLP implements rlp.Encoder From 394ef8b4bb7b548696564bd6f39c2bf570f721d4 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Thu, 3 Aug 2023 12:37:26 +0200 Subject: [PATCH 07/22] all: removing txpool.Transaction --- eth/api_backend.go | 9 +++---- eth/catalyst/api_test.go | 23 +++++----------- eth/fetcher/tx_fetcher.go | 16 +++++------ eth/fetcher/tx_fetcher_test.go | 30 ++++++++++----------- eth/handler.go | 6 ++--- eth/handler_eth_test.go | 11 +++----- eth/handler_test.go | 23 +++++----------- eth/protocols/eth/broadcast.go | 8 +++--- eth/protocols/eth/handler.go | 4 +-- eth/protocols/eth/handlers.go | 2 +- les/server_requests.go | 2 +- miner/ordering_test.go | 8 +++--- miner/stress/clique/main.go | 3 +-- miner/worker.go | 22 +++++++-------- miner/worker_test.go | 8 +++--- tests/fuzzers/txfetcher/txfetcher_fuzzer.go | 3 +-- 16 files changed, 74 insertions(+), 104 deletions(-) diff --git a/eth/api_backend.go b/eth/api_backend.go index 30e249368410..1d8310476a63 100644 --- a/eth/api_backend.go +++ b/eth/api_backend.go @@ -294,7 +294,7 @@ func (b *EthAPIBackend) SubscribeLogsEvent(ch chan<- []*types.Log) event.Subscri } func (b *EthAPIBackend) SendTx(ctx context.Context, signedTx *types.Transaction) error { - return b.eth.txPool.Add([]*txpool.Transaction{{Tx: signedTx}}, true, false)[0] + return b.eth.txPool.Add([]*types.Transaction{signedTx}, true, false)[0] } func (b *EthAPIBackend) GetPoolTransactions() (types.Transactions, error) { @@ -303,7 +303,7 @@ func (b *EthAPIBackend) GetPoolTransactions() (types.Transactions, error) { for _, batch := range pending { for _, lazy := range batch { if tx := lazy.Resolve(); tx != nil { - txs = append(txs, tx.Tx) + txs = append(txs, tx) } } } @@ -311,10 +311,7 @@ func (b *EthAPIBackend) GetPoolTransactions() (types.Transactions, error) { } func (b *EthAPIBackend) GetPoolTransaction(hash common.Hash) *types.Transaction { - if tx := b.eth.txPool.Get(hash); tx != nil { - return tx.Tx - } - return nil + return b.eth.txPool.Get(hash) } func (b *EthAPIBackend) GetTransaction(ctx context.Context, txHash common.Hash) (*types.Transaction, common.Hash, uint64, uint64, error) { diff --git a/eth/catalyst/api_test.go b/eth/catalyst/api_test.go index 05ad3def48ae..c26fbd79fc87 100644 --- a/eth/catalyst/api_test.go +++ b/eth/catalyst/api_test.go @@ -35,7 +35,6 @@ import ( beaconConsensus "github.com/ethereum/go-ethereum/consensus/beacon" "github.com/ethereum/go-ethereum/consensus/ethash" "github.com/ethereum/go-ethereum/core" - "github.com/ethereum/go-ethereum/core/txpool" "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/crypto" "github.com/ethereum/go-ethereum/crypto/kzg4844" @@ -108,7 +107,7 @@ func TestEth2AssembleBlock(t *testing.T) { if err != nil { t.Fatalf("error signing transaction, err=%v", err) } - ethservice.TxPool().Add([]*txpool.Transaction{{Tx: tx}}, true, false) + ethservice.TxPool().Add([]*types.Transaction{tx}, true, false) blockParams := engine.PayloadAttributes{ Timestamp: blocks[9].Time() + 5, } @@ -145,11 +144,7 @@ func TestEth2AssembleBlockWithAnotherBlocksTxs(t *testing.T) { // Put the 10th block's tx in the pool and produce a new block txs := blocks[9].Transactions() - wrapped := make([]*txpool.Transaction, len(txs)) - for i, tx := range txs { - wrapped[i] = &txpool.Transaction{Tx: tx} - } - api.eth.TxPool().Add(wrapped, false, true) + api.eth.TxPool().Add(txs, false, true) blockParams := engine.PayloadAttributes{ Timestamp: blocks[8].Time() + 5, } @@ -189,11 +184,7 @@ func TestEth2PrepareAndGetPayload(t *testing.T) { // Put the 10th block's tx in the pool and produce a new block txs := blocks[9].Transactions() - wrapped := make([]*txpool.Transaction, len(txs)) - for i, tx := range txs { - wrapped[i] = &txpool.Transaction{Tx: tx} - } - ethservice.TxPool().Add(wrapped, true, false) + ethservice.TxPool().Add(txs, true, false) blockParams := engine.PayloadAttributes{ Timestamp: blocks[8].Time() + 5, } @@ -315,7 +306,7 @@ func TestEth2NewBlock(t *testing.T) { statedb, _ := ethservice.BlockChain().StateAt(parent.Root()) nonce := statedb.GetNonce(testAddr) tx, _ := types.SignTx(types.NewContractCreation(nonce, new(big.Int), 1000000, big.NewInt(2*params.InitialBaseFee), logCode), types.LatestSigner(ethservice.BlockChain().Config()), testKey) - ethservice.TxPool().Add([]*txpool.Transaction{{Tx: tx}}, true, false) + ethservice.TxPool().Add([]*types.Transaction{tx}, true, false) execData, err := assembleWithTransactions(api, parent.Hash(), &engine.PayloadAttributes{ Timestamp: parent.Time() + 5, @@ -484,7 +475,7 @@ func TestFullAPI(t *testing.T) { statedb, _ := ethservice.BlockChain().StateAt(parent.Root) nonce := statedb.GetNonce(testAddr) tx, _ := types.SignTx(types.NewContractCreation(nonce, new(big.Int), 1000000, big.NewInt(2*params.InitialBaseFee), logCode), types.LatestSigner(ethservice.BlockChain().Config()), testKey) - ethservice.TxPool().Add([]*txpool.Transaction{{Tx: tx}}, true, false) + ethservice.TxPool().Add([]*types.Transaction{tx}, true, false) } setupBlocks(t, ethservice, 10, parent, callback, nil) @@ -610,7 +601,7 @@ func TestNewPayloadOnInvalidChain(t *testing.T) { GasPrice: big.NewInt(2 * params.InitialBaseFee), Data: logCode, }) - ethservice.TxPool().Add([]*txpool.Transaction{{Tx: tx}}, false, true) + ethservice.TxPool().Add([]*types.Transaction{tx}, false, true) var ( params = engine.PayloadAttributes{ Timestamp: parent.Time + 1, @@ -1284,7 +1275,7 @@ func setupBodies(t *testing.T) (*node.Node, *eth.Ethereum, []*types.Block) { statedb, _ := ethservice.BlockChain().StateAt(parent.Root) nonce := statedb.GetNonce(testAddr) tx, _ := types.SignTx(types.NewContractCreation(nonce, new(big.Int), 1000000, big.NewInt(2*params.InitialBaseFee), logCode), types.LatestSigner(ethservice.BlockChain().Config()), testKey) - ethservice.TxPool().Add([]*txpool.Transaction{{Tx: tx}}, true, false) + ethservice.TxPool().Add([]*types.Transaction{tx}, false, false) } withdrawals := make([][]*types.Withdrawal, 10) diff --git a/eth/fetcher/tx_fetcher.go b/eth/fetcher/tx_fetcher.go index 63872734035c..95fef0cdeee5 100644 --- a/eth/fetcher/tx_fetcher.go +++ b/eth/fetcher/tx_fetcher.go @@ -169,9 +169,9 @@ type TxFetcher struct { alternates map[common.Hash]map[string]struct{} // In-flight transaction alternate origins if retrieval fails // Callbacks - hasTx func(common.Hash) bool // Retrieves a tx from the local txpool - addTxs func([]*txpool.Transaction) []error // Insert a batch of transactions into local txpool - fetchTxs func(string, []common.Hash) error // Retrieves a set of txs from a remote peer + hasTx func(common.Hash) bool // Retrieves a tx from the local txpool + addTxs func([]*types.Transaction) []error // Insert a batch of transactions into local txpool + fetchTxs func(string, []common.Hash) error // Retrieves a set of txs from a remote peer step chan struct{} // Notification channel when the fetcher loop iterates clock mclock.Clock // Time wrapper to simulate in tests @@ -180,14 +180,14 @@ type TxFetcher struct { // NewTxFetcher creates a transaction fetcher to retrieve transaction // based on hash announcements. -func NewTxFetcher(hasTx func(common.Hash) bool, addTxs func([]*txpool.Transaction) []error, fetchTxs func(string, []common.Hash) error) *TxFetcher { +func NewTxFetcher(hasTx func(common.Hash) bool, addTxs func([]*types.Transaction) []error, fetchTxs func(string, []common.Hash) error) *TxFetcher { return NewTxFetcherForTests(hasTx, addTxs, fetchTxs, mclock.System{}, nil) } // NewTxFetcherForTests is a testing method to mock out the realtime clock with // a simulated version and the internal randomness with a deterministic one. func NewTxFetcherForTests( - hasTx func(common.Hash) bool, addTxs func([]*txpool.Transaction) []error, fetchTxs func(string, []common.Hash) error, + hasTx func(common.Hash) bool, addTxs func([]*types.Transaction) []error, fetchTxs func(string, []common.Hash) error, clock mclock.Clock, rand *mrand.Rand) *TxFetcher { return &TxFetcher{ notify: make(chan *txAnnounce), @@ -295,11 +295,7 @@ func (f *TxFetcher) Enqueue(peer string, txs []*types.Transaction, direct bool) ) batch := txs[i:end] - wrapped := make([]*txpool.Transaction, len(batch)) - for j, tx := range batch { - wrapped[j] = &txpool.Transaction{Tx: tx} - } - for j, err := range f.addTxs(wrapped) { + for j, err := range f.addTxs(batch) { // Track the transaction hash if the price is too low for us. // Avoid re-request this transaction when we receive another // announcement. diff --git a/eth/fetcher/tx_fetcher_test.go b/eth/fetcher/tx_fetcher_test.go index c5805d6ef388..1715def99c00 100644 --- a/eth/fetcher/tx_fetcher_test.go +++ b/eth/fetcher/tx_fetcher_test.go @@ -378,7 +378,7 @@ func TestTransactionFetcherCleanup(t *testing.T) { init: func() *TxFetcher { return NewTxFetcher( func(common.Hash) bool { return false }, - func(txs []*txpool.Transaction) []error { + func(txs []*types.Transaction) []error { return make([]error, len(txs)) }, func(string, []common.Hash) error { return nil }, @@ -417,7 +417,7 @@ func TestTransactionFetcherCleanupEmpty(t *testing.T) { init: func() *TxFetcher { return NewTxFetcher( func(common.Hash) bool { return false }, - func(txs []*txpool.Transaction) []error { + func(txs []*types.Transaction) []error { return make([]error, len(txs)) }, func(string, []common.Hash) error { return nil }, @@ -455,7 +455,7 @@ func TestTransactionFetcherMissingRescheduling(t *testing.T) { init: func() *TxFetcher { return NewTxFetcher( func(common.Hash) bool { return false }, - func(txs []*txpool.Transaction) []error { + func(txs []*types.Transaction) []error { return make([]error, len(txs)) }, func(string, []common.Hash) error { return nil }, @@ -501,7 +501,7 @@ func TestTransactionFetcherMissingCleanup(t *testing.T) { init: func() *TxFetcher { return NewTxFetcher( func(common.Hash) bool { return false }, - func(txs []*txpool.Transaction) []error { + func(txs []*types.Transaction) []error { return make([]error, len(txs)) }, func(string, []common.Hash) error { return nil }, @@ -539,7 +539,7 @@ func TestTransactionFetcherBroadcasts(t *testing.T) { init: func() *TxFetcher { return NewTxFetcher( func(common.Hash) bool { return false }, - func(txs []*txpool.Transaction) []error { + func(txs []*types.Transaction) []error { return make([]error, len(txs)) }, func(string, []common.Hash) error { return nil }, @@ -644,7 +644,7 @@ func TestTransactionFetcherTimeoutRescheduling(t *testing.T) { init: func() *TxFetcher { return NewTxFetcher( func(common.Hash) bool { return false }, - func(txs []*txpool.Transaction) []error { + func(txs []*types.Transaction) []error { return make([]error, len(txs)) }, func(string, []common.Hash) error { return nil }, @@ -865,7 +865,7 @@ func TestTransactionFetcherUnderpricedDedup(t *testing.T) { init: func() *TxFetcher { return NewTxFetcher( func(common.Hash) bool { return false }, - func(txs []*txpool.Transaction) []error { + func(txs []*types.Transaction) []error { errs := make([]error, len(txs)) for i := 0; i < len(errs); i++ { if i%2 == 0 { @@ -938,7 +938,7 @@ func TestTransactionFetcherUnderpricedDoSProtection(t *testing.T) { init: func() *TxFetcher { return NewTxFetcher( func(common.Hash) bool { return false }, - func(txs []*txpool.Transaction) []error { + func(txs []*types.Transaction) []error { errs := make([]error, len(txs)) for i := 0; i < len(errs); i++ { errs[i] = txpool.ErrUnderpriced @@ -964,7 +964,7 @@ func TestTransactionFetcherOutOfBoundDeliveries(t *testing.T) { init: func() *TxFetcher { return NewTxFetcher( func(common.Hash) bool { return false }, - func(txs []*txpool.Transaction) []error { + func(txs []*types.Transaction) []error { return make([]error, len(txs)) }, func(string, []common.Hash) error { return nil }, @@ -1017,7 +1017,7 @@ func TestTransactionFetcherDrop(t *testing.T) { init: func() *TxFetcher { return NewTxFetcher( func(common.Hash) bool { return false }, - func(txs []*txpool.Transaction) []error { + func(txs []*types.Transaction) []error { return make([]error, len(txs)) }, func(string, []common.Hash) error { return nil }, @@ -1083,7 +1083,7 @@ func TestTransactionFetcherDropRescheduling(t *testing.T) { init: func() *TxFetcher { return NewTxFetcher( func(common.Hash) bool { return false }, - func(txs []*txpool.Transaction) []error { + func(txs []*types.Transaction) []error { return make([]error, len(txs)) }, func(string, []common.Hash) error { return nil }, @@ -1128,7 +1128,7 @@ func TestTransactionFetcherFuzzCrash01(t *testing.T) { init: func() *TxFetcher { return NewTxFetcher( func(common.Hash) bool { return false }, - func(txs []*txpool.Transaction) []error { + func(txs []*types.Transaction) []error { return make([]error, len(txs)) }, func(string, []common.Hash) error { return nil }, @@ -1155,7 +1155,7 @@ func TestTransactionFetcherFuzzCrash02(t *testing.T) { init: func() *TxFetcher { return NewTxFetcher( func(common.Hash) bool { return false }, - func(txs []*txpool.Transaction) []error { + func(txs []*types.Transaction) []error { return make([]error, len(txs)) }, func(string, []common.Hash) error { return nil }, @@ -1184,7 +1184,7 @@ func TestTransactionFetcherFuzzCrash03(t *testing.T) { init: func() *TxFetcher { return NewTxFetcher( func(common.Hash) bool { return false }, - func(txs []*txpool.Transaction) []error { + func(txs []*types.Transaction) []error { return make([]error, len(txs)) }, func(string, []common.Hash) error { return nil }, @@ -1217,7 +1217,7 @@ func TestTransactionFetcherFuzzCrash04(t *testing.T) { init: func() *TxFetcher { return NewTxFetcher( func(common.Hash) bool { return false }, - func(txs []*txpool.Transaction) []error { + func(txs []*types.Transaction) []error { return make([]error, len(txs)) }, func(string, []common.Hash) error { diff --git a/eth/handler.go b/eth/handler.go index 2453c08afbc0..342094f1d4a3 100644 --- a/eth/handler.go +++ b/eth/handler.go @@ -66,10 +66,10 @@ type txPool interface { // Get retrieves the transaction from local txpool with given // tx hash. - Get(hash common.Hash) *txpool.Transaction + Get(hash common.Hash) *types.Transaction // Add should add the given transactions to the pool. - Add(txs []*txpool.Transaction, local bool, sync bool) []error + Add(txs []*types.Transaction, local bool, sync bool) []error // Pending should return pending transactions. // The slice should be modifiable by the caller. @@ -285,7 +285,7 @@ func newHandler(config *handlerConfig) (*handler, error) { } return p.RequestTxs(hashes) } - addTxs := func(txs []*txpool.Transaction) []error { + addTxs := func(txs []*types.Transaction) []error { return h.txpool.Add(txs, false, false) } h.txFetcher = fetcher.NewTxFetcher(h.txpool.Has, addTxs, fetchTx) diff --git a/eth/handler_eth_test.go b/eth/handler_eth_test.go index a9ce83e2ee44..4f41c9efc362 100644 --- a/eth/handler_eth_test.go +++ b/eth/handler_eth_test.go @@ -28,7 +28,6 @@ import ( "github.com/ethereum/go-ethereum/core" "github.com/ethereum/go-ethereum/core/forkid" "github.com/ethereum/go-ethereum/core/rawdb" - "github.com/ethereum/go-ethereum/core/txpool" "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/core/vm" "github.com/ethereum/go-ethereum/eth/downloader" @@ -308,12 +307,11 @@ func testSendTransactions(t *testing.T, protocol uint) { handler := newTestHandler() defer handler.close() - insert := make([]*txpool.Transaction, 100) + insert := make([]*types.Transaction, 100) for nonce := range insert { tx := types.NewTransaction(uint64(nonce), common.Address{}, big.NewInt(0), 100000, big.NewInt(0), make([]byte, 10240)) tx, _ = types.SignTx(tx, types.HomesteadSigner{}, testKey) - - insert[nonce] = &txpool.Transaction{Tx: tx} + insert[nonce] = tx } go handler.txpool.Add(insert, false, false) // Need goroutine to not block on feed time.Sleep(250 * time.Millisecond) // Wait until tx events get out of the system (can't use events, tx broadcaster races with peer join) @@ -434,12 +432,11 @@ func testTransactionPropagation(t *testing.T, protocol uint) { defer sub.Unsubscribe() } // Fill the source pool with transactions and wait for them at the sinks - txs := make([]*txpool.Transaction, 1024) + txs := make([]*types.Transaction, 1024) for nonce := range txs { tx := types.NewTransaction(uint64(nonce), common.Address{}, big.NewInt(0), 100000, big.NewInt(0), nil) tx, _ = types.SignTx(tx, types.HomesteadSigner{}, testKey) - - txs[nonce] = &txpool.Transaction{Tx: tx} + txs[nonce] = tx } source.txpool.Add(txs, false, false) diff --git a/eth/handler_test.go b/eth/handler_test.go index 7451e17012a0..2e0a988452b7 100644 --- a/eth/handler_test.go +++ b/eth/handler_test.go @@ -72,32 +72,23 @@ func (p *testTxPool) Has(hash common.Hash) bool { // Get retrieves the transaction from local txpool with given // tx hash. -func (p *testTxPool) Get(hash common.Hash) *txpool.Transaction { +func (p *testTxPool) Get(hash common.Hash) *types.Transaction { p.lock.Lock() defer p.lock.Unlock() - - if tx := p.pool[hash]; tx != nil { - return &txpool.Transaction{Tx: tx} - } - return nil + return p.pool[hash] } // Add appends a batch of transactions to the pool, and notifies any // listeners if the addition channel is non nil -func (p *testTxPool) Add(txs []*txpool.Transaction, local bool, sync bool) []error { - unwrapped := make([]*types.Transaction, len(txs)) - for i, tx := range txs { - unwrapped[i] = tx.Tx - } +func (p *testTxPool) Add(txs []*types.Transaction, local bool, sync bool) []error { p.lock.Lock() defer p.lock.Unlock() - for _, tx := range unwrapped { + for _, tx := range txs { p.pool[tx.Hash()] = tx } - - p.txFeed.Send(core.NewTxsEvent{Txs: unwrapped}) - return make([]error, len(unwrapped)) + p.txFeed.Send(core.NewTxsEvent{Txs: txs}) + return make([]error, len(txs)) } // Pending returns all the transactions known to the pool @@ -118,7 +109,7 @@ func (p *testTxPool) Pending(enforceTips bool) map[common.Address][]*txpool.Lazy for _, tx := range batch { pending[addr] = append(pending[addr], &txpool.LazyTransaction{ Hash: tx.Hash(), - Tx: &txpool.Transaction{Tx: tx}, + Tx: tx, Time: tx.Time(), GasFeeCap: tx.GasFeeCap(), GasTipCap: tx.GasTipCap(), diff --git a/eth/protocols/eth/broadcast.go b/eth/protocols/eth/broadcast.go index c431aa4005a6..3045303f222e 100644 --- a/eth/protocols/eth/broadcast.go +++ b/eth/protocols/eth/broadcast.go @@ -81,8 +81,8 @@ func (p *Peer) broadcastTransactions() { ) for i := 0; i < len(queue) && size < maxTxPacketSize; i++ { if tx := p.txpool.Get(queue[i]); tx != nil { - txs = append(txs, tx.Tx) - size += common.StorageSize(tx.Tx.Size()) + txs = append(txs, tx) + size += common.StorageSize(tx.Size()) } hashesCount++ } @@ -151,8 +151,8 @@ func (p *Peer) announceTransactions() { for count = 0; count < len(queue) && size < maxTxPacketSize; count++ { if tx := p.txpool.Get(queue[count]); tx != nil { pending = append(pending, queue[count]) - pendingTypes = append(pendingTypes, tx.Tx.Type()) - pendingSizes = append(pendingSizes, uint32(tx.Tx.Size())) + pendingTypes = append(pendingTypes, tx.Type()) + pendingSizes = append(pendingSizes, uint32(tx.Size())) size += common.HashLength } } diff --git a/eth/protocols/eth/handler.go b/eth/protocols/eth/handler.go index 7f51d4f5cdd3..3136864270c0 100644 --- a/eth/protocols/eth/handler.go +++ b/eth/protocols/eth/handler.go @@ -23,7 +23,7 @@ import ( "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core" - "github.com/ethereum/go-ethereum/core/txpool" + "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/metrics" "github.com/ethereum/go-ethereum/p2p" "github.com/ethereum/go-ethereum/p2p/enode" @@ -90,7 +90,7 @@ type Backend interface { // TxPool defines the methods needed by the protocol handler to serve transactions. type TxPool interface { // Get retrieves the transaction from the local txpool with the given hash. - Get(hash common.Hash) *txpool.Transaction + Get(hash common.Hash) *types.Transaction } // MakeProtocols constructs the P2P protocol definitions for `eth`. diff --git a/eth/protocols/eth/handlers.go b/eth/protocols/eth/handlers.go index f9fbf72b7b1c..74e514b863a3 100644 --- a/eth/protocols/eth/handlers.go +++ b/eth/protocols/eth/handlers.go @@ -498,7 +498,7 @@ func answerGetPooledTransactions(backend Backend, query GetPooledTransactionsPac continue } // If known, encode and queue for response packet - if encoded, err := rlp.EncodeToBytes(tx.Tx); err != nil { + if encoded, err := rlp.EncodeToBytes(tx); err != nil { log.Error("Failed to encode transaction", "err", err) } else { hashes = append(hashes, hash) diff --git a/les/server_requests.go b/les/server_requests.go index 30ff2cd05fb4..ad281a8e360d 100644 --- a/les/server_requests.go +++ b/les/server_requests.go @@ -519,7 +519,7 @@ func handleSendTx(msg Decoder) (serveRequestFn, uint64, uint64, error) { hash := tx.Hash() stats[i] = txStatus(backend, hash) if stats[i].Status == txpool.TxStatusUnknown { - if errs := backend.TxPool().Add([]*txpool.Transaction{{Tx: tx}}, false, backend.AddTxsSync()); errs[0] != nil { + if errs := backend.TxPool().Add([]*types.Transaction{tx}, false, backend.AddTxsSync()); errs[0] != nil { stats[i].Error = errs[0].Error() continue } diff --git a/miner/ordering_test.go b/miner/ordering_test.go index 589633e0b8dd..bdbdc3214851 100644 --- a/miner/ordering_test.go +++ b/miner/ordering_test.go @@ -88,7 +88,7 @@ func testTransactionPriceNonceSort(t *testing.T, baseFee *big.Int) { } groups[addr] = append(groups[addr], &txpool.LazyTransaction{ Hash: tx.Hash(), - Tx: &txpool.Transaction{Tx: tx}, + Tx: tx, Time: tx.Time(), GasFeeCap: tx.GasFeeCap(), GasTipCap: tx.GasTipCap(), @@ -101,7 +101,7 @@ func testTransactionPriceNonceSort(t *testing.T, baseFee *big.Int) { txs := types.Transactions{} for tx := txset.Peek(); tx != nil; tx = txset.Peek() { - txs = append(txs, tx.Tx.Tx) + txs = append(txs, tx.Tx) txset.Shift() } if len(txs) != expectedCount { @@ -153,7 +153,7 @@ func TestTransactionTimeSort(t *testing.T) { groups[addr] = append(groups[addr], &txpool.LazyTransaction{ Hash: tx.Hash(), - Tx: &txpool.Transaction{Tx: tx}, + Tx: tx, Time: tx.Time(), GasFeeCap: tx.GasFeeCap(), GasTipCap: tx.GasTipCap(), @@ -164,7 +164,7 @@ func TestTransactionTimeSort(t *testing.T) { txs := types.Transactions{} for tx := txset.Peek(); tx != nil; tx = txset.Peek() { - txs = append(txs, tx.Tx.Tx) + txs = append(txs, tx.Tx) txset.Shift() } if len(txs) != len(keys) { diff --git a/miner/stress/clique/main.go b/miner/stress/clique/main.go index 53ff2450c55b..50128ee36162 100644 --- a/miner/stress/clique/main.go +++ b/miner/stress/clique/main.go @@ -30,7 +30,6 @@ import ( "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/fdlimit" "github.com/ethereum/go-ethereum/core" - "github.com/ethereum/go-ethereum/core/txpool" "github.com/ethereum/go-ethereum/core/txpool/legacypool" "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/crypto" @@ -133,7 +132,7 @@ func main() { if err != nil { panic(err) } - if err := backend.TxPool().Add([]*txpool.Transaction{{Tx: tx}}, true, false); err != nil { + if err := backend.TxPool().Add([]*types.Transaction{tx}, true, false); err != nil { panic(err) } nonces[index]++ diff --git a/miner/worker.go b/miner/worker.go index 97967ea2f18b..ee379eaba36e 100644 --- a/miner/worker.go +++ b/miner/worker.go @@ -539,7 +539,7 @@ func (w *worker) mainLoop() { acc, _ := types.Sender(w.current.signer, tx) txs[acc] = append(txs[acc], &txpool.LazyTransaction{ Hash: tx.Hash(), - Tx: &txpool.Transaction{Tx: tx}, + Tx: tx, Time: tx.Time(), GasFeeCap: tx.GasFeeCap(), GasTipCap: tx.GasTipCap(), @@ -734,18 +734,18 @@ func (w *worker) updateSnapshot(env *environment) { w.snapshotState = env.state.Copy() } -func (w *worker) commitTransaction(env *environment, tx *txpool.Transaction) ([]*types.Log, error) { +func (w *worker) commitTransaction(env *environment, tx *types.Transaction) ([]*types.Log, error) { var ( snap = env.state.Snapshot() gp = env.gasPool.Gas() ) - receipt, err := core.ApplyTransaction(w.chainConfig, w.chain, &env.coinbase, env.gasPool, env.state, env.header, tx.Tx, &env.header.GasUsed, *w.chain.GetVMConfig()) + receipt, err := core.ApplyTransaction(w.chainConfig, w.chain, &env.coinbase, env.gasPool, env.state, env.header, tx, &env.header.GasUsed, *w.chain.GetVMConfig()) if err != nil { env.state.RevertToSnapshot(snap) env.gasPool.SetGas(gp) return nil, err } - env.txs = append(env.txs, tx.Tx) + env.txs = append(env.txs, tx) env.receipts = append(env.receipts, receipt) return receipt.Logs, nil @@ -778,30 +778,30 @@ func (w *worker) commitTransactions(env *environment, txs *transactionsByPriceAn tx := ltx.Resolve() if tx == nil { log.Warn("Ignoring evicted transaction") - txs.Pop() continue } + // Error may be ignored here. The error has already been checked // during transaction acceptance is the transaction pool. - from, _ := types.Sender(env.signer, tx.Tx) + from, _ := types.Sender(env.signer, tx) // Check whether the tx is replay protected. If we're not in the EIP155 hf // phase, start ignoring the sender until we do. - if tx.Tx.Protected() && !w.chainConfig.IsEIP155(env.header.Number) { - log.Trace("Ignoring reply protected transaction", "hash", tx.Tx.Hash(), "eip155", w.chainConfig.EIP155Block) + if tx.Protected() && !w.chainConfig.IsEIP155(env.header.Number) { + log.Trace("Ignoring reply protected transaction", "hash", tx.Hash(), "eip155", w.chainConfig.EIP155Block) txs.Pop() continue } // Start executing the transaction - env.state.SetTxContext(tx.Tx.Hash(), env.tcount) + env.state.SetTxContext(tx.Hash(), env.tcount) logs, err := w.commitTransaction(env, tx) switch { case errors.Is(err, core.ErrNonceTooLow): // New head notification data race between the transaction pool and miner, shift - log.Trace("Skipping transaction with low nonce", "sender", from, "nonce", tx.Tx.Nonce()) + log.Trace("Skipping transaction with low nonce", "sender", from, "nonce", tx.Nonce()) txs.Shift() case errors.Is(err, nil): @@ -813,7 +813,7 @@ func (w *worker) commitTransactions(env *environment, txs *transactionsByPriceAn default: // Transaction is regarded as invalid, drop all consecutive transactions from // the same sender because of `nonce-too-high` clause. - log.Debug("Transaction failed, account skipped", "hash", tx.Tx.Hash(), "err", err) + log.Debug("Transaction failed, account skipped", "hash", tx.Hash(), "err", err) txs.Pop() } } diff --git a/miner/worker_test.go b/miner/worker_test.go index 80557d99bfcf..e46061daf199 100644 --- a/miner/worker_test.go +++ b/miner/worker_test.go @@ -63,7 +63,7 @@ var ( testUserAddress = crypto.PubkeyToAddress(testUserKey.PublicKey) // Test transactions - pendingTxs []*txpool.Transaction + pendingTxs []*types.Transaction newTxs []*types.Transaction testConfig = &Config{ @@ -93,7 +93,7 @@ func init() { Gas: params.TxGas, GasPrice: big.NewInt(params.InitialBaseFee), }) - pendingTxs = append(pendingTxs, &txpool.Transaction{Tx: tx1}) + pendingTxs = append(pendingTxs, tx1) tx2 := types.MustSignNewTx(testBankKey, signer, &types.LegacyTx{ Nonce: 1, @@ -194,8 +194,8 @@ func TestGenerateAndImportBlock(t *testing.T) { w.start() for i := 0; i < 5; i++ { - b.txPool.Add([]*txpool.Transaction{{Tx: b.newRandomTx(true)}}, true, false) - b.txPool.Add([]*txpool.Transaction{{Tx: b.newRandomTx(false)}}, true, false) + b.txPool.Add([]*types.Transaction{b.newRandomTx(true)}, true, false) + b.txPool.Add([]*types.Transaction{b.newRandomTx(false)}, true, false) select { case ev := <-sub.Chan(): diff --git a/tests/fuzzers/txfetcher/txfetcher_fuzzer.go b/tests/fuzzers/txfetcher/txfetcher_fuzzer.go index 56b6b1e64eaa..d1d6fdc66592 100644 --- a/tests/fuzzers/txfetcher/txfetcher_fuzzer.go +++ b/tests/fuzzers/txfetcher/txfetcher_fuzzer.go @@ -25,7 +25,6 @@ import ( "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/mclock" - "github.com/ethereum/go-ethereum/core/txpool" "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/eth/fetcher" ) @@ -80,7 +79,7 @@ func Fuzz(input []byte) int { f := fetcher.NewTxFetcherForTests( func(common.Hash) bool { return false }, - func(txs []*txpool.Transaction) []error { + func(txs []*types.Transaction) []error { return make([]error, len(txs)) }, func(string, []common.Hash) error { return nil }, From f756841a263b91db3e4b9493799c0b52bcf758c0 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Thu, 3 Aug 2023 12:55:53 +0200 Subject: [PATCH 08/22] miner: strip sidecar in commit --- core/types/transaction.go | 16 ++++++++++++++++ core/types/tx_blob.go | 6 ++++++ miner/worker.go | 4 ++-- 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/core/types/transaction.go b/core/types/transaction.go index 67f49a90e05e..81a614e00641 100644 --- a/core/types/transaction.go +++ b/core/types/transaction.go @@ -413,6 +413,22 @@ func (tx *Transaction) BlobGasFeeCapIntCmp(other *big.Int) int { return tx.BlobGasFeeCap().Cmp(other) } +// WithoutBlobSidecar returns a copy of tx with the blob sidecar removed. +func (tx *Transaction) WithoutBlobSidecar() *Transaction { + blobtx, ok := tx.inner.(*BlobTx) + if !ok { + return tx + } + cpy := &Transaction{ + inner: blobtx.withoutSidecar(), + time: tx.time, + } + cpy.hash.Store(tx.hash.Load()) + cpy.size.Store(tx.size.Load()) + cpy.from.Store(tx.from.Load()) + return tx +} + // SetTime sets the decoding time of a transaction. This is used by tests to set // arbitrary times and by persistent transaction pools when loading old txs from // disk. diff --git a/core/types/tx_blob.go b/core/types/tx_blob.go index e0488ffa6747..f73792df9329 100644 --- a/core/types/tx_blob.go +++ b/core/types/tx_blob.go @@ -159,6 +159,12 @@ func (tx *BlobTx) setSignatureValues(chainID, v, r, s *big.Int) { tx.S.SetFromBig(s) } +func (tx *BlobTx) withoutSidecar() *BlobTx { + cpy := *tx + cpy.Sidecar = nil + return &cpy +} + func (tx *BlobTx) encode(b *bytes.Buffer) error { if tx.Sidecar == nil { return rlp.Encode(b, tx) diff --git a/miner/worker.go b/miner/worker.go index ee379eaba36e..39717e5009f7 100644 --- a/miner/worker.go +++ b/miner/worker.go @@ -539,7 +539,7 @@ func (w *worker) mainLoop() { acc, _ := types.Sender(w.current.signer, tx) txs[acc] = append(txs[acc], &txpool.LazyTransaction{ Hash: tx.Hash(), - Tx: tx, + Tx: tx.WithoutBlobSidecar(), Time: tx.Time(), GasFeeCap: tx.GasFeeCap(), GasTipCap: tx.GasTipCap(), @@ -790,10 +790,10 @@ func (w *worker) commitTransactions(env *environment, txs *transactionsByPriceAn // phase, start ignoring the sender until we do. if tx.Protected() && !w.chainConfig.IsEIP155(env.header.Number) { log.Trace("Ignoring reply protected transaction", "hash", tx.Hash(), "eip155", w.chainConfig.EIP155Block) - txs.Pop() continue } + // Start executing the transaction env.state.SetTxContext(tx.Hash(), env.tcount) From d6945e95f2d4beb56294a7c78f626178515ccdc0 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Thu, 3 Aug 2023 12:58:41 +0200 Subject: [PATCH 09/22] eth: fix issue in test --- eth/handler_eth_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/eth/handler_eth_test.go b/eth/handler_eth_test.go index 4f41c9efc362..41619fe3000c 100644 --- a/eth/handler_eth_test.go +++ b/eth/handler_eth_test.go @@ -374,8 +374,8 @@ func testSendTransactions(t *testing.T, protocol uint) { } } for _, tx := range insert { - if _, ok := seen[tx.Tx.Hash()]; !ok { - t.Errorf("missing transaction: %x", tx.Tx.Hash()) + if _, ok := seen[tx.Hash()]; !ok { + t.Errorf("missing transaction: %x", tx.Hash()) } } } From a03f3d105867a679e6cdda1fa85b605ea00ec08c Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Thu, 3 Aug 2023 13:54:29 +0200 Subject: [PATCH 10/22] core/txpool/blobpool: check sidecar is still there after decoding tx --- core/txpool/blobpool/blobpool.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/core/txpool/blobpool/blobpool.go b/core/txpool/blobpool/blobpool.go index ea6d71d4629f..61f8fb3b5b57 100644 --- a/core/txpool/blobpool/blobpool.go +++ b/core/txpool/blobpool/blobpool.go @@ -452,6 +452,10 @@ func (p *BlobPool) parseTransaction(id uint64, size uint32, blob []byte) error { log.Error("Failed to decode blob pool entry", "id", id, "err", err) return err } + if tx.BlobSidecar() == nil { + return fmt.Errorf("missing blob sidecar in tx id %d", id) + } + meta := newBlobTxMeta(id, size, tx) sender, err := p.signer.Sender(tx) From b84a6ed0548705cc2c34fa88c0156075aac2e92e Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Thu, 3 Aug 2023 14:12:37 +0200 Subject: [PATCH 11/22] core/types: remove unused method blobGasFeeCap --- core/types/tx_blob.go | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/core/types/tx_blob.go b/core/types/tx_blob.go index f73792df9329..16f9159ae1e5 100644 --- a/core/types/tx_blob.go +++ b/core/types/tx_blob.go @@ -123,19 +123,18 @@ func (tx *BlobTx) copy() TxData { } // accessors for innerTx. -func (tx *BlobTx) txType() byte { return BlobTxType } -func (tx *BlobTx) chainID() *big.Int { return tx.ChainID.ToBig() } -func (tx *BlobTx) accessList() AccessList { return tx.AccessList } -func (tx *BlobTx) data() []byte { return tx.Data } -func (tx *BlobTx) gas() uint64 { return tx.Gas } -func (tx *BlobTx) gasFeeCap() *big.Int { return tx.GasFeeCap.ToBig() } -func (tx *BlobTx) gasTipCap() *big.Int { return tx.GasTipCap.ToBig() } -func (tx *BlobTx) gasPrice() *big.Int { return tx.GasFeeCap.ToBig() } -func (tx *BlobTx) value() *big.Int { return tx.Value.ToBig() } -func (tx *BlobTx) nonce() uint64 { return tx.Nonce } -func (tx *BlobTx) to() *common.Address { tmp := tx.To; return &tmp } -func (tx *BlobTx) blobGas() uint64 { return params.BlobTxBlobGasPerBlob * uint64(len(tx.BlobHashes)) } -func (tx *BlobTx) blobGasFeeCap() *big.Int { return tx.BlobFeeCap.ToBig() } +func (tx *BlobTx) txType() byte { return BlobTxType } +func (tx *BlobTx) chainID() *big.Int { return tx.ChainID.ToBig() } +func (tx *BlobTx) accessList() AccessList { return tx.AccessList } +func (tx *BlobTx) data() []byte { return tx.Data } +func (tx *BlobTx) gas() uint64 { return tx.Gas } +func (tx *BlobTx) gasFeeCap() *big.Int { return tx.GasFeeCap.ToBig() } +func (tx *BlobTx) gasTipCap() *big.Int { return tx.GasTipCap.ToBig() } +func (tx *BlobTx) gasPrice() *big.Int { return tx.GasFeeCap.ToBig() } +func (tx *BlobTx) value() *big.Int { return tx.Value.ToBig() } +func (tx *BlobTx) nonce() uint64 { return tx.Nonce } +func (tx *BlobTx) to() *common.Address { tmp := tx.To; return &tmp } +func (tx *BlobTx) blobGas() uint64 { return params.BlobTxBlobGasPerBlob * uint64(len(tx.BlobHashes)) } func (tx *BlobTx) effectiveGasPrice(dst *big.Int, baseFee *big.Int) *big.Int { if baseFee == nil { From e9dab4478fc09e55536c3308a4541bf8acffe83e Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Mon, 7 Aug 2023 17:04:13 +0200 Subject: [PATCH 12/22] core/txpool/blobpool: add error message --- core/txpool/blobpool/blobpool.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/core/txpool/blobpool/blobpool.go b/core/txpool/blobpool/blobpool.go index 61f8fb3b5b57..ffd0d3c0278d 100644 --- a/core/txpool/blobpool/blobpool.go +++ b/core/txpool/blobpool/blobpool.go @@ -19,6 +19,7 @@ package blobpool import ( "container/heap" + "errors" "fmt" "math" "math/big" @@ -453,7 +454,8 @@ func (p *BlobPool) parseTransaction(id uint64, size uint32, blob []byte) error { return err } if tx.BlobSidecar() == nil { - return fmt.Errorf("missing blob sidecar in tx id %d", id) + log.Error("Missing sidecar in blob pool entry", "id", id, "hash", tx.Hash()) + return errors.New("missing blob sidecar") } meta := newBlobTxMeta(id, size, tx) From 67fec27441664a6791ecbbacc3d7b7ffb9f0797f Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Mon, 7 Aug 2023 17:11:50 +0200 Subject: [PATCH 13/22] core: improve validation loop in InsertReceiptChain Seems better to have a variable for the iteration item. --- core/blockchain.go | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/core/blockchain.go b/core/blockchain.go index 3952c31b688f..05c55966d7fb 100644 --- a/core/blockchain.go +++ b/core/blockchain.go @@ -1028,19 +1028,22 @@ func (bc *BlockChain) InsertReceiptChain(blockChain types.Blocks, receiptChain [ ancientReceipts, liveReceipts []types.Receipts ) // Do a sanity check that the provided chain is actually ordered and linked - for i := 0; i < len(blockChain); i++ { + for i, block := range blockChain { if i != 0 { - if blockChain[i].NumberU64() != blockChain[i-1].NumberU64()+1 || blockChain[i].ParentHash() != blockChain[i-1].Hash() { - log.Error("Non contiguous receipt insert", "number", blockChain[i].Number(), "hash", blockChain[i].Hash(), "parent", blockChain[i].ParentHash(), - "prevnumber", blockChain[i-1].Number(), "prevhash", blockChain[i-1].Hash()) - return 0, fmt.Errorf("non contiguous insert: item %d is #%d [%x..], item %d is #%d [%x..] (parent [%x..])", i-1, blockChain[i-1].NumberU64(), - blockChain[i-1].Hash().Bytes()[:4], i, blockChain[i].NumberU64(), blockChain[i].Hash().Bytes()[:4], blockChain[i].ParentHash().Bytes()[:4]) + prev := blockChain[i-1] + if block.NumberU64() != prev.NumberU64()+1 || block.ParentHash() != prev.Hash() { + log.Error("Non contiguous receipt insert", + "number", block.Number(), "hash", block.Hash(), "parent", block.ParentHash(), + "prevnumber", prev.Number(), "prevhash", prev.Hash()) + return 0, fmt.Errorf("non contiguous insert: item %d is #%d [%x..], item %d is #%d [%x..] (parent [%x..])", + i-1, prev.NumberU64(), prev.Hash().Bytes()[:4], + i, block.NumberU64(), block.Hash().Bytes()[:4], block.ParentHash().Bytes()[:4]) } } - if blockChain[i].NumberU64() <= ancientLimit { - ancientBlocks, ancientReceipts = append(ancientBlocks, blockChain[i]), append(ancientReceipts, receiptChain[i]) + if block.NumberU64() <= ancientLimit { + ancientBlocks, ancientReceipts = append(ancientBlocks, block), append(ancientReceipts, receiptChain[i]) } else { - liveBlocks, liveReceipts = append(liveBlocks, blockChain[i]), append(liveReceipts, receiptChain[i]) + liveBlocks, liveReceipts = append(liveBlocks, block), append(liveReceipts, receiptChain[i]) } } From 1de750f0019b590afeb36eac1dde59d899f33efe Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Mon, 7 Aug 2023 17:14:56 +0200 Subject: [PATCH 14/22] core: check absence of blob sidecar in InsertReceiptChain --- core/blockchain.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/core/blockchain.go b/core/blockchain.go index 05c55966d7fb..caeb0645683b 100644 --- a/core/blockchain.go +++ b/core/blockchain.go @@ -1045,6 +1045,14 @@ func (bc *BlockChain) InsertReceiptChain(blockChain types.Blocks, receiptChain [ } else { liveBlocks, liveReceipts = append(liveBlocks, block), append(liveReceipts, receiptChain[i]) } + + // Here we also validate that blob transactions in the block do not contain a sidecar. + // While the sidecar does not affect the block hash / tx hash, sending blobs within a block is not allowed. + for txIndex, tx := range block.Transactions() { + if tx.Type() == types.BlobTxType && tx.BlobSidecar() != nil { + return 0, fmt.Errorf("block #%d contains unexpected blob sidecar in tx at index %d", block.NumberU64(), txIndex) + } + } } var ( From f37d471d36db35b0ce3a1c3220c5c88a00a6a5f1 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Mon, 7 Aug 2023 17:25:55 +0200 Subject: [PATCH 15/22] core/txpool/blobpool: simplify some code in limbo No need to keep track of the txhash in a variable when we are storing the full tx. --- core/txpool/blobpool/blobpool.go | 2 +- core/txpool/blobpool/limbo.go | 47 ++++++++++++++++---------------- 2 files changed, 25 insertions(+), 24 deletions(-) diff --git a/core/txpool/blobpool/blobpool.go b/core/txpool/blobpool/blobpool.go index ffd0d3c0278d..692ae004f219 100644 --- a/core/txpool/blobpool/blobpool.go +++ b/core/txpool/blobpool/blobpool.go @@ -723,7 +723,7 @@ func (p *BlobPool) offload(addr common.Address, nonce uint64, id uint64, inclusi log.Warn("Blob transaction swapped out by signer", "from", addr, "nonce", nonce, "id", id) return } - if err := p.limbo.push(tx.Hash(), block, &tx); err != nil { + if err := p.limbo.push(&tx, block); err != nil { log.Warn("Failed to offload blob tx into limbo", "err", err) return } diff --git a/core/txpool/blobpool/limbo.go b/core/txpool/blobpool/limbo.go index dbcf0d76e6cc..2d62593de688 100644 --- a/core/txpool/blobpool/limbo.go +++ b/core/txpool/blobpool/limbo.go @@ -30,9 +30,9 @@ import ( // to which it belongs as well as the block number in which it was included for // finality eviction. type limboBlob struct { - Owner common.Hash // Owner transaction's hash to support resurrecting reorged txs - Block uint64 // Block in which the blob transaction was included - Tx *types.Transaction + TxHash common.Hash // Owner transaction's hash to support resurrecting reorged txs + Block uint64 // Block in which the blob transaction was included + Tx *types.Transaction } // limbo is a light, indexed database to temporarily store recently included @@ -94,19 +94,19 @@ func (l *limbo) parseBlob(id uint64, data []byte) error { log.Error("Failed to decode blob limbo entry", "id", id, "err", err) return err } - if _, ok := l.index[item.Owner]; ok { + if _, ok := l.index[item.TxHash]; ok { // This path is impossible, unless due to a programming error a blob gets // inserted into the limbo which was already part of if. Recover gracefully // by ignoring this data entry. - log.Error("Dropping duplicate blob limbo entry", "owner", item.Owner, "id", id) + log.Error("Dropping duplicate blob limbo entry", "owner", item.TxHash, "id", id) return errors.New("duplicate blob") } - l.index[item.Owner] = id + l.index[item.TxHash] = id if _, ok := l.groups[item.Block]; !ok { l.groups[item.Block] = make(map[uint64]common.Hash) } - l.groups[item.Block][id] = item.Owner + l.groups[item.Block][id] = item.TxHash return nil } @@ -135,14 +135,14 @@ func (l *limbo) finalize(final *types.Header) { // push stores a new blob transaction into the limbo, waiting until finality for // it to be automatically evicted. -func (l *limbo) push(txhash common.Hash, block uint64, tx *types.Transaction) error { +func (l *limbo) push(tx *types.Transaction, block uint64) error { // If the blobs are already tracked by the limbo, consider it a programming // error. There's not much to do against it, but be loud. - if _, ok := l.index[txhash]; ok { + if _, ok := l.index[tx.Hash()]; ok { log.Error("Limbo cannot push already tracked blobs", "tx", tx) return errors.New("already tracked blob transaction") } - if err := l.setAndIndex(txhash, block, tx); err != nil { + if err := l.setAndIndex(tx, block); err != nil { log.Error("Failed to set and index liboed blobs", "tx", tx, "err", err) return err } @@ -176,33 +176,33 @@ func (l *limbo) pull(tx common.Hash) (*types.Transaction, error) { // any of it since there's no clear error case. Some errors may be due to coding // issues, others caused by signers mining MEV stuff or swapping transactions. In // all cases, the pool needs to continue operating. -func (l *limbo) update(tx common.Hash, block uint64) { +func (l *limbo) update(txhash common.Hash, block uint64) { // If the blobs are not tracked by the limbo, there's not much to do. This // can happen for example if a blob transaction is mined without pushing it // into the network first. - id, ok := l.index[tx] + id, ok := l.index[txhash] if !ok { - log.Trace("Limbo cannot update non-tracked blobs", "tx", tx) + log.Trace("Limbo cannot update non-tracked blobs", "tx", txhash) return } // If there was no change in the blob's inclusion block, don't mess around // with heavy database operations. if _, ok := l.groups[block][id]; ok { - log.Trace("Blob transaction unchanged in limbo", "tx", tx, "block", block) + log.Trace("Blob transaction unchanged in limbo", "tx", txhash, "block", block) return } // Retrieve the old blobs from the data store and write tehm back with a new // block number. IF anything fails, there's not much to do, go on. item, err := l.getAndDrop(id) if err != nil { - log.Error("Failed to get and drop limboed blobs", "tx", tx, "id", id, "err", err) + log.Error("Failed to get and drop limboed blobs", "tx", txhash, "id", id, "err", err) return } - if err := l.setAndIndex(tx, block, item.Tx); err != nil { - log.Error("Failed to set and index limboed blobs", "tx", tx, "err", err) + if err := l.setAndIndex(item.Tx, block); err != nil { + log.Error("Failed to set and index limboed blobs", "tx", txhash, "err", err) return } - log.Trace("Blob transaction updated in limbo", "tx", tx, "old-block", item.Block, "new-block", block) + log.Trace("Blob transaction updated in limbo", "tx", txhash, "old-block", item.Block, "new-block", block) } // getAndDrop retrieves a blob item from the limbo store and deletes it both from @@ -216,7 +216,7 @@ func (l *limbo) getAndDrop(id uint64) (*limboBlob, error) { if err = rlp.DecodeBytes(data, item); err != nil { return nil, err } - delete(l.index, item.Owner) + delete(l.index, item.TxHash) delete(l.groups[item.Block], id) if len(l.groups[item.Block]) == 0 { delete(l.groups, item.Block) @@ -229,11 +229,12 @@ func (l *limbo) getAndDrop(id uint64) (*limboBlob, error) { // setAndIndex assembles a limbo blob database entry and stores it, also updating // the in-memory indices. -func (l *limbo) setAndIndex(txhash common.Hash, block uint64, tx *types.Transaction) error { +func (l *limbo) setAndIndex(tx *types.Transaction, block uint64) error { + txhash := tx.Hash() item := &limboBlob{ - Owner: txhash, - Block: block, - Tx: tx, + TxHash: txhash, + Block: block, + Tx: tx, } data, err := rlp.EncodeToBytes(item) if err != nil { From 9363dd50ea51c76c11ecdac7738f7a7f2fff90d4 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Tue, 8 Aug 2023 13:17:43 +0200 Subject: [PATCH 16/22] eth/downloader: add check for blob absence in DeliverBodies --- eth/downloader/queue.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/eth/downloader/queue.go b/eth/downloader/queue.go index c1df3b4e66b0..260a7fd57e26 100644 --- a/eth/downloader/queue.go +++ b/eth/downloader/queue.go @@ -798,7 +798,7 @@ func (q *queue) DeliverBodies(id string, txLists [][]*types.Transaction, txListH } } // Blocks must have a number of blobs corresponding to the header gas usage, - // and zero before the Cancun hardfork + // and zero before the Cancun hardfork. var blobs int for _, tx := range txLists[index] { // Count the number of blobs to validate against the header's blobGasUsed @@ -814,6 +814,9 @@ func (q *queue) DeliverBodies(id string, txLists [][]*types.Transaction, txListH return errInvalidBody } } + if tx.BlobSidecar() != nil { + return errInvalidBody + } } } if header.BlobGasUsed != nil { From d2f7c473678b145c85e786a764a5f435c1d9ed5d Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Tue, 8 Aug 2023 15:16:41 +0200 Subject: [PATCH 17/22] core/types: rename BlobSidecar to BlobTxSidecar --- core/block_validator.go | 2 +- core/blockchain.go | 2 +- core/txpool/blobpool/blobpool.go | 2 +- core/txpool/blobpool/blobpool_test.go | 2 +- core/txpool/validation.go | 4 ++-- core/types/transaction.go | 8 ++++---- core/types/tx_blob.go | 10 +++++----- eth/downloader/queue.go | 2 +- miner/worker.go | 2 +- 9 files changed, 17 insertions(+), 17 deletions(-) diff --git a/core/block_validator.go b/core/block_validator.go index 7fd848a73f65..8768163b943a 100644 --- a/core/block_validator.go +++ b/core/block_validator.go @@ -90,7 +90,7 @@ func (v *BlockValidator) ValidateBody(block *types.Block) error { blobs += len(tx.BlobHashes()) // If the tx is a blob tx, it must NOT have a sidecar attached to be valid in a block. - if tx.Type() == types.BlobTxType && tx.BlobSidecar() != nil { + if tx.Type() == types.BlobTxType && tx.BlobTxSidecar() != nil { return fmt.Errorf("unexpected blob sidecar in transaction at index %d", i) } diff --git a/core/blockchain.go b/core/blockchain.go index caeb0645683b..8b0080ad36b1 100644 --- a/core/blockchain.go +++ b/core/blockchain.go @@ -1049,7 +1049,7 @@ func (bc *BlockChain) InsertReceiptChain(blockChain types.Blocks, receiptChain [ // Here we also validate that blob transactions in the block do not contain a sidecar. // While the sidecar does not affect the block hash / tx hash, sending blobs within a block is not allowed. for txIndex, tx := range block.Transactions() { - if tx.Type() == types.BlobTxType && tx.BlobSidecar() != nil { + if tx.Type() == types.BlobTxType && tx.BlobTxSidecar() != nil { return 0, fmt.Errorf("block #%d contains unexpected blob sidecar in tx at index %d", block.NumberU64(), txIndex) } } diff --git a/core/txpool/blobpool/blobpool.go b/core/txpool/blobpool/blobpool.go index 692ae004f219..d51d9528077d 100644 --- a/core/txpool/blobpool/blobpool.go +++ b/core/txpool/blobpool/blobpool.go @@ -453,7 +453,7 @@ func (p *BlobPool) parseTransaction(id uint64, size uint32, blob []byte) error { log.Error("Failed to decode blob pool entry", "id", id, "err", err) return err } - if tx.BlobSidecar() == nil { + if tx.BlobTxSidecar() == nil { log.Error("Missing sidecar in blob pool entry", "id", id, "hash", tx.Hash()) return errors.New("missing blob sidecar") } diff --git a/core/txpool/blobpool/blobpool_test.go b/core/txpool/blobpool/blobpool_test.go index 2af5ab43870c..8914301e14c3 100644 --- a/core/txpool/blobpool/blobpool_test.go +++ b/core/txpool/blobpool/blobpool_test.go @@ -209,7 +209,7 @@ func makeUnsignedTx(nonce uint64, gasTipCap uint64, gasFeeCap uint64, blobFeeCap BlobFeeCap: uint256.NewInt(blobFeeCap), BlobHashes: []common.Hash{emptyBlobVHash}, Value: uint256.NewInt(100), - Sidecar: &types.BlobSidecar{ + Sidecar: &types.BlobTxSidecar{ Blobs: []kzg4844.Blob{emptyBlob}, Commitments: []kzg4844.Commitment{emptyBlobCommit}, Proofs: []kzg4844.Proof{emptyBlobProof}, diff --git a/core/txpool/validation.go b/core/txpool/validation.go index 722411b8a2a8..5451116e0ade 100644 --- a/core/txpool/validation.go +++ b/core/txpool/validation.go @@ -110,7 +110,7 @@ func ValidateTransaction(tx *types.Transaction, head *types.Header, signer types } // Ensure blob transactions have valid commitments if tx.Type() == types.BlobTxType { - sidecar := tx.BlobSidecar() + sidecar := tx.BlobTxSidecar() if sidecar == nil { return fmt.Errorf("missing sidecar in blob transaction") } @@ -130,7 +130,7 @@ func ValidateTransaction(tx *types.Transaction, head *types.Header, signer types return nil } -func validateBlobSidecar(hashes []common.Hash, sidecar *types.BlobSidecar) error { +func validateBlobSidecar(hashes []common.Hash, sidecar *types.BlobTxSidecar) error { if len(sidecar.Blobs) != len(hashes) { return fmt.Errorf("invalid number of %d blobs compared to %d blob hashes", len(sidecar.Blobs), len(hashes)) } diff --git a/core/types/transaction.go b/core/types/transaction.go index 81a614e00641..70db8490ba68 100644 --- a/core/types/transaction.go +++ b/core/types/transaction.go @@ -395,8 +395,8 @@ func (tx *Transaction) BlobHashes() []common.Hash { return nil } -// BlobSidecar returns the sidecar of a blob transaction, nil otherwise. -func (tx *Transaction) BlobSidecar() *BlobSidecar { +// BlobTxSidecar returns the sidecar of a blob transaction, nil otherwise. +func (tx *Transaction) BlobTxSidecar() *BlobTxSidecar { if blobtx, ok := tx.inner.(*BlobTx); ok { return blobtx.Sidecar } @@ -413,8 +413,8 @@ func (tx *Transaction) BlobGasFeeCapIntCmp(other *big.Int) int { return tx.BlobGasFeeCap().Cmp(other) } -// WithoutBlobSidecar returns a copy of tx with the blob sidecar removed. -func (tx *Transaction) WithoutBlobSidecar() *Transaction { +// WithoutBlobTxSidecar returns a copy of tx with the blob sidecar removed. +func (tx *Transaction) WithoutBlobTxSidecar() *Transaction { blobtx, ok := tx.inner.(*BlobTx) if !ok { return tx diff --git a/core/types/tx_blob.go b/core/types/tx_blob.go index 16f9159ae1e5..15bd44ec3804 100644 --- a/core/types/tx_blob.go +++ b/core/types/tx_blob.go @@ -43,7 +43,7 @@ type BlobTx struct { // A blob transaction can optionally contain blobs. This field must be set when BlobTx // is used to create a transaction for sigining. - Sidecar *BlobSidecar `rlp:"-"` + Sidecar *BlobTxSidecar `rlp:"-"` // Signature values V *uint256.Int `json:"v" gencodec:"required"` @@ -51,8 +51,8 @@ type BlobTx struct { S *uint256.Int `json:"s" gencodec:"required"` } -// BlobSidecar contains the blobs of a blob transaction. -type BlobSidecar struct { +// BlobTxSidecar contains the blobs of a blob transaction. +type BlobTxSidecar struct { Blobs []kzg4844.Blob // Blobs needed by the blob pool Commitments []kzg4844.Commitment // Commitments needed by the blob pool Proofs []kzg4844.Proof // Proofs needed by the blob pool @@ -113,7 +113,7 @@ func (tx *BlobTx) copy() TxData { cpy.S.Set(tx.S) } if tx.Sidecar != nil { - cpy.Sidecar = &BlobSidecar{ + cpy.Sidecar = &BlobTxSidecar{ Blobs: append([]kzg4844.Blob(nil), tx.Sidecar.Blobs...), Commitments: append([]kzg4844.Commitment(nil), tx.Sidecar.Commitments...), Proofs: append([]kzg4844.Proof(nil), tx.Sidecar.Proofs...), @@ -202,7 +202,7 @@ func (tx *BlobTx) decode(input []byte) error { return err } *tx = *inner.BlobTx - tx.Sidecar = &BlobSidecar{ + tx.Sidecar = &BlobTxSidecar{ Blobs: inner.Blobs, Commitments: inner.Commitments, Proofs: inner.Proofs, diff --git a/eth/downloader/queue.go b/eth/downloader/queue.go index 260a7fd57e26..e55715879772 100644 --- a/eth/downloader/queue.go +++ b/eth/downloader/queue.go @@ -814,7 +814,7 @@ func (q *queue) DeliverBodies(id string, txLists [][]*types.Transaction, txListH return errInvalidBody } } - if tx.BlobSidecar() != nil { + if tx.BlobTxSidecar() != nil { return errInvalidBody } } diff --git a/miner/worker.go b/miner/worker.go index 39717e5009f7..ef3c423d293d 100644 --- a/miner/worker.go +++ b/miner/worker.go @@ -539,7 +539,7 @@ func (w *worker) mainLoop() { acc, _ := types.Sender(w.current.signer, tx) txs[acc] = append(txs[acc], &txpool.LazyTransaction{ Hash: tx.Hash(), - Tx: tx.WithoutBlobSidecar(), + Tx: tx.WithoutBlobTxSidecar(), Time: tx.Time(), GasFeeCap: tx.GasFeeCap(), GasTipCap: tx.GasTipCap(), From 1995292540d8e04843259008f4b0a39c99a4a5ce Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Wed, 9 Aug 2023 14:07:50 +0200 Subject: [PATCH 18/22] core/types: add a method to compute blob hashes from BlobTxSidecar --- core/types/tx_blob.go | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/core/types/tx_blob.go b/core/types/tx_blob.go index 15bd44ec3804..91d96f56f40b 100644 --- a/core/types/tx_blob.go +++ b/core/types/tx_blob.go @@ -18,6 +18,7 @@ package types import ( "bytes" + "crypto/sha256" "math/big" "github.com/ethereum/go-ethereum/common" @@ -58,6 +59,15 @@ type BlobTxSidecar struct { Proofs []kzg4844.Proof // Proofs needed by the blob pool } +// BlobHashes computes the blob hashes of the given blobs. +func (txs *BlobTxSidecar) BlobHashes() []common.Hash { + h := make([]common.Hash, len(txs.Commitments)) + for i := range txs.Blobs { + h[i] = blobHash(&txs.Commitments[i]) + } + return h +} + // blobTxWithBlobs is used for encoding of transactions when blobs are present. type blobTxWithBlobs struct { BlobTx *BlobTx @@ -209,3 +219,12 @@ func (tx *BlobTx) decode(input []byte) error { } return nil } + +func blobHash(commit *kzg4844.Commitment) common.Hash { + hasher := sha256.New() + hasher.Write(commit[:]) + var vhash common.Hash + hasher.Sum(vhash[:0]) + vhash[0] = params.BlobTxHashVersion + return vhash +} From 3edddbc839ad8fd58e81f9d7159f7fe11bdea096 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Wed, 9 Aug 2023 14:08:13 +0200 Subject: [PATCH 19/22] core/types: fix crash in copy for BlobTx --- core/types/transaction.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/core/types/transaction.go b/core/types/transaction.go index 70db8490ba68..948cc3bf5396 100644 --- a/core/types/transaction.go +++ b/core/types/transaction.go @@ -423,9 +423,15 @@ func (tx *Transaction) WithoutBlobTxSidecar() *Transaction { inner: blobtx.withoutSidecar(), time: tx.time, } - cpy.hash.Store(tx.hash.Load()) - cpy.size.Store(tx.size.Load()) - cpy.from.Store(tx.from.Load()) + if h := tx.hash.Load(); h != nil { + cpy.hash.Store(h) + } + if s := tx.size.Load(); s != nil { + cpy.size.Store(s) + } + if f := tx.from.Load(); f != nil { + cpy.from.Store(f) + } return tx } From b6e34aa0c574277122a224c18f04627e50d22244 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Wed, 9 Aug 2023 14:08:37 +0200 Subject: [PATCH 20/22] core/types: add a test to verify hashing of BlobTx ignores the sidecar --- core/types/tx_blob_test.go | 62 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 core/types/tx_blob_test.go diff --git a/core/types/tx_blob_test.go b/core/types/tx_blob_test.go new file mode 100644 index 000000000000..d07544782389 --- /dev/null +++ b/core/types/tx_blob_test.go @@ -0,0 +1,62 @@ +package types + +import ( + "crypto/ecdsa" + "testing" + + "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/crypto" + "github.com/ethereum/go-ethereum/crypto/kzg4844" + "github.com/holiman/uint256" +) + +// This test verifies that the transaction hash is not affected by presence of a BlobTxSidecar. +func TestBlobTxHashing(t *testing.T) { + key, _ := crypto.GenerateKey() + withBlobs := createEmptyBlobTx(key, true) + withBlobsStripped := withBlobs.WithoutBlobTxSidecar() + withoutBlobs := createEmptyBlobTx(key, false) + + hash := withBlobs.Hash() + size := withBlobs.Size() + t.Log("tx hash:", hash) + t.Log("tx size:", size) + + if h := withBlobsStripped.Hash(); h != hash { + t.Fatal("wrong tx hash after WithoutBlobTxSidecar:", h) + } + if h := withoutBlobs.Hash(); h != hash { + t.Fatal("wrong tx hash on tx created without sidecar:", h) + } +} + +var ( + emptyBlob = kzg4844.Blob{} + emptyBlobCommit, _ = kzg4844.BlobToCommitment(emptyBlob) + emptyBlobProof, _ = kzg4844.ComputeBlobProof(emptyBlob, emptyBlobCommit) +) + +func createEmptyBlobTx(key *ecdsa.PrivateKey, withSidecar bool) *Transaction { + sidecar := &BlobTxSidecar{ + Blobs: []kzg4844.Blob{emptyBlob}, + Commitments: []kzg4844.Commitment{emptyBlobCommit}, + Proofs: []kzg4844.Proof{emptyBlobProof}, + } + blobtx := &BlobTx{ + ChainID: uint256.NewInt(1), + Nonce: 5, + GasTipCap: uint256.NewInt(22), + GasFeeCap: uint256.NewInt(5), + Gas: 25000, + To: common.Address{0x03, 0x04, 0x05}, + Value: uint256.NewInt(99), + Data: make([]byte, 50), + BlobFeeCap: uint256.NewInt(15), + BlobHashes: sidecar.BlobHashes(), + } + if withSidecar { + blobtx.Sidecar = sidecar + } + signer := NewCancunSigner(blobtx.ChainID.ToBig()) + return MustSignNewTx(key, signer, blobtx) +} From e86e43e37bff4eb757c240e9003700b936ec005b Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Wed, 9 Aug 2023 14:38:24 +0200 Subject: [PATCH 21/22] core/types: make Size work correctly for BlobTx with sidecar Also fixes a pretty serious bug where the WithoutBlobSidecar method didn't actually remove the sidecar... --- core/types/transaction.go | 21 +++++++++++++++------ core/types/tx_blob.go | 24 ++++++++++++++++++++---- core/types/tx_blob_test.go | 34 +++++++++++++++++++++++++++++++--- 3 files changed, 66 insertions(+), 13 deletions(-) diff --git a/core/types/transaction.go b/core/types/transaction.go index 948cc3bf5396..bf6dfb342891 100644 --- a/core/types/transaction.go +++ b/core/types/transaction.go @@ -423,16 +423,14 @@ func (tx *Transaction) WithoutBlobTxSidecar() *Transaction { inner: blobtx.withoutSidecar(), time: tx.time, } + // Note: tx.size cache not carried over because the sidecar is included in size! if h := tx.hash.Load(); h != nil { cpy.hash.Store(h) } - if s := tx.size.Load(); s != nil { - cpy.size.Store(s) - } if f := tx.from.Load(); f != nil { cpy.from.Store(f) } - return tx + return cpy } // SetTime sets the decoding time of a transaction. This is used by tests to set @@ -470,13 +468,24 @@ func (tx *Transaction) Size() uint64 { if size := tx.size.Load(); size != nil { return size.(uint64) } + + // Cache miss, encode and cache. + // Note we rely on the assumption that all tx.inner values are RLP-encoded! c := writeCounter(0) rlp.Encode(&c, &tx.inner) - size := uint64(c) + + // For blob transactions, add the size of the blob content and the outer list of the + // tx + sidecar encoding. + if sc := tx.BlobTxSidecar(); sc != nil { + size += rlp.ListSize(sc.encodedSize()) + } + + // For typed transactions, the encoding also includes the leading type byte. if tx.Type() != LegacyTxType { - size += 1 // type byte + size += 1 } + tx.size.Store(size) return size } diff --git a/core/types/tx_blob.go b/core/types/tx_blob.go index 91d96f56f40b..da4a9b72f17a 100644 --- a/core/types/tx_blob.go +++ b/core/types/tx_blob.go @@ -60,14 +60,30 @@ type BlobTxSidecar struct { } // BlobHashes computes the blob hashes of the given blobs. -func (txs *BlobTxSidecar) BlobHashes() []common.Hash { - h := make([]common.Hash, len(txs.Commitments)) - for i := range txs.Blobs { - h[i] = blobHash(&txs.Commitments[i]) +func (sc *BlobTxSidecar) BlobHashes() []common.Hash { + h := make([]common.Hash, len(sc.Commitments)) + for i := range sc.Blobs { + h[i] = blobHash(&sc.Commitments[i]) } return h } +// encodedSize computes the RLP size of the sidecar elements. This does NOT return the +// encoded size of the BlobTxSidecar, it's just a helper for tx.Size(). +func (sc *BlobTxSidecar) encodedSize() uint64 { + var blobs, commitments, proofs uint64 + for i := range sc.Blobs { + blobs += rlp.BytesSize(sc.Blobs[i][:]) + } + for i := range sc.Commitments { + commitments += rlp.BytesSize(sc.Commitments[i][:]) + } + for i := range sc.Proofs { + proofs += rlp.BytesSize(sc.Proofs[i][:]) + } + return rlp.ListSize(blobs) + rlp.ListSize(commitments) + rlp.ListSize(proofs) +} + // blobTxWithBlobs is used for encoding of transactions when blobs are present. type blobTxWithBlobs struct { BlobTx *BlobTx diff --git a/core/types/tx_blob_test.go b/core/types/tx_blob_test.go index d07544782389..44ac48cc6fac 100644 --- a/core/types/tx_blob_test.go +++ b/core/types/tx_blob_test.go @@ -10,7 +10,7 @@ import ( "github.com/holiman/uint256" ) -// This test verifies that the transaction hash is not affected by presence of a BlobTxSidecar. +// This test verifies that tx.Hash() is not affected by presence of a BlobTxSidecar. func TestBlobTxHashing(t *testing.T) { key, _ := crypto.GenerateKey() withBlobs := createEmptyBlobTx(key, true) @@ -18,9 +18,7 @@ func TestBlobTxHashing(t *testing.T) { withoutBlobs := createEmptyBlobTx(key, false) hash := withBlobs.Hash() - size := withBlobs.Size() t.Log("tx hash:", hash) - t.Log("tx size:", size) if h := withBlobsStripped.Hash(); h != hash { t.Fatal("wrong tx hash after WithoutBlobTxSidecar:", h) @@ -30,6 +28,36 @@ func TestBlobTxHashing(t *testing.T) { } } +// This test verifies that tx.Size() takes BlobTxSidecar into account. +func TestBlobTxSize(t *testing.T) { + key, _ := crypto.GenerateKey() + withBlobs := createEmptyBlobTx(key, true) + withBlobsStripped := withBlobs.WithoutBlobTxSidecar() + withoutBlobs := createEmptyBlobTx(key, false) + + withBlobsEnc, _ := withBlobs.MarshalBinary() + withoutBlobsEnc, _ := withoutBlobs.MarshalBinary() + + size := withBlobs.Size() + t.Log("size with blobs:", size) + + sizeNoBlobs := withoutBlobs.Size() + t.Log("size without blobs:", sizeNoBlobs) + + if size != uint64(len(withBlobsEnc)) { + t.Error("wrong size with blobs:", size, "encoded length:", len(withBlobsEnc)) + } + if sizeNoBlobs != uint64(len(withoutBlobsEnc)) { + t.Error("wrong size without blobs:", sizeNoBlobs, "encoded length:", len(withoutBlobsEnc)) + } + if sizeNoBlobs >= size { + t.Error("size without blobs >= size with blobs") + } + if sz := withBlobsStripped.Size(); sz != sizeNoBlobs { + t.Fatal("wrong size on tx after WithoutBlobTxSidecar:", sz) + } +} + var ( emptyBlob = kzg4844.Blob{} emptyBlobCommit, _ = kzg4844.BlobToCommitment(emptyBlob) From de44d2682d73de0618da672011b244bc71df95b7 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Wed, 9 Aug 2023 14:46:10 +0200 Subject: [PATCH 22/22] core: remove redundant tx type check in block validator --- core/block_validator.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/block_validator.go b/core/block_validator.go index 8768163b943a..f3d65cea25ff 100644 --- a/core/block_validator.go +++ b/core/block_validator.go @@ -90,7 +90,7 @@ func (v *BlockValidator) ValidateBody(block *types.Block) error { blobs += len(tx.BlobHashes()) // If the tx is a blob tx, it must NOT have a sidecar attached to be valid in a block. - if tx.Type() == types.BlobTxType && tx.BlobTxSidecar() != nil { + if tx.BlobTxSidecar() != nil { return fmt.Errorf("unexpected blob sidecar in transaction at index %d", i) }