💬 glozow commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#issuecomment-2325113562)
reACK 100e1b9ecb2 via range-diff

⚠️ fanquake opened an issue: "ci: failure in win64 unit tests"
(https://github.com/bitcoin/bitcoin/issues/30792)
Seen in https://cirrus-ci.com/task/6387064294342656?logs=ci#L2492:
```bash
127/135 Test #129: spend_tests .......................... Passed 9.85 sec
128/135 Test #135: db_tests ............................. Passed 3.32 sec
129/135 Test #1: util_test_runner .....................***Failed 154.46 sec
2024-09-02 15:49:05,155 - ERROR - Error parsing command output as hex: non-hexadecimal number found in fromhex() arg at position 1
2024-09-02 15:49:30,298 - ERROR - FAILED_TESTCASES:
...

✅ fjahr closed an issue: "Embedded ASMap data Tracking Issue"
(https://github.com/bitcoin/bitcoin/issues/28794)

💬 fjahr commented on issue "Embedded ASMap data Tracking Issue":
(https://github.com/bitcoin/bitcoin/issues/28794#issuecomment-2325120705)
This issue hasn't seen much love but that's because there isn't much else to do by now in my mind, aside from #28792 and also most of the interesting action on ASMapt takes place outside of this repo, like in https://github.com/fjahr/asmap-data/ and https://github.com/fjahr/kartograf. So I am going to close this for now, the focus is on #28792 which should hopefully let us take the next step in adoption of this feature.

If we end up needing to put more work into ASMap than expected, I am happ
...

💬 pablomartin4btc commented on pull request "assumeutxo: Add dumptxoutset height param, remove shell scripts":
(https://github.com/bitcoin/bitcoin/pull/29553#discussion_r1741151732)
I do agree unless there's another use case that invalidates it.

💬 MarcusMWilliams commented on pull request "assumeutxo: Add dumptxoutset height param, remove shell scripts":
(https://github.com/bitcoin/bitcoin/pull/29553#issuecomment-2325156600)
1-225-316-0850

On Mon, Sep 2, 2024, 12:47 PM pablomartin4btc ***@***.***>
wrote:

> ***@***.**** commented on this pull request.
> ------------------------------
>
> In src/rpc/blockchain.cpp
> <https://github.com/bitcoin/bitcoin/pull/29553#discussion_r1741151732>:
>
> > + // data is available before starting to roll back.
> + if (node.chainman->m_blockman.IsPruneMode()) {
> + LOCK(node.chainman->GetMutex());
> + const CBlockIndex* current_tip{n
...

💬 pablomartin4btc commented on pull request "assumeutxo: Add dumptxoutset height param, remove shell scripts":
(https://github.com/bitcoin/bitcoin/pull/29553#issuecomment-2325158560)
> > As @Sjors mentioned above please consider adding -rpcclienttimeout=0 to the calls also in the documentation. Furthermore, these example outputs could be useful there too.
>
> Can you say which examples you mean? I added it to the RPC help examples in my last push but I guess you might be referring to the README examples where I didn't do it yet. Or is there another place I am still missing?

Thanks! I've checked the places where you added the `-rpcclienttimeout=0`, also it could be adde
...

👍 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