From 82a8c5525084b56fff373b416dc952a16472de08 Mon Sep 17 00:00:00 2001 From: pgherveou Date: Mon, 3 Feb 2025 23:30:39 +0100 Subject: [PATCH] fix tests --- .../fixtures/contracts/caller_contract.rs | 25 +++-- substrate/frame/revive/src/exec.rs | 95 ++++++++----------- substrate/frame/revive/src/tests.rs | 5 +- substrate/frame/revive/src/wasm/mod.rs | 5 + 4 files changed, 64 insertions(+), 66 deletions(-) diff --git a/substrate/frame/revive/fixtures/contracts/caller_contract.rs b/substrate/frame/revive/fixtures/contracts/caller_contract.rs index b6a9bf2895fa6..236aec2e863bd 100644 --- a/substrate/frame/revive/fixtures/contracts/caller_contract.rs +++ b/substrate/frame/revive/fixtures/contracts/caller_contract.rs @@ -31,7 +31,7 @@ pub extern "C" fn deploy() {} #[no_mangle] #[polkavm_derive::polkavm_export] pub extern "C" fn call() { - input!(code_hash: &[u8; 32],); + input!(code_hash: &[u8; 32], load_code_ref_time: u64,); // The value to transfer on instantiation and calls. Chosen to be greater than existential // deposit. @@ -49,8 +49,9 @@ pub extern "C" fn call() { // Fail to deploy the contract since it returns a non-zero exit status. let res = api::instantiate( - u64::MAX, // How much ref_time weight to devote for the execution. u64::MAX = use all. - u64::MAX, // How much proof_size weight to devote for the execution. u64::MAX = use all. + u64::MAX, /* How much ref_time weight to devote for the execution. u64::MAX = use + * all. */ + u64::MAX, // How much proof_size weight to devote for the execution. u64::MAX = use all. &[u8::MAX; 32], // No deposit limit. &value, &reverted_input_deploy, @@ -62,8 +63,9 @@ pub extern "C" fn call() { // Fail to deploy the contract due to insufficient ref_time weight. let res = api::instantiate( - 1u64, // too little ref_time weight - u64::MAX, // How much proof_size weight to devote for the execution. u64::MAX = use all. + 1u64, // too little ref_time weight + u64::MAX, /* How much proof_size weight to devote for the execution. u64::MAX = + * use all. */ &[u8::MAX; 32], // No deposit limit. &value, &input_deploy, @@ -75,7 +77,8 @@ pub extern "C" fn call() { // Fail to deploy the contract due to insufficient proof_size weight. let res = api::instantiate( - u64::MAX, // How much ref_time weight to devote for the execution. u64::MAX = use all. + u64::MAX, /* How much ref_time weight to devote for the execution. u64::MAX = use + * all. */ 1u64, // Too little proof_size weight &[u8::MAX; 32], // No deposit limit. &value, @@ -90,8 +93,9 @@ pub extern "C" fn call() { let mut callee = [0u8; 20]; api::instantiate( - u64::MAX, // How much ref_time weight to devote for the execution. u64::MAX = use all. - u64::MAX, // How much proof_size weight to devote for the execution. u64::MAX = use all. + u64::MAX, /* How much ref_time weight to devote for the execution. u64::MAX = use + * all. */ + u64::MAX, // How much proof_size weight to devote for the execution. u64::MAX = use all. &[u8::MAX; 32], // No deposit limit. &value, &input_deploy, @@ -118,8 +122,9 @@ pub extern "C" fn call() { let res = api::call( uapi::CallFlags::empty(), &callee, - 1u64, // Too little ref_time weight. - u64::MAX, // How much proof_size weight to devote for the execution. u64::MAX = use all. + load_code_ref_time, // Too little ref_time weight. + u64::MAX, /* How much proof_size weight to devote for the execution. u64::MAX + * = use all. */ &[u8::MAX; 32], // No deposit limit. &value, &INPUT, diff --git a/substrate/frame/revive/src/exec.rs b/substrate/frame/revive/src/exec.rs index 3d4b55fcc9edc..edfc63634f6ab 100644 --- a/substrate/frame/revive/src/exec.rs +++ b/substrate/frame/revive/src/exec.rs @@ -886,34 +886,16 @@ where } }; - let (executable, delegate_caller, nested_gas) = if let Some(DelegatedCall { + let mut nested_gas = gas_meter.nested(gas_limit); + let (executable, delegate_caller) = if let Some(DelegatedCall { executable, caller, callee, }) = delegated_call { - ( - executable, - Some(DelegateInfo { caller, callee }), - gas_meter.nested(gas_limit), - ) - - // For the first frame, charge the gas to the frame's nested gas meter - // instead of the top-level gas meter, so it's properly associated with - // the initial frame when tracing the call. - } else if origin_is_caller { - let mut nested_gas_meter = gas_meter.nested(gas_limit); - ( - E::from_storage(contract.code_hash, &mut nested_gas_meter)?, - None, - nested_gas_meter, - ) + (executable, Some(DelegateInfo { caller, callee })) } else { - ( - E::from_storage(contract.code_hash, gas_meter)?, - None, - gas_meter.nested(gas_limit), - ) + (E::from_storage(contract.code_hash, &mut nested_gas)?, None) }; ( @@ -958,6 +940,8 @@ where ) }, }; + + let nested_storage = storage_meter.nested(deposit_limit); let frame = Frame { delegate, value_transferred, @@ -965,7 +949,7 @@ where account_id, entry_point, nested_gas, - nested_storage: storage_meter.nested(deposit_limit), + nested_storage, allows_reentry: true, read_only, last_frame_output: Default::default(), @@ -1025,9 +1009,9 @@ where /// /// This can be either a call or an instantiate. fn run(&mut self, executable: E, input_data: Vec) -> Result<(), ExecError> { - let caller = self.caller(); let frame = self.top_frame(); let entry_point = frame.entry_point; + let is_delegate_call = frame.delegate.is_some(); let delegated_code_hash = if frame.delegate.is_some() { Some(*executable.code_hash()) } else { None }; @@ -1045,24 +1029,12 @@ where self.transient_storage.start_transaction(); - if_tracing(|tracer| { - let frame = top_frame_mut!(self); - let contract_addr = T::AddressMapper::to_address(&frame.account_id); - let caller_addr = caller.account_id().map(T::AddressMapper::to_address); - tracer.enter_child_span( - caller_addr.unwrap_or_default(), - contract_addr, - frame.delegate.is_some(), - frame.read_only, - frame.value_transferred, - &input_data, - frame.nested_gas.gas_left(), - ); - }); - let do_transaction = || -> ExecResult { + let caller = self.caller(); let frame = top_frame_mut!(self); - let account_id = &frame.account_id; + let read_only = frame.read_only; + let value_transferred = frame.value_transferred; + let account_id = &frame.account_id.clone(); // We need to make sure that the contract's account exists before calling its // constructor. @@ -1105,11 +1077,35 @@ where )?; } + let contract_address = T::AddressMapper::to_address(account_id); + let maybe_caller_address = caller.account_id().map(T::AddressMapper::to_address); let code_deposit = executable.code_info().deposit(); - let output = executable - .execute(self, entry_point, input_data) - .map_err(|e| ExecError { error: e.error, origin: ErrorOrigin::Callee })?; + if_tracing(|tracer| { + tracer.enter_child_span( + maybe_caller_address.unwrap_or_default(), + contract_address, + is_delegate_call, + read_only, + value_transferred, + &input_data, + frame.nested_gas.gas_left(), + ); + }); + + let output = executable.execute(self, entry_point, input_data).map_err(|e| { + if_tracing(|tracer| { + tracer.exit_child_span_with_error( + e.error, + top_frame_mut!(self).nested_gas.gas_consumed(), + ); + }); + ExecError { error: e.error, origin: ErrorOrigin::Callee } + })?; + + if_tracing(|tracer| { + tracer.exit_child_span(&output, top_frame_mut!(self).nested_gas.gas_consumed()); + }); // Avoid useless work that would be reverted anyways. if output.did_revert() { @@ -1169,18 +1165,6 @@ where self.transient_storage.rollback_transaction(); } - if_tracing(|tracer| match &output { - Ok(result) => { - tracer.exit_child_span(&result, top_frame!(self).nested_gas.gas_consumed()); - }, - Err(e) => { - tracer.exit_child_span_with_error( - e.error.into(), - top_frame_mut!(self).nested_gas.gas_consumed(), - ); - }, - }); - self.pop_frame(success); output.map(|output| { self.top_frame_mut().last_frame_output = output; @@ -1454,6 +1438,7 @@ where // We need to make sure to reset `allows_reentry` even on failure. let result = try_call(); + println!("result: {:?}", result); // Protection is on a per call basis. self.top_frame_mut().allows_reentry = true; diff --git a/substrate/frame/revive/src/tests.rs b/substrate/frame/revive/src/tests.rs index 9e48fff52370c..3564e8676767f 100644 --- a/substrate/frame/revive/src/tests.rs +++ b/substrate/frame/revive/src/tests.rs @@ -995,6 +995,7 @@ fn transient_storage_limit_in_call() { fn deploy_and_call_other_contract() { let (caller_wasm, _caller_code_hash) = compile_module("caller_contract").unwrap(); let (callee_wasm, callee_code_hash) = compile_module("return_with_data").unwrap(); + let code_load_weight = crate::wasm::code_load_weight(callee_wasm.len() as u32); ExtBuilder::default().existential_deposit(1).build().execute_with(|| { let min_balance = Contracts::min_balance(); @@ -1022,7 +1023,9 @@ fn deploy_and_call_other_contract() { // Call BOB contract, which attempts to instantiate and call the callee contract and // makes various assertions on the results from those calls. - assert_ok!(builder::call(caller_addr).data(callee_code_hash.as_ref().to_vec()).build()); + assert_ok!(builder::call(caller_addr) + .data((callee_code_hash, code_load_weight.ref_time()).encode()) + .build()); assert_eq!( System::events(), diff --git a/substrate/frame/revive/src/wasm/mod.rs b/substrate/frame/revive/src/wasm/mod.rs index dc49fae26fdaa..30418cf84b24c 100644 --- a/substrate/frame/revive/src/wasm/mod.rs +++ b/substrate/frame/revive/src/wasm/mod.rs @@ -115,6 +115,11 @@ impl Token for CodeLoadToken { } } +#[cfg(test)] +pub fn code_load_weight(code_len: u32) -> Weight { + Token::::weight(&CodeLoadToken(code_len)) +} + impl WasmBlob where BalanceOf: Into + TryFrom,