diff --git a/contracts-test/TestFeature.sol b/contracts-test/TestFeature.sol index 554eaae3b..720d72b88 100644 --- a/contracts-test/TestFeature.sol +++ b/contracts-test/TestFeature.sol @@ -13,21 +13,17 @@ contract TestFeature is BaseFeature { bytes32 constant NAME = "TestFeature"; - bool boolVal; uint uintVal; - TestDapp public dapp; constructor( ILockStorage _lockStorage, IVersionManager _versionManager, - bool _boolVal, uint _uintVal ) BaseFeature(_lockStorage, _versionManager, NAME) public { - boolVal = _boolVal; uintVal = _uintVal; dapp = new TestDapp(); } @@ -63,24 +59,29 @@ contract TestFeature is BaseFeature { * @inheritdoc IFeature */ function getStaticCallSignatures() external virtual override view returns (bytes4[] memory _sigs) { - _sigs = new bytes4[](3); + _sigs = new bytes4[](4); _sigs[0] = bytes4(keccak256("getBoolean()")); _sigs[1] = bytes4(keccak256("getUint()")); _sigs[2] = bytes4(keccak256("getAddress(address)")); + _sigs[3] = bytes4(keccak256("badStaticCall()")); } function getBoolean() public view returns (bool) { - return boolVal; + return true; } function getUint() public view returns (uint) { - return uintVal; + return 42; } function getAddress(address _addr) public pure returns (address) { return _addr; } + function badStaticCall() external { + uintVal = 123456; + } + function callDapp(address _wallet) external { diff --git a/contracts/infrastructure/ArgentWalletDetector.sol b/contracts/infrastructure/ArgentWalletDetector.sol new file mode 100644 index 000000000..05d69e698 --- /dev/null +++ b/contracts/infrastructure/ArgentWalletDetector.sol @@ -0,0 +1,133 @@ +// Copyright (C) 2018 Argent Labs Ltd. + +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . + +// SPDX-License-Identifier: GPL-3.0-only +pragma solidity ^0.6.12; +import "./base/Owned.sol"; + +interface IArgentWallet { + /** + * @notice Returns the implementation of the wallet. + * @return The wallet implementation. + */ + function implementation() external view returns (address); +} + +/** + * @title ArgentWalletDetector + * @notice Simple contract to detect if a given address represents an Argent wallet. + * The `isArgentWallet` method returns true if the codehash matches one of the deployed Proxy + * and if the target implementation matches one of the deployed BaseWallet. + * Only the owner of the contract can add code hash and implementations. + * @author Julien Niset - + */ +contract ArgentWalletDetector is Owned { + + // The accepted code hashes + bytes32[] private codes; + // The accepted implementations + address[] private implementations; + // mapping to efficiently check if a code is accepted + mapping (bytes32 => Info) public acceptedCodes; + // mapping to efficiently check is an implementation is accepted + mapping (address => Info) public acceptedImplementations; + + struct Info { + bool exists; + uint128 index; + } + + // emits when a new accepted code is added + event CodeAdded(bytes32 indexed code); + // emits when a new accepted implementation is added + event ImplementationAdded(address indexed implementation); + + constructor(bytes32[] memory _codes, address[] memory _implementations) public { + for(uint i = 0; i < _codes.length; i++) { + addCode(_codes[i]); + } + for(uint j = 0; j < _implementations.length; j++) { + addImplementation(_implementations[j]); + } + } + + /** + * @notice Adds a new accepted code hash. + * @param _code The new code hash. + */ + function addCode(bytes32 _code) public onlyOwner { + require(_code != bytes32(0), "AWR: empty _code"); + Info storage code = acceptedCodes[_code]; + if(!code.exists) { + codes.push(_code); + code.exists = true; + code.index = uint128(codes.length - 1); + emit CodeAdded(_code); + } + } + + /** + * @notice Adds a new accepted implementation. + * @param _impl The new implementation. + */ + function addImplementation(address _impl) public onlyOwner { + require(_impl != address(0), "AWR: empty _impl"); + Info storage impl = acceptedImplementations[_impl]; + if(!impl.exists) { + implementations.push(_impl); + impl.exists = true; + impl.index = uint128(implementations.length - 1); + emit ImplementationAdded(_impl); + } + } + + /** + * @notice Adds a new accepted code hash and implementation from a deployed Argent wallet. + * @param _argentWallet The deployed Argent wallet. + */ + function addCodeAndImplementationFromWallet(address _argentWallet) external onlyOwner { + bytes32 codeHash; + // solhint-disable-next-line no-inline-assembly + assembly { codeHash := extcodehash(_argentWallet) } + addCode(codeHash); + address implementation = IArgentWallet(_argentWallet).implementation(); + addImplementation(implementation); + } + + /** + * @notice Gets the list of accepted implementations. + */ + function getImplementations() public view returns (address[] memory) { + return implementations; + } + + /** + * @notice Gets the list of accepted code hash. + */ + function getCodes() public view returns (bytes32[] memory) { + return codes; + } + + /** + * @notice Checks if an address is an Argent wallet + * @param _wallet The target wallet + */ + function isArgentWallet(address _wallet) external view returns (bool) { + bytes32 codeHash; + // solhint-disable-next-line no-inline-assembly + assembly { codeHash := extcodehash(_wallet) } + return acceptedCodes[codeHash].exists && acceptedImplementations[IArgentWallet(_wallet).implementation()].exists; + } +} diff --git a/contracts/modules/ApprovedTransfer.sol b/contracts/modules/ApprovedTransfer/ApprovedTransfer.sol similarity index 96% rename from contracts/modules/ApprovedTransfer.sol rename to contracts/modules/ApprovedTransfer/ApprovedTransfer.sol index 1cddec440..4f1d5bcf4 100644 --- a/contracts/modules/ApprovedTransfer.sol +++ b/contracts/modules/ApprovedTransfer/ApprovedTransfer.sol @@ -17,11 +17,11 @@ pragma solidity ^0.6.12; pragma experimental ABIEncoderV2; -import "./common/Utils.sol"; -import "./common/LimitUtils.sol"; -import "./common/BaseTransfer.sol"; -import "../infrastructure/storage/ILimitStorage.sol"; -import "../infrastructure/storage/IGuardianStorage.sol"; +import "../common/Utils.sol"; +import "../common/LimitUtils.sol"; +import "../common/BaseTransfer.sol"; +import "../../infrastructure/storage/ILimitStorage.sol"; +import "../../infrastructure/storage/IGuardianStorage.sol"; /** * @title ApprovedTransfer diff --git a/contracts/modules/RelayerManager.sol b/contracts/modules/RelayerManager/RelayerManager.sol similarity index 97% rename from contracts/modules/RelayerManager.sol rename to contracts/modules/RelayerManager/RelayerManager.sol index f05b36e8e..64ccf3cf4 100644 --- a/contracts/modules/RelayerManager.sol +++ b/contracts/modules/RelayerManager/RelayerManager.sol @@ -17,13 +17,13 @@ pragma solidity ^0.6.12; pragma experimental ABIEncoderV2; -import "./common/Utils.sol"; -import "./common/BaseFeature.sol"; -import "./common/GuardianUtils.sol"; -import "./common/LimitUtils.sol"; -import "../infrastructure/storage/ILimitStorage.sol"; -import "../infrastructure/ITokenPriceRegistry.sol"; -import "../infrastructure/storage/IGuardianStorage.sol"; +import "../common/Utils.sol"; +import "../common/BaseFeature.sol"; +import "../common/GuardianUtils.sol"; +import "../common/LimitUtils.sol"; +import "../../infrastructure/storage/ILimitStorage.sol"; +import "../../infrastructure/ITokenPriceRegistry.sol"; +import "../../infrastructure/storage/IGuardianStorage.sol"; /** * @title RelayerManager diff --git a/contracts/modules/TokenExchanger.sol b/contracts/modules/TokenExchanger/TokenExchanger.sol similarity index 98% rename from contracts/modules/TokenExchanger.sol rename to contracts/modules/TokenExchanger/TokenExchanger.sol index 402c37337..38bb01921 100644 --- a/contracts/modules/TokenExchanger.sol +++ b/contracts/modules/TokenExchanger/TokenExchanger.sol @@ -17,11 +17,11 @@ pragma solidity ^0.6.12; pragma experimental ABIEncoderV2; -import "./common/BaseFeature.sol"; -import "../../lib/other/ERC20.sol"; -import "../../lib/paraswap/IAugustusSwapper.sol"; -import "../infrastructure/ITokenPriceRegistry.sol"; -import "../infrastructure/IDexRegistry.sol"; +import "../common/BaseFeature.sol"; +import "../../../lib/other/ERC20.sol"; +import "../../../lib/paraswap/IAugustusSwapper.sol"; +import "../../infrastructure/ITokenPriceRegistry.sol"; +import "../../infrastructure/IDexRegistry.sol"; /** * @title TokenExchanger diff --git a/contracts/modules/TransferManager.sol b/contracts/modules/TransferManager/TransferManager.sol similarity index 98% rename from contracts/modules/TransferManager.sol rename to contracts/modules/TransferManager/TransferManager.sol index 1b209e9de..4ec179b29 100644 --- a/contracts/modules/TransferManager.sol +++ b/contracts/modules/TransferManager/TransferManager.sol @@ -17,13 +17,13 @@ pragma solidity ^0.6.12; pragma experimental ABIEncoderV2; -import "./common/Utils.sol"; -import "./common/BaseTransfer.sol"; -import "./common/LimitUtils.sol"; -import "../infrastructure/storage/ILimitStorage.sol"; -import "../infrastructure/storage/ITransferStorage.sol"; -import "../infrastructure/ITokenPriceRegistry.sol"; -import "../../lib/other/ERC20.sol"; +import "../common/Utils.sol"; +import "../common/BaseTransfer.sol"; +import "../common/LimitUtils.sol"; +import "../../infrastructure/storage/ILimitStorage.sol"; +import "../../infrastructure/storage/ITransferStorage.sol"; +import "../../infrastructure/ITokenPriceRegistry.sol"; +import "../../../lib/other/ERC20.sol"; /** * @title TransferManager diff --git a/contracts/modules/VersionManager.sol b/contracts/modules/VersionManager.sol index 95fc1c236..2f1a77026 100644 --- a/contracts/modules/VersionManager.sol +++ b/contracts/modules/VersionManager.sol @@ -175,6 +175,24 @@ contract VersionManager is IVersionManager, IModule, BaseFeature, Owned { return (1, OwnerSignature.Required); } + /* ***************** Static Call Delegation ************************* */ + + /** + * @notice This method is used by the VersionManager's fallback (via an internal call) to determine whether + * the current transaction is a staticcall or not. The method succeeds if the current transaction is a static call, + * and reverts otherwise. + * @dev The use of an if/else allows to encapsulate the whole logic in a single function. + */ + function verifyStaticCall() public { + if(msg.sender != address(this)) { // first entry in the method (via an internal call) + (bool success,) = address(this).call{gas: 3000}(abi.encodeWithSelector(VersionManager(0).verifyStaticCall.selector)); + require(!success, "VM: not in a staticcall"); + } else { // second entry in the method (via an external call) + // solhint-disable-next-line no-inline-assembly + assembly { log0(0, 0) } + } + } + /** * @notice This method delegates the static call to a target feature */ @@ -182,11 +200,12 @@ contract VersionManager is IVersionManager, IModule, BaseFeature, Owned { uint256 version = walletVersions[msg.sender]; address feature = staticCallExecutors[version][msg.sig]; require(feature != address(0), "VM: static call not supported for wallet version"); + verifyStaticCall(); // solhint-disable-next-line no-inline-assembly assembly { calldatacopy(0, 0, calldatasize()) - let result := staticcall(gas(), feature, 0, calldatasize(), 0, 0) + let result := delegatecall(gas(), feature, 0, calldatasize(), 0, 0) returndatacopy(0, 0, returndatasize()) switch result case 0 {revert(0, returndatasize())} diff --git a/deployment/2_deploy_contracts.js b/deployment/2_deploy_contracts.js index f45a032ce..abf530d11 100644 --- a/deployment/2_deploy_contracts.js +++ b/deployment/2_deploy_contracts.js @@ -1,3 +1,8 @@ +const GuardianStorage = require("../build/GuardianStorage"); +const TransferStorage = require("../build/TransferStorage"); +const LockStorage = require("../build/LockStorage"); +const LimitStorage = require("../build/LimitStorage"); + const BaseWallet = require("../build/BaseWallet"); const ModuleRegistry = require("../build/ModuleRegistry"); const CompoundRegistry = require("../build/CompoundRegistry"); @@ -5,8 +10,11 @@ const MultiSig = require("../build/MultiSigWallet"); const ENS = require("../build/ENSRegistryWithFallback"); const ENSManager = require("../build/ArgentENSManager"); const ENSResolver = require("../build/ArgentENSResolver"); -const WalletFactory = require("../build-legacy/v1.6.0/WalletFactory"); -const TokenPriceProvider = require("../build-legacy/v1.6.0/TokenPriceProvider"); +const WalletFactory = require("../build/WalletFactory"); + +const TokenPriceRegistry = require("../build/TokenPriceRegistry"); +const DexRegistry = require("../build/DexRegistry"); + const MakerRegistry = require("../build/MakerRegistry"); const ScdMcdMigration = require("../build/ScdMcdMigration"); @@ -37,19 +45,32 @@ const deploy = async (network) => { const walletRootEns = prevConfig.ENS.domain; // ////////////////////////////////// - // Deploy contracts + // Deploy Storage + // ////////////////////////////////// + + // Deploy the Guardian Storage + const GuardianStorageWrapper = await deployer.deploy(GuardianStorage); + // Deploy the Transfer Storage + const TransferStorageWrapper = await deployer.deploy(TransferStorage); + // Deploy the new LockStorage + const LockStorageWrapper = await deployer.deploy(LockStorage); + // Deploy the new LimitStorage + const LimitStorageWrapper = await deployer.deploy(LimitStorage); + + // ////////////////////////////////// + // Deploy infrastructure contracts // ////////////////////////////////// // Deploy the Base Wallet Library const BaseWalletWrapper = await deployer.deploy(BaseWallet); // Deploy the MultiSig const MultiSigWrapper = await deployer.deploy(MultiSig, {}, newConfig.multisig.threshold, newConfig.multisig.owners); - // Deploy TokenPriceProvider - const TokenPriceProviderWrapper = await deployer.deploy( - TokenPriceProvider, - {}, - newConfig.Kyber ? newConfig.Kyber.contract : "0x0000000000000000000000000000000000000000", - ); + + // Deploy the new TokenPriceRegistry + const TokenPriceRegistryWrapper = await deployer.deploy(TokenPriceRegistry); + // Deploy the DexRegistry + const DexRegistryWrapper = await deployer.deploy(DexRegistry); + // Deploy Module Registry const ModuleRegistryWrapper = await deployer.deploy(ModuleRegistry); // Deploy Compound Registry @@ -61,7 +82,7 @@ const deploy = async (network) => { walletRootEns, utils.namehash(walletRootEns), newConfig.ENS.ensRegistry, ENSResolverWrapper.contractAddress); // Deploy the Wallet Factory const WalletFactoryWrapper = await deployer.deploy(WalletFactory, {}, - ModuleRegistryWrapper.contractAddress, BaseWalletWrapper.contractAddress, ENSManagerWrapper.contractAddress); + ModuleRegistryWrapper.contractAddress, BaseWalletWrapper.contractAddress, GuardianStorageWrapper.contractAddress); // Deploy and configure Maker Registry const ScdMcdMigrationWrapper = await deployer.wrapDeployedContract(ScdMcdMigration, newConfig.defi.maker.migration); @@ -114,27 +135,39 @@ const deploy = async (network) => { // ///////////////////////////////////////////////// // Update config and Upload ABIs // ///////////////////////////////////////////////// + configurator.updateModuleAddresses({ + GuardianStorage: GuardianStorageWrapper.contractAddress, + TransferStorage: TransferStorageWrapper.contractAddress, + LockStorage: LockStorageWrapper.contractAddress, + LimitStorage: LimitStorageWrapper.contractAddress, + TokenPriceRegistry: TokenPriceRegistryWrapper.contractAddress, + }); configurator.updateInfrastructureAddresses({ MultiSigWallet: MultiSigWrapper.contractAddress, WalletFactory: WalletFactoryWrapper.contractAddress, ENSResolver: ENSResolverWrapper.contractAddress, ENSManager: ENSManagerWrapper.contractAddress, - TokenPriceProvider: TokenPriceProviderWrapper.contractAddress, ModuleRegistry: ModuleRegistryWrapper.contractAddress, CompoundRegistry: CompoundRegistryWrapper.contractAddress, + DexRegistry: DexRegistryWrapper.contractAddress, BaseWallet: BaseWalletWrapper.contractAddress, }); await configurator.save(); await Promise.all([ + abiUploader.upload(GuardianStorageWrapper, "modules"), + abiUploader.upload(TransferStorageWrapper, "modules"), + abiUploader.upload(LockStorageWrapper, "modules"), + abiUploader.upload(LimitStorageWrapper, "modules"), + abiUploader.upload(TokenPriceRegistryWrapper, "modules"), abiUploader.upload(MultiSigWrapper, "contracts"), abiUploader.upload(WalletFactoryWrapper, "contracts"), abiUploader.upload(ENSResolverWrapper, "contracts"), abiUploader.upload(ENSManagerWrapper, "contracts"), - abiUploader.upload(TokenPriceProviderWrapper, "contracts"), abiUploader.upload(ModuleRegistryWrapper, "contracts"), abiUploader.upload(CompoundRegistryWrapper, "contracts"), + abiUploader.upload(DexRegistryWrapper, "contracts"), abiUploader.upload(BaseWalletWrapper, "contracts"), ]); }; diff --git a/deployment/3_setup_contracts.js b/deployment/3_setup_contracts.js index bf67a956f..0a6a10f14 100644 --- a/deployment/3_setup_contracts.js +++ b/deployment/3_setup_contracts.js @@ -1,9 +1,10 @@ const ModuleRegistry = require("../build/ModuleRegistry"); const ENSManager = require("../build/ArgentENSManager"); const ENSResolver = require("../build/ArgentENSResolver"); -const WalletFactory = require("../build-legacy/v1.6.0/WalletFactory"); -const TokenPriceProvider = require("../build-legacy/v1.6.0/TokenPriceProvider"); +const WalletFactory = require("../build/WalletFactory"); +const TokenPriceRegistry = require("../build/TokenPriceRegistry"); const CompoundRegistry = require("../build/CompoundRegistry"); +const DexRegistry = require("../build/DexRegistry"); const DeployManager = require("../utils/deploy-manager.js"); @@ -27,7 +28,15 @@ const deploy = async (network) => { const WalletFactoryWrapper = await deployer.wrapDeployedContract(WalletFactory, config.contracts.WalletFactory); const ModuleRegistryWrapper = await deployer.wrapDeployedContract(ModuleRegistry, config.contracts.ModuleRegistry); const CompoundRegistryWrapper = await deployer.wrapDeployedContract(CompoundRegistry, config.contracts.CompoundRegistry); - const TokenPriceProviderWrapper = await deployer.wrapDeployedContract(TokenPriceProvider, config.contracts.TokenPriceProvider); + const TokenPriceRegistryWrapper = await deployer.wrapDeployedContract(TokenPriceRegistry, config.modules.TokenPriceRegistry); + const DexRegistryWrapper = await deployer.wrapDeployedContract(DexRegistry, config.contracts.DexRegistry); + + // Configure DexRegistry + const authorisedExchanges = Object.values(config.defi.paraswap.authorisedExchanges); + const DexRegistrySetAuthorisedTx = await DexRegistryWrapper.contract.setAuthorised( + authorisedExchanges, Array(authorisedExchanges.length).fill(true), { gasPrice }, + ); + await DexRegistryWrapper.verboseWaitForTransaction(DexRegistrySetAuthorisedTx, "Setting up DexRegistry"); // ////////////////////////////////// // Set contracts' managers @@ -47,9 +56,9 @@ const deploy = async (network) => { const WalletFactoryAddManagerTx = await WalletFactoryWrapper.contract.addManager(account, { gasPrice }); await WalletFactoryWrapper.verboseWaitForTransaction(WalletFactoryAddManagerTx, `Set ${account} as the manager of the WalletFactory`); - const TokenPriceProviderAddManagerTx = await TokenPriceProviderWrapper.contract.addManager(account, { gasPrice }); - await TokenPriceProviderWrapper.verboseWaitForTransaction(TokenPriceProviderAddManagerTx, - `Set ${account} as the manager of the TokenPriceProvider`); + const TokenPriceRegistryAddManagerTx = await TokenPriceRegistryWrapper.contract.addManager(account, { gasPrice }); + await TokenPriceRegistryWrapper.verboseWaitForTransaction(TokenPriceRegistryAddManagerTx, + `Set ${account} as the manager of the TokenPriceRegistry`); } // ////////////////////////////////// @@ -62,7 +71,9 @@ const deploy = async (network) => { WalletFactoryWrapper, ModuleRegistryWrapper, CompoundRegistryWrapper, - TokenPriceProviderWrapper]; + TokenPriceRegistryWrapper, + DexRegistryWrapper]; + for (let idx = 0; idx < wrappers.length; idx += 1) { const wrapper = wrappers[idx]; const changeOwnerTx = await wrapper.contract.changeOwner(config.contracts.MultiSigWallet, { gasPrice }); diff --git a/deployment/5_deploy_modules.js b/deployment/5_deploy_modules.js index 180dade04..c7cf946e8 100644 --- a/deployment/5_deploy_modules.js +++ b/deployment/5_deploy_modules.js @@ -1,21 +1,21 @@ const childProcess = require("child_process"); -const GuardianStorage = require("../build-legacy/v1.6.0/GuardianStorage"); -const TransferStorage = require("../build-legacy/v1.6.0/TransferStorage"); - -const GuardianManager = require("../build-legacy/v1.6.0/GuardianManager"); -const TokenExchanger = require("../build-legacy/v1.6.0/TokenExchanger"); -const LockManager = require("../build-legacy/v1.6.0/LockManager"); -const RecoveryManager = require("../build-legacy/v1.6.0/RecoveryManager"); -const ApprovedTransfer = require("../build-legacy/v1.6.0/ApprovedTransfer"); -const TransferManager = require("../build-legacy/v1.6.0/TransferManager"); -const NftTransfer = require("../build-legacy/v1.6.0/NftTransfer"); -const CompoundManager = require("../build-legacy/v1.6.0/CompoundManager"); -const MakerV2Manager = require("../build-legacy/v1.6.0/MakerV2Manager"); + +const ApprovedTransfer = require("../build/ApprovedTransfer"); +const CompoundManager = require("../build/CompoundManager"); +const GuardianManager = require("../build/GuardianManager"); +const LockManager = require("../build/LockManager"); +const NftTransfer = require("../build/NftTransfer"); +const RecoveryManager = require("../build/RecoveryManager"); +const TokenExchanger = require("../build/TokenExchanger"); +const MakerV2Manager = require("../build/MakerV2Manager"); +const TransferManager = require("../build/TransferManager"); +const RelayerManager = require("../build/RelayerManager"); +const VersionManager = require("../build/VersionManager"); const DeployManager = require("../utils/deploy-manager.js"); // /////////////////////////////////////////////////////// -// Version 1.4 +// Version 2.1 // /////////////////////////////////////////////////////// const deploy = async (network) => { @@ -34,24 +34,29 @@ const deploy = async (network) => { console.log(config); // ////////////////////////////////// - // Deploy Storage + // Deploy VersionManager module // ////////////////////////////////// - - // Deploy the Guardian Storage - const GuardianStorageWrapper = await deployer.deploy(GuardianStorage); - // Deploy the Transfer Storage - const TransferStorageWrapper = await deployer.deploy(TransferStorage); + const VersionManagerWrapper = await deployer.deploy( + VersionManager, + {}, + config.contracts.ModuleRegistry, + config.modules.LockStorage, + config.modules.GuardianStorage, + config.modules.TransferStorage, + config.modules.LimitStorage, + ); // ////////////////////////////////// - // Deploy Modules + // Deploy features // ////////////////////////////////// // Deploy the GuardianManager module const GuardianManagerWrapper = await deployer.deploy( GuardianManager, {}, - config.contracts.ModuleRegistry, - GuardianStorageWrapper.contractAddress, + config.modules.LockStorage, + config.modules.GuardianStorage, + VersionManagerWrapper.contractAddress, config.settings.securityPeriod || 0, config.settings.securityWindow || 0, ); @@ -59,88 +64,104 @@ const deploy = async (network) => { const LockManagerWrapper = await deployer.deploy( LockManager, {}, - config.contracts.ModuleRegistry, - GuardianStorageWrapper.contractAddress, + config.modules.LockStorage, + config.modules.GuardianStorage, + VersionManagerWrapper.contractAddress, config.settings.lockPeriod || 0, ); // Deploy the RecoveryManager module const RecoveryManagerWrapper = await deployer.deploy( RecoveryManager, {}, - config.contracts.ModuleRegistry, - GuardianStorageWrapper.contractAddress, + config.modules.LockStorage, + config.modules.GuardianStorage, + VersionManagerWrapper.contractAddress, config.settings.recoveryPeriod || 0, config.settings.lockPeriod || 0, - config.settings.securityPeriod || 0, - config.settings.securityWindow || 0, ); // Deploy the ApprovedTransfer module const ApprovedTransferWrapper = await deployer.deploy( ApprovedTransfer, {}, - config.contracts.ModuleRegistry, - GuardianStorageWrapper.contractAddress, + config.modules.LockStorage, + config.modules.GuardianStorage, + config.modules.LimitStorage, + VersionManagerWrapper.contractAddress, + config.defi.weth, ); // Deploy the TransferManager module const TransferManagerWrapper = await deployer.deploy( TransferManager, {}, - config.contracts.ModuleRegistry, - TransferStorageWrapper.contractAddress, - GuardianStorageWrapper.contractAddress, - config.contracts.TokenPriceProvider, + config.modules.LockStorage, + config.modules.TransferStorage, + config.modules.LimitStorage, + config.modules.TokenPriceRegistry, + VersionManagerWrapper.contractAddress, config.settings.securityPeriod || 0, config.settings.securityWindow || 0, config.settings.defaultLimit || "1000000000000000000", - "0x0000000000000000000000000000000000000000", + config.defi.weth, + ["test", "staging", "prod"].includes(network) ? config.modules.TransferManager : "0x0000000000000000000000000000000000000000", ); // Deploy the TokenExchanger module const TokenExchangerWrapper = await deployer.deploy( TokenExchanger, {}, - config.contracts.ModuleRegistry, - GuardianStorageWrapper.contractAddress, - config.Kyber ? config.Kyber.contract : "0x0000000000000000000000000000000000000000", - config.contracts.MultiSigWallet, - config.settings.feeRatio || 0, + config.modules.LockStorage, + config.modules.TokenPriceRegistry, + VersionManagerWrapper.contractAddress, + config.contracts.DexRegistry, + config.defi.paraswap.contract, + "argent", ); // Deploy the NFTTransfer module const NftTransferWrapper = await deployer.deploy( NftTransfer, {}, - config.contracts.ModuleRegistry, - GuardianStorageWrapper.contractAddress, + config.modules.LockStorage, + config.modules.TokenPriceRegistry, + VersionManagerWrapper.contractAddress, config.CryptoKitties.contract, ); // Deploy the CompoundManager module const CompoundManagerWrapper = await deployer.deploy( CompoundManager, {}, - config.contracts.ModuleRegistry, - GuardianStorageWrapper.contractAddress, + config.modules.LockStorage, config.defi.compound.comptroller, config.contracts.CompoundRegistry, + VersionManagerWrapper.contractAddress, ); // Deploy MakerManagerV2 const MakerV2ManagerWrapper = await deployer.deploy( MakerV2Manager, {}, - config.contracts.ModuleRegistry, - GuardianStorageWrapper.contractAddress, + config.modules.LockStorage, config.defi.maker.migration, config.defi.maker.pot, config.defi.maker.jug, config.contracts.MakerRegistry, config.defi.uniswap.factory, + VersionManagerWrapper.contractAddress, + ); + // Deploy RelayerManager + const RelayerManagerWrapper = await deployer.deploy( + RelayerManager, + {}, + config.modules.LockStorage, + config.modules.GuardianStorage, + config.modules.LimitStorage, + config.modules.TokenPriceRegistry, + VersionManagerWrapper.contractAddress, ); // ///////////////////////////////////////////////// // Update config and Upload ABIs // ///////////////////////////////////////////////// + // TODO: change name from "module" to "feature" where appropriate configurator.updateModuleAddresses({ - GuardianStorage: GuardianStorageWrapper.contractAddress, - TransferStorage: TransferStorageWrapper.contractAddress, GuardianManager: GuardianManagerWrapper.contractAddress, LockManager: LockManagerWrapper.contractAddress, RecoveryManager: RecoveryManagerWrapper.contractAddress, @@ -150,6 +171,8 @@ const deploy = async (network) => { NftTransfer: NftTransferWrapper.contractAddress, CompoundManager: CompoundManagerWrapper.contractAddress, MakerV2Manager: MakerV2ManagerWrapper.contractAddress, + RelayerManager: RelayerManagerWrapper.contractAddress, + VersionManager: VersionManagerWrapper.contractAddress, }); const gitHash = childProcess.execSync("git rev-parse HEAD").toString("utf8").replace(/\n$/, ""); @@ -158,8 +181,6 @@ const deploy = async (network) => { await configurator.save(); await Promise.all([ - abiUploader.upload(GuardianStorageWrapper, "modules"), - abiUploader.upload(TransferStorageWrapper, "modules"), abiUploader.upload(GuardianManagerWrapper, "modules"), abiUploader.upload(LockManagerWrapper, "modules"), abiUploader.upload(RecoveryManagerWrapper, "modules"), @@ -169,6 +190,8 @@ const deploy = async (network) => { abiUploader.upload(NftTransferWrapper, "modules"), abiUploader.upload(MakerV2ManagerWrapper, "modules"), abiUploader.upload(CompoundManagerWrapper, "modules"), + abiUploader.upload(RelayerManagerWrapper, "modules"), + abiUploader.upload(VersionManagerWrapper, "modules"), ]); console.log("Config:", config); diff --git a/deployment/6_register_modules.js b/deployment/6_register_modules.js index 922df9f3a..c54de517f 100644 --- a/deployment/6_register_modules.js +++ b/deployment/6_register_modules.js @@ -1,15 +1,17 @@ const ModuleRegistry = require("../build/ModuleRegistry"); const MultiSig = require("../build/MultiSigWallet"); -const GuardianManager = require("../build-legacy/v1.6.0/GuardianManager"); -const TokenExchanger = require("../build-legacy/v1.6.0/TokenExchanger"); -const LockManager = require("../build-legacy/v1.6.0/LockManager"); -const RecoveryManager = require("../build-legacy/v1.6.0/RecoveryManager"); -const ApprovedTransfer = require("../build-legacy/v1.6.0/ApprovedTransfer"); -const TransferManager = require("../build-legacy/v1.6.0/TransferManager"); -const NftTransfer = require("../build-legacy/v1.6.0/NftTransfer"); -const CompoundManager = require("../build-legacy/v1.6.0/CompoundManager"); -const MakerV2Manager = require("../build-legacy/v1.6.0/MakerV2Manager"); +const ApprovedTransfer = require("../build/ApprovedTransfer"); +const CompoundManager = require("../build/CompoundManager"); +const GuardianManager = require("../build/GuardianManager"); +const LockManager = require("../build/LockManager"); +const NftTransfer = require("../build/NftTransfer"); +const RecoveryManager = require("../build/RecoveryManager"); +const TokenExchanger = require("../build/TokenExchanger"); +const MakerV2Manager = require("../build/MakerV2Manager"); +const TransferManager = require("../build/TransferManager"); +const RelayerManager = require("../build/RelayerManager"); +const VersionManager = require("../build/VersionManager"); const utils = require("../utils/utilities.js"); @@ -43,24 +45,44 @@ const deploy = async (network) => { const NftTransferWrapper = await deployer.wrapDeployedContract(NftTransfer, config.modules.NftTransfer); const CompoundManagerWrapper = await deployer.wrapDeployedContract(CompoundManager, config.modules.CompoundManager); const MakerV2ManagerWrapper = await deployer.wrapDeployedContract(MakerV2Manager, config.modules.MakerV2Manager); + const RelayerManagerWrapper = await deployer.wrapDeployedContract(RelayerManager, config.modules.RelayerManager); + const VersionManagerWrapper = await deployer.wrapDeployedContract(VersionManager, config.modules.VersionManager); const ModuleRegistryWrapper = await deployer.wrapDeployedContract(ModuleRegistry, config.contracts.ModuleRegistry); const MultiSigWrapper = await deployer.wrapDeployedContract(MultiSig, config.contracts.MultiSigWallet); - const wrappers = [ - GuardianManagerWrapper, - LockManagerWrapper, - RecoveryManagerWrapper, - ApprovedTransferWrapper, - TransferManagerWrapper, - TokenExchangerWrapper, - NftTransferWrapper, - CompoundManagerWrapper, - MakerV2ManagerWrapper, + const wrappers = [VersionManagerWrapper]; + + // Add Features to Version Manager + const features = [ + GuardianManagerWrapper.contractAddress, + LockManagerWrapper.contractAddress, + RecoveryManagerWrapper.contractAddress, + ApprovedTransferWrapper.contractAddress, + TransferManagerWrapper.contractAddress, + TokenExchangerWrapper.contractAddress, + NftTransferWrapper.contractAddress, + CompoundManagerWrapper.contractAddress, + MakerV2ManagerWrapper.contractAddress, + RelayerManagerWrapper.contractAddress, ]; + const featuresWithNoInit = [ // all features except the TransferManager + GuardianManagerWrapper.contractAddress, + LockManagerWrapper.contractAddress, + RecoveryManagerWrapper.contractAddress, + ApprovedTransferWrapper.contractAddress, + TokenExchangerWrapper.contractAddress, + NftTransferWrapper.contractAddress, + CompoundManagerWrapper.contractAddress, + MakerV2ManagerWrapper.contractAddress, + RelayerManagerWrapper.contractAddress, + ]; + const featureToInit = features.filter((f) => !featuresWithNoInit.includes(f)); + const VersionManagerAddVersionTx = await VersionManagerWrapper.contract.addVersion(features, featureToInit, { gasPrice }); + await VersionManagerWrapper.verboseWaitForTransaction(VersionManagerAddVersionTx, "Adding New Version"); // ////////////////////////////////// - // Register modules + // Register and configure VersionManager // ////////////////////////////////// const multisigExecutor = new MultisigExecutor(MultiSigWrapper, deploymentWallet, config.multisig.autosign, { gasPrice }); @@ -71,6 +93,9 @@ const deploy = async (network) => { [wrapper.contractAddress, utils.asciiToBytes32(wrapper._contract.contractName)]); } + const changeOwnerTx = await VersionManagerWrapper.contract.changeOwner(config.contracts.MultiSigWallet, { gasPrice }); + await VersionManagerWrapper.verboseWaitForTransaction(changeOwnerTx, "Set the MultiSig as the owner of VersionManagerWrapper"); + // ////////////////////////////////// // Upload Version // ////////////////////////////////// diff --git a/deployment/7_upgrade_2_0.js b/deployment/7_upgrade_2_2.js similarity index 67% rename from deployment/7_upgrade_2_0.js rename to deployment/7_upgrade_2_2.js index ef6c38209..bfecf65d7 100644 --- a/deployment/7_upgrade_2_0.js +++ b/deployment/7_upgrade_2_2.js @@ -6,11 +6,6 @@ const Upgrader = require("../build/UpgraderToVersionManager"); const DeployManager = require("../utils/deploy-manager.js"); const MultisigExecutor = require("../utils/multisigexecutor.js"); -const LimitStorage = require("../build/LimitStorage"); -const TokenPriceRegistry = require("../build/TokenPriceRegistry"); -const LockStorage = require("../build/LockStorage"); -const DexRegistry = require("../build/DexRegistry"); - const ApprovedTransfer = require("../build/ApprovedTransfer"); const CompoundManager = require("../build/CompoundManager"); const GuardianManager = require("../build/GuardianManager"); @@ -23,30 +18,15 @@ const TransferManager = require("../build/TransferManager"); const RelayerManager = require("../build/RelayerManager"); const VersionManager = require("../build/VersionManager"); -const BaseWallet = require("../build/BaseWallet"); -const WalletFactory = require("../build/WalletFactory"); - const utils = require("../utils/utilities.js"); -const TARGET_VERSION = "2.1.0"; +const TARGET_VERSION = "2.2.0"; const MODULES_TO_ENABLE = [ "VersionManager", ]; -const MODULES_TO_DISABLE = [ - "MakerManager", - "ApprovedTransfer", - "CompoundManager", - "GuardianManager", - "LockManager", - "NftTransfer", - "RecoveryManager", - "TokenExchanger", - "MakerV2Manager", - "TransferManager", - "RelayerModule", -]; +const MODULES_TO_DISABLE = []; -const BACKWARD_COMPATIBILITY = 3; +const BACKWARD_COMPATIBILITY = 4; const deploy = async (network) => { if (!["kovan", "kovan-fork", "staging", "prod"].includes(network)) { @@ -77,35 +57,18 @@ const deploy = async (network) => { const MultiSigWrapper = await deployer.wrapDeployedContract(MultiSig, config.contracts.MultiSigWallet); const multisigExecutor = new MultisigExecutor(MultiSigWrapper, deploymentWallet, config.multisig.autosign, { gasPrice }); - // ////////////////////////////////// - // Deploy infrastructure contracts - // ////////////////////////////////// - - // Deploy the Base Wallet Library - const BaseWalletWrapper = await deployer.deploy(BaseWallet); - // Deploy the Wallet Factory - const WalletFactoryWrapper = await deployer.deploy(WalletFactory, {}, - ModuleRegistryWrapper.contractAddress, BaseWalletWrapper.contractAddress, config.modules.GuardianStorage); - // Deploy the new LockStorage - const LockStorageWrapper = await deployer.deploy(LockStorage); - // Deploy the new LimitStorage - const LimitStorageWrapper = await deployer.deploy(LimitStorage); - // Deploy the new TokenPriceRegistry - const TokenPriceRegistryWrapper = await deployer.deploy(TokenPriceRegistry); - // Deploy the DexRegistry - const DexRegistryWrapper = await deployer.deploy(DexRegistry); - // ////////////////////////////////// // Deploy new modules // ////////////////////////////////// + const VersionManagerWrapper = await deployer.deploy( VersionManager, {}, config.contracts.ModuleRegistry, - LockStorageWrapper.contractAddress, + config.modules.LockStorage, config.modules.GuardianStorage, config.modules.TransferStorage, - LimitStorageWrapper.contractAddress, + config.modules.LimitStorage, ); newModuleWrappers.push(VersionManagerWrapper); @@ -115,9 +78,9 @@ const deploy = async (network) => { const ApprovedTransferWrapper = await deployer.deploy( ApprovedTransfer, {}, - LockStorageWrapper.contractAddress, + config.modules.LockStorage, config.modules.GuardianStorage, - LimitStorageWrapper.contractAddress, + config.modules.LimitStorage, VersionManagerWrapper.contractAddress, config.defi.weth, ); @@ -125,7 +88,7 @@ const deploy = async (network) => { const CompoundManagerWrapper = await deployer.deploy( CompoundManager, {}, - LockStorageWrapper.contractAddress, + config.modules.LockStorage, config.defi.compound.comptroller, config.contracts.CompoundRegistry, VersionManagerWrapper.contractAddress, @@ -134,7 +97,7 @@ const deploy = async (network) => { const GuardianManagerWrapper = await deployer.deploy( GuardianManager, {}, - LockStorageWrapper.contractAddress, + config.modules.LockStorage, config.modules.GuardianStorage, VersionManagerWrapper.contractAddress, config.settings.securityPeriod || 0, @@ -144,7 +107,7 @@ const deploy = async (network) => { const LockManagerWrapper = await deployer.deploy( LockManager, {}, - LockStorageWrapper.contractAddress, + config.modules.LockStorage, config.modules.GuardianStorage, VersionManagerWrapper.contractAddress, config.settings.lockPeriod || 0, @@ -153,8 +116,8 @@ const deploy = async (network) => { const NftTransferWrapper = await deployer.deploy( NftTransfer, {}, - LockStorageWrapper.contractAddress, - TokenPriceRegistryWrapper.contractAddress, + config.modules.LockStorage, + config.modules.TokenPriceRegistry, VersionManagerWrapper.contractAddress, config.CryptoKitties.contract, ); @@ -162,7 +125,7 @@ const deploy = async (network) => { const RecoveryManagerWrapper = await deployer.deploy( RecoveryManager, {}, - LockStorageWrapper.contractAddress, + config.modules.LockStorage, config.modules.GuardianStorage, VersionManagerWrapper.contractAddress, config.settings.recoveryPeriod || 0, @@ -172,10 +135,10 @@ const deploy = async (network) => { const TokenExchangerWrapper = await deployer.deploy( TokenExchanger, {}, - LockStorageWrapper.contractAddress, - TokenPriceRegistryWrapper.contractAddress, + config.modules.LockStorage, + config.modules.TokenPriceRegistry, VersionManagerWrapper.contractAddress, - DexRegistryWrapper.contractAddress, + config.contracts.DexRegistry, config.defi.paraswap.contract, "argent", ); @@ -183,7 +146,7 @@ const deploy = async (network) => { const MakerV2ManagerWrapper = await deployer.deploy( MakerV2Manager, {}, - LockStorageWrapper.contractAddress, + config.modules.LockStorage, config.defi.maker.migration, config.defi.maker.pot, config.defi.maker.jug, @@ -195,10 +158,10 @@ const deploy = async (network) => { const TransferManagerWrapper = await deployer.deploy( TransferManager, {}, - LockStorageWrapper.contractAddress, + config.modules.LockStorage, config.modules.TransferStorage, - LimitStorageWrapper.contractAddress, - TokenPriceRegistryWrapper.contractAddress, + config.modules.LimitStorage, + config.modules.TokenPriceRegistry, VersionManagerWrapper.contractAddress, config.settings.securityPeriod || 0, config.settings.securityWindow || 0, @@ -210,10 +173,10 @@ const deploy = async (network) => { const RelayerManagerWrapper = await deployer.deploy( RelayerManager, {}, - LockStorageWrapper.contractAddress, + config.modules.LockStorage, config.modules.GuardianStorage, - LimitStorageWrapper.contractAddress, - TokenPriceRegistryWrapper.contractAddress, + config.modules.LimitStorage, + config.modules.TokenPriceRegistry, VersionManagerWrapper.contractAddress, ); @@ -245,56 +208,18 @@ const deploy = async (network) => { const VersionManagerAddVersionTx = await VersionManagerWrapper.contract.addVersion(newFeatures, newFeatureToInit, { gasPrice }); await VersionManagerWrapper.verboseWaitForTransaction(VersionManagerAddVersionTx, "Adding New Version"); - // ////////////////////////////////// - // Setup new infrastructure - // ////////////////////////////////// - - // Setup DexRegistry - const authorisedExchanges = Object.values(config.defi.paraswap.authorisedExchanges); - const DexRegistrySetAuthorisedTx = await DexRegistryWrapper.contract.setAuthorised( - authorisedExchanges, Array(authorisedExchanges.length).fill(true), { gasPrice }, - ); - await DexRegistryWrapper.verboseWaitForTransaction(DexRegistrySetAuthorisedTx, "Setting up DexRegistry"); - - // ////////////////////////////////// - // Set contracts' managers - // ////////////////////////////////// - - for (const idx in config.backend.accounts) { - const account = config.backend.accounts[idx]; - const WalletFactoryAddManagerTx = await WalletFactoryWrapper.contract.addManager(account, { gasPrice }); - await WalletFactoryWrapper.verboseWaitForTransaction(WalletFactoryAddManagerTx, `Set ${account} as the manager of the WalletFactory`); - - const TokenPriceRegistryAddManagerTx = await TokenPriceRegistryWrapper.contract.addManager(account, { gasPrice }); - await TokenPriceRegistryWrapper.verboseWaitForTransaction(TokenPriceRegistryAddManagerTx, - `Set ${account} as the manager of the TokenPriceRegistry`); - } - // ////////////////////////////////// // Set contracts' owners // ////////////////////////////////// - let changeOwnerTx = await WalletFactoryWrapper.contract.changeOwner(config.contracts.MultiSigWallet, { gasPrice }); - await WalletFactoryWrapper.verboseWaitForTransaction(changeOwnerTx, "Set the MultiSig as the owner of WalletFactory"); - - changeOwnerTx = await TokenPriceRegistryWrapper.contract.changeOwner(config.contracts.MultiSigWallet, { gasPrice }); - await TokenPriceRegistryWrapper.verboseWaitForTransaction(changeOwnerTx, "Set the MultiSig as the owner of TokenPriceRegistryWrapper"); - - changeOwnerTx = await VersionManagerWrapper.contract.changeOwner(config.contracts.MultiSigWallet, { gasPrice }); + const changeOwnerTx = await VersionManagerWrapper.contract.changeOwner(config.contracts.MultiSigWallet, { gasPrice }); await VersionManagerWrapper.verboseWaitForTransaction(changeOwnerTx, "Set the MultiSig as the owner of VersionManagerWrapper"); - changeOwnerTx = await DexRegistryWrapper.contract.changeOwner(config.contracts.MultiSigWallet, { gasPrice }); - await DexRegistryWrapper.verboseWaitForTransaction(changeOwnerTx, "Set the MultiSig as the owner of DexRegistryWrapper"); - // ///////////////////////////////////////////////// // Update config and Upload ABIs // ///////////////////////////////////////////////// - // TODO: change name from "module" to "feature" where appropriate configurator.updateModuleAddresses({ - LimitStorage: LimitStorageWrapper.contractAddress, - TokenPriceRegistry: TokenPriceRegistryWrapper.contractAddress, - LockStorage: LockStorageWrapper.contractAddress, ApprovedTransfer: ApprovedTransferWrapper.contractAddress, CompoundManager: CompoundManagerWrapper.contractAddress, GuardianManager: GuardianManagerWrapper.contractAddress, @@ -305,13 +230,8 @@ const deploy = async (network) => { MakerV2Manager: MakerV2ManagerWrapper.contractAddress, TransferManager: TransferManagerWrapper.contractAddress, RelayerManager: RelayerManagerWrapper.contractAddress, - VersionManager: VersionManagerWrapper.contractAddress, - }); - configurator.updateInfrastructureAddresses({ - BaseWallet: BaseWalletWrapper.contractAddress, - WalletFactory: WalletFactoryWrapper.contractAddress, - DexRegistry: DexRegistryWrapper.contractAddress, + VersionManager: VersionManagerWrapper.contractAddress, }); const gitHash = childProcess.execSync("git rev-parse HEAD").toString("utf8").replace(/\n$/, ""); @@ -319,7 +239,6 @@ const deploy = async (network) => { await configurator.save(); await Promise.all([ - abiUploader.upload(VersionManagerWrapper, "modules"), abiUploader.upload(ApprovedTransferWrapper, "modules"), abiUploader.upload(CompoundManagerWrapper, "modules"), abiUploader.upload(GuardianManagerWrapper, "modules"), @@ -330,12 +249,8 @@ const deploy = async (network) => { abiUploader.upload(MakerV2ManagerWrapper, "modules"), abiUploader.upload(TransferManagerWrapper, "modules"), abiUploader.upload(RelayerManagerWrapper, "modules"), - abiUploader.upload(LimitStorageWrapper, "contracts"), - abiUploader.upload(TokenPriceRegistryWrapper, "contracts"), - abiUploader.upload(LockStorageWrapper, "contracts"), - abiUploader.upload(BaseWalletWrapper, "contracts"), - abiUploader.upload(WalletFactoryWrapper, "contracts"), - abiUploader.upload(DexRegistryWrapper, "contracts"), + + abiUploader.upload(VersionManagerWrapper, "modules"), ]); // ////////////////////////////////// @@ -380,11 +295,15 @@ const deploy = async (network) => { const upgraderName = `${version.fingerprint}_${fingerprint}`; + // if upgrading from a version strictly older than 2.1 (toRemove.length > 1), we use the "old LockStorage", + // which was part of the GuardianStorage prior to 2.1. Otherwise (toRemove.length === 1), we use the new LockStorage introduced in 2.1 + const lockStorage = (toRemove.length === 1) ? config.modules.LockStorage : config.modules.GuardianStorage; + const UpgraderWrapper = await deployer.deploy( Upgrader, {}, config.contracts.ModuleRegistry, - config.modules.GuardianStorage, // using the "old LockStorage" here which was part of the GuardianStorage in 1.6 + lockStorage, toRemove.map((module) => module.address), VersionManagerWrapper.contractAddress, // to add ); diff --git a/deployment/module_upgrade_template.js b/deployment/module_upgrade_template.js new file mode 100644 index 000000000..fb9d432cd --- /dev/null +++ b/deployment/module_upgrade_template.js @@ -0,0 +1,148 @@ +const semver = require("semver"); +const childProcess = require("child_process"); +const MultiSig = require("../build/MultiSigWallet"); +const ModuleRegistry = require("../build/ModuleRegistry"); +const VersionManager = require("../build/VersionManager"); +const Upgrader = require("../build/UpgraderToVersionManager"); +const DeployManager = require("../utils/deploy-manager.js"); +const MultisigExecutor = require("../utils/multisigexecutor.js"); + +const utils = require("../utils/utilities.js"); + +const TARGET_VERSION = "2.1.0"; +const MODULES_TO_ENABLE = [ + "VersionManager", +]; +const MODULES_TO_DISABLE = []; + +const BACKWARD_COMPATIBILITY = 3; + +const deploy = async (network) => { + if (!["kovan", "kovan-fork", "staging", "prod"].includes(network)) { + console.warn("------------------------------------------------------------------------"); + console.warn(`WARNING: The MakerManagerV2 module is not fully functional on ${network}`); + console.warn("------------------------------------------------------------------------"); + } + + const newModuleWrappers = []; + const newVersion = {}; + + // ////////////////////////////////// + // Setup + // ////////////////////////////////// + + const manager = new DeployManager(network); + await manager.setup(); + + const { configurator } = manager; + const { deployer } = manager; + const { versionUploader } = manager; + const { gasPrice } = deployer.defaultOverrides; + const deploymentWallet = deployer.signer; + const { config } = configurator; + + const ModuleRegistryWrapper = await deployer.wrapDeployedContract(ModuleRegistry, config.contracts.ModuleRegistry); + const MultiSigWrapper = await deployer.wrapDeployedContract(MultiSig, config.contracts.MultiSigWallet); + const multisigExecutor = new MultisigExecutor(MultiSigWrapper, deploymentWallet, config.multisig.autosign, { gasPrice }); + + // ////////////////////////////////// + // Initialise the new version + // ////////////////////////////////// + const VersionManagerWrapper = await deployer.wrapDeployedContract(VersionManager, config.modules.VersionManager); + + // ////////////////////////////////// + // Setup new infrastructure + // ////////////////////////////////// + + // ////////////////////////////////// + // Set contracts' managers + // ////////////////////////////////// + + // ////////////////////////////////// + // Set contracts' owners + // ////////////////////////////////// + + // ///////////////////////////////////////////////// + // Update config and Upload ABIs + // ///////////////////////////////////////////////// + + configurator.updateModuleAddresses({ + }); + + configurator.updateInfrastructureAddresses({ + }); + + const gitHash = childProcess.execSync("git rev-parse HEAD").toString("utf8").replace(/\n$/, ""); + configurator.updateGitHash(gitHash); + await configurator.save(); + + await Promise.all([ + ]); + + // ////////////////////////////////// + // Register new modules + // ////////////////////////////////// + + for (let idx = 0; idx < newModuleWrappers.length; idx += 1) { + const wrapper = newModuleWrappers[idx]; + await multisigExecutor.executeCall(ModuleRegistryWrapper, "registerModule", + [wrapper.contractAddress, utils.asciiToBytes32(wrapper._contract.contractName)]); + } + + // ////////////////////////////////// + // Deploy and Register upgraders + // ////////////////////////////////// + + let fingerprint; + const versions = await versionUploader.load(BACKWARD_COMPATIBILITY); + for (let idx = 0; idx < versions.length; idx += 1) { + const version = versions[idx]; + let toAdd; let toRemove; + if (idx === 0) { + const moduleNamesToRemove = MODULES_TO_DISABLE.concat(MODULES_TO_ENABLE); + toRemove = version.modules.filter((module) => moduleNamesToRemove.includes(module.name)); + toAdd = newModuleWrappers.map((wrapper) => ({ + address: wrapper.contractAddress, + name: wrapper._contract.contractName, + })); + const toKeep = version.modules.filter((module) => !moduleNamesToRemove.includes(module.name)); + const modulesInNewVersion = toKeep.concat(toAdd); + fingerprint = utils.versionFingerprint(modulesInNewVersion); + newVersion.version = semver.lt(version.version, TARGET_VERSION) ? TARGET_VERSION : semver.inc(version.version, "patch"); + newVersion.createdAt = Math.floor((new Date()).getTime() / 1000); + newVersion.modules = modulesInNewVersion; + newVersion.fingerprint = fingerprint; + } else { + // add all modules present in newVersion that are not present in version + toAdd = newVersion.modules.filter((module) => !version.modules.map((m) => m.address).includes(module.address)); + // remove all modules from version that are no longer present in newVersion + toRemove = version.modules.filter((module) => !newVersion.modules.map((m) => m.address).includes(module.address)); + } + + const upgraderName = `${version.fingerprint}_${fingerprint}`; + + const UpgraderWrapper = await deployer.deploy( + Upgrader, + {}, + config.contracts.ModuleRegistry, + config.modules.GuardianStorage, // using the "old LockStorage" here which was part of the GuardianStorage in 1.6 + toRemove.map((module) => module.address), + VersionManagerWrapper.contractAddress, // to add + ); + await multisigExecutor.executeCall(ModuleRegistryWrapper, "registerModule", + [UpgraderWrapper.contractAddress, utils.asciiToBytes32(upgraderName)]); + + await multisigExecutor.executeCall(ModuleRegistryWrapper, "registerUpgrader", + [UpgraderWrapper.contractAddress, utils.asciiToBytes32(upgraderName)]); + } + + // ////////////////////////////////// + // Upload Version + // ////////////////////////////////// + + await versionUploader.upload(newVersion); +}; + +module.exports = { + deploy, +}; diff --git a/package-lock.json b/package-lock.json index 4ac0d92f5..1da85e8dc 100644 --- a/package-lock.json +++ b/package-lock.json @@ -8113,9 +8113,9 @@ } }, "node-fetch": { - "version": "2.6.0", - "resolved": "https://registry.npmjs.org/node-fetch/-/node-fetch-2.6.0.tgz", - "integrity": "sha512-8dG4H5ujfvFiqDmVu9fQ5bOHUC15JMjMY/Zumv26oOvvVJjM67KF8koCWIabKQ1GJIa9r2mMZscBq/TbdOcmNA==" + "version": "2.6.1", + "resolved": "https://registry.npmjs.org/node-fetch/-/node-fetch-2.6.1.tgz", + "integrity": "sha512-V4aYg89jEoVRxRb2fJdAg8FHvI7cEyYdVAh94HH0UIK8oJxUfkjlDQN9RbMx+bEjP7+ggMiFRprSti032Oipxw==" }, "node-gyp-build": { "version": "4.2.2", diff --git a/package.json b/package.json index 012987adf..f70ed6e26 100644 --- a/package.json +++ b/package.json @@ -13,7 +13,7 @@ "compile:infrastructure_0.5": "npx etherlime compile --workingDirectory contracts/infrastructure_0.5 --solcVersion 0.5.4 --runs=999", "compile:infrastructure_0.6": "npx etherlime compile --workingDirectory contracts/infrastructure --runs=999", "compile:infrastructure": "npm run compile:infrastructure_0.5 && npm run compile:infrastructure_0.6", - "compile:modules": "npx etherlime compile --workingDirectory contracts/modules --runs=999", + "compile:modules": "npx etherlime compile --workingDirectory contracts/modules --runs=999 && rm build/ApprovedTransfer.json && npx etherlime compile --workingDirectory contracts/modules/ApprovedTransfer --runs=999 && rm build/RelayerManager.json && npx etherlime compile --workingDirectory contracts/modules/RelayerManager --runs=999 && rm build/TransferManager.json && npx etherlime compile --workingDirectory contracts/modules/TransferManager --runs=999 && rm build/TokenExchanger.json && npx etherlime compile --workingDirectory contracts/modules/TokenExchanger --runs=999", "compile:wallet": "npx etherlime compile --workingDirectory contracts/wallet --runs=999", "compile:test": "npx etherlime compile --workingDirectory contracts-test --runs=999", "compile": "npm run compile:infrastructure && npm run compile:modules && npm run compile:wallet", @@ -78,7 +78,7 @@ "inquirer": "^7.0.0", "istanbul": "^0.4.5", "lint-staged": "^10.2.11", - "node-fetch": "^2.3.0", + "node-fetch": "^2.6.1", "openzeppelin-solidity": "2.3", "ps-node": "^0.1.6", "semver": "^7.1.3", diff --git a/scripts/configReader.js b/scripts/configReader.js new file mode 100644 index 000000000..c62240a44 --- /dev/null +++ b/scripts/configReader.js @@ -0,0 +1,45 @@ +// /////////////////////////////////////////////////////////////////// +// Script to print environment configuration from AWS. +// +// Can be executed (from the project root as we're loading .env file from root via `dotenv`) as: +// bash ./scripts/execute_script.sh --no-compile scripts/configReader.js +// +// where: +// - network = [test, staging, prod] +// note: ganache configuration in solution under ./utils/config/ganache.json +// //////////////////////////////////////////////////////////////////// + +require("dotenv").config(); + +const ConfiguratorLoader = require("../utils/configurator-loader.js"); +const Configurator = require("../utils/configurator.js"); + +async function main() { + const idx = process.argv.indexOf("--network"); + const network = process.argv[idx + 1]; + const remotelyManagedNetworks = (process.env.S3_BUCKET_SUFFIXES || "").split(":"); + + // Ensure a supported network is requested + if (!remotelyManagedNetworks.includes(network)) { + console.error("Error: Invalid network selected"); + return; + } + + const bucket = `${process.env.S3_BUCKET_PREFIX}-${network}`; + const key = process.env.S3_CONFIG_KEY; + const configLoader = new ConfiguratorLoader.S3(bucket, key); + + const configurator = new Configurator(configLoader); + + // This will allow the config to be printed regardless of whether it's valid or not + await configurator.load(false); + const configuration = configurator.copyConfig(); + console.log(JSON.stringify(configuration, null, 4)); + + // Validate the configuration. Prints any validation error. + configurator._validate(); +} + +main().catch((err) => { + throw err; +}); diff --git a/scripts/deploy_wallet_detector.js b/scripts/deploy_wallet_detector.js new file mode 100644 index 000000000..1cdc478e0 --- /dev/null +++ b/scripts/deploy_wallet_detector.js @@ -0,0 +1,63 @@ +// /////////////////////////////////////////////////////////////////// +// Script to deploy a new ArgentWalletDectector. +// +// To deploy a new ArgentWalletDectector: +// ./execute_script.sh deploy_wallet_detector.js +// +// where: +// - network = [test, staging, prod] +// //////////////////////////////////////////////////////////////////// + +const ArgentWalletDetector = require("../build/ArgentWalletDetector"); +const DeployManager = require("../utils/deploy-manager.js"); + +const defaultNetwork = "test"; + +const PROXYWALLET_CODEHASH = [ + "0x0b44c9be520023d9f6091278e7e5a8853257eb9fb3d78e6951315df59679e3b2", // factory prod Mar-30-2020 + "0x83baa4b265772664a88dcfc8be0e24e1fe969a3c66f03851c6aa2f5da73cd7fd", // factory prod Feb-04-2019 +]; + +const BASEWALLET_IMPL = [ + "0xb1dd690cc9af7bb1a906a9b5a94f94191cc553ce", // prod Feb-04-2019 + "0xb6d64221451edbac7736d4c3da7fc827457dec03", // prod Mar-30-2020 + "0x8cbe893fb3372e3ce1e63ad0262b2a544fa1fb9c", // staging Jan-24-2019 + "0x609282d2d8f9ba4bb87ac9c38de20ed5de86596b", // staging Dec-06-2019 + "0xb11da8fbd8126f4f66c093070ecb8316734a7130", // staging Mar-10-2020 +]; // mainnet only + +async function main() { + // Read Command Line Arguments + const idx = process.argv.indexOf("--network"); + const network = idx > -1 ? process.argv[idx + 1] : defaultNetwork; + + const deployManager = new DeployManager(network); + await deployManager.setup(); + const { deployer, configurator, abiUploader } = deployManager; + const { config } = configurator; + + // Deploy ArgentWalletDetector contract + console.log("Deploying ArgentWalletDetector..."); + const ArgentWalletDetectortWrapper = await deployer.deploy(ArgentWalletDetector, {}, PROXYWALLET_CODEHASH, BASEWALLET_IMPL); + + // Transfer ownership to the multisig + console.log("Transferring ownership to the Multisig..."); + await ArgentWalletDetectortWrapper.changeOwner(config.contracts.MultiSigWallet); + + // Update config + configurator.updateInfrastructureAddresses({ + ArgentWalletDetector: ArgentWalletDetectortWrapper.contractAddress, + }); + await configurator.save(); + + // Upload ABI + await Promise.all([ + abiUploader.upload(ArgentWalletDetectortWrapper, "contracts"), + ]); +} + +main().catch((err) => { + throw err; +}); + +// contract deployed to prod at 0xeca4B0bDBf7c55E9b7925919d03CbF8Dc82537E8 diff --git a/scripts/execute_script.sh b/scripts/execute_script.sh index 8a6947b11..ef2a46fba 100755 --- a/scripts/execute_script.sh +++ b/scripts/execute_script.sh @@ -14,7 +14,7 @@ if [ $NOCOMPILE != "--no-compile" ]; then npm run compile:lib npm run compile npm run compile:legacy - FILE=NO-COMPILE + FILE=$NOCOMPILE else FILE=$1 shift diff --git a/scripts/update_wallet_detector.js b/scripts/update_wallet_detector.js new file mode 100644 index 000000000..9646ef9fa --- /dev/null +++ b/scripts/update_wallet_detector.js @@ -0,0 +1,80 @@ +// /////////////////////////////////////////////////////////////////// +// Script to add a new wallet version (code and/or implementation) to the ArgentWalletDetector. +// +// ////////////////////////// WARNING /////////////////////////////////////////////// +// There is only one instance deployed that detects wallets for both staging and prod. +// ////////////////////////////////////////////////////////////////////////////////// +// +// To add a new wallet (code + implementation): +// ./execute_script.sh update_wallet_detector.js --wallet +// +// To add a new implementation: +// ./execute_script.sh update_wallet_detector.js --implementation +// +// To add a new code: +// ./execute_script.sh update_wallet_detector.js --code +// +// where: +// - network = [test, prod] +// //////////////////////////////////////////////////////////////////// + +const ArgentWalletDetector = require("../build/ArgentWalletDetector"); +const MultiSig = require("../build/MultiSigWallet"); + +const DeployManager = require("../utils/deploy-manager.js"); +const MultisigExecutor = require("../utils/multisigexecutor.js"); + +async function main() { + let wallet; + let code; + let implementation; + + // Read Command Line Arguments + let idx = process.argv.indexOf("--network"); + const network = process.argv[idx + 1]; + + idx = process.argv.indexOf("--wallet"); + if (idx > 0) { + wallet = process.argv[idx + 1]; + } else { + idx = process.argv.indexOf("--implementation"); + if (idx > 0) { + implementation = process.argv[idx + 1]; + } else { + idx = process.argv.indexOf("--code"); + if (idx > 0) { + code = process.argv[idx + 1]; + } else { + console.log("Error: No argument provided"); + return; + } + } + } + + // Setup deployer + const manager = new DeployManager(network); + await manager.setup(); + const { configurator } = manager; + const { deployer } = manager; + const deploymentWallet = deployer.signer; + const { config } = configurator; + + const ArgentWalletDetectorWrapper = await deployer.wrapDeployedContract(ArgentWalletDetector, config.contracts.ArgentWalletDetector); + const MultiSigWrapper = await deployer.wrapDeployedContract(MultiSig, config.contracts.MultiSigWallet); + const multisigExecutor = new MultisigExecutor(MultiSigWrapper, deploymentWallet, config.multisig.autosign); + + if (wallet) { + console.log(`Adding wallet code and implementation from ${wallet}`); + await multisigExecutor.executeCall(ArgentWalletDetectorWrapper, "addCodeAndImplementationFromWallet", [wallet]); + } else if (code) { + console.log(`Adding wallet code ${code}`); + await multisigExecutor.executeCall(ArgentWalletDetectorWrapper, "addCode", [code]); + } else if (implementation) { + console.log(`Adding wallet implementation ${implementation}`); + await multisigExecutor.executeCall(ArgentWalletDetectorWrapper, "addImplementation", [implementation]); + } +} + +main().catch((err) => { + throw err; +}); diff --git a/test/argentWalletDetector.js b/test/argentWalletDetector.js new file mode 100644 index 000000000..dc424f91b --- /dev/null +++ b/test/argentWalletDetector.js @@ -0,0 +1,111 @@ +/* global utils */ +const ethers = require("ethers"); +const ArgentWalletDetector = require("../build/ArgentWalletDetector"); +const Proxy = require("../build/Proxy"); +const BaseWallet = require("../build/BaseWallet"); + +const TestManager = require("../utils/test-manager"); + +const RANDOM_CODE = "0x880ac7547a884027b93f5eaba5ff545919fdeb3c23ed0d2094db66303b3a80ac"; +const ZERO_BYTES32 = ethers.constants.HashZero; +const ZERO_ADDRESS = ethers.constants.AddressZero; + +const EMPTY_CODE_MSG = "AWR: empty _code"; +const EMPTY_IMPL_MSG = "AWR: empty _impl"; + +describe("ArgentWalletDetector", () => { + const manager = new TestManager(); + + let deployer; + let detector; + let implementation1; + let implementation2; + let proxy1; + let proxy2; + let argentCode; + + before(async () => { + deployer = manager.newDeployer(); + implementation1 = await deployer.deploy(BaseWallet); + implementation2 = await deployer.deploy(BaseWallet); + proxy1 = await deployer.deploy(Proxy, {}, implementation1.contractAddress); + proxy2 = await deployer.deploy(Proxy, {}, implementation2.contractAddress); + argentCode = ethers.utils.keccak256(proxy1._contract.deployedBytecode); + }); + + beforeEach(async () => { + detector = await deployer.deploy(ArgentWalletDetector, {}, [], []); + }); + + describe("add info", () => { + it("should deploy with codes and implementations", async () => { + const c = [argentCode, RANDOM_CODE]; + const i = [implementation1.contractAddress, implementation2.contractAddress]; + detector = await deployer.deploy(ArgentWalletDetector, {}, c, i); + const implementations = await detector.getImplementations(); + assert.equal(implementations[0], implementation1.contractAddress, "should have added first implementation"); + assert.equal(implementations[1], implementation2.contractAddress, "should have added second implementation"); + const codes = await detector.getCodes(); + assert.equal(codes[0], argentCode, "should have added first code"); + assert.equal(codes[1], RANDOM_CODE, "should have added second code"); + }); + + it("should add implementations", async () => { + await detector.addImplementation(implementation1.contractAddress); + await detector.addImplementation(implementation2.contractAddress); + const implementations = await detector.getImplementations(); + assert.equal(implementations[0], implementation1.contractAddress, "should have added first implementation"); + assert.equal(implementations[1], implementation2.contractAddress, "should have added second implementation"); + }); + + it("should add codes", async () => { + await detector.addCode(argentCode); + await detector.addCode(RANDOM_CODE); + const codes = await detector.getCodes(); + assert.equal(codes[0], argentCode, "should have added first code"); + assert.equal(codes[1], RANDOM_CODE, "should have added second code"); + }); + + it("should not add an existing implementation", async () => { + await detector.addImplementation(implementation1.contractAddress); + const tx = await detector.addImplementation(implementation1.contractAddress); + const txReceipt = await detector.verboseWaitForTransaction(tx); + assert.isFalse(await utils.hasEvent(txReceipt, detector, "ImplementationAdded"), "should not have generated ImplementationAdded event"); + }); + + it("should not add an existing code", async () => { + await detector.addCode(argentCode); + const tx = await detector.addCode(argentCode); + const txReceipt = await detector.verboseWaitForTransaction(tx); + assert.isFalse(await utils.hasEvent(txReceipt, detector, "CodeAdded"), "should not have generated CodeAdded event"); + }); + + it("should fail to add an empty code", async () => { + await assert.revertWith(detector.addCode(ZERO_BYTES32), EMPTY_CODE_MSG); + }); + + it("should fail to add an empty implementation", async () => { + await assert.revertWith(detector.addImplementation(ZERO_ADDRESS), EMPTY_IMPL_MSG); + }); + + it("should add code and implementation from a wallet", async () => { + await detector.addCodeAndImplementationFromWallet(proxy1.contractAddress); + const isArgent = await detector.isArgentWallet(proxy1.contractAddress); + assert.isTrue(isArgent, "should return true for an Argent wallet"); + }); + + it("should return false when the code is not correct", async () => { + await detector.addImplementation(implementation1.contractAddress); + await detector.addCode(RANDOM_CODE); + const isArgent = await detector.isArgentWallet(proxy1.contractAddress); + assert.isFalse(isArgent, "should return false when the code is not correct"); + }); + + it("should return false when the implementation is not correct", async () => { + await detector.addImplementation(implementation1.contractAddress); + await detector.addCode(argentCode); + const isArgent = await detector.isArgentWallet(proxy2.contractAddress); + assert.isFalse(isArgent, "should return false when the implementation is not correct"); + }); + }); +}); diff --git a/test/baseWallet.js b/test/baseWallet.js index 5994e5946..03349c996 100644 --- a/test/baseWallet.js +++ b/test/baseWallet.js @@ -42,7 +42,6 @@ describe("BaseWallet", function () { const feature = await deployer.deploy(TestFeature, {}, lockStorage.contractAddress, module.contractAddress, - true, 42); await module.addVersion([feature.contractAddress], []); return { module, feature }; diff --git a/test/makerV2Manager_loan.js b/test/makerV2Manager_loan.js index 4af504c25..bc96a1a88 100644 --- a/test/makerV2Manager_loan.js +++ b/test/makerV2Manager_loan.js @@ -763,7 +763,7 @@ describe("MakerV2 Vaults", function () { it("should not allow (fake) feature to give unowned vault", async () => { // Deploy a (fake) bad feature - const badFeature = await deployer.deploy(BadFeature, {}, lockStorage.contractAddress, versionManager.contractAddress, false, 0); + const badFeature = await deployer.deploy(BadFeature, {}, lockStorage.contractAddress, versionManager.contractAddress, 0); // Add the bad feature to the wallet await versionManager.addVersion([ diff --git a/test/nftTransfer.js b/test/nftTransfer.js index bf48155c1..66cb72618 100644 --- a/test/nftTransfer.js +++ b/test/nftTransfer.js @@ -8,7 +8,7 @@ const VersionManager = require("../build/VersionManager"); const RelayerManager = require("../build/RelayerManager"); const LockStorage = require("../build/LockStorage"); const GuardianStorage = require("../build/GuardianStorage"); -const NftModule = require("../build/NftTransfer"); +const NftTransfer = require("../build/NftTransfer"); const TokenPriceRegistry = require("../build/TokenPriceRegistry"); const ERC721 = require("../build/TestERC721"); @@ -19,7 +19,7 @@ const ERC20Approver = require("../build/ERC20Approver"); const ZERO_BYTES32 = ethers.constants.HashZero; const TestManager = require("../utils/test-manager"); -const { parseRelayReceipt } = require("../utils/utilities.js"); +const { parseRelayReceipt, callStatic } = require("../utils/utilities.js"); describe("Token Transfer", function () { this.timeout(100000); @@ -71,7 +71,7 @@ describe("Token Transfer", function () { ck = await deployer.deploy(CK); tokenPriceRegistry = await deployer.deploy(TokenPriceRegistry); await tokenPriceRegistry.addManager(infrastructure.address); - nftFeature = await deployer.deploy(NftModule, {}, + nftFeature = await deployer.deploy(NftTransfer, {}, lockStorage.contractAddress, tokenPriceRegistry.contractAddress, versionManager.contractAddress, @@ -112,7 +112,7 @@ describe("Token Transfer", function () { } } else { const txPromise = nftFeature.from(owner1) - .transferNFT(wallet1.contractAddress, nftContract.contractAddress, recipientAddress, nftId, safe, ZERO_BYTES32); + .transferNFT(wallet1.contractAddress, nftContract.contractAddress, recipientAddress, nftId, safe, ZERO_BYTES32, { gasLimit: 300000 }); if (shouldSucceed) { await txPromise; } else { @@ -230,5 +230,17 @@ describe("Token Transfer", function () { }); }); }); + + describe("Static calls", () => { + it("should delegate onERC721Received static calls to the NftTransfer feature", async () => { + const ERC721_RECEIVED = ethers.utils.keccak256(ethers.utils.toUtf8Bytes("onERC721Received(address,address,uint256,bytes)")).slice(0, 10); + const erc721ReceivedDelegate = await wallet1.enabled(ERC721_RECEIVED); + assert.equal(erc721ReceivedDelegate, versionManager.contractAddress); + + const walletAsTransferManager = deployer.wrapDeployedContract(NftTransfer, wallet1.contractAddress); + const result = await callStatic(walletAsTransferManager, "onERC721Received", infrastructure.address, infrastructure.address, 0, "0x"); + assert.equal(result, ERC721_RECEIVED); + }); + }); }); }); diff --git a/test/relayer.js b/test/relayer.js index 886087163..ca282049e 100644 --- a/test/relayer.js +++ b/test/relayer.js @@ -110,8 +110,8 @@ describe("RelayerManager", function () { versionManager.contractAddress, 36, 24 * 5); - testFeature = await deployer.deploy(TestFeature, {}, lockStorage.contractAddress, versionManager.contractAddress, false, 0); - testFeatureNew = await deployer.deploy(TestFeature, {}, lockStorage.contractAddress, versionManager.contractAddress, false, 0); + testFeature = await deployer.deploy(TestFeature, {}, lockStorage.contractAddress, versionManager.contractAddress, 0); + testFeatureNew = await deployer.deploy(TestFeature, {}, lockStorage.contractAddress, versionManager.contractAddress, 0); limitFeature = await deployer.deploy(TestLimitFeature, {}, lockStorage.contractAddress, limitStorage.contractAddress, versionManager.contractAddress); diff --git a/test/transferManager.js b/test/transferManager.js index 30c986320..cfc96e6d0 100644 --- a/test/transferManager.js +++ b/test/transferManager.js @@ -24,7 +24,7 @@ const ERC20 = require("../build/TestERC20"); const WETH = require("../build/WETH9"); const TestContract = require("../build/TestContract"); -const { ETH_TOKEN, hasEvent } = require("../utils/utilities.js"); +const { ETH_TOKEN, hasEvent, personalSign } = require("../utils/utilities.js"); const ETH_LIMIT = 1000000; const SECURITY_PERIOD = 2; @@ -888,4 +888,36 @@ describe("TransferManager", function () { await doApproveTokenAndCallContract({ amount, state: 3, wrapEth: true }); }); }); + + describe("Static calls", () => { + it("should delegate isValidSignature static calls to the TransferManager", async () => { + const ERC1271_ISVALIDSIGNATURE_BYTES32 = ethers.utils.keccak256(ethers.utils.toUtf8Bytes("isValidSignature(bytes32,bytes)")).slice(0, 10); + const isValidSignatureDelegate = await wallet.enabled(ERC1271_ISVALIDSIGNATURE_BYTES32); + assert.equal(isValidSignatureDelegate, versionManager.contractAddress); + + const walletAsTransferManager = deployer.wrapDeployedContract(TransferManager, wallet.contractAddress); + const signHash = ethers.utils.keccak256("0x1234"); + const sig = await personalSign(signHash, owner); + const valid = await walletAsTransferManager.isValidSignature(signHash, sig); + assert.equal(valid, ERC1271_ISVALIDSIGNATURE_BYTES32); + }); + it("should revert isValidSignature static call for invalid signature", async () => { + const walletAsTransferManager = deployer.wrapDeployedContract(TransferManager, wallet.contractAddress); + const signHash = ethers.utils.keccak256("0x1234"); + const sig = `${await personalSign(signHash, owner)}a1`; + + await assert.revertWith( + walletAsTransferManager.isValidSignature(signHash, sig), "TM: invalid signature length", + ); + }); + it("should revert isValidSignature static call for invalid signer", async () => { + const walletAsTransferManager = deployer.wrapDeployedContract(TransferManager, wallet.contractAddress); + const signHash = ethers.utils.keccak256("0x1234"); + const sig = await personalSign(signHash, nonowner); + + await assert.revertWith( + walletAsTransferManager.isValidSignature(signHash, sig), "TM: Invalid signer", + ); + }); + }); }); diff --git a/test/versionManager.js b/test/versionManager.js index 2fad59d52..c2bfc5043 100644 --- a/test/versionManager.js +++ b/test/versionManager.js @@ -1,5 +1,6 @@ /* global accounts */ const ethers = require("ethers"); + const GuardianManager = require("../build/GuardianManager"); const LockStorage = require("../build/LockStorage"); const GuardianStorage = require("../build/GuardianStorage"); @@ -7,8 +8,13 @@ const Proxy = require("../build/Proxy"); const BaseWallet = require("../build/BaseWallet"); const RelayerManager = require("../build/RelayerManager"); const VersionManager = require("../build/VersionManager"); +const TransferStorage = require("../build/TransferStorage"); +const LimitStorage = require("../build/LimitStorage"); +const TokenPriceRegistry = require("../build/TokenPriceRegistry"); +const TransferManager = require("../build/TransferManager"); const Registry = require("../build/ModuleRegistry"); const TestFeature = require("../build/TestFeature"); +const UpgraderToVersionManager = require("../build/UpgraderToVersionManager"); const TestManager = require("../utils/test-manager"); @@ -22,6 +28,7 @@ describe("VersionManager", function () { let deployer; let wallet; let walletImplementation; + let registry; let lockStorage; let guardianStorage; let guardianManager; @@ -35,7 +42,7 @@ describe("VersionManager", function () { }); beforeEach(async () => { - const registry = await deployer.deploy(Registry); + registry = await deployer.deploy(Registry); lockStorage = await deployer.deploy(LockStorage); guardianStorage = await deployer.deploy(GuardianStorage); versionManager = await deployer.deploy(VersionManager, {}, @@ -59,7 +66,6 @@ describe("VersionManager", function () { testFeature = await deployer.deploy(TestFeature, {}, lockStorage.contractAddress, versionManager.contractAddress, - true, 42); await versionManager.addVersion([guardianManager.contractAddress, relayerManager.contractAddress, testFeature.contractAddress], []); manager.setRelayerManager(relayerManager); @@ -142,5 +148,49 @@ describe("VersionManager", function () { lock = await newGuardianStorage.getLock(wallet.contractAddress); assert.isTrue(lock.eq(0), "Lock should not be set"); }); + + it("should not allow the fallback to be called via a non-static call", async () => { + // Deploy new VersionManager with TransferManager + const versionManager2 = await deployer.deploy(VersionManager, {}, + registry.contractAddress, + lockStorage.contractAddress, + guardianStorage.contractAddress, + ethers.constants.AddressZero, + ethers.constants.AddressZero); + const tokenPriceRegistry = await deployer.deploy(TokenPriceRegistry); + const transferStorage = await deployer.deploy(TransferStorage); + const limitStorage = await deployer.deploy(LimitStorage); + const transferManager = await deployer.deploy(TransferManager, {}, + lockStorage.contractAddress, + transferStorage.contractAddress, + limitStorage.contractAddress, + tokenPriceRegistry.contractAddress, + versionManager2.contractAddress, + 3600, + 3600, + 10000, + ethers.constants.AddressZero, + ethers.constants.AddressZero); + await versionManager2.addVersion([transferManager.contractAddress], []); + await registry.registerModule(versionManager2.contractAddress, ethers.utils.formatBytes32String("VersionManager2")); + + // Deploy Upgrader to new VersionManager + const upgrader = await deployer.deploy(UpgraderToVersionManager, {}, + registry.contractAddress, + lockStorage.contractAddress, + [versionManager.contractAddress], // toDisable + versionManager2.contractAddress); + await registry.registerModule(upgrader.contractAddress, ethers.utils.formatBytes32String("Upgrader")); + + // Upgrade wallet to new VersionManger + await versionManager.from(owner).addModule(wallet.contractAddress, upgrader.contractAddress); + + // Attempt to call a malicious (non-static) call on the old VersionManager + const data = testFeature.contract.interface.functions.badStaticCall.encode([]); + await assert.revertWith( + transferManager.from(owner).callContract(wallet.contractAddress, versionManager.contractAddress, 0, data), + "VM: not in a staticcall", + ); + }); }); }); diff --git a/utils/config-schema.json b/utils/config-schema.json index 13234b78e..a462c9470 100644 --- a/utils/config-schema.json +++ b/utils/config-schema.json @@ -131,6 +131,9 @@ }, "DexRegistry": { "$ref": "#/definitions/ethaddress" + }, + "ArgentWalletDetector": { + "$ref": "#/definitions/ethaddress" } }, "required": [ @@ -165,6 +168,9 @@ "TokenPriceStorage": { "$ref": "#/definitions/ethaddress" }, + "TokenPriceRegistry": { + "$ref": "#/definitions/ethaddress" + }, "LockManager": { "$ref": "#/definitions/ethaddress" }, diff --git a/utils/config/ganache.json b/utils/config/ganache.json index 05cfeef22..d0a36d727 100644 --- a/utils/config/ganache.json +++ b/utils/config/ganache.json @@ -1 +1 @@ -{"ENS":{"deployOwnRegistry":true,"ensRegistry":"0x54453dF26C5C638C68dF46e3f1B83360b0Ab29fd","domain":"argent.xyz"},"backend":{"accounts":["0xD9995BAE12FEe327256FFec1e3184d492bD94C31"]},"multisig":{"owners":["0xD9995BAE12FEe327256FFec1e3184d492bD94C31"],"threshold":1,"autosign":true},"settings":{"deployer":{"type":"ganache"},"lockPeriod":480,"recoveryPeriod":480,"securityPeriod":240,"securityWindow":240,"feeRatio":15,"defaultLimit":"1000000000000000000"},"CryptoKitties":{"contract":"0x0000000000000000000000000000000000000000"},"defi":{"weth":"0x0000000000000000000000000000000000000000","maker":{"deployOwn":true,"tub":"0x0000000000000000000000000000000000000000","pot":"0x0000000000000000000000000000000000000000","jug":"0x0000000000000000000000000000000000000000","migration":"0xC07f212AaEeF5Dc948BBC5302cB46D9D799417Ec"},"uniswap":{"deployOwn":true,"factory":"0x4d5440AC7CEA2DDBc92Fe35756c90EEBbAe58cEb"},"compound":{"comptroller":"0x0000000000000000000000000000000000000000","markets":{"0x0000000000000000000000000000000000000000":"0x0000000000000000000000000000000000000000"}},"paraswap":{"deployOwn":true,"contract":"0x41B10475131cA22A3D574DEa3B3670B5ab2AD90E","authorisedExchanges":{"Kyber":"0x8f8f0454f35F5daE79E6129329Dd3C80AEc3A8cc"}}},"contracts":{"MultiSigWallet":"0x019e04b6fc11305c273924B58F93e8c7ecF934B9","WalletFactory":"0x2F2c4e8e7b8c31d836e1158A10902b504D0Bc71F","ENSResolver":"0x061ABd0F2579b1A41a3C8b741a74485eA96e9AF0","ENSManager":"0x39641b8415416d924BC625362571826485e6DB1b","TokenPriceProvider":"0xE09C322B745434Fbb2D18c107c13795D4bDF3657","ModuleRegistry":"0x6B74146d42286a96C656f0368C0dE4A43c3b546a","BaseWallet":"0x406F6ae39429b35853d54e1d75De71F793fB5C6b","CompoundRegistry":"0xfDEd0b9b98a28964208A3c3a1351F7414eD743EC","MakerRegistry":"0x355d26E5f04255F60E910F1aA1251DE5D10996E8","DexRegistry":"0xbf7E0dcC7a3D725Fb0768836cfd32c13a9ef1324"},"modules":{"GuardianStorage":"0x6c4539ea80eFF2b0FA546ca0A795A8Cf84d1b31C","TransferStorage":"0x3Ab452E9436fCc8Ba1CA86F9112Ffe18653Ca6Be","GuardianManager":"0xa375B16db7412f1104A8820f8C7485F19dA5deA0","LockManager":"0x215a1EfB566aA85c756C69Fc748E584AEc13CCb1","RecoveryManager":"0x1E8c7A43E530c73FA69F0DE1805AABa73336eD34","ApprovedTransfer":"0x6CaDE3FAa7FF8CA5961AB26F5f25D56E7420f584","TokenExchanger":"0xC8ECF2fb6DE001995D35FB707E2655B070b7a06E","NftTransfer":"0x33454011c495540CC30bfFaed39644F0F779cf73","MakerManager":"0x2718cAce241213B18F67C6d66B245EE77DB0Ce2a","TransferManager":"0x47C5A7F8F78c531c789A25adACf8169cA1d8B173","CompoundManager":"0xD33fA1776ff3073d110874eBe54f405Ae10BbD46","MakerV2Manager":"0x0F0A56e838323Df990DDD41cd95e4A93942bC6ff","LimitStorage":"0x9A9BA9ec5A949f453162fa6C0541F580162Ae400","TokenPriceStorage":"0x5dA4F81c17776495e3d7999ad9217748D257f074","RelayerModule":"0x171eb25c80da6adF0436668126285ca51e37eB30","RelayerManager":"0xC9737dA2aAFA08F296805C50bE83A7086682beAf","VersionManager":"0x320762f547A460453A1115251F72F6E4daef3c6c","LockStorage":"0x189F52364c5fC5dd16600b42094F2a04078AE928"},"gitCommit":"37220550ce6b4dbe303069d309ea56765e48be3a"} \ No newline at end of file +{"ENS":{"deployOwnRegistry":true,"ensRegistry":"0xec5a038096A3bbb667F6f6bAeB6452E3dEa08FAE","domain":"argent.xyz"},"backend":{"accounts":["0xD9995BAE12FEe327256FFec1e3184d492bD94C31"]},"multisig":{"owners":["0xD9995BAE12FEe327256FFec1e3184d492bD94C31"],"threshold":1,"autosign":true},"settings":{"deployer":{"type":"ganache"},"lockPeriod":480,"recoveryPeriod":480,"securityPeriod":240,"securityWindow":240,"feeRatio":15,"defaultLimit":"1000000000000000000"},"CryptoKitties":{"contract":"0x0000000000000000000000000000000000000000"},"defi":{"weth":"0x0000000000000000000000000000000000000000","maker":{"deployOwn":true,"tub":"0x0000000000000000000000000000000000000000","pot":"0x0000000000000000000000000000000000000000","jug":"0x0000000000000000000000000000000000000000","migration":"0xe61A2910280f84c79B08cea5Ea1250aA3D94A956"},"uniswap":{"deployOwn":true,"factory":"0x593258e0672cAB972d493087C0400012973b3392"},"compound":{"comptroller":"0x0000000000000000000000000000000000000000","markets":{"0x0000000000000000000000000000000000000000":"0x0000000000000000000000000000000000000000"}},"paraswap":{"deployOwn":true,"contract":"0x403cB18300b16b3019Ea1b3a74F5C2bB2036B8a5","authorisedExchanges":{"Kyber":"0x0e902fAaeA3931C6cBe2aE6952D35bF6F94dda5b"}}},"contracts":{"MultiSigWallet":"0x4A654B3814e634f705BC68f0b8aC88708Bf273C5","WalletFactory":"0xBfC94C3625D9939F42A010aeB43d7B52423a28a7","ENSResolver":"0xb3FF69d8fe13D8e3e079cC19c37d79d5f4BcE931","ENSManager":"0x4E053eD2c9F52ac3e49ae91610044d2dafB49DE3","TokenPriceProvider":"0xE09C322B745434Fbb2D18c107c13795D4bDF3657","ModuleRegistry":"0x28654Ae24FC1A9F8B67Aa7c59E2C649Bb0b95da3","BaseWallet":"0xe7E43334B154F6f206FbE5a8ad6Bf833eB53A9Ec","CompoundRegistry":"0x4F25631Fd027DF7718853469f00c40Ca9F7C9ec3","MakerRegistry":"0x355d26E5f04255F60E910F1aA1251DE5D10996E8","DexRegistry":"0x20704257D0dFcea212e0477ecDF9c83358398cA1"},"modules":{"GuardianStorage":"0x230cB3766cd793Fc5a4a0758e80bdb68aa7A0AA1","TransferStorage":"0x0e3aB1A327703c20792c09F6F9a6C5341b74466C","GuardianManager":"0xD8516Fe9e2b04cd16c3d8DF32919EaA4bEa1d290","LockManager":"0xff13C8EA0c254c2483DB68A39C837DAE7c76AB99","RecoveryManager":"0x7BBE875A29C7F3057124eE40947A4b0AD891862C","ApprovedTransfer":"0x24bCE36F41AF503806fa93Bf8e403649f70b676F","TokenExchanger":"0x79136e33B3AB104a50F8D6987dd541E8DdaCbAfF","NftTransfer":"0xC5634C4fEF9C125e7F41526B77ffd814a2726024","MakerManager":"0x2718cAce241213B18F67C6d66B245EE77DB0Ce2a","TransferManager":"0xeDa4A2478d48162f638bDd5449aC303C7F16989D","CompoundManager":"0x71E86c118CcbEA7Ab080E0fCD2A0c165C4722E4D","MakerV2Manager":"0x2AFe5ec8F1b61EE207af4ccda0b26F8C32EEf968","LimitStorage":"0x0353C0A8774bF8C0014f43460a1968D55bddE070","TokenPriceStorage":"0x5dA4F81c17776495e3d7999ad9217748D257f074","RelayerModule":"0x171eb25c80da6adF0436668126285ca51e37eB30","RelayerManager":"0x837b4CAD2aB35281de25Fa9Bad1f7ba93590a286","VersionManager":"0x12f117EA2266DB9c697A677a41e27383Dc631575","LockStorage":"0x0979bb118063E91911015296657C6a9cC39a3AD8","TokenPriceRegistry":"0xa254E4C169D60Aa31B4E5FD55D198b6FCd869578"},"gitCommit":"80a9a710b7a48e300bf3548ab770d635f6c59582"} \ No newline at end of file diff --git a/utils/utilities.js b/utils/utilities.js index 6f5a0407a..fc07ce66c 100644 --- a/utils/utilities.js +++ b/utils/utilities.js @@ -135,4 +135,15 @@ module.exports = { 32, ); }, + + personalSign: async (signHash, signer) => ethers.utils.joinSignature(signer.signingKey.signDigest(signHash)), + + callStatic: async (contractWrapper, method, ...args) => { + const contract = new ethers.Contract( + contractWrapper.contractAddress, + contractWrapper.contract.interface.abi, + ethers.getDefaultProvider(contractWrapper.provider.connection.url), + ); + return contract.callStatic[method](...args); + }, }; diff --git a/utils/versions/ganache/latest.json b/utils/versions/ganache/latest.json index b21b186a7..efa8eaec4 100644 --- a/utils/versions/ganache/latest.json +++ b/utils/versions/ganache/latest.json @@ -1 +1 @@ -{"version":"2.0.1","createdAt":1599484922,"modules":[{"address":"0x320762f547A460453A1115251F72F6E4daef3c6c","name":"VersionManager"}],"fingerprint":"0x9ef43672"} \ No newline at end of file +{"modules":[{"address":"0x12f117EA2266DB9c697A677a41e27383Dc631575","name":"VersionManager"}],"fingerprint":"0xbf3b947b","version":"1.0.0","createdAt":1603262427} \ No newline at end of file