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

When forc new wallet support input password-non-interactive and silent options #202

Open
lispking opened this issue Sep 10, 2024 · 10 comments

Comments

@lispking
Copy link

lispking commented Sep 10, 2024

Could you consider supporting the "no-password" parameter when executing "forc wallet new", so that it is convenient for scripts to directly generate wallets without a password?

Could you consider supporting the password-non-interactive and silent options parameter when executing "forc wallet new", so that it is convenient for scripts to directly generate wallets with password in password-non-interactive?

@sdankel
Copy link
Member

sdankel commented Sep 10, 2024

Hi @lispking , could you explain more about your use case? We require wallets to have a password as a security measure.

@kayagokalp
Copy link
Member

kayagokalp commented Sep 10, 2024

I don't think we should have the option to create a wallet without password. But for improving usability inside interactive context, we can accept them as a parameter, with a fair warning attached to the related doc string. This param should then be available for creating and also using the wallet, anywhere we currently ask for a password. That would be my advice but curious about what @FuelLabs/tooling thinks about this

@lispking
Copy link
Author

To be precise, it's not about not using a password, but rather about including the password directly in the command line when creating a wallet. This makes it convenient to create a wallet with a single command in the command line, without the need for multiple interactions to create the wallet. @sdankel @kayagokalp

like this: https://github.com/FuelLabs/forc-wallet/pull/201/files

@lispking lispking changed the title Could wallet new command support no-password? When forc new wallet support input password-non-interactive and silent options Sep 11, 2024
@lispking
Copy link
Author

lispking commented Sep 11, 2024

Hi @lispking , could you explain more about your use case? We require wallets to have a password as a security measure.

Hello, @sdankel. The use case is as follows: recently, I've been integrating forc tool into our service and found that this kind of command interactivity is not very convenient for the expansion of our service.

This includes the subsequent wallet import command, which also needs to support the usage of private_key + password in the command line. This way, users can deploy their own Sway contracts with a single command using their own wallets.

If there are no issues with the code patch here (https://github.com/FuelLabs/forc-wallet/pull/201/files), I will submit another patch for the wallet import today to support this feature.

For more information, please refer to the website: https://docs.amphitheatre.app/web3-developer-guide/blockchains/

@sdankel
Copy link
Member

sdankel commented Sep 11, 2024

It looks like the foundry wallet CLI has an --unsafe-password option, and also lets you set a password via environment variable. We could go this route.

The CLI argument really isn't safe an shouldn't be used in any production code other than tests. Perhaps we should only allow the env variable option. Would that satisfy your use case @lispking ?

@lispking
Copy link
Author

lispking commented Sep 11, 2024

It looks like the foundry wallet CLI has an --unsafe-password option, and also lets you set a password via environment variable. We could go this route.

The CLI argument really isn't safe an shouldn't be used in any production code other than tests. Perhaps we should only allow the env variable option. Would that satisfy your use case @lispking ?

yes, @sdankel you can. It has been modified. The code is here.

@sdankel
Copy link
Member

sdankel commented Sep 11, 2024

To clarify, I think we should still not add a CLI argument, but instead support reading the password from an environment variable like FORC_WALLET_PASSWORD.

@lispking
Copy link
Author

lispking commented Sep 11, 2024

To clarify, I think we should still not add a CLI argument, but instead support reading the password from an environment variable like FORC_WALLET_PASSWORD.

Yeah, Using environment variables is also an option, but the CLI tool could be extended to support configuring some environment variables, thereby giving the choice to the user.

A good practice is to support both environment variables and command-line options; many tools implement it this way. @sdankel

@lispking
Copy link
Author

Certainly, I hope that there won't be too much interactive information printed here, so my patch also adds a silent capability.

@sdankel
Copy link
Member

sdankel commented Sep 11, 2024

A good practice is to support both environment variables and command-line options; many tools implement it this way. @sdankel

That's generally true but not for passwords.

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

No branches or pull requests

3 participants