💬 hodlinator commented on pull request "rest: Reject truncated hex txid early in getutxos parsing":
(https://github.com/bitcoin/bitcoin/pull/30482#discussion_r1687924682)
> Only src/util/time.h, but those are just type-related; And src/logging.h, but those are just aliases; So I don't think they apply here.

Thanks, those are good examples. I guess they are/were used more widely and directly compared to SetHex(), making deprecation a more obvious approach. If you deem this sufficiently big to break up into multiple PRs, I trust your judgement.

`SetHexDeprecated` or `SetHexDiscouraged` work for me.

💬 vasild commented on issue "Feature Request: Broadcast Pool":
(https://github.com/bitcoin/bitcoin/issues/30471#issuecomment-2245028663)
> Sparrow broadcasts over Tor via a random external service such as mempool.space for greater privacy. As I understand it, this would no longer be necessary with `-privatebroadcast=1`

Correct.

> But for full Electrum servers like ElectrumX and Fulcrum which use RPCs like getrawmempool, some other way to retrieve unbroadcast transactions will be necessary

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
...

💬 alfonsoromanz commented on pull request "test: assumeutxo: add missing tests in wallet_assumeutxo.py":
(https://github.com/bitcoin/bitcoin/pull/30455#discussion_r1687958674)
Thank you very much for your explanation and suggestions! I'll definitely check `feature_index_prune.py` and https://github.com/bitcoin/bitcoin/pull/23813 to better understand how this works.

Here is a branch where I pushed my current status: [debugging_assumeutxo_tests](https://github.com/alfonsoromanz/bitcoin/tree/debugging_assumeutxo_tests)

🤔 TheCharlatan reviewed a pull request: "guix: (explicitly) build Linux GCC with `--enable-cet`"
(https://github.com/bitcoin/bitcoin/pull/30438#pullrequestreview-2193845385)
Guix builds (aarch64)
```
find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
8ac09f23c01b91cd88d3f1329c0d27105f98d31cd1735bba4a0299de18c607c0 guix-build-1d2758f6f044/output/aarch64-linux-gnu/SHA256SUMS.part
94dc28c726e81bc51df3ca7dba068d74a74828b6e5162f130c7e210e4f4a2eff guix-build-1d2758f6f044/output/aarch64-linux-gnu/bitcoin-1d2758f6f044-aarch64-linux-gnu-debug.tar.gz
101f0a64ea62eb82a060827edac75b71562628520d4d3da2dc796
...

💬 dergoegge commented on pull request "fuzz: Deglobalize signature cache in sigcache test":
(https://github.com/bitcoin/bitcoin/pull/30447#discussion_r1687978640)
Should be fine for this test since there isn't a lot of reachable code here, i.e. coverage quickly plateaus.

Could be avoided by fixing the cache size but I don't think it matters too much.

💬 Sjors commented on pull request "net, net_processing: additional and consistent disconnect logging":
(https://github.com/bitcoin/bitcoin/pull/28521#issuecomment-2245128004)
Pushed an update to consistently replace any `LogPrint` we touch with `LogDebug`, and every `LogPrintf ` with `LogInfo`. Also fixed some missing `\n` characters as found by tidy.

🚀 fanquake merged a pull request: "doc: use proper doxygen formatting for CTxMemPool::cs"
(https://github.com/bitcoin/bitcoin/pull/30504)

📝 glozow opened a pull request: "m_tx_download_mutex followups"
(https://github.com/bitcoin/bitcoin/pull/30507)
Followup to #30111. Includes suggestions:
- https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1686303768
- https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1686314984
- https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1683186792
- https://github.com/bitcoin/bitcoin/pull/30111#issuecomment-2242819514
- https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1686372826

💬 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#issuecomment-2245143075)
> In the master branch, the design of the TxOrphanage class is thread-safe for all purposes. I don't think that the fact that m_orphanage does not require additional synchronization justifies the change of TxOrphanage class design.

See https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1601902513 for background. `TxOrphanage` is not a multi-purpose container. It has 1 specific purpose, and its user will need to externally synchronize it with other data structures to fulfill that purpo
...

💬 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_r1687993425)
thanks, added 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_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