💬 achow101 commented on pull request "wallet: migration, avoid creating spendable wallet from a watch-only legacy wallet":
(https://github.com/bitcoin/bitcoin/pull/31423#discussion_r2114913663)
In b78990734621b8fe46c68a6e7edaf1fbd2f7d351 "wallet: migration, avoid creating spendable wallet from a watch-only legacy wallet"
I think `empty_local_wallet` is unnecessary and the condition can check `local_wallet->GetAllScriptPubKeyMans().empty()` directly.
(https://github.com/bitcoin/bitcoin/pull/31423#discussion_r2114913663)
In b78990734621b8fe46c68a6e7edaf1fbd2f7d351 "wallet: migration, avoid creating spendable wallet from a watch-only legacy wallet"
I think `empty_local_wallet` is unnecessary and the condition can check `local_wallet->GetAllScriptPubKeyMans().empty()` directly.
📝 theStack opened a pull request: "test: update BIP340 test vectors and implementation (variable-length messages)"
(https://github.com/bitcoin/bitcoin/pull/32642)
This PR updates the Schnorr signatures implementation in the functional test framework to the latest BIP changes (see https://github.com/bitcoin/bips/pull/1446,commit 200f9b26fe0a2f235a2af8b30c4be9f12f6bc9cb) and syncs the [test vectors](https://github.com/bitcoin/bips/blob/master/bip-0340/test-vectors.csv) accordingly. Practically, we probably don't need non-32-bytes message signing/verifying any time soon, but it seems good practice anyways to update anyways.
(https://github.com/bitcoin/bitcoin/pull/32642)
This PR updates the Schnorr signatures implementation in the functional test framework to the latest BIP changes (see https://github.com/bitcoin/bips/pull/1446,commit 200f9b26fe0a2f235a2af8b30c4be9f12f6bc9cb) and syncs the [test vectors](https://github.com/bitcoin/bips/blob/master/bip-0340/test-vectors.csv) accordingly. Practically, we probably don't need non-32-bytes message signing/verifying any time soon, but it seems good practice anyways to update anyways.
💬 davidgumberg commented on pull request "walletdb: Log additional exception error messages for corrupted wallets":
(https://github.com/bitcoin/bitcoin/pull/32598#issuecomment-2920938821)
ACK https://github.com/bitcoin/bitcoin/commit/ad9a13fc424e9deb262e2b1d54bcdc7370263ea0
(https://github.com/bitcoin/bitcoin/pull/32598#issuecomment-2920938821)
ACK https://github.com/bitcoin/bitcoin/commit/ad9a13fc424e9deb262e2b1d54bcdc7370263ea0
💬 davidgumberg commented on pull request "deps: Bump lief to 0.16.6":
(https://github.com/bitcoin/bitcoin/pull/32431#issuecomment-2921038966)
- Rebased against master to include https://github.com/bitcoin/bitcoin/pull/32634/commits/dbb2d4c3d54759f8c346882b6f98d26d5bb8e816
- Bumped LIEF to 0.16.6 which resolves https://github.com/lief-project/LIEF/issues/1217
- Added a workaround for https://github.com/lief-project/LIEF/pull/1218, this is acceptable and won't need to be resolved once the upstream patch is merged, since this change tightens our MachO `has_nx` checks to both the stack and heap.
- Added a commit to enable a previously
...
(https://github.com/bitcoin/bitcoin/pull/32431#issuecomment-2921038966)
- Rebased against master to include https://github.com/bitcoin/bitcoin/pull/32634/commits/dbb2d4c3d54759f8c346882b6f98d26d5bb8e816
- Bumped LIEF to 0.16.6 which resolves https://github.com/lief-project/LIEF/issues/1217
- Added a workaround for https://github.com/lief-project/LIEF/pull/1218, this is acceptable and won't need to be resolved once the upstream patch is merged, since this change tightens our MachO `has_nx` checks to both the stack and heap.
- Added a commit to enable a previously
...
👋 davidgumberg's pull request is ready for review: "deps: Bump lief to 0.16.6"
(https://github.com/bitcoin/bitcoin/pull/32431)
(https://github.com/bitcoin/bitcoin/pull/32431)
💬 Sjors commented on pull request "Update libmultiprocess subtree to fix clang-tidy errors":
(https://github.com/bitcoin/bitcoin/pull/32641#issuecomment-2921262265)
ACK 9f6565488fc169898abc5f7ee620ce7a9a4e5e50
Tested with `git-subtree-check.sh -r`
(https://github.com/bitcoin/bitcoin/pull/32641#issuecomment-2921262265)
ACK 9f6565488fc169898abc5f7ee620ce7a9a4e5e50
Tested with `git-subtree-check.sh -r`
💬 Sjors commented on pull request "Add checkBlock() to Mining interface":
(https://github.com/bitcoin/bitcoin/pull/31981#discussion_r2115179447)
At least we have a test that checks the UTXO set isn't updated: https://github.com/bitcoin/bitcoin/blob/1def3bc3773b3013a92f54daa75acb82c30beaac/test/functional/mining_template_verification.py#L145-L156
But that doesn't cover all conceivable future ways that we might accidentally flush.
If we do want to add a belt-and-suspenders way to prevent such a flush, it seems better for a followup.
> there could be better ways of preventing it like adding a read-only option to `CCoinsViewCache` t
...
(https://github.com/bitcoin/bitcoin/pull/31981#discussion_r2115179447)
At least we have a test that checks the UTXO set isn't updated: https://github.com/bitcoin/bitcoin/blob/1def3bc3773b3013a92f54daa75acb82c30beaac/test/functional/mining_template_verification.py#L145-L156
But that doesn't cover all conceivable future ways that we might accidentally flush.
If we do want to add a belt-and-suspenders way to prevent such a flush, it seems better for a followup.
> there could be better ways of preventing it like adding a read-only option to `CCoinsViewCache` t
...
💬 Sjors commented on pull request "cmake: Move internal binaries from bin/ to libexec/":
(https://github.com/bitcoin/bitcoin/pull/31679#issuecomment-2921299759)
Just #28710, no conflict with #31375? Will re-review shortly.
(https://github.com/bitcoin/bitcoin/pull/31679#issuecomment-2921299759)
Just #28710, no conflict with #31375? Will re-review shortly.
🚀 fanquake merged a pull request: "Update libmultiprocess subtree to fix clang-tidy errors"
(https://github.com/bitcoin/bitcoin/pull/32641)
(https://github.com/bitcoin/bitcoin/pull/32641)
⚠️ fanquake opened an issue: "fs: use `ftruncate` on OpenBSD in `AllocateFileRange`"
(https://github.com/bitcoin/bitcoin/issues/32643)
OpenBSD doesn't have `fallocate` or `posix_fallocate`, which means it's hitting our fallback. We should instead have it use `ftruncate`. See related discussion in #32460.
cc @theStack.
(https://github.com/bitcoin/bitcoin/issues/32643)
OpenBSD doesn't have `fallocate` or `posix_fallocate`, which means it's hitting our fallback. We should instead have it use `ftruncate`. See related discussion in #32460.
cc @theStack.
💬 fanquake commented on pull request "fs: remove `_POSIX_C_SOURCE` defining":
(https://github.com/bitcoin/bitcoin/pull/32460#issuecomment-2921749068)
Opened #32643 to followup.
(https://github.com/bitcoin/bitcoin/pull/32460#issuecomment-2921749068)
Opened #32643 to followup.
🚀 fanquake merged a pull request: "fs: remove `_POSIX_C_SOURCE` defining"
(https://github.com/bitcoin/bitcoin/pull/32460)
(https://github.com/bitcoin/bitcoin/pull/32460)
📝 fanquake converted_to_draft a pull request: "threading: remove ancient CRITICAL_SECTION macros"
(https://github.com/bitcoin/bitcoin/pull/32592)
Now that #32467 is merged, the only remaining usage of our old `CRITICAL_SECTION` macros (other than tests) is in `getblocktemplate()` and it can safely be replaced with a `REVERSE_LOCK`.
This PR makes that replacement, replaces the old `CRITICAL_SECTION` macro usage in tests, then deletes the macros themselves.
While testing this a few weeks ago, I noticed that `REVERSE_LOCK` does not currently work properly with our thread-safety annotations as after the `REVERSE_LOCK` is acquired, clang
...
(https://github.com/bitcoin/bitcoin/pull/32592)
Now that #32467 is merged, the only remaining usage of our old `CRITICAL_SECTION` macros (other than tests) is in `getblocktemplate()` and it can safely be replaced with a `REVERSE_LOCK`.
This PR makes that replacement, replaces the old `CRITICAL_SECTION` macro usage in tests, then deletes the macros themselves.
While testing this a few weeks ago, I noticed that `REVERSE_LOCK` does not currently work properly with our thread-safety annotations as after the `REVERSE_LOCK` is acquired, clang
...
💬 fanquake commented on pull request "test: update BIP340 test vectors and implementation (variable-length messages)":
(https://github.com/bitcoin/bitcoin/pull/32642#issuecomment-2921839411)
cc @sipa @real-or-random @jonasnick
(https://github.com/bitcoin/bitcoin/pull/32642#issuecomment-2921839411)
cc @sipa @real-or-random @jonasnick
💬 Sjors commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-2921871074)
https://github.com/bitcoin-core/libmultiprocess/pull/172 landed and the subtree was updated in #32641.
Marking ready to review again.
Although #31679 is orthogonal, it's also worth reviewing. There should be no impact in user experience, because thanks to https://github.com/bitcoin/bitcoin/pull/31375 `bitcoin -m` will find `bitcoin-node` whether it's in `/usr/local/bin` or `/usr/local/libexec`. But an upgrade would leave one binary in both paths, so it's nicer to get that PR in the same re
...
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-2921871074)
https://github.com/bitcoin-core/libmultiprocess/pull/172 landed and the subtree was updated in #32641.
Marking ready to review again.
Although #31679 is orthogonal, it's also worth reviewing. There should be no impact in user experience, because thanks to https://github.com/bitcoin/bitcoin/pull/31375 `bitcoin -m` will find `bitcoin-node` whether it's in `/usr/local/bin` or `/usr/local/libexec`. But an upgrade would leave one binary in both paths, so it's nicer to get that PR in the same re
...
👋 Sjors's pull request is ready for review: "Add bitcoin-{node,gui} to release binaries for IPC"
(https://github.com/bitcoin/bitcoin/pull/31802)
(https://github.com/bitcoin/bitcoin/pull/31802)
📝 fanquake opened a pull request: "doc: miscellaneous changes"
(https://github.com/bitcoin/bitcoin/pull/32644)
A round up of #32629 + some other changes that had previously been PR'd.
(https://github.com/bitcoin/bitcoin/pull/32644)
A round up of #32629 + some other changes that had previously been PR'd.
🚀 fanquake merged a pull request: "[28.x] Backport guix: accomodate migration to codeberg"
(https://github.com/bitcoin/bitcoin/pull/32639)
(https://github.com/bitcoin/bitcoin/pull/32639)
💬 TheCharlatan commented on pull request "headerssync: Keep tests ahead of increasing params":
(https://github.com/bitcoin/bitcoin/pull/32579#issuecomment-2921899092)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/32579#issuecomment-2921899092)
Concept ACK
🚀 fanquake merged a pull request: "wallet, rpc, gui: List legacy wallets with a message about migration"
(https://github.com/bitcoin/bitcoin/pull/32619)
(https://github.com/bitcoin/bitcoin/pull/32619)