-
Notifications
You must be signed in to change notification settings - Fork 1
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
feat(forge): allow --verifier custom
option (#9311)
#6
base: master
Are you sure you want to change the base?
Conversation
* feat(forge): allow `--verifier custom` option * Changes after review: add description of custom verifier, reorg err message, add custom verifier api key * Fix descriptions * Update crates/verify/src/provider.rs Co-authored-by: zerosnacks <95942363+zerosnacks@users.noreply.github.com> --------- Co-authored-by: zerosnacks <95942363+zerosnacks@users.noreply.github.com>
Reviewer's Guide by SourceryThis PR adds support for custom verification providers by introducing a new No diagrams generated as the changes look simple and do not need a visual representation. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @Dargon789 - I've reviewed your changes - here's some feedback:
Overall Comments:
- Please add tests for the custom verifier functionality to ensure it works as expected
- The PR description is empty - please fill out the motivation and solution sections to help reviewers understand the context and purpose of these changes
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
* begin api and rough comments * impl cheatcode * add check for eoa * fix eoa check on each prank call * add to assets * prank compiling * delegate call working, storage not upating * delegate call working, some tidy up * add prank2 calls * impl remaining tests * formatting * forge fmt * add pranks to cheatcodes.json * run cargo cheats * If verbosity level is 1 or higher, it shows dirty files. * Fix, add EOA prank test * Revert "If verbosity level is 1 or higher, it shows dirty files." This reverts commit d03ac1d. * Fix test * apply on extdelegatecall --------- Co-authored-by: mgiagante <251503-mgiagante@users.noreply.gitlab.com> Co-authored-by: grandizzy <grandizzy.the.egg@gmail.com>
Head branch was pushed to by a user without write access
…9321) * add --sizes and --names JSON compatibility + generalize report kind * add additional json output tests * fix feedback nit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
feat(forge): allow --verifier custom option (foundry-rs#9311) #6
* test: enhance tests * update ws url * Assert json unordered * Update crates/test-utils/src/util.rs Co-authored-by: DaniPopes <57450786+DaniPopes@users.noreply.github.com> * Changes after review * Fix rpc url test --------- Co-authored-by: grandizzy <grandizzy.the.egg@gmail.com> Co-authored-by: grandizzy <38490174+grandizzy@users.noreply.github.com> Co-authored-by: DaniPopes <57450786+DaniPopes@users.noreply.github.com>
Locking 40 packages to latest compatible versions Updating allocator-api2 v0.2.19 -> v0.2.20 Updating alloy-dyn-abi v0.8.11 -> v0.8.12 Updating alloy-json-abi v0.8.11 -> v0.8.12 Updating alloy-primitives v0.8.11 -> v0.8.12 Updating alloy-sol-macro v0.8.11 -> v0.8.12 Updating alloy-sol-macro-expander v0.8.11 -> v0.8.12 Updating alloy-sol-macro-input v0.8.11 -> v0.8.12 Updating alloy-sol-type-parser v0.8.11 -> v0.8.12 Updating alloy-sol-types v0.8.11 -> v0.8.12 Updating aws-sdk-sts v1.49.0 -> v1.50.0 Updating axum v0.7.7 -> v0.7.9 Updating bstr v1.10.0 -> v1.11.0 Updating cc v1.1.37 -> v1.2.1 Updating clap v4.5.20 -> v4.5.21 Updating clap_builder v4.5.20 -> v4.5.21 Updating clap_complete v4.5.37 -> v4.5.38 Updating clap_lex v0.7.2 -> v0.7.3 Updating comfy-table v7.1.1 -> v7.1.3 Updating cpufeatures v0.2.14 -> v0.2.15 Removing crossterm v0.27.0 Adding diff v0.1.13 Updating flate2 v1.0.34 -> v1.0.35 Updating indicatif v0.17.8 -> v0.17.9 Adding indoc v2.0.5 Updating instability v0.3.2 -> v0.3.3 Removing instant v0.1.13 Updating libc v0.2.162 -> v0.2.164 Adding pretty_assertions v1.4.1 Updating quinn v0.11.5 -> v0.11.6 Updating quinn-proto v0.11.8 -> v0.11.9 Updating regex-automata v0.4.8 -> v0.4.9 Updating rustix v0.38.39 -> v0.38.40 Updating rustls v0.23.16 -> v0.23.17 Updating scc v2.2.4 -> v2.2.5 Updating serde v1.0.214 -> v1.0.215 Updating serde_derive v1.0.214 -> v1.0.215 Updating syn-solidity v0.8.11 -> v0.8.12 Removing thiserror v1.0.68 Adding thiserror v1.0.69 (available: v2.0.3) Adding thiserror v2.0.3 Removing thiserror-impl v1.0.68 Adding thiserror-impl v1.0.69 Adding thiserror-impl v2.0.3 Adding web-time v1.1.0 note: pass `--verbose` to see 44 unchanged dependencies behind latest Co-authored-by: mattsse <19890894+mattsse@users.noreply.github.com>
Head branch was pushed to by a user without write access
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
update cargo.lock
Head branch was pushed to by a user without write access
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
feat(forge): allow --verifier custom option (foundry-rs#9311) #6
test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test again
* feat(cast storage): allow ugly printing of layout Prior to this change, `cast storage $ADDRESS --rpc-url $RPC_URL --etherscan-api-key $ETHERSCAN_API_KEY` always provided a prettified output. This change adds a `--pretty` flag to `cast storage` which defaults to `true` thus retaining backwards compatibility. Passing `--pretty=false` to `cast storage` results in the json output of the storage layout being produced instead. * fix: remove default value from help text The default value is accessible via `cast storage --help` * fix(cast storage): provide output json path * test(cast): add storage_layout_simple_json test * fix(cast storage): use `--json` flag to ugly print * fix(cast storage): include values in json mode * fix(cast-storage): quiet compilation in all cases * chore: cargo clippy * use fixtures, assert JSON * only quiet if JSON mode, avoid unnecessary warning (if you pass an API key you already expect to fetch remote, very likely default) --------- Co-authored-by: zerosnacks <95942363+zerosnacks@users.noreply.github.com> Co-authored-by: zerosnacks <zerosnacks@protonmail.com>
Head branch was pushed to by a user without write access
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
crates file test
…les to compile (#9325) * If verbosity level is 1 or higher, it shows dirty files. * Adds verbose message variant for compilation. * Removing `if..else` statement to always display `self.send_msg`. * Changes order of messages. * Removes semicolons and adds comment on message order. * Removes verbose variant in favor of the already existing variant. * nits, sort the dirty files list and prefix with - * Raises verbosity level to 5+ * Update crates/common/src/term.rs Co-authored-by: DaniPopes <57450786+DaniPopes@users.noreply.github.com> --------- Co-authored-by: mgiagante <251503-mgiagante@users.noreply.gitlab.com> Co-authored-by: zerosnacks <zerosnacks@protonmail.com> Co-authored-by: DaniPopes <57450786+DaniPopes@users.noreply.github.com>
* chore: move archive endpoints to different provider * Make archive endpoints configurable in env vars * Truncate fork url in err * Include only provider in failed fork message * Add env vars from secrets * Fix tests --------- Co-authored-by: Matthias Seitz <matthias.seitz@outlook.de>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
custom option test
Fix typographical error in default value assignment for FOUNDRY_DIR
Co-authored-by: Matthias Seitz <matthias.seitz@outlook.de>
inherit secrets, use alchemy as default for external PRs, comment out infura
Co-authored-by: grandizzy <grandizzy.the.egg@gmail.com>
* add EIP-7702 cheatcodes: createDelegation, signDelegation, attachDelegation * add cheatcode implementations for EIP-7702: createDelegationCall, signDelegationCall, attachDelegationCall; modify broadcast to check if sender has a delegation * add delegations hashmap to Cheatcodes struct * add revm crate * create AttachDelegationTest for EIP-7702 transactions * regen cheatcodes.json * cargo fmt * move broadcast under attachDelegation * combine createDelegationCall logic with signDelegationCall in order to create and sign delegation in a single call; remove delegation logic from broadcast() - no need to track delegations here * remove revm import from workspace * combine createDelegation logic inton signDelegation for simplicity * remove revm from forge script deps * combine createDelegation with signDelegation * WIP - refactor test to use SimpleDelegateContract and ERC20 - test currently failing bc 7702 implementation.execute not executed as Alice EOA * add logic to include authorization_list for EIP 7702 in TransactionRequest by searching delegations hash map by active_delegation * add address authority param to attachDelegation; remove nonce param from signDelegation, as it can be constructed in cheatcode. * remove 7702 tx request construction logic - now handled in attachDelegation cheatcode implementation * refactor attachDelegation cheatcode implementation to handle verifying signature and setting bytecode on EOA; refactor signDelegation cheatcode implementation to get nonce from signer * remove nonce param from attachDelegation cheatcode in favor of loading from authority account * refactor test to check for code on alice account and call execute on alice account through SimpleDelegateContract * revert refactor on TransactionRequest * format * cargo fmt * fix clippy errors * remove faulty logic comparing nonce to itself - nonce still checked by recovered signature * add more tests to cover revert cases on attachDelegation and multiple calls via delegation contract * cargo fmt * restore logic to check if there's an active delegation when building TransactionRequest; add fixed values for gas and max_priority_fee_per_gas to ensure tx success, with TODO comment to explain what's left * remove obsolete comment * add comments explaining delegations and active_delegation * cargo fmt * add logic to increase gas limit by PER_EMPTY_ACCOUNT_COST(25k) if tx includes authorization list for EIP 7702 tx, which is seemingly not accounted for in gas estimation; remove hardcoded gas values from call_with_executor * revert logic to add PER_EMPTY_ACCOUNT_COST for EIP 7702 txs - handled inside of revm now * remove manually setting transaction type to 4 if auth list is present - handled in revm * add method set_delegation to Executor for setting EIP-7702 authorization list in the transaction environment; call set_delegation from simulate_and_fill if auth list is not empty * remove redundancy with TransactionMaybeSigned var tx * cargo fmt * refactor: use authorization_list() helper to return authorization_list and set delegation * refactor: change Cheatcodes::active_delegation to Option<SignedAuthorization> and remove delegations hashmap - tx will only use one active delegation at a time, so no need for mapping * replace verbose logic to set bytecode on EOA with journaled_state.set_code helper * cargo fmt * increment nonce of authority account * add logic to set authorization_list to None if active_delegation is None * add test testSwitchDelegation to assert that attaching an additional delegation switches the implementation on the EOA * remove set_delegation logic in favor of adding call_raw_with_authorization - previous approach kept the delegation in the TxEnv, resulting in higher gas cost for all subsequent calls after the delegation was applied * refactor signDelegation to return struct SignedDelegation and for attachDelegation to accept SignedDelegation * update delegation tests to reflect change in cheatcode interface for signDelegation and attachDelegation * add cheatcode signAndAttachDelegation * add signAndAttachDelegationCall cheatcode logic; refactor helper methods for shared logic used in 7702 delegation cheatcodes * add test testCallSingleSignAndAttachDelegation for new cheatcode signAndAttachDelegation * add comments to SignedDelegation struct and cargo fmt * cargo fmt * fix ci * fix spec --------- Co-authored-by: Arsenii Kulikov <klkvrr@gmail.com> Co-authored-by: grandizzy <38490174+grandizzy@users.noreply.github.com>
* rename ShellOpts to GlobalOpts * prefer arg / command over clap * add global opts * remove redundant GlobalOpts injection, only use where access to the global variables is required * add global thread pool * add try_jobs method for global rayon pool * limit unnecessary globalopts injection where shell::* is preferred * fix tests * port custom threads iterator to use global rayon thread pool * remove redundant ignores * remove leftover from merge conflict, fix clashing args with inlined global in nodeargs / anvil top level args * leftovers * add back global args in script args * fix unused global opts * ignore attempted multiple initializations of the global thread pool * add init, default spawn with default rayon settings on forge test * make test thread number configurable * add back max threads back test to reduce pressure * remove global --jobs rayon pool, revert to current implementation * fix import
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nexttest fail
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fork test
* bump alloy to 0.6.1 * fix: ui - use AnyRpcBlock * fix: wallets - use PrimitveSig * bump 0.6.2 * replace: AnyNetworkBlock with AnyRpcBlock + HeaderResponse with BlockHeader * fix: configure_tx_env * fix: crypto cheatcodes * fix: anvil_core tx * fix * fix: verify-bytecode * fix cast + get_pretty_tx_attr * fix(`anvil`): core TypedTx + BlockListener task * fix * fix: anvil tests * fix: test_deser_block * fix: transaction_build * fix: test load state files * fix: deny.toml * fix: try_from AnyRpcTx to DepositTx + bump op-alloy types * bump * fix: configure_tx_env * fix: UiFmt * fix: vb * fix: common-fmt tests * nit * fix: sig encoding * fix: process deposit tx in transaction_build * fix: common-fmt tests * fix * Update deny.toml Co-authored-by: zerosnacks <95942363+zerosnacks@users.noreply.github.com> * fixes * fix: use alloy impls for conversions * nit * fix: transaction_build * nit * fix: eip155 check and rm anvil core utils * clippy * nits * fix * nit * fix: impl UIfmt for TxEnvelope and AnyTxEnvelope * make header in pretty_block_basics exhaustive * clippy * fix * fix: txpool_content * fix * fix * fix overriding hashes * fix --------- Co-authored-by: zerosnacks <95942363+zerosnacks@users.noreply.github.com> Co-authored-by: grandizzy <38490174+grandizzy@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
feat(forge)
Head branch was pushed to by a user without write access
* feat: add global -j, --threads * Update crates/cli/src/opts/global.rs * fix tests after comment update --------- Co-authored-by: zerosnacks <95942363+zerosnacks@users.noreply.github.com> Co-authored-by: zerosnacks <zerosnacks@protonmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cargo.tomi
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mod file test
feat(forge): allow
--verifier custom
optionChanges after review: add description of custom verifier, reorg err message, add custom verifier api key
Fix descriptions
Update crates/verify/src/provider.rs
Motivation
Solution
Summary by Sourcery
Add support for a custom verification provider by introducing the
--verifier custom
option, along with the ability to specify a custom verifier API key and URL.New Features:
--verifier custom
option to allow users to specify a custom verification provider.Enhancements: