LBRYCommunity/lbrycrd
1
0
Fork 0
Code Issues 29 Pull requests 1 Projects Releases 32 Packages Wiki Activity

Merge #16239: wallet/rpc: follow-up clean-up/fixes to avoid_reuse

Browse source 
71d0344cf2 docs: release note wording (Karl-Johan Alm)
3d2ff37913 wallet/rpc: use static help text (Karl-Johan Alm)
53c3c1ea9e 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 https://github.com/bitcoin/bitcoin/pull/13756#discussion_r284907468
  * Second commit addresses https://github.com/bitcoin/bitcoin/pull/13756#discussion_r294868973

  Ping jnewbery and achow101 as they pointed out these issues.

ACKs for commit 71d034:
  jnewbery:
    ACK 71d0344cf2
  meshcollider:
    re-utACK 71d0344cf2

Tree-SHA512: 5e28822af0574ad07dbbed21aa2fe7866bf5770b4c0a1c150ad0da8af3152bcfb7170330a7497fa500326c594740ecf63733cf58325821e2811d7b911d5783a0
This commit is contained in:
MeshCollider 2019-06-22 21:58:52 +12:00
commit 2cbcc55ba6
No known key found for this signature in database
GPG key ID: D300116E1C875A3D
3 changed files with 49 additions and 15 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

10
doc/release-notes-13756.md
View file

@ -7,8 +7,8 @@ A new wallet flag `avoid_reuse` has been added (default off). When enabled,
a wallet will distinguish between used and unused addresses, and default to not
use the former in coin selection.
(Note: rescanning the blockchain is required, to correctly mark previously
used destinations.)
Rescanning the blockchain is required, to correctly mark previously
used destinations.
Together with "avoid partial spends" (present as of Bitcoin v0.17), this
addresses a serious privacy issue where a malicious user can track spends by
@ -30,10 +30,12 @@ These include:
- createwallet
- getbalance
- getbalances
- sendtoaddress
In addition, `sendtoaddress` has been changed to enable `-avoidpartialspends` when
`avoid_reuse` is enabled.
In addition, `sendtoaddress` has been changed to avoid partial spends when `avoid_reuse`
is enabled (if not already enabled via the `-avoidpartialspends` command line flag),
as it would otherwise risk using up the "wrong" UTXO for an address reuse case.
The listunspent RPC has also been updated to now include a "reused" bool, for nodes
with "avoid_reuse" enabled.

30
src/wallet/rpcwallet.cpp
View file

@ -383,7 +383,7 @@ static UniValue sendtoaddress(const JSONRPCRequest& request)
" \"UNSET\"\n"
" \"ECONOMICAL\"\n"
" \"CONSERVATIVE\""},
{"avoid_reuse", RPCArg::Type::BOOL, /* default */ pwallet->IsWalletFlagSet(WALLET_FLAG_AVOID_REUSE) ? "true" : "unavailable", "Avoid spending from dirty addresses; addresses are considered\n"
{"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"
" dirty if they have previously been used in a transaction."},
},
RPCResult{
@ -743,7 +743,7 @@ static UniValue getbalance(const JSONRPCRequest& request)
{"dummy", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "Remains for backward compatibility. Must be excluded or set to \"*\"."},
{"minconf", RPCArg::Type::NUM, /* default */ "0", "Only include transactions confirmed at least this many times."},
{"include_watchonly", RPCArg::Type::BOOL, /* default */ "false", "Also include balance in watch-only addresses (see 'importaddress')"},
{"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."},
{"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."},
},
RPCResult{
"amount (numeric) The total amount in " + CURRENCY_UNIT + " received for this wallet.\n"
@ -2409,6 +2409,7 @@ static UniValue getbalances(const JSONRPCRequest& request)
" \"trusted\": xxx (numeric) trusted balance (outputs created by the wallet or confirmed outputs)\n"
" \"untrusted_pending\": xxx (numeric) untrusted pending balance (outputs created by others that are in the mempool)\n"
" \"immature\": xxx (numeric) balance from immature coinbase outputs\n"
" \"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"
" },\n"
" \"watchonly\": { (object) watchonly balances (not present if wallet does not watch anything)\n"
" \"trusted\": xxx (numeric) trusted balance (outputs created by the wallet or confirmed outputs)\n"
@ -2441,6 +2442,12 @@ static UniValue getbalances(const JSONRPCRequest& request)
balances_mine.pushKV("trusted", ValueFromAmount(bal.m_mine_trusted));
balances_mine.pushKV("untrusted_pending", ValueFromAmount(bal.m_mine_untrusted_pending));
balances_mine.pushKV("immature", ValueFromAmount(bal.m_mine_immature));
if (wallet.IsWalletFlagSet(WALLET_FLAG_AVOID_REUSE)) {
// If the AVOID_REUSE flag is set, bal has been set to just the un-reused address balance. Get
// the total balance, and then subtract bal to get the reused address balance.
const auto full_bal = wallet.GetBalance(0, false);
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));
}
balances.pushKV("mine", balances_mine);
}
if (wallet.HaveWatchOnly()) {
@ -2885,11 +2892,8 @@ static UniValue listunspent(const JSONRPCRequest& request)
return NullUniValue;
}
bool avoid_reuse = pwallet->IsWalletFlagSet(WALLET_FLAG_AVOID_REUSE);
if (request.fHelp || request.params.size() > 5)
throw std::runtime_error(
RPCHelpMan{"listunspent",
const RPCHelpMan help{
"listunspent",
"\nReturns array of unspent transaction outputs\n"
"with between minconf and maxconf (inclusive) confirmations.\n"
"Optionally filter to only include txouts paid to specified addresses.\n",
@ -2926,9 +2930,7 @@ static UniValue listunspent(const JSONRPCRequest& request)
" \"witnessScript\" : \"script\" (string) witnessScript if the scriptPubKey is P2WSH or P2SH-P2WSH\n"
" \"spendable\" : xxx, (bool) Whether we have the private keys to spend this output\n"
" \"solvable\" : xxx, (bool) Whether we know how to spend this output, ignoring the lack of keys\n"
+ (avoid_reuse ?
" \"reused\" : xxx, (bool) Whether this output is reused/dirty (sent to an address that was previously spent from)\n" :
"") +
" \"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"
" \"desc\" : xxx, (string, only when solvable) A descriptor for spending this output\n"
" \"safe\" : xxx (bool) Whether this output is considered safe to spend. Unconfirmed transactions\n"
" from outside keys and unconfirmed replacement transactions are considered unsafe\n"
@ -2944,7 +2946,11 @@ static UniValue listunspent(const JSONRPCRequest& request)
+ HelpExampleCli("listunspent", "6 9999999 '[]' true '{ \"minimumAmount\": 0.005 }'")
+ HelpExampleRpc("listunspent", "6, 9999999, [] , true, { \"minimumAmount\": 0.005 } ")
},
}.ToString());
};
if (request.fHelp || !help.IsValidNumArgs(request.params.size())) {
throw std::runtime_error(help.ToString());
}
int nMinDepth = 1;
if (!request.params[0].isNull()) {
@ -3017,6 +3023,8 @@ static UniValue listunspent(const JSONRPCRequest& request)
LOCK(pwallet->cs_wallet);
const bool avoid_reuse = pwallet->IsWalletFlagSet(WALLET_FLAG_AVOID_REUSE);
for (const COutput& out : vecOutputs) {
CTxDestination address;
const CScript& scriptPubKey = out.tx->tx->vout[out.i].scriptPubKey;

24
test/functional/wallet_avoidreuse.py
View file

@ -63,6 +63,12 @@ def assert_unspent(node, total_count=None, total_sum=None, reused_supported=None
if reused_sum is not None:
assert_approx(stats["reused"]["sum"], reused_sum, 0.001)
def assert_balances(node, mine):
'''Make assertions about a node's getbalances output'''
got = node.getbalances()["mine"]
for k,v in mine.items():
assert_approx(got[k], v, 0.001)
class AvoidReuseTest(BitcoinTestFramework):
def set_test_params(self):
@ -140,6 +146,10 @@ class AvoidReuseTest(BitcoinTestFramework):
# listunspent should show 1 single, unused 10 btc output
assert_unspent(self.nodes[1], total_count=1, total_sum=10, reused_supported=True, reused_count=0)
# getbalances should show no used, 10 btc trusted
assert_balances(self.nodes[1], mine={"used": 0, "trusted": 10})
# node 0 should not show a used entry, as it does not enable avoid_reuse
assert("used" not in self.nodes[0].getbalances()["mine"])
self.nodes[1].sendtoaddress(retaddr, 5)
self.nodes[0].generate(1)
@ -147,6 +157,8 @@ class AvoidReuseTest(BitcoinTestFramework):
# listunspent should show 1 single, unused 5 btc output
assert_unspent(self.nodes[1], total_count=1, total_sum=5, reused_supported=True, reused_count=0)
# getbalances should show no used, 5 btc trusted
assert_balances(self.nodes[1], mine={"used": 0, "trusted": 5})
self.nodes[0].sendtoaddress(fundaddr, 10)
self.nodes[0].generate(1)
@ -154,11 +166,15 @@ class AvoidReuseTest(BitcoinTestFramework):
# listunspent should show 2 total outputs (5, 10 btc), one unused (5), one reused (10)
assert_unspent(self.nodes[1], total_count=2, total_sum=15, reused_count=1, reused_sum=10)
# getbalances should show 10 used, 5 btc trusted
assert_balances(self.nodes[1], mine={"used": 10, "trusted": 5})
self.nodes[1].sendtoaddress(address=retaddr, amount=10, avoid_reuse=False)
# listunspent should show 1 total outputs (5 btc), unused
assert_unspent(self.nodes[1], total_count=1, total_sum=5, reused_count=0)
# getbalances should show no used, 5 btc trusted
assert_balances(self.nodes[1], mine={"used": 0, "trusted": 5})
# node 1 should now have about 5 btc left (for both cases)
assert_approx(self.nodes[1].getbalance(), 5, 0.001)
@ -183,6 +199,8 @@ class AvoidReuseTest(BitcoinTestFramework):
# listunspent should show 1 single, unused 10 btc output
assert_unspent(self.nodes[1], total_count=1, total_sum=10, reused_supported=True, reused_count=0)
# getbalances should show no used, 10 btc trusted
assert_balances(self.nodes[1], mine={"used": 0, "trusted": 10})
self.nodes[1].sendtoaddress(retaddr, 5)
self.nodes[0].generate(1)
@ -190,6 +208,8 @@ class AvoidReuseTest(BitcoinTestFramework):
# listunspent should show 1 single, unused 5 btc output
assert_unspent(self.nodes[1], total_count=1, total_sum=5, reused_supported=True, reused_count=0)
# getbalances should show no used, 5 btc trusted
assert_balances(self.nodes[1], mine={"used": 0, "trusted": 5})
self.nodes[0].sendtoaddress(fundaddr, 10)
self.nodes[0].generate(1)
@ -197,6 +217,8 @@ class AvoidReuseTest(BitcoinTestFramework):
# listunspent should show 2 total outputs (5, 10 btc), one unused (5), one reused (10)
assert_unspent(self.nodes[1], total_count=2, total_sum=15, reused_count=1, reused_sum=10)
# getbalances should show 10 used, 5 btc trusted
assert_balances(self.nodes[1], mine={"used": 10, "trusted": 5})
# node 1 should now have a balance of 5 (no dirty) or 15 (including dirty)
assert_approx(self.nodes[1].getbalance(), 5, 0.001)
@ -208,6 +230,8 @@ class AvoidReuseTest(BitcoinTestFramework):
# listunspent should show 2 total outputs (1, 10 btc), one unused (1), one reused (10)
assert_unspent(self.nodes[1], total_count=2, total_sum=11, reused_count=1, reused_sum=10)
# getbalances should show 10 used, 1 btc trusted
assert_balances(self.nodes[1], mine={"used": 10, "trusted": 1})
# node 1 should now have about 1 btc left (no dirty) and 11 (including dirty)
assert_approx(self.nodes[1].getbalance(), 1, 0.001)