Skip to content

Commit

Permalink
make ENS label optional in the wallet factory
Browse files Browse the repository at this point in the history
  • Loading branch information
@juniset
juniset committed Jul 15, 2020
1 parent d26bff0 commit da0026e
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 40 deletions.
32 changes: 13 additions & 19 deletions contracts/wallet/WalletFactory.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ contract WalletFactory is Owned, Managed {
* 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 _label Optional ENS label of the new wallet, e.g. franck.
* @param _guardian The guardian address.
*/
function createWallet(
Expand All @@ -82,7 +82,6 @@ contract WalletFactory is Owned, Managed {
onlyManager
{
validateInputs(_owner, _modules, _guardian);
validateENSLabel(_label);
Proxy proxy = new Proxy(walletImplementation);
address payable wallet = address(proxy);
configureWallet(BaseWallet(wallet), _owner, _modules, _label, _guardian);
Expand All @@ -94,7 +93,7 @@ contract WalletFactory is Owned, Managed {
* 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 _label Optional ENS label of the new wallet, e.g. franck.
* @param _guardian The guardian address.
* @param _salt The salt.
*/
Expand All @@ -110,8 +109,6 @@ contract WalletFactory is Owned, Managed {
returns (address _wallet)
{
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);
Expand Down Expand Up @@ -180,7 +177,7 @@ contract WalletFactory is Owned, Managed {
* @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 _label Optional ENS label of the new wallet, e.g. franck.
* @param _guardian The guardian address.
*/
function configureWallet(
Expand All @@ -203,7 +200,7 @@ contract WalletFactory is Owned, Managed {
// add guardian
IGuardianStorage(guardianStorage).addGuardian(address(_wallet), _guardian);

// register ENS
// register ENS if needed
registerWalletENS(address(_wallet), _label);
// remove the factory from the authorised modules
_wallet.authoriseModule(address(this), false);
Expand Down Expand Up @@ -234,23 +231,20 @@ contract WalletFactory is Owned, Managed {
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");
}

/**
* @dev Register an ENS subname to a wallet.
* @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 {
// claim reverse
address ensResolver = IENSManager(ensManager).ensResolver();
bytes memory methodData = abi.encodeWithSignature("claimWithResolver(address,address)", ensManager, ensResolver);
address ensReverseRegistrar = IENSManager(ensManager).getENSReverseRegistrar();
BaseWallet(_wallet).invoke(ensReverseRegistrar, 0, methodData);
// register with ENS manager
IENSManager(ensManager).register(_label, _wallet);
if (bytes(_label).length > 0) {
// claim reverse
address ensResolver = IENSManager(ensManager).ensResolver();
bytes memory methodData = abi.encodeWithSignature("claimWithResolver(address,address)", ensManager, ensResolver);
address ensReverseRegistrar = IENSManager(ensManager).getENSReverseRegistrar();
BaseWallet(_wallet).invoke(ensReverseRegistrar, 0, methodData);
// register with ENS manager
IENSManager(ensManager).register(_label, _wallet);
}
}
}
43 changes: 22 additions & 21 deletions test/factory.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,15 @@ describe("Wallet Factory", function () {
assert.equal(res, ensResolver.contractAddress);
});

it("should create when there is no ENS", async () => {
const modules = [module1.contractAddress, module2.contractAddress];
// we create the wallet
const tx = await factory.from(infrastructure).createWallet(owner.address, modules, NO_ENS, guardian.address);
const txReceipt = await factory.verboseWaitForTransaction(tx);
const walletAddr = txReceipt.events.filter((event) => event.event === "WalletCreated")[0].args.wallet;
assert.notEqual(walletAddr, ZERO_ADDRESS, "wallet shoudl be created");
});

it("should fail to create when the guardian is empty", async () => {
// we create the wallet
const label = `wallet${index}`;
Expand All @@ -236,12 +245,6 @@ describe("Wallet Factory", function () {
"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, 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];
Expand Down Expand Up @@ -271,13 +274,6 @@ describe("Wallet Factory", function () {
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, guardian.address),
"WF: ENS label must be defined");
});
});

describe("Create wallets with CREATE2", () => {
Expand Down Expand Up @@ -360,6 +356,19 @@ describe("Wallet Factory", function () {
assert.equal(res, ensResolver.contractAddress);
});

it("should create when there is no ENS", async () => {
const salt = utilities.generateSaltValue();
const modules = [module1.contractAddress, module2.contractAddress];
// we get the future address
const futureAddr = await factory.getAddressForCounterfactualWallet(owner.address, modules, guardian.address, salt);
// we create the wallet
const tx = await factory.from(deployer).createCounterfactualWallet(owner.address, modules, NO_ENS, 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 guardian", async () => {
const salt = utilities.generateSaltValue();
const label = `wallet${index}`;
Expand Down Expand Up @@ -402,14 +411,6 @@ describe("Wallet Factory", function () {
"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, guardian.address, salt),
"WF: ENS label must be defined");
});

it("should fail to create when the guardian is empty", async () => {
const salt = utilities.generateSaltValue();
const label = `wallet${index}`;
Expand Down

0 comments on commit da0026e

Please sign in to comment.