💬 l0rinc commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2353628930)
fantastic, I had something similar but only got to 90%.
Added the change as node 3 since it has a similar starting condition as the rest of the nodes, simplifies the header send to only create the needed headers and send all of them and fixed the waiting condition.
Added you as coauthor.
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2353628930)
fantastic, I had something similar but only got to 90%.
Added the change as node 3 since it has a similar starting condition as the rest of the nodes, simplifies the header send to only create the needed headers and send all of them and fixed the waiting condition.
Added you as coauthor.
💬 l0rinc commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2353634828)
I have added a reason string with limited scope next to the `bool`, let me know what you think. Added you ad coauthor.
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2353634828)
I have added a reason string with limited scope next to the `bool`, let me know what you think. Added you ad coauthor.
💬 l0rinc commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2353201643)
The args are only invalid in relation to the blocks, so we need at least one block to invalidate the args.
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2353201643)
The args are only invalid in relation to the blocks, so we need at least one block to invalidate the args.
💬 l0rinc commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2353652988)
Ii have reverted the naming
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2353652988)
Ii have reverted the naming
💬 l0rinc commented on pull request "help: enrich help text for `-loadblock`":
(https://github.com/bitcoin/bitcoin/pull/33343#issuecomment-3300371394)
I think we should try it ourselves before recommending it in the documentation
(https://github.com/bitcoin/bitcoin/pull/33343#issuecomment-3300371394)
I think we should try it ourselves before recommending it in the documentation
💬 achow101 commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2353675490)
I'm going to leave this as-is for now since changing the type causes a ton of other things to be changed, particularly in PSBT and SignatureData. We also use the fact that it's a vector to indicate when a pubnonce was not successfully created, and turning it into an array requires changing all of those returns as well.
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2353675490)
I'm going to leave this as-is for now since changing the type causes a ton of other things to be changed, particularly in PSBT and SignatureData. We also use the fact that it's a vector to indicate when a pubnonce was not successfully created, and turning it into an array requires changing all of those returns as well.
📝 hebasto opened a pull request: "msvc: Update vcpkg manifest"
(https://github.com/bitcoin/bitcoin/pull/33408)
This PR updates the vcpkg manifest baseline from the ["2025.03.19 Release"](https://github.com/microsoft/vcpkg/releases/tag/2025.03.19) to the ["2025.08.27 Release"](https://github.com/microsoft/vcpkg/releases/tag/2025.08.27), with the following package
changes:
- `boost`: 1.87.0 --> 1.88.0
- `qtbase`: 6.8.2#1 -> 6.9.1
- `qttools`: 6.8.2 -> 6.9.1
- `sqlite3`: 3.49.1 --> 3.50.4
The previous update was made in https://github.com/bitcoin/bitcoin/pull/32213.
Additionally, the obsolete
...
(https://github.com/bitcoin/bitcoin/pull/33408)
This PR updates the vcpkg manifest baseline from the ["2025.03.19 Release"](https://github.com/microsoft/vcpkg/releases/tag/2025.03.19) to the ["2025.08.27 Release"](https://github.com/microsoft/vcpkg/releases/tag/2025.08.27), with the following package
changes:
- `boost`: 1.87.0 --> 1.88.0
- `qtbase`: 6.8.2#1 -> 6.9.1
- `qttools`: 6.8.2 -> 6.9.1
- `sqlite3`: 3.49.1 --> 3.50.4
The previous update was made in https://github.com/bitcoin/bitcoin/pull/32213.
Additionally, the obsolete
...
👋 willcl-ark's pull request is ready for review: "Backport Cirrus runners to 28.x"
(https://github.com/bitcoin/bitcoin/pull/33406)
(https://github.com/bitcoin/bitcoin/pull/33406)
💬 achow101 commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2353681237)
We want to avoid libsecp includes and linking being everywhere throughout the codebase that a pubnonce might appear, so we prefer to use something that contains the serialized nonce, and deserializing it into `secp256k1_musig_pubnonce` when needed.
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2353681237)
We want to avoid libsecp includes and linking being everywhere throughout the codebase that a pubnonce might appear, so we prefer to use something that contains the serialized nonce, and deserializing it into `secp256k1_musig_pubnonce` when needed.
💬 l0rinc commented on pull request "log: show reindex progress in `ImportBlocks`":
(https://github.com/bitcoin/bitcoin/pull/33353#discussion_r2353681440)
Thanks @enirox001, I think the 0% is fine and we've just calculated `total_files`, no need to guard and the percentage is the same as the `nFile+1/total_files`, I don't see the need to duplicate it.
(https://github.com/bitcoin/bitcoin/pull/33353#discussion_r2353681440)
Thanks @enirox001, I think the 0% is fine and we've just calculated `total_files`, no need to guard and the percentage is the same as the `nFile+1/total_files`, I don't see the need to duplicate it.
💬 achow101 commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2353682010)
It's to avoid having to include and link the libsecp module in a bunch of irrelevant places.
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2353682010)
It's to avoid having to include and link the libsecp module in a bunch of irrelevant places.
💬 hebasto commented on pull request "ci: remove 3rd party js from windows dll gha job":
(https://github.com/bitcoin/bitcoin/pull/32513#issuecomment-3300386434)
@m3dwards
Could you please rebase this PR to refresh the CI?
(https://github.com/bitcoin/bitcoin/pull/32513#issuecomment-3300386434)
@m3dwards
Could you please rebase this PR to refresh the CI?
💬 l0rinc commented on pull request "coins: warn on oversized `-dbcache`":
(https://github.com/bitcoin/bitcoin/pull/33333#discussion_r2353687423)
I have explained it in the commit message
> The added test checks if the value is roughly correct by checking if the CI platforms are returning any value and if the value is at least 1 GiB and not more than 1 TiB.
The max value is only validated on 64 bits, since it's not unreasonable for 32 bits to have max memory, but on 64 bits it's likely an error.
Do you think it needs more explanation than that?
(https://github.com/bitcoin/bitcoin/pull/33333#discussion_r2353687423)
I have explained it in the commit message
> The added test checks if the value is roughly correct by checking if the CI platforms are returning any value and if the value is at least 1 GiB and not more than 1 TiB.
The max value is only validated on 64 bits, since it's not unreasonable for 32 bits to have max memory, but on 64 bits it's likely an error.
Do you think it needs more explanation than that?
💬 willcl-ark commented on pull request "ci: Skip Centos job on GHA runners":
(https://github.com/bitcoin/bitcoin/pull/33365#issuecomment-3300404149)
> To be clear, this will start skipping the job in [bitcoin-core/gui](https://github.com/bitcoin-core/gui?rgh-link-date=2025-09-12T09%3A39%3A51Z) ?
That is currently correct, as I understand it.
Though I thought I heard that @m3dwards had been successful in persuading Cirrus to allow us to share our runners with both orgs? In which case it would automagically work there when USE_CIRRUS_RUNNERS was set.
(https://github.com/bitcoin/bitcoin/pull/33365#issuecomment-3300404149)
> To be clear, this will start skipping the job in [bitcoin-core/gui](https://github.com/bitcoin-core/gui?rgh-link-date=2025-09-12T09%3A39%3A51Z) ?
That is currently correct, as I understand it.
Though I thought I heard that @m3dwards had been successful in persuading Cirrus to allow us to share our runners with both orgs? In which case it would automagically work there when USE_CIRRUS_RUNNERS was set.
🤔 enirox001 reviewed a pull request: "test: don't throw from the destructor of DebugLogHelper"
(https://github.com/bitcoin/bitcoin/pull/33388#pullrequestreview-3231754426)
Concept ACK 7f87cdd
(https://github.com/bitcoin/bitcoin/pull/33388#pullrequestreview-3231754426)
Concept ACK 7f87cdd
💬 enirox001 commented on pull request "test: don't throw from the destructor of DebugLogHelper":
(https://github.com/bitcoin/bitcoin/pull/33388#discussion_r2353692055)
The `message` parameter was optimized to `const std::string&`, but `match` still uses pass-by-value. For consistency, shouldn't `match` also be `const MatchFn&`?
(https://github.com/bitcoin/bitcoin/pull/33388#discussion_r2353692055)
The `message` parameter was optimized to `const std::string&`, but `match` still uses pass-by-value. For consistency, shouldn't `match` also be `const MatchFn&`?
💬 enirox001 commented on pull request "test: don't throw from the destructor of DebugLogHelper":
(https://github.com/bitcoin/bitcoin/pull/33388#discussion_r2353697142)
This commit mixes exception safety fixes with parameter optimization. Like @l0rinc mention perhaps consider splitting the parameter changes into a separate commit for cleaner git history and easier debugging.
(https://github.com/bitcoin/bitcoin/pull/33388#discussion_r2353697142)
This commit mixes exception safety fixes with parameter optimization. Like @l0rinc mention perhaps consider splitting the parameter changes into a separate commit for cleaner git history and easier debugging.
💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2353702784)
After reviewing the mempool eviction unit test that was failing in an intermediate commit, I ended up reworking the test altogether, so this should be better now.
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2353702784)
After reviewing the mempool eviction unit test that was failing in an intermediate commit, I ended up reworking the test altogether, so this should be better now.
💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2353704384)
Took this patch, thanks.
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2353704384)
Took this patch, thanks.
💬 l0rinc commented on pull request "test: don't throw from the destructor of DebugLogHelper":
(https://github.com/bitcoin/bitcoin/pull/33388#discussion_r2353708443)
I prefer passing both by value and moving into the members, since most call sites pass temporaries from string literals for the message.
(https://github.com/bitcoin/bitcoin/pull/33388#discussion_r2353708443)
I prefer passing both by value and moving into the members, since most call sites pass temporaries from string literals for the message.