💬 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

💬 maflcko commented on pull request "build: work around issue with Boost <= 1.80 and Clang >= 18":
(https://github.com/bitcoin/bitcoin/pull/30821#issuecomment-2334046805)
Ok, I may have only compiled `bitcoind`. If the error is only in the tests, I wonder if the tests can simply be rewritten. I'll take another look later.

💬 ryanofsky commented on pull request "refactor: Generalize `CScript`'s `std::vector` push to accept `std::array`, too":
(https://github.com/bitcoin/bitcoin/pull/30765#discussion_r1747126073)
> 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.

Yeah sorry for any confusion. My suggestion in https://github.com/bitcoin/bitcoin/pull/30765#discussion_r1741079145 here was to use `std::byte` if it was easy or `BasicByte` as a fallback if would be too much work to convert other code to use std::byte. Having two overloads is also fine.

💬 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_r1747142070)
> `BasicByte`

IIRC the reason that serialize code uses a template on the inner type of the span is that call sites have to explicitly specify that they want the "different" span-serialization.

I think the concern doesn't apply to CScript, so a template may be better to avoid, but I haven't tried this myself.

💬 andrewtoth commented on issue "28.0rc1 synchronizes much slower on Windows":
(https://github.com/bitcoin/bitcoin/issues/30833#issuecomment-2334078040)
Reason I suspect #30326 is because `try_emplace` must be doing more work than just `find` and is only called after blocks stop being full because previous coins are now being looked up to be spent.

Relevant stackoverflow https://stackoverflow.com/questions/24209592/performance-of-emplace-is-worse-than-check-followed-by-emplace

💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#issuecomment-2334086913)
CI failures seem like spurious timeouts of random tests

💬 sdaftuar commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#issuecomment-2334102610)
> @sdaftuar Is it possible that some of these clusters are "related" (as in are really modifications of one another with a few transactions added/removed)? That could explain the strong correlations between consecutive ones.

I do believe those last 4 examples are likely to be related, as all were found within minutes of each other on the same day.

💬 ryanofsky commented on pull request "interfaces: #30697 follow ups":
(https://github.com/bitcoin/bitcoin/pull/30828#discussion_r1747172543)
In commit "chain: disallow overwriting settings with null values" (4627174d80838e1c6848f9dff47a2ca4a9a35091)

Would be good to just delete this line entirely and make overwriteRwSetting just call updateRwSetting without making a special case for null. Setting a null setting should just delete a setting consistently and not have different behavior for different functions.

(Sorry I didn't notice this line before and mistakenly thought overwriteRwSetting would allow writing null values to the
...

📝 maflcko opened a pull request: " test: Work around boost compilation error "
(https://github.com/bitcoin/bitcoin/pull/30834)

💬 Sjors commented on pull request "test: add mocked Sock that can read/write custom data and/or CNetMessages":
(https://github.com/bitcoin/bitcoin/pull/30205#issuecomment-2334121626)
re-ACK e7cf8e8fc658e4559fcc64279c7bd854773ad6c2

This makes my dimmed zebra very happy.

💬 maflcko commented on pull request "build: work around issue with Boost <= 1.80 and Clang >= 18":
(https://github.com/bitcoin/bitcoin/pull/30821#issuecomment-2334122738)
I still haven't tested this, but the draft at https://github.com/bitcoin/bitcoin/pull/30834 may or may not work around the test compilation error.