[WIP] Sync with main repo: CMake + Qt 6 #470
Draft
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…for CCheckQueueControl and drop legacy locking macro usage fd29073 validation: clean up and clarify CheckInputScripts logic (Cory Fields) 1a37507 validation: use a lock for CCheckQueueControl (Cory Fields) c3b0e6c validation: make CCheckQueueControl's CCheckQueue non-optional (Cory Fields) 4c8c90b validation: only create a CCheckQueueControl if it's actually going to be used (Cory Fields) 11fed83 threading: add LOCK_ARGS macro (Cory Fields) Pull request description: As part of an effort to cleanup our threading primitives and add safe `SharedMutex`/`SharedLock` impls, I'd like to get rid of the last of our legacy `ENTER_CRITICAL_SECTION`/`LEAVE_CRITICAL_SECTION` usage. This, along with a follow-up [after fixing REVERSE_LOCK](bitcoin/bitcoin#32465) will allow us to do that. This replaces the old macros with an RAII lock, while simplifying `CCheckQueueControl`. It now requires a `CCheckQueue`, and optionality is handled externally. In the case of validation, it is wrapped in a `std::optional`. It also adds an `LOCK_ARGS` macro for `UniqueLock` initialization which may be helpful elsewhere. ACKs for top commit: fjahr: re-ACK fd29073 ryanofsky: Code review ACK fd29073, just removing assert since last review. Thanks for considering all the comments and feedback! TheCharlatan: Re-ACK fd29073 Tree-SHA512: 54b9db604ee1bda7d11bce1653a88d3dcbc4f525eed6a85abdd4d6409138674af4bb8b00afa4e0d3d29dadd045a3a39de253a45f0ef9c05f56cba1aac5b59303
…RTIONS in centos task fa07953 ci: Downgrade DEBUG=1 to -D_GLIBCXX_ASSERTIONS in centos task (MarcoFalke) Pull request description: to work around bitcoin/bitcoin#32524 (comment) closes #32524 ACKs for top commit: laanwj: ACK fa07953 fanquake: ACK fa07953 - we can followup Tree-SHA512: 2d8b914c7390bbf22d8b2eb906bd2a363f92e1954646677a010b15721fca0887d5987a0af932fd0013f5c1b35c0a80c67579004a0cf635916954331c80c7bef0
This `getwalletinfo()` result field was only ever returned for legacy wallets and is hence not relevant anymore, so we can delete it and the corresponding CWallet/ScriptPubKeyMan code behind it.
…ed RPCs Now that we only ever operate on descriptor wallets, mentioning that a faster rescan is only available for them is redundant and can be removed. These texts were originally introduced in commit ca48a46 (PR #25957).
…examples This is the RPC example counterpart to commit 86de8c1 (PR #32544). Since the recent legacy wallet removal this parameter *must* be true, so providing it in the examples doesn't contain valuable information anymore and it seems best to remove them.
We'll reuse it for a target where the coins view is a DB.
…Disk` Co-authored-by: Jon Atack <[email protected]>
…removal cleanups in RPCs e5cbea4 rpc: doc: remove redundant "descriptors" parameter in `createwallet` examples (Sebastian Falbesoner) 7a05f94 rpc: doc: drop descriptor wallet mentions in fast wallet rescan related RPCs (Sebastian Falbesoner) db465a5 wallet, rpc: remove obsolete "keypoololdest" result field/code (Sebastian Falbesoner) Pull request description: This PR contains a few smaller wallet RPC cleanups based on that we only ever operate on descriptor wallets now: * remove the now obsolete "keypoololdest" field from the `getwalletinfo` RPC and the corresponding CWallet/ScriptPubKeyMan methods * in RPCs where potential fast wallet rescan is documented, remove the "descriptor wallet" mentions (back then introduced in commit ca48a46, PR #25957) * for the `createwallet` RPC examples, remove the "descriptors" parameters that always have to be true now (proposed in bitcoin/bitcoin#31250 (comment); corresponds to 86de8c1, PR #32544 which did the same for functional tests) ACKs for top commit: achow101: ACK e5cbea4 1440000bytes: ACK bitcoin/bitcoin@e5cbea4 rkrux: ACK e5cbea4 Tree-SHA512: d785f621af3f3987b258e5d7fb8309344fb13c2cf41855f8adf99ff89f581142db36e3ba59919d6abf82662caa3f7e4a2bd38eba1be9e23665e6a4a23ee52acd
It is cheap to calculate and the caller does not have to take a lock to calculate it. Also turn pointers that can never be null into references.
Ensure `ReadBlock` rejects a block when the tip’s `phashBlock` differs from the expected hash.
Eliminate one SHA‑256 double‑hash computation of the header per block read by reusing the hash for: * proof‑of‑work verification; * (optional) integrity check against the supplied hash.
Instead of allowing users to load a legacy wallet from the "Open Wallet" menu, show the legacy wallet greyed out with a message that the wallet needs to be migrated.
See https://guix.gnu.org/blog/2025/migrating-to-codeberg/. When interacting with the old repo you may now also see: ```bash warning: redirecting to https://codeberg.org/guix/guix/ ```
f66b14d test: fix pushdata scripts (Greg Sanders) Pull request description: The original scripts were done incorrectly, so they are changed to represent two different 2-byte pushes. Fixes bitcoin/bitcoin#32114 (comment) ACKs for top commit: ajtowns: ACK f66b14d TheCharlatan: Re-ACK f66b14d Tree-SHA512: 0956124ee0d2e8b6a594f9feeb47c1f598c68e24d277e874f81a093268113e9da2c75a02863dbaab68b962063f7d910bfd10abe3ad33ec182bc21d72908f06e6
…alid_txs 8fcd684 test: ensure reason is checked for invalid blocks rejection (Greg Sanders) 1a689a2 test: fix block tests of invalid_txs (Greg Sanders) Pull request description: We are not actually testing some cases accidentally, for block inclusion. Issue discovered while reviewing bitcoin/bitcoin#32533 ACKs for top commit: maflcko: review ACK 8fcd684 🔶 theStack: ACK 8fcd684 BrandonOdiwuor: Code Review ACK 8fcd684 TheCharlatan: ACK 8fcd684 Tree-SHA512: 7e79ea35b64f56b29811b29df5752945cb10ec62465b385be5e4e2d295c3237b15d5dacf9e99661346967e84899da6fc82e3d2bd0ef6e5c51da85247da31e26a
c8d9baa guix: accomodate migration to codeberg (fanquake) Pull request description: See https://guix.gnu.org/blog/2025/migrating-to-codeberg/. ACKs for top commit: maflcko: lgtm ACK c8d9baa hebasto: ACK c8d9baa. Tree-SHA512: 6a2221faf49c84cd2a4027e4b721553d700a9af0736dbbea80aa94289ff7665e7f4681c12fa3b5195e21dc3ac91fb1796635f26c1298bb5da1dee5690697edd0
…able a5ac43d doc: Add release notes describing bitcoin wrapper executable (Ryan Ofsky) 258bda8 doc: Mention bitcoin wrapper executable in documentation (Ryan Ofsky) d2739d7 build: add bitcoin.exe to windows installer (Sjors Provoost) ba649c0 ci: Run multiprocess tests through wrapper executable (Ryan Ofsky) 29bdd74 test: Support BITCOIN_CMD environment variable (Ryan Ofsky) 9c8c688 multiprocess: Add bitcoin wrapper executable (Ryan Ofsky) 5076d20 util: Add cross-platform ExecVp and GetExePath functions (Ryan Ofsky) Pull request description: Intended to make bitcoin command line features more discoverable and allow installing new multiprocess binaries in libexec/ instead of bin/ so they don't cause confusion. Idea and implementation of this were discussed in bitcoin/bitcoin#30983. --- Initial implementation of this feature is deliberately minimal so the UX can evolve in response to feedback and there are not too many details to debate and discuss in a single PR. But many improvements are possible or planned: - Adding manpage and bash completions. - Showing nicer error messages that detect if an executable isn't installed and suggest how to fix [(comment)](bitcoin/bitcoin#31375 (comment)) - Showing wrapper command lines in subcommand in help output [(comment)](bitcoin/bitcoin#31375 (comment)). This could be done conditionally as suggested in the comment or be unconditional. - Showing wrapper command lines in subcommand error output. There is a bitcoin-cli error pointed out in [(comment)](bitcoin/bitcoin#31375 (comment)) that is needlessly confusing. - Integrating help so `bitcoin help subcommand` invokes `bitcoin subcommand -h`. `bitcoin -h subcommand` should also be supported and be equivalent [(comment)](bitcoin/bitcoin#31375 (comment)) - Adding support for `bitcoin-util` subcommands. Ideal interface would probably be more like `bitcoin grind` not `bitcoin util grind` but this has been punted for now. Supporting subcommands directly would require some ArgsManager modifications - Adding a dedicated python functional test for the wrapper. Right now there is some CI coverage by setting the `BITCOIN_CMD` variable, but this doesn't cover things like the help output and version output, and support for different directory layouts. - Better `--multiprocess` (`-m`) / `--monolithic` (`-M`) default selection. Right now, default is monolithic but it probably makes sense to chose more intelligently depending on whether -ipc options are enabled and what binaries are available. - Maybe parsing `bitcoin.conf` and supporting options to control wrapper behavior like custom locations or preferences or aliases. - Better command command line usability. Allow combining short options like (`-ah`). Allow fuzzy matching of subcommands or suggestions if you misspell. (suggested by stickies in review club) - Not directly related to this PR but `bitcoin-cli named` implementation used by the wrapper should do a better job disambiguating named arguments from base64 arguments ending in = as pointed out in [(comment)](bitcoin/bitcoin#31375 (comment)) --- This PR is part of the [process separation project](bitcoin/bitcoin#28722). A review club meeting for it took place in https://bitcoincore.reviews/31375 ACKs for top commit: Sjors: utACK a5ac43d achow101: ACK a5ac43d vasild: ACK a5ac43d theStack: ACK a5ac43d ismaelsadeeq: fwiw my last review implied an ACK a5ac43d hodlinator: ACK a5ac43d Tree-SHA512: 570e6a4ff8bd79ef6554da3d01f36c0a7c6d2dd7dace8f8732eca98f4a8bc2284474a9beadeba783114fe2f3dd08b2041b3da7753bae0b7f881ec50668cb821f
fa4b8b1 test: Add missing ipc subtree to lint (MarcoFalke) Pull request description: The subtree docs and lint checks list the subtrees in three places, making it hard to follow and update and easy to miss one. Fix all issues by including the missing one and removing the list in one place. ACKs for top commit: l0rinc: ACK fa4b8b1 davidgumberg: ACK bitcoin/bitcoin@fa4b8b1 achow101: ACK fa4b8b1 hebasto: re-ACK fa4b8b1, only a [comment](bitcoin/bitcoin#32623 (comment)) has been addressed since my recent [review](bitcoin/bitcoin#32623 (review)). BrandonOdiwuor: Re-ACK fa4b8b1 Tree-SHA512: 97a39cf63be2707b2e03502546797ef639a611030e0a860618256c7c0683381be0f05c8bf850f9542a25266a52f58cef690abf9cc243498b6e026c7a6f9a52b2
…ash in `ReadBlock` 09ee8b7 node: avoid recomputing block hash in `ReadBlock` (Lőrinc) 2bf1732 test: exercise `ReadBlock` hash‑mismatch path (Lőrinc) Pull request description: Eliminate one block header hash calculation per block-read by reusing the hash for: * proof‑of‑work verification; * (optional) integrity check against the supplied hash. This part of the code wasn't covered by tests either, so the first commit exercises this part first, before pushing the validation to the delegate method. ACKs for top commit: maflcko: lgtm ACK 09ee8b7 achow101: ACK 09ee8b7 jonatack: ACK 09ee8b7 pinheadmz: ACK 09ee8b7 Tree-SHA512: 43fe51b478ea574b6d4c952684b13ca54fb8cbd67c3b6c136f460122d9ee953cc70b88778537117eecea71ccb8d88311faeac21b866e11d117f1145973204ed4
The current test directly uses invalidatblock to trigger mempool re-entry of transactions. Unfortunately, the behavior doesn't match what a real reorg would look like. As a result you get surprising behavior such as the mempool descendant chain limits being exceeded, or if a fork is greater than 10 blocks deep, evicted block transactions stop being submitted back into in the mempool. Fix this by preparing an empty fork chain, and then continuing with the logic, finally submitting the fork chain once the rest of the test is prepared. This triggers a more typical codepath. Also, extend the descendant limit to 100, like ancestor limit.
53e9b71 log: print reason for why should_write was triggered in `FlushStateToDisk` (Lőrinc) Pull request description: This PR addresses a leftover logging nit found while reviewing bitcoin/bitcoin#30611 (review). This was also needed to validate its behavior properly, because currently there's no way to visualize how often (and why) we're flushing/syncing. Starting with `-debug=coindb` will now add log lines such as ``` 2025-05-03T08:34:57Z [coindb] Writing chainstate to disk: flush mode=PERIODIC, prune=0, large=0, critical=0, periodic=1 2025-05-03T09:26:52Z [coindb] Writing chainstate to disk: flush mode=PERIODIC, prune=0, large=0, critical=0, periodic=1 2025-05-03T10:27:58Z [coindb] Writing chainstate to disk: flush mode=PERIODIC, prune=0, large=0, critical=0, periodic=1 2025-05-03T11:39:20Z [coindb] Writing chainstate to disk: flush mode=PERIODIC, prune=0, large=0, critical=0, periodic=1 2025-05-03T12:41:48Z [coindb] Writing chainstate to disk: flush mode=PERIODIC, prune=0, large=0, critical=0, periodic=1 2025-05-03T13:40:08Z [coindb] Writing chainstate to disk: flush mode=PERIODIC, prune=0, large=0, critical=0, periodic=1 2025-05-03T14:49:16Z [coindb] Writing chainstate to disk: flush mode=PERIODIC, prune=0, large=0, critical=0, periodic=1 2025-05-03T15:14:37Z [coindb] Writing chainstate to disk: flush mode=ALWAYS, prune=0, large=0, critical=0, periodic=0 2025-05-03T15:17:28Z [coindb] Writing chainstate to disk: flush mode=ALWAYS, prune=0, large=0, critical=0, periodic=0 ``` ACKs for top commit: davidgumberg: ACK bitcoin/bitcoin@53e9b71 jonatack: diff-only review ACK 53e9b71 per `git range-diff 2df824 cfc8056 53e9b71` achow101: ACK 53e9b71 andrewtoth: utACK 53e9b71 Tree-SHA512: 09f3a38cf3ecaa32cf3aba350a9e9dff9345c5468a05070c8b20987f0fdb23a8b1dc59370829c64ea356d2fc0ce99a66cc7240de7fa8c19ef3133da06db6bf3d
Github-Pull: bitcoin-core#198 Rebased-From: 9cc991f
This is not used at all. This is a remnant from a time where the idea was to have the navbar encapsulated within the Wizard control, and this bool would dictate what navbutton to display. We have since moved to each page provides its navbar and its navbuttons. Github-Pull: bitcoin-core#198 Rebased-From: 5120735
Github-Pull: bitcoin-core#198 Rebased-From: 8619e50
This is no longer providing any value and is not needed. Github-Pull: bitcoin-core#198 Rebased-From: 6c29ddf
Github-Pull: bitcoin-core#204 Rebased-From: 76287e4
Currently, the parent SwipeView of the AboutOptions page needs to have an id of introductions. We abuse the knowledge that in our codebase we will have that available, but this limits when and where the AboutOptions can be used. This entangles the About and Developer options so that we can avoid this, and have these pages truly be reusable. Github-Pull: bitcoin-core#203 Rebased-From: 6ac40e8 Co-Authored-By: Johnny Carlson <[email protected]>
Github-Pull: bitcoin-core#203 Rebased-From: 6037471 Co-Authored-By: Johnny Carlson <[email protected]>
Github-Pull: bitcoin-core#210 Rebased-From: 4dc32c4
This does away with a loadable detail and places a check mark icon in it's place. Github-Pull: bitcoin-core#205 Rebased-From: 2c8ac00
Github-Pull: bitcoin-core#206 Rebased-From: a9812ff
Github-Pull: bitcoin-core#199 Rebased-From: 956605f
…nslatable. Github-Pull: bitcoin-core#217 Rebased-From: d528468
Github-Pull: bitcoin-core#194 Rebased-From: ecd1409
With the ScrollView now containing the InformationPages, the ContinueButton was not able to stick to the bottom of the view due to contentHeight being too limited. This allows contentHeight to grow with the screen while limiting it so the button doesn't overlap with the information. Github-Pull: bitcoin-core#194 Rebased-From: d17e309
Github-Pull: bitcoin-core#207 Rebased-From: 4c6562e
Github-Pull: bitcoin-core#207 Rebased-From: 93f78f8
Wires both the easy and advanced configuration options related to storage amount to the options model backend. Additionally, this removes an unused and unwanted storage location setting from the advanced storage amount settings table. Github-Pull: bitcoin-core#207 Rebased-From: 63162b0
refactor initerror window to adhere to designers specification Github-Pull: bitcoin-core#164 Rebased-From: 86fde6b
Github-Pull: bitcoin-core#222 Rebased-From: faeb551
Github-Pull: bitcoin-core#222 Rebased-From: 04cb898
Github-Pull: bitcoin-core#222 Rebased-From: 30dec24
Github-Pull: bitcoin-core#222 Rebased-From: 15291b2
- This method would allow to get the blocktime value of the block at the given height. Github-Pull: bitcoin-core#220 Rebased-From: 1fce853
Github-Pull: bitcoin-core#220 Rebased-From: 094a2c4 Co-authored-by: shaavan <[email protected]>
Additional properties to Header will allow configuring Header's description color and bold values as well as Header's header-text bold value. Github-Pull: bitcoin-core#220 Rebased-From: 8db7f0f Co-authored-by: shaavan <[email protected]>
The properties will allow the gui to show an estimate for the time remaining before the node is synced. Github-Pull: bitcoin-core#220 Rebased-From: eb945ac Co-authored-by: shaavan <[email protected]>
The ChainModel is responsible for managing the block information that the gui components need to show. In this case, the ChainModel will provide a list of ratios for the block times of the last 12 hours to be rendered on the block clock's dial. Github-Pull: bitcoin-core#220 Rebased-From: 60bd8cb Co-authored-by: shaavan <[email protected]>
Implements a few of the BlockClock dial states. Sync progress while downloading, rendering blocks after sync, and pausing/unpausing the node. An additional Model is added, ChainModel, to provide the dial block information. Changes to NodeModel are also made to support the pausing/unpausing Dial feature. Github-Pull: bitcoin-core#220 Rebased-From: 6701ad2 Co-authored-by: shaavan <[email protected]>
|
concept ACK, outrageously amazing work. Built on macos/arm64 same way I build bitcoin/bitcoin, goodbye auto tools! Build was successful, lots of buggy things remain but will continue to test as the branch develops. Crashed here with Command-Q to quit on regtest resulted in |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Note: Android is not supported at this time. Synced up to a56361d.
Base: bitcoin/bitcoin@e217437.
For a detailed explanation of the chosen directory layout, please refer to the following blog posts and Qt documentation:
Note: Android is not supported at this time.
TODO:
Any help is much appreciated!