From 6de7452fe13025d09a19de2ba9c39b28fb529e67 Mon Sep 17 00:00:00 2001 From: Jack May Date: Tue, 19 Apr 2022 02:59:06 -0700 Subject: [PATCH 1/2] Fix signature count (#24471) * Fix signature count * protect the signature count futher (cherry picked from commit 5d1adf1270759970731301f064ebf9748e1c59c9) --- runtime/src/bank.rs | 84 ++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 79 insertions(+), 5 deletions(-) diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 59629234ad7737..37ef714b85a151 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -4096,8 +4096,6 @@ impl Bank { feature_set_clone_time.as_us() ); - signature_count += u64::from(tx.message().header().num_required_signatures); - let tx_wide_compute_cap = feature_set.is_active(&tx_wide_compute_cap::id()); let mut compute_budget = self .compute_budget @@ -4121,13 +4119,11 @@ impl Bank { } } - let durable_nonce_fee = nonce.as_ref().map(DurableNonceFee::from); - self.execute_loaded_transaction( tx, loaded_transaction, compute_budget, - durable_nonce_fee, + nonce.as_ref().map(DurableNonceFee::from), enable_cpi_recording, enable_log_recording, timings, @@ -4226,6 +4222,10 @@ impl Bank { } if execution_result.was_executed() { + // Signature count must be accumulated only if the transaction + // is executed, otherwise a mismatched count between banking and + // replay could occur + signature_count += u64::from(tx.message().header().num_required_signatures); executed_transactions_count += 1; } @@ -16136,6 +16136,80 @@ pub(crate) mod tests { bank.process_transaction(&tx).unwrap(); } + #[test] + fn test_failed_compute_request_instruction() { + solana_logger::setup(); + let GenesisConfigInfo { + genesis_config, + mint_keypair, + .. + } = create_genesis_config_with_leader( + 1_000_000_000_000_000, + &Pubkey::new_unique(), + bootstrap_validator_stake_lamports(), + ); + let mut bank = Bank::new_for_tests(&genesis_config); + + let payer0_keypair = Keypair::new(); + let payer1_keypair = Keypair::new(); + bank.transfer(10, &mint_keypair, &payer0_keypair.pubkey()) + .unwrap(); + bank.transfer(10, &mint_keypair, &payer1_keypair.pubkey()) + .unwrap(); + + fn mock_ix_processor( + _first_instruction_account: usize, + invoke_context: &mut InvokeContext, + ) -> std::result::Result<(), InstructionError> { + let compute_budget = invoke_context.get_compute_budget(); + assert_eq!( + *compute_budget, + ComputeBudget { + max_units: 1, + heap_size: Some(48 * 1024), + ..ComputeBudget::default() + } + ); + Ok(()) + } + let program_id = solana_sdk::pubkey::new_rand(); + bank.add_builtin("mock_program", &program_id, mock_ix_processor); + + // This message will not be executed because the compute budget request is invalid + let message0 = Message::new( + &[ + ComputeBudgetInstruction::request_heap_frame(1), + Instruction::new_with_bincode(program_id, &0, vec![]), + ], + Some(&payer0_keypair.pubkey()), + ); + // This message will be processed successfully + let message1 = Message::new( + &[ + ComputeBudgetInstruction::request_units(1, 0), + ComputeBudgetInstruction::request_heap_frame(48 * 1024), + Instruction::new_with_bincode(program_id, &0, vec![]), + ], + Some(&payer1_keypair.pubkey()), + ); + let txs = vec![ + Transaction::new(&[&payer0_keypair], message0, bank.last_blockhash()), + Transaction::new(&[&payer1_keypair], message1, bank.last_blockhash()), + ]; + let results = bank.process_transactions(txs.iter()); + + assert_eq!( + results[0], + Err(TransactionError::InstructionError( + 0, + InstructionError::InvalidInstructionData + )) + ); + assert_eq!(results[1], Ok(())); + // two transfers and the mock program + assert_eq!(bank.signature_count(), 3); + } + #[test] fn test_verify_and_hash_transaction_sig_len() { let GenesisConfigInfo { From 3b5c12cbd55464c87b096066e8540dcf0f6daaa4 Mon Sep 17 00:00:00 2001 From: Justin Starry Date: Tue, 19 Apr 2022 18:35:49 +0800 Subject: [PATCH 2/2] fix conflict --- runtime/src/bank.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 37ef714b85a151..1987eb2cdf547d 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -16159,6 +16159,7 @@ pub(crate) mod tests { fn mock_ix_processor( _first_instruction_account: usize, + _data: &[u8], invoke_context: &mut InvokeContext, ) -> std::result::Result<(), InstructionError> { let compute_budget = invoke_context.get_compute_budget();