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

PP-183: create a tool to generate L2 private keys #29

Conversation

kenny08gt
Copy link

@kenny08gt kenny08gt commented Aug 2, 2022

What

Create a command to generate a wallet on L2, with giving credentials (address and private key) or generate a new random one.

Why

In deployment of the project that was needed

Ref

https://rsklabs.atlassian.net/browse/PP-183

@kenny08gt
Copy link
Author

kenny08gt commented Aug 3, 2022

Added a rust file to generate the wallet, the output is the print of the key
ETH Address: 0xe0072a000f359e97164282581aae281358571614 Private key: 0x4361ba8675c0560561bc32f2978fdfdf3863a3a72c9bb64d3d02a2e992bafb21

Navigate to ri-aggregation/core/lib/wallet_creator, build with cargo build and then run it with cargo run rif_aggreation_wallet_creator to create a random one.

run cargo run rif_aggreation_wallet_creator <eth_address> <private_key> to add a L1 to L2.

@kenny08gt kenny08gt marked this pull request as ready for review August 8, 2022 17:36
Copy link
Collaborator

@antomor antomor left a comment

Choose a reason for hiding this comment

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

@kenny08gt Thank you for this.
Apart from the comments I left, I think we should also include the new command in the related readme if any, if not we should create a new one to include the instructions on how to run it.

infrastructure/zcli/src/index.ts Outdated Show resolved Hide resolved
infrastructure/zcli/src/index.ts Outdated Show resolved Hide resolved
@kenny08gt kenny08gt requested a review from antomor August 12, 2022 04:53
Copy link
Member

@jurajpiar jurajpiar left a comment

Choose a reason for hiding this comment

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

LG

Copy link
Collaborator

@antomor antomor left a comment

Choose a reason for hiding this comment

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

@kenny08gt Thank you for these changes. I've just left some minor comments.

Alan Hurtarte and others added 2 commits August 19, 2022 09:12
Rename variable for more clarity.

Co-authored-by: Antonio Morrone <dev@antomor.com>
@kenny08gt kenny08gt requested a review from antomor August 19, 2022 23:48
Co-authored-by: Antonio Morrone <dev@antomor.com>
Copy link
Collaborator

@antomor antomor left a comment

Choose a reason for hiding this comment

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

Apart from the comments I left,

  1. In the README.md file I'd specify that we need to an instance of the zkserver running locally. QQ: where the server address is retrieved exactly? Could we use the command pointing to a remote server?
  2. I'd print the private key of the wallet just generated, otherwise the tools isn't useful 😅

core/lib/wallet_creator/src/main.rs Outdated Show resolved Hide resolved
@kenny08gt
Copy link
Author

kenny08gt commented Aug 22, 2022

@antomor thanks for the review.

In the README.md file I'd specify that we need to an instance of the zkserver running locally.

Added more instructions.

where the server address is retrieved exactly? Could we use the command pointing to a remote server?

Added support to multiple networks. Here is setup the server to point and in line 51 the provider is initialized.

I'd print the private key of the wallet just generated, otherwise the tools isn't useful

Is already printing the addresses.
Screen Shot 2022-08-22 at 14 28 16

Also added three unit tests

@kenny08gt kenny08gt requested a review from antomor August 22, 2022 21:14
@antomor antomor changed the title PP-183: create a tool to generate l 2 private keys PP-183: create a tool to generate L2 private keys Aug 26, 2022
@antomor
Copy link
Collaborator

antomor commented Aug 26, 2022

@kenny08gt Thank you for the changes.

I'd print the private key of the wallet just generated, otherwise the tools isn't useful

Is already printing the addresses.

Yes, this is true, but unless I'm missing something, we are currently printing the address/private key received, while we should print the L2 private key we are creating.

The test part is really appreciated, thank you. Not sure about what's the standard in rust, but I think it would be better to have them in a separate file, possibly in the test folder.

Furthermore, I've suggested a clarification for the readme file.

@kenny08gt
Copy link
Author

@antomor thanks for the explanation. Now printing the L2 private key and also separate the tests to separate file.

Screen Shot 2022-09-06 at 10 25 58

Clarify the testnet and mainent references

Co-authored-by: Antonio Morrone <dev@antomor.com>
@antomor antomor deleted the branch rsk_merge_master_Dec2021 June 6, 2023 09:37
@antomor antomor closed this Jun 6, 2023
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