Skip to content

refactor(cheatcodes): mv ScriptWallets into Cheatcode#9106

Merged
yash-atreya merged 5 commits into
masterfrom
yash/mv-script-wallets
Oct 14, 2024
Merged

refactor(cheatcodes): mv ScriptWallets into Cheatcode#9106
yash-atreya merged 5 commits into
masterfrom
yash/mv-script-wallets

Conversation

@yash-atreya

@yash-atreya yash-atreya commented Oct 14, 2024

Copy link
Copy Markdown
Contributor

Motivation

Closes #9104

Solution

  • Move ScriptWallets into Cheatcode from CheatsConfig.
  • Add wallets and wallets(&self, w: ScriptWallets) -> Self to InspectorStackBuilder. This holds the script wallets till we build the Cheatcodes.
  • Remove CheatsConfig cloning i.e Arc::make_mut(&mut state.config).
  • Rename ScriptWallets to Wallets as this refactor opens up space for adding unlocked wallets in the test environment as well.

@yash-atreya yash-atreya enabled auto-merge (squash) October 14, 2024 10:39

@grandizzy grandizzy left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

lgtm, have one suggestion, pls check

/// Returns the configured script wallets.
pub fn script_wallets(&self) -> Option<&ScriptWallets> {
self.config.script_wallets.as_ref()
pub fn script_wallets(&self) -> Option<&Wallets> {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit, maybe instead script_wallets and set_wallets fns we could have single

pub fn script_wallets(&mut self) -> &Wallets {
    self.wallets.get_or_insert(Wallets::new(MultiWallet::default(), None))
}

then we can simplify inject_wallet to

fn inject_wallet(state: &mut Cheatcodes, wallet: LocalSigner<SigningKey>) -> Address {
    let address = wallet.address();
    state.script_wallets().add_local_signer(wallet);
    address
}

and getWalletsCall to

impl Cheatcode for getWalletsCall {
    fn apply_stateful(&self, ccx: &mut CheatsCtxt) -> Result {
        let signers = ccx.state.script_wallets().signers().unwrap_or_default();
        Ok(signers.abi_encode())
    }
}

wdyt?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

oops didn't notice the auto-merge, @yash-atreya please take a look, I think this is reasonable

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sorry @grandizzy, it was on auto-merge. I'll make a follow up PR

@yash-atreya yash-atreya merged commit 9a813d5 into master Oct 14, 2024
@yash-atreya yash-atreya deleted the yash/mv-script-wallets branch October 14, 2024 12:07
@yash-atreya yash-atreya self-assigned this Oct 14, 2024
rplusq pushed a commit to rplusq/foundry that referenced this pull request Nov 29, 2024
…rs#9106)

* refactor(`cheatcodes`): mv `ScriptWallets` into `Cheatcode` from `CheatsConfig`

* nit

* rename `ScriptWallets` to `Wallets`

* rename cheatcode

* doc nits
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refactor(cheatcodes): move script_wallets into Cheatcode

3 participants