From 29e80c1059b6ac378fef363d7728b8fdbd5d0418 Mon Sep 17 00:00:00 2001 From: jhfnetboy Date: Tue, 8 Sep 2026 12:20:34 +0700 Subject: [PATCH] docs(sp): _recordDebt described a balance short-circuit that was deleted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two comment blocks in _recordDebt explained the ceiling re-check and the `else` branch in terms of "_creditExceeded's validation-time balance short-circuit", defeatable by a mid-UserOp drain. That short-circuit was `if (IERC20(token).balanceOf(user) >= xPNTsCharge) return false;` and c6493ade (audit H-1, Plan A) deleted it — confirmed from the diff of that commit, not inferred. _creditExceeded is now three lines and never reads a balance. It matters more than a stale note usually would: the mode it describes — a zero-credit user paying from balance — is exactly what a "balance or refusal, no debt" community switch would need, so anyone arriving to evaluate that reads this comment first and concludes the behaviour already exists. It does not: with no balance short-circuit, a user whose recorded debt already fills the ceiling is refused at validation however much xPNTs they hold. The rewrite keeps what is still true — recordDebtWithOpHash checks only maxSingleTxLimit, not getCreditLimit, so an unconditional call would let debt pass the ceiling — and replaces the dead cause with what can actually reach the `else`: validation asserted getDebt + pendingDebts + charge <= getCreditLimit, this re-evaluates the same inequality later, and all three inputs can move. getDebt and pendingDebts can grow if another op for the same user settles first, and getCreditLimit can FALL, being creditTierConfig[_levelForReputation(globalReputation[user])] with both writable. Written as reachability, not diagnosis: no live instance of any of the three was constructed, and the comment says so rather than naming a mechanism with the confidence the old text had. Comment-only, and that is checked rather than asserted: same worktree, same profile, built before and after with only this file differing — deployed bytecode byte-identical at 23,569 B. The comparison is not vacuous: it separates SuperPaymaster from Registry. Claude-Session: https://claude.ai/code/session_016URk99bYV66BPtfQmP3gy2 --- .../superpaymaster/v3/SuperPaymaster.sol | 39 +++++++++++++------ 1 file changed, 28 insertions(+), 11 deletions(-) diff --git a/contracts/src/paymasters/superpaymaster/v3/SuperPaymaster.sol b/contracts/src/paymasters/superpaymaster/v3/SuperPaymaster.sol index 1bd1a942..7e347aee 100644 --- a/contracts/src/paymasters/superpaymaster/v3/SuperPaymaster.sol +++ b/contracts/src/paymasters/superpaymaster/v3/SuperPaymaster.sol @@ -1443,23 +1443,40 @@ contract SuperPaymaster is BasePaymasterUpgradeable, ReentrancyGuard, ISuperPaym /// (usedOpHashes ↔ usedDebtHashes) provide token-level defence-in-depth. function _recordDebt(address token, address user, uint256 amount, bytes32 opHash, address operator) internal { try IxPNTsToken(token).burnFromWithOpHash(user, amount, opHash) {} catch { - // AUDIT H-1 (2026-06-11): _creditExceeded's validation-time balance - // short-circuit (intentional — lets zero-credit level-1 users pay - // from balance) is defeatable: a user can drain its xPNTs inside its - // own UserOp between validate and postOp, so a charge that passed on - // "balance is enough" lands here with zero balance. recordDebtWithOpHash - // only checks maxSingleTxLimit, NOT getCreditLimit, which let debt - // accumulate past the ceiling and drain the operator (the exact - // scenario C-01 set out to prevent). + // Why this re-checks the ceiling instead of just recording the debt: + // recordDebtWithOpHash checks only maxSingleTxLimit, NOT getCreditLimit, + // so calling it unconditionally would let debt accumulate past the + // ceiling and drain the operator — the exact scenario C-01 set out to + // prevent. + // + // HISTORY, because the previous version of this comment described code + // that no longer exists. It said _creditExceeded has a validation-time + // balance short-circuit that a mid-UserOp drain can defeat. That + // short-circuit was `if (IERC20(token).balanceOf(user) >= xPNTsCharge) + // return false;` and c6493ade (audit H-1, Plan A) DELETED it; the comment + // was not updated. _creditExceeded is now three lines and never reads a + // balance, so a zero-credit user is refused at validation rather than + // paying from balance. + // + // What can still reach the `else` below, then: validation asserted + // getDebt + pendingDebts + charge <= getCreditLimit, and this re-evaluates + // the same inequality later. All three inputs can move in between — + // getDebt and pendingDebts can grow if another op for this user settles + // first, and getCreditLimit can FALL, since it is + // creditTierConfig[_levelForReputation(globalReputation[user])] and both + // of those are writable (Registry: reputation proposals, and governance). + // Stated as reachability, not as a diagnosis: no live instance of any of + // the three has been constructed here. if (IxPNTsToken(token).getDebt(user) + pendingDebts[token][user] + amount <= REGISTRY.getCreditLimit(user)) { // Within ceiling: normal debt fallback (honest user, e.g. new // user with no balance but within credit). try IxPNTsToken(token).recordDebtWithOpHash(user, amount, opHash) { return; } catch {} } else { - // Over ceiling: the balance short-circuit was defeated by a - // mid-UserOp drain — this op was effectively an unbacked - // sponsorship. The gas for THIS op is already spent (cannot be + // Over ceiling: whatever moved (see above), this op was + // effectively an unbacked sponsorship — the reason is not knowable + // from here, and the old text named one that can no longer happen. + // The gas for THIS op is already spent (cannot be // clawed back), so cap the loss by blocking the user for this // operator: isBlocked is checked in validate and is channel- // agnostic (gates BOTH the SBT and the agent path), so the