Repository navigation
backport: v0.26 bitcoin#25193, bitcoin#27720 - #72
DCG-Claude wants to merge 2 commits into
Conversation
…t, allow interaction with reindex-chainstate 97844d9 index: Enable reindex-chainstate with active indexes (Martin Zumsande) 60bec3c index: Use first block from locator instead of looking for fork point (Martin Zumsande) Pull request description: This makes two improvements to the index init phase: **1) Prevent index corruption in case a reorg happens when the index was switched off**: This is done by reading in the top block stored in the locator instead of looking for a fork point already in `BaseIndex::Init()`. Before, we'd just go back to the fork point by calling `FindForkInGlobalIndex()`, which would have corrupted the coinstatsindex because its saved muhash needs to be reverted step by step by un-applying all blocks in between, which wasn't done before. This is now being done a bit later in `ThreadSync()`, which has existing logic to call the custom `Rewind()` method when going back along the chain to the forking point (thanks ryanofsky for pointing this out to me!). **2) Allow using the `-reindex-chainstate` option without needing to disabling indexes**: With `BaseIndex::Init()` not calling `FindForkInGlobalIndex()` anymore, we can allow `reindex-chainstate` with active indexes. `reindex-chainstate` deletes the chain and rebuilds it later in `ThreadImport`, so there is no chain available during `BaseIndex::Init()`, which would lead to problems (see bitcoin#24789). But now we'll only need the chain a bit later in `BaseIndex::ThreadSync`, which will wait for the reindex-chainstate in `ThreadImport` to finish and will continue syncing after that. ACKs for top commit: ryanofsky: Code review ACK 97844d9. Just simple rebase since last review Tree-SHA512: e24973fc22e0b87a49026f4820aecb0a4e415f4d381bade9969dd31cf97afecfea0449dce7fcc797343b792199cc8287276d1f5ffa4433dcb54fb24a808db6fb Dash adaptations: - src/index/base.cpp: dropped upstream's `SetSyscallSandboxPolicy(SyscallSandboxPolicy::TX_INDEX);` line that shares the ThreadSync hunk — Dash has no syscall sandbox (zero occurrences in src/); only the g_indexes_ready_to_sync wait loop was taken - src/index/base.cpp: kept Dash's existing `using node::PruneLockInfo/ReadBlockFromDisk/fPruneMode` and appended `using node::g_indexes_ready_to_sync` - src/init.cpp: the -reindex-chainstate help-text change was applied to Dash's copy of the arg, which lives in OptionsCategory::INDEXING (Dash moved -reindex/-reindex-chainstate out of OPTIONS), not in the OPTIONS block where upstream edits it - src/init.cpp: kept Dash's `-prune` help text (`>%u`), which only appeared in the conflict as context — the `>=%u` wording is unrelated to this PR - src/init.cpp: `using node::g_indexes_ready_to_sync;` placed in Dash's lowercase using-group after `using node::fReindex;` (Dash already had fReindex imported) - src/init.cpp: removed only the three index-vs-reindex-chainstate InitErrors; Dash's own `Prune mode is incompatible with -reindex-chainstate` check (~line 1247) is Dash-specific and untouched, and the surrounding LLMQ qvvec try/catch block was preserved - src/node/blockstorage.{h,cpp}: new declaration/definition inserted after fReindex, ahead of Dash's fPruneMode/nPruneTarget (Dash still keeps those in node namespace; upstream had already moved them) - src/test/util/setup_common.cpp: `node::g_indexes_ready_to_sync = true;` placed after the noui block, before Dash's `bls::bls_legacy_scheme.store(true);` - test/functional/p2p_blockfilters.py: removed the -reindex-chainstate init-error case while keeping Dash's `self.test_special_transactions_in_filters()` call and the AssetLockTx helpers
…r setting 'synced' flag 3126454 index: prevent race by calling 'CustomInit' prior setting 'synced' flag (furszy) Pull request description: Decoupled from bitcoin#27607. Fixed a potential race condition in master (not possible so far) that could become an actual issue soon. Where the index's `CustomAppend` method could be called (from `BlockConnected`) before its `CustomInit` method, causing the index to try to update itself before it is initialized. This could happen because we set the index `m_synced` flag (which enables `BlockConnected` events) before calling to the child class init function (`CustomInit`). So, for example, the block filter index could process a block before initialize the next filter position field and end up overwriting the first stored filter. This race was introduced in bitcoin@bef4e40 from bitcoin#25494. ACKs for top commit: achow101: ACK 3126454 mzumsande: Code review ACK 3126454 TheCharlatan: Nice, ACK 3126454 Tree-SHA512: 7a53fed1d2035cb4c1f331d6ee9f92d499b6cbb618ea534c6440f5a45ff9b3ac4dcff5fb4b88937f45a0be249e3a9c6dc6c3ac77180f12ae25fc56856ba39735
The pasted log contains no failing test, assertion or compiler diagnostic — only actions/checkout git-config lines and container/network teardown ('Cleaning up orphan processes'), which is the signature of a runner/infra abort rather than a source failure, and it hit all five variants identically at the same moment. I re-checked both commits against upstream 60bec3c/97844d92 (bitcoin#25193) and 3126454 (bitcoin#27720): src/index/base.cpp, init.cpp, node/blockstorage.{h,cpp}, test/util/setup_common.cpp and the two functional tests match upstream apart from the documented Dash adaptations, and g_indexes_ready_to_sync is both set in ThreadImport and gated on fReindexChainState in AppInitMain. Please rerun fork CI; if it reproduces, post the tail of the failing step's log so the actual error is visible. 🤖 backportsys, on behalf of the Dash backport pipeline. |
The attached log contains no compiler error, no unit/functional test failure and no assertion — only the post-job container removal and 'Cleaning up orphan processes' lines, which is the signature of a runner/infrastructure abort rather than a source defect, and all five jobs died identically at the same teardown point. I re-checked the branch's actual changes: g_indexes_ready_to_sync is declared in src/node/blockstorage.h, defined and set in src/node/blockstorage.cpp, imported in src/index/base.cpp, latched true in BasicTestingSetup, and CustomInit exists on BaseIndex plus both derived indexes, so there is nothing here that would break a build or test uniformly. Please rerun fork CI. 🤖 backportsys, on behalf of the Dash backport pipeline. |
The attached log contains no build error, test assertion or stack trace — only container teardown and orphan-process cleanup — and all five test jobs aborted at the same step simultaneously, which is the signature of a runner/infra abort rather than a source failure. I re-checked the branch: g_indexes_ready_to_sync is declared, defined, set at the end of ThreadImport, set in BasicTestingSetup and gated on fReindexChainState in AppInitMain exactly as upstream does, and no remaining functional test expects the three removed -reindex-chainstate init errors. Please rerun the fork CI. 🤖 backportsys, on behalf of the Dash backport pipeline. |
|
This pull request has conflicts, please rebase. |
1 participant
Automated Bitcoin Core v0.26 backports, batch
backport-0.26-b029-src-node.92d2e996491442053ba2Provenance
Each commit passed: cherry-pick (adapted by an Opus lane only where conflicts existed), build, touched tests, a mechanical diff-of-diffs check (every upstream hunk landed; no added line without an upstream counterpart), and an independent Opus verification lane where anything was adapted. Gate rows and lane artifacts are in the backportsys DB.