Details

Buildable 20893
Build 41443: Build Diff	build-without-wallet · lint-circular-dependencies · build-diff · build-debug · build-clang-tidy · build-clang
Build 41442: arc lint + arc unit

Event Timeline

PiRK created this revision.Nov 14 2022, 08:42

Herald added a reviewer: Restricted Project. · View Herald TranscriptNov 14 2022, 08:42

PiRK requested review of this revision.Nov 14 2022, 08:42

teamcity edited the summary of this revision. (Show Details)Nov 14 2022, 08:43

PiRK edited the summary of this revision. (Show Details)Nov 14 2022, 08:43

Harbormaster completed remote builds in B20893: Diff 36266.Nov 14 2022, 09:25

PiRK added a child revision: D12481: [validation] de-duplicate package transactions already in mempool.Nov 14 2022, 09:51

Fabien requested changes to this revision.Nov 15 2022, 10:24

Fabien added a subscriber: Fabien.

Fabien added inline comments.

src/validation.cpp
358
1048	Actually if you submit a package with all parents being confirmed, it will not return this error despite its name. I also think it will be accepted with no error. Is that the expected behavior ?
1120	This looks misplaced (but same in PR)

This revision now requires changes to proceed.Nov 15 2022, 10:24

Fabien mentioned this in D12481: [validation] de-duplicate package transactions already in mempool.Nov 15 2022, 10:39

Fabien added inline comments.Nov 15 2022, 16:19

src/validation.cpp
860

PiRK marked 3 inline comments as done.Nov 16 2022, 07:33

add a test for a package with a confirmed transaction
minor comments fixups from https://github.com/bitcoin/bitcoin/pull/23804/commits/9d88853e0c85f765f7d982b15e8122ede50110ed
one lint change that for some reason was not detected earlier

PiRK added inline comments.Nov 16 2022, 08:43

src/validation.cpp
1048	For now the code at this stage is not able to detect when a transaction in the package is already confirmed. It will fail later when it actually tries to submit transactions to the mempool.

Harbormaster completed remote builds in B20932: Diff 36319.Nov 16 2022, 08:50

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

Tail of the build log:

[0m[0;32mwallet_importprunedfunds.py               | ✓ Passed  | 3 s
[0m[0;32mwallet_importprunedfunds.py --descriptors | ✓ Passed  | 3 s
[0m[0;32mwallet_keypool.py                         | ✓ Passed  | 3 s
[0m[0;32mwallet_keypool_topup.py                   | ✓ Passed  | 3 s
[0m[0;32mwallet_keypool_topup.py --descriptors     | ✓ Passed  | 8 s
[0m[0;32mwallet_labels.py                          | ✓ Passed  | 5 s
[0m[0;32mwallet_labels.py --descriptors            | ✓ Passed  | 5 s
[0m[0;32mwallet_listreceivedby.py                  | ✓ Passed  | 9 s
[0m[0;32mwallet_listsinceblock.py                  | ✓ Passed  | 7 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  | 3 s
[0m[0;32mwallet_multiwallet.py                     | ✓ Passed  | 21 s
[0m[0;32mwallet_multiwallet.py --usecli            | ✓ Passed  | 16 s
[0m[0;32mwallet_reorgsrestore.py                   | ✓ Passed  | 4 s
[0m[0;32mwallet_resendwallettransactions.py        | ✓ Passed  | 3 s
[0m[0;32mwallet_send.py                            | ✓ Passed  | 9 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  | 3 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  | 1855 s (accumulated) 
[0mRuntime: 372 s

[172/481] Test Bitcoin RPC authentication...
...
----------------------------------------------------------------------
Ran 3 tests in 0.006s

OK
[174/481] cd /work/contrib/devtools/chainparams && /usr/bin/python3.9 ./test_make_chainparams.py
.....
----------------------------------------------------------------------
Ran 5 tests in 0.001s

OK
[210/481] Running seeder test suite
PASSED: seeder test suite
[368/481] bitcoin: testing txpackage_tests
FAILED: src/test/CMakeFiles/check-bitcoin-txpackage_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-txpackage_tests.log /work/abc-ci-builds/build-debug/src/test/test_bitcoin --run_test=txpackage_tests --logger=HRF,message:JUNIT,message,bitcoin-txpackage_tests.xml --catch_system_errors=no
Running 4 test cases...
Assertion failed: lock cs_main held in ../../src/validation.cpp:4475; locks held:
'cs_main' in ../../src/test/txpackage_tests.cpp:244 (in thread '')
Aborted (core dumped)
[439/481] Running secp256k1 test suite
PASSED: secp256k1 test suite
[451/481] Running avalanche test suite
PASSED: avalanche test suite
[453/481] Running pow test suite
PASSED: pow test suite
[466/481] Running bitcoin-qt test suite
PASSED: bitcoin-qt test suite
[478/481] 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

don't lock cs_main when calling CreateAndProcessBlock because ProcessNewBlock asserts that it is not held

Harbormaster completed remote builds in B20934: Diff 36321.Nov 16 2022, 09:31

rebase onto master

Harbormaster completed remote builds in B20939: Diff 36328.Nov 16 2022, 11:50

Fabien requested changes to this revision.Nov 16 2022, 14:02

Fabien added inline comments.

src/test/txpackage_tests.cpp
400 ↗	(On Diff #36328)	You need to check the tx error is the expected one. You expect "bad-txns-inputs-missingorspent" or "txn-already-known" but here you'll get "bad-txns-premature-spend-of-coinbase" so it's not testing what you imagine

This revision now requires changes to proceed.Nov 16 2022, 14:02

Fabien added inline comments.Nov 16 2022, 14:04

src/test/txpackage_tests.cpp
401 ↗	(On Diff #36328)

use the first coinbase as confirmed parent to get trigger "txn-already-known" instead of "bad-txns-premature-spend-of-coinbase", adjust the pool size accordingly

Harbormaster completed remote builds in B20943: Diff 36336.Nov 16 2022, 14:43

Fabien accepted this revision.Nov 16 2022, 16:56

Fabien added inline comments.

src/test/txpackage_tests.cpp
381 ↗	(On Diff #36336)	Please add a comment explaining why you need it to make the code easier to follow

This revision is now accepted and ready to land.Nov 16 2022, 16:56

add a comment for expected_pool_size -= 1;, and don't rebuild txns that already exist from a previous test.

Harbormaster completed remote builds in B20965: Diff 36361.Nov 17 2022, 07:32

Closed by commit rABC5b41420a65f2: [validation] package accept + mempool submission, require packages to be child… (authored by glozow <gloriajzhao@gmail.com>, committed by PiRK). · Explain WhyNov 17 2022, 09:52

This revision was automatically updated to reflect the committed changes.

PiRK added a commit: rABC5b41420a65f2: [validation] package accept + mempool submission, require packages to be child….

[validation] package accept + mempool submission, require packages to be child-with-unconfirmed-parents
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 36266

src/policy/mempool.h

src/test/txpackage_tests.cpp

src/validation.h

src/validation.cpp

[validation] package accept + mempool submission, require packages to be child-with-unconfirmed-parentsClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 36266

src/policy/mempool.h

src/test/txpackage_tests.cpp

src/validation.h

src/validation.cpp

[validation] package accept + mempool submission, require packages to be child-with-unconfirmed-parents
ClosedPublic
Actions

Revision Contents
Changeset List