Skip to content

Commit

Permalink
fix: internalise request timeout and lower l2 inclusion timeout
Browse files Browse the repository at this point in the history
  • Loading branch information
glihm committed Nov 1, 2024
1 parent b10fd79 commit 4faa650
Show file tree
Hide file tree
Showing 13 changed files with 35 additions and 100 deletions.
9 changes: 2 additions & 7 deletions bin/sozo/src/commands/call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,13 +67,8 @@ impl CallArgs {
};

config.tokio_handle().block_on(async {
let (world_diff, provider, _) = utils::get_world_diff_and_provider(
self.starknet.clone(),
self.world,
Default::default(),
&ws,
)
.await?;
let (world_diff, provider, _) =
utils::get_world_diff_and_provider(self.starknet.clone(), self.world, &ws).await?;

let calldata = if let Some(cd) = self.calldata {
calldata_decoder::decode_calldata(&cd)?
Expand Down
11 changes: 3 additions & 8 deletions bin/sozo/src/commands/execute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,14 +81,9 @@ impl ExecuteArgs {
let txn_config: TxnConfig = self.transaction.into();

config.tokio_handle().block_on(async {
let (world_diff, account, _) = utils::get_world_diff_and_account(
self.account,
self.starknet,
self.world,
txn_config,
&ws,
)
.await?;
let (world_diff, account, _) =
utils::get_world_diff_and_account(self.account, self.starknet, self.world, &ws)
.await?;

let contract_address = match &descriptor {
ContractDescriptor::Address(address) => Some(*address),
Expand Down
2 changes: 1 addition & 1 deletion bin/sozo/src/commands/inspect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ impl InspectArgs {

config.tokio_handle().block_on(async {
let (world_diff, _, _) =
utils::get_world_diff_and_provider(starknet.clone(), world, None, &ws).await?;
utils::get_world_diff_and_provider(starknet.clone(), world, &ws).await?;

if let Some(resource) = resource {
inspect_resource(&resource, &world_diff);
Expand Down
11 changes: 5 additions & 6 deletions bin/sozo/src/commands/migrate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,15 +54,14 @@ impl MigrateArgs {
let mut spinner =
MigrationUi::Spinner(Spinner::new(frames, "Evaluating world diff...", None));

let mut txn_config: TxnConfig = self.transaction.into();
txn_config.wait = true;

let (world_diff, account, rpc_url) =
utils::get_world_diff_and_account(account, starknet, world, txn_config, &ws)
.await?;
utils::get_world_diff_and_account(account, starknet, world, &ws).await?;

let world_address = world_diff.world_info.address;

let mut txn_config: TxnConfig = self.transaction.into();
txn_config.wait = true;

let migration = Migration::new(
world_diff,
WorldContract::new(world_address, &account),
Expand Down Expand Up @@ -101,7 +100,7 @@ pub struct Banner {

/// Prints the migration banner.
async fn print_banner(ws: &Workspace<'_>, starknet: &StarknetOptions) -> Result<()> {
let (provider, rpc_url) = starknet.provider(None, None)?;
let (provider, rpc_url) = starknet.provider(None)?;

let chain_id = provider.chain_id().await?;
let chain_id = parse_cairo_short_string(&chain_id)
Expand Down
16 changes: 6 additions & 10 deletions bin/sozo/src/commands/options/starknet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,25 +21,21 @@ pub struct StarknetOptions {
}

impl StarknetOptions {
const DEFAULT_REQUEST_TIMEOUT_MS: u64 = 20_000;

/// Returns a [`JsonRpcClient`] and the rpc url.
///
/// It would be convenient to have the rpc url retrievable from the Provider trait instead.
pub fn provider(
&self,
env_metadata: Option<&Environment>,
request_timeout_ms: Option<u64>,
) -> Result<(JsonRpcClient<HttpTransport>, String)> {
let url = self.url(env_metadata)?;
trace!(?url, timeout = ?request_timeout_ms, "Creating JsonRpcClient.");

let client = if let Some(request_timeout_ms) = request_timeout_ms {
ClientBuilder::default()
.timeout(Duration::from_millis(request_timeout_ms))
.build()
.unwrap()
} else {
ClientBuilder::default().build().unwrap()
};
let client = ClientBuilder::default()
.timeout(Duration::from_millis(Self::DEFAULT_REQUEST_TIMEOUT_MS))
.build()
.unwrap();

let transport = HttpTransport::new_with_client(url.clone(), client);
Ok((JsonRpcClient::new(transport), url.to_string()))
Expand Down
8 changes: 0 additions & 8 deletions bin/sozo/src/commands/options/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,6 @@ pub struct TransactionOptions {
#[arg(help = "Display the link to debug the transaction with Walnut.")]
#[arg(global = true)]
pub walnut: bool,

#[arg(long)]
#[arg(help = "The timeout in milliseconds for the transaction wait.")]
#[arg(value_name = "TIMEOUT-MS")]
#[arg(global = true)]
pub timeout: Option<u64>,
}

impl TransactionOptions {
Expand All @@ -67,7 +61,6 @@ impl TransactionOptions {
max_fee_raw: self.max_fee_raw,
fee_estimate_multiplier: self.fee_estimate_multiplier,
walnut: self.walnut,
timeout_ms: self.timeout,
}),
}
}
Expand All @@ -81,7 +74,6 @@ impl From<TransactionOptions> for TxnConfig {
receipt: value.receipt,
max_fee_raw: value.max_fee_raw,
walnut: value.walnut,
timeout_ms: value.timeout,
}
}
}
7 changes: 2 additions & 5 deletions bin/sozo/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ use std::str::FromStr;
use anyhow::{anyhow, Context, Result};
use camino::Utf8PathBuf;
use colored::*;
use dojo_utils::TxnConfig;
use dojo_world::config::ProfileConfig;
use dojo_world::diff::WorldDiff;
use dojo_world::local::WorldLocal;
Expand Down Expand Up @@ -103,7 +102,6 @@ pub fn is_address(tag_or_address: &str) -> bool {
pub async fn get_world_diff_and_provider(
starknet: StarknetOptions,
world: WorldOptions,
provider_request_timeout_ms: Option<u64>,
ws: &Workspace<'_>,
) -> Result<(WorldDiff, JsonRpcClient<HttpTransport>, String)> {
let world_local = ws.load_world_local()?;
Expand All @@ -113,7 +111,7 @@ pub async fn get_world_diff_and_provider(

let world_address = get_world_address(&profile_config, &world, &world_local)?;

let (provider, rpc_url) = starknet.provider(env, provider_request_timeout_ms)?;
let (provider, rpc_url) = starknet.provider(env)?;
trace!(?provider, "Provider initialized.");

let spec_version = provider.spec_version().await?;
Expand Down Expand Up @@ -145,14 +143,13 @@ pub async fn get_world_diff_and_account(
account: AccountOptions,
starknet: StarknetOptions,
world: WorldOptions,
txn_config: TxnConfig,
ws: &Workspace<'_>,
) -> Result<(WorldDiff, SozoAccount<JsonRpcClient<HttpTransport>>, String)> {
let profile_config = ws.load_profile_config()?;
let env = profile_config.env.as_ref();

let (world_diff, provider, rpc_url) =
get_world_diff_and_provider(starknet.clone(), world, txn_config.timeout_ms, ws).await?;
get_world_diff_and_provider(starknet.clone(), world, ws).await?;

let account = {
account
Expand Down
10 changes: 1 addition & 9 deletions crates/dojo/utils/src/tx/declarer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@

use std::collections::HashMap;
use std::sync::Arc;
use std::time::Duration;

use starknet::accounts::ConnectedAccount;
use starknet::core::types::{
Expand Down Expand Up @@ -103,15 +102,8 @@ where
"Declared class."
);

// Since TxnConfig::wait doesn't work for now, we wait for the transaction manually.
if txn_config.wait {
let receipt = if let Some(timeout_ms) = txn_config.timeout_ms {
TransactionWaiter::new(transaction_hash, &account.provider())
.with_timeout(Duration::from_millis(timeout_ms))
.await?
} else {
TransactionWaiter::new(transaction_hash, &account.provider()).await?
};
let receipt = TransactionWaiter::new(transaction_hash, &account.provider()).await?;

if txn_config.receipt {
return Ok(TransactionResult::HashReceipt(transaction_hash, Box::new(receipt)));
Expand Down
11 changes: 2 additions & 9 deletions crates/dojo/utils/src/tx/deployer.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
//! The deployer is in charge of deploying contracts to starknet.

use std::time::Duration;

use starknet::accounts::ConnectedAccount;
use starknet::core::types::{
BlockId, BlockTag, Call, Felt, InvokeTransactionResult, StarknetError,
Expand Down Expand Up @@ -74,13 +72,8 @@ where
);

if self.txn_config.wait {
let receipt = if let Some(timeout_ms) = self.txn_config.timeout_ms {
TransactionWaiter::new(transaction_hash, &self.account.provider())
.with_timeout(Duration::from_millis(timeout_ms))
.await?
} else {
TransactionWaiter::new(transaction_hash, &self.account.provider()).await?
};
let receipt =
TransactionWaiter::new(transaction_hash, &self.account.provider()).await?;

if self.txn_config.receipt {
return Ok(TransactionResult::HashReceipt(transaction_hash, Box::new(receipt)));
Expand Down
20 changes: 4 additions & 16 deletions crates/dojo/utils/src/tx/invoker.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
//! Invoker to invoke contracts.

use std::time::Duration;

use starknet::accounts::ConnectedAccount;
use starknet::core::types::Call;
use tracing::trace;
Expand Down Expand Up @@ -61,13 +59,8 @@ where
trace!(transaction_hash = format!("{:#066x}", tx.transaction_hash), "Invoke contract.");

if self.txn_config.wait {
let receipt = if let Some(timeout_ms) = self.txn_config.timeout_ms {
TransactionWaiter::new(tx.transaction_hash, &self.account.provider())
.with_timeout(Duration::from_millis(timeout_ms))
.await?
} else {
TransactionWaiter::new(tx.transaction_hash, &self.account.provider()).await?
};
let receipt =
TransactionWaiter::new(tx.transaction_hash, &self.account.provider()).await?;

if self.txn_config.receipt {
return Ok(TransactionResult::HashReceipt(tx.transaction_hash, Box::new(receipt)));
Expand All @@ -94,13 +87,8 @@ where
);

if self.txn_config.wait {
let receipt = if let Some(timeout_ms) = self.txn_config.timeout_ms {
TransactionWaiter::new(tx.transaction_hash, &self.account.provider())
.with_timeout(Duration::from_millis(timeout_ms))
.await?
} else {
TransactionWaiter::new(tx.transaction_hash, &self.account.provider()).await?
};
let receipt =
TransactionWaiter::new(tx.transaction_hash, &self.account.provider()).await?;

if self.txn_config.receipt {
return Ok(TransactionResult::HashReceipt(tx.transaction_hash, Box::new(receipt)));
Expand Down
8 changes: 4 additions & 4 deletions crates/dojo/utils/src/tx/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,14 @@ pub struct TxnConfig {
/// The multiplier for how much the actual transaction max fee should be relative to the
/// estimated fee. If `None` is provided, the multiplier is set to `1.1`.
pub fee_estimate_multiplier: Option<f64>,
/// Whether to wait for the transaction to be accepted or reverted on L2.
pub wait: bool,
/// Whether to display the transaction receipt.
pub receipt: bool,
/// The maximum fee to pay for the transaction.
pub max_fee_raw: Option<Felt>,
/// Whether to use the `walnut` fee estimation strategy.
pub walnut: bool,
pub timeout_ms: Option<u64>,
}

#[derive(Debug, Copy, Clone)]
Expand All @@ -40,11 +43,8 @@ pub enum TxnAction {
wait: bool,
receipt: bool,
max_fee_raw: Option<Felt>,
/// The multiplier for how much the actual transaction max fee should be relative to the
/// estimated fee. If `None` is provided, the multiplier is set to `1.1`.
fee_estimate_multiplier: Option<f64>,
walnut: bool,
timeout_ms: Option<u64>,
},
Estimate,
Simulate,
Expand Down
3 changes: 2 additions & 1 deletion crates/dojo/utils/src/tx/waiter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,8 @@ impl<'a, P> TransactionWaiter<'a, P>
where
P: Provider + Send,
{
const DEFAULT_TIMEOUT: Duration = Duration::from_secs(300);
/// The default timeout for a transaction to be accepted or reverted on L2.
const DEFAULT_TIMEOUT: Duration = Duration::from_secs(30);
/// Interval for use with 3rd party provider without burning the API rate limit.
const DEFAULT_INTERVAL: Duration = Duration::from_millis(2500);

Expand Down
19 changes: 3 additions & 16 deletions crates/sozo/ops/src/account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -256,14 +256,7 @@ pub async fn deploy(
};

match txn_action {
TxnAction::Send {
wait,
receipt,
max_fee_raw,
fee_estimate_multiplier,
walnut,
timeout_ms,
} => {
TxnAction::Send { wait, receipt, max_fee_raw, fee_estimate_multiplier, walnut } => {
let max_fee = if let Some(max_fee_raw) = max_fee_raw {
MaxFeeType::Manual { max_fee: max_fee_raw }
} else {
Expand Down Expand Up @@ -292,14 +285,8 @@ pub async fn deploy(
};

let account_deployment = account_deployment.max_fee(max_fee.max_fee());
let txn_config = TxnConfig {
fee_estimate_multiplier,
wait,
receipt,
max_fee_raw,
walnut,
timeout_ms,
};
let txn_config =
TxnConfig { fee_estimate_multiplier, wait, receipt, max_fee_raw, walnut };
do_account_deploy(
max_fee,
txn_config,
Expand Down

0 comments on commit 4faa650

Please sign in to comment.