Skip to content

fix(payment): approve the vault's real merkle charge, not the pool sum - #207

Merged
Nic-dorman merged 1 commit into
mainfrom
fix/merkle-allowance-charge
Aug 7, 2026
Merged

fix(payment): approve the vault's real merkle charge, not the pool sum#207
Nic-dorman merged 1 commit into
mainfrom
fix/merkle-allowance-charge

Conversation

@Nic-dorman

Copy link
Copy Markdown
Member

Problem

A tester's merkle payment fails on every retry with (readable as of rc.3's un-truncation fix):

Payment would fail on-chain: The contract function "payForMerkleTree" reverted with the following signature: 0xfb8f41b2

0xfb8f41b2 decodes to ERC20InsufficientAllowance(address,uint256,uint256) — the vault is authorized to pull less ANT than the payment needs, even though ensureAllowance checks (and tops up) before paying.

Root cause

PaymentVaultV2.payForMerkleTree charges median(winner pool's 16 candidate amounts) × 2^depth (median16 = element 8 of the sorted 16). Our allowance check used maxPoolCost = the largest pool's sum of candidate amounts ≈ 16 × median.

For depth ≥ 5 the real charge exceeds that bound — 2× at depth 5, 8× at depth 7 (a ~460MB / 119-chunk file), 64× at depth 10. Any wallet whose remaining allowance fell between the wrong bound and the real charge skipped the approve and then reverted deterministically. The bug hid behind two masks: shallow trees (charge ≤ pool sum for depth ≤ 4) and fresh standing allowances (1 ANT dwarfs typical charges); a drained allowance plus a deep tree exposes it.

Fix

New merkleMaxCharge(depth, pools) mirrors the contract's math exactly: median at sorted index 8 (the contract's median16 k), × 2^depth, worst case across pools — the winner pool is selected on-chain from sender+timestamp and is unknowable client-side, so the upper bound over pools is the tightest correct answer. ensureAllowance now receives that instead of the pool sum.

For context: upstream evmlib's own wallet sidesteps this by approving U256::MAX; our bounded-allowance design (standing 1 ANT, #191) just needs the bound to be correct.

Testing

  • Vitest 16/16 in the payment suite, including: contract-math mirror (1152 = 9 × 2^7 for amounts 1..16), a regression assertion that the new bound exceeds the old pool-sum bound at depth 7, worst-case-across-pools, and empty-pool safety.
  • nuxi typecheck clean.

🤖 Generated with Claude Code

A tester's rc.3 error (readable thanks to the un-truncation fix) showed
payForMerkleTree reverting with 0xfb8f41b2 =
ERC20InsufficientAllowance(address,uint256,uint256).

Root cause: PaymentVaultV2.payForMerkleTree charges
median(winner pool's 16 candidate amounts) * 2^depth, but
ensureAllowance was fed maxPoolCost = the largest pool's SUM of
candidates (~16 * median). For depth >= 5 the real charge exceeds that
bound (2x at depth 5, 8x at depth 7, 64x at depth 10), so a wallet
whose remaining allowance fell between the two skipped the approve and
then reverted deterministically on every retry. Shallow trees and
fat standing allowances masked the bug; drained allowances plus deep
trees surface it.

New merkleMaxCharge() mirrors the contract's math exactly - median at
sorted index 8 (the contract's median16 k), times 2^depth, worst case
across pools since the winner pool is selected on-chain and is
unknowable client-side. (Upstream evmlib avoids the issue by approving
U256::MAX; our bounded-allowance design just needs the correct bound.)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Nic-dorman
Nic-dorman merged commit f9f24af into main Aug 7, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant