diff --git a/core/txpool/blobpool/blobpool.go b/core/txpool/blobpool/blobpool.go index 64ee3fcd9a..6fb114f7ef 100644 --- a/core/txpool/blobpool/blobpool.go +++ b/core/txpool/blobpool/blobpool.go @@ -1298,6 +1298,13 @@ func (p *BlobPool) GetMetadata(hash common.Hash) *txpool.TxMetadata { } // GetBlobs returns a number of blobs and proofs for the given versioned hashes. +// Blobpool must place responses in the order given in the request, using null for +// any missing blobs. +// +// For instance, if the request is [A_versioned_hash, B_versioned_hash, C_versioned_hash] +// and blobpool has data for blobs A and C, but doesn't have data for B, the +// response MUST be [A, null, C]. +// // 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) { @@ -1317,12 +1324,13 @@ func (p *BlobPool) GetBlobs(vhashes []common.Hash, version byte) ([]*kzg4844.Blo if _, ok := filled[vhash]; ok { continue } - // Retrieve the corresponding blob tx with the vhash + // Retrieve the corresponding blob tx with the vhash, skip blob resolution + // if it's not found locally and place the null instead. p.lock.RLock() txID, exists := p.lookup.storeidOfBlob(vhash) p.lock.RUnlock() if !exists { - return nil, nil, nil, fmt.Errorf("blob with vhash %x is not found", vhash) + continue } data, err := p.store.Get(txID) if err != nil { diff --git a/core/txpool/blobpool/blobpool_test.go b/core/txpool/blobpool/blobpool_test.go index c246928974..20e6571371 100644 --- a/core/txpool/blobpool/blobpool_test.go +++ b/core/txpool/blobpool/blobpool_test.go @@ -22,8 +22,10 @@ import ( "crypto/sha256" "errors" "fmt" + "github.com/ethereum/go-ethereum/internal/testrand" "math" "math/big" + "math/rand" "os" "path/filepath" "reflect" @@ -1814,10 +1816,10 @@ func TestGetBlobs(t *testing.T) { } cases := []struct { - start int - limit int - version byte - expErr bool + start int + limit int + fillRandom bool + version byte }{ { start: 0, limit: 6, @@ -1827,6 +1829,14 @@ func TestGetBlobs(t *testing.T) { start: 0, limit: 6, version: types.BlobSidecarVersion1, }, + { + start: 0, limit: 6, fillRandom: true, + version: types.BlobSidecarVersion0, + }, + { + start: 0, limit: 6, fillRandom: true, + version: types.BlobSidecarVersion1, + }, { start: 3, limit: 9, version: types.BlobSidecarVersion0, @@ -1835,6 +1845,14 @@ func TestGetBlobs(t *testing.T) { start: 3, limit: 9, version: types.BlobSidecarVersion1, }, + { + start: 3, limit: 9, fillRandom: true, + version: types.BlobSidecarVersion0, + }, + { + start: 3, limit: 9, fillRandom: true, + version: types.BlobSidecarVersion1, + }, { start: 3, limit: 15, version: types.BlobSidecarVersion0, @@ -1843,6 +1861,14 @@ func TestGetBlobs(t *testing.T) { start: 3, limit: 15, version: types.BlobSidecarVersion1, }, + { + start: 3, limit: 15, fillRandom: true, + version: types.BlobSidecarVersion0, + }, + { + start: 3, limit: 15, fillRandom: true, + version: types.BlobSidecarVersion1, + }, { start: 0, limit: 18, version: types.BlobSidecarVersion0, @@ -1852,58 +1878,79 @@ func TestGetBlobs(t *testing.T) { version: types.BlobSidecarVersion1, }, { - start: 18, limit: 20, + start: 0, limit: 18, fillRandom: true, version: types.BlobSidecarVersion0, - expErr: true, + }, + { + start: 0, limit: 18, fillRandom: true, + version: types.BlobSidecarVersion1, }, } for i, c := range cases { - var vhashes []common.Hash + var ( + vhashes []common.Hash + filled = make(map[int]struct{}) + ) + if c.fillRandom { + filled[len(vhashes)] = struct{}{} + vhashes = append(vhashes, testrand.Hash()) + } for j := c.start; j < c.limit; j++ { vhashes = append(vhashes, testBlobVHashes[j]) + if c.fillRandom && rand.Intn(2) == 0 { + filled[len(vhashes)] = struct{}{} + vhashes = append(vhashes, testrand.Hash()) + } + } + if c.fillRandom { + filled[len(vhashes)] = struct{}{} + vhashes = append(vhashes, testrand.Hash()) } blobs, _, proofs, err := pool.GetBlobs(vhashes, c.version) + if err != nil { + t.Errorf("Unexpected error for case %d, %v", i, err) + } - if c.expErr { - if err == nil { - t.Errorf("Unexpected return, want error for case %d", i) - } - } else { - if err != nil { - t.Errorf("Unexpected error for case %d, %v", i, err) - } - // Cross validate what we received vs what we wanted - length := c.limit - c.start - if len(blobs) != length || len(proofs) != length { - t.Errorf("retrieved blobs/proofs size mismatch: have %d/%d, want %d", len(blobs), len(proofs), length) + // Cross validate what we received vs what we wanted + length := c.limit - c.start + wantLen := length + len(filled) + if len(blobs) != wantLen || len(proofs) != wantLen { + t.Errorf("retrieved blobs/proofs size mismatch: have %d/%d, want %d", len(blobs), len(proofs), wantLen) + continue + } + + var unknown int + for j := 0; j < len(blobs); j++ { + if _, exist := filled[j]; exist { + if blobs[j] != nil || proofs[j] != nil { + t.Errorf("Unexpected blob and proof, item %d", j) + } + unknown++ continue } - for j := 0; j < len(blobs); j++ { - // 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]) - continue + // 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]) + continue + } + // Item retrieved, make sure the blob matches the expectation + if *blobs[j] != *testBlobs[c.start+j-unknown] { + 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] { + t.Errorf("retrieved proof mismatch: item %d, hash %x", j, vhashes[j]) } - // Item retrieved, make sure the blob matches the expectation - if *blobs[j] != *testBlobs[c.start+j] { - 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] { - t.Errorf("retrieved proof mismatch: item %d, hash %x", j, vhashes[j]) - } - } else { - want, _ := kzg4844.ComputeCellProofs(blobs[j]) - if !reflect.DeepEqual(want, proofs[j]) { - t.Errorf("retrieved proof mismatch: item %d, hash %x", j, vhashes[j]) - } + } else { + want, _ := kzg4844.ComputeCellProofs(blobs[j]) + if !reflect.DeepEqual(want, proofs[j]) { + t.Errorf("retrieved proof mismatch: item %d, hash %x", j, vhashes[j]) } } } } - pool.Close() } diff --git a/eth/catalyst/api.go b/eth/catalyst/api.go index 7f6dd40907..db3c9f3538 100644 --- a/eth/catalyst/api.go +++ b/eth/catalyst/api.go @@ -468,6 +468,10 @@ func (api *ConsensusAPI) GetBlobsV1(hashes []common.Hash) ([]*engine.BlobAndProo } res := make([]*engine.BlobAndProofV1, len(hashes)) for i := 0; i < len(blobs); i++ { + // Skip the non-existing blob + if blobs[i] == nil { + continue + } res[i] = &engine.BlobAndProofV1{ Blob: blobs[i][:], Proof: proofs[i][0][:], @@ -498,6 +502,10 @@ func (api *ConsensusAPI) GetBlobsV2(hashes []common.Hash) ([]*engine.BlobAndProo } res := make([]*engine.BlobAndProofV2, len(hashes)) for i := 0; i < len(blobs); i++ { + // Skip the non-existing blob + if blobs[i] == nil { + continue + } var cellProofs []hexutil.Bytes for _, proof := range proofs[i] { cellProofs = append(cellProofs, proof[:])