💬 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.

💬 fanquake commented on pull request "test: Work around boost compilation error":
(https://github.com/bitcoin/bitcoin/pull/30834#issuecomment-2334129103)
It looks like this works using clang 20 (`Debian clang version 20.0.0 (++20240903115437+0748f4227cd6-1~exp1~20240903115620.935)`) & Boost 1.74.0.

👋 maflcko's pull request is ready for review: "test: Work around boost compilation error"
(https://github.com/bitcoin/bitcoin/pull/30834)

💬 maflcko commented on pull request "test: Work around boost compilation error":
(https://github.com/bitcoin/bitcoin/pull/30834#issuecomment-2334133682)
Thanks for testing. It would be good to also check with Clang 18 (and any other known-to-fail combination).

💬 fanquake commented on pull request "test: Work around boost compilation error":
(https://github.com/bitcoin/bitcoin/pull/30834#issuecomment-2334134054)
Note also, I somehow discovered only now that there is a separate Boost 1.81.0 package available for Bookworm, and the same package is in the backports for Bullseye. So we can also direct any users to those packages.

💬 fanquake commented on pull request "[28.x] Further backports and rc2":
(https://github.com/bitcoin/bitcoin/pull/30827#discussion_r1747184890)
This can now be swapped out for #30834.

💬 sipa commented on issue "28.0rc1 synchronizes much slower on Windows":
(https://github.com/bitcoin/bitcoin/issues/30833#issuecomment-2334157710)
@andrewtoth I cannot imagine that causing a 5x slowdown, though?

💬 hebasto commented on pull request "test: Work around boost compilation error":
(https://github.com/bitcoin/bitcoin/pull/30834#issuecomment-2334163228)
> It would be good to also check with Clang 18 (and any other known-to-fail combination).

Tested on Fedora 40 using the downloaded [Boost 1.74](https://archives.boost.io/release/1.74.0/source/) and commands as follows:
```
$ cmake -B build -DCMAKE_C_COMPILER=clang-18 -DCMAKE_CXX_COMPILER=clang++-18 -DBoost_INCLUDE_DIR=/home/hebasto/boost_1_74_0
$ cmake --build build -t test_bitcoin
```

No build errors.

👍 hebasto approved a pull request: "test: Work around boost compilation error"
(https://github.com/bitcoin/bitcoin/pull/30834#pullrequestreview-2286400951)
ACK fa9d7d5d205ada8915cbbc29599ab8e7bf1fffe0.

💬 jamesob commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#issuecomment-2334181480)
I think this is ready for "actual" review, in case anyone was wondering.

💬 instagibbs commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#issuecomment-2334213839)
Confirmed that at least for example 19, it simply can't find the optimal solution until the commit it's included, even if total iterations are cranked up a couple orders of magnitude.

💬 sipa commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#issuecomment-2334217170)
Example cluster 18: