💬 stickies-v commented on pull request "[27.x] More backports":
(https://github.com/bitcoin/bitcoin/pull/30467#issuecomment-2245478885)
ACK 4f23c8636498f7e5adbe0264a0dc66a566c4b1b9

👍 hebasto approved a pull request: "depends: build expat with CMake"
(https://github.com/bitcoin/bitcoin/pull/29878#pullrequestreview-2194240049)
re-ACK a517029646ac86f9d72fcea204ff45db41702e37.

💬 fanquake commented on pull request "guix: use GCC 13 to builds releases":
(https://github.com/bitcoin/bitcoin/pull/29881#discussion_r1688215889)
> or do you need https://packages.debian.org/trixie/mingw-w64-x86-64-dev ?

Yea at some point (#30210), we'll need the newer version, I'll switch to Noble here, and see if things are still broken.

💬 paplorinc commented on pull request "rest: Reject truncated hex txid early in getutxos parsing":
(https://github.com/bitcoin/bitcoin/pull/30482#discussion_r1688210420)
is this exactly the same as before?

💬 paplorinc commented on pull request "rest: Reject truncated hex txid early in getutxos parsing":
(https://github.com/bitcoin/bitcoin/pull/30482#discussion_r1688214916)
not sure I get the SetHexFragile/FromHex naming duality here :/

💬 paplorinc commented on pull request "rest: Reject truncated hex txid early in getutxos parsing":
(https://github.com/bitcoin/bitcoin/pull/30482#discussion_r1688212501)
hmmm, what's the point of the Assert inside?

💬 paplorinc commented on pull request "rest: Reject truncated hex txid early in getutxos parsing":
(https://github.com/bitcoin/bitcoin/pull/30482#discussion_r1688216506)
after we get rid of the deprecation, these checks should ideally be done while iterating over the values to avoid multiple iterations (e.g. how Base58 parsing is done)

💬 ryanofsky commented on pull request "multiprocess: add bitcoin-mine test program":
(https://github.com/bitcoin/bitcoin/pull/30437#issuecomment-2245485761)
re: https://github.com/bitcoin/bitcoin/pull/30437#issuecomment-2242645460

> We should then aim to get everything merged in the PR, maybe with the exception of `multiprocess: Add bitcoin-mine test program`.

To separate essential and nonessential parts of this PR, I created #30509 and #30510 which I think contain all the needed functionality from the PR, and I also broke the changes down and wrote tests for them. I will rebase this PR on top of those two.

💬 Sjors commented on pull request "multiprocess: Add -ipcbind option to bitcoin-node":
(https://github.com/bitcoin/bitcoin/pull/30509#issuecomment-2245502110)
One thing that may be useful to clarify, IIUC, although you can connect, nothing can be done via these connections yet. That is what #30510 introduces specifically for the Mining interface.

👍 willcl-ark approved a pull request: "[27.x] More backports"
(https://github.com/bitcoin/bitcoin/pull/30467#pullrequestreview-2194315990)
ACK 4f23c8636498f7e5adbe0264a0dc66a566c4b1b9

Checked all commits were backported cleanly and had a release note for their original PR.

I agree with stickies that carving out the relevant portion of the test for https://github.com/bitcoin/bitcoin/commit/ab422066527992d53c92bb482c6b993f089b2999 makes best sense for a backport.

🤔 instagibbs reviewed a pull request: "[WIP] Cluster mempool implementation"
(https://github.com/bitcoin/bitcoin/pull/28676#pullrequestreview-2192522762)
(after fixing the reported iterations done) Seeing a lot of linerarizations being done that are kind of odd looking repeats with same number of iterations?
```
2024-07-22T21:01:29.226080Z [bench] InvokeSort linearize cluster: 15 txs, 0.0184ms, 120 iter, 153.1ns/iter
2024-07-22T21:01:29.226348Z [bench] InvokeSort linearize cluster: 15 txs, 0.0187ms, 120 iter, 155.7ns/iter
2024-07-22T21:01:29.226633Z [bench] InvokeSort linearize cluster: 15 txs, 0.0180ms, 120 iter, 149.8ns/iter
2024-07-22T21:
...

💬 instagibbs commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1687121805)
`iterations` is not modified here, so you're just reporting 0 iters used everywhere, and always PostLinearizing

Do we want to change the `Linearize` signature to report iters left, which can infer optimal or not as well?

💬 ryanofsky commented on pull request "multiprocess: Add -ipcbind option to bitcoin-node":
(https://github.com/bitcoin/bitcoin/pull/30509#issuecomment-2245563392)
> One thing that may be useful to clarify, IIUC, although you can connect, nothing can be done via these connections yet. That is what #30510 introduces specifically for the Mining interface.

Thank you, added this to the description

💬 maflcko commented on pull request "fix: Make TxidFromString() respect string_view length":
(https://github.com/bitcoin/bitcoin/pull/30436#issuecomment-2245579974)
re-ACK 09ce3501fa2ea2885a857e380eddb74605f7038c 🕓

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: re-ACK 09ce3501fa2ea2885a85
...

💬 Sjors commented on pull request "multiprocess: Add IPC wrapper for Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30510#issuecomment-2245586329)
Is the idea to incorporate the interface changes proposed in #30440 and #30409 after they're merged? And in the mean time keep them in #30437?

💬 Sjors commented on pull request "indexes: Stop using node internal types and locking cs_main, improve sync logic":
(https://github.com/bitcoin/bitcoin/pull/24230#issuecomment-2245622810)
There's discussion in #29770 about whether to use some or all the things in this PR. Can you give it a rebase?

💬 Sjors commented on pull request "p2p: For assumeutxo, download snapshot chain before background chain":
(https://github.com/bitcoin/bitcoin/pull/29519#issuecomment-2245638221)
In the unlikely event of such a deep reorg I think it's acceptable if the user _might_ need to restart their node in order to follow the new longest headers.

💬 maflcko commented on pull request "test: Add arguments for creating a slimmer TestingSetup":
(https://github.com/bitcoin/bitcoin/pull/30399#issuecomment-2245641239)
Upgrading my review with a test:

* `FUZZ=utxo_snapshot /usr/bin/time --format='%MKB' ./src/test/fuzz/fuzz -runs=1 ../b-c-qa-assets/fuzz_seed_corpus/utxo_snapshot/` gives: 2MB less peak memory (~1%) and 600 less `cov` (~10%) (which is desired here, to avoid reporting non-fuzzing coverage in a fuzz target) and no speedup/slowdown
* Same for `FUZZ=coins_view` gives: 50MB less peak memory (~25%) and no other change

lgtm

📝 fanquake opened a pull request: "guix: GCC 12 consolidation"
(https://github.com/bitcoin/bitcoin/pull/30511)
This PR contains 3 changes:

* Bump GCC in Guix from [12.3.0 to 12.4.0](https://gcc.gnu.org/gcc-12/). A patch was sent upstream, https://lists.gnu.org/archive/html/guix-patches/2024-06/msg01025.html, but has not landed.
* Consolidate all build environments back to using a GCC 12 toolchain. After #21778, the macOS environment is no-longer pinned to 11 (12 would otherwise cause issues building cctools). So, instead of requiring all builders to compile an additional GCC toolchain, use 12.
* Use
...

👍 dergoegge approved a pull request: "test: Add arguments for creating a slimmer TestingSetup"
(https://github.com/bitcoin/bitcoin/pull/30399#pullrequestreview-2194451669)
utACK f46b2202560a76b473e229b77303b8f877c16cac