diff --git a/basics/counter/native/program/src/lib.rs b/basics/counter/native/program/src/lib.rs index f8bded9dc..c6fa0b31b 100644 --- a/basics/counter/native/program/src/lib.rs +++ b/basics/counter/native/program/src/lib.rs @@ -20,24 +20,34 @@ use solana_program::entrypoint; entrypoint!(process_instruction); pub fn process_instruction( - _program_id: &Pubkey, + program_id: &Pubkey, accounts: &[AccountInfo], instruction_data: &[u8], ) -> ProgramResult { + // Reject empty instruction data explicitly rather than panicking via split_at. + if instruction_data.is_empty() { + msg!("Error: instruction data is empty"); + return Err(ProgramError::InvalidInstructionData); + } let (instruction_discriminant, instruction_data_inner) = instruction_data.split_at(1); match instruction_discriminant[0] { 0 => { msg!("Instruction: Increment"); - process_increment_counter(accounts, instruction_data_inner)?; + process_increment_counter(program_id, accounts, instruction_data_inner)?; } _ => { - msg!("Error: unknown instruction") + // Previously this branch logged a message and returned Ok(()), which let + // unknown discriminants succeed silently. Return InvalidInstructionData + // so callers get a real failure. + msg!("Error: unknown instruction"); + return Err(ProgramError::InvalidInstructionData); } } Ok(()) } pub fn process_increment_counter( + program_id: &Pubkey, accounts: &[AccountInfo], _instruction_data: &[u8], ) -> Result<(), ProgramError> { @@ -49,6 +59,13 @@ pub fn process_increment_counter( "Counter account must be writable" ); + // Owner check: without this the program would happily decode any 8 bytes of + // data as a Counter, even when the account belongs to a different program. + if counter_account.owner != program_id { + msg!("Error: counter account is not owned by this program"); + return Err(ProgramError::IncorrectProgramId); + } + let mut counter = Counter::try_from_slice(&counter_account.try_borrow_mut_data()?)?; counter.count += 1; counter.serialize(&mut *counter_account.data.borrow_mut())?; diff --git a/basics/counter/native/program/tests/test.rs b/basics/counter/native/program/tests/test.rs index 474631327..aae5344bd 100644 --- a/basics/counter/native/program/tests/test.rs +++ b/basics/counter/native/program/tests/test.rs @@ -9,13 +9,47 @@ use solana_rent::Rent; use solana_system_interface::instruction::create_account; use solana_transaction::Transaction; +// The .so is built into ../../tests/fixtures by `pnpm build-and-test` (which runs +// `cargo build-sbf --sbf-out-dir=./tests/fixtures` from the package root). Run +// that script (or `cargo build-sbf` with --sbf-out-dir set accordingly) before +// `cargo test`. +const PROGRAM_SO: &[u8] = include_bytes!("../../tests/fixtures/counter_solana_native.so"); + +fn setup_with_counter() -> (LiteSVM, Pubkey, Keypair, Keypair) { + let program_id = Pubkey::new_unique(); + + let mut svm = LiteSVM::new(); + svm.add_program(program_id, PROGRAM_SO).unwrap(); + + let payer = Keypair::new(); + let counter_account = Keypair::new(); + + svm.airdrop(&payer.pubkey(), LAMPORTS_PER_SOL * 10).unwrap(); + + let counter_account_size = std::mem::size_of::(); + let create_ix = create_account( + &payer.pubkey(), + &counter_account.pubkey(), + Rent::default().minimum_balance(counter_account_size), + counter_account_size as u64, + &program_id, + ); + let tx = Transaction::new_signed_with_payer( + &[create_ix], + Some(&payer.pubkey()), + &[&payer, &counter_account], + svm.latest_blockhash(), + ); + svm.send_transaction(tx).unwrap(); + (svm, program_id, payer, counter_account) +} + #[test] fn test_counter() { let program_id = Pubkey::new_unique(); - let program_bytes = include_bytes!("../../tests/fixtures/counter_solana_native.so"); let mut svm = LiteSVM::new(); - svm.add_program(program_id, program_bytes).unwrap(); + svm.add_program(program_id, PROGRAM_SO).unwrap(); let payer = Keypair::new(); let counter_account = Keypair::new(); @@ -38,7 +72,7 @@ fn test_counter() { &[&payer, &counter_account], svm.latest_blockhash(), ); - assert!(svm.send_transaction(tx).is_ok()); + svm.send_transaction(tx).unwrap(); let ix = Instruction { program_id, @@ -53,9 +87,79 @@ fn test_counter() { svm.latest_blockhash(), ); - let _ = svm.send_transaction(tx).is_ok(); + // Actually assert the transaction succeeded - the previous shape used + // `let _ = ...is_ok();` which discarded the result. + svm.send_transaction(tx).unwrap(); let counter_account_data = svm.get_account(&counter_account.pubkey()).unwrap().data; let counter = Counter::try_from_slice(&counter_account_data).unwrap(); assert_eq!(counter.count, 1); } + +#[test] +fn test_unknown_instruction_fails() { + let (mut svm, program_id, payer, counter_account) = setup_with_counter(); + + // Discriminant 9 is not handled and must now return an error rather than Ok(()). + let ix = Instruction { + program_id, + accounts: vec![AccountMeta::new(counter_account.pubkey(), false)], + data: vec![9], + }; + let tx = Transaction::new_signed_with_payer( + &[ix], + Some(&payer.pubkey()), + &[&payer], + svm.latest_blockhash(), + ); + assert!( + svm.send_transaction(tx).is_err(), + "unknown instruction discriminant must fail" + ); +} + +#[test] +fn test_wrong_owner_fails() { + let program_id = Pubkey::new_unique(); + + let mut svm = LiteSVM::new(); + svm.add_program(program_id, PROGRAM_SO).unwrap(); + + let payer = Keypair::new(); + let counter_account = Keypair::new(); + svm.airdrop(&payer.pubkey(), LAMPORTS_PER_SOL * 10).unwrap(); + + // Create the counter account but owned by a different (random) program. + let counter_account_size = std::mem::size_of::(); + let wrong_owner = Pubkey::new_unique(); + let create_ix = create_account( + &payer.pubkey(), + &counter_account.pubkey(), + Rent::default().minimum_balance(counter_account_size), + counter_account_size as u64, + &wrong_owner, + ); + let tx = Transaction::new_signed_with_payer( + &[create_ix], + Some(&payer.pubkey()), + &[&payer, &counter_account], + svm.latest_blockhash(), + ); + svm.send_transaction(tx).unwrap(); + + let ix = Instruction { + program_id, + accounts: vec![AccountMeta::new(counter_account.pubkey(), false)], + data: vec![0], + }; + let tx = Transaction::new_signed_with_payer( + &[ix], + Some(&payer.pubkey()), + &[&payer], + svm.latest_blockhash(), + ); + assert!( + svm.send_transaction(tx).is_err(), + "counter owned by a different program must be rejected" + ); +} diff --git a/basics/counter/pinocchio/program/src/lib.rs b/basics/counter/pinocchio/program/src/lib.rs index ed0a0de8d..2e47d5bd4 100644 --- a/basics/counter/pinocchio/program/src/lib.rs +++ b/basics/counter/pinocchio/program/src/lib.rs @@ -21,6 +21,10 @@ pub fn process_instruction( accounts: &[AccountView], instruction_data: &[u8], ) -> ProgramResult { + if instruction_data.is_empty() { + log!("Error: instruction data is empty"); + return Err(ProgramError::InvalidInstructionData); + } let (instruction_discriminant, instruction_data_inner) = instruction_data.split_at(1); match instruction_discriminant[0] { 0 => { @@ -28,7 +32,11 @@ pub fn process_instruction( process_increment_counter(accounts, instruction_data_inner)?; } _ => { + // Previously this branch logged a message and returned Ok(()), which let + // unknown discriminants succeed silently. Return InvalidInstructionData + // so callers get a real failure. log!("Error: unknown instruction"); + return Err(ProgramError::InvalidInstructionData); } } Ok(()) diff --git a/basics/counter/pinocchio/program/tests/test.rs b/basics/counter/pinocchio/program/tests/test.rs index b1eb2ccff..6f9a333a5 100644 --- a/basics/counter/pinocchio/program/tests/test.rs +++ b/basics/counter/pinocchio/program/tests/test.rs @@ -8,13 +8,18 @@ use solana_rent::Rent; use solana_system_interface::instruction::create_account; use solana_transaction::Transaction; +// The .so is built into ../../tests/fixtures by `pnpm build-and-test` (which runs +// `cargo build-sbf --sbf-out-dir=./tests/fixtures` from the package root). Run +// that script (or `cargo build-sbf` with --sbf-out-dir set accordingly) before +// `cargo test`. +const PROGRAM_SO: &[u8] = include_bytes!("../../tests/fixtures/counter_solana_pinocchio.so"); + #[test] fn test_counter() { let program_id = Pubkey::new_unique(); - let program_bytes = include_bytes!("../../tests/fixtures/counter_solana_pinocchio.so"); let mut svm = LiteSVM::new(); - svm.add_program(program_id, program_bytes).unwrap(); + svm.add_program(program_id, PROGRAM_SO).unwrap(); let payer = Keypair::new(); let counter_account = Keypair::new(); @@ -38,8 +43,7 @@ fn test_counter() { svm.latest_blockhash(), ); - let res = svm.send_transaction(tx); - assert!(res.is_ok()); + svm.send_transaction(tx).unwrap(); let ix = Instruction { program_id, @@ -54,11 +58,55 @@ fn test_counter() { svm.latest_blockhash(), ); - let res = svm.send_transaction(tx); - assert!(res.is_ok()); + svm.send_transaction(tx).unwrap(); let counter_account_data = svm.get_account(&counter_account.pubkey()).unwrap().data; let counter_bytes: [u8; 8] = counter_account_data[0..8].try_into().unwrap(); let count = u64::from_le_bytes(counter_bytes); assert_eq!(count, 1); } + +#[test] +fn test_unknown_instruction_fails() { + let program_id = Pubkey::new_unique(); + + let mut svm = LiteSVM::new(); + svm.add_program(program_id, PROGRAM_SO).unwrap(); + + let payer = Keypair::new(); + let counter_account = Keypair::new(); + svm.airdrop(&payer.pubkey(), LAMPORTS_PER_SOL * 10).unwrap(); + + let counter_account_size = std::mem::size_of::(); + let create_ix = create_account( + &payer.pubkey(), + &counter_account.pubkey(), + Rent::default().minimum_balance(counter_account_size), + counter_account_size as u64, + &program_id, + ); + let tx = Transaction::new_signed_with_payer( + &[create_ix], + Some(&payer.pubkey()), + &[&payer, &counter_account], + svm.latest_blockhash(), + ); + svm.send_transaction(tx).unwrap(); + + // Discriminant 9 is not handled and must now return an error rather than Ok(()). + let ix = Instruction { + program_id, + accounts: vec![AccountMeta::new(counter_account.pubkey(), false)], + data: vec![9], + }; + let tx = Transaction::new_signed_with_payer( + &[ix], + Some(&payer.pubkey()), + &[&payer], + svm.latest_blockhash(), + ); + assert!( + svm.send_transaction(tx).is_err(), + "unknown instruction discriminant must fail" + ); +} diff --git a/basics/repository-layout/anchor/programs/carnival/src/error.rs b/basics/repository-layout/anchor/programs/carnival/src/error.rs index 9db2fad45..fc136669d 100644 --- a/basics/repository-layout/anchor/programs/carnival/src/error.rs +++ b/basics/repository-layout/anchor/programs/carnival/src/error.rs @@ -1 +1,17 @@ -// For any custom errors +use anchor_lang::prelude::*; + +// Real failure modes for the carnival examples. Previously this file was empty +// and the rejection branches in the instruction handlers just logged a message +// and returned Ok(()), which made it impossible for callers (or tests) to +// distinguish "the ride happened" from "the ride refused service". +#[error_code] +pub enum CarnivalError { + // The rider/gamer/eater did not bring enough tickets for the requested + // attraction. + #[msg("Not enough tickets for the requested attraction")] + NotEnoughTickets, + + // The rider is below the minimum height required for the ride. + #[msg("Rider is below the minimum height for this ride")] + RiderTooShort, +} diff --git a/basics/repository-layout/anchor/programs/carnival/src/instructions/eat_food.rs b/basics/repository-layout/anchor/programs/carnival/src/instructions/eat_food.rs index 833ca6647..2e3a62a06 100644 --- a/basics/repository-layout/anchor/programs/carnival/src/instructions/eat_food.rs +++ b/basics/repository-layout/anchor/programs/carnival/src/instructions/eat_food.rs @@ -1,6 +1,6 @@ use anchor_lang::prelude::*; -use crate::state::food; +use crate::{error::CarnivalError, state::food}; // Instruction Data @@ -24,10 +24,12 @@ pub fn eat_food(ix: EatFoodInstructionData) -> Result<()> { food_stand.food_type, food_stand.tickets ); - } else { - msg!(" Enjoy your {}!", food_stand.food_type); - }; + // Refuse service rather than logging and returning Ok(()), so + // callers can distinguish a sale from a refusal. + return Err(CarnivalError::NotEnoughTickets.into()); + } + msg!(" Enjoy your {}!", food_stand.food_type); return Ok(()); } } diff --git a/basics/repository-layout/anchor/programs/carnival/src/instructions/get_on_ride.rs b/basics/repository-layout/anchor/programs/carnival/src/instructions/get_on_ride.rs index 751fa6071..7e67d97ee 100644 --- a/basics/repository-layout/anchor/programs/carnival/src/instructions/get_on_ride.rs +++ b/basics/repository-layout/anchor/programs/carnival/src/instructions/get_on_ride.rs @@ -1,6 +1,6 @@ use anchor_lang::prelude::*; -use crate::state::ride; +use crate::{error::CarnivalError, state::ride}; // Instruction Data @@ -18,6 +18,9 @@ pub fn get_on_ride(ix: GetOnRideInstructionData) -> Result<()> { if ix.ride.eq(&ride.name) { msg!("You're about to ride the {}!", ride.name); + // Refuse service: failures used to log + return Ok(()), which made + // them indistinguishable from a successful ride for callers and + // tests. Return a real error instead. if ix.rider_ticket_count < ride.tickets { msg!( " Sorry {}, you need {} tickets to ride the {}!", @@ -25,7 +28,7 @@ pub fn get_on_ride(ix: GetOnRideInstructionData) -> Result<()> { ride.tickets, ride.name ); - return Ok(()); + return Err(CarnivalError::NotEnoughTickets.into()); }; if ix.rider_height < ride.min_height { @@ -35,7 +38,7 @@ pub fn get_on_ride(ix: GetOnRideInstructionData) -> Result<()> { ride.min_height, ride.name ); - return Ok(()); + return Err(CarnivalError::RiderTooShort.into()); }; msg!(" Welcome aboard the {}!", ride.name); diff --git a/basics/repository-layout/anchor/programs/carnival/src/instructions/play_game.rs b/basics/repository-layout/anchor/programs/carnival/src/instructions/play_game.rs index c8785fe52..4867f98e8 100644 --- a/basics/repository-layout/anchor/programs/carnival/src/instructions/play_game.rs +++ b/basics/repository-layout/anchor/programs/carnival/src/instructions/play_game.rs @@ -1,6 +1,6 @@ use anchor_lang::prelude::*; -use crate::state::game; +use crate::{error::CarnivalError, state::game}; // Instruction Data @@ -24,14 +24,17 @@ pub fn play_game(ix: PlayGameInstructionData) -> Result<()> { game.tickets, game.name ); - } else { - msg!(" Let's see what you got!"); - msg!( - " You get {} attempts and the prize is a {}!", - game.tries, - game.prize - ); - }; + // Refuse service rather than logging and returning Ok(()), so + // callers can tell a successful play from a refusal. + return Err(CarnivalError::NotEnoughTickets.into()); + } + + msg!(" Let's see what you got!"); + msg!( + " You get {} attempts and the prize is a {}!", + game.tries, + game.prize + ); return Ok(()); } diff --git a/basics/repository-layout/anchor/programs/carnival/tests/test_carnival.rs b/basics/repository-layout/anchor/programs/carnival/tests/test_carnival.rs index ff2221246..7ec028bcf 100644 --- a/basics/repository-layout/anchor/programs/carnival/tests/test_carnival.rs +++ b/basics/repository-layout/anchor/programs/carnival/tests/test_carnival.rs @@ -14,102 +14,148 @@ fn setup() -> (LiteSVM, solana_keypair::Keypair) { (svm, payer) } -#[test] -fn test_go_on_rides() { - let (mut svm, payer) = setup(); - +fn go_on_ride_ix( + payer: &solana_keypair::Keypair, + name: &str, + height: u32, + ticket_count: u32, + ride_name: &str, +) -> Instruction { let accounts = carnival::accounts::CarnivalContext { payer: payer.pubkey(), } .to_account_metas(None); - - let instructions: Vec = vec![ - ("Jimmy", 36u32, 15u32, "Scrambler"), - ("Mary", 52, 1, "Ferris Wheel"), - ("Alice", 56, 15, "Scrambler"), - ("Bob", 49, 6, "Tilt-a-Whirl"), - ] - .into_iter() - .map(|(name, height, tickets, ride)| { - Instruction::new_with_bytes( - carnival::id(), - &carnival::instruction::GoOnRide { - name: name.to_string(), - height, - ticket_count: tickets, - ride_name: ride.to_string(), - } - .data(), - accounts.clone(), - ) - }) - .collect(); - - send_transaction_from_instructions(&mut svm, instructions, &[&payer], &payer.pubkey()).unwrap(); + Instruction::new_with_bytes( + carnival::id(), + &carnival::instruction::GoOnRide { + name: name.to_string(), + height, + ticket_count, + ride_name: ride_name.to_string(), + } + .data(), + accounts, + ) } -#[test] -fn test_play_games() { - let (mut svm, payer) = setup(); +fn play_game_ix( + payer: &solana_keypair::Keypair, + name: &str, + ticket_count: u32, + game_name: &str, +) -> Instruction { + let accounts = carnival::accounts::CarnivalContext { + payer: payer.pubkey(), + } + .to_account_metas(None); + Instruction::new_with_bytes( + carnival::id(), + &carnival::instruction::PlayGame { + name: name.to_string(), + ticket_count, + game_name: game_name.to_string(), + } + .data(), + accounts, + ) +} +fn eat_food_ix( + payer: &solana_keypair::Keypair, + name: &str, + ticket_count: u32, + food_stand_name: &str, +) -> Instruction { let accounts = carnival::accounts::CarnivalContext { payer: payer.pubkey(), } .to_account_metas(None); + Instruction::new_with_bytes( + carnival::id(), + &carnival::instruction::EatFood { + name: name.to_string(), + ticket_count, + food_stand_name: food_stand_name.to_string(), + } + .data(), + accounts, + ) +} - let instructions: Vec = vec![ - ("Jimmy", 15u32, "I Got It!"), - ("Mary", 1, "Ring Toss"), - ("Alice", 15, "Ladder Climb"), - ("Bob", 6, "Ring Toss"), - ] - .into_iter() - .map(|(name, tickets, game)| { - Instruction::new_with_bytes( - carnival::id(), - &carnival::instruction::PlayGame { - name: name.to_string(), - ticket_count: tickets, - game_name: game.to_string(), - } - .data(), - accounts.clone(), - ) - }) - .collect(); +// Riders that have enough tickets and meet the height requirement succeed. +#[test] +fn test_go_on_ride_succeeds() { + let (mut svm, payer) = setup(); + // Scrambler costs 3 tickets and requires 48" min height; Alice has 15 + // tickets and is 56" tall. + let ix = go_on_ride_ix(&payer, "Alice", 56, 15, "Scrambler"); + send_transaction_from_instructions(&mut svm, vec![ix], &[&payer], &payer.pubkey()).unwrap(); +} - send_transaction_from_instructions(&mut svm, instructions, &[&payer], &payer.pubkey()).unwrap(); +// Insufficient tickets must produce a real error (NotEnoughTickets), not a +// silent Ok(). +#[test] +fn test_go_on_ride_rejects_insufficient_tickets() { + let (mut svm, payer) = setup(); + // Tilt-a-Whirl costs 3 tickets; Bob only has 1. + let ix = go_on_ride_ix(&payer, "Bob", 49, 1, "Tilt-a-Whirl"); + let result = send_transaction_from_instructions(&mut svm, vec![ix], &[&payer], &payer.pubkey()); + assert!(result.is_err(), "ride must reject insufficient tickets"); } +// A rider below the minimum height must be rejected with RiderTooShort. #[test] -fn test_eat_food() { +fn test_go_on_ride_rejects_too_short() { let (mut svm, payer) = setup(); + // Ferris Wheel requires 55" min height; Jimmy is 36" tall but has plenty + // of tickets. + let ix = go_on_ride_ix(&payer, "Jimmy", 36, 15, "Ferris Wheel"); + let result = send_transaction_from_instructions(&mut svm, vec![ix], &[&payer], &payer.pubkey()); + assert!(result.is_err(), "ride must reject riders below min height"); +} - let accounts = carnival::accounts::CarnivalContext { - payer: payer.pubkey(), - } - .to_account_metas(None); +// Unknown attraction name routes to ProgramError::InvalidInstructionData. +#[test] +fn test_go_on_ride_rejects_unknown_ride() { + let (mut svm, payer) = setup(); + let ix = go_on_ride_ix(&payer, "Alice", 60, 100, "Quantum Coaster"); + let result = send_transaction_from_instructions(&mut svm, vec![ix], &[&payer], &payer.pubkey()); + assert!(result.is_err(), "unknown ride must be rejected"); +} - let instructions: Vec = vec![ - ("Jimmy", 15u32, "Taco Shack"), - ("Mary", 1, "Larry's Pizza"), - ("Alice", 15, "Dough Boy's"), - ("Bob", 6, "Dough Boy's"), - ] - .into_iter() - .map(|(name, tickets, food_stand)| { - Instruction::new_with_bytes( - carnival::id(), - &carnival::instruction::EatFood { - name: name.to_string(), - ticket_count: tickets, - food_stand_name: food_stand.to_string(), - } - .data(), - accounts.clone(), - ) - }) - .collect(); +#[test] +fn test_play_game_succeeds() { + let (mut svm, payer) = setup(); + // Ring Toss costs 3 tickets; Alice has 15. + let ix = play_game_ix(&payer, "Alice", 15, "Ring Toss"); + send_transaction_from_instructions(&mut svm, vec![ix], &[&payer], &payer.pubkey()).unwrap(); +} - send_transaction_from_instructions(&mut svm, instructions, &[&payer], &payer.pubkey()).unwrap(); +#[test] +fn test_play_game_rejects_insufficient_tickets() { + let (mut svm, payer) = setup(); + // Ring Toss costs 3 tickets; Mary only has 1. + let ix = play_game_ix(&payer, "Mary", 1, "Ring Toss"); + let result = send_transaction_from_instructions(&mut svm, vec![ix], &[&payer], &payer.pubkey()); + assert!(result.is_err(), "game must reject insufficient tickets"); +} + +#[test] +fn test_eat_food_succeeds() { + let (mut svm, payer) = setup(); + // Dough Boy's costs 1 ticket; Bob has 6. + let ix = eat_food_ix(&payer, "Bob", 6, "Dough Boy's"); + send_transaction_from_instructions(&mut svm, vec![ix], &[&payer], &payer.pubkey()).unwrap(); +} + +#[test] +fn test_eat_food_rejects_insufficient_tickets() { + let (mut svm, payer) = setup(); + // Larry's Pizza costs 3 tickets; Mary has 1. + let ix = eat_food_ix(&payer, "Mary", 1, "Larry's Pizza"); + let result = send_transaction_from_instructions(&mut svm, vec![ix], &[&payer], &payer.pubkey()); + assert!( + result.is_err(), + "food stand must reject insufficient tickets" + ); } diff --git a/compression/cnft-burn/anchor/programs/cnft-burn/src/lib.rs b/compression/cnft-burn/anchor/programs/cnft-burn/src/lib.rs index 4010da16d..28d55dfbc 100644 --- a/compression/cnft-burn/anchor/programs/cnft-burn/src/lib.rs +++ b/compression/cnft-burn/anchor/programs/cnft-burn/src/lib.rs @@ -143,7 +143,11 @@ pub struct BurnCnft<'info> { /// CHECK: This account is neither written to nor read from. pub log_wrapper: UncheckedAccount<'info>, pub compression_program: Program<'info, SPLCompression>, - /// CHECK: This account is neither written to nor read from. + // Pin the bubblegum program account to the known mpl-bubblegum id. Without + // this constraint the caller could pass any account and a malicious one + // could short-circuit the CPI in unexpected ways. + /// CHECK: address constrained to the mpl-bubblegum program id. + #[account(address = MPL_BUBBLEGUM_ID)] pub bubblegum_program: UncheckedAccount<'info>, pub system_program: Program<'info, System>, } diff --git a/compression/cnft-vault/anchor/programs/cnft-vault/src/instructions/withdraw_cnft.rs b/compression/cnft-vault/anchor/programs/cnft-vault/src/instructions/withdraw_cnft.rs index 702daac25..b9d66f687 100644 --- a/compression/cnft-vault/anchor/programs/cnft-vault/src/instructions/withdraw_cnft.rs +++ b/compression/cnft-vault/anchor/programs/cnft-vault/src/instructions/withdraw_cnft.rs @@ -4,7 +4,7 @@ use anchor_lang::solana_program::{ program::invoke_signed, }; -use crate::{build_transfer_instruction, TransferArgs, SPLCompression}; +use crate::{build_transfer_instruction, TransferArgs, SPLCompression, MPL_BUBBLEGUM_ID}; #[derive(Accounts)] pub struct Withdraw<'info> { @@ -30,7 +30,10 @@ pub struct Withdraw<'info> { /// CHECK: This account is neither written to nor read from. pub log_wrapper: UncheckedAccount<'info>, pub compression_program: Program<'info, SPLCompression>, - /// CHECK: This account is neither written to nor read from. + // Pin the bubblegum program account to the known mpl-bubblegum id. Without + // this constraint the caller could pass any account to the CPI. + /// CHECK: address constrained to the mpl-bubblegum program id. + #[account(address = MPL_BUBBLEGUM_ID)] pub bubblegum_program: UncheckedAccount<'info>, pub system_program: Program<'info, System>, } diff --git a/compression/cnft-vault/anchor/programs/cnft-vault/src/instructions/withdraw_two_cnfts.rs b/compression/cnft-vault/anchor/programs/cnft-vault/src/instructions/withdraw_two_cnfts.rs index 7df355da8..84c1d7952 100644 --- a/compression/cnft-vault/anchor/programs/cnft-vault/src/instructions/withdraw_two_cnfts.rs +++ b/compression/cnft-vault/anchor/programs/cnft-vault/src/instructions/withdraw_two_cnfts.rs @@ -4,7 +4,7 @@ use anchor_lang::solana_program::{ program::invoke_signed, }; -use crate::{build_transfer_instruction, TransferArgs, SPLCompression}; +use crate::{build_transfer_instruction, TransferArgs, SPLCompression, MPL_BUBBLEGUM_ID}; #[derive(Accounts)] pub struct WithdrawTwo<'info> { @@ -45,7 +45,10 @@ pub struct WithdrawTwo<'info> { /// CHECK: This account is neither written to nor read from. pub log_wrapper: UncheckedAccount<'info>, pub compression_program: Program<'info, SPLCompression>, - /// CHECK: This account is neither written to nor read from. + // Pin the bubblegum program account to the known mpl-bubblegum id. Without + // this constraint the caller could pass any account to the two CPI calls. + /// CHECK: address constrained to the mpl-bubblegum program id. + #[account(address = MPL_BUBBLEGUM_ID)] pub bubblegum_program: UncheckedAccount<'info>, pub system_program: Program<'info, System>, } diff --git a/tokens/escrow/anchor/programs/escrow/src/instructions/cancel_offer.rs b/tokens/escrow/anchor/programs/escrow/src/instructions/cancel_offer.rs new file mode 100644 index 000000000..85f1e2b66 --- /dev/null +++ b/tokens/escrow/anchor/programs/escrow/src/instructions/cancel_offer.rs @@ -0,0 +1,101 @@ +use anchor_lang::prelude::*; + +use anchor_spl::{ + associated_token::AssociatedToken, + token_interface::{ + close_account, transfer_checked, CloseAccount, Mint, TokenAccount, TokenInterface, + TransferChecked, + }, +}; + +use crate::Offer; + +// Cancel an outstanding offer. Without this handler, an abandoned offer would +// keep the maker's token-A locked in the vault forever (and the offer +// account's rent unclaimed). The maker signs, the vault tokens flow back to +// the maker, and both the vault and the offer accounts are closed. +#[derive(Accounts)] +pub struct CancelOffer<'info> { + #[account(mut)] + pub maker: Signer<'info>, + + pub token_mint_a: InterfaceAccount<'info, Mint>, + + #[account( + mut, + associated_token::mint = token_mint_a, + associated_token::authority = maker, + associated_token::token_program = token_program, + )] + pub maker_token_account_a: InterfaceAccount<'info, TokenAccount>, + + #[account( + mut, + close = maker, + has_one = maker, + has_one = token_mint_a, + seeds = [b"offer", maker.key().as_ref(), offer.id.to_le_bytes().as_ref()], + bump = offer.bump, + )] + pub offer: Account<'info, Offer>, + + #[account( + mut, + associated_token::mint = token_mint_a, + associated_token::authority = offer, + associated_token::token_program = token_program, + )] + pub vault: InterfaceAccount<'info, TokenAccount>, + + pub associated_token_program: Program<'info, AssociatedToken>, + pub token_program: Interface<'info, TokenInterface>, + pub system_program: Program<'info, System>, +} + +pub fn handle_cancel_offer(context: Context) -> Result<()> { + let maker_key = context.accounts.maker.key(); + let id_bytes = context.accounts.offer.id.to_le_bytes(); + let seeds = &[ + b"offer".as_ref(), + maker_key.as_ref(), + id_bytes.as_ref(), + &[context.accounts.offer.bump], + ]; + let signer_seeds = [&seeds[..]]; + + // Move all tokens back from the vault to the maker. + let vault_amount = context.accounts.vault.amount; + let transfer_accounts = TransferChecked { + from: context.accounts.vault.to_account_info(), + mint: context.accounts.token_mint_a.to_account_info(), + to: context.accounts.maker_token_account_a.to_account_info(), + authority: context.accounts.offer.to_account_info(), + }; + let cpi_context = CpiContext::new_with_signer( + context.accounts.token_program.key(), + transfer_accounts, + &signer_seeds, + ); + transfer_checked( + cpi_context, + vault_amount, + context.accounts.token_mint_a.decimals, + )?; + + // Close the vault, sending its rent lamports back to the maker. + let close_accounts = CloseAccount { + account: context.accounts.vault.to_account_info(), + destination: context.accounts.maker.to_account_info(), + authority: context.accounts.offer.to_account_info(), + }; + let cpi_context = CpiContext::new_with_signer( + context.accounts.token_program.key(), + close_accounts, + &signer_seeds, + ); + close_account(cpi_context)?; + + // The offer account itself is closed by the `close = maker` constraint + // above, which refunds its rent to the maker. + Ok(()) +} diff --git a/tokens/escrow/anchor/programs/escrow/src/instructions/make_offer.rs b/tokens/escrow/anchor/programs/escrow/src/instructions/make_offer.rs index 653eb40d7..43f06a847 100644 --- a/tokens/escrow/anchor/programs/escrow/src/instructions/make_offer.rs +++ b/tokens/escrow/anchor/programs/escrow/src/instructions/make_offer.rs @@ -30,6 +30,19 @@ pub struct MakeOffer<'info> { )] pub maker_token_account_a: InterfaceAccount<'info, TokenAccount>, + // The maker's token-B ATA used to be init_if_needed on the taker side, which + // meant the taker paid the maker's rent. Initialize it here (paid by the + // maker) so the rent burden lives with the party who chose to open the + // offer. + #[account( + init_if_needed, + payer = maker, + associated_token::mint = token_mint_b, + associated_token::authority = maker, + associated_token::token_program = token_program + )] + pub maker_token_account_b: InterfaceAccount<'info, TokenAccount>, + #[account( init, payer = maker, diff --git a/tokens/escrow/anchor/programs/escrow/src/instructions/mod.rs b/tokens/escrow/anchor/programs/escrow/src/instructions/mod.rs index cf1dd986b..d0e3d27d6 100644 --- a/tokens/escrow/anchor/programs/escrow/src/instructions/mod.rs +++ b/tokens/escrow/anchor/programs/escrow/src/instructions/mod.rs @@ -4,5 +4,8 @@ pub use make_offer::*; pub mod take_offer; pub use take_offer::*; +pub mod cancel_offer; +pub use cancel_offer::*; + pub mod shared; pub use shared::*; diff --git a/tokens/escrow/anchor/programs/escrow/src/instructions/take_offer.rs b/tokens/escrow/anchor/programs/escrow/src/instructions/take_offer.rs index ae7f1262b..11acc6871 100644 --- a/tokens/escrow/anchor/programs/escrow/src/instructions/take_offer.rs +++ b/tokens/escrow/anchor/programs/escrow/src/instructions/take_offer.rs @@ -41,9 +41,11 @@ pub struct TakeOffer<'info> { )] pub taker_token_account_b: Box>, + // The maker's token-B ATA is initialized in make_offer (paid by the maker), + // so the taker no longer pays its rent. Treat it as a plain existing account + // here. #[account( - init_if_needed, - payer = taker, + mut, associated_token::mint = token_mint_b, associated_token::authority = maker, associated_token::token_program = token_program, diff --git a/tokens/escrow/anchor/programs/escrow/src/lib.rs b/tokens/escrow/anchor/programs/escrow/src/lib.rs index d65cb38a2..1635afeae 100644 --- a/tokens/escrow/anchor/programs/escrow/src/lib.rs +++ b/tokens/escrow/anchor/programs/escrow/src/lib.rs @@ -27,4 +27,12 @@ pub mod escrow { instructions::take_offer::handle_send_wanted_tokens_to_maker(&context)?; instructions::take_offer::handle_withdraw_and_close_vault(context) } + + // Cancel an outstanding offer. The maker signs, the vault tokens flow back + // to the maker, and both the vault and offer accounts are closed (rent + // refunded to the maker). Without this, abandoned offers would lock funds + // forever. + pub fn cancel_offer(context: Context) -> Result<()> { + instructions::cancel_offer::handle_cancel_offer(context) + } } diff --git a/tokens/escrow/anchor/programs/escrow/tests/test_escrow.rs b/tokens/escrow/anchor/programs/escrow/tests/test_escrow.rs index 3d97ccbac..05f83ea7d 100644 --- a/tokens/escrow/anchor/programs/escrow/tests/test_escrow.rs +++ b/tokens/escrow/anchor/programs/escrow/tests/test_escrow.rs @@ -135,6 +135,7 @@ fn test_make_offer() { token_mint_a: es.mint_a, token_mint_b: es.mint_b, maker_token_account_a: es.alice_ata_a, + maker_token_account_b: es.alice_ata_b, offer: offer_pda, vault, associated_token_program: ata_program_id(), @@ -201,6 +202,7 @@ fn test_take_offer() { token_mint_a: es.mint_a, token_mint_b: es.mint_b, maker_token_account_a: es.alice_ata_a, + maker_token_account_b: es.alice_ata_b, offer: offer_pda, vault, associated_token_program: ata_program_id(), @@ -277,3 +279,180 @@ fn test_take_offer() { "Offer should be closed after take_offer" ); } + +#[test] +fn test_cancel_offer() { + let mut es = full_setup(); + + let offer_id: u64 = 3; + let token_a_offered_amount: u64 = 500_000; + let token_b_wanted_amount: u64 = 1_000_000; + + let (offer_pda, _bump) = Pubkey::find_program_address( + &[ + b"offer", + es.alice.pubkey().as_ref(), + &offer_id.to_le_bytes(), + ], + &es.program_id, + ); + let vault = derive_ata(&offer_pda, &es.mint_a); + + // Snapshot Alice's token-A balance before the offer. + let alice_a_before = get_token_account_balance(&es.svm, &es.alice_ata_a).unwrap(); + + // Alice makes the offer. + let make_offer_ix = Instruction::new_with_bytes( + es.program_id, + &escrow::instruction::MakeOffer { + id: offer_id, + token_a_offered_amount, + token_b_wanted_amount, + } + .data(), + escrow::accounts::MakeOffer { + maker: es.alice.pubkey(), + token_mint_a: es.mint_a, + token_mint_b: es.mint_b, + maker_token_account_a: es.alice_ata_a, + maker_token_account_b: es.alice_ata_b, + offer: offer_pda, + vault, + associated_token_program: ata_program_id(), + token_program: token_program_id(), + system_program: system_program::id(), + } + .to_account_metas(None), + ); + send_transaction_from_instructions( + &mut es.svm, + vec![make_offer_ix], + &[&es.payer, &es.alice], + &es.payer.pubkey(), + ) + .unwrap(); + + assert_eq!( + get_token_account_balance(&es.svm, &vault).unwrap(), + token_a_offered_amount + ); + + // Alice cancels the offer. + let cancel_offer_ix = Instruction::new_with_bytes( + es.program_id, + &escrow::instruction::CancelOffer {}.data(), + escrow::accounts::CancelOffer { + maker: es.alice.pubkey(), + token_mint_a: es.mint_a, + maker_token_account_a: es.alice_ata_a, + offer: offer_pda, + vault, + associated_token_program: ata_program_id(), + token_program: token_program_id(), + system_program: system_program::id(), + } + .to_account_metas(None), + ); + send_transaction_from_instructions( + &mut es.svm, + vec![cancel_offer_ix], + &[&es.payer, &es.alice], + &es.payer.pubkey(), + ) + .unwrap(); + + // The offer and vault accounts should be closed. + assert!( + es.svm.get_account(&offer_pda).is_none(), + "Offer should be closed after cancel" + ); + assert!( + es.svm.get_account(&vault).is_none(), + "Vault should be closed after cancel" + ); + + // Alice should have her token-A back to its pre-make balance. + let alice_a_after = get_token_account_balance(&es.svm, &es.alice_ata_a).unwrap(); + assert_eq!(alice_a_after, alice_a_before); +} + +#[test] +fn test_cancel_offer_rejects_non_maker() { + let mut es = full_setup(); + + let offer_id: u64 = 4; + let token_a_offered_amount: u64 = 500_000; + let token_b_wanted_amount: u64 = 1_000_000; + + let (offer_pda, _bump) = Pubkey::find_program_address( + &[ + b"offer", + es.alice.pubkey().as_ref(), + &offer_id.to_le_bytes(), + ], + &es.program_id, + ); + let vault = derive_ata(&offer_pda, &es.mint_a); + + // Alice makes the offer. + let make_offer_ix = Instruction::new_with_bytes( + es.program_id, + &escrow::instruction::MakeOffer { + id: offer_id, + token_a_offered_amount, + token_b_wanted_amount, + } + .data(), + escrow::accounts::MakeOffer { + maker: es.alice.pubkey(), + token_mint_a: es.mint_a, + token_mint_b: es.mint_b, + maker_token_account_a: es.alice_ata_a, + maker_token_account_b: es.alice_ata_b, + offer: offer_pda, + vault, + associated_token_program: ata_program_id(), + token_program: token_program_id(), + system_program: system_program::id(), + } + .to_account_metas(None), + ); + send_transaction_from_instructions( + &mut es.svm, + vec![make_offer_ix], + &[&es.payer, &es.alice], + &es.payer.pubkey(), + ) + .unwrap(); + + // Bob tries to cancel Alice's offer - the has_one = maker / signer + seeds + // constraints should reject this. + let bob_ata_a = + create_associated_token_account(&mut es.svm, &es.bob.pubkey(), &es.mint_a, &es.payer) + .unwrap(); + let cancel_offer_ix = Instruction::new_with_bytes( + es.program_id, + &escrow::instruction::CancelOffer {}.data(), + escrow::accounts::CancelOffer { + maker: es.bob.pubkey(), + token_mint_a: es.mint_a, + maker_token_account_a: bob_ata_a, + offer: offer_pda, + vault, + associated_token_program: ata_program_id(), + token_program: token_program_id(), + system_program: system_program::id(), + } + .to_account_metas(None), + ); + let result = send_transaction_from_instructions( + &mut es.svm, + vec![cancel_offer_ix], + &[&es.payer, &es.bob], + &es.payer.pubkey(), + ); + assert!( + result.is_err(), + "Bob must not be able to cancel Alice's offer" + ); +} diff --git a/tokens/token-extensions/default-account-state/native/program/src/lib.rs b/tokens/token-extensions/default-account-state/native/program/src/lib.rs index c43a1e3b7..d0c65e0fa 100644 --- a/tokens/token-extensions/default-account-state/native/program/src/lib.rs +++ b/tokens/token-extensions/default-account-state/native/program/src/lib.rs @@ -107,19 +107,25 @@ fn process_instruction( ], )?; - // Update the Default Account State to Initialized + // Update the Default Account State to Initialized. + // + // The freeze authority set in initialize_mint above is `mint_authority`, so + // the update_default_account_state CPI must be signed by mint_authority - + // not by payer. The previous version passed payer.key here, which only + // worked when the caller happened to use the same account for both, and + // silently rejected any caller who didn't. invoke( &update_default_account_state( token_program.key, mint_account.key, - payer.key, - &[payer.key], + mint_authority.key, + &[mint_authority.key], &AccountState::Initialized, ) .unwrap(), &[ mint_account.clone(), - payer.clone(), + mint_authority.clone(), token_program.clone(), system_program.clone(), ], diff --git a/tokens/token-extensions/nft-meta-data-pointer/anchor-example/anchor/programs/extension_nft/src/lib.rs b/tokens/token-extensions/nft-meta-data-pointer/anchor-example/anchor/programs/extension_nft/src/lib.rs index 8073a6a29..6515214fe 100644 --- a/tokens/token-extensions/nft-meta-data-pointer/anchor-example/anchor/programs/extension_nft/src/lib.rs +++ b/tokens/token-extensions/nft-meta-data-pointer/anchor-example/anchor/programs/extension_nft/src/lib.rs @@ -7,6 +7,13 @@ pub mod instructions; pub mod state; use instructions::*; +// WARNING: This example depends on the `session-keys` crate, which has not +// been independently audited. It is included here purely for educational +// purposes - demonstrating how a game might let a player sign with a +// short-lived session token instead of their main wallet. Do not ship this +// crate (or this program) to mainnet in its current form: review the upstream +// `session-keys` source, get an audit, and harden the session-token issuance +// and expiry handling first. declare_id!("9aZZ7TJ2fQZxY8hMtWXywp5y6BgqC4N2BPcr9FDT47sW"); #[program] diff --git a/tokens/token-swap/anchor/programs/token-swap/src/errors.rs b/tokens/token-swap/anchor/programs/token-swap/src/errors.rs index 39b5457d7..c821b0195 100644 --- a/tokens/token-swap/anchor/programs/token-swap/src/errors.rs +++ b/tokens/token-swap/anchor/programs/token-swap/src/errors.rs @@ -5,9 +5,6 @@ pub enum TutorialError { #[msg("Invalid fee value")] InvalidFee, - #[msg("Invalid mint for the pool")] - InvalidMint, - #[msg("Depositing too little liquidity")] DepositTooSmall, @@ -16,4 +13,11 @@ pub enum TutorialError { #[msg("Invariant does not hold")] InvariantViolated, + + // Returned when a caller asks to deposit or swap more tokens than they hold. + // Previously the program silently clamped to the available balance, which broke + // slippage protection for callers (they expected their input to be the actual + // amount used). We now fail fast so callers can react. + #[msg("Requested amount exceeds available balance")] + InsufficientBalance, } diff --git a/tokens/token-swap/anchor/programs/token-swap/src/instructions/deposit_liquidity.rs b/tokens/token-swap/anchor/programs/token-swap/src/instructions/deposit_liquidity.rs index 0e1e3045f..d0d48c33e 100644 --- a/tokens/token-swap/anchor/programs/token-swap/src/instructions/deposit_liquidity.rs +++ b/tokens/token-swap/anchor/programs/token-swap/src/instructions/deposit_liquidity.rs @@ -1,7 +1,7 @@ use anchor_lang::prelude::*; use anchor_spl::{ associated_token::AssociatedToken, - token::{self, Mint, MintTo, Token, TokenAccount, Transfer}, + token::{self, Mint, MintTo, Token, TokenAccount, TransferChecked}, }; use fixed::types::I64F64; @@ -16,17 +16,17 @@ pub fn handle_deposit_liquidity( amount_a: u64, amount_b: u64, ) -> Result<()> { - // Prevent depositing assets the depositor does not own - let mut amount_a = if amount_a > context.accounts.depositor_account_a.amount { - context.accounts.depositor_account_a.amount - } else { - amount_a - }; - let mut amount_b = if amount_b > context.accounts.depositor_account_b.amount { - context.accounts.depositor_account_b.amount - } else { - amount_b - }; + // Fail fast if the depositor lacks the requested balance. Previously this + // silently clamped to the available balance, which broke slippage protection + // for callers building on top - they expected their input amount to be the + // amount actually deposited. + if amount_a > context.accounts.depositor_account_a.amount + || amount_b > context.accounts.depositor_account_b.amount + { + return err!(TutorialError::InsufficientBalance); + } + let mut amount_a = amount_a; + let mut amount_b = amount_b; // Making sure they are provided in the same proportion as existing liquidity let pool_a = &context.accounts.pool_account_a; @@ -75,28 +75,34 @@ pub fn handle_deposit_liquidity( liquidity -= MINIMUM_LIQUIDITY; } - // Transfer tokens to the pool - token::transfer( + // Transfer tokens to the pool using transfer_checked. transfer_checked + // includes the mint and decimals in the CPI, which guards callers against + // decimal-mismatch bugs (and is the modern recommended path). + token::transfer_checked( CpiContext::new( context.accounts.token_program.key(), - Transfer { + TransferChecked { from: context.accounts.depositor_account_a.to_account_info(), + mint: context.accounts.mint_a.to_account_info(), to: context.accounts.pool_account_a.to_account_info(), authority: context.accounts.depositor.to_account_info(), }, ), amount_a, + context.accounts.mint_a.decimals, )?; - token::transfer( + token::transfer_checked( CpiContext::new( context.accounts.token_program.key(), - Transfer { + TransferChecked { from: context.accounts.depositor_account_b.to_account_info(), + mint: context.accounts.mint_b.to_account_info(), to: context.accounts.pool_account_b.to_account_info(), authority: context.accounts.depositor.to_account_info(), }, ), amount_b, + context.accounts.mint_b.decimals, )?; // Mint the liquidity to user diff --git a/tokens/token-swap/anchor/programs/token-swap/src/instructions/swap_exact_tokens_for_tokens.rs b/tokens/token-swap/anchor/programs/token-swap/src/instructions/swap_exact_tokens_for_tokens.rs index 40ced6c68..c210bc46e 100644 --- a/tokens/token-swap/anchor/programs/token-swap/src/instructions/swap_exact_tokens_for_tokens.rs +++ b/tokens/token-swap/anchor/programs/token-swap/src/instructions/swap_exact_tokens_for_tokens.rs @@ -1,7 +1,7 @@ use anchor_lang::prelude::*; use anchor_spl::{ associated_token::AssociatedToken, - token::{self, Mint, Token, TokenAccount, Transfer}, + token::{self, Mint, Token, TokenAccount, TransferChecked}, }; use fixed::types::I64F64; @@ -17,14 +17,18 @@ pub fn handle_swap_exact_tokens_for_tokens( input_amount: u64, min_output_amount: u64, ) -> Result<()> { - // Prevent depositing assets the depositor does not own - let input = if swap_a && input_amount > context.accounts.trader_account_a.amount { - context.accounts.trader_account_a.amount - } else if !swap_a && input_amount > context.accounts.trader_account_b.amount { - context.accounts.trader_account_b.amount - } else { - input_amount - }; + // Fail fast if the trader lacks the requested input balance. Previously this + // silently clamped to the available balance, which broke slippage protection + // for callers - their min_output_amount is computed against the requested + // input, not the clamped one, so the trade could succeed with worse terms + // than expected. + if swap_a && input_amount > context.accounts.trader_account_a.amount { + return err!(TutorialError::InsufficientBalance); + } + if !swap_a && input_amount > context.accounts.trader_account_b.amount { + return err!(TutorialError::InsufficientBalance); + } + let input = input_amount; // Apply trading fee, used to compute the output let amm = &context.accounts.amm; @@ -72,53 +76,64 @@ pub fn handle_swap_exact_tokens_for_tokens( &[authority_bump], ]; let signer_seeds = &[&authority_seeds[..]]; + // Use transfer_checked so the mint + decimals are verified at the token + // program. This protects callers from decimal-mismatch bugs and is the + // modern recommended path. if swap_a { - token::transfer( + token::transfer_checked( CpiContext::new( context.accounts.token_program.key(), - Transfer { + TransferChecked { from: context.accounts.trader_account_a.to_account_info(), + mint: context.accounts.mint_a.to_account_info(), to: context.accounts.pool_account_a.to_account_info(), authority: context.accounts.trader.to_account_info(), }, ), input, + context.accounts.mint_a.decimals, )?; - token::transfer( + token::transfer_checked( CpiContext::new_with_signer( context.accounts.token_program.key(), - Transfer { + TransferChecked { from: context.accounts.pool_account_b.to_account_info(), + mint: context.accounts.mint_b.to_account_info(), to: context.accounts.trader_account_b.to_account_info(), authority: context.accounts.pool_authority.to_account_info(), }, signer_seeds, ), output, + context.accounts.mint_b.decimals, )?; } else { - token::transfer( + token::transfer_checked( CpiContext::new_with_signer( context.accounts.token_program.key(), - Transfer { + TransferChecked { from: context.accounts.pool_account_a.to_account_info(), + mint: context.accounts.mint_a.to_account_info(), to: context.accounts.trader_account_a.to_account_info(), authority: context.accounts.pool_authority.to_account_info(), }, signer_seeds, ), input, + context.accounts.mint_a.decimals, )?; - token::transfer( + token::transfer_checked( CpiContext::new( context.accounts.token_program.key(), - Transfer { + TransferChecked { from: context.accounts.trader_account_b.to_account_info(), + mint: context.accounts.mint_b.to_account_info(), to: context.accounts.pool_account_b.to_account_info(), authority: context.accounts.trader.to_account_info(), }, ), output, + context.accounts.mint_b.decimals, )?; } diff --git a/tokens/token-swap/anchor/programs/token-swap/src/instructions/withdraw_liquidity.rs b/tokens/token-swap/anchor/programs/token-swap/src/instructions/withdraw_liquidity.rs index 0ae4e3b11..51d5edff7 100644 --- a/tokens/token-swap/anchor/programs/token-swap/src/instructions/withdraw_liquidity.rs +++ b/tokens/token-swap/anchor/programs/token-swap/src/instructions/withdraw_liquidity.rs @@ -1,7 +1,7 @@ use anchor_lang::prelude::*; use anchor_spl::{ associated_token::AssociatedToken, - token::{self, Burn, Mint, Token, TokenAccount, Transfer}, + token::{self, Burn, Mint, Token, TokenAccount, TransferChecked}, }; use fixed::types::I64F64; @@ -31,17 +31,20 @@ pub fn handle_withdraw_liquidity(context: Context, amount: u6 .unwrap() .floor() .to_num::(); - token::transfer( + // transfer_checked verifies the mint + decimals at the token program. + token::transfer_checked( CpiContext::new_with_signer( context.accounts.token_program.key(), - Transfer { + TransferChecked { from: context.accounts.pool_account_a.to_account_info(), + mint: context.accounts.mint_a.to_account_info(), to: context.accounts.depositor_account_a.to_account_info(), authority: context.accounts.pool_authority.to_account_info(), }, signer_seeds, ), amount_a, + context.accounts.mint_a.decimals, )?; let amount_b = I64F64::from_num(amount) @@ -53,17 +56,19 @@ pub fn handle_withdraw_liquidity(context: Context, amount: u6 .unwrap() .floor() .to_num::(); - token::transfer( + token::transfer_checked( CpiContext::new_with_signer( context.accounts.token_program.key(), - Transfer { + TransferChecked { from: context.accounts.pool_account_b.to_account_info(), + mint: context.accounts.mint_b.to_account_info(), to: context.accounts.depositor_account_b.to_account_info(), authority: context.accounts.pool_authority.to_account_info(), }, signer_seeds, ), amount_b, + context.accounts.mint_b.decimals, )?; // Burn the liquidity tokens