diff options
author | fanquake <fanquake@gmail.com> | 2020-05-22 07:27:28 +0800 |
---|---|---|
committer | fanquake <fanquake@gmail.com> | 2020-05-22 07:51:51 +0800 |
commit | ad3a61c5f5c9eebff98ee59c8cc4612c1affa046 (patch) | |
tree | 620fa4a42d380e06e03c8132d32636963f2572eb /test | |
parent | 9abed46871a7f44277a0a59c645a8593ce9e5628 (diff) | |
parent | 651f1d816f054cb9c637f8a99c9360bba381ef58 (diff) | |
download | bitcoin-ad3a61c5f5c9eebff98ee59c8cc4612c1affa046.tar.xz |
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
Diffstat (limited to 'test')
-rwxr-xr-x | test/functional/mempool_packages.py | 10 | ||||
-rwxr-xr-x | test/functional/mempool_unbroadcast.py | 12 | ||||
-rwxr-xr-x | test/functional/test_framework/mininode.py | 10 |
3 files changed, 32 insertions, 0 deletions
diff --git a/test/functional/mempool_packages.py b/test/functional/mempool_packages.py index a07dad18d6..5b7216b253 100755 --- a/test/functional/mempool_packages.py +++ b/test/functional/mempool_packages.py @@ -7,6 +7,7 @@ from decimal import Decimal from test_framework.messages import COIN +from test_framework.mininode import P2PTxInvStore from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, @@ -58,6 +59,7 @@ class MempoolPackagesTest(BitcoinTestFramework): def run_test(self): # Mine some blocks and have them mature. + self.nodes[0].add_p2p_connection(P2PTxInvStore()) # keep track of invs self.nodes[0].generate(101) utxo = self.nodes[0].listunspent(10) txid = utxo[0]['txid'] @@ -72,6 +74,10 @@ class MempoolPackagesTest(BitcoinTestFramework): value = sent_value chain.append(txid) + # 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 + self.nodes[0].p2p.wait_for_broadcast(chain) + # Check mempool has MAX_ANCESTORS transactions in it, and descendant and ancestor # count and fees should look correct mempool = self.nodes[0].getrawmempool(True) @@ -212,6 +218,10 @@ class MempoolPackagesTest(BitcoinTestFramework): for tx in chain[:MAX_ANCESTORS_CUSTOM]: assert tx in mempool1 # TODO: more detailed check of node1's mempool (fees etc.) + # check transaction unbroadcast info (should be false if in both mempools) + mempool = self.nodes[0].getrawmempool(True) + for tx in mempool: + assert_equal(mempool[tx]['unbroadcast'], False) # TODO: test ancestor size limits diff --git a/test/functional/mempool_unbroadcast.py b/test/functional/mempool_unbroadcast.py index a561f28b91..dedf5b8a47 100755 --- a/test/functional/mempool_unbroadcast.py +++ b/test/functional/mempool_unbroadcast.py @@ -53,6 +53,13 @@ class MempoolUnbroadcastTest(BitcoinTestFramework): txFS = node.signrawtransactionwithwallet(txF["hex"]) rpc_tx_hsh = node.sendrawtransaction(txFS["hex"]) + # check transactions are in unbroadcast using rpc + mempoolinfo = self.nodes[0].getmempoolinfo() + assert_equal(mempoolinfo['unbroadcastcount'], 2) + mempool = self.nodes[0].getrawmempool(True) + for tx in mempool: + assert_equal(mempool[tx]['unbroadcast'], True) + # check that second node doesn't have these two txns mempool = self.nodes[1].getrawmempool() assert rpc_tx_hsh not in mempool @@ -71,6 +78,11 @@ class MempoolUnbroadcastTest(BitcoinTestFramework): assert rpc_tx_hsh in mempool assert wallet_tx_hsh in mempool + # check that transactions are no longer in first node's unbroadcast set + mempool = self.nodes[0].getrawmempool(True) + for tx in mempool: + assert_equal(mempool[tx]['unbroadcast'], False) + self.log.info("Add another connection & ensure transactions aren't broadcast again") conn = node.add_p2p_connection(P2PTxInvStore()) diff --git a/test/functional/test_framework/mininode.py b/test/functional/test_framework/mininode.py index bbd7350bf1..95a63717d6 100755 --- a/test/functional/test_framework/mininode.py +++ b/test/functional/test_framework/mininode.py @@ -645,6 +645,7 @@ class P2PTxInvStore(P2PInterface): self.tx_invs_received = defaultdict(int) def on_inv(self, message): + super().on_inv(message) # Send getdata in response. # Store how many times invs have been received for each tx. for i in message.inv: if i.type == MSG_TX: @@ -654,3 +655,12 @@ class P2PTxInvStore(P2PInterface): def get_invs(self): with mininode_lock: return list(self.tx_invs_received.keys()) + + def wait_for_broadcast(self, txns, timeout=60): + """Waits for the txns (list of txids) to complete initial broadcast. + The mempool should mark unbroadcast=False for these transactions. + """ + # Wait until invs have been received (and getdatas sent) for each txid. + self.wait_until(lambda: set(self.get_invs()) == set([int(tx, 16) for tx in txns]), timeout) + # Flush messages and wait for the getdatas to be processed + self.sync_with_ping() |