Skip to content

PLT-457: StorageRWv1 contract + pinned ABI + binding - #42

Merged
bdchatham merged 6 commits into
mainfrom
brandon2/plt-457-storagerwv1-contract
Jun 11, 2026
Merged

PLT-457: StorageRWv1 contract + pinned ABI + binding#42
bdchatham merged 6 commits into
mainfrom
brandon2/plt-457-storagerwv1-contract

Conversation

@bdchatham

Copy link
Copy Markdown
Contributor

Implements PLT-457 — the contention contract for the sei-load workload modeler. Contract + binding only; no scenario wiring (PLT-461/465).

What

  • generator/contracts/StorageRWv1.solmapping-backed store; write/read/rmw over a caller-selected slot. recordcount/keyspace is purely generator-side (no baked array size) → resize with no redeploy.
  • read is a state-touching tx, not view (folds the loaded value into readAccumulator so the SLOAD can't be elided and the SUT pays for it).
  • Trailing bytes calldata _pad on each function lets callers vary tx size independently of the key.
  • Generated generator/bindings/StorageRWv1.go, committed.

Locked decisions honored

Mapping-backed; read non-view; versioned StorageRWv1; added alongside ERC20Conflict (untouched). Function selectors write(uint256,uint256,bytes) / read(uint256,bytes) / rmw(uint256,bytes) are the pinned one-way door.

Toolchain

solc 0.8.19 (repo-pinned) + abigen pinned to the repo's go-ethereum version. go build ./... passes. On CI/Linux the binding regenerates via make setup-node && make generate.

Decision brief: designs/sei-load-workload-modeler/PLT-457-storagerwv1-contract.md (sei-protocol/bdchatham-designs).

🤖 Generated with Claude Code

Add a mapping-backed StorageRWv1 contract (read/write/rmw over a caller-selected
slot, with a trailing bytes calldata pad for tx-size variation) plus its
generated Go binding. read is a state-touching (non-view) tx so the SLOAD is
real. recordcount/keyspace is purely generator-side (no baked array size), so
the keyspace resizes with no redeploy. Versioned name so a v2 can coexist.

Added alongside ERC20Conflict (deprecate-don't-delete); no scenario/distribution
wiring (deferred to PLT-461/465). solc 0.8.19, abigen pinned to the repo's
go-ethereum version. go build ./... green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@cursor

cursor Bot commented Jun 11, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
New load-test contract bytecode is additive, but Makefile compile flag changes alter deployment bytecode for all contracts and could affect gas benchmarks if redeployed without coordination.

Overview
Adds StorageRWv1 for sei-load contention testing: a mapping-backed store, caller-chosen slot, and write / state-touching read (via readAccumulator) / rmw, plus optional _pad calldata. Commits generator/contracts/StorageRWv1.sol and generator/bindings/StorageRWv1.go; no scenario wiring in this PR.

The Makefile now pins solc 0.8.19 (sha256-verified download), EVM paris, --metadata-hash none, and abigen to the go.mod go-ethereum version. New targets generate-bindings, check-bindings, and install-abigen enforce committed bindings match contracts. Existing binding Bin fields change only from that reproducible compile (selectors/ABI unchanged).

A new Bindings Check GitHub Actions workflow runs make check-bindings on push/PR to main.

Reviewed by Cursor Bugbot for commit cd43b29. Bugbot is set up for automated code reviews on this repo. Configure here.

read()'s `readAccumulator += store[slot]` used checked math: with
caller-controlled write values it could overflow and revert (Panic
0x11). Since the accumulator is monotonic and unrecoverable, the first
overflow would brick every subsequent read permanently and silently
collapse goodput. Wrap it in unchecked so every tx keeps succeeding at
constant gas. Wrap rmw's increment the same way for gas-profile parity
(overflow there is unreachable, but the result is never asserted on).

Add `uint256 public constant VERSION = 1` so downstream consumers can
pin to v1 once StorageRWv2 coexists on a persistent chain.

Regenerated the Go binding (solc 0.8.19, abigen v1.16.1): adds the
VERSION getter (0xffa1ad74); read/rmw/write selectors unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@bdchatham

Copy link
Copy Markdown
Contributor Author

Expert review — converged (no remaining concerns)

Reviewed by independent specialists (not the authoring agent), two rounds:

  • Security / correctness: ✅ resolved a blocking defect — read's readAccumulator += store[slot] used 0.8 checked math and could overflow→revert (Panic 0x11) under caller-controlled write values, permanently bricking all subsequent reads and silently collapsing goodput. Now wrapped in unchecked (verified at the bytecode level: plain ADD, no PANIC path; rmw likewise for gas-profile parity). Confirmed unchecked does not alter the read/rmw/write selectors and introduces no new safety surface (values are never asserted on).
  • Idiom (Solidity + binding): ✅ resolved — added uint256 public constant VERSION = 1 (supports the locked v2-coexistence decision); binding regenerated via the repo's solc 0.8.19 + pinned abigen (pipeline-consistent, file mode 0644). Named bytes calldata _pad vetted as the correct readable choice.

Scenario/factory wiring is intentionally out of scope (deferred to PLT-461/465); the contract + pinned binding are what this ticket delivers. go build ./... green.

generate-bindings rebuilds only the .sol->abi/bin->binding chain (no
scenarios/factory). install-abigen pins abigen to the go.mod
go-ethereum version for reproducible output. check-bindings regenerates
and fails on diff. New bindings-check workflow runs it on PRs to main.

Verified: regenerating reproduces the committed StorageRWv1.go
byte-identically (macOS solc -> linux solc-static-linux, abigen v1.16.1).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit aa93ebf. Configure here.

Comment thread Makefile Outdated
bdchatham and others added 3 commits June 11, 2026 15:05
- check-bindings: force clean rebuild via `make -B generate-bindings` so
  Make's mtime logic can't skip regen (CI checkout can leave a committed
  binding newer than rebuilt .abi/.bin); `git add -N` makes the gate also
  fail on untracked NEW bindings; print full diff (not --stat) on failure.
- Pin solc 0.8.19 by sha256 (SOLC_SHA256), verified after download in both
  setup-node and the CI step. Hash cross-verified against the official
  Solidity release index (binaries.soliditylang.org list.json).
- bindings-check.yml: add top-level `permissions: contents: read`.
- Fix `make help` dash-column alignment (pad to 17 chars).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
check-bindings reset the index for $(BINDINGS_DIR) regardless of drift
outcome so a local run leaves git status untouched (it previously left
`git add -N` intent staged). solc downloads in setup-node and CI now use
`curl --fail` so an HTTP error fails loudly instead of writing an HTML
error page that only the hash check would catch.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Pin the solc compile target explicitly and strip the CBOR metadata hash.

Why paris: it is solc 0.8.19's highest supported target (its implicit
default), so pinning it makes that default a written invariant. A future
solc bump can no longer silently emit newer opcodes (PUSH0/MCOPY/TSTORE)
and shift the bytecode/gas surface. paris is a strict subset of Sei's
Cancun/Pectra-era forks (paris ⊂ Sei), so paris-targeted bytecode is
unconditionally safe to deploy; runtime gas is set by the chain's active
fork regardless of compile target, so the target never distorts the
load-benchmark measurements.

Why --metadata-hash none: strips the trailing CBOR IPFS metadata hash
(which embeds source paths), making bytecode reproducible across repo
paths / build hosts and slightly smaller. It does not touch the ABI or
function selectors.

Regenerates all 6 bindings: only the Bin (bytecode) field changes — the
metadata trailer drops from a264...0033 to the minimal a164...000a form.
ABI is byte-identical across all contracts; StorageRWv1 selectors
(read/rmw/write/VERSION) are signature-derived and unchanged. check-bindings
and go build pass.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@bdchatham

Copy link
Copy Markdown
Contributor Author

Systems review — APPROVE (+ EVM-target decision, researched)

Final systems-engineer pass: APPROVE, no blocking items. Two strongly-recommended solc-flag hardenings (the angle security/idiom structurally don't cover — what the compiler stamps into the deployed artifact) are now applied:

  • --evm-version paris (explicit). Researched against sei-chain (x/evm/types/config.go, HEAD 2026-06-07): Sei runs a geth fork v1.15.7-sei-16 with Shanghai + Cancun + Prague active from genesis ("Pectra w/o blobs"), so paris-targeted bytecode is a strict subset — unconditionally safe to deploy. Crucially, --evm-version is a compile-time opcode-availability knob, not a runtime-gas knob: runtime gas is charged by the chain's active fork regardless of target, so paris-targeting does not distort measured gas. paris is solc 0.8.19's ceiling anyway, and StorageRWv1 (only SLOAD/SSTORE/ADD/mapping) emits identical bytecode under paris/shanghai/cancun. Pinning it explicitly prevents a future solc bump from silently changing emitted opcodes (the "measurement discontinuity" risk). Consistent with all existing repo contracts.
  • --metadata-hash none. Removes the path-dependent IPFS metadata trailer → reproducible bytecode across repo layout / build host, and avoids phantom "drift" on a future repo reorg.

Both flags regenerate all 6 bindings — verified ABIs are byte-identical (only bytecode changed) and all selectors are preserved.

Deferred (not now): matching Cancun via a solc bump (≥0.8.24) buys nothing measurable for the current SLOAD/SSTORE contracts; un-defer only if we add a transient-storage (TSTORE/TLOAD) benchmark variant or want to measure Cancun-specific opcodes as first-class instruments.

@bdchatham
bdchatham requested a review from masih June 11, 2026 22:29
@bdchatham bdchatham assigned amir-deris and unassigned amir-deris Jun 11, 2026
@bdchatham
bdchatham requested a review from amir-deris June 11, 2026 22:29
@bdchatham
bdchatham merged commit e7374af into main Jun 11, 2026
4 checks passed
@bdchatham
bdchatham deleted the brandon2/plt-457-storagerwv1-contract branch June 11, 2026 22:49
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.

2 participants