Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make ENS label optional in the wallet factory #135

Merged
merged 2 commits into from
Jul 16, 2020

Conversation

juniset
Copy link
Member

@juniset juniset commented Jul 15, 2020

We make the ENS label optional when creating a wallet. If the _label parameter of createWallet or createCounterfactualWallet is empty the wallet is created without an ENS. The ENS can be registered later.

@juniset juniset self-assigned this Jul 15, 2020
@juniset juniset changed the title make ENS label optional in the wallet factory Make ENS label optional in the wallet factory Jul 15, 2020
test/factory.js Outdated
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");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo => shoudl

@juniset juniset force-pushed the feature/optional_ens_in_factory branch from da0026e to bf18da9 Compare July 16, 2020 11:52
@elenadimitrova elenadimitrova force-pushed the feature/optional_ens_in_factory branch from bf18da9 to f00ca2a Compare July 16, 2020 12:54
@elenadimitrova elenadimitrova merged commit 065fcea into develop Jul 16, 2020
@elenadimitrova elenadimitrova deleted the feature/optional_ens_in_factory branch July 16, 2020 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants