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

Add a cast rpc method for raw JSON-RPC reqs #2030

Merged
merged 5 commits into from
Jun 23, 2022

Conversation

jpopesculian
Copy link
Contributor

Motivation

Cast doesn't have any way to interact with non-standard RPC endpoints. The main use case here would be something like anvil_mine

Solution

Implement a cast request subcommand to send raw JSON-RPC requests.

cast-request 
Perform a raw JSON-RPC request

USAGE:
    cast request [OPTIONS] <METHOD> [PARAMS]...

ARGS:
    <METHOD>
            RPC method name

    <PARAMS>...
            RPC parameters
            
            Parameters are interpreted as strings. If you wish to pass a JSON value you can prepend
            the value with a ':'. To start a string with a ':' prepend with '::'. For example:
            
            request doThing 123   => {"method": "doThing", "params": ["123"] ... }
            request doThing :123  => {"method": "doThing", "params": [123] ... }
            request doThing ::123 => {"method": "doThing", "params": [":123"] ... }

OPTIONS:
    -h, --help
            Print help information

    -r, --rpc-url <URL>
            [env: ETH_RPC_URL=]

For example running anvil_mine for 100 blocks would be as simple as cast rq anvil_mine :100. Of course, I think there can be some discussion about incorporating the anvil commands directly into cast as a subcommand tree itself.

Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a great command.

a cast integration test for this would be nice

cli/src/opts/cast.rs Outdated Show resolved Hide resolved
@mattsse mattsse added T-feature Type: feature C-cast Command: cast labels Jun 20, 2022
cli/src/cast.rs Outdated
@@ -680,6 +680,25 @@ async fn main() -> eyre::Result<()> {
generate(shell, &mut Opts::command(), "cast", &mut std::io::stdout())
}
Subcommands::Run(cmd) => cmd.run()?,
Subcommands::Request { rpc_url, method, params } => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do want to call this cast rpc instead to match seth rpc?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah nice! haha I was debating which to call it actually but I didn't realize the matching here...

@jpopesculian
Copy link
Contributor Author

Okay @mattsse I fixed it up per our convo and it now looks like this

cast-rpc 
Perform a raw JSON-RPC request

USAGE:
    cast rpc [OPTIONS] <METHOD> [PARAMS]...

ARGS:
    <METHOD>
            RPC method name

    <PARAMS>...
            RPC parameters
            
            Parameters are interpreted as JSON and then fall back to string. For example:
            
            rpc eth_getBlockByNumber 0x123 false
                => {"method": "eth_getBlockByNumber", "params": ["0x123", false] ... }

OPTIONS:
    -d, --direct-params
            Do not put parameters in an array
            
            If --direct-params is passed the first PARAM will be taken as the value of "params". For
            example:
            
            rpc --direct-params eth_getBlockByNumber '["0x123", false]'
                => {"method": "eth_getBlockByNumber", "params": ["0x123", false] ... }

    -h, --help
            Print help information

    -r, --rpc-url <URL>
            [env: ETH_RPC_URL=]

@tynes
Copy link
Contributor

tynes commented Jun 20, 2022

seth uses the command seth rpc for this, would prefer to keep the name 1:1 with seth if possible for backwards compat if possible

#29 (comment)

edit: just noticed this was already called out

@jpopesculian jpopesculian changed the title Add a request method to cast for raw JSON-RPC reqs Add a cast rpc method to cast for raw JSON-RPC reqs Jun 21, 2022
@jpopesculian jpopesculian changed the title Add a cast rpc method to cast for raw JSON-RPC reqs Add a cast rpc method for raw JSON-RPC reqs Jun 21, 2022
Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice, this is great!

a couple of smol nits :)

Comment on lines 828 to 856
Rpc {
#[clap(short, long, env = "ETH_RPC_URL", value_name = "URL")]
rpc_url: Option<String>,
#[clap(
short,
long,
help = "Do not put parameters in an array",
long_help = r#"Do not put parameters in an array

If --direct-params is passed the first PARAM will be taken as the value of "params". For example:

rpc --direct-params eth_getBlockByNumber '["0x123", false]'
=> {"method": "eth_getBlockByNumber", "params": ["0x123", false] ... }"#
)]
direct_params: bool,
#[clap(value_name = "METHOD", help = "RPC method name")]
method: String,
#[clap(
value_name = "PARAMS",
help = "RPC parameters",
long_help = r#"RPC parameters

Parameters are interpreted as JSON and then fall back to string. For example:

rpc eth_getBlockByNumber 0x123 false
=> {"method": "eth_getBlockByNumber", "params": ["0x123", false] ... }"#
)]
params: Vec<String>,
},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we make this a standalone struct please and move to new cmd/cast/rpc.rs, same for the code in cast.rs?

help = "Do not put parameters in an array",
long_help = r#"Do not put parameters in an array

If --direct-params is passed the first PARAM will be taken as the value of "params". For example:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm not sure about the naming yet, maybe --raw would be an alternative?

Also, I would like to change the position in the example and move it after the array so that it is better distinguished from the example of the "params" field.

cli/src/cast.rs Outdated
eyre::bail!(r#"Expected exactly one argument for "params""#);
}
let param = params.into_iter().next().unwrap();
serde_json::from_str(&param).unwrap_or(serde_json::Value::String(param))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: turn into local closure/fn, maybe let to_value = |param:&str| ...;

#[clap(
short,
long,
help = "Do not put parameters in an array",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe something like: Pass the "params" as is

rpc_url: Option<String>,
#[clap(
short,
long,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe we can try adding requires = params here

Fix and add tests as well

Signed-off-by: Julian Popescu <[email protected]>
Params are parsed as a JSON first and fallback to string.
In addition --direct-params was added to handle non-array params

Signed-off-by: Julian Popescu <[email protected]>
And add stdin parsing

Signed-off-by: Julian Popescu <[email protected]>
Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm,

good to merge?

Comment on lines +396 to +399
pub fn stdin(&mut self, fun: impl FnOnce(process::ChildStdin) + 'static) -> &mut TestCommand {
self.stdin_fun = Some(Box::new(fun));
self
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is quite neat!

@gakonst gakonst merged commit d89f6af into foundry-rs:master Jun 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-cast Command: cast T-feature Type: feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants