💬 l0rinc commented on pull request "tinyformat: refactor: increase compile-time checks and don't throw for tfm::format_error":
(https://github.com/bitcoin/bitcoin/pull/30928#discussion_r1780837147)
Can you expand on that? We've changed other `strprintf` instance here to enable compile-time validation, how come this one didn't make the cut?

💬 l0rinc commented on pull request "tinyformat: refactor: increase compile-time checks and don't throw for tfm::format_error":
(https://github.com/bitcoin/bitcoin/pull/30928#discussion_r1780838413)
Ok, it's not that important

💬 maflcko commented on pull request "tinyformat: refactor: increase compile-time checks and don't throw for tfm::format_error":
(https://github.com/bitcoin/bitcoin/pull/30928#issuecomment-2382746995)
Only changes are some minor style-only test-only fixups.

re-ACK 2e33dbadcf69838c8ea591dccdc666f809c3134a 🔰

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpw
...

💬 stickies-v commented on pull request "tinyformat: refactor: increase compile-time checks and don't throw for tfm::format_error":
(https://github.com/bitcoin/bitcoin/pull/30928#discussion_r1780845253)
Hmm, then I don't think I understand what you mean. Would you be able to provide a diff/example?

👍 l0rinc approved a pull request: "log: Enforce trailing newline at compile time"
(https://github.com/bitcoin/bitcoin/pull/30929#pullrequestreview-2337059996)
ACK facfdcef4a512d4249c63134e632f6154652f022

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-2382848929)
`abec00bb80...4101c7ac8d`: rebase due to conflicts

💬 maflcko commented on pull request "log: Enforce trailing newline at compile time":
(https://github.com/bitcoin/bitcoin/pull/30929#issuecomment-2382851754)
It would be possible to drop the first commit completely and just replace it with a patch that appends a newline if it is missing. This means that no behavior is enforced at compile-time and the newline placement in code could be inconsistent, but the thing that matters is fixed: A trailing newline is ensured to be always present.

```diff
diff --git a/src/logging.h b/src/logging.h
index f465622d0b..9998f85c35 100644
--- a/src/logging.h
+++ b/src/logging.h
@@ -256,7 +256,7 @@ inline void
...

💬 maflcko commented on pull request "tinyformat: refactor: increase compile-time checks and don't throw for tfm::format_error":
(https://github.com/bitcoin/bitcoin/pull/30928#discussion_r1780895517)
last commit: Not that it matters, but my preference would be to not change behavior here and keep the previous formatting. If `fmt` has a trailing newline, it would be "removed" by this patch.

💬 l0rinc commented on pull request "tinyformat: refactor: increase compile-time checks and don't throw for tfm::format_error":
(https://github.com/bitcoin/bitcoin/pull/30928#discussion_r1780898391)
My mistake, rereading it it's not clear what I was asking.
So I think we could simplify the nested formats by pulling up the internal pattern:
Having `%2s` in the main pattern and having a `%s%s` inside could be simplified to having `%s%s` at the parent and layout out the values in the parameters:
```patch
Index: src/bitcoin-cli.cpp
<+>UTF-8
===================================================================
diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp
--- a/src/bitcoin-cli.cpp
...

💬 vasild commented on pull request "rpc: provide per message stats for global traffic via new RPC 'getnetmsgstats'":
(https://github.com/bitcoin/bitcoin/pull/29418#issuecomment-2382864737)
`f85814052d...a8e531f92f`: rebase due to conflicts

👍 vasild approved a pull request: "net: Replace libnatpmp with built-in PCP+NATPMP implementation"
(https://github.com/bitcoin/bitcoin/pull/30043#pullrequestreview-2337161558)
ACK 5c7cacf649a6b474b876a7d219c7dc683a25e33d

💬 l0rinc commented on pull request "log: Enforce trailing newline at compile time":
(https://github.com/bitcoin/bitcoin/pull/30929#issuecomment-2382870857)
> replace it with a patch that appends a newline if it is missing

Didn't you call this code-golf in https://github.com/bitcoin/bitcoin/pull/30929#discussion_r1778763759?
I'd prefer keeping this explicit (as it i now) - or renaming the method to end with `ln` if we're appending.

💬 l0rinc commented on pull request "Cover remaining tinyformat usages in CheckFormatSpecifiers":
(https://github.com/bitcoin/bitcoin/pull/30999#discussion_r1780907250)
Valid complaint, I've [added](https://github.com/bitcoin/bitcoin/compare/23f2887af3d6d088433d25cf682575955b5bccd8..44aa62e98b113e118a620d9cf0cee92d8684c8fb) a `consteval ValidFormatSpecifiers` local delegate and split the valid tests from the `BOOST_CHECK_EXCEPTION`s.

💬 l0rinc commented on pull request "Cover remaining tinyformat usages in CheckFormatSpecifiers":
(https://github.com/bitcoin/bitcoin/pull/30999#discussion_r1780907365)
Good idea, thanks

💬 l0rinc commented on pull request "Cover remaining tinyformat usages in CheckFormatSpecifiers":
(https://github.com/bitcoin/bitcoin/pull/30999#discussion_r1780907498)
[Added back](https://github.com/bitcoin/bitcoin/compare/23f2887af3d6d088433d25cf682575955b5bccd8..44aa62e98b113e118a620d9cf0cee92d8684c8fb) the part that I think is relevant, let me know if you'd like me to rewrite it.

💬 l0rinc commented on pull request "tinyformat: refactor: increase compile-time checks and don't throw for tfm::format_error":
(https://github.com/bitcoin/bitcoin/pull/30928#discussion_r1780912027)
Yes, your review is welcome there as well

💬 l0rinc commented on pull request "doc/build-osx.md:brew relinking note":
(https://github.com/bitcoin/bitcoin/pull/30993#discussion_r1780919060)
Is this related to https://github.com/bitcoin/bitcoin/pull/30454/files#r1709145204?
> [l0rinc](https://github.com/l0rinc) [on Aug 8](https://github.com/bitcoin/bitcoin/pull/30454/files#r1709145204)
👍 GUI is working with cmake, but I had to execute a `brew link qt5 --force` as well after `brew install qt@5` (applies to Autotools as well)

💬 Sjors commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2382905543)
ACK 5c7cacf649a6b474b876a7d219c7dc683a25e33d

Just a documentation squash since my last test and review.

💬 Sjors commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2382907372)
@hebasto this was on macOS, not a cross-compile. `$HOST` is `my-computer.localdomain`

Your patch seems to work, it's building now. Will let you know if I find other issues.

📝 marcofleon opened a pull request: "refactor: ensure type safety for txid and wtxid in `RelayTransaction`"
(https://github.com/bitcoin/bitcoin/pull/31001)
This PR updates the `RelayTransaction` function to replace `uint256` with the `Txid` and `Wtxid` types, improving type safety and helping with the gradual transition to using these types throughout the codebase.