💬 davidgumberg commented on pull request "ci: remove 3rd party js from windows dll gha job":
(https://github.com/bitcoin/bitcoin/pull/32513#discussion_r2102887073)
I think this approach is no less invasive then what is done currently using an external script, setting up an environment that behaves the same as the VS Powershell utility. The advantage of this approach is that it makes use of Visual Studio utilities and commands zero maintenance cost, manually setting variables voids that, and we are subject to being broken by MS upstream, or wrestling with strange misconfigurations when some command that works on a local VS powershell environment is broken o
...

💬 hebasto commented on pull request "ci: remove 3rd party js from windows dll gha job":
(https://github.com/bitcoin/bitcoin/pull/32513#discussion_r2102902795)
> The advantage of this approach is that it makes use of Visual Studio utilities and commands zero maintenance cost, manually setting variables voids that, and we are subject to being broken by MS upstream, or wrestling with strange misconfigurations when some command that works on a local VS powershell environment is broken on CI because some environment variable is not set.

That sounds compelling.

💬 romanz commented on pull request "rest: fetch spent transaction outputs by blockhash":
(https://github.com/bitcoin/bitcoin/pull/32540#issuecomment-2901770608)
> Not sure we should export a custom binary format for undo data like this.

In my opinion, using a binary format allows us to achieve significantly better performance when building an external index (compared to using `/rest/block/HASH.json`).

I have followed the example of the `/rest/getutxos/` endpoint, which also uses binary encoding when it returns a `std::vector<CCoin> outs`:
https://github.com/bitcoin/bitcoin/blob/d2c9fc84e17120f186a54ef92bab76ea7e8d31b5/src/rest.cpp#L906

💬 romanz commented on pull request "rest: fetch spent transaction outputs by blockhash":
(https://github.com/bitcoin/bitcoin/pull/32540#issuecomment-2901770885)
> At least there should be a JSON option?

Sounds good, added in https://github.com/bitcoin/bitcoin/pull/32540/commits/9d7e23e2f505ce6cbe830fc607cf203b1a48ba0d.

🤔 jonatack reviewed a pull request: "Replace cluster linearization algorithm with SFL"
(https://github.com/bitcoin/bitcoin/pull/32545#pullrequestreview-2861830018)
Concept ACK

💬 jonatack commented on pull request "Replace cluster linearization algorithm with SFL":
(https://github.com/bitcoin/bitcoin/pull/32545#discussion_r2102935031)
Appreciate the excellent doxygen documentation here.

💬 hebasto commented on pull request "ci, iwyu: Treat warnings as errors for specific targets":
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2102983324)
> I think the message you posted in the PR description only shows a single error in a single dependency. I am wondering what happens if there is more than one error in different targets, possibly ones where cmake will never get to print the second error.

[Here](https://api.cirrus-ci.com/v1/task/5614699137466368/logs/ci.log) multiple error are reported.

👋 hebasto's pull request is ready for review: "ci, iwyu: Treat warnings as errors for specific targets"
(https://github.com/bitcoin/bitcoin/pull/31308)

💬 hebasto commented on pull request "ci, iwyu: Treat warnings as errors for specific targets":
(https://github.com/bitcoin/bitcoin/pull/31308#issuecomment-2901917485)
Rebased.

🚀 fanquake merged a pull request: "checkqueue: make the queue non-optional for CCheckQueueControl and drop legacy locking macro usage"
(https://github.com/bitcoin/bitcoin/pull/32467)

👍 fanquake approved a pull request: "ci: Downgrade DEBUG=1 to -D_GLIBCXX_ASSERTIONS in centos task"
(https://github.com/bitcoin/bitcoin/pull/32586#pullrequestreview-2861961840)
ACK fa079538e32d20aec6786c93e1117da1e8ea0cab - we can followup

✅ fanquake closed an issue: "intermittent issue in rpc_signer.py (enumeratesigners timeout) under GLIBCXX debug mode"
(https://github.com/bitcoin/bitcoin/issues/32524)

🚀 fanquake merged a pull request: "ci: Downgrade DEBUG=1 to -D_GLIBCXX_ASSERTIONS in centos task"
(https://github.com/bitcoin/bitcoin/pull/32586)

👋 romanz's pull request is ready for review: "rest: fetch spent transaction outputs by blockhash"
(https://github.com/bitcoin/bitcoin/pull/32540)

💬 fanquake commented on pull request "log: Additional compact block logging":
(https://github.com/bitcoin/bitcoin/pull/32582#issuecomment-2901996108)
cc @instagibbs

💬 jonasschnelli commented on issue "seeds: seed.bitcoin.jonasschnelli.ch not returning results":
(https://github.com/bitcoin/bitcoin/issues/32590#issuecomment-2902032188)
Fixed.

💬 pinheadmz commented on pull request "tests: Expand HTTP coverage to assert libevent behavior":
(https://github.com/bitcoin/bitcoin/pull/32408#discussion_r2103080376)
The server timer *should* start after receiving the last packet from the client, because its an idle timer not a session timer. So its more correct to have:

> send, server timer starts immediately after (B)

I tried to to nail this in https://github.com/bitcoin/bitcoin/commit/ce9847e284a361fb050f366848cdf1a38b2b933b but that also failed.

And then, if we start the timer too early, we will always get a duration > 1 no matter what the server actually does... maybe its the initial TCP connec
...

🤔 instagibbs reviewed a pull request: "log: Additional compact block logging"
(https://github.com/bitcoin/bitcoin/pull/32582#pullrequestreview-2862076315)
concept ACK

seem like sensible things to be reporting

💬 instagibbs commented on pull request "log: Additional compact block logging":
(https://github.com/bitcoin/bitcoin/pull/32582#discussion_r2103091393)
seems odd to add it then modify it immediately next commit, can be one commit?

💬 instagibbs commented on pull request "log: Additional compact block logging":
(https://github.com/bitcoin/bitcoin/pull/32582#discussion_r2103089215)
to make it more explicit it's not a tx count thing
```Suggestion
LogDebug(BCLog::CMPCTBLOCK, "Initializing PartiallyDownloadedBlock for block %s using a cmpctblock of %lu bytes\n", cmpctblock.header.GetHash().ToString(), GetSerializeSize(cmpctblock));
```