Skip to content

Commit bda7445

Browse files
MarcoFalkevijaydasmp
MarcoFalke
authored and
vijaydasmp
committed
(partial) Merge 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#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 Signed-off-by: Vijay <[email protected]>
1 parent 1a82687 commit bda7445

File tree

6 files changed

+46
-47
lines changed

6 files changed

+46
-47
lines changed

‎test/functional/mempool_compatibility.py‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,7 @@ def run_test(self):
6868
self.log.info("Add unbroadcasted tx to mempool on new node and shutdown")
6969
unbroadcasted_tx_hash = new_wallet.send_self_transfer(from_node=new_node)['txid']
7070
assert unbroadcasted_tx_hash in new_node.getrawmempool()
71-
mempool = new_node.getrawmempool(True)
72-
assert mempool[unbroadcasted_tx_hash]['unbroadcast']
71+
assert new_node.getmempoolentry(unbroadcasted_tx_hash)['unbroadcast']
7372
self.stop_node(1)
7473

7574
self.log.info("Move mempool.dat from new to old node")

‎test/functional/mempool_package_limits.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ def test_chain_limits_helper(self, mempool_count, package_count):
7272
txid = tx.rehash()
7373
if i < mempool_count:
7474
node.sendrawtransaction(txhex)
75-
assert_equal(node.getrawmempool(verbose=True)[txid]["ancestorcount"], i + 1)
75+
assert_equal(node.getmempoolentry(txid)["ancestorcount"], i + 1)
7676
else:
7777
chain_hex.append(txhex)
7878
chain_txns.append(tx)

‎test/functional/mempool_package_onemore.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ def run_test(self):
6464
(second_chain, second_chain_value) = self.chain_transaction(self.nodes[0], [utxo[1]['txid']], [utxo[1]['vout']], utxo[1]['amount'], fee, 1)
6565

6666
# Check mempool has MAX_ANCESTORS + 1 transactions in it
67-
assert_equal(len(self.nodes[0].getrawmempool(True)), MAX_ANCESTORS + 1)
67+
assert_equal(len(self.nodes[0].getrawmempool()), MAX_ANCESTORS + 1)
6868

6969
# Adding one more transaction on to the chain should fail.
7070
assert_raises_rpc_error(-26, "too-long-mempool-chain, too many unconfirmed ancestors [limit: 25]", self.chain_transaction, self.nodes[0], [txid], [0], value, fee, 1)
@@ -81,7 +81,7 @@ def run_test(self):
8181
self.chain_transaction(self.nodes[0], [second_chain], [0], second_chain_value, fee, 1)
8282

8383
# Finally, check that we added two transactions
84-
assert_equal(len(self.nodes[0].getrawmempool(True)), MAX_ANCESTORS + 3)
84+
assert_equal(len(self.nodes[0].getrawmempool()), MAX_ANCESTORS + 3)
8585

8686
if __name__ == '__main__':
8787
MempoolPackagesTest().main()

‎test/functional/mempool_packages.py‎

Lines changed: 31 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -98,28 +98,28 @@ def run_test(self):
9898
assert_equal(entry, mempool[x])
9999

100100
# Check that the descendant calculations are correct
101-
assert_equal(mempool[x]['descendantcount'], descendant_count)
102-
descendant_fees += mempool[x]['fee']
103-
assert_equal(mempool[x]['modifiedfee'], mempool[x]['fee'])
104-
assert_equal(mempool[x]['fees']['base'], mempool[x]['fee'])
105-
assert_equal(mempool[x]['fees']['modified'], mempool[x]['modifiedfee'])
106-
assert_equal(mempool[x]['descendantfees'], descendant_fees * COIN)
107-
assert_equal(mempool[x]['fees']['descendant'], descendant_fees)
108-
descendant_vsize += mempool[x]['vsize']
109-
assert_equal(mempool[x]['descendantsize'], descendant_vsize)
101+
assert_equal(entry['descendantcount'], descendant_count)
102+
descendant_fees += entry['fee']
103+
assert_equal(entry['modifiedfee'], entry['fee'])
104+
assert_equal(entry['fees']['base'], entry['fee'])
105+
assert_equal(entry['fees']['modified'], entry['modifiedfee'])
106+
assert_equal(entry['descendantfees'], descendant_fees * COIN)
107+
assert_equal(entry['fees']['descendant'], descendant_fees)
108+
descendant_vsize += entry['vsize']
109+
assert_equal(entry['descendantsize'], descendant_vsize)
110110
descendant_count += 1
111111

112112
# Check that ancestor calculations are correct
113-
assert_equal(mempool[x]['ancestorcount'], ancestor_count)
114-
assert_equal(mempool[x]['ancestorfees'], ancestor_fees * COIN)
115-
assert_equal(mempool[x]['ancestorsize'], ancestor_vsize)
116-
ancestor_vsize -= mempool[x]['vsize']
117-
ancestor_fees -= mempool[x]['fee']
113+
assert_equal(entry['ancestorcount'], ancestor_count)
114+
assert_equal(entry['ancestorfees'], ancestor_fees * COIN)
115+
assert_equal(entry['ancestorsize'], ancestor_vsize)
116+
ancestor_vsize -= entry['vsize']
117+
ancestor_fees -= entry['fee']
118118
ancestor_count -= 1
119119

120120
# Check that parent/child list is correct
121-
assert_equal(mempool[x]['spentby'], descendants[-1:])
122-
assert_equal(mempool[x]['depends'], ancestors[-2:-1])
121+
assert_equal(entry['spentby'], descendants[-1:])
122+
assert_equal(entry['depends'], ancestors[-2:-1])
123123

124124
# Check that getmempooldescendants is correct
125125
assert_equal(sorted(descendants), sorted(self.nodes[0].getmempooldescendants(x)))
@@ -162,26 +162,26 @@ def run_test(self):
162162
# Check that ancestor modified fees includes fee deltas from
163163
# prioritisetransaction
164164
self.nodes[0].prioritisetransaction(chain[0], 1000)
165-
mempool = self.nodes[0].getrawmempool(True)
166165
ancestor_fees = 0
167166
for x in chain:
168-
ancestor_fees += mempool[x]['fee']
169-
assert_equal(mempool[x]['fees']['ancestor'], ancestor_fees + Decimal('0.00001'))
170-
assert_equal(mempool[x]['ancestorfees'], ancestor_fees * COIN + 1000)
167+
entry = self.nodes[0].getmempoolentry(x)
168+
ancestor_fees += entry['fee']
169+
assert_equal(entry['fees']['ancestor'], ancestor_fees + Decimal('0.00001'))
170+
assert_equal(entry['ancestorfees'], ancestor_fees * COIN + 1000)
171171

172172
# Undo the prioritisetransaction for later tests
173173
self.nodes[0].prioritisetransaction(chain[0], -1000)
174174

175175
# Check that descendant modified fees includes fee deltas from
176176
# prioritisetransaction
177177
self.nodes[0].prioritisetransaction(chain[-1], 1000)
178-
mempool = self.nodes[0].getrawmempool(True)
179178

180179
descendant_fees = 0
181180
for x in reversed(chain):
182-
descendant_fees += mempool[x]['fee']
183-
assert_equal(mempool[x]['fees']['descendant'], descendant_fees + Decimal('0.00001'))
184-
assert_equal(mempool[x]['descendantfees'], descendant_fees * COIN + 1000)
181+
entry = self.nodes[0].getmempoolentry(x)
182+
descendant_fees += entry['fee']
183+
assert_equal(entry['fees']['descendant'], descendant_fees + Decimal('0.00001'))
184+
assert_equal(entry['descendantfees'], descendant_fees * COIN + 1000)
185185

186186
# Adding one more transaction on to the chain should fail.
187187
assert_raises_rpc_error(-26, "too-long-mempool-chain", self.chain_transaction, self.nodes[0], txid, vout, value, fee, 1)
@@ -199,16 +199,15 @@ def run_test(self):
199199
self.nodes[1].invalidateblock(self.nodes[1].getbestblockhash())
200200

201201
# Now check that the transaction is in the mempool, with the right modified fee
202-
mempool = self.nodes[0].getrawmempool(True)
203-
204202
descendant_fees = 0
205203
for x in reversed(chain):
206-
descendant_fees += mempool[x]['fee']
204+
entry = self.nodes[0].getmempoolentry(x)
205+
descendant_fees += entry['fee']
207206
if (x == chain[-1]):
208-
assert_equal(mempool[x]['modifiedfee'], mempool[x]['fee']+satoshi_round(0.00002))
209-
assert_equal(mempool[x]['fees']['modified'], mempool[x]['fee']+satoshi_round(0.00002))
210-
assert_equal(mempool[x]['descendantfees'], descendant_fees * COIN + 2000)
211-
assert_equal(mempool[x]['fees']['descendant'], descendant_fees+satoshi_round(0.00002))
207+
assert_equal(entry['modifiedfee'], entry['fee']+satoshi_round(0.00002))
208+
assert_equal(entry['fees']['modified'], entry['fee']+satoshi_round(0.00002))
209+
assert_equal(entry['descendantfees'], descendant_fees * COIN + 2000)
210+
assert_equal(entry['fees']['descendant'], descendant_fees+satoshi_round(0.00002))
212211

213212
# Check that node1's mempool is as expected (-> custom ancestor limit)
214213
mempool0 = self.nodes[0].getrawmempool(False)
@@ -264,7 +263,7 @@ def run_test(self):
264263
# - txs from previous ancestor test (-> custom ancestor limit)
265264
# - parent tx for descendant test
266265
# - txs chained off parent tx (-> custom descendant limit)
267-
self.wait_until(lambda: len(self.nodes[1].getrawmempool(False)) ==
266+
self.wait_until(lambda: len(self.nodes[1].getrawmempool()) ==
268267
MAX_ANCESTORS_CUSTOM + 1 + MAX_DESCENDANTS_CUSTOM, timeout=10)
269268
mempool0 = self.nodes[0].getrawmempool(False)
270269
mempool1 = self.nodes[1].getrawmempool(False)

‎test/functional/mempool_updatefromblock.py‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ def transaction_graph_test(self, size, n_tx_to_mine=None, start_input_txid='', e
8686
unsigned_raw_tx = self.nodes[0].createrawtransaction(inputs, outputs)
8787
signed_raw_tx = self.nodes[0].signrawtransactionwithwallet(unsigned_raw_tx)
8888
tx_id.append(self.nodes[0].sendrawtransaction(signed_raw_tx['hex']))
89-
tx_size.append(self.nodes[0].getrawmempool(True)[tx_id[-1]]['vsize'])
89+
tx_size.append(self.nodes[0].getmempoolentry(tx_id[-1])['vsize'])
9090

9191
if tx_count in n_tx_to_mine:
9292
# 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
109109
self.log.info('Checking descendants/ancestors properties of all of the in-mempool transactions...')
110110
for k, tx in enumerate(tx_id):
111111
self.log.debug('Check transaction #{}.'.format(k))
112-
assert_equal(self.nodes[0].getrawmempool(True)[tx]['descendantcount'], size - k)
113-
assert_equal(self.nodes[0].getrawmempool(True)[tx]['descendantsize'], sum(tx_size[k:size]))
114-
assert_equal(self.nodes[0].getrawmempool(True)[tx]['ancestorcount'], k + 1)
115-
assert_equal(self.nodes[0].getrawmempool(True)[tx]['ancestorsize'], sum(tx_size[0:(k + 1)]))
112+
entry = self.nodes[0].getmempoolentry(tx)
113+
assert_equal(entry['descendantcount'], size - k)
114+
assert_equal(entry['descendantsize'], sum(tx_size[k:size]))
115+
assert_equal(entry['ancestorcount'], k + 1)
116+
assert_equal(entry['ancestorsize'], sum(tx_size[0:(k + 1)]))
116117

117118
def run_test(self):
118119
# Use batch size limited by DEFAULT_ANCESTOR_LIMIT = 25 to not fire "too many unconfirmed parents" error.

‎test/functional/rpc_fundrawtransaction.py‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -368,7 +368,7 @@ def test_fee_p2pkh(self):
368368

369369
# Create same transaction over sendtoaddress.
370370
txId = self.nodes[0].sendtoaddress(self.nodes[1].getnewaddress(), 11)
371-
signedFee = self.nodes[0].getrawmempool(True)[txId]['fee']
371+
signedFee = self.nodes[0].getmempoolentry(txId)['fee']
372372

373373
# Compare fee.
374374
feeDelta = Decimal(fundedTx['fee']) - Decimal(signedFee)
@@ -391,7 +391,7 @@ def test_fee_p2pkh_multi_out(self):
391391

392392
# Create same transaction over sendtoaddress.
393393
txId = self.nodes[0].sendmany("", outputs)
394-
signedFee = self.nodes[0].getrawmempool(True)[txId]['fee']
394+
signedFee = self.nodes[0].getmempoolentry(txId)['fee']
395395

396396
# Compare fee.
397397
feeDelta = Decimal(fundedTx['fee']) - Decimal(signedFee)
@@ -415,7 +415,7 @@ def test_fee_p2sh(self):
415415

416416
# Create same transaction over sendtoaddress.
417417
txId = self.nodes[0].sendtoaddress(mSigObj, 11)
418-
signedFee = self.nodes[0].getrawmempool(True)[txId]['fee']
418+
signedFee = self.nodes[0].getmempoolentry(txId)['fee']
419419

420420
# Compare fee.
421421
feeDelta = Decimal(fundedTx['fee']) - Decimal(signedFee)
@@ -456,7 +456,7 @@ def test_fee_4of5(self):
456456

457457
# Create same transaction over sendtoaddress.
458458
txId = self.nodes[0].sendtoaddress(mSigObj, 11)
459-
signedFee = self.nodes[0].getrawmempool(True)[txId]['fee']
459+
signedFee = self.nodes[0].getmempoolentry(txId)['fee']
460460

461461
# Compare fee.
462462
feeDelta = Decimal(fundedTx['fee']) - Decimal(signedFee)
@@ -589,7 +589,7 @@ def test_many_inputs_fee(self):
589589

590590
# Create same transaction over sendtoaddress.
591591
txId = self.nodes[1].sendmany("", outputs)
592-
signedFee = self.nodes[1].getrawmempool(True)[txId]['fee']
592+
signedFee = self.nodes[1].getmempoolentry(txId)['fee']
593593

594594
# Compare fee.
595595
feeDelta = Decimal(fundedTx['fee']) - Decimal(signedFee)

0 commit comments

Comments
 (0)