📝 fanquake opened a pull request: "build: work around issue with Boost <= 1.80 and Clang >= 18"
(https://github.com/bitcoin/bitcoin/pull/30821)
Our current minimum supported Boost is `1.73.0`. However, when compiling with Boost `1.74.0` (Debian Stable), using Clang `18`, compilation fails with:
```bash
In file included from /usr/include/boost/mpl/integral_c.hpp:32:
/usr/include/boost/mpl/aux_/integral_wrapper.hpp:73:31: error: integer value -1 is outside the valid range of values [0, 3] for the enumeration type 'udt_builtin_mixture_enum' [-Wenum-constexpr-conversion]
73 | typedef AUX_WRAPPER_INST( BOOST_MPL_AUX_STATIC_CAST(AU
...

💬 stickies-v commented on pull request "test: support std::optional in BOOST_CHECK_* and increase FromUserHex fuzz feature coverage":
(https://github.com/bitcoin/bitcoin/pull/30618#discussion_r1745535351)
I agree, we shouldn't overoptimize. My last suggestion was not about performance, just about making the test more maintainable by removing tests we know don't add anything. We seem to disagree, but I'm okay with keeping the duplication if you think it's better, there's no big cost to it. You can mark this as resolved.

💬 fanquake commented on issue "build: Boost 1.74.0 incompatible with Clang 18":
(https://github.com/bitcoin/bitcoin/issues/30751#issuecomment-2331677969)
> The change to Boost Numeric in 1.81.0 may be enough to avoid the issue.

I think this is the case, PR'd a change in #30821.

👍 hebasto approved a pull request: "qt, build: remove unneeded `Q_IMPORT_PLUGIN` macro calls"
(https://github.com/bitcoin-core/gui/pull/834#pullrequestreview-2283058900)
ACK 7346b0109208c67798bdbd451e509048308eff5d.

Guix-built binaries have been tested on:
- Ubuntu 23.10:
```
2024-09-05T13:22:12Z Bitcoin Core version v28.99.0-g7346b0109208c67798bdbd451e509048308eff5d (release build)
2024-09-05T13:22:12Z Qt 5.15.14 (static), plugin=xcb
2024-09-05T13:22:12Z Static plugins:
2024-09-05T13:22:12Z QMinimalIntegrationPlugin, version 331520
2024-09-05T13:22:12Z QXcbIntegrationPlugin, version 331520
2024-09-05T13:22:12Z Style: fusion / QFusionStyle
2024-09-
...

🚀 hebasto merged a pull request: "qt, build: remove unneeded `Q_IMPORT_PLUGIN` macro calls"
(https://github.com/bitcoin-core/gui/pull/834)

💬 l0rinc commented on pull request "Remove unsafe uint256S() and test-only uint160S()":
(https://github.com/bitcoin/bitcoin/pull/30773#discussion_r1745557271)
You know at the implementation, that's why it seems the same.
The test would also pass for a simple re-cast roundtrip:
```C++
auto ArithToUint256_mock(const arith_uint256 &a) { return *reinterpret_cast<const uint256*>(&a); }
auto UintToArith256_mock(const uint256 &a) { return *reinterpret_cast<const arith_uint256*>(&a); }
BOOST_AUTO_TEST_CASE(conversion)
{
for (const arith_uint256& arith : {ZeroL, OneL, R1L, R2L}) {
const auto u256{uint256::FromHex(arith.GetHex()).value()};

...

💬 l0rinc commented on pull request "Remove unsafe uint256S() and test-only uint160S()":
(https://github.com/bitcoin/bitcoin/pull/30773#discussion_r1745564600)
> so I feel like since the review is happening anyway the test improvements I've introduced here make sense and add little overhead?

Agree, let's leave the campground cleaner than we found it.

💬 TheCharlatan commented on pull request "build: work around issue with Boost <= 1.80 and Clang >= 18":
(https://github.com/bitcoin/bitcoin/pull/30821#discussion_r1745569113)
Should this be in a `#if defined(__clang__)` block?

💬 l0rinc commented on pull request "test: support std::optional in BOOST_CHECK_* and increase FromUserHex fuzz feature coverage":
(https://github.com/bitcoin/bitcoin/pull/30618#discussion_r1745569348)
> We seem to disagree

We don't have to, if the above discussion didn't change your mind I'll gladly remove them.

💬 fanquake commented on pull request "build: work around issue with Boost <= 1.80 and Clang >= 18":
(https://github.com/bitcoin/bitcoin/pull/30821#discussion_r1745570403)
A bit annoying, but yea, looks like we'll need something like that to work around the windows compiler in any case.

💬 fanquake commented on pull request "build: work around issue with Boost <= 1.80 and Clang >= 18":
(https://github.com/bitcoin/bitcoin/pull/30821#discussion_r1745578011)
(depending on what GCC does, over the lifetime of the 28.x branch, at some point, it may turn in and #ifndef MSVC etc)

👍 stickies-v approved a pull request: "[28.x] rc backports"
(https://github.com/bitcoin/bitcoin/pull/30762#pullrequestreview-2283131161)
ACK b2a137929a20baed161988e24de592b1f59c0096

Verified that all backports are clean. Didn't review changes in detail but they seem sensible.

💬 l0rinc commented on pull request "test: support std::optional in BOOST_CHECK_* and increase FromUserHex fuzz feature coverage":
(https://github.com/bitcoin/bitcoin/pull/30618#discussion_r1745582376)
Done.
Also applied it to the `assumevalid` example:
```C++
BOOST_CHECK_EQUAL(get_valid_opts({"-assumevalid=0x12"}).assumed_valid_block, uint256{0x12});
```

💬 l0rinc commented on pull request "test: support std::optional in BOOST_CHECK_* and increase FromUserHex fuzz feature coverage":
(https://github.com/bitcoin/bitcoin/pull/30618#discussion_r1745582878)
Done

🚀 fanquake merged a pull request: "lint: Check for release note snippets in the wrong folder"
(https://github.com/bitcoin/bitcoin/pull/30812)

💬 stickies-v commented on pull request "Remove unsafe uint256S() and test-only uint160S()":
(https://github.com/bitcoin/bitcoin/pull/30773#discussion_r1745612258)
> You already know at the implementation

At which points are these test relying on implementation?

Thanks for providing an example. If you have a diff that shows where the "hardcoded" tests catch something that "my" tests don't, that would really help. I struggle to see how `BOOST_CHECK_EQUAL(ArithToUint256(ZeroL), uint256::ZERO);` is any different from `BOOST_CHECK_EQUAL(ArithToUint256(arith_uint256{0}), uint256{0});` (or how that substitution is relying on implementation details, which y
...

💬 stickies-v commented on pull request "test: support std::optional in BOOST_CHECK_* and increase FromUserHex fuzz feature coverage":
(https://github.com/bitcoin/bitcoin/pull/30618#discussion_r1745635476)
I'm hesitant to add more to the back-and-forth, but I think we may have been talking past each other. In my [latest comment](https://github.com/bitcoin/bitcoin/pull/30618#discussion_r1744124233) I suggested updating the `FromHex()` tests, where based on the [latest force-push](https://github.com/bitcoin/bitcoin/compare/6fa4c92e4c9bf5ea90e25d41b525c88f9c2d834d..2cee181a6cc00840def16c311aae69883130eebd) it seems you were talking about the `FromUserHex()` tests, which I agreed I was okay with based
...

💬 maflcko commented on pull request "util: Use consteval checked format string in FatalErrorf":
(https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1745637880)
In the future more (constant) errors will be added, so I think checking what type of error was thrown is useful and doesn't seem harmful either way in the tests?

✅ achow101 closed an issue: "Testnet4 consensus failure due to timewarp related "softfork""
(https://github.com/bitcoin/bitcoin/issues/30786)

💬 achow101 commented on issue "Testnet4 consensus failure due to timewarp related "softfork"":
(https://github.com/bitcoin/bitcoin/issues/30786#issuecomment-2331823260)
My nodes' logs indicate this was resolved at 2024-09-05 02:06:40 UTC when the new chain reorg'd the timewarped one.

💬 ryanofsky commented on pull request "Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange, drop CRPCSignals & g_best_block":
(https://github.com/bitcoin/bitcoin/pull/30409#issuecomment-2331826005)
> > the change in logic is a bit tricky to follow.
>
> Any particular commit that's problematic? I could try splitting it.

I think the PR is good in it's current form, but if I wanted to be more confident it wasn't changing behavior I would not change code from using `g_best_block` and `::latestblock` directly to calling `waitTipChanged()`. Instead, I would first remove `g_best_block` and replace it with `KernelNotifications::m_tip_block`, then I would remove `::latestblock` and replace it
...