Page MenuHomePhabricator
Differential D14687

validation: add ChainMan logic for completing UTXO snapshot validation
ClosedPublic
Actions

Authored by PiRK on Oct 25 2023, 06:27.

Details

Reviewers
Fabien
Group Reviewers
Restricted Project
Commits
rABC587d2b747ffa: validation: add ChainMan logic for completing UTXO snapshot validation
Summary

add ChainstateManager.GetSnapshot{BaseHeight,BaseBlock}()

For use in later commits.

https://github.com/bitcoin/bitcoin/pull/25740/commits/5ee22cdafd2562bcb8bf0ae6025e4b53c826382d

add Chainstate::HasCoinsViews()

Used in subsequent commits. Also cleans up asserts in
coins_views-related convenience methods to be more exact.

https://github.com/bitcoin/bitcoin/pull/25740/commits/637a90b973f60555ea4fef4b845ffa7533dcb866

validation: add ChainMan logic for completing UTXO snapshot validation

Trigger completion when a background validation chainstate reaches the
same height as a UTXO snapshot, and handle cleaning up the chainstate
on subsequent startup.

https://github.com/bitcoin/bitcoin/pull/25740/commits/d96c59cc5cd2f73f1f55c133c52208671fe75ef3

log: add LoadBlockIndex() message for assumedvalid blocks

I found this useful during unittest debugging.

https://github.com/bitcoin/bitcoin/pull/25740/commits/7300ced9de22e6d1bff816e6538d3370cebe7501

refactor: make MempoolMutex() public

for use in the following unittests.

https://github.com/bitcoin/bitcoin/pull/25740/commits/d70919a88fc90a2662f9a844deb085d03ee7b5d8

test: add snapshot completion unittests

Also adjusts the previous snapshot chainstate init tests
to account for the fact that the init process is now attempting to
validate and complete background chainstates whose tip is at the
snapshot base block. We use a DisconnectTip() hack to preserve the
nature of the test.

https://github.com/bitcoin/bitcoin/pull/25740/commits/87a1108c81fe0cb15c3860e3a67dc1f43ffec705

This completes backport of core#25740

Depends on D14686

Test Plan

ninja all check-all

Diff Detail

Event Timeline

PiRK created this revision.Oct 25 2023, 06:27
Herald added a reviewer: Restricted Project. · View Herald TranscriptOct 25 2023, 06:27
PiRK requested review of this revision.Oct 25 2023, 06:27
teamcity edited the summary of this revision. (Show Details)Oct 25 2023, 06:27
Harbormaster completed remote builds in B25430: Diff 42770.Oct 25 2023, 06:33
teamcity added a comment.Oct 25 2023, 06:33
Build Bitcoin ABC Diffs / Diff Testing (build-clang-tidy) failed.

Tail of the build log:

[360/541] Building C object src/secp256k1/CMakeFiles/sign-bench.dir/src/bench_sign.c.o
[361/541] Building C object src/secp256k1/CMakeFiles/ecmult-bench.dir/src/bench_ecmult.c.o
[362/541] Linking C executable src/secp256k1/sign-bench
[363/541] Linking C executable src/secp256k1/ecmult-bench
[364/541] Building CXX object src/CMakeFiles/server.dir/rpc/mining.cpp.o
[365/541] Building C object src/secp256k1/CMakeFiles/internal-bench.dir/src/bench_internal.c.o
[366/541] Linking C executable src/secp256k1/internal-bench
[367/541] Building CXX object src/test/CMakeFiles/testutil.dir/util/blockindex.cpp.o
[368/541] Building CXX object src/CMakeFiles/server.dir/rpc/net.cpp.o
[369/541] Building CXX object src/test/CMakeFiles/testutil.dir/util/logging.cpp.o
[370/541] Building CXX object src/CMakeFiles/bitcoin-tx.dir/bitcoin-tx.cpp.o
[371/541] Linking CXX executable src/bitcoin-tx
[372/541] Building CXX object src/test/CMakeFiles/testutil.dir/util/str.cpp.o
[373/541] Building CXX object src/CMakeFiles/server.dir/rpc/avalanche.cpp.o
[374/541] Building CXX object src/CMakeFiles/server.dir/txorphanage.cpp.o
[375/541] Building CXX object src/test/CMakeFiles/testutil.dir/util/transaction_utils.cpp.o
[376/541] Building CXX object src/CMakeFiles/server.dir/torcontrol.cpp.o
[377/541] Building CXX object src/wallet/CMakeFiles/wallet.dir/bdb.cpp.o
[378/541] Building CXX object src/CMakeFiles/server.dir/txmempool.cpp.o
[379/541] Building CXX object src/wallet/CMakeFiles/wallet.dir/context.cpp.o
[380/541] Building CXX object src/CMakeFiles/server.dir/net_processing.cpp.o
[381/541] Building CXX object src/wallet/CMakeFiles/wallet.dir/coincontrol.cpp.o
[382/541] Building CXX object src/wallet/CMakeFiles/wallet.dir/coinselection.cpp.o
[383/541] Building CXX object src/wallet/CMakeFiles/wallet.dir/crypter.cpp.o
[384/541] Building CXX object src/wallet/CMakeFiles/wallet.dir/db.cpp.o
[385/541] Building CXX object src/test/CMakeFiles/testutil.dir/util/blockfilter.cpp.o
[386/541] Building CXX object src/test/CMakeFiles/testutil.dir/util/wallet.cpp.o
[387/541] Building CXX object src/CMakeFiles/server.dir/rpc/rawtransaction.cpp.o
[388/541] Building CXX object src/wallet/CMakeFiles/wallet-tool.dir/wallettool.cpp.o
[389/541] Building CXX object src/test/CMakeFiles/testutil.dir/util/validation.cpp.o
[390/541] Building CXX object src/test/CMakeFiles/testutil.dir/util/mining.cpp.o
[391/541] Building CXX object src/CMakeFiles/server.dir/rpc/blockchain.cpp.o
[392/541] Building CXX object src/test/CMakeFiles/testutil.dir/util/net.cpp.o
[393/541] Building CXX object src/CMakeFiles/server.dir/wallet/init.cpp.o
[394/541] Building CXX object src/wallet/CMakeFiles/wallet.dir/fees.cpp.o
[395/541] Building CXX object src/wallet/CMakeFiles/wallet.dir/load.cpp.o
[396/541] Building CXX object src/wallet/CMakeFiles/wallet.dir/interfaces.cpp.o
[397/541] Building CXX object src/wallet/CMakeFiles/wallet.dir/receive.cpp.o
[398/541] Building CXX object src/wallet/CMakeFiles/wallet.dir/sqlite.cpp.o
[399/541] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpc/signmessage.cpp.o
[400/541] Building CXX object src/wallet/CMakeFiles/wallet.dir/transaction.cpp.o
[401/541] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpc/encrypt.cpp.o
[402/541] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/dns.cpp.o
[403/541] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpc/backup.cpp.o
[404/541] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpc/util.cpp.o
[405/541] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpcwallet.cpp.o
[406/541] Building CXX object src/wallet/CMakeFiles/wallet.dir/salvage.cpp.o
[407/541] Building CXX object src/wallet/CMakeFiles/wallet.dir/walletutil.cpp.o
[408/541] Building CXX object src/wallet/CMakeFiles/wallet.dir/scriptpubkeyman.cpp.o
[409/541] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/options.cpp.o
[410/541] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/db.cpp.o
[411/541] Building CXX object src/test/CMakeFiles/testutil.dir/util/setup_common.cpp.o
[412/541] Building CXX object src/wallet/CMakeFiles/wallet.dir/spend.cpp.o
[413/541] Building CXX object src/seeder/CMakeFiles/bitcoin-seeder.dir/main.cpp.o
[414/541] Building CXX object src/wallet/CMakeFiles/wallet.dir/walletdb.cpp.o
[415/541] Building CXX object src/wallet/CMakeFiles/wallet.dir/wallet.cpp.o
[416/541] Linking CXX static library src/wallet/libwallet.a
[417/541] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/bitcoin.cpp.o
ninja: build stopped: cannot make progress due to previous errors.
Build build-clang-tidy failed with exit code 1
PiRK updated this revision to Diff 42771.Oct 25 2023, 07:55

use braces

Harbormaster completed remote builds in B25431: Diff 42771.Oct 25 2023, 08:15
Fabien requested changes to this revision.Oct 25 2023, 10:10
Fabien added a subscriber: Fabien.
Fabien added inline comments.
src/node/chainstate.cpp
236 ↗(On Diff #42771)

Nit: use a scope above to avoid this

src/validation.cpp
6350 ↗(On Diff #42771)

? Also duplicated from the doxygen string

6547 ↗(On Diff #42771)

The /* continued */ are useless and should be removed. This is not the only occurrence (but I think it's the worst one) so please grep them all

6575 ↗(On Diff #42771)

As per this comment: https://github.com/bitcoin/bitcoin/pull/25740/commits/d96c59cc5cd2f73f1f55c133c52208671fe75ef3#r1134114096
The func will end up duplicated. Can you check if this has been fixed and then do it as a follow up, or remove them ?

This revision now requires changes to proceed.Oct 25 2023, 10:10
PiRK updated this revision to Diff 42787.EditedOct 25 2023, 11:57

use a scope to avoid shadowing [init_status, init_error], remove the "continued" comments I missed, fix func duplication in logging, remove pointless and misplaced XXX comment

PiRK added inline comments.Oct 25 2023, 11:59
src/validation.cpp
6575 ↗(On Diff #42771)

It is still broken for Bitcoin Core

Harbormaster completed remote builds in B25436: Diff 42787.Oct 25 2023, 12:13
Fabien accepted this revision.Oct 26 2023, 09:05
This revision is now accepted and ready to land.Oct 26 2023, 09:05
Closed by commit rABC587d2b747ffa: validation: add ChainMan logic for completing UTXO snapshot validation (authored by James O'Beirne <james.obeirne@pm.me>, committed by PiRK). · Explain WhyOct 26 2023, 11:39
This revision was automatically updated to reflect the committed changes.
PiRK added a commit: rABC587d2b747ffa: validation: add ChainMan logic for completing UTXO snapshot validation.

Revision Contents
Changeset List

PathSize
src/
node/
57 lines
test/
183 lines
89 lines
351 lines

Diff 42787

View Options

src/node/chainstate.cpp

Loading...
View Options

src/test/validation_chainstatemanager_tests.cpp

Loading...
View Options

src/validation.h

Loading...
View Options

src/validation.cpp

Loading...