-
Notifications
You must be signed in to change notification settings - Fork 37.7k
test: use local CBlockIndex
in block read hash mismatch check
#33154
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
test: use local CBlockIndex
in block read hash mismatch check
#33154
Conversation
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code Coverage & BenchmarksFor details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33154. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
1f7821b
to
73e0f07
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approach ACK
…id data race Co-authored-by: stickies-v <stickies-v@protonmail.com>
73e0f07
to
cb173b8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK cb173b8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm ACK cb173b8
fake_index->phashBlock = &uint256::ONE; // invalid block hash | ||
CBlockIndex index; | ||
{ | ||
LOCK(cs_main); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Could use the m_node.chainman->GetMutex()
alias for new code.
…88ae28a bce88ae28a kernel: Fix bitcoin-chainstate for windows 3a7e9f0eaf kernel: Add Purpose section to header documentation 5bae79ace5 kernel: Allowing reducing exports d0308a2489 kernel: Add pure kernel bitcoin-chainstate 05a569070c kernel: Add functions to get the block hash from a block 8566ec6e83 kernel: Add block index utility functions to C header b4d0e80f84 kernel: Add function to read block undo data from disk to C header 488999ac77 kernel: Add functions to read block from disk to C header 3dc76bb7f7 kernel: Add function for copying block data to C header 6151b45a42 kernel: Add functions for the block validation state to C header 5d00432f27 kernel: Add validation interface to C header facf209aee kernel: Add interrupt function to C header 129f553e4e kernel: Add import blocks function to C header f7ed7b944d kernel: Add chainstate load options for in-memory dbs in C header 67d9f53a98 kernel: Add options for reindexing in C header ebc826319f kernel: Add block validation to C header 511a1c8a78 kernel: Add chainstate loading when instantiating a ChainstateManager aad295899e kernel: Add chainstate manager option for setting worker threads c701cb2405 kernel: Add chainstate manager object to C header 1df8b87602 kernel: Add notifications context option to C header 571c1a2acb kernel: Add chain params context option to C header a2cab9f1cd kernel: Add kernel library context object 944ef6b630 kernel: Add logging to kernel library C header d0cb841fba kernel: Introduce initial kernel C header API 04c115dfde Merge bitcoin/bitcoin#33078: kernel: improve BlockChecked ownership semantics bc797d2271 Merge bitcoin/bitcoin#33154: test: use local `CBlockIndex` in block read hash mismatch check d3c58a5be9 Merge bitcoin/bitcoin#33193: Release: Prepare "Translation string freeze" step 9cf7b3d90c Merge bitcoin/bitcoin#33211: test: modify logging_filesize_rate_limit params f5f853d952 Merge bitcoin/bitcoin#32878: index: fix wrong assert of current_tip == m_best_block_index 5dda364c4b test: modify logging_filesize_rate_limit params 0df2c3c42e qt: Update `src/qt/locale/bitcoin_en.xlf` translation source file 22e689587a Merge bitcoin/bitcoin#33209: cmake: Drop python dependency for translate be356fc49b Merge bitcoin/bitcoin#32896: wallet, rpc: add v3 transaction creation and wallet support 3c4a109aa8 cmake: Drop python dependency for translate f58de8749e Merge bitcoin/bitcoin#32345: ipc: Handle unclean shutdowns better d31dc8f818 Merge bitcoin/bitcoin#33200: cmake: Introduce translate.cmake script for translate target 05255d5d1e cmake: Drop dependency on sed for translate target d5054beca5 cmake: Introduce translate.cmake script for translate target 57e8f34fe2 Merge bitcoin/bitcoin#32977: wallet: Remove wallet version and several legacy related functions 97593c1fd3 Merge bitcoin/bitcoin#32975: assumevalid: log every script validation state change 5c8bf7b39e doc: add release notes for version 3 transactions 4ef8065a5e test: add truc wallet tests 5d932e14db test: extract `bulk_vout` from `bulk_tx` so it can be used by wallet tests 2cb473d9f2 rpc: Support version 3 transaction creation 4c20343b4d rpc: Add transaction min standard version parameter c5a2d08011 wallet: don't return utxos from multiple truc txs in AvailableCoins da8748ad62 wallet: limit v3 tx weight in coin selection 85c5410615 wallet: mark unconfirmed v3 siblings as mempool conflicts 0804fc3cb1 wallet: throw error at conflicting tx versions in pre-selected inputs cc155226fe wallet: set m_version in coin control to default value 2e9617664e wallet: don't include unconfirmed v3 txs with children in available coins ec2676becd wallet: unconfirmed ancestors and descendants are always truc 7b4a1350df Merge bitcoin/bitcoin#33183: validation: rename block script verification error from "mandatory" to "block" c99f5c5e1b Merge bitcoin/bitcoin#33106: policy: lower the default blockmintxfee, incrementalrelayfee, minrelaytxfee 578b512bdd Merge bitcoin/bitcoin#33011: log: rate limiting followups 8405fdb06e Merge bitcoin/bitcoin#33169: interfaces, chain, refactor: Remove unused getTipLocator and incaccurate getActiveChainLocator c0d91fc69c Add release note for #33050 and #33183 error string changes e17b5da0d6 Merge bitcoin/bitcoin#33179: doc: update wallet build instruction 9b1a7c3e8d Merge bitcoin/bitcoin#33116: refactor: Convert uint256 to Txid b3f781a0ef contrib: adapt max reject string size in tracing demo 9a04635432 scripted-diff: validation: rename mandatory errors into block errors dbf8b0980b Merge bitcoin/bitcoin#33171: ci: Update `actions/checkout` version d6887f0cec Merge bitcoin/bitcoin#33178: guix: increase maximum allowed (runtime) GCC to 7 2b00030af8 interfaces, chain, refactor: Remove inaccurate getActiveChainLocator 110a0f405c interfaces, chain, refactor: Remove unused getTipLocator dadf15f88c Merge bitcoin/bitcoin#33050: net, validation: don't punish peers for consensus-invalid txs cb173b8e93 test: use local `CBlockIndex` in block read hash mismatch test to avoid data race 73972d5617 Merge bitcoin/bitcoin#31296: wallet: Translate [default wallet] string in progress messages 67e186deb0 doc: update wallet build instruction 5c74a0b397 config: add DEBUG_ONLY -logratelimit 9f3b017bcc test: logging_filesize_rate_limit improvements 350193e5e2 test: don't leak log category mask across tests 05d7c22479 test: add ReadDebugLogLines helper function 3d630c2544 log: make m_limiter a shared_ptr ec484bd5ce Merge bitcoin/bitcoin#31453: util: detect and warn when using exFAT on MacOS 776a163374 guix: increase maximum allowed (runtime) GCC to 7 ba84a25dee [doc] update mempool-replacements.md for incremental relay feerate change 273e600e65 Merge bitcoin/bitcoin#33021: test/refactor: revive test verifying that `GetCoinsCacheSizeState` switches from OK→LARGE→CRITICAL 18720bc5d5 [doc] release note for min feerate changes 6da5de58ca [policy] lower default minrelaytxfee and incrementalrelayfee to 100sat/kvB 2e515d2897 [prep/test] make wallet_fundrawtransaction's minrelaytxfee assumption explicit 457cfb61b5 [prep/util] help MockMempoolMinFee handle more precise feerates 3eab8b7240 [prep/test] replace magic number 1000 with respective feerate vars 5f2df0ef78 [miner] lower default -blockmintxfee to 1sat/kvB d6213d6aa1 [doc] assert that default min relay feerate and incremental are the same 1fbee5d7b6 [test] explicitly check default -minrelaytxfee and -incrementalrelayfee 72dc18467d [test] RBF rule 4 for various incrementalrelayfee settings 85f498893f [test] check bypass of minrelay for various minrelaytxfee settings e5f896bb1f [test] check miner doesn't select 0fee transactions de0675f9de refactor: Move `transaction_identifier.h` to primitives 6f068f65de Remove implicit uint256 conversion and comparison 9c24cda72e refactor: Convert remaining instances from uint256 to Txid d2ecd6815d policy, refactor: Convert uint256 to Txid f6c0d1d231 mempool, refactor: Convert uint256 to Txid aeb0f78330 refactor: Convert `mini_miner` from uint256 to Txid 326f244724 refactor: Convert RPCs and `merkleblock` from uint256 to Txid 41642d43b3 Merge bitcoin/bitcoin#33162: test: fix scripts in `blockfilter_basic_test` f83c01d882 ci: Update `actions/checkout` version a27430e259 Merge bitcoin/bitcoin#32473: Introduce per-txin sighash midstate cache for legacy/p2sh/segwitv0 scripts 34b366fa2c Merge bitcoin/bitcoin#33155: contrib: drop `bitcoin-util` exception from FORTIFY check ca64b71ed5 test: fix scripts in `blockfilter_basic_test` fab2980bdc assumevalid: log every script validation state change e8f9c37a3b log: clean up LogPrintStr_ and Reset, prefix all logs with "[*]" when there are suppressions 3c7cae49b6 log: change LogLimitStats to struct LogRateLimiter::Stats 876dbdfb47 tests: drop expect_disconnect behaviour for tx relay b29ae9efdf validation: only check input scripts once 266dd0e10d net_processing: drop MaybePunishNodeForTx db3228042b util: detect and warn when using exFAT on macOS 4bff4ce561 contrib: drop bitcoin-util exception from FORTIFY check 83950275ed qa: unit test sighash caching b221aa80a0 qa: simple differential fuzzing for sighash with/without caching 92af9f74d7 script: (optimization) introduce sighash midstate caching 8f3ddb0bcc script: (refactor) prepare for introducing sighash midstate cache 9014d4016a tests: add sighash caching tests to feature_taproot 49b3d3a92a Clean up `FindTxForGetData` 2581258ec2 ipc: Handle bitcoin-wallet disconnections 2160995916 ipc: Add Ctrl-C handler for spawned subprocesses 0c28068ceb doc: Improve IPC interface comments 7f65aac78b ipc: Avoid waiting for clients to disconnect when shutting down 6eb09fd614 test: Add unit test coverage for Init and Shutdown code 9a9fb19536 ipc: Use EventLoopRef instead of addClient/removeClient 5c45bc989b Merge commit 'e886c65b6b37aaaf5d22ca68bc14e55d8ec78212' into pr/ipc-stop-base e886c65b6b Squashed 'src/ipc/libmultiprocess/' changes from 27c7e8e5a581..b4120d34bad2 3aef38f44b test: exercise index reorg assertion failure acf50233cd index: fix wrong assert of current_tip == m_best_block_index 1d9f1cb4bd kernel: improve BlockChecked ownership semantics 554befd873 test: revive `getcoinscachesizestate` 64ed0fa6b7 refactor: modernize `LargeCoinsCacheThreshold` 1b40dc02a6 refactor: extract `LargeCoinsCacheThreshold` from `GetCoinsCacheSizeState` 8319a13468 log: clarify RATELIMIT_MAX_BYTES comment, use RATELIMIT_WINDOW 5f70bc80df log: remove const qualifier from arguments in LogPrintFormatInternal b8e92fb3d4 log: avoid double hashing in SourceLocationHasher 616bc22f13 test: remove noexcept(false) comment in ~DebugLogHelper 9ba1fff29e kernel: refactor: ConnectTip to pass block pointer by value 60d1042b9a wallet: Remove unused `WalletFeature` enums 66de58208a wallet: Remove `CWallet::nWalletVersion` and related functions 7cda3d0f5b wallet: Remove `IsFeatureSupported()` and `CanSupportFeature()` ba01585229 wallet: `MigrateToDescriptor` no longer calls `CanSupportFeature` 63acee2797 wallet: Remove `GetClosestWalletFeature()` e27da3150b wallet: Remove `GetVersion()` db225cea56 wallet, refactor: Replace GetDisplayName() with LogName() 01737883b3 wallet: Translate [default wallet] string in progress messages REVERT: 35fced5df7 kernel: Fix bitcoin-chainstate for windows REVERT: c164264fd8 kernel: Add Purpose section to header documentation REVERT: 9096c35c05 kernel: Allowing reducing exports REVERT: 6d0d4b2507 kernel: Add pure kernel bitcoin-chainstate REVERT: ccd85f0333 kernel: Add functions to get the block hash from a block REVERT: 925f21c37b kernel: Add block index utility functions to C header REVERT: b7441841c9 kernel: Add function to read block undo data from disk to C header REVERT: bc0e6f098e kernel: Add functions to read block from disk to C header REVERT: 5120302f96 kernel: Add function for copying block data to C header REVERT: 718ccee732 kernel: Add functions for the block validation state to C header REVERT: eb363ab30e kernel: Add validation interface to C header REVERT: 246886c6ea kernel: Add interrupt function to C header REVERT: f3b34ca457 kernel: Add import blocks function to C header REVERT: fe08857d52 kernel: Add chainstate load options for in-memory dbs in C header REVERT: f93f171e01 kernel: Add options for reindexing in C header REVERT: dca7b4c26e kernel: Add block validation to C header REVERT: f031e9ce03 kernel: Add chainstate loading when instantiating a ChainstateManager REVERT: 3cb99f73ec kernel: Add chainstate manager option for setting worker threads REVERT: 9454ad8512 kernel: Add chainstate manager object to C header REVERT: 3bead9ebdd kernel: Add notifications context option to C header REVERT: dda805dfb6 kernel: Add chain params context option to C header REVERT: ea5334925d kernel: Add kernel library context object REVERT: 36fafbaef9 kernel: Add logging to kernel library C header REVERT: d28eef5cf2 kernel: Introduce initial kernel C header API git-subtree-dir: libbitcoinkernel-sys/bitcoin git-subtree-split: bce88ae28ab2cd12f32aead1fbf47153c50c3b05
Avoid mutating the shared active tip
CBlockIndex
in theblockmanager_readblock_hash_mismatch
test.Instead, construct a local
CBlockIndex
with only the required fields set, ensuring the test remains self-contained and hopefully eliminating the data race reported in #33150.