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

Add ability to generate addresses directly from env file in celotool #1294

Merged
merged 12 commits into from
Oct 10, 2019

Conversation

annakaz
Copy link
Contributor

@annakaz annakaz commented Oct 10, 2019

Description

Add command to celotool to generate account addresses that require the mnemonic (ex: validator, load_testing, tx_node, bootnode and faucet) from an env file.

Previously, this required manually passing in private keys. This now just uses the mnemonic in the encrypted env file.

Tested

Confirmed that celotooljs generate validator-address --celo-env alfajores --i 0 --a validator generates the correct faucet address for alfajores and for integration.

Confirmed that an address is returned for all account types listed above.

Related issues

Backwards compatibility

Yes

Note that if we start using a node other than validator 0 as the faucet account, we will have to update this

@annakaz annakaz requested a review from nambrot October 10, 2019 18:19
@annakaz annakaz changed the title Add ability to read faucet-address to celotool Add ability to generate faucet-address in celotool Oct 10, 2019
@codecov
Copy link

codecov bot commented Oct 10, 2019

Codecov Report

Merging #1294 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1294   +/-   ##
=======================================
  Coverage   66.41%   66.41%           
=======================================
  Files         264      264           
  Lines        7745     7745           
  Branches      444      444           
=======================================
  Hits         5144     5144           
  Misses       2502     2502           
  Partials       99       99
Flag Coverage Δ
#mobile 66.41% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 56cdd6e...69e0c80. Read the comment docs.

@annakaz annakaz changed the title Add ability to generate faucet-address in celotool Add ability to generate addresses directly from env file in celotool Oct 10, 2019
@@ -0,0 +1,23 @@
/* tslint:disable no-console */
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just renamed this file which used to be account-address.ts

Copy link
Contributor

Choose a reason for hiding this comment

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

With this rename, we might have to hunt down other places in the codebase that currently calls this command, it might be easier to rename your "new" command

@@ -0,0 +1,18 @@
/* tslint:disable no-console */
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wanted to keep this as a separate command, as the address returned from the "faucet" Account Type in account-address.ts is no longer what we're using

Copy link
Contributor

Choose a reason for hiding this comment

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

Sgtm, though we are still using the faucet account type right? Just for the public facing faucet, not for the internal faucet?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, on second thought I think this will lead to more confusion, so removing the command for now

@annakaz annakaz added the automerge Have PR merge automatically when checks pass label Oct 10, 2019
@ashishb ashishb merged commit 1b3c2d9 into master Oct 10, 2019
@ashishb ashishb deleted the annakaz/faucet-address branch October 10, 2019 20:54
aaronmgdr added a commit that referenced this pull request Oct 23, 2019
* master:
  Integration deploy, don't overwrite genesis block if upgrading testnet (#1315)
  Fix broken Discord link (#1317)
  [Wallet] Add check for $ENVFILE to pre-dev script (#1324)
  Add GRADLE_OPTS note to SETUP.md (#1311)
  Allow validators to specify an attestationKey with which they sign attestations (#1313)
  Add step to install typescript and other minor edits (#1256)
  [Wallet] Add script to build sdk for env before running yarn dev (#1312)
  [Wallet] Fix ListView deprecation warning (#1293)
  Set IN_MEMORY_DISCOVERY_TABLE=true for integration (#1296)
  Add ability to generate addresses directly from env file in celotool (#1294)
aaronmgdr added a commit that referenced this pull request Oct 23, 2019
* master:
  Integration deploy, don't overwrite genesis block if upgrading testnet (#1315)
  Fix broken Discord link (#1317)
  [Wallet] Add check for $ENVFILE to pre-dev script (#1324)
  Add GRADLE_OPTS note to SETUP.md (#1311)
  Allow validators to specify an attestationKey with which they sign attestations (#1313)
  Add step to install typescript and other minor edits (#1256)
  [Wallet] Add script to build sdk for env before running yarn dev (#1312)
  [Wallet] Fix ListView deprecation warning (#1293)
  Set IN_MEMORY_DISCOVERY_TABLE=true for integration (#1296)
  Add ability to generate addresses directly from env file in celotool (#1294)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Have PR merge automatically when checks pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants