From 6c37f7fd78832442a26e56bd0787974927df4fb2 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Mon, 16 Jun 2014 14:45:32 +0200 Subject: [PATCH] `getrawchangeaddress` should fail when keypool exhausted An user on IRC reported an issue where `getrawchangeaddress` keeps returning a single address when the keypool is exhausted. In my opinion this is strange behaviour. - Change CReserveKey to fail when running out of keys in the keypool. - Make `getrawchangeaddress` return RPC_WALLET_KEYPOOL_RAN_OUT when unable to create an address. - Add a Python RPC test for checking the keypool behaviour in combination with encrypted wallets. --- qa/rpc-tests/keypool.py | 132 ++++++++++++++++++++++++++++++++++++++++ src/miner.cpp | 3 + src/rpcwallet.cpp | 2 +- src/wallet.cpp | 6 +- 4 files changed, 137 insertions(+), 6 deletions(-) create mode 100755 qa/rpc-tests/keypool.py diff --git a/qa/rpc-tests/keypool.py b/qa/rpc-tests/keypool.py new file mode 100755 index 00000000..86ad20de --- /dev/null +++ b/qa/rpc-tests/keypool.py @@ -0,0 +1,132 @@ +#!/usr/bin/env python +# Copyright (c) 2014 The Bitcoin Core developers +# Distributed under the MIT/X11 software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. + +# Exercise the wallet keypool, and interaction with wallet encryption/locking + +# Add python-bitcoinrpc to module search path: +import os +import sys +sys.path.append(os.path.join(os.path.dirname(os.path.abspath(__file__)), "python-bitcoinrpc")) + +import json +import shutil +import subprocess +import tempfile +import traceback + +from bitcoinrpc.authproxy import AuthServiceProxy, JSONRPCException +from util import * + + +def check_array_result(object_array, to_match, expected): + """ + Pass in array of JSON objects, a dictionary with key/value pairs + to match against, and another dictionary with expected key/value + pairs. + """ + num_matched = 0 + for item in object_array: + all_match = True + for key,value in to_match.items(): + if item[key] != value: + all_match = False + if not all_match: + continue + for key,value in expected.items(): + if item[key] != value: + raise AssertionError("%s : expected %s=%s"%(str(item), str(key), str(value))) + num_matched = num_matched+1 + if num_matched == 0: + raise AssertionError("No objects matched %s"%(str(to_match))) + +def run_test(nodes, tmpdir): + # Encrypt wallet and wait to terminate + nodes[0].encryptwallet('test') + bitcoind_processes[0].wait() + # Restart node 0 + nodes[0] = start_node(0, tmpdir) + # Keep creating keys + addr = nodes[0].getnewaddress() + try: + addr = nodes[0].getnewaddress() + raise AssertionError('Keypool should be exhausted after one address') + except JSONRPCException,e: + assert(e.error['code']==-12) + + # put three new keys in the keypool + nodes[0].walletpassphrase('test', 12000) + nodes[0].keypoolrefill(3) + nodes[0].walletlock() + + # drain the keys + addr = set() + addr.add(nodes[0].getrawchangeaddress()) + addr.add(nodes[0].getrawchangeaddress()) + addr.add(nodes[0].getrawchangeaddress()) + addr.add(nodes[0].getrawchangeaddress()) + # assert that four unique addresses were returned + assert(len(addr) == 4) + # the next one should fail + try: + addr = nodes[0].getrawchangeaddress() + raise AssertionError('Keypool should be exhausted after three addresses') + except JSONRPCException,e: + assert(e.error['code']==-12) + + +def main(): + import optparse + + parser = optparse.OptionParser(usage="%prog [options]") + parser.add_option("--nocleanup", dest="nocleanup", default=False, action="store_true", + help="Leave bitcoinds and test.* datadir on exit or error") + parser.add_option("--srcdir", dest="srcdir", default="../../src", + help="Source directory containing bitcoind/bitcoin-cli (default: %default%)") + parser.add_option("--tmpdir", dest="tmpdir", default=tempfile.mkdtemp(prefix="test"), + help="Root directory for datadirs") + (options, args) = parser.parse_args() + + os.environ['PATH'] = options.srcdir+":"+os.environ['PATH'] + + check_json_precision() + + success = False + nodes = [] + try: + print("Initializing test directory "+options.tmpdir) + if not os.path.isdir(options.tmpdir): + os.makedirs(options.tmpdir) + initialize_chain(options.tmpdir) + + nodes = start_nodes(1, options.tmpdir) + + run_test(nodes, options.tmpdir) + + success = True + + except AssertionError as e: + print("Assertion failed: "+e.message) + except JSONRPCException as e: + print("JSONRPC error: "+e.error['message']) + traceback.print_tb(sys.exc_info()[2]) + except Exception as e: + print("Unexpected exception caught during testing: "+str(sys.exc_info()[0])) + traceback.print_tb(sys.exc_info()[2]) + + if not options.nocleanup: + print("Cleaning up") + stop_nodes(nodes) + wait_bitcoinds() + shutil.rmtree(options.tmpdir) + + if success: + print("Tests successful") + sys.exit(0) + else: + print("Failed") + sys.exit(1) + +if __name__ == '__main__': + main() diff --git a/src/miner.cpp b/src/miner.cpp index 17918a12..ec56c711 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -467,7 +467,10 @@ void static BitcoinMiner(CWallet *pwallet) auto_ptr pblocktemplate(CreateNewBlockWithKey(reservekey)); if (!pblocktemplate.get()) + { + LogPrintf("Error in BitcoinMiner: Keypool ran out, please call keypoolrefill before restarting the mining thread\n"); return; + } CBlock *pblock = &pblocktemplate->block; IncrementExtraNonce(pblock, pindexPrev, nExtraNonce); diff --git a/src/rpcwallet.cpp b/src/rpcwallet.cpp index 1e461290..e8c62fd3 100644 --- a/src/rpcwallet.cpp +++ b/src/rpcwallet.cpp @@ -201,7 +201,7 @@ Value getrawchangeaddress(const Array& params, bool fHelp) CReserveKey reservekey(pwalletMain); CPubKey vchPubKey; if (!reservekey.GetReservedKey(vchPubKey)) - throw JSONRPCError(RPC_WALLET_ERROR, "Error: Unable to obtain key for change"); + throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, "Error: Keypool ran out, please call keypoolrefill first"); reservekey.KeepKey(); diff --git a/src/wallet.cpp b/src/wallet.cpp index a54494f9..8fdc5f4b 100644 --- a/src/wallet.cpp +++ b/src/wallet.cpp @@ -2010,11 +2010,7 @@ bool CReserveKey::GetReservedKey(CPubKey& pubkey) if (nIndex != -1) vchPubKey = keypool.vchPubKey; else { - if (pwallet->vchDefaultKey.IsValid()) { - LogPrintf("CReserveKey::GetReservedKey(): Warning: Using default key instead of a new key, top up your keypool!"); - vchPubKey = pwallet->vchDefaultKey; - } else - return false; + return false; } } assert(vchPubKey.IsValid());