From 061a6eff97549131639f7fcf6365b41618836a26 Mon Sep 17 00:00:00 2001 From: Ammar Arif Date: Thu, 29 Aug 2024 17:55:33 -0400 Subject: [PATCH] fix(katana): set bouncer weight to max (#2367) the `StatefulValidator` runs the full execution flow for `DeployAccount` (as opposed to just the validation logic), the execution flow include a 'block limit' check and because currently we're setting the block limit to zero, executing a deploy account tx using the validator, will always return `Transaction size exceeds the maximum block capacity.` error. why this doesn't affect normal execution (ie `BlockExecutor`'s execution) ? bcs of the 'execute' function we're calling here: https://github.com/dojoengine/dojo/blob/fc1f894de7c25faef290399d8c904b290033a729/crates/katana/executor/src/implementation/blockifier/utils.rs#L86-L91 in blockifier, the execute logic is duplicated on both `Transaction` and `AccountTransaction` structs. the execute logic in `Transaction` is the one that includes the block limit check, but based on above, we're calling the execute method of `AccountTransaction`. This is the 'execute' we're using in `BlockExecutor`: https://github.com/dojoengine/blockifier/blob/031eef1b54766bc9799e97c43f63e36b63af30ee/ crates/blockifier/src/transaction/account_transaction.rs#L635 and this is the one used in stateful validator: https://github.com/dojoengine/blockifier/blob/08ac6f38519f1ca87684665d084a7a62448009cc/crates/blockifier/src/transaction/transaction_execution.rs#L155-L190 so the fix is to just naively increase the block limit to max. considering we're not using this in our execution path, this change is fine. even once we include it, at this point we dont really care about block limit, so keeping it at max is still fine. the `deploy_account` test doesn't directly test for the block limit values, but its still a good test to have so imma keep that in. --- .../src/implementation/blockifier/utils.rs | 3 +- crates/katana/rpc/rpc/tests/starknet.rs | 73 +++++++++++++++++-- 2 files changed, 68 insertions(+), 8 deletions(-) diff --git a/crates/katana/executor/src/implementation/blockifier/utils.rs b/crates/katana/executor/src/implementation/blockifier/utils.rs index cc202d3695..8dffe1a1f6 100644 --- a/crates/katana/executor/src/implementation/blockifier/utils.rs +++ b/crates/katana/executor/src/implementation/blockifier/utils.rs @@ -3,6 +3,7 @@ use std::num::NonZeroU128; use std::sync::Arc; use blockifier::blockifier::block::{BlockInfo, GasPrices}; +use blockifier::bouncer::BouncerConfig; use blockifier::context::{BlockContext, ChainInfo, FeeTokenAddresses, TransactionContext}; use blockifier::execution::call_info::{ CallExecution, CallInfo, OrderedEvent, OrderedL2ToL1Message, @@ -392,7 +393,7 @@ pub fn block_context_from_envs(block_env: &BlockEnv, cfg_env: &CfgEnv) -> BlockC versioned_constants.validate_max_n_steps = cfg_env.validate_max_n_steps; versioned_constants.invoke_tx_max_n_steps = cfg_env.invoke_tx_max_n_steps; - BlockContext::new(block_info, chain_info, versioned_constants, Default::default()) + BlockContext::new(block_info, chain_info, versioned_constants, BouncerConfig::max()) } pub(super) fn state_update_from_cached_state( diff --git a/crates/katana/rpc/rpc/tests/starknet.rs b/crates/katana/rpc/rpc/tests/starknet.rs index f6c5163b61..d98311e0e3 100644 --- a/crates/katana/rpc/rpc/tests/starknet.rs +++ b/crates/katana/rpc/rpc/tests/starknet.rs @@ -12,26 +12,28 @@ use dojo_test_utils::sequencer::{get_default_test_starknet_config, TestSequencer use indexmap::IndexSet; use katana_core::sequencer::SequencerConfig; use katana_primitives::genesis::constant::{ - DEFAULT_FEE_TOKEN_ADDRESS, DEFAULT_PREFUNDED_ACCOUNT_BALANCE, DEFAULT_UDC_ADDRESS, + DEFAULT_FEE_TOKEN_ADDRESS, DEFAULT_OZ_ACCOUNT_CONTRACT_CLASS_HASH, + DEFAULT_PREFUNDED_ACCOUNT_BALANCE, DEFAULT_UDC_ADDRESS, }; use starknet::accounts::{ - Account, AccountError, Call, ConnectedAccount, ExecutionEncoding, SingleOwnerAccount, + Account, AccountError, AccountFactory, Call, ConnectedAccount, ExecutionEncoding, + OpenZeppelinAccountFactory, SingleOwnerAccount, }; use starknet::core::types::contract::legacy::LegacyContractClass; use starknet::core::types::{ - BlockId, BlockTag, DeclareTransactionReceipt, ExecutionResult, Felt, StarknetError, - TransactionFinalityStatus, TransactionReceipt, + BlockId, BlockTag, DeclareTransactionReceipt, DeployAccountTransactionReceipt, ExecutionResult, + Felt, StarknetError, TransactionFinalityStatus, TransactionReceipt, }; use starknet::core::utils::get_contract_address; use starknet::macros::{felt, selector}; use starknet::providers::{Provider, ProviderError}; -use starknet::signers::{LocalWallet, SigningKey}; +use starknet::signers::{LocalWallet, Signer, SigningKey}; use tokio::sync::Mutex; mod common; #[tokio::test] -async fn test_send_declare_and_deploy_contract() -> Result<()> { +async fn declare_and_deploy_contract() -> Result<()> { let sequencer = TestSequencer::start(SequencerConfig::default(), get_default_test_starknet_config()).await; @@ -86,7 +88,7 @@ async fn test_send_declare_and_deploy_contract() -> Result<()> { } #[tokio::test] -async fn test_send_declare_and_deploy_legacy_contract() -> Result<()> { +async fn declare_and_deploy_legacy_contract() -> Result<()> { let sequencer = TestSequencer::start(SequencerConfig::default(), get_default_test_starknet_config()).await; @@ -139,6 +141,63 @@ async fn test_send_declare_and_deploy_legacy_contract() -> Result<()> { Ok(()) } +#[rstest::rstest] +#[tokio::test] +async fn deploy_account( + #[values(true, false)] disable_fee: bool, + #[values(None, Some(1000))] block_time: Option, +) -> Result<()> { + // setup test sequencer with the given configuration + let mut starknet_config = get_default_test_starknet_config(); + starknet_config.disable_fee = disable_fee; + let sequencer_config = SequencerConfig { block_time, ..Default::default() }; + + let sequencer = TestSequencer::start(sequencer_config, starknet_config).await; + + let provider = sequencer.provider(); + let funding_account = sequencer.account(); + let chain_id = provider.chain_id().await?; + + // Precompute the contract address of the new account with the given parameters: + let signer = LocalWallet::from(SigningKey::from_random()); + let class_hash = DEFAULT_OZ_ACCOUNT_CONTRACT_CLASS_HASH; + let salt = felt!("0x123"); + let ctor_args = [signer.get_public_key().await?.scalar()]; + let computed_address = get_contract_address(salt, class_hash, &ctor_args, Felt::ZERO); + + // Fund the new account + abigen_legacy!(FeeToken, "crates/katana/rpc/rpc/tests/test_data/erc20.json"); + let contract = FeeToken::new(DEFAULT_FEE_TOKEN_ADDRESS.into(), &funding_account); + + // send enough tokens to the new_account's address just to send the deploy account tx + let amount = Uint256 { low: felt!("0x100000000000"), high: Felt::ZERO }; + let recipient = computed_address; + let res = contract.transfer(&recipient, &amount).send().await?; + dojo_utils::TransactionWaiter::new(res.transaction_hash, &provider).await?; + + // starknet-rs's utility for deploying an OpenZeppelin account + let factory = OpenZeppelinAccountFactory::new(class_hash, chain_id, &signer, &provider).await?; + let res = factory.deploy_v1(salt).send().await?; + // the contract address in the send tx result must be the same as the computed one + assert_eq!(res.contract_address, computed_address); + + let receipt = dojo_utils::TransactionWaiter::new(res.transaction_hash, &provider).await?; + assert_matches!( + receipt.receipt, + TransactionReceipt::DeployAccount(DeployAccountTransactionReceipt { contract_address, .. }) => { + // the contract address in the receipt must be the same as the computed one + assert_eq!(contract_address, computed_address) + } + ); + + // Verify the `getClassHashAt` returns the same class hash that we use for the account + // deployment + let res = provider.get_class_hash_at(BlockId::Tag(BlockTag::Pending), computed_address).await?; + assert_eq!(res, class_hash); + + Ok(()) +} + #[tokio::test] async fn estimate_fee() -> Result<()> { let sequencer =