From 5f5cb674f0ac7fc6659fb99ce3498df7a66c1b4c Mon Sep 17 00:00:00 2001 From: Mike MacCana Date: Sun, 4 Oct 2026 16:51:48 +0000 Subject: [PATCH] Remove false re-entry claims from checks-effects-interactions comments Several handlers labelled their state-before-transfer ordering "checks-effects-interactions" and said it stops a token hook or a future token extension re-entering the program to double-withdraw. On Solana that cannot happen: the runtime rejects a call into a program already on the call stack with ReentrancyNotAllowed, unless the program is calling itself directly. In the Anchor versions the claim was also wrong about the code, because the zeroed fields are written to the account only when the handler exits, after the transfers. Each comment now says what the code does, plus the true reason the order is safe where one was given (a failed transfer reverts the whole transaction). The order-book READMEs state the runtime rule. No code changes. Claude-Session: https://claude.ai/code/session_014pXG1pw56BE99p4A8qmrQk --- finance/fundraiser/anchor-v1/README.md | 2 +- .../fundraiser/src/instructions/contribute.rs | 1 - .../programs/fundraiser/src/instructions/refund.rs | 1 - finance/fundraiser/anchor/README.md | 2 +- .../fundraiser/src/instructions/contribute.rs | 1 - .../programs/fundraiser/src/instructions/refund.rs | 1 - .../quasar/src/instructions/check_contributions.rs | 1 - .../fundraiser/quasar/src/instructions/contribute.rs | 1 - finance/fundraiser/quasar/src/instructions/refund.rs | 1 - .../managed-fund/src/instructions/collect_fees.rs | 2 +- .../managed-fund/src/instructions/withdraw.rs | 2 +- .../managed-fund/src/instructions/collect_fees.rs | 2 +- .../managed-fund/src/instructions/withdraw.rs | 2 +- finance/managed-fund/quasar/README.md | 5 ++--- .../quasar/managed-fund/src/instructions/withdraw.rs | 2 +- finance/order-book/anchor-v1/README.md | 12 ++++++------ .../order-book/src/instructions/settle_funds.rs | 10 +++------- finance/order-book/anchor/README.md | 12 ++++++------ .../order-book/src/instructions/settle_funds.rs | 10 +++------- finance/order-book/quasar/README.md | 4 ++-- .../quasar/src/instructions/settle_funds.rs | 7 +++---- .../src/instructions/admin/claim_admin_fees.rs | 4 ++-- .../token-swap/src/instructions/swap_tokens.rs | 4 ++-- .../src/instructions/admin/claim_admin_fees.rs | 4 ++-- .../token-swap/src/instructions/swap_tokens.rs | 4 ++-- .../quasar/src/instructions/claim_admin_fees.rs | 5 ++--- .../quasar/src/instructions/swap_tokens.rs | 10 ++++------ .../src/instructions/transfer_tokens.rs | 4 ++-- .../src/instructions/transfer_tokens.rs | 4 ++-- .../external-delegate-token-master/quasar/src/lib.rs | 4 ++-- 30 files changed, 52 insertions(+), 72 deletions(-) diff --git a/finance/fundraiser/anchor-v1/README.md b/finance/fundraiser/anchor-v1/README.md index 2873c324c..f72a3afe6 100644 --- a/finance/fundraiser/anchor-v1/README.md +++ b/finance/fundraiser/anchor-v1/README.md @@ -81,7 +81,7 @@ All token accounts use `anchor_spl::token_interface` types (`InterfaceAccount Result<()> { FundraiserError::TargetMet ); - // Checks-effects-interactions: update state before the transfer CPI. let refund_amount = accounts.contribution.amount; accounts.fundraiser.current_amount = accounts .fundraiser diff --git a/finance/fundraiser/anchor/README.md b/finance/fundraiser/anchor/README.md index 1c69d7ccf..df28f3bd7 100644 --- a/finance/fundraiser/anchor/README.md +++ b/finance/fundraiser/anchor/README.md @@ -82,7 +82,7 @@ All token accounts use `anchor_spl::token_interface` types (`InterfaceAccount Result<()> { FundraiserError::TargetMet ); - // Checks-effects-interactions: update state before the transfer CPI. let refund_amount = accounts.contribution.amount; accounts.fundraiser.current_amount = accounts .fundraiser diff --git a/finance/fundraiser/quasar/src/instructions/check_contributions.rs b/finance/fundraiser/quasar/src/instructions/check_contributions.rs index ee5dae34e..84a6d9c74 100644 --- a/finance/fundraiser/quasar/src/instructions/check_contributions.rs +++ b/finance/fundraiser/quasar/src/instructions/check_contributions.rs @@ -59,7 +59,6 @@ pub fn handle_check_contributions( FundraiserError::TargetNotMet ); - // Update state before the transfer CPI (checks-effects-interactions). accounts.fundraiser.claimed = PodBool::from(true); // Fundraiser PDA signer seeds: ["fundraiser", maker, bump]. diff --git a/finance/fundraiser/quasar/src/instructions/contribute.rs b/finance/fundraiser/quasar/src/instructions/contribute.rs index c9c6bd886..67518546f 100644 --- a/finance/fundraiser/quasar/src/instructions/contribute.rs +++ b/finance/fundraiser/quasar/src/instructions/contribute.rs @@ -69,7 +69,6 @@ pub fn handle_contribute( )?; require!(now < deadline, FundraiserError::FundraiserEnded); - // Update state before the transfer CPI (checks-effects-interactions). let current_amount: u64 = accounts.fundraiser.current_amount.into(); accounts.fundraiser.current_amount = PodU64::from( current_amount diff --git a/finance/fundraiser/quasar/src/instructions/refund.rs b/finance/fundraiser/quasar/src/instructions/refund.rs index c065bc6ec..b54c08e87 100644 --- a/finance/fundraiser/quasar/src/instructions/refund.rs +++ b/finance/fundraiser/quasar/src/instructions/refund.rs @@ -74,7 +74,6 @@ pub fn handle_refund( let refund_amount: u64 = accounts.contribution.amount.into(); - // Update state before the transfer CPI (checks-effects-interactions). accounts.fundraiser.current_amount = PodU64::from( current_amount .checked_sub(refund_amount) diff --git a/finance/managed-fund/anchor-v1/programs/managed-fund/src/instructions/collect_fees.rs b/finance/managed-fund/anchor-v1/programs/managed-fund/src/instructions/collect_fees.rs index dabbfb217..9db78424a 100644 --- a/finance/managed-fund/anchor-v1/programs/managed-fund/src/instructions/collect_fees.rs +++ b/finance/managed-fund/anchor-v1/programs/managed-fund/src/instructions/collect_fees.rs @@ -80,7 +80,7 @@ pub fn handle_collect_fees(context: Context) -> R return Ok(()); } - // Checks-effects-interactions: update total_shares before CPI + // Add the fee shares to the recorded supply context.accounts.fund.total_shares = total_shares .checked_add(fee_shares) .ok_or(FundError::MathOverflow)?; diff --git a/finance/managed-fund/anchor-v1/programs/managed-fund/src/instructions/withdraw.rs b/finance/managed-fund/anchor-v1/programs/managed-fund/src/instructions/withdraw.rs index 26fcf66c7..f7c3d4d90 100644 --- a/finance/managed-fund/anchor-v1/programs/managed-fund/src/instructions/withdraw.rs +++ b/finance/managed-fund/anchor-v1/programs/managed-fund/src/instructions/withdraw.rs @@ -107,7 +107,7 @@ pub fn handle_withdraw<'info>( *amount = proportion(context.accounts.fund.asset_holdings[index])?; } - // Checks-effects-interactions: shrink supply and holdings before any transfer. + // Shrink the share supply and the recorded holdings by the withdrawn slice. let fund = &mut context.accounts.fund; fund.total_shares = total_shares .checked_sub(shares_to_burn) diff --git a/finance/managed-fund/anchor/programs/managed-fund/src/instructions/collect_fees.rs b/finance/managed-fund/anchor/programs/managed-fund/src/instructions/collect_fees.rs index 8b0ec6cbc..12d2e854a 100644 --- a/finance/managed-fund/anchor/programs/managed-fund/src/instructions/collect_fees.rs +++ b/finance/managed-fund/anchor/programs/managed-fund/src/instructions/collect_fees.rs @@ -80,7 +80,7 @@ pub fn handle_collect_fees(context: &mut Context) return Ok(()); } - // Checks-effects-interactions: update total_shares before CPI + // Add the fee shares to the recorded supply context.accounts.fund.total_shares = total_shares .checked_add(fee_shares) .ok_or(FundError::MathOverflow)?; diff --git a/finance/managed-fund/anchor/programs/managed-fund/src/instructions/withdraw.rs b/finance/managed-fund/anchor/programs/managed-fund/src/instructions/withdraw.rs index 77cac09df..63650b43d 100644 --- a/finance/managed-fund/anchor/programs/managed-fund/src/instructions/withdraw.rs +++ b/finance/managed-fund/anchor/programs/managed-fund/src/instructions/withdraw.rs @@ -107,7 +107,7 @@ pub fn handle_withdraw( *amount = proportion(context.accounts.fund.asset_holdings[index])?; } - // Checks-effects-interactions: shrink supply and holdings before any transfer. + // Shrink the share supply and the recorded holdings by the withdrawn slice. let fund = &mut context.accounts.fund; fund.total_shares = total_shares .checked_sub(shares_to_burn) diff --git a/finance/managed-fund/quasar/README.md b/finance/managed-fund/quasar/README.md index 264de5c1d..788678790 100644 --- a/finance/managed-fund/quasar/README.md +++ b/finance/managed-fund/quasar/README.md @@ -102,9 +102,8 @@ withdraw), in index order. the Fund PDA; only the deployed program can move them, and it does so only along deposit, withdraw, and rebalance. There is no manager path to withdraw holdings, nor to choose a trade: rebalancing is sized by the program. -- The share supply is updated before any mint or burn (checks-effects- - interactions), and value computations use u128 intermediates with checked - arithmetic, flooring in the fund's favour. +- Value computations use u128 intermediates with checked arithmetic, flooring + in the fund's favour. - The management fee is capped (10% per year) and the slippage tolerance is capped (10%), so neither can be configured to drain the fund. - Price feeds are validated against the address recorded on the asset config and diff --git a/finance/managed-fund/quasar/managed-fund/src/instructions/withdraw.rs b/finance/managed-fund/quasar/managed-fund/src/instructions/withdraw.rs index a3f391195..d6a98ec41 100644 --- a/finance/managed-fund/quasar/managed-fund/src/instructions/withdraw.rs +++ b/finance/managed-fund/quasar/managed-fund/src/instructions/withdraw.rs @@ -100,7 +100,7 @@ pub fn handle_withdraw( *amount = proportion(asset_holdings[index])?; } - // Checks-effects-interactions: shrink supply and holdings before any transfer. + // Shrink the share supply and the recorded holdings by the withdrawn slice. fund.total_shares = total_shares .checked_sub(shares_to_burn) .ok_or(FundError::MathOverflow)?; diff --git a/finance/order-book/anchor-v1/README.md b/finance/order-book/anchor-v1/README.md index 4bd40db2f..9d14322b6 100644 --- a/finance/order-book/anchor-v1/README.md +++ b/finance/order-book/anchor-v1/README.md @@ -839,12 +839,12 @@ mint checks on token accounts, PDA seeds). Both transfers are CPIs to the Token program, signed by the `Market` PDA using seeds `["market", base_mint, quote_mint, bump]`. -Order of operations is checks-effects-interactions: the -`unsettled_*` counters are zeroed *before* the transfer CPIs, then -the transfers run. Solana CPIs aren't reentrant in the EVM sense, -but zeroing state first means no future token-program extension or -transfer hook can observe stale unsettled balances mid-CPI and -double-withdraw. +The handler zeroes the `unsettled_*` counters and the transfers pay +out the amounts they held. If a transfer fails, the whole transaction +reverts, zeroing included. Neither the token program nor a transfer +hook can call back into this program during the transfer: Solana's +runtime rejects a call into a program already on the call stack with +`ReentrancyNotAllowed`, unless the program is calling itself directly. **State changes:** diff --git a/finance/order-book/anchor-v1/programs/order-book/src/instructions/settle_funds.rs b/finance/order-book/anchor-v1/programs/order-book/src/instructions/settle_funds.rs index 4a9f53ad0..23097c7b9 100644 --- a/finance/order-book/anchor-v1/programs/order-book/src/instructions/settle_funds.rs +++ b/finance/order-book/anchor-v1/programs/order-book/src/instructions/settle_funds.rs @@ -10,13 +10,9 @@ pub fn handle_settle_funds(context: Context) -> R let market_user = &mut context.accounts.market_user; let market = &context.accounts.market; - // Snapshot the amounts the user is owed, then zero the counters - // BEFORE the token transfers. Checks-effects-interactions: even though - // Solana CPIs don't reenter in the EVM sense, if either transfer ever - // gained a path that called back into this program (custom token - // hooks, transfer-fee extensions with side effects, ...), having stale - // unsettled_* values readable mid-transfer would let a re-entry double- - // withdraw. Updating state first makes that class of bug impossible. + // Snapshot the amounts the user is owed and zero the counters; the + // transfers below pay out the snapshot. If a transfer fails, the whole + // transaction reverts, zeroing included. let base_amount = market_user.unsettled_base; let quote_amount = market_user.unsettled_quote; market_user.unsettled_base = 0; diff --git a/finance/order-book/anchor/README.md b/finance/order-book/anchor/README.md index 300ce5573..bf96c3fb7 100644 --- a/finance/order-book/anchor/README.md +++ b/finance/order-book/anchor/README.md @@ -847,12 +847,12 @@ mint checks on token accounts, PDA seeds). Both transfers are CPIs to the Token program, signed by the `Market` PDA using seeds `["market", base_mint, quote_mint, bump]`. -Order of operations is checks-effects-interactions: the -`unsettled_*` counters are zeroed *before* the transfer CPIs, then -the transfers run. Solana CPIs aren't reentrant in the EVM sense, -but zeroing state first means no future token-program extension or -transfer hook can observe stale unsettled balances mid-CPI and -double-withdraw. +The handler zeroes the `unsettled_*` counters and the transfers pay +out the amounts they held. If a transfer fails, the whole transaction +reverts, zeroing included. Neither the token program nor a transfer +hook can call back into this program during the transfer: Solana's +runtime rejects a call into a program already on the call stack with +`ReentrancyNotAllowed`, unless the program is calling itself directly. **State changes:** diff --git a/finance/order-book/anchor/programs/order-book/src/instructions/settle_funds.rs b/finance/order-book/anchor/programs/order-book/src/instructions/settle_funds.rs index 2a9087284..f9c3a456a 100644 --- a/finance/order-book/anchor/programs/order-book/src/instructions/settle_funds.rs +++ b/finance/order-book/anchor/programs/order-book/src/instructions/settle_funds.rs @@ -9,13 +9,9 @@ use crate::state::{Market, MarketUser, MARKET_SEED, MARKET_USER_SEED}; pub fn handle_settle_funds(context: &mut Context) -> Result<()> { let market_user = &mut context.accounts.market_user; - // Snapshot the amounts the user is owed, then zero the counters - // BEFORE the token transfers. Checks-effects-interactions: even though - // Solana CPIs don't reenter in the EVM sense, if either transfer ever - // gained a path that called back into this program (custom token - // hooks, transfer-fee extensions with side effects, ...), having stale - // unsettled_* values readable mid-transfer would let a re-entry double- - // withdraw. Updating state first makes that class of bug impossible. + // Snapshot the amounts the user is owed and zero the counters; the + // transfers below pay out the snapshot. If a transfer fails, the whole + // transaction reverts, zeroing included. let base_amount = market_user.unsettled_base; let quote_amount = market_user.unsettled_quote; market_user.unsettled_base = 0; diff --git a/finance/order-book/quasar/README.md b/finance/order-book/quasar/README.md index 91da4d5dd..21db44730 100644 --- a/finance/order-book/quasar/README.md +++ b/finance/order-book/quasar/README.md @@ -116,8 +116,8 @@ are all consequences of Quasar being zero-copy, `no_std`, and zero-allocation: - Every vault transfer out is signed by the **market PDA** via `invoke_signed`; only the deployed program can move locked funds. -- `settle_funds` zeroes a user's `unsettled_*` **before** transferring (checks-effects-interactions), so no - token-hook re-entry could double-withdraw. +- `settle_funds` zeroes a user's `unsettled_*` counters and pays out the amounts they held, so a second call has + nothing to pay. - `place_order` binds every market-owned account (`base_vault`, `quote_vault`, `fee_vault`, both mints, the order book) to the addresses stored on the `Market` PDA with `has_one`, so a caller can't substitute the fee vault for a user vault and drain fees. diff --git a/finance/order-book/quasar/src/instructions/settle_funds.rs b/finance/order-book/quasar/src/instructions/settle_funds.rs index b04837bbe..7b95f79c4 100644 --- a/finance/order-book/quasar/src/instructions/settle_funds.rs +++ b/finance/order-book/quasar/src/instructions/settle_funds.rs @@ -44,10 +44,9 @@ pub struct SettleFundsAccountConstraints { pub fn handle_settle_funds( accounts: &mut SettleFundsAccountConstraints, ) -> Result<(), ProgramError> { - // Snapshot the amounts owed, then zero the counters BEFORE the token - // transfers (checks-effects-interactions): updating state first makes a - // re-entry double-withdraw impossible even if a token hook ever gained a - // path back into this program. + // Snapshot the amounts owed and zero the counters; the transfers below + // pay out the snapshot. If a transfer fails, the whole transaction + // reverts, zeroing included. let mut market_user = snapshot_market_user(&accounts.market_user); let base_amount = market_user.unsettled_base; let quote_amount = market_user.unsettled_quote; diff --git a/finance/token-swap/anchor-v1/programs/token-swap/src/instructions/admin/claim_admin_fees.rs b/finance/token-swap/anchor-v1/programs/token-swap/src/instructions/admin/claim_admin_fees.rs index edeee5a86..72db9e02d 100644 --- a/finance/token-swap/anchor-v1/programs/token-swap/src/instructions/admin/claim_admin_fees.rs +++ b/finance/token-swap/anchor-v1/programs/token-swap/src/instructions/admin/claim_admin_fees.rs @@ -40,8 +40,8 @@ pub fn handle_claim_admin_fees(context: Context