👍 pablomartin4btc approved a pull request: "assumeutxo: Add dumptxoutset height param, remove shell scripts"
(https://github.com/bitcoin/bitcoin/pull/29553#pullrequestreview-2276033026)
> `$ git range-diff master 46c8d75d24d3355ec468f7f608effe94636e16db 94b0adcc371540732453d70309c4083d4bd9cd6b
`

re-ACK 94b0adcc371540732453d70309c4083d4bd9cd6b

💬 Sjors commented on pull request "Have miner account for timewarp mitigation, activate on regtest, lower nPowTargetTimespan to 144 and add test":
(https://github.com/bitcoin/bitcoin/pull/30681#issuecomment-2325165514)
Reviving #17032 seems like a good way to get good test coverage for the timewarp, with and without mitigation.

> we should add tests to regtest that demonstrate that timewarp is not mitigated, which is the case for mainnet

This can still be achieved using the previous releases functionality.

💬 l0rinc commented on pull request "validation: write chainstate to disk every hour":
(https://github.com/bitcoin/bitcoin/pull/30611#discussion_r1741182975)
got it, you can resolve this comment

💬 glozow commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#issuecomment-2325191289)
Rebased for autotools removal, and made the txdownloadman_one_honest_peer fuzzer more interesting. Dishonest peers now have a lot of different transactions/invs they can send that can fail for more reasons.

💬 luke-jr commented on pull request "depends: Qt 5.15.15":
(https://github.com/bitcoin/bitcoin/pull/30774#issuecomment-2325220737)
The [release announcement](https://www.qt.io/blog/commercial-lts-qt-5.15.15-released) says "From now on, the Qt 5.15 patch releases will only be available to subscription license holders."

But the linked git tree still has open source licenses in the root directory.

Anyone know for sure if we can use it or not?

💬 ryanofsky commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-2325264926)
Thanks for the update. It's good to drop the error codes so the C API can correspond 1:1 with the C++ API and not be tied to a more old fashioned and cumbersome error handling paradigm (for callers that want to know which errors are possible and not have to code defensively or fall back to failing generically).

I am still -0 on the approach of introducing a C API to begin with, but happy to help review this and get merged and maintain it if other developers think this is the right approach to
...

💬 hodlinator commented on pull request "test: increase FromUserHex FUZZ and unit testing coverage":
(https://github.com/bitcoin/bitcoin/pull/30618#issuecomment-2325272047)
Maybe my communication was not clear enough. There are 2 separate things that you might be conflating?

1. I like that one can compare `optional<T>` to `T` as per https://en.cppreference.com/w/cpp/utility/optional/operator_cmp (21-33).
That's the `BOOST_CHECK_EQUAL(uint256::FromUserHex(""), uint256::ZERO);`-example.

2. I don't see why we want to prevent being able to do:
`BOOST_CHECK_EQUAL(uint256::FromUserHex("Not valid"), std::nullopt);`
one could do the following as an alt
...

💬 ryanofsky commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-2325272366)
Another idea worth mentioning is that a bitcoin kernel C API could be implemented as a separate C library depending on the C++ library. The new code here does necessarily need to be part of the main bitcoin core git repository, and it could be in a separate project. A benefit of this approach is it could relieve bitcoin core developers from the responsibility of updating the C API and API documention when they change the C++ code. But a drawback is that C API might not always be up to date with
...

💬 tobtoht commented on pull request "depends: Qt 5.15.15":
(https://github.com/bitcoin/bitcoin/pull/30774#issuecomment-2325275364)
@luke-jr Due to an agreement with KDE, Qt is required to license releases under LGPL within one year. I can imagine they do not like reminding paying subscribers of this.

See:

- https://www.qt.io/faq/3.2.-why-do-you-have-an-agreement-with-kde-about-your-licensing-what-kde-is-and-whats-the-history-of-qt-and-kde
- https://kde.org/community/whatiskde/Software_License_Agreement_2015_Text.pdf (Specifically section 3 II)

💬 luke-jr commented on pull request "depends: Qt 5.15.15":
(https://github.com/bitcoin/bitcoin/pull/30774#issuecomment-2325280364)
My understanding is that the agreement relaxes the license on the existing public version (in this context, 5.15.14) in the event that Qt does not release the next version (5.15.15) under the GPL. It is not a guarantee that 5.15.15 will be itself be released under the GPL, and certainly not immediately upon release of 5.15.15.

💬 hebasto commented on pull request "build: Use correct variable name":
(https://github.com/bitcoin/bitcoin/pull/30791#issuecomment-2325285909)
Please review https://github.com/bitcoin-core/secp256k1/pull/1600 first.

💬 fanquake commented on pull request "build: Use correct variable name":
(https://github.com/bitcoin/bitcoin/pull/30791#issuecomment-2325287083)
I guess this is also now a follow up for: https://github.com/hebasto/bitcoin/pull/192#discussion_r1660826044.

💬 hebasto commented on pull request "build: Use correct variable name":
(https://github.com/bitcoin/bitcoin/pull/30791#issuecomment-2325288195)
> I guess this is also now a follow up for: [hebasto#192 (comment)](https://github.com/hebasto/bitcoin/pull/192#discussion_r1660826044).

Indeed.

💬 1440000bytes commented on issue "Testnet4 consensus failure due to timewarp related "softfork"":
(https://github.com/bitcoin/bitcoin/issues/30786#issuecomment-2325291375)
> cc @mempool

I don't think this notified anyone

💬 tobtoht commented on pull request "depends: Qt 5.15.15":
(https://github.com/bitcoin/bitcoin/pull/30774#issuecomment-2325298740)
5.15.15 was released last year on [August 31, 2023](https://wiki.qt.io/Qt_5.15_Release). The git tag for this release is `v5.15.15-lts-lgpl`. It seems to follow the pattern of previous commercial 5.15.x point releases.

💬 andrewtoth commented on pull request "validation: write chainstate to disk every hour":
(https://github.com/bitcoin/bitcoin/pull/30611#discussion_r1741250422)
This seems like just replacing `if (m_next_write == decltype(m_next_write){}) {` with `if (m_next_write == NodeClock::time_point::max()) {`, and since it's max we can just move the `if` block below `fPeriodicWrite` assignment. I don't see how this is an improvement? Doesn't having multiple checks for `fDoFullFlush` make it more complex?

📝 theStack opened a pull request: "qt, build: remove unneeded `Q_IMPORT_PLUGIN` macro calls"
(https://github.com/bitcoin-core/gui/pull/834)
After the recent full removal of Autotools (PR [#30664](https://github.com/bitcoin/bitcoin/pull/30664)), these macros are not needed anymore in the .cpp files according to the TODO in qt's CMakeLists.txt. Tested building on OpenBSD 7.5, where the XCB plugin was still imported according to the debug log:
```
2024-09-02T21:13:27Z Bitcoin Core version v28.99.0-7346b0109208 (release build)
2024-09-02T21:13:27Z Qt 5.15.12 (dynamic), plugin=xcb
2024-09-02T21:13:27Z No static plugins.
2024-09-02T2
...

💬 hebasto commented on pull request "qt, build: remove unneeded `Q_IMPORT_PLUGIN` macro calls":
(https://github.com/bitcoin-core/gui/pull/834#issuecomment-2325309737)
Concept ACK.

💬 l0rinc commented on pull request "validation: write chainstate to disk every hour":
(https://github.com/bitcoin/bitcoin/pull/30611#discussion_r1741253014)
it's not just replacing the if, please apply this patch on top to see what I mean:
```patch
diff --git a/src/validation.cpp b/src/validation.cpp
index 87a144cb99..6fdc070db3 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -2778,13 +2778,6 @@ CoinsCacheSizeState Chainstate::GetCoinsCacheSizeState(
return CoinsCacheSizeState::OK;
}

-NodeClock::time_point CalculateNextWrite(NodeClock::time_point after)
-{
- const auto time{after + DATABASE_WRITE_INTERVAL_MIN};
-
...

💬 hebasto commented on pull request "qt, build: remove unneeded `Q_IMPORT_PLUGIN` macro calls":
(https://github.com/bitcoin-core/gui/pull/834#issuecomment-2325316004)
> Tested building on OpenBSD 7.5, where the XCB plugin was still imported according to the debug log:
>
> ```
> 2024-09-02T21:13:27Z Bitcoin Core version v28.99.0-7346b0109208 (release build)
> 2024-09-02T21:13:27Z Qt 5.15.12 (dynamic), plugin=xcb
> 2024-09-02T21:13:27Z No static plugins.
> 2024-09-02T21:13:27Z Style: fusion / QFusionStyle
> 2024-09-02T21:13:27Z System: OpenBSD 7.5, x86_64-little_endian-lp64
> ```

The log shows "No static plugins."

To verify that static plugins ar
...