feat: on-chain pending holds + unified spend() settlement#120
feat: on-chain pending holds + unified spend() settlement#120seongyun-ko wants to merge 1 commit intomasterfrom
Conversation
Introduces PendingHoldsModule (PHM) as a provider-namespaced on-chain hold registry for Rain / Reap / PIX card transactions, and unifies spend() + forceSpend() into a single settlement path where "Settlement is KING". - addHold/updateHold/releaseHold — EtherFi wallet lifecycle at auth-ack / incremental auth / network reversal; spending limit charged at auth time - forceAddHold — operator recovery path, bypasses balance check (forced=true) - settlementSyncHold / settlementSetRemainingHold / removeHold — CashModuleCore-only callbacks at settlement; no reentrancy back into Core - Unified spend(): hold sync (create-if-missing) → partial-aware token transfer → finalize (remove on full, update-remaining on partial). Spend event always reflects actual spent amount, not requested settlement. - Three-way limit accounting: no-hold bypass, non-forced delta, forced full - No-PHM guard: partial settlement reverts with InsufficientBalance when pendingHoldsModule is unset (no remainder tracking → preserve strict debit semantics for legacy / pre-PHM deployments). - CashLens exposes holds-aware spendable() and canSpend() - SpendingLimitLib.release() added to credit back limit on downward deltas Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Certora Run Started (Certora verification)
Certora Run Summary
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit aeea9d7. Configure here.
| currentLimit(limit); | ||
| limit.spentToday = amount <= limit.spentToday ? limit.spentToday - amount : 0; | ||
| limit.spentThisMonth = amount <= limit.spentThisMonth ? limit.spentThisMonth - amount : 0; | ||
| } |
There was a problem hiding this comment.
Cross-day hold release silently inflates daily spending capacity
Medium Severity
SpendingLimitLib.release() floors spentToday at zero after calling currentLimit(), which resets the daily counter on day boundaries. When a hold created on Day 1 is released on Day 2 (after spentToday resets to 0), the release over-credits the daily counter, erasing legitimate Day 2 spending. For example: Day 1 addHold(100) sets spentToday=100; Day 2 spend(80) sets spentToday=80; Day 2 releaseHold(100) floors spentToday to 0 instead of keeping it at 80 — the user can then spend the full daily limit again. The monthly counter is unaffected when stays within the same month.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit aeea9d7. Configure here.
![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: aeea9d70db
ℹ️ 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".
| $$.transactionCleared[txId] = true; | ||
| $$.spendingLimit.spend(totalSpendingInUsd); | ||
| // NOTE: spendingLimit.spend() is NOT called here. Callers are responsible: |
There was a problem hiding this comment.
The new hold registry is keyed by (safe, providerCode, txId) (PendingHoldsModule._holdKey), so different providers can legitimately share the same txId, but spend() still marks replay state in a single transactionCleared[txId] slot. Once one provider settles, a second provider settlement with the same txId will revert with TransactionAlreadyCleared, leaving its hold unreconciled (and potentially blocking withdrawals until manual release). Replay tracking should include provider namespace to match the new hold-key model.
Useful? React with 👍 / 👎.
| external | ||
| whenNotPaused | ||
| nonReentrant | ||
| onlyEtherFiWallet | ||
| { |
There was a problem hiding this comment.
addHold accepts any address and proceeds without verifying that safe is an EtherFi Safe. That allows backend typos/non-safe addresses to consume spending-limit state and create holds that cannot be settled through spend() (which is onlyEtherFiSafe), forcing manual cleanup and risking stale state on incorrect addresses. Adding safe validation at hold entry points (or in the CashModule callback) would prevent this operationally expensive failure mode.
Useful? React with 👍 / 👎.
1 participant
Summary
Introduces PendingHoldsModule (PHM) — a provider-namespaced on-chain registry of card transaction holds — and unifies
spend()+forceSpend()into a single settlement path where Settlement is KING.addHold. The limit shrinks the moment the merchant authorizes a swipe, so subsequent spends can't double-draw against already-authorized balance.Spendwith the actually spent amount, and leave a forced residual hold tracking the remaining debt.What shipped
New contracts
src/modules/cash/PendingHoldsModule.sol— UUPS module, ERC-7201 namespaced storage, provider-namespaced hold keys (keccak256(safe, providerCode, txId)).src/interfaces/IPendingHoldsModule.sol—HoldRecord,ReleaseReason, events, errors.CashModuleCore refactor
forceSpend(). Unified intospend()with three internal phases:_phmSettleHold→ transfer →_phmFinalize._spendDebitPartial()— caps each token at available balance, returns actual USD transferred, emitsSpendwith actual amounts._phmSettleHoldperforms all limit accounting in Core (no PHM→Core callback), avoiding reentrancy into thenonReentrantcontext.InsufficientBalancewhenpendingHoldsModule == address(0)— there is no remainder tracking in legacy/pre-PHM mode, so strict debit semantics are preserved.Supporting changes
SpendingLimitLib.release()— credits back amount to daily/monthly counters, with time-based resets applied first.CashLens— addsspendable(),canSpend(),holdsSummary()views that subtracttotalPendingHoldsfromrawSpendable.CashModuleSetters—consumeSpendingLimit/releaseSpendingLimitPHM→Core callbacks foraddHold/releaseHold/updateHold.E2E call flow — Rain card transaction
Below is the full lifecycle. Same mechanics apply to Reap / PIX with their own
providerCode.1. Cardholder swipes — auth-ack
CashModuleCore.consumeSpendingLimit(safe, amountUsd)ExceededDailySpendingLimit/ExceededMonthlySpendingLimitif the hold would breach the limitHoldRecord { amountUsd, createdAt, providerCode=RAIN, forced=false }totalHolds[safe] += amountUsdHoldAdded(safe, RAIN, txId, amountUsd, createdAt, forced=false)Effect:
spendable(safe) = rawSpendable(safe) − totalPendingHolds(safe)shrinks immediately. User cannot double-spend against the authorized amount.2. Merchant adjusts the auth (tip, fuel top-up, incremental auth)
newAmountUsd > oldAmountUsd: callsconsumeSpendingLimit(delta)→ charges delta to limitnewAmountUsd < oldAmountUsd: callsreleaseSpendingLimit(delta)→ credits delta backtotalHolds[safe]and the recordHoldUpdated(safe, RAIN, txId, oldAmountUsd, newAmountUsd, updatedAt)3. Settlement arrives (happy path)
Inside
spend()(whenNotPaused, nonReentrant, onlyEtherFiWallet):3a.
_validateSpendtransactionCleared[txId] = true(idempotency — replays revert withTransactionAlreadyCleared)totalSpendingInUsd3b.
_phmSettleHold(safe, Rain, txId, totalSpendingInUsd)Calls
PHM.settlementSyncHold(...)which returns(existed, wasForced, oldAmount)with no callbackUpdates hold record to
totalSpendingInUsd, adjuststotalHolds[safe]Core then applies three-way limit logic:
!existed(no prior hold)existed && !wasForcedsettlement − oldAmount)existed && wasForced(forceAddHold path)For the normal Rain flow:
settlement == oldAmount→ delta = 0 → limit untouched (it was already consumed at auth-ack).3c. Token transfer
_spendCredit— borrows full settlement from DebtManager, all-or-nothing_spendDebitPartial— for each token, transfersmin(required, available), scales USD proportionally, returnsactualSpendInUsdpendingHoldsModule == address(0)andactualSpendInUsd < totalSpendingInUsd, revertInsufficientBalance. The remainder has nowhere to go; legacy/pre-PHM deployments keep strict debit semantics.3d.
_phmFinalize(safe, Rain, txId, total, actual)remaining = total − actualremaining == 0:PHM.removeHold(safe, Rain, txId)→ deletes record, decrementstotalHolds, emitsHoldRemovedremaining > 0:PHM.settlementSetRemainingHold(safe, Rain, txId, remaining)→ updates hold to debt amount, marksforced=true, no limit adjustment (limit was charged for the full settlement at 3b). A separate special-function clearance path handles the residual.3e.
_cashback— accrues cashback againstactualSpendInUsd(partial-aware)Emits:
Spend(safe, txId, Rain, tokens, actualAmounts, actualUsd, actualSpendInUsd, Mode)whereactualSpendInUsd ≤ totalSpendingInUsd.4. Reversal — auth never settled
releaseSpendingLimit(safe, holdAmount)→ credits limit backtotalHolds[safe]HoldReleased(safe, RAIN, txId, holdAmount, REVERSAL, releasedAt)Effect: user's limit is fully restored; no token movement.
5. Operator recovery — settlement without auth-ack (backend gap)
If a settlement arrives for a tx that never went through auth-ack (e.g., EtherFi wallet outage):
Option A — force-capture before settlement:
forceAddHoldcreatesHoldRecord { forced=true }, bypasses the balance checkconsumeSpendingLimitcallback at creation_phmSettleHoldseeswasForced=true→ charges full settlement to the limitOption B — spend with no prior hold (late settlement):
_phmSettleHoldcallssettlementSyncHold; no record exists → PHM creates a forced hold withsettlementAmount(existed=false, ...)→ Core bypasses the limit entirely ("Settlement is KING")_phmFinalizeremoves or updates the hold per partial/full result6. Admin release (non-reversal ops action)
Same mechanics as reversal, but
reason=ADMINin the event for downstream ops indexing.Invariants
Throughout every transition:
rawSpendable= raw daily/monthly limit headroom from SpendingLimitLibtotalPendingHolds= sum of all active hold amounts (forced + non-forced)transactionCleared[txId]is monotonic — once true, never flips back; replays hard-revertTwo-removal-path invariant
removeHold()— post-spend only, callable only by CashModuleCorereleaseHold()— non-spend path (reversals / admin), callable only by EtherFi walletNo hold can be removed without one of these two explicit calls.
Provider namespacing
Keys are
keccak256(abi.encode(safe, providerCode, txId)). Rain / Reap / PIX / CardOrder each have their ownbytes4namespace, so a RAIN txId cannot collide with a REAP hold.IPendingHoldsModule.providerCodeFromBinSponsor(BinSponsor)is the canonical converter used at settlement time. Backend is trusted (onlyEtherFiWallet) to send canonical codes at hold creation; no on-chain allowlist so new providers can ship without a contract upgrade.Test plan
requestWithdrawal; release unblocks itspendable,canSpend,holdsSummaryreflect hold consumptionInsufficientBalancewhen PHM is unsetDeployment notes
DeployPendingHoldsModule.s.solscriptUpgradeCashModuleWithPendingHolds.s.sol— wires PHM into existing Core deploymentsUpgradeCashLensWithPendingHolds.s.sol— upgrades Lens to holds-aware implementationCashLensconstructor now takes(cashModule, dataProvider, pendingHoldsModule)— all existing deploy/upgrade scripts updated to passaddress(0)placeholder; real address wired in PHM deployment flowPost-Deploy Monitoring & Validation
HoldAdded/HoldRemoved/HoldReleasedevent volumes match Rain backend's auth → settlement volumesHoldAdded { forced: true }fromforceAddHold— should be operator-invoked only, rareSpendevents whereamountInUsd < totalSpendingInUsd(partial settlement — track remaining holds)totalPendingHoldsdrift vs. sum of open holds in backend DBspendable == rawSpendable − totalPendingHoldson a sample of safes viaCashLens.holdsSummary🤖 Generated with Claude Code
Note
High Risk
High risk because it changes core
CashModulesettlement/spending-limit accounting and introduces a new on-chain holds registry that gates withdrawals and enables partial debit settlements when configured.Overview
Introduces an on-chain
PendingHoldsModuleto track provider-namespaced pending card holds, including hold lifecycle APIs/events (add/update/release at auth time; sync/remove/update at settlement) and controller-managedcashModuleCorewiring.Refactors
CashModuleCore.spend()into a unified settlement path that syncs/creates holds, adjusts spending limits via deltas, supports partial debit settlement (emittingSpendfor the actual transferred amount) and records any remainder as a forced hold; legacy deployments without PHM now revert on partial debit settlement to avoid silent debt.Adds spending-limit “credit back” support via
SpendingLimitLib.release(), blocks withdrawals when holds exist, and updatesCashLensto take a PHM address (immutable) and expose hold-aware views (spendable,holdsSummary). Deployment/upgrade scripts and tests are updated to pass the newCashLensconstructor arg and to add PHM deploy + Gnosis upgrade bundles.Reviewed by Cursor Bugbot for commit aeea9d7. Bugbot is set up for automated code reviews on this repo. Configure here.