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.

Sign up for GitHub

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

Jump to bottom

refactor(cheatcodes): move script_wallets into Cheatcode #9104

Closed
yash-atreya opened this issue Oct 14, 2024 · 1 comment · Fixed by #9106
Closed

refactor(cheatcodes): move script_wallets into Cheatcode #9104

yash-atreya opened this issue Oct 14, 2024 · 1 comment · Fixed by #9106
Assignees
@yash-atreya
Labels
A-cheatcodes Area: cheatcodes Cmd-forge-script Command: forge script Cmd-forge-test Command: forge test T-feature Type: feature
Milestone
v1.0.0

Comments

@yash-atreya
Copy link
Member

yash-atreya commented Oct 14, 2024 •
edited
Loading 

          lgtm, note on performance, would like us to revisit script wallets handling if we want to support scripting cheats in tests as well

Originally posted by @klkvr in #9087 (review)

#9087 intoduces cheatcodes that lets users add unlocked wallets to the forge environment not only in scripts but also in tests.

This works well for scripts but can lead to a performance lag when used in tests as we may clone the cheatcodes_config via Arc::make_mut(&mut state.config) to modify script_wallets

Possible solution: Move script_wallets into Cheatcodes and rename it to wallets. Add new API method wallets(&self, wallets) to the InspectorStackBuilder interface. Use that while initializing cheatcodes for tests and scripts.
@github-project-automation github-project-automation bot moved this to Todo in Foundry Oct 14, 2024
@yash-atreya yash-atreya self-assigned this Oct 14, 2024
@yash-atreya yash-atreya added Cmd-forge-test Command: forge test A-cheatcodes Area: cheatcodes Cmd-forge-script Command: forge script T-feature Type: feature labels Oct 14, 2024
@yash-atreya yash-atreya changed the title perf(cheatcodes): move script_wallets into Cheatcode feature(cheatcodes): move script_wallets into Cheatcode Oct 14, 2024
@yash-atreya yash-atreya changed the title feature(cheatcodes): move script_wallets into Cheatcode feat(cheatcodes): move script_wallets into Cheatcode Oct 14, 2024
@yash-atreya
Copy link
Member Author

@klkvr wdyt about this approach?

@yash-atreya yash-atreya added this to the v1.0.0 milestone Oct 14, 2024
@yash-atreya yash-atreya mentioned this issue Oct 14, 2024
Merged
4 tasks
@yash-atreya yash-atreya moved this from Todo to In Progress in Foundry Oct 14, 2024
@yash-atreya yash-atreya moved this from In Progress to Ready For Review in Foundry Oct 14, 2024
@yash-atreya yash-atreya changed the title feat(cheatcodes): move script_wallets into Cheatcode refactor(cheatcodes): move script_wallets into Cheatcode Oct 14, 2024
@github-project-automation github-project-automation bot moved this from Ready For Review to Done in Foundry Oct 14, 2024
@grandizzy grandizzy moved this from Done to Completed in Foundry Oct 14, 2024
@yash-atreya yash-atreya moved this from Completed to Done in Foundry Oct 14, 2024
@yash-atreya yash-atreya removed the status in Foundry Oct 21, 2024
@yash-atreya yash-atreya moved this to Completed in Foundry Oct 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@yash-atreya yash-atreya

Labels
A-cheatcodes Area: cheatcodes Cmd-forge-script Command: forge script Cmd-forge-test Command: forge test T-feature Type: feature
Projects
Foundry
Status: Completed
Milestone
v1.0.0
Development

Successfully merging a pull request may close this issue.

refactor(cheatcodes): mv ScriptWallets into Cheatcode foundry-rs/foundry
1 participant
@yash-atreya