diff --git a/CHANGELOG.md b/CHANGELOG.md index 03b3f077..d6a730d4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,21 @@ All notable changes to this repository are documented here. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). +## [2026-10-04] - Escrow: the taker signs the terms + +### Fixed + +- `finance/escrow` (Anchor v2, Anchor v1, Quasar, native) let a maker switch + an offer under a taker. The offer's address is its maker and `id`, so the + maker could cancel and re-make the same `id` at worse terms while a + taker's `take_offer` was in flight, and the transaction would trade at the + new terms. `take_offer` now takes `minimum_token_a_out` and + `maximum_token_b_in`, and refuses the take with the new `OfferTermsChanged` + error before any token moves if the vault holds less token A or the offer + wants more token B. Tested by `test_take_offer_rejects_switched_offer` and + `test_take_offer_rejects_switched_offer_wanting_more_token_b` in each copy; + the Kani model gains `proof_take_offer_honors_taker_terms`. + ## [2026-10-03] - Managed Fund rejects wide-confidence prices ### Fixed diff --git a/finance/escrow/anchor-v1/CHANGELOG.md b/finance/escrow/anchor-v1/CHANGELOG.md index 555cf023..611b4fc2 100644 --- a/finance/escrow/anchor-v1/CHANGELOG.md +++ b/finance/escrow/anchor-v1/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +## 2026-10-04 + +- `take_offer` takes two arguments the taker signs, `minimum_token_a_out` and `maximum_token_b_in`, and refuses the take with `OfferTermsChanged` before any token moves if the vault holds less token A or the offer wants more token B than those bounds. An offer's address is its maker and `id`, so a maker could cancel an offer and re-make the same `id` at worse terms while a taker's transaction was in flight, and the transaction would trade at the new terms. `test_take_offer_rejects_switched_offer` and `test_take_offer_rejects_switched_offer_wanting_more_token_b` run that switch. + ## 2026-09-29 - `make_offer` refuses an offer with zero tokens on either side, with `ZeroAmount`. An offer of a token for a different amount of itself was already refused before the handler runs, because the maker's token-A and token-B accounts would be the same account and Anchor refuses the same mutable account twice (`ConstraintDuplicateMutableAccount`); a test now pins that. diff --git a/finance/escrow/anchor-v1/README.md b/finance/escrow/anchor-v1/README.md index feffa996..2b9435eb 100644 --- a/finance/escrow/anchor-v1/README.md +++ b/finance/escrow/anchor-v1/README.md @@ -23,7 +23,9 @@ The maker pays the rent for the offer account and the vault, and every path that A maker opens an offer with `make_offer`, passing the `id`, `token_a_offered_amount`, and `token_b_wanted_amount`. The maker signs and pays all rent. The handler creates the offer PDA and the vault, creates the maker's token-B associated token account if needed (paid by the maker, so the eventual taker never funds a maker-owned account), moves the offered token A into the vault with `transfer_checked`, and records the offer state. It refuses an offer with zero tokens on either side (`ZeroAmount`). An offer of a token for itself never reaches the handler: the maker's token-A and token-B accounts would be the same account, which Anchor refuses (`ConstraintDuplicateMutableAccount`). -A taker settles the offer with `take_offer`. The taker signs. Anchor's constraints bind every account to the stored offer state (`has_one` on the maker and both mints, associated-token constraints on the vault and all token accounts, and the PDA seeds on the offer itself). The handler sends the wanted token B from the taker to the maker, releases the vault's token A to the taker signed by the offer PDA, and closes both the vault and the offer account back to the maker, who paid their rent. The taker's own token-A account is created on the fly if needed, paid by the taker. +A taker settles the offer with `take_offer`, passing `minimum_token_a_out` (the least token A the taker accepts from the vault) and `maximum_token_b_in` (the most token B the taker will pay). The taker signs, so the bounds are the terms the taker agreed to. Anchor's constraints bind every account to the stored offer state (`has_one` on the maker and both mints, associated-token constraints on the vault and all token accounts, and the PDA seeds on the offer itself). The handler sends the wanted token B from the taker to the maker, releases the vault's token A to the taker signed by the offer PDA, and closes both the vault and the offer account back to the maker, who paid their rent. The taker's own token-A account is created on the fly if needed, paid by the taker. + +The two bounds close a bait and switch. An offer's address comes from its maker and `id`, so while a taker's transaction is in flight the maker could cancel the offer and make it again under the same `id` at worse terms, and the transaction would land on the new offer at the same address. Before any token moves, `take_offer` refuses the take with `OfferTermsChanged` if the vault holds less token A than `minimum_token_a_out` or the offer wants more token B than `maximum_token_b_in`. On the ordinary path a client passes the terms it read from the offer: the vault's balance and the `token_b_wanted_amount`. A maker abandons an offer with `cancel_offer`. Only the maker can call it; without it, an unwanted offer would lock the maker's tokens in the vault forever. The handler returns the vault's token A to the maker and closes the vault and offer accounts, refunding both rents to the maker. @@ -45,7 +47,7 @@ The tests are Rust integration tests running against [LiteSVM](https://www.ancho cargo test ``` -(`anchor test` runs the same command, per `Anchor.toml`.) The tests cover the make/take flow, the make/cancel flow, rejection of a non-maker cancel, rejection of offers with zero tokens on either side or the same token on both, token balances on every leg, and the rent refunds (the maker's lamports recover the offer and vault rent after both take and cancel). +(`anchor test` runs the same command, per `Anchor.toml`.) The tests cover the make/take flow, the make/cancel flow, rejection of a non-maker cancel, rejection of a take that lands on an offer the maker cancelled and re-made at worse terms (`test_take_offer_rejects_switched_offer` for less token A, `test_take_offer_rejects_switched_offer_wanting_more_token_b` for more token B), rejection of offers with zero tokens on either side or the same token on both, token balances on every leg, and the rent refunds (the maker's lamports recover the offer and vault rent after both take and cancel). ## FAQ diff --git a/finance/escrow/anchor-v1/programs/escrow/src/error.rs b/finance/escrow/anchor-v1/programs/escrow/src/error.rs index a8c4644c..34d8a612 100644 --- a/finance/escrow/anchor-v1/programs/escrow/src/error.rs +++ b/finance/escrow/anchor-v1/programs/escrow/src/error.rs @@ -4,4 +4,6 @@ use anchor_lang::prelude::*; pub enum EscrowError { #[msg("An offer must offer and want more than zero tokens")] ZeroAmount, + #[msg("The offer pays less token A, or wants more token B, than the taker agreed to")] + OfferTermsChanged, } diff --git a/finance/escrow/anchor-v1/programs/escrow/src/instructions/take_offer.rs b/finance/escrow/anchor-v1/programs/escrow/src/instructions/take_offer.rs index 20739860..1f9b393e 100644 --- a/finance/escrow/anchor-v1/programs/escrow/src/instructions/take_offer.rs +++ b/finance/escrow/anchor-v1/programs/escrow/src/instructions/take_offer.rs @@ -5,7 +5,7 @@ use anchor_spl::{ token_interface::{Mint, TokenAccount, TokenInterface}, }; -use crate::Offer; +use crate::{error::EscrowError, Offer}; use super::{close_token_account, transfer_tokens}; @@ -71,6 +71,22 @@ pub struct TakeOfferAccountConstraints<'info> { pub system_program: Program<'info, System>, } +// Refuse the take unless the offer still holds the terms the taker signed +// for: at least `minimum_token_a_out` of token A in the vault, and no more +// than `maximum_token_b_in` of token B wanted. Runs before any transfer. +pub fn handle_check_offer_terms( + context: &Context, + minimum_token_a_out: u64, + maximum_token_b_in: u64, +) -> Result<()> { + require!( + context.accounts.vault.amount >= minimum_token_a_out + && context.accounts.offer.token_b_wanted_amount <= maximum_token_b_in, + EscrowError::OfferTermsChanged + ); + Ok(()) +} + pub fn handle_send_wanted_tokens_to_maker( context: &Context, ) -> Result<()> { diff --git a/finance/escrow/anchor-v1/programs/escrow/src/lib.rs b/finance/escrow/anchor-v1/programs/escrow/src/lib.rs index bbe6076b..225ae74b 100644 --- a/finance/escrow/anchor-v1/programs/escrow/src/lib.rs +++ b/finance/escrow/anchor-v1/programs/escrow/src/lib.rs @@ -30,7 +30,20 @@ pub mod escrow { instructions::make_offer::handle_save_offer(context, id, token_b_wanted_amount) } - pub fn take_offer(context: Context) -> Result<()> { + // The taker signs the terms they agreed to. An offer's address is its + // maker and id, so a maker can cancel and re-make the same id at worse + // terms while the taker's transaction is in flight; these bounds make that + // transaction fail instead of trading at the new terms. + pub fn take_offer( + context: Context, + minimum_token_a_out: u64, + maximum_token_b_in: u64, + ) -> Result<()> { + instructions::take_offer::handle_check_offer_terms( + &context, + minimum_token_a_out, + maximum_token_b_in, + )?; instructions::take_offer::handle_send_wanted_tokens_to_maker(&context)?; instructions::take_offer::handle_withdraw_and_close_vault(context) } diff --git a/finance/escrow/anchor-v1/programs/escrow/tests/test_escrow.rs b/finance/escrow/anchor-v1/programs/escrow/tests/test_escrow.rs index 3e9c22e8..f6cc380b 100644 --- a/finance/escrow/anchor-v1/programs/escrow/tests/test_escrow.rs +++ b/finance/escrow/anchor-v1/programs/escrow/tests/test_escrow.rs @@ -241,7 +241,11 @@ fn test_take_offer() { // Step 2: Bob takes the offer let take_offer_ix = Instruction::new_with_bytes( es.program_id, - &escrow::instruction::TakeOffer {}.data(), + &escrow::instruction::TakeOffer { + minimum_token_a_out: token_a_offered_amount, + maximum_token_b_in: token_b_wanted_amount, + } + .data(), escrow::accounts::TakeOfferAccountConstraints { taker: es.bob.pubkey(), maker: es.alice.pubkey(), @@ -606,3 +610,155 @@ fn test_make_offer_rejects_same_mint() { alice_balance_before ); } + +// Bob's `take_offer` for Alice's offer `offer_id`, signed for the given terms. +fn take_offer_instruction( + es: &EscrowSetup, + offer_id: u64, + minimum_token_a_out: u64, + maximum_token_b_in: u64, +) -> Instruction { + 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); + Instruction::new_with_bytes( + es.program_id, + &escrow::instruction::TakeOffer { + minimum_token_a_out, + maximum_token_b_in, + } + .data(), + escrow::accounts::TakeOfferAccountConstraints { + taker: es.bob.pubkey(), + maker: es.alice.pubkey(), + token_mint_a: es.mint_a, + token_mint_b: es.mint_b, + taker_token_account_a: es.bob_ata_a, + taker_token_account_b: es.bob_ata_b, + 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), + ) +} + +// Alice's `cancel_offer` for her offer `offer_id`, sent and unwrapped. +fn cancel_offer(es: &mut EscrowSetup, offer_id: u64) { + 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); + let cancel_offer_ix = Instruction::new_with_bytes( + es.program_id, + &escrow::instruction::CancelOffer {}.data(), + escrow::accounts::CancelOfferAccountConstraints { + 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 bait and switch: Alice makes an offer, Bob signs a `take_offer` for its +// terms, and before Bob's transaction lands Alice cancels and re-makes the +// same id at the switched terms. The offer is at the same address, so Bob's +// transaction reaches the new offer; it must fail with `OfferTermsChanged` +// and leave Bob's tokens where they were. +fn assert_switched_offer_refused(switched_a_offered: u64, switched_b_wanted: u64) { + let mut es = full_setup(); + let (mint_b, alice_ata_b) = (es.mint_b, es.alice_ata_b); + let offer_id: u64 = 8; + let token_a_offered_amount: u64 = 1_000_000; + let token_b_wanted_amount: u64 = 1_000_000; + + try_make_offer( + &mut es, + offer_id, + token_a_offered_amount, + token_b_wanted_amount, + mint_b, + alice_ata_b, + ) + .unwrap(); + + // Bob signs for the terms he saw. + let take_offer_ix = + take_offer_instruction(&es, offer_id, token_a_offered_amount, token_b_wanted_amount); + + // Alice switches the offer before Bob's transaction lands. + cancel_offer(&mut es, offer_id); + try_make_offer( + &mut es, + offer_id, + switched_a_offered, + switched_b_wanted, + mint_b, + alice_ata_b, + ) + .unwrap(); + + let bob_b_before = get_token_account_balance(&es.svm, &es.bob_ata_b).unwrap(); + let alice_b_before = get_token_account_balance(&es.svm, &es.alice_ata_b).unwrap(); + + let result = send_transaction_from_instructions( + &mut es.svm, + vec![take_offer_ix], + &[&es.payer, &es.bob], + &es.payer.pubkey(), + ); + + assert_fails_with(result, escrow::error::EscrowError::OfferTermsChanged); + assert_eq!( + get_token_account_balance(&es.svm, &es.bob_ata_b).unwrap(), + bob_b_before, + "the taker must not pay token B for a switched offer" + ); + assert!( + es.svm.get_account(&es.bob_ata_a).is_none(), + "the taker must receive no token A from a switched offer" + ); + assert_eq!( + get_token_account_balance(&es.svm, &es.alice_ata_b).unwrap(), + alice_b_before + ); +} + +#[test] +fn test_take_offer_rejects_switched_offer() { + // Alice re-makes the offer putting a thousandth of the token A in the vault. + assert_switched_offer_refused(1_000, 1_000_000); +} + +#[test] +fn test_take_offer_rejects_switched_offer_wanting_more_token_b() { + // Alice re-makes the offer asking for twice the token B. + assert_switched_offer_refused(1_000_000, 2_000_000); +} diff --git a/finance/escrow/anchor/CHANGELOG.md b/finance/escrow/anchor/CHANGELOG.md index 555cf023..611b4fc2 100644 --- a/finance/escrow/anchor/CHANGELOG.md +++ b/finance/escrow/anchor/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +## 2026-10-04 + +- `take_offer` takes two arguments the taker signs, `minimum_token_a_out` and `maximum_token_b_in`, and refuses the take with `OfferTermsChanged` before any token moves if the vault holds less token A or the offer wants more token B than those bounds. An offer's address is its maker and `id`, so a maker could cancel an offer and re-make the same `id` at worse terms while a taker's transaction was in flight, and the transaction would trade at the new terms. `test_take_offer_rejects_switched_offer` and `test_take_offer_rejects_switched_offer_wanting_more_token_b` run that switch. + ## 2026-09-29 - `make_offer` refuses an offer with zero tokens on either side, with `ZeroAmount`. An offer of a token for a different amount of itself was already refused before the handler runs, because the maker's token-A and token-B accounts would be the same account and Anchor refuses the same mutable account twice (`ConstraintDuplicateMutableAccount`); a test now pins that. diff --git a/finance/escrow/anchor/README.md b/finance/escrow/anchor/README.md index 186845c4..08e23933 100644 --- a/finance/escrow/anchor/README.md +++ b/finance/escrow/anchor/README.md @@ -23,7 +23,9 @@ The maker pays the rent for the offer account and the vault, and every path that A maker opens an offer with `make_offer`, passing the `id`, `token_a_offered_amount`, and `token_b_wanted_amount`. The maker signs and pays all rent. The handler creates the offer PDA and the vault, creates the maker's token-B associated token account if needed (paid by the maker, so the eventual taker never funds a maker-owned account), moves the offered token A into the vault with `transfer_checked`, and records the offer state. It refuses an offer with zero tokens on either side (`ZeroAmount`). An offer of a token for itself never reaches the handler: the maker's token-A and token-B accounts would be the same account, which Anchor refuses (`ConstraintDuplicateMutableAccount`). -A taker settles the offer with `take_offer`. The taker signs. Anchor's constraints bind every account to the stored offer state (`address = offer.maker` on the maker and `address = offer.token_mint_a` / `address = offer.token_mint_b` on the mints, associated-token constraints on the vault and all token accounts, and the PDA seeds on the offer itself). The handler sends the wanted token B from the taker to the maker, releases the vault's token A to the taker signed by the offer PDA, and closes both the vault and the offer account back to the maker, who paid their rent. The taker's own token-A account is created on the fly if needed, paid by the taker. +A taker settles the offer with `take_offer`, passing `minimum_token_a_out` (the least token A the taker accepts from the vault) and `maximum_token_b_in` (the most token B the taker will pay). The taker signs, so the bounds are the terms the taker agreed to. Anchor's constraints bind every account to the stored offer state (`address = offer.maker` on the maker and `address = offer.token_mint_a` / `address = offer.token_mint_b` on the mints, associated-token constraints on the vault and all token accounts, and the PDA seeds on the offer itself). The handler sends the wanted token B from the taker to the maker, releases the vault's token A to the taker signed by the offer PDA, and closes both the vault and the offer account back to the maker, who paid their rent. The taker's own token-A account is created on the fly if needed, paid by the taker. + +The two bounds close a bait and switch. An offer's address comes from its maker and `id`, so while a taker's transaction is in flight the maker could cancel the offer and make it again under the same `id` at worse terms, and the transaction would land on the new offer at the same address. Before any token moves, `take_offer` refuses the take with `OfferTermsChanged` if the vault holds less token A than `minimum_token_a_out` or the offer wants more token B than `maximum_token_b_in`. On the ordinary path a client passes the terms it read from the offer: the vault's balance and the `token_b_wanted_amount`. A maker abandons an offer with `cancel_offer`. Only the maker can call it; without it, an unwanted offer would lock the maker's tokens in the vault forever. The handler returns the vault's token A to the maker and closes the vault and offer accounts, refunding both rents to the maker. @@ -45,7 +47,7 @@ The tests are Rust integration tests running against [LiteSVM](https://www.ancho cargo test ``` -(`anchor test` runs the same command, per `Anchor.toml`.) The tests cover the make/take flow, the make/cancel flow, rejection of a non-maker cancel, rejection of offers with zero tokens on either side or the same token on both, token balances on every leg, and the rent refunds (the maker's lamports recover the offer and vault rent after both take and cancel). +(`anchor test` runs the same command, per `Anchor.toml`.) The tests cover the make/take flow, the make/cancel flow, rejection of a non-maker cancel, rejection of a take that lands on an offer the maker cancelled and re-made at worse terms (`test_take_offer_rejects_switched_offer` for less token A, `test_take_offer_rejects_switched_offer_wanting_more_token_b` for more token B), rejection of offers with zero tokens on either side or the same token on both, token balances on every leg, and the rent refunds (the maker's lamports recover the offer and vault rent after both take and cancel). ## FAQ diff --git a/finance/escrow/anchor/programs/escrow/src/error.rs b/finance/escrow/anchor/programs/escrow/src/error.rs index a8c4644c..34d8a612 100644 --- a/finance/escrow/anchor/programs/escrow/src/error.rs +++ b/finance/escrow/anchor/programs/escrow/src/error.rs @@ -4,4 +4,6 @@ use anchor_lang::prelude::*; pub enum EscrowError { #[msg("An offer must offer and want more than zero tokens")] ZeroAmount, + #[msg("The offer pays less token A, or wants more token B, than the taker agreed to")] + OfferTermsChanged, } diff --git a/finance/escrow/anchor/programs/escrow/src/instructions/take_offer.rs b/finance/escrow/anchor/programs/escrow/src/instructions/take_offer.rs index f1d6f58a..3a800600 100644 --- a/finance/escrow/anchor/programs/escrow/src/instructions/take_offer.rs +++ b/finance/escrow/anchor/programs/escrow/src/instructions/take_offer.rs @@ -5,7 +5,7 @@ use anchor_spl::{ token_interface::{Mint, TokenAccount, TokenInterface}, }; -use crate::Offer; +use crate::{error::EscrowError, Offer}; use super::{close_token_account, transfer_tokens}; @@ -70,6 +70,22 @@ pub struct TakeOfferAccountConstraints { pub system_program: Program, } +// Refuse the take unless the offer still holds the terms the taker signed +// for: at least `minimum_token_a_out` of token A in the vault, and no more +// than `maximum_token_b_in` of token B wanted. Runs before any transfer. +pub fn handle_check_offer_terms( + context: &mut Context, + minimum_token_a_out: u64, + maximum_token_b_in: u64, +) -> Result<()> { + require!( + context.accounts.vault.amount() >= minimum_token_a_out + && context.accounts.offer.token_b_wanted_amount <= maximum_token_b_in, + EscrowError::OfferTermsChanged + ); + Ok(()) +} + pub fn handle_send_wanted_tokens_to_maker( context: &mut Context, ) -> Result<()> { diff --git a/finance/escrow/anchor/programs/escrow/src/lib.rs b/finance/escrow/anchor/programs/escrow/src/lib.rs index 9244aa4e..b8eb47ad 100644 --- a/finance/escrow/anchor/programs/escrow/src/lib.rs +++ b/finance/escrow/anchor/programs/escrow/src/lib.rs @@ -30,7 +30,20 @@ pub mod escrow { instructions::make_offer::handle_save_offer(context, id, token_b_wanted_amount) } - pub fn take_offer(context: &mut Context) -> Result<()> { + // The taker signs the terms they agreed to. An offer's address is its + // maker and id, so a maker can cancel and re-make the same id at worse + // terms while the taker's transaction is in flight; these bounds make that + // transaction fail instead of trading at the new terms. + pub fn take_offer( + context: &mut Context, + minimum_token_a_out: u64, + maximum_token_b_in: u64, + ) -> Result<()> { + instructions::take_offer::handle_check_offer_terms( + context, + minimum_token_a_out, + maximum_token_b_in, + )?; instructions::take_offer::handle_send_wanted_tokens_to_maker(context)?; instructions::take_offer::handle_withdraw_and_close_vault(context) } diff --git a/finance/escrow/anchor/programs/escrow/tests/test_escrow.rs b/finance/escrow/anchor/programs/escrow/tests/test_escrow.rs index c9de14b2..f785a03e 100644 --- a/finance/escrow/anchor/programs/escrow/tests/test_escrow.rs +++ b/finance/escrow/anchor/programs/escrow/tests/test_escrow.rs @@ -239,7 +239,11 @@ fn test_take_offer() { // Step 2: Bob takes the offer let take_offer_ix = Instruction::new_with_bytes( es.program_id, - &escrow::instruction::TakeOffer {}.data(), + &escrow::instruction::TakeOffer { + minimum_token_a_out: token_a_offered_amount, + maximum_token_b_in: token_b_wanted_amount, + } + .data(), escrow::accounts::TakeOfferAccountConstraints { taker: es.bob.pubkey(), maker: es.alice.pubkey(), @@ -604,3 +608,155 @@ fn test_make_offer_rejects_same_mint() { alice_balance_before ); } + +// Bob's `take_offer` for Alice's offer `offer_id`, signed for the given terms. +fn take_offer_instruction( + es: &EscrowSetup, + offer_id: u64, + minimum_token_a_out: u64, + maximum_token_b_in: u64, +) -> Instruction { + let (offer_pda, _bump) = Address::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); + Instruction::new_with_bytes( + es.program_id, + &escrow::instruction::TakeOffer { + minimum_token_a_out, + maximum_token_b_in, + } + .data(), + escrow::accounts::TakeOfferAccountConstraints { + taker: es.bob.pubkey(), + maker: es.alice.pubkey(), + token_mint_a: es.mint_a, + token_mint_b: es.mint_b, + taker_token_account_a: es.bob_ata_a, + taker_token_account_b: es.bob_ata_b, + 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), + ) +} + +// Alice's `cancel_offer` for her offer `offer_id`, sent and unwrapped. +fn cancel_offer(es: &mut EscrowSetup, offer_id: u64) { + let (offer_pda, _bump) = Address::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); + let cancel_offer_ix = Instruction::new_with_bytes( + es.program_id, + &escrow::instruction::CancelOffer {}.data(), + escrow::accounts::CancelOfferAccountConstraints { + 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 bait and switch: Alice makes an offer, Bob signs a `take_offer` for its +// terms, and before Bob's transaction lands Alice cancels and re-makes the +// same id at the switched terms. The offer is at the same address, so Bob's +// transaction reaches the new offer; it must fail with `OfferTermsChanged` +// and leave Bob's tokens where they were. +fn assert_switched_offer_refused(switched_a_offered: u64, switched_b_wanted: u64) { + let mut es = full_setup(); + let (mint_b, alice_ata_b) = (es.mint_b, es.alice_ata_b); + let offer_id: u64 = 8; + let token_a_offered_amount: u64 = 1_000_000; + let token_b_wanted_amount: u64 = 1_000_000; + + try_make_offer( + &mut es, + offer_id, + token_a_offered_amount, + token_b_wanted_amount, + mint_b, + alice_ata_b, + ) + .unwrap(); + + // Bob signs for the terms he saw. + let take_offer_ix = + take_offer_instruction(&es, offer_id, token_a_offered_amount, token_b_wanted_amount); + + // Alice switches the offer before Bob's transaction lands. + cancel_offer(&mut es, offer_id); + try_make_offer( + &mut es, + offer_id, + switched_a_offered, + switched_b_wanted, + mint_b, + alice_ata_b, + ) + .unwrap(); + + let bob_b_before = get_token_account_balance(&es.svm, &es.bob_ata_b).unwrap(); + let alice_b_before = get_token_account_balance(&es.svm, &es.alice_ata_b).unwrap(); + + let result = send_transaction_from_instructions( + &mut es.svm, + vec![take_offer_ix], + &[&es.payer, &es.bob], + &es.payer.pubkey(), + ); + + assert_fails_with(result, escrow::error::EscrowError::OfferTermsChanged); + assert_eq!( + get_token_account_balance(&es.svm, &es.bob_ata_b).unwrap(), + bob_b_before, + "the taker must not pay token B for a switched offer" + ); + assert!( + es.svm.get_account(&es.bob_ata_a).is_none(), + "the taker must receive no token A from a switched offer" + ); + assert_eq!( + get_token_account_balance(&es.svm, &es.alice_ata_b).unwrap(), + alice_b_before + ); +} + +#[test] +fn test_take_offer_rejects_switched_offer() { + // Alice re-makes the offer putting a thousandth of the token A in the vault. + assert_switched_offer_refused(1_000, 1_000_000); +} + +#[test] +fn test_take_offer_rejects_switched_offer_wanting_more_token_b() { + // Alice re-makes the offer asking for twice the token B. + assert_switched_offer_refused(1_000_000, 2_000_000); +} diff --git a/finance/escrow/kani-proofs/README.md b/finance/escrow/kani-proofs/README.md index 8bf2ffdc..58081db0 100644 --- a/finance/escrow/kani-proofs/README.md +++ b/finance/escrow/kani-proofs/README.md @@ -23,6 +23,7 @@ the invariants the program relies on: - `proof_close_offer_conserves_on_success`: Closing the offer account conserves lamports and empties the source. - `proof_close_offer_conserves_lamports_unconditionally`: **Finding (now fixed)**: lamport conservation holds with equality on every path (see below). - `proof_take_offer_conserves_value`: A take conserves total mint A and total mint B, drains the vault, and pays the maker exactly the price. +- `proof_take_offer_honors_taker_terms`: A take either fails with `OfferTermsChanged` having moved nothing, or pays the taker at least `minimum_token_a_out` of mint A for at most `maximum_token_b_in` of mint B. - `proof_take_offer_guard_never_overflows`: The `checked_add` conservation guards in `take_offer` are unreachable dead code. - `proof_take_offer_guard_dead_under_spl_invariant`: Same, shown explicitly under the SPL supply invariant. - `proof_cancel_offer_returns_all_to_maker`: Cancelling returns every vault token to the maker and conserves mint A. diff --git a/finance/escrow/kani-proofs/src/lib.rs b/finance/escrow/kani-proofs/src/lib.rs index e00dd605..fe4d236a 100644 --- a/finance/escrow/kani-proofs/src/lib.rs +++ b/finance/escrow/kani-proofs/src/lib.rs @@ -182,7 +182,9 @@ fn proof_close_offer_conserves_lamports_unconditionally() { // take_offer: the atomic swap // --------------------------------------------------------------------------- // -// take_offer performs two transfers: +// take_offer first refuses the take (OfferTermsChanged) unless the vault holds +// at least the taker's `minimum_token_a_out` and the offer wants no more than +// the taker's `maximum_token_b_in`. It then performs two transfers: // (B) taker -> maker of `token_b_wanted_amount` of mint B // (A) vault -> taker of the vault's entire mint A balance // then re-reads balances and asserts (with checked_add) that each receiver @@ -199,6 +201,9 @@ pub struct TakeBalances { #[derive(Debug, PartialEq, Eq)] pub enum TakeError { + /// The offer pays less token A, or wants more token B, than the taker + /// signed for (`EscrowError::OfferTermsChanged`). + OfferTermsChanged, Token(TokenError), /// The post-transfer conservation check itself overflowed /// (`EscrowError::ArithmeticOverflow`). @@ -207,11 +212,21 @@ pub enum TakeError { /// Faithful model of the token movement + conservation checks in /// `take_offer::process`. -pub fn take_offer(b: &mut TakeBalances, wanted_b: u64) -> Result<(), TakeError> { +pub fn take_offer( + b: &mut TakeBalances, + wanted_b: u64, + minimum_token_a_out: u64, + maximum_token_b_in: u64, +) -> Result<(), TakeError> { let taker_a_before = b.taker_a; let maker_b_before = b.maker_b; let vault_a = b.vault_a; + // The taker's terms are checked before any token moves. + if vault_a < minimum_token_a_out || wanted_b > maximum_token_b_in { + return Err(TakeError::OfferTermsChanged); + } + // (B) taker pays the maker the wanted mint-B amount. token_transfer(&mut b.taker_b, &mut b.maker_b, wanted_b).map_err(TakeError::Token)?; // (A) vault releases all of its mint A to the taker. @@ -241,6 +256,8 @@ fn proof_take_offer_conserves_value() { let maker_b: u64 = kani::any(); let vault_a: u64 = kani::any(); let wanted_b: u64 = kani::any(); + let minimum_token_a_out: u64 = kani::any(); + let maximum_token_b_in: u64 = kani::any(); let mut b = TakeBalances { taker_a, @@ -251,7 +268,7 @@ fn proof_take_offer_conserves_value() { let total_a_before = taker_a as u128 + vault_a as u128; let total_b_before = taker_b as u128 + maker_b as u128; - if take_offer(&mut b, wanted_b).is_ok() { + if take_offer(&mut b, wanted_b, minimum_token_a_out, maximum_token_b_in).is_ok() { // Conservation across both mints: nothing minted, nothing burned. assert_eq!(b.taker_a as u128 + b.vault_a as u128, total_a_before); assert_eq!(b.taker_b as u128 + b.maker_b as u128, total_b_before); @@ -264,6 +281,43 @@ fn proof_take_offer_conserves_value() { } } +/// The taker's signed terms bind: a take either fails without moving a token, +/// or the taker receives at least `minimum_token_a_out` of mint A and pays at +/// most `maximum_token_b_in` of mint B. A maker who re-makes the offer at +/// worse terms after the taker signed cannot make the take succeed. +#[cfg(kani)] +#[kani::proof] +fn proof_take_offer_honors_taker_terms() { + let taker_a: u64 = kani::any(); + let taker_b: u64 = kani::any(); + let maker_b: u64 = kani::any(); + let vault_a: u64 = kani::any(); + let wanted_b: u64 = kani::any(); + let minimum_token_a_out: u64 = kani::any(); + let maximum_token_b_in: u64 = kani::any(); + + let mut b = TakeBalances { + taker_a, + taker_b, + maker_b, + vault_a, + }; + + match take_offer(&mut b, wanted_b, minimum_token_a_out, maximum_token_b_in) { + Ok(()) => { + assert!(b.taker_a - taker_a >= minimum_token_a_out); + assert!(taker_b - b.taker_b <= maximum_token_b_in); + } + Err(TakeError::OfferTermsChanged) => { + assert_eq!(b.taker_a, taker_a); + assert_eq!(b.taker_b, taker_b); + assert_eq!(b.maker_b, maker_b); + assert_eq!(b.vault_a, vault_a); + } + Err(_) => {} + } +} + /// FINDING (this harness PASSES, and that is the finding): the on-chain /// `checked_add` conservation guards in `take_offer` are unreachable defensive /// code. Their `ArithmeticOverflow` arm can never be taken, because the @@ -282,6 +336,8 @@ fn proof_take_offer_guard_never_overflows() { let maker_b: u64 = kani::any(); let vault_a: u64 = kani::any(); let wanted_b: u64 = kani::any(); + let minimum_token_a_out: u64 = kani::any(); + let maximum_token_b_in: u64 = kani::any(); let mut b = TakeBalances { taker_a, @@ -290,7 +346,7 @@ fn proof_take_offer_guard_never_overflows() { vault_a, }; assert_ne!( - take_offer(&mut b, wanted_b), + take_offer(&mut b, wanted_b, minimum_token_a_out, maximum_token_b_in), Err(TakeError::ConservationOverflow) ); } @@ -308,6 +364,8 @@ fn proof_take_offer_guard_dead_under_spl_invariant() { let maker_b: u64 = kani::any(); let vault_a: u64 = kani::any(); let wanted_b: u64 = kani::any(); + let minimum_token_a_out: u64 = kani::any(); + let maximum_token_b_in: u64 = kani::any(); // SPL token invariant: a successful transfer means the receiver's resulting // balance fit in u64. That is precisely `before + amount <= u64::MAX`. @@ -321,7 +379,7 @@ fn proof_take_offer_guard_dead_under_spl_invariant() { vault_a, }; assert_ne!( - take_offer(&mut b, wanted_b), + take_offer(&mut b, wanted_b, minimum_token_a_out, maximum_token_b_in), Err(TakeError::ConservationOverflow) ); } @@ -471,13 +529,36 @@ mod tests { maker_b: 0, vault_a: 10, }; - take_offer(&mut b, 7).unwrap(); + take_offer(&mut b, 7, 10, 7).unwrap(); assert_eq!(b.vault_a, 0); assert_eq!(b.taker_a, 10); assert_eq!(b.maker_b, 7); assert_eq!(b.taker_b, 43); } + #[test] + fn take_offer_refuses_switched_terms() { + let before = TakeBalances { + taker_a: 0, + taker_b: 50, + maker_b: 0, + vault_a: 10, + }; + // Less token A in the vault than the taker signed for. + let mut b = before; + assert_eq!( + take_offer(&mut b, 7, 11, 7), + Err(TakeError::OfferTermsChanged) + ); + // More token B wanted than the taker signed for. + let mut b = before; + assert_eq!( + take_offer(&mut b, 8, 10, 7), + Err(TakeError::OfferTermsChanged) + ); + assert_eq!((b.taker_a, b.taker_b, b.maker_b, b.vault_a), (0, 50, 0, 10)); + } + #[test] fn close_conserves() { let mut offer = 900u64; diff --git a/finance/escrow/native/CHANGELOG.md b/finance/escrow/native/CHANGELOG.md index d8d5a985..2ce77cd4 100644 --- a/finance/escrow/native/CHANGELOG.md +++ b/finance/escrow/native/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +## 2026-10-04 + +- The `TakeOffer` instruction carries two fields the taker signs, `minimum_token_a_out` and `maximum_token_b_in`, and the handler refuses the take with `OfferTermsChanged` (appended to `EscrowError`, so existing codes keep their numbers) before any token moves if the vault holds less token A or the offer wants more token B than those bounds. An offer's address is its maker and `id`, so a maker could cancel an offer and re-make the same `id` at worse terms while a taker's transaction was in flight, and the transaction would trade at the new terms. `test_take_offer_rejects_switched_offer` and `test_take_offer_rejects_switched_offer_wanting_more_token_b` run that switch. + ## 2026-09-29 - `make_offer` refuses an offer with zero tokens on either side (`ZeroAmount`) and an offer of a token for a different amount of itself (`SameMint`). diff --git a/finance/escrow/native/README.md b/finance/escrow/native/README.md index 981f0a98..5b87169c 100644 --- a/finance/escrow/native/README.md +++ b/finance/escrow/native/README.md @@ -17,11 +17,13 @@ The maker pays the rent for the offer account and the vault (and for their own m A maker opens an offer with the `MakeOffer` instruction, passing the offer `id`, the amount of token A offered, and the amount of token B wanted. The maker signs and pays all rent. The handler derives and creates the offer PDA, creates the vault, creates the maker's mint B token account if needed (so the eventual taker never pays rent for a maker-owned account), and moves the offered token A into the vault with `transfer_checked`. It then verifies the vault holds exactly the offered amount before writing the offer state. -A taker settles the offer with the `TakeOffer` instruction. The taker signs. The handler validates every passed account against the stored offer state (maker, both mints, the vault address, and the offer PDA itself), requires the maker's mint B token account to already exist, and lazily creates the taker's mint A token account (rent paid by the taker, since it is the taker's own account). It then transfers the wanted token B from the taker to the maker, releases the vault's token A to the taker signed by the offer PDA, and verifies conservation with checked arithmetic: the taker gained exactly the vault balance and the maker gained exactly the wanted amount. Finally it closes the vault and the offer account, refunding both rents to the maker. +A taker settles the offer with the `TakeOffer` instruction, passing `minimum_token_a_out` (the least token A the taker accepts from the vault) and `maximum_token_b_in` (the most token B the taker will pay). The taker signs, so the bounds are the terms the taker agreed to. The handler validates every passed account against the stored offer state (maker, both mints, the vault address, and the offer PDA itself), requires the maker's mint B token account to already exist, and lazily creates the taker's mint A token account (rent paid by the taker, since it is the taker's own account). It then transfers the wanted token B from the taker to the maker, releases the vault's token A to the taker signed by the offer PDA, and verifies conservation with checked arithmetic: the taker gained exactly the vault balance and the maker gained exactly the wanted amount. Finally it closes the vault and the offer account, refunding both rents to the maker. + +The two bounds close a bait and switch. An offer's address comes from its maker and `id`, so while a taker's transaction is in flight the maker could cancel the offer and make it again under the same `id` at worse terms, and the transaction would land on the new offer at the same address. Before any token moves, the handler refuses the take with `OfferTermsChanged` if the vault holds less token A than `minimum_token_a_out` or the offer wants more token B than `maximum_token_b_in`. On the ordinary path a client passes the terms it read from the offer: the vault's balance and the `token_b_wanted_amount`. A maker abandons an offer with the `CancelOffer` instruction. Only the maker can call it; without it, an unwanted offer would lock the maker's tokens in the vault forever. The handler validates the accounts against the offer state, returns the vault's token A to the maker's token account, verifies the maker received exactly the vault balance, and closes the vault and offer accounts back to the maker. -Errors are reported through the named `EscrowError` enum in `program/src/error.rs` (key mismatches, missing maker token account, conservation violations, arithmetic overflow). +Errors are reported through the named `EscrowError` enum in `program/src/error.rs` (key mismatches, missing maker token account, conservation violations, arithmetic overflow, and `OfferTermsChanged` for a take whose terms no longer hold). ## Setup @@ -43,4 +45,4 @@ The Rust + [LiteSVM](https://www.anchor-lang.com/docs/testing/litesvm) tests loa cargo test --manifest-path=./program/Cargo.toml ``` -The tests cover the make/take flow, the make/cancel flow, rejection of a non-maker cancel, rejection of offers with zero tokens on either side or the same token on both, token balances on every leg, and the rent refunds (the maker's lamports recover the offer and vault rent after both take and cancel). +The tests cover the make/take flow, the make/cancel flow, rejection of a non-maker cancel, rejection of a take that lands on an offer the maker cancelled and re-made at worse terms (`test_take_offer_rejects_switched_offer` for less token A, `test_take_offer_rejects_switched_offer_wanting_more_token_b` for more token B), rejection of offers with zero tokens on either side or the same token on both, token balances on every leg, and the rent refunds (the maker's lamports recover the offer and vault rent after both take and cancel). diff --git a/finance/escrow/native/program/src/error.rs b/finance/escrow/native/program/src/error.rs index 9ee00b51..560cde16 100644 --- a/finance/escrow/native/program/src/error.rs +++ b/finance/escrow/native/program/src/error.rs @@ -29,6 +29,9 @@ pub enum EscrowError { #[error("An offer must exchange two different tokens")] SameMint, + + #[error("The offer pays less token A, or wants more token B, than the taker agreed to")] + OfferTermsChanged, } impl From for ProgramError { diff --git a/finance/escrow/native/program/src/instructions/take_offer.rs b/finance/escrow/native/program/src/instructions/take_offer.rs index fe42d101..c4a22874 100644 --- a/finance/escrow/native/program/src/instructions/take_offer.rs +++ b/finance/escrow/native/program/src/instructions/take_offer.rs @@ -16,11 +16,24 @@ use { }, }; +/// The terms the taker signs for. An offer's address is its maker and id, so +/// a maker can cancel and re-make the same id at worse terms while the +/// taker's transaction is in flight; these bounds make that transaction fail +/// instead of trading at the new terms. #[derive(BorshDeserialize, BorshSerialize, Debug)] -pub struct TakeOffer {} +pub struct TakeOffer { + /// The least token A the taker accepts from the vault. + pub minimum_token_a_out: u64, + /// The most token B the taker will pay. + pub maximum_token_b_in: u64, +} impl TakeOffer { - pub fn process(program_id: &Pubkey, accounts: &[AccountInfo<'_>]) -> ProgramResult { + pub fn process( + program_id: &Pubkey, + accounts: &[AccountInfo<'_>], + args: TakeOffer, + ) -> ProgramResult { let [offer_info, token_mint_a, token_mint_b, maker_token_account_b, taker_token_account_a, taker_token_account_b, vault, maker, taker, token_program, associated_token_program, system_program] = accounts else { @@ -65,6 +78,17 @@ impl TakeOffer { assert_is_associated_token_account(taker_token_account_a.key, taker.key, token_mint_a.key)?; assert_is_associated_token_account(vault.key, offer_info.key, token_mint_a.key)?; + // Refuse the take unless the offer still holds the terms the taker + // signed for: at least `minimum_token_a_out` of token A in the vault, + // and no more than `maximum_token_b_in` of token B wanted. This runs + // before any account is created or any token moves. + let vault_amount_a = TokenAccount::unpack(&vault.data.borrow())?.amount; + if vault_amount_a < args.minimum_token_a_out + || offer.token_b_wanted_amount > args.maximum_token_b_in + { + return Err(EscrowError::OfferTermsChanged.into()); + } + // Create the taker's token A account if needed. The taker pays this // rent: it is the taker's own account. if taker_token_account_a.lamports() == 0 { @@ -94,7 +118,6 @@ impl TakeOffer { return Err(EscrowError::MakerTokenAccountBNotInitialized.into()); } - let vault_amount_a = TokenAccount::unpack(&vault.data.borrow())?.amount; let taker_amount_a_before_transfer = TokenAccount::unpack(&taker_token_account_a.data.borrow())?.amount; let maker_amount_b_before_transfer = diff --git a/finance/escrow/native/program/src/lib.rs b/finance/escrow/native/program/src/lib.rs index ecfa1e64..3157aaf8 100644 --- a/finance/escrow/native/program/src/lib.rs +++ b/finance/escrow/native/program/src/lib.rs @@ -22,7 +22,7 @@ fn process_instruction( match instruction { EscrowInstruction::MakeOffer(data) => MakeOffer::process(program_id, accounts, data), - EscrowInstruction::TakeOffer => TakeOffer::process(program_id, accounts), + EscrowInstruction::TakeOffer(data) => TakeOffer::process(program_id, accounts, data), EscrowInstruction::CancelOffer => CancelOffer::process(program_id, accounts), } } @@ -30,6 +30,6 @@ fn process_instruction( #[derive(BorshSerialize, BorshDeserialize, Debug)] enum EscrowInstruction { MakeOffer(MakeOffer), - TakeOffer, + TakeOffer(TakeOffer), CancelOffer, } diff --git a/finance/escrow/native/program/tests/test.rs b/finance/escrow/native/program/tests/test.rs index c06c2180..a8c40a65 100644 --- a/finance/escrow/native/program/tests/test.rs +++ b/finance/escrow/native/program/tests/test.rs @@ -210,6 +210,20 @@ fn make_offer_instruction_for( } fn take_offer_instruction(es: &EscrowSetup) -> Instruction { + take_offer_instruction_for(es, AMOUNT_A, AMOUNT_B) +} + +/// A `take_offer` instruction signed for the given terms: the least token A +/// the taker accepts and the most token B the taker pays. +fn take_offer_instruction_for( + es: &EscrowSetup, + minimum_token_a_out: u64, + maximum_token_b_in: u64, +) -> Instruction { + let mut take_data = vec![TAKE_OFFER]; + take_data.extend_from_slice(&minimum_token_a_out.to_le_bytes()); + take_data.extend_from_slice(&maximum_token_b_in.to_le_bytes()); + Instruction { program_id: es.program_id, accounts: vec![ @@ -226,7 +240,7 @@ fn take_offer_instruction(es: &EscrowSetup) -> Instruction { AccountMeta::new_readonly(spl_associated_token_account_interface::program::id(), false), AccountMeta::new_readonly(solana_system_interface::program::ID, false), ], - data: vec![TAKE_OFFER], + data: take_data, } } @@ -375,6 +389,7 @@ fn test_cancel_offer_rejects_non_maker() { // `InstructionError::Custom`. const ZERO_AMOUNT: u32 = 7; const SAME_MINT: u32 = 8; +const OFFER_TERMS_CHANGED: u32 = 9; /// Send a `make_offer` the program should refuse, and check it failed with /// `expected_code`, left the maker's tokens where they were, and created no @@ -425,3 +440,69 @@ fn test_make_offer_rejects_same_mint() { ); assert_make_offer_refused(&mut es, instruction, SAME_MINT); } + +/// The bait and switch: the maker makes an offer, the taker signs a +/// `take_offer` for its terms, and before the taker's transaction lands the +/// maker cancels and re-makes the same id at the switched terms. The offer is +/// at the same address, so the taker's transaction reaches the new offer; it +/// must fail with `OfferTermsChanged` and leave the taker's tokens where they +/// were. +fn assert_switched_offer_refused(switched_a_offered: u64, switched_b_wanted: u64) { + let mut es = setup(); + let payer = es.payer.insecure_clone(); + let maker = es.maker.insecure_clone(); + let taker = es.taker.insecure_clone(); + + let make_ix = make_offer_instruction(&es); + send(&mut es.svm, &payer, &[make_ix], &[&maker]); + + // The taker signs for the terms they saw. + let take_ix = take_offer_instruction_for(&es, AMOUNT_A, AMOUNT_B); + + // The maker switches the offer before the taker's transaction lands. + let cancel_ix = cancel_offer_instruction(&es, &es.maker.pubkey()); + send(&mut es.svm, &payer, &[cancel_ix], &[&maker]); + let switched_make_ix = make_offer_instruction_for( + &es, + switched_a_offered, + switched_b_wanted, + &es.mint_b.pubkey(), + &es.maker_account_b, + ); + send(&mut es.svm, &payer, &[switched_make_ix], &[&maker]); + + let result = try_send(&mut es.svm, &payer, &[take_ix], &[&taker]); + let error = format!( + "{:?}", + result.expect_err("take_offer should have failed").err + ); + assert!( + error.contains(&format!("Custom({OFFER_TERMS_CHANGED})")), + "expected error {OFFER_TERMS_CHANGED}, got: {error}" + ); + + assert_eq!( + token_amount(&es.svm, &es.taker_account_b), + MINTED_AMOUNT, + "the taker must not pay token B for a switched offer" + ); + assert_eq!( + lamports(&es.svm, &es.taker_account_a), + 0, + "the taker must receive no token A from a switched offer" + ); + assert_eq!(token_amount(&es.svm, &es.maker_account_b), 0); + assert_eq!(token_amount(&es.svm, &es.vault), switched_a_offered); +} + +#[test] +fn test_take_offer_rejects_switched_offer() { + // The maker re-makes the offer with a thousandth of the token A. + assert_switched_offer_refused(AMOUNT_A / 1_000, AMOUNT_B); +} + +#[test] +fn test_take_offer_rejects_switched_offer_wanting_more_token_b() { + // The maker re-makes the offer asking for twice the token B. + assert_switched_offer_refused(AMOUNT_A, 2 * AMOUNT_B); +} diff --git a/finance/escrow/quasar/CHANGELOG.md b/finance/escrow/quasar/CHANGELOG.md index 499d9a55..ac61d7bc 100644 --- a/finance/escrow/quasar/CHANGELOG.md +++ b/finance/escrow/quasar/CHANGELOG.md @@ -1,5 +1,20 @@ # Changelog +## [2026-10-04] + +### Fixed + +- `take_offer` takes two arguments the taker signs, `minimum_token_a_out` + and `maximum_token_b_in`, and refuses the take with `OfferTermsChanged` + (error 6001) before any token moves if the vault holds less token A or the + offer's `receive` is more than those bounds. An offer's address is its + maker and `id`, so a maker could cancel an offer and re-make the same `id` + at worse terms while a taker's transaction was in flight, and the + transaction would trade at the new terms. + `test_take_offer_rejects_switched_offer` and + `test_take_offer_rejects_switched_offer_wanting_more_token_b` run that + switch. + ## [2026-09-29] ### Changed diff --git a/finance/escrow/quasar/README.md b/finance/escrow/quasar/README.md index 5f36e221..d428e2bb 100644 --- a/finance/escrow/quasar/README.md +++ b/finance/escrow/quasar/README.md @@ -9,6 +9,7 @@ See also: the [repository catalog](../../../README.md). - **Offer**: a PDA with seeds `["offer", maker, id]` (the same seeds as the Anchor variant, so clients work against either build). It stores the maker, both mints, the maker's token B account, the vault address, the wanted `receive` amount, and the bump. `take_offer` and `cancel_offer` validate every passed account against this stored state via `has_one` bindings. - **Vault**: a token account owned by the offer PDA holding the maker's offered token A while the offer is open. - The maker pays the rent for the offer account and the vault in `make_offer`; both `take_offer` and `cancel_offer` close those accounts back to the maker. +- `take_offer` takes `minimum_token_a_out` (the least token A the taker accepts from the vault) and `maximum_token_b_in` (the most token B the taker will pay), signed by the taker. An offer's address comes from its maker and `id`, so while a taker's transaction is in flight the maker could cancel the offer and make it again under the same `id` at worse terms, and the transaction would land on the new offer. Before any token moves, `take_offer` refuses the take with `OfferTermsChanged` if the vault holds less token A than `minimum_token_a_out` or the offer's `receive` is more than `maximum_token_b_in`. `test_take_offer_rejects_switched_offer` and `test_take_offer_rejects_switched_offer_wanting_more_token_b` in `src/tests.rs` run that switch and check the take fails with the taker's tokens untouched. - See the [Anchor variant](../anchor/README.md) for the full walkthrough. ## Setup diff --git a/finance/escrow/quasar/src/error.rs b/finance/escrow/quasar/src/error.rs index 9e27d910..6841a48d 100644 --- a/finance/escrow/quasar/src/error.rs +++ b/finance/escrow/quasar/src/error.rs @@ -7,4 +7,8 @@ pub enum EscrowError { // program-specific error codes (Quasar's #[error_code] starts at 0 // unless told otherwise; framework errors occupy 3000+). ZeroAmount = 6000, + /// The offer pays less token A, or wants more token B, than the taker + /// agreed to: the maker re-made the offer at worse terms after the taker + /// signed. + OfferTermsChanged, } diff --git a/finance/escrow/quasar/src/instructions/take_offer.rs b/finance/escrow/quasar/src/instructions/take_offer.rs index 73036612..ceb26f3d 100644 --- a/finance/escrow/quasar/src/instructions/take_offer.rs +++ b/finance/escrow/quasar/src/instructions/take_offer.rs @@ -1,5 +1,8 @@ use { - crate::state::Offer, quasar_lang::cpi::Seed, quasar_lang::prelude::*, quasar_spl::prelude::*, + crate::{error::EscrowError, state::Offer}, + quasar_lang::cpi::Seed, + quasar_lang::prelude::*, + quasar_spl::prelude::*, }; #[derive(Accounts)] @@ -44,6 +47,23 @@ pub struct TakeOfferAccountConstraints { pub system_program: Program, } +/// Refuse the take unless the offer still holds the terms the taker signed +/// for: at least `minimum_token_a_out` of token A in the vault, and no more +/// than `maximum_token_b_in` of token B wanted. Runs before any transfer. +#[inline(always)] +pub fn handle_check_offer_terms( + accounts: &TakeOfferAccountConstraints, + minimum_token_a_out: u64, + maximum_token_b_in: u64, +) -> Result<(), ProgramError> { + require!( + accounts.vault.amount() >= minimum_token_a_out + && u64::from(accounts.offer.receive) <= maximum_token_b_in, + EscrowError::OfferTermsChanged + ); + Ok(()) +} + #[inline(always)] pub fn handle_transfer_tokens( accounts: &mut TakeOfferAccountConstraints, diff --git a/finance/escrow/quasar/src/lib.rs b/finance/escrow/quasar/src/lib.rs index 4dc8904d..f3009909 100644 --- a/finance/escrow/quasar/src/lib.rs +++ b/finance/escrow/quasar/src/lib.rs @@ -30,8 +30,21 @@ mod quasar_escrow { instructions::make_offer::handle_deposit_tokens(&mut ctx.accounts, deposit) } + /// The taker signs the terms they agreed to. An offer's address is its + /// maker and id, so a maker can cancel and re-make the same id at worse + /// terms while the taker's transaction is in flight; these bounds make + /// that transaction fail instead of trading at the new terms. #[instruction(discriminator = 1)] - pub fn take_offer(ctx: Ctx) -> Result<(), ProgramError> { + pub fn take_offer( + ctx: Ctx, + minimum_token_a_out: u64, + maximum_token_b_in: u64, + ) -> Result<(), ProgramError> { + instructions::take_offer::handle_check_offer_terms( + &ctx.accounts, + minimum_token_a_out, + maximum_token_b_in, + )?; instructions::take_offer::handle_transfer_tokens(&mut ctx.accounts)?; instructions::take_offer::handle_withdraw_tokens_and_close_take( &mut ctx.accounts, diff --git a/finance/escrow/quasar/src/tests.rs b/finance/escrow/quasar/src/tests.rs index 9691a211..be43cf3c 100644 --- a/finance/escrow/quasar/src/tests.rs +++ b/finance/escrow/quasar/src/tests.rs @@ -1,6 +1,7 @@ //! quasar-test integration tests: make an offer (deposit into the vault), //! take it (swap the tokens, close the offer and vault back to the maker), -//! cancel it, and reject substituted accounts and non-maker signers. +//! cancel it, and reject substituted accounts, non-maker signers, and a take +//! that lands on an offer the maker switched to worse terms. use { crate::{ @@ -141,6 +142,8 @@ fn take_offer_swaps_tokens_and_returns_rent_to_the_maker(test: &mut Test) { taker_token_account_b: TAKER_TOKEN_ACCOUNT_B, maker_token_account_b: MAKER_TOKEN_ACCOUNT_B, vault: VAULT, + minimum_token_a_out: DEPOSIT_AMOUNT, + maximum_token_b_in: RECEIVE_AMOUNT, }) .succeeds() // Token balances: the taker received the vault's mint A, the maker @@ -193,6 +196,8 @@ fn take_offer_rejects_a_mint_that_does_not_match_the_offer(test: &mut Test) { taker_token_account_b: TAKER_TOKEN_ACCOUNT_B, maker_token_account_b: MAKER_TOKEN_ACCOUNT_B, vault: VAULT, + minimum_token_a_out: DEPOSIT_AMOUNT, + maximum_token_b_in: RECEIVE_AMOUNT, }); assert!( result.is_err(), @@ -231,6 +236,8 @@ fn take_offer_rejects_a_vault_that_does_not_match_the_offer(test: &mut Test) { taker_token_account_b: TAKER_TOKEN_ACCOUNT_B, maker_token_account_b: MAKER_TOKEN_ACCOUNT_B, vault: WRONG_VAULT, + minimum_token_a_out: DEPOSIT_AMOUNT, + maximum_token_b_in: RECEIVE_AMOUNT, }); assert!( result.is_err(), @@ -345,3 +352,80 @@ fn make_offer_rejects_an_offer_of_a_token_for_itself(test: &mut Test) { make_offer(test, DEPOSIT_AMOUNT, RECEIVE_AMOUNT, TOKEN_MINT_A) .fails(ProgramError::Runtime("AccountBorrowFailed".into())); } + +/// The taker's `take_offer`, signed for the given terms. +fn take_offer_instruction(minimum_token_a_out: u64, maximum_token_b_in: u64) -> Instruction { + TakeOfferInstruction { + taker: TAKER, + offer_id_seed: OFFER_ID, + maker: MAKER, + token_mint_a: TOKEN_MINT_A, + token_mint_b: TOKEN_MINT_B, + taker_token_account_a: TAKER_TOKEN_ACCOUNT_A, + taker_token_account_b: TAKER_TOKEN_ACCOUNT_B, + maker_token_account_b: MAKER_TOKEN_ACCOUNT_B, + vault: VAULT, + minimum_token_a_out, + maximum_token_b_in, + } + .into() +} + +/// The bait and switch: the maker makes an offer, the taker signs a +/// `take_offer` for its terms, and before the taker's transaction lands the +/// maker cancels and re-makes the same id at the switched terms. The offer is +/// at the same address, so the taker's transaction reaches the new offer; it +/// must fail with `OfferTermsChanged` and leave the taker's tokens where they +/// were. +fn assert_switched_offer_refused(test: &mut Test, switched_deposit: u64, switched_receive: u64) { + maker_holding_token_a(test); + test.add(Wallet::new().at(TAKER)); + test.add( + TokenAccount::new(TOKEN_MINT_B, TAKER) + .at(TAKER_TOKEN_ACCOUNT_B) + .amount(10_000), + ); + + make_offer(test, DEPOSIT_AMOUNT, RECEIVE_AMOUNT, TOKEN_MINT_B).succeeds(); + + // The taker signs for the terms they saw. + let take = take_offer_instruction(DEPOSIT_AMOUNT, RECEIVE_AMOUNT); + + // The maker switches the offer before the taker's transaction lands. + test.send(CancelOfferInstruction { + maker: MAKER, + offer_id_seed: OFFER_ID, + token_mint_a: TOKEN_MINT_A, + maker_token_account_a: MAKER_TOKEN_ACCOUNT_A, + vault: VAULT, + }) + .succeeds(); + make_offer(test, switched_deposit, switched_receive, TOKEN_MINT_B).succeeds(); + + test.send(take).fails_with(EscrowError::OfferTermsChanged); + + assert_eq!( + test.tokens(TAKER_TOKEN_ACCOUNT_B), + 10_000, + "the taker must not pay token B for a switched offer" + ); + assert!( + test.account(TAKER_TOKEN_ACCOUNT_A) + .is_none_or(|account| account.lamports == 0), + "the taker must receive no token A from a switched offer" + ); + assert_eq!(test.tokens(MAKER_TOKEN_ACCOUNT_B), 0); + assert_eq!(test.tokens(VAULT), switched_deposit); +} + +#[quasar_test] +fn test_take_offer_rejects_switched_offer(test: &mut Test) { + // The maker re-makes the offer with a single token A in the vault. + assert_switched_offer_refused(test, 1, RECEIVE_AMOUNT); +} + +#[quasar_test] +fn test_take_offer_rejects_switched_offer_wanting_more_token_b(test: &mut Test) { + // The maker re-makes the offer asking for twice the token B. + assert_switched_offer_refused(test, DEPOSIT_AMOUNT, 2 * RECEIVE_AMOUNT); +}