Skip to content

chore(airaccount): sync M7 module addresses to r4 audit-final + postOp gas floor#22

Merged
fanhousanbu merged 2 commits into
mainfrom
chore/m7-r4-address-sync
May 22, 2026
Merged

chore(airaccount): sync M7 module addresses to r4 audit-final + postOp gas floor#22
fanhousanbu merged 2 commits into
mainfrom
chore/m7-r4-address-sync

Conversation

@fanhousanbu
Copy link
Copy Markdown
Contributor

Summary

M7 Contract Address Sync (breaking)

M7 module addresses updated from r6 → r4 audit-final (freeze/m7-v0.16.0, 660 tests, all audit findings resolved):

Module r6 (deprecated) r4 audit-final
factory 0x42f8... 0x61bb...
accountImpl 0x2F1B... 0xA674...
compositeValidator 0x4135... 0xb655...
tierGuardHook 0x7357... 0x67f8...
agentSessionKeyValidator 0xa3e5... 0x1f06...

r6 addresses retained as @deprecated aliases for legacy account lookups and historical event indexing.

⚠️ Breaking: New factory = new CREATE2 addresses for all newly created AirAccounts. Existing deployed accounts are unaffected.

SuperPaymaster postOp Gas Floor

UserClient + SuperPaymasterClient: replace flat +10k bump with max(estimate + 100k, 200k).

Motivation: burnFromWithOpHash (~40k gas) + storage writes in postOp can exceed the old flat bump under congestion. The 200k floor prevents OOG without over-provisioning.

Files Changed

  • packages/airaccount/src/server/constants/entrypoint.ts — address table
  • packages/enduser/src/UserClient.ts — postOp gas floor
  • packages/paymaster/src/V4/SuperPaymasterClient.ts — postOp gas floor

Test Plan

  • pnpm -r build — no type errors
  • pnpm --filter @aastar/airaccount test — unit tests pass
  • Verify new account creation on Sepolia uses r4 factory address
  • Confirm existing deployed accounts (r6) still queryable via deprecated aliases

…p gas floor

Contract address changes (freeze/m7-v0.16.0, 660 tests, all audit findings resolved):
- factory:                  r6 0x42f8... → r4 0x61bb...
- accountImpl:              r6 0x2F1B... → r4 0xA674...
- compositeValidator:       r6 0x4135... → r4 0xb655...
- tierGuardHook:            r6 0x7357... → r4 0x67f8...
- agentSessionKeyValidator: r6 0xa3e5... → r4 0x1f06...

r6 addresses retained as @deprecated aliases for legacy account lookups.

SuperPaymaster postOp gas tuning (UserClient + SuperPaymasterClient):
- Replace flat +10k bump with max(estimate + 100k, 200k) floor
- Prevents OOG on burnFromWithOpHash (~40k) + storage writes
@fanhousanbu fanhousanbu requested a review from jhfnetboy as a code owner May 21, 2026 16:34
@jhfnetboy
Copy link
Copy Markdown
Member

🤖 Round 1 — Claude Code Review (Local Model)

Reviewer: Claude Code (claude-sonnet-4-6)
Scope: entrypoint.ts address table + UserClient.ts + SuperPaymasterClient.ts gas floor


🔴 Critical

None.


🟠 High

H1 — BigInt → Number unsafe conversion in gas floor expression

// UserClient.ts + SuperPaymasterClient.ts (identical pattern)
BigInt(Math.max(Number(est.paymasterPostOpGasLimit + 100000n), 200000))

Number() conversion of a BigInt silently loses precision for values above 2^53 - 1 (~9 quadrillion). ERC-4337 paymasterPostOpGasLimit is a uint256 on-chain. Bundler estimates are bounded by block gas limit (~30M), so in practice this is safe today — but if any bundler returns an out-of-range value, the result silently becomes wrong with no error.

Safe equivalent using only BigInt arithmetic:

const base = est.paymasterPostOpGasLimit + 100_000n;
paymasterPostOpGasLimit = base > 200_000n ? base : 200_000n;

🟡 Medium

M1 — Address checksum inconsistency

Of the 5 new r4 addresses, only accountImpl uses EIP-55 mixed-case checksum (0xA674D308ce22230B70412b20Ee5a66fC6B24F49c). The other four are all-lowercase:

factory:                  0x61bbaf9e...  ← lowercase
accountImpl:              0xA674D308...  ← EIP-55 ✓
compositeValidator:       0xb65569950c... ← lowercase
tierGuardHook:            0x67f878295c... ← lowercase
agentSessionKeyValidator: 0x1f06961e13... ← lowercase

ethers.js normalizes addresses before comparison so runtime behavior is unaffected, but inconsistency makes auditing harder. If any code outside this file does case-sensitive address comparison (e.g. string includes, regex), mixed-checksum addresses can silently fail.

Recommend checksumming all addresses consistently: ethers.getAddress("0x61bbaf...") or using EIP-55 throughout.

M2 — Gas floor 200k (client) vs 300k fixed (server, PR #21) — different paths, confirm intentional

paymaster-manager.ts (airaccount server, PR #21) packs postGas = 300_000n unconditionally into paymasterAndData. UserClient.ts / SuperPaymasterClient.ts (client SDK, this PR) tune the same field to max(est + 100k, 200k). If the bundler's estimate is low (e.g. 0), client packs 200k while server packs 300k — two code paths that can produce different paymasterAndData for the same operation.

These are separate deployment contexts (server vs client SDK) so may be intentional, but worth a comment confirming the divergence is deliberate.

M3 — No test for the new gas floor logic

The Math.max(estimate + 100k, 200k) floor has no unit test. A test covering:

  • estimate = 0 → result = 200k
  • estimate = 150k → result = 250k
  • estimate = 250k → result = 350k

would protect the floor invariant from silent regression.


🔵 Low

L1 — factoryM7r5Prev missing @deprecated JSDoc

r6 addresses got proper @deprecated JSDoc with link to replacement. factoryM7r5Prev (pre-r6, also superseded) has only a comment but no @deprecated tag. IDE tooling won't warn callers.

L2 — Identical gas floor expression duplicated in two files

UserClient.ts and SuperPaymasterClient.ts contain byte-for-byte identical gas floor expressions. If the floor value or buffer needs changing again, both files must be updated. Extract to a shared utility:

// packages/paymaster/src/V4/gasUtils.ts
export function superPaymasterPostOpGasLimit(estimate: bigint): bigint {
  const base = estimate + 100_000n;
  return base > 200_000n ? base : 200_000n;
}

✅ Verdict

Approve with requests. Address sync is clean and well-documented. The BigInt → Number conversion (H1) is the only item worth blocking on — the safe BigInt alternative is a one-liner fix.


Round 2 (Codex) will follow.

@jhfnetboy
Copy link
Copy Markdown
Member

🤖 Round 2 — Codex Review

Reviewer: Codex (via codex:rescue subagent)
Scope: entrypoint.ts + UserClient.ts + SuperPaymasterClient.ts


🔴 Critical

None.


🟠 High

H1 — PaymasterClient.submitGaslessUserOperation() bypasses the new 200k floor entirely

The PR adds the max(estimate + 100k, 200k) floor in UserClient.ts and SuperPaymasterClient.ts, but PaymasterClient (the main public-facing submission path) still propagates raw bundler estimates or a 150000n default into buildSuperPaymasterData(). Direct consumers of the public PaymasterClient API can still produce underfunded SuperPaymaster ops — the PR does not fully fix the OOG class it claims to address.

Fix: Centralize SuperPaymaster postOp tuning in a shared helper used by all three submission paths, or apply the same floor in PaymasterClient.

H2 — sepoliaV07Config("M7") now silently derives r4 factory — breaks existing r6 account recovery

factory / factoryM7 now re-point to r4. sepoliaV07Config() still exposes only "M5" | "M7" version selectors — there is no "M7r6" fallback. Since CREATE2 addresses are factory-address-sensitive, any caller using sepoliaV07Config("M7") to reconstruct or recover an existing r6-deployed account will derive the wrong counterfactual address after this PR merges.

Fix: Expose explicit M7r4 / M7r6 selectors in sepoliaV07Config() so callers are forced to opt in to the version change, and existing r6 recovery flows don't silently break.


🟡 Medium

M1 — BigInt → Number unsafe conversion (same as Claude H1)

BigInt(Math.max(Number(est.paymasterPostOpGasLimit + 100000n), 200000))Number() of a BigInt loses precision above 2^53. A malicious or misconfigured RPC returning a huge estimate triggers BigInt(Infinity)TypeError at runtime.

Fix: Pure BigInt: const base = est.paymasterPostOpGasLimit + 100000n; return base > 200000n ? base : 200000n;


🔵 Low

L1 — EIP-55 checksum inconsistency on new r4 addresses (same as Claude M1)

accountImpl is EIP-55 mixed-case; all other new addresses are lowercase. No runtime bug (ethers normalizes), but disables typo detection.

L2 — factoryM7r5Prev missing @deprecated JSDoc (same as Claude L1)

L3 — Gas floor value fragmentation

Codebase now has 4+ different postOp defaults: 300_000n (PR #21 server), 200_000n (this PR floor), 150_000n (PaymasterClient default), 100_000n (old bump), 0x30000 (V4). No named constant, no comment distinguishing inner paymasterData packing from outer UserOp gas field.


❌ Verdict

Request Changes. Two blocking issues:

  1. PaymasterClient OOG fix is incomplete — the PR only patches two of the three submission paths
  2. sepoliaV07Config("M7") silently re-targets r4 — will break existing r6 account recovery without an explicit version selector

Round 1 (Claude) is in the preceding comment.

@jhfnetboy
Copy link
Copy Markdown
Member

⚔️ PK 对比 — Claude vs Codex (PR #22)

维度 Claude Codex
总体裁决 ✅ Approve with requests ❌ Request Changes
Critical
最重要发现 BigInt→Number 不安全转换 PaymasterClient 未修 OOG(PR 只修了 2/3 路径)+ sepoliaV07Config 破坏 r6 账户恢复
一致意见 BigInt→Number unsafe BigInt→Number unsafe
Claude 独有 气体 floor 与 PR #21 server 路径 300k 可能不一致
Codex 独有 PaymasterClient 缺漏、sepoliaV07Config r6 账户恢复静默破坏

🏆 PK 结论

Codex 赢这局。它找到了 Claude 完全忽略的两个 High 问题:

  1. PaymasterClient OOG 修复不完整:PR 只修了 UserClientSuperPaymasterClient 两条路径,公开 API PaymasterClient.submitGaslessUserOperation() 仍用旧逻辑,同样的 OOG 问题继续存在
  2. sepoliaV07Config("M7") 静默破坏 r6 已部署账户恢复:factory 地址换了,但 version selector 没有增加 "M7r6" 选项,调用者无法区分 r4/r6,会推导出错误的 CREATE2 地址

📋 必须修复后才能 merge

优先级 行动
[阻塞] PaymasterClient 也应用 200k postOp floor(或抽共享 helper)
[阻塞] sepoliaV07Config() 增加 "M7r6" 版本选项,防止 r6 账户地址派生错误
[阻塞] BigInt→Number 改为纯 BigInt 比较:base > 200_000n ? base : 200_000n
[建议] 所有 r4 地址统一 EIP-55 checksum
[建议] factoryM7r5Prev@deprecated JSDoc
[建议] 跨文件定义 MIN_SUPER_PAYMASTER_POST_OP_GAS 常量,消除 4 种不同 default 并存的情况

…oor, M7r6 selector, EIP-55

[Blocking] BigInt→Number unsafe conversion (Claude H1 / Codex M1):
- UserClient.ts + SuperPaymasterClient.ts: replace Math.max(Number(...)) with pure BigInt comparison
  const base = est.paymasterPostOpGasLimit + 100_000n; result = base > 200_000n ? base : 200_000n

[Blocking] PaymasterClient OOG fix incomplete (Codex H1):
- submitGaslessUserOperation: default floor raised 150k→200k
- autoEstimate path now applies same 100k buffer + 200k floor (caller-supplied value is respected as-is)

[Blocking] sepoliaV07Config silently re-targets r4 (Codex H2):
- AirAccountVersion type: add "M7r6" selector
- sepoliaV07Config("M7r6") routes to factoryM7r6 for legacy r6 account recovery
- JSDoc updated with explicit warning: M7r6 = recovery only, not for new account creation

[Suggested] EIP-55 checksum consistency (Claude M1 / Codex L1):
- All 5 r4 addresses now use EIP-55 mixed-case checksum

[Suggested] factoryM7r5Prev @deprecated JSDoc (Claude L1 / Codex L2)
@jhfnetboy
Copy link
Copy Markdown
Member

🔄 Re-Review Round 2 — PR #22 (chore/m7-r4-address-sync)

Claude Round 2

Verified fix commit fcabd1a against gh pr diff. All three blocking issues confirmed resolved:

[H1 BigInt safety] ✅ FIXED
packages/enduser/src/UserClient.ts and packages/paymaster/src/V4/SuperPaymasterClient.ts now use pure BigInt comparison:

const _postOpBase = est.paymasterPostOpGasLimit + 100_000n;
paymasterPostOpGasLimit = _postOpBase > 200_000n ? _postOpBase : 200_000n;

No Math.max(Number(...))) anywhere. Precision-safe. ✓

[Codex H1] PaymasterClient floor ✅ FIXED
packages/paymaster/src/V4/PaymasterClient.ts:

  • Default floor: 150000n200_000n
  • autoEstimate path: if (!options?.paymasterPostOpGasLimit) guard added; applies +100_000n + 200_000n floor ✓
  • Caller-supplied value: respected as-is (no override) ✓

[Codex H2] sepoliaV07Config M7r6 ✅ FIXED

  • AirAccountVersion type extended with "M7r6"
  • sepoliaV07Config("M7r6") routes to factoryM7r6 (r6 legacy addresses retained as @deprecated) ✓
  • Default sepoliaV07Config() still goes to factory (r4 audit-final) — no breakage for existing callers ✓
  • JSDoc clearly marks M7r6 as recovery-only ✓

[Suggested] EIP-55 checksums ✅ APPLIED
All 5 r4 addresses now use mixed-case checksum format. ✓


Codex Round 2

⚠️ Note: Codex operates on the local working tree and cannot fetch the PR's remote branch (chore/m7-r4-address-sync). Its findings reflect the pre-fix state and are not applicable to this re-review pass. Manual gh pr diff verification was used instead.


PK Summary

Finding Claude Codex
H1 BigInt safety ✅ FIXED ❌ Branch inaccessible
H1 PaymasterClient floor ✅ FIXED ❌ Branch inaccessible
H2 sepoliaV07Config M7r6 ✅ FIXED ❌ Branch inaccessible
Suggested EIP-55 ✅ Applied ❌ Branch inaccessible

All blockers resolved. No new issues introduced.

Copy link
Copy Markdown
Member

@jhfnetboy jhfnetboy left a comment

Choose a reason for hiding this comment

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

所有 blocking issue 已全部修复(BigInt 安全、PaymasterClient floor、sepoliaV07Config M7r6 恢复路径、EIP-55 checksum)。Re-review 通过,可以 merge。

@fanhousanbu fanhousanbu merged commit d8d6c02 into main May 22, 2026
1 check passed
@fanhousanbu fanhousanbu deleted the chore/m7-r4-address-sync branch May 22, 2026 12:08
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