Skip to content

Commit bf0a08b

Browse files
author
MarcoFalke
committed
Merge #10330: [wallet] fix zapwallettxes interaction with persistent mempool
4c3b538 [logs] fix zapwallettxes startup logs (John Newbery) e7a2181 [wallet] fix zapwallettxes interaction with persistent mempool (John Newbery) ff7365e [tests] fix flake8 warnings in zapwallettxes.py (John Newbery) Pull request description: zapwallettxes previously did not interact well with persistent mempool. zapwallettxes would cause wallet transactions to be zapped, but they would then be reloaded from the mempool on startup. This commit softsets persistmempool to false if zapwallettxes is enabled so transactions are actually zapped. This PR also fixes the zapwallettxes.py functional test, which did not properly test this feature. The test line: ```py assert_raises(JSONRPCException, self.nodes[0].gettransaction, [txid3]) #there must be a expection because the unconfirmed wallettx0 must be gone by now ``` is not actually testing the presence of the transaction since the RPC is being called incorrectly (with an array instead of a string). The `assert_raises()` passes since an assert is raised, but it's not the one the test writer had in mind! Fixes #9710 . Tree-SHA512: e3236efc7a2fd2b3bf1d9e2e8a7726d470c57f5d95cf41b7bde264edc8817bd36a6f3feff52f8de8db0ef64b7247c88b24e7ff7cefaa706cba86fe4e2135a508
2 parents 3895e25 + 4c3b538 commit bf0a08b

File tree

2 files changed

+57
-55
lines changed

2 files changed

+57
-55
lines changed

src/wallet/wallet.cpp

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4048,13 +4048,19 @@ bool CWallet::ParameterInteraction()
40484048
}
40494049
}
40504050

4051-
// -zapwallettx implies a rescan
4052-
if (GetBoolArg("-zapwallettxes", false)) {
4051+
int zapwallettxes = GetArg("-zapwallettxes", 0);
4052+
// -zapwallettxes implies dropping the mempool on startup
4053+
if (zapwallettxes != 0 && SoftSetBoolArg("-persistmempool", false)) {
4054+
LogPrintf("%s: parameter interaction: -zapwallettxes=%s -> setting -persistmempool=0\n", __func__, zapwallettxes);
4055+
}
4056+
4057+
// -zapwallettxes implies a rescan
4058+
if (zapwallettxes != 0) {
40534059
if (is_multiwallet) {
40544060
return InitError(strprintf("%s is only allowed with a single wallet file", "-zapwallettxes"));
40554061
}
40564062
if (SoftSetBoolArg("-rescan", true)) {
4057-
LogPrintf("%s: parameter interaction: -zapwallettxes=<mode> -> setting -rescan=1\n", __func__);
4063+
LogPrintf("%s: parameter interaction: -zapwallettxes=%s -> setting -rescan=1\n", __func__, zapwallettxes);
40584064
}
40594065
}
40604066

test/functional/zapwallettxes.py

Lines changed: 48 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -4,77 +4,73 @@
44
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
55
"""Test the zapwallettxes functionality.
66
7-
- start three bitcoind nodes
8-
- create four transactions on node 0 - two are confirmed and two are
9-
unconfirmed.
10-
- restart node 1 and verify that both the confirmed and the unconfirmed
7+
- start two bitcoind nodes
8+
- create two transactions on node 0 - one is confirmed and one is unconfirmed.
9+
- restart node 0 and verify that both the confirmed and the unconfirmed
1110
transactions are still available.
12-
- restart node 0 and verify that the confirmed transactions are still
13-
available, but that the unconfirmed transaction has been zapped.
11+
- restart node 0 with zapwallettxes and persistmempool, and verify that both
12+
the confirmed and the unconfirmed transactions are still available.
13+
- restart node 0 with just zapwallettxed and verify that the confirmed
14+
transactions are still available, but that the unconfirmed transaction has
15+
been zapped.
1416
"""
1517
from test_framework.test_framework import BitcoinTestFramework
16-
from test_framework.util import *
17-
18+
from test_framework.util import (assert_equal,
19+
assert_raises_jsonrpc,
20+
)
1821

1922
class ZapWalletTXesTest (BitcoinTestFramework):
2023

2124
def __init__(self):
2225
super().__init__()
2326
self.setup_clean_chain = True
24-
self.num_nodes = 3
25-
26-
def setup_network(self):
27-
super().setup_network()
28-
connect_nodes_bi(self.nodes,0,2)
27+
self.num_nodes = 2
2928

30-
def run_test (self):
29+
def run_test(self):
3130
self.log.info("Mining blocks...")
3231
self.nodes[0].generate(1)
3332
self.sync_all()
34-
self.nodes[1].generate(101)
35-
self.sync_all()
36-
37-
assert_equal(self.nodes[0].getbalance(), 50)
38-
39-
txid0 = self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), 11)
40-
txid1 = self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), 10)
33+
self.nodes[1].generate(100)
4134
self.sync_all()
35+
36+
# This transaction will be confirmed
37+
txid1 = self.nodes[0].sendtoaddress(self.nodes[1].getnewaddress(), 10)
38+
4239
self.nodes[0].generate(1)
4340
self.sync_all()
44-
45-
txid2 = self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), 11)
46-
txid3 = self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), 10)
47-
48-
tx0 = self.nodes[0].gettransaction(txid0)
49-
assert_equal(tx0['txid'], txid0) #tx0 must be available (confirmed)
50-
51-
tx1 = self.nodes[0].gettransaction(txid1)
52-
assert_equal(tx1['txid'], txid1) #tx1 must be available (confirmed)
53-
54-
tx2 = self.nodes[0].gettransaction(txid2)
55-
assert_equal(tx2['txid'], txid2) #tx2 must be available (unconfirmed)
56-
57-
tx3 = self.nodes[0].gettransaction(txid3)
58-
assert_equal(tx3['txid'], txid3) #tx3 must be available (unconfirmed)
59-
60-
#restart bitcoind
41+
42+
# This transaction will not be confirmed
43+
txid2 = self.nodes[0].sendtoaddress(self.nodes[1].getnewaddress(), 20)
44+
45+
# Confirmed and unconfirmed transactions are now in the wallet.
46+
assert_equal(self.nodes[0].gettransaction(txid1)['txid'], txid1)
47+
assert_equal(self.nodes[0].gettransaction(txid2)['txid'], txid2)
48+
49+
# Stop-start node0. Both confirmed and unconfirmed transactions remain in the wallet.
50+
self.stop_node(0)
51+
self.nodes[0] = self.start_node(0, self.options.tmpdir)
52+
53+
assert_equal(self.nodes[0].gettransaction(txid1)['txid'], txid1)
54+
assert_equal(self.nodes[0].gettransaction(txid2)['txid'], txid2)
55+
56+
# Stop node0 and restart with zapwallettxes and persistmempool. The unconfirmed
57+
# transaction is zapped from the wallet, but is re-added when the mempool is reloaded.
6158
self.stop_node(0)
62-
self.nodes[0] = self.start_node(0,self.options.tmpdir)
63-
64-
tx3 = self.nodes[0].gettransaction(txid3)
65-
assert_equal(tx3['txid'], txid3) #tx must be available (unconfirmed)
66-
59+
self.nodes[0] = self.start_node(0, self.options.tmpdir, ["-persistmempool=1", "-zapwallettxes=2"])
60+
61+
assert_equal(self.nodes[0].gettransaction(txid1)['txid'], txid1)
62+
assert_equal(self.nodes[0].gettransaction(txid2)['txid'], txid2)
63+
64+
# Stop node0 and restart with zapwallettxes, but not persistmempool.
65+
# The unconfirmed transaction is zapped and is no longer in the wallet.
6766
self.stop_node(0)
68-
69-
#restart bitcoind with zapwallettxes
70-
self.nodes[0] = self.start_node(0,self.options.tmpdir, ["-zapwallettxes=1"])
71-
72-
assert_raises(JSONRPCException, self.nodes[0].gettransaction, [txid3])
73-
#there must be an exception because the unconfirmed wallettx0 must be gone by now
67+
self.nodes[0] = self.start_node(0, self.options.tmpdir, ["-zapwallettxes=2"])
7468

75-
tx0 = self.nodes[0].gettransaction(txid0)
76-
assert_equal(tx0['txid'], txid0) #tx0 (confirmed) must still be available because it was confirmed
69+
# tx1 is still be available because it was confirmed
70+
assert_equal(self.nodes[0].gettransaction(txid1)['txid'], txid1)
7771

72+
# This will raise an exception because the unconfirmed transaction has been zapped
73+
assert_raises_jsonrpc(-5, 'Invalid or non-wallet transaction id', self.nodes[0].gettransaction, txid2)
7874

7975
if __name__ == '__main__':
80-
ZapWalletTXesTest ().main ()
76+
ZapWalletTXesTest().main()

0 commit comments

Comments
 (0)