Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: Remove TransactionSnapshot structure #4782

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
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
Loading