core/txpool/blobpool: add BlobTxForPool type by healthykim · Pull Request #34882 · ethereum/go-ethereum

healthykim · 2026-05-05T21:35:54Z

This PR introduces a separate transaction pool type for sparse blobpool.

In sparse blobpool, PooledTransactions message delivers transactions without
blobs, partial or full cells are downloaded by Cells message. Blobpool no longer
stores transactions with complete sidecars, and it stores transactions without
blobs, along with the corresponding cells. Because of this, a dedicated type
distinct from types.Transaction is required.

This PR introduces a type called BlobTxForPool and stores each sidecar field
independently, in order to bypass the assumption that a sidecar always exists as
a complete unit.

Reintroducing the conversion queue was considered, but was ultimately omitted
because type conversion should be sufficiently fast. With sparse blobpool, blob
-> cell computation would take about ~13ms per blob. Not sure whether this is
fast enough, but otherwise we can add the conversion queue later on the sparse
blobpool branch.

fjl · 2026-05-06T08:07:28Z

If it is at all possible, we should move this type into core/txpool/blobpool. The EncodeForNetwork function can be moved into the eth protocol package later, or it can stay in blobpool package. Either way, for GetRLP on the blobpool, we should document the returned RLP format.

fjl · 2026-05-06T09:33:19Z


-// GetRLP returns a RLP-encoded transaction for network if it is contained in the pool.
+// GetRLP returns a RLP-encoded transaction for p2p messages, converted from
+// the pool's internal type, if it is contained in the pool.


We have to make a small decision here. GetRLP is supposed to be an optimization that makes the pool return objects which are directly sendable in a p2p message. With the new format, this is not easily possible anymore. So the question now becomes, if we have to transform the format anyway, should we transform it here or in the p2p protocol implementation?

I think it's fine to have knowledge of the p2p encoding in the blobpool. But we should document the format returned by this method, i.e. say

// This method returns the RLP format used by the eth protocol: // txType || [..., version, blobs, commitments, proofs]

In the eth/72 protocol, the blobs in this encoding is always empty !! 😭
But @healthykim confirmed that we will later change this method to pass in the protocol version for which it should be encoded.

I think we can leave it like this here

I looked a bit more into where the GET operations are actually used.

Get is only used by the resolver, at least in blobpool. (legacypool also uses it at the P2P level for broadcasting. It is a bit tricky since they share the same interface.)

GetRLP is only used for P2P serving.

So I think the possible approach for sparse blobpool would be:

keep GetRLP as the P2P path and take a version number,

make Get return everything except sidecars,

split sidecar/cell queries into separate functions

fjl · 2026-05-06T09:37:22Z

+	txRLP := txBytes[1:]
+
+	// 2. Find the version of sidecar.
+	version, _, err := rlp.SplitString(elems[1])


Can it use rlp.SplitUint64? The version is an integer.

fjl · 2026-05-06T09:38:45Z

 }

+// recheck verifies the pool's content for a specific account and drops anything
+// that does not


this comment was duplicated accidentally

fjl · 2026-05-06T09:40:06Z

 	}
-	var tx types.Transaction
-	if err = rlp.DecodeBytes(data, &tx); err != nil {
+	ptx := new(blobTxForPool)


Style thing, why use new(T) instead of var ptx T

fjl · 2026-05-06T09:51:16Z

+		tx := new(types.Transaction)
+		if err := rlp.DecodeBytes(blob, tx); err != nil {
+			return false, err
+		}


Do we really have to do this? It is quite an expensive way to check. We could also just check if blob[0] == 3 or something.

fjl · 2026-05-06T09:59:26Z

+	if p.lookup.exists(meta.hash) {
+		return false, errors.New("duplicate blob entry")
+	}


This is duplicated in trackTransaction

fjl · 2026-05-06T10:01:38Z

+// blobTxForPool is the storage representation of a blob transaction in the
+// blobpool.
+type blobTxForPool struct {
+	Tx          *types.Transaction // tx without sidecar


We could embed this field. Then you can just call ptx.Hash().

fjl · 2026-05-06T10:27:22Z

 	TxHash common.Hash // Owner transaction's hash to support resurrecting reorged txs
 	Block  uint64      // Block in which the blob transaction was included
-	Tx     *types.Transaction
+	Ptx    *blobTxForPool


rename is good 🌼

…n and trackTransaction

fjl · 2026-05-07T12:34:52Z

+}
+
+// WithSidecar copies the sidecar's fields into the flat fields.
+func (ptx *blobTxForPool) WithSidecar(sc *types.BlobTxSidecar) {


We typically use the WithX naming for methods that copy the receiver and return it modified. This method just modifies the receiver so it should be called ApplySidecar.

fjl · 2026-05-07T12:55:53Z

-	var fails []uint64
+	var (
+		fails      []uint64
+		convertTxs []*types.Transaction


Just a note here, if the blobpool is at full capacity, this slice could potentially hold multiple gigabytes worth of transactions. I don't think it will occur in practice, but might be better to store the ids, then pull up the transactions from billy one-by-one for the conversion.

fjl · 2026-05-07T12:56:35Z

 	// Index all transactions on disk and delete anything unprocessable
-	var fails []uint64
+	var (
+		fails      []uint64


Also fails here is a bit strange name. The idea is that it contains the ids that will be deleted. So perhaps it should be called toDelete.

fjl · 2026-05-07T13:35:07Z

+	var ptx blobTxForPool
+	if err := rlp.DecodeBytes(blob, &ptx); err != nil {
+		kind, _, _, splitErr := rlp.Split(blob)
+		if splitErr == nil && kind == rlp.String {


you could still check for the first byte being 3 here, like this

func isLegacyBlobTx(b []byte) bool { k, content, _, err := rlp.Split(b) return err == nil && k == rlp.String && len(content) > 1 && content[0] == 3 }

It's basically free to check this.

fjl · 2026-05-07T13:36:07Z

+			return true, nil
+		}
+		return false, err


Actually, it's a bit strange to return a boolean. You could also return a special error errLegacyTx and then match on it like err == errLegacyTx in the caller.

cuiweixie · 2026-05-13T09:10:48Z

#34960. May you check this?

core: add BlobTxForPool type

bddf792

healthykim requested a review from rjl493456442 as a code owner May 5, 2026 21:35

healthykim added 2 commits May 6, 2026 10:28

core: move blobTxForPool type to blobpool.go

e3f25c2

core/txpool: add comments on GetRLP

c3ea9c6

fjl reviewed May 6, 2026

View reviewed changes

healthykim added 8 commits May 6, 2026 15:06

core/txpool/blobpool: add comments

a3f088e

core/txpool/blobpool: use SplitUint64 instead of SplitString

d1a0c5c

core/txpool/blobpool: remove duplicated comment

5905a2b

core/txpool/blobpool: use var ptx blobTxForPool instead of new()

c03867d

core/txpool/blobpool: fix bug on converTx append

3c8b108

core/txpool/blobpool: cheaper legacy tx detection in parseTransaction

66177b1

core/txpool/blobpool: add a test for conversion

a627a77

core/txpool/blobpool: remove duplicated check between parseTransactio…

296c236

…n and trackTransaction

fjl reviewed May 7, 2026

View reviewed changes

healthykim added 4 commits May 12, 2026 12:20

core/txpool/blobpool: rename ApplySidecar

9c376a4

core/txpool/blobpool: fix convertTxs to record ids instead of txs

723e730

core/txpool/blobpool: check the first byte on legacy tx check

f814ec3

core/txpool/blobpool: change parseTransaction to return errLegacyTx

11531f7

fjl approved these changes May 12, 2026

View reviewed changes

fjl added this to the 1.17.4 milestone May 12, 2026

fjl changed the title ~~core: add BlobTxForPool type~~ core/txpool/blobpool: add BlobTxForPool type May 12, 2026

fjl merged commit 726d657 into ethereum:master May 12, 2026
10 checks passed

Conversation

healthykim commented May 5, 2026

Uh oh!

fjl commented May 6, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fjl May 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cuiweixie commented May 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

fjl May 7, 2026 •

edited

Loading

cuiweixie commented May 13, 2026 •

edited

Loading