From 642988975ad2dd973297462781fc3ac7501024f9 Mon Sep 17 00:00:00 2001 From: mohanson Date: Mon, 20 Jan 2025 15:28:39 +0800 Subject: [PATCH] Follow xuejie's comments --- script/src/scheduler.rs | 47 +++++++++++++++++++--------------- script/src/syscalls/exec_v2.rs | 10 +++++--- script/src/syscalls/spawn.rs | 10 +++++--- script/src/types.rs | 11 +++++--- 4 files changed, 45 insertions(+), 33 deletions(-) diff --git a/script/src/scheduler.rs b/script/src/scheduler.rs index 924f5ace83..8fdd3c757b 100644 --- a/script/src/scheduler.rs +++ b/script/src/scheduler.rs @@ -8,8 +8,8 @@ use crate::verify::TransactionScriptsSyscallsGenerator; use crate::ScriptVersion; use crate::types::{ - CoreMachineType, DataPieceId, Fd, FdArgs, FullSuspendedState, Machine, Message, ReadState, - RunMode, TxData, VmId, VmState, WriteState, FIRST_FD_SLOT, FIRST_VM_ID, + CoreMachineType, DataLocation, DataPieceId, Fd, FdArgs, FullSuspendedState, Machine, Message, + ReadState, RunMode, TxData, VmId, VmState, WriteState, FIRST_FD_SLOT, FIRST_VM_ID, }; use ckb_traits::{CellDataProvider, ExtensionProvider, HeaderProvider}; use ckb_types::core::Cycle; @@ -207,7 +207,14 @@ where if self.states.is_empty() { // Booting phase, we will need to initialize the first VM. assert_eq!( - self.boot_vm(&DataPieceId::Program, 0, u64::MAX, None)?, + self.boot_vm( + &DataLocation { + data_piece_id: DataPieceId::Program, + offset: 0, + length: u64::MAX, + }, + None + )?, ROOT_VM_ID ); } @@ -355,11 +362,11 @@ where self.load_vm_program( &context, &mut new_machine, - &args.data_piece_id, - args.offset, - args.length, + &args.location, Some((vm_id, args.argc, args.argv)), )?; + // The insert operation removes the old vm instance and adds the new vm instance. + debug_assert!(self.instantiated.contains_key(&vm_id)); self.instantiated.insert(vm_id, (context, new_machine)); } Message::Spawn(vm_id, args) => { @@ -374,12 +381,8 @@ where machine.machine.set_register(A0, MAX_VMS_SPAWNED as u64); continue; } - let spawned_vm_id = self.boot_vm( - &args.data_piece_id, - args.offset, - args.length, - Some((vm_id, args.argc, args.argv)), - )?; + let spawned_vm_id = + self.boot_vm(&args.location, Some((vm_id, args.argc, args.argv)))?; // Move passed fds from spawner to spawnee for fd in &args.fds { self.fds.insert(*fd, spawned_vm_id); @@ -772,15 +775,13 @@ where /// Boot a vm by given program and args. pub fn boot_vm( &mut self, - data_piece_id: &DataPieceId, - offset: u64, - length: u64, + location: &DataLocation, args: Option<(u64, u64, u64)>, ) -> Result { let id = self.next_vm_id; self.next_vm_id += 1; let (context, mut machine) = self.create_dummy_vm(&id)?; - self.load_vm_program(&context, &mut machine, data_piece_id, offset, length, args)?; + self.load_vm_program(&context, &mut machine, location, args)?; // Newly booted VM will be instantiated by default while self.instantiated.len() >= MAX_INSTANTIATED_VMS { // Instantiated is a BTreeMap, first_entry will maintain key order @@ -802,13 +803,12 @@ where &mut self, context: &MachineContext
, machine: &mut Machine, - data_piece_id: &DataPieceId, - offset: u64, - length: u64, + location: &DataLocation, args: Option<(u64, u64, u64)>, ) -> Result { let mut sc = context.snapshot2_context().lock().expect("lock"); - let (program, _) = sc.load_data(data_piece_id, offset, length)?; + let (program, _) = + sc.load_data(&location.data_piece_id, location.offset, location.length)?; let metadata = parse_elf::(&program, machine.machine.version())?; let bytes = match args { Some((vm_id, argc, argv)) => { @@ -818,7 +818,12 @@ where } None => machine.load_program_with_metadata(&program, &metadata, vec![].into_iter())?, }; - sc.mark_program(&mut machine.machine, &metadata, data_piece_id, offset)?; + sc.mark_program( + &mut machine.machine, + &metadata, + &location.data_piece_id, + location.offset, + )?; machine .machine .add_cycles_no_checking(transferred_byte_cycles(bytes))?; diff --git a/script/src/syscalls/exec_v2.rs b/script/src/syscalls/exec_v2.rs index f7e66a9475..03f63fff44 100644 --- a/script/src/syscalls/exec_v2.rs +++ b/script/src/syscalls/exec_v2.rs @@ -2,7 +2,7 @@ use crate::syscalls::{ Place, Source, EXEC, INDEX_OUT_OF_BOUND, SLICE_OUT_OF_BOUND, SOURCE_ENTRY_MASK, SOURCE_GROUP_FLAG, }; -use crate::types::{DataPieceId, ExecV2Args, Message, TxData, VmId}; +use crate::types::{DataLocation, DataPieceId, ExecV2Args, Message, TxData, VmId}; use ckb_traits::{CellDataProvider, ExtensionProvider, HeaderProvider}; use ckb_vm::{ registers::{A0, A1, A2, A3, A4, A5, A7}, @@ -110,9 +110,11 @@ where .push(Message::ExecV2( self.id, ExecV2Args { - data_piece_id, - offset, - length, + location: DataLocation { + data_piece_id, + offset, + length, + }, argc, argv, }, diff --git a/script/src/syscalls/spawn.rs b/script/src/syscalls/spawn.rs index e042415699..f4c7e82b87 100644 --- a/script/src/syscalls/spawn.rs +++ b/script/src/syscalls/spawn.rs @@ -2,7 +2,7 @@ use crate::syscalls::{ Source, INDEX_OUT_OF_BOUND, SLICE_OUT_OF_BOUND, SOURCE_ENTRY_MASK, SOURCE_GROUP_FLAG, SPAWN, SPAWN_EXTRA_CYCLES_BASE, SPAWN_YIELD_CYCLES_BASE, }; -use crate::types::{DataPieceId, Fd, Message, SpawnArgs, TxData, VmId}; +use crate::types::{DataLocation, DataPieceId, Fd, Message, SpawnArgs, TxData, VmId}; use ckb_traits::{CellDataProvider, ExtensionProvider, HeaderProvider}; use ckb_vm::{ machine::SupportMachine, @@ -142,9 +142,11 @@ where .push(Message::Spawn( self.id, SpawnArgs { - data_piece_id, - offset, - length, + location: DataLocation { + data_piece_id, + offset, + length, + }, argc: argc.to_u64(), argv: argv.to_u64(), fds, diff --git a/script/src/types.rs b/script/src/types.rs index 6a03270ad9..8f25a98799 100644 --- a/script/src/types.rs +++ b/script/src/types.rs @@ -351,19 +351,22 @@ pub enum VmState { } #[derive(Clone, Debug)] -pub struct ExecV2Args { +pub struct DataLocation { pub data_piece_id: DataPieceId, pub offset: u64, pub length: u64, +} + +#[derive(Clone, Debug)] +pub struct ExecV2Args { + pub location: DataLocation, pub argc: u64, pub argv: u64, } #[derive(Clone, Debug)] pub struct SpawnArgs { - pub data_piece_id: DataPieceId, - pub offset: u64, - pub length: u64, + pub location: DataLocation, pub argc: u64, pub argv: u64, pub fds: Vec,