From db0fe7eec4830c362745e96a1ea7d11a8002c8e8 Mon Sep 17 00:00:00 2001 From: healthykim Date: Wed, 20 May 2026 16:04:45 +0200 Subject: [PATCH] refactor validation logic --- core/txpool/blobpool/blobpool.go | 76 +++++++++++++--------------- core/txpool/blobpool/buffer.go | 10 ++-- core/txpool/errors.go | 3 ++ core/txpool/validation.go | 85 +++++++++++++++++++------------- core/types/tx_blob.go | 23 --------- eth/fetcher/tx_fetcher.go | 2 +- eth/handler_test.go | 2 +- 7 files changed, 97 insertions(+), 104 deletions(-) diff --git a/core/txpool/blobpool/blobpool.go b/core/txpool/blobpool/blobpool.go index 2c698ac7a6..21e8f16381 100644 --- a/core/txpool/blobpool/blobpool.go +++ b/core/txpool/blobpool/blobpool.go @@ -156,28 +156,26 @@ type blobTxMeta struct { // blobpool. type BlobTxForPool struct { Tx *types.Transaction // tx without sidecar - Version byte - Commitments []kzg4844.Commitment - Proofs []kzg4844.Proof - Cells []kzg4844.Cell - Custody types.CustodyBitmap + CellSidecar *types.BlobTxCellSidecar } // Sidecar returns BlobTxSidecar of pooled transaction. Since this function // recovers the blob field in sidecar, it is expansive and needs to be // avoided if possible. Returns error if recovery fails (e.g. insufficient cells). func (ptx *BlobTxForPool) sidecar() (*types.BlobTxSidecar, error) { - blobs, err := kzg4844.RecoverBlobs(ptx.Cells, ptx.Custody.Indices()) + sidecar := ptx.CellSidecar + blobs, err := kzg4844.RecoverBlobs(sidecar.Cells, sidecar.Custody.Indices()) if err != nil { return nil, err } - return types.NewBlobTxSidecar(ptx.Version, blobs, ptx.Commitments, ptx.Proofs), nil + return types.NewBlobTxSidecar(sidecar.Version, blobs, sidecar.Commitments, sidecar.Proofs), nil } func (ptx *BlobTxForPool) toV1() error { // todo: If we have a function to compute proofs from cells, // we can avoid blob recovery here - blobs, err := kzg4844.RecoverBlobs(ptx.Cells, ptx.Custody.Indices()) + sidecar := ptx.CellSidecar + blobs, err := kzg4844.RecoverBlobs(sidecar.Cells, sidecar.Custody.Indices()) if err != nil { return err } @@ -189,33 +187,37 @@ func (ptx *BlobTxForPool) toV1() error { } proofs = append(proofs, proof...) } - ptx.Proofs = proofs - ptx.Version = types.BlobSidecarVersion1 + sidecar.Proofs = proofs + sidecar.Version = types.BlobSidecarVersion1 return nil } // TxSize returns the transaction size on the network without // reconstructing the transaction. func (ptx *BlobTxForPool) txSize() uint64 { + sidecar := ptx.CellSidecar + var commitments, proofs uint64 - for i := range ptx.Commitments { - commitments += rlp.BytesSize(ptx.Commitments[i][:]) + for i := range sidecar.Commitments { + commitments += rlp.BytesSize(sidecar.Commitments[i][:]) } - for i := range ptx.Proofs { - proofs += rlp.BytesSize(ptx.Proofs[i][:]) + for i := range sidecar.Proofs { + proofs += rlp.BytesSize(sidecar.Proofs[i][:]) } var blob kzg4844.Blob - blobs := uint64(len(ptx.Commitments)) * rlp.BytesSize(blob[:]) + blobs := uint64(len(sidecar.Commitments)) * rlp.BytesSize(blob[:]) return ptx.Tx.Size() + rlp.ListSize(rlp.ListSize(blobs)+rlp.ListSize(commitments)+rlp.ListSize(proofs)) } func (ptx *BlobTxForPool) txSizeWithoutBlob() uint64 { + sidecar := ptx.CellSidecar + var commitments, proofs uint64 - for i := range ptx.Commitments { - commitments += rlp.BytesSize(ptx.Commitments[i][:]) + for i := range sidecar.Commitments { + commitments += rlp.BytesSize(sidecar.Commitments[i][:]) } - for i := range ptx.Proofs { - proofs += rlp.BytesSize(ptx.Proofs[i][:]) + for i := range sidecar.Proofs { + proofs += rlp.BytesSize(sidecar.Proofs[i][:]) } return ptx.Tx.Size() + rlp.ListSize(rlp.ListSize(0)+rlp.ListSize(commitments)+rlp.ListSize(proofs)) } @@ -239,13 +241,16 @@ func newBlobTxForPool(tx *types.Transaction) (*BlobTxForPool, error) { if err != nil { return nil, err } - return &BlobTxForPool{ - Tx: tx.WithoutBlobTxSidecar(), + sidecar := types.BlobTxCellSidecar{ Version: sc.Version, Commitments: sc.Commitments, Proofs: sc.Proofs, Cells: cells, Custody: *types.CustodyBitmapAll, + } + return &BlobTxForPool{ + Tx: tx.WithoutBlobTxSidecar(), + CellSidecar: &sidecar, }, nil } @@ -345,7 +350,7 @@ func newBlobTxMeta(id uint64, storageSize uint32, ptx *BlobTxForPool) *blobTxMet meta := &blobTxMeta{ hash: ptx.Tx.Hash(), vhashes: ptx.Tx.BlobHashes(), - version: ptx.Version, + version: ptx.CellSidecar.Version, id: id, storageSize: storageSize, size: ptx.txSize(), @@ -357,7 +362,7 @@ func newBlobTxMeta(id uint64, storageSize uint32, ptx *BlobTxForPool) *blobTxMet blobFeeCap: uint256.MustFromBig(ptx.Tx.BlobGasFeeCap()), execGas: ptx.Tx.Gas(), blobGas: ptx.Tx.BlobGas(), - custody: &ptx.Custody, + custody: &ptx.CellSidecar.Custody, } meta.basefeeJumps = dynamicFeeJumps(meta.execFeeCap) meta.blobfeeJumps = dynamicBlobFeeJumps(meta.blobFeeCap) @@ -1374,7 +1379,7 @@ func (p *BlobPool) reinject(addr common.Address, txhash common.Hash) error { // could theoretically halt a Geth node for ~1.2s by reorging per block. However, // this attack is financially inefficient to execute. head := p.head.Load() - if p.chain.Config().IsOsaka(head.Number, head.Time) && ptx.Version == types.BlobSidecarVersion0 { + if p.chain.Config().IsOsaka(head.Number, head.Time) && ptx.CellSidecar.Version == types.BlobSidecarVersion0 { if err := ptx.toV1(); err != nil { log.Error("Failed to convert the legacy sidecar", "err", err) return err @@ -1845,8 +1850,8 @@ func (p *BlobPool) GetBlobCells(vhashes []common.Hash, mask types.CustodyBitmap) continue } tx := ptx.Tx - cellsPerBlob := ptx.Custody.OneCount() - storedIndices := ptx.Custody.Indices() + cellsPerBlob := ptx.CellSidecar.Custody.OneCount() + storedIndices := ptx.CellSidecar.Custody.Indices() for blobIdx, hash := range tx.BlobHashes() { indices, ok := vindex[hash] @@ -1867,11 +1872,11 @@ func (p *BlobPool) GetBlobCells(vhashes []common.Hash, mask types.CustodyBitmap) } } if pos >= 0 { - cell := ptx.Cells[blobIdx*cellsPerBlob+pos] + cell := ptx.CellSidecar.Cells[blobIdx*cellsPerBlob+pos] blobCells[i] = &cell proofIdx := blobIdx*kzg4844.CellProofsPerBlob + int(cellIdx) - if proofIdx < len(ptx.Proofs) { - proof := ptx.Proofs[proofIdx] + if proofIdx < len(ptx.CellSidecar.Proofs) { + proof := ptx.CellSidecar.Proofs[proofIdx] blobProofs[i] = &proof } } @@ -1940,14 +1945,6 @@ func (p *BlobPool) AddPooledTx(ptx *BlobTxForPool) (err error) { // Only for internal use. func (p *BlobPool) addLocked(ptx *BlobTxForPool, checkGapped bool) (err error) { tx := ptx.Tx - //todo: remove this type and also ToBlobTxCellSidecar function - cellSidecar := &types.BlobTxCellSidecar{ - Version: ptx.Version, - Cells: ptx.Cells, - Commitments: ptx.Commitments, - Proofs: ptx.Proofs, - Custody: ptx.Custody, - } // Ensure the transaction is valid from all perspectives if err := p.validateTx(tx); err != nil { @@ -1989,10 +1986,7 @@ func (p *BlobPool) addLocked(ptx *BlobTxForPool, checkGapped bool) (err error) { } return err } - if err := txpool.ValidateBlobSidecar(tx, cellSidecar, p.head.Load(), &txpool.ValidationOptions{ - Config: p.chain.Config(), - MaxBlobCount: maxBlobsPerTx, - }); err != nil { + if err := txpool.ValidateCells(ptx.CellSidecar); err != nil { return err } // If the address is not yet known, request exclusivity to track the account diff --git a/core/txpool/blobpool/buffer.go b/core/txpool/blobpool/buffer.go index 86301dcbf5..6bf4ac233a 100644 --- a/core/txpool/blobpool/buffer.go +++ b/core/txpool/blobpool/buffer.go @@ -138,7 +138,7 @@ func (b *BlobBuffer) AddCells(hash common.Hash, deliveries map[string]*PeerDeliv return nil } -// todo returning error here is strange +// todo: this is very strange // add verifies cells per-peer, sorts them, and adds to the pool. func (b *BlobBuffer) add(hash common.Hash, tx *types.Transaction, cells *cellEntry) error { sidecar := tx.BlobTxSidecar() @@ -153,14 +153,18 @@ func (b *BlobBuffer) add(hash common.Hash, tx *types.Transaction, cells *cellEnt blobCount := len(tx.BlobHashes()) sorted, custody := sortCells(cells, blobCount) - pooledTx := &BlobTxForPool{ - Tx: tx.WithoutBlobTxSidecar(), + cellSidecar := types.BlobTxCellSidecar{ Version: sidecar.Version, Commitments: sidecar.Commitments, Proofs: sidecar.Proofs, Cells: sorted, Custody: *custody, } + pooledTx := &BlobTxForPool{ + Tx: tx.WithoutBlobTxSidecar(), + CellSidecar: &cellSidecar, + } + err := b.addToPool(pooledTx) delete(b.cells, hash) delete(b.txs, hash) diff --git a/core/txpool/errors.go b/core/txpool/errors.go index 8285cbf10e..b1c8b2006e 100644 --- a/core/txpool/errors.go +++ b/core/txpool/errors.go @@ -74,4 +74,7 @@ var ( // ErrKZGVerificationError is returned when a KZG proof was not verified correctly. ErrKZGVerificationError = errors.New("KZG verification error") + + // ErrSidecarFormatError is returned when sidecar is malformed + ErrSidecarFormatError = errors.New("Wrong sidecar format") ) diff --git a/core/txpool/validation.go b/core/txpool/validation.go index 9189a627ec..fb0e91da47 100644 --- a/core/txpool/validation.go +++ b/core/txpool/validation.go @@ -148,54 +148,72 @@ func ValidateTransaction(tx *types.Transaction, head *types.Header, signer types return errors.New("set code tx must have at least one authorization tuple") } } + if tx.Type() == types.BlobTxType { + return validateBlobSidecar(tx, head, opts) + } return nil } -func ValidateBlobSidecar(tx *types.Transaction, sidecar *types.BlobTxCellSidecar, head *types.Header, opts *ValidationOptions) error { - if sidecar.Custody.OneCount() == 0 { - return errors.New("blobless blob transaction") - } - // Ensure the blob fee cap satisfies the minimum blob gas price +func validateBlobSidecar(tx *types.Transaction, head *types.Header, opts *ValidationOptions) error { if tx.BlobGasFeeCapIntCmp(blobTxMinBlobGasPrice) < 0 { return fmt.Errorf("%w: blob fee cap %v, minimum needed %v", ErrTxGasPriceTooLow, tx.BlobGasFeeCap(), blobTxMinBlobGasPrice) } - // Verify whether the blob count is consistent with other parts of the sidecar and the transaction - blobCount := len(sidecar.Cells) / sidecar.Custody.OneCount() + sidecar := tx.BlobTxSidecar() + if sidecar == nil { + return errors.New("missing sidecar in blob transaction") + } hashes := tx.BlobHashes() - if blobCount == 0 { - return errors.New("blobless blob transaction") - } - if blobCount != len(sidecar.Commitments) || blobCount != len(hashes) { - return fmt.Errorf("invalid number of %d blobs compared to %d commitments and %d blob hashes", blobCount, len(sidecar.Commitments), len(tx.BlobHashes())) - } - - // Check whether the blob count does not exceed the max blob count - if blobCount > opts.MaxBlobCount { - return fmt.Errorf("%w: blob count %v, limit %v", ErrTxBlobLimitExceeded, blobCount, opts.MaxBlobCount) - } - if err := sidecar.ValidateBlobCommitmentHashes(hashes); err != nil { return err } - // Ensure the sidecar version is correct for the current fork (master: bd77b77ed) - version := types.BlobSidecarVersion0 - if opts.Config.IsOsaka(head.Number, head.Time) { - version = types.BlobSidecarVersion1 + if len(hashes) > opts.MaxBlobCount { + return fmt.Errorf("%w: blob count %v, limit %v", ErrTxBlobLimitExceeded, len(hashes), opts.MaxBlobCount) } - if sidecar.Version != version { - return fmt.Errorf("unexpected sidecar version, want: %d, got: %d", version, sidecar.Version) + + expected := types.BlobSidecarVersion0 + if opts.Config.IsOsaka(head.Number, head.Time) { + expected = types.BlobSidecarVersion1 + } + if sidecar.Version != expected { + return fmt.Errorf("%w: unexpected sidecar version, want: %d, got: %d", ErrSidecarFormatError, expected, sidecar.Version) + } + + switch sidecar.Version { + case types.BlobSidecarVersion0: + if len(sidecar.Proofs) != len(sidecar.Commitments) { + return fmt.Errorf("%w: invalid number of %d blob proofs expected %d", ErrSidecarFormatError, len(sidecar.Proofs), len(sidecar.Commitments)) + } + case types.BlobSidecarVersion1: + if len(sidecar.Proofs) != len(sidecar.Commitments)*kzg4844.CellProofsPerBlob { + return fmt.Errorf("%w: invalid number of %d blob proofs expected %d", ErrSidecarFormatError, len(sidecar.Proofs), len(sidecar.Commitments)*kzg4844.CellProofsPerBlob) + } + } + return nil +} + +func ValidateCells(sidecar *types.BlobTxCellSidecar) error { + // Two checks here (custody count check and blobCount check) is duplicated in buffer.go + // However it is required to 1) serve eth71 peer and direct submission 2) catch any bug in + // merging cell delivery. + if sidecar.Custody.OneCount() == 0 { + return errors.New("blobless blob transaction") + } + // Verify whether the blob count is consistent with other parts of the sidecar and the transaction + blobCount := len(sidecar.Cells) / sidecar.Custody.OneCount() + if blobCount == 0 { + return errors.New("blobless blob transaction") + } + if blobCount != len(sidecar.Commitments) { + return fmt.Errorf("invalid number of %d blobs compared to %d commitments", blobCount, len(sidecar.Commitments)) } // Fork-specific sidecar checks, including proof verification. if sidecar.Version == types.BlobSidecarVersion1 { - return validateBlobSidecarOsaka(sidecar, hashes) + return validateCellsOsaka(sidecar) } - return validateBlobSidecarLegacy(sidecar, hashes) + return validateCellsLegacy(sidecar) } -func validateBlobSidecarLegacy(sidecar *types.BlobTxCellSidecar, hashes []common.Hash) error { - if len(sidecar.Proofs) != len(hashes) { - return fmt.Errorf("invalid number of %d blob proofs expected %d", len(sidecar.Proofs), len(hashes)) - } +func validateCellsLegacy(sidecar *types.BlobTxCellSidecar) error { blobs, err := kzg4844.RecoverBlobs(sidecar.Cells, sidecar.Custody.Indices()) if err != nil { return fmt.Errorf("%w: %v", ErrKZGVerificationError, err) @@ -208,10 +226,7 @@ func validateBlobSidecarLegacy(sidecar *types.BlobTxCellSidecar, hashes []common return nil } -func validateBlobSidecarOsaka(sidecar *types.BlobTxCellSidecar, hashes []common.Hash) error { - if len(sidecar.Proofs) != len(hashes)*kzg4844.CellProofsPerBlob { - return fmt.Errorf("invalid number of %d blob proofs expected %d", len(sidecar.Proofs), len(hashes)*kzg4844.CellProofsPerBlob) - } +func validateCellsOsaka(sidecar *types.BlobTxCellSidecar) error { indices := sidecar.Custody.Indices() cellProofs := make([]kzg4844.Proof, 0) for blobIdx := range len(sidecar.Commitments) { diff --git a/core/types/tx_blob.go b/core/types/tx_blob.go index b941c8876f..5b0d8a0985 100644 --- a/core/types/tx_blob.go +++ b/core/types/tx_blob.go @@ -176,20 +176,6 @@ func (sc *BlobTxSidecar) Copy() *BlobTxSidecar { } } -func (sc *BlobTxSidecar) ToBlobTxCellSidecar() (*BlobTxCellSidecar, error) { - cells, err := kzg4844.ComputeCells(sc.Blobs) - if err != nil { - return nil, err - } - return &BlobTxCellSidecar{ - Version: sc.Version, - Cells: cells, - Commitments: sc.Commitments, - Proofs: sc.Proofs, - Custody: *CustodyBitmapAll, - }, nil -} - type BlobTxCellSidecar struct { Version byte Cells []kzg4844.Cell @@ -198,15 +184,6 @@ type BlobTxCellSidecar struct { Custody CustodyBitmap } -// ValidateBlobCommitmentHashes checks whether the given hashes correspond to the -// commitments in the sidecar -func (c *BlobTxCellSidecar) ValidateBlobCommitmentHashes(hashes []common.Hash) error { - sc := BlobTxSidecar{ - Commitments: c.Commitments, - } - return sc.ValidateBlobCommitmentHashes(hashes) -} - // blobTxWithBlobs represents blob tx with its corresponding sidecar. // This is an interface because sidecars are versioned. type blobTxWithBlobs interface { diff --git a/eth/fetcher/tx_fetcher.go b/eth/fetcher/tx_fetcher.go index c3c25f677e..20a6d031f2 100644 --- a/eth/fetcher/tx_fetcher.go +++ b/eth/fetcher/tx_fetcher.go @@ -369,7 +369,7 @@ func (f *TxFetcher) Enqueue(peer string, txs []*types.Transaction, direct bool) case errors.Is(err, txpool.ErrUnderpriced) || errors.Is(err, txpool.ErrReplaceUnderpriced) || errors.Is(err, txpool.ErrTxGasPriceTooLow): underpriced++ - case errors.Is(err, txpool.ErrKZGVerificationError): + case errors.Is(err, txpool.ErrKZGVerificationError) || errors.Is(err, txpool.ErrSidecarFormatError): // KZG verification failed, terminate transaction processing immediately. // Since KZG verification is computationally expensive, this acts as a // defensive measure against potential DoS attacks. diff --git a/eth/handler_test.go b/eth/handler_test.go index 0f3aa074ad..f53bbaecd2 100644 --- a/eth/handler_test.go +++ b/eth/handler_test.go @@ -257,7 +257,7 @@ func (p *testTxPool) AddPooledTx(pooledTx *blobpool.BlobTxForPool) error { p.lock.Lock() defer p.lock.Unlock() hash := pooledTx.Tx.Hash() - p.cellPool[hash] = pooledTx.Cells + p.cellPool[hash] = pooledTx.CellSidecar.Cells p.txPool[hash] = pooledTx.Tx return nil }