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