From ee5c1ce5a6155d502d8e779d3490d6cf66374c3f Mon Sep 17 00:00:00 2001 From: Karl-Johan Alm Date: Wed, 11 Jan 2017 15:31:16 +0900 Subject: [PATCH 1/2] Bug-fix: listsinceblock: use closest common ancestor when a block hash was provided for a chain that was not the main chain. --- src/wallet/rpcwallet.cpp | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 2428ef04e..f99aed519 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -1679,7 +1679,7 @@ UniValue listsinceblock(const JSONRPCRequest& request) LOCK2(cs_main, pwalletMain->cs_wallet); - CBlockIndex *pindex = NULL; + const CBlockIndex *pindex = NULL; int target_confirms = 1; isminefilter filter = ISMINE_SPENDABLE; @@ -1690,7 +1690,16 @@ UniValue listsinceblock(const JSONRPCRequest& request) blockId.SetHex(request.params[0].get_str()); BlockMap::iterator it = mapBlockIndex.find(blockId); if (it != mapBlockIndex.end()) + { pindex = it->second; + if (chainActive[pindex->nHeight] != pindex) + { + // the block being asked for is a part of a deactivated chain; + // we don't want to depend on its perceived height in the block + // chain, we want to instead use the last common ancestor + pindex = chainActive.FindFork(pindex); + } + } } if (request.params.size() > 1) @@ -1701,9 +1710,10 @@ UniValue listsinceblock(const JSONRPCRequest& request) throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter"); } - if(request.params.size() > 2) - if(request.params[2].get_bool()) - filter = filter | ISMINE_WATCH_ONLY; + if (request.params.size() > 2 && request.params[2].get_bool()) + { + filter = filter | ISMINE_WATCH_ONLY; + } int depth = pindex ? (1 + chainActive.Height() - pindex->nHeight) : -1; From 7ba0a00aae03b438e63d0ada9ff68a7618c7f907 Mon Sep 17 00:00:00 2001 From: Karl-Johan Alm Date: Wed, 11 Jan 2017 20:02:25 +0900 Subject: [PATCH 2/2] Testing: listsinceblock should not use orphan block height. --- qa/pull-tester/rpc-tests.py | 1 + qa/rpc-tests/listsinceblock.py | 80 ++++++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+) create mode 100755 qa/rpc-tests/listsinceblock.py diff --git a/qa/pull-tester/rpc-tests.py b/qa/pull-tester/rpc-tests.py index 83b6bdfe4..f3aa34a44 100755 --- a/qa/pull-tester/rpc-tests.py +++ b/qa/pull-tester/rpc-tests.py @@ -152,6 +152,7 @@ testScripts = [ 'nulldummy.py', 'import-rescan.py', 'rpcnamedargs.py', + 'listsinceblock.py', ] if ENABLE_ZMQ: testScripts.append('zmq_test.py') diff --git a/qa/rpc-tests/listsinceblock.py b/qa/rpc-tests/listsinceblock.py new file mode 100755 index 000000000..ca67b8ece --- /dev/null +++ b/qa/rpc-tests/listsinceblock.py @@ -0,0 +1,80 @@ +#!/usr/bin/env python3 +# Copyright (c) 2017 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. + +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import assert_equal + +class ListSinceBlockTest (BitcoinTestFramework): + + def __init__(self): + super().__init__() + self.setup_clean_chain = True + self.num_nodes = 4 + + def run_test (self): + ''' + `listsinceblock` did not behave correctly when handed a block that was + no longer in the main chain: + + ab0 + / \ + aa1 [tx0] bb1 + | | + aa2 bb2 + | | + aa3 bb3 + | + bb4 + + Consider a client that has only seen block `aa3` above. It asks the node + to `listsinceblock aa3`. But at some point prior the main chain switched + to the bb chain. + + Previously: listsinceblock would find height=4 for block aa3 and compare + this to height=5 for the tip of the chain (bb4). It would then return + results restricted to bb3-bb4. + + Now: listsinceblock finds the fork at ab0 and returns results in the + range bb1-bb4. + + This test only checks that [tx0] is present. + ''' + + assert_equal(self.is_network_split, False) + self.nodes[2].generate(101) + self.sync_all() + + assert_equal(self.nodes[0].getbalance(), 0) + assert_equal(self.nodes[1].getbalance(), 0) + assert_equal(self.nodes[2].getbalance(), 50) + assert_equal(self.nodes[3].getbalance(), 0) + + # Split network into two + self.split_network() + assert_equal(self.is_network_split, True) + + # send to nodes[0] from nodes[2] + senttx = self.nodes[2].sendtoaddress(self.nodes[0].getnewaddress(), 1) + + # generate on both sides + lastblockhash = self.nodes[1].generate(6)[5] + self.nodes[2].generate(7) + print('lastblockhash=%s' % (lastblockhash)) + + self.sync_all() + + self.join_network() + + # listsinceblock(lastblockhash) should now include tx, as seen from nodes[0] + lsbres = self.nodes[0].listsinceblock(lastblockhash) + found = False + for tx in lsbres['transactions']: + if tx['txid'] == senttx: + found = True + break + assert_equal(found, True) + +if __name__ == '__main__': + ListSinceBlockTest().main()