Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

ganache-cli doesn't really lock accounts #996

Closed
NicsTr opened this issue Oct 27, 2019 · 3 comments · Fixed by #1605
Closed

ganache-cli doesn't really lock accounts #996

NicsTr opened this issue Oct 27, 2019 · 3 comments · Fixed by #1605
Assignees
Milestone

Comments

@NicsTr
Copy link

NicsTr commented Oct 27, 2019

Expected Behavior

When using --secure, ganache-cli should not allow transactions from locked accounts.

Current Behavior

Transactions are done as if the accounts were not locked.

Possible Solution

Steps to Reproduce (for bugs)

  1. npx ganache-cli --secure --deterministic -p 8000 -a 11
  2. After right configuration of web3js' provider to ganache : web3.eth.personal.sendTransaction({from: "0xFFcf8FDEE72ac11b5c542428B35EEF5769C409f0", to: "0x90F8bf6A479f320ead074411a4B0e7944Ea8c9C1", value: web3.utils.toWei("1") }, '')

Context

Unless I wrongly understood how locked accounts should work, this seems like a pretty serious bug.
Please not that I figured a not so elegant workaround by using --db option and by relaunching ganache-cli with a smaller -a argument, causing it to initialize the 11 eleven accounts as expected but forbidding every transactions from an address that is not present in the second execution log.

Your Environment

  • Version used: v6.7.0
  • Version of Truffle/Remix/Other tools used: ganache-core 2.8.0, web3js 1.2.2
  • NodeJS Version: 10.16.3
@davidmurdoch
Copy link
Member

Because you are using personal.sendTransaction with a password of '' (the default password when locking accounts via the --secure flag) you are temporarily unlocking the account for this transaction. This is by design.

With that said, I think I agree with you that it shouldn't work this way. However, I'm not sure what the default password for accounts locked with --secure should be. What are your thoughts and expectations?

@NicsTr
Copy link
Author

NicsTr commented Oct 28, 2019

Oh, I surely didn't expect that kind of default behavior.

IMHO, It'll be less strange to add a command-line argument to set the password or use the seed to generate it (and display it) as for the private keys.

NicsTr referenced this issue in NicsTr/ganache-cli Oct 28, 2019
Avoid ambiguity as explained in #688
@davidmurdoch davidmurdoch transferred this issue from trufflesuite/ganache-cli-archive Aug 19, 2021
@leeftk leeftk added this to the 7.0.0 milestone Oct 6, 2021
@davidmurdoch
Copy link
Member

davidmurdoch commented Oct 6, 2021

For ganache v7, let's rename the --secure flag to --lock and --wallet.lock (while keeping the --secure flag for legacy reasons)

We'll also add a --passphrase flag. The --passphrase {string} flag by itself (sans --lock/--secure) will not automatically lock accounts and will not be an error. This enables using personal_lockAccount(address) to lock the account (with the user-specified default passphrase) after start up.

Using --lock/--secure without the --passphrase flag will continue using "" (empty string) as the passphrase.

Implementation detail:

We store account private keys that were created by personal_ methods in their encrypted form, but really don't need to do this for accounts created with the default "" passphrase, so let's take advantage of this and optimize for better start up time by storing the private key directly (unencrypted).

MicaiahReid added a commit that referenced this issue Nov 6, 2021
User-supplied passphrase is now used to encrypt wallet. If no
passphrase is supplied and the `--lock` options is not specified,
initial accounts are not encrypted to improve startup speeds.

A keyfile record is now added for every account. Previously, this
wasn't done, making it impossible to actually unlock initial
accounts. This fixes #996.

Updates api with the above changes to the wallet.
@MicaiahReid MicaiahReid self-assigned this Nov 6, 2021
MicaiahReid added a commit that referenced this issue Nov 17, 2021
User-supplied passphrase is now used to encrypt wallet. If no
passphrase is supplied and the `--lock` options is not specified,
initial accounts are not encrypted to improve startup speeds.

A keyfile record is now added for every account. Previously, this
wasn't done, making it impossible to actually unlock initial
accounts. This fixes #996.

Updates api with the above changes to the wallet.
MicaiahReid added a commit that referenced this issue Nov 17, 2021
User-supplied passphrase is now used to encrypt wallet. If no
passphrase is supplied and the `--lock` options is not specified,
initial accounts are not encrypted to improve startup speeds.

A keyfile record is now added for every account. Previously, this
wasn't done, making it impossible to actually unlock initial
accounts. This fixes #996.

Updates api with the above changes to the wallet.
MicaiahReid added a commit that referenced this issue Nov 17, 2021
User-supplied passphrase is now used to encrypt wallet. If no
passphrase is supplied and the `--lock` options is not specified,
initial accounts are not encrypted to improve startup speeds.

A keyfile record is now added for every account. Previously, this
wasn't done, making it impossible to actually unlock initial
accounts. This fixes #996.

Updates api with the above changes to the wallet.
@MicaiahReid MicaiahReid linked a pull request Nov 17, 2021 that will close this issue
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants