Skip to content

Commit 28bb47c

Browse files
committed
fix(token-swap): audit pass - checked_sub, basis-points constant, admin folder, test fix
- Fix test compile break: withdraw accounts field is `withdrawer`, not `depositor` (test_swap.rs was passing the old field name) - Replace raw `pool.amount - admin_fees_owed` with checked_sub across swap_tokens, deposit_liquidity, withdraw_liquidity - a BPF release underflow would wrap to a giant effective reserve - Extract the basis-points divisor (10_000) into a named BASIS_POINTS_DIVISOR constant; use it in the swap fee math and create_config constraints - Move the admin-only claim_admin_fees handler into instructions/admin/
1 parent f828c76 commit 28bb47c

9 files changed

Lines changed: 71 additions & 18 deletions

File tree

tokens/token-swap/anchor/programs/token-swap/src/constants.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,13 @@ use anchor_lang::prelude::*;
33
#[constant]
44
pub const MINIMUM_LIQUIDITY: u64 = 100;
55

6+
/// Basis-points denominator. Fees and the admin's fee share are stored in
7+
/// basis points (1 bp = 1/10_000), so dividing by this converts a bp value to
8+
/// a fraction. Using the named constant keeps the 10_000 out of the math as a
9+
/// bare literal.
10+
#[constant]
11+
pub const BASIS_POINTS_DIVISOR: u64 = 10_000;
12+
613
#[constant]
714
pub const CONFIG_SEED: &[u8] = b"config";
815

tokens/token-swap/anchor/programs/token-swap/src/instructions/claim_admin_fees.rs renamed to tokens/token-swap/anchor/programs/token-swap/src/instructions/admin/claim_admin_fees.rs

File renamed without changes.
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
mod claim_admin_fees;
2+
3+
pub use claim_admin_fees::*;

tokens/token-swap/anchor/programs/token-swap/src/instructions/create_config.rs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
use anchor_lang::prelude::*;
22

3-
use crate::{constants::CONFIG_SEED, errors::*, state::Config};
3+
use crate::{
4+
constants::{BASIS_POINTS_DIVISOR, CONFIG_SEED},
5+
errors::*,
6+
state::Config,
7+
};
48

59
pub fn handle_create_config(
610
mut context: Context<CreateConfigAccounts>,
@@ -26,8 +30,8 @@ pub struct CreateConfigAccounts<'info> {
2630
space = Config::DISCRIMINATOR.len() + Config::INIT_SPACE,
2731
seeds = [CONFIG_SEED],
2832
bump,
29-
constraint = fee < 10000 @ AmmError::InvalidFee,
30-
constraint = admin_share_bps < 10000 @ AmmError::AdminShareTooHigh,
33+
constraint = (fee as u64) < BASIS_POINTS_DIVISOR @ AmmError::InvalidFee,
34+
constraint = (admin_share_bps as u64) < BASIS_POINTS_DIVISOR @ AmmError::AdminShareTooHigh,
3135
)]
3236
pub config: Account<'info, Config>,
3337

tokens/token-swap/anchor/programs/token-swap/src/instructions/deposit_liquidity.rs

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,16 @@ pub fn handle_deposit_liquidity(
6666
let pool_a = &context.accounts.pool_a;
6767
let pool_b = &context.accounts.pool_b;
6868
let pool_config = &context.accounts.pool_config;
69-
let effective_pool_a = pool_a.amount - pool_config.admin_fees_owed_a;
70-
let effective_pool_b = pool_b.amount - pool_config.admin_fees_owed_b;
69+
// checked_sub: admin_fees_owed is an invariant subset of the vault balance;
70+
// a raw `-` would wrap silently on a BPF release build if that ever broke.
71+
let effective_pool_a = pool_a
72+
.amount
73+
.checked_sub(pool_config.admin_fees_owed_a)
74+
.ok_or(AmmError::MathOverflow)?;
75+
let effective_pool_b = pool_b
76+
.amount
77+
.checked_sub(pool_config.admin_fees_owed_b)
78+
.ok_or(AmmError::MathOverflow)?;
7179
// Defining pool creation like this allows attackers to frontrun pool creation with bad ratios
7280
let pool_creation = effective_pool_a == 0 && effective_pool_b == 0;
7381
(amount_a, amount_b) = if pool_creation {

tokens/token-swap/anchor/programs/token-swap/src/instructions/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
1-
mod claim_admin_fees;
1+
mod admin;
22
mod create_config;
33
mod create_pool;
44
mod deposit_liquidity;
55
mod swap_tokens;
66
mod withdraw_liquidity;
77

8-
pub use claim_admin_fees::*;
8+
pub use admin::*;
99
pub use create_config::*;
1010
pub use create_pool::*;
1111
pub use deposit_liquidity::*;

tokens/token-swap/anchor/programs/token-swap/src/instructions/swap_tokens.rs

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use anchor_spl::{
55
};
66

77
use crate::{
8-
constants::{AUTHORITY_SEED, CONFIG_SEED},
8+
constants::{AUTHORITY_SEED, BASIS_POINTS_DIVISOR, CONFIG_SEED},
99
errors::*,
1010
state::{Config, PoolConfig},
1111
};
@@ -42,12 +42,12 @@ pub fn handle_swap_tokens(
4242
let fee_amount = (input_amount as u128)
4343
.checked_mul(config.fee as u128)
4444
.ok_or(AmmError::MathOverflow)?
45-
.checked_div(10_000)
45+
.checked_div(BASIS_POINTS_DIVISOR as u128)
4646
.ok_or(AmmError::MathOverflow)?;
4747
let admin_portion = fee_amount
4848
.checked_mul(config.admin_share_bps as u128)
4949
.ok_or(AmmError::MathOverflow)?
50-
.checked_div(10_000)
50+
.checked_div(BASIS_POINTS_DIVISOR as u128)
5151
.ok_or(AmmError::MathOverflow)?;
5252
// Narrow back to u64 for storage / transfer. The fee can never exceed
5353
// `input` (`fee_amount <= input * 9999 / 10_000 < input`, and `input`
@@ -68,8 +68,17 @@ pub fn handle_swap_tokens(
6868
let pool_a = &context.accounts.pool_a;
6969
let pool_b = &context.accounts.pool_b;
7070
let pool_config = &context.accounts.pool_config;
71-
let effective_pool_a = pool_a.amount - pool_config.admin_fees_owed_a;
72-
let effective_pool_b = pool_b.amount - pool_config.admin_fees_owed_b;
71+
// checked_sub: admin_fees_owed is an invariant subset of the vault balance,
72+
// but a raw `-` would wrap silently on a BPF release build if that invariant
73+
// were ever violated, handing the curve a giant effective reserve.
74+
let effective_pool_a = pool_a
75+
.amount
76+
.checked_sub(pool_config.admin_fees_owed_a)
77+
.ok_or(AmmError::MathOverflow)?;
78+
let effective_pool_b = pool_b
79+
.amount
80+
.checked_sub(pool_config.admin_fees_owed_b)
81+
.ok_or(AmmError::MathOverflow)?;
7382

7483
// Constant-product output formula:
7584
// output = taxed_input * other_reserve / (this_reserve + taxed_input)
@@ -231,8 +240,18 @@ pub fn handle_swap_tokens(
231240
context.accounts.pool_a.reload()?;
232241
context.accounts.pool_b.reload()?;
233242
let pool_config = &context.accounts.pool_config;
234-
let effective_pool_a_after = context.accounts.pool_a.amount - pool_config.admin_fees_owed_a;
235-
let effective_pool_b_after = context.accounts.pool_b.amount - pool_config.admin_fees_owed_b;
243+
let effective_pool_a_after = context
244+
.accounts
245+
.pool_a
246+
.amount
247+
.checked_sub(pool_config.admin_fees_owed_a)
248+
.ok_or(AmmError::MathOverflow)?;
249+
let effective_pool_b_after = context
250+
.accounts
251+
.pool_b
252+
.amount
253+
.checked_sub(pool_config.admin_fees_owed_b)
254+
.ok_or(AmmError::MathOverflow)?;
236255
let new_invariant = (effective_pool_a_after as u128)
237256
.checked_mul(effective_pool_b_after as u128)
238257
.ok_or(AmmError::MathOverflow)?;

tokens/token-swap/anchor/programs/token-swap/src/instructions/withdraw_liquidity.rs

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,20 @@ pub fn handle_withdraw_liquidity(
3131
// owed slice physically remains in the vaults but is not distributed to
3232
// exiting LPs - it's claimed separately via `claim_admin_fees`.
3333
let pool_config = &context.accounts.pool_config;
34-
let effective_pool_a = context.accounts.pool_a.amount - pool_config.admin_fees_owed_a;
35-
let effective_pool_b = context.accounts.pool_b.amount - pool_config.admin_fees_owed_b;
34+
// checked_sub: admin_fees_owed is an invariant subset of the vault balance;
35+
// a raw `-` would wrap silently on a BPF release build if that ever broke.
36+
let effective_pool_a = context
37+
.accounts
38+
.pool_a
39+
.amount
40+
.checked_sub(pool_config.admin_fees_owed_a)
41+
.ok_or(AmmError::MathOverflow)?;
42+
let effective_pool_b = context
43+
.accounts
44+
.pool_b
45+
.amount
46+
.checked_sub(pool_config.admin_fees_owed_b)
47+
.ok_or(AmmError::MathOverflow)?;
3648

3749
// Proportional-withdraw formula:
3850
// amount_out = lp_amount * effective_reserve / (lp_supply + MINIMUM_LIQUIDITY)

tokens/token-swap/anchor/programs/token-swap/tests/test_swap.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -421,7 +421,7 @@ fn test_withdraw_liquidity() {
421421
config: ts.config_key,
422422
pool_config: ts.pool_config_key,
423423
pool_authority: ts.pool_authority,
424-
depositor: ts.admin.pubkey(),
424+
withdrawer: ts.admin.pubkey(),
425425
liquidity_provider_mint: ts.liquidity_provider_mint,
426426
mint_a: ts.mint_a,
427427
mint_b: ts.mint_b,
@@ -1239,7 +1239,7 @@ fn withdraw_ix_with_min(
12391239
config: ts.config_key,
12401240
pool_config: ts.pool_config_key,
12411241
pool_authority: ts.pool_authority,
1242-
depositor: ts.admin.pubkey(),
1242+
withdrawer: ts.admin.pubkey(),
12431243
liquidity_provider_mint: ts.liquidity_provider_mint,
12441244
mint_a: ts.mint_a,
12451245
mint_b: ts.mint_b,

0 commit comments

Comments
 (0)