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

1dcba99 Coinbase receivedby rpcs release notes (Andrew Toth)
b569675 Test including coinbase transactions in receivedby wallet rpcs (Andrew Toth)
bce20c3 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 bitcoin/bitcoin#14654.

ACKs for top commit:
  jnewbery:
    reACK 1dcba99

Tree-SHA512: bfc43b81279fea5b6770a4620b196f6bc7c818d221b228623e9f535ec75a2406bc440e3df911608a3680f11ab64c5a4103917162114f5ff7c4ca8ab07bb9d3df

Author: MarcoFalke

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.

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" },

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,27 +935,38 @@ 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");
         }
         filtered_address = DecodeDestination(params[3].get_str());
         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);
 },
     };
 }

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 @@
 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 @@ def run_test(self):
         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()