Skip to content

feat(arbos60): add multi-gas constraints pricing model#700

Open
AnkushinDaniil wants to merge 68 commits intomainfrom
daniil/arbos60-multigas-constraints
Open

feat(arbos60): add multi-gas constraints pricing model#700
AnkushinDaniil wants to merge 68 commits intomainfrom
daniil/arbos60-multigas-constraints

Conversation

@AnkushinDaniil
Copy link
Copy Markdown
Collaborator

@AnkushinDaniil AnkushinDaniil commented Mar 2, 2026

Summary

Implements ArbOS 60 multi-gas constraints pricing model with per-resource weights. Includes MultiGas tracking fixes and comparison mode test isolation improvements (merged from #723).

Key Features

Core Pricing Model

  • GasModel enum for routing: Legacy, SingleGasConstraints, MultiGasConstraints
  • Weighted backlog calculation with saturating arithmetic
  • Per-resource exponent calculation with MaxPricingExponentBips cap (85,000)
  • Input validation and bounds checking throughout
  • Production wiring: UpdateGasPool now calls GrowBacklog(computeGas, AccumulatedMultiGas) for ArbOS 60+

MultiGas Tracking (from #723)

  • Register receipt decoder so MultiGas fields persist correctly in storage
  • Track precompile output data as Computation gas
  • Track code deposit gas as StorageGrowth
  • Owner precompiles don't charge multigas (return zero gas)

Comparison Mode (from #723)

  • Reset BlockTree internal state (Head, BestKnownNumber) between tests via IResettableBlockTree
  • Add Genesis null checks to handle reinitialized state gracefully
  • Enable debug logging for troubleshooting

Architecture

Gas Model Selection

flowchart TD
    Start([GetGasModelToUse]) --> V60{ArbOS >= 60?}
    V60 -->|Yes| MC{MultiGasConstraints<br/>Length > 0?}
    MC -->|Yes| MGC[MultiGasConstraints]
    MC -->|No| V50
    V60 -->|No| V50{ArbOS >= 50?}
    V50 -->|Yes| SC{Constraints<br/>Length > 0?}
    SC -->|Yes| SGC[SingleGasConstraints]
    SC -->|No| LEG[Legacy]
    V50 -->|No| LEG

    style MGC fill:#22c55e,stroke:#16a34a,color:#000
    style SGC fill:#3b82f6,stroke:#2563eb,color:#000
    style LEG fill:#a1a1aa,stroke:#71717a,color:#000
Loading

Storage Layout

classDiagram
    class MultiGasConstraint {
        <<12 slots>>
        +ulong Target
        +uint AdjustmentWindow
        +ulong Backlog
        +ulong MaxWeight
        +ulong WeightedResources_8
        +GrowBacklog(MultiGas)
        +ShrinkBacklog(MultiGas)
    }

    class MultiGasFees {
        <<16 slots>>
        +UInt256 NextBlockFees_8
        +UInt256 CurrentBlockFees_8
        +CommitNextToCurrent()
    }

    class GasConstraint {
        <<3 slots - Legacy>>
        +ulong Target
        +ulong AdjustmentWindow
        +ulong Backlog
    }

    class ResourceKind {
        <<enum>>
        Unknown
        Computation
        HistoryGrowth
        StorageAccess
        StorageGrowth
        L1Calldata
        L2Calldata
        WasmComputation
    }

    MultiGasConstraint --> ResourceKind : weights per kind
    MultiGasFees --> ResourceKind : fees per kind
Loading

Pricing Update Flow (Per Block)

sequenceDiagram
    participant Block as Block Producer
    participant L2P as L2PricingState
    participant MGC as MultiGasConstraint
    participant MGF as MultiGasFees
    participant Storage as BaseFeeWei

    Block->>L2P: UpdatePricingModel(timePassed)
    
    Note over L2P: Model = MultiGasConstraints
    
    loop For each constraint
        L2P->>MGC: Get backlog, target
        L2P->>L2P: newBacklog = backlog - timePassed * target
        L2P->>MGC: SetBacklog(newBacklog)
    end
    
    L2P->>L2P: CalcMultiGasConstraintsExponents()
    Note over L2P: exponent = backlog * weight * BIPS / divisor
    
    loop For each ResourceKind
        L2P->>L2P: baseFee = minBaseFee * exp(exponent)
        L2P->>MGF: SetNextBlockFee(kind, baseFee)
    end
    
    L2P->>Storage: Set max of all baseFees
    
    Block->>L2P: CommitMultiGasFees()
    L2P->>MGF: CommitNextToCurrent()
    Note over MGF: nextBlockFees copied to currentBlockFees
Loading

Transaction Gas Tracking

flowchart LR
    subgraph TX [Transaction Execution]
        OP[EVM Opcode] -->|gas + resourceKind| ACC[Accumulate MultiGas]
    end

    subgraph GROW [Post-TX: UpdateGasPool → GrowBacklog]
        ACC -->|TxExecContext.AccumulatedMultiGas| UB[GrowBacklog]
        UB -->|for each constraint| WB[Weighted Update]
        WB -->|add weighted amount| MGC2[MultiGasConstraint]
    end

    subgraph SHRINK [Block End: ShrinkBacklog]
        TIME[timePassed * target] --> SB[ShrinkBacklog]
        SB -->|subtract capacity| MGC3[MultiGasConstraint]
    end

    style TX fill:#60a5fa,stroke:#3b82f6,color:#000
    style GROW fill:#f87171,stroke:#ef4444,color:#000
    style SHRINK fill:#4ade80,stroke:#22c55e,color:#000
Loading

Weighted Backlog Calculation

flowchart TD
    subgraph Input [Input]
        MG[MultiGas<br/>per-resource usage]
        W[Constraint Weights<br/>per-resource multiplier]
    end

    subgraph Calculation [Calculation]
        MG -->|amount| MUL[Multiply]
        W -->|weight| MUL
        MUL -->|saturating| WA[weightedAmount]
        WA -->|saturating add/sub| BL[totalBacklog]
    end

    subgraph Output [Output]
        BL --> SET[constraint.SetBacklog]
    end

    style Input fill:#fbbf24,stroke:#f59e0b,color:#000
    style Calculation fill:#c084fc,stroke:#a855f7,color:#000
    style Output fill:#4ade80,stroke:#22c55e,color:#000
Loading

System Test Validation

New logic has been validated by Nitro comparison system tests. The following 4 multi-gas specific tests were added to the passing list as part of this PR:

Test What it validates
TestSetAndGetGasPricingConstraints Single-gas constraint configuration round-trip
TestSetAndGetMultiGasPricingConstraints Multi-gas constraint configuration round-trip
TestMultiGasRefundForNormalTx Per-resource gas refund accounting for normal transactions
TestMultiGasRefundForRetryableTx Per-resource gas refund accounting for retryable transactions

These tests run in comparison mode against Nitro, validating state root parity at every block.

Total passing comparison tests: 46 (see tools/comparison/passing-tests.txt).


Unit & Integration Tests

  • MultiGasConstraintTests.cs — Storage operations, backlog updates (including Nitro-mirrored weighted accumulation tests)
  • MultiGasFeesTests.cs — Fee commit workflow
  • PricingModelMultiGasTests.cs — Pricing model equivalence tests
  • L2PricingStateTests.cs — Constraint lifecycle, exponent calculation

Dependencies

PR Repository Description
#10765 nethermind Submodule changes: IResettableBlockTree, MultiGas gas tracking in EVM
#723 nethermind-arbitrum MultiGas tracking fixes and comparison mode isolation (merged into this PR)

@AnkushinDaniil AnkushinDaniil force-pushed the daniil/arbos60-multigas-constraints branch 6 times, most recently from c1c7363 to f338aff Compare March 3, 2026 10:58
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 3, 2026

Codecov Report

❌ Patch coverage is 65.88670% with 277 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.04%. Comparing base (9258dc3) to head (661407e).

Files with missing lines Patch % Lines
...rmind.Arbitrum/Core/ResettableArbitrumBlockTree.cs 0.00% 85 Missing ⚠️
src/Nethermind.Arbitrum/Precompiles/ArbOwner.cs 2.12% 46 Missing ⚠️
...Arbitrum/Execution/ArbitrumTransactionProcessor.cs 41.30% 25 Missing and 2 partials ⚠️
src/Nethermind.Arbitrum/ArbitrumPlugin.cs 32.00% 16 Missing and 1 partial ⚠️
....Arbitrum/Rpc/ReceiptForRpcPolymorphicConverter.cs 0.00% 14 Missing ⚠️
...rbitrum/Genesis/ArbitrumGenesisStateInitializer.cs 45.45% 9 Missing and 3 partials ⚠️
...Nethermind.Arbitrum/Precompiles/Abi/AbiMetadata.cs 78.00% 6 Missing and 5 partials ⚠️
src/Nethermind.Arbitrum/Arbos/ArbosState.cs 23.07% 9 Missing and 1 partial ⚠️
...rbitrum/Arbos/Storage/FilteredTransactionsState.cs 0.00% 10 Missing ⚠️
...mind.Arbitrum/Precompiles/Parser/ArbOwnerParser.cs 30.76% 9 Missing ⚠️
... and 13 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #700      +/-   ##
==========================================
- Coverage   76.84%   76.04%   -0.81%     
==========================================
  Files         217      224       +7     
  Lines       13222    13836     +614     
  Branches     1886     1981      +95     
==========================================
+ Hits        10161    10522     +361     
- Misses       2383     2618     +235     
- Partials      678      696      +18     
Files with missing lines Coverage Δ
src/Nethermind.Arbitrum/Arbos/ArbosAddresses.cs 100.00% <100.00%> (ø)
src/Nethermind.Arbitrum/Arbos/ArbosSubspaceIDs.cs 100.00% <100.00%> (ø)
src/Nethermind.Arbitrum/Arbos/Precompiles.cs 100.00% <100.00%> (ø)
...thermind.Arbitrum/Arbos/Programs/StylusPrograms.cs 73.37% <100.00%> (ø)
...Nethermind.Arbitrum/Arbos/Storage/GasConstraint.cs 100.00% <100.00%> (ø)
...rmind.Arbitrum/Arbos/Storage/MultiGasConstraint.cs 100.00% <100.00%> (ø)
.../Nethermind.Arbitrum/Arbos/Storage/MultiGasFees.cs 100.00% <100.00%> (ø)
...ethermind.Arbitrum/Arbos/Storage/RetryableState.cs 98.33% <100.00%> (ø)
src/Nethermind.Arbitrum/Config/ArbitrumConfig.cs 89.65% <100.00%> (+0.36%) ⬆️
src/Nethermind.Arbitrum/Core/ArbitrumBlockTree.cs 100.00% <100.00%> (+25.00%) ⬆️
... and 33 more
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@AnkushinDaniil AnkushinDaniil marked this pull request as ready for review March 3, 2026 15:15
Copilot AI review requested due to automatic review settings March 3, 2026 15:15
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Implements ArbOS 60 multi-gas constraints pricing with per-resource weighted backlogs and per-resource base fees, and wires it into transaction processing and pricing updates.

Changes:

  • Added storage-backed MultiGasConstraint and MultiGasFees primitives for ArbOS 60+ multi-dimensional pricing.
  • Updated L2PricingState to select a GasModel, update multi-gas backlogs/exponents, and commit per-resource fees.
  • Integrated multi-gas backlog growth in tx processing and added extensive tests across pricing, storage, and precompile-related behaviors.

Reviewed changes

Copilot reviewed 20 out of 20 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
src/Nethermind.Arbitrum/Math/MathExtensions.cs Adds inlining hints for saturating arithmetic helpers used by backlog math.
src/Nethermind.Arbitrum/Execution/ArbitrumTransactionProcessor.cs Routes gas pool/backlog updates by GasModel, wiring multi-gas growth.
src/Nethermind.Arbitrum/Evm/MultiGas.cs Adds CheckResourceKind helper and adjusts docs.
src/Nethermind.Arbitrum/Evm/ArbitrumGasPolicy.cs Tracks code deposit under StorageGrowth for multi-gas accounting.
src/Nethermind.Arbitrum/Arbos/Storage/MultiGasFees.cs New storage struct for next/current per-resource base fees with commit rotation.
src/Nethermind.Arbitrum/Arbos/Storage/MultiGasConstraint.cs New storage struct for weighted multi-resource constraints and weighted backlog updates.
src/Nethermind.Arbitrum/Arbos/Storage/L2PricingState.cs Core pricing/state changes: GasModel, multi-gas exponent calc, fee update/commit, backlog routing.
src/Nethermind.Arbitrum/Arbos/Storage/GasConstraint.cs Cleans up storage clearing via new Clear() methods; makes class sealed.
src/Nethermind.Arbitrum/Arbos/Storage/ArbosStorage.cs Adds Clear() to slot and storage-backed primitives.
src/Nethermind.Arbitrum/Arbos/ArbosVersion.cs Introduces version 60 constants / alias for multi-gas constraints enablement.
src/Nethermind.Arbitrum.Test/Precompiles/PrecompileContextTests.cs Adds tests for multi-gas tracking (some currently don’t exercise production code).
src/Nethermind.Arbitrum.Test/Precompiles/ConstraintsPrecompileTests.cs Adds tests around configuring and using multi-gas constraints.
src/Nethermind.Arbitrum.Test/Infrastructure/ArbosStateTestExtensions.cs Adds test-only helpers for reading/writing multi-gas fees.
src/Nethermind.Arbitrum.Test/Evm/MultiGasTests.cs Adds tests for CheckResourceKind validation behavior.
src/Nethermind.Arbitrum.Test/Arbos/Storage/L2PricingStateTests.cs Expands test coverage for gas model selection, backlog updates, multi-gas pricing, and guards.
src/Nethermind.Arbitrum.Test/Arbos/L2Pricing/PricingModelMultiGasTests.cs Adds equivalence tests across legacy/single/multi-gas models.
src/Nethermind.Arbitrum.Test/Arbos/L2Pricing/MultiGasFeesTests.cs Adds tests for per-resource fees and commit semantics.
src/Nethermind.Arbitrum.Test/Arbos/L2Pricing/MultiGasConstraintTests.cs Adds tests for weighted backlog math and storage behavior.
src/Nethermind.Arbitrum.Test/Arbos/ArbosStorageTests.BackedByType.cs Adds tests for new Clear() methods on storage-backed primitives.
src/Nethermind Updates submodule pointer.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

wurdum
wurdum previously approved these changes Mar 4, 2026
Copy link
Copy Markdown
Contributor

@wurdum wurdum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add integration tests to make sure e2e compatibility with nitro

@AnkushinDaniil
Copy link
Copy Markdown
Collaborator Author

Please add integration tests to make sure e2e compatibility with nitro

Added more tests. The next step in my plan is to adopt system tests

@AnkushinDaniil AnkushinDaniil force-pushed the daniil/arbos60-multigas-constraints branch 3 times, most recently from d900d77 to f55414e Compare March 4, 2026 15:26
Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 5, 2026

@AnkushinDaniil I've opened a new pull request, #712, to work on those changes. Once the pull request is ready, I'll request review from you.

@svlachakis
Copy link
Copy Markdown
Collaborator

svlachakis commented Mar 5, 2026

I would wait for system tests inclusion for better stability and assurance. I don't think there is any reason to rush this..my humble opinion.

Add ArbOS 60 multi-gas constraints with per-resource weights:
- MultiGasConstraint: storage-backed constraint with weighted resources
- MultiGasFees: per-resource base fee storage (next-block/current-block)
- L2PricingState: constraint management, exponent calculation, pricing updates
- GasModel enum for routing: Legacy, SingleGasConstraints, MultiGasConstraints
- Input validation and bounds checking throughout
- Add Clear() to storage-backed types (ArbosStorageSlot, ArbosStorageBackedUInt/ULong/UInt256)
- Add MultiGas.CheckResourceKind() validation for defensive programming
- Throw InvalidOperationException on divisor == 0 instead of silent continue
- Move test-only methods to test project via extension methods
- Move weights dictionary inside loop to match reference implementation
In ArbOS 60+ (multi-gas constraints), BacklogUpdateCost returns a
static cost (StorageReadCost + StorageWriteCost) regardless of
constraint count. The C# implementation was missing this early
return and continued with additional overhead calculations.

Added tests to verify the static cost behavior in ArbOS 60 and
the dynamic cost calculation in ArbOS 51.
…ization

Performance improvements for multi-gas constraints implementation:

- Add [AggressiveInlining] to SaturateMul and SaturateSub for consistency
  with SaturateAdd (MathExtensions.cs)
- Seal leaf classes (MultiGasConstraint, GasConstraint, MultiGasFees,
  L2PricingState) to enable JIT devirtualization
- Replace yield iterator in CalcMultiGasConstraintsExponents with direct
  loop to avoid state machine allocation per-call
- Use inline array FeeBuffer instead of heap-allocated UInt256[] in
  MultiDimensionalPriceForRefund to eliminate per-transaction allocation
Add 24 tests to achieve full patch coverage:

- Exception guards: divisor-zero checks in CalcMultiGasConstraintsExponents
  and UpdatePricingModelMultiConstraints
- Migration path: SetMultiGasConstraintsFromSingleGasConstraints conversion,
  uint32 clamping, and clearing existing constraints
- Legacy pricing: GasPoolUpdateCost for ArbOS 49/50, legacy backlog
  grow/shrink, high backlog pricing
- Multi-gas fees: CommitMultiGasFees early return and commit paths,
  L1Calldata/zero fee fallback to baseFeeWei
- MultiGas: CheckResourceKind validation for valid and invalid kinds
- ArbosStorage: Clear methods for UInt, ULong, UInt256 backed storage
Remove ShouldUseGasConstraints() which was a C#-only helper that
didn't exist in Nitro. Use GetGasModelToUse() directly instead,
which matches Nitro's GasModelToUse() function.

Changes:
- Remove ShouldUseGasConstraints() from L2PricingState
- Update tests to use GetGasModelToUse() assertions
- Remove tests that directly tested the removed method
…sPool

UpdateGasPool now routes to GrowBacklog for ArbOS 60+ MultiGasConstraints mode,
matching Nitro's tx_processor behavior. Legacy and SingleGasConstraints modes
continue using AddToGasPool.

Added tests mirroring Nitro's TestMultiGasConstraintBacklogGrowth and
TestMultiGasConstraintBacklogDecay patterns for weighted accumulation.
Remove Arrange/Act/Assert comments from ConstraintsPrecompileTests
and PrecompileContextTests per code style guidelines.
Nethermind unconditionally zeroed multigas for all callers to owner
precompiles via RestoreBurnedMultiGas in the finally block. Nitro's
OwnerPrecompile.Call returns burner.gasUsed (non-zero) for non-owner
paths but multigas.ZeroGas() only for actual owners.

Adds AddOwnerCheckMultiGasDelta helper that computes the system
burner's multigas delta and injects it into the gas policy accumulator
for non-owner and OOG paths before RestoreBurnedMultiGas cleans up.

Constraint: _systemBurner.BurnedMultiGas is never consumed by final accounting
Constraint: context.Burn(gasUsed) single-arg overload does not track multigas
Rejected: Conditional restore only for owners | leaves stale delta in shared _systemBurner
Confidence: high
Scope-risk: narrow
…erns

Replace field-by-field BlockTree reset with ResettableArbitrumBlockTree
decorator that creates fresh instances. ArbOSVersionOverride now uses
thread-safe consume-once pattern via Interlocked.Exchange.

- Delete TrieStoreClearableCache (TrieStore implements IClearableCache directly)
- Register HeaderStore, BlockStore, ChainLevelInfoRepository as IClearableCache
- ArbitrumBlockTree simplified to pure constructor delegation

Constraint: BlockTree constructor must be fast on empty DBs for reset performance
Rejected: Field-level reset | fragile, requires tracking every new field
Rejected: Keep TrieStoreClearableCache wrapper | unnecessary indirection
Confidence: medium
Scope-risk: moderate
Not-tested: BlockTree decorator reset performance under load
Failed tests are retried up to N times (default 3, --retries flag).
Duration accumulates across attempts. Summary distinguishes flaky
tests (passed on retry) from hard failures. Parallel runner checks
worker health between retry attempts and restarts if needed.

Constraint: Retry wraps _run_single_test — no changes to test execution itself
Rejected: Retry only non-deterministic failures | impossible to classify reliably
Confidence: high
Scope-risk: narrow
…eset config

Multi-gas resource split: cold access costs now correctly split between
StorageAccess (cold premium) and Computation (warm base), matching
Nitro's operations_acl.go pattern: ColdCost - WarmRead → StorageAccess,
WarmRead → Computation.

BurnArbGas: changed from expecting throw to asserting GasLeft==0,
matching Nitro's ArbosTest.go which intentionally ignores the Burn error
(//nolint:errcheck) and silently consumes all remaining gas.

Redeem backlog: removed Math.Min(long.MaxValue, gasToDonate) cap that
caused SaturateSub to zero the backlog, triggering StorageWriteZeroCost
(5000) instead of StorageWriteCost (20000) — a 15000 gas discrepancy.

CodeDb: assert NOT cleared during reinitialize since code is
content-addressed and immutable across reinitializations.

EnableTestReset: new config flag (default: false) gates test-only
behavior — ResettableArbitrumBlockTree, debug_reinitialize RPC,
and skipVersionCheck for per-test ArbOS version overrides.

Constraint: Comparison tests send varying ArbOS versions per test
Rejected: effectiveArbOSVersion override | failed when test version != chainspec version
Directive: EnableTestReset must remain false in production — it gates mutable BlockTree and debug RPC
Confidence: high
Scope-risk: moderate
Not-tested: EnableTestReset in non-comparison production scenarios (always false)
The smart rebuild feature (git-based change detection, build state
tracking, auto Nethermind build) added unnecessary complexity.
Users should build manually before running comparison tests.

Removed: _get_git_hash, _needs_rebuild, _record_build, build_nethermind,
BUILD_STATE_FILE, and git-based skip logic from compile_test_binary.
Revert comment edits, style changes, and formatting-only modifications
that were inadvertently included in the multigas feature branch.
Preserves all functional multigas/ResourceKind changes.

Files affected: 13 C# files across Arbos, Evm, Execution, Genesis,
Modules, and Rpc layers.
The unconditional MixHash recomputation in InitializeAndBuildGenesisBlock
could overwrite the chainspec's canonical MixHash value in production,
causing genesis block hash mismatch. Now only recomputes when
EnableTestReset is true, where the init message may carry a different
ArbOS version than the chainspec.

Constraint: Production genesis must use chainspec MixHash verbatim
Rejected: Always recompute MixHash | breaks production genesis hash
Confidence: high
Scope-risk: narrow
Replace misleading comments with actionable TODO referencing
Nitro arbosstate.go:240-256. NativeTokenSupplyManagementEnabled
and TransactionFilteringEnabled are hardcoded to disabled — correct
for standard chains but needs implementation for custom Orbit chains.

Confidence: high
Scope-risk: narrow
Apply ruff format to 3 Python files and add missing flaky/retries_enabled
args to SummaryStats constructors in test_formatters.py.

Confidence: high
Scope-risk: narrow
…as-constraints

# Conflicts:
#	src/Nethermind
#	src/Nethermind.Arbitrum/ArbitrumPlugin.cs
#	src/Nethermind.Arbitrum/Precompiles/ArbitrumPrecompileExecutionContext.cs
@AnkushinDaniil AnkushinDaniil marked this pull request as draft March 22, 2026 14:34
Replace direct IClearableCache implementation on CacheCodeInfoRepository
with a nested CacheClearService class. CacheCodeInfoRepository has scoped
dependencies (IWorldState) that prevent singleton resolution as IClearableCache.
The nested class has zero dependencies and accesses the private static cache directly.

Constraint: CacheCodeInfoRepository depends on scoped IWorldState
Rejected: InternalsVisibleTo | anti-pattern between production assemblies
Rejected: IClearableCache on CacheCodeInfoRepository | DI resolution failure at runtime
Confidence: high
Scope-risk: narrow
- Remove dead FeeBuffer InlineArray struct from L2PricingState
- Move allocating test-only methods (UsedResources, GetResourcesWithWeights)
  from MultiGasConstraint to MultiGasConstraintTestExtensions
- Add XML documentation to all public members of L2PricingState

Constraint: Extension methods use public GetResourceWeight() API, not private fields
Confidence: high
Scope-risk: narrow
Rewrite comments that referenced the upstream Go codebase by name,
replacing them with self-contained descriptions of the behavior.
Single-use method that just wraps `new TxGasInfo(header.BaseFeePerGas)`.
Inlined at the only call site and removed the now-unused `spec` variable.
Align with submodule change — AccountAccessKind.SelfDestructBeneficiary
fully encodes the "no warm charge" semantic, making chargeForWarm
redundant. Updates ArbitrumGasPolicy and fixes 3 SELFDESTRUCT tests to
use AccountAccessKind.SelfDestructBeneficiary with correct cold-path
assertions (full ColdAccountAccess to StorageAccess, no split).

Constraint: chargeForWarm was only false at the SELFDESTRUCT call site
Rejected: Keep both parameters for flexibility | no other caller needs chargeForWarm=false
Copy link
Copy Markdown
Contributor

@wurdum wurdum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

if (storageKey is null or { Length: 0 })
{
// Nethermind's SetNonce throws if account is null, so we must create first.
if (!_db.AccountExists(_account))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, do we have some issue without account init?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Nitro similarly ensures the account exists before touching storage.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't get. Why did it work before, and doesn't now?

public class ArbitrumSpecHelper(ArbitrumChainSpecEngineParameters parameters) : IArbitrumSpecHelper
public class ArbitrumSpecHelper(
ArbitrumChainSpecEngineParameters parameters,
IArbOSVersionOverride? versionOverride = null) : IArbitrumSpecHelper
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This optional overrider doesn't look good.
Why don't we extend the IArbitrumSpecHelper to have OverrideArbOsVersion? Same functionality, but less confusing invariants.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I considered this, but IArbitrumSpecHelper is better to be a read-only spec-resolution interface, it's injected widely. Adding OverrideArbOsVersion leads to the situation when every consumer would see a mutation method that only ArbitrumDebugRpcModule and test-reset code actually call.
The current IArbOSVersionOverride is a narrow, separate interface that only the two callers that actually need override capability depend on.
If you still prefer consolidating, I can add it to IArbitrumSpecHelper.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. Let's make IArbOSVersionOverride required (not have a default null assignment), and introduce something like DisabledArbOsVersionOverride that does nothing.

The idea here is to ensure we always provide all the dependencies explicitly. Having optional or non-typed (func, action) dependencies is a road to uncontrolled dependency tree.

- Remove unused `spec` parameter from PrepareBlockHeader
- Use explicit AbiSignature type instead of var
- Rename s_optionsCache to _optionsCache per naming conventions
- Remove redundant using Nethermind.Arbitrum.Evm
- Explain PrepareBlockHeader's CommitMultiGasFees runs in a temporary
  scope that gets discarded
- Explain transient registration is required for Func<T> factory pattern
  in test reset mode
Remove redundant scalar `_gasBurnt` field from SystemBurner. All gas
burns now go through `Burn(ResourceKind, ulong)` with MultiGas tracking.
`Burned` property returns `BurnedMultiGas.Total` as single source of truth.

Classify previously untyped scalar burns:
- ArbosStorage code hash cost → StorageAccess
- StylusPrograms init cost → WasmComputation
- StylusParams precompile call cost → Computation
- ArbitrumVirtualMachine owner check → Computation

Also fixes TestBurner bug where `Burned` returned remaining gas instead
of consumed gas.
- Remove `= false` default from ArbitrumModule enableTestReset parameter
- Replace `var` with explicit `Dictionary<ResourceKind, ulong>` in tests
- Add explicit `enableTestReset: false` at call sites that relied on default
Replace the custom ReceiptForRpcPolymorphicConverter (which cloned
JsonSerializerOptions with itself removed to prevent infinite recursion)
with the standard .NET 7+ IJsonTypeInfoResolver approach using
JsonPolymorphismOptions. Cleaner, no self-removal hack, no
ConcurrentDictionary cache needed.
Resolve conflicts between multigas-constraints branch and sequencer
support: ArbitrumModule now accepts IArbitrumConfig, keep both
EnableTestReset logic and ArbitrumSequencerModule registration,
merge IClearableCache registrations with ArbitrumBlockFactory,
take main's TxGasInfo and l1BlockNumber in ArbitrumEthRpcModule.
Submodule had diverged from main: our 11 custom commits (MultiGas
tracking, IResettableBlockTree, IClearableCache, etc.) were based on
an older common ancestor missing 10 upstream commits (SIMD fix,
eth_getLogs fix, RLP decoders, transaction receipts subscription).

Rebased our custom commits onto main's submodule pointer (40aa1f34)
so both upstream and branch changes are included.
…x gasPrice

Arbitrum always reports the block's BaseFeePerGas as the effective gas
price in receipts, matching Nitro's behavior. The merge from main
introduced tx.GetGasInfo() which computes per-transaction gas price,
causing mismatches: system txs reported 0 instead of baseFee, and user
txs reported their gasPrice instead of baseFee.
@wurdum
Copy link
Copy Markdown
Contributor

wurdum commented Mar 25, 2026

Well done. Figuring out where to put all these CommitMultiGasFees is crazy 🤯

…pport

The IJsonTypeInfoResolver approach (da1ebcb) broke polymorphic
serialization — ArbitrumReceiptForRpc properties (GasUsedForL1,
MultiGasUsed) were not serialized in comparison mode responses.
Revert to a custom JsonConverter<ReceiptForRpc> that delegates to
the runtime type, restoring correct receipt comparison.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants