PLT-461: StorageRW scenario scaffold + deploy wiring - #47
Merged
Conversation
Add a "storagerw" scenario over the PLT-457 StorageRWv1 binding, mirroring ERC20Conflict: a ContractScenarioBase[StorageRWv1] that deploys the mapping-backed contract (no constructor args), binds it, and produces a fixed rmw tx against slot 0 with an empty pad. This proves the contract is reachable as a scenario end-to-end; the per-tx slot/value/pad distribution lands in PLT-465. Register "storagerw" in the factory's auto-generated block. Hand-written to match the current ERC20Conflict idiom rather than via `make generate`: the template script still emits the pre-PLT-457 signatures (NewXScenario()/NewContractScenarioBase(scenario)) and a sol-derived name, neither of which matches current base.go or the ticket's "storagerw" name. Test mirrors the mock-deploy generator coverage at the scenario level: attach at a known address, generate, and assert the tx targets the contract and carries the rmw selector (cross-checked against the binding ABI). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The rmw scaffold tx left auth.GasLimit at the 200k CreateTransactionOpts default while every sibling scenario tunes it. On a gas-limit-admission chain that under-packs blocks by ~7x, depressing observed TPS. rmw is SLOAD+SSTORE on one slot (~26k warm, ~44k cold-first); 50k covers cold-first-touch with headroom and packs ~4x denser. PLT-465 revisits once the calldata pad is distribution-driven. Pin the fixed scaffold calldata in the test (slot 0, empty pad). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
PR SummaryLow Risk Overview Each tx sets Tests cover factory lookup by name and mock-deploy Generate output (contract Reviewed by Cursor Bugbot for commit 74e53ed. Bugbot is set up for automated code reviews on this repo. Configure here. |
Add generator/scenarios/doc.go documenting the contract-scenario pattern (ContractScenarioBase/ContractDeployer, MockDeploy attach, factory registration) and the StorageRW scaffold with the full gas-sizing derivation. Lean StorageRW.go: consolidate the three PLT-465 deferral notes to one terse note, and reduce the dense gas comment to a critical one-liner plus a pointer to the package doc. Comments and doc only; no behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
amir-deris
approved these changes
Jun 15, 2026
amir-deris
left a comment
amir-deris
left a comment
There was a problem hiding this comment.
- Resource leak in Attach — ethclient.Dial creates a TCP connection that is never Close()d. This appears to be consistent with sibling scenarios (ERC20Conflict, etc.), so it's a pre-existing
pattern, but worth noting if scenarios are attached/detached frequently. - storageRWPad = []byte{} as a package-level var is mutable. It's harmless now (empty slice), but once PLT-465 introduces a real pad, this var likely goes away entirely. No action needed.
- Factory comment direction — The existing factory.go has // DO NOT EDIT ABOVE THIS LINE - AUTO-GENERATED CONTENT but the PR description and doc.go say auto-generated entries go below the marker.
The wording is inverted (should be "BELOW"). Pre-existing issue, PLT-472 is already filed to fix the hand-wired entry; the comment confusion is a pre-existing nit. - StorageRWScenario carries both the embedded base and an explicit contract field — this is consistent with ERC20ConflictScenario but means the contract reference lives in two places. Again,
pre-existing pattern.
| // block space the rmw never spends and throttles achievable throughput. PLT-465 | ||
| // revisits the limit once the calldata pad is distribution-driven, since pad size | ||
| // changes calldata gas. | ||
| package scenarios |
There was a problem hiding this comment.
Would it be better if this would be a .md file instead?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements PLT-461 — makes StorageRWv1 reachable as a sei-load scenario. Scaffold only; per-tx key/size distribution comes in PLT-465.
What
generator/scenarios/StorageRW.gomirroringERC20Conflict.go(ContractScenarioBase[bindings.StorageRWv1], parameterless mapping-backed deploy), registered as"storagerw"in the factory.rmw(slot=0, empty pad)to prove the deploy+send path end-to-end.auth.GasLimit = 50000(matches sibling scenarios; the 200k default under-packs blocks ~7×). Mock-deploy + full-calldata + selector test.Review (solidity + idiom)
bytespad encoding, nonce sequencing). Idiom: faithful ERC20Conflict mirror.rmwis a scaffold; PLT-465 (distribution wiring) is the gate before any real throughput numbers. Follow-up PLT-472 filed for the drifted scenario-generator script (hand-written here).Decision brief:
designs/sei-load-workload-modeler/PLT-461-storagerw-scaffold.md.🤖 Generated with Claude Code
📐 Design: decision brief — PLT-461 · parent design