Merge pull request #4782 from xxuejie/remove-transaction-snapshot

refactor: Remove TransactionSnapshot structure

Author: zhangsoledad

script/src/lib.rs

@@ -12,7 +12,7 @@ pub use crate::error::{ScriptError, TransactionScriptError};
 pub use crate::scheduler::{Scheduler, ROOT_VM_ID};
 pub use crate::types::{
     ChunkCommand, CoreMachine, DataPieceId, RunMode, ScriptGroup, ScriptGroupType, ScriptVersion,
-    TransactionSnapshot, TransactionState, TxData, VerifyResult, VmIsa, VmState, VmVersion,
+    TransactionState, TxData, VerifyResult, VmIsa, VmState, VmVersion,
 };
 pub use crate::verify::{TransactionScriptsSyscallsGenerator, TransactionScriptsVerifier};
 pub use crate::verify_env::TxVerifyEnv;

script/src/types.rs

@@ -1,4 +1,3 @@
-use ckb_error::Error;
 use ckb_types::{
     core::{Cycle, ScriptHashType},
     packed::{Byte32, Script},
@@ -186,20 +185,7 @@ impl fmt::Display for ScriptGroupType {
 }
 
 /// Struct specifies which script has verified so far.
-/// Snapshot is lifetime free, but capture snapshot need heavy memory copy
-pub struct TransactionSnapshot {
-    /// current suspended script index
-    pub current: usize,
-    /// vm snapshots
-    pub state: Option<FullSuspendedState>,
-    /// current consumed cycle
-    pub current_cycles: Cycle,
-    /// limit cycles when snapshot create
-    pub limit_cycles: Cycle,
-}
-
-/// Struct specifies which script has verified so far.
-/// State lifetime bound with vm machine.
+/// State is lifetime free, but capture snapshot need heavy memory copy
 pub struct TransactionState {
     /// current suspended script index
     pub current: usize,
@@ -240,41 +226,6 @@ impl TransactionState {
     }
 }
 
-impl TransactionSnapshot {
-    /// Return next limit cycles according to max_cycles and step_cycles
-    pub fn next_limit_cycles(&self, step_cycles: Cycle, max_cycles: Cycle) -> (Cycle, bool) {
-        let remain = max_cycles - self.current_cycles;
-        let next_limit = self.limit_cycles + step_cycles;
-
-        if next_limit < remain {
-            (next_limit, false)
-        } else {
-            (remain, true)
-        }
-    }
-}
-
-impl TryFrom<TransactionState> for TransactionSnapshot {
-    type Error = Error;
-
-    fn try_from(state: TransactionState) -> Result<Self, Self::Error> {
-        let TransactionState {
-            current,
-            state,
-            current_cycles,
-            limit_cycles,
-            ..
-        } = state;
-
-        Ok(TransactionSnapshot {
-            current,
-            state,
-            current_cycles,
-            limit_cycles,
-        })
-    }
-}
-
 /// Enum represent resumable verify result
 #[allow(clippy::large_enum_variant)]
 #[derive(Debug)]
@@ -285,16 +236,6 @@ pub enum VerifyResult {
     Suspended(TransactionState),
 }
 
-impl std::fmt::Debug for TransactionSnapshot {
-    fn fmt(&self, f: &mut ::core::fmt::Formatter) -> std::fmt::Result {
-        f.debug_struct("TransactionSnapshot")
-            .field("current", &self.current)
-            .field("current_cycles", &self.current_cycles)
-            .field("limit_cycles", &self.limit_cycles)
-            .finish()
-    }
-}
-
 impl std::fmt::Debug for TransactionState {
     fn fmt(&self, f: &mut ::core::fmt::Formatter) -> std::fmt::Result {
         f.debug_struct("TransactionState")

script/src/verify.rs

@@ -15,7 +15,7 @@ use crate::{
     type_id::TypeIdSystemScript,
     types::{
         CoreMachine, DebugPrinter, Indices, ScriptGroup, ScriptGroupType, ScriptVersion,
-        TransactionSnapshot, TransactionState, VerifyResult,
+        TransactionState, VerifyResult,
     },
     verify_env::TxVerifyEnv,
 };
@@ -730,7 +730,7 @@ where
     ///
     /// ## Params
     ///
-    /// * `snap` - Captured transaction verification snapshot.
+    /// * `snap` - Captured transaction verification state.
     ///
     /// * `limit_cycles` - Maximum allowed cycles to run the scripts. The verification quits early
     ///   when the consumed cycles exceed the limit.
@@ -741,7 +741,7 @@ where
     /// If verify is suspended, a borrowed state will returned.
     pub fn resume_from_snap(
         &self,
-        snap: &TransactionSnapshot,
+        snap: &TransactionState,
         limit_cycles: Cycle,
     ) -> Result<VerifyResult, Error> {
         let mut cycles = snap.current_cycles;
@@ -881,15 +881,15 @@ where
     ///
     /// ## Params
     ///
-    /// * `snap` - Captured transaction verification snapshot.
+    /// * `snap` - Captured transaction verification state.
     ///
     /// * `max_cycles` - Maximum allowed cycles to run the scripts. The verification quits early
     ///   when the consumed cycles exceed the limit.
     ///
     /// ## Returns
     ///
     /// It returns the total consumed cycles on completed, Otherwise it returns the verification error.
-    pub fn complete(&self, snap: &TransactionSnapshot, max_cycles: Cycle) -> Result<Cycle, Error> {
+    pub fn complete(&self, snap: &TransactionState, max_cycles: Cycle) -> Result<Cycle, Error> {
         let mut cycles = snap.current_cycles;
 
         let (_hash, current_group) = self.groups().nth(snap.current).ok_or_else(|| {

script/src/verify/tests/ckb_latest/features_since_v2021.rs

@@ -894,11 +894,11 @@ fn _check_typical_secp256k1_blake160_2_in_2_out_tx_with_snap(step_cycles: Cycle)
 
     let result = verifier.verify_map(script_version, &rtx, |verifier| {
         #[allow(unused_assignments)]
-        let mut init_snap: Option<TransactionSnapshot> = None;
+        let mut init_snap: Option<TransactionState> = None;
         let mut init_state: Option<TransactionState> = None;
 
         match verifier.resumable_verify(step_cycles).unwrap() {
-            VerifyResult::Suspended(state) => init_snap = Some(state.try_into().unwrap()),
+            VerifyResult::Suspended(state) => init_snap = Some(state),
             VerifyResult::Completed(cycle) => return Ok(cycle),
         }
 
@@ -911,7 +911,7 @@ fn _check_typical_secp256k1_blake160_2_in_2_out_tx_with_snap(step_cycles: Cycle)
                 match verifier.resume_from_snap(&snap, limit_cycles).unwrap() {
                     VerifyResult::Suspended(state) => {
                         if count % 500 == 0 {
-                            init_snap = Some(state.try_into().unwrap());
+                            init_snap = Some(state);
                         } else {
                             init_state = Some(state);
                         }
@@ -928,7 +928,7 @@ fn _check_typical_secp256k1_blake160_2_in_2_out_tx_with_snap(step_cycles: Cycle)
                 match verifier.resume_from_state(state, limit_cycles).unwrap() {
                     VerifyResult::Suspended(state) => {
                         if count % 500 == 0 {
-                            init_snap = Some(state.try_into().unwrap());
+                            init_snap = Some(state);
                         } else {
                             init_state = Some(state);
                         }
@@ -983,21 +983,21 @@ fn check_typical_secp256k1_blake160_2_in_2_out_tx_with_complete() {
     let mut cycles = 0;
     let verifier = TransactionScriptsVerifierWithEnv::new();
     let result = verifier.verify_map(script_version, &rtx, |verifier| {
-        let mut init_snap: Option<TransactionSnapshot> = None;
+        let mut init_snap: Option<TransactionState> = None;
 
         if let VerifyResult::Suspended(state) = verifier
             .resumable_verify(TWO_IN_TWO_OUT_CYCLES / 10)
             .unwrap()
         {
-            init_snap = Some(state.try_into().unwrap());
+            init_snap = Some(state);
         }
 
         for _ in 0..2 {
             let snap = init_snap.take().unwrap();
             let (limit_cycles, _last) =
                 snap.next_limit_cycles(TWO_IN_TWO_OUT_CYCLES / 10, TWO_IN_TWO_OUT_CYCLES);
             match verifier.resume_from_snap(&snap, limit_cycles).unwrap() {
-                VerifyResult::Suspended(state) => init_snap = Some(state.try_into().unwrap()),
+                VerifyResult::Suspended(state) => init_snap = Some(state),
                 VerifyResult::Completed(_) => {
                     unreachable!()
                 }
@@ -1133,10 +1133,10 @@ fn load_code_with_snapshot() {
     let max_cycles = Cycle::MAX;
     let verifier = TransactionScriptsVerifierWithEnv::new();
     let result = verifier.verify_map(script_version, &rtx, |verifier| {
-        let mut init_snap: Option<TransactionSnapshot> = None;
+        let mut init_snap: Option<TransactionState> = None;
 
         if let VerifyResult::Suspended(state) = verifier.resumable_verify(max_cycles).unwrap() {
-            init_snap = Some(state.try_into().unwrap());
+            init_snap = Some(state);
         }
 
         let snap = init_snap.take().unwrap();
@@ -1232,10 +1232,10 @@ fn load_code_with_snapshot_more_times() {
     let verifier = TransactionScriptsVerifierWithEnv::new();
 
     verifier.verify_map(script_version, &rtx, |verifier| {
-        let mut init_snap: Option<TransactionSnapshot> = None;
+        let mut init_snap: Option<TransactionState> = None;
 
         if let VerifyResult::Suspended(state) = verifier.resumable_verify(max_cycles).unwrap() {
-            init_snap = Some(state.try_into().unwrap());
+            init_snap = Some(state);
         }
 
         loop {
@@ -1244,7 +1244,7 @@ fn load_code_with_snapshot_more_times() {
 
             match result.unwrap() {
                 VerifyResult::Suspended(state) => {
-                    init_snap = Some(state.try_into().unwrap());
+                    init_snap = Some(state);
                 }
                 VerifyResult::Completed(cycle) => {
                     cycles = cycle;

script/src/verify/tests/ckb_latest/features_since_v2023.rs

@@ -597,8 +597,7 @@ fn check_spawn_state() {
 
             loop {
                 times += 1;
-                let state: TransactionSnapshot =
-                    init_state.take().unwrap().try_into().expect("no snapshot");
+                let state: TransactionState = init_state.take().unwrap();
                 match verifier.resume_from_snap(&state, max_cycles).unwrap() {
                     VerifyResult::Suspended(state) => {
                         init_state = Some(state);

script/src/verify/tests/utils.rs

@@ -29,7 +29,7 @@ use ckb_types::{
 };
 use faster_hex::hex_encode;
 use std::sync::Arc;
-use std::{convert::TryInto as _, fs::File, path::Path};
+use std::{fs::File, path::Path};
 use tempfile::TempDir;
 
 use crate::verify::*;
@@ -236,7 +236,7 @@ impl TransactionScriptsVerifierWithEnv {
             times += 1;
 
             let mut init_snap = match verifier.resumable_verify(max_cycles).unwrap() {
-                VerifyResult::Suspended(state) => Some(state.try_into().unwrap()),
+                VerifyResult::Suspended(state) => Some(state),
                 VerifyResult::Completed(cycle) => {
                     cycles = cycle;
                     return Ok((cycles, times));
@@ -248,7 +248,7 @@ impl TransactionScriptsVerifierWithEnv {
                 let snap = init_snap.take().unwrap();
                 match verifier.resume_from_snap(&snap, max_cycles) {
                     Ok(VerifyResult::Suspended(state)) => {
-                        init_snap = Some(state.try_into().unwrap());
+                        init_snap = Some(state);
                     }
                     Ok(VerifyResult::Completed(cycle)) => {
                         cycles = cycle;

tx-pool/src/chunk_process.rs

@@ -16,7 +16,7 @@ use ckb_verification::cache::TxVerificationCache;
 use ckb_verification::{
     cache::{CacheEntry, Completed},
     ContextualWithoutScriptTransactionVerifier, DaoScriptSizeVerifier, ScriptError, ScriptVerifier,
-    ScriptVerifyResult, ScriptVerifyState, TimeRelativeTransactionVerifier, TransactionSnapshot,
+    ScriptVerifyResult, ScriptVerifyState, TimeRelativeTransactionVerifier, TransactionState,
     TxVerifyEnv,
 };
 use std::sync::Arc;
@@ -37,7 +37,7 @@ pub(crate) enum ChunkCommand {
 
 enum State {
     Stopped,
-    Suspended(Arc<TransactionSnapshot>),
+    Suspended(Arc<TransactionState>),
     Completed(Cycle),
 }
 
@@ -133,7 +133,7 @@ impl ChunkProcess {
         &mut self,
         rtx: Arc<ResolvedTransaction>,
         data_loader: DL,
-        mut init_snap: Option<Arc<TransactionSnapshot>>,
+        mut init_snap: Option<Arc<TransactionState>>,
         max_cycles: Cycle,
         consensus: Arc<Consensus>,
         tx_env: Arc<TxVerifyEnv>,

verification/src/cache.rs

@@ -1,6 +1,6 @@
 //! TX verification cache
 
-use ckb_script::TransactionSnapshot;
+use ckb_script::TransactionState;
 use ckb_types::{
     core::{Capacity, Cycle, EntryCompleted},
     packed::Byte32,
@@ -25,8 +25,8 @@ pub type CacheEntry = Completed;
 pub struct Suspended {
     /// Cached tx fee
     pub fee: Capacity,
-    /// Snapshot
-    pub snap: Arc<TransactionSnapshot>,
+    /// State
+    pub state: Arc<TransactionState>,
 }
 
 /// Completed entry

verification/src/lib.rs

@@ -26,8 +26,8 @@ pub use crate::transaction_verifier::{
     TimeRelativeTransactionVerifier,
 };
 pub use ckb_script::{
-    ScriptError, ScriptGroupType, TransactionSnapshot, TransactionState as ScriptVerifyState,
-    TxVerifyEnv, VerifyResult as ScriptVerifyResult,
+    ScriptError, ScriptGroupType, TransactionState as ScriptVerifyState, TxVerifyEnv,
+    VerifyResult as ScriptVerifyResult,
 };
 
 /// Maximum amount of time that a block timestamp is allowed to exceed the

verification/src/transaction_verifier.rs

@@ -7,7 +7,7 @@ use ckb_dao_utils::DaoError;
 use ckb_error::Error;
 #[cfg(not(target_family = "wasm"))]
 use ckb_script::ChunkCommand;
-use ckb_script::{TransactionScriptsVerifier, TransactionSnapshot};
+use ckb_script::{TransactionScriptsVerifier, TransactionState};
 use ckb_traits::{
     CellDataProvider, EpochProvider, ExtensionProvider, HeaderFieldsProvider, HeaderProvider,
 };
@@ -201,15 +201,15 @@ where
         &self,
         max_cycles: Cycle,
         skip_script_verify: bool,
-        snapshot: &TransactionSnapshot,
+        state: &TransactionState,
     ) -> Result<Completed, Error> {
         self.compatible.verify()?;
         self.time_relative.verify()?;
         self.capacity.verify()?;
         let cycles = if skip_script_verify {
             0
         } else {
-            self.script.complete(snapshot, max_cycles)?
+            self.script.complete(state, max_cycles)?
         };
         let fee = self.fee_calculator.transaction_fee()?;
         Ok(Completed { cycles, fee })