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: cast mktx #7056

Merged
merged 4 commits into from
Mar 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
109 changes: 109 additions & 0 deletions crates/cast/bin/cmd/mktx.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
use crate::tx;
use clap::Parser;
use ethers_core::types::NameOrAddress;
use ethers_middleware::MiddlewareBuilder;
use ethers_providers::Middleware;
use ethers_signers::Signer;
use eyre::Result;
use foundry_cli::{
opts::{EthereumOpts, TransactionOpts},
utils,
};
use foundry_common::types::ToAlloy;
use foundry_config::Config;
use std::str::FromStr;

/// CLI arguments for `cast mktx`.
#[derive(Debug, Parser)]
pub struct MakeTxArgs {
/// The destination of the transaction.
///
/// If not provided, you must use `cast mktx --create`.
#[arg(value_parser = NameOrAddress::from_str)]
to: Option<NameOrAddress>,

/// The signature of the function to call.
sig: Option<String>,

/// The arguments of the function to call.
args: Vec<String>,

/// Reuse the latest nonce for the sender account.
#[arg(long, conflicts_with = "nonce")]
resend: bool,

#[command(subcommand)]
command: Option<MakeTxSubcommands>,

#[command(flatten)]
tx: TransactionOpts,

#[command(flatten)]
eth: EthereumOpts,
}

#[derive(Debug, Parser)]
pub enum MakeTxSubcommands {
/// Use to deploy raw contract bytecode.
#[clap(name = "--create")]
Create {
/// The initialization bytecode of the contract to deploy.
code: String,

/// The signature of the constructor.
sig: Option<String>,

/// The constructor arguments.
args: Vec<String>,
},
}

impl MakeTxArgs {
pub async fn run(self) -> Result<()> {
let MakeTxArgs { to, mut sig, mut args, resend, command, mut tx, eth } = self;

let code = if let Some(MakeTxSubcommands::Create {
code,
sig: constructor_sig,
args: constructor_args,
}) = command
{
sig = constructor_sig;
args = constructor_args;
Some(code)
} else {
None
};

tx::validate_to_address(&code, &to)?;

let config = Config::from(&eth);
let provider = utils::get_provider(&config)?;
let chain = utils::get_chain(config.chain, &provider).await?;
let api_key = config.get_etherscan_api_key(Some(chain));

// Retrieve the signer, and bail if it can't be constructed.
let signer = eth.wallet.signer().await?;
let from = signer.address();

tx::validate_from_address(eth.wallet.from, from.to_alloy())?;

if resend {
tx.nonce = Some(provider.get_transaction_count(from, None).await?.to_alloy());
}

let provider = provider.with_signer(signer);

let (mut tx, _) =
tx::build_tx(&provider, from, to, code, sig, args, tx, chain, api_key).await?;

// Fill nonce, gas limit, gas price, and max priority fee per gas if needed
provider.fill_transaction(&mut tx, None).await?;

let signature = provider.sign_transaction(&tx, from).await?;
let signed_tx = tx.rlp_signed(&signature);
Comment on lines +103 to +104
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since we're actually signing the transaction, this probably should be behind cast wallet similar to the existing cast wallet sign. What do you think about also renaming this to cast wallet sign-tx to make it more explicit? I don't think cast needs to be backwards compatible with seth anymore.

I could imagine a separate cast mktx command that returns a populated (optionally serialized) transaction, without signing it

Copy link
Contributor Author

@ay ay Feb 9, 2024

Choose a reason for hiding this comment

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

I went back and forth a few times on this and initially implemented this as cast wallet sign-tx, but ultimately decided cast mktx is better. The cast wallet commands don't use TxBuilder, don't use an RPC connection, and therefore have no way of filling a transaction with gas info, nonce, ENS resolution, etc. Having it as cast mktx allows it to be as close to cast send as possible, with the difference being it prints the encoded transaction instead of broadcasting it. You get everything provided by TxBuilder if you need it, but can still specify all the options manually if you have no connection to an RPC endpoint.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I suppose it can be renamed to something like cast sign-tx, but since the behavior is identical to seth mktx (which builds and signs transactions) I went with cast mktx.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think a good general principle is to implement the UX that's best for the user, even if it's more work because cast wallet commands don't yet have TxBuilder or RPC connections. Most foundry users haven't used (or heard of) dapptools and seth is no longer maintained, so we should use the best naming/UX regardless of what seth did.

However, IMO the cast UX is already messy and needs breaking change cleanup, so I won't block this PR on trying to keep cast UX clean or changing command names. Just sharing my thoughts, and will defer to you and other foundry maintainers from here :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I agree. I wasn't sure what the best place for this was either. Because of how close its logic is to cast send, I put it adjacent to where that is. I can rename/move it wherever (though moving it under cast wallet will require a few more changes like importing TxBuilder and the utility functions in common with cast send).

println!("{signed_tx}");

Ok(())
}
}
1 change: 1 addition & 0 deletions crates/cast/bin/cmd/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ pub mod estimate;
pub mod find_block;
pub mod interface;
pub mod logs;
pub mod mktx;
pub mod rpc;
pub mod run;
pub mod send;
Expand Down
62 changes: 14 additions & 48 deletions crates/cast/bin/cmd/send.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use cast::{Cast, TxBuilder};
use crate::tx;
use cast::Cast;
use clap::Parser;
use ethers_core::types::NameOrAddress;
use ethers_middleware::SignerMiddleware;
Expand Down Expand Up @@ -82,7 +83,7 @@ impl SendTxArgs {
let SendTxArgs {
eth,
to,
sig,
mut sig,
cast_async,
mut args,
mut tx,
Expand All @@ -93,24 +94,20 @@ impl SendTxArgs {
unlocked,
} = self;

let mut sig = sig.unwrap_or_default();
let code = if let Some(SendTxSubcommands::Create {
code,
sig: constructor_sig,
args: constructor_args,
}) = command
{
sig = constructor_sig.unwrap_or_default();
sig = constructor_sig;
args = constructor_args;
Some(code)
} else {
None
};

// ensure mandatory fields are provided
if code.is_none() && to.is_none() {
eyre::bail!("Must specify a recipient address or contract code to deploy");
}
tx::validate_to_address(&code, &to)?;

let config = Config::from(&eth);
let provider = utils::get_provider(&config)?;
Expand Down Expand Up @@ -155,7 +152,8 @@ impl SendTxArgs {
config.sender.to_ethers(),
to,
code,
(sig, args),
sig,
args,
tx,
chain,
api_key,
Expand All @@ -173,19 +171,7 @@ impl SendTxArgs {
let signer = eth.wallet.signer().await?;
let from = signer.address();

// prevent misconfigured hwlib from sending a transaction that defies
// user-specified --from
if let Some(specified_from) = eth.wallet.from {
if specified_from != from.to_alloy() {
eyre::bail!(
"\
The specified sender via CLI/env vars does not match the sender configured via
the hardware wallet's HD Path.
Please use the `--hd-path <PATH>` parameter to specify the BIP32 Path which
corresponds to the sender, or let foundry automatically detect it by not specifying any sender address."
)
}
}
tx::validate_from_address(eth.wallet.from, from.to_alloy())?;

if resend {
tx.nonce = Some(provider.get_transaction_count(from, None).await?.to_alloy());
Expand All @@ -198,7 +184,8 @@ corresponds to the sender, or let foundry automatically detect it by not specify
from,
to,
code,
(sig, args),
sig,
args,
tx,
chain,
api_key,
Expand All @@ -217,7 +204,8 @@ async fn cast_send<M: Middleware, F: Into<NameOrAddress>, T: Into<NameOrAddress>
from: F,
to: Option<T>,
code: Option<String>,
args: (String, Vec<String>),
sig: Option<String>,
args: Vec<String>,
tx: TransactionOpts,
chain: Chain,
etherscan_api_key: Option<String>,
Expand All @@ -228,30 +216,8 @@ async fn cast_send<M: Middleware, F: Into<NameOrAddress>, T: Into<NameOrAddress>
where
M::Error: 'static,
{
let (sig, params) = args;
let params = if !sig.is_empty() { Some((&sig[..], params)) } else { None };
let mut builder = TxBuilder::new(&provider, from, to, chain, tx.legacy).await?;
builder
.etherscan_api_key(etherscan_api_key)
.gas(tx.gas_limit)
.gas_price(tx.gas_price)
.priority_gas_price(tx.priority_gas_price)
.value(tx.value)
.nonce(tx.nonce);

if let Some(code) = code {
let mut data = hex::decode(code)?;

if let Some((sig, args)) = params {
let (mut sigdata, _) = builder.create_args(sig, args).await?;
data.append(&mut sigdata);
}

builder.set_data(data);
} else {
builder.args(params).await?;
};
let builder_output = builder.build();
let builder_output =
tx::build_tx(&provider, from, to, code, sig, args, tx, chain, etherscan_api_key).await?;

let cast = Cast::new(provider);

Expand Down
2 changes: 2 additions & 0 deletions crates/cast/bin/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ use std::time::Instant;

pub mod cmd;
pub mod opts;
pub mod tx;

use opts::{Cast as Opts, CastSubcommand, ToBaseArgs};

Expand Down Expand Up @@ -366,6 +367,7 @@ async fn main() -> Result<()> {
// Calls & transactions
CastSubcommand::Call(cmd) => cmd.run().await?,
CastSubcommand::Estimate(cmd) => cmd.run().await?,
CastSubcommand::MakeTx(cmd) => cmd.run().await?,
CastSubcommand::PublishTx { raw_tx, cast_async, rpc } => {
let config = Config::from(&rpc);
let provider = utils::get_provider(&config)?;
Expand Down
7 changes: 6 additions & 1 deletion crates/cast/bin/opts.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
use crate::cmd::{
access_list::AccessListArgs, bind::BindArgs, call::CallArgs, create2::Create2Args,
estimate::EstimateArgs, find_block::FindBlockArgs, interface::InterfaceArgs, logs::LogsArgs,
rpc::RpcArgs, run::RunArgs, send::SendTxArgs, storage::StorageArgs, wallet::WalletSubcommands,
mktx::MakeTxArgs, rpc::RpcArgs, run::RunArgs, send::SendTxArgs, storage::StorageArgs,
wallet::WalletSubcommands,
};
use alloy_primitives::{Address, B256, U256};
use clap::{Parser, Subcommand, ValueHint};
Expand Down Expand Up @@ -381,6 +382,10 @@ pub enum CastSubcommand {
bytecode: String,
},

/// Build and sign a transaction.
#[command(name = "mktx", visible_alias = "m")]
MakeTx(MakeTxArgs),

/// Calculate the ENS namehash of a name.
#[command(visible_aliases = &["na", "nh"])]
Namehash { name: Option<String> },
Expand Down
73 changes: 73 additions & 0 deletions crates/cast/bin/tx.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
use alloy_primitives::Address;
use cast::{TxBuilder, TxBuilderOutput};
use ethers_core::types::NameOrAddress;
use ethers_providers::Middleware;
use eyre::Result;
use foundry_cli::opts::TransactionOpts;
use foundry_config::Chain;

/// Prevents a misconfigured hwlib from sending a transaction that defies user-specified --from
pub fn validate_from_address(
specified_from: Option<Address>,
signer_address: Address,
) -> Result<()> {
if let Some(specified_from) = specified_from {
if specified_from != signer_address {
eyre::bail!(
"\
The specified sender via CLI/env vars does not match the sender configured via
the hardware wallet's HD Path.
Please use the `--hd-path <PATH>` parameter to specify the BIP32 Path which
corresponds to the sender, or let foundry automatically detect it by not specifying any sender address."
)
}
}
Ok(())
}

/// Ensures the transaction is either a contract deployment or a recipient address is specified
pub fn validate_to_address(code: &Option<String>, to: &Option<NameOrAddress>) -> Result<()> {
if code.is_none() && to.is_none() {
eyre::bail!("Must specify a recipient address or contract code to deploy");
}
Ok(())
}

#[allow(clippy::too_many_arguments)]
pub async fn build_tx<M: Middleware, F: Into<NameOrAddress>, T: Into<NameOrAddress>>(
provider: &M,
from: F,
to: Option<T>,
code: Option<String>,
sig: Option<String>,
args: Vec<String>,
tx: TransactionOpts,
chain: impl Into<Chain>,
etherscan_api_key: Option<String>,
) -> Result<TxBuilderOutput> {
let mut builder = TxBuilder::new(provider, from, to, chain, tx.legacy).await?;
builder
.etherscan_api_key(etherscan_api_key)
.gas(tx.gas_limit)
.gas_price(tx.gas_price)
.priority_gas_price(tx.priority_gas_price)
.value(tx.value)
.nonce(tx.nonce);

let params = sig.as_deref().map(|sig| (sig, args));
if let Some(code) = code {
let mut data = hex::decode(code)?;

if let Some((sig, args)) = params {
let (mut sigdata, _) = builder.create_args(sig, args).await?;
data.append(&mut sigdata);
}

builder.set_data(data);
} else {
builder.args(params).await?;
}

let builder_output = builder.build();
Ok(builder_output)
}
4 changes: 2 additions & 2 deletions crates/cast/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ use std::{
sync::atomic::{AtomicBool, Ordering},
};
use tokio::signal::ctrl_c;
use tx::{TxBuilderOutput, TxBuilderPeekOutput};
use tx::TxBuilderPeekOutput;

use foundry_common::abi::encode_function_args_packed;
pub use foundry_evm::*;
Expand All @@ -43,7 +43,7 @@ pub use rusoto_core::{
request::HttpClient as AwsHttpClient, Client as AwsClient,
};
pub use rusoto_kms::KmsClient;
pub use tx::TxBuilder;
pub use tx::{TxBuilder, TxBuilderOutput};

pub mod base;
pub mod errors;
Expand Down
Loading
Loading