💬 glozow commented on pull request "locks: introduce mutex for tx download, flush rejection filters once per tip change":
(https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1687993769)
no we don't, changed in #30507

💬 glozow commented on pull request "locks: introduce mutex for tx download, flush rejection filters once per tip change":
(https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1687994059)
good idea, done in #30507

👋 fanquake's pull request is ready for review: "[27.x] More backports"
(https://github.com/bitcoin/bitcoin/pull/30467)

💬 craigraw commented on issue "Feature Request: Broadcast Pool":
(https://github.com/bitcoin/bitcoin/issues/30471#issuecomment-2245169909)
> Maybe. Note that usually a transaction will be in unbroadcast pool and not in the mempool only for a few seconds, until it round-trips through the network back to us.

Yes, this would be the normal case. But if the transaction cannot enter the mempool because of low fee rate/mempool size constraints, then it will remain only in the unbroadcast pool as you described above. In this case, the Electrum server should be able to retrieve the transaction in order to include it in the list of releva
...

💬 hodlinator commented on pull request "fix: Make TxidFromString() respect string_view length":
(https://github.com/bitcoin/bitcoin/pull/30436#issuecomment-2245200216)
Credit @ryanofsky as author of `TxidFromString()` test with a minor transformation.

Use `BOOST_CHECK_EQUAL_COLLECTIONS()`.

`SetHex()` - Largely reset back to previously reviewed implementation with 3 ACKs. Sacrificing both my `find_if()` and @paplorinc & mine's very nice final loop. Keeping the `trimmed` `string_view` variable and `const`ed the parameter as a hat-tip to @paplorinc.

As I was changing `SetHex()` I initially forgot to bring back `std::fill(m_data.begin(), m_data.end(), 0);
...

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