Skip to content

fix(security): H-01 — chunked retryPendingDebt + audit findings docs#227

Merged
jhfnetboy merged 4 commits into
sec/c02-c03-x402-authfrom
sec/h01-pending-debt
Jun 1, 2026
Merged

fix(security): H-01 — chunked retryPendingDebt + audit findings docs#227
jhfnetboy merged 4 commits into
sec/c02-c03-x402-authfrom
sec/h01-pending-debt

Conversation

@jhfnetboy
Copy link
Copy Markdown
Member

Stacked PR 4/4 (merge after C-02/C-03). Base: sec/c02-c03-x402-auth.

Fixes H-01: retryPendingDebt always retried the full pending balance, so a balance > the token's maxSingleTxLimit reverted forever (only clearPendingDebt write-off remained). Now takes an explicit amount (clamped) so a large balance is recovered in chunks. Reuses existing recordDebt + pendingDebts. Ops runbook: docs/operations/pending-debt-runbook.md. Also lands the final findings-resolution + KMS #16 consistency docs. forge test 970 green.

jhfnetboy added 4 commits June 1, 2026 16:51
…yer x402 paths confirmed separate at TA layer
…eTxLimit)

retryPendingDebt always retried the FULL pending balance, so once the accumulator
exceeded the token's per-tx limit (5000 aPNTs) recordDebt reverted forever and the
debt could only be written off via clearPendingDebt. Now takes an explicit
(clamped to the balance) so a large balance is recovered in chunks over multiple
owner calls. Reuses recordDebt + the existing pendingDebts accumulator — no new state.

- Test: test_RetryPendingDebt_Chunked (accumulate → drain in 2 chunks → empty reverts).
- Ops runbook: docs/operations/pending-debt-runbook.md (management-backend retry rule).

forge test 970 passed / 0. SuperPaymaster 24,159 bytes (EIP-170 OK, 417 spare).
Stale callers used the old retryPendingDebt(address,address) selector after H-01 added
the chunk `amount` param. test-group-B5 now passes amount=0 (full balance) and the
ABI JSON is regenerated. Flagged by the stop-time review gate.
@jhfnetboy jhfnetboy requested a review from fanhousanbu as a code owner June 1, 2026 12:55
Copy link
Copy Markdown
Contributor

@fanhousanbu fanhousanbu left a comment

Choose a reason for hiding this comment

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

✅ APPROVE — H-01 chunked retryPendingDebt

Logic verified:

function retryPendingDebt(address token, address user, uint256 amount) external onlyOwner nonReentrant {
    uint256 pending = pendingDebts[token][user];
    if (pending == 0) revert NoPendingDebt();
    if (amount == 0 || amount > pending) amount = pending;  // clamp
    pendingDebts[token][user] = pending - amount;           // safe: amount ≤ pending
    IxPNTsToken(token).recordDebt(user, amount);
    emit PendingDebtRetried(token, user, amount);
}
  • Clamp on entry (not after): prevents over-retry and handles the amount == 0 → "full balance" shortcut cleanly. ✅
  • NoPendingDebt check on the pre-clamp pending value → correct (if amount == 0 would be clamped to pending, you'd still want to revert when pending == 0). ✅
  • pendingDebts updated BEFORE recordDebt external call. The nonReentrant guard is present, and the write-before-external-call ordering is correct anyway (CEI). ✅
  • Event carries actual retried amount (post-clamp), not the input. ✅

Operability: If recordDebt itself reverts (e.g., the amount chunk still exceeds xPNTs maxSingleTxLimit), the pendingDebts update is rolled back atomically. The caller must retry with a smaller chunk. The pending-debt runbook should document the chunk sizing strategy. This is by design.

Breaking change: 2-arg → 3-arg signature. In-repo E2E callers updated in the same PR. ✅


All 4 fixes are independent, minimal, and correctly address the attack vectors. Stack ready to merge in order: #225#226#228#227.

@jhfnetboy jhfnetboy merged commit ae22643 into sec/c02-c03-x402-auth Jun 1, 2026
1 check passed
@jhfnetboy jhfnetboy deleted the sec/h01-pending-debt branch June 1, 2026 13:12
@github-actions github-actions Bot locked and limited conversation to collaborators Jun 1, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants