-
Notifications
You must be signed in to change notification settings - Fork 158
Add MultisigScriptDeposit extension for L1→L2 deposits #172
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
base: main
Are you sure you want to change the base?
Conversation
✅ Heimdall Review Status
|
|
Since PR #170 makes some changes to the |
jackchuma
left a comment
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.
I'm no longer convinced this is the right way to do things. If the calculated gas limit changes between sign, approve and run calls, the signatures will no longer be valid. I think moving this estimation into the signer tool during the validation file generation might make more sense to guarantee that the value does not change. The validation file could get an optional l2GasLimit field where, if set, the signer tool knows to add that as an env var. From the context of this script, expecting the env var to be set would work in that case
Thanks, good call with this, was an implementation oversight on my part! I've switched back to the env var approach now. |
This PR adds
MultisigScriptDeposit, an extension ofMultisigScriptthat simplifies L1→L2 deposit transactions.More specifically:
_buildL2Calls()instead of manually wrapping indepositTransactionOptimismPortal.depositTransactiontargetingCBMulticallon L2