From 1601f398d4558e6c060a158fa10b7570ce58cd6b Mon Sep 17 00:00:00 2001 From: Csaba Kiraly Date: Fri, 19 Sep 2025 16:21:15 +0200 Subject: [PATCH] core/txpool/blobpool: remove conversion in GetBlobs (#32578) This disables blob proof conversion in `GetBlobs` by default, making it conditional. --------- Co-authored-by: Felix Lange --- core/txpool/blobpool/blobpool.go | 22 +++++++++++---- core/txpool/blobpool/blobpool_test.go | 40 ++++++++++++++++----------- eth/catalyst/api.go | 4 +-- 3 files changed, 43 insertions(+), 23 deletions(-) diff --git a/core/txpool/blobpool/blobpool.go b/core/txpool/blobpool/blobpool.go index 55d24c7a93..2229f1544c 100644 --- a/core/txpool/blobpool/blobpool.go +++ b/core/txpool/blobpool/blobpool.go @@ -1331,7 +1331,11 @@ func (p *BlobPool) GetMetadata(hash common.Hash) *txpool.TxMetadata { // // This is a utility method for the engine API, enabling consensus clients to // retrieve blobs from the pools directly instead of the network. -func (p *BlobPool) GetBlobs(vhashes []common.Hash, version byte) ([]*kzg4844.Blob, []kzg4844.Commitment, [][]kzg4844.Proof, error) { +// +// The version argument specifies the type of proofs to return, either the +// blob proofs (version 0) or the cell proofs (version 1). Proofs conversion is +// CPU intensive, so only done if explicitly requested with the convert flag. +func (p *BlobPool) GetBlobs(vhashes []common.Hash, version byte, convert bool) ([]*kzg4844.Blob, []kzg4844.Commitment, [][]kzg4844.Proof, error) { var ( blobs = make([]*kzg4844.Blob, len(vhashes)) commitments = make([]kzg4844.Commitment, len(vhashes)) @@ -1343,13 +1347,14 @@ func (p *BlobPool) GetBlobs(vhashes []common.Hash, version byte) ([]*kzg4844.Blo for i, h := range vhashes { indices[h] = append(indices[h], i) } + for _, vhash := range vhashes { - // Skip duplicate vhash that was already resolved in a previous iteration if _, ok := filled[vhash]; ok { + // Skip vhash that was already resolved in a previous iteration continue } - // Retrieve the corresponding blob tx with the vhash, skip blob resolution - // if it's not found locally and place the null instead. + + // Retrieve the corresponding blob tx with the vhash. p.lock.RLock() txID, exists := p.lookup.storeidOfBlob(vhash) p.lock.RUnlock() @@ -1379,6 +1384,14 @@ func (p *BlobPool) GetBlobs(vhashes []common.Hash, version byte) ([]*kzg4844.Blo if !ok { continue // non-interesting blob } + // Mark hash as seen. + filled[hash] = struct{}{} + if sidecar.Version != version && !convert { + // Skip blobs with incompatible version. Note we still track the blob hash + // in `filled` here, ensuring that we do not resolve this tx another time. + continue + } + // Get or convert the proof. var pf []kzg4844.Proof switch version { case types.BlobSidecarVersion0: @@ -1411,7 +1424,6 @@ func (p *BlobPool) GetBlobs(vhashes []common.Hash, version byte) ([]*kzg4844.Blo commitments[index] = sidecar.Commitments[i] proofs[index] = pf } - filled[hash] = struct{}{} } } return blobs, commitments, proofs, nil diff --git a/core/txpool/blobpool/blobpool_test.go b/core/txpool/blobpool/blobpool_test.go index e46529a241..57d27962ce 100644 --- a/core/txpool/blobpool/blobpool_test.go +++ b/core/txpool/blobpool/blobpool_test.go @@ -423,11 +423,11 @@ func verifyBlobRetrievals(t *testing.T, pool *BlobPool) { hashes = append(hashes, tx.vhashes...) } } - blobs1, _, proofs1, err := pool.GetBlobs(hashes, types.BlobSidecarVersion0) + blobs1, _, proofs1, err := pool.GetBlobs(hashes, types.BlobSidecarVersion0, false) if err != nil { t.Fatal(err) } - blobs2, _, proofs2, err := pool.GetBlobs(hashes, types.BlobSidecarVersion1) + blobs2, _, proofs2, err := pool.GetBlobs(hashes, types.BlobSidecarVersion1, false) if err != nil { t.Fatal(err) } @@ -441,22 +441,18 @@ func verifyBlobRetrievals(t *testing.T, pool *BlobPool) { return } for i, hash := range hashes { - // If an item is missing, but shouldn't, error - if blobs1[i] == nil || proofs1[i] == nil { - t.Errorf("tracked blob retrieval failed: item %d, hash %x", i, hash) - continue - } - if blobs2[i] == nil || proofs2[i] == nil { + // If an item is missing from both, but shouldn't, error + if (blobs1[i] == nil || proofs1[i] == nil) && (blobs2[i] == nil || proofs2[i] == nil) { t.Errorf("tracked blob retrieval failed: item %d, hash %x", i, hash) continue } // Item retrieved, make sure it matches the expectation index := testBlobIndices[hash] - if *blobs1[i] != *testBlobs[index] || proofs1[i][0] != testBlobProofs[index] { + if blobs1[i] != nil && (*blobs1[i] != *testBlobs[index] || proofs1[i][0] != testBlobProofs[index]) { t.Errorf("retrieved blob or proof mismatch: item %d, hash %x", i, hash) continue } - if *blobs2[i] != *testBlobs[index] || !slices.Equal(proofs2[i], testBlobCellProofs[index]) { + if blobs2[i] != nil && (*blobs2[i] != *testBlobs[index] || !slices.Equal(proofs2[i], testBlobCellProofs[index])) { t.Errorf("retrieved blob or proof mismatch: item %d, hash %x", i, hash) continue } @@ -1926,8 +1922,9 @@ func TestGetBlobs(t *testing.T) { cases := []struct { start int limit int - fillRandom bool - version byte + fillRandom bool // Whether to randomly fill some of the requested blobs with unknowns + version byte // Blob sidecar version to request + convert bool // Whether to convert version on retrieval }{ { start: 0, limit: 6, @@ -1993,6 +1990,11 @@ func TestGetBlobs(t *testing.T) { start: 0, limit: 18, fillRandom: true, version: types.BlobSidecarVersion1, }, + { + start: 0, limit: 18, fillRandom: true, + version: types.BlobSidecarVersion1, + convert: true, // Convert some version 0 blobs to version 1 while retrieving + }, } for i, c := range cases { var ( @@ -2014,7 +2016,7 @@ func TestGetBlobs(t *testing.T) { filled[len(vhashes)] = struct{}{} vhashes = append(vhashes, testrand.Hash()) } - blobs, _, proofs, err := pool.GetBlobs(vhashes, c.version) + blobs, _, proofs, err := pool.GetBlobs(vhashes, c.version, c.convert) if err != nil { t.Errorf("Unexpected error for case %d, %v", i, err) } @@ -2029,6 +2031,7 @@ func TestGetBlobs(t *testing.T) { var unknown int for j := 0; j < len(blobs); j++ { + testBlobIndex := c.start + j - unknown if _, exist := filled[j]; exist { if blobs[j] != nil || proofs[j] != nil { t.Errorf("Unexpected blob and proof, item %d", j) @@ -2038,17 +2041,22 @@ func TestGetBlobs(t *testing.T) { } // If an item is missing, but shouldn't, error if blobs[j] == nil || proofs[j] == nil { - t.Errorf("tracked blob retrieval failed: item %d, hash %x", j, vhashes[j]) + // This is only an error if there was no version mismatch + if c.convert || + (c.version == types.BlobSidecarVersion1 && 6 <= testBlobIndex && testBlobIndex < 12) || + (c.version == types.BlobSidecarVersion0 && (testBlobIndex < 6 || 12 <= testBlobIndex)) { + t.Errorf("tracked blob retrieval failed: item %d, hash %x", j, vhashes[j]) + } continue } // Item retrieved, make sure the blob matches the expectation - if *blobs[j] != *testBlobs[c.start+j-unknown] { + if *blobs[j] != *testBlobs[testBlobIndex] { t.Errorf("retrieved blob mismatch: item %d, hash %x", j, vhashes[j]) continue } // Item retrieved, make sure the proof matches the expectation if c.version == types.BlobSidecarVersion0 { - if proofs[j][0] != testBlobProofs[c.start+j-unknown] { + if proofs[j][0] != testBlobProofs[testBlobIndex] { t.Errorf("retrieved proof mismatch: item %d, hash %x", j, vhashes[j]) } } else { diff --git a/eth/catalyst/api.go b/eth/catalyst/api.go index 17299d9296..b37c26149f 100644 --- a/eth/catalyst/api.go +++ b/eth/catalyst/api.go @@ -495,7 +495,7 @@ func (api *ConsensusAPI) GetBlobsV1(hashes []common.Hash) ([]*engine.BlobAndProo if len(hashes) > 128 { return nil, engine.TooLargeRequest.With(fmt.Errorf("requested blob count too large: %v", len(hashes))) } - blobs, _, proofs, err := api.eth.BlobTxPool().GetBlobs(hashes, types.BlobSidecarVersion0) + blobs, _, proofs, err := api.eth.BlobTxPool().GetBlobs(hashes, types.BlobSidecarVersion0, false) if err != nil { return nil, engine.InvalidParams.With(err) } @@ -555,7 +555,7 @@ func (api *ConsensusAPI) GetBlobsV2(hashes []common.Hash) ([]*engine.BlobAndProo return nil, nil } - blobs, _, proofs, err := api.eth.BlobTxPool().GetBlobs(hashes, types.BlobSidecarVersion1) + blobs, _, proofs, err := api.eth.BlobTxPool().GetBlobs(hashes, types.BlobSidecarVersion1, false) if err != nil { return nil, engine.InvalidParams.With(err) }