search

FuelLabs / forc-wallet

A forc plugin for managing Fuel wallets.
98 stars 54 forks source link

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

Open lispking opened 1 week ago

lispking commented 1 week ago

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 commented 1 week ago

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

kayagokalp commented 1 week ago

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 commented 1 week ago

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 commented 1 week ago

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 commented 1 week ago

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 commented 1 week ago

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 commented 1 week ago

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 commented 1 week ago

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 commented 1 week ago

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

sdankel commented 1 week ago

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.