Page MenuHomePhabricator
Differential D9025

[mempool] sanity check that all unbroadcast txns are in mempool
ClosedPublic
Actions

Authored by PiRK on Jan 22 2021, 02:08.

Details

Reviewers
Fabien
Group Reviewers
Restricted Project
Commits
rABC002b33b419aa: [mempool] sanity check that all unbroadcast txns are in mempool
Summary
  • before reattempting broadcast for unbroadcast txns, check they are in mempool and remove if not
  • this protects from memory leaks and network spam just in case unbroadcast set (incorrectly) has extra txns
  • check that tx is in mempool before adding to unbroadcast set to try to prevent this from happening

This is a backport of Core PR18895 [3/3]
https://github.com/bitcoin/bitcoin/pull/18895/commits/9d3f7eb9860254eb787ebe2734fd6a26bcf365c1
Depends on D9024

Test Plan

ninja all check-all

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

PiRK created this revision.Jan 22 2021, 02:08
Herald added a reviewer: Restricted Project. · View Herald TranscriptJan 22 2021, 02:08
PiRK requested review of this revision.Jan 22 2021, 02:08
teamcity edited the summary of this revision. (Show Details)Jan 22 2021, 02:08
Harbormaster completed remote builds in B14928: Diff 27219.Jan 22 2021, 02:25
teamcity added a comment.Jan 22 2021, 02:25
Build Bitcoin ABC Diffs / Diff Testing (build-debug) failed.

Failed tests logs:

====== Bitcoin ABC functional tests: mempool_packages.py ======

------- Stdout: -------
2021-01-22T02:22:29.984000Z TestFramework (INFO): Initializing test directory /work/abc-ci-builds/build-debug/test/tmp/test_runner_₿₵_  _20210122_022051/mempool_packages_262
2021-01-22T02:22:33.156000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
  File "/work/test/functional/test_framework/test_framework.py", line 120, in main
    self.run_test()
  File "/work/test/functional/mempool_packages.py", line 87, in run_test
    assert_equal(entry, mempool[x])
  File "/work/test/functional/test_framework/util.py", line 60, in assert_equal
    for arg in (thing1, thing2) + args)))
AssertionError: not({'fees': {'base': Decimal('0.00010000'), 'modified': Decimal('0.00010000'), 'ancestor': Decimal('0.00080000'), 'descendant': Decimal('0.00430000')}, 'size': 191, 'fee': Decimal('0.00010000'), 'modifiedfee': Decimal('0.00010000'), 'time': 1611282150, 'height': 301, 'descendantcount': 43, 'descendantsize': 8212, 'descendantfees': 430000, 'ancestorcount': 8, 'ancestorsize': 1528, 'ancestorfees': 80000, 'depends': ['13ba5dab11983ee7144e8129e6c37da02831aff785d7ef9cd9d0a91a5f7f9445'], 'spentby': ['35fa0e9dcc4f1dc83e046758eeeb7b03298d5421ca96c9c00220f1e5e3029662'], 'unbroadcast': False} == {'fees': {'base': Decimal('0.00010000'), 'modified': Decimal('0.00010000'), 'ancestor': Decimal('0.00080000'), 'descendant': Decimal('0.00430000')}, 'size': 191, 'fee': Decimal('0.00010000'), 'modifiedfee': Decimal('0.00010000'), 'time': 1611282150, 'height': 301, 'descendantcount': 43, 'descendantsize': 8212, 'descendantfees': 430000, 'ancestorcount': 8, 'ancestorsize': 1528, 'ancestorfees': 80000, 'depends': ['13ba5dab11983ee7144e8129e6c37da02831aff785d7ef9cd9d0a91a5f7f9445'], 'spentby': ['35fa0e9dcc4f1dc83e046758eeeb7b03298d5421ca96c9c00220f1e5e3029662'], 'unbroadcast': True})
2021-01-22T02:22:33.207000Z TestFramework (INFO): Stopping nodes
2021-01-22T02:22:33.309000Z TestFramework (WARNING): Not cleaning up dir /work/abc-ci-builds/build-debug/test/tmp/test_runner_₿₵_  _20210122_022051/mempool_packages_262
2021-01-22T02:22:33.309000Z TestFramework (ERROR): Test failed. Test logging available at /work/abc-ci-builds/build-debug/test/tmp/test_runner_₿₵_  _20210122_022051/mempool_packages_262/test_framework.log
2021-01-22T02:22:33.310000Z TestFramework (ERROR): Hint: Call /work/test/functional/combine_logs.py '/work/abc-ci-builds/build-debug/test/tmp/test_runner_₿₵_  _20210122_022051/mempool_packages_262' to consolidate all logs

Each failure log is accessible here:
Bitcoin ABC functional tests: mempool_packages.py

teamcity added a comment.Jan 22 2021, 02:29
Build Bitcoin ABC Diffs / Diff Testing (build-diff) failed.

Failed tests logs:

====== Bitcoin ABC functional tests: mempool_packages.py ======

------- Stdout: -------
2021-01-22T02:24:09.185000Z TestFramework (INFO): Initializing test directory /work/abc-ci-builds/build-diff/test/tmp/test_runner_₿₵_  _20210122_022241/mempool_packages_210
2021-01-22T02:24:12.857000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
  File "/work/test/functional/test_framework/test_framework.py", line 120, in main
    self.run_test()
  File "/work/test/functional/mempool_packages.py", line 87, in run_test
    assert_equal(entry, mempool[x])
  File "/work/test/functional/test_framework/util.py", line 60, in assert_equal
    for arg in (thing1, thing2) + args)))
AssertionError: not({'fees': {'base': Decimal('0.00010000'), 'modified': Decimal('0.00010000'), 'ancestor': Decimal('0.00020000'), 'descendant': Decimal('0.00490000')}, 'size': 191, 'fee': Decimal('0.00010000'), 'modifiedfee': Decimal('0.00010000'), 'time': 1611282250, 'height': 301, 'descendantcount': 49, 'descendantsize': 9359, 'descendantfees': 490000, 'ancestorcount': 2, 'ancestorsize': 382, 'ancestorfees': 20000, 'depends': ['93797ea899dd24488b1a418b2d48d264d753434ba975b25196d5038a645a533b'], 'spentby': ['e36e14e6b8dac9a77936c12d6e023513e31ad4dab0daa1d09c04b80dac8ef288'], 'unbroadcast': False} == {'fees': {'base': Decimal('0.00010000'), 'modified': Decimal('0.00010000'), 'ancestor': Decimal('0.00020000'), 'descendant': Decimal('0.00490000')}, 'size': 191, 'fee': Decimal('0.00010000'), 'modifiedfee': Decimal('0.00010000'), 'time': 1611282250, 'height': 301, 'descendantcount': 49, 'descendantsize': 9359, 'descendantfees': 490000, 'ancestorcount': 2, 'ancestorsize': 382, 'ancestorfees': 20000, 'depends': ['93797ea899dd24488b1a418b2d48d264d753434ba975b25196d5038a645a533b'], 'spentby': ['e36e14e6b8dac9a77936c12d6e023513e31ad4dab0daa1d09c04b80dac8ef288'], 'unbroadcast': True})
2021-01-22T02:24:12.908000Z TestFramework (INFO): Stopping nodes
2021-01-22T02:24:13.662000Z TestFramework (WARNING): Not cleaning up dir /work/abc-ci-builds/build-diff/test/tmp/test_runner_₿₵_  _20210122_022241/mempool_packages_210
2021-01-22T02:24:13.662000Z TestFramework (ERROR): Test failed. Test logging available at /work/abc-ci-builds/build-diff/test/tmp/test_runner_₿₵_  _20210122_022241/mempool_packages_210/test_framework.log
2021-01-22T02:24:13.663000Z TestFramework (ERROR): Hint: Call /work/test/functional/combine_logs.py '/work/abc-ci-builds/build-diff/test/tmp/test_runner_₿₵_  _20210122_022241/mempool_packages_210' to consolidate all logs
====== Bitcoin ABC functional tests with the next upgrade activated: mempool_unbroadcast.py ======

------- Stdout: -------
2021-01-22T02:28:38.186000Z TestFramework (INFO): Initializing test directory /work/abc-ci-builds/build-diff/test/tmp/test_runner_₿₵_  _20210122_022543/mempool_unbroadcast_337
2021-01-22T02:28:38.815000Z TestFramework (INFO): Test that mempool reattempts delivery of locally submitted transaction
2021-01-22T02:28:39.119000Z TestFramework (INFO): Generate transactions that only node 0 knows about
2021-01-22T02:28:39.455000Z TestFramework (ERROR): JSONRPC error
Traceback (most recent call last):
  File "/work/test/functional/test_framework/test_framework.py", line 120, in main
    self.run_test()
  File "/work/test/functional/mempool_unbroadcast.py", line 28, in run_test
    self.test_broadcast()
  File "/work/test/functional/mempool_unbroadcast.py", line 55, in test_broadcast
    rpc_tx_hsh = node.sendrawtransaction(txFS["hex"])
  File "/work/test/functional/test_framework/coverage.py", line 48, in __call__
    return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs)
  File "/work/test/functional/test_framework/authproxy.py", line 159, in __call__
    raise JSONRPCException(response['error'], status)
test_framework.authproxy.JSONRPCException: txn-mempool-conflict (-26)
2021-01-22T02:28:39.508000Z TestFramework (INFO): Stopping nodes
2021-01-22T02:28:40.167000Z TestFramework (WARNING): Not cleaning up dir /work/abc-ci-builds/build-diff/test/tmp/test_runner_₿₵_  _20210122_022543/mempool_unbroadcast_337
2021-01-22T02:28:40.168000Z TestFramework (ERROR): Test failed. Test logging available at /work/abc-ci-builds/build-diff/test/tmp/test_runner_₿₵_  _20210122_022543/mempool_unbroadcast_337/test_framework.log
2021-01-22T02:28:40.168000Z TestFramework (ERROR): Hint: Call /work/test/functional/combine_logs.py '/work/abc-ci-builds/build-diff/test/tmp/test_runner_₿₵_  _20210122_022543/mempool_unbroadcast_337' to consolidate all logs

Each failure log is accessible here:
Bitcoin ABC functional tests: mempool_packages.py
Bitcoin ABC functional tests with the next upgrade activated: mempool_unbroadcast.py

PiRK planned changes to this revision.Jan 22 2021, 02:30
PiRK updated this revision to Diff 27222.Jan 22 2021, 07:45

rebase after squashing commit 4 into commit 2 to fix the broken test

PiRK edited the summary of this revision. (Show Details)Jan 22 2021, 07:50
Harbormaster completed remote builds in B14931: Diff 27222.Jan 22 2021, 07:54
teamcity added a comment.Jan 22 2021, 07:55
Build Bitcoin ABC Diffs / Diff Testing (build-debug) failed.

Failed tests logs:

====== Bitcoin ABC functional tests: mempool_packages.py ======

------- Stdout: -------
2021-01-22T07:52:12.832000Z TestFramework (INFO): Initializing test directory /work/abc-ci-builds/build-debug/test/tmp/test_runner_₿₵_  _20210122_075040/mempool_packages_505
2021-01-22T07:52:14.259000Z TestFramework (ERROR): Unexpected exception caught during testing
Traceback (most recent call last):
  File "/work/test/functional/test_framework/test_framework.py", line 120, in main
    self.run_test()
  File "/work/test/functional/mempool_packages.py", line 77, in run_test
    self.nodes[0].p2p.wait_for_broadcast(chain)
  File "/work/test/functional/test_framework/mininode.py", line 769, in wait_for_broadcast
    self.wait_until(lambda: set(self.get_invs()) == set(
AttributeError: 'P2PTxInvStore' object has no attribute 'wait_until'
2021-01-22T07:52:14.310000Z TestFramework (INFO): Stopping nodes
2021-01-22T07:52:14.612000Z TestFramework (WARNING): Not cleaning up dir /work/abc-ci-builds/build-debug/test/tmp/test_runner_₿₵_  _20210122_075040/mempool_packages_505
2021-01-22T07:52:14.613000Z TestFramework (ERROR): Test failed. Test logging available at /work/abc-ci-builds/build-debug/test/tmp/test_runner_₿₵_  _20210122_075040/mempool_packages_505/test_framework.log
2021-01-22T07:52:14.613000Z TestFramework (ERROR): Hint: Call /work/test/functional/combine_logs.py '/work/abc-ci-builds/build-debug/test/tmp/test_runner_₿₵_  _20210122_075040/mempool_packages_505' to consolidate all logs

Each failure log is accessible here:
Bitcoin ABC functional tests: mempool_packages.py

PiRK added a comment.Jan 22 2021, 08:02

This has a dependency on PR18617.

PiRK planned changes to this revision.Jan 22 2021, 08:03
teamcity added a comment.Jan 22 2021, 08:04
Build Bitcoin ABC Diffs / Diff Testing (build-diff) failed.

Failed tests logs:

====== Bitcoin ABC functional tests: mempool_packages.py ======

------- Stdout: -------
2021-01-22T07:57:54.663000Z TestFramework (INFO): Initializing test directory /work/abc-ci-builds/build-diff/test/tmp/test_runner_₿₵_  _20210122_075624/mempool_packages_531
2021-01-22T07:57:55.686000Z TestFramework (ERROR): Unexpected exception caught during testing
Traceback (most recent call last):
  File "/work/test/functional/test_framework/test_framework.py", line 120, in main
    self.run_test()
  File "/work/test/functional/mempool_packages.py", line 77, in run_test
    self.nodes[0].p2p.wait_for_broadcast(chain)
  File "/work/test/functional/test_framework/mininode.py", line 769, in wait_for_broadcast
    self.wait_until(lambda: set(self.get_invs()) == set(
AttributeError: 'P2PTxInvStore' object has no attribute 'wait_until'
2021-01-22T07:57:55.737000Z TestFramework (INFO): Stopping nodes
2021-01-22T07:57:56.044000Z TestFramework (WARNING): Not cleaning up dir /work/abc-ci-builds/build-diff/test/tmp/test_runner_₿₵_  _20210122_075624/mempool_packages_531
2021-01-22T07:57:56.044000Z TestFramework (ERROR): Test failed. Test logging available at /work/abc-ci-builds/build-diff/test/tmp/test_runner_₿₵_  _20210122_075624/mempool_packages_531/test_framework.log
2021-01-22T07:57:56.044000Z TestFramework (ERROR): Hint: Call /work/test/functional/combine_logs.py '/work/abc-ci-builds/build-diff/test/tmp/test_runner_₿₵_  _20210122_075624/mempool_packages_531' to consolidate all logs
====== Bitcoin ABC functional tests with the next upgrade activated: mempool_packages.py ======

------- Stdout: -------
2021-01-22T08:01:09.820000Z TestFramework (INFO): Initializing test directory /work/abc-ci-builds/build-diff/test/tmp/test_runner_₿₵_  _20210122_075940/mempool_packages_238
2021-01-22T08:01:11.710000Z TestFramework (ERROR): Unexpected exception caught during testing
Traceback (most recent call last):
  File "/work/test/functional/test_framework/test_framework.py", line 120, in main
    self.run_test()
  File "/work/test/functional/mempool_packages.py", line 77, in run_test
    self.nodes[0].p2p.wait_for_broadcast(chain)
  File "/work/test/functional/test_framework/mininode.py", line 769, in wait_for_broadcast
    self.wait_until(lambda: set(self.get_invs()) == set(
AttributeError: 'P2PTxInvStore' object has no attribute 'wait_until'
2021-01-22T08:01:11.761000Z TestFramework (INFO): Stopping nodes
2021-01-22T08:01:12.213000Z TestFramework (WARNING): Not cleaning up dir /work/abc-ci-builds/build-diff/test/tmp/test_runner_₿₵_  _20210122_075940/mempool_packages_238
2021-01-22T08:01:12.213000Z TestFramework (ERROR): Test failed. Test logging available at /work/abc-ci-builds/build-diff/test/tmp/test_runner_₿₵_  _20210122_075940/mempool_packages_238/test_framework.log
2021-01-22T08:01:12.213000Z TestFramework (ERROR): Hint: Call /work/test/functional/combine_logs.py '/work/abc-ci-builds/build-diff/test/tmp/test_runner_₿₵_  _20210122_075940/mempool_packages_238' to consolidate all logs

Each failure log is accessible here:
Bitcoin ABC functional tests: mempool_packages.py
Bitcoin ABC functional tests with the next upgrade activated: mempool_packages.py

PiRK updated this revision to Diff 27227.Jan 22 2021, 08:33

rebase onto D9027

Fabien accepted this revision.Jan 22 2021, 09:04
This revision is now accepted and ready to land.Jan 22 2021, 09:04
Harbormaster completed remote builds in B14935: Diff 27227.Jan 22 2021, 09:12
Closed by commit rABC002b33b419aa: [mempool] sanity check that all unbroadcast txns are in mempool (authored by gzhao408 <gzhao408@berkeley.edu>, committed by PiRK). · Explain WhyJan 22 2021, 09:44
This revision was automatically updated to reflect the committed changes.
PiRK added a commit: rABC002b33b419aa: [mempool] sanity check that all unbroadcast txns are in mempool.

Revision Contents
Changeset List

PathSize
src/
7 lines
5 lines

Diff 27236

View Options

src/net_processing.cpp

Loading...
View Options

src/txmempool.h

Loading...