generate RPC call leaves reservekey in keypool #6268

1. 
   gavinandresen commented at 7:15 pm on June 10, 2015: contributor

   A regression test I’m writing failed because I was using invalidateblock() and then re-generating another chain.

   But I got an identical block hash, so the newly-generated block was immediately rejected. I believe the root cause is the generate RPC call not calling reservekey.keepKey() to remove it from the keypool.

   I’m opening this issue in case I missed some discussion when generate was implemented, and this is the intended behavior. Fixing it will slow down regression tests that call generate repeatedly (because new keys will be created instead of the same key being used repeatedly).

2. gavinandresen added the label Tests on Jun 10, 2015
3. gavinandresen added the label RPC on Jun 10, 2015
4. 
   jonasschnelli commented at 7:33 pm on June 10, 2015: contributor

   Just checked and right: generate does not use keepKey(). PR #5994 would fix this (next to decoupling wallet / miner). I’m also happy to write a standalone-fix for this issue if no one already started.
5. 
   laanwj commented at 8:29 am on June 11, 2015: member

   I think this was done on purpose, to avoid having to generate a new key for each block and thus speed up generation. @sipa do you remember?

   Edit: but of course, if you seriously need this, we could just make it a flag.

6. 
   dexX7 commented at 11:56 pm on June 13, 2015: contributor

   If I’m not mistaken, then this was introduced by https://github.com/bitcoin/bitcoin/commit/18379875bf7dd91637e4a206db58cd0968b5d47f.

   For me this was pretty confusing, because it has an impact on "getnewaddress", and the “new” address may already have received mined coins. To reproduce in regtest mode, starting from scratch:

    0./src/bitcoin-cli "generate" 101
    1[
    2  # block hashes
    3]
    4./src/bitcoin-cli "listunspent"
    5[
    6  {
    7    "txid": "b148b7d13c3351875d070d0567855bce47fcc797b3504247c4fcd68d902523ab",
    8    "vout": 0,
    9    "address": "mu1ZwmfRH4HHowDrrjSTsqSfMKLBeZkn2a",
   10    "scriptPubKey": "2103ad57ff9cd560507938698630455ab926b070add217a3b30aead57cd578b935feac",
   11    "amount": 50.00000000,
   12    "confirmations": 101,
   13    "spendable": true
   14  }
   15]
   16./src/bitcoin-cli "getnewaddress"   # ... or "getrawchangeaddress"
   17mu1ZwmfRH4HHowDrrjSTsqSfMKLBeZkn2a  # the new address isn't empty, but has 50.0 BTC
   

   Additional coins will be mined to that address, too, if "generate" is used again.

7. 
   sipa commented at 12:03 pm on June 14, 2015: member

   Using the same key for multiple blocks was intentional afaik, not marking it as used at all is not. I’ll have a look later.
8. 
   TheBlueMatt commented at 8:28 am on June 15, 2015: member

   Heh, I ran into this exact same bug the other day writing a unit test as well :). Anyway, yea, we should use the same key for multiple blocks since they won’t be the same anyway, but we should be keeping the key, otherwise things break :).
9. 
   TheBlueMatt commented at 8:28 am on June 15, 2015: member

   (Also, I have a fix for this coming in the next frt pull, soon)
10. TheBlueMatt referenced this in commit 0e3f76077b on Jul 10, 2015
11. 
    TheBlueMatt commented at 11:10 pm on July 17, 2015: member

    This was fixed in https://github.com/bitcoin/bitcoin/commit/5496253966abec287ea61a648cb518d14903f91f#diff-c2c990fee1c3381462640e80ae7db0d0L168
12. laanwj closed this on Jul 21, 2015

    ---

13. 
    laanwj commented at 10:48 am on July 21, 2015: member

    Thanks for noticing @TheBlueMatt
14. MarcoFalke locked this on Sep 8, 2021

Contributors
gavinandresen jonasschnelli laanwj dexX7 sipa TheBlueMatt

Labels
Tests RPC/REST/ZMQ