From e58aa946bcc7e0adeeb84812171ffe3cf2d20a07 Mon Sep 17 00:00:00 2001 From: Ricardo Guilherme Schmidt <3esmit@gmail.com> Date: Mon, 11 May 2026 12:44:46 -0300 Subject: [PATCH] fix(ata): lock down `ATA::Transfer` recipient contract Enforce at the ATA layer that the recipient token holding is already initialized, owned by the same token program as the sender ATA, decodes to a valid `TokenHolding`, and points at the same token definition as the sender. Align the core instruction doc and guest wrapper doc with that contract, and cover the boundary with unit tests (default, foreign-owned, malformed, mismatched-definition recipients, plus the missing-owner-auth and happy paths) and end-to-end integration tests (default and mismatched-definition recipients). Without this, the downstream `token::Transfer` default-recipient `Claim::Authorized` path was reachable through ATA, so integrators had to reverse-engineer recipient semantics from token/runtime internals. --- ata/core/src/lib.rs | 5 +- ata/methods/guest/src/bin/ata.rs | 3 +- ata/src/tests.rs | 128 +++++++++++++++++++++++++++++++ ata/src/transfer.rs | 35 ++++++++- integration_tests/tests/ata.rs | 79 +++++++++++++++++++ 5 files changed, 244 insertions(+), 6 deletions(-) diff --git a/ata/core/src/lib.rs b/ata/core/src/lib.rs index 4ea46bf..19171a0 100644 --- a/ata/core/src/lib.rs +++ b/ata/core/src/lib.rs @@ -24,7 +24,10 @@ pub enum Instruction { /// Required accounts (3): /// - Owner account (authorized) /// - Sender ATA (owner's token holding) - /// - Recipient token holding (must be initialized) + /// - Recipient token holding. Must be: + /// - already initialized (not a default account), + /// - owned by the same token program as the sender ATA, + /// - and point at the same token definition as the sender. /// /// `token_program_id` is derived from `sender_ata.account.program_owner`. Transfer { diff --git a/ata/methods/guest/src/bin/ata.rs b/ata/methods/guest/src/bin/ata.rs index ad8a950..a3c4b6d 100644 --- a/ata/methods/guest/src/bin/ata.rs +++ b/ata/methods/guest/src/bin/ata.rs @@ -29,7 +29,8 @@ mod ata { } /// Transfer tokens FROM owner's ATA to a recipient token holding account. - /// The recipient holding account must already be initialized. + /// The recipient holding must already be initialized, be owned by the same token program + /// as the sender ATA, and point at the same token definition as the sender. #[instruction] pub fn transfer( owner: AccountWithMetadata, diff --git a/ata/src/tests.rs b/ata/src/tests.rs index 07a2185..8ee4df9 100644 --- a/ata/src/tests.rs +++ b/ata/src/tests.rs @@ -162,3 +162,131 @@ fn get_associated_token_account_id_differs_by_definition() { get_associated_token_account_id(&ATA_PROGRAM_ID, &compute_ata_seed(owner_id(), other_def)); assert_ne!(id1, id2); } + +fn recipient_id() -> AccountId { + AccountId::new([0x03u8; 32]) +} + +fn initialized_recipient_account() -> AccountWithMetadata { + AccountWithMetadata { + account: Account { + program_owner: TOKEN_PROGRAM_ID, + balance: 0, + data: Data::from(&TokenHolding::Fungible { + definition_id: definition_id(), + balance: 0, + }), + nonce: nssa_core::account::Nonce(0), + }, + is_authorized: false, + account_id: recipient_id(), + } +} + +#[test] +fn transfer_emits_chained_call_for_initialized_recipient() { + let (post_states, chained_calls) = crate::transfer::transfer_from_associated_token_account( + owner_account(), + initialized_ata_account(), + initialized_recipient_account(), + ATA_PROGRAM_ID, + 25, + ); + + assert_eq!(post_states.len(), 3); + assert_eq!(chained_calls.len(), 1); + + let mut sender_auth = initialized_ata_account(); + sender_auth.is_authorized = true; + let expected_call = ChainedCall::new( + TOKEN_PROGRAM_ID, + vec![sender_auth, initialized_recipient_account()], + &token_core::Instruction::Transfer { + amount_to_transfer: 25, + }, + ) + .with_pda_seeds(vec![compute_ata_seed(owner_id(), definition_id())]); + + assert_eq!(chained_calls, vec![expected_call]); +} + +#[test] +#[should_panic(expected = "Owner authorization is missing")] +fn transfer_panics_when_owner_not_authorized() { + let mut unauthorized_owner = owner_account(); + unauthorized_owner.is_authorized = false; + + crate::transfer::transfer_from_associated_token_account( + unauthorized_owner, + initialized_ata_account(), + initialized_recipient_account(), + ATA_PROGRAM_ID, + 1, + ); +} + +#[test] +#[should_panic(expected = "Recipient token holding must be initialized")] +fn transfer_panics_when_recipient_is_default() { + let default_recipient = AccountWithMetadata { + account: Account::default(), + is_authorized: false, + account_id: recipient_id(), + }; + + crate::transfer::transfer_from_associated_token_account( + owner_account(), + initialized_ata_account(), + default_recipient, + ATA_PROGRAM_ID, + 1, + ); +} + +#[test] +#[should_panic(expected = "Recipient must be owned by the same token program as the sender ATA")] +fn transfer_panics_when_recipient_is_foreign_owned() { + let mut foreign_recipient = initialized_recipient_account(); + foreign_recipient.account.program_owner = [9u32; 8]; + + crate::transfer::transfer_from_associated_token_account( + owner_account(), + initialized_ata_account(), + foreign_recipient, + ATA_PROGRAM_ID, + 1, + ); +} + +#[test] +#[should_panic(expected = "Recipient must hold a valid token")] +fn transfer_panics_when_recipient_data_is_malformed() { + let mut malformed_recipient = initialized_recipient_account(); + malformed_recipient.account.data = Data::try_from(vec![0xFFu8, 0xFE, 0xFD]).unwrap(); + + crate::transfer::transfer_from_associated_token_account( + owner_account(), + initialized_ata_account(), + malformed_recipient, + ATA_PROGRAM_ID, + 1, + ); +} + +#[test] +#[should_panic(expected = "Recipient and sender token definitions do not match")] +fn transfer_panics_when_recipient_definition_mismatches_sender() { + let mut mismatched_recipient = initialized_recipient_account(); + mismatched_recipient.account.data = Data::from(&TokenHolding::Fungible { + definition_id: AccountId::new([0xAAu8; 32]), + balance: 0, + }); + + crate::transfer::transfer_from_associated_token_account( + owner_account(), + initialized_ata_account(), + mismatched_recipient, + ATA_PROGRAM_ID, + 1, + ); +} diff --git a/ata/src/transfer.rs b/ata/src/transfer.rs index c3198fa..3fa1668 100644 --- a/ata/src/transfer.rs +++ b/ata/src/transfer.rs @@ -1,5 +1,5 @@ use nssa_core::{ - account::AccountWithMetadata, + account::{Account, AccountWithMetadata}, program::{AccountPostState, ChainedCall, ProgramId}, }; use token_core::TokenHolding; @@ -13,11 +13,38 @@ pub fn transfer_from_associated_token_account( ) -> (Vec, Vec) { let token_program_id = sender_ata.account.program_owner; assert!(owner.is_authorized, "Owner authorization is missing"); - let definition_id = TokenHolding::try_from(&sender_ata.account.data) + let sender_definition_id = TokenHolding::try_from(&sender_ata.account.data) .expect("Sender ATA must hold a valid token") .definition_id(); - let sender_seed = - ata_core::verify_ata_and_get_seed(&sender_ata, &owner, definition_id, ata_program_id); + let sender_seed = ata_core::verify_ata_and_get_seed( + &sender_ata, + &owner, + sender_definition_id, + ata_program_id, + ); + + // The recipient contract: ATA::Transfer requires a recipient token holding that is already + // initialized, owned by the same token program as the sender ATA, and that points at the same + // token definition as the sender. Anything else fails here rather than being silently + // materialized by the downstream token transfer (e.g. via `Claim::Authorized` on a default + // recipient), so integrators get an ATA-level failure rather than having to reverse-engineer + // token/runtime semantics. + assert_ne!( + recipient.account, + Account::default(), + "Recipient token holding must be initialized" + ); + assert_eq!( + recipient.account.program_owner, token_program_id, + "Recipient must be owned by the same token program as the sender ATA" + ); + let recipient_definition_id = TokenHolding::try_from(&recipient.account.data) + .expect("Recipient must hold a valid token") + .definition_id(); + assert_eq!( + recipient_definition_id, sender_definition_id, + "Recipient and sender token definitions do not match" + ); let post_states = vec![ AccountPostState::new(owner.account.clone()), diff --git a/integration_tests/tests/ata.rs b/integration_tests/tests/ata.rs index dd2eef7..b487713 100644 --- a/integration_tests/tests/ata.rs +++ b/integration_tests/tests/ata.rs @@ -260,6 +260,85 @@ fn ata_transfer() { ); } +#[test] +fn ata_transfer_rejects_default_recipient() { + let mut state = state_for_ata_tests(); + + let instruction = ata_core::Instruction::Transfer { + ata_program_id: Ids::ata_program(), + amount: 1_u128, + }; + + let message = public_transaction::Message::try_new( + Ids::ata_program(), + vec![Ids::owner(), Ids::owner_ata(), Ids::recipient_ata()], + vec![Nonce(0)], + instruction, + ) + .unwrap(); + + let witness_set = public_transaction::WitnessSet::for_message(&message, &[&Keys::owner_key()]); + + let tx = PublicTransaction::new(message, witness_set); + assert!(state.transition_from_public_transaction(&tx, 0, 0).is_err()); + + assert_eq!( + state.get_account_by_id(Ids::owner_ata()), + Accounts::owner_ata_init() + ); + assert_eq!( + state.get_account_by_id(Ids::recipient_ata()), + Account::default() + ); +} + +#[test] +fn ata_transfer_rejects_mismatched_definition_recipient() { + let mut state = state_for_ata_tests_with_precreated_recipient_ata(); + + // Replace the recipient ATA with a token holding pointing at a different definition. + let foreign_definition_id = AccountId::from(&PublicKey::new_from_private_key( + &PrivateKey::try_new([42; 32]).expect("valid private key"), + )); + let mismatched_recipient = Account { + program_owner: Ids::token_program(), + balance: 0_u128, + data: Data::from(&TokenHolding::Fungible { + definition_id: foreign_definition_id, + balance: 0_u128, + }), + nonce: Nonce(0), + }; + state.force_insert_account(Ids::recipient_ata(), mismatched_recipient.clone()); + + let instruction = ata_core::Instruction::Transfer { + ata_program_id: Ids::ata_program(), + amount: 1_u128, + }; + + let message = public_transaction::Message::try_new( + Ids::ata_program(), + vec![Ids::owner(), Ids::owner_ata(), Ids::recipient_ata()], + vec![Nonce(0)], + instruction, + ) + .unwrap(); + + let witness_set = public_transaction::WitnessSet::for_message(&message, &[&Keys::owner_key()]); + + let tx = PublicTransaction::new(message, witness_set); + assert!(state.transition_from_public_transaction(&tx, 0, 0).is_err()); + + assert_eq!( + state.get_account_by_id(Ids::owner_ata()), + Accounts::owner_ata_init() + ); + assert_eq!( + state.get_account_by_id(Ids::recipient_ata()), + mismatched_recipient + ); +} + #[test] fn ata_burn() { let mut state = state_for_ata_tests();