txorphanage: Extract OrphanageAddTx and drop AddOrphanTx
ClosedPublic
Actions

Authored by PiRK on May 18 2022, 08:44.

Details

Reviewers

Fabien

Group Reviewers

Restricted Project

Commits

rABCc15f8c9cc49d: txorphanage: Extract OrphanageAddTx and drop AddOrphanTx

Summary

txorphanage: Extract OrphanageAddTx

Extract code from AddOrphanTx into OrphanageAddTx.

https://github.com/bitcoin/bitcoin/pull/21148/commits/1041616d7eb66281bb4de51ffbc83df0923b2f7e

denialofservices_tests: check txorphanage's AddTx

Rather than checking net_processing's internal implementation of
AddOrphanTx, test txorphanage's exported AddTx interface. Note that
this means AddToCompactExtraTransactions is no longer tested here.

https://github.com/bitcoin/bitcoin/pull/21148/commits/26d1a6ccd5fcc7abec737c0d8c67238561627d59

net_processing: drop AddOrphanTx

All the interesting functionality of AddOrphanTx is already in other
functions, so call those functions directly in the one place that
AddOrphanTx was used.

https://github.com/bitcoin/bitcoin/pull/21148/commits/3c4c3c2fdda3a361e3802e97bc3566f815b75de1

This is a backport of core#21148 [7, 8 & 9/14]

Depends on D11486

Note: the way Core used the lock in denialofservice_tests depends on RandomOrphan not taking the locks, which Core does in a subsequent commit of the same PR.

Test Plan

With clang and debug:

ninja all check-all

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

PiRK created this revision.May 18 2022, 08:44

Herald added a reviewer: Restricted Project. · View Herald TranscriptMay 18 2022, 08:44

PiRK requested review of this revision.May 18 2022, 08:44

teamcity edited the summary of this revision. (Show Details)May 18 2022, 08:44

PiRK retitled this revision from txorphanage: Extract OrphanageAddTx to txorphanage: Extract OrphanageAddTx and drop AddOrphanTx.May 18 2022, 08:46

PiRK edited the summary of this revision. (Show Details)

Harbormaster completed remote builds in B19076: Diff 33578.May 18 2022, 08:52

Build Bitcoin ABC Diffs / Diff Testing (build-debug) failed.

Tail of the build log:

[0m[0;32mwallet_keypool_topup.py --descriptors                  | ✓ Passed  | 4 s
[0m[0;32mwallet_labels.py                                       | ✓ Passed  | 3 s
[0m[0;32mwallet_labels.py --descriptors                         | ✓ Passed  | 2 s
[0m[0;32mwallet_listreceivedby.py                               | ✓ Passed  | 15 s
[0m[0;32mwallet_listsinceblock.py                               | ✓ Passed  | 8 s
[0m[0;32mwallet_listsinceblock.py --descriptors                 | ✓ Passed  | 10 s
[0m[0;32mwallet_listtransactions.py                             | ✓ Passed  | 4 s
[0m[0;32mwallet_listtransactions.py --descriptors               | ✓ Passed  | 4 s
[0m[0;32mwallet_multiwallet.py                                  | ✓ Passed  | 22 s
[0m[0;32mwallet_multiwallet.py --usecli                         | ✓ Passed  | 20 s
[0m[0;32mwallet_reorgsrestore.py                                | ✓ Passed  | 5 s
[0m[0;32mwallet_resendwallettransactions.py                     | ✓ Passed  | 4 s
[0m[0;32mwallet_send.py                                         | ✓ Passed  | 12 s
[0m[0;32mwallet_startup.py                                      | ✓ Passed  | 3 s
[0m[0;32mwallet_txn_clone.py                                    | ✓ Passed  | 3 s
[0m[0;32mwallet_txn_clone.py --mineblock                        | ✓ Passed  | 4 s
[0m[0;32mwallet_txn_doublespend.py                              | ✓ Passed  | 3 s
[0m[0;32mwallet_txn_doublespend.py --mineblock                  | ✓ Passed  | 4 s
[0m[0;32mwallet_watchonly.py                                    | ✓ Passed  | 1 s
[0m[0;32mwallet_watchonly.py --usecli                           | ✓ Passed  | 2 s
[0m[1m
ALL                                                    | ✓ Passed  | 1789 s (accumulated) 
[0mRuntime: 359 s

----------------------------------------------------------------------
Ran 10 tests in 0.147s

OK

[34/471] Test Bitcoin RPC authentication...
...
----------------------------------------------------------------------
Ran 3 tests in 0.004s

OK
[36/471] cd /work/contrib/devtools/chainparams && /usr/bin/python3.7 ./test_make_chainparams.py
.....
----------------------------------------------------------------------
Ran 5 tests in 0.001s

OK
[191/471] Running seeder test suite
PASSED: seeder test suite
[215/471] Running bitcoin-qt test suite
PASSED: bitcoin-qt test suite
[219/471] Running avalanche test suite
PASSED: avalanche test suite
[316/471] bitcoin: testing denialofservice_tests
FAILED: src/test/CMakeFiles/check-bitcoin-denialofservice_tests 
cd /work/abc-ci-builds/build-debug/src/test && /usr/bin/cmake -E make_directory /work/abc-ci-builds/build-debug/test/junit && /usr/bin/cmake -E make_directory /work/abc-ci-builds/build-debug/test/log && /usr/bin/cmake -E env /work/cmake/utils/log-and-print-on-failure.sh /work/abc-ci-builds/build-debug/test/log/bitcoin-denialofservice_tests.log /work/abc-ci-builds/build-debug/src/test/test_bitcoin --run_test=denialofservice_tests --logger=HRF,message:JUNIT,message,bitcoin-denialofservice_tests.xml --catch_system_errors=no
Running 5 test cases...
Assertion failed: detected inconsistent lock order for 'cs_main' in ../../src/test/denialofservice_tests.cpp:339 (in thread ''), details in debug log.
Aborted (core dumped)
[444/471] Running pow test suite
PASSED: pow test suite
[462/471] Running secp256k1 test suite
PASSED: secp256k1 test suite
[468/471] Running utility command for check-bitcoin-coins_tests
ninja: build stopped: cannot make progress due to previous errors.
Build build-debug failed with exit code 1

I missed a lock inconsistency error message

don't take locks on RandomOrphan helper function, but annotate it to require g_cs_orphan. Core did this out of sequence, in a later commit of this same PR.

Harbormaster completed remote builds in B19077: Diff 33579.May 18 2022, 10:18

PiRK added a child revision: D11489: orphanage: Extract EraseOrphansForBlock.May 18 2022, 14:23

Fabien accepted this revision.May 18 2022, 15:02

This revision is now accepted and ready to land.May 18 2022, 15:02

rebase

Harbormaster completed remote builds in B19097: Diff 33600.May 19 2022, 08:28

Closed by commit rABCc15f8c9cc49d: txorphanage: Extract OrphanageAddTx and drop AddOrphanTx (authored by PiRK). · Explain WhyMay 19 2022, 16:07

This revision was automatically updated to reflect the committed changes.

PiRK added a commit: rABCc15f8c9cc49d: txorphanage: Extract OrphanageAddTx and drop AddOrphanTx.