core, internal, miner, signer: convert legacy sidecar in Osaka fork #32347
Conversation
rjl493456442
requested review from
MariusVanDerWijden,
fjl,
s1na and
lightclient
as code owners
| chainHead := api.b.CurrentHeader() | ||
| isOsaka := api.b.ChainConfig().IsOsaka(chainHead.Number, chainHead.Time) | ||
| if isOsaka { | ||
| config.blobSidecarVersion = types.BlobSidecarVersion1 |
There was a problem hiding this comment.
Note, the legacy proofs submitted by users will be implicitly converted to cell proofs.
| @@ -1603,7 +1630,7 @@ func (api *TransactionAPI) SignTransaction(ctx context.Context, args Transaction | |||
| // no longer retains the blobs, only the blob hashes. In this step, we need | |||
| // to put back the blob(s). | |||
| if args.IsEIP4844() { | |||
| signed = signed.WithBlobTxSidecar(types.NewBlobTxSidecar(types.BlobSidecarVersion0, args.Blobs, args.Commitments, args.Proofs)) | |||
| signed = signed.WithBlobTxSidecar(types.NewBlobTxSidecar(sidecarVersion, args.Blobs, args.Commitments, args.Proofs)) | |||
There was a problem hiding this comment.
Note, the legacy proofs submitted by users will be implicitly converted to cell proofs.
| @@ -425,21 +424,6 @@ func (miner *Miner) commitTransactions(env *environment, plainTxs, blobTxs *tran | |||
| if !env.txFitsSize(tx) { | |||
| break | |||
| } | |||
|
|
|||
| // Make sure all transactions after osaka have cell proofs | |||
| if isOsaka { | |||
There was a problem hiding this comment.
The conversion is unnecessary and useless.
If only affects the referenced transaction object, while the one in the blobpool/datadir won't be changed.
What really matters if the ones in the blobpool, for serving GetBlob APIs at the engine api.
There was a problem hiding this comment.
Besides, the conversion during block production is very dangerous, as it's time costy.
It's very likely to miss the slot due to the expensive proof generation.
| for i, tx := range txs { | ||
| if errs[i] != nil { | ||
| continue | ||
| } |
There was a problem hiding this comment.
One part is still missing:
During the Osaka activation, we should somehow convert the legacy sidecars to new format. It should be done in a following PR.
|
Ah looks like some test has been failing because of this |
This pull request implements #32235 , constructing blob sidecar in new format (cell proof)
if the Osaka has been activated.
Apart from that, it introduces a pre-conversion step in the blob pool before adding the txs.
This mechanism is essential for handling the remote legacy blob txs from the network.
One thing is still missing and probably is worthy being highlighted here: the blobpool may
contain several legacy blob txs before the Osaka and these txs should be converted once
Osaka is activated. While the
GetBlobAPI in blobpool is capable for generating cell proofsat the runtime, converting legacy txs at one time is much cheaper overall.