Skip to content

Commit

Permalink
fix(katana): set bouncer weight to max (#2367)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
kariy authored Aug 29, 2024
1 parent fc1f894 commit 061a6ef
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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<S: StateDb>(
Expand Down
73 changes: 66 additions & 7 deletions crates/katana/rpc/rpc/tests/starknet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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<u64>,
) -> 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 =
Expand Down

0 comments on commit 061a6ef

Please sign in to comment.