💬 andrewtoth commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1835766013)
This is unfortunate that `privatebroadcast` is incompatible with `connect`. The use case would be to connect to a single trusted peer for listening, but broadcast through ephemeral connections to not link the trusted peer to our wallet.

💬 Sjors commented on pull request "Add multiprocess binaries to release build":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2466864426)
@fanquake done and rebased after #31105.

💬 1440000bytes commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-2466865095)
> @1440000bytes, thanks for asking! There is some discussion at #27509 (the previous attempt on this).
>
> > Is it necessary to open new short lived tor/i2p connections for broadcasting the transaction?
>
> Yes, it is. See below.
>
> > What are the trade-offs in this implementation vs a simple implementation to relay tx to one or more peers that our node is already connected to?
>
> Sending the transaction over clearnet reveals the IP address/geolocation of the sender. A spy with man
...

💬 Sjors commented on pull request "Add multiprocess binaries to release build":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2466904285)
It looks like the `no multiprocess, i686, DEBUG` functional tests are still trying to check multiprocess stuff despite `NO_MULTIPROCESS=1`.

> FileNotFoundError: [Errno 2] No such file or directory: 'bitcoin-node'

🤔 l0rinc reviewed a pull request: "tinyformat: Add compile-time checking for literal format strings"
(https://github.com/bitcoin/bitcoin/pull/31174#pullrequestreview-2425834191)
Nice, simple approach, like it a lot!
I think we can simplify the validator a bit more - let me know what you think.

Also, not exactly sure why `%n` parity wasn't added like in https://github.com/bitcoin/bitcoin/pull/30999/files#diff-71badc1cc71ba46244f7841a088251bb294265f4fe9e662c0ad6b15be540eee4R69 (is it controversial or unnecessary or not useful)?

💬 l0rinc commented on pull request "tinyformat: Add compile-time checking for literal format strings":
(https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1835766554)
Given that we have two separate number "parsers" (one that keeps the result and one that throws it away), we might as well extract number parsing to a local lambda like you did with the other ones.

<details>
<summary>Diff</summary>

```patch
diff --git a/src/util/string.h b/src/util/string.h
--- a/src/util/string.h (revision ecc5cb9a89c6b001df839675b23d8fc1f7ac69ba)
+++ b/src/util/string.h (date 1731267170701)
@@ -45,14 +45,16 @@
continue;
}

+
...

💬 l0rinc commented on pull request "tinyformat: Add compile-time checking for literal format strings":
(https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1835773438)
Checked and failures seem to be validated successfully from command line, but - unlike the previous versions - doesn't seem to be shown in the IDE... Weird :/

💬 l0rinc commented on pull request "tinyformat: Add compile-time checking for literal format strings":
(https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1835770569)
These are all here to check if we're inside a format string, but we don't have a `%%`, right?
Could we maybe simplify that to something like:
```suggestion
if (*it != '%' || *(++it) == '%') continue; // Skip escaped %%
```
?

💬 l0rinc commented on pull request "tinyformat: Add compile-time checking for literal format strings":
(https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1835770704)
In C++23 this could be a simple `.contains`, but even in C++20 we should be able to group the flags to something like:
```suggestion
while ("#0- +"sv.find(*it) != std::string_view::npos) ++it;
```
(we could even extract the flag in which case we could get rid of the comment)

💬 Sjors commented on pull request "Add multiprocess binaries to release build":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2466909081)
I set `export BITCOIND=bitcoin-node` just for the ARM job for now.

💬 grzegorzblaszczyk commented on something "":
(https://github.com/bitcoin/bitcoin/commit/c5a9d2ca9e3234db9687c8cbec4b5b93ec161190#commitcomment-148931454)
@ditto-b Nice catch!

💬 luke-jr commented on pull request "cli: Improve error message on multiwallet cli-side commands":
(https://github.com/bitcoin/bitcoin/pull/26990#issuecomment-2466952908)
> Removed validation commit which I'll added in a separate PR as https://github.com/bitcoin/bitcoin/pull/26990#issuecomment-2354091658 by @jonatack.

Does this exist?

💬 fjahr commented on pull request "test: Introduce ensure_for helper":
(https://github.com/bitcoin/bitcoin/pull/30893#discussion_r1835806968)
Initially it had the `timeout_factor` and also it seemed nice to have it next to `wait_until` but sure, I have made it a util function now.

💬 fjahr commented on pull request "test: Introduce ensure_for helper":
(https://github.com/bitcoin/bitcoin/pull/30893#issuecomment-2466975008)
Addressed feedback from @maflcko and made `ensure_for` a util function.

⚠️ Sjors opened an issue: "v27.2 guix build fails with hash mismatch"
(https://github.com/bitcoin/bitcoin/issues/31266)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

```
git checkout v27.2
./contrib/guix/guix-build
...
```

This fails for `gnutls-3.8.1.tar.xz.drv`, so I run that one specifically:

```
$ guix build --cores=1 /gnu/store/b0cvv1d3kz4ilf14iz5b9iq4dafxcv4a-gnutls-3.8.1.tar.xz.drv
The following derivation will be built:
/gnu/store/b0cvv1d3kz4ilf14iz5b9iq4dafxcv4a-gnutls-3.8.1.tar.xz.drv
building /gnu/store/b0cvv1d3kz4ilf14iz5b9i
...

🤔 tdb3 reviewed a pull request: "Ephemeral Dust"
(https://github.com/bitcoin/bitcoin/pull/30239#pullrequestreview-2425867463)
Concept ACK

Great policy update that will provide flexibility to L2s.
Left a few comments, and want to take a deeper look at test cases.

💬 tdb3 commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1835811847)
If it's decided to allow non-zero modified fee, then the latter half of the `||` might need to be adjusted here (https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1831104732).

💬 tdb3 commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1835815612)
Maybe I'm missing something simple, but could this line insert the parent txid into `processed_parent_set` even when the parent couldn't be checked for dust (`parent_ref` is `nullptr`)? If so, would we want to handle this as an error condition?

Maybe it's not an issue currently (e.g. reliance on 1P1C), but could be defensive to check to protect in the event of future change.

💬 tdb3 commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1835805164)
nit: The comment blocks before each function could be updated to use doxygen commands. Could be left for follow-up.

Example:
```diff
/** Must be called for each transaction once transaction fees are known.
* Does context-less checks about a single transaction.
- * Returns false if the fee is non-zero and dust exists, populating state. True otherwise.
+ * @returns false if the fee is non-zero and dust exists, populating state. True otherwise.
*/
bool CheckValidEphemeralTx(const CT
...

💬 tdb3 commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1835796375)
Thought about what guarantees we would have for `get_utxo()` to not return one of the dust. At first glance, looks like currently `get_utxo()` would return the largest utxo (i.e. not dust). Could have a stronger guarantee if we used `get_utxo(confirmed_only=True)` instead (dusty_tx is in the mempool but not yet confirmed). Could be left for a follow-up unless updating for another reason.

💬 tdb3 commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1835810249)
The function also returns `nullopt` for an invalid package. Maybe I'm missing something, but this seems to conjoin a failure case (invalid package) with the success case (all dust properly spent). Something like https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1834726168 could increase consistency.