-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Description
Component
Forge
Have you ensured that all of these are up to date?
- Foundry
- Foundryup
What version of Foundry are you on?
forge Version: 1.3.1-v1.3.1
What version of Foundryup are you on?
foundryup: 1.0.1
What command(s) is the bug in?
forge script Example --mnemonics (anything that is not --private-key)
Operating System
None
Describe the bug
For context, when using vm.startBroadcast()
, I always assume the msg.sender
is set to the wallet when there is precisely one signer. And this behavior is true and can be observed if forge script Example --private-key 0x0001234
were used to run the script.
But that's not the case if we use other wallet types, e.g. forge script script/TokenDeployment.sol --mnemonics "abandon abandon..." --mnemonic-indexes 1
or --ledger
or etc.
This behavior is potentially dangerous when a deployer assumes --private-key
is a type of wallet identical to any other type of wallet. And the "wallet module" will correctly substitute msg.sender
. E.g., I test a workflow in testnet using --private-key
and switch to mnemonics or ledger and expect the same behavior, but ownership/funds sent to the foundry default sender.
And probably a few have already made this mistake (almost for me), see 10K+ in this wallet https://etherscan.io/address/0x1804c8AB1F12E6bbf3894d4083f33e07309d1f38#asset-multichain
For context, narrative where this behavior is implied:
https://getfoundry.sh/reference/cheatcodes/start-broadcast
Using the address that calls the test contract or the address / private key provided as the sender, has all subsequent calls (at this call depth only and excluding cheatcode calls) create transactions that can later be signed and sent onchain.
/// Has all subsequent calls (at this call depth only) create transactions that can later be signed and sent onchain. /// Broadcasting address is determined by checking the following in order: /// 1. If `--sender` argument was provided, that address is used. /// 2. If exactly one signer (e.g. private key, hw wallet, keystore) is set when `forge broadcast` is invoked, that signer is used. /// 3. Otherwise, default foundry sender (1804c8AB1F12E6bbf3894d4083f33e07309d1f38) is used. function startBroadcast() external;
As you can see in the code, maybe_load_private_key
is where this msg.sender
is replaced/set when --private-key
arg was used.
foundry/crates/script/src/lib.rs
Lines 235 to 237 in 780dcf9
if let Some(sender) = self.maybe_load_private_key()? { | |
evm_opts.sender = sender; | |
} |
I expected the behavior of msg.sender
to be replaced/set in this part of the code:
foundry/crates/cheatcodes/src/script.rs
Lines 354 to 375 in 710a158
fn broadcast(ccx: &mut CheatsCtxt, new_origin: Option<&Address>, single_call: bool) -> Result { | |
let depth = ccx.ecx.journaled_state.depth(); | |
ensure!( | |
ccx.state.get_prank(depth).is_none(), | |
"you have an active prank; broadcasting and pranks are not compatible" | |
); | |
ensure!(ccx.state.broadcast.is_none(), "a broadcast is active already"); | |
let mut new_origin = new_origin.copied(); | |
if new_origin.is_none() { | |
let mut wallets = ccx.state.wallets().inner.lock(); | |
if let Some(provided_sender) = wallets.provided_sender { | |
new_origin = Some(provided_sender); | |
} else { | |
let signers = wallets.multi_wallet.signers()?; | |
if signers.len() == 1 { | |
let address = signers.keys().next().unwrap(); | |
new_origin = Some(*address); | |
} | |
} | |
} |
A note on potentially dangerous breaking changes (if any changes were to be made to fix this issue):
fn preprocess() and fn maybe_load_private_key()
running earlier in the script lifecycle (before vm.startBroadcast()
). We cannot move this behavior down to a later lifecycle stage as developers will already assume msg.sender
is set to the private key they passed in—moving it down will have devastating consequences. Any changes to this behavior must be additional without breaking the above assumption. (I can further elaborate if needed)
Metadata
Metadata
Assignees
Labels
Type
Projects
Status