security: fix broken take_offer invariant + add missing RefundOffer instruction

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 refund_offer;
+pub use refund_offer::*;
+
 pub mod shared;
 pub use shared::*;

tokens/escrow/anchor/programs/escrow/src/instructions/refund_offer.rs

@@ -0,0 +1,87 @@
+use anchor_lang::prelude::*;
+
+use anchor_spl::token_interface::{
+    close_account, transfer_checked, CloseAccount, Mint, TokenAccount, TokenInterface,
+    TransferChecked,
+};
+
+use crate::Offer;
+
+#[derive(Accounts)]
+pub struct RefundOffer<'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
+    )]
+    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 token_program: Interface<'info, TokenInterface>,
+}
+
+pub fn withdraw_and_close_vault_for_refund(ctx: Context<RefundOffer>) -> Result<()> {
+    let seeds = &[
+        b"offer",
+        ctx.accounts.maker.to_account_info().key.as_ref(),
+        &ctx.accounts.offer.id.to_le_bytes()[..],
+        &[ctx.accounts.offer.bump],
+    ];
+    let signer_seeds = [&seeds[..]];
+
+    let accounts = TransferChecked {
+        from: ctx.accounts.vault.to_account_info(),
+        mint: ctx.accounts.token_mint_a.to_account_info(),
+        to: ctx.accounts.maker_token_account_a.to_account_info(),
+        authority: ctx.accounts.offer.to_account_info(),
+    };
+
+    let cpi_context = CpiContext::new_with_signer(
+        ctx.accounts.token_program.key(),
+        accounts,
+        &signer_seeds,
+    );
+
+    transfer_checked(
+        cpi_context,
+        ctx.accounts.vault.amount,
+        ctx.accounts.token_mint_a.decimals,
+    )?;
+
+    let accounts = CloseAccount {
+        account: ctx.accounts.vault.to_account_info(),
+        destination: ctx.accounts.maker.to_account_info(),
+        authority: ctx.accounts.offer.to_account_info(),
+    };
+
+    let cpi_context = CpiContext::new_with_signer(
+        ctx.accounts.token_program.key(),
+        accounts,
+        &signer_seeds,
+    );
+
+    close_account(cpi_context)
+}

tokens/escrow/anchor/programs/escrow/src/lib.rs

@@ -29,4 +29,8 @@ pub mod escrow {
         instructions::take_offer::send_wanted_tokens_to_maker(&context)?;
         instructions::take_offer::withdraw_and_close_vault(context)
     }
+
+    pub fn refund_offer(context: Context<RefundOffer>) -> Result<()> {
+        instructions::refund_offer::withdraw_and_close_vault_for_refund(context)
+    }
 }

tokens/escrow/anchor/tests/litesvm.test.ts

@@ -202,4 +202,69 @@ describe('Escrow LiteSVM example', () => {
     it("Puts the tokens from the vault into Bob's account, and gives Alice Bob's tokens, when Bob takes an offer", async () => {
         await take();
     });
+
+    it('Returns the vaulted tokens to Alice when she refunds her own offer', async () => {
+        await make();
+
+        const aliceTokenAAccountBefore = await getAccount(
+            connection,
+            accounts.makerTokenAccountA,
+            'processed',
+            TOKEN_PROGRAM,
+        );
+        const aliceBalanceBefore = new BN(aliceTokenAAccountBefore.amount.toString());
+
+        const _transactionSignature = await program.methods
+            .refundOffer()
+            .accounts({ ...accounts })
+            .signers([alice])
+            .rpc();
+
+        // anchor-litesvm's connection proxy throws rather than returning null
+        // for a missing account, so closure is verified by expecting a throw.
+        const isClosed = async (address: PublicKey) => {
+            try {
+                await connection.getAccountInfo(address);
+                return false;
+            } catch {
+                return true;
+            }
+        };
+        assert(await isClosed(accounts.offer), 'offer account not closed');
+        assert(await isClosed(accounts.vault), 'vault account not closed');
+
+        const aliceTokenAAccountAfter = await getAccount(
+            connection,
+            accounts.makerTokenAccountA,
+            'processed',
+            TOKEN_PROGRAM,
+        );
+        const aliceBalanceAfter = new BN(aliceTokenAAccountAfter.amount.toString());
+        assert(aliceBalanceAfter.eq(aliceBalanceBefore.add(tokenAOfferedAmount)));
+    });
+
+    it('Rejects a refund attempt from a non-maker signer', async () => {
+        await make();
+
+        // Bob attempts to refund Alice's offer by claiming to be its maker.
+        // The mismatched accounts are intentional (that's what this test is
+        // proving the program rejects), so the strict IDL-derived account
+        // type doesn't apply here.
+        const forgedAccounts: Record<string, PublicKey> = {
+            ...accounts,
+            maker: accounts.taker,
+            makerTokenAccountA: accounts.takerTokenAccountA,
+        };
+        let threw = false;
+        try {
+            await program.methods
+                .refundOffer()
+                .accounts(forgedAccounts as any)
+                .signers([bob])
+                .rpc();
+        } catch {
+            threw = true;
+        }
+        assert(threw, 'expected a non-maker refund to fail');
+    });
 });

tokens/escrow/native/program/src/instructions/mod.rs

@@ -3,3 +3,6 @@ pub use make_offer::*;
 
 pub mod take_offer;
 pub use take_offer::*;
+
+pub mod refund_offer;
+pub use refund_offer::*;

tokens/escrow/native/program/src/instructions/refund_offer.rs

@@ -0,0 +1,118 @@
+use {
+    crate::{error::*, state::*, utils::*},
+    borsh::BorshDeserialize,
+    solana_program::{
+        account_info::AccountInfo, entrypoint::ProgramResult, program::invoke_signed, program_error::ProgramError,
+        program_pack::Pack, pubkey::Pubkey,
+    },
+    spl_token_interface::{instruction as token_instruction, state::Account as TokenAccount},
+};
+
+#[derive(BorshDeserialize, Debug)]
+pub struct RefundOffer {}
+
+impl RefundOffer {
+    pub fn process(program_id: &Pubkey, accounts: &[AccountInfo<'_>]) -> ProgramResult {
+        // accounts in order
+        //
+        let [
+            offer_info, // offer account info
+            token_mint_a, // token mint a
+            maker_token_account_a, // maker token a account, receives the refund
+            vault, // vault
+            maker, // maker
+            token_program, // token program
+            system_program// system program
+        ] = accounts else {
+            return Err(ProgramError::NotEnoughAccountKeys);
+        };
+
+        // ensure the maker signs the instruction
+        //
+        if !maker.is_signer {
+            return Err(ProgramError::MissingRequiredSignature);
+        }
+
+        // ensure the caller didn't substitute a fake token program - the real
+        // program is what actually enforces the transfer/close below
+        //
+        spl_token_interface::check_program_account(token_program.key)?;
+
+        // get the offer data
+        //
+        let offer = Offer::try_from_slice(&offer_info.data.borrow()[..])?;
+
+        // only the maker who created the offer may refund it
+        //
+        assert_eq!(&offer.maker, maker.key);
+        assert_eq!(&offer.token_mint_a, token_mint_a.key);
+
+        // validate the offer account with signer seeds
+        //
+        let offer_signer_seeds = &[Offer::SEED_PREFIX, maker.key.as_ref(), &offer.id.to_le_bytes(), &[offer.bump]];
+
+        let offer_key = Pubkey::create_program_address(offer_signer_seeds, program_id)?;
+
+        // make sure the offer key is the same
+        //
+        if *offer_info.key != offer_key {
+            return Err(EscrowError::OfferKeyMismatch.into());
+        };
+
+        // validate the maker's receiving address
+        //
+        assert_is_associated_token_account(maker_token_account_a.key, maker.key, token_mint_a.key)?;
+
+        // validate the vault is the offer's actual vault, not a substitute
+        // token-A account that also happens to be owned by the offer PDA
+        //
+        assert_is_associated_token_account(vault.key, offer_info.key, token_mint_a.key)?;
+
+        // return the vaulted tokens to the maker
+        //
+        let vault_amount_a = TokenAccount::unpack(&vault.data.borrow())?.amount;
+
+        invoke_signed(
+            &token_instruction::transfer(
+                token_program.key,
+                vault.key,
+                maker_token_account_a.key,
+                offer_info.key,
+                &[offer_info.key],
+                vault_amount_a,
+            )?,
+            &[vault.clone(), maker_token_account_a.clone(), offer_info.clone(), token_program.clone()],
+            &[offer_signer_seeds],
+        )?;
+
+        // close the vault account, rent to the maker
+        //
+        invoke_signed(
+            &spl_token_interface::instruction::close_account(
+                token_program.key,
+                vault.key,
+                maker.key,
+                offer_info.key,
+                &[],
+            )?,
+            &[vault.clone(), maker.clone(), offer_info.clone()],
+            &[offer_signer_seeds],
+        )?;
+
+        // Send the rent back to the maker
+        //
+        let lamports = offer_info.lamports();
+        **offer_info.lamports.borrow_mut() -= lamports;
+        **maker.lamports.borrow_mut() += lamports;
+
+        // Realloc the account to zero
+        //
+        offer_info.resize(0)?;
+
+        // Assign the account to the System Program
+        //
+        offer_info.assign(system_program.key);
+
+        Ok(())
+    }
+}

tokens/escrow/native/program/src/instructions/take_offer.rs

@@ -168,7 +168,7 @@ impl TakeOffer {
         let maker_amount_b = TokenAccount::unpack(&maker_token_account_b.data.borrow())?.amount;
 
         assert_eq!(taker_amount_a, taker_amount_a_before_transfer + vault_amount_a);
-        assert_eq!(maker_amount_b, taker_amount_a_before_transfer + offer.token_b_wanted_amount);
+        assert_eq!(maker_amount_b, maker_amount_b_before_transfer + offer.token_b_wanted_amount);
 
         let taker_amount_b = TokenAccount::unpack(&taker_token_account_b.data.borrow())?.amount;
         let vault_amount_a = TokenAccount::unpack(&vault.data.borrow())?.amount;

tokens/escrow/native/program/src/lib.rs

@@ -17,11 +17,15 @@ fn process_instruction(program_id: &Pubkey, accounts: &[AccountInfo], instructio
     match instruction {
         EscrowInstruction::MakeOffer(data) => MakeOffer::process(program_id, accounts, data),
         EscrowInstruction::TakeOffer => TakeOffer::process(program_id, accounts),
+        EscrowInstruction::RefundOffer => RefundOffer::process(program_id, accounts),
     }
 }
 
 #[derive(BorshSerialize, BorshDeserialize, Debug)]
+// "Offer" is domain vocabulary, not a redundant postfix - keep MakeOffer/TakeOffer/RefundOffer.
+#[allow(clippy::enum_variant_names)]
 enum EscrowInstruction {
     MakeOffer(MakeOffer),
     TakeOffer,
+    RefundOffer,
 }

tokens/escrow/native/tests/instruction.ts

@@ -6,6 +6,7 @@ import * as borsh from 'borsh';
 enum EscrowInstruction {
     MakeOffer = 0,
     TakeOffer = 1,
+    RefundOffer = 2,
 }
 
 const MakeOfferSchema = {
@@ -23,6 +24,12 @@ const TakeOfferSchema = {
     },
 };
 
+const RefundOfferSchema = {
+    struct: {
+        instruction: 'u8',
+    },
+};
+
 function borshSerialize(schema: borsh.Schema, data: object): Uint8Array {
     return borsh.serialize(schema, data);
 }
@@ -102,3 +109,30 @@ export function buildTakeOffer(props: {
         data,
     };
 }
+
+export function buildRefundOffer(props: {
+    offer: Address;
+    mint_a: Address;
+    maker_token_a: Address;
+    vault: Address;
+    maker: TransactionSigner;
+    programId: Address;
+}) {
+    const data = borshSerialize(RefundOfferSchema, {
+        instruction: EscrowInstruction.RefundOffer,
+    });
+
+    return {
+        programAddress: props.programId,
+        accounts: [
+            { address: props.offer, role: AccountRole.WRITABLE },
+            { address: props.mint_a, role: AccountRole.READONLY },
+            { address: props.maker_token_a, role: AccountRole.WRITABLE },
+            { address: props.vault, role: AccountRole.WRITABLE },
+            { address: props.maker.address, role: AccountRole.WRITABLE_SIGNER, signer: props.maker },
+            { address: TOKEN_PROGRAM_ADDRESS, role: AccountRole.READONLY },
+            { address: SYSTEM_PROGRAM_ADDRESS, role: AccountRole.READONLY },
+        ],
+        data,
+    };
+}