💬 hebasto commented on pull request "subprocess: Handle quoted tokens properly":
(https://github.com/bitcoin/bitcoin/pull/32577#discussion_r2102807235)
No longer relevant.

💬 fanquake commented on pull request "subprocess: Handle quoted tokens properly":
(https://github.com/bitcoin/bitcoin/pull/32577#discussion_r2102813675)
Why is this test being scoped to only Windows?

💬 i-am-yuvi commented on pull request "BIP-348 (OP_CHECKSIGFROMSTACK) (regtest only)":
(https://github.com/bitcoin/bitcoin/pull/32247#issuecomment-2901612231)
Concept ACK

📝 romanz converted_to_draft a pull request: "rest: fetch spent transaction outputs by blockhash"
(https://github.com/bitcoin/bitcoin/pull/32540)
Today, it is possible to fetch a block's spent prevouts in order to build an external index by using the `/rest/block/BLOCKHASH.json` endpoint. However, its performance is low due to JSON serialization overhead.

We can significantly optimize it by adding a new REST endpoint, using a binary response format:

```
$ BLOCKHASH=00000000000000000002a7c4c1e48d76c5a37902165a270156b7a8d72728a054

$ ab -k -c 1 -n 100 http://localhost:8332/rest/block/$BLOCKHASH.json
Document Length: 1327815
...

💬 danielabrozzoni commented on pull request "Drop log category in `SeedStartup`":
(https://github.com/bitcoin/bitcoin/pull/29480#issuecomment-2901629864)
Hey! I found this PR while looking at the up for grabs, but I noticed that #30750 replaced LogPrint with LogDebug, and currently on master the log line is printed. As far as I understand, this PR shouldn't be needed anymore, and the "Up for grabs" label can be removed :)

```
❯ ./build/bin/bitcoind -debug=rand
2025-05-22T15:16:52Z [rand] Feeding 133269 bytes of environment data into RNG
```

💬 hebasto commented on pull request "subprocess: Handle quoted tokens properly":
(https://github.com/bitcoin/bitcoin/pull/32577#discussion_r2102823739)
On other platforms, `execl()` might fail only if there is problem with `/bin/sh`. I can't see any trivial way to adjust the test to trigger a failure. Any suggestions?

💬 romanz commented on pull request "rest: fetch spent transaction outputs by blockhash":
(https://github.com/bitcoin/bitcoin/pull/32540#discussion_r2102866377)
Thanks, fixed in https://github.com/bitcoin/bitcoin/pull/32540/commits/9d7e23e2f505ce6cbe830fc607cf203b1a48ba0d.

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