diff --git a/implementations/.prettierrc b/implementations/.prettierrc index 034b7b16..c63dee4f 100644 --- a/implementations/.prettierrc +++ b/implementations/.prettierrc @@ -9,6 +9,14 @@ "trailingComma": "all", "singleQuote": true } + }, + { + "files": "*.sol", + "options": { + "tabWidth": 4, + "printWidth": 100, + "compiler": "0.8.10" + } } ] } diff --git a/implementations/contracts/ERC725XCore.sol b/implementations/contracts/ERC725XCore.sol index 7e821bdc..b1e250fa 100644 --- a/implementations/contracts/ERC725XCore.sol +++ b/implementations/contracts/ERC725XCore.sol @@ -33,8 +33,6 @@ import { * This is the basis for a smart contract based account system, but could also be used as a proxy account system */ abstract contract ERC725XCore is OwnableUnset, ERC165, IERC725X { - /* Public functions */ - /** * @inheritdoc IERC725X */ @@ -42,175 +40,172 @@ abstract contract ERC725XCore is OwnableUnset, ERC165, IERC725X { uint256 operation, address to, uint256 value, - bytes calldata data - ) public payable virtual override onlyOwner returns (bytes memory result) { - uint256 txGas = gasleft(); + bytes memory data + ) public payable virtual override onlyOwner returns (bytes memory) { + + require(address(this).balance >= value, "ERC725X: insufficient balance"); // CALL - if (operation == OPERATION_CALL) { - require(address(this).balance >= value, "ERC725X: insufficient balance for call"); - - result = executeCall(to, value, data, txGas); - - emit Executed(operation, to, value, bytes4(data)); - - // STATICCALL - } else if (operation == OPERATION_STATICCALL) { - require(value == 0, "ERC725X: cannot transfer value with operation STATICCALL"); - - result = executeStaticCall(to, data, txGas); - - emit Executed(operation, to, value, bytes4(data)); - - // DELEGATECALL - // WARNING! - // delegatecall is a dangerous operation type! - // - // delegate allows to call another deployed contract and use its functions - // to update the state of the current calling contract - // - // this can lead to unexpected behaviour on the contract storage, such as: - // - // - updating any state variables (even if these are protected) - // - update the contract owner - // - run selfdestruct in the context of this contract - // - // use with EXTRA CAUTION - } else if (operation == OPERATION_DELEGATECALL) { - require(value == 0, "ERC725X: cannot transfer value with operation DELEGATECALL"); - - result = executeDelegateCall(to, data, txGas); - - emit Executed(operation, to, value, bytes4(data)); - - // CREATE - } else if (operation == OPERATION_CREATE) { - require( - to == address(0), - "ERC725X: CREATE operations require the receiver address to be empty" - ); - require(address(this).balance >= value, "ERC725X: insufficient balance for call"); - - address contractAddress = performCreate(value, data); - result = abi.encodePacked(contractAddress); - - emit ContractCreated(operation, contractAddress, value); - - // CREATE2 - } else if (operation == OPERATION_CREATE2) { - require( - to == address(0), - "ERC725X: CREATE operations require the receiver address to be empty" - ); - require(address(this).balance >= value, "ERC725X: insufficient balance for call"); - - bytes32 salt = BytesLib.toBytes32(data, data.length - 32); - bytes memory bytecode = BytesLib.slice(data, 0, data.length - 32); - - address contractAddress = Create2.deploy(value, salt, bytecode); - result = abi.encodePacked(contractAddress); - - emit ContractCreated(operation, contractAddress, value); - } else { - revert("Wrong operation type"); - } + if (operation == OPERATION_CALL) return _executeCall(to, value, data); + + // Deploy with CREATE + if (operation == OPERATION_CREATE) return _deployCreate(to, value, data); + + // Deploy with CREATE2 + if (operation == OPERATION_CREATE2) return _deployCreate2(to, value, data); + + // STATICCALL + if (operation == OPERATION_STATICCALL) return _executeStaticCall(to, value, data); + + // DELEGATECALL + // + // WARNING! delegatecall is a dangerous operation type! use with EXTRA CAUTION + // + // delegate allows to call another deployed contract and use its functions + // to update the state of the current calling contract. + // + // this can lead to unexpected behaviour on the contract storage, such as: + // - updating any state variables (even if these are protected) + // - update the contract owner + // - run selfdestruct in the context of this contract + // + if (operation == OPERATION_DELEGATECALL) return _executeDelegateCall(to, value, data); + + revert("ERC725X: Unknown operation type"); + } + + /* Overrides functions */ + + /** + * @inheritdoc ERC165 + */ + function supportsInterface(bytes4 interfaceId) + public + view + virtual + override(IERC165, ERC165) + returns (bool) + { + return interfaceId == _INTERFACEID_ERC725X || super.supportsInterface(interfaceId); } /* Internal functions */ /** * @dev perform call using operation 0 - * Taken from GnosisSafe: https://github.com/gnosis/safe-contracts/blob/main/contracts/base/Executor.sol - * * @param to The address on which call is executed * @param value The value to be sent with the call * @param data The data to be sent with the call - * @param txGas The amount of gas for performing call * @return result The data from the call */ - function executeCall( + function _executeCall( address to, uint256 value, - bytes memory data, - uint256 txGas - ) internal returns (bytes memory result) { - // solhint-disable avoid-low-level-calls - (bool success, bytes memory returnData) = to.call{gas: txGas, value: value}(data); + bytes memory data + ) internal virtual returns (bytes memory result) { + // solhint-disable avoid-low-level-calls + (bool success, bytes memory returnData) = to.call{value: value}(data); result = Address.verifyCallResult(success, returnData, "ERC725X: Unknown Error"); + + emit Executed(OPERATION_CALL, to, value, bytes4(data)); } /** * @dev perform staticcall using operation 3 * @param to The address on which staticcall is executed - * @param data The data to be sent with the call - * @param txGas The amount of gas for performing staticcall - * @return result The data from the call + * @param value The value passed to the execute(...) function (MUST be 0) + * @param data The data to be sent with the staticcall + * @return result The data returned from the staticcall */ - function executeStaticCall( + function _executeStaticCall( address to, - bytes memory data, - uint256 txGas - ) internal view returns (bytes memory result) { - (bool success, bytes memory returnData) = to.staticcall{gas: txGas}(data); + uint256 value, + bytes memory data + ) internal virtual returns (bytes memory result) { + require(value == 0, "ERC725X: cannot transfer value with operation STATICCALL"); + // solhint-disable avoid-low-level-calls + (bool success, bytes memory returnData) = to.staticcall(data); result = Address.verifyCallResult(success, returnData, "ERC725X: Unknown Error"); + + emit Executed(OPERATION_STATICCALL, to, value, bytes4(data)); } /** * @dev perform delegatecall using operation 4 - * Taken from GnosisSafe: https://github.com/gnosis/safe-contracts/blob/main/contracts/base/Executor.sol - * * @param to The address on which delegatecall is executed - * @param data The data to be sent with the call - * @param txGas The amount of gas for performing delegatecall - * @return result The data from the call + * @param value The value passed to the execute(...) function (MUST be 0) + * @param data The data to be sent with the delegatecall + * @return result The data returned from the delegatecall */ - function executeDelegateCall( + function _executeDelegateCall( address to, - bytes memory data, - uint256 txGas - ) internal returns (bytes memory result) { - // solhint-disable avoid-low-level-calls - (bool success, bytes memory returnData) = to.delegatecall{gas: txGas}(data); + uint256 value, + bytes memory data + ) internal virtual returns (bytes memory result) { + require(value == 0, "ERC725X: cannot transfer value with operation DELEGATECALL"); + // solhint-disable avoid-low-level-calls + (bool success, bytes memory returnData) = to.delegatecall(data); result = Address.verifyCallResult(success, returnData, "ERC725X: Unknown Error"); + + emit Executed(OPERATION_DELEGATECALL, to, value, bytes4(data)); } /** * @dev perform contract creation using operation 1 - * Taken from GnosisSafe: https://github.com/gnosis/safe-contracts/blob/main/contracts/libraries/CreateCall.sol - * + * @param to The recipient address passed to execute(...) (MUST be address(0) for CREATE) * @param value The value to be sent to the contract created - * @param deploymentData The contract bytecode to deploy - * @return newContract The address of the contract created + * @param data The contract bytecode to deploy + * @return newContract The address of the contract created as bytes */ - function performCreate(uint256 value, bytes memory deploymentData) - internal - returns (address newContract) - { - require(deploymentData.length != 0, "no contract bytecode provided"); - + function _deployCreate( + address to, + uint256 value, + bytes memory data + ) internal virtual returns (bytes memory newContract) { + require( + to == address(0), + "ERC725X: CREATE operations require the receiver address to be empty" + ); + require(data.length != 0, "ERC725X: No contract bytecode provided"); + + address contractAddress; // solhint-disable no-inline-assembly assembly { - newContract := create(value, add(deploymentData, 0x20), mload(deploymentData)) + contractAddress := create(value, add(data, 0x20), mload(data)) } - require(newContract != address(0), "Could not deploy contract"); - } + require(contractAddress != address(0), "ERC725X: Could not deploy contract"); - /* Overrides functions */ + newContract = abi.encodePacked(contractAddress); + emit ContractCreated(OPERATION_CREATE, contractAddress, value); + } /** - * @inheritdoc ERC165 + * @dev perform contract creation using operation 2 + * @param to The recipient address passed to execute(...) (MUST be address(0) for CREATE2) + * @param value The value to be sent to the contract created + * @param data The contract bytecode to deploy appended with a bytes32 salt + * @return newContract The address of the contract created as bytes */ - function supportsInterface(bytes4 interfaceId) - public - view - virtual - override(IERC165, ERC165) - returns (bool) - { - return interfaceId == _INTERFACEID_ERC725X || super.supportsInterface(interfaceId); + function _deployCreate2( + address to, + uint256 value, + bytes memory data + ) internal virtual returns (bytes memory newContract) { + require( + to == address(0), + "ERC725X: CREATE operations require the receiver address to be empty" + ); + require(data.length != 0, "ERC725X: No contract bytecode provided"); + + bytes32 salt = BytesLib.toBytes32(data, data.length - 32); + bytes memory bytecode = BytesLib.slice(data, 0, data.length - 32); + address contractAddress = Create2.deploy(value, salt, bytecode); + + newContract = abi.encodePacked(contractAddress); + emit ContractCreated(OPERATION_CREATE2, contractAddress, value); } } diff --git a/implementations/test/ERC725X.behaviour.ts b/implementations/test/ERC725X.behaviour.ts index 9f165b15..bf7272da 100644 --- a/implementations/test/ERC725X.behaviour.ts +++ b/implementations/test/ERC725X.behaviour.ts @@ -90,6 +90,7 @@ export const shouldBehaveLikeERC725X = ( context = await buildContext(); provider = ethers.provider; abiCoder = new ethers.utils.AbiCoder(); + // fund erc725X contract valueToSend = ethers.utils.parseEther("50"); await context.erc725X @@ -509,7 +510,7 @@ export const shouldBehaveLikeERC725X = ( txParams.value, txParams.data ) - ).to.be.revertedWith("ERC725X: insufficient balance for call"); + ).to.be.revertedWith("ERC725X: insufficient balance"); }); }); }); @@ -952,7 +953,7 @@ export const shouldBehaveLikeERC725X = ( txParams.value, txParams.data ) - ).to.be.revertedWith("Could not deploy contract"); + ).to.be.revertedWith("ERC725X: Could not deploy contract"); }); }); @@ -1051,7 +1052,7 @@ export const shouldBehaveLikeERC725X = ( txParams.value, txParams.data ) - ).to.be.revertedWith("ERC725X: insufficient balance for call"); + ).to.be.revertedWith("ERC725X: insufficient balance"); }); }); @@ -1073,7 +1074,7 @@ export const shouldBehaveLikeERC725X = ( txParams.value, txParams.data ) - ).to.be.revertedWith("Could not deploy contract"); + ).to.be.revertedWith("ERC725X: Could not deploy contract"); }); }); }); @@ -1405,7 +1406,7 @@ export const shouldBehaveLikeERC725X = ( txParams.value, txParams.data ) - ).to.be.revertedWith("ERC725X: insufficient balance for call"); + ).to.be.revertedWith("ERC725X: insufficient balance"); }); }); @@ -1917,7 +1918,7 @@ export const shouldBehaveLikeERC725X = ( txParams.value, txParams.data ) - ).to.be.revertedWith("Wrong operation type"); + ).to.be.revertedWith("ERC725X: Unknown operation type"); }); }); });