Skip to content

Commit e32e478

Browse files
p-diogoclaude
andauthored
feat(contracts): TotalReclawAccount — revocable session-key signing (P3-1, P3-2, P3-4) (#636)
* feat(contracts): TotalReclawAccount — revocable session-key signing (P3-1, P3-2, P3-4) Implements Option E Phase 3's account-model decision (D1, approved): a `SimpleAccount` (eth-infinitism v0.7.0) subclass, `TotalReclawAccount`, that accepts an owner-EOA signature (owner-first, byte-identical to stock) OR a scoped, revocable session-key signature — replacing the standalone-module design (`SessionKeyModule.sol`, never deployed, and per the account-model memo §5 unable to pass ERC-7562 bundler validation because its grant registry keys on `signer` rather than `msg.sender`). P3-1/P3-2 — `contracts/account/contracts/TotalReclawAccount.sol`: - Constructor-less (hardcoded canonical v0.7 EntryPoint), so CREATE2 init code is bare `creationCode`. - All new session-key state in ERC-7201 namespaced storage; zero plain state variables added (verified — see storage-layout gate below). - `_grantDigest`, the four EIP-712 typehash/domain constants, `_isCallDataInScope`/`_isBatchInScope`/`_inSelectors`, `_recoverEcdsa` (EIP-2 malleability guard), and monotonic `_minNonces` replay protection ported VERBATIM from `SessionKeyModule.sol` per the spec's explicit instruction. - P3-3's `verifyingContract` → account-address binding applied as part of the module→account move (falls out of `address(this)` now meaning the account, not a separate module). - Deliberate deviation from a literal "call stock's exact code" port: the owner path uses `ECDSA.tryRecover` instead of `ECDSA.recover` (which reverts on non-65-byte input), because a literal port would revert the whole `validateUserOp` on every session-key lazy-install UserOp before ever reaching the session-key branch. Reasoned through and documented at length in the function's NatSpec — flagged in the PR description as the one thing to scrutinize hardest. - No custom errors: `ISessionKeyModule.sol`'s three dead errors and `SessionKeyModule.sol`'s two more (never reverted, by the same defect) are not carried forward; validation fails closed via SIG_VALIDATION_FAILED and `revokeSessionKey` reuses the inherited `onlyOwner` modifier's existing "only owner" require. - `SessionKeyModule.sol` gains a `grantDigest` external pure wrapper so `SessionKeyModule.t.sol`'s `_grantDigestExt` calls the production digest path instead of a hand-copied mirror of it (the exact duplication pattern that let the §2 `bytes4[]` selector-hashing bug ship unnoticed). P3-4 — tests (`contracts/account/test/`): - `TotalReclawAccount.t.sol`: 29 fast, non-forked unit tests (fresh ERC1967Proxy + Foundry-controlled owner) — the six executeBatch scope-invariant tests ported from `SessionKeyModule.t.sol:424-591`, plus owner-path/session-key/revoke/replay/grant-binding coverage, a pinned CREATE2-address test (NOT the tautological `test_create2_address_unchanged_after_module_install` pattern), and a local slot-0 storage check. - `TotalReclawAccountFork.t.sol`: 6 read-only fork tests against live Gnosis mainnet. Upgrades the REAL, already-deployed, 66-UserOp-history account (`0x2c0CF74B2b76110708CA431796367779e3738250`) via the owner EOA's direct `upgradeToAndCall` (the audit-risk memo §5 escape hatch) and asserts owner()/slot-0/EntryPoint-nonce are byte-unchanged, plus upgrade idempotence. A second account, deployed fresh through the real, live factory with a Foundry-controlled owner (we do not and should not hold the real owner's key), exercises the full signature paths: owner-signed validates, session-key lazy-install + steady-state validates, out-of-scope is rejected, and revocation actually blocks reuse. - `storage-layout-diff.sh`: CI gate wiring `forge inspect ... storageLayout` (already emitted, never consumed before this) — asserts `TotalReclawAccount`'s storage array is byte-identical to the `SimpleAccount` it subclasses. Currently: one entry, slot 0, `owner`, in both. Isolation note: `contracts/account/` is a deliberately SEPARATE Foundry project (own `foundry.toml`/`lib`/`remappings.txt`), not new files under the existing `contracts/` tree. Adding `account-abstraction`/ `openzeppelin-contracts` to the outer project's `lib/`+`remappings.txt` was tried first and reverted: solc embeds the full project-wide `settings.remappings` into EVERY compiled contract's metadata regardless of use, which shifted `EventfulDataEdge`'s bytecode and broke `DataEdgeStagingCreate2.t.sol`'s pinned CREATE2 address for the real deployed staging DataEdge — a self-inflicted change to a contract this PR has nothing to do with. Full isolation via a separate project root was the only fix that left the outer project's compiled output byte-for-byte unaffected (re-verified: 26/26 outer tests pass unchanged). Test results: 35/35 passing in contracts/account (29 unit + 6 fork, including live-Gnosis fork tests); 26/26 unchanged in the outer contracts/ project. Out of scope (per assignment): core Rust (P3-5), Python client (P3-6+), SPA (P3-13), subgraph (P3-14), and P3-15 docs (CLAUDE.md/SECURITY.md). Never deployed anything; never sent a transaction; fork tests are read-only simulation against a public RPC. Refs: totalreclaw-internal #621 task 4 (P3-1, P3-2, P3-4). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(contracts): make TotalReclawAccount creationCode deterministic `test_pinned_implementation_create2_address` passed locally and FAILED in CI, predicting a different address: CI: 0x7D12D9f0Ad437895c65F9dC7580cC712c8268066 pinned: 0x25D4E71865E8A47F010219526A618f950a8f86B4 Not a flaky test — the build was not reproducible. Foundry's default `bytecode_hash = "ipfs"` appends a hash of the solc metadata JSON to the bytecode, and that JSON varies with toolchain details beyond the pinned `solc_version`, so the same source compiled in two environments produces two different `creationCode` values and therefore two different CREATE2 addresses. Pinning `solc_version` and the optimizer settings (already done) is necessary but not sufficient. This matters well beyond the test. The deployment plan pins a CREATE2 address for the implementation, and a pin that depends on WHO compiled the contract is not a pin — nobody could independently reproduce or verify the deployed implementation, which is precisely the property an external auditor and any user checking our deployment would rely on. Fix: `bytecode_hash = "none"` + `cbor_metadata = false` in contracts/account/foundry.toml. `creationCode` becomes a pure function of (source, solc version, optimizer settings). Verified the CBOR metadata trailer is gone from the compiled object. Pinned address updated to the post-fix deterministic value 0x6d17938F29072B3b7E883d410076B0D73676Cf34, with the reasoning recorded on the test so a future reader does not "fix" it by re-pinning to whatever their own machine emits. Trade-off accepted and noted: stripping the metadata hash makes block-explorer verification marginally less automatic. Exact-settings verification still works; determinism is worth more for a CREATE2 deploy. Verified after the change: account project 29 unit + 6 fork (live Gnosis) = 35/35; outer contracts project 26/26 unaffected. Refs: #621 task 4, P3-1/P3-4 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent c6056db commit e32e478

11 files changed

Lines changed: 2017 additions & 30 deletions

File tree

‎.github/workflows/forge-test.yml‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,3 +70,77 @@ jobs:
7070
- name: Snapshot gas (diagnostic)
7171
run: forge snapshot --check --diff || true
7272
continue-on-error: true
73+
74+
forge-test-account:
75+
name: Forge Test (TotalReclawAccount)
76+
# Option E Phase 3 (P3-1/P3-2/P3-4). This is a SEPARATE job (not a step
77+
# folded into `forge-test` above) because `contracts/account/` is a
78+
# deliberately SEPARATE Foundry project with its own `foundry.toml` and
79+
# `lib/` — see `contracts/account/foundry.toml`'s header comment for
80+
# why: adding `account-abstraction`/`openzeppelin-contracts` to the
81+
# OUTER project's `lib/`+`remappings.txt` was tried first and reverted,
82+
# because solc embeds the full project-wide `settings.remappings` list
83+
# into EVERY compiled contract's metadata regardless of whether that
84+
# contract uses them — verified to shift `EventfulDataEdge`'s bytecode
85+
# and break `test/DataEdgeStagingCreate2.t.sol`'s pinned CREATE2
86+
# address for the real, deployed staging DataEdge. Keeping this a
87+
# fully separate `forge build`/`forge test` invocation (own working
88+
# directory, own project root) is what guarantees the outer job's
89+
# compiled artifacts can never be perturbed by this one.
90+
runs-on: ubuntu-latest
91+
timeout-minutes: 15
92+
defaults:
93+
run:
94+
working-directory: contracts/account
95+
96+
steps:
97+
- name: Checkout
98+
uses: actions/checkout@v4
99+
with:
100+
submodules: recursive
101+
102+
- name: Install Foundry
103+
uses: foundry-rs/foundry-toolchain@v1
104+
with:
105+
version: stable
106+
107+
- name: Show forge version
108+
run: forge --version
109+
110+
# contracts/account/lib/ is gitignored — install pinned versions
111+
# (matching contracts/account/foundry.lock) fresh on every run.
112+
- name: Install vendored libs (forge-std, account-abstraction, openzeppelin-contracts)
113+
run: |
114+
forge install --no-git foundry-rs/forge-std@v1.16.1
115+
forge install --no-git OpenZeppelin/openzeppelin-contracts@v5.0.2
116+
forge install --no-git eth-infinitism/account-abstraction@v0.7.0
117+
118+
- name: Build
119+
run: forge build --sizes
120+
121+
# Storage-layout diff gate (B2 mitigation) — TotalReclawAccount must
122+
# declare ZERO plain state variables beyond what it inherits from
123+
# SimpleAccount; all new session-key state lives in ERC-7201
124+
# namespaced storage, which correctly does not appear in solc's
125+
# storageLayout output. See script/storage-layout-diff.sh.
126+
- name: Storage layout diff (TotalReclawAccount vs SimpleAccount baseline)
127+
run: ./script/storage-layout-diff.sh
128+
129+
# Includes the fork test suite (TotalReclawAccountFork.t.sol), which
130+
# forks live Gnosis mainnet against a public RPC
131+
# (https://rpc.gnosischain.com by default; override with
132+
# GNOSIS_RPC_URL repo/environment secret for a private endpoint with
133+
# less exposure to public-RPC rate limits). Read-only — never
134+
# broadcasts a transaction. If this specific job is flaky on public
135+
# RPC availability, that is a CI-infra follow-up, not a reason to
136+
# drop the fork tests — they are the highest-value test in the phase
137+
# (phase3-impl spec §3.4) and must keep running on every PR that
138+
# touches this contract.
139+
- name: Run tests (including fork tests against live Gnosis)
140+
env:
141+
# GH Actions `||` falls back on an unset/empty secret (empty
142+
# string is falsy in expression context) — do NOT rely on
143+
# `vm.envOr`'s unset-vs-empty handling here, resolve it in YAML
144+
# instead so an absent secret can never resolve to "".
145+
GNOSIS_RPC_URL: ${{ secrets.GNOSIS_RPC_URL || 'https://rpc.gnosischain.com' }}
146+
run: forge test -vvv

‎.gitignore‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,4 +132,12 @@ mcp/package-lock.json
132132
# Foundry vendored libraries — install via 'forge install foundry-rs/forge-std' from contracts/
133133
contracts/lib/forge-std/
134134
contracts/out/
135+
136+
# contracts/account/ is its OWN Foundry project (Option E Phase 3 --
137+
# TotalReclawAccount), deliberately isolated from contracts/foundry.toml.
138+
# See contracts/account/foundry.toml's header comment. Libs pinned in
139+
# contracts/account/foundry.lock, reinstalled by CI and by hand locally.
140+
contracts/account/lib/
141+
contracts/account/out/
142+
contracts/account/cache/
135143
.desloppify/

‎contracts/account/contracts/TotalReclawAccount.sol‎

Lines changed: 670 additions & 0 deletions
Large diffs are not rendered by default.

‎contracts/account/foundry.lock‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
{
2+
"lib/forge-std": {
3+
"tag": {
4+
"name": "v1.16.1",
5+
"rev": "620536fa5277db4e3fd46772d5cbc1ea0696fb43"
6+
}
7+
},
8+
"lib/openzeppelin-contracts": {
9+
"tag": {
10+
"name": "v5.0.2",
11+
"rev": "dbb6104ce834628e473d2173bbc9d47f81a9eec3"
12+
}
13+
},
14+
"lib/account-abstraction": {
15+
"tag": {
16+
"name": "v0.7.0",
17+
"rev": "7af70c8993a6f42973f520ae0752386a5032abe7"
18+
}
19+
}
20+
}

‎contracts/account/foundry.toml‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
# Foundry config for `TotalReclawAccount` (Option E Phase 3, P3-1/P3-2).
2+
#
3+
# WHY THIS IS A SEPARATE FOUNDRY PROJECT FROM `contracts/foundry.toml`,
4+
# NOT JUST A NEW FILE IN THE EXISTING ONE:
5+
#
6+
# `TotalReclawAccount` subclasses eth-infinitism's `SimpleAccount`, which
7+
# needs the real `account-abstraction` (v0.7.0) and `openzeppelin-contracts`
8+
# (v5.0.2) packages as Foundry libs. Adding those libs' directories (or a
9+
# `remappings.txt` naming them) under the SHARED `contracts/lib/` /
10+
# `contracts/remappings.txt` was tried first and reverted: solc embeds the
11+
# FULL, PROJECT-WIDE `settings.remappings` list into every compiled
12+
# contract's metadata, REGARDLESS of whether that specific contract
13+
# actually imports anything through those remappings. Verified empirically:
14+
# adding the two new remapping lines (with no other change, not even the
15+
# lib directories present) shifted `EventfulDataEdge`'s compiled bytecode
16+
# and broke `test/DataEdgeStagingCreate2.t.sol`'s pinned CREATE2 address
17+
# assertion for the STAGING DataEdge (`0xE7a4D2677B686e13775Ba9092631089e35F0BB91`,
18+
# a REAL deployed contract) — a self-inflicted, purely incidental bytecode
19+
# change to a contract this PR has nothing to do with. A nested Foundry
20+
# project (its own `foundry.toml`, `lib/`, `remappings.txt`) is the only
21+
# way to add `account-abstraction`/`openzeppelin-contracts` without that
22+
# collateral effect: `forge` resolves the project root as the nearest
23+
# `foundry.toml` walking up from the working directory, so a `forge build`
24+
# / `forge test` run from THIS directory never sees, and cannot perturb,
25+
# the outer project's compiler settings or vendored libs.
26+
#
27+
# Run from here: `cd contracts/account && forge build && forge test`.
28+
# CI wiring: `.github/workflows/forge-test.yml` (separate job/step, see
29+
# comment there for why it must not be folded into the outer job).
30+
31+
[profile.default]
32+
src = "contracts"
33+
test = "test"
34+
out = "out"
35+
libs = ["lib"]
36+
solc_version = "0.8.24"
37+
# Deterministic CREATE2: strip the appended metadata hash so `creationCode`
38+
# is a pure function of (source, solc version, optimizer settings) and the
39+
# predicted implementation address reproduces on any machine/toolchain.
40+
bytecode_hash = "none"
41+
cbor_metadata = false
42+
optimizer = true
43+
optimizer_runs = 200
44+
via_ir = false
45+
extra_output = ["storageLayout"]
46+
47+
[fmt]
48+
line_length = 100
49+
tab_width = 4
50+
bracket_spacing = false

‎contracts/account/remappings.txt‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
@openzeppelin/contracts/=lib/openzeppelin-contracts/contracts/
2+
account-abstraction/=lib/account-abstraction/
3+
forge-std/=lib/forge-std/src/
Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
#!/usr/bin/env bash
2+
# Option E Phase 3 (P3-4) — storage-layout diff CI gate.
3+
#
4+
# `TotalReclawAccount` MUST declare zero plain state variables beyond what
5+
# it inherits from `SimpleAccount` (B2 mitigation: a state variable that
6+
# collides with `owner` at slot 0 is catastrophic and silent, per
7+
# `docs/plans/2026-08-02-option-e-phase3-audit-risk.md` §2). All new
8+
# session-key state lives in ERC-7201 namespaced storage, accessed via a
9+
# fixed-slot inline-assembly getter, which does NOT show up in solc's
10+
# `storageLayout.storage` array — so the strongest possible assertion of
11+
# "no new plain state variables" is: this array is BYTE-IDENTICAL between
12+
# `TotalReclawAccount` and the `SimpleAccount` it subclasses.
13+
#
14+
# `contracts/account/foundry.toml` already sets `extra_output =
15+
# ["storageLayout"]`; this script is what consumes it (the phase3-impl
16+
# spec §3.1/§3.4 note that nothing did, before this).
17+
#
18+
# Usage: contracts/account/script/storage-layout-diff.sh
19+
# Exit 0 = layouts match. Exit 1 = they differ (or either inspect failed).
20+
21+
set -euo pipefail
22+
cd "$(dirname "$0")/.."
23+
24+
echo "Building..."
25+
forge build >/dev/null
26+
27+
echo "Inspecting TotalReclawAccount storage layout..."
28+
forge inspect TotalReclawAccount storageLayout --json > /tmp/tra-storage-layout.json
29+
30+
echo "Inspecting SimpleAccount (the deployed baseline's implementation contract) storage layout..."
31+
forge inspect SimpleAccount storageLayout --json > /tmp/simple-account-storage-layout.json
32+
33+
# Compare only the `.storage` array (slot, offset, type, label) — the
34+
# `.types` dictionary can differ trivially in ordering/naming without any
35+
# real layout difference, so it is not part of the gate.
36+
python3 - <<'PY'
37+
import json
38+
import sys
39+
40+
with open("/tmp/tra-storage-layout.json") as f:
41+
tra = json.load(f)["storage"]
42+
with open("/tmp/simple-account-storage-layout.json") as f:
43+
simple = json.load(f)["storage"]
44+
45+
def normalize(entries):
46+
return [
47+
{"slot": e["slot"], "offset": e["offset"], "label": e["label"], "type": e["type"]}
48+
for e in entries
49+
]
50+
51+
tra_n = normalize(tra)
52+
simple_n = normalize(simple)
53+
54+
if tra_n != simple_n:
55+
print("STORAGE LAYOUT MISMATCH — TotalReclawAccount declares state that", file=sys.stderr)
56+
print("SimpleAccount does not (or vice versa). This is exactly the B2", file=sys.stderr)
57+
print("failure class (silent slot-0 collision risk) — do not deploy.", file=sys.stderr)
58+
print("", file=sys.stderr)
59+
print("TotalReclawAccount:", file=sys.stderr)
60+
for e in tra_n:
61+
print(f" slot {e['slot']:>3} offset {e['offset']} {e['label']}: {e['type']}", file=sys.stderr)
62+
print("SimpleAccount:", file=sys.stderr)
63+
for e in simple_n:
64+
print(f" slot {e['slot']:>3} offset {e['offset']} {e['label']}: {e['type']}", file=sys.stderr)
65+
sys.exit(1)
66+
67+
print(f"OK — {len(tra_n)} plain state variable(s), byte-identical between TotalReclawAccount and SimpleAccount:")
68+
for e in tra_n:
69+
print(f" slot {e['slot']:>3} offset {e['offset']} {e['label']}: {e['type']}")
70+
PY

0 commit comments

Comments
 (0)