Fix #4573: readonly call gas computation (#325)

Signed-off-by: Jean-François <jfm@laposte.net>
massalabs · Dec 22, 2023 · bbab9f3 · bbab9f3
1 parent d1dce67
commit bbab9f3
Show file tree

Hide file tree

Showing 7 changed files with 65 additions and 54 deletions.
diff --git a/src/as_execution/common.rs b/src/as_execution/common.rs
@@ -25,30 +25,25 @@ pub(crate) fn call_module(
     let env = get_env(ctx)?;
     let bytecode = env.get_interface().init_call(address, raw_coins)?;
     let interface = env.get_interface();
+    let remaining_gas = get_remaining_gas(&env, ctx)?;
 
-    let remaining_gas = if cfg!(feature = "gas_calibration") {
-        u64::MAX
-    } else {
-        get_remaining_points(&env, ctx)?
-    };
+    let module = interface
+        .get_module(&bytecode, remaining_gas)
+        .map_err(|e| {
+            super::ABIError::Error(anyhow::anyhow!(format!(
+                "call to {}:{} error: {}",
+                address,
+                function,
+                e.to_string()
+            )))
+        })?;
 
-    let (module, post_module_gas) =
-        interface
-            .get_module(&bytecode, remaining_gas)
-            .map_err(|e| {
-                super::ABIError::Error(anyhow::anyhow!(format!(
-                    "call to {}:{} error: {}",
-                    address,
-                    function,
-                    e.to_string()
-                )))
-            })?;
     let resp = crate::execution::run_function(
         &*interface,
         module,
         function,
         param,
-        post_module_gas,
+        remaining_gas,
         env.get_gas_costs(),
     )?;
     if cfg!(not(feature = "gas_calibration")) {
@@ -67,26 +62,23 @@ pub(crate) fn local_call(
     tmp: bool,
 ) -> ABIResult<Response> {
     let env = get_env(ctx)?;
+    let gas_costs = env.get_gas_costs();
     let interface = env.get_interface();
+    let remaining_gas = get_remaining_gas(&env, ctx)?;
 
-    let remaining_gas = if cfg!(feature = "gas_calibration") {
-        u64::MAX
-    } else {
-        get_remaining_points(&env, ctx)?
-    };
-
-    let (module, post_module_gas) = if tmp {
+    let module = if tmp {
         interface.get_tmp_module(bytecode, remaining_gas)?
     } else {
         interface.get_module(bytecode, remaining_gas)?
     };
+
     let resp = crate::execution::run_function(
         &*interface,
         module,
         function,
         param,
-        post_module_gas,
-        env.get_gas_costs(),
+        remaining_gas,
+        gas_costs,
     )?;
     if cfg!(not(feature = "gas_calibration")) {
         set_remaining_points(&env, ctx, resp.remaining_gas)?;
@@ -110,15 +102,23 @@ pub(crate) fn function_exists(
     let env = get_env(ctx)?;
     let interface = env.get_interface();
     let bytecode = interface.raw_get_bytecode_for(address)?;
+    let remaining_gas = get_remaining_gas(&env, ctx)?;
+
+    let function_exists = interface
+        .get_module(&bytecode, remaining_gas)?
+        .function_exists(function);
 
+    Ok(function_exists)
+}
+
+pub(crate) fn get_remaining_gas(
+    env: &ASEnv,
+    ctx: &mut FunctionEnvMut<'_, ASEnv>,
+) -> Result<u64, super::ABIError> {
     let remaining_gas = if cfg!(feature = "gas_calibration") {
         u64::MAX
     } else {
-        get_remaining_points(&env, ctx)?
+        get_remaining_points(env, ctx)?
     };
-
-    Ok(interface
-        .get_module(&bytecode, remaining_gas)?
-        .0
-        .function_exists(function))
+    Ok(remaining_gas)
 }
diff --git a/src/as_execution/context.rs b/src/as_execution/context.rs
@@ -89,7 +89,6 @@ impl ASContext {
             }
             set_remaining_points(&self.env, store, remaining_gas - metering_initial_cost)?;
         }
-
         // Now can exec
         let wasm_func = instance.exports.get_function(function)?;
         let argc = wasm_func.param_arity(store);

diff --git a/src/as_execution/mod.rs b/src/as_execution/mod.rs
@@ -210,6 +210,10 @@ pub(crate) fn exec_as_module(
     };
     let mut store = Store::new(engine);
     let mut context = ASContext::new(interface, as_module.binary_module, gas_costs);
+
+    // save the gas remaining before subexecution: used by readonly execution
+    interface.save_gas_remaining_before_subexecution(limit);
+
     let (instance, init_rem_points) = context.create_vm_instance_and_init_env(&mut store)?;
     let init_cost = as_module.initial_limit.saturating_sub(init_rem_points);
 

diff --git a/src/tests/mod.rs b/src/tests/mod.rs
@@ -110,18 +110,18 @@ impl Interface for TestInterface {
         })
     }
 
-    fn get_module(&self, bytecode: &[u8], gas_limit: u64) -> Result<(RuntimeModule, u64)> {
+    fn get_module(&self, bytecode: &[u8], gas_limit: u64) -> Result<RuntimeModule> {
         println!("Get module");
         let as_module = ASModule::new(bytecode, gas_limit, GasCosts::default(), Compiler::CL)?;
         let module = RuntimeModule::ASModule(as_module);
-        Ok((module, 0))
+        Ok(module)
     }
 
-    fn get_tmp_module(&self, bytecode: &[u8], gas_limit: u64) -> Result<(RuntimeModule, u64)> {
+    fn get_tmp_module(&self, bytecode: &[u8], gas_limit: u64) -> Result<RuntimeModule> {
         println!("Get tmp module");
         let as_module = ASModule::new(bytecode, gas_limit, GasCosts::default(), Compiler::SP)?;
         let module = RuntimeModule::ASModule(as_module);
-        Ok((module, 0))
+        Ok(module)
     }
 
     fn get_owned_addresses(&self) -> Result<Vec<String>> {
@@ -805,6 +805,10 @@ impl Interface for TestInterface {
     fn chain_id(&self) -> Result<u64> {
         Ok(7)
     }
+
+    fn save_gas_remaining_before_subexecution(&self, gas_used_until: u64) {
+        println!("save_gas_remaining_before_subexecution: {}", gas_used_until);
+    }
 }
 
 #[cfg(feature = "gas_calibration")]

diff --git a/src/types.rs b/src/types.rs
@@ -390,12 +390,12 @@ pub trait Interface: Send + Sync + InterfaceClone {
     /// * Compile it if not
     ///
     /// Returns a CL compiled module and the remaining gas after loading
-    fn get_module(&self, bytecode: &[u8], gas_limit: u64) -> Result<(RuntimeModule, u64)>;
+    fn get_module(&self, bytecode: &[u8], gas_limit: u64) -> Result<RuntimeModule>;
 
     /// Compile a temportary module from the given bytecode
     ///
     /// Returns a SP compiled module and the remaining gas after loading
-    fn get_tmp_module(&self, bytecode: &[u8], gas_limit: u64) -> Result<(RuntimeModule, u64)>;
+    fn get_tmp_module(&self, bytecode: &[u8], gas_limit: u64) -> Result<RuntimeModule>;
 
     /// Sends an async message
     ///
@@ -533,6 +533,8 @@ pub trait Interface: Send + Sync + InterfaceClone {
     ) -> Result<ComparisonResult>;
 
     fn compare_pub_key_wasmv1(&self, left: &str, right: &str) -> Result<ComparisonResult>;
+
+    fn save_gas_remaining_before_subexecution(&self, gas_used_until: u64);
 }
 
 impl dyn Interface {

diff --git a/src/wasmv1_execution/abi/abis.rs b/src/wasmv1_execution/abi/abis.rs
@@ -139,13 +139,13 @@ fn abi_call(store_env: FunctionEnvMut<ABIEnv>, arg_offset: i32) -> Result<i32, W
                 })?;
 
             let remaining_gas = handler.get_remaining_gas();
-            let (module, gas) = helper_get_module(handler, bytecode, remaining_gas)?;
+            let module = helper_get_module(handler, bytecode, remaining_gas)?;
             let response = crate::execution::run_function(
                 handler.interface,
                 module,
                 &req.target_function_name,
                 &req.function_arg,
-                gas,
+                remaining_gas,
                 handler.get_gas_costs().clone(),
             )
             .map_err(|err| WasmV1Error::RuntimeError(format!("Could not run function: {}", err)))?;
@@ -169,14 +169,14 @@ fn abi_local_call(store_env: FunctionEnvMut<ABIEnv>, arg_offset: i32) -> Result<
         |handler, req: CallRequest| {
             let bytecode = helper_get_bytecode(handler, req.target_sc_address)?;
             let remaining_gas = handler.get_remaining_gas();
-            let (module, gas) = helper_get_module(handler, bytecode, remaining_gas)?;
+            let module = helper_get_module(handler, bytecode, remaining_gas)?;
 
             let response = crate::execution::run_function(
                 handler.interface,
                 module,
                 &req.target_function_name,
                 &req.function_arg,
-                gas,
+                remaining_gas,
                 handler.get_gas_costs().clone(),
             )
             .map_err(|err| WasmV1Error::RuntimeError(format!("Could not run function: {}", err)))?;
@@ -901,14 +901,14 @@ fn abi_local_execution(
         arg_offset,
         |handler, req: LocalExecutionRequest| {
             let remaining_gas = handler.get_remaining_gas();
-            let (module, gas) = helper_get_tmp_module(handler, req.bytecode, remaining_gas)?;
+            let module = helper_get_tmp_module(handler, req.bytecode, remaining_gas)?;
 
             match crate::execution::run_function(
                 handler.interface,
                 module,
                 &req.target_function_name,
                 &req.function_arg,
-                gas,
+                remaining_gas,
                 handler.get_gas_costs().clone(),
             ) {
                 Ok(response) => {
@@ -963,7 +963,8 @@ fn abi_function_exists(
                 handler.get_remaining_gas()
             };
 
-            let Ok((module, _gas)) = helper_get_module(handler, bytecode, remaining_gas) else {
+            // FIXME set updated value to store_env
+            let Ok(module) = helper_get_module(handler, bytecode, remaining_gas) else {
                 return resp_ok!(FunctionExistsResult, {
                     exists: false
                 });
@@ -995,24 +996,22 @@ fn helper_get_module(
     handler: &mut super::handler::ABIHandler,
     bytecode: Vec<u8>,
     remaining_gas: u64,
-) -> Result<(crate::RuntimeModule, u64), WasmV1Error> {
-    let (module, post_module_gas) = handler
+) -> Result<crate::RuntimeModule, WasmV1Error> {
+    handler
         .interface
         .get_module(&bytecode, remaining_gas)
-        .map_err(|err| WasmV1Error::RuntimeError(format!("Could not get module: {}", err)))?;
-    Ok((module, post_module_gas))
+        .map_err(|err| WasmV1Error::RuntimeError(format!("Could not get module: {}", err)))
 }
 
 fn helper_get_tmp_module(
     handler: &mut super::handler::ABIHandler,
     bytecode: Vec<u8>,
     remaining_gas: u64,
-) -> Result<(crate::RuntimeModule, u64), WasmV1Error> {
-    let (module, post_module_gas) = handler
+) -> Result<crate::RuntimeModule, WasmV1Error> {
+    handler
         .interface
         .get_tmp_module(&bytecode, remaining_gas)
-        .map_err(|err| WasmV1Error::RuntimeError(format!("Could not get module: {}", err)))?;
-    Ok((module, post_module_gas))
+        .map_err(|err| WasmV1Error::RuntimeError(format!("Could not get module: {}", err)))
 }
 
 #[named]

diff --git a/src/wasmv1_execution/mod.rs b/src/wasmv1_execution/mod.rs
@@ -211,6 +211,9 @@ pub(crate) fn exec_wasmv1_module(
     let shared_abi_env: ABIEnv = Arc::new(Mutex::new(None));
     let import_object = register_abis(&mut store, shared_abi_env.clone());
 
+    // save the gas remaining before subexecution: used by readonly execution
+    interface.save_gas_remaining_before_subexecution(gas_limit);
+
     // Create an instance of the execution environment.
     let execution_env =
         ExecutionEnv::create_instance(&mut store, &module, interface, gas_costs, &import_object)