Logic running with -keypool=0 was wrong (empty keys were being returned). #467

pull gavinandresen wants to merge 3 commits into bitcoin:master from gavinandresen:keypoolzero changing 5 files +47 −27

gavinandresen commented at 8:37 PM on August 12, 2011: contributor

Logic running with -keypool=0 was wrong (empty keys were being returned). Fixes #445

sipa, Matt: can you sanity check this? I did some quick testing with/without wallet encryption, running with -keypool=0.

Logic is: GetOrReuseKey method should never return empty key.

And GetAccountAddress should always complain if GetOrReuseKey has to reuse (== return the default key).
sipa commented at 3:19 PM on August 14, 2011: member

I haven't tested it, but it looks correct - a bit of a hack though. Wouldn't it be nicer to have a

bool GetKeyFromPool(vector<unsigned char> &vchPubKey, bool fAllowReuse = true)

function which can fail, and call that with fAllowReuse=false in GetAccountAddress?
gavinandresen commented at 10:20 PM on August 14, 2011: contributor

Much less hacky to rename GetOrReuse to GetKeyFromPool with a flag. I'll do that.

in src/wallet.cpp:None in 54df7f0a39 outdated

1315 | +        {
1316 | +            result = vchDefaultKey;
1317 | +            return true;
1318 | +        }
1319 | +        if (IsLocked()) return false;
1320 | +        result = GenerateNewKey();

TheBlueMatt commented at 4:28 PM on August 30, 2011:

Might want to put a cs around this

TheBlueMatt commented at 4:28 PM on August 30, 2011: member

Aside from maybe adding a lock around IsLocked and GenerateNewKey, looks good
gavinandresen commented at 8:47 PM on August 30, 2011: contributor

Thanks. RE: locks: I'm in the middle of reworking the wallet/keystore locks to fix potential deadlocks; should be done with that work tomorrow.
Logic running with -keypool=0 was wrong (empty keys were being returned). Fixes #445
Renames GetOrReuseKeyFromKeyPool to GetKeyFromPool, with fAllowReuse arg and bool result.
7db3b75b3e
Fix RPC call name in error message. 123e5bd998
obtain cs_wallet mutex to protect vchDefaultKey ed02c95d50
gavinandresen referenced this in commit 783c636c73 on Sep 1, 2011
gavinandresen merged this on Sep 1, 2011
gavinandresen closed this on Sep 1, 2011
coblee referenced this in commit a1293fffe0 on Jul 17, 2012
zathras-crypto referenced this in commit abd65329d9 on May 9, 2017
kallewoof referenced this in commit cecfe80cc5 on Oct 4, 2019
lateminer referenced this in commit d802c24dc7 on Oct 16, 2019
rajarshimaitra referenced this in commit 05f16dd77f on Aug 5, 2021
DrahtBot locked this on Sep 8, 2021

Contributors

gavinandresen

sipa

TheBlueMatt

Linked (view graph)

#445 -keypool=0 leads to permanent coin loss