From d95913fc432f0fde9dec743884b14c5df83727af Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Wed, 29 Sep 2021 17:50:26 +0200 Subject: [PATCH 1/3] rpc: fix "trusted" description in TransactionDescriptionString The helps for RPCs gettransaction, listtransactions, and listsinceblock returned by TransactionDescriptionString() state that the "trusted" boolean field is only present if the transaction is trusted and safe to spend from. The "trusted" boolean field is in fact returned by WalletTxToJSON() when the transaction has 0 confirmations, or negative confirmations, if conflicted, and it can be true or false. This commit updates TransactionDescriptionString() to a more accurate description for "trusted" and updates the existing line of test coverage to fail more helpfully. --- src/wallet/rpcwallet.cpp | 5 +++-- test/functional/wallet_import_rescan.py | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index c430b1db5c6..e90bd207d77 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -1389,7 +1389,8 @@ static const std::vector TransactionDescriptionString() return{{RPCResult::Type::NUM, "confirmations", "The number of confirmations for the transaction. Negative confirmations means the\n" "transaction conflicted that many blocks ago."}, {RPCResult::Type::BOOL, "generated", /* optional */ true, "Only present if transaction only input is a coinbase one."}, - {RPCResult::Type::BOOL, "trusted", /* optional */ true, "Only present if we consider transaction to be trusted and so safe to spend from."}, + {RPCResult::Type::BOOL, "trusted", /* optional */ true, "Whether we consider the transaction to be trusted and safe to spend from.\n" + "Only present when the transaction has 0 confirmations (or negative confirmations, if conflicted)."}, {RPCResult::Type::STR_HEX, "blockhash", /* optional */ true, "The block hash containing the transaction."}, {RPCResult::Type::NUM, "blockheight", /* optional */ true, "The block height containing the transaction."}, {RPCResult::Type::NUM, "blockindex", /* optional */ true, "The index of the transaction in the block that includes it."}, @@ -1407,7 +1408,7 @@ static const std::vector TransactionDescriptionString() {RPCResult::Type::NUM_TIME, "timereceived", "The time received expressed in " + UNIX_EPOCH_TIME + "."}, {RPCResult::Type::STR, "comment", /* optional */ true, "If a comment is associated with the transaction, only present if not empty."}, {RPCResult::Type::STR, "bip125-replaceable", "(\"yes|no|unknown\") Whether this transaction could be replaced due to BIP125 (replace-by-fee);\n" - "may be unknown for unconfirmed transactions not in the mempool"}}; + "may be unknown for unconfirmed transactions not in the mempool."}}; } static RPCHelpMan listtransactions() diff --git a/test/functional/wallet_import_rescan.py b/test/functional/wallet_import_rescan.py index cbe3e9bfddd..27a2a42dac5 100755 --- a/test/functional/wallet_import_rescan.py +++ b/test/functional/wallet_import_rescan.py @@ -99,7 +99,7 @@ class Variant(collections.namedtuple("Variant", "call data address_type rescan p assert_equal(tx["label"], self.label) assert_equal(tx["txid"], txid) assert_equal(tx["confirmations"], 1 + current_height - confirmation_height) - assert_equal("trusted" not in tx, True) + assert "trusted" not in tx address, = [ad for ad in addresses if txid in ad["txids"]] assert_equal(address["address"], self.address["address"]) From 296cfa312fd9ce19f1f820aeafa37d87764ad21d Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Wed, 29 Sep 2021 18:10:12 +0200 Subject: [PATCH 2/3] test: add listtransactions/listsinceblock "trusted" coverage --- test/functional/wallet_listsinceblock.py | 11 +++++++++++ test/functional/wallet_listtransactions.py | 4 ++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/test/functional/wallet_listsinceblock.py b/test/functional/wallet_listsinceblock.py index bd3b29c81c7..f4a00a8ec88 100755 --- a/test/functional/wallet_listsinceblock.py +++ b/test/functional/wallet_listsinceblock.py @@ -44,6 +44,14 @@ class ListSinceBlockTest(BitcoinTestFramework): def test_no_blockhash(self): self.log.info("Test no blockhash") txid = self.nodes[2].sendtoaddress(self.nodes[0].getnewaddress(), 1) + self.sync_all() + assert_array_result(self.nodes[0].listtransactions(), {"txid": txid}, { + "category": "receive", + "amount": 1, + "confirmations": 0, + "trusted": False, + }) + blockhash, = self.generate(self.nodes[2], 1) blockheight = self.nodes[2].getblockheader(blockhash)['height'] self.sync_all() @@ -56,6 +64,9 @@ class ListSinceBlockTest(BitcoinTestFramework): "blockheight": blockheight, "confirmations": 1, }) + assert_equal(len(txs), 1) + assert "trusted" not in txs[0] + assert_equal( self.nodes[0].listsinceblock(), {"lastblock": blockhash, diff --git a/test/functional/wallet_listtransactions.py b/test/functional/wallet_listtransactions.py index a14bfe345cc..9e2f08960ff 100755 --- a/test/functional/wallet_listtransactions.py +++ b/test/functional/wallet_listtransactions.py @@ -31,10 +31,10 @@ class ListTransactionsTest(BitcoinTestFramework): self.sync_all() assert_array_result(self.nodes[0].listtransactions(), {"txid": txid}, - {"category": "send", "amount": Decimal("-0.1"), "confirmations": 0}) + {"category": "send", "amount": Decimal("-0.1"), "confirmations": 0, "trusted": True}) assert_array_result(self.nodes[1].listtransactions(), {"txid": txid}, - {"category": "receive", "amount": Decimal("0.1"), "confirmations": 0}) + {"category": "receive", "amount": Decimal("0.1"), "confirmations": 0, "trusted": False}) self.log.info("Test confirmations change after mining a block") blockhash = self.generate(self.nodes[0], 1)[0] blockheight = self.nodes[0].getblockheader(blockhash)['height'] From 66f6efc70a72cc1613906fd3c10281f9af0ba0db Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Wed, 29 Sep 2021 18:12:21 +0200 Subject: [PATCH 3/3] rpc: improve TransactionDescriptionString() "generated" help --- src/wallet/rpcwallet.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index e90bd207d77..f45cf940702 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -1388,7 +1388,7 @@ static const std::vector TransactionDescriptionString() { return{{RPCResult::Type::NUM, "confirmations", "The number of confirmations for the transaction. Negative confirmations means the\n" "transaction conflicted that many blocks ago."}, - {RPCResult::Type::BOOL, "generated", /* optional */ true, "Only present if transaction only input is a coinbase one."}, + {RPCResult::Type::BOOL, "generated", /* optional */ true, "Only present if the transaction's only input is a coinbase one."}, {RPCResult::Type::BOOL, "trusted", /* optional */ true, "Whether we consider the transaction to be trusted and safe to spend from.\n" "Only present when the transaction has 0 confirmations (or negative confirmations, if conflicted)."}, {RPCResult::Type::STR_HEX, "blockhash", /* optional */ true, "The block hash containing the transaction."},