Skip to content

perf(gas): transient reentrancy lock via solc 0.8.28+, and one fewer configHash SLOAD - #55

Draft
pviti wants to merge 4 commits into
v2.0.0from
perf/gas-transient-lock-and-slot-writes
Draft

perf(gas): transient reentrancy lock via solc 0.8.28+, and one fewer configHash SLOAD#55
pviti wants to merge 4 commits into
v2.0.0from
perf/gas-transient-lock-and-slot-writes

Conversation

@pviti

@pviti pviti commented Jul 29, 2026

Copy link
Copy Markdown
Member

Two of #45's three items. Every guarded entrypoint gets cheaper:

entrypoint main now Δ
authorize 151,870 149,973 −1,897
charge 153,077 151,180 −1,897
capture 178,347 176,692 −1,655
void 137,247 135,455 −1,792
release 135,647 133,811 −1,836
refund 204,193 202,229 −1,964

Measured against a clean main worktree, not a hand-reverted tree — partial reverts move this contract's optimizer output enough to produce a false baseline, which cost me a wrong number earlier in review.

1. Reentrancy lock → EIP-1153 transient storage

The issue estimated ~2k per call and that is what it delivers. (An earlier ~4.8k estimate I gave in discussion was wrong: it ignored the EIP-3529 refund for restoring a slot to its original value. The 1 → 2 → 1 pattern earns that refund.)

Solidity 0.8.27 has no transient keyword — that arrived in 0.8.28 — so this is two one-line assembly helpers, with the Reentrancy() custom error kept in Solidity rather than hand-rolled in asm.

Chain safety is not a new requirement

This was the part worth being sure about, so it is established two independent ways:

  • the deployed bytecode already contains MCOPY (a Cancun opcode, EIP-5656) — so all six live deployments are on Cancun chains today, and they work;
  • TLOAD was probed directly on all five active chains earlier (Arc, Base Sepolia, OP Sepolia, Arbitrum Sepolia, Polygon Amoy).

evm_version is now pinned to "cancun" rather than inherited from forge's default (currently "prague"). That states the dependency, is more conservative than today's implicit target, and stops a future forge default from silently raising the requirement.

The semantic trap, pinned

A transient slot clears at the end of the transaction, not the call. So the guard must still release the lock explicitly — otherwise the first guarded call in a transaction poisons every later one, breaking any multicall or smart-account batch.

test_Reentrancy_TwoGuardedCallsInOneTransaction covers exactly that, and nothing else in the suite would: every other test makes one guarded call per transaction. Removing the release makes it fail with Reentrancy() — verified, not assumed.

2. One fewer SLOAD in refund

_loadAndVerify already loads the configHash to compare it against _hash(p), so it now returns it. refund stops re-reading the same slot to derive its EIP-3009 nonce.

3. The third item is deliberately NOT included — it costs gas

Collapsing capture's two packed-field writes into one struct write measures +2,352 gas (179,044 vs 176,692), turning a saving into a regression.

The IR optimizer was already coalescing the two field writes. Writing the struct instead forces the whole 256-bit word to be assembled from four values. The issue's premise — that this "removes reliance on the IR optimizer coalescing the writes" — was right about the mechanism and wrong about the price.

test_Capture_PreservesAnOpenDispute was written while evaluating it and is kept: a successful capture must not clear an open dispute, test_Capture_IsNotBlockedByAnOpenDispute does not cover it (it charges first, so the capture reverts and a dropped flag would go unnoticed), and sabotaging the flag makes the new test fail with the right message.

I'll leave #45 open for that item with the measurement recorded, rather than closing it as done.

Verification

122 tests pass. forge fmt --check clean. forge lint reports the same 9 pre-existing block-timestamp warnings as main — compared side by side, not assumed.

Bundle with the version bump that #53 and #54 already need.

Refs #45

🤖 Generated with Claude Code


⚠️ TODO before merge — a fifth file now needs the ^0.8.31 pragma

This branch pins solc = "0.8.31" in foundry.toml and unifies four pragmas to ^0.8.31:

  • src/RAIL0.sol
  • src/interfaces/IERC20.sol
  • script/Deploy.s.sol
  • test/RAIL0.t.sol

#57 (on main) adds a fifth: src/interfaces/IEIP3009.sol. It splits IEIP3009 out of IERC20.sol into its own file, created at ^0.8.27 to match main. This branch's base (v2.0.0v1.4.0 → pre-#57 main) does not contain that file, so the bump cannot be applied here — the fix has to happen when the two meet.

Whichever lands second must bump it:

# after #57 and this branch are in the same tree
grep -rn 'pragma solidity' contracts/ | grep -v '0.8.31'   # must print nothing

Leaving it at ^0.8.27 would not break the build — ^ is a floor, and solc = "0.8.31" satisfies it — which is exactly why it would go unnoticed. The reason to fix it anyway is the one this branch already argues: one stated version across the repo, so no file silently claims compatibility with a compiler nobody tested it against.

Two of #45's three items. Every guarded entrypoint gets cheaper:

  authorize  151,870 → 149,973   -1,897
  charge     153,077 → 151,180   -1,897
  capture    178,347 → 176,692   -1,655
  void       137,247 → 135,455   -1,792
  release    135,647 → 133,811   -1,836
  refund     204,193 → 202,229   -1,964

Measured against a clean `main` worktree, not a hand-reverted tree — partial reverts
move this contract's optimizer output enough to produce a false baseline.

1) The reentrancy lock moves from storage to EIP-1153 transient storage. The issue
estimated ~2k per call and that is what it delivers; an earlier estimate of ~4.8k in
discussion was wrong because it ignored the EIP-3529 refund for restoring a slot to
its original value.

Solidity 0.8.27 has no `transient` keyword (0.8.28+), so this is two one-line
assembly helpers with the custom error kept in Solidity.

Chain safety is not a new requirement: the deployed bytecode ALREADY contains MCOPY,
a Cancun opcode, so all six live deployments are on Cancun chains today. TLOAD was
also probed directly on all five active chains earlier. evm_version is now pinned to
"cancun" rather than inherited from forge's default (currently "prague"), so the
dependency is stated and a future forge default cannot silently raise it.

Transient storage clears at the end of the TRANSACTION, not the call, so the guard
must still release the lock explicitly — otherwise the first guarded call in a
transaction poisons every later one, breaking a multicall or smart-account batch.
test_Reentrancy_TwoGuardedCallsInOneTransaction pins it; removing the release makes
it fail with Reentrancy(), verified.

2) _loadAndVerify now returns the configHash it already loads to compare, so refund
stops re-reading the same slot to derive its EIP-3009 nonce.

3) The third item — collapsing capture's two packed-field writes into one struct
write — is DELIBERATELY NOT INCLUDED. Measured, it costs +2,352 gas in capture
(179,044 vs 176,692), turning a saving into a regression. The IR optimizer was
already coalescing the two field writes; writing the struct instead forces the whole
256-bit word to be assembled from four values. The issue's premise that this "removes
reliance on the optimizer" was right about the mechanism and wrong about the price.

test_Capture_PreservesAnOpenDispute was written while evaluating that item and is
kept: a successful capture must not clear an open dispute, nothing else covered it,
and sabotaging the flag makes it fail with the right message.

Suite: 122 tests pass. forge fmt --check clean; forge lint reports the same 9
pre-existing block-timestamp warnings as main.

Refs #45

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Replaces the inline assembly from the previous commit with a `transient` state
variable, which needs Solidity 0.8.28+. The pragma on RAIL0.sol moves to ^0.8.28
accordingly; foundry.toml pins the exact build at 0.8.31.

This is not only tidier — it is cheaper, measured:

                    asm      transient    Δ
  authorize      149,973 →    149,884    -89
  charge         151,180 →    151,091    -89
  capture        176,692 →    176,336   -356
  void           135,455 →    135,277   -178
  release        133,811 →    133,633   -178
  refund         202,229 →    202,051   -178

Which brings the totals against main to roughly -2,000 on every guarded entrypoint:

  authorize  151,870 → 149,884   -1,986
  charge     153,077 → 151,091   -1,986
  capture    178,347 → 176,336   -2,011
  void       137,247 → 135,277   -1,970
  release    135,647 → 133,633   -2,014
  refund     204,193 → 202,051   -2,142

`uint256 transient` rather than `bool transient`, also measured: the bool costs
140–557 gas MORE because every read and write carries its 0/1 normalisation. Nothing
here needs a bool.

Dropping the assembly matters beyond gas. foundry.toml already excludes the
asm-keccak256 lint with the note that "inline assembly hurts readability — not worth
the trade", so a hand-rolled tload/tstore block sat against the project's own stated
preference. Now there is none: zero assembly blocks in the contract.

0.8.29, 0.8.30 and 0.8.31 were each built and tested — identical gas, 122 tests
passing on all. 0.8.31 is pinned as the latest patch release, since staying lower
carries any compiler fix since for no measurable benefit; evm_version is pinned
separately, so the compiler version does not drag the EVM target with it.

Only RAIL0.sol's pragma moves. interfaces/IERC20.sol and the test do not use
`transient`, so ^0.8.27 remains an accurate statement of what they require — the
pragma says what a file needs, not what the project happens to build with.

README's two version claims now also state the Cancun requirement explicitly, which
was previously implicit (the deployed bytecode has always contained MCOPY).

Suite: 122 tests pass. forge fmt --check clean; forge lint reports the same 9
pre-existing block-timestamp warnings as main.

Refs #45
@pviti pviti changed the title perf(gas): transient reentrancy lock and one fewer configHash SLOAD perf(gas): transient reentrancy lock via solc 0.8.28+, and one fewer configHash SLOAD Jul 29, 2026
@pviti

pviti commented Jul 29, 2026

Copy link
Copy Markdown
Member Author

Updated: the assembly is gone — 0.8.28's transient keyword replaces it, and it is cheaper

Answering "can we move to Solidity 0.8.28 or higher": yes, nothing blocked it, and it improves this PR on both axes.

The only pin was contracts/foundry.toml (solc = "0.8.27"); the pragma was already ^0.8.27, which admits 0.8.28+. Beyond that, two README lines and one CI comment mention the version — all updated.

The lock is now a transient state variable, not inline assembly

Measured, both on 0.8.31:

entrypoint asm transient Δ
authorize 149,973 149,884 −89
charge 151,180 151,091 −89
capture 176,692 176,336 −356
void 135,455 135,277 −178
release 133,811 133,633 −178
refund 202,229 202,051 −178

So the tidier form is also the cheaper one — the compiler tracks the slot itself rather than working around an opaque asm block. Zero assembly blocks remain in the contract.

That matters beyond gas: foundry.toml already excludes the asm-keccak256 lint with the note that "inline assembly hurts readability — not worth the trade". A hand-rolled tload/tstore block sat directly against the project's own stated preference. It no longer exists.

uint256 transient, not bool transient — also measured

A bool transient costs 140–577 gas more per entrypoint, because every read and write carries the bool's 0/1 normalisation. Nothing here needs the type to be a bool.

Totals against main

entrypoint main now Δ
authorize 151,870 149,884 −1,986
charge 153,077 151,091 −1,986
capture 178,347 176,336 −2,011
void 137,247 135,277 −1,970
release 135,647 133,633 −2,014
refund 204,193 202,051 −2,142

≈ −2,000 across the board, matching the issue's original estimate.

Version choices, stated rather than defaulted

  • solc = "0.8.31". 0.8.28, 0.8.29, 0.8.30 and 0.8.31 were each built and tested: identical gas, 122 tests passing on all four. 0.8.31 is pinned as the latest patch, since staying lower carries any compiler fix since for no measurable benefit. evm_version is pinned separately, so the compiler version does not drag the EVM target along.
  • Only RAIL0.sol's pragma moves to ^0.8.28. interfaces/IERC20.sol and the test do not use transient, so ^0.8.27 remains an accurate statement of what those files require. A pragma should say what a file needs, not what the project happens to build with — say if you'd rather they were uniform.
  • README now states the Cancun requirement explicitly. It was previously implicit: the deployed bytecode has always contained MCOPY.

122 tests pass, forge fmt --check clean, forge lint unchanged from main (same 9 pre-existing block-timestamp warnings).

pviti added 2 commits July 30, 2026 00:50
Follow-up on review: all three pragmas now name 0.8.31, matching the pinned
compiler, instead of RAIL0.sol at ^0.8.28 and the other two left at ^0.8.27.

The earlier split was defended as "a pragma states what a file requires". That is
true in the abstract and wrong here, for a reason worth recording: NOTHING below the
pin is ever compiled. CI runs `forge build` and `forge test` with the foundry.toml
pin and nothing else, so ^0.8.27 advertised compatibility with 0.8.27 through 0.8.30
that no test exercises — on a contract that holds funds.

Nor does anything need the lower bound. No repo imports these sources: the gateway,
indexer and CLI consume the compiled ABI, and every other reference to RAIL0.sol
across the project is a comment citing its behaviour.

So the range was a claim, not a requirement, and narrowing it costs nothing.

README's two version lines follow.

122 tests pass; gas unchanged (capture 176,336); forge fmt --check clean.

Refs #45
script/Deploy.s.sol was left at ^0.8.27 by the previous commit: the sweep that
updated the pragmas covered src/ and test/ but not script/, and the earlier survey
that did include it collapsed identical lines through `sort -u`, so the omission was
invisible in both.

It is the one file where a stale pragma would matter most — the deploy script is what
produces the bytecode that goes on chain.

All four .sol files under src/, test/ and script/ now read ^0.8.31; verified by
enumerating every .sol in the repo rather than the directories I expected to matter.

122 tests pass, forge fmt --check clean, build succeeds.

Refs #45
@pviti

pviti commented Jul 29, 2026

Copy link
Copy Markdown
Member Author

Pragmas unified at ^0.8.31 — the split was wrong

On review: all four .sol files now name 0.8.31, matching the pinned compiler, instead of RAIL0.sol at ^0.8.28 with the rest left at ^0.8.27.

I defended the split as "a pragma states what a file requires". True in the abstract, wrong here, for a reason worth recording:

  • Nothing below the pin is ever compiled. CI runs forge build and forge test with the foundry.toml pin and nothing else. ^0.8.27 advertised compatibility with 0.8.27–0.8.30 that no test exercises — on a contract that holds funds.
  • Nothing needs the lower bound. No repo imports these sources: the gateway, indexer and CLI consume the compiled ABI, and every other RAIL0.sol reference across the project is a comment citing behaviour.

So the range was a claim, not a requirement, and narrowing it costs nothing.

And it caught a real miss

script/Deploy.s.sol was still on ^0.8.27 after the first pass. My sweep had covered src/ and test/ but not script/ — and the earlier survey that did include it collapsed identical lines through sort -u, so the omission was invisible twice. It is the file where a stale pragma matters most, since the deploy script produces the bytecode that goes on chain.

Fixed by enumerating every .sol in the repo rather than the directories I assumed were relevant:

src/RAIL0.sol              ^0.8.31
src/interfaces/IERC20.sol  ^0.8.31
test/RAIL0.t.sol           ^0.8.31
script/Deploy.s.sol        ^0.8.31

Optional, if you want it stricter

^0.8.31 still permits 0.8.32+ once released — also untested. Dropping the caret (pragma solidity 0.8.31;) would allow only the exact compiler the deployments use, which is common practice for audited contracts and costs nothing here since nobody imports the sources. The trade is that a future compiler bump touches four files instead of one — arguably a feature, since it makes the bump explicit. Say the word.

122 tests pass, gas unchanged (capture 176,336), forge fmt --check clean.

@pviti

pviti commented Jul 30, 2026

Copy link
Copy Markdown
Member Author

Cross-repo note, added to the description too so it is not lost in the timeline.

#57 (targeting main) splits IEIP3009 out of IERC20.sol into a new src/interfaces/IEIP3009.sol, created at pragma solidity ^0.8.27 to match main. This branch unifies four pragmas to ^0.8.31 and pins solc = "0.8.31" — but its base predates #57, so that fifth file cannot be bumped here.

Whichever of the two lands second has to bump it. The check:

grep -rn 'pragma solidity' contracts/ | grep -v '0.8.31'   # must print nothing

It would not break the build — ^0.8.27 is a floor and 0.8.31 satisfies it — which is precisely why it would slip through unnoticed.

(#57 needs no version bump of its own: interfaces are compile-time only and its bytecode is byte-identical to main, verified at 7,212 bytes / exec-hash 6ce7c95abc273d34.)

@pviti pviti added this to the v2.0.0 milestone Jul 30, 2026
@pviti pviti added the target: v2.0.0 Merges into the v2.0.0 branch — breaking contract release label Jul 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

target: v2.0.0 Merges into the v2.0.0 branch — breaking contract release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant