make the LiteSVM struct thread-safe (#127)

* make the LiteSVM struct thread-safe * switch to upload-artifact v4 to fix ci * replace more download-artifact usage
LiteSVM · Jan 23, 2025 · cb0a442 · cb0a442
1 parent 26e57fc
commit cb0a442
Show file tree

Hide file tree

Showing 2 changed files with 61 additions and 41 deletions.
diff --git a/.github/workflows/node-build.yml b/.github/workflows/node-build.yml
@@ -97,7 +97,7 @@ jobs:
         if: ${{ !matrix.settings.docker }}
         shell: bash
       - name: Upload artifact
-        uses: actions/upload-artifact@v3
+        uses: actions/upload-artifact@v4
         with:
           name: bindings-${{ matrix.settings.target }}
           path: crates/node-litesvm/litesvm/litesvm.*.node
@@ -124,7 +124,7 @@ jobs:
       - name: Install dependencies
         run: cd crates/node-litesvm && yarn install
       - name: Download artifacts
-        uses: actions/download-artifact@v3
+        uses: actions/download-artifact@v4
         with:
           name: bindings-x86_64-unknown-linux-gnu
           path: crates/node-litesvm/litesvm/
@@ -155,7 +155,7 @@ jobs:
           yarn config set supportedArchitectures.libc "musl"
           yarn install
       - name: Download artifacts
-        uses: actions/download-artifact@v3
+        uses: actions/download-artifact@v4
         with:
           name: bindings-x86_64-unknown-linux-musl
           path: crates/node-litesvm/litesvm/
@@ -178,12 +178,12 @@ jobs:
       - name: Install dependencies
         run: cd crates/node-litesvm && yarn install
       - name: Download macOS x64 artifact
-        uses: actions/download-artifact@v3
+        uses: actions/download-artifact@v4
         with:
           name: bindings-x86_64-apple-darwin
           path: crates/node-litesvm/artifacts
       - name: Download macOS arm64 artifact
-        uses: actions/download-artifact@v3
+        uses: actions/download-artifact@v4
         with:
           name: bindings-aarch64-apple-darwin
           path: crates/node-litesvm/artifacts
@@ -192,7 +192,7 @@ jobs:
       - name: Test universal binary
         run: cd crates/node-litesvm && cp litesvm.darwin-universal.node litesvm/ && yarn test
       - name: Upload artifact
-        uses: actions/upload-artifact@v3
+        uses: actions/upload-artifact@v4
         with:
           name: bindings-universal-apple-darwin
           path: crates/node-litesvm/litesvm/litesvm.*.node

diff --git a/crates/litesvm/src/lib.rs b/crates/litesvm/src/lib.rs
@@ -81,7 +81,6 @@ pub struct LiteSVM {
     airdrop_kp: Keypair,
     feature_set: Arc<FeatureSet>,
     latest_blockhash: Hash,
-    log_collector: Rc<RefCell<LogCollector>>,
     history: TransactionHistory,
     compute_budget: Option<ComputeBudget>,
     sigverify: bool,
@@ -97,7 +96,6 @@ impl Default for LiteSVM {
             airdrop_kp: Keypair::new(),
             feature_set: Default::default(),
             latest_blockhash: create_blockhash(b"genesis"),
-            log_collector: Default::default(),
             history: TransactionHistory::new(),
             compute_budget: None,
             sigverify: false,
@@ -404,7 +402,7 @@ impl LiteSVM {
             .unwrap_or_default()
             .slot;
         let mut loaded_program = solana_bpf_loader_program::load_program_from_bytes(
-            Some(self.log_collector.clone()),
+            None,
             &mut LoadProgramMetrics::default(),
             account.data(),
             account.owner(),
@@ -489,6 +487,7 @@ impl LiteSVM {
         &self,
         tx: &SanitizedTransaction,
         compute_budget_limits: ComputeBudgetLimits,
+        log_collector: Rc<RefCell<LogCollector>>,
     ) -> (
         Result<(), TransactionError>,
         u64,
@@ -646,7 +645,7 @@ impl LiteSVM {
                             0,
                             &self.accounts.sysvar_cache,
                         ),
-                        Some(self.log_collector.clone()),
+                        Some(log_collector),
                         compute_budget,
                     ),
                     &mut ExecuteTimings::default(),
@@ -704,21 +703,30 @@ impl LiteSVM {
         Ok(())
     }
 
-    fn execute_transaction_no_verify(&mut self, tx: VersionedTransaction) -> ExecutionResult {
+    fn execute_transaction_no_verify(
+        &mut self,
+        tx: VersionedTransaction,
+        log_collector: Rc<RefCell<LogCollector>>,
+    ) -> ExecutionResult {
         map_sanitize_result(self.sanitize_transaction_no_verify(tx), |s_tx| {
-            self.execute_sanitized_transaction(s_tx)
+            self.execute_sanitized_transaction(s_tx, log_collector)
         })
     }
 
-    fn execute_transaction(&mut self, tx: VersionedTransaction) -> ExecutionResult {
+    fn execute_transaction(
+        &mut self,
+        tx: VersionedTransaction,
+        log_collector: Rc<RefCell<LogCollector>>,
+    ) -> ExecutionResult {
         map_sanitize_result(self.sanitize_transaction(tx), |s_tx| {
-            self.execute_sanitized_transaction(s_tx)
+            self.execute_sanitized_transaction(s_tx, log_collector)
         })
     }
 
     fn execute_sanitized_transaction(
         &mut self,
         sanitized_tx: SanitizedTransaction,
+        log_collector: Rc<RefCell<LogCollector>>,
     ) -> ExecutionResult {
         let CheckAndProcessTransactionSuccess {
             core:
@@ -729,7 +737,7 @@ impl LiteSVM {
                 },
             fee,
             payer_key,
-        } = match self.check_and_process_transaction(&sanitized_tx) {
+        } = match self.check_and_process_transaction(&sanitized_tx, log_collector) {
             Ok(value) => value,
             Err(value) => return value,
         };
@@ -744,6 +752,7 @@ impl LiteSVM {
     fn execute_sanitized_transaction_readonly(
         &self,
         sanitized_tx: SanitizedTransaction,
+        log_collector: Rc<RefCell<LogCollector>>,
     ) -> ExecutionResult {
         let CheckAndProcessTransactionSuccess {
             core:
@@ -753,7 +762,7 @@ impl LiteSVM {
                     context,
                 },
             ..
-        } = match self.check_and_process_transaction(&sanitized_tx) {
+        } = match self.check_and_process_transaction(&sanitized_tx, log_collector) {
             Ok(value) => value,
             Err(value) => return value,
         };
@@ -782,12 +791,13 @@ impl LiteSVM {
     fn check_and_process_transaction(
         &self,
         sanitized_tx: &SanitizedTransaction,
+        log_collector: Rc<RefCell<LogCollector>>,
     ) -> Result<CheckAndProcessTransactionSuccess, ExecutionResult> {
         self.maybe_blockhash_check(sanitized_tx)?;
         let compute_budget_limits = get_compute_budget_limits(sanitized_tx, &self.feature_set)?;
         self.maybe_history_check(sanitized_tx)?;
         let (result, compute_units_consumed, context, fee, payer_key) =
-            self.process_transaction(sanitized_tx, compute_budget_limits);
+            self.process_transaction(sanitized_tx, compute_budget_limits, log_collector);
         Ok(CheckAndProcessTransactionSuccess {
             core: {
                 CheckAndProcessTransactionSuccessCore {
@@ -824,20 +834,33 @@ impl LiteSVM {
         Ok(())
     }
 
-    fn execute_transaction_readonly(&self, tx: VersionedTransaction) -> ExecutionResult {
+    fn execute_transaction_readonly(
+        &self,
+        tx: VersionedTransaction,
+        log_collector: Rc<RefCell<LogCollector>>,
+    ) -> ExecutionResult {
         map_sanitize_result(self.sanitize_transaction(tx), |s_tx| {
-            self.execute_sanitized_transaction_readonly(s_tx)
+            self.execute_sanitized_transaction_readonly(s_tx, log_collector)
         })
     }
 
-    fn execute_transaction_no_verify_readonly(&self, tx: VersionedTransaction) -> ExecutionResult {
+    fn execute_transaction_no_verify_readonly(
+        &self,
+        tx: VersionedTransaction,
+        log_collector: Rc<RefCell<LogCollector>>,
+    ) -> ExecutionResult {
         map_sanitize_result(self.sanitize_transaction_no_verify(tx), |s_tx| {
-            self.execute_sanitized_transaction_readonly(s_tx)
+            self.execute_sanitized_transaction_readonly(s_tx, log_collector)
         })
     }
 
     /// Submits a signed transaction.
     pub fn send_transaction(&mut self, tx: impl Into<VersionedTransaction>) -> TransactionResult {
+        let log_collector = LogCollector {
+            bytes_limit: self.log_bytes_limit,
+            ..Default::default()
+        };
+        let log_collector = Rc::new(RefCell::new(log_collector));
         let vtx: VersionedTransaction = tx.into();
         let ExecutionResult {
             post_accounts,
@@ -848,19 +871,15 @@ impl LiteSVM {
             return_data,
             included,
         } = if self.sigverify {
-            self.execute_transaction(vtx)
+            self.execute_transaction(vtx, log_collector.clone())
         } else {
-            self.execute_transaction_no_verify(vtx)
+            self.execute_transaction_no_verify(vtx, log_collector.clone())
+        };
+        let Ok(logs) = Rc::try_unwrap(log_collector).map(|lc| lc.into_inner().messages) else {
+            unreachable!("Log collector should not be used after send_transaction returns")
         };
-
         let meta = TransactionMetadata {
-            logs: self
-                .log_collector
-                .replace(LogCollector {
-                    bytes_limit: self.log_bytes_limit,
-                    ..Default::default()
-                })
-                .into_messages(),
+            logs,
             inner_instructions,
             compute_units_consumed,
             return_data,
@@ -889,6 +908,11 @@ impl LiteSVM {
         &self,
         tx: impl Into<VersionedTransaction>,
     ) -> Result<SimulatedTransactionInfo, FailedTransactionMetadata> {
+        let log_collector = LogCollector {
+            bytes_limit: self.log_bytes_limit,
+            ..Default::default()
+        };
+        let log_collector = Rc::new(RefCell::new(log_collector));
         let ExecutionResult {
             post_accounts,
             tx_result,
@@ -898,20 +922,16 @@ impl LiteSVM {
             return_data,
             ..
         } = if self.sigverify {
-            self.execute_transaction_readonly(tx.into())
+            self.execute_transaction_readonly(tx.into(), log_collector.clone())
         } else {
-            self.execute_transaction_no_verify_readonly(tx.into())
+            self.execute_transaction_no_verify_readonly(tx.into(), log_collector.clone())
+        };
+        let Ok(logs) = Rc::try_unwrap(log_collector).map(|lc| lc.into_inner().messages) else {
+            unreachable!("Log collector should not be used after simulate_transaction returns")
         };
-
         let meta = TransactionMetadata {
             signature,
-            logs: self
-                .log_collector
-                .replace(LogCollector {
-                    bytes_limit: self.log_bytes_limit,
-                    ..Default::default()
-                })
-                .into_messages(),
+            logs,
             inner_instructions,
             compute_units_consumed,
             return_data,