💬 l0rinc commented on pull request "refactor: Generalize `CScript`'s `std::vector` push to `std::span` to accept `std::array`, too":
(https://github.com/bitcoin/bitcoin/pull/30765#discussion_r1746943429)
> But I do think it would be nice if this PR just added support for std::span and std::byte without necessarily making bigger changes.

I've added support for `const std::span<const unsigned char>` (for vector & array), but `std::byte` seems like a bigger change (as explained now in the PR's description) - unless you'd like me to add it as an extra overload - let me know if that's what you meant.

💬 fanquake commented on pull request "build: work around issue with Boost <= 1.80 and Clang >= 18":
(https://github.com/bitcoin/bitcoin/pull/30821#issuecomment-2333821161)
I had hoped we'd be able to land this workaround for `28.x` (to atleast avoid compile failures, without any other changes to other dependency or compiler requirements), and then remove it in master fairly soon, by following up with either a wholesale bump to our minimum required Boost, to 1.81.0+, or I guess a clang-18+ restriction to using Boost 1.81.0+.

I re-tested master (including this change) using Boost 1.74.0:
* clang-18 builds with no output
* clang-19 builds with no output (unrelat
...

💬 sipa commented on issue "28.0rc1 synchronizes much slower on Windows":
(https://github.com/bitcoin/bitcoin/issues/30833#issuecomment-2333833557)
Try whether #28280 had any impact.

💬 hebasto commented on pull request "ci: Add missed configuration options to "Win64 native" job":
(https://github.com/bitcoin/bitcoin/pull/30755#discussion_r1746957633)
> > In suggestions about the place for such a note.
>
> I guess somewhere in the Windows build docs?

Thanks! Done.

> Seems like we could just add the relevant options from the preset to the ci config file for now

Implemented.

💬 vostrnad commented on issue "28.0rc1 synchronizes much slower on Windows":
(https://github.com/bitcoin/bitcoin/issues/30833#issuecomment-2333846869)
I actually benchmarked #28280 right before it got merged: https://github.com/bitcoin/bitcoin/pull/28280#issuecomment-2268695679

From the results it definitely seems that PR is not to blame and the regression happened later.

👍 fanquake approved a pull request: "ci: Add missed configuration options to "Win64 native" job"
(https://github.com/bitcoin/bitcoin/pull/30755#pullrequestreview-2286047596)
ACK ee22bf55e39e4a40c31465e8ef2663eb7fefc783

💬 andrewtoth commented on issue "28.0rc1 synchronizes much slower on Windows":
(https://github.com/bitcoin/bitcoin/issues/30833#issuecomment-2333868669)
https://github.com/bitcoin/bitcoin/pull/30326?

💬 ismaelsadeeq commented on pull request "init: fix init fatal error on invalid negated option value":
(https://github.com/bitcoin/bitcoin/pull/30684#discussion_r1746980280)
adding negatives value to the values makes this test to fail.
should we also prevent passing negative integers?

💬 hebasto commented on pull request "build: Fix linking for `fuzz` target when building with MSan":
(https://github.com/bitcoin/bitcoin/pull/30778#discussion_r1746980931)
> * I'm probably missing something, but maybe a cleaner workaround instead of setting `Debug` build type and then erasing all the debug flags is just to set a custom build type -DCMAKE_BUILD_TYPE=msan that doesn't have the flags to begin with?

I agree with you, and that was my initial preference when I worked on the CMake staging branch. However, using undefined build configurations is not documented. As a result, the rough consensus among the CMake staging branch contributors was to adher
...

💬 hebasto commented on issue "cmake: passing options from depends build to main build":
(https://github.com/bitcoin/bitcoin/issues/30813#issuecomment-2333907613)
> Right now looking at `depends/toolchain.cmake.in` it seems like both of these things can happen in some cases, but neither of them happen reliably. Specifically in the current system:
>
> * The toolchain file force-writes into the cache and overwrites user settings.
>
> ...
>
> I think to implement desired behavior, `toolchain.cmake.in` could be changed to:
>
> 1. Stop setting cache variables like `BUILD_GUI` and `WITH_QRENCODE` and instead set custom non-cache variables like `
...

💬 maflcko commented on pull request "ci: Add missed configuration options to "Win64 native" job":
(https://github.com/bitcoin/bitcoin/pull/30755#issuecomment-2333973232)
review-only ACK ee22bf55e39e4a40c31465e8ef2663eb7fefc783

💬 maflcko commented on pull request "build: work around issue with Boost <= 1.80 and Clang >= 18":
(https://github.com/bitcoin/bitcoin/pull/30821#issuecomment-2333991068)
> clang 20 produces warning output for the pragma, and then fails to compile

Are you sure? Compilation did pass for me this morning. Do you still happen to have the compile failure or steps to reproduce?

💬 maflcko commented on pull request "refactor: Generalize `CScript`'s `std::vector` push to accept `std::array`, too":
(https://github.com/bitcoin/bitcoin/pull/30765#discussion_r1747087717)
An overload for `std::byte` could be one more line of code, but would avoid having to touch all the touched non-header lines in this pull request again. Unless I am missing something obvious, there shouldn't be any downside.

💬 maflcko commented on pull request "util: Use consteval checked format string in FatalErrorf":
(https://github.com/bitcoin/bitcoin/pull/30546#issuecomment-2334000135)
Force pushed to include all follow-ups from my future commits and to allow positional args. The implementation should now be final and there hopefully won't be any need to touch it in a future PR again. I've updated the PR description to list the two additional bugs in the python code.

🚀 fanquake merged a pull request: "ci: Add missed configuration options to "Win64 native" job"
(https://github.com/bitcoin/bitcoin/pull/30755)

💬 ryanofsky commented on pull request "init: fix init fatal error on invalid negated option value":
(https://github.com/bitcoin/bitcoin/pull/30684#discussion_r1747106280)
> adding negative value to the values makes this test to fail. should we also prevent passing negative integers?

`-nowallet=-1` should be treated the same as `-nowallet=1` which is the same as `-nowallet` which just clears the lists of wallets to load and is not expected to be an error.

This test is testing what happens when `-nowallet=0` is passed. Previously before https://github.com/bitcoin/bitcoin/pull/22217 `-nowallet=0` was the treated the same as `-wallet=1`, and would cause a walle
...

💬 glozow commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#issuecomment-2334033449)
Examples 16-19 seem to get slower?
My results on the last commit look similar to sipa's:

| ns/op | op/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 17,187,614.00 | 58.18 | 4.0% | 0.19 | `LinearizeOptimallyExample00`
| 96,365.70 | 10,377.14 | 0.2% | 0.01 | `LinearizeOptimallyExample01`
| 87,832.50 | 11,385.31 | 3
...

💬 fanquake commented on pull request "build: work around issue with Boost <= 1.80 and Clang >= 18":
(https://github.com/bitcoin/bitcoin/pull/30821#issuecomment-2334037507)
> Do you still happen to have the compile failure or steps to reproduce?

Using this Clang:
```bash
clang++-20 --version
Debian clang version 20.0.0 (++20240903115437+0748f4227cd6-1~exp1~20240903115620.935)
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/lib/llvm-20/bin
```
and master (bbf95c0cc57147827b9f4577c641b12dd4170e78). I see:
```bash
cmake -B build -DCMAKE_C_COMPILER=clang-20 -DCMAKE_CXX_COMPILER=clang++-20
cmake --build build --verbose
<snip>
[ 72%] Bu
...

💬 sipa commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#issuecomment-2334042896)
Thanks for the graphs, @glozow. That confirms the impression we got: 16-19 seem to just be really hard, and the later optimizations just don't help with it (and increase per-iteration cost, for no meaningful iteration reduction).

It doesn't surprise me that this is possible (they are after all very far below the conjectured upper bound of $\mathcal{O}(\sqrt{2^N})$, but it does surprise me that such clusters are seen in historical real-world mempool data.

Perhaps it's worth investigating (in a
...

💬 fanquake commented on pull request "interfaces: #30697 follow ups":
(https://github.com/bitcoin/bitcoin/pull/30828#issuecomment-2334046532)
cc @stickies-v @ryanofsky