Skip to content

Commit

Permalink
refactor: Remove TransactionSnapshot structure
Browse files Browse the repository at this point in the history
As of now, TransactionSnapshot and TransactionState from ckb-script
package are exactly the same. There is no need to keep them both.
  • Loading branch information
xxuejie committed Jan 15, 2025
1 parent 9f48908 commit 16a0b42
Show file tree
Hide file tree
Showing 10 changed files with 34 additions and 94 deletions.
2 changes: 1 addition & 1 deletion script/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
61 changes: 1 addition & 60 deletions script/src/types.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use ckb_error::Error;
use ckb_types::{
core::{Cycle, ScriptHashType},
packed::{Byte32, Script},
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)]
Expand All @@ -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")
Expand Down
10 changes: 5 additions & 5 deletions script/src/verify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use crate::{
type_id::TypeIdSystemScript,
types::{
CoreMachine, DebugPrinter, Indices, ScriptGroup, ScriptGroupType, ScriptVersion,
TransactionSnapshot, TransactionState, VerifyResult,
TransactionState, VerifyResult,
},
verify_env::TxVerifyEnv,
};
Expand Down Expand Up @@ -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.
Expand All @@ -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;
Expand Down Expand Up @@ -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(|| {
Expand Down
24 changes: 12 additions & 12 deletions script/src/verify/tests/ckb_latest/features_since_v2021.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}

Expand All @@ -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);
}
Expand All @@ -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);
}
Expand Down Expand Up @@ -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!()
}
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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 {
Expand All @@ -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;
Expand Down
3 changes: 1 addition & 2 deletions script/src/verify/tests/ckb_latest/features_since_v2023.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
6 changes: 3 additions & 3 deletions script/src/verify/tests/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;
Expand Down Expand Up @@ -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));
Expand All @@ -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;
Expand Down
6 changes: 3 additions & 3 deletions tx-pool/src/chunk_process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -37,7 +37,7 @@ pub(crate) enum ChunkCommand {

enum State {
Stopped,
Suspended(Arc<TransactionSnapshot>),
Suspended(Arc<TransactionState>),
Completed(Cycle),
}

Expand Down Expand Up @@ -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>,
Expand Down
6 changes: 3 additions & 3 deletions verification/src/cache.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! TX verification cache
use ckb_script::TransactionSnapshot;
use ckb_script::TransactionState;
use ckb_types::{
core::{Capacity, Cycle, EntryCompleted},
packed::Byte32,
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions verification/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions verification/src/transaction_verifier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down Expand Up @@ -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 })
Expand Down

0 comments on commit 16a0b42

Please sign in to comment.