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 support for account new --save-to-file #3425

Open
wants to merge 2 commits into
base: staging
Choose a base branch
from

Conversation

Raphexion
Copy link

@Raphexion Raphexion commented Oct 30, 2024

Motivation

When creating ephemeral networks for integration testing, it is convenient to be able to generate accounts programmatically and save the account information into files, especially the private-key-file for a new account.

Therefore, we introduce the command line switch --save-to-file.

snarkos account new --save-to-file /path/to/test-account-1

This feature, will create good synergy with the existing feature to use the command-line switch --private-key-file.

The PR is split into two commits

  1. Add the feature to use --save-to-file for account new
  2. Add tempfile as a dependency

Test Plan

Make sure that you can use the switches --save-to-file and --private-key-file together when using the command-line interface (cli).

Related PRs

@Raphexion Raphexion marked this pull request as draft October 30, 2024 11:38
@Raphexion Raphexion force-pushed the nijo/add-accounts-new-save-to-file branch 2 times, most recently from d251e51 to 9a88d43 Compare October 30, 2024 12:54
@Raphexion Raphexion marked this pull request as ready for review October 30, 2024 13:01
@Raphexion Raphexion force-pushed the nijo/add-accounts-new-save-to-file branch from 9a88d43 to dfd83e3 Compare October 30, 2024 13:32
cli/src/commands/account.rs Outdated Show resolved Hide resolved
cli/src/commands/account.rs Outdated Show resolved Hide resolved
@Raphexion Raphexion force-pushed the nijo/add-accounts-new-save-to-file branch 2 times, most recently from 969e5ab to 18df382 Compare October 31, 2024 12:41
@Raphexion Raphexion requested a review from ljedrz October 31, 2024 12:44
cli/src/commands/account.rs Outdated Show resolved Hide resolved
@ljedrz
Copy link
Collaborator

ljedrz commented Oct 31, 2024

One more thing I forgot before: we need to adjust the file's permissions too; see this PR for reference.

@Raphexion
Copy link
Author

@ljedrz Thank you for the link to the PR. I've implemented the permissions feature and unit test in 8ce2982

@Raphexion Raphexion requested a review from ljedrz November 1, 2024 08:22
cli/src/commands/account.rs Outdated Show resolved Hide resolved
cli/src/commands/account.rs Outdated Show resolved Hide resolved
cli/src/commands/account.rs Outdated Show resolved Hide resolved
@Raphexion Raphexion force-pushed the nijo/add-accounts-new-save-to-file branch from 8ce2982 to ef05e86 Compare November 6, 2024 12:06
@Raphexion Raphexion requested a review from ljedrz November 6, 2024 12:57
cli/src/commands/account.rs Outdated Show resolved Hide resolved
@Raphexion Raphexion requested a review from ljedrz November 6, 2024 14:02
cli/src/lib.rs Outdated Show resolved Hide resolved
@Raphexion Raphexion force-pushed the nijo/add-accounts-new-save-to-file branch 2 times, most recently from 85de9df to 739d635 Compare November 7, 2024 08:51
@Raphexion Raphexion requested a review from ljedrz November 7, 2024 08:52
@Raphexion Raphexion force-pushed the nijo/add-accounts-new-save-to-file branch from 739d635 to fbde50f Compare November 7, 2024 08:54
Copy link
Collaborator

@ljedrz ljedrz left a comment

Choose a reason for hiding this comment

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

LGTM 👌

@zosorock zosorock requested a review from zkxuerb November 15, 2024 22:27
@zosorock zosorock requested review from zklimaleo and a team November 15, 2024 22:27
zkxuerb
zkxuerb previously approved these changes Nov 16, 2024
Copy link
Collaborator

@zkxuerb zkxuerb left a comment

Choose a reason for hiding this comment

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

LGTM

@zosorock zosorock added enhancement v1.2.1 Canary release v1.2.1 labels Nov 19, 2024
@zosorock
Copy link
Contributor

@Raphexion
Copy link
Author

I can see the problem in the CI. I will fix 👍

cli/src/lib.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@ljedrz ljedrz left a comment

Choose a reason for hiding this comment

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

Left one suggestion, other than that LGTM 👍.

@Raphexion Raphexion force-pushed the nijo/add-accounts-new-save-to-file branch from bdb6400 to 35a1b4e Compare November 19, 2024 12:10
@Raphexion Raphexion requested a review from zkxuerb November 19, 2024 12:11
@Raphexion Raphexion force-pushed the nijo/add-accounts-new-save-to-file branch from 35a1b4e to 3b097a2 Compare November 19, 2024 12:21
@zosorock zosorock removed the v1.2.1 Canary release v1.2.1 label Nov 19, 2024
@Raphexion Raphexion force-pushed the nijo/add-accounts-new-save-to-file branch from 3b097a2 to 302cb55 Compare December 11, 2024 08:51
@Raphexion
Copy link
Author

@zkxuerb I fixed the formatting that was detected by the CI (check-fmt). I am sorry about that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants