From 3b852f6a948ba6c7ae36ecdab85696e8dc284023 Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Wed, 18 Dec 2024 23:02:16 +0900 Subject: [PATCH] Replace Option> with saner type --- ledger/benches/blockstore_processor.rs | 4 ++-- ledger/src/blockstore_processor.rs | 28 +++++++++++++++----------- runtime/src/bank.rs | 17 +++++++++------- unified-scheduler-pool/src/lib.rs | 6 +++++- 4 files changed, 33 insertions(+), 22 deletions(-) diff --git a/ledger/benches/blockstore_processor.rs b/ledger/benches/blockstore_processor.rs index 711c5381b63b8c..afa293727720e4 100644 --- a/ledger/benches/blockstore_processor.rs +++ b/ledger/benches/blockstore_processor.rs @@ -12,7 +12,7 @@ use { genesis_utils::{create_genesis_config, GenesisConfigInfo}, }, solana_runtime::{ - bank::Bank, + bank::{Bank, PreCommitCallbackResult}, bank_forks::BankForks, prioritization_fee_cache::PrioritizationFeeCache, transaction_batch::{OwnedOrBorrowed, TransactionBatch}, @@ -162,7 +162,7 @@ fn bench_execute_batch( &mut timing, None, &prioritization_fee_cache, - None:: Option>>, + None:: PreCommitCallbackResult>>, ); } }); diff --git a/ledger/src/blockstore_processor.rs b/ledger/src/blockstore_processor.rs index dc8ae2bede64be..63c470c36d291f 100644 --- a/ledger/src/blockstore_processor.rs +++ b/ledger/src/blockstore_processor.rs @@ -28,7 +28,9 @@ use { solana_rayon_threadlimit::{get_max_thread_count, get_thread_count}, solana_runtime::{ accounts_background_service::{AbsRequestSender, SnapshotRequestKind}, - bank::{Bank, KeyedRewardsAndNumPartitions, TransactionBalancesSet}, + bank::{ + Bank, KeyedRewardsAndNumPartitions, PreCommitCallbackResult, TransactionBalancesSet, + }, bank_forks::{BankForks, SetRootError}, bank_utils, commitment::VOTE_THRESHOLD_SIZE, @@ -156,7 +158,7 @@ pub fn execute_batch( timings: &mut ExecuteTimings, log_messages_bytes_limit: Option, prioritization_fee_cache: &PrioritizationFeeCache, - pre_commit_callback: Option Option>>, + pre_commit_callback: Option PreCommitCallbackResult>>, ) -> Result<()> { let TransactionBatchWithIndexes { batch, @@ -183,11 +185,11 @@ pub fn execute_batch( transaction_indexes.to_mut().push(index); } }) - .is_some() + .map(|_| ()) } }); - let Some((commit_results, balances)) = batch + let Ok((commit_results, balances)) = batch .bank() .load_execute_and_commit_transactions_with_pre_commit_callback( batch, @@ -350,7 +352,7 @@ fn execute_batches_internal( &mut timings, log_messages_bytes_limit, prioritization_fee_cache, - None:: Option>>, + None:: PreCommitCallbackResult>>, )); let thread_index = replay_tx_thread_pool.current_thread_index().unwrap(); @@ -2391,7 +2393,7 @@ pub mod tests { solana_entry::entry::{create_ticks, next_entry, next_entry_mut}, solana_program_runtime::declare_process_instruction, solana_runtime::{ - bank::bank_hash_details::SlotDetails, + bank::{bank_hash_details::SlotDetails, PreCommitCallbackFailed}, genesis_utils::{ self, create_genesis_config_with_vote_accounts, ValidatorVoteKeypairs, }, @@ -5113,7 +5115,9 @@ pub mod tests { do_test_schedule_batches_for_execution(false); } - fn do_test_execute_batch_pre_commit_callback(poh_result: Option>) { + fn do_test_execute_batch_pre_commit_callback( + poh_result: PreCommitCallbackResult>, + ) { solana_logger::setup(); let dummy_leader_pubkey = solana_sdk::pubkey::new_rand(); let GenesisConfigInfo { @@ -5131,7 +5135,7 @@ pub mod tests { OwnedOrBorrowed::Borrowed(&txs[0..1]), ); batch.set_needs_unlock(false); - let poh_with_index = matches!(poh_result, Some(Some(_))); + let poh_with_index = matches!(poh_result, Ok(Some(_))); let transaction_indexes = if poh_with_index { vec![] } else { vec![3] }; let batch = TransactionBatchWithIndexes { batch, @@ -5152,7 +5156,7 @@ pub mod tests { &prioritization_fee_cache, Some(|| poh_result), ); - let should_succeed = poh_result.is_some(); + let should_succeed = poh_result.is_ok(); if should_succeed { assert_matches!(result, Ok(())); assert_eq!(bank.transaction_count(), 1); @@ -5179,17 +5183,17 @@ pub mod tests { #[test] fn test_execute_batch_pre_commit_callback_success() { - do_test_execute_batch_pre_commit_callback(Some(None)); + do_test_execute_batch_pre_commit_callback(Ok(None)); } #[test] fn test_execute_batch_pre_commit_callback_success_with_index() { - do_test_execute_batch_pre_commit_callback(Some(Some(4))); + do_test_execute_batch_pre_commit_callback(Ok(Some(4))); } #[test] fn test_execute_batch_pre_commit_callback_failure() { - do_test_execute_batch_pre_commit_callback(None); + do_test_execute_batch_pre_commit_callback(Err(PreCommitCallbackFailed)); } #[test] diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 4aca6b843ff839..b3ca5f489906d5 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -367,6 +367,10 @@ impl TransactionBalancesSet { } pub type TransactionBalances = Vec>; +#[derive(Clone, Copy, Debug)] +pub struct PreCommitCallbackFailed; +pub type PreCommitCallbackResult = std::result::Result; + #[derive(Serialize, Deserialize, Debug, PartialEq, Eq)] pub enum TransactionLogCollectorFilter { All, @@ -5041,12 +5045,11 @@ impl Bank { recording_config, timings, log_messages_bytes_limit, - None:: bool>, + None:: PreCommitCallbackResult<()>>, ) .unwrap() } - #[must_use] pub fn load_execute_and_commit_transactions_with_pre_commit_callback( &self, batch: &TransactionBatch, @@ -5055,8 +5058,8 @@ impl Bank { recording_config: ExecutionRecordingConfig, timings: &mut ExecuteTimings, log_messages_bytes_limit: Option, - pre_commit_callback: Option bool>, - ) -> Option<(Vec, TransactionBalancesSet)> { + pre_commit_callback: Option PreCommitCallbackResult<()>>, + ) -> PreCommitCallbackResult<(Vec, TransactionBalancesSet)> { let pre_balances = if collect_balances { self.collect_balances(batch) } else { @@ -5085,8 +5088,8 @@ impl Bank { if let Some(pre_commit_callback) = pre_commit_callback { if let Some(e) = processing_results.first() { assert_eq!(processing_results.len(), 1); - if e.is_ok() && !pre_commit_callback() { - return None; + if e.is_ok() && pre_commit_callback().is_err() { + return Err(PreCommitCallbackFailed); } } } @@ -5102,7 +5105,7 @@ impl Bank { } else { vec![] }; - Some(( + Ok(( commit_results, TransactionBalancesSet::new(pre_balances, post_balances), )) diff --git a/unified-scheduler-pool/src/lib.rs b/unified-scheduler-pool/src/lib.rs index 747ff0e4cbe2bf..f44e0bb3fc6e8c 100644 --- a/unified-scheduler-pool/src/lib.rs +++ b/unified-scheduler-pool/src/lib.rs @@ -22,6 +22,7 @@ use { }, solana_poh::poh_recorder::{RecordTransactionsSummary, TransactionRecorder}, solana_runtime::{ + bank::PreCommitCallbackFailed, installed_scheduler_pool::{ initialized_result_with_timings, InstalledScheduler, InstalledSchedulerBox, InstalledSchedulerPool, InstalledSchedulerPoolArc, ResultWithTimings, ScheduleResult, @@ -474,7 +475,10 @@ impl TaskHandler for DefaultTaskHandler { .as_ref() .unwrap() .record_transactions(bank.slot(), vec![transaction.to_versioned_transaction()]); - result.ok().map(|()| starting_transaction_index) + match result { + Ok(()) => Ok(starting_transaction_index), + Err(_) => Err(PreCommitCallbackFailed), + } }), };