refactor(stablecoin): address review on Position migration

programs/stablecoin/src/open_position.rs

@@ -78,7 +78,7 @@ pub fn open_position(
         vault_account_id: vault.account_id,
         collateral_amount,
         normalized_debt_amount: 0,
-        // TODO(Plan 3): read from ctx clock once `open_position` is rebuilt with
+        // TODO(#173): read from ctx clock once `open_position` is rebuilt with
         // the fee-aware flow. Setting 0 keeps Plan 1 a pure refactor.
         opened_at: 0,
     });

programs/stablecoin/src/repay_debt.rs

@@ -12,13 +12,13 @@ use token_core::TokenHolding;
 /// amount. The position post-state uses plain [`AccountPostState::new`] — the
 /// PDA was already claimed at `open_position` time.
 ///
-/// Until Plan 3 (stability fee accrual) lands, the fee-accrual step is a
+/// Until #173 (stability fee accrual) lands, the fee-accrual step is a
 /// no-op (every position structurally has `normalized_debt_amount = 0` today
 /// because `generate_debt` is unimplemented; "fees-accrued" is therefore
-/// vacuously true). A `// TODO(Plan 3)` comment marks where the accrual code
+/// vacuously true). A `// TODO(#173)` comment marks where the accrual code
 /// will plug in — right before the `checked_sub` below.
 ///
-/// Until Plan 3 (`generate_debt`) records the stablecoin definition into
+/// Until #173 (`generate_debt`) records the stablecoin definition into
 /// `Position`, this instruction cannot validate that `stablecoin_definition`
 /// is the correct one for the position's debt. The caller is trusted.
 ///
@@ -54,7 +54,7 @@ pub fn repay_debt(
     let position_data = Position::try_from(&position.account.data)
         .expect("Position account must hold valid Position state");
     // `verify_position_and_get_seed` asserts the position address matches the
-    // (owner, collateral_definition) PDA derivation. The returned seed is
+    // (owner, position_nonce) PDA derivation. The returned seed is
     // dropped — the position is already PDA-claimed.
     let _position_seed = verify_position_and_get_seed(
         &position,
@@ -62,6 +62,12 @@ pub fn repay_debt(
         position_data.position_nonce,
         stablecoin_program_id,
     );
+    // The PDA derivation above already binds the owner; this guards the stored
+    // discovery copy against silently drifting out of sync.
+    assert_eq!(
+        position_data.owner_account_id, owner.account_id,
+        "Position owner_account_id does not match the owner account"
+    );
 
     assert!(
         user_stablecoin_holding.is_authorized,
@@ -89,7 +95,7 @@ pub fn repay_debt(
         "Stablecoin holding does not match the provided stablecoin definition"
     );
 
-    // TODO(Plan 3): accrue stability fees onto position_data.normalized_debt_amount
+    // TODO(#173): accrue stability fees onto position_data.normalized_debt_amount
     // here, before the checked_sub below. Today every position has
     // normalized_debt_amount = 0 (no generate_debt yet), so the precondition is
     // trivially met.

programs/stablecoin/src/tests.rs

@@ -959,3 +959,56 @@ fn repay_debt_rejects_overrepay() {
         200,
     );
 }
+
+/// Mutates only the stored discovery copy, leaving the position address (and so
+/// the `(owner, position_nonce)` PDA derivation) valid — the state a corrupted
+/// or malformed `Position` would present.
+fn position_with_mutated_fields(mutate: impl FnOnce(&mut Position)) -> AccountWithMetadata {
+    let mut position = init_position_account(500, 0);
+    let mut data = Position::try_from(&position.account.data).expect("valid Position");
+    mutate(&mut data);
+    position.account.data = Data::from(&data);
+    position
+}
+
+#[test]
+#[should_panic(expected = "Position owner_account_id does not match the owner account")]
+fn withdraw_collateral_rejects_position_with_stale_owner_field() {
+    crate::withdraw_collateral::withdraw_collateral(
+        owner_account(),
+        position_with_mutated_fields(|p| p.owner_account_id = AccountId::new([0xAAu8; 32])),
+        init_vault_account(),
+        destination_holding_account(),
+        STABLECOIN_PROGRAM_ID,
+        100,
+    );
+}
+
+#[test]
+#[should_panic(expected = "Position vault_account_id does not match the vault account")]
+fn withdraw_collateral_rejects_position_with_stale_vault_field() {
+    crate::withdraw_collateral::withdraw_collateral(
+        owner_account(),
+        position_with_mutated_fields(|p| p.vault_account_id = AccountId::new([0xBBu8; 32])),
+        init_vault_account(),
+        destination_holding_account(),
+        STABLECOIN_PROGRAM_ID,
+        100,
+    );
+}
+
+#[test]
+#[should_panic(expected = "Position owner_account_id does not match the owner account")]
+fn repay_debt_rejects_position_with_stale_owner_field() {
+    crate::repay_debt::repay_debt(
+        owner_account(),
+        position_with_mutated_fields(|p| {
+            p.normalized_debt_amount = 300;
+            p.owner_account_id = AccountId::new([0xAAu8; 32]);
+        }),
+        stablecoin_definition_account(),
+        user_stablecoin_holding_account(1_000),
+        STABLECOIN_PROGRAM_ID,
+        100,
+    );
+}

programs/stablecoin/src/withdraw_collateral.rs

@@ -13,7 +13,7 @@ use token_core::TokenHolding;
 /// the initial PDA claim already happened in
 /// [`crate::open_position::open_position`].
 ///
-/// Until Plan 3 lands (redemption price, price feed, stability fee accrual),
+/// Until #173 lands (redemption price, price feed, stability fee accrual),
 /// this instruction hard-asserts `Position.normalized_debt_amount == 0`.
 /// When that lands, this guard is replaced by real fee accrual + a
 /// collateralization-ratio check against the post-withdrawal collateral.
@@ -52,7 +52,7 @@ pub fn withdraw_collateral(
     let position_data = Position::try_from(&position.account.data)
         .expect("Position account must hold valid Position state");
     // `verify_position_and_get_seed` asserts the position address matches the
-    // (owner, collateral_definition) PDA derivation. We do not use the seed
+    // (owner, position_nonce) PDA derivation. We do not use the seed
     // downstream — the position is already PDA-claimed.
     let _position_seed = verify_position_and_get_seed(
         &position,
@@ -60,15 +60,25 @@ pub fn withdraw_collateral(
         position_data.position_nonce,
         stablecoin_program_id,
     );
+    // The PDA derivation above already binds the owner; this guards the stored
+    // discovery copy against silently drifting out of sync.
+    assert_eq!(
+        position_data.owner_account_id, owner.account_id,
+        "Position owner_account_id does not match the owner account"
+    );
     let vault_seed =
         verify_position_vault_and_get_seed(&vault, position.account_id, stablecoin_program_id);
+    assert_eq!(
+        position_data.vault_account_id, vault.account_id,
+        "Position vault_account_id does not match the vault account"
+    );
 
     let vault_holding = TokenHolding::try_from(&vault.account.data)
         .expect("Vault account must hold a valid TokenHolding");
     // The vault PDA is verified to belong to this position, so its holding's
     // definition is the authoritative collateral definition. Plan 1 dropped the
     // redundant copy from `Position`; `ProtocolParameters` owns the global
-    // collateral definition from Plan 3 onward.
+    // collateral definition from #173 onward.
     let collateral_definition_id = vault_holding.definition_id();
 
     let token_program_id = vault.account.program_owner;
@@ -91,7 +101,7 @@ pub fn withdraw_collateral(
 
     assert_eq!(
         position_data.normalized_debt_amount, 0,
-        "withdraw_collateral with debt is not supported yet — fee accrual + collateralization check land in Plan 3"
+        "withdraw_collateral with debt is not supported yet — fee accrual + collateralization check land in #173"
     );
     let new_collateral = position_data
         .collateral_amount