From b140318af6581e499506b11128a892e3f7a52aeb Mon Sep 17 00:00:00 2001 From: Shebin John Date: Tue, 16 Jan 2024 19:03:04 +0530 Subject: [PATCH] Adding `ISafe` interface for `CompatibilityFallbackHandler` (#722) TODO: - [x] Creating `ISafe` interface along with other Safe dependency interfaces. - [x] Using `ISafe` interface for Tests. - [x] Extra: Added `codesize` script for checking the codesize of contracts. Closes #701 --- certora/applyHarness.patch | 60 ++++-- contracts/Safe.sol | 130 +++---------- contracts/base/Executor.sol | 2 +- contracts/base/FallbackManager.sol | 15 +- contracts/base/GuardManager.sol | 19 +- contracts/base/ModuleManager.sol | 70 ++----- contracts/base/OwnerManager.sol | 65 ++----- .../examples/guards/DebugTransactionGuard.sol | 6 +- .../guards/DelegateCallTransactionGuard.sol | 2 +- contracts/examples/guards/OnlyOwnersGuard.sol | 7 +- .../guards/ReentrancyTransactionGuard.sol | 2 +- .../handler/CompatibilityFallbackHandler.sol | 14 +- contracts/interfaces/IFallbackManager.sol | 19 ++ contracts/interfaces/IGuardManager.sol | 22 +++ contracts/interfaces/IModuleManager.sol | 83 ++++++++ contracts/interfaces/IOwnerManager.sol | 63 ++++++ contracts/interfaces/ISafe.sol | 179 ++++++++++++++++++ contracts/{common => libraries}/Enum.sol | 4 +- contracts/libraries/Safe130To141Migration.sol | 5 +- contracts/libraries/Safe150Migration.sol | 8 +- contracts/libraries/SafeToL2Migration.sol | 14 +- contracts/libraries/SignMessageLib.sol | 4 +- package.json | 1 + 23 files changed, 506 insertions(+), 288 deletions(-) create mode 100644 contracts/interfaces/IFallbackManager.sol create mode 100644 contracts/interfaces/IGuardManager.sol create mode 100644 contracts/interfaces/IModuleManager.sol create mode 100644 contracts/interfaces/IOwnerManager.sol create mode 100644 contracts/interfaces/ISafe.sol rename contracts/{common => libraries}/Enum.sol (75%) diff --git a/certora/applyHarness.patch b/certora/applyHarness.patch index baff9c113..88eac36d8 100644 --- a/certora/applyHarness.patch +++ b/certora/applyHarness.patch @@ -1,7 +1,7 @@ diff -druN Safe.sol Safe.sol ---- Safe.sol 2023-11-10 09:50:31 -+++ Safe.sol 2023-11-20 11:06:39 -@@ -76,7 +76,7 @@ +--- Safe.sol 2024-01-16 17:25:51 ++++ Safe.sol 2024-01-16 17:45:52 +@@ -72,20 +72,22 @@ * so we create a Safe with 0 owners and threshold 1. * This is an unusable Safe, perfect for the singleton */ @@ -9,10 +9,7 @@ diff -druN Safe.sol Safe.sol + // threshold = 1; MUNGED: remove and add to constructor of the harness } - /** -@@ -93,15 +93,17 @@ - * @param paymentReceiver Address that should receive the payment (or 0 if tx.origin) - */ + // @inheritdoc ISafe function setup( - address[] calldata _owners, + address[] memory _owners, @@ -24,26 +21,26 @@ diff -druN Safe.sol Safe.sol address paymentToken, uint256 payment, address payable paymentReceiver -- ) external { -+ ) public { +- ) external override { ++ ) public override { + // MUNGED: had to change the method visibility and location of the variables to be able to call it from the harness + // constructor // setupOwners checks if the Threshold is already set, therefore preventing that this method is called twice setupOwners(_owners, _threshold); if (fallbackHandler != address(0)) internalSetFallbackHandler(fallbackHandler); -@@ -451,7 +453,8 @@ +@@ -391,7 +393,8 @@ address gasToken, address refundReceiver, uint256 _nonce -- ) public view returns (bytes32) { +- ) public view override returns (bytes32) { + ) internal view returns (bytes32) { + // MUNGED: The function was made internal to enable CVL summaries. return keccak256(encodeTransactionData(to, value, data, operation, safeTxGas, baseGas, gasPrice, gasToken, refundReceiver, _nonce)); } } diff -druN base/Executor.sol base/Executor.sol ---- base/Executor.sol 2023-11-10 09:50:31 -+++ base/Executor.sol 2023-11-20 10:28:47 +--- base/Executor.sol 2024-01-16 16:57:39 ++++ base/Executor.sol 2024-01-16 17:46:29 @@ -26,12 +26,8 @@ uint256 txGas ) internal returns (bool success) { @@ -59,3 +56,40 @@ diff -druN base/Executor.sol base/Executor.sol } else { /* solhint-disable no-inline-assembly */ /// @solidity memory-safe-assembly +diff -druN interfaces/ISafe.sol interfaces/ISafe.sol +--- interfaces/ISafe.sol 2024-01-16 17:28:41 ++++ interfaces/ISafe.sol 2024-01-16 17:47:01 +@@ -119,33 +119,6 @@ + function domainSeparator() external view returns (bytes32); + + /** +- * @notice Returns transaction hash to be signed by owners. +- * @param to Destination address. +- * @param value Ether value. +- * @param data Data payload. +- * @param operation Operation type. +- * @param safeTxGas Gas that should be used for the safe transaction. +- * @param baseGas Gas costs for data used to trigger the safe transaction. +- * @param gasPrice Maximum gas price that should be used for this transaction. +- * @param gasToken Token address (or 0 if ETH) that is used for the payment. +- * @param refundReceiver Address of receiver of gas payment (or 0 if tx.origin). +- * @param _nonce Transaction nonce. +- * @return Transaction hash. +- */ +- function getTransactionHash( +- address to, +- uint256 value, +- bytes calldata data, +- Enum.Operation operation, +- uint256 safeTxGas, +- uint256 baseGas, +- uint256 gasPrice, +- address gasToken, +- address refundReceiver, +- uint256 _nonce +- ) external view returns (bytes32); +- +- /** + * External getter function for state variables. + */ + diff --git a/contracts/Safe.sol b/contracts/Safe.sol index f0541ab05..baefd4a88 100644 --- a/contracts/Safe.sol +++ b/contracts/Safe.sol @@ -10,9 +10,10 @@ import {Singleton} from "./common/Singleton.sol"; import {SignatureDecoder} from "./common/SignatureDecoder.sol"; import {SecuredTokenTransfer} from "./common/SecuredTokenTransfer.sol"; import {StorageAccessible} from "./common/StorageAccessible.sol"; -import {Enum} from "./common/Enum.sol"; +import {Enum} from "./libraries/Enum.sol"; import {ISignatureValidator, ISignatureValidatorConstants} from "./interfaces/ISignatureValidator.sol"; import {SafeMath} from "./external/SafeMath.sol"; +import {ISafe} from "./interfaces/ISafe.sol"; /** * @title Safe - A multisignature wallet with support for confirmations using signed messages based on EIP-712. @@ -40,11 +41,12 @@ contract Safe is SecuredTokenTransfer, ISignatureValidatorConstants, FallbackManager, - StorageAccessible + StorageAccessible, + ISafe { using SafeMath for uint256; - string public constant VERSION = "1.4.1"; + string public constant override VERSION = "1.4.1"; // keccak256( // "EIP712Domain(uint256 chainId,address verifyingContract)" @@ -56,18 +58,12 @@ contract Safe is // ); bytes32 private constant SAFE_TX_TYPEHASH = 0xbb8310d486368db6bd6f849402fdd73ad53d316b5a4b2644ad6efe0f941286d8; - event SafeSetup(address indexed initiator, address[] owners, uint256 threshold, address initializer, address fallbackHandler); - event ApproveHash(bytes32 indexed approvedHash, address indexed owner); - event SignMsg(bytes32 indexed msgHash); - event ExecutionFailure(bytes32 indexed txHash, uint256 payment); - event ExecutionSuccess(bytes32 indexed txHash, uint256 payment); - - uint256 public nonce; + uint256 public override nonce; bytes32 private _deprecatedDomainSeparator; // Mapping to keep track of all message hashes that have been approved by ALL REQUIRED owners - mapping(bytes32 => uint256) public signedMessages; + mapping(bytes32 => uint256) public override signedMessages; // Mapping to keep track of all hashes (message or transaction) that have been approved by ANY owners - mapping(address => mapping(bytes32 => uint256)) public approvedHashes; + mapping(address => mapping(bytes32 => uint256)) public override approvedHashes; // This constructor ensures that this contract can only be used as a singleton for Proxy contracts constructor() { @@ -79,19 +75,7 @@ contract Safe is threshold = 1; } - /** - * @notice Sets an initial storage of the Safe contract. - * @dev This method can only be called once. - * If a proxy was created without setting up, anyone can call setup and claim the proxy. - * @param _owners List of Safe owners. - * @param _threshold Number of required confirmations for a Safe transaction. - * @param to Contract address for optional delegate call. - * @param data Data payload for optional delegate call. - * @param fallbackHandler Handler for fallback calls to this contract - * @param paymentToken Token that should be used for the payment (0 is ETH) - * @param payment Value that should be paid - * @param paymentReceiver Address that should receive the payment (or 0 if tx.origin) - */ + // @inheritdoc ISafe function setup( address[] calldata _owners, uint256 _threshold, @@ -101,7 +85,7 @@ contract Safe is address paymentToken, uint256 payment, address payable paymentReceiver - ) external { + ) external override { // setupOwners checks if the Threshold is already set, therefore preventing that this method is called twice setupOwners(_owners, _threshold); if (fallbackHandler != address(0)) internalSetFallbackHandler(fallbackHandler); @@ -116,26 +100,7 @@ contract Safe is emit SafeSetup(msg.sender, _owners, _threshold, to, fallbackHandler); } - /** @notice Executes a `operation` {0: Call, 1: DelegateCall}} transaction to `to` with `value` (Native Currency) - * and pays `gasPrice` * `gasLimit` in `gasToken` token to `refundReceiver`. - * @dev The fees are always transferred, even if the user transaction fails. - * This method doesn't perform any sanity check of the transaction, such as: - * - if the contract at `to` address has code or not - * - if the `gasToken` is a contract or not - * It is the responsibility of the caller to perform such checks. - * @param to Destination address of Safe transaction. - * @param value Ether value of Safe transaction. - * @param data Data payload of Safe transaction. - * @param operation Operation type of Safe transaction. - * @param safeTxGas Gas that should be used for the Safe transaction. - * @param baseGas Gas costs that are independent of the transaction execution(e.g. base transaction fee, signature check, payment of the refund) - * @param gasPrice Gas price that should be used for the payment calculation. - * @param gasToken Token address (or 0 if ETH) that is used for the payment. - * @param refundReceiver Address of receiver of gas payment (or 0 if tx.origin). - * @param signatures Signature data that should be verified. - * Can be packed ECDSA signature ({bytes32 r}{bytes32 s}{uint8 v}), contract signature (EIP-1271) or approved hash. - * @return success Boolean indicating transaction's success. - */ + // @inheritdoc ISafe function execTransaction( address to, uint256 value, @@ -147,7 +112,7 @@ contract Safe is address gasToken, address payable refundReceiver, bytes memory signatures - ) public payable virtual returns (bool success) { + ) public payable virtual override returns (bool success) { bytes32 txHash; // Use scope here to limit variable lifetime and prevent `stack too deep` errors { @@ -282,24 +247,13 @@ contract Safe is if (ISignatureValidator(owner).isValidSignature(dataHash, contractSignature) != EIP1271_MAGIC_VALUE) revertWithError("GS024"); } - /** - * @notice Checks whether the signature provided is valid for the provided data and hash. Reverts otherwise. - * @param dataHash Hash of the data (could be either a message hash or transaction hash) - * @param signatures Signature data that should be verified. - * Can be packed ECDSA signature ({bytes32 r}{bytes32 s}{uint8 v}), contract signature (EIP-1271) or approved hash. - */ - function checkSignatures(bytes32 dataHash, bytes memory signatures) public view { + // @inheritdoc ISafe + function checkSignatures(bytes32 dataHash, bytes memory signatures) public view override { checkSignatures(dataHash, "", signatures); } - /** - * @notice Checks whether the signature provided is valid for the provided data and hash. Reverts otherwise. - * @param dataHash Hash of the data (could be either a message hash or transaction hash) - * @param signatures Signature data that should be verified. - * Can be packed ECDSA signature ({bytes32 r}{bytes32 s}{uint8 v}), contract signature (EIP-1271) or approved hash. - * @dev This function makes it compatible with previous versions. - */ - function checkSignatures(bytes32 dataHash, bytes memory /* IGNORED */, bytes memory signatures) public view { + // @inheritdoc ISafe + function checkSignatures(bytes32 dataHash, bytes memory /* IGNORED */, bytes memory signatures) public view override { // Load threshold to avoid multiple storage loads uint256 _threshold = threshold; // Check that a threshold is set @@ -307,18 +261,13 @@ contract Safe is checkNSignatures(msg.sender, dataHash, signatures, _threshold); } - /** - * @notice Checks whether the signature provided is valid for the provided data and hash. Reverts otherwise. - * @dev Since the EIP-1271 does an external call, be mindful of reentrancy attacks. - * @param executor Address that executing the transaction. - * ⚠️⚠️⚠️ Make sure that the executor address is a legitmate executor. - * Incorrectly passed the executor might reduce the threshold by 1 signature. ⚠️⚠️⚠️ - * @param dataHash Hash of the data (could be either a message hash or transaction hash) - * @param signatures Signature data that should be verified. - * Can be packed ECDSA signature ({bytes32 r}{bytes32 s}{uint8 v}), contract signature (EIP-1271) or approved hash. - * @param requiredSignatures Amount of required valid signatures. - */ - function checkNSignatures(address executor, bytes32 dataHash, bytes memory signatures, uint256 requiredSignatures) public view { + // @inheritdoc ISafe + function checkNSignatures( + address executor, + bytes32 dataHash, + bytes memory signatures, + uint256 requiredSignatures + ) public view override { // Check that the provided signature data is not too short if (signatures.length < requiredSignatures.mul(65)) revertWithError("GS020"); // There cannot be an owner with address 0. @@ -366,23 +315,15 @@ contract Safe is } } - /** - * @notice Marks hash `hashToApprove` as approved. - * @dev This can be used with a pre-approved hash transaction signature. - * IMPORTANT: The approved hash stays approved forever. There's no revocation mechanism, so it behaves similarly to ECDSA signatures - * @param hashToApprove The hash to mark as approved for signatures that are verified by this contract. - */ - function approveHash(bytes32 hashToApprove) external { + // @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); } - /** - * @dev Returns the domain separator for this contract, as defined in the EIP-712 standard. - * @return bytes32 The domain separator hash. - */ - function domainSeparator() public view returns (bytes32) { + // @inheritdoc ISafe + function domainSeparator() public view override returns (bytes32) { uint256 chainId; /* solhint-disable no-inline-assembly */ /// @solidity memory-safe-assembly @@ -438,20 +379,7 @@ contract Safe is return abi.encodePacked(bytes1(0x19), bytes1(0x01), domainSeparator(), safeTxHash); } - /** - * @notice Returns transaction hash to be signed by owners. - * @param to Destination address. - * @param value Ether value. - * @param data Data payload. - * @param operation Operation type. - * @param safeTxGas Gas that should be used for the safe transaction. - * @param baseGas Gas costs for data used to trigger the safe transaction. - * @param gasPrice Maximum gas price that should be used for this transaction. - * @param gasToken Token address (or 0 if ETH) that is used for the payment. - * @param refundReceiver Address of receiver of gas payment (or 0 if tx.origin). - * @param _nonce Transaction nonce. - * @return Transaction hash. - */ + // @inheritdoc ISafe function getTransactionHash( address to, uint256 value, @@ -463,7 +391,7 @@ contract Safe is address gasToken, address refundReceiver, uint256 _nonce - ) public view returns (bytes32) { + ) public view override returns (bytes32) { return keccak256(encodeTransactionData(to, value, data, operation, safeTxGas, baseGas, gasPrice, gasToken, refundReceiver, _nonce)); } } diff --git a/contracts/base/Executor.sol b/contracts/base/Executor.sol index 157c08820..08fa97faa 100644 --- a/contracts/base/Executor.sol +++ b/contracts/base/Executor.sol @@ -1,6 +1,6 @@ // SPDX-License-Identifier: LGPL-3.0-only pragma solidity >=0.7.0 <0.9.0; -import {Enum} from "../common/Enum.sol"; +import {Enum} from "../libraries/Enum.sol"; /** * @title Executor - A contract that can execute transactions diff --git a/contracts/base/FallbackManager.sol b/contracts/base/FallbackManager.sol index 4203974f1..f7333a41b 100644 --- a/contracts/base/FallbackManager.sol +++ b/contracts/base/FallbackManager.sol @@ -2,14 +2,13 @@ pragma solidity >=0.7.0 <0.9.0; import {SelfAuthorized} from "../common/SelfAuthorized.sol"; +import {IFallbackManager} from "../interfaces/IFallbackManager.sol"; /** * @title Fallback Manager - A contract managing fallback calls made to this contract * @author Richard Meissner - @rmeissner */ -abstract contract FallbackManager is SelfAuthorized { - event ChangedFallbackHandler(address indexed handler); - +abstract contract FallbackManager is SelfAuthorized, IFallbackManager { // keccak256("fallback_manager.handler.address") bytes32 internal constant FALLBACK_HANDLER_STORAGE_SLOT = 0x6c9a6c4a39284e37ed1cf53d337577d14212a4870fb976a4366c693b939918d5; @@ -41,14 +40,8 @@ abstract contract FallbackManager is SelfAuthorized { /* solhint-enable no-inline-assembly */ } - /** - * @notice Set Fallback Handler to `handler` for the Safe. - * @dev Only fallback calls without value and with data will be forwarded. - * This can only be done via a Safe transaction. - * Cannot be set to the Safe itself. - * @param handler contract to handle fallback calls. - */ - function setFallbackHandler(address handler) public authorized { + // @inheritdoc IFallbackManager + function setFallbackHandler(address handler) public override authorized { internalSetFallbackHandler(handler); emit ChangedFallbackHandler(handler); } diff --git a/contracts/base/GuardManager.sol b/contracts/base/GuardManager.sol index d71799f64..2413d19d7 100644 --- a/contracts/base/GuardManager.sol +++ b/contracts/base/GuardManager.sol @@ -2,9 +2,10 @@ /* solhint-disable one-contract-per-file */ pragma solidity >=0.7.0 <0.9.0; -import {Enum} from "../common/Enum.sol"; +import {Enum} from "../libraries/Enum.sol"; import {SelfAuthorized} from "../common/SelfAuthorized.sol"; import {IERC165} from "../interfaces/IERC165.sol"; +import {IGuardManager} from "../interfaces/IGuardManager.sol"; /// @title Guard Interface interface Guard is IERC165 { @@ -70,22 +71,12 @@ abstract contract BaseGuard is Guard { * @title Guard Manager - A contract managing transaction guards which perform pre and post-checks on Safe transactions. * @author Richard Meissner - @rmeissner */ -abstract contract GuardManager is SelfAuthorized { - event ChangedGuard(address indexed guard); - +abstract contract GuardManager is SelfAuthorized, IGuardManager { // keccak256("guard_manager.guard.address") bytes32 internal constant GUARD_STORAGE_SLOT = 0x4a204f620c8c5ccdca3fd54d003badd85ba500436a431f0cbda4f558c93c34c8; - /** - * @dev Set a guard that checks transactions before execution - * This can only be done via a Safe transaction. - * ⚠️ IMPORTANT: Since a guard has full power to block Safe transaction execution, - * a broken guard can cause a denial of service for the Safe. Make sure to carefully - * audit the guard code and design recovery mechanisms. - * @notice Set Transaction Guard `guard` for the Safe. Make sure you trust the guard. - * @param guard The address of the guard to be used or the 0 address to disable the guard - */ - function setGuard(address guard) external authorized { + // @inheritdoc IGuardManager + function setGuard(address guard) external override authorized { if (guard != address(0) && !Guard(guard).supportsInterface(type(Guard).interfaceId)) revertWithError("GS300"); /* solhint-disable no-inline-assembly */ /// @solidity memory-safe-assembly diff --git a/contracts/base/ModuleManager.sol b/contracts/base/ModuleManager.sol index 45bc0bbb0..4f8b12092 100644 --- a/contracts/base/ModuleManager.sol +++ b/contracts/base/ModuleManager.sol @@ -1,9 +1,10 @@ // SPDX-License-Identifier: LGPL-3.0-only pragma solidity >=0.7.0 <0.9.0; -import {Enum} from "../common/Enum.sol"; +import {Enum} from "../libraries/Enum.sol"; import {SelfAuthorized} from "../common/SelfAuthorized.sol"; import {Executor} from "./Executor.sol"; import {GuardManager, Guard} from "./GuardManager.sol"; +import {IModuleManager} from "../interfaces/IModuleManager.sol"; /** * @title Module Manager - A contract managing Safe modules @@ -14,12 +15,7 @@ import {GuardManager, Guard} from "./GuardManager.sol"; * @author Stefan George - @Georgi87 * @author Richard Meissner - @rmeissner */ -abstract contract ModuleManager is SelfAuthorized, Executor, GuardManager { - event EnabledModule(address indexed module); - event DisabledModule(address indexed module); - event ExecutionFromModuleSuccess(address indexed module); - event ExecutionFromModuleFailure(address indexed module); - +abstract contract ModuleManager is SelfAuthorized, Executor, GuardManager, IModuleManager { address internal constant SENTINEL_MODULES = address(0x1); mapping(address => address) internal modules; @@ -80,12 +76,8 @@ abstract contract ModuleManager is SelfAuthorized, Executor, GuardManager { else emit ExecutionFromModuleFailure(msg.sender); } - /** - * @notice Enables the module `module` for the Safe. - * @dev This can only be done via a Safe transaction. - * @param module Module to be whitelisted. - */ - function enableModule(address module) public authorized { + // @inheritdoc IModuleManager + function enableModule(address module) public override authorized { // Module address cannot be null or sentinel. if (module == address(0) || module == SENTINEL_MODULES) revertWithError("GS101"); // Module cannot be added twice. @@ -95,13 +87,8 @@ abstract contract ModuleManager is SelfAuthorized, Executor, GuardManager { emit EnabledModule(module); } - /** - * @notice Disables the module `module` for the Safe. - * @dev This can only be done via a Safe transaction. - * @param prevModule Previous module in the modules linked list. - * @param module Module to be removed. - */ - function disableModule(address prevModule, address module) public authorized { + // @inheritdoc IModuleManager + function disableModule(address prevModule, address module) public override authorized { // Validate module address and check that it corresponds to module index. if (module == address(0) || module == SENTINEL_MODULES) revertWithError("GS101"); if (modules[prevModule] != module) revertWithError("GS103"); @@ -110,41 +97,25 @@ abstract contract ModuleManager is SelfAuthorized, Executor, GuardManager { emit DisabledModule(module); } - /** - * @notice Execute `operation` (0: Call, 1: DelegateCall) to `to` with `value` (Native Token) - * @dev Function is virtual to allow overriding for L2 singleton to emit an event for indexing. - * @param to Destination address of module transaction. - * @param value Ether value of module transaction. - * @param data Data payload of module transaction. - * @param operation Operation type of module transaction. - * @return success Boolean flag indicating if the call succeeded. - */ + // @inheritdoc IModuleManager function execTransactionFromModule( address to, uint256 value, bytes memory data, Enum.Operation operation - ) public virtual returns (bool success) { + ) public virtual override returns (bool success) { (address guard, bytes32 guardHash) = preModuleExecution(to, value, data, operation); success = execute(to, value, data, operation, type(uint256).max); postModuleExecution(guard, guardHash, success); } - /** - * @notice Execute `operation` (0: Call, 1: DelegateCall) to `to` with `value` (Native Token) and return data - * @param to Destination address of module transaction. - * @param value Ether value of module transaction. - * @param data Data payload of module transaction. - * @param operation Operation type of module transaction. - * @return success Boolean flag indicating if the call succeeded. - * @return returnData Data returned by the call. - */ + // @inheritdoc IModuleManager function execTransactionFromModuleReturnData( address to, uint256 value, bytes memory data, Enum.Operation operation - ) public returns (bool success, bytes memory returnData) { + ) public override returns (bool success, bytes memory returnData) { (address guard, bytes32 guardHash) = preModuleExecution(to, value, data, operation); success = execute(to, value, data, operation, type(uint256).max); /* solhint-disable no-inline-assembly */ @@ -164,24 +135,13 @@ abstract contract ModuleManager is SelfAuthorized, Executor, GuardManager { postModuleExecution(guard, guardHash, success); } - /** - * @notice Returns if an module is enabled - * @return True if the module is enabled - */ - function isModuleEnabled(address module) public view returns (bool) { + // @inheritdoc IModuleManager + function isModuleEnabled(address module) public view override returns (bool) { return SENTINEL_MODULES != module && modules[module] != address(0); } - /** - * @notice Returns an array of modules. - * If all entries fit into a single page, the next pointer will be 0x1. - * If another page is present, next will be the last element of the returned array. - * @param start Start of the page. Has to be a module or start pointer (0x1 address) - * @param pageSize Maximum number of modules that should be returned. Has to be > 0 - * @return array Array of modules. - * @return next Start of the next page. - */ - function getModulesPaginated(address start, uint256 pageSize) external view returns (address[] memory array, address next) { + // @inheritdoc IModuleManager + function getModulesPaginated(address start, uint256 pageSize) external view override returns (address[] memory array, address next) { if (start != SENTINEL_MODULES && !isModuleEnabled(start)) revertWithError("GS105"); if (pageSize == 0) revertWithError("GS106"); // Init array with max page size diff --git a/contracts/base/OwnerManager.sol b/contracts/base/OwnerManager.sol index 9ffb5a6b9..e32ac0456 100644 --- a/contracts/base/OwnerManager.sol +++ b/contracts/base/OwnerManager.sol @@ -1,6 +1,7 @@ // SPDX-License-Identifier: LGPL-3.0-only pragma solidity >=0.7.0 <0.9.0; import {SelfAuthorized} from "../common/SelfAuthorized.sol"; +import {IOwnerManager} from "../interfaces/IOwnerManager.sol"; /** * @title OwnerManager - Manages Safe owners and a threshold to authorize transactions. @@ -9,11 +10,7 @@ import {SelfAuthorized} from "../common/SelfAuthorized.sol"; * @author Stefan George - @Georgi87 * @author Richard Meissner - @rmeissner */ -abstract contract OwnerManager is SelfAuthorized { - event AddedOwner(address indexed owner); - event RemovedOwner(address indexed owner); - event ChangedThreshold(uint256 threshold); - +abstract contract OwnerManager is SelfAuthorized, IOwnerManager { address internal constant SENTINEL_OWNERS = address(0x1); mapping(address => address) internal owners; @@ -50,13 +47,8 @@ abstract contract OwnerManager is SelfAuthorized { threshold = _threshold; } - /** - * @notice Adds the owner `owner` to the Safe and updates the threshold to `_threshold`. - * @dev This can only be done via a Safe transaction. - * @param owner New owner address. - * @param _threshold New threshold. - */ - function addOwnerWithThreshold(address owner, uint256 _threshold) public authorized { + // @inheritdoc IOwnerManager + function addOwnerWithThreshold(address owner, uint256 _threshold) public override authorized { // Owner address cannot be null, the sentinel or the Safe itself. if (owner == address(0) || owner == SENTINEL_OWNERS || owner == address(this)) revertWithError("GS203"); // No duplicate owners allowed. @@ -69,14 +61,8 @@ abstract contract OwnerManager is SelfAuthorized { if (threshold != _threshold) changeThreshold(_threshold); } - /** - * @notice Removes the owner `owner` from the Safe and updates the threshold to `_threshold`. - * @dev This can only be done via a Safe transaction. - * @param prevOwner Owner that pointed to the owner to be removed in the linked list - * @param owner Owner address to be removed. - * @param _threshold New threshold. - */ - function removeOwner(address prevOwner, address owner, uint256 _threshold) public authorized { + // @inheritdoc IOwnerManager + function removeOwner(address prevOwner, address owner, uint256 _threshold) public override authorized { // Only allow to remove an owner, if threshold can still be reached. if (ownerCount - 1 < _threshold) revertWithError("GS201"); // Validate owner address and check that it corresponds to owner index. @@ -90,14 +76,8 @@ abstract contract OwnerManager is SelfAuthorized { if (threshold != _threshold) changeThreshold(_threshold); } - /** - * @notice Replaces the owner `oldOwner` in the Safe with `newOwner`. - * @dev This can only be done via a Safe transaction. - * @param prevOwner Owner that pointed to the owner to be replaced in the linked list - * @param oldOwner Owner address to be replaced. - * @param newOwner New owner address. - */ - function swapOwner(address prevOwner, address oldOwner, address newOwner) public authorized { + // @inheritdoc IOwnerManager + function swapOwner(address prevOwner, address oldOwner, address newOwner) public override authorized { // Owner address cannot be null, the sentinel or the Safe itself. if (newOwner == address(0) || newOwner == SENTINEL_OWNERS || newOwner == address(this)) revertWithError("GS203"); // No duplicate owners allowed. @@ -112,12 +92,8 @@ abstract contract OwnerManager is SelfAuthorized { emit AddedOwner(newOwner); } - /** - * @notice Changes the threshold of the Safe to `_threshold`. - * @dev This can only be done via a Safe transaction. - * @param _threshold New threshold. - */ - function changeThreshold(uint256 _threshold) public authorized { + // @inheritdoc IOwnerManager + function changeThreshold(uint256 _threshold) public override authorized { // Validate that threshold is smaller than number of owners. if (_threshold > ownerCount) revertWithError("GS201"); // There has to be at least one Safe owner. @@ -126,27 +102,18 @@ abstract contract OwnerManager is SelfAuthorized { emit ChangedThreshold(threshold); } - /** - * @notice Returns the number of required confirmations for a Safe transaction aka the threshold. - * @return Threshold number. - */ - function getThreshold() public view returns (uint256) { + // @inheritdoc IOwnerManager + function getThreshold() public view override returns (uint256) { return threshold; } - /** - * @notice Returns if `owner` is an owner of the Safe. - * @return Boolean if owner is an owner of the Safe. - */ - function isOwner(address owner) public view returns (bool) { + // @inheritdoc IOwnerManager + function isOwner(address owner) public view override returns (bool) { return !(owner == SENTINEL_OWNERS || owners[owner] == address(0)); } - /** - * @notice Returns a list of Safe owners. - * @return Array of Safe owners. - */ - function getOwners() public view returns (address[] memory) { + // @inheritdoc IOwnerManager + function getOwners() public view override returns (address[] memory) { address[] memory array = new address[](ownerCount); // populate return array diff --git a/contracts/examples/guards/DebugTransactionGuard.sol b/contracts/examples/guards/DebugTransactionGuard.sol index ae3dc8093..2db8f2a01 100644 --- a/contracts/examples/guards/DebugTransactionGuard.sol +++ b/contracts/examples/guards/DebugTransactionGuard.sol @@ -1,9 +1,9 @@ // SPDX-License-Identifier: LGPL-3.0-only pragma solidity >=0.7.0 <0.9.0; -import {Enum} from "../../common/Enum.sol"; +import {Enum} from "../../libraries/Enum.sol"; import {BaseGuard} from "../../base/GuardManager.sol"; -import {Safe} from "../../Safe.sol"; +import {ISafe} from "../../interfaces/ISafe.sol"; /** * @title Debug Transaction Guard - Emits transaction events with extended information. @@ -75,7 +75,7 @@ contract DebugTransactionGuard is BaseGuard { uint256 nonce; bytes32 txHash; { - Safe safe = Safe(payable(msg.sender)); + ISafe safe = ISafe(payable(msg.sender)); nonce = safe.nonce() - 1; txHash = safe.getTransactionHash(to, value, data, operation, safeTxGas, baseGas, gasPrice, gasToken, refundReceiver, nonce); } diff --git a/contracts/examples/guards/DelegateCallTransactionGuard.sol b/contracts/examples/guards/DelegateCallTransactionGuard.sol index 5da49130f..08120499a 100644 --- a/contracts/examples/guards/DelegateCallTransactionGuard.sol +++ b/contracts/examples/guards/DelegateCallTransactionGuard.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: LGPL-3.0-only pragma solidity >=0.7.0 <0.9.0; -import {Enum} from "../../common/Enum.sol"; +import {Enum} from "../../libraries/Enum.sol"; import {BaseGuard} from "../../base/GuardManager.sol"; /** diff --git a/contracts/examples/guards/OnlyOwnersGuard.sol b/contracts/examples/guards/OnlyOwnersGuard.sol index 9297dce04..fea28f56f 100644 --- a/contracts/examples/guards/OnlyOwnersGuard.sol +++ b/contracts/examples/guards/OnlyOwnersGuard.sol @@ -2,12 +2,9 @@ /* solhint-disable one-contract-per-file */ pragma solidity >=0.7.0 <0.9.0; -import {Enum} from "../../common/Enum.sol"; +import {Enum} from "../../libraries/Enum.sol"; import {BaseGuard} from "../../base/GuardManager.sol"; - -interface ISafe { - function isOwner(address owner) external view returns (bool); -} +import {ISafe} from "../../interfaces/ISafe.sol"; /** * @title OnlyOwnersGuard - Only allows owners to execute transactions. diff --git a/contracts/examples/guards/ReentrancyTransactionGuard.sol b/contracts/examples/guards/ReentrancyTransactionGuard.sol index 327d85854..c8a973038 100644 --- a/contracts/examples/guards/ReentrancyTransactionGuard.sol +++ b/contracts/examples/guards/ReentrancyTransactionGuard.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: LGPL-3.0-only pragma solidity >=0.7.0 <0.9.0; -import {Enum} from "../../common/Enum.sol"; +import {Enum} from "../../libraries/Enum.sol"; import {BaseGuard} from "../../base/GuardManager.sol"; /** diff --git a/contracts/handler/CompatibilityFallbackHandler.sol b/contracts/handler/CompatibilityFallbackHandler.sol index 8ea8a6b12..85ac4ef8e 100644 --- a/contracts/handler/CompatibilityFallbackHandler.sol +++ b/contracts/handler/CompatibilityFallbackHandler.sol @@ -3,7 +3,7 @@ pragma solidity >=0.7.0 <0.9.0; import {TokenCallbackHandler} from "./TokenCallbackHandler.sol"; import {ISignatureValidator} from "../interfaces/ISignatureValidator.sol"; -import {Safe} from "../Safe.sol"; +import {ISafe} from "../interfaces/ISafe.sol"; import {HandlerContext} from "./HandlerContext.sol"; /** @@ -24,7 +24,7 @@ contract CompatibilityFallbackHandler is TokenCallbackHandler, ISignatureValidat * @return Message hash. */ function getMessageHash(bytes memory message) public view returns (bytes32) { - return getMessageHashForSafe(Safe(payable(msg.sender)), message); + return getMessageHashForSafe(ISafe(payable(msg.sender)), message); } /** @@ -33,7 +33,7 @@ contract CompatibilityFallbackHandler is TokenCallbackHandler, ISignatureValidat * @param message Message that should be encoded. * @return Encoded message. */ - function encodeMessageDataForSafe(Safe safe, bytes memory message) public view returns (bytes memory) { + function encodeMessageDataForSafe(ISafe safe, bytes memory message) public view returns (bytes memory) { bytes32 safeMessageHash = keccak256(abi.encode(SAFE_MSG_TYPEHASH, keccak256(message))); return abi.encodePacked(bytes1(0x19), bytes1(0x01), safe.domainSeparator(), safeMessageHash); } @@ -44,7 +44,7 @@ contract CompatibilityFallbackHandler is TokenCallbackHandler, ISignatureValidat * @param message Message that should be hashed. * @return Message hash. */ - function getMessageHashForSafe(Safe safe, bytes memory message) public view returns (bytes32) { + function getMessageHashForSafe(ISafe safe, bytes memory message) public view returns (bytes32) { return keccak256(encodeMessageDataForSafe(safe, message)); } @@ -56,7 +56,7 @@ contract CompatibilityFallbackHandler is TokenCallbackHandler, ISignatureValidat */ function isValidSignature(bytes32 _dataHash, bytes calldata _signature) public view override returns (bytes4) { // Caller should be a Safe - Safe safe = Safe(payable(msg.sender)); + ISafe safe = ISafe(payable(msg.sender)); bytes memory messageData = encodeMessageDataForSafe(safe, abi.encode(_dataHash)); bytes32 messageHash = keccak256(messageData); if (_signature.length == 0) { @@ -73,7 +73,7 @@ contract CompatibilityFallbackHandler is TokenCallbackHandler, ISignatureValidat */ function getModules() external view returns (address[] memory) { // Caller should be a Safe - Safe safe = Safe(payable(msg.sender)); + ISafe safe = ISafe(payable(msg.sender)); (address[] memory array, ) = safe.getModulesPaginated(SENTINEL_MODULES, 10); return array; } @@ -175,6 +175,6 @@ contract CompatibilityFallbackHandler is TokenCallbackHandler, ISignatureValidat bytes memory signatures, uint256 requiredSignatures ) public view { - Safe(payable(_manager())).checkNSignatures(_msgSender(), dataHash, signatures, requiredSignatures); + ISafe(payable(_manager())).checkNSignatures(_msgSender(), dataHash, signatures, requiredSignatures); } } diff --git a/contracts/interfaces/IFallbackManager.sol b/contracts/interfaces/IFallbackManager.sol new file mode 100644 index 000000000..45b626d87 --- /dev/null +++ b/contracts/interfaces/IFallbackManager.sol @@ -0,0 +1,19 @@ +// SPDX-License-Identifier: LGPL-3.0-only +pragma solidity >=0.7.0 <0.9.0; + +/** + * @title IFallbackManager - A contract interface managing fallback calls made to this contract. + * @author @safe-global/safe-protocol + */ +interface IFallbackManager { + event ChangedFallbackHandler(address indexed handler); + + /** + * @notice Set Fallback Handler to `handler` for the Safe. + * @dev Only fallback calls without value and with data will be forwarded. + * This can only be done via a Safe transaction. + * Cannot be set to the Safe itself. + * @param handler contract to handle fallback calls. + */ + function setFallbackHandler(address handler) external; +} diff --git a/contracts/interfaces/IGuardManager.sol b/contracts/interfaces/IGuardManager.sol new file mode 100644 index 000000000..f5f125424 --- /dev/null +++ b/contracts/interfaces/IGuardManager.sol @@ -0,0 +1,22 @@ +// SPDX-License-Identifier: LGPL-3.0-only +/* solhint-disable one-contract-per-file */ +pragma solidity >=0.7.0 <0.9.0; + +/** + * @title IGuardManager - A contract interface managing transaction guards which perform pre and post-checks on Safe transactions. + * @author @safe-global/safe-protocol + */ +interface IGuardManager { + event ChangedGuard(address indexed guard); + + /** + * @dev Set a guard that checks transactions before execution + * This can only be done via a Safe transaction. + * ⚠️ IMPORTANT: Since a guard has full power to block Safe transaction execution, + * a broken guard can cause a denial of service for the Safe. Make sure to carefully + * audit the guard code and design recovery mechanisms. + * @notice Set Transaction Guard `guard` for the Safe. Make sure you trust the guard. + * @param guard The address of the guard to be used or the 0 address to disable the guard + */ + function setGuard(address guard) external; +} diff --git a/contracts/interfaces/IModuleManager.sol b/contracts/interfaces/IModuleManager.sol new file mode 100644 index 000000000..b449d7d16 --- /dev/null +++ b/contracts/interfaces/IModuleManager.sol @@ -0,0 +1,83 @@ +// SPDX-License-Identifier: LGPL-3.0-only +pragma solidity >=0.7.0 <0.9.0; +import {Enum} from "../libraries/Enum.sol"; +import {IGuardManager} from "./IGuardManager.sol"; + +/** + * @title IModuleManager - An interface of contract managing Safe modules + * @notice Modules are extensions with unlimited access to a Safe that can be added to a Safe by its owners. + ⚠️ WARNING: Modules are a security risk since they can execute arbitrary transactions, + so only trusted and audited modules should be added to a Safe. A malicious module can + completely takeover a Safe. + * @author @safe-global/safe-protocol + */ +interface IModuleManager is IGuardManager { + event EnabledModule(address indexed module); + event DisabledModule(address indexed module); + event ExecutionFromModuleSuccess(address indexed module); + event ExecutionFromModuleFailure(address indexed module); + + /** + * @notice Enables the module `module` for the Safe. + * @dev This can only be done via a Safe transaction. + * @param module Module to be whitelisted. + */ + function enableModule(address module) external; + + /** + * @notice Disables the module `module` for the Safe. + * @dev This can only be done via a Safe transaction. + * @param prevModule Previous module in the modules linked list. + * @param module Module to be removed. + */ + function disableModule(address prevModule, address module) external; + + /** + * @notice Execute `operation` (0: Call, 1: DelegateCall) to `to` with `value` (Native Token) + * @dev Function is virtual to allow overriding for L2 singleton to emit an event for indexing. + * @param to Destination address of module transaction. + * @param value Ether value of module transaction. + * @param data Data payload of module transaction. + * @param operation Operation type of module transaction. + * @return success Boolean flag indicating if the call succeeded. + */ + function execTransactionFromModule( + address to, + uint256 value, + bytes memory data, + Enum.Operation operation + ) external returns (bool success); + + /** + * @notice Execute `operation` (0: Call, 1: DelegateCall) to `to` with `value` (Native Token) and return data + * @param to Destination address of module transaction. + * @param value Ether value of module transaction. + * @param data Data payload of module transaction. + * @param operation Operation type of module transaction. + * @return success Boolean flag indicating if the call succeeded. + * @return returnData Data returned by the call. + */ + function execTransactionFromModuleReturnData( + address to, + uint256 value, + bytes memory data, + Enum.Operation operation + ) external returns (bool success, bytes memory returnData); + + /** + * @notice Returns if an module is enabled + * @return True if the module is enabled + */ + function isModuleEnabled(address module) external view returns (bool); + + /** + * @notice Returns an array of modules. + * If all entries fit into a single page, the next pointer will be 0x1. + * If another page is present, next will be the last element of the returned array. + * @param start Start of the page. Has to be a module or start pointer (0x1 address) + * @param pageSize Maximum number of modules that should be returned. Has to be > 0 + * @return array Array of modules. + * @return next Start of the next page. + */ + function getModulesPaginated(address start, uint256 pageSize) external view returns (address[] memory array, address next); +} diff --git a/contracts/interfaces/IOwnerManager.sol b/contracts/interfaces/IOwnerManager.sol new file mode 100644 index 000000000..17942a37b --- /dev/null +++ b/contracts/interfaces/IOwnerManager.sol @@ -0,0 +1,63 @@ +// SPDX-License-Identifier: LGPL-3.0-only +pragma solidity >=0.7.0 <0.9.0; + +/** + * @title IOwnerManager - Interface for contract which manages Safe owners and a threshold to authorize transactions. + * @author @safe-global/safe-protocol + */ +interface IOwnerManager { + event AddedOwner(address indexed owner); + event RemovedOwner(address indexed owner); + event ChangedThreshold(uint256 threshold); + + /** + * @notice Adds the owner `owner` to the Safe and updates the threshold to `_threshold`. + * @dev This can only be done via a Safe transaction. + * @param owner New owner address. + * @param _threshold New threshold. + */ + function addOwnerWithThreshold(address owner, uint256 _threshold) external; + + /** + * @notice Removes the owner `owner` from the Safe and updates the threshold to `_threshold`. + * @dev This can only be done via a Safe transaction. + * @param prevOwner Owner that pointed to the owner to be removed in the linked list + * @param owner Owner address to be removed. + * @param _threshold New threshold. + */ + function removeOwner(address prevOwner, address owner, uint256 _threshold) external; + + /** + * @notice Replaces the owner `oldOwner` in the Safe with `newOwner`. + * @dev This can only be done via a Safe transaction. + * @param prevOwner Owner that pointed to the owner to be replaced in the linked list + * @param oldOwner Owner address to be replaced. + * @param newOwner New owner address. + */ + function swapOwner(address prevOwner, address oldOwner, address newOwner) external; + + /** + * @notice Changes the threshold of the Safe to `_threshold`. + * @dev This can only be done via a Safe transaction. + * @param _threshold New threshold. + */ + function changeThreshold(uint256 _threshold) external; + + /** + * @notice Returns the number of required confirmations for a Safe transaction aka the threshold. + * @return Threshold number. + */ + function getThreshold() external view returns (uint256); + + /** + * @notice Returns if `owner` is an owner of the Safe. + * @return Boolean if owner is an owner of the Safe. + */ + function isOwner(address owner) external view returns (bool); + + /** + * @notice Returns a list of Safe owners. + * @return Array of Safe owners. + */ + function getOwners() external view returns (address[] memory); +} diff --git a/contracts/interfaces/ISafe.sol b/contracts/interfaces/ISafe.sol new file mode 100644 index 000000000..98e32c9f7 --- /dev/null +++ b/contracts/interfaces/ISafe.sol @@ -0,0 +1,179 @@ +// SPDX-License-Identifier: LGPL-3.0-only +pragma solidity >=0.7.0 <0.9.0; + +import {Enum} from "../libraries/Enum.sol"; +import {IModuleManager} from "./IModuleManager.sol"; +import {IOwnerManager} from "./IOwnerManager.sol"; +import {IFallbackManager} from "./IFallbackManager.sol"; + +/** + * @title ISafe - A multisignature wallet interface with support for confirmations using signed messages based on EIP-712. + * @author @safe-global/safe-protocol + */ +interface ISafe is IModuleManager, IOwnerManager, IFallbackManager { + event SafeSetup(address indexed initiator, address[] owners, uint256 threshold, address initializer, address fallbackHandler); + event ApproveHash(bytes32 indexed approvedHash, address indexed owner); + event SignMsg(bytes32 indexed msgHash); + event ExecutionFailure(bytes32 indexed txHash, uint256 payment); + event ExecutionSuccess(bytes32 indexed txHash, uint256 payment); + + /** + * @notice Sets an initial storage of the Safe contract. + * @dev This method can only be called once. + * If a proxy was created without setting up, anyone can call setup and claim the proxy. + * @param _owners List of Safe owners. + * @param _threshold Number of required confirmations for a Safe transaction. + * @param to Contract address for optional delegate call. + * @param data Data payload for optional delegate call. + * @param fallbackHandler Handler for fallback calls to this contract + * @param paymentToken Token that should be used for the payment (0 is ETH) + * @param payment Value that should be paid + * @param paymentReceiver Address that should receive the payment (or 0 if tx.origin) + */ + function setup( + address[] calldata _owners, + uint256 _threshold, + address to, + bytes calldata data, + address fallbackHandler, + address paymentToken, + uint256 payment, + address payable paymentReceiver + ) external; + + /** @notice Executes a `operation` {0: Call, 1: DelegateCall}} transaction to `to` with `value` (Native Currency) + * and pays `gasPrice` * `gasLimit` in `gasToken` token to `refundReceiver`. + * @dev The fees are always transferred, even if the user transaction fails. + * This method doesn't perform any sanity check of the transaction, such as: + * - if the contract at `to` address has code or not + * - if the `gasToken` is a contract or not + * It is the responsibility of the caller to perform such checks. + * @param to Destination address of Safe transaction. + * @param value Ether value of Safe transaction. + * @param data Data payload of Safe transaction. + * @param operation Operation type of Safe transaction. + * @param safeTxGas Gas that should be used for the Safe transaction. + * @param baseGas Gas costs that are independent of the transaction execution(e.g. base transaction fee, signature check, payment of the refund) + * @param gasPrice Gas price that should be used for the payment calculation. + * @param gasToken Token address (or 0 if ETH) that is used for the payment. + * @param refundReceiver Address of receiver of gas payment (or 0 if tx.origin). + * @param signatures Signature data that should be verified. + * Can be packed ECDSA signature ({bytes32 r}{bytes32 s}{uint8 v}), contract signature (EIP-1271) or approved hash. + * @return success Boolean indicating transaction's success. + */ + 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 returns (bool success); + + /** + * @notice Checks whether the signature provided is valid for the provided data and hash. Reverts otherwise. + * @param dataHash Hash of the data (could be either a message hash or transaction hash) + * @param signatures Signature data that should be verified. + * Can be packed ECDSA signature ({bytes32 r}{bytes32 s}{uint8 v}), contract signature (EIP-1271) or approved hash. + */ + function checkSignatures(bytes32 dataHash, bytes memory signatures) external view; + + /** + * @notice Checks whether the signature provided is valid for the provided data and hash. Reverts otherwise. + * @param dataHash Hash of the data (could be either a message hash or transaction hash) + * @param signatures Signature data that should be verified. + * Can be packed ECDSA signature ({bytes32 r}{bytes32 s}{uint8 v}), contract signature (EIP-1271) or approved hash. + * @dev This function makes it compatible with previous versions. + */ + function checkSignatures(bytes32 dataHash, bytes memory /* IGNORED */, bytes memory signatures) external view; + + /** + * @notice Checks whether the signature provided is valid for the provided data and hash. Reverts otherwise. + * @dev Since the EIP-1271 does an external call, be mindful of reentrancy attacks. + * @param executor Address that executing the transaction. + * ⚠️⚠️⚠️ Make sure that the executor address is a legitmate executor. + * Incorrectly passed the executor might reduce the threshold by 1 signature. ⚠️⚠️⚠️ + * @param dataHash Hash of the data (could be either a message hash or transaction hash) + * @param signatures Signature data that should be verified. + * Can be packed ECDSA signature ({bytes32 r}{bytes32 s}{uint8 v}), contract signature (EIP-1271) or approved hash. + * @param requiredSignatures Amount of required valid signatures. + */ + function checkNSignatures(address executor, bytes32 dataHash, bytes memory signatures, uint256 requiredSignatures) external view; + + /** + * @notice Marks hash `hashToApprove` as approved. + * @dev This can be used with a pre-approved hash transaction signature. + * IMPORTANT: The approved hash stays approved forever. There's no revocation mechanism, so it behaves similarly to ECDSA signatures + * @param hashToApprove The hash to mark as approved for signatures that are verified by this contract. + */ + function approveHash(bytes32 hashToApprove) external; + + /** + * @dev Returns the domain separator for this contract, as defined in the EIP-712 standard. + * @return bytes32 The domain separator hash. + */ + function domainSeparator() external view returns (bytes32); + + /** + * @notice Returns transaction hash to be signed by owners. + * @param to Destination address. + * @param value Ether value. + * @param data Data payload. + * @param operation Operation type. + * @param safeTxGas Gas that should be used for the safe transaction. + * @param baseGas Gas costs for data used to trigger the safe transaction. + * @param gasPrice Maximum gas price that should be used for this transaction. + * @param gasToken Token address (or 0 if ETH) that is used for the payment. + * @param refundReceiver Address of receiver of gas payment (or 0 if tx.origin). + * @param _nonce Transaction nonce. + * @return Transaction hash. + */ + function getTransactionHash( + address to, + uint256 value, + bytes calldata data, + Enum.Operation operation, + uint256 safeTxGas, + uint256 baseGas, + uint256 gasPrice, + address gasToken, + address refundReceiver, + uint256 _nonce + ) external view returns (bytes32); + + /** + * External getter function for state variables. + */ + + /** + * @notice Returns the version of the Safe contract. + * @return Version string. + */ + // solhint-disable-next-line + function VERSION() external view returns (string memory); + + /** + * @notice Returns the nonce of the Safe contract. + * @return Nonce. + */ + function nonce() external view returns (uint256); + + /** + * @notice Returns a uint if the messageHash is signed by the owner. + * @param messageHash Hash of message that should be checked. + * @return Number denoting if an owner signed the hash. + */ + function signedMessages(bytes32 messageHash) external view returns (uint256); + + /** + * @notice Returns a uint if the messageHash is approved by the owner. + * @param owner Owner address that should be checked. + * @param messageHash Hash of message that should be checked. + * @return Number denoting if an owner approved the hash. + */ + function approvedHashes(address owner, bytes32 messageHash) external view returns (uint256); +} diff --git a/contracts/common/Enum.sol b/contracts/libraries/Enum.sol similarity index 75% rename from contracts/common/Enum.sol rename to contracts/libraries/Enum.sol index 9ce0f5d53..4247232d8 100644 --- a/contracts/common/Enum.sol +++ b/contracts/libraries/Enum.sol @@ -3,9 +3,9 @@ pragma solidity >=0.7.0 <0.9.0; /** * @title Enum - Collection of enums used in Safe contracts. - * @author Richard Meissner - @rmeissner + * @author @safe-global/safe-protocol */ -abstract contract Enum { +library Enum { enum Operation { Call, DelegateCall diff --git a/contracts/libraries/Safe130To141Migration.sol b/contracts/libraries/Safe130To141Migration.sol index 8596a16c7..688fa7704 100644 --- a/contracts/libraries/Safe130To141Migration.sol +++ b/contracts/libraries/Safe130To141Migration.sol @@ -3,10 +3,7 @@ pragma solidity >=0.7.0 <0.9.0; import {SafeStorage} from "../libraries/SafeStorage.sol"; - -interface ISafe { - function setFallbackHandler(address handler) external; -} +import {ISafe} from "../interfaces/ISafe.sol"; /** * @title Migration Contract for Safe Upgrade diff --git a/contracts/libraries/Safe150Migration.sol b/contracts/libraries/Safe150Migration.sol index 251755c85..a6db8571e 100644 --- a/contracts/libraries/Safe150Migration.sol +++ b/contracts/libraries/Safe150Migration.sol @@ -4,13 +4,7 @@ pragma solidity >=0.7.0 <0.9.0; import {SafeStorage} from "../libraries/SafeStorage.sol"; import {Guard} from "../base/GuardManager.sol"; - -// Interface for interacting with the Safe contract -interface ISafe { - function setFallbackHandler(address handler) external; - - function setGuard(address guard) external; -} +import {ISafe} from "../interfaces/ISafe.sol"; /** * @title Migration Contract for Safe Upgrade diff --git a/contracts/libraries/SafeToL2Migration.sol b/contracts/libraries/SafeToL2Migration.sol index 34a475085..0621370d5 100644 --- a/contracts/libraries/SafeToL2Migration.sol +++ b/contracts/libraries/SafeToL2Migration.sol @@ -3,18 +3,8 @@ pragma solidity >=0.7.0 <0.9.0; import {SafeStorage} from "../libraries/SafeStorage.sol"; -import {Enum} from "../common/Enum.sol"; - -interface ISafe { - // solhint-disable-next-line - function VERSION() external view returns (string memory); - - function setFallbackHandler(address handler) external; - - function getOwners() external view returns (address[] memory); - - function getThreshold() external view returns (uint256); -} +import {Enum} from "../libraries/Enum.sol"; +import {ISafe} from "../interfaces/ISafe.sol"; /** * @title Migration Contract for updating a Safe from 1.1.1/1.3.0/1.4.1 versions to a L2 version. Useful when replaying a Safe from a non L2 network in a L2 network. diff --git a/contracts/libraries/SignMessageLib.sol b/contracts/libraries/SignMessageLib.sol index 34d122fc7..35dab60eb 100644 --- a/contracts/libraries/SignMessageLib.sol +++ b/contracts/libraries/SignMessageLib.sol @@ -2,7 +2,7 @@ pragma solidity >=0.7.0 <0.9.0; import {SafeStorage} from "./SafeStorage.sol"; -import {Safe} from "../Safe.sol"; +import {ISafe} from "../interfaces/ISafe.sol"; /** * @title SignMessageLib - Allows to sign messages on-chain by writing the signed message hashes on-chain. @@ -32,6 +32,6 @@ contract SignMessageLib is SafeStorage { */ function getMessageHash(bytes memory message) public view returns (bytes32) { bytes32 safeMessageHash = keccak256(abi.encode(SAFE_MSG_TYPEHASH, keccak256(message))); - return keccak256(abi.encodePacked(bytes1(0x19), bytes1(0x01), Safe(payable(address(this))).domainSeparator(), safeMessageHash)); + return keccak256(abi.encodePacked(bytes1(0x19), bytes1(0x01), ISafe(payable(address(this))).domainSeparator(), safeMessageHash)); } } diff --git a/package.json b/package.json index 0c102d09c..00241cc54 100644 --- a/package.json +++ b/package.json @@ -20,6 +20,7 @@ "test": "hardhat test", "test:parallel": "hardhat test --parallel", "coverage": "hardhat coverage", + "codesize": "hardhat codesize", "benchmark": "npm run test benchmark/*.ts", "deploy-custom": "rm -rf deployments/custom && npm run deploy-all custom", "deploy-all": "hardhat deploy-contracts --network",