fix(katana): unbound call execution from the block context limit (#3026)

crates/katana/contracts/Scarb.lock

@@ -12,6 +12,13 @@ dependencies = [
 name = "katana_messaging"
 version = "0.1.0"
 
+[[package]]
+name = "katana_misc"
+version = "0.1.0"
+dependencies = [
+ "openzeppelin",
+]
+
 [[package]]
 name = "openzeppelin"
 version = "0.17.0"

crates/katana/contracts/Scarb.toml

@@ -1,5 +1,5 @@
 [workspace]
-members = [ "account", "messaging/cairo" ]
+members = [ "account", "messaging/cairo", "misc"]
 
 [workspace.package]
 version = "0.1.0"

crates/katana/contracts/misc/Scarb.toml

@@ -0,0 +1,14 @@
+[package]
+name = "katana_misc"
+version.workspace = true
+edition.workspace = true
+
+# See more keys and their definitions at https://docs.swmansion.com/scarb/docs/reference/manifest.html
+
+[dependencies]
+starknet.workspace = true
+openzeppelin.workspace = true
+
+[[target.starknet-contract]]
+sierra = true
+casm = true

crates/katana/contracts/misc/src/lib.cairo

@@ -0,0 +1,16 @@
+#[starknet::contract]
+pub mod CallTest {
+    #[storage]
+    struct Storage { }
+
+    #[external(v0)]
+    fn bounded_call(self: @ContractState, iterations: u64) {
+        let mut i = 0;
+        loop {
+            if i >= iterations {
+                break;
+            }
+            i += 1;
+        }
+    }
+}

crates/katana/executor/src/abstraction/mod.rs

@@ -103,7 +103,7 @@ pub struct ExecutionOutput {
     pub transactions: Vec<(TxWithHash, ExecutionResult)>,
 }
 
-#[derive(Debug)]
+#[derive(Debug, Clone)]
 pub struct EntryPointCall {
     /// The address of the contract whose function you're calling.
     pub contract_address: ContractAddress,

crates/katana/executor/src/implementation/blockifier/call.rs

@@ -0,0 +1,165 @@
+use std::sync::Arc;
+
+use blockifier::context::{BlockContext, TransactionContext};
+use blockifier::execution::call_info::CallInfo;
+use blockifier::execution::entry_point::{
+    CallEntryPoint, EntryPointExecutionContext, EntryPointExecutionResult,
+};
+use blockifier::state::cached_state::CachedState;
+use blockifier::state::state_api::StateReader;
+use blockifier::transaction::objects::{DeprecatedTransactionInfo, TransactionInfo};
+use katana_cairo::cairo_vm::vm::runners::cairo_runner::{ExecutionResources, RunResources};
+use katana_cairo::starknet_api::core::EntryPointSelector;
+use katana_cairo::starknet_api::transaction::Calldata;
+use katana_primitives::Felt;
+
+use super::utils::to_blk_address;
+use crate::{EntryPointCall, ExecutionError};
+
+/// Perform a function call on a contract and retrieve the return values.
+pub fn execute_call<S: StateReader>(
+    request: EntryPointCall,
+    state: S,
+    block_context: &BlockContext,
+    max_gas: u64,
+) -> Result<Vec<Felt>, ExecutionError> {
+    let mut state = CachedState::new(state);
+    let res = execute_call_inner(request, &mut state, block_context, max_gas)?;
+    Ok(res.execution.retdata.0)
+}
+
+fn execute_call_inner<S: StateReader>(
+    request: EntryPointCall,
+    state: &mut CachedState<S>,
+    block_context: &BlockContext,
+    max_gas: u64,
+) -> EntryPointExecutionResult<CallInfo> {
+    let call = CallEntryPoint {
+        initial_gas: max_gas,
+        calldata: Calldata(Arc::new(request.calldata)),
+        storage_address: to_blk_address(request.contract_address),
+        entry_point_selector: EntryPointSelector(request.entry_point_selector),
+        ..Default::default()
+    };
+
+    // The run resources for a call execution will either be constraint ONLY by the block context
+    // limits OR based on the tx fee and gas prices. As can be seen here, the upper bound will
+    // always be limited the block max invoke steps https://github.com/dojoengine/blockifier/blob/5f58be8961ddf84022dd739a8ab254e32c435075/crates/blockifier/src/execution/entry_point.rs#L253
+    // even if the it's max steps is derived from the tx fees.
+    //
+    // This if statement here determines how execution will be constraint to <https://github.com/dojoengine/blockifier/blob/5f58be8961ddf84022dd739a8ab254e32c435075/crates/blockifier/src/execution/entry_point.rs#L206-L208>.
+    // This basically means, we can not set an arbitrary gas limit for this call without modifying
+    // the block context. So, we just set the run resources here manually to bypass that.
+
+    // The values for these parameters are essentially useless as we manually set the run resources
+    // later anyway.
+    let limit_steps_by_resources = true;
+    let tx_info = DeprecatedTransactionInfo::default();
+
+    let mut ctx = EntryPointExecutionContext::new_invoke(
+        Arc::new(TransactionContext {
+            block_context: block_context.clone(),
+            tx_info: TransactionInfo::Deprecated(tx_info),
+        }),
+        limit_steps_by_resources,
+    )
+    .unwrap();
+
+    // manually override the run resources
+    ctx.vm_run_resources = RunResources::new(max_gas as usize);
+    call.execute(state, &mut ExecutionResources::default(), &mut ctx)
+}
+
+#[cfg(test)]
+mod tests {
+    use std::str::FromStr;
+
+    use blockifier::context::BlockContext;
+    use blockifier::state::cached_state::{self};
+    use katana_primitives::class::ContractClass;
+    use katana_primitives::{address, felt, ContractAddress};
+    use katana_provider::test_utils;
+    use katana_provider::traits::contract::ContractClassWriter;
+    use katana_provider::traits::state::{StateFactoryProvider, StateWriter};
+    use starknet::macros::selector;
+
+    use super::execute_call_inner;
+    use crate::implementation::blockifier::state::StateProviderDb;
+    use crate::EntryPointCall;
+
+    #[test]
+    fn max_steps() {
+        // -------------------- Preparations -------------------------------
+
+        let json = include_str!("../../../tests/fixtures/call_test.json");
+        let class = ContractClass::from_str(json).unwrap();
+        let class_hash = class.class_hash().unwrap();
+        let casm_hash = class.clone().compile().unwrap().class_hash().unwrap();
+
+        // Initialize provider with the test contract
+        let provider = test_utils::test_provider();
+        // Declare test contract
+        provider.set_class(class_hash, class).unwrap();
+        provider.set_compiled_class_hash_of_class_hash(class_hash, casm_hash).unwrap();
+        // Deploy test contract
+        let address = address!("0x1337");
+        provider.set_class_hash_of_contract(address, class_hash).unwrap();
+
+        let state = provider.latest().unwrap();
+        let state = StateProviderDb::new(state, Default::default());
+
+        // ---------------------------------------------------------------
+
+        let mut state = cached_state::CachedState::new(state);
+        let ctx = BlockContext::create_for_testing();
+
+        let mut req = EntryPointCall {
+            calldata: Vec::new(),
+            contract_address: address,
+            entry_point_selector: selector!("bounded_call"),
+        };
+
+        // all the values for the calldata are handpicked as it's difficult to write a function that
+        // consumes a specific number of gas
+
+        let max_gas_1 = 1_000_000;
+        {
+            // ~900,000 gas
+            req.calldata = vec![felt!("460")];
+            let info = execute_call_inner(req.clone(), &mut state, &ctx, max_gas_1).unwrap();
+            assert!(max_gas_1 >= info.execution.gas_consumed);
+
+            req.calldata = vec![felt!("600")];
+            let result = execute_call_inner(req.clone(), &mut state, &ctx, max_gas_1);
+            assert!(result.is_err(), "should fail due to out of run resources")
+        }
+
+        let max_gas_2 = 10_000_000;
+        {
+            // rougly equivalent to 9,000,000 gas
+            req.calldata = vec![felt!("4600")];
+            let info = execute_call_inner(req.clone(), &mut state, &ctx, max_gas_2).unwrap();
+            assert!(max_gas_2 >= info.execution.gas_consumed);
+            assert!(max_gas_1 < info.execution.gas_consumed);
+
+            req.calldata = vec![felt!("5000")];
+            let result = execute_call_inner(req.clone(), &mut state, &ctx, max_gas_2);
+            assert!(result.is_err(), "should fail due to out of run resources")
+        }
+
+        let max_gas_3 = 100_000_000;
+        {
+            req.calldata = vec![felt!("47000")];
+            let info = execute_call_inner(req.clone(), &mut state, &ctx, max_gas_3).unwrap();
+            assert!(max_gas_3 >= info.execution.gas_consumed);
+            assert!(max_gas_2 < info.execution.gas_consumed);
+
+            req.calldata = vec![felt!("60000")];
+            let result = execute_call_inner(req.clone(), &mut state, &ctx, max_gas_3);
+            assert!(result.is_err(), "should fail due to out of run resources")
+        }
+
+        // Check that 'call' isn't bounded by the block context max invoke steps
+        assert!(max_gas_3 > ctx.versioned_constants().invoke_tx_max_n_steps as u64);
+    }
+}

crates/katana/executor/src/implementation/blockifier/mod.rs

@@ -2,6 +2,7 @@
 pub use blockifier;
 use blockifier::bouncer::{Bouncer, BouncerConfig, BouncerWeights};
 
+pub mod call;
 mod error;
 pub mod state;
 pub mod utils;
@@ -328,7 +329,7 @@ impl ExecutorExt for StarknetVMProcessor<'_> {
         let block_context = &self.block_context;
         let mut state = self.state.inner.lock();
         let state = MutRefState::new(&mut state.cached_state);
-        let retdata = utils::call(call, state, block_context, 1_000_000_000)?;
+        let retdata = call::execute_call(call, state, block_context, 1_000_000_000)?;
         Ok(retdata)
     }
 }

crates/katana/executor/src/implementation/blockifier/utils.rs

@@ -4,23 +4,19 @@ use std::sync::Arc;
 
 use blockifier::blockifier::block::{BlockInfo, GasPrices};
 use blockifier::bouncer::{Bouncer, BouncerConfig};
-use blockifier::context::{BlockContext, ChainInfo, FeeTokenAddresses, TransactionContext};
+use blockifier::context::{BlockContext, ChainInfo, FeeTokenAddresses};
 use blockifier::execution::call_info::{
     CallExecution, CallInfo, OrderedEvent, OrderedL2ToL1Message,
 };
-use blockifier::execution::common_hints::ExecutionMode;
 use blockifier::execution::contract_class::{
     ClassInfo, ContractClass, ContractClassV0, ContractClassV1,
 };
-use blockifier::execution::entry_point::{CallEntryPoint, CallType, EntryPointExecutionContext};
+use blockifier::execution::entry_point::CallType;
 use blockifier::fee::fee_utils::get_fee_by_gas_vector;
 use blockifier::state::cached_state::{self, TransactionalState};
 use blockifier::state::state_api::{StateReader, UpdatableState};
 use blockifier::transaction::account_transaction::AccountTransaction;
-use blockifier::transaction::objects::{
-    DeprecatedTransactionInfo, FeeType, HasRelatedFeeType, TransactionExecutionInfo,
-    TransactionInfo,
-};
+use blockifier::transaction::objects::{FeeType, HasRelatedFeeType, TransactionExecutionInfo};
 use blockifier::transaction::transaction_execution::Transaction;
 use blockifier::transaction::transactions::{
     DeclareTransaction, DeployAccountTransaction, ExecutableTransaction, InvokeTransaction,
@@ -56,7 +52,7 @@ use katana_provider::traits::contract::ContractClassProvider;
 use starknet::core::utils::parse_cairo_short_string;
 
 use super::state::CachedState;
-use crate::abstraction::{EntryPointCall, ExecutionFlags};
+use crate::abstraction::ExecutionFlags;
 use crate::utils::build_receipt;
 use crate::{ExecutionError, ExecutionResult, ExecutorResult};
 
@@ -156,51 +152,6 @@ pub fn transact<S: StateReader>(
     }
 }
 
-/// Perform a function call on a contract and retrieve the return values.
-pub fn call<S: StateReader>(
-    request: EntryPointCall,
-    state: S,
-    block_context: &BlockContext,
-    initial_gas: u128,
-) -> Result<Vec<Felt>, ExecutionError> {
-    let mut state = cached_state::CachedState::new(state);
-
-    let call = CallEntryPoint {
-        initial_gas: initial_gas as u64,
-        storage_address: to_blk_address(request.contract_address),
-        entry_point_selector: core::EntryPointSelector(request.entry_point_selector),
-        calldata: Calldata(Arc::new(request.calldata)),
-        ..Default::default()
-    };
-
-    // TODO: this must be false if fees are disabled I assume.
-    let limit_steps_by_resources = true;
-
-    // Now, the max step is not given directly to this function.
-    // It's computed by a new function max_steps, and it tooks the values
-    // from the block context itself instead of the input give. The dojoengine
-    // fork of the blockifier ensures we're not limited by the min function applied
-    // by starkware.
-    // https://github.com/starkware-libs/blockifier/blob/4fd71645b45fd1deb6b8e44802414774ec2a2ec1/crates/blockifier/src/execution/entry_point.rs#L159
-    // https://github.com/dojoengine/blockifier/blob/5f58be8961ddf84022dd739a8ab254e32c435075/crates/blockifier/src/execution/entry_point.rs#L188
-
-    let res = call.execute(
-        &mut state,
-        &mut ExecutionResources::default(),
-        &mut EntryPointExecutionContext::new(
-            Arc::new(TransactionContext {
-                block_context: block_context.clone(),
-                tx_info: TransactionInfo::Deprecated(DeprecatedTransactionInfo::default()),
-            }),
-            ExecutionMode::Execute,
-            limit_steps_by_resources,
-        )
-        .expect("shouldn't fail"),
-    )?;
-
-    Ok(res.execution.retdata.0)
-}
-
 pub fn to_executor_tx(tx: ExecutableTxWithHash) -> Transaction {
     let hash = tx.hash;
 
@@ -739,6 +690,7 @@ mod tests {
 
     use std::collections::{HashMap, HashSet};
 
+    use blockifier::execution::entry_point::CallEntryPoint;
     use katana_cairo::cairo_vm::types::builtin_name::BuiltinName;
     use katana_cairo::cairo_vm::vm::runners::cairo_runner::ExecutionResources;
     use katana_cairo::starknet_api::core::EntryPointSelector;