From 67a74f44cd1d73fbc6ff9de8934aa9782a6a09bb Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 11 Sep 2020 14:00:30 +0000 Subject: [PATCH 01/47] Bump node-fetch from 2.6.0 to 2.6.1 Bumps [node-fetch](https://github.com/bitinn/node-fetch) from 2.6.0 to 2.6.1. - [Release notes](https://github.com/bitinn/node-fetch/releases) - [Changelog](https://github.com/node-fetch/node-fetch/blob/master/docs/CHANGELOG.md) - [Commits](https://github.com/bitinn/node-fetch/compare/v2.6.0...v2.6.1) Signed-off-by: dependabot[bot] --- package-lock.json | 6 +++--- package.json | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) 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..8889bd1cc 100644 --- a/package.json +++ b/package.json @@ -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", From 532ced7be755a5fad67a0f75da19bce47c6531da Mon Sep 17 00:00:00 2001 From: elenadimitrova Date: Thu, 8 Oct 2020 16:35:28 +0300 Subject: [PATCH 02/47] Add configuration reader script --- scripts/configReader.js | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 scripts/configReader.js diff --git a/scripts/configReader.js b/scripts/configReader.js new file mode 100644 index 000000000..4cf6f0a28 --- /dev/null +++ b/scripts/configReader.js @@ -0,0 +1,36 @@ +// Example Usage: +// from the project root (as we're loading .env file from root via `dotenv`) +// bash ./scripts/execute.sh scripts/configReader.js staging + +require("dotenv").config(); + +const path = require("path"); + +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 env = process.env.CONFIG_ENVIRONMENT; + const remotelyManagedNetworks = (process.env.S3_BUCKET_SUFFIXES || "").split(":"); + + let configLoader; + if (remotelyManagedNetworks.includes(network)) { + const bucket = `${process.env.S3_BUCKET_PREFIX}-${network}`; + const key = process.env.S3_CONFIG_KEY; + configLoader = new ConfiguratorLoader.S3(bucket, key); + } else { + const fileName = env ? `${network}.${env}.json` : `${network}.json`; + const filePath = path.join(__dirname, "./config", fileName); + configLoader = new ConfiguratorLoader.Local(filePath); + } + const configurator = new Configurator(configLoader); + await configurator.load(); + const configuration = configurator.copyConfig(); + console.log(configuration); +} + +main().catch((err) => { + throw err; +}); From b5f3db5c756a0f8df02523e5e022981e122ba531 Mon Sep 17 00:00:00 2001 From: elenadimitrova Date: Mon, 19 Oct 2020 10:48:22 +0300 Subject: [PATCH 03/47] Update script format to match the changes for execute_script from PR #161 --- scripts/configReader.js | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/scripts/configReader.js b/scripts/configReader.js index 4cf6f0a28..c3197ee0f 100644 --- a/scripts/configReader.js +++ b/scripts/configReader.js @@ -1,6 +1,13 @@ -// Example Usage: -// from the project root (as we're loading .env file from root via `dotenv`) -// bash ./scripts/execute.sh scripts/configReader.js staging +// /////////////////////////////////////////////////////////////////// +// 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(); @@ -15,16 +22,16 @@ async function main() { const env = process.env.CONFIG_ENVIRONMENT; const remotelyManagedNetworks = (process.env.S3_BUCKET_SUFFIXES || "").split(":"); - let configLoader; - if (remotelyManagedNetworks.includes(network)) { - const bucket = `${process.env.S3_BUCKET_PREFIX}-${network}`; - const key = process.env.S3_CONFIG_KEY; - configLoader = new ConfiguratorLoader.S3(bucket, key); - } else { - const fileName = env ? `${network}.${env}.json` : `${network}.json`; - const filePath = path.join(__dirname, "./config", fileName); - configLoader = new ConfiguratorLoader.Local(filePath); + // 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); await configurator.load(); const configuration = configurator.copyConfig(); From 24b448903230ef1931de3699fad3ed7e5ff0192e Mon Sep 17 00:00:00 2001 From: elenadimitrova Date: Mon, 19 Oct 2020 11:07:51 +0300 Subject: [PATCH 04/47] Allow configuration to be printed even if it's invalid --- scripts/configReader.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/scripts/configReader.js b/scripts/configReader.js index c3197ee0f..b4337509a 100644 --- a/scripts/configReader.js +++ b/scripts/configReader.js @@ -33,9 +33,14 @@ async function main() { const configLoader = new ConfiguratorLoader.S3(bucket, key); const configurator = new Configurator(configLoader); - await configurator.load(); + + // 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(configuration); + + // Validate the configuration. Prints any validation error. + configurator._validate(); } main().catch((err) => { From 9109b7a7803390f55512628f425e2ec1f42757a4 Mon Sep 17 00:00:00 2001 From: elenadimitrova Date: Mon, 19 Oct 2020 13:26:18 +0300 Subject: [PATCH 05/47] json.stringify the config to show all entries --- scripts/configReader.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/configReader.js b/scripts/configReader.js index b4337509a..2f2f86247 100644 --- a/scripts/configReader.js +++ b/scripts/configReader.js @@ -37,7 +37,7 @@ async function main() { // 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(configuration); + console.log(JSON.stringify(configuration, null, 4)); // Validate the configuration. Prints any validation error. configurator._validate(); From 694124d156fae104bf5e01303f1d59e23099763c Mon Sep 17 00:00:00 2001 From: elenadimitrova Date: Mon, 19 Oct 2020 13:26:47 +0300 Subject: [PATCH 06/47] json.stringify the config to show all entries --- scripts/configReader.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/scripts/configReader.js b/scripts/configReader.js index 2f2f86247..c62240a44 100644 --- a/scripts/configReader.js +++ b/scripts/configReader.js @@ -11,15 +11,12 @@ require("dotenv").config(); -const path = require("path"); - 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 env = process.env.CONFIG_ENVIRONMENT; const remotelyManagedNetworks = (process.env.S3_BUCKET_SUFFIXES || "").split(":"); // Ensure a supported network is requested From 2c637888b9c8f88a48976c0883c3387b5bd70dc8 Mon Sep 17 00:00:00 2001 From: elenadimitrova Date: Mon, 12 Oct 2020 17:14:03 +0300 Subject: [PATCH 07/47] Move GuardianStorage and TransferStorage to be instantiated in the 2_deploy_contracts script --- deployment/2_deploy_contracts.js | 20 +++++++++++++++++- deployment/5_deploy_modules.js | 35 +++++++++----------------------- deployment/7_upgrade_2_0.js | 5 ----- 3 files changed, 29 insertions(+), 31 deletions(-) diff --git a/deployment/2_deploy_contracts.js b/deployment/2_deploy_contracts.js index f45a032ce..627645de6 100644 --- a/deployment/2_deploy_contracts.js +++ b/deployment/2_deploy_contracts.js @@ -1,3 +1,6 @@ +const GuardianStorage = require("../build-legacy/v1.6.0/GuardianStorage"); +const TransferStorage = require("../build-legacy/v1.6.0/TransferStorage"); + const BaseWallet = require("../build/BaseWallet"); const ModuleRegistry = require("../build/ModuleRegistry"); const CompoundRegistry = require("../build/CompoundRegistry"); @@ -36,6 +39,15 @@ const deploy = async (network) => { const deploymentAccount = await deploymentWallet.getAddress(); const walletRootEns = prevConfig.ENS.domain; + // ////////////////////////////////// + // Deploy Storage + // ////////////////////////////////// + + // Deploy the Guardian Storage + const GuardianStorageWrapper = await deployer.deploy(GuardianStorage); + // Deploy the Transfer Storage + const TransferStorageWrapper = await deployer.deploy(TransferStorage); + // ////////////////////////////////// // Deploy contracts // ////////////////////////////////// @@ -61,7 +73,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,6 +126,10 @@ const deploy = async (network) => { // ///////////////////////////////////////////////// // Update config and Upload ABIs // ///////////////////////////////////////////////// + configurator.updateModuleAddresses({ + GuardianStorage: GuardianStorageWrapper.contractAddress, + TransferStorage: TransferStorageWrapper.contractAddress, + }); configurator.updateInfrastructureAddresses({ MultiSigWallet: MultiSigWrapper.contractAddress, @@ -128,6 +144,8 @@ const deploy = async (network) => { await configurator.save(); await Promise.all([ + abiUploader.upload(GuardianStorageWrapper, "modules"), + abiUploader.upload(TransferStorageWrapper, "modules"), abiUploader.upload(MultiSigWrapper, "contracts"), abiUploader.upload(WalletFactoryWrapper, "contracts"), abiUploader.upload(ENSResolverWrapper, "contracts"), diff --git a/deployment/5_deploy_modules.js b/deployment/5_deploy_modules.js index 180dade04..fbe4daadc 100644 --- a/deployment/5_deploy_modules.js +++ b/deployment/5_deploy_modules.js @@ -1,6 +1,4 @@ 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"); @@ -33,15 +31,6 @@ const deploy = async (network) => { const { config } = configurator; console.log(config); - // ////////////////////////////////// - // Deploy Storage - // ////////////////////////////////// - - // Deploy the Guardian Storage - const GuardianStorageWrapper = await deployer.deploy(GuardianStorage); - // Deploy the Transfer Storage - const TransferStorageWrapper = await deployer.deploy(TransferStorage); - // ////////////////////////////////// // Deploy Modules // ////////////////////////////////// @@ -51,7 +40,7 @@ const deploy = async (network) => { GuardianManager, {}, config.contracts.ModuleRegistry, - GuardianStorageWrapper.contractAddress, + config.modules.GuardianStorage, config.settings.securityPeriod || 0, config.settings.securityWindow || 0, ); @@ -60,7 +49,7 @@ const deploy = async (network) => { LockManager, {}, config.contracts.ModuleRegistry, - GuardianStorageWrapper.contractAddress, + config.modules.GuardianStorage, config.settings.lockPeriod || 0, ); // Deploy the RecoveryManager module @@ -68,7 +57,7 @@ const deploy = async (network) => { RecoveryManager, {}, config.contracts.ModuleRegistry, - GuardianStorageWrapper.contractAddress, + config.modules.GuardianStorage, config.settings.recoveryPeriod || 0, config.settings.lockPeriod || 0, config.settings.securityPeriod || 0, @@ -79,15 +68,15 @@ const deploy = async (network) => { ApprovedTransfer, {}, config.contracts.ModuleRegistry, - GuardianStorageWrapper.contractAddress, + config.modules.GuardianStorage, ); // Deploy the TransferManager module const TransferManagerWrapper = await deployer.deploy( TransferManager, {}, config.contracts.ModuleRegistry, - TransferStorageWrapper.contractAddress, - GuardianStorageWrapper.contractAddress, + config.modules.TransferStorage, + config.modules.GuardianStorage, config.contracts.TokenPriceProvider, config.settings.securityPeriod || 0, config.settings.securityWindow || 0, @@ -99,7 +88,7 @@ const deploy = async (network) => { TokenExchanger, {}, config.contracts.ModuleRegistry, - GuardianStorageWrapper.contractAddress, + config.modules.GuardianStorage, config.Kyber ? config.Kyber.contract : "0x0000000000000000000000000000000000000000", config.contracts.MultiSigWallet, config.settings.feeRatio || 0, @@ -109,7 +98,7 @@ const deploy = async (network) => { NftTransfer, {}, config.contracts.ModuleRegistry, - GuardianStorageWrapper.contractAddress, + config.modules.GuardianStorage, config.CryptoKitties.contract, ); // Deploy the CompoundManager module @@ -117,7 +106,7 @@ const deploy = async (network) => { CompoundManager, {}, config.contracts.ModuleRegistry, - GuardianStorageWrapper.contractAddress, + config.modules.GuardianStorage, config.defi.compound.comptroller, config.contracts.CompoundRegistry, ); @@ -126,7 +115,7 @@ const deploy = async (network) => { MakerV2Manager, {}, config.contracts.ModuleRegistry, - GuardianStorageWrapper.contractAddress, + config.modules.GuardianStorage, config.defi.maker.migration, config.defi.maker.pot, config.defi.maker.jug, @@ -139,8 +128,6 @@ const deploy = async (network) => { // ///////////////////////////////////////////////// configurator.updateModuleAddresses({ - GuardianStorage: GuardianStorageWrapper.contractAddress, - TransferStorage: TransferStorageWrapper.contractAddress, GuardianManager: GuardianManagerWrapper.contractAddress, LockManager: LockManagerWrapper.contractAddress, RecoveryManager: RecoveryManagerWrapper.contractAddress, @@ -158,8 +145,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"), diff --git a/deployment/7_upgrade_2_0.js b/deployment/7_upgrade_2_0.js index ef6c38209..edfe00fa8 100644 --- a/deployment/7_upgrade_2_0.js +++ b/deployment/7_upgrade_2_0.js @@ -81,11 +81,6 @@ const deploy = async (network) => { // 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 From a8d8abe0d3a3c7cb140bb103d47754ac0544df6e Mon Sep 17 00:00:00 2001 From: elenadimitrova Date: Mon, 12 Oct 2020 17:29:21 +0300 Subject: [PATCH 08/47] Move the instantiation of LimitStorage and LockStorage --- deployment/2_deploy_contracts.js | 10 ++++++++++ deployment/7_upgrade_2_0.js | 15 --------------- 2 files changed, 10 insertions(+), 15 deletions(-) diff --git a/deployment/2_deploy_contracts.js b/deployment/2_deploy_contracts.js index 627645de6..df190c0de 100644 --- a/deployment/2_deploy_contracts.js +++ b/deployment/2_deploy_contracts.js @@ -1,5 +1,7 @@ const GuardianStorage = require("../build-legacy/v1.6.0/GuardianStorage"); const TransferStorage = require("../build-legacy/v1.6.0/TransferStorage"); +const LockStorage = require("../build/LockStorage"); +const LimitStorage = require("../build/LimitStorage"); const BaseWallet = require("../build/BaseWallet"); const ModuleRegistry = require("../build/ModuleRegistry"); @@ -47,6 +49,10 @@ const deploy = async (network) => { 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 contracts @@ -140,12 +146,16 @@ const deploy = async (network) => { ModuleRegistry: ModuleRegistryWrapper.contractAddress, CompoundRegistry: CompoundRegistryWrapper.contractAddress, BaseWallet: BaseWalletWrapper.contractAddress, + LockStorage: LockStorageWrapper.contractAddress, + LimitStorage: LimitStorageWrapper.contractAddress, }); await configurator.save(); await Promise.all([ abiUploader.upload(GuardianStorageWrapper, "modules"), abiUploader.upload(TransferStorageWrapper, "modules"), + abiUploader.upload(LockStorageWrapper, "contracts"), + abiUploader.upload(LimitStorageWrapper, "contracts"), abiUploader.upload(MultiSigWrapper, "contracts"), abiUploader.upload(WalletFactoryWrapper, "contracts"), abiUploader.upload(ENSResolverWrapper, "contracts"), diff --git a/deployment/7_upgrade_2_0.js b/deployment/7_upgrade_2_0.js index edfe00fa8..8021a56c4 100644 --- a/deployment/7_upgrade_2_0.js +++ b/deployment/7_upgrade_2_0.js @@ -6,9 +6,7 @@ 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"); @@ -23,9 +21,6 @@ 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"; @@ -81,10 +76,6 @@ const deploy = async (network) => { // Deploy infrastructure contracts // ////////////////////////////////// - // 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 @@ -287,9 +278,7 @@ const deploy = async (network) => { // 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, @@ -304,8 +293,6 @@ const deploy = async (network) => { }); configurator.updateInfrastructureAddresses({ - BaseWallet: BaseWalletWrapper.contractAddress, - WalletFactory: WalletFactoryWrapper.contractAddress, DexRegistry: DexRegistryWrapper.contractAddress, }); @@ -325,9 +312,7 @@ 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"), From affde684b4688d1c27f520719fbb1aeeaa21b101 Mon Sep 17 00:00:00 2001 From: elenadimitrova Date: Mon, 12 Oct 2020 18:34:32 +0300 Subject: [PATCH 09/47] Add TokenPriceRegistry and DexRegistry setup in deployment scripts --- deployment/2_deploy_contracts.js | 25 +++++++++++-------- deployment/3_setup_contracts.js | 23 ++++++++++++----- deployment/7_upgrade_2_0.js | 43 +------------------------------- 3 files changed, 33 insertions(+), 58 deletions(-) diff --git a/deployment/2_deploy_contracts.js b/deployment/2_deploy_contracts.js index df190c0de..a95a0f4c9 100644 --- a/deployment/2_deploy_contracts.js +++ b/deployment/2_deploy_contracts.js @@ -11,7 +11,10 @@ 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 TokenPriceRegistry = require("../build/TokenPriceRegistry"); +const DexRegistry = require("../build/DexRegistry"); + const MakerRegistry = require("../build/MakerRegistry"); const ScdMcdMigration = require("../build/ScdMcdMigration"); @@ -55,19 +58,19 @@ const deploy = async (network) => { const LimitStorageWrapper = await deployer.deploy(LimitStorage); // ////////////////////////////////// - // Deploy contracts + // 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 @@ -135,6 +138,7 @@ const deploy = async (network) => { configurator.updateModuleAddresses({ GuardianStorage: GuardianStorageWrapper.contractAddress, TransferStorage: TransferStorageWrapper.contractAddress, + TokenPriceRegistry: TokenPriceRegistryWrapper.contractAddress, }); configurator.updateInfrastructureAddresses({ @@ -142,9 +146,9 @@ const deploy = async (network) => { WalletFactory: WalletFactoryWrapper.contractAddress, ENSResolver: ENSResolverWrapper.contractAddress, ENSManager: ENSManagerWrapper.contractAddress, - TokenPriceProvider: TokenPriceProviderWrapper.contractAddress, ModuleRegistry: ModuleRegistryWrapper.contractAddress, CompoundRegistry: CompoundRegistryWrapper.contractAddress, + DexRegistry: DexRegistryWrapper.contractAddress, BaseWallet: BaseWalletWrapper.contractAddress, LockStorage: LockStorageWrapper.contractAddress, LimitStorage: LimitStorageWrapper.contractAddress, @@ -160,9 +164,10 @@ const deploy = async (network) => { 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(TokenPriceRegistryWrapper, "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..d44af24cc 100644 --- a/deployment/3_setup_contracts.js +++ b/deployment/3_setup_contracts.js @@ -2,8 +2,9 @@ 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 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.TokenPriceRegistry); + + // 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/7_upgrade_2_0.js b/deployment/7_upgrade_2_0.js index 8021a56c4..bbe65b16a 100644 --- a/deployment/7_upgrade_2_0.js +++ b/deployment/7_upgrade_2_0.js @@ -6,9 +6,6 @@ const Upgrader = require("../build/UpgraderToVersionManager"); const DeployManager = require("../utils/deploy-manager.js"); const MultisigExecutor = require("../utils/multisigexecutor.js"); -const TokenPriceRegistry = require("../build/TokenPriceRegistry"); -const DexRegistry = require("../build/DexRegistry"); - const ApprovedTransfer = require("../build/ApprovedTransfer"); const CompoundManager = require("../build/CompoundManager"); const GuardianManager = require("../build/GuardianManager"); @@ -72,15 +69,6 @@ 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 new TokenPriceRegistry - const TokenPriceRegistryWrapper = await deployer.deploy(TokenPriceRegistry); - // Deploy the DexRegistry - const DexRegistryWrapper = await deployer.deploy(DexRegistry); - // ////////////////////////////////// // Deploy new modules // ////////////////////////////////// @@ -235,50 +223,23 @@ const deploy = async (network) => { // 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 }); 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({ - TokenPriceRegistry: TokenPriceRegistryWrapper.contractAddress, ApprovedTransfer: ApprovedTransferWrapper.contractAddress, CompoundManager: CompoundManagerWrapper.contractAddress, GuardianManager: GuardianManagerWrapper.contractAddress, @@ -293,7 +254,7 @@ const deploy = async (network) => { }); configurator.updateInfrastructureAddresses({ - DexRegistry: DexRegistryWrapper.contractAddress, + }); const gitHash = childProcess.execSync("git rev-parse HEAD").toString("utf8").replace(/\n$/, ""); @@ -312,10 +273,8 @@ const deploy = async (network) => { abiUploader.upload(MakerV2ManagerWrapper, "modules"), abiUploader.upload(TransferManagerWrapper, "modules"), abiUploader.upload(RelayerManagerWrapper, "modules"), - abiUploader.upload(TokenPriceRegistryWrapper, "contracts"), abiUploader.upload(BaseWalletWrapper, "contracts"), abiUploader.upload(WalletFactoryWrapper, "contracts"), - abiUploader.upload(DexRegistryWrapper, "contracts"), ]); // ////////////////////////////////// From 4bfc981151c5b0b3362f11615480f1f0e058ad6b Mon Sep 17 00:00:00 2001 From: elenadimitrova Date: Tue, 13 Oct 2020 08:22:06 +0300 Subject: [PATCH 10/47] Move module registrations to deploy scripts --- deployment/3_setup_contracts.js | 2 +- deployment/5_deploy_modules.js | 99 ++++++++++++------- deployment/7_upgrade_2_0.js | 162 +------------------------------- 3 files changed, 69 insertions(+), 194 deletions(-) diff --git a/deployment/3_setup_contracts.js b/deployment/3_setup_contracts.js index d44af24cc..8fbae6137 100644 --- a/deployment/3_setup_contracts.js +++ b/deployment/3_setup_contracts.js @@ -29,7 +29,7 @@ const deploy = async (network) => { const ModuleRegistryWrapper = await deployer.wrapDeployedContract(ModuleRegistry, config.contracts.ModuleRegistry); const CompoundRegistryWrapper = await deployer.wrapDeployedContract(CompoundRegistry, config.contracts.CompoundRegistry); const TokenPriceRegistryWrapper = await deployer.wrapDeployedContract(TokenPriceRegistry, config.modules.TokenPriceRegistry); - const DexRegistryWrapper = await deployer.wrapDeployedContract(DexRegistry, config.contracts.TokenPriceRegistry); + const DexRegistryWrapper = await deployer.wrapDeployedContract(DexRegistry, config.contracts.DexRegistry); // Configure DexRegistry const authorisedExchanges = Object.values(config.defi.paraswap.authorisedExchanges); diff --git a/deployment/5_deploy_modules.js b/deployment/5_deploy_modules.js index fbe4daadc..eef310282 100644 --- a/deployment/5_deploy_modules.js +++ b/deployment/5_deploy_modules.js @@ -1,19 +1,21 @@ const childProcess = require("child_process"); -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) => { @@ -32,15 +34,29 @@ const deploy = async (network) => { console.log(config); // ////////////////////////////////// - // Deploy Modules + // Deploy new modules + // ////////////////////////////////// + const VersionManagerWrapper = await deployer.deploy( + VersionManager, + {}, + config.contracts.ModuleRegistry, + config.modules.LockStorage, + config.modules.GuardianStorage, + config.modules.TransferStorage, + config.modules.LimitStorage, + ); + + // ////////////////////////////////// + // Deploy new features // ////////////////////////////////// // Deploy the GuardianManager module const GuardianManagerWrapper = await deployer.deploy( GuardianManager, {}, - config.contracts.ModuleRegistry, + config.modules.LockStorage, config.modules.GuardianStorage, + VersionManagerWrapper.contractAddress, config.settings.securityPeriod || 0, config.settings.securityWindow || 0, ); @@ -48,79 +64,96 @@ const deploy = async (network) => { const LockManagerWrapper = await deployer.deploy( LockManager, {}, - config.contracts.ModuleRegistry, + 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, + 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, + 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, + config.modules.LockStorage, config.modules.TransferStorage, - config.modules.GuardianStorage, - config.contracts.TokenPriceProvider, + 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, - config.modules.GuardianStorage, - 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, - config.modules.GuardianStorage, + config.modules.LockStorage, + config.modules.TokenPriceRegistry, + VersionManagerWrapper.contractAddress, config.CryptoKitties.contract, ); // Deploy the CompoundManager module const CompoundManagerWrapper = await deployer.deploy( CompoundManager, {}, - config.contracts.ModuleRegistry, - config.modules.GuardianStorage, + config.modules.LockStorage, config.defi.compound.comptroller, config.contracts.CompoundRegistry, + VersionManagerWrapper.contractAddress, ); // Deploy MakerManagerV2 const MakerV2ManagerWrapper = await deployer.deploy( MakerV2Manager, {}, - config.contracts.ModuleRegistry, - config.modules.GuardianStorage, + 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, ); // ///////////////////////////////////////////////// @@ -154,6 +187,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/7_upgrade_2_0.js b/deployment/7_upgrade_2_0.js index bbe65b16a..e280afb93 100644 --- a/deployment/7_upgrade_2_0.js +++ b/deployment/7_upgrade_2_0.js @@ -6,37 +6,13 @@ const Upgrader = require("../build/UpgraderToVersionManager"); const DeployManager = require("../utils/deploy-manager.js"); const MultisigExecutor = require("../utils/multisigexecutor.js"); -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"); const TARGET_VERSION = "2.1.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; @@ -59,7 +35,6 @@ const deploy = async (network) => { const { configurator } = manager; const { deployer } = manager; - const { abiUploader } = manager; const { versionUploader } = manager; const { gasPrice } = deployer.defaultOverrides; const deploymentWallet = deployer.signer; @@ -69,128 +44,6 @@ const deploy = async (network) => { const MultiSigWrapper = await deployer.wrapDeployedContract(MultiSig, config.contracts.MultiSigWallet); const multisigExecutor = new MultisigExecutor(MultiSigWrapper, deploymentWallet, config.multisig.autosign, { gasPrice }); - // ////////////////////////////////// - // Deploy new modules - // ////////////////////////////////// - const VersionManagerWrapper = await deployer.deploy( - VersionManager, - {}, - config.contracts.ModuleRegistry, - LockStorageWrapper.contractAddress, - config.modules.GuardianStorage, - config.modules.TransferStorage, - LimitStorageWrapper.contractAddress, - ); - newModuleWrappers.push(VersionManagerWrapper); - - // ////////////////////////////////// - // Deploy new features - // ////////////////////////////////// - const ApprovedTransferWrapper = await deployer.deploy( - ApprovedTransfer, - {}, - LockStorageWrapper.contractAddress, - config.modules.GuardianStorage, - LimitStorageWrapper.contractAddress, - VersionManagerWrapper.contractAddress, - config.defi.weth, - ); - - const CompoundManagerWrapper = await deployer.deploy( - CompoundManager, - {}, - LockStorageWrapper.contractAddress, - config.defi.compound.comptroller, - config.contracts.CompoundRegistry, - VersionManagerWrapper.contractAddress, - ); - - const GuardianManagerWrapper = await deployer.deploy( - GuardianManager, - {}, - LockStorageWrapper.contractAddress, - config.modules.GuardianStorage, - VersionManagerWrapper.contractAddress, - config.settings.securityPeriod || 0, - config.settings.securityWindow || 0, - ); - - const LockManagerWrapper = await deployer.deploy( - LockManager, - {}, - LockStorageWrapper.contractAddress, - config.modules.GuardianStorage, - VersionManagerWrapper.contractAddress, - config.settings.lockPeriod || 0, - ); - - const NftTransferWrapper = await deployer.deploy( - NftTransfer, - {}, - LockStorageWrapper.contractAddress, - TokenPriceRegistryWrapper.contractAddress, - VersionManagerWrapper.contractAddress, - config.CryptoKitties.contract, - ); - - const RecoveryManagerWrapper = await deployer.deploy( - RecoveryManager, - {}, - LockStorageWrapper.contractAddress, - config.modules.GuardianStorage, - VersionManagerWrapper.contractAddress, - config.settings.recoveryPeriod || 0, - config.settings.lockPeriod || 0, - ); - - const TokenExchangerWrapper = await deployer.deploy( - TokenExchanger, - {}, - LockStorageWrapper.contractAddress, - TokenPriceRegistryWrapper.contractAddress, - VersionManagerWrapper.contractAddress, - DexRegistryWrapper.contractAddress, - config.defi.paraswap.contract, - "argent", - ); - - const MakerV2ManagerWrapper = await deployer.deploy( - MakerV2Manager, - {}, - LockStorageWrapper.contractAddress, - config.defi.maker.migration, - config.defi.maker.pot, - config.defi.maker.jug, - config.contracts.MakerRegistry, - config.defi.uniswap.factory, - VersionManagerWrapper.contractAddress, - ); - - const TransferManagerWrapper = await deployer.deploy( - TransferManager, - {}, - LockStorageWrapper.contractAddress, - config.modules.TransferStorage, - LimitStorageWrapper.contractAddress, - TokenPriceRegistryWrapper.contractAddress, - VersionManagerWrapper.contractAddress, - config.settings.securityPeriod || 0, - config.settings.securityWindow || 0, - config.settings.defaultLimit || "1000000000000000000", - config.defi.weth, - ["test", "staging", "prod"].includes(network) ? config.modules.TransferManager : "0x0000000000000000000000000000000000000000", - ); - - const RelayerManagerWrapper = await deployer.deploy( - RelayerManager, - {}, - LockStorageWrapper.contractAddress, - config.modules.GuardianStorage, - LimitStorageWrapper.contractAddress, - TokenPriceRegistryWrapper.contractAddress, - VersionManagerWrapper.contractAddress, - ); - // Add Features to Version Manager const newFeatures = [ GuardianManagerWrapper.contractAddress, @@ -262,19 +115,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"), - abiUploader.upload(LockManagerWrapper, "modules"), - abiUploader.upload(NftTransferWrapper, "modules"), - abiUploader.upload(RecoveryManagerWrapper, "modules"), - abiUploader.upload(TokenExchangerWrapper, "modules"), - abiUploader.upload(MakerV2ManagerWrapper, "modules"), - abiUploader.upload(TransferManagerWrapper, "modules"), - abiUploader.upload(RelayerManagerWrapper, "modules"), - abiUploader.upload(BaseWalletWrapper, "contracts"), - abiUploader.upload(WalletFactoryWrapper, "contracts"), ]); // ////////////////////////////////// From 226bcb4af58870693581e0277ddf82fec858ee6d Mon Sep 17 00:00:00 2001 From: elenadimitrova Date: Tue, 13 Oct 2020 09:33:22 +0300 Subject: [PATCH 11/47] Move module and feature instantiation in deployment scripts --- deployment/2_deploy_contracts.js | 10 ++--- deployment/3_setup_contracts.js | 2 +- deployment/5_deploy_modules.js | 3 ++ deployment/6_register_modules.js | 65 +++++++++++++++++++++--------- deployment/7_upgrade_2_0.js | 48 +++------------------- scripts/deploy.sh | 5 +-- utils/config/ganache.json | 2 +- utils/versions/ganache/latest.json | 2 +- 8 files changed, 62 insertions(+), 75 deletions(-) diff --git a/deployment/2_deploy_contracts.js b/deployment/2_deploy_contracts.js index a95a0f4c9..f528d41be 100644 --- a/deployment/2_deploy_contracts.js +++ b/deployment/2_deploy_contracts.js @@ -1,5 +1,5 @@ -const GuardianStorage = require("../build-legacy/v1.6.0/GuardianStorage"); -const TransferStorage = require("../build-legacy/v1.6.0/TransferStorage"); +const GuardianStorage = require("../build/GuardianStorage"); +const TransferStorage = require("../build/TransferStorage"); const LockStorage = require("../build/LockStorage"); const LimitStorage = require("../build/LimitStorage"); @@ -10,7 +10,7 @@ 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 WalletFactory = require("../build/WalletFactory"); const TokenPriceRegistry = require("../build/TokenPriceRegistry"); const DexRegistry = require("../build/DexRegistry"); @@ -139,6 +139,8 @@ const deploy = async (network) => { GuardianStorage: GuardianStorageWrapper.contractAddress, TransferStorage: TransferStorageWrapper.contractAddress, TokenPriceRegistry: TokenPriceRegistryWrapper.contractAddress, + LockStorage: LockStorageWrapper.contractAddress, + LimitStorage: LimitStorageWrapper.contractAddress, }); configurator.updateInfrastructureAddresses({ @@ -150,8 +152,6 @@ const deploy = async (network) => { CompoundRegistry: CompoundRegistryWrapper.contractAddress, DexRegistry: DexRegistryWrapper.contractAddress, BaseWallet: BaseWalletWrapper.contractAddress, - LockStorage: LockStorageWrapper.contractAddress, - LimitStorage: LimitStorageWrapper.contractAddress, }); await configurator.save(); diff --git a/deployment/3_setup_contracts.js b/deployment/3_setup_contracts.js index 8fbae6137..0a6a10f14 100644 --- a/deployment/3_setup_contracts.js +++ b/deployment/3_setup_contracts.js @@ -1,7 +1,7 @@ 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 WalletFactory = require("../build/WalletFactory"); const TokenPriceRegistry = require("../build/TokenPriceRegistry"); const CompoundRegistry = require("../build/CompoundRegistry"); const DexRegistry = require("../build/DexRegistry"); diff --git a/deployment/5_deploy_modules.js b/deployment/5_deploy_modules.js index eef310282..d2a9d8ed1 100644 --- a/deployment/5_deploy_modules.js +++ b/deployment/5_deploy_modules.js @@ -160,6 +160,7 @@ const deploy = async (network) => { // Update config and Upload ABIs // ///////////////////////////////////////////////// + // TODO: change name from "module" to "feature" where appropriate configurator.updateModuleAddresses({ GuardianManager: GuardianManagerWrapper.contractAddress, LockManager: LockManagerWrapper.contractAddress, @@ -170,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$/, ""); diff --git a/deployment/6_register_modules.js b/deployment/6_register_modules.js index 922df9f3a..7b41cec2c 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 newFeatures = [ + GuardianManagerWrapper.contractAddress, + LockManagerWrapper.contractAddress, + RecoveryManagerWrapper.contractAddress, + ApprovedTransferWrapper.contractAddress, + TransferManagerWrapper.contractAddress, + TokenExchangerWrapper.contractAddress, + NftTransferWrapper.contractAddress, + CompoundManagerWrapper.contractAddress, + MakerV2ManagerWrapper.contractAddress, + RelayerManagerWrapper.contractAddress, ]; + const newFeaturesWithNoInit = [ // all features except the TransferManager + GuardianManagerWrapper.contractAddress, + LockManagerWrapper.contractAddress, + RecoveryManagerWrapper.contractAddress, + ApprovedTransferWrapper.contractAddress, + TokenExchangerWrapper.contractAddress, + NftTransferWrapper.contractAddress, + CompoundManagerWrapper.contractAddress, + MakerV2ManagerWrapper.contractAddress, + RelayerManagerWrapper.contractAddress, + ]; + const newFeatureToInit = newFeatures.filter((f) => !newFeaturesWithNoInit.includes(f)); + const VersionManagerAddVersionTx = await VersionManagerWrapper.contract.addVersion(newFeatures, newFeatureToInit, { 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_0.js index e280afb93..fb9d432cd 100644 --- a/deployment/7_upgrade_2_0.js +++ b/deployment/7_upgrade_2_0.js @@ -2,6 +2,7 @@ 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"); @@ -44,33 +45,10 @@ const deploy = async (network) => { const MultiSigWrapper = await deployer.wrapDeployedContract(MultiSig, config.contracts.MultiSigWallet); const multisigExecutor = new MultisigExecutor(MultiSigWrapper, deploymentWallet, config.multisig.autosign, { gasPrice }); - // Add Features to Version Manager - const newFeatures = [ - GuardianManagerWrapper.contractAddress, - LockManagerWrapper.contractAddress, - RecoveryManagerWrapper.contractAddress, - ApprovedTransferWrapper.contractAddress, - TransferManagerWrapper.contractAddress, - TokenExchangerWrapper.contractAddress, - NftTransferWrapper.contractAddress, - CompoundManagerWrapper.contractAddress, - MakerV2ManagerWrapper.contractAddress, - RelayerManagerWrapper.contractAddress, - ]; - const newFeaturesWithNoInit = [ // all features except the TransferManager - GuardianManagerWrapper.contractAddress, - LockManagerWrapper.contractAddress, - RecoveryManagerWrapper.contractAddress, - ApprovedTransferWrapper.contractAddress, - TokenExchangerWrapper.contractAddress, - NftTransferWrapper.contractAddress, - CompoundManagerWrapper.contractAddress, - MakerV2ManagerWrapper.contractAddress, - RelayerManagerWrapper.contractAddress, - ]; - const newFeatureToInit = newFeatures.filter((f) => !newFeaturesWithNoInit.includes(f)); - const VersionManagerAddVersionTx = await VersionManagerWrapper.contract.addVersion(newFeatures, newFeatureToInit, { gasPrice }); - await VersionManagerWrapper.verboseWaitForTransaction(VersionManagerAddVersionTx, "Adding New Version"); + // ////////////////////////////////// + // Initialise the new version + // ////////////////////////////////// + const VersionManagerWrapper = await deployer.wrapDeployedContract(VersionManager, config.modules.VersionManager); // ////////////////////////////////// // Setup new infrastructure @@ -84,30 +62,14 @@ const deploy = async (network) => { // Set contracts' owners // ////////////////////////////////// - changeOwnerTx = await VersionManagerWrapper.contract.changeOwner(config.contracts.MultiSigWallet, { gasPrice }); - await VersionManagerWrapper.verboseWaitForTransaction(changeOwnerTx, "Set the MultiSig as the owner of VersionManagerWrapper"); - // ///////////////////////////////////////////////// // Update config and Upload ABIs // ///////////////////////////////////////////////// - // TODO: change name from "module" to "feature" where appropriate configurator.updateModuleAddresses({ - ApprovedTransfer: ApprovedTransferWrapper.contractAddress, - CompoundManager: CompoundManagerWrapper.contractAddress, - GuardianManager: GuardianManagerWrapper.contractAddress, - LockManager: LockManagerWrapper.contractAddress, - NftTransfer: NftTransferWrapper.contractAddress, - RecoveryManager: RecoveryManagerWrapper.contractAddress, - TokenExchanger: TokenExchangerWrapper.contractAddress, - MakerV2Manager: MakerV2ManagerWrapper.contractAddress, - TransferManager: TransferManagerWrapper.contractAddress, - RelayerManager: RelayerManagerWrapper.contractAddress, - VersionManager: VersionManagerWrapper.contractAddress, }); configurator.updateInfrastructureAddresses({ - }); const gitHash = childProcess.execSync("git rev-parse HEAD").toString("utf8").replace(/\n$/, ""); diff --git a/scripts/deploy.sh b/scripts/deploy.sh index 40f43011d..0807c10ee 100755 --- a/scripts/deploy.sh +++ b/scripts/deploy.sh @@ -21,10 +21,7 @@ else shift fi -rm -rf build -npm run compile:lib -npm run compile -npm run compile:legacy + for IDX in "$@" do diff --git a/utils/config/ganache.json b/utils/config/ganache.json index 05cfeef22..4b6e92abf 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":"0x7E93afFaa1452a7e07B961Aab04EF20a4Bc12b78","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":"0x101b60626A8073271fE2eFE59FB8fD74c108c2eC"},"uniswap":{"deployOwn":true,"factory":"0x59f30a6703F8Af4FAeAe0e8AFaB14202222F335f"},"compound":{"comptroller":"0x0000000000000000000000000000000000000000","markets":{"0x0000000000000000000000000000000000000000":"0x0000000000000000000000000000000000000000"}},"paraswap":{"deployOwn":true,"contract":"0x22eF7a1De649A8f027384D6a6eDd301AC5FD30D7","authorisedExchanges":{"Kyber":"0x3f906DDA5fa9DA5259d2d865157f5001Ce626e31"}}},"contracts":{"MultiSigWallet":"0xD822e79E2CA49c668f7d97DB1906330bc64459EB","WalletFactory":"0xD7A53fDd46a663C678da0580A66843Ae9b7783Da","ENSResolver":"0xC790c0E4D6811884b52121A5A8d0c0198F515030","ENSManager":"0x75eC71B541b0b2e0f9A4a2bea381c23fd7B0fB0c","TokenPriceProvider":"0xE09C322B745434Fbb2D18c107c13795D4bDF3657","ModuleRegistry":"0x61B2D9C034cD0DAE1738D039cdF9af860ee6B31A","BaseWallet":"0xF346Ee8d0b5738E037DbF22355949411187Da4bC","CompoundRegistry":"0x42ecEbe8946c4954aFc0C2333977adCb243614ed","MakerRegistry":"0x355d26E5f04255F60E910F1aA1251DE5D10996E8","DexRegistry":"0x4A74DE038616791359aEfED19f3f6B75FF6dd340"},"modules":{"GuardianStorage":"0xeE20db41be0CB3D09C5557dDD99d95A4fFf15076","TransferStorage":"0xF8E2BBA1AD8Fd1034DFA1b8Fb6bF5a724fa973ab","GuardianManager":"0x1046E04ED27A22079f69674Bb8bDfbe7c11f43d1","LockManager":"0xCFCE66b4A7904Ad0c4Fe43CB524759C222154BD9","RecoveryManager":"0x8dA7E75A6578F8597eB2c605Fa178F37DbD7E34d","ApprovedTransfer":"0xa6379f76FD8603a44F699EEB42735Ffd00d617D0","TokenExchanger":"0xcF809DD0f9fd4d4b64BcEf64de890BF5579A74A2","NftTransfer":"0x7f7d740Aaf4996C6F4718Fd9003Bb3CC565A11dC","MakerManager":"0x2718cAce241213B18F67C6d66B245EE77DB0Ce2a","TransferManager":"0xc49b23b3403fcEDa2b512D2178fA7020E8f5227E","CompoundManager":"0x55C914A069a49c4163b1dC5FbB216e3ae0339f98","MakerV2Manager":"0x950B78F4B2ecA78C552F4797c0cf10d1aaDDAD2c","LimitStorage":"0xb9dC99d37D77B915768029767C3dC0ff004540E5","TokenPriceStorage":"0x5dA4F81c17776495e3d7999ad9217748D257f074","RelayerModule":"0x171eb25c80da6adF0436668126285ca51e37eB30","RelayerManager":"0x6E5727685D31C3d9bE0c19F5A635551733567e15","VersionManager":"0xf4146041EdE0B35A6573DA0D820cA6Da785dFf17","LockStorage":"0x731a09EEda7ed25080A5f92F117076f6a8ED0E14","TokenPriceRegistry":"0x7e7acD252559062f0E6355847f4Fbb1c62872f14"},"gitCommit":"29a0aee0fd3e30c570e440b69dc14e7ca93755c9"} \ No newline at end of file diff --git a/utils/versions/ganache/latest.json b/utils/versions/ganache/latest.json index b21b186a7..bb7281b29 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":"0xf4146041EdE0B35A6573DA0D820cA6Da785dFf17","name":"VersionManager"}],"fingerprint":"0xde5fa3e0","version":"1.0.0","createdAt":1602570421} \ No newline at end of file From bf37057eedb4abd6dcd5027fcd41d9b79802bc64 Mon Sep 17 00:00:00 2001 From: elenadimitrova Date: Tue, 13 Oct 2020 09:38:57 +0300 Subject: [PATCH 12/47] Revert deploy script changes --- scripts/deploy.sh | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/scripts/deploy.sh b/scripts/deploy.sh index 0807c10ee..40f43011d 100755 --- a/scripts/deploy.sh +++ b/scripts/deploy.sh @@ -21,7 +21,10 @@ else shift fi - +rm -rf build +npm run compile:lib +npm run compile +npm run compile:legacy for IDX in "$@" do From 91dbc2dfe2743fe2a954fa61db037c750401cfc8 Mon Sep 17 00:00:00 2001 From: elenadimitrova Date: Mon, 19 Oct 2020 15:27:34 +0300 Subject: [PATCH 13/47] Review suggestions from Olivier --- deployment/2_deploy_contracts.js | 2 +- deployment/5_deploy_modules.js | 4 ++-- deployment/6_register_modules.js | 8 ++++---- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/deployment/2_deploy_contracts.js b/deployment/2_deploy_contracts.js index f528d41be..e478bc440 100644 --- a/deployment/2_deploy_contracts.js +++ b/deployment/2_deploy_contracts.js @@ -138,7 +138,6 @@ const deploy = async (network) => { configurator.updateModuleAddresses({ GuardianStorage: GuardianStorageWrapper.contractAddress, TransferStorage: TransferStorageWrapper.contractAddress, - TokenPriceRegistry: TokenPriceRegistryWrapper.contractAddress, LockStorage: LockStorageWrapper.contractAddress, LimitStorage: LimitStorageWrapper.contractAddress, }); @@ -150,6 +149,7 @@ const deploy = async (network) => { ENSManager: ENSManagerWrapper.contractAddress, ModuleRegistry: ModuleRegistryWrapper.contractAddress, CompoundRegistry: CompoundRegistryWrapper.contractAddress, + TokenPriceRegistry: TokenPriceRegistryWrapper.contractAddress, DexRegistry: DexRegistryWrapper.contractAddress, BaseWallet: BaseWalletWrapper.contractAddress, }); diff --git a/deployment/5_deploy_modules.js b/deployment/5_deploy_modules.js index d2a9d8ed1..c7cf946e8 100644 --- a/deployment/5_deploy_modules.js +++ b/deployment/5_deploy_modules.js @@ -34,7 +34,7 @@ const deploy = async (network) => { console.log(config); // ////////////////////////////////// - // Deploy new modules + // Deploy VersionManager module // ////////////////////////////////// const VersionManagerWrapper = await deployer.deploy( VersionManager, @@ -47,7 +47,7 @@ const deploy = async (network) => { ); // ////////////////////////////////// - // Deploy new features + // Deploy features // ////////////////////////////////// // Deploy the GuardianManager module diff --git a/deployment/6_register_modules.js b/deployment/6_register_modules.js index 7b41cec2c..c54de517f 100644 --- a/deployment/6_register_modules.js +++ b/deployment/6_register_modules.js @@ -54,7 +54,7 @@ const deploy = async (network) => { const wrappers = [VersionManagerWrapper]; // Add Features to Version Manager - const newFeatures = [ + const features = [ GuardianManagerWrapper.contractAddress, LockManagerWrapper.contractAddress, RecoveryManagerWrapper.contractAddress, @@ -66,7 +66,7 @@ const deploy = async (network) => { MakerV2ManagerWrapper.contractAddress, RelayerManagerWrapper.contractAddress, ]; - const newFeaturesWithNoInit = [ // all features except the TransferManager + const featuresWithNoInit = [ // all features except the TransferManager GuardianManagerWrapper.contractAddress, LockManagerWrapper.contractAddress, RecoveryManagerWrapper.contractAddress, @@ -77,8 +77,8 @@ const deploy = async (network) => { MakerV2ManagerWrapper.contractAddress, RelayerManagerWrapper.contractAddress, ]; - const newFeatureToInit = newFeatures.filter((f) => !newFeaturesWithNoInit.includes(f)); - const VersionManagerAddVersionTx = await VersionManagerWrapper.contract.addVersion(newFeatures, newFeatureToInit, { gasPrice }); + const featureToInit = features.filter((f) => !featuresWithNoInit.includes(f)); + const VersionManagerAddVersionTx = await VersionManagerWrapper.contract.addVersion(features, featureToInit, { gasPrice }); await VersionManagerWrapper.verboseWaitForTransaction(VersionManagerAddVersionTx, "Adding New Version"); // ////////////////////////////////// From a4c34b52ae50d63669703823b02d4c10eef30c11 Mon Sep 17 00:00:00 2001 From: elenadimitrova Date: Mon, 19 Oct 2020 17:06:55 +0300 Subject: [PATCH 14/47] Rename upgrade script to template --- deployment/{7_upgrade_2_0.js => module_upgrade_template.js} | 0 package.json | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) rename deployment/{7_upgrade_2_0.js => module_upgrade_template.js} (100%) diff --git a/deployment/7_upgrade_2_0.js b/deployment/module_upgrade_template.js similarity index 100% rename from deployment/7_upgrade_2_0.js rename to deployment/module_upgrade_template.js diff --git a/package.json b/package.json index 8889bd1cc..83db3a46f 100644 --- a/package.json +++ b/package.json @@ -27,7 +27,7 @@ "test:coverage": "bash ./scripts/provision_lib_artefacts.sh & npx etherlime coverage && istanbul check-coverage --statements 94 --branches 83 --functions 96 --lines 94", "lint:js": "eslint .", "lint:contracts": "npx solhint contracts/**/*.sol && npx solhint contracts/**/**/*.sol", - "test:deployment": "./scripts/deploy.sh ganache `seq 1 7`", + "test:deployment": "./scripts/deploy.sh ganache `seq 1 6`", "test:benchmark": "./scripts/deploy.sh ganache 999", "security:slither": "npm run security:slither:infrastructure && npm run security:slither:modules", "security:slither:infrastructure": "rm -rf build && npm run compile:infrastructure && slither . --ignore-compile --filter-paths lib --solc-disable-warnings --exclude-low --exclude-informational --exclude=naming-convention,unused-state-variables,solc-version,assembly-usage,low-level-calls,public-function-that-could-be-declared-as-external", From 7ece36b3548b77c7256fbf7e0b6c376f789a4a43 Mon Sep 17 00:00:00 2001 From: elenadimitrova Date: Wed, 21 Oct 2020 09:45:37 +0300 Subject: [PATCH 15/47] Move TokenPriceRegistry to modules where it's currently held Update schema and fix a few incorrect abi uploads --- deployment/2_deploy_contracts.js | 8 ++++---- utils/config-schema.json | 3 +++ utils/config/ganache.json | 2 +- utils/versions/ganache/latest.json | 2 +- 4 files changed, 9 insertions(+), 6 deletions(-) diff --git a/deployment/2_deploy_contracts.js b/deployment/2_deploy_contracts.js index e478bc440..abf530d11 100644 --- a/deployment/2_deploy_contracts.js +++ b/deployment/2_deploy_contracts.js @@ -140,6 +140,7 @@ const deploy = async (network) => { TransferStorage: TransferStorageWrapper.contractAddress, LockStorage: LockStorageWrapper.contractAddress, LimitStorage: LimitStorageWrapper.contractAddress, + TokenPriceRegistry: TokenPriceRegistryWrapper.contractAddress, }); configurator.updateInfrastructureAddresses({ @@ -149,7 +150,6 @@ const deploy = async (network) => { ENSManager: ENSManagerWrapper.contractAddress, ModuleRegistry: ModuleRegistryWrapper.contractAddress, CompoundRegistry: CompoundRegistryWrapper.contractAddress, - TokenPriceRegistry: TokenPriceRegistryWrapper.contractAddress, DexRegistry: DexRegistryWrapper.contractAddress, BaseWallet: BaseWalletWrapper.contractAddress, }); @@ -158,15 +158,15 @@ const deploy = async (network) => { await Promise.all([ abiUploader.upload(GuardianStorageWrapper, "modules"), abiUploader.upload(TransferStorageWrapper, "modules"), - abiUploader.upload(LockStorageWrapper, "contracts"), - abiUploader.upload(LimitStorageWrapper, "contracts"), + 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(ModuleRegistryWrapper, "contracts"), abiUploader.upload(CompoundRegistryWrapper, "contracts"), - abiUploader.upload(TokenPriceRegistryWrapper, "contracts"), abiUploader.upload(DexRegistryWrapper, "contracts"), abiUploader.upload(BaseWalletWrapper, "contracts"), ]); diff --git a/utils/config-schema.json b/utils/config-schema.json index 13234b78e..4277bb840 100644 --- a/utils/config-schema.json +++ b/utils/config-schema.json @@ -165,6 +165,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 4b6e92abf..d0a36d727 100644 --- a/utils/config/ganache.json +++ b/utils/config/ganache.json @@ -1 +1 @@ -{"ENS":{"deployOwnRegistry":true,"ensRegistry":"0x7E93afFaa1452a7e07B961Aab04EF20a4Bc12b78","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":"0x101b60626A8073271fE2eFE59FB8fD74c108c2eC"},"uniswap":{"deployOwn":true,"factory":"0x59f30a6703F8Af4FAeAe0e8AFaB14202222F335f"},"compound":{"comptroller":"0x0000000000000000000000000000000000000000","markets":{"0x0000000000000000000000000000000000000000":"0x0000000000000000000000000000000000000000"}},"paraswap":{"deployOwn":true,"contract":"0x22eF7a1De649A8f027384D6a6eDd301AC5FD30D7","authorisedExchanges":{"Kyber":"0x3f906DDA5fa9DA5259d2d865157f5001Ce626e31"}}},"contracts":{"MultiSigWallet":"0xD822e79E2CA49c668f7d97DB1906330bc64459EB","WalletFactory":"0xD7A53fDd46a663C678da0580A66843Ae9b7783Da","ENSResolver":"0xC790c0E4D6811884b52121A5A8d0c0198F515030","ENSManager":"0x75eC71B541b0b2e0f9A4a2bea381c23fd7B0fB0c","TokenPriceProvider":"0xE09C322B745434Fbb2D18c107c13795D4bDF3657","ModuleRegistry":"0x61B2D9C034cD0DAE1738D039cdF9af860ee6B31A","BaseWallet":"0xF346Ee8d0b5738E037DbF22355949411187Da4bC","CompoundRegistry":"0x42ecEbe8946c4954aFc0C2333977adCb243614ed","MakerRegistry":"0x355d26E5f04255F60E910F1aA1251DE5D10996E8","DexRegistry":"0x4A74DE038616791359aEfED19f3f6B75FF6dd340"},"modules":{"GuardianStorage":"0xeE20db41be0CB3D09C5557dDD99d95A4fFf15076","TransferStorage":"0xF8E2BBA1AD8Fd1034DFA1b8Fb6bF5a724fa973ab","GuardianManager":"0x1046E04ED27A22079f69674Bb8bDfbe7c11f43d1","LockManager":"0xCFCE66b4A7904Ad0c4Fe43CB524759C222154BD9","RecoveryManager":"0x8dA7E75A6578F8597eB2c605Fa178F37DbD7E34d","ApprovedTransfer":"0xa6379f76FD8603a44F699EEB42735Ffd00d617D0","TokenExchanger":"0xcF809DD0f9fd4d4b64BcEf64de890BF5579A74A2","NftTransfer":"0x7f7d740Aaf4996C6F4718Fd9003Bb3CC565A11dC","MakerManager":"0x2718cAce241213B18F67C6d66B245EE77DB0Ce2a","TransferManager":"0xc49b23b3403fcEDa2b512D2178fA7020E8f5227E","CompoundManager":"0x55C914A069a49c4163b1dC5FbB216e3ae0339f98","MakerV2Manager":"0x950B78F4B2ecA78C552F4797c0cf10d1aaDDAD2c","LimitStorage":"0xb9dC99d37D77B915768029767C3dC0ff004540E5","TokenPriceStorage":"0x5dA4F81c17776495e3d7999ad9217748D257f074","RelayerModule":"0x171eb25c80da6adF0436668126285ca51e37eB30","RelayerManager":"0x6E5727685D31C3d9bE0c19F5A635551733567e15","VersionManager":"0xf4146041EdE0B35A6573DA0D820cA6Da785dFf17","LockStorage":"0x731a09EEda7ed25080A5f92F117076f6a8ED0E14","TokenPriceRegistry":"0x7e7acD252559062f0E6355847f4Fbb1c62872f14"},"gitCommit":"29a0aee0fd3e30c570e440b69dc14e7ca93755c9"} \ 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/versions/ganache/latest.json b/utils/versions/ganache/latest.json index bb7281b29..efa8eaec4 100644 --- a/utils/versions/ganache/latest.json +++ b/utils/versions/ganache/latest.json @@ -1 +1 @@ -{"modules":[{"address":"0xf4146041EdE0B35A6573DA0D820cA6Da785dFf17","name":"VersionManager"}],"fingerprint":"0xde5fa3e0","version":"1.0.0","createdAt":1602570421} \ 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 From 06621099f55c475d9e395f1d96761b21edb2a9ce Mon Sep 17 00:00:00 2001 From: Julien Niset Date: Wed, 23 Sep 2020 17:44:57 +0200 Subject: [PATCH 16/47] contract and script to deploy the ArgentWalletDetector exposing a single isArgentWallet(address) method. --- contracts/helpers/ArgentWalletDetector.sol | 131 +++++++++++++++++++++ scripts/deploy_wallet_detector.js | 45 +++++++ test/argentWalletDetector.js | 86 ++++++++++++++ 3 files changed, 262 insertions(+) create mode 100644 contracts/helpers/ArgentWalletDetector.sol create mode 100644 scripts/deploy_wallet_detector.js create mode 100644 test/argentWalletDetector.js diff --git a/contracts/helpers/ArgentWalletDetector.sol b/contracts/helpers/ArgentWalletDetector.sol new file mode 100644 index 000000000..9df7bc043 --- /dev/null +++ b/contracts/helpers/ArgentWalletDetector.sol @@ -0,0 +1,131 @@ +// 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 "../infrastructure/base/Owned.sol"; + +/** + * @title IArgentProxy + * @notice Interface for all Argent Proxy contracts exposing the target implementation. + */ +interface IArgentProxy { + 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 ad implementations. + * @author Julien Niset - + */ +contract ArgentWalletDetector is Owned { + + // The accepeted code hash + bytes32[] private codes; + // The accepted implementations + address[] private implementations; + // mapping to efficiently check if a code is accepeted + mapping (bytes32 => Info) public acceptedCodes; + // mapping to efficiently check is an implementation is accepeted + mapping (address => Info) public acceptedImplementations; + + struct Info { + bool exists; + uint128 index; + } + + // emits when a new accepeted code is added + event CodeAdded(bytes32 indexed code); + // emits when a new accepeted 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 acceted 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 acceted 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 acceted code hash and implementation from a deployed Argent wallet. + * @param _argentWallet The deployed Argent wallet. + */ + function addCodeAndImplementationFromWallet(address _argentWallet) external onlyOwner { + bytes32 codeHash; + assembly { codeHash := extcodehash(_argentWallet) } + addCode(codeHash); + address implementation = IArgentProxy(_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; + assembly { codeHash := extcodehash(_wallet) } + return acceptedCodes[codeHash].exists && acceptedImplementations[IArgentProxy(_wallet).implementation()].exists; + } +} diff --git a/scripts/deploy_wallet_detector.js b/scripts/deploy_wallet_detector.js new file mode 100644 index 000000000..e330b2e6a --- /dev/null +++ b/scripts/deploy_wallet_detector.js @@ -0,0 +1,45 @@ +// Usage: ./execute.sh --no-compile deploy_wallet_detector.js staging +const ethers = require("ethers"); +const ArgentWalletDetector = require("../build/ArgentWalletDetector"); +const DeployManager = require("../utils/deploy-manager.js"); + +const defaultNetwork = "test"; + +const MULTISIG = "0xa5c603e1C27a96171487aea0649b01c56248d2e8"; + +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 } = deployManager; + + // 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(MULTISIG); +} + +main().catch((err) => { + throw err; +}); + +// contract deployed to prod at 0xeca4B0bDBf7c55E9b7925919d03CbF8Dc82537E8 diff --git a/test/argentWalletDetector.js b/test/argentWalletDetector.js new file mode 100644 index 000000000..cc9d5e24c --- /dev/null +++ b/test/argentWalletDetector.js @@ -0,0 +1,86 @@ +const ArgentWalletDetector = require("../build/ArgentWalletDetector"); +const Proxy = require("../build/Proxy"); +const BaseWallet = require("../build/BaseWallet"); + +const TestManager = require("../utils/test-manager"); + +const argent_code = "0x7899f8a5e2362ec6ae586d08ca7a344acd3122abf2d23c5df2a18d7a540dd500"; +const random_code = "0x880ac7547a884027b93f5eaba5ff545919fdeb3c23ed0d2094db66303b3a80ac" + +describe("ArgentWalletDetector", function () { + const manager = new TestManager(); + + let deployer; + let detector, implementation1, implementation2, proxy1, proxy2; + + 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); + }); + + beforeEach(async () => { + detector = await deployer.deploy(ArgentWalletDetector, {}, [],[]); + }); + + describe("add info", () => { + + it('should deploy with codes and implementations', async () => { + let c = [argent_code, random_code]; + let 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], argent_code, "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(argent_code); + await detector.addCode(random_code); + const codes = await detector.getCodes(); + assert.equal(codes[0], argent_code, "should have added first code"); + assert.equal(codes[1], random_code, "should have added second code"); + }); + + it('should add code and implementation from a wallet', async () => { + await detector.addCodeAndImplementationFromWallet(proxy1.contractAddress); + let isArgent = await detector.isArgentWallet(proxy1.contractAddress); + assert.isTrue(isArgent, "should return true for an Argent wallet"); + }); + + it('should return true for an Argent wallet', async () => { + await detector.addImplementation(implementation1.contractAddress); + await detector.addCode(argent_code); + let 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(argent_code); + let isArgent = await detector.isArgentWallet(detector.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(argent_code); + let isArgent = await detector.isArgentWallet(proxy2.contractAddress); + assert.isFalse(isArgent, "should return false when the implementation is not correct"); + }); + }); + +}); From b17656e06d238ae83b5245211f65a329bd622403 Mon Sep 17 00:00:00 2001 From: Julien Niset Date: Fri, 9 Oct 2020 09:58:40 +0200 Subject: [PATCH 17/47] Moving ArgentWalletDetector to /infrastructure. Adding implementation getter to IWallet interface. Adding script to update the detector. --- contracts-test/FakeWallet.sol | 2 +- .../ArgentWalletDetector.sol | 17 ++-- contracts/wallet/BaseWallet.sol | 2 +- contracts/wallet/IWallet.sol | 7 ++ scripts/update_wallet_detector.js | 77 +++++++++++++++++++ 5 files changed, 91 insertions(+), 14 deletions(-) rename contracts/{helpers => infrastructure}/ArgentWalletDetector.sol (89%) create mode 100644 scripts/update_wallet_detector.js diff --git a/contracts-test/FakeWallet.sol b/contracts-test/FakeWallet.sol index b74e1a5f9..1a190cde3 100644 --- a/contracts-test/FakeWallet.sol +++ b/contracts-test/FakeWallet.sol @@ -36,7 +36,7 @@ contract FakeWallet is IWallet { } // The implementation of the proxy - address public implementation; + address public override implementation; // The owner address public override owner; // The authorised modules diff --git a/contracts/helpers/ArgentWalletDetector.sol b/contracts/infrastructure/ArgentWalletDetector.sol similarity index 89% rename from contracts/helpers/ArgentWalletDetector.sol rename to contracts/infrastructure/ArgentWalletDetector.sol index 9df7bc043..e675fc792 100644 --- a/contracts/helpers/ArgentWalletDetector.sol +++ b/contracts/infrastructure/ArgentWalletDetector.sol @@ -15,22 +15,15 @@ // SPDX-License-Identifier: GPL-3.0-only pragma solidity ^0.6.12; -import "../infrastructure/base/Owned.sol"; - -/** - * @title IArgentProxy - * @notice Interface for all Argent Proxy contracts exposing the target implementation. - */ -interface IArgentProxy { - function implementation() external view returns (address); -} +import "./base/Owned.sol"; +import "../wallet/IWallet.sol"; /** * @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 ad implementations. + * Only the owner of the contract can add code hash and implementations. * @author Julien Niset - */ contract ArgentWalletDetector is Owned { @@ -101,7 +94,7 @@ contract ArgentWalletDetector is Owned { bytes32 codeHash; assembly { codeHash := extcodehash(_argentWallet) } addCode(codeHash); - address implementation = IArgentProxy(_argentWallet).implementation(); + address implementation = IWallet(_argentWallet).implementation(); addImplementation(implementation); } @@ -126,6 +119,6 @@ contract ArgentWalletDetector is Owned { function isArgentWallet(address _wallet) external view returns (bool) { bytes32 codeHash; assembly { codeHash := extcodehash(_wallet) } - return acceptedCodes[codeHash].exists && acceptedImplementations[IArgentProxy(_wallet).implementation()].exists; + return acceptedCodes[codeHash].exists && acceptedImplementations[IWallet(_wallet).implementation()].exists; } } diff --git a/contracts/wallet/BaseWallet.sol b/contracts/wallet/BaseWallet.sol index 1de6121fa..e05dc2246 100644 --- a/contracts/wallet/BaseWallet.sol +++ b/contracts/wallet/BaseWallet.sol @@ -27,7 +27,7 @@ import "./IWallet.sol"; contract BaseWallet is IWallet { // The implementation of the proxy - address public implementation; + address public override implementation; // The owner address public override owner; // The authorised modules diff --git a/contracts/wallet/IWallet.sol b/contracts/wallet/IWallet.sol index 5428edb80..700acb043 100644 --- a/contracts/wallet/IWallet.sol +++ b/contracts/wallet/IWallet.sol @@ -21,6 +21,13 @@ pragma solidity >=0.5.4 <0.7.0; * @notice Interface for the BaseWallet */ interface IWallet { + + /** + * @notice Returns the implementation of the wallet. + * @return The wallet implementation. + */ + function implementation() external view returns (address); + /** * @notice Returns the wallet owner. * @return The wallet owner address. diff --git a/scripts/update_wallet_detector.js b/scripts/update_wallet_detector.js new file mode 100644 index 000000000..86f8c112b --- /dev/null +++ b/scripts/update_wallet_detector.js @@ -0,0 +1,77 @@ +// /////////////////////////////////////////////////////////////////// +// Script to add/remove a module from the ModuleRegistry +// +// To add a new wallet (code + implementation): +// ./execute_script.sh update_wallet_detector.js --wallet +// +// To add a new implementation: +// ./execute_script.sh update_module_registry.js --implementation +// +// To add a new code: +// ./execute_script.sh update_module_registry.js --code +// +// where: +// - network = [test, staging, 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, code, 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]); + } + if (code) { + console.log(`Adding wallet code ${code}`); + await multisigExecutor.executeCall(ArgentWalletDetectorWrapper, "addCode", [code]); + } + if (implementation) { + console.log(`Adding wallet implementation ${implementation}`); + await multisigExecutor.executeCall(ArgentWalletDetectorWrapper, "addImplementation", [implementation]); + } +} + +main().catch((err) => { + throw err; +}); \ No newline at end of file From 9144c16772211b5e8d87c3899d76b0e2e35d6af9 Mon Sep 17 00:00:00 2001 From: Julien Niset Date: Fri, 9 Oct 2020 11:03:39 +0200 Subject: [PATCH 18/47] cleaning scripts --- scripts/deploy_wallet_detector.js | 44 ++++++++++----- scripts/update_wallet_detector.js | 7 +-- test/argentWalletDetector.js | 90 ++++++++++++++++--------------- 3 files changed, 81 insertions(+), 60 deletions(-) diff --git a/scripts/deploy_wallet_detector.js b/scripts/deploy_wallet_detector.js index e330b2e6a..1cdc478e0 100644 --- a/scripts/deploy_wallet_detector.js +++ b/scripts/deploy_wallet_detector.js @@ -1,23 +1,29 @@ -// Usage: ./execute.sh --no-compile deploy_wallet_detector.js staging -const ethers = require("ethers"); +// /////////////////////////////////////////////////////////////////// +// 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 MULTISIG = "0xa5c603e1C27a96171487aea0649b01c56248d2e8"; - const PROXYWALLET_CODEHASH = [ - '0x0b44c9be520023d9f6091278e7e5a8853257eb9fb3d78e6951315df59679e3b2', // factory prod Mar-30-2020 - '0x83baa4b265772664a88dcfc8be0e24e1fe969a3c66f03851c6aa2f5da73cd7fd', // factory prod Feb-04-2019 + "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 + "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() { @@ -27,7 +33,8 @@ async function main() { const deployManager = new DeployManager(network); await deployManager.setup(); - const { deployer } = deployManager; + const { deployer, configurator, abiUploader } = deployManager; + const { config } = configurator; // Deploy ArgentWalletDetector contract console.log("Deploying ArgentWalletDetector..."); @@ -35,7 +42,18 @@ async function main() { // Transfer ownership to the multisig console.log("Transferring ownership to the Multisig..."); - await ArgentWalletDetectortWrapper.changeOwner(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) => { diff --git a/scripts/update_wallet_detector.js b/scripts/update_wallet_detector.js index 86f8c112b..4dcdc66b0 100644 --- a/scripts/update_wallet_detector.js +++ b/scripts/update_wallet_detector.js @@ -21,8 +21,9 @@ const DeployManager = require("../utils/deploy-manager.js"); const MultisigExecutor = require("../utils/multisigexecutor.js"); async function main() { - - let wallet, code, implementation; + let wallet; + let code; + let implementation; // Read Command Line Arguments let idx = process.argv.indexOf("--network"); @@ -74,4 +75,4 @@ async function main() { main().catch((err) => { throw err; -}); \ No newline at end of file +}); diff --git a/test/argentWalletDetector.js b/test/argentWalletDetector.js index cc9d5e24c..71f9b1cab 100644 --- a/test/argentWalletDetector.js +++ b/test/argentWalletDetector.js @@ -4,14 +4,18 @@ const BaseWallet = require("../build/BaseWallet"); const TestManager = require("../utils/test-manager"); -const argent_code = "0x7899f8a5e2362ec6ae586d08ca7a344acd3122abf2d23c5df2a18d7a540dd500"; -const random_code = "0x880ac7547a884027b93f5eaba5ff545919fdeb3c23ed0d2094db66303b3a80ac" +const argentCode = "0x7899f8a5e2362ec6ae586d08ca7a344acd3122abf2d23c5df2a18d7a540dd500"; +const randomCode = "0x880ac7547a884027b93f5eaba5ff545919fdeb3c23ed0d2094db66303b3a80ac"; -describe("ArgentWalletDetector", function () { +describe("ArgentWalletDetector", () => { const manager = new TestManager(); let deployer; - let detector, implementation1, implementation2, proxy1, proxy2; + let detector; + let implementation1; + let implementation2; + let proxy1; + let proxy2; before(async () => { deployer = manager.newDeployer(); @@ -22,65 +26,63 @@ describe("ArgentWalletDetector", function () { }); beforeEach(async () => { - detector = await deployer.deploy(ArgentWalletDetector, {}, [],[]); + detector = await deployer.deploy(ArgentWalletDetector, {}, [], []); }); describe("add info", () => { - - it('should deploy with codes and implementations', async () => { - let c = [argent_code, random_code]; - let i = [implementation1.contractAddress, implementation2.contractAddress]; + it("should deploy with codes and implementations", async () => { + const c = [argentCode, randomCode]; + 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], argent_code, "should have added first code"); - assert.equal(codes[1], random_code, "should have added second code"); - }); + assert.equal(codes[0], argentCode, "should have added first code"); + assert.equal(codes[1], randomCode, "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 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(argent_code); - await detector.addCode(random_code); - const codes = await detector.getCodes(); - assert.equal(codes[0], argent_code, "should have added first code"); - assert.equal(codes[1], random_code, "should have added second code"); + it("should add codes", async () => { + await detector.addCode(argentCode); + await detector.addCode(randomCode); + const codes = await detector.getCodes(); + assert.equal(codes[0], argentCode, "should have added first code"); + assert.equal(codes[1], randomCode, "should have added second code"); }); - it('should add code and implementation from a wallet', async () => { - await detector.addCodeAndImplementationFromWallet(proxy1.contractAddress); - let isArgent = await detector.isArgentWallet(proxy1.contractAddress); - assert.isTrue(isArgent, "should return true for an Argent wallet"); + 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 true for an Argent wallet', async () => { - await detector.addImplementation(implementation1.contractAddress); - await detector.addCode(argent_code); - let isArgent = await detector.isArgentWallet(proxy1.contractAddress); - assert.isTrue(isArgent, "should return true for an Argent wallet"); + it("should return true for an Argent wallet", async () => { + await detector.addImplementation(implementation1.contractAddress); + await detector.addCode(argentCode); + 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(argent_code); - let isArgent = await detector.isArgentWallet(detector.contractAddress); - assert.isFalse(isArgent, "should return false when the code is not correct"); + it("should return false when the code is not correct", async () => { + await detector.addImplementation(implementation1.contractAddress); + await detector.addCode(argentCode); + const isArgent = await detector.isArgentWallet(detector.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(argent_code); - let isArgent = await detector.isArgentWallet(proxy2.contractAddress); - assert.isFalse(isArgent, "should return false when the implementation 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"); }); }); - }); From fae6b32ed3c322336faf67c4b5e22c5fdd8d7af9 Mon Sep 17 00:00:00 2001 From: Julien Niset Date: Fri, 9 Oct 2020 11:15:09 +0200 Subject: [PATCH 19/47] add warning to update script --- scripts/update_wallet_detector.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/scripts/update_wallet_detector.js b/scripts/update_wallet_detector.js index 4dcdc66b0..130948c1a 100644 --- a/scripts/update_wallet_detector.js +++ b/scripts/update_wallet_detector.js @@ -1,5 +1,9 @@ // /////////////////////////////////////////////////////////////////// -// Script to add/remove a module from the ModuleRegistry +// 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 @@ -11,7 +15,7 @@ // ./execute_script.sh update_module_registry.js --code // // where: -// - network = [test, staging, prod] +// - network = [test, prod] // //////////////////////////////////////////////////////////////////// const ArgentWalletDetector = require("../build/ArgentWalletDetector"); From 6ecc9cbfe05807d2fd244721d2462e0090dce20d Mon Sep 17 00:00:00 2001 From: Julien Niset Date: Fri, 9 Oct 2020 14:59:12 +0200 Subject: [PATCH 20/47] addition of ArgenWalletDetector to config-schema.json --- utils/config-schema.json | 3 +++ 1 file changed, 3 insertions(+) diff --git a/utils/config-schema.json b/utils/config-schema.json index 4277bb840..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": [ From b548064fc0d066db4af3d65ed27227930773ebb4 Mon Sep 17 00:00:00 2001 From: Julien Niset Date: Thu, 15 Oct 2020 16:08:28 +0200 Subject: [PATCH 21/47] fixing Solidity linting and removing redundant test. --- contracts/infrastructure/ArgentWalletDetector.sol | 2 ++ test/argentWalletDetector.js | 7 ------- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/contracts/infrastructure/ArgentWalletDetector.sol b/contracts/infrastructure/ArgentWalletDetector.sol index e675fc792..0a3316a47 100644 --- a/contracts/infrastructure/ArgentWalletDetector.sol +++ b/contracts/infrastructure/ArgentWalletDetector.sol @@ -92,6 +92,7 @@ contract ArgentWalletDetector is Owned { */ function addCodeAndImplementationFromWallet(address _argentWallet) external onlyOwner { bytes32 codeHash; + // solhint-disable-next-line no-inline-assembly assembly { codeHash := extcodehash(_argentWallet) } addCode(codeHash); address implementation = IWallet(_argentWallet).implementation(); @@ -118,6 +119,7 @@ contract ArgentWalletDetector is Owned { */ 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[IWallet(_wallet).implementation()].exists; } diff --git a/test/argentWalletDetector.js b/test/argentWalletDetector.js index 71f9b1cab..f0d23e839 100644 --- a/test/argentWalletDetector.js +++ b/test/argentWalletDetector.js @@ -64,13 +64,6 @@ describe("ArgentWalletDetector", () => { assert.isTrue(isArgent, "should return true for an Argent wallet"); }); - it("should return true for an Argent wallet", async () => { - await detector.addImplementation(implementation1.contractAddress); - await detector.addCode(argentCode); - 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(argentCode); From 3e22689b5ad9a8193c96f950f90719593168130e Mon Sep 17 00:00:00 2001 From: Julien Niset Date: Fri, 16 Oct 2020 12:31:22 +0200 Subject: [PATCH 22/47] fixing typos --- contracts/infrastructure/ArgentWalletDetector.sol | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/contracts/infrastructure/ArgentWalletDetector.sol b/contracts/infrastructure/ArgentWalletDetector.sol index 0a3316a47..c3d7f324e 100644 --- a/contracts/infrastructure/ArgentWalletDetector.sol +++ b/contracts/infrastructure/ArgentWalletDetector.sol @@ -28,13 +28,13 @@ import "../wallet/IWallet.sol"; */ contract ArgentWalletDetector is Owned { - // The accepeted code hash + // The accepted code hashes bytes32[] private codes; // The accepted implementations address[] private implementations; - // mapping to efficiently check if a code is accepeted + // mapping to efficiently check if a code is accepted mapping (bytes32 => Info) public acceptedCodes; - // mapping to efficiently check is an implementation is accepeted + // mapping to efficiently check is an implementation is accepted mapping (address => Info) public acceptedImplementations; struct Info { @@ -42,9 +42,9 @@ contract ArgentWalletDetector is Owned { uint128 index; } - // emits when a new accepeted code is added + // emits when a new accepted code is added event CodeAdded(bytes32 indexed code); - // emits when a new accepeted implementation is added + // emits when a new accepted implementation is added event ImplementationAdded(address indexed implementation); constructor(bytes32[] memory _codes, address[] memory _implementations) public { From 73fd211160ca01cd22b1a22dc3a2a8fed209b27c Mon Sep 17 00:00:00 2001 From: Julien Niset Date: Fri, 16 Oct 2020 12:36:15 +0200 Subject: [PATCH 23/47] fixing indentation issues --- .../infrastructure/ArgentWalletDetector.sol | 110 +++++++++--------- 1 file changed, 55 insertions(+), 55 deletions(-) diff --git a/contracts/infrastructure/ArgentWalletDetector.sol b/contracts/infrastructure/ArgentWalletDetector.sol index c3d7f324e..ecb512487 100644 --- a/contracts/infrastructure/ArgentWalletDetector.sol +++ b/contracts/infrastructure/ArgentWalletDetector.sol @@ -28,99 +28,99 @@ import "../wallet/IWallet.sol"; */ 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 + // 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; + // mapping to efficiently check is an implementation is accepted + mapping (address => Info) public acceptedImplementations; - struct Info { + 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); + // 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]); - } - } + 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 acceted code hash. * @param _code The new code hash. */ - function addCode(bytes32 _code) public onlyOwner { + 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); - } + if(!code.exists) { + codes.push(_code); + code.exists = true; + code.index = uint128(codes.length - 1); + emit CodeAdded(_code); + } } - /** + /** * @notice Adds a new acceted implementation. * @param _impl The new implementation. */ - function addImplementation(address _impl) public onlyOwner { + 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); - } + if(!impl.exists) { + implementations.push(_impl); + impl.exists = true; + impl.index = uint128(implementations.length - 1); + emit ImplementationAdded(_impl); + } } - /** + /** * @notice Adds a new acceted 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) } + // solhint-disable-next-line no-inline-assembly + assembly { codeHash := extcodehash(_argentWallet) } addCode(codeHash); address implementation = IWallet(_argentWallet).implementation(); addImplementation(implementation); } - /** + /** * @notice Gets the list of accepted implementations. */ - function getImplementations() public view returns (address[] memory) { - return 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; - } + function getCodes() public view returns (bytes32[] memory) { + return codes; + } - /** + /** * @notice Checks if an address is an Argent wallet - * @param _wallet The target 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[IWallet(_wallet).implementation()].exists; - } + 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[IWallet(_wallet).implementation()].exists; + } } From a5089274fc23d0fffe6a5767414e3c03c443ba67 Mon Sep 17 00:00:00 2001 From: Julien Niset Date: Fri, 16 Oct 2020 16:56:12 +0200 Subject: [PATCH 24/47] fixing test --- test/argentWalletDetector.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/test/argentWalletDetector.js b/test/argentWalletDetector.js index f0d23e839..434e8a08c 100644 --- a/test/argentWalletDetector.js +++ b/test/argentWalletDetector.js @@ -4,7 +4,7 @@ const BaseWallet = require("../build/BaseWallet"); const TestManager = require("../utils/test-manager"); -const argentCode = "0x7899f8a5e2362ec6ae586d08ca7a344acd3122abf2d23c5df2a18d7a540dd500"; +//const argentCode = "0x7899f8a5e2362ec6ae586d08ca7a344acd3122abf2d23c5df2a18d7a540dd500"; const randomCode = "0x880ac7547a884027b93f5eaba5ff545919fdeb3c23ed0d2094db66303b3a80ac"; describe("ArgentWalletDetector", () => { @@ -16,6 +16,7 @@ describe("ArgentWalletDetector", () => { let implementation2; let proxy1; let proxy2; + let argentCode; before(async () => { deployer = manager.newDeployer(); @@ -23,6 +24,7 @@ describe("ArgentWalletDetector", () => { 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 () => { @@ -66,8 +68,8 @@ describe("ArgentWalletDetector", () => { it("should return false when the code is not correct", async () => { await detector.addImplementation(implementation1.contractAddress); - await detector.addCode(argentCode); - const isArgent = await detector.isArgentWallet(detector.contractAddress); + await detector.addCode(randomCode); + const isArgent = await detector.isArgentWallet(proxy1.contractAddress); assert.isFalse(isArgent, "should return false when the code is not correct"); }); From 441bb4ead6f0ce28e320ef2623956a5e276ca4cb Mon Sep 17 00:00:00 2001 From: Julien Niset Date: Tue, 20 Oct 2020 14:56:25 +0200 Subject: [PATCH 25/47] addressing PR's comments, increasing coverage, fixing bug in execute_script.sh --- contracts-test/FakeWallet.sol | 2 +- .../infrastructure/ArgentWalletDetector.sol | 13 ++++-- contracts/wallet/BaseWallet.sol | 2 +- contracts/wallet/IWallet.sol | 7 ---- scripts/execute_script.sh | 2 +- scripts/update_wallet_detector.js | 10 ++--- test/argentWalletDetector.js | 42 +++++++++++++++---- 7 files changed, 52 insertions(+), 26 deletions(-) diff --git a/contracts-test/FakeWallet.sol b/contracts-test/FakeWallet.sol index 1a190cde3..b74e1a5f9 100644 --- a/contracts-test/FakeWallet.sol +++ b/contracts-test/FakeWallet.sol @@ -36,7 +36,7 @@ contract FakeWallet is IWallet { } // The implementation of the proxy - address public override implementation; + address public implementation; // The owner address public override owner; // The authorised modules diff --git a/contracts/infrastructure/ArgentWalletDetector.sol b/contracts/infrastructure/ArgentWalletDetector.sol index ecb512487..c98453d53 100644 --- a/contracts/infrastructure/ArgentWalletDetector.sol +++ b/contracts/infrastructure/ArgentWalletDetector.sol @@ -16,7 +16,14 @@ // SPDX-License-Identifier: GPL-3.0-only pragma solidity ^0.6.12; import "./base/Owned.sol"; -import "../wallet/IWallet.sol"; + +interface IArgentWallet { + /** + * @notice Returns the implementation of the wallet. + * @return The wallet implementation. + */ + function implementation() external view returns (address); +} /** * @title ArgentWalletDetector @@ -95,7 +102,7 @@ contract ArgentWalletDetector is Owned { // solhint-disable-next-line no-inline-assembly assembly { codeHash := extcodehash(_argentWallet) } addCode(codeHash); - address implementation = IWallet(_argentWallet).implementation(); + address implementation = IArgentWallet(_argentWallet).implementation(); addImplementation(implementation); } @@ -121,6 +128,6 @@ contract ArgentWalletDetector is Owned { bytes32 codeHash; // solhint-disable-next-line no-inline-assembly assembly { codeHash := extcodehash(_wallet) } - return acceptedCodes[codeHash].exists && acceptedImplementations[IWallet(_wallet).implementation()].exists; + return acceptedCodes[codeHash].exists && acceptedImplementations[IArgentWallet(_wallet).implementation()].exists; } } diff --git a/contracts/wallet/BaseWallet.sol b/contracts/wallet/BaseWallet.sol index e05dc2246..1de6121fa 100644 --- a/contracts/wallet/BaseWallet.sol +++ b/contracts/wallet/BaseWallet.sol @@ -27,7 +27,7 @@ import "./IWallet.sol"; contract BaseWallet is IWallet { // The implementation of the proxy - address public override implementation; + address public implementation; // The owner address public override owner; // The authorised modules diff --git a/contracts/wallet/IWallet.sol b/contracts/wallet/IWallet.sol index 700acb043..5428edb80 100644 --- a/contracts/wallet/IWallet.sol +++ b/contracts/wallet/IWallet.sol @@ -21,13 +21,6 @@ pragma solidity >=0.5.4 <0.7.0; * @notice Interface for the BaseWallet */ interface IWallet { - - /** - * @notice Returns the implementation of the wallet. - * @return The wallet implementation. - */ - function implementation() external view returns (address); - /** * @notice Returns the wallet owner. * @return The wallet owner address. 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 index 130948c1a..9646ef9fa 100644 --- a/scripts/update_wallet_detector.js +++ b/scripts/update_wallet_detector.js @@ -9,10 +9,10 @@ // ./execute_script.sh update_wallet_detector.js --wallet // // To add a new implementation: -// ./execute_script.sh update_module_registry.js --implementation +// ./execute_script.sh update_wallet_detector.js --implementation // // To add a new code: -// ./execute_script.sh update_module_registry.js --code +// ./execute_script.sh update_wallet_detector.js --code // // where: // - network = [test, prod] @@ -66,12 +66,10 @@ async function main() { if (wallet) { console.log(`Adding wallet code and implementation from ${wallet}`); await multisigExecutor.executeCall(ArgentWalletDetectorWrapper, "addCodeAndImplementationFromWallet", [wallet]); - } - if (code) { + } else if (code) { console.log(`Adding wallet code ${code}`); await multisigExecutor.executeCall(ArgentWalletDetectorWrapper, "addCode", [code]); - } - if (implementation) { + } else if (implementation) { console.log(`Adding wallet implementation ${implementation}`); await multisigExecutor.executeCall(ArgentWalletDetectorWrapper, "addImplementation", [implementation]); } diff --git a/test/argentWalletDetector.js b/test/argentWalletDetector.js index 434e8a08c..f925b9bd6 100644 --- a/test/argentWalletDetector.js +++ b/test/argentWalletDetector.js @@ -1,11 +1,17 @@ +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 argentCode = "0x7899f8a5e2362ec6ae586d08ca7a344acd3122abf2d23c5df2a18d7a540dd500"; -const randomCode = "0x880ac7547a884027b93f5eaba5ff545919fdeb3c23ed0d2094db66303b3a80ac"; +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(); @@ -33,7 +39,7 @@ describe("ArgentWalletDetector", () => { describe("add info", () => { it("should deploy with codes and implementations", async () => { - const c = [argentCode, randomCode]; + const c = [argentCode, RANDOM_CODE]; const i = [implementation1.contractAddress, implementation2.contractAddress]; detector = await deployer.deploy(ArgentWalletDetector, {}, c, i); const implementations = await detector.getImplementations(); @@ -41,7 +47,7 @@ describe("ArgentWalletDetector", () => { 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], randomCode, "should have added second code"); + assert.equal(codes[1], RANDOM_CODE, "should have added second code"); }); it("should add implementations", async () => { @@ -54,10 +60,32 @@ describe("ArgentWalletDetector", () => { it("should add codes", async () => { await detector.addCode(argentCode); - await detector.addCode(randomCode); + await detector.addCode(RANDOM_CODE); const codes = await detector.getCodes(); assert.equal(codes[0], argentCode, "should have added first code"); - assert.equal(codes[1], randomCode, "should have added second 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 () => { @@ -68,7 +96,7 @@ describe("ArgentWalletDetector", () => { it("should return false when the code is not correct", async () => { await detector.addImplementation(implementation1.contractAddress); - await detector.addCode(randomCode); + await detector.addCode(RANDOM_CODE); const isArgent = await detector.isArgentWallet(proxy1.contractAddress); assert.isFalse(isArgent, "should return false when the code is not correct"); }); From ae17ac7401b7b223155f9e52bbb2e92483527481 Mon Sep 17 00:00:00 2001 From: Julien Niset Date: Tue, 20 Oct 2020 17:30:28 +0200 Subject: [PATCH 26/47] fixing js linting and error in deployment scripts --- deployment/3_setup_contracts.js | 2 +- deployment/5_deploy_modules.js | 8 ++++---- test/argentWalletDetector.js | 2 +- utils/config-schema.json | 6 ++++-- 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/deployment/3_setup_contracts.js b/deployment/3_setup_contracts.js index 0a6a10f14..a6ba91af2 100644 --- a/deployment/3_setup_contracts.js +++ b/deployment/3_setup_contracts.js @@ -28,7 +28,7 @@ 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 TokenPriceRegistryWrapper = await deployer.wrapDeployedContract(TokenPriceRegistry, config.modules.TokenPriceRegistry); + const TokenPriceRegistryWrapper = await deployer.wrapDeployedContract(TokenPriceRegistry, config.contracts.TokenPriceRegistry); const DexRegistryWrapper = await deployer.wrapDeployedContract(DexRegistry, config.contracts.DexRegistry); // Configure DexRegistry diff --git a/deployment/5_deploy_modules.js b/deployment/5_deploy_modules.js index c7cf946e8..63901711f 100644 --- a/deployment/5_deploy_modules.js +++ b/deployment/5_deploy_modules.js @@ -96,7 +96,7 @@ const deploy = async (network) => { config.modules.LockStorage, config.modules.TransferStorage, config.modules.LimitStorage, - config.modules.TokenPriceRegistry, + config.contracts.TokenPriceRegistry, VersionManagerWrapper.contractAddress, config.settings.securityPeriod || 0, config.settings.securityWindow || 0, @@ -109,7 +109,7 @@ const deploy = async (network) => { TokenExchanger, {}, config.modules.LockStorage, - config.modules.TokenPriceRegistry, + config.contracts.TokenPriceRegistry, VersionManagerWrapper.contractAddress, config.contracts.DexRegistry, config.defi.paraswap.contract, @@ -120,7 +120,7 @@ const deploy = async (network) => { NftTransfer, {}, config.modules.LockStorage, - config.modules.TokenPriceRegistry, + config.contracts.TokenPriceRegistry, VersionManagerWrapper.contractAddress, config.CryptoKitties.contract, ); @@ -152,7 +152,7 @@ const deploy = async (network) => { config.modules.LockStorage, config.modules.GuardianStorage, config.modules.LimitStorage, - config.modules.TokenPriceRegistry, + config.contracts.TokenPriceRegistry, VersionManagerWrapper.contractAddress, ); diff --git a/test/argentWalletDetector.js b/test/argentWalletDetector.js index f925b9bd6..dc424f91b 100644 --- a/test/argentWalletDetector.js +++ b/test/argentWalletDetector.js @@ -1,5 +1,5 @@ +/* global utils */ const ethers = require("ethers"); - const ArgentWalletDetector = require("../build/ArgentWalletDetector"); const Proxy = require("../build/Proxy"); const BaseWallet = require("../build/BaseWallet"); diff --git a/utils/config-schema.json b/utils/config-schema.json index a462c9470..f518125b5 100644 --- a/utils/config-schema.json +++ b/utils/config-schema.json @@ -132,6 +132,9 @@ "DexRegistry": { "$ref": "#/definitions/ethaddress" }, + "TokenPriceRegistry": { + "$ref": "#/definitions/ethaddress" + }, "ArgentWalletDetector": { "$ref": "#/definitions/ethaddress" } @@ -141,14 +144,13 @@ "WalletFactory", "ENSResolver", "ENSManager", - "TokenPriceProvider", "ModuleRegistry", "BaseWallet", "CompoundRegistry", "MakerRegistry", "DexRegistry" ], - "additionalProperties": false + "additionalProperties": true }, "modules": { "type": "object", From d0e1d78248dd2ad66a0599bdb63ab7443e3614ba Mon Sep 17 00:00:00 2001 From: Julien Niset Date: Wed, 21 Oct 2020 16:57:59 +0200 Subject: [PATCH 27/47] reverting deployment script changes after rebase --- deployment/3_setup_contracts.js | 2 +- deployment/5_deploy_modules.js | 8 ++++---- utils/config-schema.json | 4 +--- 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/deployment/3_setup_contracts.js b/deployment/3_setup_contracts.js index a6ba91af2..0a6a10f14 100644 --- a/deployment/3_setup_contracts.js +++ b/deployment/3_setup_contracts.js @@ -28,7 +28,7 @@ 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 TokenPriceRegistryWrapper = await deployer.wrapDeployedContract(TokenPriceRegistry, config.contracts.TokenPriceRegistry); + const TokenPriceRegistryWrapper = await deployer.wrapDeployedContract(TokenPriceRegistry, config.modules.TokenPriceRegistry); const DexRegistryWrapper = await deployer.wrapDeployedContract(DexRegistry, config.contracts.DexRegistry); // Configure DexRegistry diff --git a/deployment/5_deploy_modules.js b/deployment/5_deploy_modules.js index 63901711f..c7cf946e8 100644 --- a/deployment/5_deploy_modules.js +++ b/deployment/5_deploy_modules.js @@ -96,7 +96,7 @@ const deploy = async (network) => { config.modules.LockStorage, config.modules.TransferStorage, config.modules.LimitStorage, - config.contracts.TokenPriceRegistry, + config.modules.TokenPriceRegistry, VersionManagerWrapper.contractAddress, config.settings.securityPeriod || 0, config.settings.securityWindow || 0, @@ -109,7 +109,7 @@ const deploy = async (network) => { TokenExchanger, {}, config.modules.LockStorage, - config.contracts.TokenPriceRegistry, + config.modules.TokenPriceRegistry, VersionManagerWrapper.contractAddress, config.contracts.DexRegistry, config.defi.paraswap.contract, @@ -120,7 +120,7 @@ const deploy = async (network) => { NftTransfer, {}, config.modules.LockStorage, - config.contracts.TokenPriceRegistry, + config.modules.TokenPriceRegistry, VersionManagerWrapper.contractAddress, config.CryptoKitties.contract, ); @@ -152,7 +152,7 @@ const deploy = async (network) => { config.modules.LockStorage, config.modules.GuardianStorage, config.modules.LimitStorage, - config.contracts.TokenPriceRegistry, + config.modules.TokenPriceRegistry, VersionManagerWrapper.contractAddress, ); diff --git a/utils/config-schema.json b/utils/config-schema.json index f518125b5..7752e0ea9 100644 --- a/utils/config-schema.json +++ b/utils/config-schema.json @@ -132,9 +132,6 @@ "DexRegistry": { "$ref": "#/definitions/ethaddress" }, - "TokenPriceRegistry": { - "$ref": "#/definitions/ethaddress" - }, "ArgentWalletDetector": { "$ref": "#/definitions/ethaddress" } @@ -144,6 +141,7 @@ "WalletFactory", "ENSResolver", "ENSManager", + "TokenPriceProvider", "ModuleRegistry", "BaseWallet", "CompoundRegistry", From fb31c9bebf652e97980747de6d9b43813ac0b071 Mon Sep 17 00:00:00 2001 From: Julien Niset Date: Thu, 22 Oct 2020 10:07:09 +0200 Subject: [PATCH 28/47] fixed typos --- contracts/infrastructure/ArgentWalletDetector.sol | 6 +++--- utils/config-schema.json | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/contracts/infrastructure/ArgentWalletDetector.sol b/contracts/infrastructure/ArgentWalletDetector.sol index c98453d53..05d69e698 100644 --- a/contracts/infrastructure/ArgentWalletDetector.sol +++ b/contracts/infrastructure/ArgentWalletDetector.sol @@ -64,7 +64,7 @@ contract ArgentWalletDetector is Owned { } /** - * @notice Adds a new acceted code hash. + * @notice Adds a new accepted code hash. * @param _code The new code hash. */ function addCode(bytes32 _code) public onlyOwner { @@ -79,7 +79,7 @@ contract ArgentWalletDetector is Owned { } /** - * @notice Adds a new acceted implementation. + * @notice Adds a new accepted implementation. * @param _impl The new implementation. */ function addImplementation(address _impl) public onlyOwner { @@ -94,7 +94,7 @@ contract ArgentWalletDetector is Owned { } /** - * @notice Adds a new acceted code hash and implementation from a deployed Argent wallet. + * @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 { diff --git a/utils/config-schema.json b/utils/config-schema.json index 7752e0ea9..a462c9470 100644 --- a/utils/config-schema.json +++ b/utils/config-schema.json @@ -148,7 +148,7 @@ "MakerRegistry", "DexRegistry" ], - "additionalProperties": true + "additionalProperties": false }, "modules": { "type": "object", From 73174b83684355ce19d3c3d4c9763166c1f3c9df Mon Sep 17 00:00:00 2001 From: Olivier van den Biggelaar Date: Tue, 27 Oct 2020 11:48:03 +0000 Subject: [PATCH 29/47] Fixing isValidSignature static call --- contracts/modules/VersionManager.sol | 2 +- test/transferManager.js | 20 +++++++++++++++++--- utils/utilities.js | 2 ++ 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/contracts/modules/VersionManager.sol b/contracts/modules/VersionManager.sol index 95fc1c236..977b47c39 100644 --- a/contracts/modules/VersionManager.sol +++ b/contracts/modules/VersionManager.sol @@ -186,7 +186,7 @@ contract VersionManager is IVersionManager, IModule, BaseFeature, Owned { // 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/test/transferManager.js b/test/transferManager.js index 30c986320..77b779d97 100644 --- a/test/transferManager.js +++ b/test/transferManager.js @@ -1,5 +1,5 @@ -/* global accounts, utils */ -const ethers = require("ethers"); +/* global accounts */ +const { ethers, utils } = require("ethers"); const chai = require("chai"); const BN = require("bn.js"); const bnChai = require("bn-chai"); @@ -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,18 @@ 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 = utils.keccak256(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 = utils.keccak256("0x1234"); + const sig = await personalSign(signHash, owner); + const valid = await walletAsTransferManager.isValidSignature(signHash, sig); + assert.equal(valid, ERC1271_ISVALIDSIGNATURE_BYTES32); + }); + }); }); diff --git a/utils/utilities.js b/utils/utilities.js index 6f5a0407a..6823c54bf 100644 --- a/utils/utilities.js +++ b/utils/utilities.js @@ -135,4 +135,6 @@ module.exports = { 32, ); }, + + personalSign: async (signHash, signer) => ethers.utils.joinSignature(signer.signingKey.signDigest(signHash)), }; From 7bc1a9ac0e864f2f94eaa5587ed2c64c757297cd Mon Sep 17 00:00:00 2001 From: Olivier van den Biggelaar Date: Tue, 27 Oct 2020 12:00:13 +0000 Subject: [PATCH 30/47] test fix --- test/transferManager.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/transferManager.js b/test/transferManager.js index 77b779d97..546bfa5cb 100644 --- a/test/transferManager.js +++ b/test/transferManager.js @@ -1,5 +1,5 @@ -/* global accounts */ -const { ethers, utils } = require("ethers"); +/* global accounts, utils */ +const ethers = require("ethers"); const chai = require("chai"); const BN = require("bn.js"); const bnChai = require("bn-chai"); @@ -891,12 +891,12 @@ describe("TransferManager", function () { describe("Static calls", () => { it("should delegate isValidSignature static calls to the TransferManager", async () => { - const ERC1271_ISVALIDSIGNATURE_BYTES32 = utils.keccak256(utils.toUtf8Bytes("isValidSignature(bytes32,bytes)")).slice(0, 10); + 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 = utils.keccak256("0x1234"); + 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); From 59f6e3476ded333c2d67d44f8c652aef718223c2 Mon Sep 17 00:00:00 2001 From: Olivier van den Biggelaar Date: Tue, 27 Oct 2020 14:45:58 +0000 Subject: [PATCH 31/47] test fix (the implementation of a static call to the wallet can no longer include storage read) --- contracts-test/TestFeature.sol | 8 ++------ test/baseWallet.js | 1 - test/relayer.js | 4 ++-- test/versionManager.js | 1 - 4 files changed, 4 insertions(+), 10 deletions(-) diff --git a/contracts-test/TestFeature.sol b/contracts-test/TestFeature.sol index 554eaae3b..d247e6862 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(); } @@ -70,11 +66,11 @@ contract TestFeature is BaseFeature { } 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) { 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/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/versionManager.js b/test/versionManager.js index 2fad59d52..6e24698a7 100644 --- a/test/versionManager.js +++ b/test/versionManager.js @@ -59,7 +59,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); From 33bbb15499fa90c9fee547f2604f7864477b2d4a Mon Sep 17 00:00:00 2001 From: Olivier van den Biggelaar Date: Tue, 27 Oct 2020 16:30:30 +0000 Subject: [PATCH 32/47] test fix --- test/makerV2Manager_loan.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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([ From bf2c7d5ece419c642b6dd6868ea6ca698fcd6ae7 Mon Sep 17 00:00:00 2001 From: Olivier van den Biggelaar Date: Wed, 28 Oct 2020 12:56:26 +0000 Subject: [PATCH 33/47] Adding deployment script --- deployment/7_upgrade_2_1_fix.js | 186 ++++++++++++++++++++++++++++++++ 1 file changed, 186 insertions(+) create mode 100644 deployment/7_upgrade_2_1_fix.js diff --git a/deployment/7_upgrade_2_1_fix.js b/deployment/7_upgrade_2_1_fix.js new file mode 100644 index 000000000..9970cc275 --- /dev/null +++ b/deployment/7_upgrade_2_1_fix.js @@ -0,0 +1,186 @@ +const semver = require("semver"); +const childProcess = require("child_process"); +const MultiSig = require("../build/MultiSigWallet"); +const ModuleRegistry = require("../build/ModuleRegistry"); +const Upgrader = require("../build/UpgraderToVersionManager"); +const DeployManager = require("../utils/deploy-manager.js"); +const MultisigExecutor = require("../utils/multisigexecutor.js"); + +const VersionManager = require("../build/VersionManager"); + +const utils = require("../utils/utilities.js"); + +const TARGET_VERSION = "2.1.0"; +const MODULES_TO_ENABLE = [ + "VersionManager", +]; +const MODULES_TO_DISABLE = []; + +const BACKWARD_COMPATIBILITY = 4; + +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 { abiUploader } = 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 }); + + // ////////////////////////////////// + // Deploy new modules + // ////////////////////////////////// + + const VersionManagerWrapper = await deployer.deploy( + VersionManager, + {}, + config.contracts.ModuleRegistry, + config.modules.LockStorage, + config.modules.GuardianStorage, + config.modules.TransferStorage, + config.modules.LimitStorage, + ); + newModuleWrappers.push(VersionManagerWrapper); + + // Add Features to Version Manager + const features = [ + config.modules.GuardianManager, + config.modules.LockManager, + config.modules.RecoveryManager, + config.modules.ApprovedTransfer, + config.modules.TransferManager, + config.modules.TokenExchanger, + config.modules.NftTransfer, + config.modules.CompoundManager, + config.modules.MakerV2Manager, + config.modules.RelayerManager, + ]; + const featuresWithNoInit = [ // all features except the TransferManager + config.modules.GuardianManager, + config.modules.LockManager, + config.modules.RecoveryManager, + config.modules.ApprovedTransfer, + config.modules.TokenExchanger, + config.modules.NftTransfer, + config.modules.CompoundManager, + config.modules.MakerV2Manager, + config.modules.RelayerManager, + ]; + const featureToInit = features.filter((f) => !featuresWithNoInit.includes(f)); + const VersionManagerAddVersionTx = await VersionManagerWrapper.contract.addVersion(features, featureToInit, { gasPrice }); + await VersionManagerWrapper.verboseWaitForTransaction(VersionManagerAddVersionTx, "Adding New Version"); + + // ////////////////////////////////// + // Set contracts' owners + // ////////////////////////////////// + + const changeOwnerTx = await VersionManagerWrapper.contract.changeOwner(config.contracts.MultiSigWallet, { gasPrice }); + await VersionManagerWrapper.verboseWaitForTransaction(changeOwnerTx, "Set the MultiSig as the owner of VersionManagerWrapper"); + + // ///////////////////////////////////////////////// + // Update config and Upload ABIs + // ///////////////////////////////////////////////// + + configurator.updateModuleAddresses({ + VersionManager: VersionManagerWrapper.contractAddress, + }); + + const gitHash = childProcess.execSync("git rev-parse HEAD").toString("utf8").replace(/\n$/, ""); + configurator.updateGitHash(gitHash); + await configurator.save(); + + await Promise.all([ + abiUploader.upload(VersionManagerWrapper, "modules"), + ]); + + // ////////////////////////////////// + // 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}`; + + // 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, + lockStorage, + 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, +}; From eb0b88501faad164f5eab95081ff79a838f62b45 Mon Sep 17 00:00:00 2001 From: Olivier van den Biggelaar Date: Thu, 29 Oct 2020 10:48:01 +0000 Subject: [PATCH 34/47] Adding feature deployment --- deployment/7_upgrade_2_1_fix.js | 187 ++++++++++++++++++++++++++++---- 1 file changed, 163 insertions(+), 24 deletions(-) diff --git a/deployment/7_upgrade_2_1_fix.js b/deployment/7_upgrade_2_1_fix.js index 9970cc275..7ed6acdcf 100644 --- a/deployment/7_upgrade_2_1_fix.js +++ b/deployment/7_upgrade_2_1_fix.js @@ -6,7 +6,16 @@ const Upgrader = require("../build/UpgraderToVersionManager"); const DeployManager = require("../utils/deploy-manager.js"); const MultisigExecutor = require("../utils/multisigexecutor.js"); -const VersionManager = require("../build/VersionManager"); +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 utils = require("../utils/utilities.js"); @@ -62,32 +71,140 @@ const deploy = async (network) => { ); newModuleWrappers.push(VersionManagerWrapper); + // ////////////////////////////////// + // Deploy new features + // ////////////////////////////////// + const ApprovedTransferWrapper = await deployer.deploy( + ApprovedTransfer, + {}, + config.modules.LockStorage, + config.modules.GuardianStorage, + config.modules.LimitStorage, + VersionManagerWrapper.contractAddress, + config.defi.weth, + ); + + const CompoundManagerWrapper = await deployer.deploy( + CompoundManager, + {}, + config.modules.LockStorage, + config.defi.compound.comptroller, + config.contracts.CompoundRegistry, + VersionManagerWrapper.contractAddress, + ); + + const GuardianManagerWrapper = await deployer.deploy( + GuardianManager, + {}, + config.modules.LockStorage, + config.modules.GuardianStorage, + VersionManagerWrapper.contractAddress, + config.settings.securityPeriod || 0, + config.settings.securityWindow || 0, + ); + + const LockManagerWrapper = await deployer.deploy( + LockManager, + {}, + config.modules.LockStorage, + config.modules.GuardianStorage, + VersionManagerWrapper.contractAddress, + config.settings.lockPeriod || 0, + ); + + const NftTransferWrapper = await deployer.deploy( + NftTransfer, + {}, + config.modules.LockStorage, + config.modules.TokenPriceRegistry, + VersionManagerWrapper.contractAddress, + config.CryptoKitties.contract, + ); + + const RecoveryManagerWrapper = await deployer.deploy( + RecoveryManager, + {}, + config.modules.LockStorage, + config.modules.GuardianStorage, + VersionManagerWrapper.contractAddress, + config.settings.recoveryPeriod || 0, + config.settings.lockPeriod || 0, + ); + + const TokenExchangerWrapper = await deployer.deploy( + TokenExchanger, + {}, + config.modules.LockStorage, + config.modules.TokenPriceRegistry, + VersionManagerWrapper.contractAddress, + config.contracts.DexRegistry, + config.defi.paraswap.contract, + "argent", + ); + + const MakerV2ManagerWrapper = await deployer.deploy( + MakerV2Manager, + {}, + config.modules.LockStorage, + config.defi.maker.migration, + config.defi.maker.pot, + config.defi.maker.jug, + config.contracts.MakerRegistry, + config.defi.uniswap.factory, + VersionManagerWrapper.contractAddress, + ); + + const TransferManagerWrapper = await deployer.deploy( + TransferManager, + {}, + 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", + config.defi.weth, + ["test", "staging", "prod"].includes(network) ? config.modules.TransferManager : "0x0000000000000000000000000000000000000000", + ); + + const RelayerManagerWrapper = await deployer.deploy( + RelayerManager, + {}, + config.modules.LockStorage, + config.modules.GuardianStorage, + config.modules.LimitStorage, + config.modules.TokenPriceRegistry, + VersionManagerWrapper.contractAddress, + ); + // Add Features to Version Manager - const features = [ - config.modules.GuardianManager, - config.modules.LockManager, - config.modules.RecoveryManager, - config.modules.ApprovedTransfer, - config.modules.TransferManager, - config.modules.TokenExchanger, - config.modules.NftTransfer, - config.modules.CompoundManager, - config.modules.MakerV2Manager, - config.modules.RelayerManager, + const newFeatures = [ + 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 - config.modules.GuardianManager, - config.modules.LockManager, - config.modules.RecoveryManager, - config.modules.ApprovedTransfer, - config.modules.TokenExchanger, - config.modules.NftTransfer, - config.modules.CompoundManager, - config.modules.MakerV2Manager, - config.modules.RelayerManager, + const newFeaturesWithNoInit = [ // 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 }); + const newFeatureToInit = newFeatures.filter((f) => !newFeaturesWithNoInit.includes(f)); + const VersionManagerAddVersionTx = await VersionManagerWrapper.contract.addVersion(newFeatures, newFeatureToInit, { gasPrice }); await VersionManagerWrapper.verboseWaitForTransaction(VersionManagerAddVersionTx, "Adding New Version"); // ////////////////////////////////// @@ -102,6 +219,17 @@ const deploy = async (network) => { // ///////////////////////////////////////////////// configurator.updateModuleAddresses({ + ApprovedTransfer: ApprovedTransferWrapper.contractAddress, + CompoundManager: CompoundManagerWrapper.contractAddress, + GuardianManager: GuardianManagerWrapper.contractAddress, + LockManager: LockManagerWrapper.contractAddress, + NftTransfer: NftTransferWrapper.contractAddress, + RecoveryManager: RecoveryManagerWrapper.contractAddress, + TokenExchanger: TokenExchangerWrapper.contractAddress, + MakerV2Manager: MakerV2ManagerWrapper.contractAddress, + TransferManager: TransferManagerWrapper.contractAddress, + RelayerManager: RelayerManagerWrapper.contractAddress, + VersionManager: VersionManagerWrapper.contractAddress, }); @@ -110,6 +238,17 @@ const deploy = async (network) => { await configurator.save(); await Promise.all([ + abiUploader.upload(ApprovedTransferWrapper, "modules"), + abiUploader.upload(CompoundManagerWrapper, "modules"), + abiUploader.upload(GuardianManagerWrapper, "modules"), + abiUploader.upload(LockManagerWrapper, "modules"), + abiUploader.upload(NftTransferWrapper, "modules"), + abiUploader.upload(RecoveryManagerWrapper, "modules"), + abiUploader.upload(TokenExchangerWrapper, "modules"), + abiUploader.upload(MakerV2ManagerWrapper, "modules"), + abiUploader.upload(TransferManagerWrapper, "modules"), + abiUploader.upload(RelayerManagerWrapper, "modules"), + abiUploader.upload(VersionManagerWrapper, "modules"), ]); From bd124a7178a76cbb83ee31d4cf49f406ebe1a63a Mon Sep 17 00:00:00 2001 From: Olivier van den Biggelaar Date: Fri, 30 Oct 2020 14:50:49 +0000 Subject: [PATCH 35/47] increasing coverage for isValidSignature() --- test/transferManager.js | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/test/transferManager.js b/test/transferManager.js index 546bfa5cb..cfc96e6d0 100644 --- a/test/transferManager.js +++ b/test/transferManager.js @@ -901,5 +901,23 @@ describe("TransferManager", function () { 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", + ); + }); }); }); From c9a37f229b55c3320fdd6ee6097c190eee4055ed Mon Sep 17 00:00:00 2001 From: Olivier van den Biggelaar Date: Fri, 30 Oct 2020 14:51:35 +0000 Subject: [PATCH 36/47] adding test for onERC721Received() --- test/nftTransfer.js | 18 +++++++++++++++--- utils/utilities.js | 9 +++++++++ 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/test/nftTransfer.js b/test/nftTransfer.js index bf48155c1..ebb6dd6fa 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, @@ -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/utils/utilities.js b/utils/utilities.js index 6823c54bf..fc07ce66c 100644 --- a/utils/utilities.js +++ b/utils/utilities.js @@ -137,4 +137,13 @@ module.exports = { }, 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); + }, }; From f7c77bd732de4f418a31c632b738ffb13bd85b63 Mon Sep 17 00:00:00 2001 From: Olivier van den Biggelaar Date: Fri, 30 Oct 2020 17:54:32 +0000 Subject: [PATCH 37/47] Fixing deploy script --- deployment/7_upgrade_2_1_fix.js | 1 + 1 file changed, 1 insertion(+) diff --git a/deployment/7_upgrade_2_1_fix.js b/deployment/7_upgrade_2_1_fix.js index 7ed6acdcf..50cc19055 100644 --- a/deployment/7_upgrade_2_1_fix.js +++ b/deployment/7_upgrade_2_1_fix.js @@ -16,6 +16,7 @@ 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"); From 0216648bce51d59ebbf21c619e26da7160c4facc Mon Sep 17 00:00:00 2001 From: Olivier van den Biggelaar Date: Mon, 2 Nov 2020 09:38:17 +0000 Subject: [PATCH 38/47] deploying 2.2.0 and readding script 7 to test:deployment --- deployment/{7_upgrade_2_1_fix.js => 7_upgrade_2_2.js} | 2 +- package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) rename deployment/{7_upgrade_2_1_fix.js => 7_upgrade_2_2.js} (99%) diff --git a/deployment/7_upgrade_2_1_fix.js b/deployment/7_upgrade_2_2.js similarity index 99% rename from deployment/7_upgrade_2_1_fix.js rename to deployment/7_upgrade_2_2.js index 50cc19055..bfecf65d7 100644 --- a/deployment/7_upgrade_2_1_fix.js +++ b/deployment/7_upgrade_2_2.js @@ -20,7 +20,7 @@ const VersionManager = require("../build/VersionManager"); const utils = require("../utils/utilities.js"); -const TARGET_VERSION = "2.1.0"; +const TARGET_VERSION = "2.2.0"; const MODULES_TO_ENABLE = [ "VersionManager", ]; diff --git a/package.json b/package.json index 83db3a46f..8889bd1cc 100644 --- a/package.json +++ b/package.json @@ -27,7 +27,7 @@ "test:coverage": "bash ./scripts/provision_lib_artefacts.sh & npx etherlime coverage && istanbul check-coverage --statements 94 --branches 83 --functions 96 --lines 94", "lint:js": "eslint .", "lint:contracts": "npx solhint contracts/**/*.sol && npx solhint contracts/**/**/*.sol", - "test:deployment": "./scripts/deploy.sh ganache `seq 1 6`", + "test:deployment": "./scripts/deploy.sh ganache `seq 1 7`", "test:benchmark": "./scripts/deploy.sh ganache 999", "security:slither": "npm run security:slither:infrastructure && npm run security:slither:modules", "security:slither:infrastructure": "rm -rf build && npm run compile:infrastructure && slither . --ignore-compile --filter-paths lib --solc-disable-warnings --exclude-low --exclude-informational --exclude=naming-convention,unused-state-variables,solc-version,assembly-usage,low-level-calls,public-function-that-could-be-declared-as-external", From 5534a6754ed870b99aa8f936eae108ea27e67679 Mon Sep 17 00:00:00 2001 From: elenadimitrova Date: Tue, 3 Nov 2020 16:27:20 +0200 Subject: [PATCH 39/47] Independently compile contracts with ABIEncoderV2 due to https://github.com/ethereum/solidity/issues/9573 --- .../{ => ApprovedTransfer}/ApprovedTransfer.sol | 10 +++++----- .../{ => RelayerManager}/RelayerManager.sol | 14 +++++++------- .../{ => TokenExchanger}/TokenExchanger.sol | 10 +++++----- .../{ => TransferManager}/TransferManager.sol | 14 +++++++------- package.json | 2 +- 5 files changed, 25 insertions(+), 25 deletions(-) rename contracts/modules/{ => ApprovedTransfer}/ApprovedTransfer.sol (96%) rename contracts/modules/{ => RelayerManager}/RelayerManager.sol (97%) rename contracts/modules/{ => TokenExchanger}/TokenExchanger.sol (98%) rename contracts/modules/{ => TransferManager}/TransferManager.sol (98%) 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/package.json b/package.json index 8889bd1cc..82ba2c15d 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 && npx etherlime compile --workingDirectory contracts/modules/ApprovedTransfer --runs=999 && npx etherlime compile --workingDirectory contracts/modules/RelayerManager --runs=999 && npx etherlime compile --workingDirectory contracts/modules/TransferManager --runs=999 && 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", From d562b2a6d39a75b3730d34372ee6d9869be2e9ac Mon Sep 17 00:00:00 2001 From: elenadimitrova Date: Wed, 4 Nov 2020 12:24:48 +0200 Subject: [PATCH 40/47] Remove contract artifacts before recompiling them --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 82ba2c15d..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 && npx etherlime compile --workingDirectory contracts/modules/ApprovedTransfer --runs=999 && npx etherlime compile --workingDirectory contracts/modules/RelayerManager --runs=999 && npx etherlime compile --workingDirectory contracts/modules/TransferManager --runs=999 && npx etherlime compile --workingDirectory contracts/modules/TokenExchanger --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", From 65995f0b5f3c3c2b07c3bec9155df907011a5c8f Mon Sep 17 00:00:00 2001 From: Olivier van den Biggelaar Date: Mon, 9 Nov 2020 16:25:37 +0000 Subject: [PATCH 41/47] Preventing non-static calls in VersionManager's fallback --- contracts-test/TestFeature.sol | 7 +- contracts/modules/VersionManager.sol | 21 +++++ .../modules/common/StaticDelegateCaller.sol | 82 +++++++++++++++++++ test/versionManager.js | 54 +++++++++++- 4 files changed, 162 insertions(+), 2 deletions(-) create mode 100644 contracts/modules/common/StaticDelegateCaller.sol diff --git a/contracts-test/TestFeature.sol b/contracts-test/TestFeature.sol index d247e6862..720d72b88 100644 --- a/contracts-test/TestFeature.sol +++ b/contracts-test/TestFeature.sol @@ -59,10 +59,11 @@ 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) { @@ -77,6 +78,10 @@ contract TestFeature is BaseFeature { return _addr; } + function badStaticCall() external { + uintVal = 123456; + } + function callDapp(address _wallet) external { diff --git a/contracts/modules/VersionManager.sol b/contracts/modules/VersionManager.sol index 977b47c39..a090c007b 100644 --- a/contracts/modules/VersionManager.sol +++ b/contracts/modules/VersionManager.sol @@ -175,6 +175,26 @@ 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 verifyIsInStaticCall() public { + if(msg.sender != address(this)) { // first entry in the method (via an internal call) + (bool success,) = address(this).call(abi.encodeWithSelector(VersionManager(0).verifyIsInStaticCall.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 { + sstore(47474747, 1) + } + } + } + /** * @notice This method delegates the static call to a target feature */ @@ -182,6 +202,7 @@ 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"); + verifyIsInStaticCall(); // solhint-disable-next-line no-inline-assembly assembly { diff --git a/contracts/modules/common/StaticDelegateCaller.sol b/contracts/modules/common/StaticDelegateCaller.sol new file mode 100644 index 000000000..8cde31e60 --- /dev/null +++ b/contracts/modules/common/StaticDelegateCaller.sol @@ -0,0 +1,82 @@ +// 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; + + +/** + * @title StaticDelegateCaller + * @notice An abstract utility class providing the ability to perform "static" delegatecalls to a subclass. + * Based on https://github.com/gnosis/util-contracts/pull/31/files + * @author Olivier VDB + */ +abstract contract StaticDelegateCaller { + + bool private unlocked; + + bytes4 constant internal DO_DELEGATECALL_AND_REVERT_SELECTOR = bytes4(keccak256("doDelegateCallAndRevert(address,bytes)")); + + /* ***************** External Methods ************************* */ + + /** + * @dev Performs a delegetecall on a target contract in the context of self. + * Internally reverts execution to avoid side effects (making it static). Returns encoded result as revert message + * concatenated with the success flag of the inner call as a last byte. + * @param target address of the contract containing the code to execute. + * @param data calldata that should be sent to the target contract (encoded method name and arguments). + */ + function doDelegateCallAndRevert(address target, bytes memory data) external returns (bytes memory) { + // require(msg.sender == address(this), "SDC: sender should be this"); + // require(unlocked, "SDC: should be called via doStaticDelegatecall()"); + (bool success, bytes memory response) = target.delegatecall(data); + bytes memory packed = abi.encodePacked(response, success); + // solhint-disable-next-line no-inline-assembly + assembly { + revert(add(packed, 0x20), mload(packed)) // return the packed response without appending sig("Error(string)") to them + } + } + + /* ***************** Internal Methods ************************* */ + + /** + * @dev Performs a delegetecall on a target contract in the context of self. + * Internally reverts execution to avoid side effects (making it static). Catches revert and returns encoded result as bytes. + * @param target address of the contract containing the code to execute. + * @param data calldata that should be sent to the target contract (encoded method name and arguments). + */ + function doStaticDelegatecall( + address target, + bytes memory data + ) internal returns (bytes memory) { + bytes memory innerCall = abi.encodeWithSelector(DO_DELEGATECALL_AND_REVERT_SELECTOR, target, data); + // unlocked = true; + (, bytes memory response) = address(this).delegatecall(innerCall); + // unlocked = false; + bool innerSuccess = response[response.length - 1] == 0x01; + // solhint-disable-next-line no-inline-assembly + assembly { + mstore(response, sub(mload(response), 1)) // popping the success flag off the response + } + if (innerSuccess) { + return response; + } else { + // solhint-disable-next-line no-inline-assembly + assembly { + revert(add(response, 0x20), mload(response)) // return the raw error bytes without appending sig("Error(string)") to them + } + } + } +} \ No newline at end of file diff --git a/test/versionManager.js b/test/versionManager.js index 6e24698a7..2af872336 100644 --- a/test/versionManager.js +++ b/test/versionManager.js @@ -1,5 +1,7 @@ /* global accounts */ const ethers = require("ethers"); + +const { AddressZero } = ethers.constants; const GuardianManager = require("../build/GuardianManager"); const LockStorage = require("../build/LockStorage"); const GuardianStorage = require("../build/GuardianStorage"); @@ -7,8 +9,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 +29,7 @@ describe("VersionManager", function () { let deployer; let wallet; let walletImplementation; + let registry; let lockStorage; let guardianStorage; let guardianManager; @@ -35,7 +43,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, {}, @@ -141,5 +149,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, + AddressZero, + 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", + ); + }); }); }); From 564e96dafec82962f7f020a9be324ab39ac0f728 Mon Sep 17 00:00:00 2001 From: Olivier van den Biggelaar Date: Mon, 9 Nov 2020 17:27:28 +0000 Subject: [PATCH 42/47] Using log0 instead of sstore as non-static test operation --- contracts/modules/VersionManager.sol | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/contracts/modules/VersionManager.sol b/contracts/modules/VersionManager.sol index a090c007b..069ab8992 100644 --- a/contracts/modules/VersionManager.sol +++ b/contracts/modules/VersionManager.sol @@ -189,9 +189,7 @@ contract VersionManager is IVersionManager, IModule, BaseFeature, Owned { 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 { - sstore(47474747, 1) - } + assembly { log0(0, 0) } } } From 44f5d2ba5e4e88fc0e87c1399fd52a4fd7d40769 Mon Sep 17 00:00:00 2001 From: Olivier van den Biggelaar Date: Mon, 9 Nov 2020 17:29:48 +0000 Subject: [PATCH 43/47] Deleting unused file --- .../modules/common/StaticDelegateCaller.sol | 82 ------------------- 1 file changed, 82 deletions(-) delete mode 100644 contracts/modules/common/StaticDelegateCaller.sol diff --git a/contracts/modules/common/StaticDelegateCaller.sol b/contracts/modules/common/StaticDelegateCaller.sol deleted file mode 100644 index 8cde31e60..000000000 --- a/contracts/modules/common/StaticDelegateCaller.sol +++ /dev/null @@ -1,82 +0,0 @@ -// 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; - - -/** - * @title StaticDelegateCaller - * @notice An abstract utility class providing the ability to perform "static" delegatecalls to a subclass. - * Based on https://github.com/gnosis/util-contracts/pull/31/files - * @author Olivier VDB - */ -abstract contract StaticDelegateCaller { - - bool private unlocked; - - bytes4 constant internal DO_DELEGATECALL_AND_REVERT_SELECTOR = bytes4(keccak256("doDelegateCallAndRevert(address,bytes)")); - - /* ***************** External Methods ************************* */ - - /** - * @dev Performs a delegetecall on a target contract in the context of self. - * Internally reverts execution to avoid side effects (making it static). Returns encoded result as revert message - * concatenated with the success flag of the inner call as a last byte. - * @param target address of the contract containing the code to execute. - * @param data calldata that should be sent to the target contract (encoded method name and arguments). - */ - function doDelegateCallAndRevert(address target, bytes memory data) external returns (bytes memory) { - // require(msg.sender == address(this), "SDC: sender should be this"); - // require(unlocked, "SDC: should be called via doStaticDelegatecall()"); - (bool success, bytes memory response) = target.delegatecall(data); - bytes memory packed = abi.encodePacked(response, success); - // solhint-disable-next-line no-inline-assembly - assembly { - revert(add(packed, 0x20), mload(packed)) // return the packed response without appending sig("Error(string)") to them - } - } - - /* ***************** Internal Methods ************************* */ - - /** - * @dev Performs a delegetecall on a target contract in the context of self. - * Internally reverts execution to avoid side effects (making it static). Catches revert and returns encoded result as bytes. - * @param target address of the contract containing the code to execute. - * @param data calldata that should be sent to the target contract (encoded method name and arguments). - */ - function doStaticDelegatecall( - address target, - bytes memory data - ) internal returns (bytes memory) { - bytes memory innerCall = abi.encodeWithSelector(DO_DELEGATECALL_AND_REVERT_SELECTOR, target, data); - // unlocked = true; - (, bytes memory response) = address(this).delegatecall(innerCall); - // unlocked = false; - bool innerSuccess = response[response.length - 1] == 0x01; - // solhint-disable-next-line no-inline-assembly - assembly { - mstore(response, sub(mload(response), 1)) // popping the success flag off the response - } - if (innerSuccess) { - return response; - } else { - // solhint-disable-next-line no-inline-assembly - assembly { - revert(add(response, 0x20), mload(response)) // return the raw error bytes without appending sig("Error(string)") to them - } - } - } -} \ No newline at end of file From 1ea89005f722d82e8827d025d8f8766081e6eea0 Mon Sep 17 00:00:00 2001 From: Olivier van den Biggelaar Date: Mon, 9 Nov 2020 20:50:53 +0000 Subject: [PATCH 44/47] Fixing tests --- test/nftTransfer.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/nftTransfer.js b/test/nftTransfer.js index ebb6dd6fa..66cb72618 100644 --- a/test/nftTransfer.js +++ b/test/nftTransfer.js @@ -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 { From f8cf36defe4bf5f4a9118a36c4b3c01007e0f37e Mon Sep 17 00:00:00 2001 From: Olivier van den Biggelaar Date: Mon, 9 Nov 2020 21:20:38 +0000 Subject: [PATCH 45/47] test cleanup --- test/versionManager.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/test/versionManager.js b/test/versionManager.js index 2af872336..c2bfc5043 100644 --- a/test/versionManager.js +++ b/test/versionManager.js @@ -1,7 +1,6 @@ /* global accounts */ const ethers = require("ethers"); -const { AddressZero } = ethers.constants; const GuardianManager = require("../build/GuardianManager"); const LockStorage = require("../build/LockStorage"); const GuardianStorage = require("../build/GuardianStorage"); @@ -170,8 +169,8 @@ describe("VersionManager", function () { 3600, 3600, 10000, - AddressZero, - AddressZero); + ethers.constants.AddressZero, + ethers.constants.AddressZero); await versionManager2.addVersion([transferManager.contractAddress], []); await registry.registerModule(versionManager2.contractAddress, ethers.utils.formatBytes32String("VersionManager2")); From 156917a6b3c784c902bc7eadc3f9ac01108c7445 Mon Sep 17 00:00:00 2001 From: Olivier van den Biggelaar Date: Tue, 10 Nov 2020 11:16:55 +0000 Subject: [PATCH 46/47] Limiting the gas passed to subcall in verifyStaticCall --- contracts/modules/VersionManager.sol | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/contracts/modules/VersionManager.sol b/contracts/modules/VersionManager.sol index 069ab8992..26a11b894 100644 --- a/contracts/modules/VersionManager.sol +++ b/contracts/modules/VersionManager.sol @@ -183,9 +183,9 @@ contract VersionManager is IVersionManager, IModule, BaseFeature, Owned { * and reverts otherwise. * @dev The use of an if/else allows to encapsulate the whole logic in a single function. */ - function verifyIsInStaticCall() public { + function verifyStaticCall() public { if(msg.sender != address(this)) { // first entry in the method (via an internal call) - (bool success,) = address(this).call(abi.encodeWithSelector(VersionManager(0).verifyIsInStaticCall.selector)); + (bool success,) = address(this).call{gas: 1000}(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 @@ -200,7 +200,7 @@ 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"); - verifyIsInStaticCall(); + verifyStaticCall(); // solhint-disable-next-line no-inline-assembly assembly { From d6c3e07e80c83cae0f531b727479fa826f0a112b Mon Sep 17 00:00:00 2001 From: Olivier van den Biggelaar Date: Tue, 10 Nov 2020 14:56:17 +0000 Subject: [PATCH 47/47] Increasing gas stipend of verifyStaticCall()'s subcall --- contracts/modules/VersionManager.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/modules/VersionManager.sol b/contracts/modules/VersionManager.sol index 26a11b894..2f1a77026 100644 --- a/contracts/modules/VersionManager.sol +++ b/contracts/modules/VersionManager.sol @@ -185,7 +185,7 @@ contract VersionManager is IVersionManager, IModule, BaseFeature, Owned { */ function verifyStaticCall() public { if(msg.sender != address(this)) { // first entry in the method (via an internal call) - (bool success,) = address(this).call{gas: 1000}(abi.encodeWithSelector(VersionManager(0).verifyStaticCall.selector)); + (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