Merge bitcoin/bitcoin#14707: [RPC] Include coinbase transactions in receivedby RPCs

1dcba996d30d83aebe8c73f42f5d4056d6472166 Coinbase receivedby rpcs release notes (Andrew Toth) b5696750a925c07261287b043ffdfb393cbb1327 Test including coinbase transactions in receivedby wallet rpcs (Andrew Toth) bce20c34d6b999e700a560f95351c212ed8c36f4 Include coinbase transactions in receivedby wallet rpcs (Andrew Toth) Pull request description: The current `*receivedby*` RPCs filter out coinbase transactions. This doesn't seem correct since an output to your address in a coinbase transaction *is* receiving those coins. This PR corrects this behaviour. Also, a new option `include_immature_coinbase` is added (default=`false`) that includes immature coinbase transactions when set to true. However, since this is potentially a breaking change this PR introduces a hidden configuration option `-deprecatedrpc=exclude_coinbase`. This can be set to revert to previous behaviour. If no reports of broken workflow are received, then this option can be removed in a future release. Fixes https://github.com/bitcoin/bitcoin/issues/14654. ACKs for top commit: jnewbery: reACK 1dcba996d30d83aebe8c73f42f5d4056d6472166 Tree-SHA512: bfc43b81279fea5b6770a4620b196f6bc7c818d221b228623e9f535ec75a2406bc440e3df911608a3680f11ab64c5a4103917162114f5ff7c4ca8ab07bb9d3df
author: MarcoFalke <falke.marco@gmail.com> 2021-12-07 20:47:37 +0100
committer: MarcoFalke <falke.marco@gmail.com> 2021-12-07 20:52:13 +0100
commit: 63c63b5533e8d1c682aae3ae6d35b76836ab8341 (patch)
tree: 2bb3c5f5d0509a7228f210f029b457c8ea33ec96
parent: eaf1c56502455006962ee50390d7485befde0869 (diff)
parent: 1dcba996d30d83aebe8c73f42f5d4056d6472166 (diff)
4 files changed, 197 insertions, 14 deletions
diff --git a/doc/release-notes-14707.md b/doc/release-notes-14707.md
new file mode 100644
index 0000000000..b53204f788
--- /dev/null
+++ b/doc/release-notes-14707.md
@@ -0,0 +1,19 @@
+Wallet `receivedby` RPCs now include coinbase transactions
+-------------
+
+Previously, the following wallet RPCs excluded coinbase transactions:
+
+`getreceivedbyaddress`
+
+`getreceivedbylabel`
+
+`listreceivedbyaddress`
+
+`listreceivedbylabel`
+
+This release changes this behaviour and returns results accounting for received coins from coinbase outputs.
+
+A new option, `include_immature_coinbase` (default=`false`), determines whether to account for immature coinbase transactions.
+Immature coinbase transactions are coinbase transactions that have 100 or fewer confirmations, and are not spendable.
+
+The previous behaviour can be restored using the configuration `-deprecatedrpc=exclude_coinbase`, but may be removed in a future release.
diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp
index 90fbd823a4..c0c9e4f005 100644
--- a/src/rpc/client.cpp
+++ b/src/rpc/client.cpp
@@ -46,13 +46,17 @@ static const CRPCConvertParam vRPCConvertParams[] =
     { "settxfee", 0, "amount" },
     { "sethdseed", 0, "newkeypool" },
     { "getreceivedbyaddress", 1, "minconf" },
+    { "getreceivedbyaddress", 2, "include_immature_coinbase" },
     { "getreceivedbylabel", 1, "minconf" },
+    { "getreceivedbylabel", 2, "include_immature_coinbase" },
     { "listreceivedbyaddress", 0, "minconf" },
     { "listreceivedbyaddress", 1, "include_empty" },
     { "listreceivedbyaddress", 2, "include_watchonly" },
+    { "listreceivedbyaddress", 4, "include_immature_coinbase" },
     { "listreceivedbylabel", 0, "minconf" },
     { "listreceivedbylabel", 1, "include_empty" },
     { "listreceivedbylabel", 2, "include_watchonly" },
+    { "listreceivedbylabel", 3, "include_immature_coinbase" },
     { "getbalance", 1, "minconf" },
     { "getbalance", 2, "include_watchonly" },
     { "getbalance", 3, "avoid_reuse" },
diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp
index 73949f26cb..057f000a2c 100644
--- a/src/wallet/rpcwallet.cpp
+++ b/src/wallet/rpcwallet.cpp
@@ -528,11 +528,26 @@ static CAmount GetReceived(const CWallet& wallet, const UniValue& params, bool b
     if (!params[1].isNull())
         min_depth = params[1].get_int();
 
+    const bool include_immature_coinbase{params[2].isNull() ? false : params[2].get_bool()};
+
+    // Excluding coinbase outputs is deprecated
+    // It can be enabled by setting deprecatedrpc=exclude_coinbase
+    const bool include_coinbase{!wallet.chain().rpcEnableDeprecated("exclude_coinbase")};
+
+    if (include_immature_coinbase && !include_coinbase) {
+        throw JSONRPCError(RPC_INVALID_PARAMETER, "include_immature_coinbase is incompatible with deprecated exclude_coinbase");
+    }
+
     // Tally
     CAmount amount = 0;
     for (const std::pair<const uint256, CWalletTx>& wtx_pair : wallet.mapWallet) {
         const CWalletTx& wtx = wtx_pair.second;
-        if (wtx.IsCoinBase() || !wallet.chain().checkFinalTx(*wtx.tx) || wallet.GetTxDepthInMainChain(wtx) < min_depth) {
+        int depth{wallet.GetTxDepthInMainChain(wtx)};
+        if (depth < min_depth
+            // Coinbase with less than 1 confirmation is no longer in the main chain
+            || (wtx.IsCoinBase() && (depth < 1 || !include_coinbase))
+            || (wallet.IsTxImmatureCoinBase(wtx) && !include_immature_coinbase)
+            || !wallet.chain().checkFinalTx(*wtx.tx)) {
             continue;
         }
 
@@ -555,6 +570,7 @@ static RPCHelpMan getreceivedbyaddress()
                 {
                     {"address", RPCArg::Type::STR, RPCArg::Optional::NO, "The bitcoin address for transactions."},
                     {"minconf", RPCArg::Type::NUM, RPCArg::Default{1}, "Only include transactions confirmed at least this many times."},
+                    {"include_immature_coinbase", RPCArg::Type::BOOL, RPCArg::Default{false}, "Include immature coinbase transactions."},
                 },
                 RPCResult{
                     RPCResult::Type::STR_AMOUNT, "amount", "The total amount in " + CURRENCY_UNIT + " received at this address."
@@ -566,6 +582,8 @@ static RPCHelpMan getreceivedbyaddress()
             + HelpExampleCli("getreceivedbyaddress", "\"" + EXAMPLE_ADDRESS[0] + "\" 0") +
             "\nThe amount with at least 6 confirmations\n"
             + HelpExampleCli("getreceivedbyaddress", "\"" + EXAMPLE_ADDRESS[0] + "\" 6") +
+            "\nThe amount with at least 6 confirmations including immature coinbase outputs\n"
+            + HelpExampleCli("getreceivedbyaddress", "\"" + EXAMPLE_ADDRESS[0] + "\" 6 true") +
             "\nAs a JSON-RPC call\n"
             + HelpExampleRpc("getreceivedbyaddress", "\"" + EXAMPLE_ADDRESS[0] + "\", 6")
                 },
@@ -593,6 +611,7 @@ static RPCHelpMan getreceivedbylabel()
                 {
                     {"label", RPCArg::Type::STR, RPCArg::Optional::NO, "The selected label, may be the default label using \"\"."},
                     {"minconf", RPCArg::Type::NUM, RPCArg::Default{1}, "Only include transactions confirmed at least this many times."},
+                    {"include_immature_coinbase", RPCArg::Type::BOOL, RPCArg::Default{false}, "Include immature coinbase transactions."},
                 },
                 RPCResult{
                     RPCResult::Type::STR_AMOUNT, "amount", "The total amount in " + CURRENCY_UNIT + " received for this label."
@@ -604,8 +623,10 @@ static RPCHelpMan getreceivedbylabel()
             + HelpExampleCli("getreceivedbylabel", "\"tabby\" 0") +
             "\nThe amount with at least 6 confirmations\n"
             + HelpExampleCli("getreceivedbylabel", "\"tabby\" 6") +
+            "\nThe amount with at least 6 confirmations including immature coinbase outputs\n"
+            + HelpExampleCli("getreceivedbylabel", "\"tabby\" 6 true") +
             "\nAs a JSON-RPC call\n"
-            + HelpExampleRpc("getreceivedbylabel", "\"tabby\", 6")
+            + HelpExampleRpc("getreceivedbylabel", "\"tabby\", 6, true")
                 },
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
 {
@@ -894,7 +915,7 @@ struct tallyitem
     }
 };
 
-static UniValue ListReceived(const CWallet& wallet, const UniValue& params, bool by_label) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet)
+static UniValue ListReceived(const CWallet& wallet, const UniValue& params, const bool by_label, const bool include_immature_coinbase) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet)
 {
     // Minimum confirmations
     int nMinDepth = 1;
@@ -914,7 +935,7 @@ static UniValue ListReceived(const CWallet& wallet, const UniValue& params, bool
 
     bool has_filtered_address = false;
     CTxDestination filtered_address = CNoDestination();
-    if (!by_label && params.size() > 3) {
+    if (!by_label && !params[3].isNull() && !params[3].get_str().empty()) {
         if (!IsValidDestinationString(params[3].get_str())) {
             throw JSONRPCError(RPC_WALLET_ERROR, "address_filter parameter was invalid");
         }
@@ -922,19 +943,30 @@ static UniValue ListReceived(const CWallet& wallet, const UniValue& params, bool
         has_filtered_address = true;
     }
 
+    // Excluding coinbase outputs is deprecated
+    // It can be enabled by setting deprecatedrpc=exclude_coinbase
+    const bool include_coinbase{!wallet.chain().rpcEnableDeprecated("exclude_coinbase")};
+
+    if (include_immature_coinbase && !include_coinbase) {
+        throw JSONRPCError(RPC_INVALID_PARAMETER, "include_immature_coinbase is incompatible with deprecated exclude_coinbase");
+    }
+
     // Tally
     std::map<CTxDestination, tallyitem> mapTally;
     for (const std::pair<const uint256, CWalletTx>& pairWtx : wallet.mapWallet) {
         const CWalletTx& wtx = pairWtx.second;
 
-        if (wtx.IsCoinBase() || !wallet.chain().checkFinalTx(*wtx.tx)) {
-            continue;
-        }
-
         int nDepth = wallet.GetTxDepthInMainChain(wtx);
         if (nDepth < nMinDepth)
             continue;
 
+        // Coinbase with less than 1 confirmation is no longer in the main chain
+        if ((wtx.IsCoinBase() && (nDepth < 1 || !include_coinbase))
+            || (wallet.IsTxImmatureCoinBase(wtx) && !include_immature_coinbase)
+            || !wallet.chain().checkFinalTx(*wtx.tx)) {
+            continue;
+        }
+
         for (const CTxOut& txout : wtx.tx->vout)
         {
             CTxDestination address;
@@ -1049,7 +1081,8 @@ static RPCHelpMan listreceivedbyaddress()
                     {"minconf", RPCArg::Type::NUM, RPCArg::Default{1}, "The minimum number of confirmations before payments are included."},
                     {"include_empty", RPCArg::Type::BOOL, RPCArg::Default{false}, "Whether to include addresses that haven't received any payments."},
                     {"include_watchonly", RPCArg::Type::BOOL, RPCArg::DefaultHint{"true for watch-only wallets, otherwise false"}, "Whether to include watch-only addresses (see 'importaddress')"},
-                    {"address_filter", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "If present, only return information on this address."},
+                    {"address_filter", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "If present and non-empty, only return information on this address."},
+                    {"include_immature_coinbase", RPCArg::Type::BOOL, RPCArg::Default{false}, "Include immature coinbase transactions."},
                 },
                 RPCResult{
                     RPCResult::Type::ARR, "", "",
@@ -1071,8 +1104,9 @@ static RPCHelpMan listreceivedbyaddress()
                 RPCExamples{
                     HelpExampleCli("listreceivedbyaddress", "")
             + HelpExampleCli("listreceivedbyaddress", "6 true")
+            + HelpExampleCli("listreceivedbyaddress", "6 true true \"\" true")
             + HelpExampleRpc("listreceivedbyaddress", "6, true, true")
-            + HelpExampleRpc("listreceivedbyaddress", "6, true, true, \"" + EXAMPLE_ADDRESS[0] + "\"")
+            + HelpExampleRpc("listreceivedbyaddress", "6, true, true, \"" + EXAMPLE_ADDRESS[0] + "\", true")
                 },
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
 {
@@ -1083,9 +1117,11 @@ static RPCHelpMan listreceivedbyaddress()
     // the user could have gotten from another RPC command prior to now
     pwallet->BlockUntilSyncedToCurrentChain();
 
+    const bool include_immature_coinbase{request.params[4].isNull() ? false : request.params[4].get_bool()};
+
     LOCK(pwallet->cs_wallet);
 
-    return ListReceived(*pwallet, request.params, false);
+    return ListReceived(*pwallet, request.params, false, include_immature_coinbase);
 },
     };
 }
@@ -1098,6 +1134,7 @@ static RPCHelpMan listreceivedbylabel()
                     {"minconf", RPCArg::Type::NUM, RPCArg::Default{1}, "The minimum number of confirmations before payments are included."},
                     {"include_empty", RPCArg::Type::BOOL, RPCArg::Default{false}, "Whether to include labels that haven't received any payments."},
                     {"include_watchonly", RPCArg::Type::BOOL, RPCArg::DefaultHint{"true for watch-only wallets, otherwise false"}, "Whether to include watch-only addresses (see 'importaddress')"},
+                    {"include_immature_coinbase", RPCArg::Type::BOOL, RPCArg::Default{false}, "Include immature coinbase transactions."},
                 },
                 RPCResult{
                     RPCResult::Type::ARR, "", "",
@@ -1114,7 +1151,7 @@ static RPCHelpMan listreceivedbylabel()
                 RPCExamples{
                     HelpExampleCli("listreceivedbylabel", "")
             + HelpExampleCli("listreceivedbylabel", "6 true")
-            + HelpExampleRpc("listreceivedbylabel", "6, true, true")
+            + HelpExampleRpc("listreceivedbylabel", "6, true, true, true")
                 },
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
 {
@@ -1125,9 +1162,11 @@ static RPCHelpMan listreceivedbylabel()
     // the user could have gotten from another RPC command prior to now
     pwallet->BlockUntilSyncedToCurrentChain();
 
+    const bool include_immature_coinbase{request.params[3].isNull() ? false : request.params[3].get_bool()};
+
     LOCK(pwallet->cs_wallet);
 
-    return ListReceived(*pwallet, request.params, true);
+    return ListReceived(*pwallet, request.params, true, include_immature_coinbase);
 },
     };
 }
diff --git a/test/functional/wallet_listreceivedby.py b/test/functional/wallet_listreceivedby.py
index 42a2685a0f..48b92796fc 100755
--- a/test/functional/wallet_listreceivedby.py
+++ b/test/functional/wallet_listreceivedby.py
@@ -2,9 +2,10 @@
 # Copyright (c) 2014-2021 The Bitcoin Core developers
 # Distributed under the MIT software license, see the accompanying
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
-"""Test the listreceivedbyaddress RPC."""
+"""Test the listreceivedbyaddress, listreceivedbylabel, getreceivedybaddress, and getreceivedbylabel RPCs."""
 from decimal import Decimal
 
+from test_framework.blocktools import COINBASE_MATURITY
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
     assert_array_result,
@@ -17,6 +18,8 @@ from test_framework.wallet_util import test_address
 class ReceivedByTest(BitcoinTestFramework):
     def set_test_params(self):
         self.num_nodes = 2
+        # Test deprecated exclude coinbase on second node
+        self.extra_args = [[], ["-deprecatedrpc=exclude_coinbase"]]
 
     def skip_test_if_missing_module(self):
         self.skip_if_no_wallet()
@@ -162,5 +165,123 @@ class ReceivedByTest(BitcoinTestFramework):
         balance = self.nodes[1].getreceivedbylabel("mynewlabel")
         assert_equal(balance, Decimal("0.0"))
 
+        self.log.info("Tests for including coinbase outputs")
+
+        # Generate block reward to address with label
+        label = "label"
+        address = self.nodes[0].getnewaddress(label)
+
+        reward = Decimal("25")
+        self.generatetoaddress(self.nodes[0], 1, address, sync_fun=self.no_op)
+        hash = self.nodes[0].getbestblockhash()
+
+        self.log.info("getreceivedbyaddress returns nothing with defaults")
+        balance = self.nodes[0].getreceivedbyaddress(address)
+        assert_equal(balance, 0)
+
+        self.log.info("getreceivedbyaddress returns block reward when including immature coinbase")
+        balance = self.nodes[0].getreceivedbyaddress(address=address, include_immature_coinbase=True)
+        assert_equal(balance, reward)
+
+        self.log.info("getreceivedbylabel returns nothing with defaults")
+        balance = self.nodes[0].getreceivedbylabel("label")
+        assert_equal(balance, 0)
+
+        self.log.info("getreceivedbylabel returns block reward when including immature coinbase")
+        balance = self.nodes[0].getreceivedbylabel(label="label", include_immature_coinbase=True)
+        assert_equal(balance, reward)
+
+        self.log.info("listreceivedbyaddress does not include address with defaults")
+        assert_array_result(self.nodes[0].listreceivedbyaddress(),
+                            {"address": address},
+                            {}, True)
+
+        self.log.info("listreceivedbyaddress includes address when including immature coinbase")
+        assert_array_result(self.nodes[0].listreceivedbyaddress(minconf=1, include_immature_coinbase=True),
+                            {"address": address},
+                            {"address": address, "amount": reward})
+
+        self.log.info("listreceivedbylabel does not include label with defaults")
+        assert_array_result(self.nodes[0].listreceivedbylabel(),
+                            {"label": label},
+                            {}, True)
+
+        self.log.info("listreceivedbylabel includes label when including immature coinbase")
+        assert_array_result(self.nodes[0].listreceivedbylabel(minconf=1, include_immature_coinbase=True),
+                            {"label": label},
+                            {"label": label, "amount": reward})
+
+        self.log.info("Generate 100 more blocks")
+        self.generate(self.nodes[0], COINBASE_MATURITY, sync_fun=self.no_op)
+
+        self.log.info("getreceivedbyaddress returns reward with defaults")
+        balance = self.nodes[0].getreceivedbyaddress(address)
+        assert_equal(balance, reward)
+
+        self.log.info("getreceivedbylabel returns reward with defaults")
+        balance = self.nodes[0].getreceivedbylabel("label")
+        assert_equal(balance, reward)
+
+        self.log.info("listreceivedbyaddress includes address with defaults")
+        assert_array_result(self.nodes[0].listreceivedbyaddress(),
+                            {"address": address},
+                            {"address": address, "amount": reward})
+
+        self.log.info("listreceivedbylabel includes label with defaults")
+        assert_array_result(self.nodes[0].listreceivedbylabel(),
+                            {"label": label},
+                            {"label": label, "amount": reward})
+
+        self.log.info("Invalidate block that paid to address")
+        self.nodes[0].invalidateblock(hash)
+
+        self.log.info("getreceivedbyaddress does not include invalidated block when minconf is 0 when including immature coinbase")
+        balance = self.nodes[0].getreceivedbyaddress(address=address, minconf=0, include_immature_coinbase=True)
+        assert_equal(balance, 0)
+
+        self.log.info("getreceivedbylabel does not include invalidated block when minconf is 0 when including immature coinbase")
+        balance = self.nodes[0].getreceivedbylabel(label="label", minconf=0, include_immature_coinbase=True)
+        assert_equal(balance, 0)
+
+        self.log.info("listreceivedbyaddress does not include invalidated block when minconf is 0 when including immature coinbase")
+        assert_array_result(self.nodes[0].listreceivedbyaddress(minconf=0, include_immature_coinbase=True),
+                            {"address": address},
+                            {}, True)
+
+        self.log.info("listreceivedbylabel does not include invalidated block when minconf is 0 when including immature coinbase")
+        assert_array_result(self.nodes[0].listreceivedbylabel(minconf=0, include_immature_coinbase=True),
+                            {"label": label},
+                            {}, True)
+
+        # Test exclude_coinbase
+        address2 = self.nodes[1].getnewaddress(label)
+        self.generatetoaddress(self.nodes[1], COINBASE_MATURITY + 1, address2, sync_fun=self.no_op)
+
+        self.log.info("getreceivedbyaddress returns nothing when excluding coinbase")
+        balance = self.nodes[1].getreceivedbyaddress(address2)
+        assert_equal(balance, 0)
+
+        self.log.info("getreceivedbylabel returns nothing when excluding coinbase")
+        balance = self.nodes[1].getreceivedbylabel("label")
+        assert_equal(balance, 0)
+
+        self.log.info("listreceivedbyaddress does not include address when excluding coinbase")
+        assert_array_result(self.nodes[1].listreceivedbyaddress(),
+                            {"address": address2},
+                            {}, True)
+
+        self.log.info("listreceivedbylabel does not include label when excluding coinbase")
+        assert_array_result(self.nodes[1].listreceivedbylabel(),
+                            {"label": label},
+                            {}, True)
+
+        self.log.info("getreceivedbyaddress throws when setting include_immature_coinbase with deprecated exclude_coinbase")
+        assert_raises_rpc_error(-8, 'include_immature_coinbase is incompatible with deprecated exclude_coinbase', self.nodes[1].getreceivedbyaddress, address2, 1, True)
+
+
+        self.log.info("listreceivedbyaddress throws when setting include_immature_coinbase with deprecated exclude_coinbase")
+        assert_raises_rpc_error(-8, 'include_immature_coinbase is incompatible with deprecated exclude_coinbase', self.nodes[1].listreceivedbyaddress, 1, False, False, "", True)
+
+
 if __name__ == '__main__':
     ReceivedByTest().main()
author	MarcoFalke <falke.marco@gmail.com>	2021-12-07 20:47:37 +0100
committer	MarcoFalke <falke.marco@gmail.com>	2021-12-07 20:52:13 +0100
commit	63c63b5533e8d1c682aae3ae6d35b76836ab8341 (patch)
tree	2bb3c5f5d0509a7228f210f029b457c8ea33ec96
parent	eaf1c56502455006962ee50390d7485befde0869 (diff)
parent	1dcba996d30d83aebe8c73f42f5d4056d6472166 (diff)