Skip to content

Conversation

@jnewbery
Copy link
Contributor

@jnewbery jnewbery commented Jun 28, 2018

A bug in the initial implementation of loadwallet meant that if the
arguement was a directory that didn't contain a wallet.dat file, a new
wallet would be created in that directory. Fix that so that if a
directory is passed in, it must contain a wallet.dat file.

Bug reported by promag (João Barbosa).

A bug in the initial implementation of loadwallet meant that if the
arguement was a directory that didn't contain a wallet.dat file, a new
wallet would be created in that directory. Fix that so that if a
directory is passed in, it must contain a wallet.dat file.

Bug reported by promag (João Barbosa).
@jnewbery jnewbery force-pushed the dont_load_nonexistent_wallet branch from 9b9e421 to ea65182 Compare June 28, 2018 17:38
@jnewbery
Copy link
Contributor Author

cc @promag

Copy link
Contributor

@promag promag left a comment

Choose a reason for hiding this comment

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

Looks good, will test.

@maflcko maflcko added this to the 0.17.0 milestone Jun 28, 2018
@promag
Copy link
Contributor

promag commented Jun 29, 2018

FYI I'll have to move these checks to another place to reuse in the node interface (for the UI).

Could also test an absolute path?

@promag
Copy link
Contributor

promag commented Jun 30, 2018

Tested ACK ea65182. Expected behaviour with local or external wallets. Also works if the directory is a symlink. Just 2 nits:

  • test an absolute path (external wallet);
  • error could be invalid wallet - I mean, could avoid wallet.dat detail.

@laanwj
Copy link
Member

laanwj commented Jul 5, 2018

at some point we might want to put wallet.dat in some constant, instead of repeating the string everywhere. But not necessarily here.
utACK ea65182

@ken2812221
Copy link
Contributor

utACK ea65182

@maflcko maflcko merged commit ea65182 into bitcoin:master Jul 7, 2018
maflcko pushed a commit that referenced this pull request Jul 7, 2018
ea65182 [wallet] loadwallet shouldn't create new wallets. (John Newbery)

Pull request description:

  A bug in the initial implementation of loadwallet meant that if the
  arguement was a directory that didn't contain a wallet.dat file, a new
  wallet would be created in that directory. Fix that so that if a
  directory is passed in, it must contain a wallet.dat file.

  Bug reported by promag (João Barbosa).

Tree-SHA512: 0a59fa8a33fde51a88544ad288b00e4995284fe16424f643076aaba42b8244fff362145217650ee53d518dfab7efbed4237632c34cdd3dcbbecaa9ecaab5fd7b
@maflcko
Copy link
Member

maflcko commented Jul 7, 2018

utACK ea65182

jasonbcox pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Oct 25, 2019
Summary:
ea65182 [wallet] loadwallet shouldn't create new wallets. (John Newbery)

Pull request description:

  A bug in the initial implementation of loadwallet meant that if the
  arguement was a directory that didn't contain a wallet.dat file, a new
  wallet would be created in that directory. Fix that so that if a
  directory is passed in, it must contain a wallet.dat file.

  Bug reported by promag (João Barbosa).

Tree-SHA512: 0a59fa8a33fde51a88544ad288b00e4995284fe16424f643076aaba42b8244fff362145217650ee53d518dfab7efbed4237632c34cdd3dcbbecaa9ecaab5fd7b

Backport of Core PR13564
bitcoin/bitcoin#13564

Depends on D4236

Test Plan:
  make check
  test_runner.py

Reviewers: deadalnix, Fabien, jasonbcox, O1 Bitcoin ABC, #bitcoin_abc

Reviewed By: deadalnix, O1 Bitcoin ABC, #bitcoin_abc

Differential Revision: https://reviews.bitcoinabc.org/D4239
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jul 7, 2020
ea65182 [wallet] loadwallet shouldn't create new wallets. (John Newbery)

Pull request description:

  A bug in the initial implementation of loadwallet meant that if the
  arguement was a directory that didn't contain a wallet.dat file, a new
  wallet would be created in that directory. Fix that so that if a
  directory is passed in, it must contain a wallet.dat file.

  Bug reported by promag (João Barbosa).

Tree-SHA512: 0a59fa8a33fde51a88544ad288b00e4995284fe16424f643076aaba42b8244fff362145217650ee53d518dfab7efbed4237632c34cdd3dcbbecaa9ecaab5fd7b
UdjinM6 pushed a commit to dashpay/dash that referenced this pull request Jul 8, 2020
#3592)

ea65182 [wallet] loadwallet shouldn't create new wallets. (John Newbery)

Pull request description:

  A bug in the initial implementation of loadwallet meant that if the
  arguement was a directory that didn't contain a wallet.dat file, a new
  wallet would be created in that directory. Fix that so that if a
  directory is passed in, it must contain a wallet.dat file.

  Bug reported by promag (João Barbosa).

Tree-SHA512: 0a59fa8a33fde51a88544ad288b00e4995284fe16424f643076aaba42b8244fff362145217650ee53d518dfab7efbed4237632c34cdd3dcbbecaa9ecaab5fd7b

Co-authored-by: MarcoFalke <[email protected]>
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this pull request Aug 23, 2020
dashpay#3592)

ea65182 [wallet] loadwallet shouldn't create new wallets. (John Newbery)

Pull request description:

  A bug in the initial implementation of loadwallet meant that if the
  arguement was a directory that didn't contain a wallet.dat file, a new
  wallet would be created in that directory. Fix that so that if a
  directory is passed in, it must contain a wallet.dat file.

  Bug reported by promag (João Barbosa).

Tree-SHA512: 0a59fa8a33fde51a88544ad288b00e4995284fe16424f643076aaba42b8244fff362145217650ee53d518dfab7efbed4237632c34cdd3dcbbecaa9ecaab5fd7b

Co-authored-by: MarcoFalke <[email protected]>
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants