chore(cairo_native): temporarily revert EP gas deduction and align te…

…sts (#2214)
starkware-libs · Nov 21, 2024 · 4f06698 · 4f06698
1 parent 5def620
commit 4f06698
Show file tree

Hide file tree

Showing 13 changed files with 102 additions and 112 deletions.
diff --git a/crates/blockifier/src/execution/native/entry_point_execution.rs b/crates/blockifier/src/execution/native/entry_point_execution.rs
@@ -1,7 +1,6 @@
 use cairo_native::execution_result::ContractExecutionResult;
 use cairo_native::utils::BuiltinCosts;
 use cairo_vm::vm::runners::cairo_runner::ExecutionResources;
-use num_traits::ToPrimitive;
 use starknet_api::execution_resources::GasAmount;
 
 use crate::execution::call_info::{CallExecution, CallInfo, ChargedResources, Retdata};
@@ -40,10 +39,13 @@ pub fn execute_entry_point_call(
         mul_mod: gas_costs.mul_mod_gas_cost,
     };
 
+    // Fund the initial budget since the native executor charges it before the run.
+    // TODO(Yoni): revert once the VM is aligned with this.
+    let gas = syscall_handler.call.initial_gas + gas_costs.entry_point_initial_budget;
     let execution_result = contract_class.executor.run(
         entry_point.selector.0,
         &syscall_handler.call.calldata.0.clone(),
-        Some(syscall_handler.call.initial_gas.into()),
+        Some(gas),
         Some(builtin_costs),
         &mut syscall_handler,
     );
@@ -61,22 +63,24 @@ fn create_callinfo(
     call_result: ContractExecutionResult,
     syscall_handler: NativeSyscallHandler<'_>,
 ) -> Result<CallInfo, EntryPointExecutionError> {
-    let remaining_gas =
-        call_result.remaining_gas.to_u64().ok_or(PostExecutionError::MalformedReturnData {
-            error_message: format!(
-                "Unexpected remaining gas. Gas value is bigger than u64: {}",
-                call_result.remaining_gas
-            ),
-        })?;
+    let mut remaining_gas = call_result.remaining_gas;
 
     if remaining_gas > syscall_handler.call.initial_gas {
-        return Err(PostExecutionError::MalformedReturnData {
-            error_message: format!(
-                "Unexpected remaining gas. Used gas is greater than initial gas: {} > {}",
-                remaining_gas, syscall_handler.call.initial_gas
-            ),
+        if remaining_gas - syscall_handler.call.initial_gas
+            <= syscall_handler.context.gas_costs().entry_point_initial_budget
+        {
+            // Revert the refund.
+            // TODO(Yoni): temporary hack - this is probably a bug. Investigate and fix native.
+            remaining_gas = syscall_handler.call.initial_gas;
+        } else {
+            return Err(PostExecutionError::MalformedReturnData {
+                error_message: format!(
+                    "Unexpected remaining gas. Used gas is greater than initial gas: {} > {}",
+                    remaining_gas, syscall_handler.call.initial_gas
+                ),
+            }
+            .into());
         }
-        .into());
     }
 
     let gas_consumed = syscall_handler.call.initial_gas - remaining_gas;

diff --git a/crates/blockifier/src/execution/syscalls/syscall_tests/call_contract.rs b/crates/blockifier/src/execution/syscalls/syscall_tests/call_contract.rs
@@ -72,22 +72,16 @@ fn test_call_contract_that_panics() {
     feature = "cairo_native",
     test_case(
       FeatureContract::TestContract(CairoVersion::Native),
-      FeatureContract::TestContract(CairoVersion::Native),
-      190370;
+      FeatureContract::TestContract(CairoVersion::Native);
       "Call Contract between two contracts using Native"
     )
 )]
 #[test_case(
     FeatureContract::TestContract(CairoVersion::Cairo1),
-    FeatureContract::TestContract(CairoVersion::Cairo1),
-    REQUIRED_GAS_CALL_CONTRACT_TEST;
+    FeatureContract::TestContract(CairoVersion::Cairo1);
     "Call Contract between two contracts using VM"
 )]
-fn test_call_contract(
-    outer_contract: FeatureContract,
-    inner_contract: FeatureContract,
-    expected_gas: u64,
-) {
+fn test_call_contract(outer_contract: FeatureContract, inner_contract: FeatureContract) {
     let chain_info = &ChainInfo::create_for_testing();
     let mut state = test_state(chain_info, BALANCE, &[(outer_contract, 1), (inner_contract, 1)]);
 
@@ -110,7 +104,7 @@ fn test_call_contract(
         entry_point_call.execute_directly(&mut state).unwrap().execution,
         CallExecution {
             retdata: retdata![felt!(48_u8)],
-            gas_consumed: expected_gas,
+            gas_consumed: REQUIRED_GAS_CALL_CONTRACT_TEST,
             ..CallExecution::default()
         }
     );

diff --git a/crates/blockifier/src/execution/syscalls/syscall_tests/deploy.rs b/crates/blockifier/src/execution/syscalls/syscall_tests/deploy.rs
@@ -14,12 +14,12 @@ use crate::test_utils::contracts::FeatureContract;
 use crate::test_utils::initial_test_state::test_state;
 use crate::test_utils::{calldata_for_deploy_test, trivial_external_entry_point_new, CairoVersion};
 
-#[test_case(FeatureContract::TestContract(CairoVersion::Cairo1), 205200;"VM")]
+#[test_case(FeatureContract::TestContract(CairoVersion::Cairo1);"VM")]
 #[cfg_attr(
     feature = "cairo_native",
-    test_case(FeatureContract::TestContract(CairoVersion::Native), 215200;"Native")
+    test_case(FeatureContract::TestContract(CairoVersion::Native);"Native")
 )]
-fn no_constructor(deployer_contract: FeatureContract, expected_gas: u64) {
+fn no_constructor(deployer_contract: FeatureContract) {
     // TODO(Yoni): share the init code of the tests in this file.
 
     let empty_contract = FeatureContract::Empty(CairoVersion::Cairo1);
@@ -41,11 +41,7 @@ fn no_constructor(deployer_contract: FeatureContract, expected_gas: u64) {
     let deploy_call = &entry_point_call.execute_directly(&mut state).unwrap();
     assert_eq!(
         deploy_call.execution,
-        CallExecution {
-            retdata: retdata![],
-            gas_consumed: expected_gas,
-            ..CallExecution::default()
-        }
+        CallExecution { retdata: retdata![], gas_consumed: 205200, ..CallExecution::default() }
     );
 
     let deployed_contract_address = calculate_contract_address(
@@ -96,16 +92,12 @@ fn no_constructor_nonempty_calldata(deployer_contract: FeatureContract) {
     ));
 }
 
-#[test_case(FeatureContract::TestContract(CairoVersion::Cairo1),214550, 4610;"VM")]
+#[test_case(FeatureContract::TestContract(CairoVersion::Cairo1);"VM")]
 #[cfg_attr(
     feature = "cairo_native",
-    test_case(FeatureContract::TestContract(CairoVersion::Native),234550, 14610;"Native")
+    test_case(FeatureContract::TestContract(CairoVersion::Native);"Native")
 )]
-fn with_constructor(
-    deployer_contract: FeatureContract,
-    expected_gas: u64,
-    expected_constructor_gas: u64,
-) {
+fn with_constructor(deployer_contract: FeatureContract) {
     let mut state = test_state(&ChainInfo::create_for_testing(), Fee(0), &[(deployer_contract, 1)]);
 
     let class_hash = deployer_contract.get_class_hash();
@@ -134,11 +126,7 @@ fn with_constructor(
     let deploy_call = &entry_point_call.execute_directly(&mut state).unwrap();
     assert_eq!(
         deploy_call.execution,
-        CallExecution {
-            retdata: retdata![],
-            gas_consumed: expected_gas,
-            ..CallExecution::default()
-        }
+        CallExecution { retdata: retdata![], gas_consumed: 214550, ..CallExecution::default() }
     );
 
     let constructor_call = &deploy_call.inner_calls[0];
@@ -150,7 +138,7 @@ fn with_constructor(
             // The test contract constructor returns its first argument.
             retdata: retdata![constructor_calldata[0]],
             // This reflects the gas cost of storage write syscall.
-            gas_consumed: expected_constructor_gas,
+            gas_consumed: 4610,
             ..CallExecution::default()
         }
     );

diff --git a/crates/blockifier/src/execution/syscalls/syscall_tests/emit_event.rs b/crates/blockifier/src/execution/syscalls/syscall_tests/emit_event.rs
@@ -28,10 +28,10 @@ const N_EMITTED_EVENTS: [Felt; 1] = [Felt::from_hex_unchecked("0x1")];
 
 #[cfg_attr(
   feature = "cairo_native",
-  test_case(FeatureContract::TestContract(CairoVersion::Native), 57330; "Native")
+  test_case(FeatureContract::TestContract(CairoVersion::Native); "Native")
 )]
-#[test_case(FeatureContract::TestContract(CairoVersion::Cairo1), 47330; "VM")]
-fn positive_flow(test_contract: FeatureContract, expected_gas: u64) {
+#[test_case(FeatureContract::TestContract(CairoVersion::Cairo1); "VM")]
+fn positive_flow(test_contract: FeatureContract) {
     let call_info = emit_events(test_contract, &N_EMITTED_EVENTS, &KEYS, &DATA)
         .expect("emit_events failed with valued parameters");
     let event = EventContent {
@@ -43,7 +43,7 @@ fn positive_flow(test_contract: FeatureContract, expected_gas: u64) {
         call_info.execution,
         CallExecution {
             events: vec![OrderedEvent { order: 0, event }],
-            gas_consumed: expected_gas,
+            gas_consumed: 47330,
             ..Default::default()
         }
     );

diff --git a/crates/blockifier/src/execution/syscalls/syscall_tests/get_block_hash.rs b/crates/blockifier/src/execution/syscalls/syscall_tests/get_block_hash.rs
@@ -42,10 +42,10 @@ fn initialize_state(test_contract: FeatureContract) -> (CachedState<DictStateRea
 
 #[cfg_attr(
     feature = "cairo_native",
-    test_case(FeatureContract::TestContract(CairoVersion::Native), 15220; "Native")
+    test_case(FeatureContract::TestContract(CairoVersion::Native); "Native")
 )]
-#[test_case(FeatureContract::TestContract(CairoVersion::Cairo1), 5220; "VM")]
-fn positive_flow(test_contract: FeatureContract, expected_gas: u64) {
+#[test_case(FeatureContract::TestContract(CairoVersion::Cairo1); "VM")]
+fn positive_flow(test_contract: FeatureContract) {
     let (mut state, block_number, block_hash) = initialize_state(test_contract);
 
     let calldata = calldata![block_number];
@@ -57,10 +57,7 @@ fn positive_flow(test_contract: FeatureContract, expected_gas: u64) {
 
     assert_eq!(
         entry_point_call.clone().execute_directly(&mut state).unwrap().execution,
-        CallExecution {
-            gas_consumed: expected_gas,
-            ..CallExecution::from_retdata(retdata![block_hash])
-        }
+        CallExecution { gas_consumed: 5220, ..CallExecution::from_retdata(retdata![block_hash]) }
     );
 }
 

diff --git a/crates/blockifier/src/execution/syscalls/syscall_tests/get_class_hash_at.rs b/crates/blockifier/src/execution/syscalls/syscall_tests/get_class_hash_at.rs
@@ -17,12 +17,12 @@ use crate::test_utils::{trivial_external_entry_point_new, CairoVersion, BALANCE}
 /// 3. Execution succeeds with expected gas for valid cases.
 /// 4. Execution fails if `address` has a different `class_hash`.
 /// 5. Execution succeeds and returns `class_hash` = 0 if `address` is absent.
-#[test_case(FeatureContract::TestContract(CairoVersion::Cairo1), REQUIRED_GAS_GET_CLASS_HASH_AT_TEST; "VM")]
+#[test_case(FeatureContract::TestContract(CairoVersion::Cairo1); "VM")]
 #[cfg_attr(
     feature = "cairo_native",
-    test_case(FeatureContract::TestContract(CairoVersion::Native), 17830; "Native"))
+    test_case(FeatureContract::TestContract(CairoVersion::Native); "Native"))
 ]
-fn test_get_class_hash_at(test_contract: FeatureContract, expected_gas: u64) {
+fn test_get_class_hash_at(test_contract: FeatureContract) {
     let chain_info = &ChainInfo::create_for_testing();
     let mut state = test_state(chain_info, BALANCE, &[(test_contract, 1)]);
     let address = contract_address!("0x111");
@@ -42,7 +42,7 @@ fn test_get_class_hash_at(test_contract: FeatureContract, expected_gas: u64) {
         positive_call_info.execution,
         CallExecution {
             retdata: retdata!(),
-            gas_consumed: expected_gas,
+            gas_consumed: REQUIRED_GAS_GET_CLASS_HASH_AT_TEST,
             failed: false,
             ..CallExecution::default()
         }

diff --git a/crates/blockifier/src/execution/syscalls/syscall_tests/keccak.rs b/crates/blockifier/src/execution/syscalls/syscall_tests/keccak.rs
@@ -10,12 +10,12 @@ use crate::test_utils::contracts::FeatureContract;
 use crate::test_utils::initial_test_state::test_state;
 use crate::test_utils::{trivial_external_entry_point_new, CairoVersion, BALANCE};
 
-#[test_case(FeatureContract::TestContract(CairoVersion::Cairo1), 254910; "VM")]
+#[test_case(FeatureContract::TestContract(CairoVersion::Cairo1); "VM")]
 #[cfg_attr(
     feature = "cairo_native",
-    test_case(FeatureContract::TestContract(CairoVersion::Native), 264910; "Native")
+    test_case(FeatureContract::TestContract(CairoVersion::Native); "Native")
 )]
-fn test_keccak(test_contract: FeatureContract, expected_gas: u64) {
+fn test_keccak(test_contract: FeatureContract) {
     let chain_info = &ChainInfo::create_for_testing();
     let mut state = test_state(chain_info, BALANCE, &[(test_contract, 1)]);
 
@@ -28,6 +28,6 @@ fn test_keccak(test_contract: FeatureContract, expected_gas: u64) {
 
     pretty_assertions::assert_eq!(
         entry_point_call.execute_directly(&mut state).unwrap().execution,
-        CallExecution { gas_consumed: expected_gas, ..CallExecution::from_retdata(retdata![]) }
+        CallExecution { gas_consumed: 254910, ..CallExecution::from_retdata(retdata![]) }
     );
 }