test: Add mempool_updatefromblock.py

hebasto commented at 6:31 pm on March 31, 2020: member

This PR adds a new test for mempool update of transaction descendants/ancestors information (count, size) when transactions have been re-added from a disconnected block to the mempool.

It could be helpful for working on PRs like #17925, #18191.

DrahtBot added the label Tests on Mar 31, 2020

JeremyRubin commented at 8:08 pm on March 31, 2020: contributor

Concept ACK.

I don’t see any reason not to accept this test as is; but it could be nice to:

Make it a bit more generic (e.g., specify a param N rather than writing out each transaction)
Test some more complicated transaction graphs

JeremyRubin added this to the "General Testing" column in a project

hebasto force-pushed on Apr 3, 2020

hebasto commented at 6:12 pm on April 3, 2020: member

Updated 7ed5558042889e08cee941e697b7547213877e9b -> cfaa79b9f4d0c220c220ae0d8a6825581aa89133 (pr18485.01 -> pr18485.02, diff). @JeremyRubin

Make it a bit more generic (e.g., specify a param N rather than writing out each transaction)

Done.

Test some more complicated transaction graphs

Done.

JeremyRubin commented at 1:32 am on April 4, 2020: contributor

Looks great :)

hebasto force-pushed on Apr 4, 2020

hebasto commented at 8:37 am on April 4, 2020: member

Updated cfaa79b9f4d0c220c220ae0d8a6825581aa89133 -> b9156fd3d10dcf5cf18a4728ae42f39826a00586 (pr18485.02 -> pr18485.03, diff):

fixed some flake8 style warnings.

in test/functional/mempool_updatefromblock.py:8 in b9156fd3d1 outdated

0@@ -0,0 +1,121 @@
1+#!/usr/bin/env python3
2+# Copyright (c) 2020 The Bitcoin Core developers
3+# Distributed under the MIT software license, see the accompanying
4+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
5+"""Test mempool descendants/ancestors information update.
6+
7+Test mempool update of transaction descendants/ancestors information (count, size)
8+when transactions have been re-added from a disconnected block to the mempool.

ariard commented at 0:43 am on April 24, 2020:

Maybe hints directly to UpdateTransactionsFromBlock

hebasto commented at 3:26 pm on April 24, 2020:

Maybe hints directly to UpdateTransactionsFromBlock

This could be more appropriate for unit tests rather for functional ones, no?

ariard commented at 9:32 pm on April 24, 2020:

I’m don’t have a strong opinon, but sometimes for newcomers is great to lead them directly to what part of the codebase is tested.

in test/functional/mempool_updatefromblock.py:38 in b9156fd3d1 outdated

33+        the following holds:
34+            the tx[K] transaction:
35+            - has N-K descendants (including this one), and
36+            - has K+1 ancestors (including this one)
37+
38+        More details: https://en.wikipedia.org/wiki/Tournament_(graph_theory)

ariard commented at 0:46 am on April 24, 2020:

A tournament can be a direct cyclic graph? But utxo graph has to be a direct acyclic graph and should be forbid in the mempool, for preciseness, you may indicate it generates a subset of tournament.

hebasto commented at 2:54 pm on April 24, 2020:

I think “acyclic” property is implied in this case :)

hebasto commented at 3:23 pm on April 24, 2020:

Updated.

in test/functional/mempool_updatefromblock.py:25 in b9156fd3d1 outdated

20+        self.extra_args = [['-limitdescendantsize=1000', '-limitancestorsize=1000']]
21+
22+    def skip_test_if_missing_module(self):
23+        self.skip_if_no_wallet()
24+
25+    def transaction_graph_test(self, size, n_tx_to_mine=None, start_input_txid='', end_address='', fee=Decimal(0.00100000)):

ariard commented at 0:48 am on April 24, 2020:

How people should use start_input_txid and end_address ? Do you envision having different helpers each with a specific topology and being able to connect them to test different mempool entry points ? Like a N-fan-out connected to a linear chain ?

hebasto commented at 2:55 pm on April 24, 2020:

Do you envision having different helpers each with a specific topology and being able to connect them to test different mempool entry points ? Like a N-fan-out connected to a linear chain ?

You read my mind :)

ariard commented at 9:32 pm on April 24, 2020:

Great, will review next ones too!

in test/functional/mempool_updatefromblock.py:62 in b9156fd3d1 outdated

56+                inputs_value = self.nodes[0].gettxout(start_input_txid, 0)['value']
57+            else:
58+                inputs = []
59+                inputs_value = 0
60+                for j, tx in enumerate(tx_id[0:i]):
61+                    vout = i - j - 1

ariard commented at 0:49 am on April 24, 2020:

“Transaction N is children of every previous transactions at their output N-1”

hebasto commented at 3:23 pm on April 24, 2020:

Updated.

in test/functional/mempool_updatefromblock.py:73 in b9156fd3d1 outdated

66+            self.log.debug('inputs_value={}'.format(inputs_value))
67+
68+            # Prepare outputs.
69+            tx_count = i + 1
70+            if tx_count < size:
71+                n_outputs = size - tx_count

ariard commented at 0:50 am on April 24, 2020:

“Transaction N is ancestor of every transactions[N+1…size]”

hebasto commented at 3:24 pm on April 24, 2020:

Updated.

ariard approved

ariard commented at 0:51 am on April 24, 2020: member

ACK b9156fd

Cool work! It’s mostly suggestions to make test clearer, feel free to ignore them.

test: Add mempool_updatefromblock.py 8098dea069

hebasto force-pushed on Apr 24, 2020

hebasto commented at 3:23 pm on April 24, 2020: member

Updated b9156fd3d10dcf5cf18a4728ae42f39826a00586 -> 8098dea06944f9de8b285f44958eb98761f133ee (pr18485.03 -> pr18485.04, diff):

addressed @ariard’s comments:

ACK b9156fd

Cool work! It’s mostly suggestions to make test clearer, feel free to ignore them.

ariard commented at 9:31 pm on April 24, 2020: member

ACK 8098dea

MarcoFalke merged this on Apr 29, 2020

MarcoFalke closed this on Apr 29, 2020

hebasto deleted the branch on Apr 29, 2020

sidhujag referenced this in commit b991503208 on Apr 29, 2020

Fabcien referenced this in commit 4a29fe3f7a on Jan 25, 2021

DrahtBot locked this on Feb 15, 2022

test: Add mempool_updatefromblock.py #18485