From 66a585c589fda05647992bdbcfdcbdc8aca617fd Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Tue, 8 Apr 2025 12:40:34 +0200 Subject: [PATCH] core/txpool: properly implement blobsv2 support --- beacon/engine/types.go | 10 ++-- beacon/engine/types_test.go | 3 +- core/txpool/blobpool/blobpool.go | 38 +++------------ core/txpool/blobpool/blobpool_test.go | 19 +++++++- core/txpool/legacypool/legacypool.go | 5 +- core/txpool/subpool.go | 3 +- core/txpool/txpool.go | 11 ++--- core/txpool/validation.go | 37 ++++++++++++-- core/types/tx_blob.go | 46 +++++++++++------- crypto/kzg4844/kzg4844.go | 2 + eth/catalyst/api.go | 69 ++++++++++++++++++++------- 11 files changed, 153 insertions(+), 90 deletions(-) diff --git a/beacon/engine/types.go b/beacon/engine/types.go index eb83ce130e..a492067d41 100644 --- a/beacon/engine/types.go +++ b/beacon/engine/types.go @@ -336,13 +336,9 @@ func BlockToExecutableData(block *types.Block, fees *big.Int, sidecars []*types. for j := range sidecar.Blobs { bundle.Blobs = append(bundle.Blobs, hexutil.Bytes(sidecar.Blobs[j][:])) bundle.Commitments = append(bundle.Commitments, hexutil.Bytes(sidecar.Commitments[j][:])) - if len(sidecar.CellProofs) == 0 { - bundle.Proofs = append(bundle.Proofs, hexutil.Bytes(sidecar.Proofs[j][:])) - } else { - for _, proof := range sidecar.CellProofs[j] { - bundle.Proofs = append(bundle.Proofs, hexutil.Bytes(proof[:])) - } - } + } + for _, proof := range sidecar.Proofs { + bundle.Proofs = append(bundle.Proofs, hexutil.Bytes(proof[:])) } } diff --git a/beacon/engine/types_test.go b/beacon/engine/types_test.go index 2fe06b6a40..7ffdf4e818 100644 --- a/beacon/engine/types_test.go +++ b/beacon/engine/types_test.go @@ -47,8 +47,7 @@ func TestBlobs(t *testing.T) { sidecarWithCellProofs := &types.BlobTxSidecar{ Blobs: []kzg4844.Blob{*emptyBlob}, Commitments: []kzg4844.Commitment{emptyBlobCommit}, - Proofs: []kzg4844.Proof{emptyBlobProof}, - CellProofs: [][]kzg4844.Proof{emptyCellProof}, + Proofs: emptyCellProof, } env = BlockToExecutableData(block, common.Big0, []*types.BlobTxSidecar{sidecarWithCellProofs}, nil) if len(env.BlobsBundle.Proofs) != 128 { diff --git a/core/txpool/blobpool/blobpool.go b/core/txpool/blobpool/blobpool.go index e0792d5ba4..87aefb9756 100644 --- a/core/txpool/blobpool/blobpool.go +++ b/core/txpool/blobpool/blobpool.go @@ -36,7 +36,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" @@ -1295,28 +1294,13 @@ func (p *BlobPool) GetMetadata(hash common.Hash) *txpool.TxMetadata { } } -// GetBlobs returns a number of blobs are proofs for the given versioned hashes. +// GetBlobs returns a number of blobs and proofs for the given versioned hashes. // 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) ([]*kzg4844.Blob, []*kzg4844.Proof, [][]kzg4844.Proof) { - // Create a map of the blob hash to indices for faster fills - var ( - blobs = make([]*kzg4844.Blob, len(vhashes)) - proofs = make([]*kzg4844.Proof, len(vhashes)) - cell_proofs = make([][]kzg4844.Proof, len(vhashes)) - ) - index := make(map[common.Hash]int) - for i, vhash := range vhashes { - index[vhash] = i - } - // Iterate over the blob hashes, pulling transactions that fill it. Take care - // to also fill anything else the transaction might include (probably will). - for i, vhash := range vhashes { - // If already filled by a previous fetch, skip - if blobs[i] != nil { - continue - } - // Unfilled, retrieve the datastore item (in a short lock) +func (p *BlobPool) GetBlobs(vhashes []common.Hash) []*types.BlobTxSidecar { + sidecars := make([]*types.BlobTxSidecar, len(vhashes)) + for idx, vhash := range vhashes { + // Retrieve the datastore item (in a short lock) p.lock.RLock() id, exists := p.lookup.storeidOfBlob(vhash) if !exists { @@ -1336,17 +1320,9 @@ func (p *BlobPool) GetBlobs(vhashes []common.Hash) ([]*kzg4844.Blob, []*kzg4844. log.Error("Blobs corrupted for traced transaction", "id", id, "err", err) continue } - // Fill anything requested, not just the current versioned hash - sidecar := item.BlobTxSidecar() - for j, blobhash := range item.BlobHashes() { - if idx, ok := index[blobhash]; ok { - blobs[idx] = &sidecar.Blobs[j] - proofs[idx] = &sidecar.Proofs[j] - cell_proofs[idx] = sidecar.CellProofs[j] - } - } + sidecars[idx] = item.BlobTxSidecar() } - return blobs, proofs, cell_proofs + return sidecars } // Add inserts a set of blob transactions into the pool if they pass validation (both diff --git a/core/txpool/blobpool/blobpool_test.go b/core/txpool/blobpool/blobpool_test.go index ed4ef963ae..060b241e25 100644 --- a/core/txpool/blobpool/blobpool_test.go +++ b/core/txpool/blobpool/blobpool_test.go @@ -417,8 +417,23 @@ func verifyBlobRetrievals(t *testing.T, pool *BlobPool) { for i := range testBlobVHashes { copy(hashes[i][:], testBlobVHashes[i][:]) } - blobs, proofs, _ := pool.GetBlobs(hashes) - + sidecars := pool.GetBlobs(hashes) + var blobs []*kzg4844.Blob + var proofs []*kzg4844.Proof + for idx, sidecar := range sidecars { + if sidecar == nil { + blobs = append(blobs, nil) + proofs = append(proofs, nil) + continue + } + blobHashes := sidecar.BlobHashes() + for i, hash := range blobHashes { + if hash == hashes[idx] { + blobs = append(blobs, &sidecar.Blobs[i]) + proofs = append(proofs, &sidecar.Proofs[i]) + } + } + } // Cross validate what we received vs what we wanted if len(blobs) != len(hashes) || len(proofs) != len(hashes) { t.Errorf("retrieved blobs/proofs size mismatch: have %d/%d, want %d", len(blobs), len(proofs), len(hashes)) diff --git a/core/txpool/legacypool/legacypool.go b/core/txpool/legacypool/legacypool.go index e5f354d1e6..f1143c850f 100644 --- a/core/txpool/legacypool/legacypool.go +++ b/core/txpool/legacypool/legacypool.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" @@ -1065,8 +1064,8 @@ func (pool *LegacyPool) GetMetadata(hash common.Hash) *txpool.TxMetadata { // GetBlobs is not supported by the legacy transaction pool, it is just here to // implement the txpool.SubPool interface. -func (pool *LegacyPool) GetBlobs(vhashes []common.Hash) ([]*kzg4844.Blob, []*kzg4844.Proof, [][]kzg4844.Proof) { - return nil, nil, nil +func (pool *LegacyPool) GetBlobs(vhashes []common.Hash) []*types.BlobTxSidecar { + return nil } // Has returns an indicator whether txpool has a transaction cached with the diff --git a/core/txpool/subpool.go b/core/txpool/subpool.go index 95607b9bc8..70595934a5 100644 --- a/core/txpool/subpool.go +++ b/core/txpool/subpool.go @@ -23,7 +23,6 @@ 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" "github.com/holiman/uint256" ) @@ -136,7 +135,7 @@ type SubPool interface { // GetBlobs returns a number of blobs are proofs for the given versioned hashes. // This is a utility method for the engine API, enabling consensus clients to // retrieve blobs from the pools directly instead of the network. - GetBlobs(vhashes []common.Hash) ([]*kzg4844.Blob, []*kzg4844.Proof, [][]kzg4844.Proof) + GetBlobs(vhashes []common.Hash) []*types.BlobTxSidecar // ValidateTxBasics checks whether a transaction is valid according to the consensus // rules, but does not check state-dependent validation such as sufficient balance. diff --git a/core/txpool/txpool.go b/core/txpool/txpool.go index 4e62b5a839..5bc83bab58 100644 --- a/core/txpool/txpool.go +++ b/core/txpool/txpool.go @@ -26,7 +26,6 @@ import ( "github.com/ethereum/go-ethereum/core" "github.com/ethereum/go-ethereum/core/state" "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/params" @@ -311,17 +310,17 @@ func (p *TxPool) GetMetadata(hash common.Hash) *TxMetadata { // GetBlobs returns a number of blobs are proofs for the given versioned hashes. // 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 *TxPool) GetBlobs(vhashes []common.Hash) ([]*kzg4844.Blob, []*kzg4844.Proof, [][]kzg4844.Proof) { +func (p *TxPool) GetBlobs(vhashes []common.Hash) []*types.BlobTxSidecar { for _, subpool := range p.subpools { // It's an ugly to assume that only one pool will be capable of returning - // anything meaningful for this call, but anythingh else requires merging + // anything meaningful for this call, but anything else requires merging // partial responses and that's too annoying to do until we get a second // blobpool (probably never). - if blobs, proofs, cellProofs := subpool.GetBlobs(vhashes); blobs != nil { - return blobs, proofs, cellProofs + if sidecars := subpool.GetBlobs(vhashes); sidecars != nil { + return sidecars } } - return nil, nil, nil + return nil } // Add enqueues a batch of transactions into the pool if they are valid. Due diff --git a/core/txpool/validation.go b/core/txpool/validation.go index e370f2ce84..da4828eab8 100644 --- a/core/txpool/validation.go +++ b/core/txpool/validation.go @@ -152,9 +152,16 @@ func ValidateTransaction(tx *types.Transaction, head *types.Header, signer types if len(hashes) > maxBlobs { return fmt.Errorf("too many blobs in transaction: have %d, permitted %d", len(hashes), maxBlobs) } - // Ensure commitments, proofs and hashes are valid - if err := validateBlobSidecar(hashes, sidecar); err != nil { - return err + if opts.Config.IsOsaka(head.Number, head.Time) { + // Ensure commitments, cell proofs and hashes are valid + if err := validateBlobSidecarOsaka(hashes, sidecar); err != nil { + return err + } + } else { + // Ensure commitments, proofs and hashes are valid + if err := validateBlobSidecar(hashes, sidecar); err != nil { + return err + } } } if tx.Type() == types.SetCodeTxType { @@ -185,6 +192,30 @@ func validateBlobSidecar(hashes []common.Hash, sidecar *types.BlobTxSidecar) err return nil } +func validateBlobSidecarOsaka(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)) + } + if len(sidecar.Proofs)*kzg4844.CellProofsPerBlob != len(hashes) { + return fmt.Errorf("invalid number of %d blob proofs compared to %d blob hashes", len(sidecar.Proofs), len(hashes)) + } + if err := sidecar.ValidateBlobCommitmentHashes(hashes); err != nil { + return err + } + // Blob commitments match with the hashes in the transaction, verify the + // blobs themselves via KZG + for i := range sidecar.Blobs { + // TODO verify the cell proof here + _ = i + /* + 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 +} + // ValidationOptionsWithState define certain differences between stateful transaction // validation across the different pools without having to duplicate those checks. type ValidationOptionsWithState struct { diff --git a/core/types/tx_blob.go b/core/types/tx_blob.go index 393ad84978..742539b8b4 100644 --- a/core/types/tx_blob.go +++ b/core/types/tx_blob.go @@ -58,7 +58,6 @@ 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 - CellProofs [][]kzg4844.Proof // Cell proofs } // BlobHashes computes the blob hashes of the given blobs. @@ -71,6 +70,20 @@ func (sc *BlobTxSidecar) BlobHashes() []common.Hash { return h } +// CellProofsAt returns the cell proofs for blob with index idx. +func (sc *BlobTxSidecar) CellProofsAt(idx int) []kzg4844.Proof { + var cellProofs []kzg4844.Proof + for i := range kzg4844.CellProofsPerBlob { + index := idx*kzg4844.CellProofsPerBlob + i + if index > len(sc.Proofs) { + return nil + } + proof := sc.Proofs[index] + cellProofs = append(cellProofs, proof) + } + return cellProofs +} + // 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 { @@ -111,6 +124,14 @@ type blobTxWithBlobs struct { Proofs []kzg4844.Proof } +type versionedBlobTxWithBlobs struct { + Version byte + 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{ @@ -158,15 +179,10 @@ func (tx *BlobTx) copy() TxData { cpy.S.Set(tx.S) } if tx.Sidecar != nil { - cellProofs := make([][]kzg4844.Proof, len(tx.Sidecar.CellProofs)) - for i := range tx.Sidecar.CellProofs { - cellProofs[i] = append([]kzg4844.Proof(nil), tx.Sidecar.CellProofs[i]...) - } 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...), - CellProofs: cellProofs, } } return cpy @@ -252,26 +268,24 @@ func (tx *BlobTx) decode(input []byte) error { 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 - // compute the cell proof - cellProofs := make([][]kzg4844.Proof, 0) - for i := range len(inner.Blobs) { - cellProof, err := kzg4844.ComputeCells(&inner.Blobs[i]) - if err != nil { + var innerWithVersion versionedBlobTxWithBlobs + if err := rlp.DecodeBytes(input, &innerWithVersion); err != nil { return err } - cellProofs = append(cellProofs, cellProof) + inner.BlobTx = innerWithVersion.BlobTx + inner.Blobs = innerWithVersion.Blobs + inner.Commitments = innerWithVersion.Commitments + inner.Proofs = innerWithVersion.Proofs } + *tx = *inner.BlobTx tx.Sidecar = &BlobTxSidecar{ Blobs: inner.Blobs, Commitments: inner.Commitments, Proofs: inner.Proofs, - CellProofs: cellProofs, } return nil } diff --git a/crypto/kzg4844/kzg4844.go b/crypto/kzg4844/kzg4844.go index 264bba8ece..8e578bf79d 100644 --- a/crypto/kzg4844/kzg4844.go +++ b/crypto/kzg4844/kzg4844.go @@ -34,6 +34,8 @@ var ( blobT = reflect.TypeOf(Blob{}) commitmentT = reflect.TypeOf(Commitment{}) proofT = reflect.TypeOf(Proof{}) + + CellProofsPerBlob = 128 ) // Blob represents a 4844 data blob. diff --git a/eth/catalyst/api.go b/eth/catalyst/api.go index 210b4cb4e4..13b85fc454 100644 --- a/eth/catalyst/api.go +++ b/eth/catalyst/api.go @@ -18,6 +18,7 @@ package catalyst import ( + "crypto/sha256" "errors" "fmt" "strconv" @@ -32,6 +33,7 @@ import ( "github.com/ethereum/go-ethereum/core/stateless" "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/core/vm" + "github.com/ethereum/go-ethereum/crypto/kzg4844" "github.com/ethereum/go-ethereum/eth" "github.com/ethereum/go-ethereum/eth/ethconfig" "github.com/ethereum/go-ethereum/internal/version" @@ -558,14 +560,28 @@ 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))) } - res := make([]*engine.BlobAndProofV1, len(hashes)) + var ( + res = make([]*engine.BlobAndProofV1, len(hashes)) + hasher = sha256.New() + index = make(map[common.Hash]int) + sidecars = api.eth.TxPool().GetBlobs(hashes) + ) - blobs, proofs, _ := api.eth.TxPool().GetBlobs(hashes) - for i := 0; i < len(blobs); i++ { - if blobs[i] != nil { - res[i] = &engine.BlobAndProofV1{ - Blob: (*blobs[i])[:], - Proof: (*proofs[i])[:], + for i, hash := range hashes { + index[hash] = i + } + for i, sidecar := range sidecars { + if res[i] != nil { + // already filled + continue + } + for cIdx, commitment := range sidecar.Commitments { + computed := kzg4844.CalcBlobHashV1(hasher, &commitment) + if idx, ok := index[computed]; ok { + res[idx] = &engine.BlobAndProofV1{ + Blob: sidecar.Blobs[cIdx][:], + Proof: sidecar.Proofs[cIdx][:], + } } } } @@ -577,18 +593,35 @@ func (api *ConsensusAPI) GetBlobsV2(hashes []common.Hash) ([]*engine.BlobAndProo if len(hashes) > 128 { return nil, engine.TooLargeRequest.With(fmt.Errorf("requested blob count too large: %v", len(hashes))) } - res := make([]*engine.BlobAndProofV2, len(hashes)) + var ( + res = make([]*engine.BlobAndProofV2, len(hashes)) + index = make(map[common.Hash]int) + sidecars = api.eth.TxPool().GetBlobs(hashes) + ) - blobs, _, cellProofs := api.eth.TxPool().GetBlobs(hashes) - for i := 0; i < len(blobs); i++ { - if blobs[i] != nil { - var proofs []hexutil.Bytes - for _, proof := range cellProofs[i] { - proofs = append(proofs, hexutil.Bytes(proof[:])) - } - res[i] = &engine.BlobAndProofV2{ - Blob: (*blobs[i])[:], - CellProofs: proofs, + for i, hash := range hashes { + index[hash] = i + } + for i, sidecar := range sidecars { + if res[i] != nil { + // already filled + continue + } + if len(sidecar.Blobs) != len(sidecar.Proofs)*kzg4844.CellProofsPerBlob { + return nil, errors.New("NORMAL PROOFS IN GETBLOBSV2, THIS SHOULD NEVER HAPPEN, PLEASE REPORT") + } + blobHashes := sidecar.BlobHashes() + for bIdx, hash := range blobHashes { + if idx, ok := index[hash]; ok { + proofs := sidecar.CellProofsAt(bIdx) + var cellProofs []hexutil.Bytes + for _, proof := range proofs { + cellProofs = append(cellProofs, proof[:]) + } + res[idx] = &engine.BlobAndProofV2{ + Blob: sidecar.Blobs[bIdx][:], + CellProofs: cellProofs, + } } } }