dash/test/functional/mempool_packages.py

335 lines
15 KiB
Python
Raw Normal View History

#!/usr/bin/env python3
# Copyright (c) 2014-2020 The Bitcoin Core developers
# Distributed under the MIT software license, see the accompanying
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
Backports 0.15 pr2 (#2597) * Merge #9815: Trivial: use EXIT_ codes instead of magic numbers a87d02a use EXIT_ codes instead of magic numbers (Marko Bencun) * Merge #9801: Removed redundant parameter from mempool.PrioritiseTransaction eaea2bb Removed redundant parameter from mempool.PrioritiseTransaction (gubatron) * remove extra parameter (see 3a3745bb) in dash specific code * Merge #9819: Remove harmless read of unusued priority estimates bc8fd12 Remove harmless read of unusued priority estimates (Alex Morcos) * Merge #9766: Add --exclude option to rpc-tests.py c578408 Add exclude option to rpc-tests.py (John Newbery) * Merge #9577: Fix docstrings in qa tests 3f95a80 Fix docstrings in qa tests (John Newbery) * Merge #9823: qa: Set correct path for binaries in rpc tests 3333ad0 qa: Set correct path for binaries in rpc tests (MarcoFalke) * Merge #9833: Trivial: fix comments referencing AppInit2 ef9f495 Trivial: fix comments referencing AppInit2 (Marko Bencun) * Merge #9612: [trivial] Rephrase the definition of difficulty. dc222f8 Trivial: Rephrase the definition of difficulty in the code. (Karl-Johan Alm) * Merge #9847: Extra test vector for BIP32 30aedcb BIP32 extra test vector (Pieter Wuille) * Merge #9839: [qa] Make import-rescan.py watchonly check reliable 864890a [qa] Make import-rescan.py watchonly check reliable (Russell Yanofsky) Tree-SHA512: ea0e2b1d4fc8f35174c3d575fb751b428daf6ad3aa944fad4e3ddcc9195e4f17051473acabc54203b1d27cca64cf911b737ab92e986c40ef384410652e2dbea1 * Change back file params
2019-01-07 10:55:35 +01:00
"""Test descendant package tracking code."""
Merge #13054: tests: Enable automatic detection of undefined names in Python tests scripts. Remove wildcard imports. 68400d8b96 tests: Use explicit imports (practicalswift) Pull request description: Enable automatic detection of undefined names in Python tests scripts. Remove wildcard imports. Wildcard imports make it unclear which names are present in the namespace, confusing both readers and many automated tools. An additional benefit of not using wildcard imports in tests scripts is that readers of a test script then can infer the rough testing scope just by looking at the imports. Before this commit: ``` $ contrib/devtools/lint-python.sh | head -10 ./test/functional/feature_rbf.py:8:1: F403 'from test_framework.util import *' used; unable to detect undefined names ./test/functional/feature_rbf.py:9:1: F403 'from test_framework.script import *' used; unable to detect undefined names ./test/functional/feature_rbf.py:10:1: F403 'from test_framework.mininode import *' used; unable to detect undefined names ./test/functional/feature_rbf.py:15:12: F405 bytes_to_hex_str may be undefined, or defined from star imports: test_framework.mininode, test_framework.script, test_framework.util ./test/functional/feature_rbf.py:17:58: F405 CScript may be undefined, or defined from star imports: test_framework.mininode, test_framework.script, test_framework.util ./test/functional/feature_rbf.py:25:13: F405 COIN may be undefined, or defined from star imports: test_framework.mininode, test_framework.script, test_framework.util ./test/functional/feature_rbf.py:26:31: F405 satoshi_round may be undefined, or defined from star imports: test_framework.mininode, test_framework.script, test_framework.util ./test/functional/feature_rbf.py:26:60: F405 COIN may be undefined, or defined from star imports: test_framework.mininode, test_framework.script, test_framework.util ./test/functional/feature_rbf.py:30:41: F405 satoshi_round may be undefined, or defined from star imports: test_framework.mininode, test_framework.script, test_framework.util ./test/functional/feature_rbf.py:30:68: F405 COIN may be undefined, or defined from star imports: test_framework.mininode, test_framework.script, test_framework.util $ ``` After this commit: ``` $ contrib/devtools/lint-python.sh | head -10 $ ``` Tree-SHA512: 3f826d39cffb6438388e5efcb20a9622ff8238247e882d68f7b38609877421b2a8e10e9229575f8eb6a8fa42dec4256986692e92922c86171f750a0e887438d9
2018-08-13 14:24:43 +02:00
from decimal import Decimal
from test_framework.blocktools import COINBASE_MATURITY
Merge #13054: tests: Enable automatic detection of undefined names in Python tests scripts. Remove wildcard imports. 68400d8b96 tests: Use explicit imports (practicalswift) Pull request description: Enable automatic detection of undefined names in Python tests scripts. Remove wildcard imports. Wildcard imports make it unclear which names are present in the namespace, confusing both readers and many automated tools. An additional benefit of not using wildcard imports in tests scripts is that readers of a test script then can infer the rough testing scope just by looking at the imports. Before this commit: ``` $ contrib/devtools/lint-python.sh | head -10 ./test/functional/feature_rbf.py:8:1: F403 'from test_framework.util import *' used; unable to detect undefined names ./test/functional/feature_rbf.py:9:1: F403 'from test_framework.script import *' used; unable to detect undefined names ./test/functional/feature_rbf.py:10:1: F403 'from test_framework.mininode import *' used; unable to detect undefined names ./test/functional/feature_rbf.py:15:12: F405 bytes_to_hex_str may be undefined, or defined from star imports: test_framework.mininode, test_framework.script, test_framework.util ./test/functional/feature_rbf.py:17:58: F405 CScript may be undefined, or defined from star imports: test_framework.mininode, test_framework.script, test_framework.util ./test/functional/feature_rbf.py:25:13: F405 COIN may be undefined, or defined from star imports: test_framework.mininode, test_framework.script, test_framework.util ./test/functional/feature_rbf.py:26:31: F405 satoshi_round may be undefined, or defined from star imports: test_framework.mininode, test_framework.script, test_framework.util ./test/functional/feature_rbf.py:26:60: F405 COIN may be undefined, or defined from star imports: test_framework.mininode, test_framework.script, test_framework.util ./test/functional/feature_rbf.py:30:41: F405 satoshi_round may be undefined, or defined from star imports: test_framework.mininode, test_framework.script, test_framework.util ./test/functional/feature_rbf.py:30:68: F405 COIN may be undefined, or defined from star imports: test_framework.mininode, test_framework.script, test_framework.util $ ``` After this commit: ``` $ contrib/devtools/lint-python.sh | head -10 $ ``` Tree-SHA512: 3f826d39cffb6438388e5efcb20a9622ff8238247e882d68f7b38609877421b2a8e10e9229575f8eb6a8fa42dec4256986692e92922c86171f750a0e887438d9
2018-08-13 14:24:43 +02:00
from test_framework.messages import COIN
Merge #19760: test: Remove confusing mininode terminology d5800da5199527a366024bc80cad7fcca17d5c4a [test] Remove final references to mininode (John Newbery) 5e8df3312e47a73e747ee892face55ed9ababeea test: resort imports (John Newbery) 85165d4332b0f72d30e0c584b476249b542338e6 scripted-diff: Rename mininode to p2p (John Newbery) 9e2897d020b114a10c860f90c5405be029afddba scripted-diff: Rename mininode_lock to p2p_lock (John Newbery) Pull request description: New contributors are often confused by the terminology in the test framework, and what the difference between a _node_ and a _peer_ is. To summarize: - a 'node' is a bitcoind instance. This is the thing whose behavior is being tested. Each bitcoind node is managed by a python `TestNode` object which is used to start/stop the node, manage the node's data directory, read state about the node (eg process status, log file), and interact with the node over different interfaces. - one of the interfaces that we can use to interact with the node is the p2p interface. Each connection to a node using this interface is managed by a python `P2PInterface` or derived object (which is owned by the `TestNode` object). We can open zero, one or many p2p connections to each bitcoind node. The node sees these connections as 'peers'. For historic reasons, the word 'mininode' has been used to refer to those p2p interface objects that we use to connect to the bitcoind node (the code was originally taken from the 'mini-node' branch of https://github.com/jgarzik/pynode/tree/mini-node). However that name has proved to be confusing for new contributors, so rename the remaining references. ACKs for top commit: amitiuttarwar: ACK d5800da519 MarcoFalke: ACK d5800da5199527a366024bc80cad7fcca17d5c4a 🚞 Tree-SHA512: 2c46c2ac3c4278b6e3c647cfd8108428a41e80788fc4f0e386e5b0c47675bc687d94779496c09a3e5ea1319617295be10c422adeeff2d2bd68378e00e0eeb5de
2024-01-15 20:35:29 +01:00
from test_framework.p2p import P2PTxInvStore
from test_framework.test_framework import BitcoinTestFramework
Merge #17461: test: check custom descendant limit in mempool_packages.py b902bd66b0f35c5016dc5d7aaf501940935edd62 test: check custom descendant limit in mempool_packages.py (Sebastian Falbesoner) Pull request description: This is a follow-up PR to #17435, testing the custom descendant limit, passed by the argument `-limitdescendantcount`. ~~It was more tricky than expected, mainly because we don't know for sure at which point node1 has got all the transactions broadcasted from node0 (for the ancestor test this wasn't a problem since the txs were immediately available through `invalidateblock`) -- a simple `sync_mempools()` doesn't work here since the mempool contents are not equal due to different ancestor/descendant limits. Hence I came up with a "hacky manual sync":~~ 1. ~~wait until the mempool has the _expected_ tx count (see conditions below)~~ 2. ~~after that, wait some time and get sure that the mempool contents haven't changed in-between~~ ~~Like for~~ Similar to the ancestor test, we overall check for ~~three~~ four conditions: - the # of txs in the node1 mempool is equal to the descendant limit (plus 1 for the parent tx, plus the # txs from the previous ancestor test which are still in) ~~(done by the hacky sync above)~~ - all txs in node1 mempool are a subset of txs in node0 mempool - part of the constructed descendant-chain (the first ones up to the limit) are contained in node1 mempool - the remaining part of the constructed descendant-chain (all after the first ones up to the limit) is *not* contained in node1 mempool ACKs for top commit: JeremyRubin: Excellent. utACK b902bd6 Tree-SHA512: 7de96dd248f16ab740e178ac5b64b57ead18cdcf74adfe989709d215e4a67b6b6d20de22c48e885d5f2edc55caaddd44a4261e996c5c87687ceb6a47f1d1fdaf
2020-02-27 21:02:14 +01:00
from test_framework.util import (
assert_equal,
assert_raises_rpc_error,
chain_transaction,
Merge #17461: test: check custom descendant limit in mempool_packages.py b902bd66b0f35c5016dc5d7aaf501940935edd62 test: check custom descendant limit in mempool_packages.py (Sebastian Falbesoner) Pull request description: This is a follow-up PR to #17435, testing the custom descendant limit, passed by the argument `-limitdescendantcount`. ~~It was more tricky than expected, mainly because we don't know for sure at which point node1 has got all the transactions broadcasted from node0 (for the ancestor test this wasn't a problem since the txs were immediately available through `invalidateblock`) -- a simple `sync_mempools()` doesn't work here since the mempool contents are not equal due to different ancestor/descendant limits. Hence I came up with a "hacky manual sync":~~ 1. ~~wait until the mempool has the _expected_ tx count (see conditions below)~~ 2. ~~after that, wait some time and get sure that the mempool contents haven't changed in-between~~ ~~Like for~~ Similar to the ancestor test, we overall check for ~~three~~ four conditions: - the # of txs in the node1 mempool is equal to the descendant limit (plus 1 for the parent tx, plus the # txs from the previous ancestor test which are still in) ~~(done by the hacky sync above)~~ - all txs in node1 mempool are a subset of txs in node0 mempool - part of the constructed descendant-chain (the first ones up to the limit) are contained in node1 mempool - the remaining part of the constructed descendant-chain (all after the first ones up to the limit) is *not* contained in node1 mempool ACKs for top commit: JeremyRubin: Excellent. utACK b902bd6 Tree-SHA512: 7de96dd248f16ab740e178ac5b64b57ead18cdcf74adfe989709d215e4a67b6b6d20de22c48e885d5f2edc55caaddd44a4261e996c5c87687ceb6a47f1d1fdaf
2020-02-27 21:02:14 +01:00
)
2019-11-12 20:53:31 +01:00
# default limits
MAX_ANCESTORS = 25
MAX_DESCENDANTS = 25
2019-11-12 20:53:31 +01:00
# custom limits for node1
MAX_ANCESTORS_CUSTOM = 5
Merge #17461: test: check custom descendant limit in mempool_packages.py b902bd66b0f35c5016dc5d7aaf501940935edd62 test: check custom descendant limit in mempool_packages.py (Sebastian Falbesoner) Pull request description: This is a follow-up PR to #17435, testing the custom descendant limit, passed by the argument `-limitdescendantcount`. ~~It was more tricky than expected, mainly because we don't know for sure at which point node1 has got all the transactions broadcasted from node0 (for the ancestor test this wasn't a problem since the txs were immediately available through `invalidateblock`) -- a simple `sync_mempools()` doesn't work here since the mempool contents are not equal due to different ancestor/descendant limits. Hence I came up with a "hacky manual sync":~~ 1. ~~wait until the mempool has the _expected_ tx count (see conditions below)~~ 2. ~~after that, wait some time and get sure that the mempool contents haven't changed in-between~~ ~~Like for~~ Similar to the ancestor test, we overall check for ~~three~~ four conditions: - the # of txs in the node1 mempool is equal to the descendant limit (plus 1 for the parent tx, plus the # txs from the previous ancestor test which are still in) ~~(done by the hacky sync above)~~ - all txs in node1 mempool are a subset of txs in node0 mempool - part of the constructed descendant-chain (the first ones up to the limit) are contained in node1 mempool - the remaining part of the constructed descendant-chain (all after the first ones up to the limit) is *not* contained in node1 mempool ACKs for top commit: JeremyRubin: Excellent. utACK b902bd6 Tree-SHA512: 7de96dd248f16ab740e178ac5b64b57ead18cdcf74adfe989709d215e4a67b6b6d20de22c48e885d5f2edc55caaddd44a4261e996c5c87687ceb6a47f1d1fdaf
2020-02-27 21:02:14 +01:00
MAX_DESCENDANTS_CUSTOM = 10
assert MAX_DESCENDANTS_CUSTOM >= MAX_ANCESTORS_CUSTOM
class MempoolPackagesTest(BitcoinTestFramework):
def set_test_params(self):
self.num_nodes = 2
2019-11-12 20:53:31 +01:00
self.extra_args = [
Merge #17461: test: check custom descendant limit in mempool_packages.py b902bd66b0f35c5016dc5d7aaf501940935edd62 test: check custom descendant limit in mempool_packages.py (Sebastian Falbesoner) Pull request description: This is a follow-up PR to #17435, testing the custom descendant limit, passed by the argument `-limitdescendantcount`. ~~It was more tricky than expected, mainly because we don't know for sure at which point node1 has got all the transactions broadcasted from node0 (for the ancestor test this wasn't a problem since the txs were immediately available through `invalidateblock`) -- a simple `sync_mempools()` doesn't work here since the mempool contents are not equal due to different ancestor/descendant limits. Hence I came up with a "hacky manual sync":~~ 1. ~~wait until the mempool has the _expected_ tx count (see conditions below)~~ 2. ~~after that, wait some time and get sure that the mempool contents haven't changed in-between~~ ~~Like for~~ Similar to the ancestor test, we overall check for ~~three~~ four conditions: - the # of txs in the node1 mempool is equal to the descendant limit (plus 1 for the parent tx, plus the # txs from the previous ancestor test which are still in) ~~(done by the hacky sync above)~~ - all txs in node1 mempool are a subset of txs in node0 mempool - part of the constructed descendant-chain (the first ones up to the limit) are contained in node1 mempool - the remaining part of the constructed descendant-chain (all after the first ones up to the limit) is *not* contained in node1 mempool ACKs for top commit: JeremyRubin: Excellent. utACK b902bd6 Tree-SHA512: 7de96dd248f16ab740e178ac5b64b57ead18cdcf74adfe989709d215e4a67b6b6d20de22c48e885d5f2edc55caaddd44a4261e996c5c87687ceb6a47f1d1fdaf
2020-02-27 21:02:14 +01:00
[
"-maxorphantxsize=1000",
"-whitelist=noban@127.0.0.1", # immediate tx relay
],
[
"-maxorphantxsize=1000",
"-limitancestorcount={}".format(MAX_ANCESTORS_CUSTOM),
"-limitdescendantcount={}".format(MAX_DESCENDANTS_CUSTOM),
],
2019-11-12 20:53:31 +01:00
]
def skip_test_if_missing_module(self):
self.skip_if_no_wallet()
def run_test(self):
# Mine some blocks and have them mature.
Merge #19804: test/refactor: reference p2p objects explicitly and remove confusing Test_Node.p2p property 10d61505fe77880d6989115defa5e08417f3de2d [test] remove confusing p2p property (gzhao408) 549d30faf04612d9589c81edf9770c99e3221885 scripted-diff: replace p2p with p2ps[0] in p2p_invalid_tx (gzhao408) 7a0de46aeafb351cffa3410e1aae9809fd4698ad [doc] sample code for test framework p2p objects (gzhao408) 784f757994c1306bb6584b14c0c78617d6248432 [refactor] clarify tests by referencing p2p objects directly (gzhao408) Pull request description: The `TestNode` has a `p2p` property which is an alias for `p2ps[0]`. I think this should be removed because it can be confusing and misleading (to both the test writer and reviewer), especially if a TestNode has multiple p2ps connected (which is the case for many tests). Another example is when a test has multiple subtests that connect 1 p2p and use the `p2p` property to reference it. If the subtests don't completely clean up after themselves, the subtests may affect one another. The best way to refer to a connected p2p is use the object returned by `add_p2p_connection` like this: ```py p2p_conn = node.add_p2p_connection(P2PInterface()) ``` A good example is [p2p_invalid_locator.py](https://github.com/bitcoin/bitcoin/blob/master/test/functional/p2p_invalid_locator.py), which cleans up after itself (waits in both `wait_for_disconnect` and in `disconnect_p2ps`) but wouldn't need so much complexity if it just referenced the connections directly. If there is only one connected, it's not really that tedious to just use `node.p2ps[0]` instead of `node.p2p` (and it can always be aliased inside the test itself). ACKs for top commit: robot-dreams: utACK 10d61505fe77880d6989115defa5e08417f3de2d jnewbery: utACK 10d61505fe77880d6989115defa5e08417f3de2d guggero: Concept ACK 10d61505. Tree-SHA512: 5965548929794ec660dae03467640cb2156d7d826cefd26d3a126472cbc2494b855c1d26bbb7b412281fbdc92b9798b9765a85c27bc1a97f7798f27f64db6f13
2020-09-25 14:18:21 +02:00
peer_inv_store = self.nodes[0].add_p2p_connection(P2PTxInvStore()) # keep track of invs
self.generate(self.nodes[0], COINBASE_MATURITY + 1)
utxo = self.nodes[0].listunspent(10)
txid = utxo[0]['txid']
vout = utxo[0]['vout']
value = utxo[0]['amount']
fee = Decimal("0.0001")
# MAX_ANCESTORS transactions off a confirmed tx should be fine
chain = []
Merge #19674: refactor: test: use throwaway _ variable for unused loop counters dac7a111bdd3b0233d94cf68dae7a8bfc6ac9c64 refactor: test: use _ variable for unused loop counters (Sebastian Falbesoner) Pull request description: This tiny PR substitutes Python loops in the form of `for x in range(N): ...` by `for _ in range(N): ...` where applicable. The idea is indicating to the reader that a block (or statement, in list comprehensions) is just repeated N times, and that the loop counter is not used in the body, hence using the throwaway variable. This is already done quite often in the current tests (see e.g. `$ git grep "for _ in range("`). Another alternative would be using `itertools.repeat` (according to Python core developer Raymond Hettinger it's [even faster](https://twitter.com/raymondh/status/1144527183341375488)), but that doesn't seem to be widespread in use and I'm not sure about a readability increase. The only drawback I see is that whenever one wants to debug loop iterations, one would need to introduce a loop variable again. Reviewing this is basically a no-brainer, since tests would fail immediately if a a substitution has taken place on a loop where the variable is used. Instances to replace were found by `$ git grep "for.*in range("` and manually checked. ACKs for top commit: darosior: ACK dac7a111bdd3b0233d94cf68dae7a8bfc6ac9c64 instagibbs: manual inspection ACK https://github.com/bitcoin/bitcoin/pull/19674/commits/dac7a111bdd3b0233d94cf68dae7a8bfc6ac9c64 practicalswift: ACK dac7a111bdd3b0233d94cf68dae7a8bfc6ac9c64 -- the updated code is easier to reason about since the throwaway nature of a variable is expressed explicitly (using the Pythonic `_` idiom) instead of implicitly. Explicit is better than implicit was we all know by now :) Tree-SHA512: 5f43ded9ce14e5e00b3876ec445b90acda1842f813149ae7bafa93f3ac3d510bb778e2c701187fd2c73585e6b87797bb2d2987139bd1a9ba7d58775a59392406
2020-08-11 02:50:34 +02:00
for _ in range(MAX_ANCESTORS):
(txid, sent_value) = chain_transaction(self.nodes[0], [txid], [0], value, fee, 1)
value = sent_value
chain.append(txid)
Merge #18895: p2p: unbroadcast followups: rpcs, nLastResend, mempool sanity check 651f1d816f054cb9c637f8a99c9360bba381ef58 [test] wait for inital broadcast before comparing mempool entries (gzhao408) 9d3f7eb9860254eb787ebe2734fd6a26bcf365c1 [mempool] sanity check that all unbroadcast txns are in mempool (gzhao408) a7ebe48b94c5a9195c8eabd193204c499cb4bfdb [rpc] add unbroadcast info to mempool entries and getmempoolinfo (gzhao408) d16006960443c2efe37c896e46edae9dca86c57d [wallet] remove nLastResend logic (gzhao408) Pull request description: Followup to #18038 by amitiuttarwar which introduces the unbroadcast set: "a mechanism for the mempool to track locally submitted transactions" and decreases the frequency of rebroadcast from 10-15 minutes to 12-36 hours. This PR addresses some of the outstanding TODOs building on top of it: - remove `nLastResend` logic, which is used to ensure rebroadcast doesn't happen again if no new block has been mined (makes sense in 10-15 min period, but not necessary for 12-36 hour period). (#18038 [comment](https://github.com/bitcoin/bitcoin/pull/18038#discussion_r416826914)) - expose unbroadcast info via RPCs, for more informative queries and testing (#18038 [comment](https://github.com/bitcoin/bitcoin/pull/18038#discussion_r416837980)) - add sanity check to verify unbroadcast transactions still exist in mempool before rebroadcasting (#18038 [comment](https://github.com/bitcoin/bitcoin/pull/18038#discussion_r416861609)) ACKs for top commit: naumenkogs: Code review ACK 651f1d816f054cb9c637f8a99c9360bba381ef58 amitiuttarwar: ACK 651f1d816f054cb9c637f8a99c9360bba381ef58 🎉 MarcoFalke: Review ACK 651f1d816f054cb9c637f8a99c9360bba381ef58 Tree-SHA512: d5327e95ef39d44152b48df5c610502ae11c168f43dbbfb2885340c93d1ba9426eb3a5794573f5fc843502109cb3ffb63efa3f2db4f8f112efcde8f76d9a8845
2020-05-22 01:27:28 +02:00
# Wait until mempool transactions have passed initial broadcast (sent inv and received getdata)
# Otherwise, getrawmempool may be inconsistent with getmempoolentry if unbroadcast changes in between
Merge #19804: test/refactor: reference p2p objects explicitly and remove confusing Test_Node.p2p property 10d61505fe77880d6989115defa5e08417f3de2d [test] remove confusing p2p property (gzhao408) 549d30faf04612d9589c81edf9770c99e3221885 scripted-diff: replace p2p with p2ps[0] in p2p_invalid_tx (gzhao408) 7a0de46aeafb351cffa3410e1aae9809fd4698ad [doc] sample code for test framework p2p objects (gzhao408) 784f757994c1306bb6584b14c0c78617d6248432 [refactor] clarify tests by referencing p2p objects directly (gzhao408) Pull request description: The `TestNode` has a `p2p` property which is an alias for `p2ps[0]`. I think this should be removed because it can be confusing and misleading (to both the test writer and reviewer), especially if a TestNode has multiple p2ps connected (which is the case for many tests). Another example is when a test has multiple subtests that connect 1 p2p and use the `p2p` property to reference it. If the subtests don't completely clean up after themselves, the subtests may affect one another. The best way to refer to a connected p2p is use the object returned by `add_p2p_connection` like this: ```py p2p_conn = node.add_p2p_connection(P2PInterface()) ``` A good example is [p2p_invalid_locator.py](https://github.com/bitcoin/bitcoin/blob/master/test/functional/p2p_invalid_locator.py), which cleans up after itself (waits in both `wait_for_disconnect` and in `disconnect_p2ps`) but wouldn't need so much complexity if it just referenced the connections directly. If there is only one connected, it's not really that tedious to just use `node.p2ps[0]` instead of `node.p2p` (and it can always be aliased inside the test itself). ACKs for top commit: robot-dreams: utACK 10d61505fe77880d6989115defa5e08417f3de2d jnewbery: utACK 10d61505fe77880d6989115defa5e08417f3de2d guggero: Concept ACK 10d61505. Tree-SHA512: 5965548929794ec660dae03467640cb2156d7d826cefd26d3a126472cbc2494b855c1d26bbb7b412281fbdc92b9798b9765a85c27bc1a97f7798f27f64db6f13
2020-09-25 14:18:21 +02:00
peer_inv_store.wait_for_broadcast(chain)
Merge #18895: p2p: unbroadcast followups: rpcs, nLastResend, mempool sanity check 651f1d816f054cb9c637f8a99c9360bba381ef58 [test] wait for inital broadcast before comparing mempool entries (gzhao408) 9d3f7eb9860254eb787ebe2734fd6a26bcf365c1 [mempool] sanity check that all unbroadcast txns are in mempool (gzhao408) a7ebe48b94c5a9195c8eabd193204c499cb4bfdb [rpc] add unbroadcast info to mempool entries and getmempoolinfo (gzhao408) d16006960443c2efe37c896e46edae9dca86c57d [wallet] remove nLastResend logic (gzhao408) Pull request description: Followup to #18038 by amitiuttarwar which introduces the unbroadcast set: "a mechanism for the mempool to track locally submitted transactions" and decreases the frequency of rebroadcast from 10-15 minutes to 12-36 hours. This PR addresses some of the outstanding TODOs building on top of it: - remove `nLastResend` logic, which is used to ensure rebroadcast doesn't happen again if no new block has been mined (makes sense in 10-15 min period, but not necessary for 12-36 hour period). (#18038 [comment](https://github.com/bitcoin/bitcoin/pull/18038#discussion_r416826914)) - expose unbroadcast info via RPCs, for more informative queries and testing (#18038 [comment](https://github.com/bitcoin/bitcoin/pull/18038#discussion_r416837980)) - add sanity check to verify unbroadcast transactions still exist in mempool before rebroadcasting (#18038 [comment](https://github.com/bitcoin/bitcoin/pull/18038#discussion_r416861609)) ACKs for top commit: naumenkogs: Code review ACK 651f1d816f054cb9c637f8a99c9360bba381ef58 amitiuttarwar: ACK 651f1d816f054cb9c637f8a99c9360bba381ef58 🎉 MarcoFalke: Review ACK 651f1d816f054cb9c637f8a99c9360bba381ef58 Tree-SHA512: d5327e95ef39d44152b48df5c610502ae11c168f43dbbfb2885340c93d1ba9426eb3a5794573f5fc843502109cb3ffb63efa3f2db4f8f112efcde8f76d9a8845
2020-05-22 01:27:28 +02:00
# Check mempool has MAX_ANCESTORS transactions in it, and descendant and ancestor
# count and fees should look correct
mempool = self.nodes[0].getrawmempool(True)
assert_equal(len(mempool), MAX_ANCESTORS)
descendant_count = 1
descendant_fees = 0
descendant_vsize = 0
ancestor_vsize = sum([mempool[tx]['vsize'] for tx in mempool])
ancestor_count = MAX_ANCESTORS
ancestor_fees = sum([mempool[tx]['fee'] for tx in mempool])
descendants = []
ancestors = list(chain)
for x in reversed(chain):
# Check that getmempoolentry is consistent with getrawmempool
entry = self.nodes[0].getmempoolentry(x)
assert_equal(entry, mempool[x])
# Check that the descendant calculations are correct
(partial) Merge bitcoin/bitcoin#22707: test: refactor use of getrawmempool in functional tests for efficiency 47c48b5f35b4910fcf87caa6e37407e67d879c80 test: only use verbose for getrawmempool when necessary in functional tests (Michael Dietz) 77349713b189e80f2c140db4df50177353a1cb83 test: use getmempoolentry instead of getrawmempool in functional tests when appropriate (Michael Dietz) 86dbd54ae8a8f9c693c0ea67114bbff24a0754df 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 https://github.com/bitcoin/bitcoin/pull/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 47c48b5f35b4910fcf87caa6e37407e67d879c80 Tree-SHA512: 40f553715f3d4649dc18c2738554eafaca9ea800c4b028c099217896cc1c466ff457ae814d59cf8564c782a8964d8fac3eda60c1b6ffb08bbee1439b2d34434b Signed-off-by: Vijay <vijaydas.mp@gmail.com>
2021-08-20 17:38:38 +02:00
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
(partial) Merge bitcoin/bitcoin#22707: test: refactor use of getrawmempool in functional tests for efficiency 47c48b5f35b4910fcf87caa6e37407e67d879c80 test: only use verbose for getrawmempool when necessary in functional tests (Michael Dietz) 77349713b189e80f2c140db4df50177353a1cb83 test: use getmempoolentry instead of getrawmempool in functional tests when appropriate (Michael Dietz) 86dbd54ae8a8f9c693c0ea67114bbff24a0754df 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 https://github.com/bitcoin/bitcoin/pull/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 47c48b5f35b4910fcf87caa6e37407e67d879c80 Tree-SHA512: 40f553715f3d4649dc18c2738554eafaca9ea800c4b028c099217896cc1c466ff457ae814d59cf8564c782a8964d8fac3eda60c1b6ffb08bbee1439b2d34434b Signed-off-by: Vijay <vijaydas.mp@gmail.com>
2021-08-20 17:38:38 +02:00
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
(partial) Merge bitcoin/bitcoin#22707: test: refactor use of getrawmempool in functional tests for efficiency 47c48b5f35b4910fcf87caa6e37407e67d879c80 test: only use verbose for getrawmempool when necessary in functional tests (Michael Dietz) 77349713b189e80f2c140db4df50177353a1cb83 test: use getmempoolentry instead of getrawmempool in functional tests when appropriate (Michael Dietz) 86dbd54ae8a8f9c693c0ea67114bbff24a0754df 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 https://github.com/bitcoin/bitcoin/pull/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 47c48b5f35b4910fcf87caa6e37407e67d879c80 Tree-SHA512: 40f553715f3d4649dc18c2738554eafaca9ea800c4b028c099217896cc1c466ff457ae814d59cf8564c782a8964d8fac3eda60c1b6ffb08bbee1439b2d34434b Signed-off-by: Vijay <vijaydas.mp@gmail.com>
2021-08-20 17:38:38 +02:00
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)))
# Check getmempooldescendants verbose output is correct
for descendant, dinfo in self.nodes[0].getmempooldescendants(x, True).items():
assert_equal(dinfo['depends'], [chain[chain.index(descendant)-1]])
if dinfo['descendantcount'] > 1:
assert_equal(dinfo['spentby'], [chain[chain.index(descendant)+1]])
else:
assert_equal(dinfo['spentby'], [])
descendants.append(x)
# Check that getmempoolancestors is correct
ancestors.remove(x)
assert_equal(sorted(ancestors), sorted(self.nodes[0].getmempoolancestors(x)))
# Check that getmempoolancestors verbose output is correct
for ancestor, ainfo in self.nodes[0].getmempoolancestors(x, True).items():
assert_equal(ainfo['spentby'], [chain[chain.index(ancestor)+1]])
if ainfo['ancestorcount'] > 1:
assert_equal(ainfo['depends'], [chain[chain.index(ancestor)-1]])
else:
assert_equal(ainfo['depends'], [])
# Check that getmempoolancestors/getmempooldescendants correctly handle verbose=true
v_ancestors = self.nodes[0].getmempoolancestors(chain[-1], True)
assert_equal(len(v_ancestors), len(chain)-1)
for x in v_ancestors.keys():
assert_equal(mempool[x], v_ancestors[x])
2021-08-27 21:03:02 +02:00
assert chain[-1] not in v_ancestors.keys()
v_descendants = self.nodes[0].getmempooldescendants(chain[0], True)
assert_equal(len(v_descendants), len(chain)-1)
for x in v_descendants.keys():
assert_equal(mempool[x], v_descendants[x])
2021-08-27 21:03:02 +02:00
assert chain[0] not in v_descendants.keys()
# Check that ancestor modified fees includes fee deltas from
# prioritisetransaction
self.nodes[0].prioritisetransaction(chain[0], 1000)
ancestor_fees = 0
for x in chain:
(partial) Merge bitcoin/bitcoin#22707: test: refactor use of getrawmempool in functional tests for efficiency 47c48b5f35b4910fcf87caa6e37407e67d879c80 test: only use verbose for getrawmempool when necessary in functional tests (Michael Dietz) 77349713b189e80f2c140db4df50177353a1cb83 test: use getmempoolentry instead of getrawmempool in functional tests when appropriate (Michael Dietz) 86dbd54ae8a8f9c693c0ea67114bbff24a0754df 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 https://github.com/bitcoin/bitcoin/pull/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 47c48b5f35b4910fcf87caa6e37407e67d879c80 Tree-SHA512: 40f553715f3d4649dc18c2738554eafaca9ea800c4b028c099217896cc1c466ff457ae814d59cf8564c782a8964d8fac3eda60c1b6ffb08bbee1439b2d34434b Signed-off-by: Vijay <vijaydas.mp@gmail.com>
2021-08-20 17:38:38 +02:00
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(chain[0], -1000)
# Check that descendant modified fees includes fee deltas from
# prioritisetransaction
self.nodes[0].prioritisetransaction(chain[-1], 1000)
descendant_fees = 0
for x in reversed(chain):
(partial) Merge bitcoin/bitcoin#22707: test: refactor use of getrawmempool in functional tests for efficiency 47c48b5f35b4910fcf87caa6e37407e67d879c80 test: only use verbose for getrawmempool when necessary in functional tests (Michael Dietz) 77349713b189e80f2c140db4df50177353a1cb83 test: use getmempoolentry instead of getrawmempool in functional tests when appropriate (Michael Dietz) 86dbd54ae8a8f9c693c0ea67114bbff24a0754df 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 https://github.com/bitcoin/bitcoin/pull/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 47c48b5f35b4910fcf87caa6e37407e67d879c80 Tree-SHA512: 40f553715f3d4649dc18c2738554eafaca9ea800c4b028c099217896cc1c466ff457ae814d59cf8564c782a8964d8fac3eda60c1b6ffb08bbee1439b2d34434b Signed-off-by: Vijay <vijaydas.mp@gmail.com>
2021-08-20 17:38:38 +02:00
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)
# Check that prioritising a tx before it's added to the mempool works
# First clear the mempool by mining a block.
self.generate(self.nodes[0], 1)
self.sync_blocks()
assert_equal(len(self.nodes[0].getrawmempool()), 0)
# Prioritise a transaction that has been mined, then add it back to the
# mempool by using invalidateblock.
self.nodes[0].prioritisetransaction(chain[-1], 2000)
self.nodes[0].invalidateblock(self.nodes[0].getbestblockhash())
# Keep node1's tip synced with node0
self.nodes[1].invalidateblock(self.nodes[1].getbestblockhash())
# Now check that the transaction is in the mempool, with the right modified fee
descendant_fees = 0
for x in reversed(chain):
(partial) Merge bitcoin/bitcoin#22707: test: refactor use of getrawmempool in functional tests for efficiency 47c48b5f35b4910fcf87caa6e37407e67d879c80 test: only use verbose for getrawmempool when necessary in functional tests (Michael Dietz) 77349713b189e80f2c140db4df50177353a1cb83 test: use getmempoolentry instead of getrawmempool in functional tests when appropriate (Michael Dietz) 86dbd54ae8a8f9c693c0ea67114bbff24a0754df 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 https://github.com/bitcoin/bitcoin/pull/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 47c48b5f35b4910fcf87caa6e37407e67d879c80 Tree-SHA512: 40f553715f3d4649dc18c2738554eafaca9ea800c4b028c099217896cc1c466ff457ae814d59cf8564c782a8964d8fac3eda60c1b6ffb08bbee1439b2d34434b Signed-off-by: Vijay <vijaydas.mp@gmail.com>
2021-08-20 17:38:38 +02:00
entry = self.nodes[0].getmempoolentry(x)
descendant_fees += entry['fee']
if (x == chain[-1]):
assert_equal(entry['modifiedfee'], entry['fee'] + Decimal("0.00002"))
assert_equal(entry['fees']['modified'], entry['fee'] + Decimal("0.00002"))
(partial) Merge bitcoin/bitcoin#22707: test: refactor use of getrawmempool in functional tests for efficiency 47c48b5f35b4910fcf87caa6e37407e67d879c80 test: only use verbose for getrawmempool when necessary in functional tests (Michael Dietz) 77349713b189e80f2c140db4df50177353a1cb83 test: use getmempoolentry instead of getrawmempool in functional tests when appropriate (Michael Dietz) 86dbd54ae8a8f9c693c0ea67114bbff24a0754df 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 https://github.com/bitcoin/bitcoin/pull/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 47c48b5f35b4910fcf87caa6e37407e67d879c80 Tree-SHA512: 40f553715f3d4649dc18c2738554eafaca9ea800c4b028c099217896cc1c466ff457ae814d59cf8564c782a8964d8fac3eda60c1b6ffb08bbee1439b2d34434b Signed-off-by: Vijay <vijaydas.mp@gmail.com>
2021-08-20 17:38:38 +02:00
assert_equal(entry['descendantfees'], descendant_fees * COIN + 2000)
assert_equal(entry['fees']['descendant'], descendant_fees + Decimal("0.00002"))
2019-11-12 20:53:31 +01:00
# Check that node1's mempool is as expected (-> custom ancestor limit)
mempool0 = self.nodes[0].getrawmempool(False)
mempool1 = self.nodes[1].getrawmempool(False)
assert_equal(len(mempool1), MAX_ANCESTORS_CUSTOM)
assert set(mempool1).issubset(set(mempool0))
for tx in chain[:MAX_ANCESTORS_CUSTOM]:
assert tx in mempool1
entry0 = self.nodes[0].getmempoolentry(tx)
entry1 = self.nodes[1].getmempoolentry(tx)
assert not entry0['unbroadcast']
assert not entry1['unbroadcast']
assert_equal(entry1['fees']['base'], entry0['fees']['base'])
assert_equal(entry1['vsize'], entry0['vsize'])
assert_equal(entry1['depends'], entry0['depends'])
# Now test descendant chain limits
txid = utxo[1]['txid']
value = utxo[1]['amount']
vout = utxo[1]['vout']
transaction_package = []
tx_children = []
# First create one parent tx with 10 children
(txid, sent_value) = chain_transaction(self.nodes[0], [txid], [vout], value, fee, 10)
parent_transaction = txid
for i in range(10):
transaction_package.append({'txid': txid, 'vout': i, 'amount': sent_value})
# Sign and send up to MAX_DESCENDANT transactions chained off the parent tx
Merge #17461: test: check custom descendant limit in mempool_packages.py b902bd66b0f35c5016dc5d7aaf501940935edd62 test: check custom descendant limit in mempool_packages.py (Sebastian Falbesoner) Pull request description: This is a follow-up PR to #17435, testing the custom descendant limit, passed by the argument `-limitdescendantcount`. ~~It was more tricky than expected, mainly because we don't know for sure at which point node1 has got all the transactions broadcasted from node0 (for the ancestor test this wasn't a problem since the txs were immediately available through `invalidateblock`) -- a simple `sync_mempools()` doesn't work here since the mempool contents are not equal due to different ancestor/descendant limits. Hence I came up with a "hacky manual sync":~~ 1. ~~wait until the mempool has the _expected_ tx count (see conditions below)~~ 2. ~~after that, wait some time and get sure that the mempool contents haven't changed in-between~~ ~~Like for~~ Similar to the ancestor test, we overall check for ~~three~~ four conditions: - the # of txs in the node1 mempool is equal to the descendant limit (plus 1 for the parent tx, plus the # txs from the previous ancestor test which are still in) ~~(done by the hacky sync above)~~ - all txs in node1 mempool are a subset of txs in node0 mempool - part of the constructed descendant-chain (the first ones up to the limit) are contained in node1 mempool - the remaining part of the constructed descendant-chain (all after the first ones up to the limit) is *not* contained in node1 mempool ACKs for top commit: JeremyRubin: Excellent. utACK b902bd6 Tree-SHA512: 7de96dd248f16ab740e178ac5b64b57ead18cdcf74adfe989709d215e4a67b6b6d20de22c48e885d5f2edc55caaddd44a4261e996c5c87687ceb6a47f1d1fdaf
2020-02-27 21:02:14 +01:00
chain = [] # save sent txs for the purpose of checking node1's mempool later (see below)
Merge #19674: refactor: test: use throwaway _ variable for unused loop counters dac7a111bdd3b0233d94cf68dae7a8bfc6ac9c64 refactor: test: use _ variable for unused loop counters (Sebastian Falbesoner) Pull request description: This tiny PR substitutes Python loops in the form of `for x in range(N): ...` by `for _ in range(N): ...` where applicable. The idea is indicating to the reader that a block (or statement, in list comprehensions) is just repeated N times, and that the loop counter is not used in the body, hence using the throwaway variable. This is already done quite often in the current tests (see e.g. `$ git grep "for _ in range("`). Another alternative would be using `itertools.repeat` (according to Python core developer Raymond Hettinger it's [even faster](https://twitter.com/raymondh/status/1144527183341375488)), but that doesn't seem to be widespread in use and I'm not sure about a readability increase. The only drawback I see is that whenever one wants to debug loop iterations, one would need to introduce a loop variable again. Reviewing this is basically a no-brainer, since tests would fail immediately if a a substitution has taken place on a loop where the variable is used. Instances to replace were found by `$ git grep "for.*in range("` and manually checked. ACKs for top commit: darosior: ACK dac7a111bdd3b0233d94cf68dae7a8bfc6ac9c64 instagibbs: manual inspection ACK https://github.com/bitcoin/bitcoin/pull/19674/commits/dac7a111bdd3b0233d94cf68dae7a8bfc6ac9c64 practicalswift: ACK dac7a111bdd3b0233d94cf68dae7a8bfc6ac9c64 -- the updated code is easier to reason about since the throwaway nature of a variable is expressed explicitly (using the Pythonic `_` idiom) instead of implicitly. Explicit is better than implicit was we all know by now :) Tree-SHA512: 5f43ded9ce14e5e00b3876ec445b90acda1842f813149ae7bafa93f3ac3d510bb778e2c701187fd2c73585e6b87797bb2d2987139bd1a9ba7d58775a59392406
2020-08-11 02:50:34 +02:00
for _ in range(MAX_DESCENDANTS - 1):
utxo = transaction_package.pop(0)
(txid, sent_value) = chain_transaction(self.nodes[0], [utxo['txid']], [utxo['vout']], utxo['amount'], fee, 10)
Merge #17461: test: check custom descendant limit in mempool_packages.py b902bd66b0f35c5016dc5d7aaf501940935edd62 test: check custom descendant limit in mempool_packages.py (Sebastian Falbesoner) Pull request description: This is a follow-up PR to #17435, testing the custom descendant limit, passed by the argument `-limitdescendantcount`. ~~It was more tricky than expected, mainly because we don't know for sure at which point node1 has got all the transactions broadcasted from node0 (for the ancestor test this wasn't a problem since the txs were immediately available through `invalidateblock`) -- a simple `sync_mempools()` doesn't work here since the mempool contents are not equal due to different ancestor/descendant limits. Hence I came up with a "hacky manual sync":~~ 1. ~~wait until the mempool has the _expected_ tx count (see conditions below)~~ 2. ~~after that, wait some time and get sure that the mempool contents haven't changed in-between~~ ~~Like for~~ Similar to the ancestor test, we overall check for ~~three~~ four conditions: - the # of txs in the node1 mempool is equal to the descendant limit (plus 1 for the parent tx, plus the # txs from the previous ancestor test which are still in) ~~(done by the hacky sync above)~~ - all txs in node1 mempool are a subset of txs in node0 mempool - part of the constructed descendant-chain (the first ones up to the limit) are contained in node1 mempool - the remaining part of the constructed descendant-chain (all after the first ones up to the limit) is *not* contained in node1 mempool ACKs for top commit: JeremyRubin: Excellent. utACK b902bd6 Tree-SHA512: 7de96dd248f16ab740e178ac5b64b57ead18cdcf74adfe989709d215e4a67b6b6d20de22c48e885d5f2edc55caaddd44a4261e996c5c87687ceb6a47f1d1fdaf
2020-02-27 21:02:14 +01:00
chain.append(txid)
if utxo['txid'] is parent_transaction:
tx_children.append(txid)
for j in range(10):
transaction_package.append({'txid': txid, 'vout': j, 'amount': sent_value})
mempool = self.nodes[0].getrawmempool(True)
assert_equal(mempool[parent_transaction]['descendantcount'], MAX_DESCENDANTS)
assert_equal(sorted(mempool[parent_transaction]['spentby']), sorted(tx_children))
for child in tx_children:
assert_equal(mempool[child]['depends'], [parent_transaction])
# Sending one more chained transaction will fail
utxo = transaction_package.pop(0)
assert_raises_rpc_error(-26, "too-long-mempool-chain", chain_transaction, self.nodes[0], [utxo['txid']], [utxo['vout']], utxo['amount'], fee, 10)
Merge #17461: test: check custom descendant limit in mempool_packages.py b902bd66b0f35c5016dc5d7aaf501940935edd62 test: check custom descendant limit in mempool_packages.py (Sebastian Falbesoner) Pull request description: This is a follow-up PR to #17435, testing the custom descendant limit, passed by the argument `-limitdescendantcount`. ~~It was more tricky than expected, mainly because we don't know for sure at which point node1 has got all the transactions broadcasted from node0 (for the ancestor test this wasn't a problem since the txs were immediately available through `invalidateblock`) -- a simple `sync_mempools()` doesn't work here since the mempool contents are not equal due to different ancestor/descendant limits. Hence I came up with a "hacky manual sync":~~ 1. ~~wait until the mempool has the _expected_ tx count (see conditions below)~~ 2. ~~after that, wait some time and get sure that the mempool contents haven't changed in-between~~ ~~Like for~~ Similar to the ancestor test, we overall check for ~~three~~ four conditions: - the # of txs in the node1 mempool is equal to the descendant limit (plus 1 for the parent tx, plus the # txs from the previous ancestor test which are still in) ~~(done by the hacky sync above)~~ - all txs in node1 mempool are a subset of txs in node0 mempool - part of the constructed descendant-chain (the first ones up to the limit) are contained in node1 mempool - the remaining part of the constructed descendant-chain (all after the first ones up to the limit) is *not* contained in node1 mempool ACKs for top commit: JeremyRubin: Excellent. utACK b902bd6 Tree-SHA512: 7de96dd248f16ab740e178ac5b64b57ead18cdcf74adfe989709d215e4a67b6b6d20de22c48e885d5f2edc55caaddd44a4261e996c5c87687ceb6a47f1d1fdaf
2020-02-27 21:02:14 +01:00
# Check that node1's mempool is as expected, containing:
# - txs from previous ancestor test (-> custom ancestor limit)
# - parent tx for descendant test
# - txs chained off parent tx (-> custom descendant limit)
(partial) Merge bitcoin/bitcoin#22707: test: refactor use of getrawmempool in functional tests for efficiency 47c48b5f35b4910fcf87caa6e37407e67d879c80 test: only use verbose for getrawmempool when necessary in functional tests (Michael Dietz) 77349713b189e80f2c140db4df50177353a1cb83 test: use getmempoolentry instead of getrawmempool in functional tests when appropriate (Michael Dietz) 86dbd54ae8a8f9c693c0ea67114bbff24a0754df 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 https://github.com/bitcoin/bitcoin/pull/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 47c48b5f35b4910fcf87caa6e37407e67d879c80 Tree-SHA512: 40f553715f3d4649dc18c2738554eafaca9ea800c4b028c099217896cc1c466ff457ae814d59cf8564c782a8964d8fac3eda60c1b6ffb08bbee1439b2d34434b Signed-off-by: Vijay <vijaydas.mp@gmail.com>
2021-08-20 17:38:38 +02:00
self.wait_until(lambda: len(self.nodes[1].getrawmempool()) ==
MAX_ANCESTORS_CUSTOM + 1 + MAX_DESCENDANTS_CUSTOM, timeout=10)
Merge #17461: test: check custom descendant limit in mempool_packages.py b902bd66b0f35c5016dc5d7aaf501940935edd62 test: check custom descendant limit in mempool_packages.py (Sebastian Falbesoner) Pull request description: This is a follow-up PR to #17435, testing the custom descendant limit, passed by the argument `-limitdescendantcount`. ~~It was more tricky than expected, mainly because we don't know for sure at which point node1 has got all the transactions broadcasted from node0 (for the ancestor test this wasn't a problem since the txs were immediately available through `invalidateblock`) -- a simple `sync_mempools()` doesn't work here since the mempool contents are not equal due to different ancestor/descendant limits. Hence I came up with a "hacky manual sync":~~ 1. ~~wait until the mempool has the _expected_ tx count (see conditions below)~~ 2. ~~after that, wait some time and get sure that the mempool contents haven't changed in-between~~ ~~Like for~~ Similar to the ancestor test, we overall check for ~~three~~ four conditions: - the # of txs in the node1 mempool is equal to the descendant limit (plus 1 for the parent tx, plus the # txs from the previous ancestor test which are still in) ~~(done by the hacky sync above)~~ - all txs in node1 mempool are a subset of txs in node0 mempool - part of the constructed descendant-chain (the first ones up to the limit) are contained in node1 mempool - the remaining part of the constructed descendant-chain (all after the first ones up to the limit) is *not* contained in node1 mempool ACKs for top commit: JeremyRubin: Excellent. utACK b902bd6 Tree-SHA512: 7de96dd248f16ab740e178ac5b64b57ead18cdcf74adfe989709d215e4a67b6b6d20de22c48e885d5f2edc55caaddd44a4261e996c5c87687ceb6a47f1d1fdaf
2020-02-27 21:02:14 +01:00
mempool0 = self.nodes[0].getrawmempool(False)
mempool1 = self.nodes[1].getrawmempool(False)
assert set(mempool1).issubset(set(mempool0))
assert parent_transaction in mempool1
for tx in chain[:MAX_DESCENDANTS_CUSTOM]:
assert tx in mempool1
for tx in chain[MAX_DESCENDANTS_CUSTOM:]:
assert tx not in mempool1
for tx in mempool1:
entry0 = self.nodes[0].getmempoolentry(tx)
entry1 = self.nodes[1].getmempoolentry(tx)
assert not entry0['unbroadcast']
assert not entry1['unbroadcast']
assert_equal(entry1['fees']['base'], entry0['fees']['base'])
assert_equal(entry1['vsize'], entry0['vsize'])
assert_equal(entry1['depends'], entry0['depends'])
# Test reorg handling
# First, the basics:
self.generate(self.nodes[0], 1)
self.sync_blocks()
self.nodes[1].invalidateblock(self.nodes[0].getbestblockhash())
self.nodes[1].reconsiderblock(self.nodes[0].getbestblockhash())
# Now test the case where node1 has a transaction T in its mempool that
# depends on transactions A and B which are in a mined block, and the
# block containing A and B is disconnected, AND B is not accepted back
# into node1's mempool because its ancestor count is too high.
# Create 8 transactions, like so:
# Tx0 -> Tx1 (vout0)
# \--> Tx2 (vout1) -> Tx3 -> Tx4 -> Tx5 -> Tx6 -> Tx7
#
# Mine them in the next block, then generate a new tx8 that spends
# Tx1 and Tx7, and add to node1's mempool, then disconnect the
# last block.
# Create tx0 with 2 outputs
utxo = self.nodes[0].listunspent()
txid = utxo[0]['txid']
value = utxo[0]['amount']
vout = utxo[0]['vout']
send_value = (value - fee) / 2
inputs = [ {'txid' : txid, 'vout' : vout} ]
outputs = {}
Merge #19674: refactor: test: use throwaway _ variable for unused loop counters dac7a111bdd3b0233d94cf68dae7a8bfc6ac9c64 refactor: test: use _ variable for unused loop counters (Sebastian Falbesoner) Pull request description: This tiny PR substitutes Python loops in the form of `for x in range(N): ...` by `for _ in range(N): ...` where applicable. The idea is indicating to the reader that a block (or statement, in list comprehensions) is just repeated N times, and that the loop counter is not used in the body, hence using the throwaway variable. This is already done quite often in the current tests (see e.g. `$ git grep "for _ in range("`). Another alternative would be using `itertools.repeat` (according to Python core developer Raymond Hettinger it's [even faster](https://twitter.com/raymondh/status/1144527183341375488)), but that doesn't seem to be widespread in use and I'm not sure about a readability increase. The only drawback I see is that whenever one wants to debug loop iterations, one would need to introduce a loop variable again. Reviewing this is basically a no-brainer, since tests would fail immediately if a a substitution has taken place on a loop where the variable is used. Instances to replace were found by `$ git grep "for.*in range("` and manually checked. ACKs for top commit: darosior: ACK dac7a111bdd3b0233d94cf68dae7a8bfc6ac9c64 instagibbs: manual inspection ACK https://github.com/bitcoin/bitcoin/pull/19674/commits/dac7a111bdd3b0233d94cf68dae7a8bfc6ac9c64 practicalswift: ACK dac7a111bdd3b0233d94cf68dae7a8bfc6ac9c64 -- the updated code is easier to reason about since the throwaway nature of a variable is expressed explicitly (using the Pythonic `_` idiom) instead of implicitly. Explicit is better than implicit was we all know by now :) Tree-SHA512: 5f43ded9ce14e5e00b3876ec445b90acda1842f813149ae7bafa93f3ac3d510bb778e2c701187fd2c73585e6b87797bb2d2987139bd1a9ba7d58775a59392406
2020-08-11 02:50:34 +02:00
for _ in range(2):
outputs[self.nodes[0].getnewaddress()] = send_value
rawtx = self.nodes[0].createrawtransaction(inputs, outputs)
signedtx = self.nodes[0].signrawtransactionwithwallet(rawtx)
txid = self.nodes[0].sendrawtransaction(signedtx['hex'])
tx0_id = txid
value = send_value
# Create tx1
tx1_id, _ = chain_transaction(self.nodes[0], [tx0_id], [0], value, fee, 1)
# Create tx2-7
vout = 1
txid = tx0_id
Merge #19674: refactor: test: use throwaway _ variable for unused loop counters dac7a111bdd3b0233d94cf68dae7a8bfc6ac9c64 refactor: test: use _ variable for unused loop counters (Sebastian Falbesoner) Pull request description: This tiny PR substitutes Python loops in the form of `for x in range(N): ...` by `for _ in range(N): ...` where applicable. The idea is indicating to the reader that a block (or statement, in list comprehensions) is just repeated N times, and that the loop counter is not used in the body, hence using the throwaway variable. This is already done quite often in the current tests (see e.g. `$ git grep "for _ in range("`). Another alternative would be using `itertools.repeat` (according to Python core developer Raymond Hettinger it's [even faster](https://twitter.com/raymondh/status/1144527183341375488)), but that doesn't seem to be widespread in use and I'm not sure about a readability increase. The only drawback I see is that whenever one wants to debug loop iterations, one would need to introduce a loop variable again. Reviewing this is basically a no-brainer, since tests would fail immediately if a a substitution has taken place on a loop where the variable is used. Instances to replace were found by `$ git grep "for.*in range("` and manually checked. ACKs for top commit: darosior: ACK dac7a111bdd3b0233d94cf68dae7a8bfc6ac9c64 instagibbs: manual inspection ACK https://github.com/bitcoin/bitcoin/pull/19674/commits/dac7a111bdd3b0233d94cf68dae7a8bfc6ac9c64 practicalswift: ACK dac7a111bdd3b0233d94cf68dae7a8bfc6ac9c64 -- the updated code is easier to reason about since the throwaway nature of a variable is expressed explicitly (using the Pythonic `_` idiom) instead of implicitly. Explicit is better than implicit was we all know by now :) Tree-SHA512: 5f43ded9ce14e5e00b3876ec445b90acda1842f813149ae7bafa93f3ac3d510bb778e2c701187fd2c73585e6b87797bb2d2987139bd1a9ba7d58775a59392406
2020-08-11 02:50:34 +02:00
for _ in range(6):
(txid, sent_value) = chain_transaction(self.nodes[0], [txid], [vout], value, fee, 1)
vout = 0
value = sent_value
# Mine these in a block
self.generate(self.nodes[0], 1)
# Now generate tx8, with a big fee
inputs = [ {'txid' : tx1_id, 'vout': 0}, {'txid' : txid, 'vout': 0} ]
outputs = { self.nodes[0].getnewaddress() : send_value + value - 4*fee }
rawtx = self.nodes[0].createrawtransaction(inputs, outputs)
signedtx = self.nodes[0].signrawtransactionwithwallet(rawtx)
txid = self.nodes[0].sendrawtransaction(signedtx['hex'])
self.sync_mempools()
# Now try to disconnect the tip on each node...
self.nodes[1].invalidateblock(self.nodes[1].getbestblockhash())
self.nodes[0].invalidateblock(self.nodes[0].getbestblockhash())
self.sync_blocks()
if __name__ == '__main__':
MempoolPackagesTest().main()