💬 hebasto commented on pull request "depends: Cleanup postprocess commands after switching to CMake":
(https://github.com/bitcoin/bitcoin/pull/30506#issuecomment-2245200526)
My Guix build:
```
x86_64
e23e8a489ca99ccd8be77790ce3af8a47355c47783c3e0aa95f1b9d0ff8812c9 guix-build-f624854665ac/output/aarch64-linux-gnu/SHA256SUMS.part
ff15f20ffa82de84e3b242de1501712db7fc07a084425e31706324c4d6698f10 guix-build-f624854665ac/output/aarch64-linux-gnu/bitcoin-f624854665ac-aarch64-linux-gnu-debug.tar.gz
0a3ac5d2a3b426f2ab8249a7876fc0f5dda0dc11d74b7f0a3704c45a22cd68af guix-build-f624854665ac/output/aarch64-linux-gnu/bitcoin-f624854665ac-aarch64-linux-gnu.tar.gz
c35067995
...

💬 hodlinator commented on pull request "fix: Make TxidFromString() respect string_view length":
(https://github.com/bitcoin/bitcoin/pull/30436#discussion_r1688034493)
Removed the `.substr()` now but kept the raw string instead of wrapping the second parameter in `uint256S()`.

💬 kevkevinpal commented on issue "Wallet fuzzing tracking issue":
(https://github.com/bitcoin/bitcoin/issues/29901#issuecomment-2245205818)
I started to take a look at the [Wallet RPC](https://github.com/bitcoin/bitcoin/issues/30458) issue, I'm still fairly new to fuzz testing so it might take longer but I am working off of [this branch](https://github.com/kevkevinpal/bitcoin/tree/fuzzwalletrpc) if you would like to track my progress.

💬 vasild commented on pull request "util: explicitly close all AutoFiles that have been written":
(https://github.com/bitcoin/bitcoin/pull/29307#issuecomment-2245216013)
@ryanofsky, I am not so attached to the current approach :) if it can be improved, then lets improve it.

For the `on_error` callback - that would be called on fclose() error from destructor only, right? Not on any error? E.g. the `write()` method throws an exception on error.

Not all callers have the file name when creating `AutoFile` which means we would have to log some generic message without a file name. Or go an extra mile and make sure we have the file name, but some callers don't us
...

💬 instagibbs commented on pull request "policy: Add PayToAnchor(P2A), `OP_1 <0x4e73>` as a standard output script for spending":
(https://github.com/bitcoin/bitcoin/pull/30352#discussion_r1688047624)
took a bit more work after removing this as I needed to add its own CTxDestination, but fixed finally

💬 Sjors commented on pull request "contrib: add tool to convert compact-serialized UTXO set to SQLite database":
(https://github.com/bitcoin/bitcoin/pull/27432#issuecomment-2245254623)
Both `gettxoutsetinfo muhash 840000` and your `calc_utxo_hash.go` script processing the .sqlite file find the same hash: `ba56574e1b9a9f64ff9091b1baec407b35c9c34634c923e60467bc5e4265a637`

💬 brunoerg commented on pull request "test: Add arguments for creating a slimmer TestingSetup":
(https://github.com/bitcoin/bitcoin/pull/30399#issuecomment-2245255502)
> (When I benchmarked an earlier version of this, I didn't find a significant speedup, but I may have done something wrong and the changes here seem useful either way)

Same here. The changes lgtm but I tried to benchmark it and I didn't see any significant improvement. Not sure if I did it right.

💬 paplorinc commented on pull request "fix: Make TxidFromString() respect string_view length":
(https://github.com/bitcoin/bitcoin/pull/30436#discussion_r1688088749)
+1

🤔 furszy reviewed a pull request: "wallet: Automatically repair corrupted metadata with doubled derivation path"
(https://github.com/bitcoin/bitcoin/pull/29124#pullrequestreview-2194035447)
ACK 1c3462e908994f0d63c1f39b5908f48d083dd10e

Could minimize the first commit diff https://github.com/furszy/bitcoin-core/commit/145737e46e71859608334ab738817f7b323df367.

💬 paplorinc commented on pull request "fix: Make TxidFromString() respect string_view length":
(https://github.com/bitcoin/bitcoin/pull/30436#discussion_r1688092025)
nit: was
```suggestion
while (digits < trimmed.size() && ::HexDigit(trimmed[digits]) != -1)
++digits;
```
also controversial?

👍 paplorinc approved a pull request: "fix: Make TxidFromString() respect string_view length"
(https://github.com/bitcoin/bitcoin/pull/30436#pullrequestreview-2194041190)
ACK 09ce3501fa2ea2885a857e380eddb74605f7038c

Note: adding co-authorship for the commits that reviewers have contributed to is a good way to make sure all participants are motivated to continue doing it.

📝 hebasto opened a pull request: "depends: Fix CMake-generated `libzmq.pc` file"
(https://github.com/bitcoin/bitcoin/pull/30508)
This is a backport of the not yet merged upstream PR: https://github.com/zeromq/libzmq/pull/4706.

Similar to https://github.com/bitcoin/bitcoin/pull/30488.

Addresses https://github.com/bitcoin/bitcoin/pull/29723#issuecomment-2239864170:
> Looking at the mingw .pc generated by this PR:
>
> ```
> Libs: -L${libdir} -lzmq
> Libs.private:
> Requires.private:
> ```
>
> It looks like we'll need to take [zeromq/libzmq#4706](https://github.com/zeromq/libzmq/pull/4706) as well for CMake.
...

💬 hebasto commented on pull request "depends: build zeromq with CMake":
(https://github.com/bitcoin/bitcoin/pull/29723#issuecomment-2245323520)
> Looking at the mingw .pc generated by this PR:
>
> ```
> Libs: -L${libdir} -lzmq
> Libs.private:
> Requires.private:
> ```
>
> It looks like we'll need to take [zeromq/libzmq#4706](https://github.com/zeromq/libzmq/pull/4706) as well for CMake. That can be done as a follow-up though, as it's not yet merged upstream.

Done in https://github.com/bitcoin/bitcoin/pull/30508.

💬 luisschwab commented on issue "`scantxoutset` should output the block hash instead of block height":
(https://github.com/bitcoin/bitcoin/issues/30478#issuecomment-2245407353)
@jbesraa I'm currently working on a PR for this issue.

💬 maflcko commented on pull request "rest: Reject truncated hex txid early in getutxos parsing":
(https://github.com/bitcoin/bitcoin/pull/30482#discussion_r1688175876)
Ok, done, went with my original suggestion :sweat_smile:

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

`*.la` files are no longer created. Hence, there is no need to remove them:
```diff
--- a/depends/packages/expat.mk
+++ b/depends/packages/expat.mk
@@ -33,5 +33,5 @@ define $(package)_stage_cmds
endef

define $(package)_postprocess_cmds
- rm -rf share lib/cmake lib/*.la
+ rm -rf share lib/cmake
endef
```

(similar to https://github.com/bitcoin/bitcoin/pull/30506)

💬 maflcko commented on pull request "rest: Reject truncated hex txid early in getutxos parsing":
(https://github.com/bitcoin/bitcoin/pull/30482#discussion_r1688177775)
The format string is txid_num, so I kept the order.

💬 hebasto commented on pull request "depends: build expat with CMake":
(https://github.com/bitcoin/bitcoin/pull/29878#issuecomment-2245427136)
My Guix build:
```
x86_64
4ed36603100265640e4e9e73fa8f04c2b10a581c0a68f37cb636f1cbff71469c guix-build-cf46c6b8f2ff/output/aarch64-linux-gnu/SHA256SUMS.part
70b0f40dec0646cc7378c17bb569c8aa292ceba93e9b7233e7a0067fde04a24c guix-build-cf46c6b8f2ff/output/aarch64-linux-gnu/bitcoin-cf46c6b8f2ff-aarch64-linux-gnu-debug.tar.gz
e778fdbf6dcf74a8cfeac8d3b353f877809f96a6f4759e497d939b001874a357 guix-build-cf46c6b8f2ff/output/aarch64-linux-gnu/bitcoin-cf46c6b8f2ff-aarch64-linux-gnu.tar.gz
c99a80fbe
...

📝 ryanofsky opened a pull request: "multiprocess: Add -ipcbind option to bitcoin-node"
(https://github.com/bitcoin/bitcoin/pull/30509)
Add `-ipcbind` option to `bitcoin-node` to make it listen on a unix socket and accept connections from other processes. The default socket path is currently `<datadir>/sockets/bitcoin-node.sock`, but this can be customized.

This option lets potential wallet, gui, index, and mining processes connect to the node and control it, see examples in #19460, #19461, and #30437. (Note that the `-ipcbind` option is not needed for #10102 which runs GUI, node, and wallet code in different processes, becau
...

💬 maflcko commented on pull request "rest: Reject truncated hex txid early in getutxos parsing":
(https://github.com/bitcoin/bitcoin/pull/30482#issuecomment-2245431136)
Pushed and replied to all feedback, except for https://github.com/bitcoin/bitcoin/pull/30482#discussion_r1687924682, which I'll do later

🤔 danielabrozzoni reviewed a pull request: "rpc: Return errors in loadtxoutset that currently go to logs"
(https://github.com/bitcoin/bitcoin/pull/30497#pullrequestreview-2194210962)
ACK fa5c253b4e9e416a6ff729365cb66e13ac0f66e7 - code looks good to me, tests are passing locally, but I didn't do any manual testing.