core/txpool/blobpool: Do not use blobs with wrong sidecar version while building block after Osaka #32577
Conversation
needed for filtering logic in block production Signed-off-by: Csaba Kiraly <[email protected]>
core/txpool/subpool.go
Outdated
| OnlyPlainTxs bool // Return only plain EVM transactions (peer-join announces, block space filling) | ||
| OnlyBlobV0Txs bool // Return only V0 encoded blob transactions (block blob-space filling) | ||
| OnlyBlobV1Txs bool // Return only V1 encoded blob transactions (block blob-space filling) |
There was a problem hiding this comment.
Changing this API seems overkill since it is only useful for a short period around the fork boundary. I would just skip over v0 blob sidecars in the miner worker after Osaka and we can remove it again later on after the fork.
There was a problem hiding this comment.
worker does not interface directly to the blobpool, only to the txpool, so I think getting a sidecar version would require even more API changes. It would either need to relay a call to the blobpool (PendingV1Only), or we would need to add blob version info to the LazyTransaction. Both seems messy to me compared to this.
Signed-off-by: Csaba Kiraly <[email protected]>
Signed-off-by: Csaba Kiraly <[email protected]>
cskiraly
force-pushed
the
blobtx-build-version-fix
branch
from
356e39a to
b98b69e
Compare
|
I've messed up the commit before, missing the miner/worker part. Fixed now. |
core/txpool/subpool.go
Outdated
| OnlyPlainTxs bool // Return only plain EVM transactions (peer-join announces, block space filling) | ||
| OnlyBlobTxs bool // Return only blob transactions (block blob-space filling) | ||
| OnlyPlainTxs bool // Return only plain EVM transactions (peer-join announces, block space filling) | ||
| OnlyBlobV0Txs bool // Return only V0 encoded blob transactions (block blob-space filling) |
There was a problem hiding this comment.
Min sounds good at first, but actually the only thing we need in the current usage is an exact BlobVersion.
Signed-off-by: Csaba Kiraly <[email protected]>
| // When BlobTxs true, return only blob transactions (block blob-space filling) | ||
| // when false, return only non-blob txs (peer-join announces, block space filling) | ||
| BlobTxs bool | ||
| BlobVersion byte // Blob tx version to include. 0 means pre-Osaka, 1 means Osaka and later |
There was a problem hiding this comment.
I still think the versioning / changing the api is overkill?
There was a problem hiding this comment.
I don't think there is a cleaner way to do this filtering, but maybe I'm wrong here.
There was a problem hiding this comment.
We discussed this on triage and the filter is what we came up with
There was a problem hiding this comment.
So we could also do the filtering in commitBlobTransactions, but
- that would require loading the sidecars
- the filtering would be hidden in that function, instead of putting it where we already do filtering and where it logically fits.
I think the current version is better.
As a consequence of moving blob sidecar version migration code around, we ended up building blocks
with a mix of v0 and v1 blob transactions (different proof encoding in the sidecar).
This PR makes sure we are not building illegal blocks after Osaka. Blob migration is left for another PR.
Related issues and PRs: