fix(p0-12a): x402 Direct settle is xPNTs-only#109
fix(p0-12a): x402 Direct settle is xPNTs-only#109jhfnetboy wants to merge 4 commits intofix/p0-13-x402-nonce-triplefrom
Conversation
P0-12a (B2-N4): `settleX402PaymentDirect` accepted any ERC20 as the
asset and called `IERC20(asset).safeTransferFrom(from, ...)`. A user
who had done a standard infinite `approve(facilitator, MAX)` for USDC
(legitimate x402 EIP-3009 pattern, recommended by the spec) could be
drained by a compromised facilitator via the Direct path, because
USDC has no in-token firewall. xPNTs tokens carry the autoApproved
spender firewall + MAX_SINGLE_TX_LIMIT — arbitrary ERC20s do not.
Defense:
- xPNTsFactory now records `mapping(address => bool) public isXPNTs`
on every successful `deployxPNTsToken`. The mapping is the
single source of truth for "this token is a factory-deployed
xPNTs" — no admin setter, no off-chain config needed.
- IxPNTsFactory exposes `isXPNTs(address)` so consumers (SP and
off-chain SDKs) can check.
- SuperPaymaster.settleX402PaymentDirect:
- Validates fee/role/nonce first (so the asset gate cannot be used
to probe nonces or short-circuit auth).
- Then `require(xpntsFactory.isXPNTs(asset))` via custom revert
`InvalidXPNTsToken`.
- If `xpntsFactory == address(0)`, revert `InvalidConfiguration`
(fail-closed: a misconfigured deploy cannot accidentally accept
arbitrary assets).
Note on aPNTs: aPNTs is the protocol settlement token, NOT a
factory-deployed xPNTs, so it is intentionally NOT whitelisted here.
Operator deposits/refunds of aPNTs go through `_consumeCredit` and
the aPNTs balance ledger, never through `settleX402PaymentDirect`.
USDC and other ERC-3009-compatible assets continue to use
`settleX402Payment` (EIP-3009 transferWithAuthorization) which carries
its own per-(asset, from, nonce) signature binding.
Storage layout: unchanged. xPNTsFactory adds one mapping (Ownable
slot 0 + existing storage; new mapping appends after deployedTokens).
SuperPaymaster has no new storage. UUPS upgrade safe.
Tests:
- contracts/test/v3/X402Direct_AssetWhitelist.t.sol (new, 6 tests):
- test_SettleDirect_RejectsNonXPNTsAsset (USDC drain blocked)
- test_SettleDirect_RejectsAPNTsAsset (aPNTs intentionally not Direct)
- test_SettleDirect_AcceptsXPNTsAsset (happy path)
- test_Factory_IsXPNTsTrueAfterDeploy
- test_Factory_IsXPNTsFalseForArbitraryToken
- test_SettleDirect_RevertsWhenFactoryUnset (fail-closed)
- contracts/test/v3/SuperPaymasterV5Features.t.sol: added
MockXPNTsFactory + wired in setUp so existing Direct settle tests
pass against the new gate. All 36 V5 features tests still green.
Refs: P0-12a in docs/security/2026-04-26-p0-prelaunch.md
Wave 2 plan: docs/security/wave-plans/wave2-funds-price.md
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 92f0b64a5c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // a token deployed by the configured xPNTs factory. | ||
| address factory = xpntsFactory; | ||
| if (factory == address(0)) revert InvalidConfiguration(); | ||
| if (!IxPNTsFactory(factory).isXPNTs(asset)) revert InvalidXPNTsToken(); |
There was a problem hiding this comment.
settleX402PaymentDirect now hard-calls IxPNTsFactory(factory).isXPNTs(asset), but pre-existing xPNTsFactory contracts do not implement this selector; on an in-place SuperPaymaster upgrade (the repo’s upgrade flow upgrades the proxy without redeploying/repointing factory), this call reverts for every direct settlement, including legitimate xPNTs payments. This introduces a production outage path unless a factory migration/backfill is performed in lockstep.
Useful? React with 👍 / 👎.
代码审查通过，含 MEDIUM 注意事项 白名单检查正确： MEDIUM — nonce 在资产白名单检查之前被消耗 PR 注释说明这是有意设计（防止攻击者反复尝试不同 asset 探测白名单而不消耗 nonce），这个理由成立。但需要在 SDK 文档和错误消息中明确说明：若 INFO — 缺失测试
|
…dComputeFee Nonce is intentionally consumed before the asset whitelist check so that replay is prevented regardless of the failure reason. Document the trade-off: a caller with a non-xPNTs asset loses the nonce and must retry with a fresh one; well-formed SDK clients always validate assets first.
|
已在 设计意图：nonce 在资产白名单检查之前被标记消耗（intentional nonce-before-whitelist-check）。 |
Add two nonce-behavior tests to X402Direct_AssetWhitelist.t.sol: - test_SettleDirect_NonceConsumed_OnAssetWhitelistFailure: verifies that EVM revert semantics roll back the nonce write when settleX402PaymentDirect reverts with InvalidXPNTsToken — the nonce is NOT consumed on failure, and the same nonce may be reused with a corrected (valid xPNTs) asset. - test_SettleDirect_NonceConsumed_OnSuccess: confirms the nonce IS durably consumed on a successful call, and that replay reverts with NonceAlreadyUsed. Also update the NatSpec on settleX402PaymentDirect in SuperPaymaster.sol to clarify the nonce/revert interaction accurately (replacing a misleading "nonce is consumed before whitelist check" note with the correct EVM revert-rollback explanation).
测试补充 + 重要发现：nonce 实际上不会被消耗关键发现PR reviewer 担忧「nonce 在 原代码注释「nonce is consumed before asset whitelist check」描述的是调用帧内的执行顺序，而非持久化结果，具有误导性，已更正。 新增测试（共 2 个）
测试结果：8/8 PASS |
The comment claimed a caller loses their nonce if they supply a wrong asset. In reality EVM revert semantics roll back the storage write, so the nonce is NOT consumed on failure. The correct behavior is already documented in the settleX402PaymentDirect NatSpec; the contradicting inline comment is simply deleted.
2 participants
Base 是
fix/p0-13-x402-nonce-triple（PR #106），不是 main —— P0-12a 修改的settleX402PaymentDirect上层依赖 P0-13 的x402NonceKey三元组。GitHub 只展示这条 commit 的 delta。P0-12a (B2-N4)
settleX402PaymentDirect(asset, from, to, amount, nonce)调IERC20(asset).safeTransferFrom(from, ...)不限制 asset 类型。用户给 USDC 做了标准approve(facilitator, MAX)（infinite approval 是常规模式）→ 攻陷 facilitator 直接抽干 USDC。xPNTs 有防火墙 + $100 单笔上限，USDC 没有这个保护。Defense (D4 决策)
mapping(address => bool) public isXPNTs，每次 deployxPNTsToken / deployaPNTsToken 写入SuperPaymaster.settleX402PaymentDirect加require(IXPNTsFactory(xpntsFactory).isXPNTs(asset), "Direct: must be xPNTs")USDC 走 EIP-3009 path（用户每次签名，符合 x402 标准），xPNTs 走 Direct path。
附加修复：删除错误注释（commit 210a7f6）
_validateX402AndComputeFee中有 5 行@dev注释声称"nonce 在 asset 白名单检查前被消耗，调用者若传入错误 asset 会丢失 nonce"。此说法与 EVM 语义矛盾：若后续revert InvalidXPNTsToken()触发，EVM 会回滚 storage write，nonce 不会被消耗。正确行为已在settleX402PaymentDirect的 NatSpec 中记录，错误的 inline 注释已删除。Tests
新测试（拒绝非 xPNTs / 接受 xPNTs / factory isXPNTs 注册）+ 现有 settleDirect tests 不破坏
Spec
docs/security/2026-04-26-p0-prelaunch.md§3 P0-12adocs/security/2026-04-26-decision-records.mdD4