Skip to content

bug: pAMM state overrides are applied against the wrong block and never take effect #4690

Description

@kirsanium

Problem

The pAMM state-override stream added in #4606 assumes a frame's blockNumber describes the block being simulated. It doesn't — it names the block the builder is about to build (chain head + 1), and the frame's words carry a freshness stamp for that block's slot timestamp.

SimulationOverrides::current() compares that number to the current head and returns None when they differ, so the overrides are withheld nearly always.

Impact

The feature is inert. pAMM routes have been simulating against previous-block state for the lifetime of the feature, which is exactly what it was introduced to avoid.

To reproduce

Driving the current code against the live stream with a real WS block watcher and sampling current() every 100ms for 5 minutes, reading the crate's own counter:

simulations_with_overrides{result="fresh"}    152
simulations_with_overrides{result="stale"}   2813     <- 94.9%

Sampling the gate once a second for 60s instead: applied=1, withheld=59, with the snapshot naming head + 1 in 55 samples and head + 2 in 4. The rare hit is the ~1s sliver right after a block lands.

Expected behaviour

Overrides are applied whenever the stream is healthy, so pAMM routes simulate against the maker's live price.

Additional detail

Relaxing the block comparison alone does not fix it. The venue reverts StaleUpdate() (0x666a2814) unless the word's freshness stamp matches the timestamp of the block being executed, and the stamp is 12s ahead of head. At head 25646525 (ts 1785424751) the live snapshot named 25646526 with stamp 1785424763, quoting 1000 USDT into WETH on the FermiSwap venue:

overrides result
none revert 0x666a2814
snapshot applied verbatim, simulated at head revert 0x666a2814
stamp rewritten to head's timestamp ok

pending is not the answer either. Its number is head + 1, matching the snapshot, but its timestamp is the node's wall clock and moves between calls:

latest: 25646068 1785419243   pending: 25646069 1785419248
latest: 25646068 1785419243   pending: 25646069 1785419251
latest: 25646068 1785419243   pending: 25646069 1785419253

The value the node will use is unknowable when the request is built and moves before it executes.

Two further defects in the same area:

  • The two consumers simulate in contexts with different block.timestamps behind one no-argument accessor — trade verification pins to head, gas estimation runs at pending. One accessor cannot serve both.
  • Frames are inserted per account. Every venue writes its lanes into the same shared registry account and each frame carries only its own, so each insert drops the others'. In a 90s sample (3125 frames, 5 venues), four distinct venues wrote that one account — 837, 839, 528 and 106 frames respectively, 1–8 lanes each, 19 distinct lanes overall.
  • Too-old, wrong-block and empty all record stale, which is why this reads as stream ill-health rather than an off-by-one.

Suggested solution

  • Replace current() with an accessor taking the simulation context (block, timestamp), so the two consumers can no longer share an answer that is wrong for one of them.
  • Serve a snapshot at or ahead of the simulated block (ahead is the normal case), withhold one behind it, and rewrite the newest frame's freshness stamp to the simulated block's timestamp.
  • Rewrite only words carrying the newest frame's stamp, so a lane the maker did not requote keeps its older stamp and stays dead, exactly as it would on chain. Only the four stamp bytes move; the remaining 28 are the maker's price.
  • Merge frames per storage slot rather than per account.
  • Split the stale label into distinct outcomes.
  • Add live conformance coverage. Its absence is why this shipped broken and stayed broken.

Alternatives considered

Relaxing only the block comparison — insufficient, see the table above.

Targeting pending, whose number does match the frame — impossible, its timestamp is wall-clock and moves before the request executes.

Rewriting every stamp rather than only the newest frame's — rejected, it would assert liveness for lanes nobody is quoting.

Applying the overrides in the block context they describe. This is the more faithful model and deserves recording in full, because it is what the stream is actually telling us to do. Instead of rewriting the stamp, pass the venue's words through untouched and move the simulation into the block they were stamped for. The settlement executes in that block, and the builder sequences the maker's update immediately before it, so that state will genuinely exist — whereas "maker's newest price stamped for head" is a state that exists at no point on the timeline. Titan's docs point the same way: the Takers page names eth_simulateV1 alongside eth_call, and eth_simulateV1 refuses to simulate a block that does not advance time past its parent.

I implemented it and verified it end to end. It works — one real settlement produces byte-identical output through both paths. It is still not what I am proposing, because the cost lands somewhere unrelated to pAMMs:

  • Only eth_simulateV1 can express a block context. eth_call cannot override the block it executes in, and eth_estimateGas cannot either. Both simulation paths have to migrate.

  • The gas path cannot opt out. Without the overrides the settlement reverts StaleUpdate(), eth_estimateGas fails, and the solution is discarded — so pAMM routes would never settle. Gas has to go through eth_simulateV1 too.

  • eth_simulateV1 cannot answer eth_estimateGas's question. They measure different things. Per geth's ExecutionResult, UsedGas is "total used gas, refunded gas is deducted" and MaxUsedGas is "maximum gas consumed during execution, excluding gas refunds", while eth_estimateGas binary-searches the smallest starting limit the call survives — which must also cover the 63/64 reserve at every nested call. On the Aave debt-swap replay:

    gas_used      =   971,920   net of refunds
    max_used_gas  = 1,133,920   gross peak
    estimate_gas  = 1,675,639   minimum viable limit
    

    Gas::new takes a single number and uses it both for scoring (a cost question, where gas_used is right) and for the submission limit via 2x (a limit question, where only estimate_gas is safe). Adopting the block-context approach forces those apart and changes gas semantics for every settlement the driver produces, not just pAMM ones: the submission cushion drops from 2x the true requirement to 1.16–1.35x, calibrated from a single settlement whose call depth may not be representative.

  • Smaller, but indicative: eth_simulateV1 defaults the simulated block's base fee to zero, so it has to be pinned explicitly or every contract in the settlement sees a different environment. There may be more differences of that kind that nobody has audited.

That is a large, risky change to shared driver code arriving as a side effect of a pAMM fix. Splitting Gas into separate cost and limit figures is the prerequisite, and it deserves its own issue and its own justification. Until then, rewriting the stamp keeps eth_estimateGas, and with it the meaning of the number: still the smallest starting limit the call survives, 63/64 reserve included, so scoring and the submission cushion are untouched. The only change is which block the estimate runs against — head instead of pending, and only when overrides are applied.

Follow-up worth filing separately

  • Gas conflates cost and limit; splitting them unblocks moving settlement simulation to eth_simulateV1, after which the pAMM overrides could be applied untouched.
  • The stream's TLS works only because the alloy websocket transport installs a rustls crypto provider as a side effect while opening the block stream. A deployment with current_block.ws_url unset falls back to HTTP polling, nothing installs a provider, and the stream task panics on its first connect.

Acceptance criteria

  • Overrides are applied against the block actually being simulated.
  • Lanes not requoted in the newest frame stay stale.
  • Frames from different venues sharing the registry account stop evicting each other.
  • The metric distinguishes why overrides were withheld.
  • Live conformance coverage that fails if the overrides stop being applied.

services version/commit hash and environment

main @ 3c4bb4122, mainnet, measured against the live pamm_quote_stream.

Fixed in #4691

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingtrack:maintenancemaintenance track

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions