MultisigProposal: Support DelegateCall #96

anajuliabit · 2025-07-10T17:03:11Z

This adds support for Safe DelegateCall operations.

Changes:

Replace MultisendCallOnly for Multisend
Add support to operation field
Rewrite _simulateActions:
This function was etching the multisend contract as safe but that doesn't support delegate calls because the safe bytecode changed to the multisend contract.
The SAFE_CREATION_BYTECODE contains the SafeL2 bytecode with a modification to the checkNSignatures bypassing signature checks.

…tion handling - Introduced a new function `getOperations` to allow customization of operations for each action in the proposal. - Updated `getCalldata` to include operations validation, ensuring consistency across targets, values, arguments, and operations arrays.

…on handling and code clarity - Removed the constant MULTISIG_BYTECODE_HASH from MultisigProposal. - Simplified the getOperations function for better readability. - Enhanced getCalldata to ensure consistent validation of targets, values, arguments, and operations. - Updated internal functions for clarity and conciseness, including _validateAction and _processStateChanges. - Improved logging for proposal actions and calldata.

Copilot

Pull Request Overview

This PR adds support for Safe DelegateCall operations in multisig proposals by updating constants, extending parameterization of operations, and rewriting the simulation logic to bypass signature checks during testing.

Introduces SAFE_CREATION_BYTECODE and a new constant address for the Safe multisend contract
Adds getOperations hook and includes operations in getCalldata
Rewrites _simulateActions to deploy a modified Safe and call execTransaction with delegatecall support

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
utils/Constants.sol	Replaced `MULTISEND_BYTECODE`, removed `SAFE_BYTECODE`, added `SAFE_CREATION_BYTECODE` and multisend address
src/proposals/Proposal.sol	Fixed duplicate `Addresses` import and adjusted comment placement in account-access filtering
src/proposals/MultisigProposal.sol	Removed old bytecode hash, added `getOperations`, updated `getCalldata`, and rewrote `_simulateActions`

Comments suppressed due to low confidence (2)

src/proposals/MultisigProposal.sol:44

[nitpick] The error message "Array lengths mismatch" now covers four arrays (targets, values, arguments, operations). Consider updating it to something like "Targets, values, arguments, and operations length mismatch" for clarity.

        require(

src/proposals/MultisigProposal.sol:85

[nitpick] The variable name addr is quite generic; using a more descriptive name like deployedSafe or safeClone can improve readability by clarifying its purpose.

        address addr;

Copilot · 2025-07-10T17:25:27Z

utils/Constants.sol


-    bytes public constant SAFE_BYTECODE =
-        hex"608060405273ffffffffffffffffffffffffffffffffffffffff600054167fa619486e0000000000000000000000000000000000000000000000000000000060003514156050578060005260206000f35b3660008037600080366000845af43d6000803e60008114156070573d6000fd5b3d6000f3fea2646970667358221220d1429297349653a4918076d650332de1a1068c5f3e07c5c82360c277770b955264736f6c63430007060033";
+    address constant SAFE_MULTISEND_COTNRACT =


There is a typo in the constant name SAFE_MULTISEND_COTNRACT. It should likely be SAFE_MULTISEND_CONTRACT.

Suggested change

address constant SAFE_MULTISEND_COTNRACT =

address constant SAFE_MULTISEND_CONTRACT =

ElliotFriedman · 2025-07-10T17:26:00Z

src/proposals/MultisigProposal.sol

+        for (uint256 i = 0; i < actionsLength; i++) {
+            operations[i] = 0;
+        }


free memory slots are always 0, but this is client side code so ig it doesn't matter

ElliotFriedman · 2025-07-10T17:44:53Z

utils/Constants.sol

+        0xA238CBeb142c10Ef7Ad8442C6D1f9E89e07e7761;
+
+    // This is a modified Safe that overrides the checkNSignatures function
+    bytes public constant SAFE_CREATION_BYTECODE =


since you're not using the constructor to attach an address, we can simplify to the runtime bytecode. then you won't need the assembly to create the contract in the first place

This is the creation bytecode, this contract (safe with checkNSignatures modified) is not deployed. I've modified the SafeL2 contract, compiled it on my machine, and grabbed the creation bytecode. I've never deployed. Would you like for me to deploy?

you don't need to deploy it to get it's runtime bytecode. you should be able to get that in solidity by just deploying the contract and logging out its bytecode

ElliotFriedman · 2025-07-10T17:45:51Z

src/proposals/MultisigProposal.sol

+        bytes memory bytecode = Constants.SAFE_CREATION_BYTECODE;
+        address addr;
+        /// @solidity memory-safe-assembly
+        assembly {
+            addr := create(0, add(bytecode, 0x20), mload(bytecode))
+        }


I'd prefer etching runtime bytecode of this safe with the checkNSignatures function stubbed out instead of needing to create it every time

then no need to deploy

How that would work without deploying. Etching only works with deployed bytecode, right?

grab the runtime bytecode from an instance you deploy on your local machine in a foundry test, log it out to the terminal, copy and paste that here, rename from SAFE_CREATION_BYTECODE to RUNTIME_BYTECODE

ElliotFriedman · 2025-07-10T17:50:36Z

src/proposals/MultisigProposal.sol

        bytes callData;
    }

+    /// @notice get operations for each action, override this to provide custom operations


could you walk me through what an example flow using this would look like? it seems like this flow would be really counter-intuitive because a user would need to:

specify the calls as they normally would using FPS - nothing amiss here

override this function and write solidity that looks something like this to index which calls you gathered up should be calls and which should be delegate calls.

/// first call is delegatecall operations[0] = 1; /// second call is delegatecall operations[1] = 1; /// third call is call operations[2] = 0; /// fourth call is delegatecall operations[3] = 1; /// fifth call is call operations[4] = 0;

I think we should brainstorm cleaner and more intuitive ways of writing this so that other users will be able to easily understand what is going on here.

Alternative approach:

all or nothing, have a method where a user must specify if all calls are either regular or delegate calls.

This is how it would look like: https://github.com/moonwell-fi/mamo-contracts/blob/feat/mamo-staking-deploy/multisig/006_EnableRewardsDistributorSafeModule.sol#L47-L52

all or nothing, have a method where a user must specify if all calls are either regular or delegate calls.

I don't like that because I might want to create a batch transaction that first enables a module (using delegatecall) and then immediately performs another action (regular call)

ok why don't we just make a flag that is isDelegateCall, when you have a delegatecall operations you flip the flag to true, otherwise it just does regular calls.

…simulator

anajuliabit added 3 commits July 10, 2025 11:25

use Safe Multisend instead of MultisendCallOnly

ebc05fc

ElliotFriedman requested a review from Copilot July 10, 2025 17:24

Copilot AI reviewed Jul 10, 2025

View reviewed changes

ElliotFriedman requested changes Jul 10, 2025

View reviewed changes

anajuliabit added 4 commits July 18, 2025 14:13

clean old address

3264a2c

Merge branch 'main' of https://github.com/anajuliabit/forge-proposal-…

170ba1e

…simulator

remove logs

896ea50

uses multisend call only

052f713

	address constant SAFE_MULTISEND_COTNRACT =
	address constant SAFE_MULTISEND_CONTRACT =

MultisigProposal: Support DelegateCall #96

Are you sure you want to change the base?

MultisigProposal: Support DelegateCall #96

Uh oh!

Conversation

anajuliabit commented Jul 10, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Jul 10, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anajuliabit Jul 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

anajuliabit Jul 10, 2025 •

edited

Loading