Page MenuHomePhabricator
Differential D5013

CheckBlockIndex before unlocking cs_main
Changes PlannedPublic
Actions

Authored by markblundeberg on Jan 18 2020, 03:06.

Details

Reviewers
fpelliccioni
Fabien
deadalnix
Group Reviewers
Restricted Project
Summary

probes the bug introduced during D4802. should be fixed after D4929.

Note: It's not a serious problem, only would affect regtest nodes having a race during reconsiderblock or parkblock RPC commands which basically nobody uses.

Depends on D4929.

Test Plan

ninja check-all

Diff Detail

Repository
rABC Bitcoin ABC
Branch
crashy
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 9045
Build 16052: Default Diff Build & Tests
Build 16051: arc lint + arc unit

Event Timeline

markblundeberg created this revision.Jan 18 2020, 03:06
Herald added a reviewer: Restricted Project. · View Herald TranscriptJan 18 2020, 03:06
Herald added a reviewer: deadalnix. · View Herald Transcript
teamcity added a comment.Jan 18 2020, 03:06

Build Bitcoin-ABC / Diffs / Diff Testing started.

markblundeberg edited the summary of this revision. (Show Details)Jan 18 2020, 03:06
Harbormaster failed remote builds in B9045: Diff 15639!Jan 18 2020, 03:11
teamcity added a comment.Jan 18 2020, 03:11
Build Bitcoin-ABC / Diffs / Diff Testing failed.

Snippet of first build failure:

[03:11:23] :	 [Step 1/1]  [0m [0;34mwallet_keypool_topup.py                 | ✓ Passed  | 3 s
[03:11:23] :	 [Step 1/1]  [0m [0;34mwallet_labels.py                        | ✓ Passed  | 3 s
[03:11:23] :	 [Step 1/1]  [0m [0;34mwallet_listreceivedby.py                | ✓ Passed  | 9 s
[03:11:23] :	 [Step 1/1]  [0m [0;34mwallet_listsinceblock.py                | ✓ Passed  | 1 s
[03:11:23] :	 [Step 1/1]  [0m [0;34mwallet_listtransactions.py              | ✓ Passed  | 11 s
[03:11:23] :	 [Step 1/1]  [0m [0;34mwallet_multiwallet.py                   | ✓ Passed  | 7 s
[03:11:23] :	 [Step 1/1]  [0m [0;34mwallet_multiwallet.py --usecli          | ✓ Passed  | 8 s
[03:11:23] :	 [Step 1/1]  [0m [0;34mwallet_resendwallettransactions.py      | ✓ Passed  | 1 s
[03:11:23] :	 [Step 1/1]  [0m [0;34mwallet_txn_clone.py                     | ✓ Passed  | 2 s
[03:11:23] :	 [Step 1/1]  [0m [0;34mwallet_txn_clone.py --mineblock         | ✓ Passed  | 2 s
[03:11:23] :	 [Step 1/1]  [0m [0;34mwallet_txn_doublespend.py               | ✓ Passed  | 1 s
[03:11:23] :	 [Step 1/1]  [0m [0;34mwallet_txn_doublespend.py --mineblock   | ✓ Passed  | 2 s
[03:11:23] :	 [Step 1/1]  [0m [0;34mwallet_zapwallettxes.py                 | ✓ Passed  | 2 s
[03:11:23] :	 [Step 1/1]  [0m [0;31mabc-finalize-block.py                   | ✖ Failed  | 0 s
[03:11:23] :	 [Step 1/1]  [0m [0;31mabc-get-invalid-block.py                | ✖ Failed  | 0 s
[03:11:23] :	 [Step 1/1]  [0m [0;31mabc-invalid-chains.py                   | ✖ Failed  | 0 s
[03:11:23] :	 [Step 1/1]  [0m [0;31mabc-mempool-coherence-on-activations.py | ✖ Failed  | 1 s
[03:11:23] :	 [Step 1/1]  [0m [0;31mabc-parkedchain.py                      | ✖ Failed  | 0 s
[03:11:23] :	 [Step 1/1]  [0m [0;31mabc-replay-protection.py                | ✖ Failed  | 1 s
[03:11:23] :	 [Step 1/1]  [0m [0;31mfeature_bip68_sequence.py               | ✖ Failed  | 18 s
[03:11:23] :	 [Step 1/1]  [0m [0;31mfeature_csv_activation.py               | ✖ Failed  | 1 s
[03:11:23] :	 [Step 1/1]  [0m [0;31mfeature_notifications.py                | ✖ Failed  | 1 s
[03:11:23] :	 [Step 1/1]  [0m [0;31mmempool_packages.py                     | ✖ Failed  | 0 s
[03:11:23] :	 [Step 1/1]  [0m [0;31mmempool_reorg.py                        | ✖ Failed  | 2 s
[03:11:23] :	 [Step 1/1]  [0m [0;31mmempool_resurrect.py                    | ✖ Failed  | 0 s
[03:11:23] :	 [Step 1/1]  [0m [0;31mmining_prioritisetransaction.py         | ✖ Failed  | 2 s
[03:11:23] :	 [Step 1/1]  [0m [0;31mp2p_sendheaders.py                      | ✖ Failed  | 4 s
[03:11:23] :	 [Step 1/1]  [0m [0;31mrpc_blockchain.py                       | ✖ Failed  | 1 s
[03:11:23] :	 [Step 1/1]  [0m [0;31mrpc_invalidateblock.py                  | ✖ Failed  | 0 s
[03:11:23] :	 [Step 1/1]  [0m [0;31mrpc_rawtransaction.py                   | ✖ Failed  | 4 s
[03:11:23] :	 [Step 1/1]  [0m [0;31mwallet_abandonconflict.py               | ✖ Failed  | 5 s
[03:11:23] :	 [Step 1/1]  [0m [0;31m [1m
[03:11:23] :	 [Step 1/1] ALL                                     | ✖ Failed  | 352 s (accumulated) 
[03:11:23]W:	 [Step 1/1] ++ print_sanitizers_log
[03:11:23]W:	 [Step 1/1] ++ for log in "${SAN_LOG_DIR}"/*.log.*
[03:11:23] :	 [Step 1/1]  [0m [0mRuntime: 203 s
[03:11:23]W:	 [Step 1/1] ++ echo '*** Output of /tmp/sanitizer_logs/*.log.* ***'
[03:11:23] :	 [Step 1/1] 
[03:11:23]W:	 [Step 1/1] ++ cat '/tmp/sanitizer_logs/*.log.*'
[03:11:23] :	 [Step 1/1] [239/352] Running bitcoin-qt test suite
[03:11:23] :	 [Step 1/1] PASSED: bitcoin-qt test suite
[03:11:23] :	 [Step 1/1] [240/352] cd /home/teamcity/buildAgent/work/c4a5708f2bae7929/contrib/devtools/chainparams && /usr/bin/python3 ./test_make_chainparams.py
[03:11:23] :	 [Step 1/1] .....
[03:11:23] :	 [Step 1/1] ----------------------------------------------------------------------
[03:11:23] :	 [Step 1/1] Ran 5 tests in 0.001s
[03:11:23] :	 [Step 1/1] 
[03:11:23] :	 [Step 1/1] OK
[03:11:23] :	 [Step 1/1] [287/352] Running secp256k1 test suite
[03:11:23] :	 [Step 1/1] PASSED: secp256k1 test suite
[03:11:23] :	 [Step 1/1] [349/352] Running bitcoin test suite
[03:11:23] :	 [Step 1/1] PASSED: bitcoin test suite
[03:11:23] :	 [Step 1/1] [351/352] Running leveldb test suite
[03:11:23] :	 [Step 1/1] PASSED: leveldb test suite
[03:11:23] :	 [Step 1/1] FAILED: test/CMakeFiles/check-functional 
[03:11:23] :	 [Step 1/1] cd /home/teamcity/buildAgent/work/c4a5708f2bae7929/build/test && /usr/bin/python3 ./functional/test_runner.py
[03:11:23] :	 [Step 1/1] ninja: build stopped: subcommand failed.
[03:11:23] :	 [Step 1/1] *** Output of /tmp/sanitizer_logs/*.log.* ***
[03:11:23]W:	 [Step 1/1] cat: '/tmp/sanitizer_logs/*.log.*': No such file or directory
[03:11:23]W:	 [Step 1/1] Process exited with code 1
[03:11:23]E:	 [Step 1/1] Process exited with code 1 (Step: Command Line)

Each failure log is accessible here:
bitcoin_abc_tests: abc-finalize-block.py
bitcoin_abc_tests: abc-get-invalid-block.py
bitcoin_abc_tests: abc-invalid-chains.py
bitcoin_abc_tests: abc-mempool-coherence-on-activations.py
bitcoin_abc_tests: abc-parkedchain.py
bitcoin_abc_tests: abc-replay-protection.py
bitcoin_abc_tests: feature_bip68_sequence.py
bitcoin_abc_tests: feature_csv_activation.py
bitcoin_abc_tests: feature_notifications.py
bitcoin_abc_tests: mempool_packages.py
bitcoin_abc_tests: mempool_reorg.py
bitcoin_abc_tests: mempool_resurrect.py
bitcoin_abc_tests: mining_prioritisetransaction.py
bitcoin_abc_tests: p2p_sendheaders.py
bitcoin_abc_tests: rpc_blockchain.py
bitcoin_abc_tests: rpc_invalidateblock.py
bitcoin_abc_tests: rpc_rawtransaction.py
bitcoin_abc_tests: wallet_abandonconflict.py

markblundeberg added a comment.Jan 18 2020, 03:12

Above failures on master, now will try rebased.

markblundeberg updated this revision to Diff 15640.Jan 18 2020, 03:22
markblundeberg edited the summary of this revision. (Show Details)
markblundeberg added a parent revision: D4929: Make last disconnected block BLOCK_FAILED_VALID, even when aborted.

rebase onto D4929

teamcity added a comment.Jan 18 2020, 03:22

Build Bitcoin-ABC / Diffs / Diff Testing started.

markblundeberg edited the summary of this revision. (Show Details)Jan 18 2020, 03:23
teamcity added a comment.Jan 18 2020, 03:25
Build Bitcoin-ABC / Diffs / Diff Testing failed.

Snippet of first build failure:

[03:25:08] :	 [Step 1/1]  [0m [0;34mwallet_basic.py                         | ✓ Passed  | 26 s
[03:25:08] :	 [Step 1/1]  [0m [0;34mwallet_createwallet.py                  | ✓ Passed  | 1 s
[03:25:08] :	 [Step 1/1]  [0m [0;34mwallet_createwallet.py --usecli         | ✓ Passed  | 2 s
[03:25:08] :	 [Step 1/1]  [0m [0;34mwallet_disable.py                       | ✓ Passed  | 0 s
[03:25:08] :	 [Step 1/1]  [0m [0;34mwallet_dump.py                          | ✓ Passed  | 3 s
[03:25:08] :	 [Step 1/1]  [0m [0;34mwallet_encryption.py                    | ✓ Passed  | 4 s
[03:25:08] :	 [Step 1/1]  [0m [0;34mwallet_groups.py                        | ✓ Passed  | 7 s
[03:25:08] :	 [Step 1/1]  [0m [0;34mwallet_hd.py                            | ✓ Passed  | 3 s
[03:25:08] :	 [Step 1/1]  [0m [0;34mwallet_import_rescan.py                 | ✓ Passed  | 3 s
[03:25:08] :	 [Step 1/1]  [0m [0;34mwallet_importmulti.py                   | ✓ Passed  | 2 s
[03:25:08] :	 [Step 1/1]  [0m [0;34mwallet_importprunedfunds.py             | ✓ Passed  | 1 s
[03:25:08] :	 [Step 1/1]  [0m [0;34mwallet_keypool.py                       | ✓ Passed  | 3 s
[03:25:08] :	 [Step 1/1]  [0m [0;34mwallet_keypool_topup.py                 | ✓ Passed  | 2 s
[03:25:08] :	 [Step 1/1]  [0m [0;34mwallet_labels.py                        | ✓ Passed  | 2 s
[03:25:08] :	 [Step 1/1]  [0m [0;34mwallet_listreceivedby.py                | ✓ Passed  | 11 s
[03:25:08] :	 [Step 1/1]  [0m [0;34mwallet_listsinceblock.py                | ✓ Passed  | 3 s
[03:25:08] :	 [Step 1/1]  [0m [0;34mwallet_listtransactions.py              | ✓ Passed  | 8 s
[03:25:08] :	 [Step 1/1]  [0m [0;34mwallet_multiwallet.py                   | ✓ Passed  | 9 s
[03:25:08] :	 [Step 1/1]  [0m [0;34mwallet_multiwallet.py --usecli          | ✓ Passed  | 9 s
[03:25:08] :	 [Step 1/1]  [0m [0;34mwallet_resendwallettransactions.py      | ✓ Passed  | 1 s
[03:25:08] :	 [Step 1/1]  [0m [0;34mwallet_txn_clone.py                     | ✓ Passed  | 2 s
[03:25:08] :	 [Step 1/1]  [0m [0;34mwallet_txn_clone.py --mineblock         | ✓ Passed  | 3 s
[03:25:08] :	 [Step 1/1]  [0m [0;34mwallet_txn_doublespend.py               | ✓ Passed  | 3 s
[03:25:08] :	 [Step 1/1]  [0m [0;34mwallet_txn_doublespend.py --mineblock   | ✓ Passed  | 2 s
[03:25:08] :	 [Step 1/1]  [0m [0;34mwallet_zapwallettxes.py                 | ✓ Passed  | 2 s
[03:25:08] :	 [Step 1/1]  [0m [0;31mabc-finalize-block.py                   | ✖ Failed  | 0 s
[03:25:08] :	 [Step 1/1]  [0m [0;31mabc-invalid-chains.py                   | ✖ Failed  | 0 s
[03:25:08] :	 [Step 1/1]  [0m [0;31mfeature_bip68_sequence.py               | ✖ Failed  | 17 s
[03:25:08] :	 [Step 1/1]  [0m [0;31mrpc_invalidateblock.py                  | ✖ Failed  | 0 s
[03:25:08] :	 [Step 1/1]  [0m [0;31m [1m
[03:25:08] :	 [Step 1/1] ALL                                     | ✖ Failed  | 463 s (accumulated) 
[03:25:08] :	 [Step 1/1]  [0m [0mRuntime: 104 s
[03:25:08] :	 [Step 1/1] 
[03:25:08] :	 [Step 1/1] [216/352] cd /home/teamcity/buildAgent/work/c4a5708f2bae7929/contrib/devtools/chainparams && /usr/bin/python3 ./test_make_chainparams.py
[03:25:08] :	 [Step 1/1] .....
[03:25:08] :	 [Step 1/1] ----------------------------------------------------------------------
[03:25:08] :	 [Step 1/1] Ran 5 tests in 0.001s
[03:25:08] :	 [Step 1/1] 
[03:25:08] :	 [Step 1/1] OK
[03:25:08] :	 [Step 1/1] [219/352] Running bitcoin-seeder test suite
[03:25:08] :	 [Step 1/1] PASSED: bitcoin-seeder test suite
[03:25:08] :	 [Step 1/1] [336/352] Running bitcoin-qt test suite
[03:25:08] :	 [Step 1/1] PASSED: bitcoin-qt test suite
[03:25:08] :	 [Step 1/1] [346/352] Running bitcoin test suite
[03:25:08] :	 [Step 1/1] PASSED: bitcoin test suite
[03:25:08] :	 [Step 1/1] [349/352] Running secp256k1 test suite
[03:25:08] :	 [Step 1/1] PASSED: secp256k1 test suite
[03:25:08] :	 [Step 1/1] [351/352] Running leveldb test suite
[03:25:08] :	 [Step 1/1] PASSED: leveldb test suite
[03:25:08] :	 [Step 1/1] FAILED: test/CMakeFiles/check-functional 
[03:25:08] :	 [Step 1/1] cd /home/teamcity/buildAgent/work/c4a5708f2bae7929/build/test && /usr/bin/python3 ./functional/test_runner.py
[03:25:08] :	 [Step 1/1] ninja: build stopped: subcommand failed.
[03:25:08]W:	 [Step 1/1] ++ print_sanitizers_log
[03:25:08]W:	 [Step 1/1] ++ for log in "${SAN_LOG_DIR}"/*.log.*
[03:25:08]W:	 [Step 1/1] ++ echo '*** Output of /tmp/sanitizer_logs/*.log.* ***'
[03:25:08]W:	 [Step 1/1] ++ cat '/tmp/sanitizer_logs/*.log.*'
[03:25:08] :	 [Step 1/1] *** Output of /tmp/sanitizer_logs/*.log.* ***
[03:25:08]W:	 [Step 1/1] cat: '/tmp/sanitizer_logs/*.log.*': No such file or directory
[03:25:08]W:	 [Step 1/1] Process exited with code 1
[03:25:08]E:	 [Step 1/1] Process exited with code 1 (Step: Command Line)

Each failure log is accessible here:
bitcoin_abc_tests: abc-finalize-block.py
bitcoin_abc_tests: abc-invalid-chains.py
bitcoin_abc_tests: feature_bip68_sequence.py
bitcoin_abc_tests: rpc_invalidateblock.py

Harbormaster failed remote builds in B9046: Diff 15640!Jan 18 2020, 03:25
markblundeberg added a comment.Jan 18 2020, 03:26

OK, just a few failures remaining now, so it's better with that parent but something still wrong...

markblundeberg added a comment.Jan 18 2020, 03:43

The remaining inconsistencies are probably related to this fix:

https://github.com/bitcoin/bitcoin/pull/16849

markblundeberg added inline comments.Jan 18 2020, 04:33
src/validation.cpp
3052 ↗(On Diff #15640)

Essentially, every time we call DisconnectTip we ought to rescan the entire block index and find blocks that have enough chainwork and add them to setBlockIndexCandidates, before we unlock cs_main.

This unfortunately seems to be a very time consuming thing if we are rewinding many blocks, but on the other hand, hardly anyone ever calls invalidateblock / parkblock RPCs so it's more important to be correct than to be broken and efficient.

markblundeberg planned changes to this revision.Jan 19 2020, 02:15

Revision Contents
Changeset List

PathSize
src/
6 lines

Diff 15639

View Options

src/validation.cpp

Loading...