3109a1f948f3c8fd500defbdc4e59bfb2953c30b refactor: Avoid locking cs_main in ProcessNewBlockHeaders (João Barbosa)
Pull request description:
Builds on #16774, this change avoids locking `cs_main` in `ProcessNewBlockHeaders` when the tip has changed - in this case the removed lock was necessary to just log a message.
Top commit has no ACKs.
Tree-SHA512: 31be6d319fa122804f72fa813cec5ed041dd7e4aef3c1921124a1f03016925c43cd4d9a272d80093e77fa7600e3506ef47b7bb821afcbffe01e6be9bceb6dc00
dcc448e3d2b97f7e4e23f5ed174762cec3530306 Avoid unnecessary "Synchronizing blockheaders" log messages (Jonas Schnelli)
Pull request description:
Fixes#16773
I'm not entirely sure why 16773 happend, but probably due to headers fallback in a compact block.
However, this PR should fix it and should have been included in #15615.
ACKs for top commit:
ajtowns:
ACK dcc448e3d2b97f7e4e23f5ed174762cec3530306 ; code review only, haven't compiled or tested.
promag:
ACK dcc448e3d2b97f7e4e23f5ed174762cec3530306.
TheBlueMatt:
utACK dcc448e3d2b97f7e4e23f5ed174762cec3530306. Went and read how pindexBestHeader is handled and this code looks correct (worst case it breaks a LogPrint, so whatever). I also ran into this on #16762.
fanquake:
ACK dcc448e3d2b97f7e4e23f5ed174762cec3530306
Tree-SHA512: f8cac3b6eb9d4e8fab53a535b55f9ea9b058e3ab6ade64801ebc56439ede4f54b5fee36d5d2b316966ab987b65b13ab9dc18849f345d08b81ecdf2722a3f5f5a
d75e704ac08fb43320e3ebcdde0d2a5392fdec9f Add log output during initial header sync (Jonas Schnelli)
Pull request description:
The non debug log output is completely quiet during the header sync. I see two main reasons to add infos about the state of the initial header sync...
* users may think the node did fail to start sync
* it's a little complicate to check if your getting throttled during header sync (repeatedly calling `getchaintips` or similar)
ACKs for top commit:
fanquake:
Concept ACK d75e704ac0
practicalswift:
utACK d75e704ac08fb43320e3ebcdde0d2a5392fdec9f
laanwj:
Tested ACK d75e704ac08fb43320e3ebcdde0d2a5392fdec9f
Tree-SHA512: 2e738571b703d7251290864603c3a829729645962c2fa3187250bab0585e66a5f01fce892e9b5b98da451fab2b40a2e4784f9b2e5a9cad75ff62c535affe7430
059e8ccc1eba6cd92f4c434325cb56b0533eb744 Change BOOST_CHECK to BOOST_CHECK_EQUAL to see mismatched values when a check fails. (Kiminuo)
Pull request description:
This is useful to see mismatched values when a check fails as specified in the [Boost documentation](https://www.boost.org/doc/libs/1_71_0/libs/test/doc/html/boost_test/utf_reference/testing_tool_ref/assertion_boost_level.html).
This PR would make #20744 PR's diff smaller by a bit.
ACKs for top commit:
MarcoFalke:
review ACK 059e8ccc1eba6cd92f4c434325cb56b0533eb744
theStack:
Code Review ACK 059e8ccc1eba6cd92f4c434325cb56b0533eb744
Tree-SHA512: 82359ef38e0d1926f12a34aeff6fde6d1d307c703a080547749b908f873c2a2f894f6f094c33470b32987c229e3a1f17f7d1e877663c53293c023bde0e7272c1
789e9dd3aa727176797529c35b2848f994630a82 validation: use std::chrono in IsCurrentForFeeEstimation() (fanquake)
47be28c8bc475eafeebd4fc58ea92f0d3df0d8c6 validation: use std::chrono in CChainState::FlushStateToDisk() (fanquake)
Pull request description:
Probably up for debate as to which type is used for the constants. Personally, swapping these to hours is more readable.
ACKs for top commit:
MarcoFalke:
ACK 789e9dd3aa727176797529c35b2848f994630a82
jonatack:
ACK 789e9dd3aa727176797529c35b2848f994630a82
Tree-SHA512: f4a25cbd00a49a54b7783a1f588be83706dd2a475cecb5c2e8b97b2d4b27c0955a7454d7486f2454e96351c44f233b300c4f4b9ca62fc7336277f10da34dd5c3
b05ec410f2d9f209796a5df31860e23efd729dfe Add unit testing for the CompressScript functions (marcaiaf)
Pull request description:
Salvaging #15104 which adds unit tests for CompressScript function in `compressor.cpp`
Tested following cases for the CScript:
- CKeyID
- CScriptID
- Uncompressed CPubKey (of size: 65)
- Compressed CPubKey (of size: 32)
ACKs for top commit:
theStack:
ACK b05ec410f2
Tree-SHA512: 7e23ace39383122802dfe5f7d38190d772f5db4045a67b7a9bd4c06797a17e0cdc41d6fac92d448057eb7df50172155dc824587c16c68c79fd1a4de37b772001
4be3b7680e6324294d9241232a6f1eae36c85a9e refactor: Cleanup walletinitinterface.h (Hennadii Stepanov)
Pull request description:
Forward declarations of `CScheduler` and `CRPCTable` classes are no longer needed after ea961c3d7256c66146b4976ab1293db4a628c0de (#14437) commit.
Including `<string>` is no longer needed after 4d4185a4f0e40c033a587871839a47cb3f89ee93 (#13190) commit.
ACKs for top commit:
theStack:
ACK 4be3b76
promag:
ACK 4be3b7680e6324294d9241232a6f1eae36c85a9e.
kristapsk:
ACK 4be3b7680e6324294d9241232a6f1eae36c85a9e (tested that it builds)
Tree-SHA512: 5ed72e3deda3d7c7fb698a1a11db76199727e6c570dfc78422690dbda9a92af32e1913920062dd3c9f618095e7498c219ff9c145a4c151486865ebeaa20a1d3c
7cd069d8ef900c6c6b904ddd6fbd64e14bd0f53e Add test for AddTimeData (Martin Zumsande)
Pull request description:
`AddTimeData()` has poor test coverage but interesting logic (including a bug turned into a feature). This PR adds a unit test for it.
ACKs for top commit:
laanwj:
ACK 7cd069d8ef900c6c6b904ddd6fbd64e14bd0f53e, thanks for adding a test
Tree-SHA512: 8228f9027e52ed534411d595c7e45cf4edeee9757f26f5141fbcfae3fc6f598a8cea7f734bb8f55238857a37ad2f2d518e859e1fe8c106c0712da976792ac132
fa566b2601ee5a40bf814e529d7db253dacd28e7 test: Add missing sync_blocks to feature_pruning (MarcoFalke)
Pull request description:
Fixes#16537Fixes#16520
ACKs for top commit:
promag:
ACK fa566b2601ee5a40bf814e529d7db253dacd28e7.
jonatack:
ACK fa566b2601ee5a40bf814e529d7db253dacd28e7. These past few months I have been seeing intermittent failures with this test on master. Ran `(for i in {1..40}; do test/functional/feature_pruning.py -l=debug; done)` overnight with this change; no failures.
Tree-SHA512: 5181d5ea525f43ad09e1c8b9ae72e32219f483948854c6dc07dda24b790cbdf4012e586253a0e158a71a980d1ca9f5fdf06aafbe95b8ea3d9154ef2c8687395b
aebafd0edf Rename Chain getLocator -> getTipLocator (Russell Yanofsky)
2c1fbaa771 Drop redundant get_value_or (Russell Yanofsky)
84adb206fc Fix ScanForWalletTransactions start_block comment (Russell Yanofsky)
2efa66b464 Document rescanblockchain returned stop_height being null (Russell Yanofsky)
db2d093233 Add suggested rescanblockchain comments (Russell Yanofsky)
a8d645c934 Update ScanForWalletTransactions result comment (Russell Yanofsky)
95a812b599 Rename ScanResult stop_block field (Russell Yanofsky)
Pull request description:
This implements suggested changes from #14711 review comments that didn't make make it in before merging.
There are no changes in behavior in this PR, just documentation updates, simplifications, and variable renames.
Tree-SHA512: 39f1a5718195732b70b5e427c3b3e4295ea5af6328a5991763a422051212dfb95383186db0c0504ce2c2782fb61998dfd2fe9851645b7cb4e75d849049483cc8
# Conflicts:
# src/interfaces/chain.cpp
# src/qt/test/wallettests.cpp
# src/wallet/test/wallet_tests.cpp
# src/wallet/wallet.cpp
44de1561a Remove remaining chainActive references from CWallet (Russell Yanofsky)
db21f0264 Convert CWallet::ScanForWalletTransactions and SyncTransaction to the new Chain apis (Russell Yanofsky)
2ffb07929 Add findFork and findBlock to the Chain interface (Russell Yanofsky)
d93c4c1d6 Add time methods to the Chain interface (Russell Yanofsky)
700c42b85 Add height, depth, and hash methods to the Chain interface (Russell Yanofsky)
Pull request description:
This change removes uses of `chainActive` and `mapBlockIndex` globals in wallet code. It is a refactoring change which does not affect external behavior.
This is the next step in the larger #10973 refactoring change, which removes all other accesses to node global variables from wallet code. Doing this is useful to provide a better defined interface between the wallet and node, and necessary to allow wallet and node code to run in separate processes in #10102.
Tree-SHA512: 4dcec8a31c458f54e2ea6ecf01e430469b0994c5b41a21a2d150efa67cd209f4c93ae210a101e064b3a87c52c6edfc70b070e979992be0e3a00fd425de6230a8
* Implement hf for reduced governance fee
Signed-off-by: pasta <pasta@dashboost.org>
* update VersionBitsDeploymentInfo
* Adjust activation times for gov fee hard fork
* Use raw timestamps
* Adjust timestamps
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
* Qt: Adds settings option to showGovernanceTab
* Qt: Adds not-yet-functional Governance tab to UI
* library: adds hook into governance rom node interface
* Qt: WIP: puts CGovernanceObject hashes on Governance tab
WIP: basically ready to be filled out, most of the infra in place
now.
* Qt: Populates Governance table values
Governance table now contains real values for current columns. Display
columns may potentially change.
- want url in table if right click option opens url?
- What set of cols wanted? Show yes/no votes?
- Decide refresh functionality.
- Do we even want filter func? (checkbox show: proposal, trigger, active coming)
* qt: Shows Voting Status on Governance page
Towards DMT like Governance page, now shows voting status, but needs to
look at super blocks to determine if proposal period has passed and show
final funded or unfunded for past proposals.
* Qt: refactor governance tab to use Model-View arch
Refactors the QT Governance tab to use a Model-View architecture. The
list of Proposals is stored in a QAbstractItemModel, and a QTableView is
used to display.
This makes a much cleaner separation of concerns in the implementation.
Also, can now use the QSortFilterProxyModel to get responsive filtering.
Less internal state inside the GovernanceList, critical sections
removed.
- Needs update loop implemented.
- Needs Proposal detail widget implemented.
* qt: adds periodic update to gov proposal list
Periodically update list of governance related proposals.
* qt: populates governancelist voting status column
* qt: governancelist Porposal shows extra info on doubleClick
* qt: governancelist: reorder .cpp impl to match .h
* qt: governancelist: fixup based on CI
- Adds LOCK(cs_main) for call to pGovObj->IsValidLocally.
- retab, removes trailing whitespace in governancelist.{cpp,h}
* qt: clang-format for governancelist
* Fixes
- drop unused include
- use proper univalue "get_" functions
- shorter/translatable statuses
- shorter dates
- add missing css styles
* qt: addresses governancelist code review items
* qt: use enum to name columns of governancelist
* fix: clear context menu before adding new items
* fix: skip potential nullptr proposals
* improve: show proposal url in context menu
* refactor: tweak enum name, make sure it's int starting from 0, use full name
* fix: column count
* improve: make it sortable, sort by start_date by default
* qt: use full col name in voting status col change signal emit
* better sorting
* use mapToSource to fix data fetching
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
62d50ef308 Add LOCKS_EXCLUDED(cs_main) to LimitValidationInterfaceQueue(...) which does AssertLockNotHeld(cs_main) (practicalswift)
Pull request description:
This PR adds compile-time checking for negative locking requirements that follow from the run-time locking requirement `AssertLockNotHeld(cs_main)` in `LimitValidationInterfaceQueue(...)`.
Changes:
* Add `LOCKS_EXCLUDED(cs_main)` to `LimitValidationInterfaceQueue(...)` which does `AssertLockNotHeld(cs_main)`
* Add `LOCKS_EXCLUDED(cs_main)` to `CChainState::ActivateBestChain(…)`, `CChainState:: InvalidateBlock(…)` and `CChainState::RewindBlockIndex(…)` which all call `LimitValidationInterfaceQueue(...)` which does `AssertLockNotHeld(cs_main)`
* Add `LOCKS_EXCLUDED(cs_main)` to `InvalidateBlock(…)` which calls `CChainState::InvalidateBlock(...)` which in turn calls `LimitValidationInterfaceQueue(...)` which does `AssertLockNotHeld(cs_main)`
* Add `LOCKS_EXCLUDED(cs_main)` to `RewindBlockIndex(…)` which calls `CChainState::RewindBlockIndex(...)` which in turn calls `LimitValidationInterfaceQueue(...)` which does `AssertLockNotHeld(cs_main)`
ACKs for commit 62d50e:
MarcoFalke:
utACK 62d50ef308
Tree-SHA512: 73d092ccd08c851ae3c5d60370c369fc030c5793f5507e2faccb6f91c851ddc0ce059fbea3899f2856330d7a8c78f2ac6a2988e8268b03154f946be9e60e3be1
2a4e60b48261d3f0ec3d85f97af998ef989134e0 Fix block index inconsistency in InvalidateBlock() (Suhas Daftuar)
Pull request description:
Previously, we could release `cs_main` while leaving the block index in a state
that would fail `CheckBlockIndex()`, because `setBlockIndexCandidates` was not being
fully populated before releasing `cs_main`.
ACKs for top commit:
TheBlueMatt:
utACK 2a4e60b48261d3f0ec3d85f97af998ef989134e0. I also discovered another issue in InvalidateBlock while reviewing, see #16856.
Sjors:
ACK 2a4e60b. Tested on top of #16899. Also tested `invalidateblock` with `-checkblockindex=1`.
fjahr:
ACK 2a4e60b. Ran tests, reviewed code, inspected behavior while manually testing `invalidateblock`.
Tree-SHA512: ced12f9dfff0d413258c709921543fb154789898165590b30d1ee0cdc72863382f189744f7669a7c924d3689a1cc623efdf4e5ae3efc60054572c1e6826de612
519b0bc5dc5155b6f7e2362c2105552bb7618ad0 Make last disconnected block BLOCK_FAILED_VALID, even when aborted (Pieter Wuille)
8d220417cd7bc34464e28a4861a885193ec091c2 Optimization: don't add txn back to mempool after 10 invalidates (Pieter Wuille)
9ce9c37004440d6a329874dbf66b51666d497dcb Prevent callback overruns in InvalidateBlock and RewindBlockIndex (Pieter Wuille)
9bb32eb571a846b66ed3bac493f55cee11a3a1b9 Release cs_main during InvalidateBlock iterations (Pieter Wuille)
9b1ff5c742dec0a6e0d6aab29b0bb771ad6d8135 Call InvalidateBlock without cs_main held (Pieter Wuille)
241b2c74ac8c4c3000e778554da1271e3f293e5d Make RewindBlockIndex interruptible (Pieter Wuille)
880ce7d46b51835c00d77a366ec28f54a05239df Call RewindBlockIndex without cs_main held (Pieter Wuille)
436f7d735f1c37e77d42ff59d4cbb1bd76d5fcfb Release cs_main during RewindBlockIndex operation (Pieter Wuille)
1d342875c21b5d0a17cf4d176063bb14b35b657e Merge the disconnection and erasing loops in RewindBlockIndex (Pieter Wuille)
32b2696ab4b079db736074b57bbc24deaee0b3d9 Move erasure of non-active blocks to a separate loop in RewindBlockIndex (Pieter Wuille)
9d6dcc52c6cb0cdcda220fddccaabb0ffd40068d Abstract EraseBlockData out of RewindBlockIndex (Pieter Wuille)
Pull request description:
This PR makes a number of improvements to the InvalidateBlock (`invalidateblock` RPC) and RewindBlockIndex functions, primarily around breaking up their long-term cs_main holding. In addition:
* They're made safely interruptible (`bitcoind` can be shutdown, and no progress in either will be lost, though if incomplete, `invalidateblock` won't continue after restart and will need to be called again)
* The validation queue is prevented from overflowing (meaning `invalidateblock` on a very old block will not drive bitcoind OOM) (see #14289).
* `invalidateblock` won't bother to move transactions back into the mempool after 10 blocks (optimization).
This is not an optimal solution, as we're relying on the scheduler call sites to make sure the scheduler doesn't overflow. Ideally, the scheduler would guarantee this directly, but that needs a few further changes (moving the signal emissions out of cs_main) to prevent deadlocks.
I have manually tested the `invalidateblock` changes (including interrupting, and running with -checkblockindex and -checkmempool), but haven't tried the rewinding (which is probably becoming increasingly unnecessary, as very few pre-0.13.1 nodes remain that would care to upgrade).
Tree-SHA512: 692e42758bd3d3efc2eb701984a8cb5db25fbeee32e7575df0183a00d0c2c30fdf72ce64c7625c32ad8c8bdc56313da72a7471658faeb0d39eefe39c4b8b8474
* replace raw owning ptr with unique ptr
Signed-off-by: pasta <pasta@dashboost.org>
* Add GUARDED_BY annotation to llmq_versionbitscache
Signed-off-by: pasta <pasta@dashboost.org>
* limit scope of locking cs_llmq_vbc
Signed-off-by: pasta <pasta@dashboost.org>
* use llmq_versionbitscache instead of versionbitscache in UpdatedBlockTip to avoid cs_main locking
Signed-off-by: pasta <pasta@dashboost.org>
* drop unneeded cs_main ::mempool.cs
* lock cs_main and mempool.cs in Db::Upgrade
