From 5bb6ffb0ca848b82e6d5de672aebba30d6e8061e Mon Sep 17 00:00:00 2001 From: Csongor Kiss Date: Mon, 29 Apr 2024 17:35:09 +0100 Subject: [PATCH] solana: transfer before burn/mint to trigger hooks --- sdk/solana/src/ntt.ts | 79 +++++++---- .../json/example_native_token_transfers.json | 58 ++++++-- .../idl/ts/example_native_token_transfers.ts | 116 +++++++++++---- .../src/instructions/initialize.rs | 5 +- .../src/instructions/release_inbound.rs | 123 ++++++++++------ .../src/instructions/transfer.rs | 133 +++++++++++++----- .../example-native-token-transfers/src/lib.rs | 9 +- .../tests/sdk/instructions/transfer.rs | 3 +- solana/tests/example-native-token-transfer.ts | 30 +--- solana/ts/sdk/ntt.ts | 91 ++++++++++-- 10 files changed, 451 insertions(+), 196 deletions(-) diff --git a/sdk/solana/src/ntt.ts b/sdk/solana/src/ntt.ts index 4b5f7754e..6b3e3f60c 100644 --- a/sdk/solana/src/ntt.ts +++ b/sdk/solana/src/ntt.ts @@ -1,5 +1,4 @@ import { Program } from "@coral-xyz/anchor"; -import { associatedAddress } from "@coral-xyz/anchor/dist/cjs/utils/token.js"; import * as splToken from "@solana/spl-token"; import { createAssociatedTokenAccountInstruction, @@ -9,6 +8,7 @@ import { Connection, Keypair, PublicKey, + SystemProgram, Transaction, TransactionInstruction, TransactionMessage, @@ -236,16 +236,11 @@ export class SolanaNtt ); } - const custodyAddress = associatedAddress({ - mint: args.mint, - owner: this.pdas.tokenAuthority(), - }); - const tokenProgram = mintInfo.owner; const limit = new BN(args.outboundLimit.toString()); const ix = await this.program.methods .initialize({ chainId, limit: limit, mode }) - .accounts({ + .accountsStrict({ payer: args.payer.publicKey, deployer: args.owner.publicKey, programData: programDataAddress(this.program.programId), @@ -254,8 +249,10 @@ export class SolanaNtt rateLimit: this.pdas.outboxRateLimitAccount(), tokenProgram, tokenAuthority: this.pdas.tokenAuthority(), - custody: custodyAddress, + custody: await this.custodyAccountAddress(args.mint, tokenProgram), bpfLoaderUpgradeableProgram: BPF_LOADER_UPGRADEABLE_PROGRAM_ID, + associatedTokenProgram: splToken.ASSOCIATED_TOKEN_PROGRAM_ID, + systemProgram: SystemProgram.programId, }) .instruction(); @@ -278,7 +275,7 @@ export class SolanaNtt const ix = await this.program.methods .registerTransceiver() - .accounts({ + .accountsStrict({ payer: args.payer.publicKey, owner: args.owner.publicKey, config: this.pdas.configAccount(), @@ -286,6 +283,7 @@ export class SolanaNtt registeredTransceiver: this.pdas.registeredTransceiver( args.transceiver ), + systemProgram: SystemProgram.programId, }) .instruction(); @@ -307,6 +305,7 @@ export class SolanaNtt feeCollector: whAccs.wormholeFeeCollector, sequence: whAccs.wormholeSequence, program: this.core.address, + systemProgram: SystemProgram.programId, }, }) .instruction(); @@ -337,11 +336,12 @@ export class SolanaNtt chainId: { id: toChainId(peer.chain) }, address: Array.from(peer.address.toUniversalAddress().toUint8Array()), }) - .accounts({ + .accountsStrict({ payer: sender, owner: sender, config: this.pdas.configAccount(), peer: this.pdas.transceiverPeerAccount(peer.chain), + systemProgram: SystemProgram.programId, }) .instruction(), this.program.methods @@ -390,12 +390,13 @@ export class SolanaNtt limit: new BN(inboundLimit.toString()), tokenDecimals: tokenDecimals, }) - .accounts({ + .accountsStrict({ payer: sender, owner: sender, config: this.pdas.configAccount(), peer: this.pdas.peerAccount(peer.chain), inboxRateLimit: this.pdas.inboxRateLimitAccount(peer.chain), + systemProgram: SystemProgram.programId, }) .instruction(); @@ -671,7 +672,7 @@ export class SolanaNtt ), shouldQueue: args.transferArgs.shouldQueue, }) - .accounts({ + .accountsStrict({ common: { payer: args.payer, config: { config: this.pdas.configAccount() }, @@ -680,10 +681,11 @@ export class SolanaNtt tokenProgram: config.tokenProgram, outboxItem: args.outboxItem, outboxRateLimit: this.pdas.outboxRateLimitAccount(), + systemProgram: SystemProgram.programId, + custody: config.custody, }, peer: this.pdas.peerAccount(recipientChain), inboxRateLimit: this.pdas.inboxRateLimitAccount(recipientChain), - custody: config.custody, sessionAuthority: sessionAuthority, }) .instruction(); @@ -712,7 +714,7 @@ export class SolanaNtt ), shouldQueue: args.transferArgs.shouldQueue, }) - .accounts({ + .accountsStrict({ common: { payer: args.payer, config: { config: this.pdas.configAccount() }, @@ -720,6 +722,9 @@ export class SolanaNtt from: args.from, outboxItem: args.outboxItem, outboxRateLimit: this.pdas.outboxRateLimitAccount(), + custody: config.custody, + tokenProgram: config.tokenProgram, + systemProgram: SystemProgram.programId, }, peer: this.pdas.peerAccount(recipientChain), inboxRateLimit: this.pdas.inboxRateLimitAccount(recipientChain), @@ -727,6 +732,7 @@ export class SolanaNtt args.fromAuthority, args.transferArgs ), + tokenAuthority: this.pdas.tokenAuthority(), }) .instruction(); } @@ -757,6 +763,7 @@ export class SolanaNtt feeCollector: whAccs.wormholeFeeCollector, sequence: whAccs.wormholeSequence, program: this.core.address, + systemProgram: SystemProgram.programId, }, }) .instruction(); @@ -773,7 +780,7 @@ export class SolanaNtt const emitterChain = wormholeNTT.emitterChain; return await this.program.methods .receiveWormholeMessage() - .accounts({ + .accountsStrict({ payer: payer, config: { config: this.pdas.configAccount() }, peer: this.pdas.transceiverPeerAccount(emitterChain), @@ -785,6 +792,7 @@ export class SolanaNtt emitterChain, nttMessage.id ), + systemProgram: SystemProgram.programId, }) .instruction(); } @@ -805,7 +813,7 @@ export class SolanaNtt return await this.program.methods .redeem({}) - .accounts({ + .accountsStrict({ payer: payer, config: this.pdas.configAccount(), peer: nttManagerPeer, @@ -818,6 +826,7 @@ export class SolanaNtt inboxItem, inboxRateLimit, outboxRateLimit: this.pdas.outboxRateLimitAccount(), + systemProgram: SystemProgram.programId, }) .instruction(); } @@ -847,7 +856,7 @@ export class SolanaNtt .releaseInboundMint({ revertOnDelay: args.revertOnDelay, }) - .accounts({ + .accountsStrict({ common: { payer: args.payer, config: { config: this.pdas.configAccount() }, @@ -858,6 +867,8 @@ export class SolanaNtt ), mint: config.mint, tokenAuthority: this.pdas.tokenAuthority(), + custody: config.custody, + tokenProgram: config.tokenProgram, }, }) .instruction(); @@ -888,7 +899,7 @@ export class SolanaNtt .releaseInboundUnlock({ revertOnDelay: args.revertOnDelay, }) - .accounts({ + .accountsStrict({ common: { payer: args.payer, config: { config: this.pdas.configAccount() }, @@ -899,17 +910,37 @@ export class SolanaNtt ), mint: config.mint, tokenAuthority: this.pdas.tokenAuthority(), + custody: config.custody, + tokenProgram: config.tokenProgram, }, - custody: config.custody, }) .instruction(); } - async custodyAccountAddress(mint: PublicKey): Promise { - return associatedAddress({ - mint: mint, - owner: this.pdas.tokenAuthority(), - }); + /** + * Returns the address of the custody account. If the config is available + * (i.e. the program is initialised), the mint is derived from the config. + * Otherwise, the mint must be provided. + */ + async custodyAccountAddress( + configOrMint: NttBindings.Config | PublicKey, + tokenProgram = splToken.TOKEN_PROGRAM_ID + ): Promise { + if (configOrMint instanceof PublicKey) { + return splToken.getAssociatedTokenAddress( + configOrMint, + this.pdas.tokenAuthority(), + true, + tokenProgram + ); + } else { + return splToken.getAssociatedTokenAddress( + configOrMint.mint, + this.pdas.tokenAuthority(), + true, + configOrMint.tokenProgram + ); + } } createUnsignedTx( diff --git a/solana/idl/json/example_native_token_transfers.json b/solana/idl/json/example_native_token_transfers.json index 296557228..f3a0b2089 100644 --- a/solana/idl/json/example_native_token_transfers.json +++ b/solana/idl/json/example_native_token_transfers.json @@ -45,9 +45,8 @@ "isMut": true, "isSigner": false, "docs": [ - "The custody account that holds tokens in locking mode.", - "NOTE: the account is unconditionally initialized, but not used in", - "burning mode.", + "The custody account that holds tokens in locking mode and temporarily", + "holds tokens in burning mode.", "function if the token account has already been created." ] }, @@ -139,6 +138,16 @@ "isMut": true, "isSigner": false }, + { + "name": "custody", + "isMut": true, + "isSigner": false, + "docs": [ + "Tokens are always transferred to the custody account first regardless of", + "the mode.", + "For an explanation, see the note in [`transfer_burn`]." + ] + }, { "name": "systemProgram", "isMut": false, @@ -159,6 +168,14 @@ { "name": "sessionAuthority", "isMut": false, + "isSigner": false, + "docs": [ + "See [`crate::SESSION_AUTHORITY_SEED`] for an explanation of the flow." + ] + }, + { + "name": "tokenAuthority", + "isMut": false, "isSigner": false } ], @@ -220,6 +237,16 @@ "isMut": true, "isSigner": false }, + { + "name": "custody", + "isMut": true, + "isSigner": false, + "docs": [ + "Tokens are always transferred to the custody account first regardless of", + "the mode.", + "For an explanation, see the note in [`transfer_burn`]." + ] + }, { "name": "systemProgram", "isMut": false, @@ -240,12 +267,10 @@ { "name": "sessionAuthority", "isMut": false, - "isSigner": false - }, - { - "name": "custody", - "isMut": true, - "isSigner": false + "isSigner": false, + "docs": [ + "See [`crate::SESSION_AUTHORITY_SEED`] for an explanation of the flow." + ] } ], "args": [ @@ -378,6 +403,11 @@ "name": "tokenProgram", "isMut": false, "isSigner": false + }, + { + "name": "custody", + "isMut": true, + "isSigner": false } ] } @@ -436,13 +466,13 @@ "name": "tokenProgram", "isMut": false, "isSigner": false + }, + { + "name": "custody", + "isMut": true, + "isSigner": false } ] - }, - { - "name": "custody", - "isMut": true, - "isSigner": false } ], "args": [ diff --git a/solana/idl/ts/example_native_token_transfers.ts b/solana/idl/ts/example_native_token_transfers.ts index fa85dfa73..e44664ead 100644 --- a/solana/idl/ts/example_native_token_transfers.ts +++ b/solana/idl/ts/example_native_token_transfers.ts @@ -45,9 +45,8 @@ export type ExampleNativeTokenTransfers = { "isMut": true, "isSigner": false, "docs": [ - "The custody account that holds tokens in locking mode.", - "NOTE: the account is unconditionally initialized, but not used in", - "burning mode.", + "The custody account that holds tokens in locking mode and temporarily", + "holds tokens in burning mode.", "function if the token account has already been created." ] }, @@ -139,6 +138,16 @@ export type ExampleNativeTokenTransfers = { "isMut": true, "isSigner": false }, + { + "name": "custody", + "isMut": true, + "isSigner": false, + "docs": [ + "Tokens are always transferred to the custody account first regardless of", + "the mode.", + "For an explanation, see the note in [`transfer_burn`]." + ] + }, { "name": "systemProgram", "isMut": false, @@ -159,6 +168,14 @@ export type ExampleNativeTokenTransfers = { { "name": "sessionAuthority", "isMut": false, + "isSigner": false, + "docs": [ + "See [`crate::SESSION_AUTHORITY_SEED`] for an explanation of the flow." + ] + }, + { + "name": "tokenAuthority", + "isMut": false, "isSigner": false } ], @@ -220,6 +237,16 @@ export type ExampleNativeTokenTransfers = { "isMut": true, "isSigner": false }, + { + "name": "custody", + "isMut": true, + "isSigner": false, + "docs": [ + "Tokens are always transferred to the custody account first regardless of", + "the mode.", + "For an explanation, see the note in [`transfer_burn`]." + ] + }, { "name": "systemProgram", "isMut": false, @@ -240,12 +267,10 @@ export type ExampleNativeTokenTransfers = { { "name": "sessionAuthority", "isMut": false, - "isSigner": false - }, - { - "name": "custody", - "isMut": true, - "isSigner": false + "isSigner": false, + "docs": [ + "See [`crate::SESSION_AUTHORITY_SEED`] for an explanation of the flow." + ] } ], "args": [ @@ -378,6 +403,11 @@ export type ExampleNativeTokenTransfers = { "name": "tokenProgram", "isMut": false, "isSigner": false + }, + { + "name": "custody", + "isMut": true, + "isSigner": false } ] } @@ -436,13 +466,13 @@ export type ExampleNativeTokenTransfers = { "name": "tokenProgram", "isMut": false, "isSigner": false + }, + { + "name": "custody", + "isMut": true, + "isSigner": false } ] - }, - { - "name": "custody", - "isMut": true, - "isSigner": false } ], "args": [ @@ -1927,9 +1957,8 @@ export const IDL: ExampleNativeTokenTransfers = { "isMut": true, "isSigner": false, "docs": [ - "The custody account that holds tokens in locking mode.", - "NOTE: the account is unconditionally initialized, but not used in", - "burning mode.", + "The custody account that holds tokens in locking mode and temporarily", + "holds tokens in burning mode.", "function if the token account has already been created." ] }, @@ -2021,6 +2050,16 @@ export const IDL: ExampleNativeTokenTransfers = { "isMut": true, "isSigner": false }, + { + "name": "custody", + "isMut": true, + "isSigner": false, + "docs": [ + "Tokens are always transferred to the custody account first regardless of", + "the mode.", + "For an explanation, see the note in [`transfer_burn`]." + ] + }, { "name": "systemProgram", "isMut": false, @@ -2041,6 +2080,14 @@ export const IDL: ExampleNativeTokenTransfers = { { "name": "sessionAuthority", "isMut": false, + "isSigner": false, + "docs": [ + "See [`crate::SESSION_AUTHORITY_SEED`] for an explanation of the flow." + ] + }, + { + "name": "tokenAuthority", + "isMut": false, "isSigner": false } ], @@ -2102,6 +2149,16 @@ export const IDL: ExampleNativeTokenTransfers = { "isMut": true, "isSigner": false }, + { + "name": "custody", + "isMut": true, + "isSigner": false, + "docs": [ + "Tokens are always transferred to the custody account first regardless of", + "the mode.", + "For an explanation, see the note in [`transfer_burn`]." + ] + }, { "name": "systemProgram", "isMut": false, @@ -2122,12 +2179,10 @@ export const IDL: ExampleNativeTokenTransfers = { { "name": "sessionAuthority", "isMut": false, - "isSigner": false - }, - { - "name": "custody", - "isMut": true, - "isSigner": false + "isSigner": false, + "docs": [ + "See [`crate::SESSION_AUTHORITY_SEED`] for an explanation of the flow." + ] } ], "args": [ @@ -2260,6 +2315,11 @@ export const IDL: ExampleNativeTokenTransfers = { "name": "tokenProgram", "isMut": false, "isSigner": false + }, + { + "name": "custody", + "isMut": true, + "isSigner": false } ] } @@ -2318,13 +2378,13 @@ export const IDL: ExampleNativeTokenTransfers = { "name": "tokenProgram", "isMut": false, "isSigner": false + }, + { + "name": "custody", + "isMut": true, + "isSigner": false } ] - }, - { - "name": "custody", - "isMut": true, - "isSigner": false } ], "args": [ diff --git a/solana/programs/example-native-token-transfers/src/instructions/initialize.rs b/solana/programs/example-native-token-transfers/src/instructions/initialize.rs index 555dfd316..9ebb9cc8b 100644 --- a/solana/programs/example-native-token-transfers/src/instructions/initialize.rs +++ b/solana/programs/example-native-token-transfers/src/instructions/initialize.rs @@ -67,9 +67,8 @@ pub struct Initialize<'info> { associated_token::authority = token_authority, associated_token::token_program = token_program, )] - /// The custody account that holds tokens in locking mode. - /// NOTE: the account is unconditionally initialized, but not used in - /// burning mode. + /// The custody account that holds tokens in locking mode and temporarily + /// holds tokens in burning mode. /// CHECK: Use init_if_needed here to prevent a denial-of-service of the [`initialize`] /// function if the token account has already been created. pub custody: InterfaceAccount<'info, token_interface::TokenAccount>, diff --git a/solana/programs/example-native-token-transfers/src/instructions/release_inbound.rs b/solana/programs/example-native-token-transfers/src/instructions/release_inbound.rs index e4fb201f8..dc5b6524e 100644 --- a/solana/programs/example-native-token-transfers/src/instructions/release_inbound.rs +++ b/solana/programs/example-native-token-transfers/src/instructions/release_inbound.rs @@ -41,6 +41,13 @@ pub struct ReleaseInbound<'info> { pub mint: InterfaceAccount<'info, token_interface::Mint>, pub token_program: Interface<'info, token_interface::TokenInterface>, + + /// CHECK: the token program checks if this indeed the right authority for the mint + #[account( + mut, + address = config.custody + )] + pub custody: InterfaceAccount<'info, token_interface::TokenAccount>, } #[derive(AnchorDeserialize, AnchorSerialize)] @@ -52,6 +59,9 @@ pub struct ReleaseInboundArgs { #[derive(Accounts)] pub struct ReleaseInboundMint<'info> { + #[account( + constraint = common.config.mode == Mode::Burning @ NTTError::InvalidMode, + )] common: ReleaseInbound<'info>, } @@ -62,8 +72,8 @@ pub struct ReleaseInboundMint<'info> { /// Setting this flag to `false` is useful when bundling this instruction /// together with [`crate::instructions::redeem`] in a transaction, so that the minting /// is attempted optimistically. -pub fn release_inbound_mint( - ctx: Context, +pub fn release_inbound_mint<'info>( + ctx: Context<'_, '_, '_, 'info, ReleaseInboundMint<'info>>, args: ReleaseInboundArgs, ) -> Result<()> { let inbox_item = &mut ctx.accounts.common.inbox_item; @@ -79,38 +89,65 @@ pub fn release_inbound_mint( } assert!(inbox_item.release_status == ReleaseStatus::Released); - match ctx.accounts.common.config.mode { - Mode::Burning => token_interface::mint_to( - CpiContext::new_with_signer( - ctx.accounts.common.token_program.to_account_info(), - token_interface::MintTo { - mint: ctx.accounts.common.mint.to_account_info(), - to: ctx.accounts.common.recipient.to_account_info(), - authority: ctx.accounts.common.token_authority.clone(), - }, - &[&[ - crate::TOKEN_AUTHORITY_SEED, - &[ctx.bumps.common.token_authority], - ]], - ), - inbox_item.amount, + + // NOTE: minting tokens is a two-step process: + // 1. Mint tokens to the custody account + // 2. Transfer the tokens from the custody account to the recipient + // + // This is done to ensure that if the token has a transfer hook defined, it + // will be called after the tokens are minted. + // Unfortunately the Token2022 program doesn't trigger transfer hooks when + // minting tokens, so we have to do it "manually" via a transfer. + // + // If we didn't do this, transfer hooks could be bypassed by transferring + // the tokens out through NTT first, then back in to the intended recipient. + // + // The [`transfer_burn`] function operates in a similar way + // (transfer to custody from sender, *then* burn). + + // Step 1: mint tokens to the custody account + token_interface::mint_to( + CpiContext::new_with_signer( + ctx.accounts.common.token_program.to_account_info(), + token_interface::MintTo { + mint: ctx.accounts.common.mint.to_account_info(), + to: ctx.accounts.common.custody.to_account_info(), + authority: ctx.accounts.common.token_authority.clone(), + }, + &[&[ + crate::TOKEN_AUTHORITY_SEED, + &[ctx.bumps.common.token_authority], + ]], ), - Mode::Locking => Err(NTTError::InvalidMode.into()), - } + inbox_item.amount, + )?; + + // Step 2: transfer the tokens from the custody account to the recipient + onchain::invoke_transfer_checked( + &ctx.accounts.common.token_program.key(), + ctx.accounts.common.custody.to_account_info(), + ctx.accounts.common.mint.to_account_info(), + ctx.accounts.common.recipient.to_account_info(), + ctx.accounts.common.token_authority.clone(), + ctx.remaining_accounts, + inbox_item.amount, + ctx.accounts.common.mint.decimals, + &[&[ + crate::TOKEN_AUTHORITY_SEED, + &[ctx.bumps.common.token_authority], + ]], + )?; + Ok(()) } // Lock/unlock #[derive(Accounts)] pub struct ReleaseInboundUnlock<'info> { - common: ReleaseInbound<'info>, - - /// CHECK: the token program checks if this indeed the right authority for the mint #[account( - mut, - address = common.config.custody + constraint = common.config.mode == Mode::Locking @ NTTError::InvalidMode, )] - pub custody: InterfaceAccount<'info, token_interface::TokenAccount>, + common: ReleaseInbound<'info>, } /// Release an inbound transfer and unlock the tokens to the recipient. @@ -136,25 +173,19 @@ pub fn release_inbound_unlock<'info>( } } - assert!(inbox_item.release_status == ReleaseStatus::Released); - match ctx.accounts.common.config.mode { - Mode::Burning => Err(NTTError::InvalidMode.into()), - Mode::Locking => { - onchain::invoke_transfer_checked( - &ctx.accounts.common.token_program.key(), - ctx.accounts.custody.to_account_info(), - ctx.accounts.common.mint.to_account_info(), - ctx.accounts.common.recipient.to_account_info(), - ctx.accounts.common.token_authority.clone(), - ctx.remaining_accounts, - inbox_item.amount, - ctx.accounts.common.mint.decimals, - &[&[ - crate::TOKEN_AUTHORITY_SEED, - &[ctx.bumps.common.token_authority], - ]], - )?; - Ok(()) - } - } + onchain::invoke_transfer_checked( + &ctx.accounts.common.token_program.key(), + ctx.accounts.common.custody.to_account_info(), + ctx.accounts.common.mint.to_account_info(), + ctx.accounts.common.recipient.to_account_info(), + ctx.accounts.common.token_authority.clone(), + ctx.remaining_accounts, + inbox_item.amount, + ctx.accounts.common.mint.decimals, + &[&[ + crate::TOKEN_AUTHORITY_SEED, + &[ctx.bumps.common.token_authority], + ]], + )?; + Ok(()) } diff --git a/solana/programs/example-native-token-transfers/src/instructions/transfer.rs b/solana/programs/example-native-token-transfers/src/instructions/transfer.rs index aab502e7f..802e0fc37 100644 --- a/solana/programs/example-native-token-transfers/src/instructions/transfer.rs +++ b/solana/programs/example-native-token-transfers/src/instructions/transfer.rs @@ -1,3 +1,17 @@ +//! This module implements the transfer instruction(s). +//! There are two types of transfers: burning and locking, depending on the +//! configuration of the NTT deployment. +//! +//! Since these two operations are very similar, we abstract out as much of the +//! common account structure as possible into the `Transfer` struct. +//! Due to some unfortunate limitations of Anchor, namely that it's impossible +//! to propagate instruction data to nested account structs, there is some +//! amount of duplication between `TransferBurn` and `TransferLock` (exactly the +//! accounts whose constraints refer to the instruction data). +//! +//! See the documentation of [`crate::SESSION_AUTHORITY_SEED`] for an +//! explanation of the approval flow. + #![allow(clippy::too_many_arguments)] use anchor_lang::prelude::*; use anchor_spl::token_interface; @@ -52,6 +66,15 @@ pub struct Transfer<'info> { #[account(mut)] pub outbox_rate_limit: Account<'info, OutboxRateLimit>, + #[account( + mut, + address = config.custody + )] + /// Tokens are always transferred to the custody account first regardless of + /// the mode. + /// For an explanation, see the note in [`transfer_burn`]. + pub custody: InterfaceAccount<'info, token_interface::TokenAccount>, + pub system_program: Program<'info, System>, } @@ -85,6 +108,9 @@ impl TransferArgs { #[derive(Accounts)] #[instruction(args: TransferArgs)] pub struct TransferBurn<'info> { + #[account( + constraint = common.config.mode == Mode::Burning @ NTTError::InvalidMode, + )] pub common: Transfer<'info>, #[account( @@ -110,16 +136,20 @@ pub struct TransferBurn<'info> { ], bump, )] + /// See [`crate::SESSION_AUTHORITY_SEED`] for an explanation of the flow. pub session_authority: AccountInfo<'info>, -} -pub fn transfer_burn(ctx: Context, args: TransferArgs) -> Result<()> { - require_eq!( - ctx.accounts.common.config.mode, - Mode::Burning, - NTTError::InvalidMode - ); + #[account( + seeds = [crate::TOKEN_AUTHORITY_SEED], + bump, + )] + pub token_authority: AccountInfo<'info>, +} +pub fn transfer_burn<'info>( + ctx: Context<'_, '_, '_, 'info, TransferBurn<'info>>, + args: TransferArgs, +) -> Result<()> { let accs = ctx.accounts; let TransferArgs { mut amount, @@ -136,30 +166,67 @@ pub fn transfer_burn(ctx: Context, args: TransferArgs) -> Result<( ) .map_err(NTTError::from)?; - let before = accs.common.from.amount; + let before = accs.common.custody.amount; + + // NOTE: burning tokens is a two-step process: + // 1. Transfer the tokens to the custody account + // 2. Burn the tokens from the custody account + // + // This is done to ensure that if the token has a transfer hook defined, it + // will be called before the tokens are burned. + // Unfortunately the Token2022 program doesn't trigger transfer hooks when + // burning tokens, so we have to do it "manually" via a transfer. + // + // If we didn't do this, transfer hooks could be bypassed by transferring + // the tokens out through NTT first, then back in to the intended recipient. + // + // The [`release_inbound_mint`] function operates in a similar way + // (mint to custody, *then* transfer to recipient). + + // Step 1: transfer to custody account + onchain::invoke_transfer_checked( + &accs.common.token_program.key(), + accs.common.from.to_account_info(), + accs.common.mint.to_account_info(), + accs.common.custody.to_account_info(), + accs.session_authority.to_account_info(), + ctx.remaining_accounts, + amount, + accs.common.mint.decimals, + &[&[ + crate::SESSION_AUTHORITY_SEED, + accs.common.from.owner.as_ref(), + args.keccak256().as_ref(), + &[ctx.bumps.session_authority], + ]], + )?; + // Step 2: burn the tokens from the custody account token_interface::burn( CpiContext::new_with_signer( accs.common.token_program.to_account_info(), token_interface::Burn { mint: accs.common.mint.to_account_info(), - from: accs.common.from.to_account_info(), - authority: accs.session_authority.to_account_info(), + from: accs.common.custody.to_account_info(), + authority: accs.token_authority.to_account_info(), }, - &[&[ - crate::SESSION_AUTHORITY_SEED, - accs.common.from.owner.as_ref(), - args.keccak256().as_ref(), - &[ctx.bumps.session_authority], - ]], + &[&[crate::TOKEN_AUTHORITY_SEED, &[ctx.bumps.token_authority]]], ), amount, )?; - accs.common.from.reload()?; - let after = accs.common.from.amount; + accs.common.custody.reload()?; + let after = accs.common.custody.amount; - if after != before - amount { + // NOTE: we currently do not support tokens with fees. Support could be + // added, but it would require the client to calculate the amount _before_ + // paying fees that results in an amount that can safely be trimmed. + // Otherwise, if the amount after paying fees has dust, then that amount + // would be lost. + // To support fee tokens, we would first transfer the amount, _then_ assert + // that the resulting amount has no dust (instead of removing dust before + // the transfer like we do now). + if after != before { return Err(NTTError::BadAmountAfterBurn.into()); } @@ -174,7 +241,9 @@ pub fn transfer_burn(ctx: Context, args: TransferArgs) -> Result<( recipient_ntt_manager, recipient_address, should_queue, - ) + )?; + + Ok(()) } // Lock/unlock @@ -182,6 +251,9 @@ pub fn transfer_burn(ctx: Context, args: TransferArgs) -> Result<( #[derive(Accounts)] #[instruction(args: TransferArgs)] pub struct TransferLock<'info> { + #[account( + constraint = common.config.mode == Mode::Locking @ NTTError::InvalidMode, + )] pub common: Transfer<'info>, #[account( @@ -207,25 +279,14 @@ pub struct TransferLock<'info> { ], bump, )] + /// See [`crate::SESSION_AUTHORITY_SEED`] for an explanation of the flow. pub session_authority: AccountInfo<'info>, - - #[account( - mut, - address = common.config.custody - )] - pub custody: InterfaceAccount<'info, token_interface::TokenAccount>, } pub fn transfer_lock<'info>( ctx: Context<'_, '_, '_, 'info, TransferLock<'info>>, args: TransferArgs, ) -> Result<()> { - require_eq!( - ctx.accounts.common.config.mode, - Mode::Locking, - NTTError::InvalidMode - ); - let accs = ctx.accounts; let TransferArgs { mut amount, @@ -242,13 +303,13 @@ pub fn transfer_lock<'info>( ) .map_err(NTTError::from)?; - let before = accs.custody.amount; + let before = accs.common.custody.amount; onchain::invoke_transfer_checked( &accs.common.token_program.key(), accs.common.from.to_account_info(), accs.common.mint.to_account_info(), - accs.custody.to_account_info(), + accs.common.custody.to_account_info(), accs.session_authority.to_account_info(), ctx.remaining_accounts, amount, @@ -261,8 +322,8 @@ pub fn transfer_lock<'info>( ]], )?; - accs.custody.reload()?; - let after = accs.custody.amount; + accs.common.custody.reload()?; + let after = accs.common.custody.amount; // NOTE: we currently do not support tokens with fees. Support could be // added, but it would require the client to calculate the amount _before_ diff --git a/solana/programs/example-native-token-transfers/src/lib.rs b/solana/programs/example-native-token-transfers/src/lib.rs index 8a2704301..b5bb5fbe5 100644 --- a/solana/programs/example-native-token-transfers/src/lib.rs +++ b/solana/programs/example-native-token-transfers/src/lib.rs @@ -77,7 +77,10 @@ pub mod example_native_token_transfers { Ok(VERSION.to_string()) } - pub fn transfer_burn(ctx: Context, args: TransferArgs) -> Result<()> { + pub fn transfer_burn<'info>( + ctx: Context<'_, '_, '_, 'info, TransferBurn<'info>>, + args: TransferArgs, + ) -> Result<()> { instructions::transfer_burn(ctx, args) } @@ -92,8 +95,8 @@ pub mod example_native_token_transfers { instructions::redeem(ctx, args) } - pub fn release_inbound_mint( - ctx: Context, + pub fn release_inbound_mint<'info>( + ctx: Context<'_, '_, '_, 'info, ReleaseInboundMint<'info>>, args: ReleaseInboundArgs, ) -> Result<()> { instructions::release_inbound_mint(ctx, args) diff --git a/solana/programs/example-native-token-transfers/tests/sdk/instructions/transfer.rs b/solana/programs/example-native-token-transfers/tests/sdk/instructions/transfer.rs index 9ab3255fc..810348e04 100644 --- a/solana/programs/example-native-token-transfers/tests/sdk/instructions/transfer.rs +++ b/solana/programs/example-native-token-transfers/tests/sdk/instructions/transfer.rs @@ -33,6 +33,7 @@ pub fn transfer_burn(ntt: &NTT, transfer: Transfer, args: TransferArgs) -> Instr inbox_rate_limit: ntt.inbox_rate_limit(chain_id), peer: transfer.peer, session_authority, + token_authority: ntt.token_authority(), }; Instruction { @@ -51,7 +52,6 @@ pub fn transfer_lock(ntt: &NTT, transfer: Transfer, args: TransferArgs) -> Instr common: common(ntt, &transfer), inbox_rate_limit: ntt.inbox_rate_limit(chain_id), peer: transfer.peer, - custody: ntt.custody(&transfer.mint), session_authority, }; Instruction { @@ -90,5 +90,6 @@ fn common(ntt: &NTT, transfer: &Transfer) -> example_native_token_transfers::acc outbox_item: transfer.outbox_item, outbox_rate_limit: ntt.outbox_rate_limit(), system_program: System::id(), + custody: ntt.custody(&transfer.mint), } } diff --git a/solana/tests/example-native-token-transfer.ts b/solana/tests/example-native-token-transfer.ts index 4221419e6..9d87a84b1 100644 --- a/solana/tests/example-native-token-transfer.ts +++ b/solana/tests/example-native-token-transfer.ts @@ -153,7 +153,7 @@ describe("example-native-token-transfers", () => { await sendAndConfirmTransaction(connection, transaction, [payer]); }); - describe("Locking", () => { + describe("Burning", () => { before(async () => { await spl.setAuthority( connection, @@ -173,7 +173,7 @@ describe("example-native-token-transfers", () => { chain: "solana", mint: mint.publicKey, outboundLimit: new BN(1000000), - mode: "locking", + mode: "burning", }); await ntt.registerTransceiver({ @@ -241,19 +241,6 @@ describe("example-native-token-transfers", () => { const balance = await connection.getTokenAccountBalance(tokenAccount); expect(balance.value.amount).to.equal("9900000"); - // grab logs - // await connection.confirmTransaction(redeemTx, 'confirmed'); - // const tx = await anchor.getProvider().connection.getParsedTransaction(redeemTx, { - // commitment: "confirmed", - // }); - // console.log(tx); - - // const log = tx.meta.logMessages[1]; - // const message = log.substring(log.indexOf(':') + 1); - // console.log(message); - - // TODO: assert other stuff in the message - // console.log(nttManagerMessage); expect((await counterValue()).toString()).to.be.eq("1") }); @@ -316,17 +303,4 @@ describe("example-native-token-transfers", () => { expect((await counterValue()).toString()).to.be.eq("2") }); }); - - // describe('Burning', () => { - // beforeEach(async () => { - // await ntt.initialize({ - // payer, - // owner, - // chain: 'solana', - // mint, - // outboundLimit: new BN(1000000), - // mode: 'burning' - // }) - // }); - // }); }); diff --git a/solana/ts/sdk/ntt.ts b/solana/ts/sdk/ntt.ts index b729c26b7..26b6299f2 100644 --- a/solana/ts/sdk/ntt.ts +++ b/solana/ts/sdk/ntt.ts @@ -11,7 +11,6 @@ import { } from './nttLayout' import { derivePostedVaaKey, getWormholeDerivedAccounts } from '@certusone/wormhole-sdk/lib/cjs/solana/wormhole' import { BN, translateError, type IdlAccounts, Program, AnchorProvider, Wallet, } from '@coral-xyz/anchor' -import { associatedAddress } from '@coral-xyz/anchor/dist/cjs/utils/token' import { getAssociatedTokenAddressSync } from '@solana/spl-token' import { PublicKey, Keypair, @@ -22,7 +21,9 @@ import { type Connection, SystemProgram, TransactionMessage, - VersionedTransaction + VersionedTransaction, + Commitment, + AccountMeta } from '@solana/web3.js' import { Keccak } from 'sha3' import { type ExampleNativeTokenTransfers as RawExampleNativeTokenTransfers } from '../../idl/ts/example_native_token_transfers' @@ -208,7 +209,7 @@ export class NTT { // little endian length prefix. const buffer = Buffer.from(txSimulation.value.returnData?.data[0], 'base64') const len = buffer.readUInt32LE(0) - return buffer.slice(4, len + 4).toString() + return buffer.subarray(4, len + 4).toString() } // Instructions @@ -354,22 +355,58 @@ export class NTT { shouldQueue: args.shouldQueue } - return await this.program.methods + const transferIx = await this.program.methods .transferBurn(transferArgs) - .accounts({ + .accountsStrict({ common: { payer: args.payer, config: { config: this.configAccountAddress() }, mint, from: args.from, + tokenProgram: await this.tokenProgram(config), outboxItem: args.outboxItem, - outboxRateLimit: this.outboxRateLimitAccountAddress() + outboxRateLimit: this.outboxRateLimitAccountAddress(), + custody: await this.custodyAccountAddress(config), + systemProgram: SystemProgram.programId, }, peer: this.peerAccountAddress(args.recipientChain), inboxRateLimit: this.inboxRateLimitAccountAddress(args.recipientChain), - sessionAuthority: this.sessionAuthorityAddress(args.fromAuthority, transferArgs) + sessionAuthority: this.sessionAuthorityAddress(args.fromAuthority, transferArgs), + tokenAuthority: this.tokenAuthorityAddress() }) .instruction() + + const mintInfo = await splToken.getMint( + this.program.provider.connection, + config.mint, + undefined, + config.tokenProgram + ) + const transferHook = splToken.getTransferHook(mintInfo) + + if (transferHook) { + const source = args.from + const mint = config.mint + const destination = await this.custodyAccountAddress(config) + const owner = this.sessionAuthorityAddress(args.fromAuthority, transferArgs) + await addExtraAccountMetasForExecute( + this.program.provider.connection, + transferIx, + transferHook.programId, + source, + mint, + destination, + owner, + // TODO(csongor): compute the amount that's passed into transfer. + // Leaving this 0 is fine unless the transfer hook accounts addresses + // depend on the amount (which is unlikely). + // If this turns out to be the case, the amount to put here is the + // untrimmed amount after removing dust. + 0, + ); + } + + return transferIx } /** @@ -413,11 +450,11 @@ export class NTT { from: args.from, tokenProgram: await this.tokenProgram(config), outboxItem: args.outboxItem, - outboxRateLimit: this.outboxRateLimitAccountAddress() + outboxRateLimit: this.outboxRateLimitAccountAddress(), + custody: await this.custodyAccountAddress(config) }, peer: this.peerAccountAddress(args.recipientChain), inboxRateLimit: this.inboxRateLimitAccountAddress(args.recipientChain), - custody: await this.custodyAccountAddress(config), sessionAuthority: this.sessionAuthorityAddress(args.fromAuthority, transferArgs) }) .instruction() @@ -530,7 +567,7 @@ export class NTT { const mint = await this.mintAccountAddress(config) - return await this.program.methods + const transferIx = await this.program.methods .releaseInboundMint({ revertOnDelay: args.revertOnDelay }) @@ -542,10 +579,38 @@ export class NTT { recipient: getAssociatedTokenAddressSync(mint, recipientAddress, true, config.tokenProgram), mint, tokenAuthority: this.tokenAuthorityAddress(), - tokenProgram: config.tokenProgram + tokenProgram: config.tokenProgram, + custody: await this.custodyAccountAddress(config) } }) .instruction() + + const mintInfo = await splToken.getMint(this.program.provider.connection, config.mint, undefined, config.tokenProgram) + const transferHook = splToken.getTransferHook(mintInfo) + + if (transferHook) { + const source = await this.custodyAccountAddress(config) + const mint = config.mint + const destination = getAssociatedTokenAddressSync(mint, recipientAddress, true, config.tokenProgram) + const owner = this.tokenAuthorityAddress() + await addExtraAccountMetasForExecute( + this.program.provider.connection, + transferIx, + transferHook.programId, + source, + mint, + destination, + owner, + // TODO(csongor): compute the amount that's passed into transfer. + // Leaving this 0 is fine unless the transfer hook accounts addresses + // depend on the amount (which is unlikely). + // If this turns out to be the case, the amount to put here is the + // untrimmed amount after removing dust. + 0, + ); + } + + return transferIx } async releaseInboundMint(args: { @@ -602,9 +667,9 @@ export class NTT { recipient: getAssociatedTokenAddressSync(mint, recipientAddress, true, config.tokenProgram), mint, tokenAuthority: this.tokenAuthorityAddress(), - tokenProgram: config.tokenProgram + tokenProgram: config.tokenProgram, + custody: await this.custodyAccountAddress(config) }, - custody: await this.custodyAccountAddress(config) }) .instruction()