💬 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));
```

💬 instagibbs commented on pull request "log: Additional compact block logging":
(https://github.com/bitcoin/bitcoin/pull/32582#discussion_r2103092771)
we don't know they were missing them really, just mention they asked for them for the specific block?

🤔 ryanofsky reviewed a pull request: "bitcoin-cli: Add -ipcconnect option"
(https://github.com/bitcoin/bitcoin/pull/32297#pullrequestreview-2859167293)
Great review! I improved a number of things in response to your comments.

Rebased 9ff5bc75659e0314f42c1477fc0c37ac0ad132fa -> 6bf727fb88009f99dab7c06bff7f8fb391147ac1 ([`pr/ipc-cli.5`](https://github.com/ryanofsky/bitcoin/commits/pr/ipc-cli.5) -> [`pr/ipc-cli.6`](https://github.com/ryanofsky/bitcoin/commits/pr/ipc-cli.6), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/ipc-cli.5-rebase..pr/ipc-cli.6)) due to conflict with #28710, also made a number of improvements to -ipcbind docume
...