aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--src/rpc/client.cpp1
-rw-r--r--src/wallet/rpcwallet.cpp80
-rwxr-xr-xtest/functional/listsinceblock.py190
3 files changed, 240 insertions, 31 deletions
diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp
index 7c75586d03..4179453782 100644
--- a/src/rpc/client.cpp
+++ b/src/rpc/client.cpp
@@ -68,6 +68,7 @@ static const CRPCConvertParam vRPCConvertParams[] =
{ "getblocktemplate", 0, "template_request" },
{ "listsinceblock", 1, "target_confirmations" },
{ "listsinceblock", 2, "include_watchonly" },
+ { "listsinceblock", 3, "include_removed" },
{ "sendmany", 1, "amounts" },
{ "sendmany", 2, "minconf" },
{ "sendmany", 4, "subtractfeefrom" },
diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp
index 2b7b4085ee..2c8e0285f9 100644
--- a/src/wallet/rpcwallet.cpp
+++ b/src/wallet/rpcwallet.cpp
@@ -1426,6 +1426,17 @@ static void MaybePushAddress(UniValue & entry, const CTxDestination &dest)
entry.push_back(Pair("address", addr.ToString()));
}
+/**
+ * List transactions based on the given criteria.
+ *
+ * @param pwallet The wallet.
+ * @param wtx The wallet transaction.
+ * @param strAccount The account, if any, or "*" for all.
+ * @param nMinDepth The minimum confirmation depth.
+ * @param fLong Whether to include the JSON version of the transaction.
+ * @param ret The UniValue into which the result is stored.
+ * @param filter The "is mine" filter bool.
+ */
void ListTransactions(CWallet* const pwallet, const CWalletTx& wtx, const std::string& strAccount, int nMinDepth, bool fLong, UniValue& ret, const isminefilter& filter)
{
CAmount nFee;
@@ -1742,14 +1753,18 @@ UniValue listsinceblock(const JSONRPCRequest& request)
return NullUniValue;
}
- if (request.fHelp || request.params.size() > 3)
+ if (request.fHelp || request.params.size() > 4)
throw std::runtime_error(
- "listsinceblock ( \"blockhash\" target_confirmations include_watchonly)\n"
- "\nGet all transactions in blocks since block [blockhash], or all transactions if omitted\n"
+ "listsinceblock ( \"blockhash\" target_confirmations include_watchonly include_removed )\n"
+ "\nGet all transactions in blocks since block [blockhash], or all transactions if omitted.\n"
+ "If \"blockhash\" is no longer a part of the main chain, transactions from the fork point onward are included.\n"
+ "Additionally, if include_removed is set, transactions affecting the wallet which were removed are returned in the \"removed\" array.\n"
"\nArguments:\n"
"1. \"blockhash\" (string, optional) The block hash to list transactions since\n"
- "2. target_confirmations: (numeric, optional) The confirmations required, must be 1 or more\n"
- "3. include_watchonly: (bool, optional, default=false) Include transactions to watch-only addresses (see 'importaddress')"
+ "2. target_confirmations: (numeric, optional, default=1) The confirmations required, must be 1 or more\n"
+ "3. include_watchonly: (bool, optional, default=false) Include transactions to watch-only addresses (see 'importaddress')\n"
+ "4. include_removed: (bool, optional, default=true) Show transactions that were removed due to a reorg in the \"removed\" array\n"
+ " (not guaranteed to work on pruned nodes)\n"
"\nResult:\n"
"{\n"
" \"transactions\": [\n"
@@ -1774,7 +1789,11 @@ UniValue listsinceblock(const JSONRPCRequest& request)
" \"comment\": \"...\", (string) If a comment is associated with the transaction.\n"
" \"label\" : \"label\" (string) A comment for the address/transaction, if any\n"
" \"to\": \"...\", (string) If a comment to is associated with the transaction.\n"
- " ],\n"
+ " ],\n"
+ " \"removed\": [\n"
+ " <structure is the same as \"transactions\" above, only present if include_removed=true>\n"
+ " Note: transactions that were readded in the active chain will appear as-is in this array, and may thus have a positive confirmation count.\n"
+ " ],\n"
" \"lastblock\": \"lastblockhash\" (string) The hash of the last block\n"
"}\n"
"\nExamples:\n"
@@ -1785,21 +1804,19 @@ UniValue listsinceblock(const JSONRPCRequest& request)
LOCK2(cs_main, pwallet->cs_wallet);
- const CBlockIndex *pindex = NULL;
+ const CBlockIndex* pindex = NULL; // Block index of the specified block or the common ancestor, if the block provided was in a deactivated chain.
+ const CBlockIndex* paltindex = NULL; // Block index of the specified block, even if it's in a deactivated chain.
int target_confirms = 1;
isminefilter filter = ISMINE_SPENDABLE;
- if (!request.params[0].isNull())
- {
+ if (!request.params[0].isNull()) {
uint256 blockId;
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)
- {
+ if (it != mapBlockIndex.end()) {
+ paltindex = 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
@@ -1808,19 +1825,20 @@ UniValue listsinceblock(const JSONRPCRequest& request)
}
}
- if (!request.params[1].isNull())
- {
+ if (!request.params[1].isNull()) {
target_confirms = request.params[1].get_int();
- if (target_confirms < 1)
+ if (target_confirms < 1) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter");
+ }
}
- if (request.params.size() > 2 && request.params[2].get_bool())
- {
+ if (!request.params[2].isNull() && request.params[2].get_bool()) {
filter = filter | ISMINE_WATCH_ONLY;
}
+ bool include_removed = (request.params[3].isNull() || request.params[3].get_bool());
+
int depth = pindex ? (1 + chainActive.Height() - pindex->nHeight) : -1;
UniValue transactions(UniValue::VARR);
@@ -1828,8 +1846,27 @@ UniValue listsinceblock(const JSONRPCRequest& request)
for (const std::pair<uint256, CWalletTx>& pairWtx : pwallet->mapWallet) {
CWalletTx tx = pairWtx.second;
- if (depth == -1 || tx.GetDepthInMainChain() < depth)
+ if (depth == -1 || tx.GetDepthInMainChain() < depth) {
ListTransactions(pwallet, tx, "*", 0, true, transactions, filter);
+ }
+ }
+
+ // when a reorg'd block is requested, we also list any relevant transactions
+ // in the blocks of the chain that was detached
+ UniValue removed(UniValue::VARR);
+ while (include_removed && paltindex && paltindex != pindex) {
+ CBlock block;
+ if (!ReadBlockFromDisk(block, paltindex, Params().GetConsensus())) {
+ throw JSONRPCError(RPC_INTERNAL_ERROR, "Can't read block from disk");
+ }
+ for (const CTransactionRef& tx : block.vtx) {
+ if (pwallet->mapWallet.count(tx->GetHash()) > 0) {
+ // We want all transactions regardless of confirmation count to appear here,
+ // even negative confirmation ones, hence the big negative.
+ ListTransactions(pwallet, pwallet->mapWallet[tx->GetHash()], "*", -100000000, true, removed, filter);
+ }
+ }
+ paltindex = paltindex->pprev;
}
CBlockIndex *pblockLast = chainActive[chainActive.Height() + 1 - target_confirms];
@@ -1837,6 +1874,7 @@ UniValue listsinceblock(const JSONRPCRequest& request)
UniValue ret(UniValue::VOBJ);
ret.push_back(Pair("transactions", transactions));
+ if (include_removed) ret.push_back(Pair("removed", removed));
ret.push_back(Pair("lastblock", lastblock.GetHex()));
return ret;
@@ -3117,7 +3155,7 @@ static const CRPCCommand commands[] =
{ "wallet", "listlockunspent", &listlockunspent, false, {} },
{ "wallet", "listreceivedbyaccount", &listreceivedbyaccount, false, {"minconf","include_empty","include_watchonly"} },
{ "wallet", "listreceivedbyaddress", &listreceivedbyaddress, false, {"minconf","include_empty","include_watchonly"} },
- { "wallet", "listsinceblock", &listsinceblock, false, {"blockhash","target_confirmations","include_watchonly"} },
+ { "wallet", "listsinceblock", &listsinceblock, false, {"blockhash","target_confirmations","include_watchonly","include_removed"} },
{ "wallet", "listtransactions", &listtransactions, false, {"account","count","skip","include_watchonly"} },
{ "wallet", "listunspent", &listunspent, false, {"minconf","maxconf","addresses","include_unsafe","query_options"} },
{ "wallet", "listwallets", &listwallets, true, {} },
diff --git a/test/functional/listsinceblock.py b/test/functional/listsinceblock.py
index f3d41e573e..ce2d556ef0 100755
--- a/test/functional/listsinceblock.py
+++ b/test/functional/listsinceblock.py
@@ -14,7 +14,15 @@ class ListSinceBlockTest (BitcoinTestFramework):
self.setup_clean_chain = True
self.num_nodes = 4
- def run_test (self):
+ def run_test(self):
+ self.nodes[2].generate(101)
+ self.sync_all()
+
+ self.test_reorg()
+ self.test_double_spend()
+ self.test_double_send()
+
+ def test_reorg(self):
'''
`listsinceblock` did not behave correctly when handed a block that was
no longer in the main chain:
@@ -43,14 +51,6 @@ class ListSinceBlockTest (BitcoinTestFramework):
This test only checks that [tx0] is present.
'''
- 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()
@@ -73,7 +73,177 @@ class ListSinceBlockTest (BitcoinTestFramework):
if tx['txid'] == senttx:
found = True
break
- assert_equal(found, True)
+ assert found
+
+ def test_double_spend(self):
+ '''
+ This tests the case where the same UTXO is spent twice on two separate
+ blocks as part of a reorg.
+
+ ab0
+ / \
+ aa1 [tx1] bb1 [tx2]
+ | |
+ aa2 bb2
+ | |
+ aa3 bb3
+ |
+ bb4
+
+ Problematic case:
+
+ 1. User 1 receives BTC in tx1 from utxo1 in block aa1.
+ 2. User 2 receives BTC in tx2 from utxo1 (same) in block bb1
+ 3. User 1 sees 2 confirmations at block aa3.
+ 4. Reorg into bb chain.
+ 5. User 1 asks `listsinceblock aa3` and does not see that tx1 is now
+ invalidated.
+
+ Currently the solution to this is to detect that a reorg'd block is
+ asked for in listsinceblock, and to iterate back over existing blocks up
+ until the fork point, and to include all transactions that relate to the
+ node wallet.
+ '''
+
+ self.sync_all()
+
+ # Split network into two
+ self.split_network()
+
+ # share utxo between nodes[1] and nodes[2]
+ utxos = self.nodes[2].listunspent()
+ utxo = utxos[0]
+ privkey = self.nodes[2].dumpprivkey(utxo['address'])
+ self.nodes[1].importprivkey(privkey)
+
+ # send from nodes[1] using utxo to nodes[0]
+ change = '%.8f' % (float(utxo['amount']) - 1.0003)
+ recipientDict = {
+ self.nodes[0].getnewaddress(): 1,
+ self.nodes[1].getnewaddress(): change,
+ }
+ utxoDicts = [{
+ 'txid': utxo['txid'],
+ 'vout': utxo['vout'],
+ }]
+ txid1 = self.nodes[1].sendrawtransaction(
+ self.nodes[1].signrawtransaction(
+ self.nodes[1].createrawtransaction(utxoDicts, recipientDict))['hex'])
+
+ # send from nodes[2] using utxo to nodes[3]
+ recipientDict2 = {
+ self.nodes[3].getnewaddress(): 1,
+ self.nodes[2].getnewaddress(): change,
+ }
+ self.nodes[2].sendrawtransaction(
+ self.nodes[2].signrawtransaction(
+ self.nodes[2].createrawtransaction(utxoDicts, recipientDict2))['hex'])
+
+ # generate on both sides
+ lastblockhash = self.nodes[1].generate(3)[2]
+ self.nodes[2].generate(4)
+
+ self.join_network()
+
+ self.sync_all()
+
+ # gettransaction should work for txid1
+ assert self.nodes[0].gettransaction(txid1)['txid'] == txid1, "gettransaction failed to find txid1"
+
+ # listsinceblock(lastblockhash) should now include txid1, as seen from nodes[0]
+ lsbres = self.nodes[0].listsinceblock(lastblockhash)
+ assert any(tx['txid'] == txid1 for tx in lsbres['removed'])
+
+ # but it should not include 'removed' if include_removed=false
+ lsbres2 = self.nodes[0].listsinceblock(blockhash=lastblockhash, include_removed=False)
+ assert 'removed' not in lsbres2
+
+ def test_double_send(self):
+ '''
+ This tests the case where the same transaction is submitted twice on two
+ separate blocks as part of a reorg. The former will vanish and the
+ latter will appear as the true transaction (with confirmations dropping
+ as a result).
+
+ ab0
+ / \
+ aa1 [tx1] bb1
+ | |
+ aa2 bb2
+ | |
+ aa3 bb3 [tx1]
+ |
+ bb4
+
+ Asserted:
+
+ 1. tx1 is listed in listsinceblock.
+ 2. It is included in 'removed' as it was removed, even though it is now
+ present in a different block.
+ 3. It is listed with a confirmations count of 2 (bb3, bb4), not
+ 3 (aa1, aa2, aa3).
+ '''
+
+ self.sync_all()
+
+ # Split network into two
+ self.split_network()
+
+ # create and sign a transaction
+ utxos = self.nodes[2].listunspent()
+ utxo = utxos[0]
+ change = '%.8f' % (float(utxo['amount']) - 1.0003)
+ recipientDict = {
+ self.nodes[0].getnewaddress(): 1,
+ self.nodes[2].getnewaddress(): change,
+ }
+ utxoDicts = [{
+ 'txid': utxo['txid'],
+ 'vout': utxo['vout'],
+ }]
+ signedtxres = self.nodes[2].signrawtransaction(
+ self.nodes[2].createrawtransaction(utxoDicts, recipientDict))
+ assert signedtxres['complete']
+
+ signedtx = signedtxres['hex']
+
+ # send from nodes[1]; this will end up in aa1
+ txid1 = self.nodes[1].sendrawtransaction(signedtx)
+
+ # generate bb1-bb2 on right side
+ self.nodes[2].generate(2)
+
+ # send from nodes[2]; this will end up in bb3
+ txid2 = self.nodes[2].sendrawtransaction(signedtx)
+
+ assert_equal(txid1, txid2)
+
+ # generate on both sides
+ lastblockhash = self.nodes[1].generate(3)[2]
+ self.nodes[2].generate(2)
+
+ self.join_network()
+
+ self.sync_all()
+
+ # gettransaction should work for txid1
+ self.nodes[0].gettransaction(txid1)
+
+ # listsinceblock(lastblockhash) should now include txid1 in transactions
+ # as well as in removed
+ lsbres = self.nodes[0].listsinceblock(lastblockhash)
+ assert any(tx['txid'] == txid1 for tx in lsbres['transactions'])
+ assert any(tx['txid'] == txid1 for tx in lsbres['removed'])
+
+ # find transaction and ensure confirmations is valid
+ for tx in lsbres['transactions']:
+ if tx['txid'] == txid1:
+ assert_equal(tx['confirmations'], 2)
+
+ # the same check for the removed array; confirmations should STILL be 2
+ for tx in lsbres['removed']:
+ if tx['txid'] == txid1:
+ assert_equal(tx['confirmations'], 2)
if __name__ == '__main__':
ListSinceBlockTest().main()
generated by cgit v1.2.3 (git 2.26.2) at 2024-07-07 20:12:28 +0000