From 999e6013a3c60427169cef287274aa2a6884dce1 Mon Sep 17 00:00:00 2001 From: healthykim Date: Thu, 18 Jun 2026 16:59:31 +0200 Subject: [PATCH] less diff, edit comments --- core/txpool/blobpool/blobpool.go | 3 +- core/txpool/blobpool/blobpool_test.go | 95 +++++++++++++-------------- core/txpool/validation.go | 4 ++ eth/catalyst/api_test.go | 16 +---- 4 files changed, 52 insertions(+), 66 deletions(-) diff --git a/core/txpool/blobpool/blobpool.go b/core/txpool/blobpool/blobpool.go index f684d58f30..090d250e65 100644 --- a/core/txpool/blobpool/blobpool.go +++ b/core/txpool/blobpool/blobpool.go @@ -1984,6 +1984,7 @@ func (p *BlobPool) addLocked(ptx *BlobTxForPool, checkGapped bool) (err error) { } return err } + //todo: validation happens twice for eth72 if err := txpool.ValidateCells(ptx.CellSidecar); err != nil { return err } @@ -2235,8 +2236,6 @@ func (p *BlobPool) drop() { // // The transactions can also be pre-filtered by the dynamic fee components to // reduce allocations and load on downstream subsystems. -// The transactions without enough cells to recover thier blobs would be skipped -// in the response. func (p *BlobPool) Pending(filter txpool.PendingFilter) (map[common.Address][]*txpool.LazyTransaction, int) { // If only plain transactions are requested, this pool is unsuitable as it // contains none, don't even bother. diff --git a/core/txpool/blobpool/blobpool_test.go b/core/txpool/blobpool/blobpool_test.go index 71810bb659..4e1672f22a 100644 --- a/core/txpool/blobpool/blobpool_test.go +++ b/core/txpool/blobpool/blobpool_test.go @@ -237,10 +237,7 @@ func makeTx(nonce uint64, gasTipCap uint64, gasFeeCap uint64, blobFeeCap uint64, // encodeForPool encodes a blob transaction in the blobTxForPool storage format. func encodeForPool(tx *types.Transaction) []byte { - ptx, err := newBlobTxForPool(tx) - if err != nil { - panic(err) - } + ptx, _ := newBlobTxForPool(tx) blob, _ := rlp.EncodeToBytes(ptx) return blob } @@ -2162,6 +2159,51 @@ func TestGetBlobs(t *testing.T) { pool.Close() } +// TestEncodeForNetwork verifies that encodeForNetwork produces the correct wire +// encoding for each (sidecar version, eth protocol version) combination. +// - eth/69, eth/70: blobs recovered from cells, output matches the original tx +// - eth/72: blob payload omitted, output matches removeBlobs(tx) +func TestEncodeForNetwork(t *testing.T) { + cases := []struct { + name string + sidecarVer byte + ethVer uint + }{ + {"v0/eth70", types.BlobSidecarVersion0, 70}, + {"v1/eth70", types.BlobSidecarVersion1, 70}, + {"v0/eth72", types.BlobSidecarVersion0, 72}, + {"v1/eth72", types.BlobSidecarVersion1, 72}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + testEncodeForNetwork(t, tc.sidecarVer, tc.ethVer) + }) + } +} + +func testEncodeForNetwork(t *testing.T, sidecarVer byte, ethVer uint) { + key, _ := crypto.GenerateKey() + tx := makeMultiBlobTx(0, 1, 1, 1, 1, 0, key, sidecarVer) + + wantTx := tx + if ethVer >= 72 { + wantTx = removeBlobs(tx) + } + wantRLP, err := rlp.EncodeToBytes(wantTx) + if err != nil { + t.Fatalf("failed to encode tx: %v", err) + } + storedRLP := encodeForPool(tx) + + gotRLP, err := encodeForNetwork(storedRLP, ethVer) + if err != nil { + t.Fatalf("encodeForNetwork failed: %v", err) + } + if !bytes.Equal(gotRLP, wantRLP) { + t.Fatalf("network encoding mismatch: got %d bytes, want %d bytes", len(gotRLP), len(wantRLP)) + } +} + // fakeBilly is a billy.Database implementation which just drops data on the floor. type fakeBilly struct { billy.Database @@ -2359,48 +2401,3 @@ func TestGetCells(t *testing.T) { }) } } - -// TestEncodeForNetwork verifies that encodeForNetwork produces the correct wire -// encoding for each (sidecar version, eth protocol version) combination. -// - eth/69, eth/70: blobs recovered from cells, output matches the original tx -// - eth/72: blob payload omitted, output matches removeBlobs(tx) -func TestEncodeForNetwork(t *testing.T) { - cases := []struct { - name string - sidecarVer byte - ethVer uint - }{ - {"v0/eth70", types.BlobSidecarVersion0, 70}, - {"v1/eth70", types.BlobSidecarVersion1, 70}, - {"v0/eth72", types.BlobSidecarVersion0, 72}, - {"v1/eth72", types.BlobSidecarVersion1, 72}, - } - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - testEncodeForNetwork(t, tc.sidecarVer, tc.ethVer) - }) - } -} - -func testEncodeForNetwork(t *testing.T, sidecarVer byte, ethVer uint) { - key, _ := crypto.GenerateKey() - tx := makeMultiBlobTx(0, 1, 1, 1, 1, 0, key, sidecarVer) - - wantTx := tx - if ethVer >= 72 { - wantTx = removeBlobs(tx) - } - wantRLP, err := rlp.EncodeToBytes(wantTx) - if err != nil { - t.Fatalf("failed to encode tx: %v", err) - } - storedRLP := encodeForPool(tx) - - gotRLP, err := encodeForNetwork(storedRLP, ethVer) - if err != nil { - t.Fatalf("encodeForNetwork failed: %v", err) - } - if !bytes.Equal(gotRLP, wantRLP) { - t.Fatalf("network encoding mismatch: got %d bytes, want %d bytes", len(gotRLP), len(wantRLP)) - } -} diff --git a/core/txpool/validation.go b/core/txpool/validation.go index fb0e91da47..c4cc213982 100644 --- a/core/txpool/validation.go +++ b/core/txpool/validation.go @@ -154,6 +154,9 @@ func ValidateTransaction(tx *types.Transaction, head *types.Header, signer types return nil } +// validateBlobSidecar implements the blob sidecar validation. +// Note that this doesn't verify the consistency between blobs(cells) and +// proofs. For proof verification, use validateCells. 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) @@ -178,6 +181,7 @@ func validateBlobSidecar(tx *types.Transaction, head *types.Header, opts *Valida return fmt.Errorf("%w: unexpected sidecar version, want: %d, got: %d", ErrSidecarFormatError, expected, sidecar.Version) } + //todo: can we drop the support for v0 blob txs in blobpool? switch sidecar.Version { case types.BlobSidecarVersion0: if len(sidecar.Proofs) != len(sidecar.Commitments) { diff --git a/eth/catalyst/api_test.go b/eth/catalyst/api_test.go index 947ad25e37..0cf2c5c8e6 100644 --- a/eth/catalyst/api_test.go +++ b/eth/catalyst/api_test.go @@ -1912,12 +1912,7 @@ func newGetBlobEnv(t testing.TB, version byte) (*node.Node, *ConsensusAPI) { tx1 := makeMultiBlobTx(&config, 0, 2, 0, key1, version) // blob[0, 2) tx2 := makeMultiBlobTx(&config, 0, 2, 2, key2, version) // blob[2, 4) tx3 := makeMultiBlobTx(&config, 0, 2, 4, key3, version) // blob[4, 6) - errs := ethServ.TxPool().Add([]*types.Transaction{tx1, tx2, tx3}, true) - for i, err := range errs { - if err != nil { - t.Logf("Add tx %d failed: %v", i, err) - } - } + ethServ.TxPool().Add([]*types.Transaction{tx1, tx2, tx3}, true) api := newConsensusAPIWithoutHeartbeat(ethServ) return n, api @@ -2121,15 +2116,6 @@ func runGetBlobs(t testing.TB, getBlobs getBlobsFn, start, limit int, fillRandom } } if !reflect.DeepEqual(result, expect) { - t.Logf("result len=%d, expect len=%d", len(result), len(expect)) - if len(result) > 0 && result[0] != nil && len(expect) > 0 && expect[0] != nil { - t.Logf("result[0].Blob len=%d, expect[0].Blob len=%d", len(result[0].Blob), len(expect[0].Blob)) - t.Logf("result[0].CellProofs len=%d, expect[0].CellProofs len=%d", len(result[0].CellProofs), len(expect[0].CellProofs)) - t.Logf("result[0].Blob == expect[0].Blob: %v", reflect.DeepEqual(result[0].Blob, expect[0].Blob)) - t.Logf("result[0].CellProofs == expect[0].CellProofs: %v", reflect.DeepEqual(result[0].CellProofs, expect[0].CellProofs)) - } else { - t.Logf("result[0]=%v, expect[0]=%v", result, expect) - } t.Fatalf("Unexpected result for case %s", name) } }