solana: safer transfer authority (#153)

Co-authored-by: A5 Pickle <[email protected]>

Author: a5-pickle

.github/workflows/solana.yml

@@ -43,8 +43,8 @@ jobs:
       - name: make lint
         run: make lint
         working-directory: ./solana
-  make-anchor-build-idl:
-    name: make idl
+  make-check-idl:
+    name: make check-idl
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v4
@@ -56,8 +56,8 @@ jobs:
       - name: Set default Rust toolchain
         run: rustup default stable
         working-directory: ./solana
-      - name: make idl
-        run: make idl
+      - name: make check-idl
+        run: make check-idl
         working-directory: ./solana
   make-anchor-test:
     name: make anchor-test

solana/Makefile

@@ -56,6 +56,9 @@ idl:
 	mkdir -p ts/src/idl/ts
 	cp -r target/idl/* ts/src/idl/json/
 	cp -r target/types/* ts/src/idl/ts/
+
+.PHONY: check-idl
+check-idl: idl
 	git diff --exit-code
 
 $(BUILD_$(NETWORK)): cargo-test

solana/programs/token-router/src/error.rs

@@ -26,8 +26,8 @@ pub enum TokenRouterError {
     AlreadyOwner = 0x204,
     NoTransferOwnershipRequest = 0x206,
     NotPendingOwner = 0x208,
-    MissingAuthority = 0x20a,
-    TooManyAuthorities = 0x20b,
+    EitherSenderOrProgramTransferAuthority = 0x20a,
+    DelegatedAmountMismatch = 0x20c,
 
     InsufficientAmount = 0x400,
     MinAmountOutTooHigh = 0x402,

solana/programs/token-router/src/processor/market_order/prepare.rs

@@ -18,7 +18,10 @@ pub struct PrepareMarketOrder<'info> {
     custodian: CheckedCustodian<'info>,
 
     /// The auction participant needs to set approval to this PDA if the sender (signer) is not
-    /// provided.
+    /// provided. The delegated amount must equal the amount in or this instruction will revert.
+    ///
+    /// NOTE: If this account is provided, the sender token's owner will be encoded as the order
+    /// sender.
     ///
     /// CHECK: Seeds must be \["transfer-authority", prepared_order.key(), args.hash()\].
     #[account(
@@ -28,11 +31,22 @@ pub struct PrepareMarketOrder<'info> {
             &args.hash().0,
         ],
         bump,
+        constraint = {
+            require_eq!(
+                sender_token.delegated_amount,
+                args.amount_in,
+                TokenRouterError::DelegatedAmountMismatch,
+            );
+
+            true
+        }
     )]
     program_transfer_authority: Option<UncheckedAccount<'info>>,
 
     /// Sender, who has the authority to transfer assets from the sender token account. If this
     /// account is not provided, the program transfer authority account must be some account.
+    ///
+    /// NOTE: If this account is provided, this pubkey will be encoded as the order sender.
     sender: Option<Signer<'info>>,
 
     #[account(
@@ -161,10 +175,57 @@ pub fn prepare_market_order(
         redeemer_message,
     } = args;
 
+    // Finally transfer amount to custody token account. We perform exclusive or because we do not
+    // want to allow specifying more than one authority.
+    let order_sender = match (
+        ctx.accounts.sender.as_ref(),
+        ctx.accounts.program_transfer_authority.as_ref(),
+    ) {
+        (Some(sender), None) => {
+            token::transfer(
+                CpiContext::new(
+                    ctx.accounts.token_program.to_account_info(),
+                    token::Transfer {
+                        from: ctx.accounts.sender_token.to_account_info(),
+                        to: ctx.accounts.prepared_custody_token.to_account_info(),
+                        authority: sender.to_account_info(),
+                    },
+                ),
+                amount_in,
+            )?;
+
+            sender.key()
+        }
+        (None, Some(program_transfer_authority)) => {
+            let sender_token = &ctx.accounts.sender_token;
+
+            token::transfer(
+                CpiContext::new_with_signer(
+                    ctx.accounts.token_program.to_account_info(),
+                    token::Transfer {
+                        from: sender_token.to_account_info(),
+                        to: ctx.accounts.prepared_custody_token.to_account_info(),
+                        authority: program_transfer_authority.to_account_info(),
+                    },
+                    &[&[
+                        TRANSFER_AUTHORITY_SEED_PREFIX,
+                        ctx.accounts.prepared_order.key().as_ref(),
+                        &hashed_args.0,
+                        &[ctx.bumps.program_transfer_authority.unwrap()],
+                    ]],
+                ),
+                amount_in,
+            )?;
+
+            sender_token.owner
+        }
+        _ => return err!(TokenRouterError::EitherSenderOrProgramTransferAuthority),
+    };
+
     // Set the values in prepared order account.
     ctx.accounts.prepared_order.set_inner(PreparedOrder {
         info: PreparedOrderInfo {
-            order_sender: ctx.accounts.sender_token.owner,
+            order_sender,
             prepared_by: ctx.accounts.payer.key(),
             order_type: OrderType::Market { min_amount_out },
             src_token: ctx.accounts.sender_token.key(),
@@ -176,41 +237,6 @@ pub fn prepare_market_order(
         redeemer_message,
     });
 
-    // Finally transfer amount to custody token account. We perform exclusive or because we do not
-    // want to allow specifying more than one authority.
-    match (
-        ctx.accounts.sender.as_ref(),
-        ctx.accounts.program_transfer_authority.as_ref(),
-    ) {
-        (Some(sender), None) => token::transfer(
-            CpiContext::new(
-                ctx.accounts.token_program.to_account_info(),
-                token::Transfer {
-                    from: ctx.accounts.sender_token.to_account_info(),
-                    to: ctx.accounts.prepared_custody_token.to_account_info(),
-                    authority: sender.to_account_info(),
-                },
-            ),
-            amount_in,
-        ),
-        (None, Some(program_transfer_authority)) => token::transfer(
-            CpiContext::new_with_signer(
-                ctx.accounts.token_program.to_account_info(),
-                token::Transfer {
-                    from: ctx.accounts.sender_token.to_account_info(),
-                    to: ctx.accounts.prepared_custody_token.to_account_info(),
-                    authority: program_transfer_authority.to_account_info(),
-                },
-                &[&[
-                    TRANSFER_AUTHORITY_SEED_PREFIX,
-                    ctx.accounts.prepared_order.key().as_ref(),
-                    &hashed_args.0,
-                    &[ctx.bumps.program_transfer_authority.unwrap()],
-                ]],
-            ),
-            amount_in,
-        ),
-        (None, None) => err!(TokenRouterError::MissingAuthority),
-        (Some(_), Some(_)) => err!(TokenRouterError::TooManyAuthorities),
-    }
+    // Done.
+    Ok(())
 }

solana/ts/src/idl/json/token_router.json

@@ -525,7 +525,10 @@
           "name": "program_transfer_authority",
           "docs": [
             "The auction participant needs to set approval to this PDA if the sender (signer) is not",
-            "provided.",
+            "provided. The delegated amount must equal the amount in or this instruction will revert.",
+            "",
+            "NOTE: If this account is provided, the sender token's owner will be encoded as the order",
+            "sender.",
             ""
           ],
           "optional": true
@@ -534,7 +537,9 @@
           "name": "sender",
           "docs": [
             "Sender, who has the authority to transfer assets from the sender token account. If this",
-            "account is not provided, the program transfer authority account must be some account."
+            "account is not provided, the program transfer authority account must be some account.",
+            "",
+            "NOTE: If this account is provided, this pubkey will be encoded as the order sender."
           ],
           "signer": true,
           "optional": true
@@ -1122,11 +1127,11 @@
     },
     {
       "code": 6522,
-      "name": "MissingAuthority"
+      "name": "EitherSenderOrProgramTransferAuthority"
     },
     {
-      "code": 6523,
-      "name": "TooManyAuthorities"
+      "code": 6524,
+      "name": "DelegatedAmountMismatch"
     },
     {
       "code": 7024,

solana/ts/src/idl/ts/token_router.ts

@@ -531,7 +531,10 @@ export type TokenRouter = {
           "name": "programTransferAuthority",
           "docs": [
             "The auction participant needs to set approval to this PDA if the sender (signer) is not",
-            "provided.",
+            "provided. The delegated amount must equal the amount in or this instruction will revert.",
+            "",
+            "NOTE: If this account is provided, the sender token's owner will be encoded as the order",
+            "sender.",
             ""
           ],
           "optional": true
@@ -540,7 +543,9 @@ export type TokenRouter = {
           "name": "sender",
           "docs": [
             "Sender, who has the authority to transfer assets from the sender token account. If this",
-            "account is not provided, the program transfer authority account must be some account."
+            "account is not provided, the program transfer authority account must be some account.",
+            "",
+            "NOTE: If this account is provided, this pubkey will be encoded as the order sender."
           ],
           "signer": true,
           "optional": true
@@ -1128,11 +1133,11 @@ export type TokenRouter = {
     },
     {
       "code": 6522,
-      "name": "missingAuthority"
+      "name": "eitherSenderOrProgramTransferAuthority"
     },
     {
-      "code": 6523,
-      "name": "tooManyAuthorities"
+      "code": 6524,
+      "name": "delegatedAmountMismatch"
     },
     {
       "code": 7024,

solana/ts/tests/02__tokenRouter.ts

@@ -543,7 +543,41 @@ describe("Token Router", function () {
                     connection,
                     [ix],
                     [payer, preparedOrder],
-                    "Error: owner does not match",
+                    "Error Code: DelegatedAmountMismatch",
+                );
+            });
+
+            it("Cannot Prepare Market Order Delegating Too Much to Program Transfer Authority", async function () {
+                const preparedOrder = Keypair.generate();
+
+                const amountIn = 69n;
+                const minAmountOut = 0n;
+                const targetChain = foreignChain;
+                const redeemer = Array.from(Buffer.alloc(32, "deadbeef", "hex"));
+                const redeemerMessage = Buffer.from("All your base are belong to us");
+                const [approveIx, ix] = await tokenRouter.prepareMarketOrderIx(
+                    {
+                        payer: payer.publicKey,
+                        preparedOrder: preparedOrder.publicKey,
+                        senderToken: payerToken,
+                    },
+                    {
+                        amountIn,
+                        minAmountOut,
+                        targetChain,
+                        redeemer,
+                        redeemerMessage,
+                    },
+                );
+
+                // Approve for more.
+                approveIx!.data.writeBigUInt64LE(amountIn + 1n, 1);
+
+                await expectIxErr(
+                    connection,
+                    [approveIx!, ix],
+                    [payer, preparedOrder],
+                    "Error Code: DelegatedAmountMismatch",
                 );
             });
 
@@ -575,9 +609,9 @@ describe("Token Router", function () {
 
                 await expectIxErr(
                     connection,
-                    [ix],
+                    [approveIx!, ix],
                     [payer, preparedOrder],
-                    "Error Code: TooManyAuthorities",
+                    "Error Code: EitherSenderOrProgramTransferAuthority",
                 );
             });
 
@@ -610,7 +644,7 @@ describe("Token Router", function () {
                     connection,
                     [ix],
                     [payer, preparedOrder],
-                    "Error Code: MissingAuthority",
+                    "Error Code: EitherSenderOrProgramTransferAuthority",
                 );
             });