chore(amm): new_definition allows only uninitialized pools

amm/src/new_definition.rs

@@ -26,7 +26,6 @@ pub fn new_definition(
     fees: u128,
     amm_program_id: ProgramId,
 ) -> (Vec<AccountPostState>, Vec<ChainedCall>) {
-    // 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
+    // Assert that pool is uninitialized (hard precondition)
-    // 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_eq!(
-        pool_account_data.liquidity_pool_supply, 0,
+        pool.account,
-        "Cannot initialize a Pool Definition with nonzero LP supply"
+        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 {
+    let pool_post: AccountPostState = AccountPostState::new_claimed(pool_post.clone());
-        AccountPostState::new_claimed(pool_post.clone())
-    } else {
-        AccountPostState::new(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.program_owner = token_program_id;
+    pool_lp_after_lock.account.data = Data::from(&TokenDefinition::Fungible {
-        pool_lp_after_lock.account.data = Data::from(&TokenDefinition::Fungible {
+        name: String::from("LP Token"),
-            name: String::from("LP Token"),
+        total_supply: MINIMUM_LIQUIDITY,
-            total_supply: MINIMUM_LIQUIDITY,
+        metadata_id: None,
-            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,
-        });
-    }
 
     let call_token_lp_user = ChainedCall::new(
         token_program_id,

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 {
+            &token_core::Instruction::NewFungibleDefinition {
-                amount_to_mint: MINIMUM_LIQUIDITY,
+                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 {
+        &token_core::Instruction::NewFungibleDefinition {
-            amount_to_mint: MINIMUM_LIQUIDITY,
+            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(),

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();