fix(minter): reserve transaction fee when selecting affordable withdrawals

[gregorydemay]

The balance tracked by the minter is the consolidated balance, that is the funds already swept into the fee payer account. Minted funds may still sit in deposit accounts awaiting consolidation, and users already hold ckSOL for them. A withdrawal can therefore be accepted for more than the consolidated balance. When planning a batch withdrawal transaction, the minter must account exactly for the cost of that transaction, transfer amounts plus fee, so that the consolidated funds are enough to cover it.

The withdrawal timer only checked the transfer amounts and ignored the fee. Recording the submitted transaction then subtracted amount plus fee and trapped when the balance was short. Because the trap drops the in-flight future, the loop's reschedule guard re-armed the timer immediately, so the minter trapped on every round until the periodic consolidation happened to make the request affordable. This was an audit blocker: any user could trigger it by withdrawing exactly the balance reported by get_minter_info.

Batching and affordability now live in one place: the state exposes an iterator that yields one affordable batch at a time, reserving a transaction fee whenever it opens a batch and stopping at the first request the remaining balance cannot cover. The withdrawal timer pulls only the batches it submits in the current round, and schedules a consolidation only once the affordable batches are exhausted while requests remain pending.

The bug is reproduced by an integration test against a Solana test validator: one deposit is minted and consolidated, a second is minted but not consolidated, and a withdrawal for exactly the consolidated balance is queued through the public withdraw endpoint. After a single withdrawal round the minter must consolidate the pending deposit right away, and the withdrawal must finalize. Without the fix the round traps and no consolidation is scheduled, so the test fails. The batching iterator has its own unit tests, with the fee boundaries covered by property tests over the transfer amount, the balance, and the shortfall.

This PR builds on #204, which lets each validator test start its own solana-test-validator.

🤖 Generated with Claude Code

[Copilot]

Copilot review overview

🟡 Changes recommended

A newly added test’s setup and assertion do not match its stated intent (“leaving exactly one transaction fee”), reducing confidence in the boundary-condition coverage.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1

Open (1)

• Transfer amount spends the fee reserve instead of leaving one fee · New

What changed in this PR

This PR fixes an audit-blocking panic in withdrawal processing by ensuring the “affordable withdrawals” selection logic reserves the Solana transaction fee per batch, matching the later state transition that subtracts amount_to_transfer + tx_fee.

Changes:

• Add regression tests for withdrawing at/near the minter’s full tracked balance to prevent panics and guard leaks.
• Refactor affordability selection into affordable_withdrawal_requests, reserving BATCH_WITHDRAWAL_TX_FEE at the start of each batch of MAX_WITHDRAWALS_PER_TX.
• Introduce BATCH_WITHDRAWAL_TX_FEE as the fee reserved for batch-withdrawal transactions (single signer).

File	Description
minter/​src/​withdraw/​tests.rs	Adds regression tests around balance boundary conditions in process_pending_withdrawals.
minter/​src/​withdraw/​mod.rs	Updates affordability selection to reserve one tx fee per batch via a helper function.
minter/​src/​sol_transfer/​mod.rs	Defines BATCH_WITHDRAWAL_TX_FEE for batch-withdrawal fee reservation.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Copilot review overview

🟡 Changes recommended

WithdrawalBatches uses saturating_add for fee reservation, which can mask u64 overflow and break the “only select affordable withdrawals” invariant in extreme cases.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 · 1

Open (2)

• Use checked addition to prevent overflow from undercounting transfer cost · New
• Transfer amount spends the fee reserve instead of leaving one fee

[Copilot]

Copilot review overview

🟡 Changes recommended

The newly added property tests can overflow on u64 arithmetic, and the batching iterator uses saturating_add in a way that can mask overflow and weaken the intended affordability guarantees.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 · 1

Open (2)

• Use checked addition to prevent overflow from undercounting transfer cost
• Prevent overflow in generated transfer amount plus batch fee · New

Resolved since last review (1)

• Transfer amount spends the fee reserve instead of leaving one fee

[Copilot]

Copilot review overview

🟢 Approval recommended

The batching/affordability logic now consistently reserves the per-transaction fee and is covered by targeted property tests plus an integration test reproducing the previously trapping scenario.

Review effort: Lite
Findings: 1 · 1

Open (2)

• Use checked addition to prevent overflow from undercounting transfer cost
• Prevent overflow in generated transfer amount plus batch fee

[Copilot]

Copilot review overview

🔵 Needs a closer look

The new property tests can overflow in generated inputs and panic, which will make the test suite flaky/fail even when the batching logic is correct.

Review effort: Lite
Findings: 1 · 1

Open (2)

• Use checked addition to prevent overflow from undercounting transfer cost
• Prevent overflow in generated transfer amount plus batch fee

[Copilot]

Copilot review overview

🟢 Approval recommended

The updated batching/affordability logic correctly accounts for per-transaction fees and is backed by targeted unit/property tests plus an integration regression test for the reported trap scenario.

Review effort: Lite
Findings: 1 · 1

Open (2)

• Use checked addition to prevent overflow from undercounting transfer cost
• Prevent overflow in generated transfer amount plus batch fee

[zeropath-ai]

✅ No security or compliance issues detected. Reviewed everything up to 46a33ba.

Security Overview

• 🔎 Scanned files: 5 changed file(s)
• 🔗 Scan Link: https://zeropath.com/app/repositories/58f1cdfe-c0c2-41c1-8d28-3d6efcaf3266?scanId=ba40e6b1-92df-4422-bbde-c3a9f453bad9&codeScanTypes=PrScan&tab=issues&detectionTypes=traditional,businessLogic,customRule,IAC,SECRETS,CICD,EOL,uncategorized

Detected Code Changes

Change Type	Relevant files
Enhancement	► integration_tests/tests/solana_test_validator.rs       Add test should_withdraw_exactly_the_consolidated_balance
Enhancement	► minter/src/sol_transfer/mod.rs       Add BATCH_WITHDRAWAL_TX_FEE constant
Refactor	► minter/src/state/mod.rs       Import BATCH_WITHDRAWAL_TX_FEE and MAX_WITHDRAWALS_PER_TX from sol_transfer       Add withdrawal_batches() method to State       Update imports to include btree_map and Peekable
Enhancement	► minter/src/state/mod.rs       Introduce WithdrawalBatches struct and its Iterator implementation to batch withdrawals
Enhancement	► minter/src/state/tests.rs       Add tests for withdrawal_batches behavior (including edge cases and property-based tests)
Refactor	► minter/src/withdraw/mod.rs       Remove unused Itertools import and adjust imports for create_signed_batch_withdrawal_transaction usage
Refactor	► minter/src/withdraw/mod.rs       Update withdrawal processing logic to utilize withdrawal_batches() for batching
Refactor	► minter/src/withdraw/mod.rs       Adjust processing to align with new batching mechanism (batches, more_to_process, num_pending_withdrawals)
Enhancement	► minter/src/withdraw/mod.rs       Integrate batch-based withdrawal creation workflow with read_state changes

[Copilot]

Copilot review overview

🟢 Approval recommended

The fee-reservation bug is addressed with a single-source-of-truth batching iterator and is covered by both property/unit tests and an integration regression test.

Review effort: Lite
Findings: None

Resolved since last review (2)

• Use checked addition to prevent overflow from undercounting transfer cost
• Prevent overflow in generated transfer amount plus batch fee

[mbjorkqvist]

This constant has to agree with what State::transaction_fee computes from the signed message (FEE_PER_SIGNATURE * num_required_signatures) when the batch is recorded as submitted. If the batch withdrawal transaction ever gains a second signer, the iterator would admit a batch that then traps in process_transaction_submitted, which is the same failure shape as the bug fixed here.

Since 2cfb9d0 removed the two timer-level tests, the validator integration test is the only thing that checks that agreement, and it will go away with the manual flow redesign (#207). Could we keep a cheap unit test in withdraw/tests.rs that drives a request costing exactly the balance (and one leaving exactly one fee) through process_pending_withdrawals and asserts the resulting balance? The removed should_process_withdrawal_leaving_exactly_one_transaction_fee does that, once its transfer amount is reduced by one more fee as Copilot pointed out.

[mbjorkqvist]

Nit: this let ... else cannot fail after the successful peek() above. Would it make sense to add an expect("BUG: peeked request vanished"), or restructuring so that the request is consumed inside the let Some(...) above?