Skip to content

Commit 2cbcc55

Browse files
meshcollidermeshcollider
committed
Merge #16239: wallet/rpc: follow-up clean-up/fixes to avoid_reuse
71d0344 docs: release note wording (Karl-Johan Alm) 3d2ff37 wallet/rpc: use static help text (Karl-Johan Alm) 53c3c1e wallet/rpc/getbalances: add entry for 'mine.used' balance in results (Karl-Johan Alm) Pull request description: This addresses a few remaining issues pointed out in #13756: * First commit addresses #13756 (comment) * Second commit addresses #13756 (comment) Ping jnewbery and achow101 as they pointed out these issues. ACKs for commit 71d034: jnewbery: ACK 71d0344 meshcollider: re-utACK 71d0344 Tree-SHA512: 5e28822af0574ad07dbbed21aa2fe7866bf5770b4c0a1c150ad0da8af3152bcfb7170330a7497fa500326c594740ecf63733cf58325821e2811d7b911d5783a0
2 parents 32e9453 + 71d0344 commit 2cbcc55

File tree

3 files changed

+49
-15
lines changed

3 files changed

+49
-15
lines changed

‎doc/release-notes-13756.md‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@ A new wallet flag `avoid_reuse` has been added (default off). When enabled,
77
a wallet will distinguish between used and unused addresses, and default to not
88
use the former in coin selection.
99

10-
(Note: rescanning the blockchain is required, to correctly mark previously
11-
used destinations.)
10+
Rescanning the blockchain is required, to correctly mark previously
11+
used destinations.
1212

1313
Together with "avoid partial spends" (present as of Bitcoin v0.17), this
1414
addresses a serious privacy issue where a malicious user can track spends by
@@ -30,10 +30,12 @@ These include:
3030

3131
- createwallet
3232
- getbalance
33+
- getbalances
3334
- sendtoaddress
3435

35-
In addition, `sendtoaddress` has been changed to enable `-avoidpartialspends` when
36-
`avoid_reuse` is enabled.
36+
In addition, `sendtoaddress` has been changed to avoid partial spends when `avoid_reuse`
37+
is enabled (if not already enabled via the `-avoidpartialspends` command line flag),
38+
as it would otherwise risk using up the "wrong" UTXO for an address reuse case.
3739

3840
The listunspent RPC has also been updated to now include a "reused" bool, for nodes
3941
with "avoid_reuse" enabled.

‎src/wallet/rpcwallet.cpp‎

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -383,7 +383,7 @@ static UniValue sendtoaddress(const JSONRPCRequest& request)
383383
" \"UNSET\"\n"
384384
" \"ECONOMICAL\"\n"
385385
" \"CONSERVATIVE\""},
386-
{"avoid_reuse", RPCArg::Type::BOOL, /* default */ pwallet->IsWalletFlagSet(WALLET_FLAG_AVOID_REUSE) ? "true" : "unavailable", "Avoid spending from dirty addresses; addresses are considered\n"
386+
{"avoid_reuse", RPCArg::Type::BOOL, /* default */ "true", "(only available if avoid_reuse wallet flag is set) Avoid spending from dirty addresses; addresses are considered\n"
387387
" dirty if they have previously been used in a transaction."},
388388
},
389389
RPCResult{
@@ -743,7 +743,7 @@ static UniValue getbalance(const JSONRPCRequest& request)
743743
{"dummy", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "Remains for backward compatibility. Must be excluded or set to \"*\"."},
744744
{"minconf", RPCArg::Type::NUM, /* default */ "0", "Only include transactions confirmed at least this many times."},
745745
{"include_watchonly", RPCArg::Type::BOOL, /* default */ "false", "Also include balance in watch-only addresses (see 'importaddress')"},
746-
{"avoid_reuse", RPCArg::Type::BOOL, /* default */ pwallet->IsWalletFlagSet(WALLET_FLAG_AVOID_REUSE) ? "true" : "unavailable", "Do not include balance in dirty outputs; addresses are considered dirty if they have previously been used in a transaction."},
746+
{"avoid_reuse", RPCArg::Type::BOOL, /* default */ "true", "(only available if avoid_reuse wallet flag is set) Do not include balance in dirty outputs; addresses are considered dirty if they have previously been used in a transaction."},
747747
},
748748
RPCResult{
749749
"amount (numeric) The total amount in " + CURRENCY_UNIT + " received for this wallet.\n"
@@ -2409,6 +2409,7 @@ static UniValue getbalances(const JSONRPCRequest& request)
24092409
" \"trusted\": xxx (numeric) trusted balance (outputs created by the wallet or confirmed outputs)\n"
24102410
" \"untrusted_pending\": xxx (numeric) untrusted pending balance (outputs created by others that are in the mempool)\n"
24112411
" \"immature\": xxx (numeric) balance from immature coinbase outputs\n"
2412+
" \"used\": xxx (numeric) (only present if avoid_reuse is set) balance from coins sent to addresses that were previously spent from (potentially privacy violating)\n"
24122413
" },\n"
24132414
" \"watchonly\": { (object) watchonly balances (not present if wallet does not watch anything)\n"
24142415
" \"trusted\": xxx (numeric) trusted balance (outputs created by the wallet or confirmed outputs)\n"
@@ -2441,6 +2442,12 @@ static UniValue getbalances(const JSONRPCRequest& request)
24412442
balances_mine.pushKV("trusted", ValueFromAmount(bal.m_mine_trusted));
24422443
balances_mine.pushKV("untrusted_pending", ValueFromAmount(bal.m_mine_untrusted_pending));
24432444
balances_mine.pushKV("immature", ValueFromAmount(bal.m_mine_immature));
2445+
if (wallet.IsWalletFlagSet(WALLET_FLAG_AVOID_REUSE)) {
2446+
// If the AVOID_REUSE flag is set, bal has been set to just the un-reused address balance. Get
2447+
// the total balance, and then subtract bal to get the reused address balance.
2448+
const auto full_bal = wallet.GetBalance(0, false);
2449+
balances_mine.pushKV("used", ValueFromAmount(full_bal.m_mine_trusted + full_bal.m_mine_untrusted_pending - bal.m_mine_trusted - bal.m_mine_untrusted_pending));
2450+
}
24442451
balances.pushKV("mine", balances_mine);
24452452
}
24462453
if (wallet.HaveWatchOnly()) {
@@ -2885,11 +2892,8 @@ static UniValue listunspent(const JSONRPCRequest& request)
28852892
return NullUniValue;
28862893
}
28872894

2888-
bool avoid_reuse = pwallet->IsWalletFlagSet(WALLET_FLAG_AVOID_REUSE);
2889-
2890-
if (request.fHelp || request.params.size() > 5)
2891-
throw std::runtime_error(
2892-
RPCHelpMan{"listunspent",
2895+
const RPCHelpMan help{
2896+
"listunspent",
28932897
"\nReturns array of unspent transaction outputs\n"
28942898
"with between minconf and maxconf (inclusive) confirmations.\n"
28952899
"Optionally filter to only include txouts paid to specified addresses.\n",
@@ -2926,9 +2930,7 @@ static UniValue listunspent(const JSONRPCRequest& request)
29262930
" \"witnessScript\" : \"script\" (string) witnessScript if the scriptPubKey is P2WSH or P2SH-P2WSH\n"
29272931
" \"spendable\" : xxx, (bool) Whether we have the private keys to spend this output\n"
29282932
" \"solvable\" : xxx, (bool) Whether we know how to spend this output, ignoring the lack of keys\n"
2929-
+ (avoid_reuse ?
2930-
" \"reused\" : xxx, (bool) Whether this output is reused/dirty (sent to an address that was previously spent from)\n" :
2931-
"") +
2933+
" \"reused\" : xxx, (bool) (only present if avoid_reuse is set) Whether this output is reused/dirty (sent to an address that was previously spent from)\n"
29322934
" \"desc\" : xxx, (string, only when solvable) A descriptor for spending this output\n"
29332935
" \"safe\" : xxx (bool) Whether this output is considered safe to spend. Unconfirmed transactions\n"
29342936
" from outside keys and unconfirmed replacement transactions are considered unsafe\n"
@@ -2944,7 +2946,11 @@ static UniValue listunspent(const JSONRPCRequest& request)
29442946
+ HelpExampleCli("listunspent", "6 9999999 '[]' true '{ \"minimumAmount\": 0.005 }'")
29452947
+ HelpExampleRpc("listunspent", "6, 9999999, [] , true, { \"minimumAmount\": 0.005 } ")
29462948
},
2947-
}.ToString());
2949+
};
2950+
2951+
if (request.fHelp || !help.IsValidNumArgs(request.params.size())) {
2952+
throw std::runtime_error(help.ToString());
2953+
}
29482954

29492955
int nMinDepth = 1;
29502956
if (!request.params[0].isNull()) {
@@ -3017,6 +3023,8 @@ static UniValue listunspent(const JSONRPCRequest& request)
30173023

30183024
LOCK(pwallet->cs_wallet);
30193025

3026+
const bool avoid_reuse = pwallet->IsWalletFlagSet(WALLET_FLAG_AVOID_REUSE);
3027+
30203028
for (const COutput& out : vecOutputs) {
30213029
CTxDestination address;
30223030
const CScript& scriptPubKey = out.tx->tx->vout[out.i].scriptPubKey;

‎test/functional/wallet_avoidreuse.py‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,12 @@ def assert_unspent(node, total_count=None, total_sum=None, reused_supported=None
6363
if reused_sum is not None:
6464
assert_approx(stats["reused"]["sum"], reused_sum, 0.001)
6565

66+
def assert_balances(node, mine):
67+
'''Make assertions about a node's getbalances output'''
68+
got = node.getbalances()["mine"]
69+
for k,v in mine.items():
70+
assert_approx(got[k], v, 0.001)
71+
6672
class AvoidReuseTest(BitcoinTestFramework):
6773

6874
def set_test_params(self):
@@ -140,25 +146,35 @@ def test_fund_send_fund_senddirty(self):
140146

141147
# listunspent should show 1 single, unused 10 btc output
142148
assert_unspent(self.nodes[1], total_count=1, total_sum=10, reused_supported=True, reused_count=0)
149+
# getbalances should show no used, 10 btc trusted
150+
assert_balances(self.nodes[1], mine={"used": 0, "trusted": 10})
151+
# node 0 should not show a used entry, as it does not enable avoid_reuse
152+
assert("used" not in self.nodes[0].getbalances()["mine"])
143153

144154
self.nodes[1].sendtoaddress(retaddr, 5)
145155
self.nodes[0].generate(1)
146156
self.sync_all()
147157

148158
# listunspent should show 1 single, unused 5 btc output
149159
assert_unspent(self.nodes[1], total_count=1, total_sum=5, reused_supported=True, reused_count=0)
160+
# getbalances should show no used, 5 btc trusted
161+
assert_balances(self.nodes[1], mine={"used": 0, "trusted": 5})
150162

151163
self.nodes[0].sendtoaddress(fundaddr, 10)
152164
self.nodes[0].generate(1)
153165
self.sync_all()
154166

155167
# listunspent should show 2 total outputs (5, 10 btc), one unused (5), one reused (10)
156168
assert_unspent(self.nodes[1], total_count=2, total_sum=15, reused_count=1, reused_sum=10)
169+
# getbalances should show 10 used, 5 btc trusted
170+
assert_balances(self.nodes[1], mine={"used": 10, "trusted": 5})
157171

158172
self.nodes[1].sendtoaddress(address=retaddr, amount=10, avoid_reuse=False)
159173

160174
# listunspent should show 1 total outputs (5 btc), unused
161175
assert_unspent(self.nodes[1], total_count=1, total_sum=5, reused_count=0)
176+
# getbalances should show no used, 5 btc trusted
177+
assert_balances(self.nodes[1], mine={"used": 0, "trusted": 5})
162178

163179
# node 1 should now have about 5 btc left (for both cases)
164180
assert_approx(self.nodes[1].getbalance(), 5, 0.001)
@@ -183,20 +199,26 @@ def test_fund_send_fund_send(self):
183199

184200
# listunspent should show 1 single, unused 10 btc output
185201
assert_unspent(self.nodes[1], total_count=1, total_sum=10, reused_supported=True, reused_count=0)
202+
# getbalances should show no used, 10 btc trusted
203+
assert_balances(self.nodes[1], mine={"used": 0, "trusted": 10})
186204

187205
self.nodes[1].sendtoaddress(retaddr, 5)
188206
self.nodes[0].generate(1)
189207
self.sync_all()
190208

191209
# listunspent should show 1 single, unused 5 btc output
192210
assert_unspent(self.nodes[1], total_count=1, total_sum=5, reused_supported=True, reused_count=0)
211+
# getbalances should show no used, 5 btc trusted
212+
assert_balances(self.nodes[1], mine={"used": 0, "trusted": 5})
193213

194214
self.nodes[0].sendtoaddress(fundaddr, 10)
195215
self.nodes[0].generate(1)
196216
self.sync_all()
197217

198218
# listunspent should show 2 total outputs (5, 10 btc), one unused (5), one reused (10)
199219
assert_unspent(self.nodes[1], total_count=2, total_sum=15, reused_count=1, reused_sum=10)
220+
# getbalances should show 10 used, 5 btc trusted
221+
assert_balances(self.nodes[1], mine={"used": 10, "trusted": 5})
200222

201223
# node 1 should now have a balance of 5 (no dirty) or 15 (including dirty)
202224
assert_approx(self.nodes[1].getbalance(), 5, 0.001)
@@ -208,6 +230,8 @@ def test_fund_send_fund_send(self):
208230

209231
# listunspent should show 2 total outputs (1, 10 btc), one unused (1), one reused (10)
210232
assert_unspent(self.nodes[1], total_count=2, total_sum=11, reused_count=1, reused_sum=10)
233+
# getbalances should show 10 used, 1 btc trusted
234+
assert_balances(self.nodes[1], mine={"used": 10, "trusted": 1})
211235

212236
# node 1 should now have about 1 btc left (no dirty) and 11 (including dirty)
213237
assert_approx(self.nodes[1].getbalance(), 1, 0.001)

0 commit comments

Comments
 (0)