💬 l0rinc commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2163644659)
By "dead" I meant that it doesn't bite back (like code does), it can state absolutely anything and the compiler won't help us. Comments usually seem to me like we've given up on trying to express our design within the boundaries of the language. In a few cases here I think the code can be slightly improved to eliminate the need for extra comments.

👍 vasild approved a pull request: "test: refactor out same-txid-diff-wtxid tx to reuse in other tests"
(https://github.com/bitcoin/bitcoin/pull/32385#pullrequestreview-2953228936)
ACK afaaba69eddd50bc22b94ca7c0f9195773aaa111

The code seems to do what is intended and I confirm that it works. I guess it is useful as a generic tool to create a new transaction and two new children of it that have the same txid and different wtxid.

That is useful for the tests of https://github.com/bitcoin/bitcoin/pull/29415 too. However those tests already have a transaction and it would be easier for them and less code, if `malleate_tx()` could create a malleated/changed version of an
...

💬 vasild commented on pull request "test: refactor out same-txid-diff-wtxid tx to reuse in other tests":
(https://github.com/bitcoin/bitcoin/pull/32385#discussion_r2163608973)
The name `build_malleated_children` might be confusing since there is nothing "malleated" in those transactions. They are created as such and never [changed or malleated](https://en.bitcoin.it/wiki/Transaction_malleability) afterwards.

Also in the name of `ValidWitnessMalleatedTx`.

💬 vasild commented on pull request "test: refactor out same-txid-diff-wtxid tx to reuse in other tests":
(https://github.com/bitcoin/bitcoin/pull/32385#discussion_r2163643282)
This works! Following the hint from https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2160767733 I could integrate it into the tests of https://github.com/bitcoin/bitcoin/pull/29415 to try to send two transactions that have the same txid and different wtxid.

However using it is a bit involved - one has to create a new dedicated parent transaction, get it in the mempool and then broadcast the two children. Also, one has to handle the fees manually. Like this:

```py
self.log.info("S
...

🤔 ismaelsadeeq reviewed a pull request: "util: detect and warn when using exFAT on MacOS"
(https://github.com/bitcoin/bitcoin/pull/31453#pullrequestreview-2953272636)
Thanks for taking my suggestion @willcl-ark

Your modified string is even better!

Just a couple of comments and then I will re-test again.

💬 ismaelsadeeq commented on pull request "util: detect and warn when using exFAT on MacOS":
(https://github.com/bitcoin/bitcoin/pull/31453#discussion_r2163635372)
nit: use `fs::quoted` here and other places, will avoid the slashes?

💬 ismaelsadeeq commented on pull request "util: detect and warn when using exFAT on MacOS":
(https://github.com/bitcoin/bitcoin/pull/31453#discussion_r2163641986)
nit: would be nice to be just generic not specific to macOs

💬 ismaelsadeeq commented on pull request "util: detect and warn when using exFAT on MacOS":
(https://github.com/bitcoin/bitcoin/pull/31453#discussion_r2163670725)
Having a constructor here will prevent the C.I failure I think, emplace_back will forward the values to constructor which is missing now.

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2163681310)
@andrewtoth, right! I will change that next time I retouch, "resolving" this.

@stratospher, thanks! Lets continue the discussion in https://github.com/bitcoin/bitcoin/pull/32385#discussion_r2163643282

👍 ismaelsadeeq approved a pull request: "mempool: use `FeeFrac` for ancestor/descendant score comparators"
(https://github.com/bitcoin/bitcoin/pull/32799#pullrequestreview-2953351336)
Code review ACK 922adf66ac7420e21ea171d3586ea84554e5d91b

💬 ismaelsadeeq commented on pull request "init: make `-blockmaxweight` startup option debug only":
(https://github.com/bitcoin/bitcoin/pull/32654#issuecomment-2999938644)
cc @fjahr and @polespinasa, friendly pinged since you take a look initially.

💬 hebasto commented on pull request "depends: Build `qt` package for FreeBSD hosts":
(https://github.com/bitcoin/bitcoin/pull/32731#discussion_r2163703635)
Accepted @vasild's patch.

> Note that `bitcoin-qt` is linked against some dynamic libraries inside `depends/` so it will not work if moved to another machine without those libraries. Is this expected?

No, it is not. This should be fixed in a follow up.

📝 PixelPil0t1 opened a pull request: "Update Docker Build-Push-Action to Latest Stable Version"
(https://github.com/bitcoin/bitcoin/pull/32801)

Updates docker/build-push-action from v5 to v6

References
docker/build-push-action@v6: https://github.com/docker/build-push-action/releases/tag/v6.18.0


This update ensures we're using the latest stable version of the Docker Build and Push Action, which includes performance improvements and bug fixes.

✅ fanquake closed a pull request: "Update Docker Build-Push-Action to Latest Stable Version"
(https://github.com/bitcoin/bitcoin/pull/32801)

🤔 hebasto reviewed a pull request: "build: add root dir to CMAKE_SYSTEM_PREFIX_PATH in toolchain"
(https://github.com/bitcoin/bitcoin/pull/32798#pullrequestreview-2953470821)
Considering the commit history of the NixOS [patch](https://github.com/NixOS/nixpkgs/blob/master/pkgs/by-name/cm/cmake/001-search-path.diff), it seems it was required for some packages, but perhaps it’s no longer needed.

On the other hand, the CMake documentation [states](https://cmake.org/cmake/help/latest/variable/CMAKE_SYSTEM_PREFIX_PATH.html):
> `CMAKE_SYSTEM_PREFIX_PATH` is not intended to be modified by the project; use [`CMAKE_PREFIX_PATH`](https://cmake.org/cmake/help/latest/variable
...

🤔 stickies-v reviewed a pull request: "refactor: Convert GenTxid to `std::variant`"
(https://github.com/bitcoin/bitcoin/pull/32631#pullrequestreview-2949718938)
I think the changes here can be simplified quite a bit by first replacing `get_iter_from_wtxid` with a `GetIter(const wtxid&)` overload, see my suggestions here and here (or full branch https://github.com/bitcoin/bitcoin/compare/master...stickies-v:bitcoin:2025-06/overload-getiter).

There are also quite a few clang-tidy violations that would be helpful to address here, left per-commit comments.

💬 stickies-v commented on pull request "refactor: Convert GenTxid to `std::variant`":
(https://github.com/bitcoin/bitcoin/pull/32631#discussion_r2161772934)
clang-tidy nit:

```suggestion
explicit CompareInvMempoolOrder(CTxMemPool* _mempool)
```

💬 stickies-v commented on pull request "refactor: Convert GenTxid to `std::variant`":
(https://github.com/bitcoin/bitcoin/pull/32631#discussion_r2163691934)
clang-tidy nit for 5b5abf88de7975babe2264456076f85f1c9f11a0:

<details>
<summary>git diff on a6f1af5f55</summary>

```diff
diff --git a/src/primitives/transaction.h b/src/primitives/transaction.h
index 54c2fb578f..adadb2a3e7 100644
--- a/src/primitives/transaction.h
+++ b/src/primitives/transaction.h
@@ -438,17 +438,18 @@ public:
friend bool operator==(const GenTxid& a, const GenTxid& b) { return a.m_is_wtxid == b.m_is_wtxid && a.m_hash == b.m_hash; }
friend bool operator<(
...

💬 stickies-v commented on pull request "refactor: Convert GenTxid to `std::variant`":
(https://github.com/bitcoin/bitcoin/pull/32631#discussion_r2161385282)
nit: this docstring imo doesn't add anything useful. Would either remove or beef up (e.g. explaining when (not) to use this).

💬 stickies-v commented on pull request "refactor: Convert GenTxid to `std::variant`":
(https://github.com/bitcoin/bitcoin/pull/32631#discussion_r2163696012)
clang-tidy nit for da9927139b0f4583c9245aa3b29185612f94cdc7:

<details>
<summary>git diff on 07822f5386</summary>

```diff
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index df66d4c93d..a0ed1e2620 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -2440,8 +2440,8 @@ void PeerManagerImpl::ProcessGetData(CNode& pfrom, Peer& peer, const std::atomic
}

CTransactionRef tx{inv.IsMsgWtx() ?
- FindTxForGetDa
...