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.

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

refactor: remove dependence on env vars and rely on forge injected entities #17

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ramenforbreakfast
Copy link

@ramenforbreakfast ramenforbreakfast commented May 1, 2024

Foundry has since been updated to the point where relying on environment variables to inject accounts is no longer necessary.

BatchScript.sol is a forge script and thus already has a signer and chainId configured when the script is run. There should be no need to rely on environment variables to duplicate what is already natively injected by forge.

In addition, vm.sign has been recently updated to allow for signing of messages directly via whatever wallet has been selected via forge script options, thus there is no need for complex string logic to assemble an external cast call anymore. forge already allows usage of plaintext keys, keystores, and hardware wallets so this shouldn't result in any loss of functionality.

Haven't tested it with hardware wallets though, not sure if vm.sign can correctly identify a --sender with a ledger/trezor provided via options.

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.

1 participant