apollo_gateway: validate replayed txs against their source block's gas prices - #15010
apollo_gateway: validate replayed txs against their source block's gas prices#15010ron-starkware wants to merge 1 commit into
Conversation
PR SummaryMedium Risk Overview In Also saturates the min-price threshold on overflow instead of wrapping via Reviewed by Cursor Bugbot for commit af23ca4. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
Reworked after a code review found the first attempt did not achieve its goal. Pushed as a second commit rather than a force-push so the review's line anchors survive; the net diff against What was wrong: overriding only the explicit What changed: the source block's whole STRK price vector is now resolved once per tx and applied in a single Also addressed from the review: resolution moved ahead of the state-snapshot pin; retry with the mempool's policy; a last-block memo so the block lookup is ~1 per block; a u64 sanity bound on recorder-supplied prices; Two review items deliberately left as follow-ups, noted in the description: no metric on the recorder dependency (needs a One partial disagreement on the fixture finding: the previous test suite did cover tx-metadata-OK plus block-metadata-absent. It's right that 500 and unparsable on the second leg were missing and that the rstest parameter was dead — both fixed, along with the hex/decimal duplication and the hardcoded block number. |
ca70dcf to
9dac5e5
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 9dac5e5. Configure here.
9dac5e5 to
eb9871a
Compare
…s prices Resource bounds are validated against the committed block's gas prices in two independent places: the explicit l2 threshold check, and blockifier's check_fee_bounds reached through run_validate_entry_point, whose BlockContext is built from the same get_block_info(). Blockifier's is the stricter of the two -- max_price_per_unit >= actual_gas_price, no percentage -- and it covers all three resources. Under replay the committed block trails the block a transaction came from, and mainnet prices move up to ~2% per block, so a transaction mainnet accepted is rejected here for having less headroom than the price fell in between. That is a deadlock rather than a delay: the bound is satisfied only once prices drop to the transaction's cap, which in a falling market first happens when the committed block IS its own block -- already built by then, so it can never be included. Being deterministic, the retry after a resync reproduces it and the block is skipped for good. Block 11926844 hit this twice in one weekend. Tx 0x1549342b caps l2 at 32000000000 against its own block's 31507218957, +1.56% headroom at the bottom of a five-block slide; 57 rejections walked the threshold from 32093567243 up to 34854264801 without ever falling under the cap. Echonet built the block with 1 of its 4 transactions and produced the same wrong hash on the retry. Nor is it only l2 or only the explicit check: of nine `Resource bounds were not satisfied` rejections over four days, seven were l1_data_gas, which only blockifier's bound looks at. Resolve the source block's whole STRK price vector once per transaction and apply it in a single block_info() seam that both consumers are built from. A transaction mainnet included in block N necessarily paid price(N), so it clears on the first forward wherever the committed block happens to be, while the check keeps its teeth: anything that could not have paid its own block's price is still rejected. Only strk_gas_prices is replaced -- the ETH vector is not replayed, and v3 transactions select the STRK vector by fee type. block_number and block_timestamp are deliberately left alone, since moving them would shift the stored-block-hash window and get_block_hash against a state that only holds up to the committed block. Resolution happens in instantiate_validator before the state snapshot is pinned, so the recorder round-trips do not age the state validation then runs against. Transient failures are retried with the policy the mempool's recorder client uses, since a silent fallback would reinstate the very rejection this removes. The last block's prices are memoized -- they are immutable and consecutive replayed transactions share a block -- so the block lookup drops to roughly one per block, leaving only the per-transaction hash lookup the mempool already makes anyway. The recorder base URL is normalized to end in a slash, because Url::join otherwise replaces the last path segment of a URL configured with a path. The threshold multiplication is now saturating rather than a Ratio product, so a price too large to scale rejects instead of wrapping. That also hardens the pre-existing path, where the price comes from state, and leaves num-rational unused by this crate, so it is dropped from its dependencies. Both recorder endpoints already exist and are already used by the mempool in this mode, so no recorder-side change is needed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
eb9871a to
af23ca4
Compare

Problem
validate_tx_l2_gas_price_within_thresholdcompares a tx'sl2 max_price_per_unitagainstmin_gas_price_percentageof the committed tip's L2 gas price. Under replay the tip trails the block the tx came from, and mainnet's L2 price moves up to ~2% per block, so a tx mainnet accepted gets rejected here for having less headroom than the price fell in between.That is a deadlock, not a delay. A tx in block N clears the threshold only once the tip's price drops to its cap, which in a falling market first happens at
tip = N— and block N is closed by then, so the tx can never enter its own block. It is deterministic, so the retry after a resync reproduces it exactly and Echonet ends up permanently skipping a mainnet block.Block 11926844 hit it twice this weekend:
Mainnet's price was in a five-block slide into that block (34.85e9 → 31.51e9), and 57
GAS_PRICE_TOO_LOWrejections walked the threshold from 32,093,567,243 (block 11926843) up to 34,854,264,801 (11926839) without ever dropping under the cap. Echonet built 11926844 with 1 of its 4 txs, produced the byte-identical wrong hash on the retry, and skipped the block. Block 11802770 four days earlier was the same mechanism.Change
In
BehaviorMode::Echonet, threshold against the L2 gas price of the block the tx actually came from, resolved from the recorder.A tx mainnet included in block N necessarily paid
price(N), soprice(N) <= capalways holds and the tx clears on its first forward, regardless of where the tip is. The check keeps its teeth: anything that could not have paid its own block's price is still rejected. Outside Echonet mode nothing changes.Why not the alternatives:
validate_resource_bounds: falseremoves the check entirely.min_gas_price_percentageweakens it for everything, and no fixed percentage is safe — the required discount depends on how far the price falls.next_l2_gas_price(which is what theTODO(Arni)at that line wants, and is a genuine latent improvement) is not enough on its own: it moves the pass condition fromtip = Ntotip = N-1, which is only reached moments before block N is built, leaving a ~1 s race against the gateway's state-sync lag. Worth doing separately, but not as the fix for this.Notes
apollo_mempool/src/communication.rs), so no recorder-side change is needed —echonet/get_block_metadataalready returnsl2_gas_price_fri.Testing
crates/apollo_gateway/src/source_block_gas_price_test.rs, 5 cases against amockitorecorder: resolves the real block-11926844 price from the exact payload the Python handler emits, and falls back on missing tx metadata, missing block metadata, an unparsable body, and a zero price.cargo test -p apollo_gateway— 170 passed, 0 failed.cargo clippy -p apollo_gateway --all-targets -- -D warningsclean.scripts/rust_fmt.shclean.🤖 Generated with Claude Code