From cc89eeb55ce75ef6b6118f52bd4f7b203daeaf00 Mon Sep 17 00:00:00 2001 From: Simon Warta Date: Wed, 6 Jan 2021 12:31:38 +0100 Subject: [PATCH 1/7] Inline account_for_externally_used_gas_impl --- packages/vm/src/environment.rs | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/packages/vm/src/environment.rs b/packages/vm/src/environment.rs index 50bfd8f081..c32cb4f86c 100644 --- a/packages/vm/src/environment.rs +++ b/packages/vm/src/environment.rs @@ -19,7 +19,6 @@ pub struct GasState { /// Gas limit for the computation. pub gas_limit: u64, /// Tracking the gas used in the cosmos SDK, in cosmwasm units. - #[allow(unused)] pub externally_used_gas: u64, } @@ -31,7 +30,6 @@ impl GasState { } } - #[allow(unused)] fn increase_externally_used_gas(&mut self, amount: u64) { self.externally_used_gas += amount; } @@ -43,7 +41,6 @@ impl GasState { /// Get the amount of gas units still left for the rest of the calculation. /// /// We need the amount of gas used in wasmer since it is not tracked inside this object. - #[allow(unused)] fn get_gas_left(&self, wasmer_used_gas: u64) -> u64 { self.gas_limit .saturating_sub(self.externally_used_gas) @@ -53,7 +50,6 @@ impl GasState { /// Get the amount of gas units used so far inside wasmer. /// /// We need the amount of gas left in wasmer since it is not tracked inside this object. - #[allow(unused)] pub(crate) fn get_gas_used_in_wasmer(&self, wasmer_gas_left: u64) -> u64 { self.gas_limit .saturating_sub(self.externally_used_gas) @@ -335,13 +331,6 @@ pub fn process_gas_info( fn account_for_externally_used_gas( env: &Environment, amount: u64, -) -> VmResult<()> { - account_for_externally_used_gas_impl(env, amount) -} - -fn account_for_externally_used_gas_impl( - env: &Environment, - used_gas: u64, ) -> VmResult<()> { env.with_context_data_mut(|context_data| { let gas_state = &mut context_data.gas_state; @@ -359,7 +348,7 @@ fn account_for_externally_used_gas_impl( }; let wasmer_used_gas = gas_state.get_gas_used_in_wasmer(gas_left); - gas_state.increase_externally_used_gas(used_gas); + gas_state.increase_externally_used_gas(amount); // These lines reduce the amount of gas available to wasmer // so it can not consume gas that was consumed externally. let new_limit = gas_state.get_gas_left(wasmer_used_gas); From 28ddc95225ae982cb6e14e6eed61d6649277ce86 Mon Sep 17 00:00:00 2001 From: Simon Warta Date: Wed, 6 Jan 2021 13:55:54 +0100 Subject: [PATCH 2/7] Remove set_gas_limit --- packages/vm/src/environment.rs | 60 +++++++++++++--------------------- packages/vm/src/imports.rs | 1 - packages/vm/src/instance.rs | 1 - 3 files changed, 22 insertions(+), 40 deletions(-) diff --git a/packages/vm/src/environment.rs b/packages/vm/src/environment.rs index c32cb4f86c..61699f6cde 100644 --- a/packages/vm/src/environment.rs +++ b/packages/vm/src/environment.rs @@ -16,9 +16,10 @@ pub struct InsufficientGasLeft; #[derive(Clone, PartialEq, Debug, Default)] pub struct GasState { - /// Gas limit for the computation. + /// Gas limit for the computation, including internally and externally used gas. + /// This is set when the Environment is created and never mutated. pub gas_limit: u64, - /// Tracking the gas used in the cosmos SDK, in cosmwasm units. + /// Tracking the gas used in the Cosmos SDK, in CosmWasm gas units. pub externally_used_gas: u64, } @@ -34,10 +35,6 @@ impl GasState { self.externally_used_gas += amount; } - pub(crate) fn set_gas_limit(&mut self, gas_limit: u64) { - self.gas_limit = gas_limit; - } - /// Get the amount of gas units still left for the rest of the calculation. /// /// We need the amount of gas used in wasmer since it is not tracked inside this object. @@ -112,13 +109,6 @@ impl Environment { callback(context_data) } - pub fn with_gas_state_mut(&self, callback: C) -> R - where - C: FnOnce(&mut GasState) -> R, - { - self.with_context_data_mut(|context_data| callback(&mut context_data.gas_state)) - } - pub fn with_gas_state(&self, callback: C) -> R where C: FnOnce(&GasState) -> R, @@ -404,8 +394,7 @@ mod tests { const DEFAULT_QUERY_GAS_LIMIT: u64 = 300_000; const TESTING_MEMORY_LIMIT: Size = Size::mebi(16); - fn make_instance() -> (Environment, Box) { - let gas_limit = TESTING_GAS_LIMIT; + fn make_instance(gas_limit: u64) -> (Environment, Box) { let env = Environment::new(MockApi::default(), gas_limit, false); let module = compile_and_use(&CONTRACT, TESTING_MEMORY_LIMIT).unwrap(); @@ -429,7 +418,6 @@ mod tests { let instance_ptr = NonNull::from(instance.as_ref()); env.set_wasmer_instance(Some(instance_ptr)); env.set_gas_left(gas_limit); - env.with_gas_state_mut(|gas_state| gas_state.set_gas_limit(gas_limit)); (env, instance) } @@ -448,7 +436,7 @@ mod tests { #[test] fn move_out_works() { - let (env, _instance) = make_instance(); + let (env, _instance) = make_instance(TESTING_GAS_LIMIT); // empty data on start let (inits, initq) = env.move_out(); @@ -473,11 +461,9 @@ mod tests { #[test] fn gas_tracking_works_correctly() { - let (env, _instance) = make_instance(); - let gas_limit = 100; - env.set_gas_left(gas_limit); - env.with_gas_state_mut(|state| state.set_gas_limit(gas_limit)); + let (env, _instance) = make_instance(gas_limit); + assert_eq!(env.get_gas_left(), 100); // Consume all the Gas that we allocated @@ -499,11 +485,9 @@ mod tests { #[test] fn gas_tracking_works_correctly_with_gas_consumption_in_wasmer() { - let (env, _instance) = make_instance(); - let gas_limit = 100; - env.set_gas_left(gas_limit); - env.with_gas_state_mut(|state| state.set_gas_limit(gas_limit)); + let (env, _instance) = make_instance(gas_limit); + assert_eq!(env.get_gas_left(), 100); // Some gas was consumed externally @@ -530,7 +514,7 @@ mod tests { #[test] fn is_storage_readonly_defaults_to_true() { - let (env, _instance) = make_instance(); + let (env, _instance) = make_instance(TESTING_GAS_LIMIT); leave_default_data(&env); assert_eq!(env.is_storage_readonly(), true); @@ -538,7 +522,7 @@ mod tests { #[test] fn set_storage_readonly_can_change_flag() { - let (env, _instance) = make_instance(); + let (env, _instance) = make_instance(TESTING_GAS_LIMIT); leave_default_data(&env); // change @@ -556,7 +540,7 @@ mod tests { #[test] fn call_function_works() { - let (env, _instance) = make_instance(); + let (env, _instance) = make_instance(TESTING_GAS_LIMIT); leave_default_data(&env); let result = env.call_function("allocate", &[10u32.into()]).unwrap(); @@ -566,7 +550,7 @@ mod tests { #[test] fn call_function_fails_for_missing_instance() { - let (env, _instance) = make_instance(); + let (env, _instance) = make_instance(TESTING_GAS_LIMIT); leave_default_data(&env); // Clear context's wasmer_instance @@ -581,7 +565,7 @@ mod tests { #[test] fn call_function_fails_for_missing_function() { - let (env, _instance) = make_instance(); + let (env, _instance) = make_instance(TESTING_GAS_LIMIT); leave_default_data(&env); let res = env.call_function("doesnt_exist", &[]); @@ -595,7 +579,7 @@ mod tests { #[test] fn call_function0_works() { - let (env, _instance) = make_instance(); + let (env, _instance) = make_instance(TESTING_GAS_LIMIT); leave_default_data(&env); env.call_function0("cosmwasm_vm_version_4", &[]).unwrap(); @@ -603,7 +587,7 @@ mod tests { #[test] fn call_function0_errors_for_wrong_result_count() { - let (env, _instance) = make_instance(); + let (env, _instance) = make_instance(TESTING_GAS_LIMIT); leave_default_data(&env); let result = env.call_function0("allocate", &[10u32.into()]); @@ -623,7 +607,7 @@ mod tests { #[test] fn call_function1_works() { - let (env, _instance) = make_instance(); + let (env, _instance) = make_instance(TESTING_GAS_LIMIT); leave_default_data(&env); let result = env.call_function1("allocate", &[10u32.into()]).unwrap(); @@ -633,7 +617,7 @@ mod tests { #[test] fn call_function1_errors_for_wrong_result_count() { - let (env, _instance) = make_instance(); + let (env, _instance) = make_instance(TESTING_GAS_LIMIT); leave_default_data(&env); let result = env.call_function1("allocate", &[10u32.into()]).unwrap(); @@ -657,7 +641,7 @@ mod tests { #[test] fn with_storage_from_context_set_get() { - let (env, _instance) = make_instance(); + let (env, _instance) = make_instance(TESTING_GAS_LIMIT); leave_default_data(&env); let val = env @@ -690,7 +674,7 @@ mod tests { #[test] #[should_panic(expected = "A panic occurred in the callback.")] fn with_storage_from_context_handles_panics() { - let (env, _instance) = make_instance(); + let (env, _instance) = make_instance(TESTING_GAS_LIMIT); leave_default_data(&env); env.with_storage_from_context::<_, ()>(|_store| { @@ -701,7 +685,7 @@ mod tests { #[test] fn with_querier_from_context_works() { - let (env, _instance) = make_instance(); + let (env, _instance) = make_instance(TESTING_GAS_LIMIT); leave_default_data(&env); let res = env @@ -724,7 +708,7 @@ mod tests { #[test] #[should_panic(expected = "A panic occurred in the callback.")] fn with_querier_from_context_handles_panics() { - let (env, _instance) = make_instance(); + let (env, _instance) = make_instance(TESTING_GAS_LIMIT); leave_default_data(&env); env.with_querier_from_context::<_, ()>(|_querier| { diff --git a/packages/vm/src/imports.rs b/packages/vm/src/imports.rs index 586432c787..7cf0dc505f 100644 --- a/packages/vm/src/imports.rs +++ b/packages/vm/src/imports.rs @@ -369,7 +369,6 @@ mod tests { let instance_ptr = NonNull::from(instance.as_ref()); env.set_wasmer_instance(Some(instance_ptr)); env.set_gas_left(gas_limit); - env.with_gas_state_mut(|gas_state| gas_state.set_gas_limit(gas_limit)); env.set_storage_readonly(false); (env, instance) diff --git a/packages/vm/src/instance.rs b/packages/vm/src/instance.rs index 331b5e6aa7..6b233ff66d 100644 --- a/packages/vm/src/instance.rs +++ b/packages/vm/src/instance.rs @@ -170,7 +170,6 @@ where let instance_ptr = NonNull::from(wasmer_instance.as_ref()); env.set_wasmer_instance(Some(instance_ptr)); env.set_gas_left(gas_limit); - env.with_gas_state_mut(|gas_state| gas_state.set_gas_limit(gas_limit)); env.move_in(backend.storage, backend.querier); let instance = Instance { _inner: wasmer_instance, From 9a39d0da592bbcf576020891abe6285cd4eff7b7 Mon Sep 17 00:00:00 2001 From: Simon Warta Date: Wed, 6 Jan 2021 14:33:47 +0100 Subject: [PATCH 3/7] Add with_wasmer_instance helper --- packages/vm/src/environment.rs | 90 ++++++++++++++---------------- packages/vm/src/errors/vm_error.rs | 7 --- 2 files changed, 43 insertions(+), 54 deletions(-) diff --git a/packages/vm/src/environment.rs b/packages/vm/src/environment.rs index 61699f6cde..4c0d1a9a17 100644 --- a/packages/vm/src/environment.rs +++ b/packages/vm/src/environment.rs @@ -9,8 +9,10 @@ use wasmer_middlewares::metering::{get_remaining_points, set_remaining_points, M use crate::backend::{Api, GasInfo, Querier, Storage}; use crate::errors::{VmError, VmResult}; +/// Never can never be instantiated. +/// Replace this with the [never primitive type](https://doc.rust-lang.org/std/primitive.never.html) when stable. #[derive(Debug)] -pub struct InsufficientGasLeft; +pub enum Never {} /** context data **/ @@ -91,7 +93,7 @@ impl Environment { } } - pub fn with_context_data_mut(&self, callback: C) -> R + fn with_context_data_mut(&self, callback: C) -> R where C: FnOnce(&mut ContextData) -> R, { @@ -100,7 +102,7 @@ impl Environment { callback(context_data) } - pub fn with_context_data(&self, callback: C) -> R + fn with_context_data(&self, callback: C) -> R where C: FnOnce(&ContextData) -> R, { @@ -116,33 +118,39 @@ impl Environment { self.with_context_data(|context_data| callback(&context_data.gas_state)) } + pub fn with_wasmer_instance(&self, callback: C) -> VmResult + where + C: FnOnce(&WasmerInstance) -> VmResult, + { + self.with_context_data(|context_data| match context_data.wasmer_instance { + Some(instance_ptr) => { + let instance_ref = unsafe { instance_ptr.as_ref() }; + callback(instance_ref) + } + None => Err(VmError::uninitialized_context_data("wasmer_instance")), + }) + } + /// Calls a function with the given name and arguments. /// The number of return values is variable and controlled by the guest. /// Usually we expect 0 or 1 return values. Use [`Self::call_function0`] /// or [`Self::call_function1`] to ensure the number of return values is checked. fn call_function(&self, name: &str, args: &[Val]) -> VmResult> { // Clone function before calling it to avoid dead locks - let func = self.with_context_data(|context_data| match context_data.wasmer_instance { - Some(instance_ptr) => { - let func = unsafe { instance_ptr.as_ref() } - .exports - .get_function(name)?; - Ok(func.clone()) - } - None => Err(VmError::uninitialized_context_data("wasmer_instance")), + let func = self.with_wasmer_instance(|instance| { + let func = instance.exports.get_function(name)?; + Ok(func.clone()) })?; func.call(args).map_err(|runtime_err| -> VmError { - self.with_context_data(|context_data| match context_data.wasmer_instance { - Some(instance_ptr) => { - let instance_ref = unsafe { instance_ptr.as_ref() }; - match get_remaining_points(instance_ref) { - MeteringPoints::Remaining(_) => VmError::from(runtime_err), - MeteringPoints::Exhausted => VmError::gas_depletion(), - } - } - None => VmError::uninitialized_context_data("wasmer_instance"), + self.with_wasmer_instance::<_, Never>(|instance| { + let err: VmError = match get_remaining_points(instance) { + MeteringPoints::Remaining(_) => VmError::from(runtime_err), + MeteringPoints::Exhausted => VmError::gas_depletion(), + }; + Err(err) }) + .unwrap_err() // with_wasmer_instance can only succeed if the callback succeeds }) } @@ -205,45 +213,35 @@ impl Environment { } pub fn get_gas_left(&self) -> u64 { - self.with_context_data_mut(|context_data| { - let instance_ptr = context_data - .wasmer_instance - .expect("Wasmer instance is not set. This is a bug."); - let instance = unsafe { instance_ptr.as_ref() }; - match get_remaining_points(instance) { + self.with_wasmer_instance(|instance| { + Ok(match get_remaining_points(instance) { MeteringPoints::Remaining(count) => count, MeteringPoints::Exhausted => 0, - } + }) }) + .expect("Wasmer instance is not set. This is a bug in the lifecycle.") } pub fn set_gas_left(&self, new_value: u64) { - self.with_context_data_mut(|context_data| { - let instance_ptr = context_data - .wasmer_instance - .expect("Wasmer instance is not set. This is a bug."); - let instance = unsafe { instance_ptr.as_ref() }; + self.with_wasmer_instance(|instance| { set_remaining_points(instance, new_value); + Ok(()) }) + .expect("Wasmer instance is not set. This is a bug in the lifecycle.") } /// Decreases gas left by the given amount. /// If the amount exceeds the available gas, the remaining gas is set to 0 and - /// an InsufficientGasLeft error is returned. - pub fn decrease_gas_left(&self, amount: u64) -> Result<(), InsufficientGasLeft> { - self.with_context_data_mut(|context_data| { - let instance_ptr = context_data - .wasmer_instance - .expect("Wasmer instance is not set. This is a bug."); - let instance = unsafe { instance_ptr.as_ref() }; - + /// an VmError::GasDepletion error is returned. + pub fn decrease_gas_left(&self, amount: u64) -> VmResult<()> { + self.with_wasmer_instance(|instance| { let remaining = match get_remaining_points(instance) { MeteringPoints::Remaining(count) => count, MeteringPoints::Exhausted => 0, }; if amount > remaining { set_remaining_points(instance, 0); - Err(InsufficientGasLeft) + Err(VmError::gas_depletion()) } else { set_remaining_points(instance, remaining - amount); Ok(()) @@ -252,19 +250,17 @@ impl Environment { } pub fn memory(&self) -> Memory { - self.with_context_data(|context_data| { - let instance_ptr = context_data - .wasmer_instance - .expect("Wasmer instance is not set. This is a bug."); - let instance = unsafe { instance_ptr.as_ref() }; + self.with_wasmer_instance(|instance| { let mut memories: Vec = instance .exports .iter() .memories() .map(|pair| pair.1.clone()) .collect(); - memories.pop().unwrap() + let memory = memories.pop().unwrap(); + Ok(memory) }) + .expect("Wasmer instance is not set. This is a bug in the lifecycle.") } /// Moves owned instances of storage and querier into the env. diff --git a/packages/vm/src/errors/vm_error.rs b/packages/vm/src/errors/vm_error.rs index e7a17c9856..a071f740c5 100644 --- a/packages/vm/src/errors/vm_error.rs +++ b/packages/vm/src/errors/vm_error.rs @@ -5,7 +5,6 @@ use thiserror::Error; use super::communication_error::CommunicationError; use crate::backend::BackendError; -use crate::environment::InsufficientGasLeft; #[derive(Error, Debug)] #[non_exhaustive] @@ -305,12 +304,6 @@ impl From for VmError { } } -impl From for VmError { - fn from(_original: InsufficientGasLeft) -> Self { - VmError::gas_depletion() - } -} - impl From for VmError { fn from(_original: std::convert::Infallible) -> Self { unreachable!(); From a2e61bfe8f295417ca5def80744e040a7f87f848 Mon Sep 17 00:00:00 2001 From: Simon Warta Date: Wed, 6 Jan 2021 15:25:36 +0100 Subject: [PATCH 4/7] Remove GasState helpers --- packages/vm/src/environment.rs | 75 +++++++++------------------------- packages/vm/src/instance.rs | 5 ++- 2 files changed, 24 insertions(+), 56 deletions(-) diff --git a/packages/vm/src/environment.rs b/packages/vm/src/environment.rs index 4c0d1a9a17..5b3ea8b0fb 100644 --- a/packages/vm/src/environment.rs +++ b/packages/vm/src/environment.rs @@ -32,28 +32,6 @@ impl GasState { externally_used_gas: 0, } } - - fn increase_externally_used_gas(&mut self, amount: u64) { - self.externally_used_gas += amount; - } - - /// Get the amount of gas units still left for the rest of the calculation. - /// - /// We need the amount of gas used in wasmer since it is not tracked inside this object. - fn get_gas_left(&self, wasmer_used_gas: u64) -> u64 { - self.gas_limit - .saturating_sub(self.externally_used_gas) - .saturating_sub(wasmer_used_gas) - } - - /// Get the amount of gas units used so far inside wasmer. - /// - /// We need the amount of gas left in wasmer since it is not tracked inside this object. - pub(crate) fn get_gas_used_in_wasmer(&self, wasmer_gas_left: u64) -> u64 { - self.gas_limit - .saturating_sub(self.externally_used_gas) - .saturating_sub(wasmer_gas_left) - } } /// A environment that provides access to the ContextData. @@ -118,6 +96,13 @@ impl Environment { self.with_context_data(|context_data| callback(&context_data.gas_state)) } + pub fn with_gas_state_mut(&self, callback: C) -> R + where + C: FnOnce(&mut GasState) -> R, + { + self.with_context_data_mut(|context_data| callback(&mut context_data.gas_state)) + } + pub fn with_wasmer_instance(&self, callback: C) -> VmResult where C: FnOnce(&WasmerInstance) -> VmResult, @@ -318,43 +303,23 @@ fn account_for_externally_used_gas( env: &Environment, amount: u64, ) -> VmResult<()> { - env.with_context_data_mut(|context_data| { - let gas_state = &mut context_data.gas_state; - - // get_gas_left implementation without a deadlock - let gas_left = { - let instance_ptr = context_data - .wasmer_instance - .expect("Wasmer instance is not set. This is a bug."); - let instance = unsafe { instance_ptr.as_ref() }; - match get_remaining_points(instance) { - MeteringPoints::Remaining(count) => count, - MeteringPoints::Exhausted => 0, - } - }; - let wasmer_used_gas = gas_state.get_gas_used_in_wasmer(gas_left); + let gas_left = env.get_gas_left(); - gas_state.increase_externally_used_gas(amount); + let new_limit = env.with_gas_state_mut(|gas_state| { + gas_state.externally_used_gas += amount; // These lines reduce the amount of gas available to wasmer // so it can not consume gas that was consumed externally. - let new_limit = gas_state.get_gas_left(wasmer_used_gas); - - // This tells wasmer how much more gas it can consume from this point in time. - // set_gas_left implementation without a deadlock - { - let instance_ptr = context_data - .wasmer_instance - .expect("Wasmer instance is not set. This is a bug."); - let instance = unsafe { instance_ptr.as_ref() }; - set_remaining_points(instance, new_limit); - } + gas_left.saturating_sub(amount) + }); - if gas_state.externally_used_gas + wasmer_used_gas > gas_state.gas_limit { - Err(VmError::gas_depletion()) - } else { - Ok(()) - } - }) + // This tells wasmer how much more gas it can consume from this point in time. + env.set_gas_left(new_limit); + + if amount > gas_left { + Err(VmError::gas_depletion()) + } else { + Ok(()) + } } #[cfg(test)] diff --git a/packages/vm/src/instance.rs b/packages/vm/src/instance.rs index 6b233ff66d..f469e76aa6 100644 --- a/packages/vm/src/instance.rs +++ b/packages/vm/src/instance.rs @@ -221,7 +221,10 @@ where limit: state.gas_limit, remaining: gas_left, used_externally: state.externally_used_gas, - used_internally: state.get_gas_used_in_wasmer(gas_left), + used_internally: state + .gas_limit + .saturating_sub(state.externally_used_gas) + .saturating_sub(gas_left), } } From 3ef38ab0379891d8aa6096ebc0c7331f4ded257a Mon Sep 17 00:00:00 2001 From: Simon Warta Date: Wed, 6 Jan 2021 16:22:46 +0100 Subject: [PATCH 5/7] Test process_gas_info --- packages/vm/src/backend.rs | 7 ++ packages/vm/src/environment.rs | 149 +++++++++++++++++++++++++++++++++ packages/vm/src/instance.rs | 3 + 3 files changed, 159 insertions(+) diff --git a/packages/vm/src/backend.rs b/packages/vm/src/backend.rs index 8e74cc76e0..cadf234c8a 100644 --- a/packages/vm/src/backend.rs +++ b/packages/vm/src/backend.rs @@ -17,6 +17,13 @@ pub struct GasInfo { } impl GasInfo { + pub fn new(cost: u64, externally_used: u64) -> Self { + GasInfo { + cost, + externally_used, + } + } + pub fn with_cost(amount: u64) -> Self { GasInfo { cost: amount, diff --git a/packages/vm/src/environment.rs b/packages/vm/src/environment.rs index 5b3ea8b0fb..516df2bcdd 100644 --- a/packages/vm/src/environment.rs +++ b/packages/vm/src/environment.rs @@ -420,6 +420,155 @@ mod tests { assert!(endq.is_none()); } + #[test] + fn process_gas_info_works_for_cost() { + let (env, _instance) = make_instance(100); + assert_eq!(env.get_gas_left(), 100); + + // Consume all the Gas that we allocated + process_gas_info::(&env, GasInfo::with_cost(70)).unwrap(); + assert_eq!(env.get_gas_left(), 30); + process_gas_info::(&env, GasInfo::with_cost(4)).unwrap(); + assert_eq!(env.get_gas_left(), 26); + process_gas_info::(&env, GasInfo::with_cost(6)).unwrap(); + assert_eq!(env.get_gas_left(), 20); + process_gas_info::(&env, GasInfo::with_cost(20)).unwrap(); + assert_eq!(env.get_gas_left(), 0); + + // Using one more unit of gas triggers a failure + match process_gas_info::(&env, GasInfo::with_cost(1)).unwrap_err() { + VmError::GasDepletion { .. } => {} + err => panic!("unexpected error: {:?}", err), + } + } + + #[test] + fn process_gas_info_works_for_externally_used() { + let (env, _instance) = make_instance(100); + assert_eq!(env.get_gas_left(), 100); + + // Consume all the Gas that we allocated + process_gas_info::(&env, GasInfo::with_externally_used(70)).unwrap(); + assert_eq!(env.get_gas_left(), 30); + process_gas_info::(&env, GasInfo::with_externally_used(4)).unwrap(); + assert_eq!(env.get_gas_left(), 26); + process_gas_info::(&env, GasInfo::with_externally_used(6)).unwrap(); + assert_eq!(env.get_gas_left(), 20); + process_gas_info::(&env, GasInfo::with_externally_used(20)).unwrap(); + assert_eq!(env.get_gas_left(), 0); + + // Using one more unit of gas triggers a failure + match process_gas_info::(&env, GasInfo::with_externally_used(1)).unwrap_err() { + VmError::GasDepletion { .. } => {} + err => panic!("unexpected error: {:?}", err), + } + } + + #[test] + fn process_gas_info_works_for_cost_and_externally_used() { + let (env, _instance) = make_instance(100); + assert_eq!(env.get_gas_left(), 100); + let gas_state = env.with_gas_state(|gas_state| gas_state.clone()); + assert_eq!(gas_state.gas_limit, 100); + assert_eq!(gas_state.externally_used_gas, 0); + + process_gas_info::(&env, GasInfo::new(17, 4)).unwrap(); + assert_eq!(env.get_gas_left(), 79); + let gas_state = env.with_gas_state(|gas_state| gas_state.clone()); + assert_eq!(gas_state.gas_limit, 100); + assert_eq!(gas_state.externally_used_gas, 4); + + process_gas_info::(&env, GasInfo::new(9, 0)).unwrap(); + assert_eq!(env.get_gas_left(), 70); + let gas_state = env.with_gas_state(|gas_state| gas_state.clone()); + assert_eq!(gas_state.gas_limit, 100); + assert_eq!(gas_state.externally_used_gas, 4); + + process_gas_info::(&env, GasInfo::new(0, 70)).unwrap(); + assert_eq!(env.get_gas_left(), 0); + let gas_state = env.with_gas_state(|gas_state| gas_state.clone()); + assert_eq!(gas_state.gas_limit, 100); + assert_eq!(gas_state.externally_used_gas, 74); + + // More cost fail but do not change stats + match process_gas_info::(&env, GasInfo::new(1, 0)).unwrap_err() { + VmError::GasDepletion { .. } => {} + err => panic!("unexpected error: {:?}", err), + } + assert_eq!(env.get_gas_left(), 0); + let gas_state = env.with_gas_state(|gas_state| gas_state.clone()); + assert_eq!(gas_state.gas_limit, 100); + assert_eq!(gas_state.externally_used_gas, 74); + + // More externally used fails and changes stats + match process_gas_info::(&env, GasInfo::new(0, 1)).unwrap_err() { + VmError::GasDepletion { .. } => {} + err => panic!("unexpected error: {:?}", err), + } + assert_eq!(env.get_gas_left(), 0); + let gas_state = env.with_gas_state(|gas_state| gas_state.clone()); + assert_eq!(gas_state.gas_limit, 100); + assert_eq!(gas_state.externally_used_gas, 75); + } + + #[test] + fn process_gas_info_zeros_gas_left_when_exceeded() { + // with_externally_used + { + let (env, _instance) = make_instance(100); + let result = process_gas_info::(&env, GasInfo::with_externally_used(120)); + match result.unwrap_err() { + VmError::GasDepletion { .. } => {} + err => panic!("unexpected error: {:?}", err), + } + assert_eq!(env.get_gas_left(), 0); + let gas_state = env.with_gas_state(|gas_state| gas_state.clone()); + assert_eq!(gas_state.gas_limit, 100); + assert_eq!(gas_state.externally_used_gas, 120); + } + + // with_cost + { + let (env, _instance) = make_instance(100); + let result = process_gas_info::(&env, GasInfo::with_cost(120)); + match result.unwrap_err() { + VmError::GasDepletion { .. } => {} + err => panic!("unexpected error: {:?}", err), + } + assert_eq!(env.get_gas_left(), 0); + let gas_state = env.with_gas_state(|gas_state| gas_state.clone()); + assert_eq!(gas_state.gas_limit, 100); + assert_eq!(gas_state.externally_used_gas, 0); + } + } + + #[test] + fn process_gas_info_works_correctly_with_gas_consumption_in_wasmer() { + let (env, _instance) = make_instance(100); + assert_eq!(env.get_gas_left(), 100); + + // Some gas was consumed externally + process_gas_info::(&env, GasInfo::with_externally_used(50)).unwrap(); + assert_eq!(env.get_gas_left(), 50); + process_gas_info::(&env, GasInfo::with_externally_used(4)).unwrap(); + assert_eq!(env.get_gas_left(), 46); + + // Consume 20 gas directly in wasmer + env.decrease_gas_left(20).unwrap(); + assert_eq!(env.get_gas_left(), 26); + + process_gas_info::(&env, GasInfo::with_externally_used(6)).unwrap(); + assert_eq!(env.get_gas_left(), 20); + process_gas_info::(&env, GasInfo::with_externally_used(20)).unwrap(); + assert_eq!(env.get_gas_left(), 0); + + // Using one more unit of gas triggers a failure + match process_gas_info::(&env, GasInfo::with_externally_used(1)).unwrap_err() { + VmError::GasDepletion { .. } => {} + err => panic!("unexpected error: {:?}", err), + } + } + #[test] fn gas_tracking_works_correctly() { let gas_limit = 100; diff --git a/packages/vm/src/instance.rs b/packages/vm/src/instance.rs index f469e76aa6..299de8c171 100644 --- a/packages/vm/src/instance.rs +++ b/packages/vm/src/instance.rs @@ -221,6 +221,9 @@ where limit: state.gas_limit, remaining: gas_left, used_externally: state.externally_used_gas, + // If externally_used_gas exceeds the gas limit, this will return 0. + // no matter how much gas was used internally. But then we error with out of gas + // anyways, and it does not matter much anymore where gas was spend. used_internally: state .gas_limit .saturating_sub(state.externally_used_gas) From d64870b88ac4d401518b4cafa7f8bcf52c505a64 Mon Sep 17 00:00:00 2001 From: Simon Warta Date: Wed, 6 Jan 2021 16:43:39 +0100 Subject: [PATCH 6/7] Inline account_for_externally_used_gas --- packages/vm/src/environment.rs | 74 +++------------------------------- 1 file changed, 6 insertions(+), 68 deletions(-) diff --git a/packages/vm/src/environment.rs b/packages/vm/src/environment.rs index 516df2bcdd..6143288a8b 100644 --- a/packages/vm/src/environment.rs +++ b/packages/vm/src/environment.rs @@ -218,6 +218,7 @@ impl Environment { /// Decreases gas left by the given amount. /// If the amount exceeds the available gas, the remaining gas is set to 0 and /// an VmError::GasDepletion error is returned. + #[allow(unused)] // used in tests pub fn decrease_gas_left(&self, amount: u64) -> VmResult<()> { self.with_wasmer_instance(|instance| { let remaining = match get_remaining_points(instance) { @@ -290,32 +291,22 @@ impl ContextData { pub fn process_gas_info( env: &Environment, info: GasInfo, -) -> VmResult<()> { - env.decrease_gas_left(info.cost)?; - account_for_externally_used_gas(env, info.externally_used)?; - Ok(()) -} - -/// Use this function to adjust the VM's gas limit when a call into the backend -/// reported there was externally metered gas used. -/// This does not increase the VM's gas usage but ensures the overall limit is not exceeded. -fn account_for_externally_used_gas( - env: &Environment, - amount: u64, ) -> VmResult<()> { let gas_left = env.get_gas_left(); let new_limit = env.with_gas_state_mut(|gas_state| { - gas_state.externally_used_gas += amount; + gas_state.externally_used_gas += info.externally_used; // These lines reduce the amount of gas available to wasmer // so it can not consume gas that was consumed externally. - gas_left.saturating_sub(amount) + gas_left + .saturating_sub(info.externally_used) + .saturating_sub(info.cost) }); // This tells wasmer how much more gas it can consume from this point in time. env.set_gas_left(new_limit); - if amount > gas_left { + if info.externally_used + info.cost > gas_left { Err(VmError::gas_depletion()) } else { Ok(()) @@ -569,59 +560,6 @@ mod tests { } } - #[test] - fn gas_tracking_works_correctly() { - let gas_limit = 100; - let (env, _instance) = make_instance(gas_limit); - - assert_eq!(env.get_gas_left(), 100); - - // Consume all the Gas that we allocated - account_for_externally_used_gas::(&env, 70).unwrap(); - assert_eq!(env.get_gas_left(), 30); - account_for_externally_used_gas::(&env, 4).unwrap(); - assert_eq!(env.get_gas_left(), 26); - account_for_externally_used_gas::(&env, 6).unwrap(); - assert_eq!(env.get_gas_left(), 20); - account_for_externally_used_gas::(&env, 20).unwrap(); - assert_eq!(env.get_gas_left(), 0); - - // Using one more unit of gas triggers a failure - match account_for_externally_used_gas::(&env, 1).unwrap_err() { - VmError::GasDepletion { .. } => {} - err => panic!("unexpected error: {:?}", err), - } - } - - #[test] - fn gas_tracking_works_correctly_with_gas_consumption_in_wasmer() { - let gas_limit = 100; - let (env, _instance) = make_instance(gas_limit); - - assert_eq!(env.get_gas_left(), 100); - - // Some gas was consumed externally - account_for_externally_used_gas::(&env, 50).unwrap(); - assert_eq!(env.get_gas_left(), 50); - account_for_externally_used_gas::(&env, 4).unwrap(); - assert_eq!(env.get_gas_left(), 46); - - // Consume 20 gas directly in wasmer - env.decrease_gas_left(20).unwrap(); - assert_eq!(env.get_gas_left(), 26); - - account_for_externally_used_gas::(&env, 6).unwrap(); - assert_eq!(env.get_gas_left(), 20); - account_for_externally_used_gas::(&env, 20).unwrap(); - assert_eq!(env.get_gas_left(), 0); - - // Using one more unit of gas triggers a failure - match account_for_externally_used_gas::(&env, 1).unwrap_err() { - VmError::GasDepletion { .. } => {} - err => panic!("unexpected error: {:?}", err), - } - } - #[test] fn is_storage_readonly_defaults_to_true() { let (env, _instance) = make_instance(TESTING_GAS_LIMIT); From cf8ebe0a18fd10a16bb2d00820a90b2f85613287 Mon Sep 17 00:00:00 2001 From: Simon Warta Date: Wed, 6 Jan 2021 16:49:24 +0100 Subject: [PATCH 7/7] Simplify Instance::get_gas_left --- packages/vm/src/instance.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/vm/src/instance.rs b/packages/vm/src/instance.rs index 299de8c171..539ab7a922 100644 --- a/packages/vm/src/instance.rs +++ b/packages/vm/src/instance.rs @@ -208,7 +208,7 @@ where /// Returns the currently remaining gas. pub fn get_gas_left(&self) -> u64 { - self.create_gas_report().remaining + self.env.get_gas_left() } /// Creates and returns a gas report.