-
Notifications
You must be signed in to change notification settings - Fork 146
feat: modify MultisigScript for OPCM tasks #167
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
|
| /// │ │ │ │ │ │ │ run() │ | ||
| /// │ │ │ │ │ │ │─────────────────────────────>│ | ||
| abstract contract MultisigScript is Script { | ||
| struct SafeTx { |
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.
Needed to add this to encapsulate inputs for one of the internal functions to avoid a stack too deep error
| _executeTransaction({safe: safes[0], data: datas[0], value: value, signatures: signatures, broadcast: true}); | ||
| (bytes[] memory datas, uint256 value,) = _transactionDatas({safes: safes}); | ||
| (Vm.AccountAccess[] memory accesses, Simulation.Payload memory simPayload) = _executeTransaction({ | ||
| safe: safes[0], to: multicallAddress, data: datas[0], value: value, signatures: signatures, broadcast: true |
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.
Should this to: still be multicallAddress?
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 believe so, yes. Only the final execute call should be directed to OPCM. The approval flow still uses multicall
Modifies
MultisigScriptto support DELEGATECALL operations to OPCM