chore(airaccount): sync M7 module addresses to r4 audit-final + postOp gas floor#22
Conversation
…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
🤖 Round 1 — Claude Code Review (Local Model)
🔴 CriticalNone. 🟠 HighH1 — // UserClient.ts + SuperPaymasterClient.ts (identical pattern)
BigInt(Math.max(Number(est.paymasterPostOpGasLimit + 100000n), 200000))
Safe equivalent using only BigInt arithmetic: const base = est.paymasterPostOpGasLimit + 100_000n;
paymasterPostOpGasLimit = base > 200_000n ? base : 200_000n;🟡 MediumM1 — Address checksum inconsistency Of the 5 new r4 addresses, only 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: M2 — Gas floor 200k (client) vs 300k fixed (server, PR #21) — different paths, confirm intentional
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
would protect the floor invariant from silent regression. 🔵 LowL1 — r6 addresses got proper L2 — Identical gas floor expression duplicated in two files
// packages/paymaster/src/V4/gasUtils.ts
export function superPaymasterPostOpGasLimit(estimate: bigint): bigint {
const base = estimate + 100_000n;
return base > 200_000n ? base : 200_000n;
}✅ VerdictApprove with requests. Address sync is clean and well-documented. The Round 2 (Codex) will follow. |
🤖 Round 2 — Codex Review
🔴 CriticalNone. 🟠 HighH1 — The PR adds the Fix: Centralize SuperPaymaster postOp tuning in a shared helper used by all three submission paths, or apply the same floor in H2 —
Fix: Expose explicit 🟡 MediumM1 —
Fix: Pure BigInt: 🔵 LowL1 — EIP-55 checksum inconsistency on new r4 addresses (same as Claude M1)
L2 — L3 — Gas floor value fragmentation Codebase now has 4+ different postOp defaults: ❌ VerdictRequest Changes. Two blocking issues:
Round 1 (Claude) is in the preceding comment. |
⚔️ PK 对比 — Claude vs Codex (PR #22)
🏆 PK 结论Codex 赢这局。它找到了 Claude 完全忽略的两个 High 问题:
📋 必须修复后才能 merge
|
…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)
🔄 Re-Review Round 2 — PR #22 (chore/m7-r4-address-sync)Claude Round 2Verified fix commit [H1 BigInt safety] ✅ FIXED const _postOpBase = est.paymasterPostOpGasLimit + 100_000n;
paymasterPostOpGasLimit = _postOpBase > 200_000n ? _postOpBase : 200_000n;No [Codex H1] PaymasterClient floor ✅ FIXED
[Codex H2] sepoliaV07Config M7r6 ✅ FIXED
[Suggested] EIP-55 checksums ✅ APPLIED Codex Round 2
PK Summary
All blockers resolved. No new issues introduced. ✅ |
jhfnetboy
left a comment
There was a problem hiding this comment.
所有 blocking issue 已全部修复(BigInt 安全、PaymasterClient floor、sepoliaV07Config M7r6 恢复路径、EIP-55 checksum)。Re-review 通过,可以 merge。
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):0x42f8...0x61bb...0x2F1B...0xA674...0x4135...0xb655...0x7357...0x67f8...0xa3e5...0x1f06...r6 addresses retained as
@deprecatedaliases for legacy account lookups and historical event indexing.SuperPaymaster postOp Gas Floor
UserClient+SuperPaymasterClient: replace flat+10kbump withmax(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 tablepackages/enduser/src/UserClient.ts— postOp gas floorpackages/paymaster/src/V4/SuperPaymasterClient.ts— postOp gas floorTest Plan
pnpm -r build— no type errorspnpm --filter @aastar/airaccount test— unit tests pass