-
Notifications
You must be signed in to change notification settings - Fork 2k
feat: eliminate sender flag if account #10647
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -233,6 +233,16 @@ impl ScriptArgs { | |||||||||||||||||
evm_opts.sender = sender; | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
// If no sender derived yet, try to derive from account | ||||||||||||||||||
if evm_opts.sender == Address::ZERO { | ||||||||||||||||||
if let Some(sender) = self.maybe_derive_sender_from_account()? { | ||||||||||||||||||
evm_opts.sender = sender; | ||||||||||||||||||
} | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
// Validate that sender matches account if both are provided | ||||||||||||||||||
self.validate_sender_account_match(&evm_opts)?; | ||||||||||||||||||
|
||||||||||||||||||
let script_config = ScriptConfig::new(config, evm_opts).await?; | ||||||||||||||||||
|
||||||||||||||||||
Ok(PreprocessedState { args: self, script_config, script_wallets }) | ||||||||||||||||||
|
@@ -348,6 +358,36 @@ impl ScriptArgs { | |||||||||||||||||
Ok(maybe_sender) | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
/// Attempts to derive the sender address from the configured account | ||||||||||||||||||
fn maybe_derive_sender_from_account(&self) -> Result<Option<Address>> { | ||||||||||||||||||
if self.evm.sender.is_some() { | ||||||||||||||||||
return Ok(None); | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
let maybe_sender = self.wallets.private_keys()?.filter(|pks| pks.len() == 1).map(|pks| { | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hm, don't really understand how this is different from existing foundry/crates/script/src/lib.rs Lines 352 to 359 in 643c7fc
|
||||||||||||||||||
let address = pks.first().unwrap().address(); | ||||||||||||||||||
trace!(target: "script", "Derived sender {} from private key", address); | ||||||||||||||||||
address | ||||||||||||||||||
}); | ||||||||||||||||||
Ok(maybe_sender) | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
/// Validates that the sender address matches the account when both are provided | ||||||||||||||||||
fn validate_sender_account_match(&self, evm_opts: &EvmOpts) -> Result<()> { | ||||||||||||||||||
if self.evm.sender.is_some() && evm_opts.sender != Address::ZERO { | ||||||||||||||||||
if let Some(private_keys) = self.wallets.private_keys()? { | ||||||||||||||||||
let sender_matches = private_keys.iter().any(|pk| pk.address() == evm_opts.sender); | ||||||||||||||||||
if !sender_matches { | ||||||||||||||||||
eyre::bail!( | ||||||||||||||||||
"Sender address {} does not match any of the provided accounts", | ||||||||||||||||||
evm_opts.sender | ||||||||||||||||||
); | ||||||||||||||||||
} | ||||||||||||||||||
} | ||||||||||||||||||
} | ||||||||||||||||||
Ok(()) | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
/// Returns the Function and calldata based on the signature | ||||||||||||||||||
/// | ||||||||||||||||||
/// If the `sig` is a valid human-readable function we find the corresponding function in the | ||||||||||||||||||
|
@@ -918,4 +958,39 @@ mod tests { | |||||||||||||||||
]); | ||||||||||||||||||
assert!(args.with_gas_price.unwrap().is_zero()); | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
// <https://github.com/foundry-rs/foundry/issues/10632> | ||||||||||||||||||
#[test] | ||||||||||||||||||
fn test_automatic_sender_derivation_from_account() { | ||||||||||||||||||
use alloy_primitives::address; | ||||||||||||||||||
|
||||||||||||||||||
let private_key = "0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80"; | ||||||||||||||||||
|
||||||||||||||||||
let args = | ||||||||||||||||||
ScriptArgs::parse_from(["foundry-cli", "Contract.sol", "--private-key", private_key]); | ||||||||||||||||||
|
||||||||||||||||||
assert_eq!(args.evm.sender, None); | ||||||||||||||||||
|
||||||||||||||||||
let derived_sender = args.maybe_derive_sender_from_account().unwrap(); | ||||||||||||||||||
assert_eq!(derived_sender, Some(address!("0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266"))); | ||||||||||||||||||
} | ||||||||||||||||||
Comment on lines
+963
to
+976
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. shoulnd't those use the --account arg as described in the issue and pr? |
||||||||||||||||||
|
||||||||||||||||||
// <https://github.com/foundry-rs/foundry/issues/10632> | ||||||||||||||||||
#[test] | ||||||||||||||||||
fn test_no_sender_derivation_when_sender_already_provided() { | ||||||||||||||||||
let private_key = "0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80"; | ||||||||||||||||||
let provided_sender = "0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266"; | ||||||||||||||||||
|
||||||||||||||||||
let args = ScriptArgs::parse_from([ | ||||||||||||||||||
"foundry-cli", | ||||||||||||||||||
"Contract.sol", | ||||||||||||||||||
"--private-key", | ||||||||||||||||||
private_key, | ||||||||||||||||||
"--sender", | ||||||||||||||||||
provided_sender, | ||||||||||||||||||
]); | ||||||||||||||||||
|
||||||||||||||||||
let derived_sender = args.maybe_derive_sender_from_account().unwrap(); | ||||||||||||||||||
assert_eq!(derived_sender, None); | ||||||||||||||||||
} | ||||||||||||||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
while this works, I'd prefer if we check if there is a sender value provided in the ScriptArgs instead
or since this is already done tin the maybe_derive_sender_from_account, we can skip the zero check here and use an else if