fix(airaccount): ECDSA algId prefix + SuperPaymaster postGas for v0.7 gasless ops#21
Conversation
…alidator M7 composite validator routes by algorithm ID prefix. Plain ECDSA signatures without the prefix caused AA24 (signature error) on first transfer for accounts without a guardian. Prepend 0x02 to the raw 65-byte ECDSA sig so the validator dispatches to the ECDSA verifier.
SuperPaymaster's postOp consumes ~117k gas for aPNTs accounting (recordXPNTsDebt, event emission). The previous limit of 100,000 caused AA50 (postOp out-of-gas), reverting successful user operations. Minimum safe value confirmed via on-chain tracing on Ethereum Sepolia.
fanhousanbu
changed the title
🤖 Round 1 — Claude Code Review (Local Model)
🔴 CriticalNone. 🟠 HighH1 —
export const ALG_ID = {
BLS: 0x01,
ECDSA: 0x02, // ← this exists
P256: 0x03,
...
} as const;But Fix: Import and use H2 — No automated regression test for the algId prefix The signature format change is critical (wrong byte → AA24 on every ECDSA op) but has no unit test. A test asserting 🟡 MediumM1 — PR description file list discrepancy PR body lists M2 — Inconsistent algId prefix encoding style ECDSA path (new): Both produce identical bytes but two different patterns in the same file creates maintenance confusion. Should standardize on one approach. M3 — 300k is ~2.56× the observed ~117k. The number is defensible but should explain itself: // recordXPNTsDebt + event emit observed ~117k; 300k gives safe headroom
const postGas = BigInt(300_000);Also note: V4 path (same file, line 187) uses 🔵 LowL1 — postGas was bumped due to gas growth. Was L2 — Milestone tag in comment will rot
✅ VerdictApprove with requests. Both fixes are mechanically correct and verified on Sepolia ( Primary asks before merge:
Round 2 (Codex review) will follow in a separate comment. |
🤖 Round 2 — Codex Review
🔴 CriticalNone. 🟠 HighH1 —
The fix prepends Suggested fix: Only prepend 🟡 MediumM1 — Observed ~117k gas; 300k is 2.56× headroom. This increases maxCost / prefund / deposit requirements for every SuperPaymaster UserOperation by ~183k gas, causing avoidable sponsorship failures under high fee conditions. Suggested fix: Use a named constant with a comment (e.g. M2 — No regression tests for the new ECDSA wire format The diff changes ECDSA UserOperation signatures from 65 to 66 bytes with no test asserting:
🔵 LowL1 — PR description does not match actual diff PR body claims ❌ VerdictRequest Changes. The Round 1 (Claude review) is in the preceding comment. |
⚔️ PK 对比 — Claude vs Codex
🏆 PK 结论Codex 的 H1 更关键： Claude 的 H1 互补： 综合建议（须在 merge 前解决）：
|
…LG_ID constants + tests
Codex H1 (blocking): useECDSA=true was applied to three paths but algId=0x02 prefix
was unconditionally added to all three. The catch{} path (validator() call failure)
cannot confirm compositeValidator — it now uses raw ECDSA to prevent AA24 on
non-compositeValidator accounts.
Fix: introduce detectSignatureStrategy() that tracks isCompositeValidator separately.
Only apply ethers.toBeHex(ALG_ID.ECDSA, 1) prefix when isCompositeValidator=true.
Claude H1: replace magic bytes "0x02"/"0x01" with ALG_ID.ECDSA / ALG_ID.BLS constants.
M2 (consistency): standardize BLS signature encoding to ethers.concat() (was string concat).
M3 (postGas): add comment explaining observed ~117k gas and 300k safety headroom.
Tests: 4 new unit tests for detectSignatureStrategy covering all three code paths;
1 new test for V4 paymasterData byte layout.
回应评审意见感谢 Claude/Codex 两轮评审！本次提交（ 🔴 阻塞问题（已修复）Codex H1 — 引入
🟠 High（已修复）Claude H1 — 魔法字节 → // Before
userOp.signature = ethers.concat(["0x02", ecdsaSig]);
userOp.signature = "0x01" + packedBls.slice(2);
// After
userOp.signature = ethers.concat([ethers.toBeHex(ALG_ID.ECDSA, 1), ecdsaSig]);
userOp.signature = ethers.concat([ethers.toBeHex(ALG_ID.BLS, 1), packedBls]);Claude H2 / Codex M2 — 添加单元测试
🟡 Medium（已修复）
未处理
284 个单元测试全部通过。 |
🤖 Round 1 (Second Pass) — Claude Code Review
✅ Resolved from Round 1
🔴 CriticalNone. 🟠 HighNone. 🟡 MediumM1 — SuperPaymaster The new A minimal fix: mock the M2 — } catch {
// validator() call failed: account may not be compositeValidator.
return { useECDSA: true, isCompositeValidator: false };
}The comment says "validator() call failed" but this catch also triggers if Suggest splitting into two try-catch blocks or adding 🔵 LowL1 — Undeployed account assumption is an undocumented protocol invariant if (accountCode === "0x") {
// Counterfactual AirAccount: will deploy as compositeValidator.
return { useECDSA: true, isCompositeValidator: true };
}The comment says "will deploy as compositeValidator" but this is enforced by the AirAccount factory — not verifiable here at runtime. If Suggest: L2 — BLS path does not check The ECDSA path now guards on ✅ VerdictApprove. All blocking issues from Round 1 are correctly resolved. Round 2 (Codex re-review) will follow in a separate comment. |
🤖 Round 2 (Second Pass) — Codex Review
🔴 Critical — STALE (already fixed in
|
⚔️ PK 对比 Round 2 — Claude vs Codex
🏆 Round 2 PK 结论这轮 Claude 更可靠 — 读到了最新的 但 Codex 的 Low 发现有价值： 📋 综合行动项（两轮 PK 共识）
🔧 Codex 工具使用备注Codex 在 re-review 时读到了旧 commit |
…review - Undeployed path: 'AirAccount factory invariant' instead of 'will deploy as' - catch block: explicitly covers both getCode() and validator() failures - BLS path: document asymmetry — BLS always compositeValidator by design
Round 2 评审回应（commit
|
jhfnetboy
left a comment
jhfnetboy
left a comment
There was a problem hiding this comment.
✅ Approved
All review items addressed across 4 commits:
| Commit | Content |
|---|---|
27af31c |
Original algId=0x02 fix |
e602529 |
postGas 100k→300k |
7fc8f7d |
detectSignatureStrategy() + ALG_ID constants + tests (Codex H1 + Claude H1) |
fab6174 |
Comment clarifications (Claude L1/L2/M2, Codex L1) |
284 tests passing. Good to merge.
Follow-up tickets (non-blocking):
- Unit test for SuperPaymaster
postGas=300_000branch (mockoperators()RPC) - Byte-layout test for
packSignature()to guard against silent double-prefix regression
2 participants
Summary
两个独立 bug fix，均影响 v0.7 EntryPoint 的 gasless 操作：
Fix 1 — ECDSA algId prefix (commit
27af31c)compositeValidator要求签名以algId=0x02开头，SDK 直接传 raw ECDSA sig → AA24 (account signature failed)BackendSignerAdapter在签名前拼接0x02algId 前缀Fix 2 — SuperPaymaster postGas (commit
e602529)paymasterPostOpGasLimit设为100,000，但 SuperPaymasterpostOp实际消耗 ~117k gas（recordXPNTsDebt+ event emit），导致 AA50 (postOp OOG)，user operation 静默回滚PaymasterManager.getPaymasterData()中 SuperPaymaster 分支postGas: BigInt(100000) → BigInt(300000)0xd6eb1c2274be1925cdb77b805f03a44803b3b436564840cbb2c462ecf2327eafFiles Changed
packages/airaccount/src/server/services/paymaster-manager.ts— postGas 100k→300k (1 line)packages/airaccount/src/server/constants/entrypoint.ts— (algId fix 相关)Test Plan
pnpm --filter @aastar/airaccount test单测通过pnpm -r build构建通过postOpgas 用量低于 300k（当前观测值 ~117k）