feat(scenarios): add an AMM swap scenario - #71
Conversation
The scenario declared 22460 gas for a mint. Measured against the deployed binding, a mint to a receiver holding none of the token needs 69319, and one to a receiver that already holds some needs 51757. Every mint the scenario sent landed in a block with a failed status, having burned the whole limit, and trackReceipts defaults to false so the run reported each one as sent. 22460 is ERC20Noop's constant, copied. PLT-1091 covers the two scenarios that still carry it. The limit is now 75000, and the test pins it against the measurement rather than against itself. Broke the constant back to 22460 and to 200000 on purpose; the test caught both. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A DeFi profile had no contract to drive. This adds a constant-product pair with the storage and gas shape of a UniswapV2 swap: both reserves, the caller's balance in each token, and an event. The contract never reverts on bookkeeping, which is the choice StorageRWv1 already makes. The balances wrap rather than check, because nothing reads them back and a load generator that fails on its own accounting stops measuring the chain. A short caller is not credited: crediting exactly what is then debited returns the slot to zero, and a zero to non-zero storage write costs four times one that changes a slot already holding a value. Under the default mix, which draws one direction, that write would land on every swap rather than the first. The reserves sit between a floor and a ceiling. Without the ceiling the input side grows without bound and the output halves every 100000 swaps, so a long run prices nothing like its start. The ceiling is also what keeps one oversized call from ending the pair: a swap of 1e49 leaves the input reserve at 1e49, and the contract has no owner and no reset. Measured, the next ordinary swap instead resets that side to the floor and pays out in full. The gas limit is 85000, read from eth_estimateGas rather than from a receipt. GasUsed is the post-refund charge and a transaction carries the pre-refund peak; sizing from a receipt put an earlier draft 20% under what its own swap needed. An account's first swap needs 79988 and every later one needs 45177, so a run in steady state declares about 44% more gas than it spends. PLT-1093 carries the prewarm change that would close that. PLT-1092 carries the chain-parameter exposure, which is the whole package rather than this constant. Every guard here was broken on purpose before it was believed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
PR SummaryLow Risk Overview The contract targets UniswapV2-like gas/storage (reserves, per-caller balances, Fixes ERC721 mint gas from 22,460 to 75,000 (measured cold mint ~69k), with a test so under-limited mints can’t silently “succeed” when receipt tracking is off. Reviewed by Cursor Bugbot for commit 6c7e41d. Bugbot is set up for automated code reviews on this repo. Configure here. |
![seidroid[bot]](https://avatars.githubusercontent.com/in/2745135?s=60&v=4){kind=small}
There was a problem hiding this comment.
Adds a well-documented constant-product AMM scenario (contract, binding, scenario, operation set, tests) and raises the ERC721 mint gas limit from a value that made every mint fail. The code is correct and well tested for chains running default EVM storage costs; the main open items are the hard-coded gas limits on Sei's public chains and missing user-facing docs for the new scenario.
Findings: 0 blocking | 6 non-blocking | 2 posted inline
Blockers
- None at the file/PR level.
Non-blocking
- [suggestion] The new
ammscenario and itsoperationskeys (swap_a_to_b/swap_b_to_a) are not documented for operators: README's "Available Scenarios" list is not updated, andconfig/doc.go's "Operation baskets" section still names onlystoragerwas declaring a frozen operation vocabulary. Since the operation names and their declaration order are a frozen wire contract,amm's set is worth writing down alongsidestoragerw's. - [suggestion] The scenario deploys exactly one pair, so every AMM transaction in a run writes the same two reserve slots (
reserveA/reserveB). Under Sei's parallel execution that makes the whole AMM slice fully conflicting and serialized, which is faithful to a single UniswapV2 pair but not to a DeFi load profile spread across many pairs. Worth either documenting explicitly next toAMMScenarioor leaving a note for a future multi-pair axis, so a profile author does not read AMM throughput as parallelizable DeFi load. - 2 suggestion(s)/nit(s) flagged inline on specific lines.
- 2 non-blocking pre-existing issue(s) listed below under pre-existing issues.
Pre-existing issues
- [suggestion]
generator/scenarios/ERC20Conflict.goandgenerator/scenarios/ERC20Noop.gocarry the same under-sized hard-coded gas limits this PR fixes for ERC721 (the PR body measures ERC20Noop short by 455 gas and ERC20Conflict by 69,710). WithtrackReceiptsdefaulting to false, runs using those scenarios report failed, limit-burning transactions as sent. Tracked as PLT-1091. - [suggestion] README's "Available Scenarios" list (README.md:145-151) was already stale before this PR — it omits
StorageRW,Disperse,EVMTransferFastandEVMTransferNoop, so it is not a reliable index of what the factory registers.
| // | ||
| // Estimating per transaction would put an eth_estimateGas on the send path, | ||
| // which is the load this tool exists to avoid adding. | ||
| const ammSwapGas = 85_000 |
There was a problem hiding this comment.
[suggestion] This limit is calibrated for the default 20,000 SSTORE_SET, and the comment above it states that pacific-1 and atlantic-2 charge ~74,700, putting an account's first swap near 185,000. On those chains every account's first swap will land with a failed status and burn the full 85,000, and with trackReceipts defaulting to false the run reports each one as sent — the exact silent failure mode the ERC721 change in this PR fixes.
I understand the exposure is package-wide and PLT-1092 covers it, so this need not block. But a brand-new scenario shipping a limit already known to fail on the two main public chains is worth at least a guard rather than only a comment: e.g. a one-time eth_estimateGas at deploy/bind time to size the constant (off the per-tx send path), or a startup warning when the scenario runs against a chain whose SSTORE_SET is above the default.
| // This constant read 22460 until it was measured. At that value every mint | ||
| // landed in a block with a failed status and burned the whole limit, and a run | ||
| // with trackReceipts off reported each one as a success. | ||
| const erc721MintGas = 75_000 |
There was a problem hiding this comment.
[suggestion] Good fix — 22,460 was clearly wrong. Note the fix is only complete for chains running default storage costs: a mint writes _owners[tokenId] (always zero→non-zero) and _balances[to] (zero→non-zero for a fresh receiver), so on pacific-1/atlantic-2 at ~74,700 per zero→non-zero write a cold mint needs roughly 180k and still fails silently under 75,000. Worth stating that residual gap in this comment the way ammSwapGas does, so the next reader does not take the constant as chain-independent.
1 participant
Adds an
ammscenario so a DeFi load profile has a contract to drive, and fixesan existing gas limit that made every ERC721 mint fail.
The AMM contract
A constant-product pair with the storage and gas shape of a UniswapV2 swap: both
reserves, the caller's balance in each token, and an event. It reproduces the
cost, not the economics. There is no router, no fee split, no price oracle and no
minimum-output check.
Two design points are worth stating, because both were wrong in an earlier draft.
The balances wrap rather than check. Nothing reads them back, and a load
generator that fails on its own accounting stops measuring the chain. The draft
credited a short caller exactly what it then debited, which returns the slot to
the value it held. For a caller starting at zero, that is zero. A zero to
non-zero storage write costs four times one that changes a slot already holding a
value, and the default operation mix draws one direction, so that expensive write
would have landed on every swap of the run rather than on the first.
The reserves sit between a floor and a ceiling. The draft had only a floor.
Measured over a long run, the input side then grows without bound and the output
halves every 100,000 swaps, so the profile stops pricing anything like it did at
the start. The ceiling also keeps one oversized call from ending the pair: a swap
of 1e49 leaves the input reserve at 1e49, and the contract has no owner and no
reset. Measured, the next ordinary swap now resets that side to the floor and
pays out in full.
Gas
The limit is a required limit read from
eth_estimateGas, not a receipt'sGasUsed.GasUsedis what the chain charges after the refund lands at the endof execution, and a transaction has to carry the peak before it. Sizing from a
receipt put an earlier draft 20% under what its own swap needed, which fails the
transaction and burns the whole limit.
Measured over six accounts against the deployed binding, on a chain running the
default storage gas costs:
One limit has to cover the higher shape, so a run in steady state declares about
44% more gas than it spends. Priming both slots during prewarm would close that;
it needs a transaction class the prewarm path does not have yet, and PLT-1093
carries it.
The calibration assumes the default 20,000 for a zero to non-zero storage write.
Sei sets that as a chain parameter and the public chains charge about 74,700, so
these numbers are right for arctic-1 and wrong for pacific-1. That exposure is
the whole package rather than this constant, and PLT-1092 covers it.
The ERC721 fix
ERC721.godeclared 22,460 gas for a mint, which isERC20Noop's constantcopied. A mint needs 69,319 to a receiver holding none of the token. Every mint
the scenario sent landed in a block with a failed status having burned the whole
limit, and
trackReceiptsdefaults to false, so the run reported each one assent.
It is fixed here rather than deferred because the
tokenopsprofile this workdeploys uses that scenario, and shipping it broken would read as a chain
regression on the canary.
ERC20Noopis short by 455 gas andERC20Conflictby 69,710. Neither appears ina profile deployed today, so PLT-1091 carries them.
What is checked, and what is not
Every guard was broken on purpose before it was believed. Six mutations, six
caught: the ERC721 limit back to 22,460 and up to 200,000; the AMM limit down to
the steady shape and up past the band; the B-to-A leg made unreachable; the
operation stamp dropped.
The reserve bounds and the balance-slot property have no guard. They were
measured by hand and no test repeats the measurement, because the repo runs no
contract against an EVM in CI. A Go test on
ethclient/simulatedguards allthree and is written, but it adds about 35 indirect modules including the full
go-ethereum node stack, which is too large a dependency change to make in
passing. PLT-1094 carries the harness question and the parked test.
Verification
gofmt,go vet,staticcheckandgolangci-lint runare clean. The fullsuite passes. All seven tracked bindings regenerate byte-identical under the
pinned solc 0.8.19 and abigen 1.16.1, so
check-bindingspasses.🤖 Generated with Claude Code