Revive Katana, Add SushiV3 - #564
Conversation
This reverts fe0395b.
a95ebc0 to
70c8f60
Compare
🛡️ Immunefi PR ReviewsWe noticed that your project isn't set up for automatic code reviews. If you'd like this PR reviewed by the Immunefi team, you can request it manually using the link below: Once submitted, we'll take care of assigning a reviewer and follow up here. |
duncancmt
left a comment
duncancmt
left a comment
There was a problem hiding this comment.
Can you please add a new script to the script/ directory that mirrors DeploySafes.s.sol, but that assumes that the chain is already mostly set up and we just need to deploy a new suite of Settlers and then reclaim ownership of the relevant Safe(s). You can assume that the current setup on-chain on Katana is typical of what this script should expect, with the exception of the specific addresses of the Safe(s) -- that should be pulled from the config.
7fbfcc1 to
9c81b49
Compare
3e54a72 to
ccaa774
Compare
|
Ok, ready for review again. Katana is Cancun, so maybe this gets pulled into dcmt/cancun for execution? (and noticed that codex replaced my free-range branch name with it's GMO |
duncancmt
left a comment
duncancmt
left a comment
There was a problem hiding this comment.
Nice! I have some more nits here. Mostly around how to be adequately defensive in the face of gas limits and future gas-changing EIPs
| return subCalls; | ||
| } | ||
|
|
||
| modifier eraVmCompat( |
There was a problem hiding this comment.
Do you think this modifier should move into the base contract?
There was a problem hiding this comment.
Pulled it into SafeMultisend here ab978ad. RedeploySettlers doesn't use it anywhere yet, since Katana isn't EraVm. Want me to also wire it here so future revives that do will be ready?
There was a problem hiding this comment.
Yes, please! The more homogeneity there is, the lower the cognitive burden, and the lower the chance of mistakes.
…ent dry-run parse failure
| if ! jq_out="$(jq -r '.transactions[] | [.transaction.from // error, .transaction.gas // error] | @tsv' "$dry_run_json")" ; then | ||
| echo "Failed to parse dry-run JSON at $dry_run_json" >&2 | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
You don't need an explicit error handler here. set -e does this for you.
|
Thanks for the reviews @duncancmt. Feeling better about Question for you @duncancmt — do you have a preference between 1) me closing this pr, or 2) reworking it to just be focused on generic |
|
The work on |
|
Superseded by #573. See the last few comments here for context |
2 participants
Restores Katana support after #557 and adds Sushi v3.
KATANA_MAINNET_RPC_URL