From 40e4e85c0a9dbccc4a8f0c183feb36c4dcf69312 Mon Sep 17 00:00:00 2001 From: --systemdf Date: Tue, 5 Aug 2025 11:05:01 -0700 Subject: [PATCH 01/12] allow arbitrary programs as long as no invocation of user --- auction-server/src/api.rs | 11 +- .../src/auction/service/verification.rs | 184 +++++++++++++++--- 2 files changed, 167 insertions(+), 28 deletions(-) diff --git a/auction-server/src/api.rs b/auction-server/src/api.rs index 50de94cf..a2e366d8 100644 --- a/auction-server/src/api.rs +++ b/auction-server/src/api.rs @@ -111,7 +111,6 @@ pub enum InstructionError { UnsupportedSplTokenInstruction(String), InvalidAssociatedTokenAccountInstruction(String), UnsupportedAssociatedTokenAccountInstruction(AssociatedTokenAccountInstruction), - UnsupportedProgram(Pubkey), TransferInstructionNotAllowed, CloseAccountInstructionNotAllowed, InvalidTransferInstructionsCount, @@ -132,6 +131,7 @@ pub enum InstructionError { MissingCreateAtaInstruction(Pubkey), InvalidMemoInstructionCount { expected: usize, found: usize }, InvalidMemoString { expected: String, found: String }, + UnsupportedInvocationOfUserWallet, } impl std::fmt::Display for InstructionError { @@ -155,9 +155,6 @@ impl std::fmt::Display for InstructionError { "Unsupported associated token account instruction {:?}", instruction ), - InstructionError::UnsupportedProgram(program) => { - write!(f, "Unsupported program {}", program) - } InstructionError::TransferInstructionNotAllowed => { write!(f, "Transfer instruction is not allowed") } @@ -278,6 +275,12 @@ impl std::fmt::Display for InstructionError { expected, found ) } + InstructionError::UnsupportedInvocationOfUserWallet => { + write!( + f, + "Invocation of user wallet is not supported in this instruction" + ) + } } } } diff --git a/auction-server/src/auction/service/verification.rs b/auction-server/src/auction/service/verification.rs index 3698d0aa..3efdbce2 100644 --- a/auction-server/src/auction/service/verification.rs +++ b/auction-server/src/auction/service/verification.rs @@ -251,7 +251,9 @@ impl Service { fn validate_swap_transaction_instructions( &self, tx: &VersionedTransaction, + user_wallet: &Pubkey, ) -> Result<(), RestError> { + let accounts = tx.message.static_account_keys(); tx.message .instructions() .iter() @@ -260,6 +262,8 @@ impl Service { self.validate_swap_transaction_instruction( ix.program_id(tx.message.static_account_keys()), ix, + user_wallet, + accounts, ) .map_err(|e| RestError::InvalidInstruction(Some(index), e)) })?; @@ -271,6 +275,8 @@ impl Service { &self, program_id: &Pubkey, ix: &CompiledInstruction, + user_wallet: &Pubkey, + accounts: &[Pubkey], ) -> Result<(), InstructionError> { if *program_id == system_program::id() { if matches!( @@ -308,7 +314,18 @@ impl Service { { Ok(()) } else { - Err(InstructionError::UnsupportedProgram(*program_id)) + ix.accounts.iter().try_for_each(|i| { + if accounts + .get(*i as usize) + .expect("account index out of bounds") + == user_wallet + { + return Err(InstructionError::UnsupportedInvocationOfUserWallet); + } + Ok(()) + })?; + + Ok(()) } } @@ -1020,13 +1037,6 @@ impl Service { }) .await .ok_or(RestError::SwapOpportunityNotFound)?; - self.validate_swap_transaction_instructions( - bid_chain_data_create_svm.get_transaction(), - )?; - let quote_tokens = get_swap_quote_tokens(&opp); - let opportunity_swap_data = get_opportunity_swap_data(&opp); - self.check_svm_swap_bid_fields(bid_data, opportunity_swap_data, "e_tokens) - .await?; let transaction_data = self .get_bid_transaction_data_swap(bid_data.transaction.clone()) @@ -1042,6 +1052,15 @@ impl Service { .. } = transaction_data.accounts; + self.validate_swap_transaction_instructions( + bid_chain_data_create_svm.get_transaction(), + &user_wallet, + )?; + let quote_tokens = get_swap_quote_tokens(&opp); + let opportunity_swap_data = get_opportunity_swap_data(&opp); + self.check_svm_swap_bid_fields(bid_data, opportunity_swap_data, "e_tokens) + .await?; + Self::check_memo_instructions(&bid_data.transaction, &opportunity_swap_data.memo) .await?; @@ -2266,6 +2285,18 @@ mod tests { #[tokio::test] async fn test_verify_bid_when_unsupported_system_program_instruction() { let (service, opportunities) = get_service(true); + let opportunity = opportunities.user_token_specified.clone(); + let bid_amount = 1; + let searcher = Keypair::new(); + let swap_instruction = svm::Svm::get_swap_instruction(GetSwapInstructionParams { + searcher: searcher.pubkey(), + opportunity_params: get_opportunity_params(opportunity.clone()), + bid_amount, + deadline: (OffsetDateTime::now_utc() + Duration::seconds(30)).unix_timestamp(), + fee_receiver_relayer: Pubkey::new_unique(), + relayer_signer: service.config.chain_config.express_relay.relayer.pubkey(), + }) + .unwrap(); let instructions = vec![ system_instruction::advance_nonce_account(&Pubkey::new_unique(), &Pubkey::new_unique()), system_instruction::create_account( @@ -2288,11 +2319,10 @@ mod tests { ), ]; for instruction in instructions.into_iter() { - let searcher = Keypair::new(); let result = get_verify_bid_result( service.clone(), - searcher, - vec![instruction], + searcher.insecure_clone(), + vec![instruction, swap_instruction.clone()], opportunities.user_token_specified.clone(), ) .await; @@ -2309,6 +2339,18 @@ mod tests { #[tokio::test] async fn test_verify_bid_when_unsupported_token_instruction() { let (service, opportunities) = get_service(true); + let opportunity = opportunities.user_token_specified.clone(); + let bid_amount = 1; + let searcher = Keypair::new(); + let swap_instruction = svm::Svm::get_swap_instruction(GetSwapInstructionParams { + searcher: searcher.pubkey(), + opportunity_params: get_opportunity_params(opportunity.clone()), + bid_amount, + deadline: (OffsetDateTime::now_utc() + Duration::seconds(30)).unix_timestamp(), + fee_receiver_relayer: Pubkey::new_unique(), + relayer_signer: service.config.chain_config.express_relay.relayer.pubkey(), + }) + .unwrap(); let instructions = vec![ spl_token::instruction::initialize_account( &spl_token::id(), @@ -2403,11 +2445,10 @@ mod tests { for instruction in instructions.into_iter() { let data = instruction.data.clone(); let ix_parsed = TokenInstruction::unpack(&data).unwrap(); - let searcher = Keypair::new(); let result = get_verify_bid_result( service.clone(), - searcher, - vec![instruction], + searcher.insecure_clone(), + vec![instruction, swap_instruction.clone()], opportunities.user_token_specified.clone(), ) .await; @@ -2424,6 +2465,18 @@ mod tests { #[tokio::test] async fn test_verify_bid_when_unsupported_associated_token_account_instruction() { let (service, opportunities) = get_service(true); + let opportunity = opportunities.user_token_specified.clone(); + let bid_amount = 1; + let searcher = Keypair::new(); + let swap_instruction = svm::Svm::get_swap_instruction(GetSwapInstructionParams { + searcher: searcher.pubkey(), + opportunity_params: get_opportunity_params(opportunity.clone()), + bid_amount, + deadline: (OffsetDateTime::now_utc() + Duration::seconds(30)).unix_timestamp(), + fee_receiver_relayer: Pubkey::new_unique(), + relayer_signer: service.config.chain_config.express_relay.relayer.pubkey(), + }) + .unwrap(); let instructions = vec![recover_nested( &Pubkey::new_unique(), &Pubkey::new_unique(), @@ -2437,11 +2490,10 @@ mod tests { InstructionError::InvalidAssociatedTokenAccountInstruction(e.to_string()) }) .unwrap(); - let searcher = Keypair::new(); let result = get_verify_bid_result( service.clone(), - searcher, - vec![instruction], + searcher.insecure_clone(), + vec![instruction, swap_instruction.clone()], opportunities.user_token_specified.clone(), ) .await; @@ -2456,16 +2508,51 @@ mod tests { } #[tokio::test] - async fn test_verify_bid_when_unsupported_program() { + async fn test_verify_bid_when_arbitrary_program_invokes_user() { let (service, opportunities) = get_service(true); - let program_id = Pubkey::new_unique(); - let instructions = vec![Instruction::new_with_bincode(program_id, &"", vec![])]; + let opportunity = opportunities.user_token_specified.clone(); + let bid_amount = 1; + let searcher = Keypair::new(); + let swap_instruction = svm::Svm::get_swap_instruction(GetSwapInstructionParams { + searcher: searcher.pubkey(), + opportunity_params: get_opportunity_params(opportunity.clone()), + bid_amount, + deadline: (OffsetDateTime::now_utc() + Duration::seconds(30)).unix_timestamp(), + fee_receiver_relayer: Pubkey::new_unique(), + relayer_signer: service.config.chain_config.express_relay.relayer.pubkey(), + }) + .unwrap(); + let SwapParams { + user_wallet_address, + router_account: _, + permission_account: _, + minimum_deadline: _, + } = get_opportunity_swap_params(opportunity); + let instructions = vec![ + Instruction::new_with_bincode( + Pubkey::new_unique(), + &"", + vec![AccountMeta { + pubkey: user_wallet_address, + is_signer: false, + is_writable: true, + }], + ), + Instruction::new_with_bincode( + Pubkey::new_unique(), + &"", + vec![AccountMeta { + pubkey: user_wallet_address, + is_signer: true, + is_writable: true, + }], + ), + ]; for instruction in instructions.into_iter() { - let searcher = Keypair::new(); let result = get_verify_bid_result( service.clone(), - searcher, - vec![instruction], + searcher.insecure_clone(), + vec![instruction, swap_instruction.clone()], opportunities.user_token_specified.clone(), ) .await; @@ -2473,12 +2560,61 @@ mod tests { result.unwrap_err(), RestError::InvalidInstruction( Some(0), - InstructionError::UnsupportedProgram(program_id) + InstructionError::UnsupportedInvocationOfUserWallet ) ); } } + #[tokio::test] + async fn test_verify_bid_when_arbitrary_program_does_not_invoke_user() { + let (service, opportunities) = get_service(true); + let opportunity = opportunities.user_token_specified.clone(); + let bid_amount = 1; + let searcher = Keypair::new(); + let swap_instruction = svm::Svm::get_swap_instruction(GetSwapInstructionParams { + searcher: searcher.pubkey(), + opportunity_params: get_opportunity_params(opportunity.clone()), + bid_amount, + deadline: (OffsetDateTime::now_utc() + Duration::seconds(30)).unix_timestamp(), + fee_receiver_relayer: Pubkey::new_unique(), + relayer_signer: service.config.chain_config.express_relay.relayer.pubkey(), + }) + .unwrap(); + let instructions = vec![Instruction::new_with_bincode( + Pubkey::new_unique(), + &"", + vec![], + )]; + let swap_params = get_opportunity_swap_params(opportunity); + + for instruction in instructions.into_iter() { + let instructions = vec![instruction, swap_instruction.clone()]; + let mut transaction = + Transaction::new_with_payer(&instructions, Some(&searcher.pubkey())); + transaction.partial_sign(&[searcher.insecure_clone()], Hash::default()); + let result = get_verify_bid_result( + service.clone(), + searcher.insecure_clone(), + instructions, + opportunities.user_token_specified.clone(), + ) + .await + .unwrap(); + + assert_eq!( + result.0, + BidChainDataSvm { + transaction: transaction.into(), + permission_account: swap_params.permission_account, + router: swap_params.router_account, + bid_payment_instruction_type: BidPaymentInstructionType::Swap, + } + ); + assert_eq!(result.1, bid_amount); + } + } + #[tokio::test] async fn test_verify_bid_when_multiple_express_relay_instructions() { let (service, opportunities) = get_service(true); From 66413a4a2df2ded61170960d7fea2062e58805b0 Mon Sep 17 00:00:00 2001 From: --systemdf Date: Tue, 5 Aug 2025 11:43:04 -0700 Subject: [PATCH 02/12] handle unapproved system/token/associatedToken ixs better --- auction-server/src/api.rs | 4 + .../src/auction/service/verification.rs | 164 +++++++++++++----- 2 files changed, 120 insertions(+), 48 deletions(-) diff --git a/auction-server/src/api.rs b/auction-server/src/api.rs index a2e366d8..e97b31c6 100644 --- a/auction-server/src/api.rs +++ b/auction-server/src/api.rs @@ -132,6 +132,7 @@ pub enum InstructionError { InvalidMemoInstructionCount { expected: usize, found: usize }, InvalidMemoString { expected: String, found: String }, UnsupportedInvocationOfUserWallet, + UnapprovedProgramId(Pubkey), } impl std::fmt::Display for InstructionError { @@ -281,6 +282,9 @@ impl std::fmt::Display for InstructionError { "Invocation of user wallet is not supported in this instruction" ) } + InstructionError::UnapprovedProgramId(program_id) => { + write!(f, "Instruction has unapproved program id: {:?}", program_id) + } } } } diff --git a/auction-server/src/auction/service/verification.rs b/auction-server/src/auction/service/verification.rs index 3efdbce2..084c2fed 100644 --- a/auction-server/src/auction/service/verification.rs +++ b/auction-server/src/auction/service/verification.rs @@ -271,14 +271,44 @@ impl Service { Ok(()) } + /// Checks if the instruction provided meets the criteria for validity. + /// Instruction must either be an approved program instruction or not contain the user wallet in its accounts. fn validate_swap_transaction_instruction( &self, program_id: &Pubkey, ix: &CompiledInstruction, user_wallet: &Pubkey, accounts: &[Pubkey], + ) -> Result<(), InstructionError> { + if self + .check_approved_program_instruction(program_id, ix) + .is_ok() + { + Ok(()) + } else { + ix.accounts.iter().try_for_each(|i| { + if accounts + .get(*i as usize) + .expect("account index out of bounds") + == user_wallet + { + return Err(InstructionError::UnsupportedInvocationOfUserWallet); + } + Ok(()) + })?; + + Ok(()) + } + } + + /// Checks if the instruction is an approved program instruction for swap transactions. + fn check_approved_program_instruction( + &self, + program_id: &Pubkey, + ix: &CompiledInstruction, ) -> Result<(), InstructionError> { if *program_id == system_program::id() { + // approve only system program transfer instructions if matches!( bincode::deserialize::(&ix.data), Ok(SystemInstruction::Transfer { .. }) @@ -291,8 +321,11 @@ impl Service { let ix_parsed = TokenInstruction::unpack(&ix.data) .map_err(InstructionError::InvalidSplTokenInstruction)?; match ix_parsed { + // approve token program close account instructions TokenInstruction::CloseAccount { .. } => Ok(()), + // approve token program sync native (for WSOL) instructions TokenInstruction::SyncNative { .. } => Ok(()), + // other token program instructions are not approved _ => Err(InstructionError::UnsupportedSplTokenInstruction(format!( "{:?}", ix_parsed @@ -304,28 +337,21 @@ impl Service { InstructionError::InvalidAssociatedTokenAccountInstruction(e.to_string()) })?; match ix_parsed { + // approve associated token account creation AssociatedTokenAccountInstruction::Create => Ok(()), + // approve associated token account idempotent creation AssociatedTokenAccountInstruction::CreateIdempotent => Ok(()), + // other associated token account instructions are not approved _ => Err(InstructionError::UnsupportedAssociatedTokenAccountInstruction(ix_parsed)), } } else if *program_id == self.config.chain_config.express_relay.program_id || *program_id == spl_memo_client::ID || *program_id == compute_budget::id() { + // all express relay program, memo program, and compute budget program instructions are allowed Ok(()) } else { - ix.accounts.iter().try_for_each(|i| { - if accounts - .get(*i as usize) - .expect("account index out of bounds") - == user_wallet - { - return Err(InstructionError::UnsupportedInvocationOfUserWallet); - } - Ok(()) - })?; - - Ok(()) + Err(InstructionError::UnapprovedProgramId(*program_id)) } } @@ -2283,7 +2309,7 @@ mod tests { } #[tokio::test] - async fn test_verify_bid_when_unsupported_system_program_instruction() { + async fn test_check_approved_program_when_unsupported_system_program_instruction() { let (service, opportunities) = get_service(true); let opportunity = opportunities.user_token_specified.clone(); let bid_amount = 1; @@ -2319,25 +2345,27 @@ mod tests { ), ]; for instruction in instructions.into_iter() { - let result = get_verify_bid_result( - service.clone(), - searcher.insecure_clone(), - vec![instruction, swap_instruction.clone()], - opportunities.user_token_specified.clone(), - ) - .await; + let program_id = instruction.program_id; + let transaction = Transaction::new_with_payer( + &[instruction.clone(), swap_instruction.clone()], + Some(&searcher.pubkey()), + ); + let instruction = transaction + .message() + .instructions + .first() + .expect("Expected at least one instruction") + .clone(); + let result = service.check_approved_program_instruction(&program_id, &instruction); assert_eq!( result.unwrap_err(), - RestError::InvalidInstruction( - Some(0), - InstructionError::UnsupportedSystemProgramInstruction - ) + InstructionError::UnsupportedSystemProgramInstruction ); } } #[tokio::test] - async fn test_verify_bid_when_unsupported_token_instruction() { + async fn test_check_approved_program_when_unsupported_token_instruction() { let (service, opportunities) = get_service(true); let opportunity = opportunities.user_token_specified.clone(); let bid_amount = 1; @@ -2445,25 +2473,27 @@ mod tests { for instruction in instructions.into_iter() { let data = instruction.data.clone(); let ix_parsed = TokenInstruction::unpack(&data).unwrap(); - let result = get_verify_bid_result( - service.clone(), - searcher.insecure_clone(), - vec![instruction, swap_instruction.clone()], - opportunities.user_token_specified.clone(), - ) - .await; + let program_id = instruction.program_id; + let transaction = Transaction::new_with_payer( + &[instruction.clone(), swap_instruction.clone()], + Some(&searcher.pubkey()), + ); + let instruction = transaction + .message() + .instructions + .first() + .expect("Expected at least one instruction") + .clone(); + let result = service.check_approved_program_instruction(&program_id, &instruction); assert_eq!( result.unwrap_err(), - RestError::InvalidInstruction( - Some(0), - InstructionError::UnsupportedSplTokenInstruction(format!("{:?}", ix_parsed)), - ) + InstructionError::UnsupportedSplTokenInstruction(format!("{:?}", ix_parsed)) ); } } #[tokio::test] - async fn test_verify_bid_when_unsupported_associated_token_account_instruction() { + async fn test_check_approved_program_when_unsupported_associated_token_account_instruction() { let (service, opportunities) = get_service(true); let opportunity = opportunities.user_token_specified.clone(); let bid_amount = 1; @@ -2483,6 +2513,7 @@ mod tests { &Pubkey::new_unique(), &spl_token::id(), )]; + for instruction in instructions.into_iter() { let data = instruction.data.clone(); let ix_parsed = AssociatedTokenAccountInstruction::try_from_slice(&data) @@ -2490,23 +2521,60 @@ mod tests { InstructionError::InvalidAssociatedTokenAccountInstruction(e.to_string()) }) .unwrap(); - let result = get_verify_bid_result( - service.clone(), - searcher.insecure_clone(), - vec![instruction, swap_instruction.clone()], - opportunities.user_token_specified.clone(), - ) - .await; + let program_id = instruction.program_id; + let transaction = Transaction::new_with_payer( + &[instruction.clone(), swap_instruction.clone()], + Some(&searcher.pubkey()), + ); + let instruction = transaction + .message() + .instructions + .first() + .expect("Expected at least one instruction") + .clone(); + let result = service.check_approved_program_instruction(&program_id, &instruction); assert_eq!( result.unwrap_err(), - RestError::InvalidInstruction( - Some(0), - InstructionError::UnsupportedAssociatedTokenAccountInstruction(ix_parsed), - ) + InstructionError::UnsupportedAssociatedTokenAccountInstruction(ix_parsed) ); } } + #[tokio::test] + async fn test_check_approved_program_when_unapproved_program_id() { + let (service, opportunities) = get_service(true); + let opportunity = opportunities.user_token_specified.clone(); + let bid_amount = 1; + let searcher = Keypair::new(); + let swap_instruction = svm::Svm::get_swap_instruction(GetSwapInstructionParams { + searcher: searcher.pubkey(), + opportunity_params: get_opportunity_params(opportunity.clone()), + bid_amount, + deadline: (OffsetDateTime::now_utc() + Duration::seconds(30)).unix_timestamp(), + fee_receiver_relayer: Pubkey::new_unique(), + relayer_signer: service.config.chain_config.express_relay.relayer.pubkey(), + }) + .unwrap(); + let program_id = Pubkey::new_unique(); + let instruction = Instruction::new_with_bincode(program_id, &"", vec![]); + + let transaction = Transaction::new_with_payer( + &[instruction.clone(), swap_instruction.clone()], + Some(&searcher.pubkey()), + ); + let instruction = transaction + .message() + .instructions + .first() + .expect("Expected at least one instruction") + .clone(); + let result = service.check_approved_program_instruction(&program_id, &instruction); + assert_eq!( + result.unwrap_err(), + InstructionError::UnapprovedProgramId(program_id) + ); + } + #[tokio::test] async fn test_verify_bid_when_arbitrary_program_invokes_user() { let (service, opportunities) = get_service(true); From 83d3b8a93b7088cab5b00bf73deda49997c9a80d Mon Sep 17 00:00:00 2001 From: --systemdf Date: Tue, 5 Aug 2025 11:57:07 -0700 Subject: [PATCH 03/12] approve token 2022 program ixs --- .../src/auction/service/verification.rs | 146 ++++++++++++++++++ 1 file changed, 146 insertions(+) diff --git a/auction-server/src/auction/service/verification.rs b/auction-server/src/auction/service/verification.rs index 084c2fed..1fbc249e 100644 --- a/auction-server/src/auction/service/verification.rs +++ b/auction-server/src/auction/service/verification.rs @@ -66,6 +66,7 @@ use { instruction::AssociatedTokenAccountInstruction, }, spl_token::instruction::TokenInstruction, + spl_token_2022::instruction::TokenInstruction as Token2022Instruction, std::{ array, collections::VecDeque, @@ -331,6 +332,20 @@ impl Service { ix_parsed ))), } + } else if *program_id == spl_token_2022::id() { + let ix_parsed = Token2022Instruction::unpack(&ix.data) + .map_err(InstructionError::InvalidSplTokenInstruction)?; + match ix_parsed { + // approve token program close account instructions + Token2022Instruction::CloseAccount { .. } => Ok(()), + // approve token program sync native (for WSOL) instructions + Token2022Instruction::SyncNative { .. } => Ok(()), + // other token program instructions are not approved + _ => Err(InstructionError::UnsupportedSplTokenInstruction(format!( + "{:?}", + ix_parsed + ))), + } } else if *program_id == spl_associated_token_account::id() { let ix_parsed = AssociatedTokenAccountInstruction::try_from_slice(&ix.data).map_err(|e| { @@ -530,6 +545,7 @@ impl Service { Ok(result) } + /// TODO: need to adapt this to allow all transfers not including user account async fn check_transfer_instruction( &self, tx: &VersionedTransaction, @@ -2492,6 +2508,136 @@ mod tests { } } + #[tokio::test] + async fn test_check_approved_program_when_unsupported_token_2022_instruction() { + let (service, opportunities) = get_service(true); + let opportunity = opportunities.user_token_specified.clone(); + let bid_amount = 1; + let searcher = Keypair::new(); + let swap_instruction = svm::Svm::get_swap_instruction(GetSwapInstructionParams { + searcher: searcher.pubkey(), + opportunity_params: get_opportunity_params(opportunity.clone()), + bid_amount, + deadline: (OffsetDateTime::now_utc() + Duration::seconds(30)).unix_timestamp(), + fee_receiver_relayer: Pubkey::new_unique(), + relayer_signer: service.config.chain_config.express_relay.relayer.pubkey(), + }) + .unwrap(); + let instructions = vec![ + spl_token_2022::instruction::initialize_account( + &spl_token_2022::id(), + &Pubkey::new_unique(), + &Pubkey::new_unique(), + &Pubkey::new_unique(), + ) + .unwrap(), + spl_token_2022::instruction::initialize_account2( + &spl_token_2022::id(), + &Pubkey::new_unique(), + &Pubkey::new_unique(), + &Pubkey::new_unique(), + ) + .unwrap(), + spl_token_2022::instruction::initialize_account3( + &spl_token_2022::id(), + &Pubkey::new_unique(), + &Pubkey::new_unique(), + &Pubkey::new_unique(), + ) + .unwrap(), + spl_token_2022::instruction::initialize_mint( + &spl_token_2022::id(), + &Pubkey::new_unique(), + &Pubkey::new_unique(), + None, + 0, + ) + .unwrap(), + spl_token_2022::instruction::initialize_mint2( + &spl_token_2022::id(), + &Pubkey::new_unique(), + &Pubkey::new_unique(), + None, + 0, + ) + .unwrap(), + spl_token_2022::instruction::transfer_checked( + &spl_token_2022::id(), + &Pubkey::new_unique(), + &Pubkey::new_unique(), + &Pubkey::new_unique(), + &Pubkey::new_unique(), + &[], + 0, + 0, + ) + .unwrap(), + spl_token_2022::instruction::approve( + &spl_token_2022::id(), + &Pubkey::new_unique(), + &Pubkey::new_unique(), + &Pubkey::new_unique(), + &[], + 0, + ) + .unwrap(), + spl_token_2022::instruction::revoke( + &spl_token_2022::id(), + &Pubkey::new_unique(), + &Pubkey::new_unique(), + &[], + ) + .unwrap(), + spl_token_2022::instruction::set_authority( + &spl_token_2022::id(), + &Pubkey::new_unique(), + None, + spl_token_2022::instruction::AuthorityType::AccountOwner, + &Pubkey::new_unique(), + &[], + ) + .unwrap(), + spl_token_2022::instruction::mint_to( + &spl_token_2022::id(), + &Pubkey::new_unique(), + &Pubkey::new_unique(), + &Pubkey::new_unique(), + &[], + 0, + ) + .unwrap(), + spl_token_2022::instruction::burn( + &spl_token_2022::id(), + &Pubkey::new_unique(), + &Pubkey::new_unique(), + &Pubkey::new_unique(), + &[], + 0, + ) + .unwrap(), + ]; + for instruction in instructions.into_iter() { + let data = instruction.data.clone(); + let ix_parsed = TokenInstruction::unpack(&data).unwrap(); + let program_id = instruction.program_id; + let transaction = Transaction::new_with_payer( + &[instruction.clone(), swap_instruction.clone()], + Some(&searcher.pubkey()), + ); + let instruction = transaction + .message() + .instructions + .first() + .expect("Expected at least one instruction") + .clone(); + let result = service.check_approved_program_instruction(&program_id, &instruction); + assert_eq!( + result.unwrap_err(), + InstructionError::UnsupportedSplTokenInstruction(format!("{:?}", ix_parsed)) + ); + } + } + #[tokio::test] async fn test_check_approved_program_when_unsupported_associated_token_account_instruction() { let (service, opportunities) = get_service(true); From 53885f12ec1b77a787669f3013e1471c6bf32737 Mon Sep 17 00:00:00 2001 From: --systemdf Date: Tue, 5 Aug 2025 12:30:02 -0700 Subject: [PATCH 04/12] account for lookup tables --- .../src/auction/service/verification.rs | 55 +++++++++---------- 1 file changed, 26 insertions(+), 29 deletions(-) diff --git a/auction-server/src/auction/service/verification.rs b/auction-server/src/auction/service/verification.rs index 1fbc249e..5b6f3cb7 100644 --- a/auction-server/src/auction/service/verification.rs +++ b/auction-server/src/auction/service/verification.rs @@ -249,54 +249,50 @@ impl Service { } } - fn validate_swap_transaction_instructions( + async fn validate_swap_transaction_instructions( &self, tx: &VersionedTransaction, user_wallet: &Pubkey, ) -> Result<(), RestError> { - let accounts = tx.message.static_account_keys(); - tx.message - .instructions() - .iter() - .enumerate() - .try_for_each(|(index, ix)| { - self.validate_swap_transaction_instruction( - ix.program_id(tx.message.static_account_keys()), - ix, - user_wallet, - accounts, - ) - .map_err(|e| RestError::InvalidInstruction(Some(index), e)) - })?; - + for (index, ix) in tx.message.instructions().iter().enumerate() { + self.validate_swap_transaction_instruction( + ix.program_id(tx.message.static_account_keys()), + ix, + tx, + user_wallet, + index, + ) + .await?; + } Ok(()) } /// Checks if the instruction provided meets the criteria for validity. /// Instruction must either be an approved program instruction or not contain the user wallet in its accounts. - fn validate_swap_transaction_instruction( + async fn validate_swap_transaction_instruction( &self, program_id: &Pubkey, ix: &CompiledInstruction, + tx: &VersionedTransaction, user_wallet: &Pubkey, - accounts: &[Pubkey], - ) -> Result<(), InstructionError> { + ix_index: usize, + ) -> Result<(), RestError> { if self .check_approved_program_instruction(program_id, ix) .is_ok() { Ok(()) } else { - ix.accounts.iter().try_for_each(|i| { - if accounts - .get(*i as usize) - .expect("account index out of bounds") - == user_wallet - { - return Err(InstructionError::UnsupportedInvocationOfUserWallet); + // TODO: this loop will be slow and invoke many rpc calls if there are many lookup accounts. either parallelize this extraction or limit number of lookup accounts + for i in 0..ix.accounts.len() { + let account_key = self.extract_account(tx, ix, i).await?; + if account_key == *user_wallet { + return Err(RestError::InvalidInstruction( + Some(ix_index), + InstructionError::UnsupportedInvocationOfUserWallet, + )); } - Ok(()) - })?; + } Ok(()) } @@ -1097,7 +1093,8 @@ impl Service { self.validate_swap_transaction_instructions( bid_chain_data_create_svm.get_transaction(), &user_wallet, - )?; + ) + .await?; let quote_tokens = get_swap_quote_tokens(&opp); let opportunity_swap_data = get_opportunity_swap_data(&opp); self.check_svm_swap_bid_fields(bid_data, opportunity_swap_data, "e_tokens) From 3142052b081ed2ab4b5e576d3c96eeda494a32f7 Mon Sep 17 00:00:00 2001 From: --systemdf Date: Tue, 12 Aug 2025 10:36:03 -0700 Subject: [PATCH 05/12] bubble up exact instruction error --- auction-server/src/api.rs | 7 ----- .../src/auction/service/verification.rs | 30 ++++++++----------- 2 files changed, 13 insertions(+), 24 deletions(-) diff --git a/auction-server/src/api.rs b/auction-server/src/api.rs index e97b31c6..41fff776 100644 --- a/auction-server/src/api.rs +++ b/auction-server/src/api.rs @@ -131,7 +131,6 @@ pub enum InstructionError { MissingCreateAtaInstruction(Pubkey), InvalidMemoInstructionCount { expected: usize, found: usize }, InvalidMemoString { expected: String, found: String }, - UnsupportedInvocationOfUserWallet, UnapprovedProgramId(Pubkey), } @@ -276,12 +275,6 @@ impl std::fmt::Display for InstructionError { expected, found ) } - InstructionError::UnsupportedInvocationOfUserWallet => { - write!( - f, - "Invocation of user wallet is not supported in this instruction" - ) - } InstructionError::UnapprovedProgramId(program_id) => { write!(f, "Instruction has unapproved program id: {:?}", program_id) } diff --git a/auction-server/src/auction/service/verification.rs b/auction-server/src/auction/service/verification.rs index 5b6f3cb7..adeecc4b 100644 --- a/auction-server/src/auction/service/verification.rs +++ b/auction-server/src/auction/service/verification.rs @@ -277,24 +277,19 @@ impl Service { user_wallet: &Pubkey, ix_index: usize, ) -> Result<(), RestError> { - if self - .check_approved_program_instruction(program_id, ix) - .is_ok() - { - Ok(()) - } else { - // TODO: this loop will be slow and invoke many rpc calls if there are many lookup accounts. either parallelize this extraction or limit number of lookup accounts - for i in 0..ix.accounts.len() { - let account_key = self.extract_account(tx, ix, i).await?; - if account_key == *user_wallet { - return Err(RestError::InvalidInstruction( - Some(ix_index), - InstructionError::UnsupportedInvocationOfUserWallet, - )); + match self.check_approved_program_instruction(program_id, ix) { + Ok(()) => Ok(()), + Err(e) => { + // TODO: this loop will be slow and invoke many rpc calls if there are many lookup accounts. either parallelize this extraction or limit number of lookup accounts + for i in 0..ix.accounts.len() { + let account_key = self.extract_account(tx, ix, i).await?; + if account_key == *user_wallet { + return Err(RestError::InvalidInstruction(Some(ix_index), e)); + } } - } - Ok(()) + Ok(()) + } } } @@ -2760,6 +2755,7 @@ mod tests { ), ]; for instruction in instructions.into_iter() { + let program_id = instruction.program_id; let result = get_verify_bid_result( service.clone(), searcher.insecure_clone(), @@ -2771,7 +2767,7 @@ mod tests { result.unwrap_err(), RestError::InvalidInstruction( Some(0), - InstructionError::UnsupportedInvocationOfUserWallet + InstructionError::UnapprovedProgramId(program_id) ) ); } From 86803116db82d7c809071794941cac5eceaa84a5 Mon Sep 17 00:00:00 2001 From: --systemdf Date: Tue, 12 Aug 2025 10:42:16 -0700 Subject: [PATCH 06/12] get rid of swap instruction in check_approved_program tests --- .../src/auction/service/verification.rs | 95 +++---------------- 1 file changed, 15 insertions(+), 80 deletions(-) diff --git a/auction-server/src/auction/service/verification.rs b/auction-server/src/auction/service/verification.rs index adeecc4b..c594f43a 100644 --- a/auction-server/src/auction/service/verification.rs +++ b/auction-server/src/auction/service/verification.rs @@ -2318,19 +2318,8 @@ mod tests { #[tokio::test] async fn test_check_approved_program_when_unsupported_system_program_instruction() { - let (service, opportunities) = get_service(true); - let opportunity = opportunities.user_token_specified.clone(); - let bid_amount = 1; + let (service, _) = get_service(true); let searcher = Keypair::new(); - let swap_instruction = svm::Svm::get_swap_instruction(GetSwapInstructionParams { - searcher: searcher.pubkey(), - opportunity_params: get_opportunity_params(opportunity.clone()), - bid_amount, - deadline: (OffsetDateTime::now_utc() + Duration::seconds(30)).unix_timestamp(), - fee_receiver_relayer: Pubkey::new_unique(), - relayer_signer: service.config.chain_config.express_relay.relayer.pubkey(), - }) - .unwrap(); let instructions = vec![ system_instruction::advance_nonce_account(&Pubkey::new_unique(), &Pubkey::new_unique()), system_instruction::create_account( @@ -2354,10 +2343,8 @@ mod tests { ]; for instruction in instructions.into_iter() { let program_id = instruction.program_id; - let transaction = Transaction::new_with_payer( - &[instruction.clone(), swap_instruction.clone()], - Some(&searcher.pubkey()), - ); + let transaction = + Transaction::new_with_payer(&[instruction.clone()], Some(&searcher.pubkey())); let instruction = transaction .message() .instructions @@ -2374,19 +2361,8 @@ mod tests { #[tokio::test] async fn test_check_approved_program_when_unsupported_token_instruction() { - let (service, opportunities) = get_service(true); - let opportunity = opportunities.user_token_specified.clone(); - let bid_amount = 1; + let (service, _) = get_service(true); let searcher = Keypair::new(); - let swap_instruction = svm::Svm::get_swap_instruction(GetSwapInstructionParams { - searcher: searcher.pubkey(), - opportunity_params: get_opportunity_params(opportunity.clone()), - bid_amount, - deadline: (OffsetDateTime::now_utc() + Duration::seconds(30)).unix_timestamp(), - fee_receiver_relayer: Pubkey::new_unique(), - relayer_signer: service.config.chain_config.express_relay.relayer.pubkey(), - }) - .unwrap(); let instructions = vec![ spl_token::instruction::initialize_account( &spl_token::id(), @@ -2482,10 +2458,8 @@ mod tests { let data = instruction.data.clone(); let ix_parsed = TokenInstruction::unpack(&data).unwrap(); let program_id = instruction.program_id; - let transaction = Transaction::new_with_payer( - &[instruction.clone(), swap_instruction.clone()], - Some(&searcher.pubkey()), - ); + let transaction = + Transaction::new_with_payer(&[instruction.clone()], Some(&searcher.pubkey())); let instruction = transaction .message() .instructions @@ -2502,19 +2476,8 @@ mod tests { #[tokio::test] async fn test_check_approved_program_when_unsupported_token_2022_instruction() { - let (service, opportunities) = get_service(true); - let opportunity = opportunities.user_token_specified.clone(); - let bid_amount = 1; + let (service, _) = get_service(true); let searcher = Keypair::new(); - let swap_instruction = svm::Svm::get_swap_instruction(GetSwapInstructionParams { - searcher: searcher.pubkey(), - opportunity_params: get_opportunity_params(opportunity.clone()), - bid_amount, - deadline: (OffsetDateTime::now_utc() + Duration::seconds(30)).unix_timestamp(), - fee_receiver_relayer: Pubkey::new_unique(), - relayer_signer: service.config.chain_config.express_relay.relayer.pubkey(), - }) - .unwrap(); let instructions = vec![ spl_token_2022::instruction::initialize_account( &spl_token_2022::id(), @@ -2612,10 +2575,8 @@ mod tests { let data = instruction.data.clone(); let ix_parsed = TokenInstruction::unpack(&data).unwrap(); let program_id = instruction.program_id; - let transaction = Transaction::new_with_payer( - &[instruction.clone(), swap_instruction.clone()], - Some(&searcher.pubkey()), - ); + let transaction = + Transaction::new_with_payer(&[instruction.clone()], Some(&searcher.pubkey())); let instruction = transaction .message() .instructions @@ -2632,19 +2593,8 @@ mod tests { #[tokio::test] async fn test_check_approved_program_when_unsupported_associated_token_account_instruction() { - let (service, opportunities) = get_service(true); - let opportunity = opportunities.user_token_specified.clone(); - let bid_amount = 1; + let (service, _) = get_service(true); let searcher = Keypair::new(); - let swap_instruction = svm::Svm::get_swap_instruction(GetSwapInstructionParams { - searcher: searcher.pubkey(), - opportunity_params: get_opportunity_params(opportunity.clone()), - bid_amount, - deadline: (OffsetDateTime::now_utc() + Duration::seconds(30)).unix_timestamp(), - fee_receiver_relayer: Pubkey::new_unique(), - relayer_signer: service.config.chain_config.express_relay.relayer.pubkey(), - }) - .unwrap(); let instructions = vec![recover_nested( &Pubkey::new_unique(), &Pubkey::new_unique(), @@ -2660,10 +2610,8 @@ mod tests { }) .unwrap(); let program_id = instruction.program_id; - let transaction = Transaction::new_with_payer( - &[instruction.clone(), swap_instruction.clone()], - Some(&searcher.pubkey()), - ); + let transaction = + Transaction::new_with_payer(&[instruction.clone()], Some(&searcher.pubkey())); let instruction = transaction .message() .instructions @@ -2680,26 +2628,13 @@ mod tests { #[tokio::test] async fn test_check_approved_program_when_unapproved_program_id() { - let (service, opportunities) = get_service(true); - let opportunity = opportunities.user_token_specified.clone(); - let bid_amount = 1; + let (service, _) = get_service(true); let searcher = Keypair::new(); - let swap_instruction = svm::Svm::get_swap_instruction(GetSwapInstructionParams { - searcher: searcher.pubkey(), - opportunity_params: get_opportunity_params(opportunity.clone()), - bid_amount, - deadline: (OffsetDateTime::now_utc() + Duration::seconds(30)).unix_timestamp(), - fee_receiver_relayer: Pubkey::new_unique(), - relayer_signer: service.config.chain_config.express_relay.relayer.pubkey(), - }) - .unwrap(); let program_id = Pubkey::new_unique(); let instruction = Instruction::new_with_bincode(program_id, &"", vec![]); - let transaction = Transaction::new_with_payer( - &[instruction.clone(), swap_instruction.clone()], - Some(&searcher.pubkey()), - ); + let transaction = + Transaction::new_with_payer(&[instruction.clone()], Some(&searcher.pubkey())); let instruction = transaction .message() .instructions From ed27155401b501775c991936a6b0ab01bdcd4a56 Mon Sep 17 00:00:00 2001 From: --systemdf Date: Thu, 14 Aug 2025 11:53:19 -0700 Subject: [PATCH 07/12] generalize to allow non approve program calls that dont invoke user + docs --- auction-server/src/api.rs | 39 +- .../src/auction/service/verification.rs | 392 +++++------------- 2 files changed, 123 insertions(+), 308 deletions(-) diff --git a/auction-server/src/api.rs b/auction-server/src/api.rs index 41fff776..df926c6f 100644 --- a/auction-server/src/api.rs +++ b/auction-server/src/api.rs @@ -111,10 +111,9 @@ pub enum InstructionError { UnsupportedSplTokenInstruction(String), InvalidAssociatedTokenAccountInstruction(String), UnsupportedAssociatedTokenAccountInstruction(AssociatedTokenAccountInstruction), - TransferInstructionNotAllowed, CloseAccountInstructionNotAllowed, - InvalidTransferInstructionsCount, - InvalidFromAccountTransferInstruction { expected: Pubkey, found: Pubkey }, + InvalidUserTransferInstructionsCount { found: usize }, + InvalidUserAccountTransferInstruction, InvalidToAccountTransferInstruction { expected: Pubkey, found: Pubkey }, InvalidAmountTransferInstruction { expected: u64, found: u64 }, InvalidSyncNativeInstructionCount(Pubkey), @@ -129,8 +128,7 @@ pub enum InstructionError { InvalidTokenProgramInCreateAtaInstruction { expected: Pubkey, found: Pubkey }, InvalidSystemProgramInCreateAtaInstruction(Pubkey), MissingCreateAtaInstruction(Pubkey), - InvalidMemoInstructionCount { expected: usize, found: usize }, - InvalidMemoString { expected: String, found: String }, + MissingMemoInstruction { expected: String }, UnapprovedProgramId(Pubkey), } @@ -155,21 +153,17 @@ impl std::fmt::Display for InstructionError { "Unsupported associated token account instruction {:?}", instruction ), - InstructionError::TransferInstructionNotAllowed => { - write!(f, "Transfer instruction is not allowed") - } InstructionError::CloseAccountInstructionNotAllowed => { write!(f, "Close account instruction is not allowed") } - InstructionError::InvalidTransferInstructionsCount => { - write!(f, "Exactly one sol transfer instruction is required") + InstructionError::InvalidUserTransferInstructionsCount { found } => { + match found { + 0 => write!(f, "At least one sol transfer instruction from the user wallet account is required"), + _ => write!(f, "Invalid number ({}) of sol transfer instructions from the user wallet account", found), + } } - InstructionError::InvalidFromAccountTransferInstruction { expected, found } => { - write!( - f, - "Invalid from account in sol transfer instruction. Expected: {:?} found: {:?}", - expected, found - ) + InstructionError::InvalidUserAccountTransferInstruction => { + write!(f, "Invalid sol transfer instruction from the user account.") } InstructionError::InvalidToAccountTransferInstruction { expected, found } => { write!( @@ -261,18 +255,11 @@ impl std::fmt::Display for InstructionError { ata ) } - InstructionError::InvalidMemoInstructionCount { expected, found } => { + InstructionError::MissingMemoInstruction { expected } => { write!( f, - "Invalid memo instruction count. Expected: {:?} found: {:?}", - expected, found - ) - } - InstructionError::InvalidMemoString { expected, found } => { - write!( - f, - "Invalid memo string in memo instruction. Expected: {:?} found: {:?}", - expected, found + "Missing memo instruction with string: {:?}", + expected, ) } InstructionError::UnapprovedProgramId(program_id) => { diff --git a/auction-server/src/auction/service/verification.rs b/auction-server/src/auction/service/verification.rs index c594f43a..73c2a6cb 100644 --- a/auction-server/src/auction/service/verification.rs +++ b/auction-server/src/auction/service/verification.rs @@ -249,6 +249,8 @@ impl Service { } } + // Checks to ensure each individual instruction in the transaction is valid. + // Does not detect if there are missing necessary instructions. async fn validate_swap_transaction_instructions( &self, tx: &VersionedTransaction, @@ -501,6 +503,7 @@ impl Service { async fn extract_transfer_instructions( &self, tx: &VersionedTransaction, + from: Pubkey, ) -> Result, RestError> { let instructions: Vec<(usize, &CompiledInstruction)> = Self::extract_program_instructions(tx, &system_program::id()) @@ -531,113 +534,80 @@ impl Service { )) } }; - result.push(transfer_instruction); + if transfer_instruction.from == from { + result.push(transfer_instruction); + } } Ok(result) } - /// TODO: need to adapt this to allow all transfers not including user account - async fn check_transfer_instruction( + // Ensures that any SOL transfer instruction from the user is only carried out when user token mint is SOL and user needs to wrap SOL. + async fn check_user_transfer_instruction( &self, tx: &VersionedTransaction, transaction_data: &entities::BidTransactionDataSwap, opportunity_swap_data: &OpportunitySvmProgramSwap, ) -> Result<(), RestError> { - let transfer_instructions = self.extract_transfer_instructions(tx).await?; - if transfer_instructions.len() > 1 { + let user_transfer_instructions = self + .extract_transfer_instructions(tx, transaction_data.accounts.user_wallet) + .await?; + if user_transfer_instructions.len() > 1 { return Err(RestError::InvalidInstruction( - transfer_instructions + user_transfer_instructions .get(1) .map(|instruction| instruction.index), - InstructionError::InvalidTransferInstructionsCount, + InstructionError::InvalidUserTransferInstructionsCount { + found: user_transfer_instructions.len(), + }, )); } - // User have to wrap Sol + // User has to wrap Sol if transaction_data.accounts.mint_user == spl_token::native_mint::id() { // Sometimes the user doesn't have enough SOL, but we want the transaction to fail in the Express Relay program with InsufficientUserFunds // Therefore we allow the user to wrap less SOL than needed so it doesn't fail in the transfer instruction let amount_user_to_wrap = opportunity_swap_data.get_user_amount_to_wrap(transaction_data.data.amount_user); - if transfer_instructions.len() != 1 { + if user_transfer_instructions.is_empty() { return Err(RestError::InvalidInstruction( None, - InstructionError::InvalidTransferInstructionsCount, + InstructionError::InvalidUserTransferInstructionsCount { found: 0 }, )); } - let transfer_instruction = transfer_instructions[0].clone(); + let user_transfer_instruction = user_transfer_instructions[0].clone(); let user_ata = get_associated_token_address( &transaction_data.accounts.user_wallet, &spl_token::native_mint::id(), ); - if transfer_instruction.from != transaction_data.accounts.user_wallet { + if user_transfer_instruction.to != user_ata { return Err(RestError::InvalidInstruction( - Some(transfer_instruction.index), - InstructionError::InvalidFromAccountTransferInstruction { - expected: transaction_data.accounts.user_wallet, - found: transfer_instruction.from, - }, - )); - } - if transfer_instruction.to != user_ata { - return Err(RestError::InvalidInstruction( - Some(transfer_instruction.index), + Some(user_transfer_instruction.index), InstructionError::InvalidToAccountTransferInstruction { expected: user_ata, - found: transfer_instruction.to, + found: user_transfer_instruction.to, }, )); } // todo: remove swap_data.amount_user != transfer_instruction.lamports once searchers have updated their sdk - if transaction_data.data.amount_user != transfer_instruction.lamports - && amount_user_to_wrap != transfer_instruction.lamports + if transaction_data.data.amount_user != user_transfer_instruction.lamports + && amount_user_to_wrap != user_transfer_instruction.lamports { return Err(RestError::InvalidInstruction( - Some(transfer_instruction.index), + Some(user_transfer_instruction.index), InstructionError::InvalidAmountTransferInstruction { expected: amount_user_to_wrap, - found: transfer_instruction.lamports, + found: user_transfer_instruction.lamports, }, )); } } - // Searcher may want to wrap Sol - // We dont care about the amount here - else if transaction_data.accounts.mint_searcher == spl_token::native_mint::id() - && transfer_instructions.len() == 1 - { - let transfer_instruction = transfer_instructions[0].clone(); - let searcher_ata = get_associated_token_address( - &transaction_data.accounts.searcher, - &spl_token::native_mint::id(), - ); - if transfer_instruction.from != transaction_data.accounts.searcher { - return Err(RestError::InvalidInstruction( - Some(transfer_instruction.index), - InstructionError::InvalidFromAccountTransferInstruction { - expected: transaction_data.accounts.searcher, - found: transfer_instruction.from, - }, - )); - } - if transfer_instruction.to != searcher_ata { - return Err(RestError::InvalidInstruction( - Some(transfer_instruction.index), - InstructionError::InvalidToAccountTransferInstruction { - expected: searcher_ata, - found: transfer_instruction.to, - }, - )); - } - } - // No transfer instruction is allowed - else if !transfer_instructions.is_empty() { + // otherwise, if user mint is not SOL, then user should not be system transferring any SOL + else if !user_transfer_instructions.is_empty() { + let user_transfer_instruction = user_transfer_instructions[0].clone(); return Err(RestError::InvalidInstruction( - transfer_instructions - .first() - .map(|instruction| instruction.index), - InstructionError::TransferInstructionNotAllowed, + Some(user_transfer_instruction.index), + InstructionError::InvalidUserAccountTransferInstruction, )); } @@ -673,6 +643,7 @@ impl Service { .collect() } + // Ensures that there is exactly 1 SyncNative instruction for the user's WSOL ata in the transaction. async fn check_sync_native_instruction_exists( &self, tx: &VersionedTransaction, @@ -769,6 +740,7 @@ impl Service { Ok(result) } + // Ensures that the user WSOL account and searcher WSOL account are closed correctly if required. Ensures no other token account close instructions. async fn check_close_account_instruction( &self, tx: &VersionedTransaction, @@ -876,38 +848,33 @@ impl Service { Ok(()) } + // Ensures that the necessary memo instruction (if applicable) is present in the transaction. async fn check_memo_instructions( tx: &VersionedTransaction, memo: &Option, ) -> Result<(), RestError> { let memo_instructions = Self::extract_program_instructions(tx, &spl_memo_client::ID); match (memo, memo_instructions.len()) { - (None, 0) => Ok(()), - (Some(memo), 1) => { - let (index, instruction) = memo_instructions[0]; // safe to index because we checked the length - if instruction.data != memo.as_bytes() { - return Err(RestError::InvalidInstruction( - Some(index), - InstructionError::InvalidMemoString { - expected: memo.clone(), - found: String::from_utf8(instruction.data.clone()) - .unwrap_or_default(), - }, - )); - } + (None, _) => Ok(()), + (Some(_), 0) => Ok(()), // todo: this is for backward compatibility, we should remove this line once searchers have updated their sdk + (Some(memo), _) => { + memo_instructions + .iter() + .find(|(_, instruction)| instruction.data == memo.as_bytes()) + .ok_or_else(|| { + RestError::InvalidInstruction( + None, + InstructionError::MissingMemoInstruction { + expected: memo.clone(), + }, + ) + })?; Ok(()) } - (Some(_), 0) => Ok(()), // todo: this is for backward compatibility, we should remove this line once searchers have updated their sdk - (_, _) => Err(RestError::InvalidInstruction( - None, - InstructionError::InvalidMemoInstructionCount { - expected: memo.as_ref().map_or(0, |_| 1), - found: memo_instructions.len(), - }, - )), } } + // Ensures that the necessary create ATA instructions are present in the transaction (and are correctly paid for). async fn check_create_ata_instructions( &self, tx: &VersionedTransaction, @@ -996,7 +963,8 @@ impl Service { // we rely on the simulation to check the other token accounts are created // but we enforce here that the searcher pays for their creation - // this includes searcher token accounts and fee token accounts + // this includes searcher token accounts and fee token accounts and other arbitrary token accounts. + // Having this check apply to arbitrary token accounts helps to ensure that the searcher (not the user or the relayer) pays for the creation costs. for account in create_ata_instructions { if account.payer != swap_accounts.searcher { return Err(RestError::InvalidInstruction( @@ -1012,17 +980,18 @@ impl Service { Ok(()) } + // Ensures that user SOL transfer/wrap/unwrap/close instructions are correct. async fn check_wrap_unwrap_native_token_instructions( &self, tx: &VersionedTransaction, transaction_data: &entities::BidTransactionDataSwap, opportunity_swap_data: &OpportunitySvmProgramSwap, ) -> Result<(), RestError> { - self.check_transfer_instruction(tx, transaction_data, opportunity_swap_data) + self.check_user_transfer_instruction(tx, transaction_data, opportunity_swap_data) .await?; if transaction_data.accounts.mint_user == spl_token::native_mint::id() { // User has to wrap Sol - // So we need to check if there is a sync native instruction + // So we need to check if there is a sync native instruction for the user wsol ata self.check_sync_native_instruction_exists(tx, &transaction_data.accounts.user_wallet) .await?; } @@ -1085,15 +1054,15 @@ impl Service { .. } = transaction_data.accounts; + let quote_tokens = get_swap_quote_tokens(&opp); + let opportunity_swap_data = get_opportunity_swap_data(&opp); + self.check_svm_swap_bid_fields(bid_data, opportunity_swap_data, "e_tokens) + .await?; self.validate_swap_transaction_instructions( bid_chain_data_create_svm.get_transaction(), &user_wallet, ) .await?; - let quote_tokens = get_swap_quote_tokens(&opp); - let opportunity_swap_data = get_opportunity_swap_data(&opp); - self.check_svm_swap_bid_fields(bid_data, opportunity_swap_data, "e_tokens) - .await?; Self::check_memo_instructions(&bid_data.transaction, &opportunity_swap_data.memo) .await?; @@ -3095,7 +3064,7 @@ mod tests { } #[tokio::test] - async fn test_verify_bid_when_no_transfer_instruction() { + async fn test_verify_bid_when_no_user_transfer_instruction() { let (service, opportunities) = get_service(false); let searcher = Keypair::new(); let opportunity = opportunities.user_token_wsol.clone(); @@ -3113,15 +3082,18 @@ mod tests { get_verify_bid_result(service, searcher, vec![swap_instruction], opportunity).await; assert_eq!( result.unwrap_err(), - RestError::InvalidInstruction(None, InstructionError::InvalidTransferInstructionsCount) + RestError::InvalidInstruction( + None, + InstructionError::InvalidUserTransferInstructionsCount { found: 0 } + ) ); } #[tokio::test] - async fn test_verify_bid_when_no_transfer_instruction_is_allowed() { + async fn test_verify_bid_when_multiple_user_transfer_instructions() { let (service, opportunities) = get_service(false); let searcher = Keypair::new(); - let opportunity = opportunities.user_token_specified.clone(); + let opportunity = opportunities.user_token_wsol.clone(); let swap_instruction = svm::Svm::get_swap_instruction(GetSwapInstructionParams { searcher: searcher.pubkey(), opportunity_params: get_opportunity_params(opportunity.clone()), @@ -3132,67 +3104,43 @@ mod tests { relayer_signer: service.config.chain_config.express_relay.relayer.pubkey(), }) .unwrap(); - let transfer_instruction = - system_instruction::transfer(&searcher.pubkey(), &Pubkey::new_unique(), 1); - let result = get_verify_bid_result( - service, - searcher, - vec![swap_instruction, transfer_instruction], - opportunity, - ) - .await; - assert_eq!( - result.unwrap_err(), - RestError::InvalidInstruction(Some(1), InstructionError::TransferInstructionNotAllowed) + let program = match opportunity.program.clone() { + OpportunitySvmProgram::Swap(program) => program, + _ => panic!("Expected swap program"), + }; + let transfer_instruction = system_instruction::transfer( + &program.user_wallet_address, + &get_associated_token_address( + &program.user_wallet_address, + &spl_token::native_mint::id(), + ), + opportunity.buy_tokens[0].amount, ); - } - - #[tokio::test] - async fn test_verify_bid_when_no_close_account_instruction_is_allowed() { - let (service, opportunities) = get_service(false); - let searcher = Keypair::new(); - let opportunity = opportunities.user_token_specified.clone(); - let swap_instruction = svm::Svm::get_swap_instruction(GetSwapInstructionParams { - searcher: searcher.pubkey(), - opportunity_params: get_opportunity_params(opportunity.clone()), - bid_amount: 1, - deadline: (OffsetDateTime::now_utc() + Duration::seconds(30)) - .unix_timestamp(), - fee_receiver_relayer: Pubkey::new_unique(), - relayer_signer: service.config.chain_config.express_relay.relayer.pubkey(), - }) - .unwrap(); - let searcher_ata = - get_associated_token_address(&searcher.pubkey(), &spl_token::native_mint::id()); - let close_account_instruction = spl_token::instruction::close_account( - &spl_token::id(), - &searcher_ata, - &searcher.pubkey(), - &searcher.pubkey(), - &[], - ) - .unwrap(); let result = get_verify_bid_result( service, searcher, - vec![swap_instruction, close_account_instruction], + vec![ + swap_instruction, + transfer_instruction.clone(), + transfer_instruction, + ], opportunity, ) .await; assert_eq!( result.unwrap_err(), RestError::InvalidInstruction( - Some(1), - InstructionError::CloseAccountInstructionNotAllowed + Some(2), + InstructionError::InvalidUserTransferInstructionsCount { found: 2 } ) ); } #[tokio::test] - async fn test_verify_bid_when_multiple_transfer_instructions() { + async fn test_verify_bid_when_no_user_transfer_instruction_is_allowed() { let (service, opportunities) = get_service(false); let searcher = Keypair::new(); - let opportunity = opportunities.user_token_wsol.clone(); + let opportunity = opportunities.user_token_specified.clone(); let swap_instruction = svm::Svm::get_swap_instruction(GetSwapInstructionParams { searcher: searcher.pubkey(), opportunity_params: get_opportunity_params(opportunity.clone()), @@ -3203,33 +3151,33 @@ mod tests { relayer_signer: service.config.chain_config.express_relay.relayer.pubkey(), }) .unwrap(); + let program = match opportunity.program.clone() { + OpportunitySvmProgram::Swap(program) => program, + _ => panic!("Expected swap program"), + }; let transfer_instruction = - system_instruction::transfer(&searcher.pubkey(), &Pubkey::new_unique(), 1); + system_instruction::transfer(&program.user_wallet_address, &Pubkey::new_unique(), 1); let result = get_verify_bid_result( service, searcher, - vec![ - swap_instruction, - transfer_instruction.clone(), - transfer_instruction, - ], + vec![swap_instruction, transfer_instruction], opportunity, ) .await; assert_eq!( result.unwrap_err(), RestError::InvalidInstruction( - Some(2), - InstructionError::InvalidTransferInstructionsCount + Some(1), + InstructionError::InvalidUserAccountTransferInstruction ) ); } #[tokio::test] - async fn test_verify_bid_when_invalid_from_account_transfer_instruction() { + async fn test_verify_bid_when_no_close_account_instruction_is_allowed() { let (service, opportunities) = get_service(false); let searcher = Keypair::new(); - let opportunity = opportunities.user_token_wsol.clone(); + let opportunity = opportunities.user_token_specified.clone(); let swap_instruction = svm::Svm::get_swap_instruction(GetSwapInstructionParams { searcher: searcher.pubkey(), opportunity_params: get_opportunity_params(opportunity.clone()), @@ -3240,17 +3188,20 @@ mod tests { relayer_signer: service.config.chain_config.express_relay.relayer.pubkey(), }) .unwrap(); - let program = match opportunity.program.clone() { - OpportunitySvmProgram::Swap(program) => program, - _ => panic!("Expected swap program"), - }; - let expected = program.user_wallet_address; - let found = Pubkey::new_unique(); - let transfer_instruction = system_instruction::transfer(&found, &Pubkey::new_unique(), 1); + let searcher_ata = + get_associated_token_address(&searcher.pubkey(), &spl_token::native_mint::id()); + let close_account_instruction = spl_token::instruction::close_account( + &spl_token::id(), + &searcher_ata, + &searcher.pubkey(), + &searcher.pubkey(), + &[], + ) + .unwrap(); let result = get_verify_bid_result( service, searcher, - vec![swap_instruction, transfer_instruction], + vec![swap_instruction, close_account_instruction], opportunity, ) .await; @@ -3258,7 +3209,7 @@ mod tests { result.unwrap_err(), RestError::InvalidInstruction( Some(1), - InstructionError::InvalidFromAccountTransferInstruction { expected, found } + InstructionError::CloseAccountInstructionNotAllowed ) ); } @@ -4421,128 +4372,6 @@ mod tests { assert_eq!(result.1, bid_amount); } - #[tokio::test] - async fn test_verify_bid_when_invalid_searcher_account_from_transfer() { - let (service, opportunities) = get_service(true); - let opportunity = opportunities.searcher_token_wsol.clone(); - let bid_amount = 1; - let searcher = Keypair::new(); - let swap_instruction = svm::Svm::get_swap_instruction(GetSwapInstructionParams { - searcher: searcher.pubkey(), - opportunity_params: get_opportunity_params(opportunity.clone()), - bid_amount, - deadline: (OffsetDateTime::now_utc() + Duration::seconds(30)).unix_timestamp(), - fee_receiver_relayer: Pubkey::new_unique(), - relayer_signer: service.config.chain_config.express_relay.relayer.pubkey(), - }) - .unwrap(); - let program = match opportunity.program.clone() { - OpportunitySvmProgram::Swap(program) => program, - _ => panic!("Expected swap program"), - }; - let ata = get_associated_token_address( - &program.user_wallet_address, - &spl_token::native_mint::id(), - ); - let close_account_instruction = spl_token::instruction::close_account( - &spl_token::id(), - &ata, - &program.user_wallet_address, - &program.user_wallet_address, - &[], - ) - .unwrap(); - let searcher_ata = - get_associated_token_address(&searcher.pubkey(), &spl_token::native_mint::id()); - let expected = searcher.pubkey(); - let found = Pubkey::new_unique(); - let transfer_instruction_searcher = - system_instruction::transfer(&found, &searcher_ata, opportunity.buy_tokens[0].amount); - let sync_native_instruction_searcher = - spl_token::instruction::sync_native(&spl_token::id(), &searcher_ata).unwrap(); - let result = get_verify_bid_result( - service, - searcher, - vec![ - transfer_instruction_searcher, - sync_native_instruction_searcher, - swap_instruction, - close_account_instruction, - ], - opportunity, - ) - .await; - assert_eq!( - result.unwrap_err(), - RestError::InvalidInstruction( - Some(0), - InstructionError::InvalidFromAccountTransferInstruction { expected, found } - ) - ); - } - - #[tokio::test] - async fn test_verify_bid_when_invalid_searcher_account_to_transfer() { - let (service, opportunities) = get_service(true); - let opportunity = opportunities.searcher_token_wsol.clone(); - let bid_amount = 1; - let searcher = Keypair::new(); - let swap_instruction = svm::Svm::get_swap_instruction(GetSwapInstructionParams { - searcher: searcher.pubkey(), - opportunity_params: get_opportunity_params(opportunity.clone()), - bid_amount, - deadline: (OffsetDateTime::now_utc() + Duration::seconds(30)).unix_timestamp(), - fee_receiver_relayer: Pubkey::new_unique(), - relayer_signer: service.config.chain_config.express_relay.relayer.pubkey(), - }) - .unwrap(); - let program = match opportunity.program.clone() { - OpportunitySvmProgram::Swap(program) => program, - _ => panic!("Expected swap program"), - }; - let ata = get_associated_token_address( - &program.user_wallet_address, - &spl_token::native_mint::id(), - ); - let close_account_instruction = spl_token::instruction::close_account( - &spl_token::id(), - &ata, - &program.user_wallet_address, - &program.user_wallet_address, - &[], - ) - .unwrap(); - let expected = - get_associated_token_address(&searcher.pubkey(), &spl_token::native_mint::id()); - let found = Pubkey::new_unique(); - let transfer_instruction_searcher = system_instruction::transfer( - &searcher.pubkey(), - &found, - opportunity.buy_tokens[0].amount, - ); - let sync_native_instruction_searcher = - spl_token::instruction::sync_native(&spl_token::id(), &found).unwrap(); - let result = get_verify_bid_result( - service, - searcher, - vec![ - transfer_instruction_searcher, - sync_native_instruction_searcher, - swap_instruction, - close_account_instruction, - ], - opportunity, - ) - .await; - assert_eq!( - result.unwrap_err(), - RestError::InvalidInstruction( - Some(0), - InstructionError::InvalidToAccountTransferInstruction { expected, found } - ) - ); - } - #[tokio::test] async fn test_verify_bid_when_user_payer_missing_user_mint_ata_creation() { let (service, opportunities) = get_service(false); @@ -5140,10 +4969,9 @@ mod tests { assert_eq!( result.unwrap_err(), RestError::InvalidInstruction( - Some(0), - InstructionError::InvalidMemoString { + None, + InstructionError::MissingMemoInstruction { expected: "memo".to_string(), - found: "invalid memo".to_string(), } ) ); From 38bbb3dd99810ef79df733dbb30eb034a28b2bd2 Mon Sep 17 00:00:00 2001 From: --systemdf Date: Thu, 14 Aug 2025 12:27:30 -0700 Subject: [PATCH 08/12] ensure relayer signer not invoked --- .../src/auction/service/verification.rs | 63 ++++++++++++++++++- 1 file changed, 61 insertions(+), 2 deletions(-) diff --git a/auction-server/src/auction/service/verification.rs b/auction-server/src/auction/service/verification.rs index 73c2a6cb..0e529acc 100644 --- a/auction-server/src/auction/service/verification.rs +++ b/auction-server/src/auction/service/verification.rs @@ -255,6 +255,7 @@ impl Service { &self, tx: &VersionedTransaction, user_wallet: &Pubkey, + relayer_wallet: &Pubkey, ) -> Result<(), RestError> { for (index, ix) in tx.message.instructions().iter().enumerate() { self.validate_swap_transaction_instruction( @@ -262,6 +263,7 @@ impl Service { ix, tx, user_wallet, + relayer_wallet, index, ) .await?; @@ -277,6 +279,7 @@ impl Service { ix: &CompiledInstruction, tx: &VersionedTransaction, user_wallet: &Pubkey, + relayer_wallet: &Pubkey, ix_index: usize, ) -> Result<(), RestError> { match self.check_approved_program_instruction(program_id, ix) { @@ -285,7 +288,7 @@ impl Service { // TODO: this loop will be slow and invoke many rpc calls if there are many lookup accounts. either parallelize this extraction or limit number of lookup accounts for i in 0..ix.accounts.len() { let account_key = self.extract_account(tx, ix, i).await?; - if account_key == *user_wallet { + if (account_key == *user_wallet) | (account_key == *relayer_wallet) { return Err(RestError::InvalidInstruction(Some(ix_index), e)); } } @@ -1061,6 +1064,7 @@ impl Service { self.validate_swap_transaction_instructions( bid_chain_data_create_svm.get_transaction(), &user_wallet, + &self.config.chain_config.express_relay.relayer.pubkey(), ) .await?; @@ -2645,7 +2649,7 @@ mod tests { vec![AccountMeta { pubkey: user_wallet_address, is_signer: false, - is_writable: true, + is_writable: false, }], ), Instruction::new_with_bincode( @@ -2677,6 +2681,61 @@ mod tests { } } + #[tokio::test] + async fn test_verify_bid_when_arbitrary_program_invokes_relayer() { + let (service, opportunities) = get_service(true); + let opportunity = opportunities.user_token_specified.clone(); + let bid_amount = 1; + let searcher = Keypair::new(); + let swap_instruction = svm::Svm::get_swap_instruction(GetSwapInstructionParams { + searcher: searcher.pubkey(), + opportunity_params: get_opportunity_params(opportunity.clone()), + bid_amount, + deadline: (OffsetDateTime::now_utc() + Duration::seconds(30)).unix_timestamp(), + fee_receiver_relayer: Pubkey::new_unique(), + relayer_signer: service.config.chain_config.express_relay.relayer.pubkey(), + }) + .unwrap(); + let relayer = service.config.chain_config.express_relay.relayer.pubkey(); + let instructions = vec![ + Instruction::new_with_bincode( + Pubkey::new_unique(), + &"", + vec![AccountMeta { + pubkey: relayer, + is_signer: false, + is_writable: false, + }], + ), + Instruction::new_with_bincode( + Pubkey::new_unique(), + &"", + vec![AccountMeta { + pubkey: relayer, + is_signer: true, + is_writable: true, + }], + ), + ]; + for instruction in instructions.into_iter() { + let program_id = instruction.program_id; + let result = get_verify_bid_result( + service.clone(), + searcher.insecure_clone(), + vec![instruction, swap_instruction.clone()], + opportunities.user_token_specified.clone(), + ) + .await; + assert_eq!( + result.unwrap_err(), + RestError::InvalidInstruction( + Some(0), + InstructionError::UnapprovedProgramId(program_id) + ) + ); + } + } + #[tokio::test] async fn test_verify_bid_when_arbitrary_program_does_not_invoke_user() { let (service, opportunities) = get_service(true); From 8ce18172c837a9b7a3ba3875fb95d2d21d23a202 Mon Sep 17 00:00:00 2001 From: --systemdf Date: Thu, 14 Aug 2025 13:12:18 -0700 Subject: [PATCH 09/12] add transfer restriction on relayer signer --- auction-server/src/api.rs | 4 + .../src/auction/service/verification.rs | 74 ++++++++++++++++--- 2 files changed, 67 insertions(+), 11 deletions(-) diff --git a/auction-server/src/api.rs b/auction-server/src/api.rs index df926c6f..947f6d06 100644 --- a/auction-server/src/api.rs +++ b/auction-server/src/api.rs @@ -130,6 +130,7 @@ pub enum InstructionError { MissingCreateAtaInstruction(Pubkey), MissingMemoInstruction { expected: String }, UnapprovedProgramId(Pubkey), + RelayerTransferInstructionNotAllowed, } impl std::fmt::Display for InstructionError { @@ -265,6 +266,9 @@ impl std::fmt::Display for InstructionError { InstructionError::UnapprovedProgramId(program_id) => { write!(f, "Instruction has unapproved program id: {:?}", program_id) } + InstructionError::RelayerTransferInstructionNotAllowed => { + write!(f, "Relayer transfer instruction is not allowed") + } } } } diff --git a/auction-server/src/auction/service/verification.rs b/auction-server/src/auction/service/verification.rs index 0e529acc..4cdd3fe9 100644 --- a/auction-server/src/auction/service/verification.rs +++ b/auction-server/src/auction/service/verification.rs @@ -506,7 +506,6 @@ impl Service { async fn extract_transfer_instructions( &self, tx: &VersionedTransaction, - from: Pubkey, ) -> Result, RestError> { let instructions: Vec<(usize, &CompiledInstruction)> = Self::extract_program_instructions(tx, &system_program::id()) @@ -537,23 +536,40 @@ impl Service { )) } }; - if transfer_instruction.from == from { - result.push(transfer_instruction); - } + result.push(transfer_instruction); } Ok(result) } - // Ensures that any SOL transfer instruction from the user is only carried out when user token mint is SOL and user needs to wrap SOL. - async fn check_user_transfer_instruction( + // Ensures no SOL transfers from relayer. Ensures that any SOL transfer instruction from the user is only carried out when user token mint is SOL and user needs to wrap SOL. + async fn check_transfer_instructions( &self, tx: &VersionedTransaction, transaction_data: &entities::BidTransactionDataSwap, opportunity_swap_data: &OpportunitySvmProgramSwap, ) -> Result<(), RestError> { - let user_transfer_instructions = self - .extract_transfer_instructions(tx, transaction_data.accounts.user_wallet) - .await?; + let transfer_instructions: Vec = + self.extract_transfer_instructions(tx).await?; + + let relayer_transfer_instructions = transfer_instructions + .iter() + .filter(|instruction| { + instruction.from == self.config.chain_config.express_relay.relayer.pubkey() + }) + .collect::>(); + if !relayer_transfer_instructions.is_empty() { + return Err(RestError::InvalidInstruction( + relayer_transfer_instructions + .first() + .map(|instruction| instruction.index), + InstructionError::RelayerTransferInstructionNotAllowed, + )); + } + + let user_transfer_instructions = transfer_instructions + .iter() + .filter(|instruction| instruction.from == transaction_data.accounts.user_wallet) + .collect::>(); if user_transfer_instructions.len() > 1 { return Err(RestError::InvalidInstruction( user_transfer_instructions @@ -877,7 +893,7 @@ impl Service { } } - // Ensures that the necessary create ATA instructions are present in the transaction (and are correctly paid for). + // Ensures that the necessary create ATA instructions are present in the transaction (and are appropriately paid for). async fn check_create_ata_instructions( &self, tx: &VersionedTransaction, @@ -990,7 +1006,7 @@ impl Service { transaction_data: &entities::BidTransactionDataSwap, opportunity_swap_data: &OpportunitySvmProgramSwap, ) -> Result<(), RestError> { - self.check_user_transfer_instruction(tx, transaction_data, opportunity_swap_data) + self.check_transfer_instructions(tx, transaction_data, opportunity_swap_data) .await?; if transaction_data.accounts.mint_user == spl_token::native_mint::id() { // User has to wrap Sol @@ -3232,6 +3248,42 @@ mod tests { ); } + #[tokio::test] + async fn test_verify_bid_when_relayer_transfer_instruction() { + let (service, opportunities) = get_service(false); + let searcher = Keypair::new(); + let opportunity = opportunities.user_token_specified.clone(); + let swap_instruction = svm::Svm::get_swap_instruction(GetSwapInstructionParams { + searcher: searcher.pubkey(), + opportunity_params: get_opportunity_params(opportunity.clone()), + bid_amount: 1, + deadline: (OffsetDateTime::now_utc() + Duration::seconds(30)) + .unix_timestamp(), + fee_receiver_relayer: Pubkey::new_unique(), + relayer_signer: service.config.chain_config.express_relay.relayer.pubkey(), + }) + .unwrap(); + let transfer_instruction = system_instruction::transfer( + &service.config.chain_config.express_relay.relayer.pubkey(), + &Pubkey::new_unique(), + 1, + ); + let result = get_verify_bid_result( + service, + searcher, + vec![swap_instruction, transfer_instruction], + opportunity, + ) + .await; + assert_eq!( + result.unwrap_err(), + RestError::InvalidInstruction( + Some(1), + InstructionError::RelayerTransferInstructionNotAllowed + ) + ); + } + #[tokio::test] async fn test_verify_bid_when_no_close_account_instruction_is_allowed() { let (service, opportunities) = get_service(false); From 05ec8a69bdc006bad1d166aff441d5db17444af2 Mon Sep 17 00:00:00 2001 From: --systemdf Date: Thu, 14 Aug 2025 15:06:08 -0700 Subject: [PATCH 10/12] add moar tests --- .../src/auction/service/verification.rs | 87 ++++++++++++++++++- 1 file changed, 84 insertions(+), 3 deletions(-) diff --git a/auction-server/src/auction/service/verification.rs b/auction-server/src/auction/service/verification.rs index 4cdd3fe9..f3666a7e 100644 --- a/auction-server/src/auction/service/verification.rs +++ b/auction-server/src/auction/service/verification.rs @@ -583,8 +583,9 @@ impl Service { // User has to wrap Sol if transaction_data.accounts.mint_user == spl_token::native_mint::id() { - // Sometimes the user doesn't have enough SOL, but we want the transaction to fail in the Express Relay program with InsufficientUserFunds - // Therefore we allow the user to wrap less SOL than needed so it doesn't fail in the transfer instruction + // Sometimes the user doesn't have enough SOL, but we want the transaction to fail in the Express Relay program with InsufficientUserFunds. + // Sometimes the user will already have a WSOL account, and they don't need to wrap the full user_amount of SOL. + // For both these reasons, we allow the user to wrap less SOL than needed so it doesn't fail in the transfer instruction. let amount_user_to_wrap = opportunity_swap_data.get_user_amount_to_wrap(transaction_data.data.amount_user); @@ -2752,6 +2753,85 @@ mod tests { } } + #[tokio::test] + async fn test_verify_bid_when_create_ata_relayer_payer() { + let (service, opportunities) = get_service(true); + let opportunity = opportunities.user_token_specified.clone(); + let bid_amount = 1; + let searcher = Keypair::new(); + let swap_instruction = svm::Svm::get_swap_instruction(GetSwapInstructionParams { + searcher: searcher.pubkey(), + opportunity_params: get_opportunity_params(opportunity.clone()), + bid_amount, + deadline: (OffsetDateTime::now_utc() + Duration::seconds(30)).unix_timestamp(), + fee_receiver_relayer: Pubkey::new_unique(), + relayer_signer: service.config.chain_config.express_relay.relayer.pubkey(), + }) + .unwrap(); + let relayer = service.config.chain_config.express_relay.relayer.pubkey(); + let program = match opportunity.program.clone() { + OpportunitySvmProgram::Swap(program) => program, + _ => panic!("Expected swap program"), + }; + let create_ata_instruction = + spl_associated_token_account::instruction::create_associated_token_account( + &relayer, + &relayer, + &opportunity.buy_tokens[0].token, + &program.token_program_user, + ); + let result = get_verify_bid_result( + service.clone(), + searcher.insecure_clone(), + vec![create_ata_instruction, swap_instruction.clone()], + opportunities.user_token_specified.clone(), + ) + .await; + assert_eq!( + result.unwrap_err(), + RestError::InvalidInstruction( + Some(0), + InstructionError::InvalidPayerInCreateAtaInstruction { + expected: searcher.pubkey(), + found: relayer, + } + ) + ); + } + + #[tokio::test] + async fn test_verify_bid_when_sol_transfer_relayer() { + let (service, opportunities) = get_service(true); + let opportunity = opportunities.user_token_specified.clone(); + let bid_amount = 1; + let searcher = Keypair::new(); + let swap_instruction = svm::Svm::get_swap_instruction(GetSwapInstructionParams { + searcher: searcher.pubkey(), + opportunity_params: get_opportunity_params(opportunity.clone()), + bid_amount, + deadline: (OffsetDateTime::now_utc() + Duration::seconds(30)).unix_timestamp(), + fee_receiver_relayer: Pubkey::new_unique(), + relayer_signer: service.config.chain_config.express_relay.relayer.pubkey(), + }) + .unwrap(); + let relayer = service.config.chain_config.express_relay.relayer.pubkey(); + let transfer_instruction = system_instruction::transfer(&relayer, &Pubkey::new_unique(), 1); + let result = get_verify_bid_result( + service.clone(), + searcher.insecure_clone(), + vec![transfer_instruction, swap_instruction.clone()], + opportunities.user_token_specified.clone(), + ) + .await; + assert_eq!( + result.unwrap_err(), + RestError::InvalidInstruction( + Some(0), + InstructionError::RelayerTransferInstructionNotAllowed + ) + ); + } + #[tokio::test] async fn test_verify_bid_when_arbitrary_program_does_not_invoke_user() { let (service, opportunities) = get_service(true); @@ -5034,6 +5114,7 @@ mod tests { let bid_amount = 1; let searcher = Keypair::new(); let memo_instruction = svm::Svm::get_memo_instruction("memo".to_string()); + let memo_instruction_extra = svm::Svm::get_memo_instruction("extra memo".to_string()); let instruction = svm::Svm::get_swap_instruction(GetSwapInstructionParams { searcher: searcher.pubkey(), opportunity_params: get_opportunity_params(opportunity.clone()), @@ -5046,7 +5127,7 @@ mod tests { get_verify_bid_result( service, searcher.insecure_clone(), - vec![memo_instruction, instruction], + vec![memo_instruction, memo_instruction_extra, instruction], opportunity.clone(), ) .await From 9b27925ae55c8905f3cfef8cd9b0380cd595196d Mon Sep 17 00:00:00 2001 From: --systemdf Date: Thu, 14 Aug 2025 15:35:24 -0700 Subject: [PATCH 11/12] get rid of comment (lookup tables already cached) --- auction-server/src/auction/service/verification.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/auction-server/src/auction/service/verification.rs b/auction-server/src/auction/service/verification.rs index f3666a7e..e403d255 100644 --- a/auction-server/src/auction/service/verification.rs +++ b/auction-server/src/auction/service/verification.rs @@ -285,7 +285,6 @@ impl Service { match self.check_approved_program_instruction(program_id, ix) { Ok(()) => Ok(()), Err(e) => { - // TODO: this loop will be slow and invoke many rpc calls if there are many lookup accounts. either parallelize this extraction or limit number of lookup accounts for i in 0..ix.accounts.len() { let account_key = self.extract_account(tx, ix, i).await?; if (account_key == *user_wallet) | (account_key == *relayer_wallet) { From 353f974bf67a76638d8354c307a9bae5535c9a43 Mon Sep 17 00:00:00 2001 From: --systemdf Date: Fri, 15 Aug 2025 10:00:38 -0700 Subject: [PATCH 12/12] address comments --- auction-server/src/api.rs | 10 +-- .../src/auction/service/verification.rs | 64 +++++-------------- 2 files changed, 22 insertions(+), 52 deletions(-) diff --git a/auction-server/src/api.rs b/auction-server/src/api.rs index 947f6d06..a39c4072 100644 --- a/auction-server/src/api.rs +++ b/auction-server/src/api.rs @@ -159,24 +159,24 @@ impl std::fmt::Display for InstructionError { } InstructionError::InvalidUserTransferInstructionsCount { found } => { match found { - 0 => write!(f, "At least one sol transfer instruction from the user wallet account is required"), - _ => write!(f, "Invalid number ({}) of sol transfer instructions from the user wallet account", found), + 0 => write!(f, "At least one SOL transfer instruction from the user wallet account is required"), + _ => write!(f, "Invalid number ({}) of SOL transfer instructions from the user wallet account", found), } } InstructionError::InvalidUserAccountTransferInstruction => { - write!(f, "Invalid sol transfer instruction from the user account.") + write!(f, "Invalid SOL transfer instruction from the user account.") } InstructionError::InvalidToAccountTransferInstruction { expected, found } => { write!( f, - "Invalid to account in sol transfer instruction. Expected: {:?} found: {:?}", + "Invalid to account in SOL transfer instruction. Expected: {:?} found: {:?}", expected, found ) } InstructionError::InvalidAmountTransferInstruction { expected, found } => { write!( f, - "Invalid amount in sol transfer instruction. Expected: {:?} found: {:?}", + "Invalid amount in SOL transfer instruction. Expected: {:?} found: {:?}", expected, found ) } diff --git a/auction-server/src/auction/service/verification.rs b/auction-server/src/auction/service/verification.rs index e403d255..f15034f8 100644 --- a/auction-server/src/auction/service/verification.rs +++ b/auction-server/src/auction/service/verification.rs @@ -583,18 +583,20 @@ impl Service { // User has to wrap Sol if transaction_data.accounts.mint_user == spl_token::native_mint::id() { // Sometimes the user doesn't have enough SOL, but we want the transaction to fail in the Express Relay program with InsufficientUserFunds. - // Sometimes the user will already have a WSOL account, and they don't need to wrap the full user_amount of SOL. + // Sometimes the user will already have a WSOL account, and they might not be able to wrap the necessary amount from their SOL balance. In this case, we want to enable the tx to proceed with whatever they have in SOL being wrapped. The tx should still succeed if they have enough SOL + WSOL to cover the amount_user. // For both these reasons, we allow the user to wrap less SOL than needed so it doesn't fail in the transfer instruction. let amount_user_to_wrap = opportunity_swap_data.get_user_amount_to_wrap(transaction_data.data.amount_user); - if user_transfer_instructions.is_empty() { - return Err(RestError::InvalidInstruction( - None, - InstructionError::InvalidUserTransferInstructionsCount { found: 0 }, - )); - } - let user_transfer_instruction = user_transfer_instructions[0].clone(); + let user_transfer_instruction = match user_transfer_instructions.first() { + Some(instruction) => instruction, + None => { + return Err(RestError::InvalidInstruction( + None, + InstructionError::InvalidUserTransferInstructionsCount { found: 0 }, + )); + } + }; let user_ata = get_associated_token_address( &transaction_data.accounts.user_wallet, &spl_token::native_mint::id(), @@ -623,9 +625,10 @@ impl Service { } // otherwise, if user mint is not SOL, then user should not be system transferring any SOL else if !user_transfer_instructions.is_empty() { - let user_transfer_instruction = user_transfer_instructions[0].clone(); return Err(RestError::InvalidInstruction( - Some(user_transfer_instruction.index), + transfer_instructions + .first() + .map(|instruction| instruction.index), InstructionError::InvalidUserAccountTransferInstruction, )); } @@ -1073,16 +1076,16 @@ impl Service { .. } = transaction_data.accounts; - let quote_tokens = get_swap_quote_tokens(&opp); - let opportunity_swap_data = get_opportunity_swap_data(&opp); - self.check_svm_swap_bid_fields(bid_data, opportunity_swap_data, "e_tokens) - .await?; self.validate_swap_transaction_instructions( bid_chain_data_create_svm.get_transaction(), &user_wallet, &self.config.chain_config.express_relay.relayer.pubkey(), ) .await?; + let quote_tokens = get_swap_quote_tokens(&opp); + let opportunity_swap_data = get_opportunity_swap_data(&opp); + self.check_svm_swap_bid_fields(bid_data, opportunity_swap_data, "e_tokens) + .await?; Self::check_memo_instructions(&bid_data.transaction, &opportunity_swap_data.memo) .await?; @@ -2798,39 +2801,6 @@ mod tests { ); } - #[tokio::test] - async fn test_verify_bid_when_sol_transfer_relayer() { - let (service, opportunities) = get_service(true); - let opportunity = opportunities.user_token_specified.clone(); - let bid_amount = 1; - let searcher = Keypair::new(); - let swap_instruction = svm::Svm::get_swap_instruction(GetSwapInstructionParams { - searcher: searcher.pubkey(), - opportunity_params: get_opportunity_params(opportunity.clone()), - bid_amount, - deadline: (OffsetDateTime::now_utc() + Duration::seconds(30)).unix_timestamp(), - fee_receiver_relayer: Pubkey::new_unique(), - relayer_signer: service.config.chain_config.express_relay.relayer.pubkey(), - }) - .unwrap(); - let relayer = service.config.chain_config.express_relay.relayer.pubkey(); - let transfer_instruction = system_instruction::transfer(&relayer, &Pubkey::new_unique(), 1); - let result = get_verify_bid_result( - service.clone(), - searcher.insecure_clone(), - vec![transfer_instruction, swap_instruction.clone()], - opportunities.user_token_specified.clone(), - ) - .await; - assert_eq!( - result.unwrap_err(), - RestError::InvalidInstruction( - Some(0), - InstructionError::RelayerTransferInstructionNotAllowed - ) - ); - } - #[tokio::test] async fn test_verify_bid_when_arbitrary_program_does_not_invoke_user() { let (service, opportunities) = get_service(true);