diff --git a/.env.sample b/.env.sample index 1f4e2e39a..266d006c8 100644 --- a/.env.sample +++ b/.env.sample @@ -10,4 +10,8 @@ ERC4337_TEST_NODE_URL= ERC4337_TEST_SINGLETON_ADDRESS= ERC4337_TEST_SAFE_FACTORY_ADDRESS= # (Optional) Tells the test runner which Safe Singleton Contract to use for testing: Safe or SafeL2. Defaults to Safe. -SAFE_CONTRACT_UNDER_TEST="Safe" \ No newline at end of file +SAFE_CONTRACT_UNDER_TEST="Safe" +# Used for compiling with different solidity testing +SOLIDITY_VERSION= # Example: '0.8.19' +# For running coverage tests, `details` section of solidity settings are required, else could be removed. +SOLIDITY_SETTINGS= # Example: '{"viaIR":true,"optimizer":{"enabled":true, "details": {"yul": true, "yulDetails": { "optimizerSteps": ""}}}}' diff --git a/.gitignore b/.gitignore index 06b33e083..336c172c1 100644 --- a/.gitignore +++ b/.gitignore @@ -13,6 +13,7 @@ coverage/ coverage.json yarn-error.log typechain-types +.vscode # Certora Formal Verification related files .certora_internal diff --git a/certora/specs/Safe.spec b/certora/specs/Safe.spec index b0e59ac0e..21e9a28c9 100644 --- a/certora/specs/Safe.spec +++ b/certora/specs/Safe.spec @@ -65,7 +65,7 @@ hook Sstore SafeHarness.(slot 24440054405305269366569402256811496959409073762505 ghostSingletonAddress = newSingletonAddress; } -invariant sigletonAddressNeverChanges() +invariant singletonAddressNeverChanges() ghostSingletonAddress == 0 filtered { f -> reachableOnly(f) && f.selector != sig:getStorageAt(uint256,uint256).selector } diff --git a/contracts/base/FallbackManager.sol b/contracts/base/FallbackManager.sol index b700f0baf..67d59d557 100644 --- a/contracts/base/FallbackManager.sol +++ b/contracts/base/FallbackManager.sol @@ -69,33 +69,26 @@ abstract contract FallbackManager is SelfAuthorized { // memory is used, therefore we need to guarantee memory safety (keeping the free memory point 0x40 slot intact, // not going beyond the scratch space, etc) // Solidity docs: https://docs.soliditylang.org/en/latest/assembly.html#memory-safety - function allocate(length) -> pos { - pos := mload(0x40) - mstore(0x40, add(pos, length)) - } - let handler := sload(slot) if iszero(handler) { return(0, 0) } - let calldataPtr := allocate(calldatasize()) - calldatacopy(calldataPtr, 0, calldatasize()) + let ptr := mload(0x40) + calldatacopy(ptr, 0, calldatasize()) // The msg.sender address is shifted to the left by 12 bytes to remove the padding // Then the address without padding is stored right after the calldata - let senderPtr := allocate(20) - mstore(senderPtr, shl(96, caller())) + mstore(add(ptr, calldatasize()), shl(96, caller())) // Add 20 bytes for the address appended add the end - let success := call(gas(), handler, 0, calldataPtr, add(calldatasize(), 20), 0, 0) + let success := call(gas(), handler, 0, ptr, add(calldatasize(), 20), 0, 0) - let returnDataPtr := allocate(returndatasize()) - returndatacopy(returnDataPtr, 0, returndatasize()) + returndatacopy(ptr, 0, returndatasize()) if iszero(success) { - revert(returnDataPtr, returndatasize()) + revert(ptr, returndatasize()) } - return(returnDataPtr, returndatasize()) + return(ptr, returndatasize()) } /* solhint-enable no-inline-assembly */ } diff --git a/contracts/base/ModuleManager.sol b/contracts/base/ModuleManager.sol index ead1c3748..cdd4ad0cc 100644 --- a/contracts/base/ModuleManager.sol +++ b/contracts/base/ModuleManager.sol @@ -123,16 +123,14 @@ abstract contract ModuleManager is SelfAuthorized, Executor, GuardManager { /// @solidity memory-safe-assembly assembly { // Load free memory location - let ptr := mload(0x40) + returnData := mload(0x40) // We allocate memory for the return data by setting the free memory location to // current free memory location + data size + 32 bytes for data size value - mstore(0x40, add(ptr, add(returndatasize(), 0x20))) + mstore(0x40, add(returnData, add(returndatasize(), 0x20))) // Store the size - mstore(ptr, returndatasize()) + mstore(returnData, returndatasize()) // Store the data - returndatacopy(add(ptr, 0x20), 0, returndatasize()) - // Point the return data to the correct memory location - returnData := ptr + returndatacopy(add(returnData, 0x20), 0, returndatasize()) } /* solhint-enable no-inline-assembly */ } diff --git a/contracts/common/SecuredTokenTransfer.sol b/contracts/common/SecuredTokenTransfer.sol index bddffd831..acf4f5187 100644 --- a/contracts/common/SecuredTokenTransfer.sol +++ b/contracts/common/SecuredTokenTransfer.sol @@ -18,7 +18,8 @@ abstract contract SecuredTokenTransfer { function transferToken(address token, address receiver, uint256 amount) internal returns (bool transferred) { // 0xa9059cbb - keccack("transfer(address,uint256)") bytes memory data = abi.encodeWithSelector(0xa9059cbb, receiver, amount); - // solhint-disable-next-line no-inline-assembly + /* solhint-disable no-inline-assembly */ + /// @solidity memory-safe-assembly assembly { // We write the return value to scratch space. // See https://docs.soliditylang.org/en/v0.7.6/internals/layout_in_memory.html#layout-in-memory @@ -34,5 +35,6 @@ abstract contract SecuredTokenTransfer { transferred := 0 } } + /* solhint-enable no-inline-assembly */ } } diff --git a/contracts/handler/CompatibilityFallbackHandler.sol b/contracts/handler/CompatibilityFallbackHandler.sol index 49f52e658..0d06284ce 100644 --- a/contracts/handler/CompatibilityFallbackHandler.sol +++ b/contracts/handler/CompatibilityFallbackHandler.sol @@ -97,12 +97,13 @@ contract CompatibilityFallbackHandler is TokenCallbackHandler, ISignatureValidat /* solhint-disable no-inline-assembly */ /// @solidity memory-safe-assembly assembly { - let internalCalldata := mload(0x40) + let ptr := mload(0x40) /** * Store `simulateAndRevert.selector`. * String representation is used to force right padding */ - mstore(internalCalldata, "\xb4\xfa\xba\x09") + mstore(ptr, "\xb4\xfa\xba\x09") + /** * Abuse the fact that both this and the internal methods have the * same signature, and differ only in symbol name (and therefore, @@ -110,7 +111,7 @@ contract CompatibilityFallbackHandler is TokenCallbackHandler, ISignatureValidat * 250 bytes of code and 300 gas at runtime over the * `abi.encodeWithSelector` builtin. */ - calldatacopy(add(internalCalldata, 0x04), 0x04, sub(calldatasize(), 0x04)) + calldatacopy(add(ptr, 0x04), 0x04, sub(calldatasize(), 0x04)) /** * `pop` is required here by the compiler, as top level expressions @@ -124,7 +125,7 @@ contract CompatibilityFallbackHandler is TokenCallbackHandler, ISignatureValidat // address() has been changed to caller() to use the implementation of the Safe caller(), 0, - internalCalldata, + ptr, calldatasize(), /** * The `simulateAndRevert` call always reverts, and diff --git a/contracts/interfaces/ViewStorageAccessible.sol b/contracts/interfaces/ViewStorageAccessible.sol index 6864aee5a..3c23a523c 100644 --- a/contracts/interfaces/ViewStorageAccessible.sol +++ b/contracts/interfaces/ViewStorageAccessible.sol @@ -1,3 +1,4 @@ +// SPDX-License-Identifier: LGPL-3.0-only pragma solidity >=0.5.0 <0.9.0; /// @title ViewStorageAccessible - Interface on top of StorageAccessible base class to allow simulations from view functions. diff --git a/contracts/libraries/MultiSend.sol b/contracts/libraries/MultiSend.sol index 1762f651a..78ee6cfc5 100644 --- a/contracts/libraries/MultiSend.sol +++ b/contracts/libraries/MultiSend.sol @@ -30,7 +30,6 @@ contract MultiSend { function multiSend(bytes memory transactions) public payable { require(address(this) != MULTISEND_SINGLETON, "MultiSend should only be called via delegatecall"); /* solhint-disable no-inline-assembly */ - /// @solidity memory-safe-assembly assembly { let length := mload(transactions) let i := 0x20 diff --git a/contracts/libraries/MultiSendCallOnly.sol b/contracts/libraries/MultiSendCallOnly.sol index 1fc1df886..362723787 100644 --- a/contracts/libraries/MultiSendCallOnly.sol +++ b/contracts/libraries/MultiSendCallOnly.sol @@ -24,7 +24,6 @@ contract MultiSendCallOnly { */ function multiSend(bytes memory transactions) public payable { /* solhint-disable no-inline-assembly */ - /// @solidity memory-safe-assembly assembly { let length := mload(transactions) let i := 0x20 diff --git a/contracts/libraries/Safe130To141Migration.sol b/contracts/libraries/Safe130To141Migration.sol index d1a021b9e..8596a16c7 100644 --- a/contracts/libraries/Safe130To141Migration.sol +++ b/contracts/libraries/Safe130To141Migration.sol @@ -125,10 +125,12 @@ contract Safe130To141Migration is SafeStorage { */ function isContract(address account) internal view returns (bool) { uint256 size; - // solhint-disable-next-line no-inline-assembly + /* solhint-disable no-inline-assembly */ + /// @solidity memory-safe-assembly assembly { size := extcodesize(account) } + /* solhint-enable no-inline-assembly */ // If the code size is greater than 0, it is a contract; otherwise, it is an EOA. return size > 0; diff --git a/contracts/libraries/Safe150Migration.sol b/contracts/libraries/Safe150Migration.sol index 949c90bad..251755c85 100644 --- a/contracts/libraries/Safe150Migration.sol +++ b/contracts/libraries/Safe150Migration.sol @@ -160,10 +160,12 @@ contract Safe150Migration is SafeStorage { */ function getGuard() internal view returns (address guard) { bytes32 slot = GUARD_STORAGE_SLOT; - // solhint-disable-next-line no-inline-assembly + /* solhint-disable no-inline-assembly */ + /// @solidity memory-safe-assembly assembly { guard := sload(slot) } + /* solhint-enable no-inline-assembly */ } /** @@ -176,10 +178,12 @@ contract Safe150Migration is SafeStorage { */ function isContract(address account) internal view returns (bool) { uint256 size; - // solhint-disable-next-line no-inline-assembly + /* solhint-disable no-inline-assembly */ + /// @solidity memory-safe-assembly assembly { size := extcodesize(account) } + /* solhint-enable no-inline-assembly */ // If the code size is greater than 0, it is a contract; otherwise, it is an EOA. return size > 0; diff --git a/contracts/libraries/SafeToL2Migration.sol b/contracts/libraries/SafeToL2Migration.sol index c4cf1f16c..00431df94 100644 --- a/contracts/libraries/SafeToL2Migration.sol +++ b/contracts/libraries/SafeToL2Migration.sol @@ -96,7 +96,7 @@ contract SafeToL2Migration is SafeStorage { 0, 0, address(0), - address(0), + payable(address(0)), "", // We cannot detect signatures additionalInfo ); @@ -166,10 +166,12 @@ contract SafeToL2Migration is SafeStorage { */ function isContract(address account) internal view returns (bool) { uint256 size; - // solhint-disable-next-line no-inline-assembly + /* solhint-disable no-inline-assembly */ + /// @solidity memory-safe-assembly assembly { size := extcodesize(account) } + /* solhint-enable no-inline-assembly */ // If the code size is greater than 0, it is a contract; otherwise, it is an EOA. return size > 0; diff --git a/contracts/test/DelegateCaller.sol b/contracts/test/DelegateCaller.sol index 1ed105a30..aa2fb060d 100644 --- a/contracts/test/DelegateCaller.sol +++ b/contracts/test/DelegateCaller.sol @@ -13,12 +13,13 @@ contract DelegateCaller { function makeDelegatecall(address _called, bytes memory _calldata) external returns (bool success, bytes memory returnData) { (success, returnData) = _called.delegatecall(_calldata); if (!success) { - // solhint-disable-next-line no-inline-assembly + /* solhint-disable no-inline-assembly */ assembly { let length := returndatasize() returndatacopy(0, 0, length) revert(0, length) } + /* solhint-enable no-inline-assembly */ } } } diff --git a/contracts/test/Token.sol b/contracts/test/Token.sol index b1eee15f0..5d7b0f75a 100644 --- a/contracts/test/Token.sol +++ b/contracts/test/Token.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: LGPL-3.0-only -pragma solidity >=0.6.0 <0.7.0; +pragma solidity >=0.6.0 <0.9.0; interface Token { function transfer(address _to, uint256 value) external returns (bool); diff --git a/docs/guidelines.md b/docs/guidelines.md index 6573e1509..2ffab2542 100644 --- a/docs/guidelines.md +++ b/docs/guidelines.md @@ -10,20 +10,24 @@ The data can be stored to this location with ``` bytes32 slot = VARIABLE_SLOT; -// solhint-disable-next-line no-inline-assembly +/* solhint-disable no-inline-assembly */ +/// @solidity memory-safe-assembly assembly { sstore(slot, value) } +/* solhint-enable no-inline-assembly */ ``` and read with ``` bytes32 slot = VARIABLE_SLOT; -// solhint-disable-next-line no-inline-assembly +/* solhint-disable no-inline-assembly */ +/// @solidity memory-safe-assembly assembly { value := sload(slot) } +/* solhint-enable no-inline-assembly */ ``` Note: Make sure to use a unique identifier else unexpected behaviour will occur diff --git a/hardhat.config.ts b/hardhat.config.ts index 6dd58ae52..b0d4933c5 100644 --- a/hardhat.config.ts +++ b/hardhat.config.ts @@ -39,7 +39,8 @@ import "./src/tasks/show_codesize"; import { BigNumber } from "@ethersproject/bignumber"; import { DeterministicDeploymentInfo } from "hardhat-deploy/dist/types"; -const primarySolidityVersion = SOLIDITY_VERSION || "0.7.6"; +const defaultSolidityVersion = "0.7.6"; +const primarySolidityVersion = SOLIDITY_VERSION || defaultSolidityVersion; const soliditySettings = SOLIDITY_SETTINGS ? JSON.parse(SOLIDITY_SETTINGS) : undefined; const deterministicDeployment = (network: string): DeterministicDeploymentInfo => { @@ -70,7 +71,7 @@ const userConfig: HardhatUserConfig = { target: "ethers-v6", }, solidity: { - compilers: [{ version: primarySolidityVersion, settings: soliditySettings }, { version: "0.6.12" }, { version: "0.5.17" }], + compilers: [{ version: primarySolidityVersion, settings: soliditySettings }, { version: defaultSolidityVersion }], }, networks: { hardhat: { diff --git a/package.json b/package.json index a9d41845f..0c102d09c 100644 --- a/package.json +++ b/package.json @@ -29,6 +29,7 @@ "lint:sol:prettier": "prettier 'contracts/**/*.sol' --list-different", "lint:ts": "eslint 'src/**/*.ts' 'test/**/*.ts' --fix", "lint:ts:prettier": "prettier 'src/**/*.ts' 'test/**/*.ts' --list-different", + "fmt": "npm run fmt:sol && npm run fmt:ts", "fmt:sol": "prettier 'contracts/**/*.sol' -w", "fmt:ts": "prettier 'src/**/*.ts' 'test/**/*.ts' -w", "prepack": "npm run build", diff --git a/test/core/Safe.Execution.spec.ts b/test/core/Safe.Execution.spec.ts index 5cadef1a2..0334c0624 100644 --- a/test/core/Safe.Execution.spec.ts +++ b/test/core/Safe.Execution.spec.ts @@ -22,10 +22,12 @@ describe("Safe", () => { contract StorageSetter { function setStorage(bytes3 data) public { bytes32 slot = 0x4242424242424242424242424242424242424242424242424242424242424242; - // solhint-disable-next-line no-inline-assembly + /* solhint-disable no-inline-assembly */ + /// @solidity memory-safe-assembly assembly { sstore(slot, data) } + /* solhint-enable no-inline-assembly */ } }`; const storageSetter = await deployContract(user1, setterSource); diff --git a/test/core/Safe.Setup.spec.ts b/test/core/Safe.Setup.spec.ts index 83e2af1ae..c41ffbec1 100644 --- a/test/core/Safe.Setup.spec.ts +++ b/test/core/Safe.Setup.spec.ts @@ -216,10 +216,12 @@ describe("Safe", () => { contract Initializer { function init(bytes4 data) public { bytes32 slot = 0x4242424242424242424242424242424242424242424242424242424242424242; - // solhint-disable-next-line no-inline-assembly + /* solhint-disable no-inline-assembly */ + /// @solidity memory-safe-assembly assembly { sstore(slot, data) } + /* solhint-enable no-inline-assembly */ } }`; const testIntializer = await deployContract(user1, source); diff --git a/test/libraries/MultiSend.spec.ts b/test/libraries/MultiSend.spec.ts index 63ef2ed8d..5833f47f1 100644 --- a/test/libraries/MultiSend.spec.ts +++ b/test/libraries/MultiSend.spec.ts @@ -18,10 +18,12 @@ describe("MultiSend", () => { contract StorageSetter { function setStorage(bytes3 data) public { bytes32 slot = 0x4242424242424242424242424242424242424242424242424242424242424242; - // solhint-disable-next-line no-inline-assembly + /* solhint-disable no-inline-assembly */ + /// @solidity memory-safe-assembly assembly { sstore(slot, data) } + /* solhint-enable no-inline-assembly */ } }`; const signers = await ethers.getSigners(); diff --git a/test/libraries/MultiSendCallOnly.spec.ts b/test/libraries/MultiSendCallOnly.spec.ts index 1d5ff490a..df4bf8bc5 100644 --- a/test/libraries/MultiSendCallOnly.spec.ts +++ b/test/libraries/MultiSendCallOnly.spec.ts @@ -18,10 +18,12 @@ describe("MultiSendCallOnly", () => { contract StorageSetter { function setStorage(bytes3 data) public { bytes32 slot = 0x4242424242424242424242424242424242424242424242424242424242424242; - // solhint-disable-next-line no-inline-assembly + /* solhint-disable no-inline-assembly */ + /// @solidity memory-safe-assembly assembly { sstore(slot, data) } + /* solhint-enable no-inline-assembly */ } }`; const signers = await ethers.getSigners();