Fix smart-contracts gas payments (#4505)

* Update packages * Pay VM creation and compilations correctly * Use max_instance_cost for default compilation * Update module costs * Minor metrics * Tmp module metrics and high max_instance value for testing * Calibrated costs for compilation and instance + compilation metrics cleanup * Change costs and use runtime branch * Add SC gas doc & improve CL compilation payments * Update gas costs & other minor changes * Clippy fixes * Cargo.toml & cfg fixes * Remove context payments and update ABI costs * Update documentation * Impl get_tmp_module * Update doc * Review updates * Doc * Loader documentation * Fmt * Remove compilation timers * Add gas to base operation (tx, roll buy & sell) Signed-off-by: Jean-François <[email protected]> * Fix majority of tests * Fix remaining tests * Update unit test SC src tag * Move SP compilation payment to OP and ABI costs * Update gas documentation * Update RuntimeModule usage, fix from_op, add sp compil to pool config, fix tests * Update gas table doc * Fix tests * Add an important little bit of doc * saturating_add on sp cost --------- Signed-off-by: Jean-François <[email protected]> Co-authored-by: Jean-François <[email protected]>
massalabs · Nov 7, 2023 · 85e95c6 · 85e95c6
1 parent d62b064
commit 85e95c6
Show file tree

Hide file tree

Showing 21 changed files with 436 additions and 335 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -101,7 +101,7 @@ massa_wallet = { path = "./massa-wallet" }
 
 # Massa projects dependencies
 massa-proto-rs = { git = "https://github.com/massalabs/massa-proto-rs", "rev" = "a2e335b1493d302aa03b87e5fd611a2f2c21afdb" }
-massa-sc-runtime = { git = "https://github.com/massalabs/massa-sc-runtime", "rev" = "04b70f8e2e39363a2cd0018e837b5643e32d09a0" }
+massa-sc-runtime = { git = "https://github.com/massalabs/massa-sc-runtime", "branch" = "fix-sc-gas" }
 peernet = { git = "https://github.com/massalabs/PeerNet", "branch" = "main" }
 
 # Common dependencies

diff --git a/massa-execution-exports/src/test_exports/config.rs b/massa-execution-exports/src/test_exports/config.rs
@@ -40,7 +40,7 @@ impl Default for ExecutionConfig {
             max_bytecode_size: MAX_BYTECODE_LENGTH,
             max_datastore_value_size: MAX_DATASTORE_VALUE_LENGTH,
             storage_costs_constants,
-            max_read_only_gas: 100_000_000,
+            max_read_only_gas: 1_000_000_000,
             gas_costs: GasCosts::new(
                 concat!(
                     env!("CARGO_MANIFEST_DIR"),

diff --git a/massa-execution-worker/src/execution.rs b/massa-execution-worker/src/execution.rs
@@ -129,7 +129,6 @@ impl ExecutionState {
         let module_cache = Arc::new(RwLock::new(ModuleCache::new(ModuleCacheConfig {
             hd_cache_path: config.hd_cache_path.clone(),
             gas_costs: config.gas_costs.clone(),
-            compilation_gas: config.max_gas_per_block,
             lru_cache_size: config.lru_cache_size,
             hd_cache_size: config.hd_cache_size,
             snip_amount: config.snip_amount,
@@ -379,7 +378,10 @@ impl ExecutionState {
         }
 
         // check remaining block gas
-        let op_gas = operation.get_gas_usage(self.config.base_operation_gas_cost);
+        let op_gas = operation.get_gas_usage(
+            self.config.base_operation_gas_cost,
+            self.config.gas_costs.sp_compilation_cost,
+        );
         let new_remaining_block_gas = remaining_block_gas.checked_sub(op_gas).ok_or_else(|| {
             ExecutionError::NotEnoughGas(
                 "not enough remaining block gas to execute operation".to_string(),
@@ -802,16 +804,10 @@ impl ExecutionState {
         };
 
         // load the tmp module
-        let module = self
+        let (module, remaining_gas) = self
             .module_cache
             .read()
             .load_tmp_module(bytecode, *max_gas)?;
-        // sub tmp module compilation cost
-        let remaining_gas = max_gas
-            .checked_sub(self.config.gas_costs.sp_compilation_cost)
-            .ok_or(ExecutionError::RuntimeError(
-                "not enough gas to pay for singlepass compilation".to_string(),
-            ))?;
         // run the VM
         massa_sc_runtime::run_main(
             &*self.execution_interface,
@@ -905,13 +901,13 @@ impl ExecutionState {
 
         // load and execute the compiled module
         // IMPORTANT: do not keep a lock here as `run_function` uses the `get_module` interface
-        let module = self.module_cache.write().load_module(&bytecode, max_gas)?;
+        let (module, remaining_gas) = self.module_cache.write().load_module(&bytecode, max_gas)?;
         let response = massa_sc_runtime::run_function(
             &*self.execution_interface,
             module,
             target_func,
             param,
-            max_gas,
+            remaining_gas,
             self.config.gas_costs.clone(),
         );
         match response {
@@ -1003,7 +999,7 @@ impl ExecutionState {
 
         // load and execute the compiled module
         // IMPORTANT: do not keep a lock here as `run_function` uses the `get_module` interface
-        let module = self
+        let (module, remaining_gas) = self
             .module_cache
             .write()
             .load_module(&bytecode, message.max_gas)?;
@@ -1012,7 +1008,7 @@ impl ExecutionState {
             module,
             &message.function,
             &message.function_params,
-            message.max_gas,
+            remaining_gas,
             self.config.gas_costs.clone(),
         );
         match response {
@@ -1427,16 +1423,23 @@ impl ExecutionState {
                     }
                 }
 
+                // pay SP compilation as read only execution has no base cost
+                let post_compil = req
+                    .max_gas
+                    .checked_sub(self.config.gas_costs.sp_compilation_cost)
+                    .ok_or(ExecutionError::NotEnoughGas(
+                        "Not enough gas to pay SP compilation in ReadOnly execution".to_string(),
+                    ))?;
                 // load the tmp module
-                let module = self
+                let (module, remaining_gas) = self
                     .module_cache
                     .read()
-                    .load_tmp_module(&bytecode, req.max_gas)?;
+                    .load_tmp_module(&bytecode, post_compil)?;
                 // run the VM
                 massa_sc_runtime::run_main(
                     &*self.execution_interface,
                     module,
-                    req.max_gas,
+                    remaining_gas,
                     self.config.gas_costs.clone(),
                 )
                 .map_err(|error| ExecutionError::VMError {
@@ -1476,7 +1479,7 @@ impl ExecutionState {
 
                 // load and execute the compiled module
                 // IMPORTANT: do not keep a lock here as `run_function` uses the `get_module` interface
-                let module = self
+                let (module, remaining_gas) = self
                     .module_cache
                     .write()
                     .load_module(&bytecode, req.max_gas)?;
@@ -1485,7 +1488,7 @@ impl ExecutionState {
                     module,
                     &target_func,
                     &parameter,
-                    req.max_gas,
+                    remaining_gas,
                     self.config.gas_costs.clone(),
                 );
                 match response {

diff --git a/massa-execution-worker/src/interface_impl.rs b/massa-execution-worker/src/interface_impl.rs
@@ -110,7 +110,6 @@ impl InterfaceImpl {
         let module_cache = Arc::new(RwLock::new(ModuleCache::new(ModuleCacheConfig {
             hd_cache_path: config.hd_cache_path.clone(),
             gas_costs: config.gas_costs.clone(),
-            compilation_gas: config.max_gas_per_block,
             lru_cache_size: config.lru_cache_size,
             hd_cache_size: config.hd_cache_size,
             snip_amount: config.snip_amount,
@@ -288,11 +287,27 @@ impl Interface for InterfaceImpl {
     /// Get the module from cache if possible, compile it if not
     ///
     /// # Returns
-    /// A `massa-sc-runtime` compiled module
-    fn get_module(&self, bytecode: &[u8], limit: u64) -> Result<RuntimeModule> {
+    /// A `massa-sc-runtime` CL compiled module & the remaining gas after loading the module
+    fn get_module(&self, bytecode: &[u8], gas_limit: u64) -> Result<(RuntimeModule, u64)> {
         let context = context_guard!(self);
-        let module = context.module_cache.write().load_module(bytecode, limit)?;
-        Ok(module)
+        let (module, remaining_gas) = context
+            .module_cache
+            .write()
+            .load_module(bytecode, gas_limit)?;
+        Ok((module, remaining_gas))
+    }
+
+    /// Compile and return a temporary module
+    ///
+    /// # Returns
+    /// A `massa-sc-runtime` SP compiled module & the remaining gas after loading the module
+    fn get_tmp_module(&self, bytecode: &[u8], gas_limit: u64) -> Result<(RuntimeModule, u64)> {
+        let context = context_guard!(self);
+        let (module, remaining_gas) = context
+            .module_cache
+            .write()
+            .load_tmp_module(bytecode, gas_limit)?;
+        Ok((module, remaining_gas))
     }
 
     /// Gets the balance of the current address address (top of the stack).

diff --git a/massa-execution-worker/src/tests/scenarios_mandatories.rs b/massa-execution-worker/src/tests/scenarios_mandatories.rs
@@ -108,7 +108,7 @@ fn test_readonly_execution() {
     let mut res = universe
         .module_controller
         .execute_readonly_request(ReadOnlyExecutionRequest {
-            max_gas: 1_000_000,
+            max_gas: 414_000_000, // 314_000_000 (SP COMPIL) + 100_000_000 (FOR EXECUTION)
             call_stack: vec![],
             target: ReadOnlyExecutionTarget::BytecodeExecution(
                 include_bytes!("./wasm/event_test.wasm").to_vec(),
@@ -409,7 +409,6 @@ fn send_and_receive_async_message() {
     let exec_cfg = ExecutionConfig {
         t0: MassaTime::from_millis(100),
         cursor_delay: MassaTime::from_millis(0),
-        max_async_gas: 100_000,
         ..ExecutionConfig::default()
     };
     let block_producer = KeyPair::generate(0).unwrap();
@@ -784,7 +783,6 @@ fn send_and_receive_async_message_with_trigger() {
     let exec_cfg = ExecutionConfig {
         t0: MassaTime::from_millis(100),
         cursor_delay: MassaTime::from_millis(0),
-        max_async_gas: 1_000_000_000,
         ..ExecutionConfig::default()
     };
     let block_producer = KeyPair::generate(0).unwrap();
@@ -1629,7 +1627,6 @@ fn sc_execution_error() {
     let exec_cfg = ExecutionConfig {
         t0: MassaTime::from_millis(100),
         cursor_delay: MassaTime::from_millis(0),
-        max_async_gas: 100_000,
         ..ExecutionConfig::default()
     };
     let block_producer = KeyPair::generate(0).unwrap();
@@ -1699,7 +1696,6 @@ fn sc_datastore() {
     let exec_cfg = ExecutionConfig {
         t0: MassaTime::from_millis(100),
         cursor_delay: MassaTime::from_millis(0),
-        max_async_gas: 100_000,
         ..ExecutionConfig::default()
     };
     let block_producer = KeyPair::generate(0).unwrap();
@@ -1765,7 +1761,6 @@ fn set_bytecode_error() {
     let exec_cfg = ExecutionConfig {
         t0: MassaTime::from_millis(100),
         cursor_delay: MassaTime::from_millis(0),
-        max_async_gas: 100_000,
         ..ExecutionConfig::default()
     };
     let block_producer = KeyPair::generate(0).unwrap();
@@ -2125,7 +2120,7 @@ fn events_from_switching_blockclique() {
 }
 
 #[test]
-fn not_enough_compilation_gas() {
+fn not_enough_instance_gas() {
     // setup the period duration
     let exec_cfg = ExecutionConfig {
         t0: MassaTime::from_millis(100),
@@ -2198,7 +2193,7 @@ fn not_enough_compilation_gas() {
         .get_filtered_sc_output_event(EventFilter::default());
     assert!(events[0]
         .data
-        .contains("not enough gas to pay for singlepass compilation"));
+        .contains("Provided max gas is below the default instance creation cost"));
 }
 
 #[test]

diff --git a/massa-execution-worker/src/tests/universe.rs b/massa-execution-worker/src/tests/universe.rs
@@ -110,7 +110,10 @@ impl ExecutionTestUniverse {
     ) -> Result<SecureShareOperation, ExecutionError> {
         let op = OperationType::ExecuteSC {
             data: data.to_vec(),
-            max_gas: 100_000_000,
+            // MAX_GAS MUST BE AT LEAST 314_000_000 (SP COMPIL)
+            // here we use 1.5B as most of the tests perform a SC creation:
+            // 314_000_000 (SP COMPIL) + 745_000_000 (CL COMPIL) + margin
+            max_gas: 1_500_000_000,
             max_coins: Amount::from_str("5000000").unwrap(),
             datastore,
         };

diff --git a/massa-execution-worker/src/tests/wasm/send_message.wasm b/massa-execution-worker/src/tests/wasm/send_message.wasm
diff --git a/massa-models/src/operation.rs b/massa-models/src/operation.rs
@@ -973,9 +973,9 @@ impl SecureShareOperation {
     ///
     /// base_operation_gas_cost comes from the configuration and
     /// is the cost of a basic operation (BASE_OPERATION_GAS_COST)
-    pub fn get_gas_usage(&self, base_operation_gas_cost: u64) -> u64 {
+    pub fn get_gas_usage(&self, base_operation_gas_cost: u64, sp_compilation_cost: u64) -> u64 {
         match &self.content.op {
-            OperationType::ExecuteSC { max_gas, .. } => *max_gas,
+            OperationType::ExecuteSC { max_gas, .. } => max_gas.saturating_add(sp_compilation_cost),
             OperationType::CallSC { max_gas, .. } => *max_gas,
             OperationType::RollBuy { .. } => 0,
             OperationType::RollSell { .. } => 0,

diff --git a/massa-module-cache/gas-table.md b/massa-module-cache/gas-table.md
@@ -0,0 +1,61 @@
+# Overview
+
+This document keeps track of where and how gas costs are handled for SC executions.
+
+It contains:
+* the gas table
+* the list of functions that effectively pay the costs
+
+# Gas table
+
+| TYPE                     | PAYMENT          |
+| ------------------------ | ---------------- |
+| **CallSC (OP)**          |                  |
+| compilation              | NOT PAID         |
+| VM instantiation         | PAID IN MAX_GAS  |
+| module instantiation     | PAID IN MAX_GAS  |
+| execution                | PAID IN MAX_GAS  |
+| **ExecuteSC (OP)**       |                  |
+| compilation              | PAID IN OP GAS   |
+| VM instantiation         | PAID IN MAX_GAS  |
+| module instantiation     | PAID IN MAX_GAS  |
+| execution                | PAID IN MAX_GAS  |
+| **Call (ABI)**           |                  |
+| compilation              | NOT PAID         |
+| VM instantiation         | PAID IN CALL GAS |
+| module instantiation     | PAID IN CALL GAS |
+| execution                | PAID IN CALL GAS |
+| base gas cost of the ABI | PAID IN ABI GAS  |
+| **LocalExecution (ABI)** |                  |
+| compilation              | PAID IN ABI GAS  |
+| VM instantiation         | PAID IN CALL GAS |
+| module instantiation     | PAID IN CALL GAS |
+| execution                | PAID IN CALL GAS |
+| base gas cost of the ABI | PAID IN ABI GAS  |
+| **CreateSC (ABI)**       |                  |
+| compilation              | PAID IN ABI GAS  |
+| **SetBytecode (ABI)**    |                  |
+| compilation              | PAID IN ABI GAS  |
+
+# Functions
+
+### Singlepass compilation
+
+1. Paid for ExecuteSC operations as OP cost in `massa-execution-worker` > `execution.rs` > `execute_operation` by `get_gas_usage`
+2. Paid for ReadOnly requests in `massa-execution-worker` > `execution.rs` > `execute_readonly_request`
+3. Paid in `massa-sc-runtime` ABI cost by `assembly_script_local_execution`. This ABI gives rise to Singlepass compilations and must have according costs to pay for it.
+
+### Cranelift compilation
+
+Paid in `massa-sc-runtime` ABI costs by `assembly_script_create_sc`, `assembly_script_set_bytecode` & `assembly_script_set_bytecode_for`. These ABIs give rise to Cranelift compilations and must have according costs to pay for it.
+
+### VM & Module instantiation
+
+1. Threshold checked in `massa-module-cache` > `controller.rs` > `load_module` & `load_tmp_module`
+2. `load_module` is used in every function calling SCs stored on the chain
+3. `load_tmp_module` is used in every function calling arbitrary code
+4. Actual cost paid in `massa-sc-runtime`
+
+### Execution
+
+Paid in `massa-sc-runtime`.
diff --git a/massa-module-cache/src/config.rs b/massa-module-cache/src/config.rs
@@ -8,8 +8,6 @@ pub struct ModuleCacheConfig {
     /// * setup `massa-sc-runtime` metering on compilation
     /// * debit compilation costs
     pub gas_costs: GasCosts,
-    /// Default gas for compilation
-    pub compilation_gas: u64,
     /// Maximum number of entries we want to keep in the LRU cache
     pub lru_cache_size: u32,
     /// Maximum number of entries we want to keep in the HD cache