solana: introduce "session authority" to remove atomic approval requirement (#304)

* solana: introduce "session authority" to remove atomic approval requirement

* solana: remove sender, just compute from the token account owner

Author: kcsongor

solana/.eslintrc.json

@@ -9,11 +9,17 @@
         "ecmaVersion": "latest",
         "sourceType": "module"
     },
-    plugins: [
-        '@stylistic'
+    "plugins": [
+        "@stylistic"
     ],
     "rules": {
-        '@stylistic/indent': ['error', 2],
-        '@stylistic/max-len': ['error', 80],
+        "@stylistic/indent": [
+            "error",
+            2
+        ],
+        "@stylistic/max-len": [
+            "error",
+            80
+        ]
     }
 }

solana/package-lock.json

@@ -7,11 +7,11 @@
     },
     "dependencies": {
         "@certusone/wormhole-sdk": "^0.10.10",
-        "@wormhole-foundation/sdk-base": "^0.5.0-beta.9",
-        "@wormhole-foundation/sdk-definitions": "^0.5.0-beta.9",
         "@coral-xyz/anchor": "^0.29.0",
         "@solana/spl-token": "^0.4.0",
         "@solana/web3.js": "^1.90.0",
+        "@wormhole-foundation/sdk-base": "^0.5.0-beta.9",
+        "@wormhole-foundation/sdk-definitions": "^0.5.0-beta.9",
         "dotenv": "^16.4.5",
         "sha3": "^2.1.4"
     },
@@ -32,6 +32,6 @@
         "mocha": "^9.0.3",
         "prettier": "^2.6.2",
         "ts-mocha": "^10.0.0",
-        "typescript": "^4.9.5"
+        "typescript": "^5.4.2"
     }
 }

solana/package.json

@@ -24,9 +24,6 @@ pub struct Transfer<'info> {
 
     pub config: NotPausedConfig<'info>,
 
-    /// This signer will be encoded in the outbox.
-    sender: Signer<'info>,
-
     #[account(
         mut,
         address = config.mint,
@@ -38,6 +35,8 @@ pub struct Transfer<'info> {
         mut,
         token::mint = mint,
     )]
+    /// CHECK: the spl token program will check that the session_authority
+    ///        account can spend these tokens.
     pub from: InterfaceAccount<'info, token_interface::TokenAccount>,
 
     pub token_program: Interface<'info, token_interface::TokenInterface>,
@@ -52,14 +51,6 @@ pub struct Transfer<'info> {
     #[account(mut)]
     pub outbox_rate_limit: Account<'info, OutboxRateLimit>,
 
-    /// CHECK: This authority will need to have been delegated authority to
-    /// transfer or burn tokens in the [from](Self::from) account.
-    #[account(
-        seeds = [crate::TOKEN_AUTHORITY_SEED],
-        bump,
-    )]
-    token_authority: AccountInfo<'info>,
-
     pub system_program: Program<'info, System>,
 }
 
@@ -71,6 +62,23 @@ pub struct TransferArgs {
     pub should_queue: bool,
 }
 
+impl TransferArgs {
+    pub fn keccak256(&self) -> solana_program::keccak::Hash {
+        let TransferArgs {
+            amount,
+            recipient_chain,
+            recipient_address,
+            should_queue,
+        } = self;
+        solana_program::keccak::hashv(&[
+            amount.to_be_bytes().as_ref(),
+            recipient_chain.id.to_be_bytes().as_ref(),
+            recipient_address,
+            &[*should_queue as u8],
+        ])
+    }
+}
+
 // Burn/mint
 
 #[derive(Accounts)]
@@ -92,9 +100,18 @@ pub struct TransferBurn<'info> {
         bump = peer.bump,
     )]
     pub peer: Account<'info, NttManagerPeer>,
+
+    #[account(
+        seeds = [
+            crate::SESSION_AUTHORITY_SEED,
+            common.from.owner.as_ref(),
+            args.keccak256().as_ref()
+        ],
+        bump,
+    )]
+    pub session_authority: AccountInfo<'info>,
 }
 
-// TODO: fees for relaying?
 pub fn transfer_burn(ctx: Context<TransferBurn>, args: TransferArgs) -> Result<()> {
     require_eq!(
         ctx.accounts.common.config.mode,
@@ -123,11 +140,13 @@ pub fn transfer_burn(ctx: Context<TransferBurn>, args: TransferArgs) -> Result<(
             token_interface::Burn {
                 mint: accs.common.mint.to_account_info(),
                 from: accs.common.from.to_account_info(),
-                authority: accs.common.token_authority.to_account_info(),
+                authority: accs.session_authority.to_account_info(),
             },
             &[&[
-                crate::TOKEN_AUTHORITY_SEED,
-                &[ctx.bumps.common.token_authority],
+                crate::SESSION_AUTHORITY_SEED,
+                accs.common.from.owner.as_ref(),
+                args.keccak256().as_ref(),
+                &[ctx.bumps.session_authority],
             ]],
         ),
         amount,
@@ -169,15 +188,23 @@ pub struct TransferLock<'info> {
     )]
     pub peer: Account<'info, NttManagerPeer>,
 
+    #[account(
+        seeds = [
+            crate::SESSION_AUTHORITY_SEED,
+            common.from.owner.as_ref(),
+            args.keccak256().as_ref()
+        ],
+        bump,
+    )]
+    pub session_authority: AccountInfo<'info>,
+
     #[account(
         mut,
         address = common.config.custody
     )]
     pub custody: InterfaceAccount<'info, token_interface::TokenAccount>,
 }
 
-// TODO: fees for relaying?
-// TODO: factor out common bits
 pub fn transfer_lock(ctx: Context<TransferLock>, args: TransferArgs) -> Result<()> {
     require_eq!(
         ctx.accounts.common.config.mode,
@@ -206,12 +233,14 @@ pub fn transfer_lock(ctx: Context<TransferLock>, args: TransferArgs) -> Result<(
             token_interface::TransferChecked {
                 from: accs.common.from.to_account_info(),
                 to: accs.custody.to_account_info(),
-                authority: accs.common.token_authority.to_account_info(),
+                authority: accs.session_authority.to_account_info(),
                 mint: accs.common.mint.to_account_info(),
             },
             &[&[
-                crate::TOKEN_AUTHORITY_SEED,
-                &[ctx.bumps.common.token_authority],
+                crate::SESSION_AUTHORITY_SEED,
+                accs.common.from.owner.as_ref(),
+                args.keccak256().as_ref(),
+                &[ctx.bumps.session_authority],
             ]],
         ),
         amount,
@@ -260,7 +289,7 @@ fn insert_into_outbox(
 
     common.outbox_item.set_inner(OutboxItem {
         amount: trimmed_amount,
-        sender: common.sender.key(),
+        sender: common.from.owner,
         recipient_chain,
         recipient_ntt_manager,
         recipient_address,

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

@@ -36,7 +36,31 @@ cfg_if::cfg_if! {
     }
 }
 
-const TOKEN_AUTHORITY_SEED: &[u8] = b"token_authority";
+pub const TOKEN_AUTHORITY_SEED: &[u8] = b"token_authority";
+
+/// The seed for the session authority account.
+///
+/// These accounts are used in the `transfer_*` instructions. The user first
+/// approves the session authority to spend the tokens, and then the session
+/// authority burns or locks the tokens.
+/// This is to avoid the user having to pass their own authority to the program,
+/// which in general is dangerous, especially for upgradeable programs.
+///
+/// There is a session authority associated with each transfer, and is seeded by
+/// the sender's pubkey, and (the hash of) all the transfer arguments.
+/// These seeds essentially encode the user's intent when approving the
+/// spending.
+///
+/// In practice, the approve instruction is going to be atomically bundled with
+/// the transfer instruction, so this encoding makes no difference.
+/// However, it does allow it to be done in a separate transaction without the
+/// risk of a malicious actor redirecting the funds by frontrunning the transfer
+/// instruction.
+/// In other words, the transfer instruction has no degrees of freedom; all the
+/// arguments are determined in the approval step. Then transfer can be
+/// permissionlessly invoked by anyone (even if in practice it's going to be the
+/// user, atomically).
+pub const SESSION_AUTHORITY_SEED: &[u8] = b"session_authority";
 
 #[program]
 pub mod example_native_token_transfers {

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

@@ -219,13 +219,13 @@ async fn test_cancel() {
         &test_data.ntt,
         &test_data.user_token_account,
         &test_data.user.pubkey(),
-        args.amount,
+        &args,
     )
     .submit_with_signers(&[&test_data.user], &mut ctx)
     .await
     .unwrap();
     transfer(&test_data.ntt, accs, args, Mode::Locking)
-        .submit_with_signers(&[&test_data.user, &outbox_item], &mut ctx)
+        .submit_with_signers(&[&outbox_item], &mut ctx)
         .await
         .unwrap();
 

solana/programs/example-native-token-transfers/tests/cancel_flow.rs

@@ -1,11 +1,13 @@
 use anchor_lang::prelude::Pubkey;
 use example_native_token_transfers::{
     config::Config,
+    instructions::TransferArgs,
     queue::{
         inbox::{InboxItem, InboxRateLimit},
         outbox::OutboxRateLimit,
     },
     registered_transceiver::RegisteredTransceiver,
+    SESSION_AUTHORITY_SEED, TOKEN_AUTHORITY_SEED,
 };
 use ntt_messages::{ntt::NativeTokenTransfer, ntt_manager::NttManagerMessage};
 use sha3::{Digest, Keccak256};
@@ -89,6 +91,31 @@ impl NTT {
         inbox_rate_limit
     }
 
+    pub fn session_authority(&self, sender: &Pubkey, args: &TransferArgs) -> Pubkey {
+        let TransferArgs {
+            amount,
+            recipient_chain,
+            recipient_address,
+            should_queue,
+        } = args;
+        let mut hasher = Keccak256::new();
+
+        hasher.update(&amount.to_be_bytes());
+        hasher.update(&recipient_chain.id.to_be_bytes());
+        hasher.update(&recipient_address);
+        hasher.update(&[*should_queue as u8]);
+
+        let (session_authority, _) = Pubkey::find_program_address(
+            &[
+                SESSION_AUTHORITY_SEED.as_ref(),
+                sender.as_ref(),
+                &hasher.finalize(),
+            ],
+            &self.program,
+        );
+        session_authority
+    }
+
     pub fn inbox_item(
         &self,
         chain: u16,
@@ -107,7 +134,7 @@ impl NTT {
 
     pub fn token_authority(&self) -> Pubkey {
         let (token_authority, _) =
-            Pubkey::find_program_address(&[b"token_authority".as_ref()], &self.program);
+            Pubkey::find_program_address(&[TOKEN_AUTHORITY_SEED.as_ref()], &self.program);
         token_authority
     }
 

solana/programs/example-native-token-transfers/tests/sdk/accounts.rs

@@ -25,12 +25,14 @@ pub fn transfer(ntt: &NTT, transfer: Transfer, args: TransferArgs, mode: Mode) -
 
 pub fn transfer_burn(ntt: &NTT, transfer: Transfer, args: TransferArgs) -> Instruction {
     let chain_id = args.recipient_chain.id;
+    let session_authority = ntt.session_authority(&transfer.from_authority, &args);
     let data = example_native_token_transfers::instruction::TransferBurn { args };
 
     let accounts = example_native_token_transfers::accounts::TransferBurn {
         common: common(ntt, &transfer),
         inbox_rate_limit: ntt.inbox_rate_limit(chain_id),
         peer: transfer.peer,
+        session_authority,
     };
 
     Instruction {
@@ -42,13 +44,15 @@ pub fn transfer_burn(ntt: &NTT, transfer: Transfer, args: TransferArgs) -> Instr
 
 pub fn transfer_lock(ntt: &NTT, transfer: Transfer, args: TransferArgs) -> Instruction {
     let chain_id = args.recipient_chain.id;
+    let session_authority = ntt.session_authority(&transfer.from_authority, &args);
     let data = example_native_token_transfers::instruction::TransferLock { args };
 
     let accounts = example_native_token_transfers::accounts::TransferLock {
         common: common(ntt, &transfer),
         inbox_rate_limit: ntt.inbox_rate_limit(chain_id),
         peer: transfer.peer,
         custody: ntt.custody(&transfer.mint),
+        session_authority,
     };
     Instruction {
         program_id: example_native_token_transfers::ID,
@@ -61,15 +65,15 @@ pub fn approve_token_authority(
     ntt: &NTT,
     user_token_account: &Pubkey,
     user: &Pubkey,
-    amount: u64,
+    args: &TransferArgs,
 ) -> Instruction {
     spl_token_2022::instruction::approve(
         &spl_token::id(), // TODO: look into how token account was originally created
         user_token_account,
-        &ntt.token_authority(),
+        &ntt.session_authority(user, args),
         user,
         &[user],
-        amount,
+        args.amount,
     )
     .unwrap()
 }
@@ -80,13 +84,11 @@ fn common(ntt: &NTT, transfer: &Transfer) -> example_native_token_transfers::acc
         config: NotPausedConfig {
             config: ntt.config(),
         },
-        sender: transfer.from_authority,
         mint: transfer.mint,
         from: transfer.from,
         token_program: Token::id(),
         outbox_item: transfer.outbox_item,
         outbox_rate_limit: ntt.outbox_rate_limit(),
-        token_authority: ntt.token_authority(),
         system_program: System::id(),
     }
 }