Replace Option<Option<_>> with saner type

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::<fn() -> Option<Option<usize>>>,
+                None::<fn() -> PreCommitCallbackResult<Option<usize>>>,
             );
         }
     });

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<usize>,
     prioritization_fee_cache: &PrioritizationFeeCache,
-    pre_commit_callback: Option<impl FnOnce() -> Option<Option<usize>>>,
+    pre_commit_callback: Option<impl FnOnce() -> PreCommitCallbackResult<Option<usize>>>,
 ) -> 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::<fn() -> Option<Option<usize>>>,
+                    None::<fn() -> PreCommitCallbackResult<Option<usize>>>,
                 ));
 
                 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<Option<usize>>) {
+    fn do_test_execute_batch_pre_commit_callback(
+        poh_result: PreCommitCallbackResult<Option<usize>>,
+    ) {
         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]

runtime/src/bank.rs

@@ -367,6 +367,10 @@ impl TransactionBalancesSet {
 }
 pub type TransactionBalances = Vec<Vec<u64>>;
 
+#[derive(Clone, Copy, Debug)]
+pub struct PreCommitCallbackFailed;
+pub type PreCommitCallbackResult<T> = std::result::Result<T, PreCommitCallbackFailed>;
+
 #[derive(Serialize, Deserialize, Debug, PartialEq, Eq)]
 pub enum TransactionLogCollectorFilter {
     All,
@@ -5041,12 +5045,11 @@ impl Bank {
             recording_config,
             timings,
             log_messages_bytes_limit,
-            None::<fn() -> bool>,
+            None::<fn() -> PreCommitCallbackResult<()>>,
         )
         .unwrap()
     }
 
-    #[must_use]
     pub fn load_execute_and_commit_transactions_with_pre_commit_callback(
         &self,
         batch: &TransactionBatch<impl TransactionWithMeta>,
@@ -5055,8 +5058,8 @@ impl Bank {
         recording_config: ExecutionRecordingConfig,
         timings: &mut ExecuteTimings,
         log_messages_bytes_limit: Option<usize>,
-        pre_commit_callback: Option<impl FnOnce() -> bool>,
-    ) -> Option<(Vec<TransactionCommitResult>, TransactionBalancesSet)> {
+        pre_commit_callback: Option<impl FnOnce() -> PreCommitCallbackResult<()>>,
+    ) -> PreCommitCallbackResult<(Vec<TransactionCommitResult>, 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),
         ))

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),
+                }
             }),
         };