feat: vm.sign for scripts
#7454
Conversation
crates/cheatcodes/src/utils.rs
Outdated
| let r = B256::from(sig.r.to_alloy()); | ||
| let s = B256::from(sig.s.to_alloy()); | ||
|
|
||
| Ok((v, r, s).abi_encode()) |
There was a problem hiding this comment.
can we synchronize the v,r,s encoding?
crates/cheatcodes/src/utils.rs
Outdated
| let recovered = sig.recover(digest.to_ethers()).map_err(|err| fmt_err!("{err}"))?; | ||
| assert_eq!(recovered.to_alloy(), signer); | ||
|
|
||
| let v = U256::from(sig.v); |
There was a problem hiding this comment.
Should v be like above? sig.v().y_parity_byte_non_eip155...
There was a problem hiding this comment.
this sig is still from ethers and we've just returned a sig.v earlier
Looking at sig.v().y_parity_byte_non_eip155().unwrap_or(sig.v().y_parity_byte() I am wondering if this is correct actually
Isn't ecrecover expects v to be 27 or 28? and here we might end up with 0 or 1
crates/cheatcodes/src/utils.rs
Outdated
| signer: Option<Address>, | ||
| digest: &B256, | ||
| ) -> Result { | ||
| if let Some(script_wallets) = &ccx.state.script_wallets { |
There was a problem hiding this comment.
let Some() = ... else { return Err() } or .as_ref().ok_or("")?
| /// | ||
| /// Raises error if none of the signers passed into the script have provided address. | ||
| #[cheatcode(group = Evm, safety = Safe)] | ||
| function sign(address signer, bytes32 digest) external pure returns (uint8 v, bytes32 r, bytes32 s); |
There was a problem hiding this comment.
should we also have sign(string) for EIP-712-prefixed messages? (not for this pr necessarily)
There was a problem hiding this comment.
you mean bindings for Signer::sign_typed_data and Signer::sign_message?
There was a problem hiding this comment.
yes, message seems more doable than typed data
klkvr
force-pushed
the
klkvr/sign-imporovements
branch
from
19f0a39
to
90759d6
Compare
* feat: vm.sign for script wallets * more tests * clippy * if let some else * review fixes
Motivation
Closes #7446
Solution
Adds
vm.sign(bytes32 digest)determining signer in a similar wayvm.broadcast()andvm.startBroadcast()do.Adds
vm.sign(address signer, bytes32 digest)similar tovm.broadcast(address)Also added more docs to explain the way we choose signer for
vm.broadcast()and it seems pretty complicated tbh. Afaik we still don't even have a reliable way to fetch signer address besidesvm.broadcast()->vm.readCallers(), perhaps we should reconsider and document that?Also those cheatcodes are not compatible with tests right now. We can probably allow that by running tests with empty
ScriptWallets. That way tests will be able to inject private keys viavm.rememberKey