From 1f8eea84422d18efab1f3e271e292ddab2ee8b95 Mon Sep 17 00:00:00 2001 From: Andrea Franz Date: Mon, 13 Apr 2026 17:11:28 +0200 Subject: [PATCH] chore(amm): new_definition allows only uninitialized pools --- amm/src/new_definition.rs | 79 ++++------------- amm/src/tests.rs | 158 ++++++++------------------------- integration_tests/tests/amm.rs | 101 --------------------- 3 files changed, 53 insertions(+), 285 deletions(-) diff --git a/amm/src/new_definition.rs b/amm/src/new_definition.rs index b357dc1..52b637e 100644 --- a/amm/src/new_definition.rs +++ b/amm/src/new_definition.rs @@ -26,7 +26,6 @@ pub fn new_definition( fees: u128, amm_program_id: ProgramId, ) -> (Vec, Vec) { - // Verify token_a and token_b are different let definition_token_a_id = token_core::TokenHolding::try_from(&user_holding_a.account.data) .expect("New definition: AMM Program expects valid Token Holding account for Token A") .definition_id(); @@ -34,13 +33,14 @@ pub fn new_definition( .expect("New definition: AMM Program expects valid Token Holding account for Token B") .definition_id(); - // both instances of the same token program let token_program = user_holding_a.account.program_owner; + // both instances of the same token program assert_eq!( user_holding_b.account.program_owner, token_program, "User Token holdings must use the same Token Program" ); + // Verify token_a and token_b are different assert!( definition_token_a_id != definition_token_b_id, "Cannot set up a swap for a token with itself" @@ -72,19 +72,11 @@ pub fn new_definition( ); assert_supported_fee_tier(fees); - // TODO: return here - // A pool can only be initialized from a fresh account state. - let is_new_pool = pool.account == Account::default(); - let pool_account_data = if is_new_pool { - PoolDefinition::default() - } else { - PoolDefinition::try_from(&pool.account.data) - .expect("AMM program expects a valid Pool account") - }; - + // Assert that pool is uninitialized (hard precondition) assert_eq!( - pool_account_data.liquidity_pool_supply, 0, - "Cannot initialize a Pool Definition with nonzero LP supply" + pool.account, + Account::default(), + "Pool account must be uninitialized" ); // LP Token minting calculation @@ -114,11 +106,7 @@ pub fn new_definition( }; pool_post.data = Data::from(&pool_post_definition); - let pool_post: AccountPostState = if is_new_pool { - AccountPostState::new_claimed(pool_post.clone()) - } else { - AccountPostState::new(pool_post.clone()) - }; + let pool_post: AccountPostState = AccountPostState::new_claimed(pool_post.clone()); let token_program_id = user_holding_a.account.program_owner; @@ -140,59 +128,26 @@ pub fn new_definition( ); // Chain call for liquidity token lock holding - let lock_instruction = if is_new_pool { - token_core::Instruction::NewFungibleDefinition { - name: String::from("LP Token"), - total_supply: MINIMUM_LIQUIDITY, - } - } else { - token_core::Instruction::Mint { - amount_to_mint: MINIMUM_LIQUIDITY, - } - }; - let mut pool_lp_auth = pool_definition_lp.clone(); pool_lp_auth.is_authorized = true; let call_token_lp_lock = ChainedCall::new( token_program_id, vec![pool_lp_auth.clone(), lp_lock_holding.clone()], - &lock_instruction, + &token_core::Instruction::NewFungibleDefinition { + name: String::from("LP Token"), + total_supply: MINIMUM_LIQUIDITY, + }, ) .with_pda_seeds(vec![compute_liquidity_token_pda_seed(pool.account_id)]); let mut pool_lp_after_lock = pool_lp_auth.clone(); - if pool_definition_lp.account == Account::default() { - pool_lp_after_lock.account.program_owner = token_program_id; - pool_lp_after_lock.account.data = Data::from(&TokenDefinition::Fungible { - name: String::from("LP Token"), - total_supply: MINIMUM_LIQUIDITY, - metadata_id: None, - }); - } else { - let token_definition = TokenDefinition::try_from(&pool_definition_lp.account.data) - .expect("New definition: AMM Program expects a valid LP Token Definition Account"); - let TokenDefinition::Fungible { - name, - total_supply, - metadata_id, - } = token_definition - else { - panic!("New definition: LP Token Definition Account must be fungible"); - }; - assert_eq!( - total_supply, 0, - "New definition: existing LP Token Definition Account must have zero supply before reinitialization" - ); - - pool_lp_after_lock.account.data = Data::from(&TokenDefinition::Fungible { - name, - total_supply: total_supply - .checked_add(MINIMUM_LIQUIDITY) - .expect("LP total supply overflow on lock mint"), - metadata_id, - }); - } + pool_lp_after_lock.account.program_owner = token_program_id; + pool_lp_after_lock.account.data = Data::from(&TokenDefinition::Fungible { + name: String::from("LP Token"), + total_supply: MINIMUM_LIQUIDITY, + metadata_id: None, + }); let call_token_lp_user = ChainedCall::new( token_program_id, diff --git a/amm/src/tests.rs b/amm/src/tests.rs index dd9c3f9..922d35a 100644 --- a/amm/src/tests.rs +++ b/amm/src/tests.rs @@ -468,14 +468,15 @@ impl ChainedCallForTests { } fn cc_new_definition_token_lp_lock() -> ChainedCall { - let mut pool_lp_auth = AccountForTests::pool_lp_reinitializable(); + let mut pool_lp_auth = AccountForTests::pool_lp_uninit(); pool_lp_auth.is_authorized = true; ChainedCall::new( TOKEN_PROGRAM_ID, vec![pool_lp_auth, AccountForTests::lp_lock_holding_uninit()], - &token_core::Instruction::Mint { - amount_to_mint: MINIMUM_LIQUIDITY, + &token_core::Instruction::NewFungibleDefinition { + name: String::from("LP Token"), + total_supply: MINIMUM_LIQUIDITY, }, ) .with_pda_seeds(vec![compute_liquidity_token_pda_seed( @@ -487,7 +488,7 @@ impl ChainedCallForTests { ChainedCall::new( TOKEN_PROGRAM_ID, vec![ - AccountForTests::pool_lp_reinitialized_after_lock(), + AccountForTests::pool_lp_created_after_lock(), AccountForTests::user_holding_lp_uninit(), ], &token_core::Instruction::Mint { @@ -732,40 +733,6 @@ impl AccountWithMetadataForTests { } } - fn pool_lp_reinitializable() -> AccountWithMetadata { - AccountWithMetadata { - account: Account { - program_owner: TOKEN_PROGRAM_ID, - balance: 0u128, - data: Data::from(&TokenDefinition::Fungible { - name: String::from("test"), - total_supply: 0, - metadata_id: None, - }), - nonce: Nonce(0), - }, - is_authorized: true, - account_id: IdForTests::token_lp_definition_id(), - } - } - - fn pool_lp_reinitialized_after_lock() -> AccountWithMetadata { - AccountWithMetadata { - account: Account { - program_owner: TOKEN_PROGRAM_ID, - balance: 0u128, - data: Data::from(&TokenDefinition::Fungible { - name: String::from("test"), - total_supply: MINIMUM_LIQUIDITY, - metadata_id: None, - }), - nonce: Nonce(0), - }, - is_authorized: true, - account_id: IdForTests::token_lp_definition_id(), - } - } - fn pool_lp_uninit() -> AccountWithMetadata { AccountWithMetadata { account: Account::default(), @@ -883,6 +850,14 @@ impl AccountWithMetadataForTests { } } + fn pool_definition_uninit() -> AccountWithMetadata { + AccountWithMetadata { + account: Account::default(), + is_authorized: true, + account_id: IdForTests::pool_definition_id(), + } + } + /// A smaller pool (reserve_a=1_000, reserve_b=500) used exclusively by /// swap-exact-output tests, whose hardcoded expected values were computed /// against these reserves. vault_a_init/vault_b_init still satisfy the @@ -1186,29 +1161,6 @@ impl AccountWithMetadataForTests { } } - fn pool_definition_zero_supply_reinitializable() -> AccountWithMetadata { - AccountWithMetadata { - account: Account { - program_owner: ProgramId::default(), - balance: 0u128, - data: Data::from(&PoolDefinition { - definition_token_a_id: IdForTests::token_a_definition_id(), - definition_token_b_id: IdForTests::token_b_definition_id(), - vault_a_id: IdForTests::vault_a_id(), - vault_b_id: IdForTests::vault_b_id(), - liquidity_pool_id: IdForTests::token_lp_definition_id(), - liquidity_pool_supply: 0, - reserve_a: 0, - reserve_b: 0, - fees: 0u128, - }), - nonce: Nonce(0), - }, - is_authorized: true, - account_id: IdForTests::pool_definition_id(), - } - } - fn pool_definition_with_wrong_id() -> AccountWithMetadata { AccountWithMetadata { account: Account { @@ -1903,14 +1855,15 @@ fn test_call_new_definition_wrong_vault_id_2() { ); } -#[should_panic(expected = "Cannot initialize a Pool Definition with nonzero LP supply")] +#[should_panic(expected = "Pool account must be uninitialized")] #[test] -fn test_call_new_definition_cannot_initialize_nonzero_supply_pool() { +fn test_call_new_definition_rejects_initialized_pool() { + // Verify that new_definition fails if passed an already-initialized pool let _post_states = new_definition( AccountWithMetadataForTests::pool_definition_init(), AccountWithMetadataForTests::vault_a_init(), AccountWithMetadataForTests::vault_b_init(), - AccountWithMetadataForTests::pool_lp_init(), + AccountWithMetadataForTests::pool_lp_uninit(), AccountWithMetadataForTests::lp_lock_holding_uninit(), AccountWithMetadataForTests::user_holding_a(), AccountWithMetadataForTests::user_holding_b(), @@ -1927,10 +1880,10 @@ fn test_call_new_definition_cannot_initialize_nonzero_supply_pool() { fn test_call_new_definition_initial_lp_too_small() { // isqrt(1000 * 1000) = 1000 == MINIMUM_LIQUIDITY, so the assertion fires. let _post_states = new_definition( - AccountWithMetadataForTests::pool_definition_zero_supply_reinitializable(), + AccountWithMetadataForTests::pool_definition_uninit(), AccountWithMetadataForTests::vault_a_init(), AccountWithMetadataForTests::vault_b_init(), - AccountWithMetadataForTests::pool_lp_reinitializable(), + AccountWithMetadataForTests::pool_lp_uninit(), AccountWithMetadataForTests::lp_lock_holding_uninit(), AccountWithMetadataForTests::user_holding_a(), AccountWithMetadataForTests::user_holding_b(), @@ -1945,10 +1898,10 @@ fn test_call_new_definition_initial_lp_too_small() { #[test] fn test_call_new_definition_chained_call_successful() { let (post_states, chained_calls) = new_definition( - AccountWithMetadataForTests::pool_definition_zero_supply_reinitializable(), + AccountWithMetadataForTests::pool_definition_uninit(), AccountWithMetadataForTests::vault_a_init(), AccountWithMetadataForTests::vault_b_init(), - AccountWithMetadataForTests::pool_lp_reinitializable(), + AccountWithMetadataForTests::pool_lp_uninit(), AccountWithMetadataForTests::lp_lock_holding_uninit(), AccountWithMetadataForTests::user_holding_a(), AccountWithMetadataForTests::user_holding_b(), @@ -2440,10 +2393,10 @@ fn swap_exact_output_overflow_protection() { #[test] fn test_new_definition_lp_asymmetric_amounts() { let (post_states, chained_calls) = new_definition( - AccountWithMetadataForTests::pool_definition_zero_supply_reinitializable(), + AccountWithMetadataForTests::pool_definition_uninit(), AccountWithMetadataForTests::vault_a_init(), AccountWithMetadataForTests::vault_b_init(), - AccountWithMetadataForTests::pool_lp_reinitializable(), + AccountWithMetadataForTests::pool_lp_uninit(), AccountWithMetadataForTests::lp_lock_holding_uninit(), AccountWithMetadataForTests::user_holding_a(), AccountWithMetadataForTests::user_holding_b(), @@ -2477,10 +2430,10 @@ fn test_new_definition_lp_symmetric_amounts() { assert_eq!(expected_lp, 2_000); let (post_states, chained_calls) = new_definition( - AccountWithMetadataForTests::pool_definition_zero_supply_reinitializable(), + AccountWithMetadataForTests::pool_definition_uninit(), AccountWithMetadataForTests::vault_a_init(), AccountWithMetadataForTests::vault_b_init(), - AccountWithMetadataForTests::pool_lp_reinitializable(), + AccountWithMetadataForTests::pool_lp_uninit(), AccountWithMetadataForTests::lp_lock_holding_uninit(), AccountWithMetadataForTests::user_holding_a(), AccountWithMetadataForTests::user_holding_b(), @@ -2498,7 +2451,7 @@ fn test_new_definition_lp_symmetric_amounts() { let chained_call_lp_lock = chained_calls[0].clone(); let chained_call_lp_user = chained_calls[1].clone(); - let mut pool_lp_auth = AccountForTests::pool_lp_reinitializable(); + let mut pool_lp_auth = AccountForTests::pool_lp_uninit(); pool_lp_auth.is_authorized = true; let expected_lp_lock_call = ChainedCall::new( TOKEN_PROGRAM_ID, @@ -2506,8 +2459,9 @@ fn test_new_definition_lp_symmetric_amounts() { pool_lp_auth.clone(), AccountForTests::lp_lock_holding_uninit(), ], - &token_core::Instruction::Mint { - amount_to_mint: MINIMUM_LIQUIDITY, + &token_core::Instruction::NewFungibleDefinition { + name: String::from("LP Token"), + total_supply: MINIMUM_LIQUIDITY, }, ) .with_pda_seeds(vec![compute_liquidity_token_pda_seed( @@ -2517,7 +2471,7 @@ fn test_new_definition_lp_symmetric_amounts() { let expected_lp_user_call = ChainedCall::new( TOKEN_PROGRAM_ID, vec![ - AccountForTests::pool_lp_reinitialized_after_lock(), + AccountForTests::pool_lp_created_after_lock(), AccountForTests::user_holding_lp_uninit(), ], &token_core::Instruction::Mint { @@ -2532,46 +2486,6 @@ fn test_new_definition_lp_symmetric_amounts() { assert_eq!(chained_call_lp_user, expected_lp_user_call); } -#[should_panic(expected = "Cannot initialize a Pool Definition with nonzero LP supply")] -#[test] -fn test_call_new_definition_reinitialization_requires_zero_pool_supply() { - let _post_states = new_definition( - AccountWithMetadataForTests::pool_definition_init(), - AccountWithMetadataForTests::vault_a_init(), - AccountWithMetadataForTests::vault_b_init(), - AccountWithMetadataForTests::pool_lp_reinitializable(), - AccountWithMetadataForTests::lp_lock_holding_uninit(), - AccountWithMetadataForTests::user_holding_a(), - AccountWithMetadataForTests::user_holding_b(), - AccountWithMetadataForTests::user_holding_lp_uninit(), - NonZero::new(BalanceForTests::vault_a_reserve_init()).unwrap(), - NonZero::new(BalanceForTests::vault_b_reserve_init()).unwrap(), - BalanceForTests::fee_tier(), - AMM_PROGRAM_ID, - ); -} - -#[should_panic( - expected = "New definition: existing LP Token Definition Account must have zero supply before reinitialization" -)] -#[test] -fn test_call_new_definition_reinitialization_requires_zero_lp_definition_supply() { - let _post_states = new_definition( - AccountWithMetadataForTests::pool_definition_zero_supply_reinitializable(), - AccountWithMetadataForTests::vault_a_init(), - AccountWithMetadataForTests::vault_b_init(), - AccountWithMetadataForTests::pool_lp_init(), - AccountWithMetadataForTests::lp_lock_holding_uninit(), - AccountWithMetadataForTests::user_holding_a(), - AccountWithMetadataForTests::user_holding_b(), - AccountWithMetadataForTests::user_holding_lp_uninit(), - NonZero::new(BalanceForTests::vault_a_reserve_init()).unwrap(), - NonZero::new(BalanceForTests::vault_b_reserve_init()).unwrap(), - BalanceForTests::fee_tier(), - AMM_PROGRAM_ID, - ); -} - #[test] fn test_minimum_liquidity_lock_and_remove_all_user_lp() { let pool_uninitialized = AccountWithMetadata { @@ -2787,10 +2701,10 @@ fn new_definition_overflow_protection() { let large_amount = u128::MAX / 2 + 1; let _result = new_definition( - AccountWithMetadataForTests::pool_definition_zero_supply_reinitializable(), + AccountWithMetadataForTests::pool_definition_uninit(), AccountWithMetadataForTests::vault_a_init(), AccountWithMetadataForTests::vault_b_init(), - AccountWithMetadataForTests::pool_lp_reinitializable(), + AccountWithMetadataForTests::pool_lp_uninit(), AccountWithMetadataForTests::lp_lock_holding_uninit(), AccountWithMetadataForTests::user_holding_a(), AccountWithMetadataForTests::user_holding_b(), @@ -3033,10 +2947,10 @@ fn test_new_definition_supports_all_fee_tiers() { FEE_TIER_BPS_100, ] { let (post_states, _) = new_definition( - AccountWithMetadataForTests::pool_definition_zero_supply_reinitializable(), + AccountWithMetadataForTests::pool_definition_uninit(), AccountWithMetadataForTests::vault_a_init(), AccountWithMetadataForTests::vault_b_init(), - AccountWithMetadataForTests::pool_lp_reinitializable(), + AccountWithMetadataForTests::pool_lp_uninit(), AccountWithMetadataForTests::lp_lock_holding_uninit(), AccountWithMetadataForTests::user_holding_a(), AccountWithMetadataForTests::user_holding_b(), @@ -3057,10 +2971,10 @@ fn test_new_definition_supports_all_fee_tiers() { #[test] fn test_new_definition_rejects_unsupported_fee_tier() { let _ = new_definition( - AccountWithMetadataForTests::pool_definition_zero_supply_reinitializable(), + AccountWithMetadataForTests::pool_definition_uninit(), AccountWithMetadataForTests::vault_a_init(), AccountWithMetadataForTests::vault_b_init(), - AccountWithMetadataForTests::pool_lp_reinitializable(), + AccountWithMetadataForTests::pool_lp_uninit(), AccountWithMetadataForTests::lp_lock_holding_uninit(), AccountWithMetadataForTests::user_holding_a(), AccountWithMetadataForTests::user_holding_b(), diff --git a/integration_tests/tests/amm.rs b/integration_tests/tests/amm.rs index 00d4128..a28dfde 100644 --- a/integration_tests/tests/amm.rs +++ b/integration_tests/tests/amm.rs @@ -1048,107 +1048,6 @@ fn amm_remove_liquidity_insufficient_user_lp_fails() { assert!(state.transition_from_public_transaction(&tx, 0).is_err()); } -#[test] -fn amm_new_definition_zero_supply_initialized_pool_and_uninit_user_lp() { - let mut state = state_for_amm_tests_with_new_def(); - state.force_insert_account(Ids::vault_a(), Accounts::vault_a_reinitializable()); - state.force_insert_account(Ids::vault_b(), Accounts::vault_b_reinitializable()); - state.force_insert_account( - Ids::pool_definition(), - Accounts::pool_definition_zero_supply_reinitializable(), - ); - state.force_insert_account( - Ids::token_lp_definition(), - Accounts::token_lp_definition_reinitializable(), - ); - - execute_new_definition(&mut state, Balances::fee_tier()); - - assert_eq!( - state.get_account_by_id(Ids::pool_definition()), - Accounts::pool_definition_new_init() - ); - assert_eq!( - state.get_account_by_id(Ids::vault_a()), - Accounts::vault_a_init() - ); - assert_eq!( - state.get_account_by_id(Ids::vault_b()), - Accounts::vault_b_init() - ); - assert_eq!( - state.get_account_by_id(Ids::token_lp_definition()), - Accounts::token_lp_definition_new_init() - ); - assert_eq!( - state.get_account_by_id(Ids::lp_lock_holding()), - Accounts::lp_lock_holding_new_init() - ); - assert_eq!( - state.get_account_by_id(Ids::user_a()), - Accounts::user_a_holding_new_init() - ); - assert_eq!( - state.get_account_by_id(Ids::user_b()), - Accounts::user_b_holding_new_init() - ); - assert_eq!( - state.get_account_by_id(Ids::user_lp()), - Accounts::user_lp_holding_new_init() - ); -} - -#[test] -fn amm_new_definition_zero_supply_initialized_pool_init_user_lp() { - let mut state = state_for_amm_tests_with_new_def(); - state.force_insert_account(Ids::vault_a(), Accounts::vault_a_reinitializable()); - state.force_insert_account(Ids::vault_b(), Accounts::vault_b_reinitializable()); - state.force_insert_account( - Ids::pool_definition(), - Accounts::pool_definition_zero_supply_reinitializable(), - ); - state.force_insert_account( - Ids::token_lp_definition(), - Accounts::token_lp_definition_reinitializable(), - ); - state.force_insert_account(Ids::user_lp(), Accounts::user_lp_holding_init_zero()); - - execute_new_definition(&mut state, Balances::fee_tier()); - - assert_eq!( - state.get_account_by_id(Ids::pool_definition()), - Accounts::pool_definition_new_init() - ); - assert_eq!( - state.get_account_by_id(Ids::vault_a()), - Accounts::vault_a_init() - ); - assert_eq!( - state.get_account_by_id(Ids::vault_b()), - Accounts::vault_b_init() - ); - assert_eq!( - state.get_account_by_id(Ids::token_lp_definition()), - Accounts::token_lp_definition_new_init() - ); - assert_eq!( - state.get_account_by_id(Ids::lp_lock_holding()), - Accounts::lp_lock_holding_new_init() - ); - assert_eq!( - state.get_account_by_id(Ids::user_a()), - Accounts::user_a_holding_new_init() - ); - assert_eq!( - state.get_account_by_id(Ids::user_b()), - Accounts::user_b_holding_new_init() - ); - assert_eq!( - state.get_account_by_id(Ids::user_lp()), - Accounts::user_lp_holding_new_init() - ); -} - #[test] fn amm_new_definition_uninitialized_pool() { let mut state = state_for_amm_tests_with_new_def();