Merge bitcoin/bitcoin#22707: test: refactor use of getrawmempool in functional tests for efficiency

47c48b5 test: only use verbose for getrawmempool when necessary in functional tests (Michael Dietz)
7734971 test: use getmempoolentry instead of getrawmempool in functional tests when appropriate (Michael Dietz)
86dbd54 test: improve mempool_updatefrom efficiency by using getmempoolentry for specific txns (Michael Dietz)

Pull request description:

  I don't think this changes the intention of the test. But it does shave ~30 seconds off the time it takes to run. From what I've seen our CI `macOS 11 native [gui] [no depends]` runs `mempool_updatefrom.py` in ~135 seconds. After this PR it should run in ~105 seconds

  I noticed this improvement should probably be made when testing performance/runtimes of bitcoin/bitcoin#22698. But I wanted to separate this out from that PR so the affects of each is decoupled

  Edit: The major change in this PR is improving mempool_updatefrom.py's runtime as this is a very long running test. Then made the same efficiency improvements across all the functional tests as it made since to do that here

ACKs for top commit:
  theStack:
    Tested ACK 47c48b5

Tree-SHA512: 40f553715f3d4649dc18c2738554eafaca9ea800c4b028c099217896cc1c466ff457ae814d59cf8564c782a8964d8fac3eda60c1b6ffb08bbee1439b2d34434b

Author: MarcoFalke

test/functional/mempool_accept_wtxid.py

@@ -90,7 +90,7 @@ def run_test(self):
 
         self.log.info("Submit child_one to the mempool")
         txid_submitted = node.sendrawtransaction(child_one.serialize().hex())
-        assert_equal(node.getrawmempool(True)[txid_submitted]['wtxid'], child_one_wtxid)
+        assert_equal(node.getmempoolentry(txid_submitted)['wtxid'], child_one_wtxid)
 
         peer_wtxid_relay.wait_for_broadcast([child_one_wtxid])
         assert_equal(node.getmempoolinfo()["unbroadcastcount"], 0)

test/functional/mempool_compatibility.py

@@ -65,8 +65,7 @@ def run_test(self):
         self.log.info("Add unbroadcasted tx to mempool on new node and shutdown")
         unbroadcasted_tx_hash = new_wallet.send_self_transfer(from_node=new_node)['txid']
         assert unbroadcasted_tx_hash in new_node.getrawmempool()
-        mempool = new_node.getrawmempool(True)
-        assert mempool[unbroadcasted_tx_hash]['unbroadcast']
+        assert new_node.getmempoolentry(unbroadcasted_tx_hash)['unbroadcast']
         self.stop_node(1)
 
         self.log.info("Move mempool.dat from new to old node")

test/functional/mempool_package_limits.py

@@ -74,7 +74,7 @@ def test_chain_limits_helper(self, mempool_count, package_count):
             txid = tx.rehash()
             if i < mempool_count:
                 node.sendrawtransaction(txhex)
-                assert_equal(node.getrawmempool(verbose=True)[txid]["ancestorcount"], i + 1)
+                assert_equal(node.getmempoolentry(txid)["ancestorcount"], i + 1)
             else:
                 chain_hex.append(txhex)
                 chain_txns.append(tx)

test/functional/mempool_package_onemore.py

@@ -51,7 +51,7 @@ def run_test(self):
         (second_chain, second_chain_value) = chain_transaction(self.nodes[0], [utxo[1]['txid']], [utxo[1]['vout']], utxo[1]['amount'], fee, 1)
 
         # Check mempool has MAX_ANCESTORS + 1 transactions in it
-        assert_equal(len(self.nodes[0].getrawmempool(True)), MAX_ANCESTORS + 1)
+        assert_equal(len(self.nodes[0].getrawmempool()), MAX_ANCESTORS + 1)
 
         # Adding one more transaction on to the chain should fail.
         assert_raises_rpc_error(-26, "too-long-mempool-chain, too many unconfirmed ancestors [limit: 25]", chain_transaction, self.nodes[0], [txid], [0], value, fee, 1)
@@ -74,7 +74,7 @@ def run_test(self):
         self.nodes[0].sendrawtransaction(signed_second_tx['hex'])
 
         # Finally, check that we added two transactions
-        assert_equal(len(self.nodes[0].getrawmempool(True)), MAX_ANCESTORS + 3)
+        assert_equal(len(self.nodes[0].getrawmempool()), MAX_ANCESTORS + 3)
 
 if __name__ == '__main__':
     MempoolPackagesTest().main()

test/functional/mempool_packages.py

@@ -89,28 +89,28 @@ def run_test(self):
             assert_equal(entry, mempool[x])
 
             # Check that the descendant calculations are correct
-            assert_equal(mempool[x]['descendantcount'], descendant_count)
-            descendant_fees += mempool[x]['fee']
-            assert_equal(mempool[x]['modifiedfee'], mempool[x]['fee'])
-            assert_equal(mempool[x]['fees']['base'], mempool[x]['fee'])
-            assert_equal(mempool[x]['fees']['modified'], mempool[x]['modifiedfee'])
-            assert_equal(mempool[x]['descendantfees'], descendant_fees * COIN)
-            assert_equal(mempool[x]['fees']['descendant'], descendant_fees)
-            descendant_vsize += mempool[x]['vsize']
-            assert_equal(mempool[x]['descendantsize'], descendant_vsize)
+            assert_equal(entry['descendantcount'], descendant_count)
+            descendant_fees += entry['fee']
+            assert_equal(entry['modifiedfee'], entry['fee'])
+            assert_equal(entry['fees']['base'], entry['fee'])
+            assert_equal(entry['fees']['modified'], entry['modifiedfee'])
+            assert_equal(entry['descendantfees'], descendant_fees * COIN)
+            assert_equal(entry['fees']['descendant'], descendant_fees)
+            descendant_vsize += entry['vsize']
+            assert_equal(entry['descendantsize'], descendant_vsize)
             descendant_count += 1
 
             # Check that ancestor calculations are correct
-            assert_equal(mempool[x]['ancestorcount'], ancestor_count)
-            assert_equal(mempool[x]['ancestorfees'], ancestor_fees * COIN)
-            assert_equal(mempool[x]['ancestorsize'], ancestor_vsize)
-            ancestor_vsize -= mempool[x]['vsize']
-            ancestor_fees -= mempool[x]['fee']
+            assert_equal(entry['ancestorcount'], ancestor_count)
+            assert_equal(entry['ancestorfees'], ancestor_fees * COIN)
+            assert_equal(entry['ancestorsize'], ancestor_vsize)
+            ancestor_vsize -= entry['vsize']
+            ancestor_fees -= entry['fee']
             ancestor_count -= 1
 
             # Check that parent/child list is correct
-            assert_equal(mempool[x]['spentby'], descendants[-1:])
-            assert_equal(mempool[x]['depends'], ancestors[-2:-1])
+            assert_equal(entry['spentby'], descendants[-1:])
+            assert_equal(entry['depends'], ancestors[-2:-1])
 
             # Check that getmempooldescendants is correct
             assert_equal(sorted(descendants), sorted(self.nodes[0].getmempooldescendants(x)))
@@ -153,26 +153,26 @@ def run_test(self):
         # Check that ancestor modified fees includes fee deltas from
         # prioritisetransaction
         self.nodes[0].prioritisetransaction(txid=chain[0], fee_delta=1000)
-        mempool = self.nodes[0].getrawmempool(True)
         ancestor_fees = 0
         for x in chain:
-            ancestor_fees += mempool[x]['fee']
-            assert_equal(mempool[x]['fees']['ancestor'], ancestor_fees + Decimal('0.00001'))
-            assert_equal(mempool[x]['ancestorfees'], ancestor_fees * COIN + 1000)
+            entry = self.nodes[0].getmempoolentry(x)
+            ancestor_fees += entry['fee']
+            assert_equal(entry['fees']['ancestor'], ancestor_fees + Decimal('0.00001'))
+            assert_equal(entry['ancestorfees'], ancestor_fees * COIN + 1000)
 
         # Undo the prioritisetransaction for later tests
         self.nodes[0].prioritisetransaction(txid=chain[0], fee_delta=-1000)
 
         # Check that descendant modified fees includes fee deltas from
         # prioritisetransaction
         self.nodes[0].prioritisetransaction(txid=chain[-1], fee_delta=1000)
-        mempool = self.nodes[0].getrawmempool(True)
 
         descendant_fees = 0
         for x in reversed(chain):
-            descendant_fees += mempool[x]['fee']
-            assert_equal(mempool[x]['fees']['descendant'], descendant_fees + Decimal('0.00001'))
-            assert_equal(mempool[x]['descendantfees'], descendant_fees * COIN + 1000)
+            entry = self.nodes[0].getmempoolentry(x)
+            descendant_fees += entry['fee']
+            assert_equal(entry['fees']['descendant'], descendant_fees + Decimal('0.00001'))
+            assert_equal(entry['descendantfees'], descendant_fees * COIN + 1000)
 
         # Adding one more transaction on to the chain should fail.
         assert_raises_rpc_error(-26, "too-long-mempool-chain", chain_transaction, self.nodes[0], [txid], [vout], value, fee, 1)
@@ -190,16 +190,15 @@ def run_test(self):
         self.nodes[1].invalidateblock(self.nodes[1].getbestblockhash())
 
         # Now check that the transaction is in the mempool, with the right modified fee
-        mempool = self.nodes[0].getrawmempool(True)
-
         descendant_fees = 0
         for x in reversed(chain):
-            descendant_fees += mempool[x]['fee']
+            entry = self.nodes[0].getmempoolentry(x)
+            descendant_fees += entry['fee']
             if (x == chain[-1]):
-                assert_equal(mempool[x]['modifiedfee'], mempool[x]['fee']+satoshi_round(0.00002))
-                assert_equal(mempool[x]['fees']['modified'], mempool[x]['fee']+satoshi_round(0.00002))
-            assert_equal(mempool[x]['descendantfees'], descendant_fees * COIN + 2000)
-            assert_equal(mempool[x]['fees']['descendant'], descendant_fees+satoshi_round(0.00002))
+                assert_equal(entry['modifiedfee'], entry['fee']+satoshi_round(0.00002))
+                assert_equal(entry['fees']['modified'], entry['fee']+satoshi_round(0.00002))
+            assert_equal(entry['descendantfees'], descendant_fees * COIN + 2000)
+            assert_equal(entry['fees']['descendant'], descendant_fees+satoshi_round(0.00002))
 
         # Check that node1's mempool is as expected (-> custom ancestor limit)
         mempool0 = self.nodes[0].getrawmempool(False)
@@ -255,7 +254,7 @@ def run_test(self):
         # - txs from previous ancestor test (-> custom ancestor limit)
         # - parent tx for descendant test
         # - txs chained off parent tx (-> custom descendant limit)
-        self.wait_until(lambda: len(self.nodes[1].getrawmempool(False)) ==
+        self.wait_until(lambda: len(self.nodes[1].getrawmempool()) ==
                                 MAX_ANCESTORS_CUSTOM + 1 + MAX_DESCENDANTS_CUSTOM, timeout=10)
         mempool0 = self.nodes[0].getrawmempool(False)
         mempool1 = self.nodes[1].getrawmempool(False)

test/functional/mempool_unbroadcast.py

@@ -94,9 +94,7 @@ def test_broadcast(self):
 
         self.log.info("Rebroadcast transaction and ensure it is not added to unbroadcast set when already in mempool")
         rpc_tx_hsh = node.sendrawtransaction(txFS["hex"])
-        mempool = node.getrawmempool(True)
-        assert rpc_tx_hsh in mempool
-        assert not mempool[rpc_tx_hsh]['unbroadcast']
+        assert not node.getmempoolentry(rpc_tx_hsh)['unbroadcast']
 
     def test_txn_removal(self):
         self.log.info("Test that transactions removed from mempool are removed from unbroadcast set")

test/functional/mempool_updatefromblock.py

@@ -86,7 +86,7 @@ def transaction_graph_test(self, size, n_tx_to_mine=None, start_input_txid='', e
             unsigned_raw_tx = self.nodes[0].createrawtransaction(inputs, outputs)
             signed_raw_tx = self.nodes[0].signrawtransactionwithwallet(unsigned_raw_tx)
             tx_id.append(self.nodes[0].sendrawtransaction(signed_raw_tx['hex']))
-            tx_size.append(self.nodes[0].getrawmempool(True)[tx_id[-1]]['vsize'])
+            tx_size.append(self.nodes[0].getmempoolentry(tx_id[-1])['vsize'])
 
             if tx_count in n_tx_to_mine:
                 # The created transactions are mined into blocks by batches.
@@ -109,10 +109,11 @@ def transaction_graph_test(self, size, n_tx_to_mine=None, start_input_txid='', e
         self.log.info('Checking descendants/ancestors properties of all of the in-mempool transactions...')
         for k, tx in enumerate(tx_id):
             self.log.debug('Check transaction #{}.'.format(k))
-            assert_equal(self.nodes[0].getrawmempool(True)[tx]['descendantcount'], size - k)
-            assert_equal(self.nodes[0].getrawmempool(True)[tx]['descendantsize'], sum(tx_size[k:size]))
-            assert_equal(self.nodes[0].getrawmempool(True)[tx]['ancestorcount'], k + 1)
-            assert_equal(self.nodes[0].getrawmempool(True)[tx]['ancestorsize'], sum(tx_size[0:(k + 1)]))
+            entry = self.nodes[0].getmempoolentry(tx)
+            assert_equal(entry['descendantcount'], size - k)
+            assert_equal(entry['descendantsize'], sum(tx_size[k:size]))
+            assert_equal(entry['ancestorcount'], k + 1)
+            assert_equal(entry['ancestorsize'], sum(tx_size[0:(k + 1)]))
 
     def run_test(self):
         # Use batch size limited by DEFAULT_ANCESTOR_LIMIT = 25 to not fire "too many unconfirmed parents" error.

test/functional/rpc_fundrawtransaction.py

@@ -380,7 +380,7 @@ def test_fee_p2pkh(self):
 
         # Create same transaction over sendtoaddress.
         txId = self.nodes[0].sendtoaddress(self.nodes[1].getnewaddress(), 1.1)
-        signedFee = self.nodes[0].getrawmempool(True)[txId]['fee']
+        signedFee = self.nodes[0].getmempoolentry(txId)['fee']
 
         # Compare fee.
         feeDelta = Decimal(fundedTx['fee']) - Decimal(signedFee)
@@ -403,7 +403,7 @@ def test_fee_p2pkh_multi_out(self):
 
         # Create same transaction over sendtoaddress.
         txId = self.nodes[0].sendmany("", outputs)
-        signedFee = self.nodes[0].getrawmempool(True)[txId]['fee']
+        signedFee = self.nodes[0].getmempoolentry(txId)['fee']
 
         # Compare fee.
         feeDelta = Decimal(fundedTx['fee']) - Decimal(signedFee)
@@ -427,7 +427,7 @@ def test_fee_p2sh(self):
 
         # Create same transaction over sendtoaddress.
         txId = self.nodes[0].sendtoaddress(mSigObj, 1.1)
-        signedFee = self.nodes[0].getrawmempool(True)[txId]['fee']
+        signedFee = self.nodes[0].getmempoolentry(txId)['fee']
 
         # Compare fee.
         feeDelta = Decimal(fundedTx['fee']) - Decimal(signedFee)
@@ -468,7 +468,7 @@ def test_fee_4of5(self):
 
         # Create same transaction over sendtoaddress.
         txId = self.nodes[0].sendtoaddress(mSigObj, 1.1)
-        signedFee = self.nodes[0].getrawmempool(True)[txId]['fee']
+        signedFee = self.nodes[0].getmempoolentry(txId)['fee']
 
         # Compare fee.
         feeDelta = Decimal(fundedTx['fee']) - Decimal(signedFee)
@@ -600,7 +600,7 @@ def test_many_inputs_fee(self):
 
         # Create same transaction over sendtoaddress.
         txId = self.nodes[1].sendmany("", outputs)
-        signedFee = self.nodes[1].getrawmempool(True)[txId]['fee']
+        signedFee = self.nodes[1].getmempoolentry(txId)['fee']
 
         # Compare fee.
         feeDelta = Decimal(fundedTx['fee']) - Decimal(signedFee)