fix(stablecoin): address open position review feedback

stablecoin/src/lib.rs

@@ -2,8 +2,6 @@
 
 pub use stablecoin_core as core;
 
-/// No-op instruction used as a heartbeat / sanity-check entry point.
-pub mod noop;
 /// Open a new collateral-only position for a calling owner.
 pub mod open_position;
 

stablecoin/src/noop.rs

@@ -1,6 +0,0 @@
-use nssa_core::{account::AccountWithMetadata, program::AccountPostState};
-
-/// Pass `account` through unchanged as a single post-state entry.
-pub fn noop(account: AccountWithMetadata) -> Vec<AccountPostState> {
-    vec![AccountPostState::new(account.account)]
-}

stablecoin/src/open_position.rs

@@ -23,11 +23,6 @@ use token_core::TokenHolding;
 /// - `user_holding` cannot be decoded as a [`TokenHolding`].
 /// - `user_holding`'s definition does not match `token_definition`.
 /// - `token_definition.program_owner` does not match `user_holding.program_owner`.
-#[allow(
-    clippy::needless_pass_by_value,
-    reason = "instruction handler shape: spel #[instruction] macro deserializes owned \
-              AccountWithMetadata values into these parameters"
-)]
 pub fn open_position(
     owner: AccountWithMetadata,
     position: AccountWithMetadata,
@@ -53,15 +48,6 @@ pub fn open_position(
         "Position vault account must be uninitialized"
     );
 
-    let position_seed = verify_position_and_get_seed(&position, &owner, stablecoin_program_id);
-    let vault_seed =
-        verify_position_vault_and_get_seed(&vault, position.account_id, stablecoin_program_id);
-
-    #[allow(
-        clippy::expect_used,
-        reason = "open_position uses the same panic-on-bad-input pattern as the surrounding \
-                  assert!/assert_eq! invariant checks; runtime must supply a valid TokenHolding"
-    )]
     let user_holding_definition_id = TokenHolding::try_from(&user_holding.account.data)
         .expect("User holding must be a valid Token Holding")
         .definition_id();
@@ -75,6 +61,15 @@ pub fn open_position(
         "Collateral token definition is not owned by the user holding's Token Program"
     );
 
+    let position_seed = verify_position_and_get_seed(
+        &position,
+        &owner,
+        token_definition.account_id,
+        stablecoin_program_id,
+    );
+    let vault_seed =
+        verify_position_vault_and_get_seed(&vault, position.account_id, stablecoin_program_id);
+
     let mut position_post = position.account;
     position_post.program_owner = stablecoin_program_id;
     position_post.data = Data::from(&Position {
@@ -92,9 +87,8 @@ pub fn open_position(
         AccountPostState::new(token_definition.account.clone()),
     ];
 
-    // Chained Token::InitializeAccount sets the vault up as a zero-balance holding of the
-    // collateral token. We hand the runtime the vault marked authorized via PDA so it can
-    // satisfy `InitializeAccount`'s authorization requirement without a user signature.
+    // Chained Token::InitializeAccount owns the vault as a Token holding. The Stablecoin
+    // program only authorizes that claim by passing the vault PDA seed to the chained call.
     let mut vault_authorized = vault.clone();
     vault_authorized.is_authorized = true;
     let initialize_call = ChainedCall::new(

stablecoin/src/tests.rs

@@ -1,5 +1,4 @@
 #![allow(
-    clippy::expect_used,
     clippy::indexing_slicing,
     clippy::panic,
     clippy::unwrap_used,
@@ -32,7 +31,11 @@ fn user_holding_id() -> AccountId {
 }
 
 fn position_id() -> AccountId {
-    compute_position_pda(STABLECOIN_PROGRAM_ID, owner_id())
+    compute_position_pda(
+        STABLECOIN_PROGRAM_ID,
+        owner_id(),
+        collateral_definition_id(),
+    )
 }
 
 fn vault_id() -> AccountId {
@@ -96,17 +99,6 @@ fn uninit_vault_account() -> AccountWithMetadata {
     }
 }
 
-#[test]
-fn noop_returns_single_post_state() {
-    let account = AccountWithMetadata {
-        account: Account::default(),
-        is_authorized: false,
-        account_id: AccountId::new([0u8; 32]),
-    };
-    let post_states = crate::noop::noop(account);
-    assert_eq!(post_states.len(), 1);
-}
-
 #[test]
 fn open_position_claims_pda_and_emits_chained_calls() {
     let collateral_amount: u128 = 500;
@@ -126,7 +118,10 @@ fn open_position_claims_pda_and_emits_chained_calls() {
     let position_post = &post_states[1];
     assert_eq!(
         position_post.required_claim(),
-        Some(Claim::Pda(compute_position_pda_seed(owner_id())))
+        Some(Claim::Pda(compute_position_pda_seed(
+            owner_id(),
+            collateral_definition_id()
+        )))
     );
     let position = Position::try_from(&position_post.account().data).expect("valid Position");
     assert_eq!(
@@ -356,23 +351,44 @@ fn open_position_rejects_definition_with_wrong_token_program() {
 }
 
 #[test]
-fn position_pda_is_deterministic_and_owner_specific() {
-    let id_a = compute_position_pda(STABLECOIN_PROGRAM_ID, owner_id());
-    let id_b = compute_position_pda(STABLECOIN_PROGRAM_ID, owner_id());
+fn position_pda_is_deterministic_and_owner_and_collateral_specific() {
+    let id_a = compute_position_pda(
+        STABLECOIN_PROGRAM_ID,
+        owner_id(),
+        collateral_definition_id(),
+    );
+    let id_b = compute_position_pda(
+        STABLECOIN_PROGRAM_ID,
+        owner_id(),
+        collateral_definition_id(),
+    );
     assert_eq!(id_a, id_b);
 
     let other_owner = AccountId::new([0x11u8; 32]);
     assert_ne!(
-        compute_position_pda(STABLECOIN_PROGRAM_ID, other_owner),
+        compute_position_pda(
+            STABLECOIN_PROGRAM_ID,
+            other_owner,
+            collateral_definition_id()
+        ),
+        id_a
+    );
+
+    let other_definition = AccountId::new([0x21u8; 32]);
+    assert_ne!(
+        compute_position_pda(STABLECOIN_PROGRAM_ID, owner_id(), other_definition),
         id_a
     );
 }
 
 #[test]
 fn position_pda_and_vault_pda_do_not_collide() {
-    // Distinct domain tags must keep the position id and its vault id disjoint, even
-    // though both derivations involve only the owner's address.
-    let position = compute_position_pda(STABLECOIN_PROGRAM_ID, owner_id());
+    // Distinct domain tags must keep the position id and its vault id disjoint.
+    let position = compute_position_pda(
+        STABLECOIN_PROGRAM_ID,
+        owner_id(),
+        collateral_definition_id(),
+    );
     let vault = compute_position_vault_pda(STABLECOIN_PROGRAM_ID, position);
     assert_ne!(position, vault);
 }