solana: Remove *_multisig instruction variants (wormhole-foundation#593)

* Replace `initialize_multisig` ix with optional multisig account in `Initialize`

* Replace `release_inbound_mint_multisig` ix with optional multisig account in `ReleaseInboundMint`

* Update cargo test to match new `Initialize` struct

* Remove redundant `.to_account_info()` call

* Update IDL

* Update TS test case and SDK

* Derive multisigTokenAuthority from token mint info

* solana: Fix logic error in Inbox release process

There are a few suspicious aspects of the inbox release logic:
1. `release_inbound_unlock` takes a flag that determines whether a
   `false` result from `try_release` should cause an error. The flag
   indicates that it should revert on delay. However, `try_release` can
   also return `false` when a transaction has not been approved. If the
   flag is set, the transaction will not return an error when a user
   tries to release a transaction that is not approved even though
   whether a transaction is approved has nothing to do with its expected
   behaviour when it is delayed
2. The custom error TransferNotApproved is defined but unused

Taken together, I believe `try_release()` should revert with the
TransferNotApproved error. This would disambiguate the various 'false'
results and provide more intuitive behaviour when a user tries to
release a transaction that is not approved.

* solana: change comment to revert_on_delay

* solana: Let redeem code return error instead of try_release

* nit: Fix grammar

* Update IDL

* Update TS SDK to use renamed args

* Update comment to match function logic

---------

Co-authored-by: John Saigle <[email protected]>

Author: nvsriram

solana/programs/example-native-token-transfers/src/instructions/admin/transfer_token_authority.rs

@@ -349,10 +349,10 @@ fn claim_from_token_authority<'info>(
 ) -> Result<()> {
     token_interface::set_authority(
         CpiContext::new_with_signer(
-            token_program.to_account_info(),
+            token_program,
             token_interface::SetAuthority {
-                account_or_mint: mint.to_account_info(),
-                current_authority: token_authority.to_account_info(),
+                account_or_mint: mint,
+                current_authority: token_authority,
             },
             &[&[crate::TOKEN_AUTHORITY_SEED, &[token_authority_bump]]],
         ),

solana/programs/example-native-token-transfers/src/instructions/initialize.rs

@@ -8,12 +8,14 @@ use crate::messages::Hack;
 
 use crate::{
     bitmap::Bitmap,
+    config::Config,
     error::NTTError,
     queue::{outbox::OutboxRateLimit, rate_limit::RateLimitState},
     spl_multisig::SplMultisig,
 };
 
 #[derive(Accounts)]
+#[instruction(args: InitializeArgs)]
 pub struct Initialize<'info> {
     #[account(mut)]
     pub payer: Signer<'info>,
@@ -30,15 +32,20 @@ pub struct Initialize<'info> {
 
     #[account(
         init,
-        space = 8 + crate::config::Config::INIT_SPACE,
+        space = 8 + Config::INIT_SPACE,
         payer = payer,
-        seeds = [crate::config::Config::SEED_PREFIX],
+        seeds = [Config::SEED_PREFIX],
         bump
     )]
-    pub config: Box<Account<'info, crate::config::Config>>,
+    pub config: Box<Account<'info, Config>>,
 
-    // NOTE: this account is unconstrained and is the responsibility of the
-    // handler to constrain it
+    #[account(
+        constraint = args.mode == Mode::Locking
+            || mint.mint_authority.unwrap() == multisig_token_authority.as_ref().map_or(
+                token_authority.key(),
+                |multisig_token_authority| multisig_token_authority.key()
+            ) @ NTTError::InvalidMintAuthority
+    )]
     pub mint: Box<InterfaceAccount<'info, token_interface::Mint>>,
 
     #[account(
@@ -62,6 +69,13 @@ pub struct Initialize<'info> {
     /// Could refactor code to use `Box<_>` to reduce stack size.
     pub token_authority: AccountInfo<'info>,
 
+    #[account(
+        constraint = multisig_token_authority.m == 1
+            && multisig_token_authority.signers.contains(&token_authority.key())
+            @ NTTError::InvalidMultisig,
+    )]
+    pub multisig_token_authority: Option<Box<InterfaceAccount<'info, SplMultisig>>>,
+
     #[account(
         init_if_needed,
         payer = payer,
@@ -92,14 +106,6 @@ pub struct InitializeArgs {
 }
 
 pub fn initialize(ctx: Context<Initialize>, args: InitializeArgs) -> Result<()> {
-    // NOTE: this check was moved into the function body to reuse the `Initialize` struct
-    // in the multisig variant while preserving ABI
-    if args.mode == Mode::Burning
-        && ctx.accounts.mint.mint_authority.unwrap() != ctx.accounts.token_authority.key()
-    {
-        return Err(NTTError::InvalidMintAuthority.into());
-    }
-
     initialize_config_and_rate_limit(
         ctx.accounts,
         ctx.bumps.config,
@@ -109,35 +115,6 @@ pub fn initialize(ctx: Context<Initialize>, args: InitializeArgs) -> Result<()>
     )
 }
 
-#[derive(Accounts)]
-#[instruction(args: InitializeArgs)]
-pub struct InitializeMultisig<'info> {
-    #[account(
-        constraint =
-            args.mode == Mode::Locking
-            || common.mint.mint_authority.unwrap() == multisig.key()
-            @ NTTError::InvalidMintAuthority,
-    )]
-    pub common: Initialize<'info>,
-
-    #[account(
-        constraint =
-            multisig.m == 1 && multisig.signers.contains(&common.token_authority.key())
-            @ NTTError::InvalidMultisig,
-    )]
-    pub multisig: InterfaceAccount<'info, SplMultisig>,
-}
-
-pub fn initialize_multisig(ctx: Context<InitializeMultisig>, args: InitializeArgs) -> Result<()> {
-    initialize_config_and_rate_limit(
-        &mut ctx.accounts.common,
-        ctx.bumps.common.config,
-        args.chain_id,
-        args.limit,
-        args.mode,
-    )
-}
-
 fn initialize_config_and_rate_limit(
     common: &mut Initialize<'_>,
     config_bump: u8,

solana/programs/example-native-token-transfers/src/instructions/release_inbound.rs

@@ -54,7 +54,7 @@ pub struct ReleaseInbound<'info> {
 
 #[derive(AnchorDeserialize, AnchorSerialize)]
 pub struct ReleaseInboundArgs {
-    pub revert_on_delay: bool,
+    pub revert_when_not_ready: bool,
 }
 
 // Burn/mint
@@ -65,11 +65,18 @@ pub struct ReleaseInboundMint<'info> {
         constraint = common.config.mode == Mode::Burning @ NTTError::InvalidMode,
     )]
     common: ReleaseInbound<'info>,
+
+    #[account(
+        constraint = multisig_token_authority.m == 1
+            && multisig_token_authority.signers.contains(&common.token_authority.key())
+            @ NTTError::InvalidMultisig,
+    )]
+    pub multisig_token_authority: Option<InterfaceAccount<'info, SplMultisig>>,
 }
 
 /// Release an inbound transfer and mint the tokens to the recipient.
-/// When `revert_on_error` is true, the transaction will revert if the
-/// release timestamp has not been reached. When `revert_on_error` is false, the
+/// When `revert_when_not_ready` is true, the transaction will revert if the
+/// release timestamp has not been reached. When `revert_when_not_ready` is false, the
 /// transaction succeeds, but the minting is not performed.
 /// Setting this flag to `false` is useful when bundling this instruction
 /// together with [`crate::instructions::redeem`] in a transaction, so that the minting
@@ -78,7 +85,10 @@ pub fn release_inbound_mint<'info>(
     ctx: Context<'_, '_, '_, 'info, ReleaseInboundMint<'info>>,
     args: ReleaseInboundArgs,
 ) -> Result<()> {
-    let inbox_item = release_inbox_item(&mut ctx.accounts.common.inbox_item, args.revert_on_delay)?;
+    let inbox_item = release_inbox_item(
+        &mut ctx.accounts.common.inbox_item,
+        args.revert_when_not_ready,
+    )?;
     if inbox_item.is_none() {
         return Ok(());
     }
@@ -106,18 +116,25 @@ pub fn release_inbound_mint<'info>(
     ]];
 
     // Step 1: mint tokens to the custody account
-    token_interface::mint_to(
-        CpiContext::new_with_signer(
+    match &ctx.accounts.multisig_token_authority {
+        Some(multisig_token_authority) => mint_to_custody_from_multisig_token_authority(
             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.to_account_info(),
-            },
+            ctx.accounts.common.mint.to_account_info(),
+            ctx.accounts.common.custody.to_account_info(),
+            multisig_token_authority.to_account_info(),
+            ctx.accounts.common.token_authority.to_account_info(),
             token_authority_sig,
-        ),
-        inbox_item.amount,
-    )?;
+            inbox_item.amount,
+        )?,
+        None => mint_to_custody_from_token_authority(
+            ctx.accounts.common.token_program.to_account_info(),
+            ctx.accounts.common.mint.to_account_info(),
+            ctx.accounts.common.custody.to_account_info(),
+            ctx.accounts.common.token_authority.to_account_info(),
+            token_authority_sig,
+            inbox_item.amount,
+        )?,
+    };
 
     // Step 2: transfer the tokens from the custody account to the recipient
     onchain::invoke_transfer_checked(
@@ -134,82 +151,49 @@ pub fn release_inbound_mint<'info>(
     Ok(())
 }
 
-#[derive(Accounts)]
-pub struct ReleaseInboundMintMultisig<'info> {
-    #[account(
-        constraint = common.config.mode == Mode::Burning @ NTTError::InvalidMode,
-    )]
-    common: ReleaseInbound<'info>,
-
-    #[account(
-        constraint =
-         multisig.m == 1 && multisig.signers.contains(&common.token_authority.key())
-            @ NTTError::InvalidMultisig,
-    )]
-    pub multisig: InterfaceAccount<'info, SplMultisig>,
+fn mint_to_custody_from_token_authority<'info>(
+    token_program: AccountInfo<'info>,
+    mint: AccountInfo<'info>,
+    custody: AccountInfo<'info>,
+    token_authority: AccountInfo<'info>,
+    token_authority_signer_seeds: &[&[&[u8]]],
+    amount: u64,
+) -> Result<()> {
+    token_interface::mint_to(
+        CpiContext::new_with_signer(
+            token_program,
+            token_interface::MintTo {
+                mint,
+                to: custody,
+                authority: token_authority,
+            },
+            token_authority_signer_seeds,
+        ),
+        amount,
+    )?;
+    Ok(())
 }
 
-pub fn release_inbound_mint_multisig<'info>(
-    ctx: Context<'_, '_, '_, 'info, ReleaseInboundMintMultisig<'info>>,
-    args: ReleaseInboundArgs,
+fn mint_to_custody_from_multisig_token_authority<'info>(
+    token_program: AccountInfo<'info>,
+    mint: AccountInfo<'info>,
+    custody: AccountInfo<'info>,
+    multisig_token_authority: AccountInfo<'info>,
+    token_authority: AccountInfo<'info>,
+    token_authority_signer_seeds: &[&[&[u8]]],
+    amount: u64,
 ) -> Result<()> {
-    let inbox_item = release_inbox_item(&mut ctx.accounts.common.inbox_item, args.revert_on_delay)?;
-    if inbox_item.is_none() {
-        return Ok(());
-    }
-    let inbox_item = inbox_item.unwrap();
-    assert!(inbox_item.release_status == ReleaseStatus::Released);
-
-    // 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).
-
-    let token_authority_sig: &[&[&[u8]]] = &[&[
-        crate::TOKEN_AUTHORITY_SEED,
-        &[ctx.bumps.common.token_authority],
-    ]];
-
-    // Step 1: mint tokens to the custody account
     solana_program::program::invoke_signed(
         &spl_token_2022::instruction::mint_to(
-            &ctx.accounts.common.token_program.key(),
-            &ctx.accounts.common.mint.key(),
-            &ctx.accounts.common.custody.key(),
-            &ctx.accounts.multisig.key(),
-            &[&ctx.accounts.common.token_authority.key()],
-            inbox_item.amount,
+            &token_program.key(),
+            &mint.key(),
+            &custody.key(),
+            &multisig_token_authority.key(),
+            &[&token_authority.key()],
+            amount,
         )?,
-        &[
-            ctx.accounts.common.custody.to_account_info(),
-            ctx.accounts.common.mint.to_account_info(),
-            ctx.accounts.common.token_authority.to_account_info(),
-            ctx.accounts.multisig.to_account_info(),
-        ],
-        token_authority_sig,
-    )?;
-
-    // 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.to_account_info(),
-        ctx.remaining_accounts,
-        inbox_item.amount,
-        ctx.accounts.common.mint.decimals,
-        token_authority_sig,
+        &[custody, mint, token_authority, multisig_token_authority],
+        token_authority_signer_seeds,
     )?;
     Ok(())
 }
@@ -225,8 +209,8 @@ pub struct ReleaseInboundUnlock<'info> {
 }
 
 /// Release an inbound transfer and unlock the tokens to the recipient.
-/// When `revert_on_error` is true, the transaction will revert if the
-/// release timestamp has not been reached. When `revert_on_error` is false, the
+/// When `revert_when_not_ready` is true, the transaction will revert if the
+/// release timestamp has not been reached. When `revert_when_not_ready` is false, the
 /// transaction succeeds, but the unlocking is not performed.
 /// Setting this flag to `false` is useful when bundling this instruction
 /// together with [`crate::instructions::redeem`], so that the unlocking
@@ -235,7 +219,10 @@ pub fn release_inbound_unlock<'info>(
     ctx: Context<'_, '_, '_, 'info, ReleaseInboundUnlock<'info>>,
     args: ReleaseInboundArgs,
 ) -> Result<()> {
-    let inbox_item = release_inbox_item(&mut ctx.accounts.common.inbox_item, args.revert_on_delay)?;
+    let inbox_item = release_inbox_item(
+        &mut ctx.accounts.common.inbox_item,
+        args.revert_when_not_ready,
+    )?;
     if inbox_item.is_none() {
         return Ok(());
     }
@@ -258,14 +245,21 @@ pub fn release_inbound_unlock<'info>(
     )?;
     Ok(())
 }
+
 fn release_inbox_item(
     inbox_item: &mut InboxItem,
-    revert_on_delay: bool,
+    revert_when_not_ready: bool,
 ) -> Result<Option<&mut InboxItem>> {
     if inbox_item.try_release()? {
         Ok(Some(inbox_item))
-    } else if revert_on_delay {
-        Err(NTTError::CantReleaseYet.into())
+    } else if revert_when_not_ready {
+        match inbox_item.release_status {
+            ReleaseStatus::NotApproved => Err(NTTError::TransferNotApproved.into()),
+            ReleaseStatus::ReleaseAfter(_) => Err(NTTError::CantReleaseYet.into()),
+            // Unreachable: if released, [`InboxItem::try_release`] will return an Error immediately
+            // rather than Ok(bool).
+            ReleaseStatus::Released => Err(NTTError::TransferAlreadyRedeemed.into()),
+        }
     } else {
         Ok(None)
     }

solana/programs/example-native-token-transfers/src/lib.rs

@@ -76,13 +76,6 @@ pub mod example_native_token_transfers {
         instructions::initialize(ctx, args)
     }
 
-    pub fn initialize_multisig(
-        ctx: Context<InitializeMultisig>,
-        args: InitializeArgs,
-    ) -> Result<()> {
-        instructions::initialize_multisig(ctx, args)
-    }
-
     pub fn initialize_lut(ctx: Context<InitializeLUT>, recent_slot: u64) -> Result<()> {
         instructions::initialize_lut(ctx, recent_slot)
     }
@@ -116,13 +109,6 @@ pub mod example_native_token_transfers {
         instructions::release_inbound_mint(ctx, args)
     }
 
-    pub fn release_inbound_mint_multisig<'info>(
-        ctx: Context<'_, '_, '_, 'info, ReleaseInboundMintMultisig<'info>>,
-        args: ReleaseInboundArgs,
-    ) -> Result<()> {
-        instructions::release_inbound_mint_multisig(ctx, args)
-    }
-
     pub fn release_inbound_unlock<'info>(
         ctx: Context<'_, '_, '_, 'info, ReleaseInboundUnlock<'info>>,
         args: ReleaseInboundArgs,