Skip to content
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

✨ Add Support for Nested Safes #26

Merged
merged 10 commits into from
Mar 18, 2025

Conversation

ipatka
Copy link
Contributor

@ipatka ipatka commented Mar 14, 2025

🕓 Changelog

This PR adds support for calculating the Safe transaction hashes for nested Safe (i.e. use a Safe as a signatory to another Safe) approval transactions. When a nested Safe needs to approve a transaction on the primary Safe, it must call the approveHash(bytes32) function on the target Safe with the Safe transaction hash to approve:

/**
 * @inheritdoc ISafe
 */
function approveHash(bytes32 hashToApprove) external override {
    if (owners[msg.sender] == address(0)) revertWithError("GS030");
    approvedHashes[msg.sender][hashToApprove] = 1;
    emit ApproveHash(hashToApprove, msg.sender);
}

Reference: https://github.com/safe-global/safe-smart-account/blob/bdcfce3a76c4d1dfb256ac2ca971be7cfd6e493a/contracts/Safe.sol#L372-L379.

    /**
     * @inheritdoc ISafe
     */
    function execTransaction(
        address to,
        uint256 value,
        bytes calldata data,
        Enum.Operation operation,
        uint256 safeTxGas,
        uint256 baseGas,
        uint256 gasPrice,
        address gasToken,
        address payable refundReceiver,
        bytes memory signatures
    ) external payable override returns (bool success) {
        onBeforeExecTransaction(to, value, data, operation, safeTxGas, baseGas, gasPrice, gasToken, refundReceiver, signatures);
        bytes32 txHash;
        // Use scope here to limit variable lifetime and prevent `stack too deep` errors
        {
            = getTransactionHash( // Transaction info
                to,
                value,
                data,
                operation,
                safeTxGas,
                // Payment info
                baseGas,
                gasPrice,
                gasToken,
                refundReceiver,
                // Signature info
                // We use the post-increment here, so the current nonce value is used and incremented afterwards.
                nonce++
            );
            checkSignatures(msg.sender, txHash, signatures); // <-- For a nested Safe `txHash` is used as input for `approveHash(bytes32)`.
        }
...

Reference: https://github.com/safe-global/safe-smart-account/blob/bdcfce3a76c4d1dfb256ac2ca971be7cfd6e493a/contracts/Safe.sol#L108-L143.

...
} else if (v == 1) {
    // If v is 1 then it is an approved hash
    // When handling approved hashes the address of the approver is encoded into r
    currentOwner = address(uint160(uint256(r)));
    // Hashes are automatically approved by the sender of the message or when they have been pre-approved via a separate transaction
    if (executor != currentOwner && approvedHashes[currentOwner][dataHash] == 0) revertWithError("GS025");
...

Reference: https://github.com/safe-global/safe-smart-account/blob/bdcfce3a76c4d1dfb256ac2ca971be7cfd6e493a/contracts/Safe.sol#L318-L323.

New Features

  • New Command-Line Arguments:

    • --nested-safe-address: Specifies the address of the nested Safe approving the transaction.
    • --nested-safe-nonce: Defines the nonce for the nested Safe's approval transaction.
  • Updated Transaction Flow:
    When these arguments are provided, the script:

    1. Computes and displays the primary Safe transaction hashes as usual.
    2. Constructs an approveHash transaction with:
      • to: The primary Safe multisig address,
      • data: Encoded approveHash(bytes32) function call with the Safe transaction hash as argument,
      • value: Set to 0,
      • operation: Set to 0 (i.e. CALL),
      • All other parameters are set to their default values (0 or the zero address 0x0000000000000000000000000000000000000000).
    3. Calculates and displays the hashes for the approval transaction.
  • Additional Improvements:

    • Adds validation for nested Safe parameters.
    • Enhances the --interactive mode support to override the nested Safe version.
    • Updates the help documentation with the new parameters and an example usage for nested Safes.
    • Adds a CI Bash formatting step using shfmt.
    • Add support of nested Safes for calculating Safe message hashes.

Example Usage

./safe_hashes.sh --network sepolia --address 0x657ff0D4eC65D82b2bC1247b0a558bcd2f80A0f1 --nonce 4 --nested-safe-address 0x6bc56d6CE87C86CB0756c616bECFD3Cd32b09251 --nested-safe-nonce 4

This will calculate and return both the primary Safe transaction hashes and the nested Safe approval transaction hashes, displaying all relevant details for each.

@ipatka ipatka marked this pull request as draft March 14, 2025 15:40
@pcaversaccio pcaversaccio self-requested a review March 14, 2025 15:51
@pcaversaccio pcaversaccio self-assigned this Mar 14, 2025
@pcaversaccio pcaversaccio added the feature 💥 New feature or request label Mar 14, 2025
@ipatka ipatka marked this pull request as ready for review March 14, 2025 16:28
@pcaversaccio pcaversaccio changed the title Add support for nested safe ✨ Add Support for Nested Safes Mar 14, 2025
@pcaversaccio pcaversaccio added the documentation 📖 Improvements or additions to documentation label Mar 14, 2025
Signed-off-by: Pascal Marco Caversaccio <[email protected]>
@pcaversaccio pcaversaccio added the ci/cd 👷‍♂️ CI/CD configurations label Mar 17, 2025
Signed-off-by: Pascal Marco Caversaccio <[email protected]>
Signed-off-by: Pascal Marco Caversaccio <[email protected]>
Signed-off-by: Pascal Marco Caversaccio <[email protected]>
Signed-off-by: Pascal Marco Caversaccio <[email protected]>
Signed-off-by: Pascal Marco Caversaccio <[email protected]>
Signed-off-by: Pascal Marco Caversaccio <[email protected]>
Copy link
Owner

@pcaversaccio pcaversaccio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thx for this!

@pcaversaccio pcaversaccio merged commit 6bdfaf5 into pcaversaccio:main Mar 18, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci/cd 👷‍♂️ CI/CD configurations documentation 📖 Improvements or additions to documentation feature 💥 New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants