Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(config): configure RPC timeout for forge script #9303

Open
kevinhalliday opened this issue Nov 12, 2024 · 7 comments
Open

feat(config): configure RPC timeout for forge script #9303

kevinhalliday opened this issue Nov 12, 2024 · 7 comments
Labels
A-config Area: config C-forge Command: forge Cmd-forge-script Command: forge script good first issue Good for newcomers T-feature Type: feature

Comments

@kevinhalliday
Copy link

Component

Forge

Describe the feature you would like

Forge script rpc requests timeout after 45 seconds. You can increase timeout for waiting for tx receipts, but you cannot increase timeout for eth_sendTransaction requests. The feature would allow using MPC signers (like fireblocks) w/ unlocked forge scripts.

Additional context

This is a "reopening" of #8667.

Describe the feature you would like
Currently forge script rpc requests timeout after 45 seconds. I believe this is not configurable (let me know if I'm wrong). Being able to specify timeout would be 🤌

Additional context
We are running forge scripts --unlocked against a proxy rpc that intercepts eth_sendTransaction requests and signs them with fireblocks. The MPC signing takes time, often longer than 45seconds. The scripts timeout pretty consistently.

The fix (#8669) only increased timeout for waiting for tx receipts. But eth_sendTransaction requests still use constant 45 second timeout.

@kevinhalliday kevinhalliday added T-feature Type: feature T-needs-triage Type: this issue needs to be labelled labels Nov 12, 2024
@github-project-automation github-project-automation bot moved this to Todo in Foundry Nov 12, 2024
@grandizzy
Copy link
Collaborator

I think we could reuse the existing eth_rpc_timeout config when creating script providers, @zerosnacks @klkvr would this work or could be confusing?

@klkvr
Copy link
Member

klkvr commented Nov 13, 2024

yep, we should make sure to add it to ScriptArgs

also right now scripts are using different code path than cast for creating providers, need to make it aware of timeout

let provider = Arc::new(try_get_http_provider(sequence.rpc_url())?);

@zerosnacks zerosnacks added C-forge Command: forge Cmd-forge-script Command: forge script A-config Area: config good first issue Good for newcomers and removed T-needs-triage Type: this issue needs to be labelled labels Nov 13, 2024
@grandizzy
Copy link
Collaborator

👍 , or maybe better an --rpc-timeout in EvmArgs along with --rpc-url ?

pub struct EvmArgs {
/// Fetch state over a remote endpoint instead of starting from an empty state.
///
/// If you want to fetch state from a specific block number, see --fork-block-number.
#[arg(long, short, visible_alias = "rpc-url", value_name = "URL")]
#[serde(rename = "eth_rpc_url", skip_serializing_if = "Option::is_none")]
pub fork_url: Option<String>,

@zerosnacks
Copy link
Member

We have an rpc_timeout in RpcOpts here

/// Timeout for the RPC request in seconds.
///
/// The specified timeout will be used to override the default timeout for RPC requests.
///
/// Default value: 45
#[arg(long, env = "ETH_RPC_TIMEOUT")]
pub rpc_timeout: Option<u64>,

@klkvr
Copy link
Member

klkvr commented Nov 13, 2024

yeah that's a bit weird because we can't use both EvmArgs and RpcOpts because of --rpc-url conflict, might make sense to try unifying those

@10kumaranurag01
Copy link

I'd love to give this a go. Please assign this to me

@mimisavage
Copy link

Could I be assigned to this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-config Area: config C-forge Command: forge Cmd-forge-script Command: forge script good first issue Good for newcomers T-feature Type: feature
Projects
Archived in project
Development

No branches or pull requests

6 participants