From b4dbce7551603e615d431e9c47fbee443ed1c463 Mon Sep 17 00:00:00 2001 From: Colin Carey Date: Tue, 31 May 2022 16:31:46 -0700 Subject: [PATCH] Add inputn and output validation --- full-service/src/service/transaction.rs | 116 ++++++++++++++++++++++++ 1 file changed, 116 insertions(+) diff --git a/full-service/src/service/transaction.rs b/full-service/src/service/transaction.rs index 4b9132fd3..2fbd31e4d 100644 --- a/full-service/src/service/transaction.rs +++ b/full-service/src/service/transaction.rs @@ -24,6 +24,7 @@ use mc_connection::{BlockchainConnection, RetryableUserTxConnection, UserTxConne use mc_fog_report_validation::FogPubkeyResolver; use mc_ledger_db::Ledger; use mc_mobilecoind::payments::TxProposal; +use mc_transaction_core::constants::{MAX_INPUTS, MAX_OUTPUTS}; use crate::{ fog_resolver::FullServiceFogResolver, @@ -197,6 +198,8 @@ where fee: Option, tombstone_block: Option, ) -> Result<(UnsignedTx, FullServiceFogResolver), TransactionServiceError> { + validate_number_outputs(addresses_and_values.len() as u64)?; + let conn = self.wallet_db.get_conn()?; transaction(&conn, || { let mut builder = WalletTransactionBuilder::new( @@ -243,6 +246,9 @@ where max_spendable_value: Option, log_tx_proposal: Option, ) -> Result { + validate_number_inputs(input_txo_ids.unwrap_or(&Vec::new()).len() as u64)?; + validate_number_outputs(addresses_and_values.len() as u64)?; + let conn = self.wallet_db.get_conn()?; transaction(&conn, || { let mut builder = WalletTransactionBuilder::new( @@ -418,6 +424,26 @@ where } } +fn validate_number_inputs(num_inputs: u64) -> Result<(), TransactionServiceError> { + if num_inputs > MAX_INPUTS { + return Err(TransactionServiceError::TransactionBuilder(WalletTransactionBuilderError::InvalidArgument( + format!("Invalid number of input txos. {:?} txo ids provided but maximum allowed number of inputs is {:?}", num_inputs, MAX_INPUTS) + ))); + } + Ok(()) +} + +fn validate_number_outputs(num_outputs: u64) -> Result<(), TransactionServiceError> { + // maximum number of outputs is 16 but we reserve 1 for change + let max_outputs = MAX_OUTPUTS - 1; + if num_outputs > max_outputs { + return Err(TransactionServiceError::TransactionBuilder(WalletTransactionBuilderError::InvalidArgument( + format!("Invalid number of recipiants. {:?} recipiants provided but maximum allowed number of outputs is {:?}", num_outputs, max_outputs) + ))); + } + Ok(()) +} + #[cfg(test)] mod tests { use super::*; @@ -805,6 +831,96 @@ mod tests { }; } + #[test_with_logger] + fn test_maximum_inputs_and_outputs(logger: Logger) { + let mut rng: StdRng = SeedableRng::from_seed([20u8; 32]); + + let known_recipients: Vec = Vec::new(); + let mut ledger_db = get_test_ledger(5, &known_recipients, 12, &mut rng); + + let service = setup_wallet_service(ledger_db.clone(), logger.clone()); + + // Create our main account for the wallet + let alice = service + .create_account( + Some("Alice's Main Account".to_string()), + "".to_string(), + "".to_string(), + "".to_string(), + ) + .unwrap(); + + // Add a block with a transaction for Alice + let alice_account_key: AccountKey = mc_util_serial::decode(&alice.account_key).unwrap(); + let alice_account_id = AccountID::from(&alice_account_key); + let alice_public_address = alice_account_key.subaddress(alice.main_subaddress_index as u64); + add_block_to_ledger_db( + &mut ledger_db, + &vec![alice_public_address.clone()], + 100 * MOB, + &vec![KeyImage::from(rng.next_u64())], + &mut rng, + ); + + manually_sync_account(&ledger_db, &service.wallet_db, &alice_account_id, &logger); + + // test ouputs + let mut outputs = Vec::new(); + for _ in 0..17 { + outputs.push(( + b58_encode_public_address(&alice_public_address).unwrap(), + (42 * MOB).to_string(), + )); + } + match service.build_transaction( + &alice.account_id_hex, + &outputs, + None, + None, + None, + None, + None, + ) { + Ok(_) => { + panic!("Should not be able to build transaction with too many ouputs") + } + Err(TransactionServiceError::TransactionBuilder( + WalletTransactionBuilderError::InvalidArgument(_), + )) => {} + Err(e) => panic!("Unexpected error {:?}", e), + }; + + // test inputs + let mut outputs = Vec::new(); + for _ in 0..2 { + outputs.push(( + b58_encode_public_address(&alice_public_address).unwrap(), + (42 * MOB).to_string(), + )); + } + let mut inputs = Vec::new(); + for _ in 0..17 { + inputs.push("fake txo id".to_string()); + } + match service.build_transaction( + &alice.account_id_hex, + &outputs, + Some(&inputs), + None, + None, + None, + None, + ) { + Ok(_) => { + panic!("Should not be able to build transaction with too many inputs") + } + Err(TransactionServiceError::TransactionBuilder( + WalletTransactionBuilderError::InvalidArgument(_), + )) => {} + Err(e) => panic!("Unexpected error {:?}", e), + }; + } + // FIXME: Test with 0 change transactions // FIXME: Test with balance > u64::max // FIXME: sending a transaction with value > u64::max