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

Integration with the SDK (MVP cont.) #3

Merged
merged 10 commits into from
Aug 19, 2022
Merged

Conversation

digorithm
Copy link
Member

@digorithm digorithm commented May 19, 2022

EDIT(iqdecay): I have taken over this PR, and would like to get this merged "as is", with the refactoring being done in #11 and some more work being done aside.

demo

Continuing the features documented here #2.

This PR integrates the forc wallet plugin with the SDK, adds wallet encryption and storage, wallet listing.

USAGE:
    forc-wallet [OPTIONS] <SUBCOMMAND>

OPTIONS:
    -h, --help            Print help information
    -o, --format <FMT>    [default: json] [possible values: json, toml]
    -V, --version         Print version information

SUBCOMMANDS:
    help      Print this message or the help of the given subcommand(s)
    import    Import a wallet from mnemonic phrase
    list      Lists all wallets stored in ~/.fuel/wallets/
    new       Randomly generate a new wallet. By default, wallets are stored in ~/.fuel/wallets/

Wallet creation now requires a password for the encryption process. Password input is protected and the outputted wallet is outputted in a safe way that can't be stored in the terminal's history.

Wallets are stored in ~/.fuel/wallets. To open these wallets you need the password you used to create them.

Creating the wallet:

./target/debug/forc-wallet new
Please enter a password to encrypt this private key:
Please confirm your password:

Then the safe output:

JSON Wallet uuid: 3ef27797-fd47-45da-b9a1-6d4f7b1f5164

Wallet public address: 3069ea2d1cca428fbe91137943dfaa5f193695730ef62d1f940fee7ffa465326

Wallet mnemonic phrase: space agree swing obvious hover chimney liquid tonight never pony electric useless

### Do not share or lose this mnemonic phrase! Press any key to complete. ###

@digorithm digorithm requested a review from Voxelot May 19, 2022 18:12
Cargo.toml Outdated Show resolved Hide resolved
@digorithm digorithm self-assigned this May 19, 2022
@digorithm digorithm added the enhancement New feature or request label May 19, 2022
@adlerjohn adlerjohn marked this pull request as draft May 22, 2022 18:11
@adlerjohn
Copy link
Contributor

Converting to draft until unblocked by new fuels release.

@iqdecay iqdecay requested a review from adlerjohn August 3, 2022 16:58
@iqdecay
Copy link
Contributor

iqdecay commented Aug 3, 2022

Only pushing version change in this PR, will add tests and other features in other more focused PRs.

@iqdecay iqdecay marked this pull request as ready for review August 3, 2022 19:12
Cargo.toml Outdated Show resolved Hide resolved
iqdecay
iqdecay previously approved these changes Aug 4, 2022
Copy link
Contributor

@iqdecay iqdecay left a comment

Choose a reason for hiding this comment

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

Shameless self-approval :)

kayagokalp
kayagokalp previously approved these changes Aug 4, 2022
Copy link
Member

@kayagokalp kayagokalp left a comment

Choose a reason for hiding this comment

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

LGTM :shipit:

@iqdecay iqdecay dismissed stale reviews from kayagokalp and themself via e52d67c August 5, 2022 15:47
@iqdecay iqdecay requested review from kayagokalp and a team August 5, 2022 16:44
kayagokalp
kayagokalp previously approved these changes Aug 5, 2022
Copy link
Member

@kayagokalp kayagokalp left a comment

Choose a reason for hiding this comment

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

lgtm

src/main.rs Show resolved Hide resolved
src/main.rs Show resolved Hide resolved
src/main.rs Show resolved Hide resolved
@iqdecay
Copy link
Contributor

iqdecay commented Aug 17, 2022

Appreciate the suggestions a lot, but this PR is more about putting "the basis" in, #11 is about refactoring.

Copy link
Contributor

@Braqzen Braqzen left a comment

Choose a reason for hiding this comment

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

Couple questions

  1. The Command enum takes 3 fields while the help message lists 4. Should the enum also contain a help field? When the main function is run it doesn't seem to match for help so I assume some magic happens somewhere and if help is seen then the program does not continue
  2. Can someone explain to me the purpose of the uuid here?

@iqdecay
Copy link
Contributor

iqdecay commented Aug 18, 2022

  • The help command is directly integrated inside the clap module.
  • The UUID is, afaik, here to ensure that we generate wallets that have unique ids/names and avoid collisions.

Copy link
Contributor

@iqdecay iqdecay left a comment

Choose a reason for hiding this comment

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

still lgtm, give another approve please just want to move forward lol

@iqdecay iqdecay merged commit 6a5af29 into master Aug 19, 2022
@iqdecay iqdecay deleted the rodrigo/SDK-integration branch August 19, 2022 10:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

6 participants