EIP-8141: support blob-carrying frame transactions (validate correctly, produce conservatively) - #12701
EIP-8141: support blob-carrying frame transactions (validate correctly, produce conservatively)#12701Marchhill wants to merge 4 commits into
Conversation
…y, produce conservatively) Align blob accounting for EIP-8141 frame transactions with the spec (~L492-499, ethereum/EIPs#11985) and EELS #3047: - Instance-level blob gate at the two block accounting sites (BlockValidator EIP-4844 loop, BlobGasCalculator.CalculateBlobGas) so a blob-carrying frame tx counts towards header.BlobGasUsed and the per-block blob budget, like type-3. TxTypeExtensions.SupportsBlobs() is left untouched (networking/decode paths). - Charge and burn the blob fee (blob_gas x blob_base_fee) in the frame processor, never refunded; enforce max_fee_per_blob_gas >= blob_base_fee. The max_cost blob leg is priced at the actual blob_base_fee for parity with EELS, matching both the payer escrow observed mid-transaction and TXPARAM_MAX_COST (0x06). - Add the blob term to the mempool max-cost/affordability check for frame txs. - Guard block production: the picker conservatively skips blob-carrying frame txs until they are metered against the block blob budget. Sidecar/networking wrapper, blob-pool routing, mempool blob-budget selection, and the per-tx blob-count/versioned-hash static validation are deferred (EIP-7594 slice).
|
Claude finished @Marchhill's task in 8m 18s —— View job Code review — EIP-8141 blob-carrying frame transactions
VerdictThe core mechanics are right and the arithmetic checks out. What I'd want addressed before merge is that the PR flips the blob-frame-tx path from fail-closed to fail-open: the validation set that would make "validate correctly" true isn't there yet. What I verified as correct:
Findings1 — High: the deferred validation is now a chain-split vector
Before this PR that was harmless: blob gas wasn't counted, so any block another client accepted failed Nethermind's The 2 — Medium: mempool side is priced right but still admits unincludable txs
3 — Medium:
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| // EIP8141: the per-transaction blob-count limit (EIP-7594) and the versioned-hash-version | ||
| // check that EELS #3047 validate_frame_transaction applies to blob-carrying frame txs are not | ||
| // enforced here yet — deferred with the rest of the EIP-7594 slice. A blob-carrying frame tx | ||
| // is still bounded by the block-level MaxBlobGasPerBlock check in BlockValidator, so it cannot | ||
| // over-fill a block; the gap is only that a single tx may carry more blobs than EELS allows. | ||
|
|
There was a problem hiding this comment.
High — deferring these two checks now costs a consensus split, not just laxness.
Before this PR a blob-carrying frame tx contributed nothing to header.BlobGasUsed, so any block containing one that another client accepted would fail Nethermind's HeaderBlobGasMismatch check — the divergence was fail-closed. After this PR Nethermind accepts such blocks, so every remaining validation gap becomes an accept-invalid-block gap:
TxValidatorregisters no blob validation forTxType.FrameTx(TxValidator.cs:90-96), andMaxBlobCountBlobTxValidatorshort-circuits on{ Type: not TxType.Blob }(TxValidator.cs:305). So neitherMaxBlobGasPerTx(EIP-7594 per-tx cap) nor the0x01versioned-hash version byte is enforced anywhere for a frame tx.- Concrete failure: a frame tx carrying
MaxBlobsPerTx + 1blobs, in a block whose total blob gas is still ≤MaxBlobGasPerBlock. EELSvalidate_frame_transactionrejects it; Nethermind now accepts and follows that chain. Same for a versioned hash whose first byte is not0x01.
The comment's claim that "the gap is only that a single tx may carry more blobs than EELS allows" understates it — that is the split vector, and the version-byte gap is a second one.
The fix is additive and cheap: register one more ITxValidator in the TxType.FrameTx list that reuses the existing logic, gated on hasBlobs:
public sealed class FrameTxBlobFieldsValidator : ITxValidator
{
public static readonly FrameTxBlobFieldsValidator Instance = new();
private FrameTxBlobFieldsValidator() { }
public ValidationResult IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec) =>
transaction.BlobVersionedHashes is not { Length: > 0 }
? ValidationResult.Success
: BlobFieldsTxValidator.ValidateBlobFieldsForBlobCarryingTx(transaction, releaseSpec);
}(extracting the ValidateBlobGasLimits + version-byte loop out of BlobFieldsTxValidator.ValidateBlobFields so both call sites share it).
If you'd rather keep the EIP-7594 slice fully deferred, the conservative counterpart to the production guard is to reject blob-carrying frame txs in FrameTxValidation for now — that keeps the fail-closed property while the block accounting lands.
| // EIP-8141: a blob-carrying frame transaction (type 6) also reserves the blob fee, so gate | ||
| // on the presence of blob hashes rather than the type-level SupportsBlobs (type-3 only). | ||
| // Priced at max_fee_per_blob_gas, an upper bound on the processor's escrow, so mempool | ||
| // affordability never admits a tx the processor cannot charge. | ||
| if (tx.BlobVersionedHashes is { Length: > 0 }) | ||
| { | ||
| // if tx.SupportsBlobs and has BlobVersionedHashes = null, it will throw on earlier step of validation, in TxValidator | ||
| overflow |= UInt256.MultiplyOverflow(Eip4844Constants.GasPerBlob, (UInt256)tx.BlobVersionedHashes!.Length, out UInt256 blobGas); | ||
| overflow |= UInt256.MultiplyOverflow(Eip4844Constants.GasPerBlob, (UInt256)tx.BlobVersionedHashes.Length, out UInt256 blobGas); | ||
| overflow |= UInt256.MultiplyOverflow(blobGas, tx.MaxFeePerBlobGas ?? UInt256.MaxValue, out UInt256 blobGasCost); | ||
| overflow |= UInt256.AddOverflow(cumulativeCost, blobGasCost, out cumulativeCost); | ||
| } |
There was a problem hiding this comment.
Medium — the pool now prices these txs correctly but still admits and gossips txs no node can ever include.
SupportsBlobs being left type-level means, for a blob-carrying frame tx:
TxTypeTxFilter/BalanceTooLowFilter/FeeTooLowFilterroute it into_transactions, not_blobTransactions.CanBeBroadcast(tx)=!tx.SupportsBlobs && …→ true, so the node re-broadcasts it as a plain tx with no sidecar. There is no frame-tx network wrapper yet, so no peer can ever obtain the blobs.CanPayForBlobGas(tx, currentBlobBaseFee)=!tx.SupportsBlobs || …→ true unconditionally, so the pool never evicts it when the head blob base fee rises above itsMaxFeePerBlobGas, even though the processor will now reject it for exactly that reason.- And with the new picker guard, Nethermind itself will never include it.
Net effect: an unincludable tx class that occupies pool slots and gets amplified across the network. Given the "produce conservatively" stance, the symmetric move is to reject blob-carrying frame txs at pool entry (a NotSupportedTxFilter-style filter) until the EIP-7594 slice lands, rather than admitting them. At minimum, CanBeBroadcast should exclude them.
| // EIP-4844 fee-collector chains (e.g. Gnosis) collect the burned blob fee instead of losing | ||
| // it, consistent with the regular path's PayFees; elsewhere the fee is simply burned. | ||
| if (!blobFee.IsZero && spec.IsEip4844FeeCollectorEnabled && spec.FeeCollector is not null) | ||
| { | ||
| WorldState.AddToBalance(payer, maxCost - spentCost, spec); | ||
| WorldState.AddToBalanceAndCreateIfNotExists(spec.FeeCollector, blobFee, spec); | ||
| } |
There was a problem hiding this comment.
Medium — this makes the frame path's fee-collector support half-complete.
PayFees on the regular path routes eip1559Fees + blobBaseFee to spec.FeeCollector. The frame path routes only the blob fee; the EIP-1559 base-fee share of a frame tx stays burned (it never leaves the maxCost escrow). On a fee-collector chain (Gnosis-style, IsEip4844FeeCollectorEnabled == true) that's a state-root divergence for every frame tx, blob-carrying or not.
The base-fee half is pre-existing (the frame path never touched the collector before), so this isn't a regression — but adding the blob leg makes the code read as if collector support is complete. Either route both legs here (UInt256.Min(header.BaseFeePerGas, effectiveGasPrice) * spentGas when spec.IsEip1559Enabled && !tx.IsFree(), mirroring PayFees), or leave an explicit marker that the 1559 leg is still unrouted so the next reader doesn't assume parity.
| [Test] | ||
| public void Execute_BlobCarryingFrameTx_ChargesAndBurnsBlobFee() | ||
| { | ||
| // EIP-8141 (spec ~L492-499): a blob-carrying frame tx follows EIP-4844 — the payer covers the | ||
| // blob fee, which is burned. With base fee 0 the whole gas premium goes to the beneficiary, so | ||
| // the only value that leaves the payer for good is the burned blob fee. Parity: EELS #3047. | ||
| DeploySmartSender(ApproveCode(TxFrame.ApproveExecutionAndPayment)); | ||
| Transaction tx = FrameTx(nonce: 0, SelfVerifyFrame()); | ||
| tx.BlobVersionedHashes = [new byte[32]]; | ||
| tx.MaxFeePerBlobGas = 1000; | ||
|
|
||
| CallOutputTracer tracer = new(); | ||
| TransactionResult result = ProcessWithBlobHeader(tx, excessBlobGas: 0, tracer: tracer); | ||
|
|
||
| Assert.That(result.TransactionExecuted, Is.True); | ||
| UInt256 spentGas = (UInt256)tracer.GasSpent; | ||
| UInt256 blobFee = ExpectedBlobFee(excessBlobGas: 0, blobCount: 1); | ||
| Assert.That(blobFee, Is.GreaterThan(UInt256.Zero), "blob fee is nonzero at the minimum blob base fee"); | ||
| using (Assert.EnterMultipleScope()) | ||
| { | ||
| Assert.That(_stateProvider.GetBalance(Beneficiary), Is.EqualTo(spentGas), "beneficiary gets the gas premium only"); | ||
| Assert.That(_stateProvider.GetBalance(Sender), Is.EqualTo(1.Ether - spentGas - blobFee), "payer pays the spent gas and the blob fee"); | ||
| Assert.That(_stateProvider.GetBalance(Sender) + _stateProvider.GetBalance(Beneficiary), | ||
| Is.EqualTo(1.Ether - blobFee), "the blob fee is burned, not paid to the beneficiary"); |
There was a problem hiding this comment.
Medium (test coverage) — the PR's central consensus claim is the one thing untested.
The description says the blob leg of max_cost is priced at the actual blob_base_fee (not max_fee_per_blob_gas) specifically to keep TXPARAM_MAX_COST (0x06) in parity with EELS #3047. This test asserts end-of-tx balances, which are identical under either pricing once the refund cancels the escrow — it cannot distinguish the two. The two consensus-visible values that can differ are:
TXPARAM_MAX_COSTas read by contract code mid-transaction, and- the payer solvency gate in
ApplyApproval(WorldState.GetBalance(resolvedTarget) < frameContext.MaxCost) — a payer funded betweengasLimit × maxFeePerGas + blobGas × blobBaseFeeand… + blobGas × maxFeePerBlobGaspays under one pricing and voids payment under the other.
Worth adding a case that reads TXPARAM 0x06 from a frame and asserts it equals txGasLimit × maxFeePerGas + blobGas × blobBaseFee with maxFeePerBlobGas set well above the base fee, plus a boundary case on the solvency gate. Otherwise a later change to either pricing passes CI silently.
Also worth a linked EELS/EIP line reference for the pricing choice — the inline comment cites "EELS #3047" but not the line, and this is a consensus-visible value.
| // EIP-8141: include blob-carrying frame transactions (type 6), not just type-3, in the | ||
| // block's blob gas — gate on blob hashes instead of the type-level SupportsBlobs. | ||
| if (tx.BlobVersionedHashes is { Length: > 0 }) |
There was a problem hiding this comment.
Low (simplification) — the guard is now redundant: GetBlobCount() is BlobVersionedHashes?.Length ?? 0, so the branch only skips adding zero. Per the repo preference for removing code over adding it, the loop body collapses to blobCount += (ulong)tx.GetBlobCount(); and the if/braces go away.
| // EIP-4844: max_fee_per_blob_gas must cover the current blob base fee, else the tx is invalid. | ||
| if (tx.MaxFeePerBlobGas.GetValueOrDefault() < feePerBlobGas) | ||
| { | ||
| TraceLogInvalidTx(tx, "INSUFFICIENT_MAX_FEE_PER_BLOB_GAS"); | ||
| return TransactionResult.ErrorType.InsufficientMaxFeePerGasForSenderBalance.WithDetail( | ||
| BlockErrorMessages.InsufficientMaxFeePerBlobGas(tx.SenderAddress, tx.MaxFeePerBlobGas, feePerBlobGas)); | ||
| } |
There was a problem hiding this comment.
Low — the regular path returns ErrorType.InsufficientSenderBalance for this exact condition (TransactionProcessor.cs:1109), this returns InsufficientMaxFeePerGasForSenderBalance. Both map to ErrorCodes.InsufficientFunds in SimulateTxExecutor, so nothing breaks, but the divergence is gratuitous and the new test pins it in. Suggest matching the regular path so debug_*/eth_simulateV1 error classification is type-independent.
| // EIP8141: blob-carrying frame transactions sit in the normal pool and are not yet | ||
| // metered against the block blob budget during production (deferred: blob-pool routing | ||
| // and blob-budget selection). Now that they count towards header.BlobGasUsed, an | ||
| // unguarded producer could exceed MaxBlobGasPerBlock and self-invalidate the block. | ||
| // Exclude them conservatively until block production tracks their blob gas. | ||
| if (currentTx.Type == TxType.FrameTx && currentTx.BlobVersionedHashes is { Length: > 0 }) | ||
| { | ||
| return args.Set(TxAction.Skip, "Blob-carrying frame transaction not yet supported in block production"); | ||
| } |
There was a problem hiding this comment.
Low — two notes on this guard:
-
HasEnoughFundswas not updated. Line 129 still gates the blob-fee reservation ontransaction.SupportsBlobs, so it silently under-reserves for a blob-carrying frame tx. That's harmless only because this guard short-circuits first. When the guard is removed, the funds check will be wrong and nothing here points at it — worth a marker at line 129 or extending it toBlobVersionedHashes is { Length: > 0 }now (it's a no-op today and correct later). -
Placement. Per
AGENTS.md, altering behaviour additively is preferred over editing shared code. A blob-carrying-frame-tx filter belongs more naturally in the tx source (FilteredTxSource/TxPoolTxSource) than in the picker: these txs currently still consume a selection slot inTxPoolTxSourceand are re-tried on every block, and the picker is a hot path shared by every chain and every tx. Not blocking — the picker is the last enforcement point and a temporary guard here is defensible — but worth a sentence on why this spot was chosen.
Also: TxAction.Skip leaves these txs in the pool forever (nothing ever evicts them, see the CanPayForBlobGas note on TransactionExtensions.cs), so the guard alone doesn't bound pool growth.
| // EIP-8141: a blob-carrying frame transaction (type 6) follows EIP-4844 too, so gate on the | ||
| // presence of blob hashes rather than the type-level SupportsBlobs (which is type-3 only). | ||
| if (transaction.BlobVersionedHashes is not { Length: > 0 }) | ||
| { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Low — this loop is now the only place a blob-carrying frame tx's MaxFeePerBlobGas >= feePerBlobGas is checked at block level, and the processor re-checks it independently (TransactionProcessorBase.FrameTx.cs:104) with a different error type. That's fine and matches type-3, but note that blobsInBlock is an int accumulating BlobVersionedHashes.Length across txs — the frame-tx decoder allows up to 128 hashes per tx (FrameTxDecoder.cs:31) versus type-3's per-tx cap, so with many frame txs this is a wider input range than before. It still can't realistically overflow int (a block's tx count bounds it), but it's another argument for enforcing the per-tx blob cap at IsWellFormed time (see the FrameTxValidation.cs comment) rather than only the block total here.
EVM Opcode Benchmark DiffAggregated runs: base=3, pr=3 No significant regressions or improvements detected. |
…me txs Enforce the EIP-7594 per-tx blob-count limit (BLOB_COUNT_LIMIT) and the versioned-hash version byte (0x01) on blob-carrying frame transactions, matching EELS validate_frame_transaction. Reuses the type-3 blob checks via the shared BlobFieldsTxValidator helpers in FrameTxFieldsTxValidator, so type-3 behaviour is unchanged.
|
Added the last |
|
Claude finished @Marchhill's task in 6m 1s —— View job Code review — EIP-8141 blob-carrying frame txs + EIP-7594 validation (at
|
| # | Severity | Finding | Status |
|---|---|---|---|
| 1 | Missing per-tx blob validation for frame txs | ✅ fixed in 86cbfbe |
|
| 2 | Medium | Blob-carrying frame txs are admitted, gossiped without sidecars, and never evicted | open |
| 3 | Medium | The max_cost blob pricing — the PR's central consensus choice — is still untestable by the tests added |
open |
| 4 | Low | MaxFeePerBlobGas: null unvalidated for frame txs + public widening |
new |
| 5 | Low | Error-type divergence + half-complete fee-collector routing | open |
| 6 | Low | HasEnoughFunds still SupportsBlobs-gated |
open |
| 7 | Low | Test DRY: parameterize the three blob-count cases, assert error messages · CalculateBlobGas guard redundant |
new / open |
#2 (Medium) — with the new validator, the pool now rejects malformed blob frame txs, but a well-formed one is still admitted, CanBeBroadcast → true (re-gossiped as a plain tx that no peer can ever get blobs for), CanPayForBlobGas → true unconditionally (never evicted when the head blob base fee passes its MaxFeePerBlobGas — exactly what the processor now rejects it for), and NotSupportedTxFilter misses it even with BlobsSupport = Disabled. The PR's deferral list covers blob-pool routing and blob-budget selection; the gossip and never-evict behaviours aren't in it and are the ones with an outward effect.
#3 (Medium) — Execute_BlobCarryingFrameTx_ChargesAndBurnsBlobFee asserts end-of-tx balances, which are identical whether the max_cost blob leg is priced at blob_base_fee or max_fee_per_blob_gas; the refund cancels the difference. The two values that differ are TXPARAM_MAX_COST (0x06) read mid-tx and the ApplyApproval solvency gate (FrameTx.cs:624) — the latter is a state-root difference, not just RPC. Neither is covered.
Verified correct
- Refund arithmetic cannot underflow.
maxCost = txGasLimit × maxFeePerGas + blobFeeis overflow-checked;effectiveGasPrice ≤ maxFeePerGas,spentGas ≤ txGasLimit⇒spentCost + blobFee ≤ maxCost. - No double burn on a batch unroll.
ApplyApprovalchargesMaxCostonce (line 618 rejects a second payer); an unroll reverts the debit viaWorldState.Restoreand resetsPayer, and a later approval re-charges the wholemaxCost. BlobBaseFeeOverrideCalculatorDecoratorcomposes — going through_blobBaseFeeCalculatorkeepseth_simulateV1blob-base-fee overrides working on the frame path.- No NRE on a header without
ExcessBlobGas—TryCalculateFeePerBlobGasnull-checks, so the tx is invalid rather than throwing. FlattenedHashesEqual(IExecutionPayloadParams.cs:213) andProcessingStatsare already instance-gated; theengine_newPayloadversioned-hash commitment check already covered frame txs.ReportFeesparity withPayFees(eip1559Fees + blobBaseFee) holds.FrameTxDecoder.cs:47always decodesMaxFeePerBlobGas, so the null case in Hive #4 is unreachable from the network — locally-constructed txs only.
Note
eth_getTransactionReceiptunder-reports.GetGasInfo(Nethermind.Evm/TransactionExtensions.cs:22) is stillSupportsBlobs-gated, so a blob-carrying frame tx reportsblobGasUsed/blobGasPriceas null even though the blob fee was charged and the header counts the blob gas. RPC-only, but now inconsistent with block accounting (BlockchainBridge.cs:116,ReceiptFinderExtensions.cs:44).- I could not run
dotnet build/dotnet test— both require approval in this environment (--allowedToolswould needBash(dotnet build:*)/Bash(dotnet test:*)). All findings come from reading the code; the "suites remain green" claim is unverified by me.
· brancheip8141-blob-support
| // EIP-7594 (ethereum/EIPs#11985): a blob-carrying frame tx is bound by the same per-tx blob-count | ||
| // limit and versioned-hash version byte as a type-3 blob tx (EELS validate_frame_transaction). | ||
| byte[]?[]? blobVersionedHashes = transaction.BlobVersionedHashes; | ||
| if (blobVersionedHashes is { Length: > 0 }) | ||
| { | ||
| ValidationResult blobGasLimitResult = BlobFieldsTxValidator.ValidateBlobGasLimits(blobVersionedHashes.Length, releaseSpec); | ||
| return !blobGasLimitResult ? blobGasLimitResult : BlobFieldsTxValidator.ValidateBlobVersionedHashes(blobVersionedHashes); | ||
| } |
There was a problem hiding this comment.
Resolves the High from the previous round. I traced the path: BlockValidator.ValidateTransactions (BlockValidator.cs:324) calls _txValidator.IsWellFormed(transaction, spec, block.Header.GasLimit) for every tx in a suggested block, and TxType.FrameTx is registered with this composite, so the per-tx blob cap (spec.GasCosts.MaxBlobGasPerTx → MaxBlobsPerTx = min(6, MaxBlobCount), ReleaseSpec.cs:108) and the 0x01 version byte are now enforced at block level, not just in the pool. The reuse of ValidateBlobGasLimits / ValidateBlobVersionedHashes keeps type-3 and frame txs from drifting apart. 👍
Two leftovers:
1. Low — MaxFeePerBlobGas: null is still unvalidated for a blob-carrying frame tx. BlobFieldsTxValidator rejects it for type-3 ({ MaxFeePerBlobGas: null } => BlobTxMissingMaxFeePerBlobGas); nothing does for a frame tx. That makes a previously unreachable branch reachable in BlockValidator.ValidateEip4844Fields:368:
if (transaction.MaxFeePerBlobGas < feePerBlobGas)MaxFeePerBlobGas is UInt256?, so with a null operand this lifted comparison evaluates to false — the block-level insufficiency check silently passes. The processor then rejects the tx (GetValueOrDefault() → 0 < feePerBlobGas, min blob base fee is 1), so the net result is still fail-closed, but the guarantee now rests on lifted-null semantics in one file and a GetValueOrDefault() in another. FrameTxDecoder.cs:47 always decodes the field, so this only reaches locally-constructed txs (simulate/RPC-built) today. Cheap to close here:
if (blobVersionedHashes is { Length: > 0 })
{
if (transaction.MaxFeePerBlobGas is null) return TxErrorMessages.BlobTxMissingMaxFeePerBlobGas;
...
}2. Low — ValidateBlobVersionedHashes was widened to public. FrameTxFieldsTxValidator lives in the same assembly, so internal static is enough. AGENTS.md prefers not adding public surface ("do not add additional interfaces or public methods"); same for ValidateBlobGasLimits if it has no external caller.
There was a problem hiding this comment.
Both addressed. FrameTxFieldsTxValidator now rejects a blob-carrying frame tx with MaxFeePerBlobGas: null (BlobTxMissingMaxFeePerBlobGas), and ValidateBlobVersionedHashes is back to internal static. ValidateBlobGasLimits stays public since BlobTransactionForRpc calls it.
| // matching EELS validate_frame_transaction. | ||
| [Test] | ||
| public void IsWellFormed_FrameTxWithSingleValidBlob_ReturnTrue() | ||
| { | ||
| Transaction tx = BuildBlobFrameTx(blobCount: 1); | ||
| TxValidator txValidator = new(TestBlockchainIds.ChainId); | ||
|
|
||
| Assert.That(txValidator.IsWellFormed(tx, Bogota.Instance).AsBool(), Is.True); | ||
| } | ||
|
|
||
| [Test] | ||
| public void IsWellFormed_FrameTxAtBlobCountLimit_ReturnTrue() | ||
| { | ||
| Transaction tx = BuildBlobFrameTx(blobCount: (int)Bogota.Instance.MaxBlobsPerTx); | ||
| TxValidator txValidator = new(TestBlockchainIds.ChainId); | ||
|
|
||
| Assert.That(txValidator.IsWellFormed(tx, Bogota.Instance).AsBool(), Is.True); | ||
| } | ||
|
|
||
| [Test] | ||
| public void IsWellFormed_FrameTxExceedsBlobCountLimit_ReturnFalse() | ||
| { | ||
| Transaction tx = BuildBlobFrameTx(blobCount: (int)Bogota.Instance.MaxBlobsPerTx + 1); | ||
| TxValidator txValidator = new(TestBlockchainIds.ChainId); | ||
|
|
||
| Assert.That(txValidator.IsWellFormed(tx, Bogota.Instance).AsBool(), Is.False); | ||
| } | ||
|
|
||
| [Test] | ||
| public void IsWellFormed_FrameTxWithWrongVersionedHashVersionByte_ReturnFalse() | ||
| { | ||
| Transaction tx = BuildBlobFrameTx(blobCount: 1, versionByte: 0x02); | ||
| TxValidator txValidator = new(TestBlockchainIds.ChainId); | ||
|
|
||
| ValidationResult result = txValidator.IsWellFormed(tx, Bogota.Instance); | ||
|
|
||
| Assert.That(result.AsBool(), Is.False); | ||
| Assert.That(result.Error, Is.EqualTo(TxErrorMessages.InvalidBlobVersionedHashVersion)); | ||
| } |
There was a problem hiding this comment.
Low (test quality). Two things, both from the AGENTS.md DRY rule ("when tests differ only by inputs and expected outputs, parameterize a single test"):
- The three blob-count tests differ only in
blobCountand the expected bool — they collapse to one[TestCase]-driven test:
[TestCase(1, true)]
[TestCase(6, true)] // BLOB_COUNT_LIMIT
[TestCase(7, false)]
public void IsWellFormed_FrameTxBlobCount(int blobCount, bool expected) =>
Assert.That(new TxValidator(TestBlockchainIds.ChainId)
.IsWellFormed(BuildBlobFrameTx(blobCount), Bogota.Instance).AsBool(), Is.EqualTo(expected));- The over-limit test asserts only
AsBool() is false. SinceBuildBlobFrameTxalso has to satisfy every otherFrameTxValidationconstraint, a future change that makes the builder produce a structurally invalid frame tx would leave this test passing for the wrong reason while the positive cases fail loudly — but only if someone reads which ones failed. AssertTxErrorMessages.BlobTxGasLimitExceeded(...)like the version-byte case does.
Also worth one case for the interaction with FrameTxValidation.BlobFeeWithoutBlobs (MaxFeePerBlobGas != 0, no hashes) if it isn't covered elsewhere — it's the only other blob-related frame constraint.
There was a problem hiding this comment.
Addressed. The two within-limit cases are now a single TestCaseSource-driven test, and the over-limit case asserts TxErrorMessages.BlobTxGasLimitExceeded. The BlobFeeWithoutBlobs interaction is already covered by FrameTxValidationTests.
| // EIP-8141: a blob-carrying frame transaction (type 6) also reserves the blob fee, so gate | ||
| // on the presence of blob hashes rather than the type-level SupportsBlobs (type-3 only). | ||
| // Priced at max_fee_per_blob_gas, an upper bound on the processor's escrow, so mempool | ||
| // affordability never admits a tx the processor cannot charge. | ||
| if (tx.BlobVersionedHashes is { Length: > 0 }) | ||
| { | ||
| // if tx.SupportsBlobs and has BlobVersionedHashes = null, it will throw on earlier step of validation, in TxValidator | ||
| overflow |= UInt256.MultiplyOverflow(Eip4844Constants.GasPerBlob, (UInt256)tx.BlobVersionedHashes!.Length, out UInt256 blobGas); | ||
| overflow |= UInt256.MultiplyOverflow(Eip4844Constants.GasPerBlob, (UInt256)tx.BlobVersionedHashes.Length, out UInt256 blobGas); | ||
| overflow |= UInt256.MultiplyOverflow(blobGas, tx.MaxFeePerBlobGas ?? UInt256.MaxValue, out UInt256 blobGasCost); |
There was a problem hiding this comment.
Medium — still open from the previous round; the new validator narrows it but doesn't close it.
The affordability term is right now, and with FrameTxFieldsTxValidator in place the pool also rejects over-limit / bad-version-byte frame txs at entry. What remains is that a well-formed blob-carrying frame tx is admitted and amplified even though no node can include it:
CanBeBroadcast(tx)=!tx.SupportsBlobs && …→ true (line 25), so it is re-gossiped as a plain tx. There is no frame-tx network wrapper yet, so no peer can ever obtain the blobs.CanPayForBlobGas(tx, currentFeePerBlobGas)=!tx.SupportsBlobs || …→ true unconditionally (line 23), soTxBroadcaster.cs:253never evicts it when the head blob base fee rises past itsMaxFeePerBlobGas— precisely the condition the processor now rejects it for.NotSupportedTxFiltergates ontx.SupportsBlobs, so even a node configured withBlobsSupport = Disabledaccepts it.- And the new picker guard means we never include it ourselves.
The PR's deferral list covers "blob-pool routing" and "mempool blob-budget selection", which is the storage side — the gossip-without-sidecar and never-evict behaviours aren't in that list and are the ones with an outward effect. The symmetric counterpart to "produce conservatively" is to reject blob-carrying frame txs at pool entry (a filter alongside NotSupportedTxFilter) until the EIP-7594 slice lands; a one-line fix if you'd rather keep them is to exclude them from CanBeBroadcast.
There was a problem hiding this comment.
Deferring the gossip/eviction side to the follow-up blob-pool-routing PR rather than editing CanBeBroadcast/CanPayForBlobGas here. That PR routes blob-carrying frame txs to the blob pool and withholds them from gossip in SpecDrivenTxGossipPolicy — which also gates the persistent-broadcast path via _gossipFilter — so the amplification path closes there. This PR keeps only the accounting predicate plus the conservative production guard.
| [Test] | ||
| public void Execute_BlobCarryingFrameTx_ChargesAndBurnsBlobFee() | ||
| { | ||
| // EIP-8141 (spec ~L492-499): a blob-carrying frame tx follows EIP-4844 — the payer covers the | ||
| // blob fee, which is burned. With base fee 0 the whole gas premium goes to the beneficiary, so | ||
| // the only value that leaves the payer for good is the burned blob fee. Parity: EELS #3047. | ||
| DeploySmartSender(ApproveCode(TxFrame.ApproveExecutionAndPayment)); | ||
| Transaction tx = FrameTx(nonce: 0, SelfVerifyFrame()); | ||
| tx.BlobVersionedHashes = [new byte[32]]; | ||
| tx.MaxFeePerBlobGas = 1000; | ||
|
|
||
| CallOutputTracer tracer = new(); | ||
| TransactionResult result = ProcessWithBlobHeader(tx, excessBlobGas: 0, tracer: tracer); | ||
|
|
||
| Assert.That(result.TransactionExecuted, Is.True); | ||
| UInt256 spentGas = (UInt256)tracer.GasSpent; | ||
| UInt256 blobFee = ExpectedBlobFee(excessBlobGas: 0, blobCount: 1); | ||
| Assert.That(blobFee, Is.GreaterThan(UInt256.Zero), "blob fee is nonzero at the minimum blob base fee"); | ||
| using (Assert.EnterMultipleScope()) | ||
| { | ||
| Assert.That(_stateProvider.GetBalance(Beneficiary), Is.EqualTo(spentGas), "beneficiary gets the gas premium only"); | ||
| Assert.That(_stateProvider.GetBalance(Sender), Is.EqualTo(1.Ether - spentGas - blobFee), "payer pays the spent gas and the blob fee"); | ||
| Assert.That(_stateProvider.GetBalance(Sender) + _stateProvider.GetBalance(Beneficiary), | ||
| Is.EqualTo(1.Ether - blobFee), "the blob fee is burned, not paid to the beneficiary"); | ||
| } |
There was a problem hiding this comment.
Medium (test coverage) — still open: the PR's central consensus decision remains the one thing not pinned by a test.
The description and the comment at TransactionProcessorBase.FrameTx.cs:88-91 state that the blob leg of max_cost is priced at the actual blob_base_fee rather than max_fee_per_blob_gas, for EELS #3047 parity. This test asserts end-of-tx balances, which are identical under either pricing — the refund cancels the escrow difference. (MaxFeePerBlobGas = 1000 vs. a base fee of 1 makes the two escrows differ by ~130M wei here, and the test passes either way.)
The two consensus-visible values that do differ:
TXPARAM_MAX_COST (0x06)read by contract code mid-transaction —frameContextis built frommaxCost, so a frame reading0x06observes the choice directly.- The payer solvency gate in
ApplyApproval(FrameTx.cs:624): a payer funded betweentxGasLimit × maxFeePerGas + blobGas × blobBaseFeeand… + blobGas × maxFeePerBlobGaspays under this pricing and voids payment under the other — a state-root difference, not just an RPC one.
Both are cheap to cover with the harness already here: one frame that TXPARAM 0x06s into memory and asserts txGasLimit × maxFeePerGas + blobGas × blobBaseFee with MaxFeePerBlobGas well above the base fee, and one boundary case on the gate. Without them a later "fix" to either pricing is green in CI.
Minor: the comment cites "EELS #3047" without a line anchor; for a consensus-visible value a permalink to the process_transaction line would age better.
There was a problem hiding this comment.
Added Execute_TxParamMaxCost_BlobCarryingFrameTx_ReservesBlobLegAtBlobBaseFeeNotMaxFee: it reads TXPARAM 0x06 from a frame and asserts max_cost = gas leg + blob_gas × blob_base_fee with MaxFeePerBlobGas set well above the base fee (and strictly below the max-fee-priced reservation). That pins the escrow value the payer solvency gate also consumes.
| // EIP-8141: include blob-carrying frame transactions (type 6), not just type-3, in the | ||
| // block's blob gas — gate on blob hashes instead of the type-level SupportsBlobs. | ||
| if (tx.BlobVersionedHashes is { Length: > 0 }) | ||
| { | ||
| blobCount += (ulong)tx.GetBlobCount(); | ||
| } |
There was a problem hiding this comment.
Low (simplification, still open). GetBlobCount() is BlobVersionedHashes?.Length ?? 0, so the guard only skips adding zero. Per the repo preference for removing code over adding it, the loop body is just:
foreach (Transaction tx in transactions)
{
// EIP-8141: blob-carrying frame transactions (type 6) count towards the block's blob gas too,
// so the count comes from the blob hashes rather than the type-level SupportsBlobs.
blobCount += (ulong)tx.GetBlobCount();
}There was a problem hiding this comment.
Done — guard removed; the loop body is now blobCount += (ulong)tx.GetBlobCount();.
| // EIP8141: blob-carrying frame transactions sit in the normal pool and are not yet | ||
| // metered against the block blob budget during production (deferred: blob-pool routing | ||
| // and blob-budget selection). Now that they count towards header.BlobGasUsed, an | ||
| // unguarded producer could exceed MaxBlobGasPerBlock and self-invalidate the block. | ||
| // Exclude them conservatively until block production tracks their blob gas. | ||
| if (currentTx.Type == TxType.FrameTx && currentTx.BlobVersionedHashes is { Length: > 0 }) | ||
| { | ||
| return args.Set(TxAction.Skip, "Blob-carrying frame transaction not yet supported in block production"); | ||
| } | ||
|
|
There was a problem hiding this comment.
Low — still open from the previous round.
HasEnoughFunds at line 129 still gates the blob-fee reservation on transaction.SupportsBlobs, so it under-reserves for a blob-carrying frame tx. Harmless today only because this guard short-circuits first — which means the day the guard is lifted, the funds check is silently wrong and nothing here points at it. Extending line 129 to transaction.BlobVersionedHashes is { Length: > 0 } is a no-op now and correct later; failing that, a marker at line 129 tying it to this guard.
(Also unchanged: TxAction.Skip leaves these txs in the pool indefinitely — see the CanPayForBlobGas note on TransactionExtensions.cs — so the guard bounds block validity, not pool growth.)
There was a problem hiding this comment.
Addressed. The blob-fee reservation in HasEnoughFunds now gates on BlobVersionedHashes is { Length: > 0 }, so it reserves correctly for a blob-carrying frame tx independent of the picker guard. Pool-growth/eviction stays with the follow-up blob-pool-routing PR.
| // burned blob fee cancels against this term in the refund below. | ||
| UInt256 blobFee = UInt256.Zero; | ||
| if (tx.BlobVersionedHashes is { Length: > 0 }) | ||
| { | ||
| if (!_blobBaseFeeCalculator.TryCalculateBlobFees(header, tx, spec.BlobBaseFeeUpdateFraction, out UInt256 feePerBlobGas, out blobFee)) | ||
| { | ||
| TraceLogInvalidTx(tx, "BLOB_BASE_FEE_OVERFLOW"); | ||
| return RequiredBalanceExceeds256Bits(tx); | ||
| } | ||
|
|
||
| // EIP-4844: max_fee_per_blob_gas must cover the current blob base fee, else the tx is invalid. | ||
| if (tx.MaxFeePerBlobGas.GetValueOrDefault() < feePerBlobGas) | ||
| { | ||
| TraceLogInvalidTx(tx, "INSUFFICIENT_MAX_FEE_PER_BLOB_GAS"); | ||
| return TransactionResult.ErrorType.InsufficientMaxFeePerGasForSenderBalance.WithDetail( | ||
| BlockErrorMessages.InsufficientMaxFeePerBlobGas(tx.SenderAddress, tx.MaxFeePerBlobGas, feePerBlobGas)); | ||
| } |
There was a problem hiding this comment.
Re-verified this block against the rest of the method; the arithmetic holds:
maxCost = txGasLimit × maxFeePerGas + blobFeeis overflow-checked,effectiveGasPrice ≤ maxFeePerGasandspentGas ≤ txGasLimit, sochargedCost = spentCost + blobFee ≤ maxCost— the subtraction at line 334 cannot underflow.ApplyApprovalchargesMaxCostexactly once (line 618 rejects a second payer) and a batch unroll reverts the debit wholesale viaWorldState.Restore, so no double burn and no path where the blob fee is refunded.- Going through
_blobBaseFeeCalculatorrather thanBlobGasCalculatorkeepsBlobBaseFeeOverrideCalculatorDecoratorworking on this path — good. TryCalculateFeePerBlobGasnull-checksheader.ExcessBlobGas, so a header without it yields an invalid-tx result rather than an NRE.
Two small leftovers from the previous round:
Low — error type. The regular path returns ErrorType.InsufficientSenderBalance for insufficient max_fee_per_blob_gas (TransactionProcessor.cs:1109); this returns InsufficientMaxFeePerGasForSenderBalance. Both map to ErrorCodes.InsufficientFunds in SimulateTxExecutor, so nothing breaks, but the divergence is gratuitous and Execute_BlobFrameTx_MaxFeePerBlobGasBelowBlobBaseFee_Invalid now pins it in. Matching the regular path keeps debug_* / eth_simulateV1 error classification tx-type-independent.
Low — fee collector. The credit at line 344 routes the blob fee to spec.FeeCollector, but the EIP-1559 base-fee share of a frame tx stays burned, whereas PayFees routes eip1559Fees + blobBaseFee. The 1559 half is pre-existing (this path never touched the collector before) and no chain enables both EIP-8141 and IsEip4844FeeCollectorEnabled today, so it isn't a live divergence — but the new code reads as complete collector support. Either route both legs (UInt256.Min(header.BaseFeePerGas, effectiveGasPrice) * spentGas when spec.IsEip1559Enabled && !tx.IsFree()), or leave a marker that the 1559 leg is still unrouted.
There was a problem hiding this comment.
Both addressed. The insufficient-max-fee-per-blob-gas path now returns InsufficientSenderBalance, matching the regular path's classification (the invalid-fee test was updated). A short marker now notes the EIP-1559 base-fee leg is still not routed to the fee collector on the frame path.
Address review follow-ups on the blob-support PR: - require MaxFeePerBlobGas for a blob-carrying frame tx and narrow the versioned-hash validator to internal - reserve the blob fee in block production via the instance blob predicate - return InsufficientSenderBalance for insufficient max fee per blob gas, matching the regular path's error classification - collapse the redundant blob-gas count guard and mark the still-unrouted EIP-1559 fee-collector leg on the frame path - pin the max_cost blob-leg pricing (TXPARAM 0x06) and parameterize the blob-count validation tests
…lob-carrying frame transactions Complete the non-consensus blob layer for EIP-8141 frame transactions on top of the consensus-correct accounting/validation/settlement in PR #12701. - Blob-pool routing: introduce an instance-level Transaction.CarriesBlobs gate (BlobVersionedHashes present) and route on it at the pool-membership sites so a blob-carrying frame tx (type 6) lands in the blob pool like a type-3 tx, without touching the type-level SupportsBlobs() that networking/decode paths key on. Type-3 routing is unchanged. TxTypeTxFilter, snapshot invalidation, eviction cascade, metrics, and ContainsTx follow the same gate. - Production blob-budget metering: routing feeds frame-blob txs through the blob selection path in TxPoolTxSource, so they are metered against the block blob budget (MaxBlobGasPerBlock / MaxProductionBlobCount) and a produced block never exceeds it. The normal-pool picker guard is kept as defense in depth for any frame-blob tx that reaches production without a resolvable sidecar. - Networking: withhold blob-carrying frame txs from gossip until the EIP-7594 sidecar wire format exists, rather than leaking them in bare consensus form. Deferred (documented markers): the EIP-7594 sidecar / network-wrapper wire format for type 6 (decode/encode + mempool acceptance of the wrapper form). Production of frame-blob txs is gated on it, since the blobs bundle cannot be built without the sidecar. Tests: frame-blob tx routes to the blob pool while a plain frame tx and a type-3 tx route unchanged; a frame-blob tx with a sidecar is metered/selected within the blob budget and excluded when it would overflow it.
1 participant
Aligns blob accounting for EIP-8141 frame transactions with the spec (
eip-8141.md~L492-499, ethereum/EIPs#11985) and EELS #3047.Changes
BlockValidator.ValidateEip4844FieldsandBlobGasCalculator.CalculateBlobGas(Transaction[])now gate onBlobVersionedHashes.Length > 0instead of the type-levelSupportsBlobs(type-3 only). A blob-carrying frame tx (type 6) now contributes toheader.BlobGasUsedand the per-block blob budget, exactly like a type-3 tx.TxTypeExtensions.SupportsBlobs()is deliberately left unchanged so networking/wrapper/decode paths are unaffected.blob_gas × blob_base_fee, charged and burned (never refunded, regardless of frame outcome);max_fee_per_blob_gas >= blob_base_feeis enforced (invalid otherwise). Themax_costblob leg is priced at the actualblob_base_fee, matching EELS Update Fast Sync configuration in Nethermind repository #3047 — this keeps both the payer escrow observed mid-transaction and the EVM-observableTXPARAM_MAX_COST (0x06)in parity. On EIP-4844 fee-collector chains the blob fee is routed to the collector, consistent with the regular path'sPayFees.MaxBlobGasPerBlockand self-invalidate.Tests
max_fee_per_blob_gas < blob_base_fee→ invalid.BlobGasCalculatorcounts blob-carrying frame txs alongside type-3.