diff --git a/contracts/wallet/WalletFactory.sol b/contracts/wallet/WalletFactory.sol index a515ff2a2..ef8d5cd04 100644 --- a/contracts/wallet/WalletFactory.sol +++ b/contracts/wallet/WalletFactory.sol @@ -44,95 +44,50 @@ contract WalletFactory is Owned, Managed { event ModuleRegistryChanged(address addr); event ENSManagerChanged(address addr); - event GuardianStorageChanged(address addr); event WalletCreated(address indexed wallet, address indexed owner, address indexed guardian); - // *************** Modifiers *************************** // - - /** - * @dev Throws if the guardian storage address is not set. - */ - modifier guardianStorageDefined { - require(guardianStorage != address(0), "IGuardianStorage address not defined"); - _; - } - // *************** Constructor ********************** // /** * @dev Default constructor. */ - constructor(address _moduleRegistry, address _walletImplementation, address _ensManager) public { + constructor(address _moduleRegistry, address _walletImplementation, address _ensManager, address _guardianStorage) public { + require(_moduleRegistry != address(0), "WF: ModuleRegistry address not defined"); + require(_walletImplementation != address(0), "WF: WalletImplementation address not defined"); + require(_ensManager != address(0), "WF: ENSManager address not defined"); + require(_guardianStorage != address(0), "WF: GuardianStorage address not defined"); moduleRegistry = _moduleRegistry; walletImplementation = _walletImplementation; ensManager = _ensManager; + guardianStorage = _guardianStorage; } // *************** External Functions ********************* // - /** * @dev Lets the manager create a wallet for an owner account. - * The wallet is initialised with a list of modules and an ENS.. - * The wallet is created using the CREATE opcode. - * @param _owner The account address. - * @param _modules The list of modules. - * @param _label ENS label of the new wallet, e.g. franck. - */ - function createWallet( - address _owner, - address[] calldata _modules, - string calldata _label - ) - external - onlyManager - { - _createWallet(_owner, _modules, _label, address(0)); - } - - /** - * @dev Lets the manager create a wallet for an owner account. - * The wallet is initialised with a list of modules, a first guardian, and an ENS.. + * The wallet is initialised with a list of modules, a first guardian, and an ENS. * The wallet is created using the CREATE opcode. * @param _owner The account address. * @param _modules The list of modules. * @param _label ENS label of the new wallet, e.g. franck. * @param _guardian The guardian address. */ - function createWalletWithGuardian( + function createWallet( address _owner, address[] calldata _modules, string calldata _label, address _guardian - ) - external - onlyManager - guardianStorageDefined - { - require(_guardian != (address(0)), "WF: guardian cannot be null"); - _createWallet(_owner, _modules, _label, _guardian); - } - - /** - * @dev Lets the manager create a wallet for an owner account at a specific address. - * The wallet is initialised with a list of modules and an ENS. - * The wallet is created using the CREATE2 opcode. - * @param _owner The account address. - * @param _modules The list of modules. - * @param _label ENS label of the new wallet, e.g. franck. - * @param _salt The salt. - */ - function createCounterfactualWallet( - address _owner, - address[] calldata _modules, - string calldata _label, - bytes32 _salt ) external onlyManager { - _createCounterfactualWallet(_owner, _modules, _label, address(0), _salt); + validateInputs(_owner, _modules, _guardian); + validateENSLabel(_label); + Proxy proxy = new Proxy(walletImplementation); + address payable wallet = address(proxy); + configureWallet(BaseWallet(wallet), _owner, _modules, _label, _guardian); } - + /** * @dev Lets the manager create a wallet for an owner account at a specific address. * The wallet is initialised with a list of modules, a first guardian, and an ENS. @@ -143,7 +98,7 @@ contract WalletFactory is Owned, Managed { * @param _guardian The guardian address. * @param _salt The salt. */ - function createCounterfactualWalletWithGuardian( + function createCounterfactualWallet( address _owner, address[] calldata _modules, string calldata _label, @@ -152,29 +107,16 @@ contract WalletFactory is Owned, Managed { ) external onlyManager - guardianStorageDefined - { - require(_guardian != (address(0)), "WF: guardian cannot be null"); - _createCounterfactualWallet(_owner, _modules, _label, _guardian, _salt); - } - - /** - * @dev Gets the address of a counterfactual wallet. - * @param _owner The account address. - * @param _modules The list of modules. - * @param _salt The salt. - * @return _wallet The address that the wallet will have when created using CREATE2 and the same input parameters. - */ - function getAddressForCounterfactualWallet( - address _owner, - address[] calldata _modules, - bytes32 _salt - ) - external - view returns (address _wallet) { - _wallet = _getAddressForCounterfactualWallet(_owner, _modules, address(0), _salt); + validateInputs(_owner, _modules, _guardian); + validateENSLabel(_label); + + bytes32 newsalt = newSalt(_salt, _owner, _modules, _guardian); + Proxy proxy = new Proxy{salt: newsalt}(walletImplementation); + address payable wallet = address(proxy); + configureWallet(BaseWallet(wallet), _owner, _modules, _label, _guardian); + return wallet; } /** @@ -185,7 +127,7 @@ contract WalletFactory is Owned, Managed { * @param _salt The salt. * @return _wallet The address that the wallet will have when created using CREATE2 and the same input parameters. */ - function getAddressForCounterfactualWalletWithGuardian( + function getAddressForCounterfactualWallet( address _owner, address[] calldata _modules, address _guardian, @@ -195,8 +137,11 @@ contract WalletFactory is Owned, Managed { view returns (address _wallet) { - require(_guardian != (address(0)), "WF: guardian cannot be null"); - _wallet = _getAddressForCounterfactualWallet(_owner, _modules, _guardian, _salt); + validateInputs(_owner, _modules, _guardian); + bytes32 newsalt = newSalt(_salt, _owner, _modules, _guardian); + bytes memory code = abi.encodePacked(type(Proxy).creationCode, uint256(walletImplementation)); + bytes32 hash = keccak256(abi.encodePacked(bytes1(0xff), address(this), newsalt, keccak256(code))); + _wallet = address(uint160(uint256(hash))); } /** @@ -219,16 +164,6 @@ contract WalletFactory is Owned, Managed { emit ENSManagerChanged(_ensManager); } - /** - * @dev Lets the owner change the address of the IGuardianStorage contract. - * @param _guardianStorage The address of the IGuardianStorage contract. - */ - function changeGuardianStorage(address _guardianStorage) external onlyOwner { - require(_guardianStorage != address(0), "WF: address cannot be null"); - guardianStorage = _guardianStorage; - emit GuardianStorageChanged(_guardianStorage); - } - /** * @dev Inits the module for a wallet by logging an event. * The method can only be called by the wallet itself. @@ -240,58 +175,15 @@ contract WalletFactory is Owned, Managed { // *************** Internal Functions ********************* // - /** - * @dev Helper method to create a wallet for an owner account. - * The wallet is initialised with a list of modules, a first guardian, and an ENS. - * The wallet is created using the CREATE opcode. - * @param _owner The account address. - * @param _modules The list of modules. - * @param _label ENS label of the new wallet, e.g. franck. - * @param _guardian (Optional) The guardian address. - */ - function _createWallet(address _owner, address[] memory _modules, string memory _label, address _guardian) internal { - _validateInputs(_owner, _modules, _label); - Proxy proxy = new Proxy(walletImplementation); - address payable wallet = address(proxy); - _configureWallet(BaseWallet(wallet), _owner, _modules, _label, _guardian); - } - - /** - * @dev Helper method to create a wallet for an owner account at a specific address. - * The wallet is initialised with a list of modules, a first guardian, and an ENS. - * The wallet is created using the CREATE2 opcode. - * @param _owner The account address. - * @param _modules The list of modules. - * @param _label ENS label of the new wallet, e.g. franck. - * @param _guardian The guardian address. - * @param _salt The salt. - */ - function _createCounterfactualWallet( - address _owner, - address[] memory _modules, - string memory _label, - address _guardian, - bytes32 _salt - ) - internal - { - _validateInputs(_owner, _modules, _label); - bytes32 newsalt = _newSalt(_salt, _owner, _modules, _guardian); - - Proxy proxy = new Proxy{salt: newsalt}(walletImplementation); - address payable wallet = address(proxy); - _configureWallet(BaseWallet(wallet), _owner, _modules, _label, _guardian); - } - /** * @dev Helper method to configure a wallet for a set of input parameters. * @param _wallet The target wallet * @param _owner The account address. * @param _modules The list of modules. * @param _label ENS label of the new wallet, e.g. franck. - * @param _guardian (Optional) The guardian address. + * @param _guardian The guardian address. */ - function _configureWallet( + function configureWallet( BaseWallet _wallet, address _owner, address[] memory _modules, @@ -308,42 +200,17 @@ contract WalletFactory is Owned, Managed { } // initialise the wallet with the owner and the extended modules _wallet.init(_owner, extendedModules); - // add guardian if needed - if (_guardian != address(0)) { - IGuardianStorage(guardianStorage).addGuardian(address(_wallet), _guardian); - } + // add guardian + IGuardianStorage(guardianStorage).addGuardian(address(_wallet), _guardian); + // register ENS - _registerWalletENS(address(_wallet), _label); + registerWalletENS(address(_wallet), _label); // remove the factory from the authorised modules _wallet.authoriseModule(address(this), false); // emit event emit WalletCreated(address(_wallet), _owner, _guardian); } - /** - * @dev Gets the address of a counterfactual wallet. - * @param _owner The account address. - * @param _modules The list of modules. - * @param _salt The salt. - * @param _guardian (Optional) The guardian address. - * @return _wallet The address that the wallet will have when created using CREATE2 and the same input parameters. - */ - function _getAddressForCounterfactualWallet( - address _owner, - address[] memory _modules, - address _guardian, - bytes32 _salt - ) - internal - view - returns (address _wallet) - { - bytes32 newsalt = _newSalt(_salt, _owner, _modules, _guardian); - bytes memory code = abi.encodePacked(type(Proxy).creationCode, uint256(walletImplementation)); - bytes32 hash = keccak256(abi.encodePacked(bytes1(0xff), address(this), newsalt, keccak256(code))); - _wallet = address(uint160(uint256(hash))); - } - /** * @dev Generates a new salt based on a provided salt, an owner, a list of modules and an optional guardian. * @param _salt The slat provided. @@ -351,12 +218,8 @@ contract WalletFactory is Owned, Managed { * @param _modules The list of modules. * @param _guardian The guardian address. */ - function _newSalt(bytes32 _salt, address _owner, address[] memory _modules, address _guardian) internal pure returns (bytes32) { - if (_guardian == address(0)) { - return keccak256(abi.encodePacked(_salt, _owner, _modules)); - } else { - return keccak256(abi.encodePacked(_salt, _owner, _modules, _guardian)); - } + function newSalt(bytes32 _salt, address _owner, address[] memory _modules, address _guardian) internal pure returns (bytes32) { + return keccak256(abi.encodePacked(_salt, _owner, _modules, _guardian)); } /** @@ -364,10 +227,14 @@ contract WalletFactory is Owned, Managed { * @param _owner The owner address. * @param _modules The list of modules. */ - function _validateInputs(address _owner, address[] memory _modules, string memory _label) internal view { + function validateInputs(address _owner, address[] memory _modules, address _guardian) internal view { require(_owner != address(0), "WF: owner cannot be null"); require(_modules.length > 0, "WF: cannot assign with less than 1 module"); require(IModuleRegistry(moduleRegistry).isRegisteredModule(_modules), "WF: one or more modules are not registered"); + require(_guardian != (address(0)), "WF: guardian cannot be null"); + } + + function validateENSLabel(string memory _label) internal pure { bytes memory labelBytes = bytes(_label); require(labelBytes.length != 0, "WF: ENS label must be defined"); } @@ -377,7 +244,7 @@ contract WalletFactory is Owned, Managed { * @param _wallet The wallet address. * @param _label ENS label of the new wallet (e.g. franck). */ - function _registerWalletENS(address payable _wallet, string memory _label) internal { + function registerWalletENS(address payable _wallet, string memory _label) internal { // claim reverse address ensResolver = IENSManager(ensManager).ensResolver(); bytes memory methodData = abi.encodeWithSignature("claimWithResolver(address,address)", ensManager, ensResolver); diff --git a/deployment/2_deploy_contracts.js b/deployment/2_deploy_contracts.js index 2b13c6522..643a1e6a7 100644 --- a/deployment/2_deploy_contracts.js +++ b/deployment/2_deploy_contracts.js @@ -5,7 +5,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/WalletFactory"); +const WalletFactory = require("../build-legacy/v1.6.0/WalletFactory"); const TokenPriceProvider = require("../build-legacy/v1.6.0/TokenPriceProvider"); const MakerRegistry = require("../build/MakerRegistry"); const ScdMcdMigration = require("../build/ScdMcdMigration"); diff --git a/deployment/3_setup_contracts.js b/deployment/3_setup_contracts.js index 1f396b0c2..bf67a956f 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/WalletFactory"); +const WalletFactory = require("../build-legacy/v1.6.0/WalletFactory"); const TokenPriceProvider = require("../build-legacy/v1.6.0/TokenPriceProvider"); const CompoundRegistry = require("../build/CompoundRegistry"); diff --git a/deployment/7_upgrade_2_0.js b/deployment/7_upgrade_2_0.js index ecce0f5ba..9f8b63599 100644 --- a/deployment/7_upgrade_2_0.js +++ b/deployment/7_upgrade_2_0.js @@ -78,7 +78,7 @@ const deploy = async (network) => { const BaseWalletWrapper = await deployer.deploy(BaseWallet); // Deploy the Wallet Factory const WalletFactoryWrapper = await deployer.deploy(WalletFactory, {}, - ModuleRegistryWrapper.contractAddress, BaseWalletWrapper.contractAddress, ENSManagerWrapper.contractAddress); + ModuleRegistryWrapper.contractAddress, BaseWalletWrapper.contractAddress, ENSManagerWrapper.contractAddress, config.modules.GuardianStorage); // Deploy the new LimitStorage const LimitStorageWrapper = await deployer.deploy(LimitStorage); // Deploy the new TokenPriceStorage diff --git a/package.json b/package.json index 1075edcef..4536a34ed 100644 --- a/package.json +++ b/package.json @@ -24,7 +24,7 @@ "test": "npx etherlime test --skip-compilation", "ctest": "npm run compile && npm run test", "provision:lib:artefacts": "bash ./scripts/provision_lib_artefacts.sh", - "test:coverage": "bash ./scripts/provision_lib_artefacts.sh & npx etherlime coverage && istanbul check-coverage --statements 92 --branches 81 --functions 93 --lines 92", + "test:coverage": "bash ./scripts/provision_lib_artefacts.sh & npx etherlime coverage && istanbul check-coverage --statements 94 --branches 83 --functions 96 --lines 94", "lint:contracts": "npx ethlint --dir .", "lint:contracts:staged": "bash ./scripts/ethlint.sh", "test:deployment": "./scripts/deploy.sh ganache `seq 1 7`", diff --git a/test/factory.js b/test/factory.js index 712ac3de4..06ca0dd04 100644 --- a/test/factory.js +++ b/test/factory.js @@ -45,7 +45,8 @@ describe("Wallet Factory", function () { let moduleRegistry; let guardianStorage; let factory; - let factoryWithoutGuardianStorage; + let module1; + let module2; before(async () => { deployer = manager.newDeployer(); @@ -69,30 +70,17 @@ describe("Wallet Factory", function () { ); implementation = await deployer.deploy(BaseWallet); - moduleRegistry = await deployer.deploy(ModuleRegistry); - guardianStorage = await deployer.deploy(GuardianStorage); - factory = await deployer.deploy(Factory, {}, moduleRegistry.contractAddress, implementation.contractAddress, - ensManager.contractAddress); + ensManager.contractAddress, + guardianStorage.contractAddress); await factory.addManager(infrastructure.address); - await factory.changeGuardianStorage(guardianStorage.contractAddress); await ensManager.addManager(factory.contractAddress); - - factoryWithoutGuardianStorage = await deployer.deploy(Factory, {}, - moduleRegistry.contractAddress, - implementation.contractAddress, - ensManager.contractAddress); - await factoryWithoutGuardianStorage.addManager(infrastructure.address); - await ensManager.addManager(factoryWithoutGuardianStorage.contractAddress); }); - let module1; - let module2; - beforeEach(async () => { // Restore the good state of factory (we set these to bad addresses in some tests) await factory.changeModuleRegistry(moduleRegistry.contractAddress); @@ -106,7 +94,39 @@ describe("Wallet Factory", function () { index += 1; }); - describe("Configure the factory", () => { + describe("Create and configure the factory", () => { + it("should not allow to be created with empty ModuleRegistry", async () => { + await assert.revertWith(deployer.deploy(Factory, {}, + ZERO_ADDRESS, + implementation.contractAddress, + ensManager.contractAddress, + guardianStorage.contractAddress), "WF: ModuleRegistry address not defined"); + }); + + it("should not allow to be created with empty WalletImplementation", async () => { + await assert.revertWith(deployer.deploy(Factory, {}, + moduleRegistry.contractAddress, + ZERO_ADDRESS, + ensManager.contractAddress, + guardianStorage.contractAddress), "WF: WalletImplementation address not defined"); + }); + + it("should not allow to be created with empty ENSManager", async () => { + await assert.revertWith(deployer.deploy(Factory, {}, + moduleRegistry.contractAddress, + implementation.contractAddress, + ZERO_ADDRESS, + guardianStorage.contractAddress), "WF: ENSManager address not defined"); + }); + + it("should not allow to be created with empty GuardianStorage", async () => { + await assert.revertWith(deployer.deploy(Factory, {}, + moduleRegistry.contractAddress, + implementation.contractAddress, + ensManager.contractAddress, + ZERO_ADDRESS), "WF: GuardianStorage address not defined"); + }); + it("should allow owner to change the module registry", async () => { const randomAddress = utilities.getRandomAddress(); await factory.changeModuleRegistry(randomAddress); @@ -139,8 +159,9 @@ describe("Wallet Factory", function () { await assert.revertWith(factory.from(other).changeENSManager(randomAddress), "Must be owner"); }); - it("should not allow guardian storage address to be set to zero", async () => { - await assert.revertWith(factory.changeGuardianStorage(ethers.constants.AddressZero), "WF: address cannot be null"); + it("should return the correct ENSManager", async () => { + const ensManagerOnFactory = await factory.ensManager(); + assert.equal(ensManagerOnFactory, ensManager.contractAddress, "should have the correct ENSManager addrress"); }); }); @@ -149,7 +170,7 @@ describe("Wallet Factory", function () { // we create the wallet const label = `wallet${index}`; const modules = [module1.contractAddress]; - const tx = await factory.from(infrastructure).createWallet(owner.address, modules, label); + const tx = await factory.from(infrastructure).createWallet(owner.address, modules, label, guardian.address); const txReceipt = await factory.verboseWaitForTransaction(tx); const walletAddr = txReceipt.events.filter((event) => event.event === "WalletCreated")[0].args.wallet; // we test that the wallet has the correct owner @@ -162,7 +183,7 @@ describe("Wallet Factory", function () { const label = `wallet${index}`; const modules = [module1.contractAddress, module2.contractAddress]; // we create the wallet - const tx = await factory.from(infrastructure).createWallet(owner.address, modules, label); + const tx = await factory.from(infrastructure).createWallet(owner.address, modules, label, guardian.address); const txReceipt = await factory.verboseWaitForTransaction(tx); const walletAddr = txReceipt.events.filter((event) => event.event === "WalletCreated")[0].args.wallet; // we test that the wallet has the correct modules @@ -173,12 +194,24 @@ describe("Wallet Factory", function () { assert.equal(isAuthorised, true, "module2 should be authorised"); }); + it("should create with the correct guardian", async () => { + // we create the wallet + const label = `wallet${index}`; + const modules = [module1.contractAddress]; + const tx = await factory.from(infrastructure).createWallet(owner.address, modules, label, guardian.address); + const txReceipt = await factory.verboseWaitForTransaction(tx); + const walletAddr = txReceipt.events.filter((event) => event.event === "WalletCreated")[0].args.wallet; + // we test that the wallet has the correct guardian + const success = await guardianStorage.isGuardian(walletAddr, guardian.address); + assert.equal(success, true, "should have the correct guardian"); + }); + it("should create with the correct ENS name", async () => { const label = `wallet${index}`; const labelNode = ethers.utils.namehash(`${label}.${subnameWallet}.${root}`); const modules = [module1.contractAddress, module2.contractAddress]; // we create the wallet - const tx = await factory.from(infrastructure).createWallet(owner.address, modules, label); + const tx = await factory.from(infrastructure).createWallet(owner.address, modules, label, guardian.address); const txReceipt = await factory.verboseWaitForTransaction(tx); const walletAddr = txReceipt.events.filter((event) => event.event === "WalletCreated")[0].args.wallet; // we test that the wallet has the correct ENS @@ -188,35 +221,46 @@ describe("Wallet Factory", function () { assert.equal(res, ensResolver.contractAddress); }); + it("should fail to create when the guardian is empty", async () => { + // we create the wallet + const label = `wallet${index}`; + const modules = [module1.contractAddress]; + await assert.revertWith(factory.from(infrastructure).createWallet(owner.address, modules, label, ZERO_ADDRESS), + "WF: guardian cannot be null"); + }); + it("should fail to create when there are no modules", async () => { const label = `wallet${index}`; const modules = []; - await assert.revertWith(factory.from(deployer).createWallet(owner.address, modules, label), "WF: cannot assign with less than 1 module"); + await assert.revertWith(factory.from(deployer).createWallet(owner.address, modules, label, guardian.address), + "WF: cannot assign with less than 1 module"); }); it("should fail to create when there is no ENS", async () => { const modules = [module1.contractAddress, module2.contractAddress]; - await assert.revertWith(factory.from(infrastructure).createWallet(owner.address, modules, NO_ENS), "WF: ENS label must be defined"); + await assert.revertWith(factory.from(infrastructure).createWallet(owner.address, modules, NO_ENS, guardian.address), + "WF: ENS label must be defined"); }); it("should fail to create with an existing ENS", async () => { const label = `wallet${index}`; const modules = [module1.contractAddress, module2.contractAddress]; - await factory.from(infrastructure).createWallet(owner.address, modules, label); - await assert.revertWith(factory.from(infrastructure).createWallet(owner.address, modules, label), "AEM: _label is alrealdy owned"); + await factory.from(infrastructure).createWallet(owner.address, modules, label, guardian.address); + await assert.revertWith(factory.from(infrastructure).createWallet(owner.address, modules, label, guardian.address), + "AEM: _label is alrealdy owned"); }); it("should fail to create with zero address as owner", async () => { const label = `wallet${index}`; const modules = [module1.contractAddress]; - await assert.revertWith(factory.from(infrastructure).createWallet(ethers.constants.AddressZero, modules, label), + await assert.revertWith(factory.from(infrastructure).createWallet(ethers.constants.AddressZero, modules, label, guardian.address), "WF: owner cannot be null"); }); it("should fail to create with no modules", async () => { const label = `wallet${index}`; const modules = []; - await assert.revertWith(factory.from(infrastructure).createWallet(owner.address, modules, label), + await assert.revertWith(factory.from(infrastructure).createWallet(owner.address, modules, label, guardian.address), "WF: cannot assign with less than 1 module"); }); @@ -224,87 +268,15 @@ describe("Wallet Factory", function () { const label = `wallet${index}`; const randomAddress = utilities.getRandomAddress(); const modules = [randomAddress]; - await assert.revertWith(factory.from(infrastructure).createWallet(owner.address, modules, label), + await assert.revertWith(factory.from(infrastructure).createWallet(owner.address, modules, label, guardian.address), "WF: one or more modules are not registered"); }); it("should fail to create with empty label", async () => { const label = ""; const modules = [module1.contractAddress]; - await assert.revertWith(factory.from(infrastructure).createWallet(owner.address, modules, label), "WF: ENS label must be defined"); - }); - }); - - describe("Create wallets with CREATE and default guardian", () => { - it("should create with the correct owner", async () => { - // we create the wallet - const label = `wallet${index}`; - const modules = [module1.contractAddress]; - const tx = await factory.from(infrastructure).createWalletWithGuardian(owner.address, modules, label, guardian.address); - const txReceipt = await factory.verboseWaitForTransaction(tx); - const walletAddr = txReceipt.events.filter((event) => event.event === "WalletCreated")[0].args.wallet; - // we test that the wallet has the correct owner - const wallet = await deployer.wrapDeployedContract(BaseWallet, walletAddr); - const walletOwner = await wallet.owner(); - assert.equal(walletOwner, owner.address, "should have the correct owner"); - }); - - it("should create with the correct modules", async () => { - const label = `wallet${index}`; - const modules = [module1.contractAddress, module2.contractAddress]; - // we create the wallet - const tx = await factory.from(infrastructure).createWalletWithGuardian(owner.address, modules, label, guardian.address); - const txReceipt = await factory.verboseWaitForTransaction(tx); - const walletAddr = txReceipt.events.filter((event) => event.event === "WalletCreated")[0].args.wallet; - // we test that the wallet has the correct modules - const wallet = await deployer.wrapDeployedContract(BaseWallet, walletAddr); - let isAuthorised = await wallet.authorised(module1.contractAddress); - assert.equal(isAuthorised, true, "module1 should be authorised"); - isAuthorised = await wallet.authorised(module2.contractAddress); - assert.equal(isAuthorised, true, "module2 should be authorised"); - }); - - it("should create with the correct guardian", async () => { - // we create the wallet - const label = `wallet${index}`; - const modules = [module1.contractAddress]; - const tx = await factory.from(infrastructure).createWalletWithGuardian(owner.address, modules, label, guardian.address); - const txReceipt = await factory.verboseWaitForTransaction(tx); - const walletAddr = txReceipt.events.filter((event) => event.event === "WalletCreated")[0].args.wallet; - // we test that the wallet has the correct guardian - const success = await guardianStorage.isGuardian(walletAddr, guardian.address); - assert.equal(success, true, "should have the correct guardian"); - }); - - it("should create with the correct ENS name", async () => { - const label = `wallet${index}`; - const labelNode = ethers.utils.namehash(`${label}.${subnameWallet}.${root}`); - const modules = [module1.contractAddress, module2.contractAddress]; - // we create the wallet - const tx = await factory.from(infrastructure).createWalletWithGuardian(owner.address, modules, label, guardian.address); - const txReceipt = await factory.verboseWaitForTransaction(tx); - const walletAddr = txReceipt.events.filter((event) => event.event === "WalletCreated")[0].args.wallet; - // we test that the wallet has the correct ENS - const nodeOwner = await ensRegistry.owner(labelNode); - assert.equal(nodeOwner, walletAddr); - const res = await ensRegistry.resolver(labelNode); - assert.equal(res, ensResolver.contractAddress); - }); - - it("should fail to create with a guardian when the guardian storage is not defined", async () => { - const label = `wallet${index}`; - const modules = [module1.contractAddress, module2.contractAddress]; - await assert.revertWith(factoryWithoutGuardianStorage.from(infrastructure) - .createWalletWithGuardian(owner.address, modules, label, guardian.address), - "GuardianStorage address not defined"); - }); - - it("should fail to create when the guardian is empty", async () => { - // we create the wallet - const label = `wallet${index}`; - const modules = [module1.contractAddress]; - await assert.revertWith(factory.from(infrastructure).createWalletWithGuardian(owner.address, modules, label, ZERO_ADDRESS), - "WF: guardian cannot be null"); + await assert.revertWith(factory.from(infrastructure).createWallet(owner.address, modules, label, guardian.address), + "WF: ENS label must be defined"); }); }); @@ -321,9 +293,9 @@ describe("Wallet Factory", function () { const label = `wallet${index}`; const modules = [module1.contractAddress, module2.contractAddress]; // we get the future address - const futureAddr = await factory.getAddressForCounterfactualWallet(owner.address, modules, salt); + const futureAddr = await factory.getAddressForCounterfactualWallet(owner.address, modules, guardian.address, salt); // we create the wallet - const tx = await factory.from(infrastructure).createCounterfactualWallet(owner.address, modules, label, salt); + const tx = await factory.from(infrastructure).createCounterfactualWallet(owner.address, modules, label, guardian.address, salt); const txReceipt = await factory.verboseWaitForTransaction(tx); const walletAddr = txReceipt.events.filter((event) => event.event === "WalletCreated")[0].args.wallet; // we test that the wallet is at the correct address @@ -335,9 +307,9 @@ describe("Wallet Factory", function () { const label = `wallet${index}`; const modules = [module1.contractAddress, module2.contractAddress]; // we get the future address - const futureAddr = await factory.getAddressForCounterfactualWallet(owner.address, modules, salt); + const futureAddr = await factory.getAddressForCounterfactualWallet(owner.address, modules, guardian.address, salt); // we create the wallet - const tx = await factory.from(infrastructure).createCounterfactualWallet(owner.address, modules, label, salt); + const tx = await factory.from(infrastructure).createCounterfactualWallet(owner.address, modules, label, guardian.address, salt); const txReceipt = await factory.verboseWaitForTransaction(tx); const walletAddr = txReceipt.events.filter((event) => event.event === "WalletCreated")[0].args.wallet; // we test that the wallet is at the correct address @@ -353,9 +325,9 @@ describe("Wallet Factory", function () { const label = `wallet${index}`; const modules = [module1.contractAddress, module2.contractAddress]; // we get the future address - const futureAddr = await factory.getAddressForCounterfactualWallet(owner.address, modules, salt); + const futureAddr = await factory.getAddressForCounterfactualWallet(owner.address, modules, guardian.address, salt); // we create the wallet - const tx = await factory.from(infrastructure).createCounterfactualWallet(owner.address, modules, label, salt); + const tx = await factory.from(infrastructure).createCounterfactualWallet(owner.address, modules, label, guardian.address, salt); const txReceipt = await factory.verboseWaitForTransaction(tx); const walletAddr = txReceipt.events.filter((event) => event.event === "WalletCreated")[0].args.wallet; // we test that the wallet is at the correct address @@ -374,9 +346,9 @@ describe("Wallet Factory", function () { const labelNode = ethers.utils.namehash(`${label}.${subnameWallet}.${root}`); const modules = [module1.contractAddress, module2.contractAddress]; // we get the future address - const futureAddr = await factory.getAddressForCounterfactualWallet(owner.address, modules, salt); + const futureAddr = await factory.getAddressForCounterfactualWallet(owner.address, modules, guardian.address, salt); // we create the wallet - const tx = await factory.from(infrastructure).createCounterfactualWallet(owner.address, modules, label, salt); + const tx = await factory.from(infrastructure).createCounterfactualWallet(owner.address, modules, label, guardian.address, salt); const txReceipt = await factory.verboseWaitForTransaction(tx); const walletAddr = txReceipt.events.filter((event) => event.event === "WalletCreated")[0].args.wallet; // we test that the wallet is at the correct address @@ -388,107 +360,14 @@ describe("Wallet Factory", function () { assert.equal(res, ensResolver.contractAddress); }); - it("should fail to create a wallet at an existing address", async () => { - const salt = utilities.generateSaltValue(); - const label = `wallet${index}`; - const modules = [module1.contractAddress, module2.contractAddress]; - // we get the future address - const futureAddr = await factory.getAddressForCounterfactualWallet(owner.address, modules, salt); - // we create the first wallet - const tx = await factory.from(infrastructure).createCounterfactualWallet(owner.address, modules, label, salt); - const txReceipt = await factory.verboseWaitForTransaction(tx); - const walletAddr = txReceipt.events.filter((event) => event.event === "WalletCreated")[0].args.wallet; - // we test that the wallet is at the correct address - assert.equal(futureAddr, walletAddr, "should have the correct address"); - // we create the second wallet - await assert.revert(factory.from(infrastructure).createCounterfactualWallet(owner.address, modules, label, salt), - "should fail when address is in use"); - }); - - it("should fail to create counterfactually when there are no modules (without guardian)", async () => { - const salt = utilities.generateSaltValue(); - const label = `wallet${index}`; - const modules = []; - await assert.revertWith(factory.from(deployer).createCounterfactualWallet(owner.address, modules, label, salt), - "WF: cannot assign with less than 1 module"); - }); - - it("should fail to create when there is no ENS", async () => { - const salt = utilities.generateSaltValue(); - const label = ""; - const modules = [module1.contractAddress, module2.contractAddress]; - await assert.revertWith(factory.from(deployer).createCounterfactualWallet(owner.address, modules, label, salt), - "WF: ENS label must be defined"); - }); - - it("should emit and event when the balance is non zero at creation", async () => { - const salt = utilities.generateSaltValue(); - const label = `wallet${index}`; - const modules = [module1.contractAddress, module2.contractAddress]; - const amount = bigNumberify("10000000000000"); - // we get the future address - const futureAddr = await factory.getAddressForCounterfactualWallet(owner.address, modules, salt); - // We send ETH to the address - await infrastructure.sendTransaction({ to: futureAddr, value: amount }); - // we create the wallet - const tx = await factory.from(infrastructure).createCounterfactualWallet(owner.address, modules, label, salt); - const txReceipt = await factory.verboseWaitForTransaction(tx); - const wallet = deployer.wrapDeployedContract(BaseWallet, futureAddr); - assert.isTrue(await utils.hasEvent(txReceipt, wallet, "Received"), "should have generated Received event"); - const log = await utils.parseLogs(txReceipt, wallet, "Received"); - assert.equal(log[0].value.toNumber(), amount, "should log the correct amount"); - assert.equal(log[0].sender, "0x0000000000000000000000000000000000000000", "sender should be address(0)"); - }); - }); - - describe("Create wallets with CREATE2 and default guardian", () => { - beforeEach(async () => { - module1 = await deployer.deploy(Module, {}, moduleRegistry.contractAddress, guardianStorage.contractAddress, ZERO_BYTES32); - module2 = await deployer.deploy(Module, {}, moduleRegistry.contractAddress, guardianStorage.contractAddress, ZERO_BYTES32); - await moduleRegistry.registerModule(module1.contractAddress, ethers.utils.formatBytes32String("module1")); - await moduleRegistry.registerModule(module2.contractAddress, ethers.utils.formatBytes32String("module2")); - }); - - it("should create a wallet at the correct address", async () => { - const salt = utilities.generateSaltValue(); - const label = `wallet${index}`; - const modules = [module1.contractAddress, module2.contractAddress]; - // we get the future address - const futureAddr = await factory.getAddressForCounterfactualWalletWithGuardian(owner.address, modules, guardian.address, salt); - // we create the wallet - const tx = await factory.from(infrastructure).createCounterfactualWalletWithGuardian(owner.address, modules, label, guardian.address, salt); - const txReceipt = await factory.verboseWaitForTransaction(tx); - const walletAddr = txReceipt.events.filter((event) => event.event === "WalletCreated")[0].args.wallet; - // we test that the wallet is at the correct address - assert.equal(futureAddr, walletAddr, "should have the correct address"); - }); - - it("should create with the correct owner", async () => { - const salt = utilities.generateSaltValue(); - const label = `wallet${index}`; - const modules = [module1.contractAddress, module2.contractAddress]; - // we get the future address - const futureAddr = await factory.getAddressForCounterfactualWalletWithGuardian(owner.address, modules, guardian.address, salt); - // we create the wallet - const tx = await factory.from(infrastructure).createCounterfactualWalletWithGuardian(owner.address, modules, label, guardian.address, salt); - const txReceipt = await factory.verboseWaitForTransaction(tx); - const walletAddr = txReceipt.events.filter((event) => event.event === "WalletCreated")[0].args.wallet; - // we test that the wallet is at the correct address - assert.equal(futureAddr, walletAddr, "should have the correct address"); - // we test that the wallet has the correct owner - const wallet = await deployer.wrapDeployedContract(BaseWallet, walletAddr); - const walletOwner = await wallet.owner(); - assert.equal(walletOwner, owner.address, "should have the correct owner"); - }); - it("should create with the correct guardian", async () => { const salt = utilities.generateSaltValue(); const label = `wallet${index}`; const modules = [module1.contractAddress, module2.contractAddress]; // we get the future address - const futureAddr = await factory.getAddressForCounterfactualWalletWithGuardian(owner.address, modules, guardian.address, salt); + const futureAddr = await factory.getAddressForCounterfactualWallet(owner.address, modules, guardian.address, salt); // we create the wallet - const tx = await factory.from(infrastructure).createCounterfactualWalletWithGuardian(owner.address, modules, label, guardian.address, salt); + const tx = await factory.from(infrastructure).createCounterfactualWallet(owner.address, modules, label, guardian.address, salt); const txReceipt = await factory.verboseWaitForTransaction(tx); const walletAddr = txReceipt.events.filter((event) => event.event === "WalletCreated")[0].args.wallet; // we test that the wallet is at the correct address @@ -498,60 +377,20 @@ describe("Wallet Factory", function () { assert.equal(success, true, "should have the correct guardian"); }); - it("should create with the correct modules", async () => { - const salt = utilities.generateSaltValue(); - const label = `wallet${index}`; - const modules = [module1.contractAddress, module2.contractAddress]; - // we get the future address - const futureAddr = await factory.getAddressForCounterfactualWalletWithGuardian(owner.address, modules, guardian.address, salt); - // we create the wallet - const tx = await factory.from(infrastructure).createCounterfactualWalletWithGuardian(owner.address, modules, label, guardian.address, salt); - const txReceipt = await factory.verboseWaitForTransaction(tx); - const walletAddr = txReceipt.events.filter((event) => event.event === "WalletCreated")[0].args.wallet; - // we test that the wallet is at the correct address - assert.equal(futureAddr, walletAddr, "should have the correct address"); - // we test that the wallet has the correct modules - const wallet = await deployer.wrapDeployedContract(BaseWallet, walletAddr); - let isAuthorised = await wallet.authorised(module1.contractAddress); - assert.equal(isAuthorised, true, "module1 should be authorised"); - isAuthorised = await wallet.authorised(module2.contractAddress); - assert.equal(isAuthorised, true, "module2 should be authorised"); - }); - - it("should create with the correct ENS name", async () => { - const salt = utilities.generateSaltValue(); - const label = `wallet${index}`; - const labelNode = ethers.utils.namehash(`${label}.${subnameWallet}.${root}`); - const modules = [module1.contractAddress, module2.contractAddress]; - // we get the future address - const futureAddr = await factory.getAddressForCounterfactualWalletWithGuardian(owner.address, modules, guardian.address, salt); - // we create the wallet - const tx = await factory.from(infrastructure).createCounterfactualWalletWithGuardian(owner.address, modules, label, guardian.address, salt); - const txReceipt = await factory.verboseWaitForTransaction(tx); - const walletAddr = txReceipt.events.filter((event) => event.event === "WalletCreated")[0].args.wallet; - // we test that the wallet is at the correct address - assert.equal(futureAddr, walletAddr, "should have the correct address"); - // we test that the wallet has the correct ENS - const nodeOwner = await ensRegistry.owner(labelNode); - assert.equal(nodeOwner, walletAddr); - const res = await ensRegistry.resolver(labelNode); - assert.equal(res, ensResolver.contractAddress); - }); - it("should fail to create a wallet at an existing address", async () => { const salt = utilities.generateSaltValue(); const label = `wallet${index}`; const modules = [module1.contractAddress, module2.contractAddress]; // we get the future address - const futureAddr = await factory.getAddressForCounterfactualWalletWithGuardian(owner.address, modules, guardian.address, salt); + const futureAddr = await factory.getAddressForCounterfactualWallet(owner.address, modules, guardian.address, salt); // we create the first wallet - const tx = await factory.from(infrastructure).createCounterfactualWalletWithGuardian(owner.address, modules, label, guardian.address, salt); + const tx = await factory.from(infrastructure).createCounterfactualWallet(owner.address, modules, label, guardian.address, salt); const txReceipt = await factory.verboseWaitForTransaction(tx); const walletAddr = txReceipt.events.filter((event) => event.event === "WalletCreated")[0].args.wallet; // we test that the wallet is at the correct address assert.equal(futureAddr, walletAddr, "should have the correct address"); // we create the second wallet - await assert.revert(factory.from(infrastructure).createCounterfactualWalletWithGuardian(owner.address, modules, label, guardian.address, salt), + await assert.revert(factory.from(infrastructure).createCounterfactualWallet(owner.address, modules, label, guardian.address, salt), "should fail when address is in use"); }); @@ -559,37 +398,49 @@ describe("Wallet Factory", function () { const salt = utilities.generateSaltValue(); const label = `wallet${index}`; const modules = []; - await assert.revertWith(factory.from(deployer).createCounterfactualWalletWithGuardian(owner.address, modules, label, guardian.address, salt), + await assert.revertWith(factory.from(deployer).createCounterfactualWallet(owner.address, modules, label, guardian.address, salt), "WF: cannot assign with less than 1 module"); }); - it("should return the correct ENSManager", async () => { - const ensManagerOnFactory = await factory.ensManager(); - assert.equal(ensManagerOnFactory, ensManager.contractAddress, "should have the correct ENSManager addrress"); + it("should fail to create when there is no ENS", async () => { + const salt = utilities.generateSaltValue(); + const label = ""; + const modules = [module1.contractAddress, module2.contractAddress]; + await assert.revertWith(factory.from(deployer).createCounterfactualWallet(owner.address, modules, label, guardian.address, salt), + "WF: ENS label must be defined"); }); - - it("should fail to create with a guardian when the guardian storage is not defined", async () => { + it("should fail to create when the guardian is empty", async () => { const salt = utilities.generateSaltValue(); const label = `wallet${index}`; const modules = [module1.contractAddress, module2.contractAddress]; - await assert.revertWith(factoryWithoutGuardianStorage.from(infrastructure) - .createCounterfactualWalletWithGuardian(owner.address, modules, label, guardian.address, salt), - "GuardianStorage address not defined"); + await assert.revertWith(factory.from(infrastructure).createCounterfactualWallet(owner.address, modules, label, ZERO_ADDRESS, salt), + "WF: guardian cannot be null"); }); - it("should fail to get an address when the guardian is empty", async () => { + it("should emit and event when the balance is non zero at creation", async () => { const salt = utilities.generateSaltValue(); + const label = `wallet${index}`; const modules = [module1.contractAddress, module2.contractAddress]; - await assert.revertWith(factory.from(infrastructure).getAddressForCounterfactualWalletWithGuardian(owner.address, modules, ZERO_ADDRESS, salt), - "WF: guardian cannot be null"); + const amount = bigNumberify("10000000000000"); + // we get the future address + const futureAddr = await factory.getAddressForCounterfactualWallet(owner.address, modules, guardian.address, salt); + // We send ETH to the address + await infrastructure.sendTransaction({ to: futureAddr, value: amount }); + // we create the wallet + const tx = await factory.from(infrastructure).createCounterfactualWallet(owner.address, modules, label, guardian.address, salt); + const txReceipt = await factory.verboseWaitForTransaction(tx); + const wallet = deployer.wrapDeployedContract(BaseWallet, futureAddr); + assert.isTrue(await utils.hasEvent(txReceipt, wallet, "Received"), "should have generated Received event"); + const log = await utils.parseLogs(txReceipt, wallet, "Received"); + assert.equal(log[0].value.toNumber(), amount, "should log the correct amount"); + assert.equal(log[0].sender, "0x0000000000000000000000000000000000000000", "sender should be address(0)"); }); - it("should fail to create when the guardian is empty", async () => { + it("should fail to get an address when the guardian is empty", async () => { const salt = utilities.generateSaltValue(); - const label = `wallet${index}`; const modules = [module1.contractAddress, module2.contractAddress]; - await assert.revertWith(factory.from(infrastructure).createCounterfactualWalletWithGuardian(owner.address, modules, label, ZERO_ADDRESS, salt), + await assert.revertWith(factory.from(infrastructure).getAddressForCounterfactualWallet(owner.address, modules, ZERO_ADDRESS, salt), "WF: guardian cannot be null"); }); });