💬 stickies-v commented on pull request "init: Remove retry for loop":
(https://github.com/bitcoin/bitcoin/pull/30968#discussion_r1778860008)
Is there a reason to set `check_ratio`? It doesn't seem necessary?

```suggestion
CTxMemPool::Options mempool_opts{};
```

🤔 stickies-v reviewed a pull request: "init: Remove retry for loop"
(https://github.com/bitcoin/bitcoin/pull/30968#pullrequestreview-2334139707)
Strong concept ACK!

🤔 l0rinc requested changes to a pull request: "tinyformat: refactor: increase compile-time checks and don't throw for tfm::format_error"
(https://github.com/bitcoin/bitcoin/pull/30928#pullrequestreview-2333963688)
I have concerns about the translations, using raw format instead of adjusting the validator, and I think we could sneak in a fix for unterminated positionals.

<details>
<summary>Patch</summary>

```patch
Index: src/bitcoin-cli.cpp
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp
--- a/src/bitcoin-cli.cpp (revision 8845a566
...

💬 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_r1778758009)
is this bug still possible now? If it is, consider updating the `strprintf` example

💬 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_r1778754066)
nit: we could avoid mentioning compile-time twice:
```suggestion
- *Rationale*: Tinyformat handles string parameters.
Using a string literal or `constexpr const char*` ensures
format strings are validated at compile time.
```

💬 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_r1778767590)
I'm not sure I understand how this change is supposed to work.

`"Copyright (C) %i-%i"` can be translated by checking the exact string from https://github.com/bitcoin/bitcoin/blob/master/src/qt/bitcoinstrings.cpp#L280, but how is `"Copyright (C) 2009-2024"` be supposed to have a translated counterpart?

💬 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_r1778855186)
Unrelated:
just realized that we should eventually add `"%1"` passes, since `while ('0' <= *it && *it <= '9') {` can go beyond the string and passed accidentally - but seems to fail in tinyformat, see: https://godbolt.org/z/nbTKnrqch.
Should I add a new PR for that of do you want to add that case here?

💬 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_r1778776885)
👍, should have been there all along

💬 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_r1778768954)
lol

💬 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_r1778783288)
for consistency, consider using strprintf here as well:
```suggestion
BOOST_CHECK_MESSAGE(ret == expected, strprintf("[%s] mismatching requestables", comment");
```

----


Would be cool if we could use `BOOST_CHECK_EQUAL_COLLECTIONS` instead, but may not be trivial to do, haven't investigated in detail.

💬 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_r1778882137)
nit:
```suggestion
// Ensure invalid format strings don't throw at run-time
```

💬 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_r1778882653)
We could add the examples from `bitcoin-cli` here:
```C++
BOOST_AUTO_TEST_CASE(ConstevalFormatString_SpecificUsageTests)
{
// These format strings contain '*' and should skip validation
PassFmt<6>("<-> type net v mping ping send recv txn blk hb %*s%*s%*s ");
PassFmt<5>("%*s %-*s%s\n");
PassFmt<22>(
"%3s %6s %5s %2s%7s%7s%5s%5s%5s%5s %2s %*s%*s%*s%*i %*s %-*s%s\n");
PassFmt<2>(" ms ms sec sec min min %*
...

💬 0xB10C commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#issuecomment-2379684081)
Played around with the `getorphantxs 1` output a bit regarding visualization. You can now plug your own `getorphantxs.json` (verbosity >=1) into this demo tool https://observablehq.com/d/fb8142e7eb7f98f3. Produces something like this based on the size and from-peers (color) of the orphans.


💬 theuni commented on pull request "refactor: Replace g_genesis_wait_cv with m_tip_block_cv":
(https://github.com/bitcoin/bitcoin/pull/30967#discussion_r1778899716)
To avoid a race, this needs to lock `kernel_notifications.m_tip_block_mutex` before calling notify. `(*node.shutdown_signal)()` doesn't necessarily need to happen under the lock, but a lock is required after the signal in order to synchronize. Otherwise the update could be missed and we could potentially wait forever.

This is currently an example of [this trap](https://www.modernescpp.com/index.php/c-core-guidelines-be-aware-of-the-traps-of-condition-variables/) (see the "An atomic predicate"
...

💬 Sjors commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#issuecomment-2379691000)
Here's an initial sketch of making Sv2Connman a subclass of SockMan. The test gets through the handshake but fails later on, so I'll need to study it a bit more closely.

https://github.com/Sjors/bitcoin/pull/64

💬 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_r1778939114)
`"Copyright (C) %i-%i"` remains the translateable string both before and after this change. The only difference is that we use the `bilingual_str` `format` overload instead of the `const std::string&` one.

👍 theuni approved a pull request: "guix: Drop no longer needed `PATH` modification"
(https://github.com/bitcoin/bitcoin/pull/30989#pullrequestreview-2334276129)
utACK f1daa80521eccebe86af6ee6fa594edf40eaa676 since guix is happy.

> 2. I don't understand why "In the Guix environment, `${BASEPREFIX}/${HOST}/native/bin` is added to the `PATH` environment variable," according to the description. Setting this seems indiscriminate, like a sledgehammer approach, something that would cause the guix build to behave differently from normal depends builds and lead to confusing issues like this one.

As @hebasto mentioned [here](https://github.com/bitcoin/bitco
...

💬 brunoerg commented on pull request "fuzz: wallet: add target for `CreateTransaction`":
(https://github.com/bitcoin/bitcoin/pull/29936#discussion_r1778950526)
Done.

💬 brunoerg commented on pull request "fuzz: wallet: add target for `CreateTransaction`":
(https://github.com/bitcoin/bitcoin/pull/29936#issuecomment-2379765693)
Force-pushed addressing https://github.com/bitcoin/bitcoin/pull/29936#discussion_r1778636559

💬 itornaza commented on issue "depends: llvm-ranlib (etc): "No such file or directory" on Intel macOS 15.0":
(https://github.com/bitcoin/bitcoin/issues/30978#issuecomment-2379782629)
Following the [macOS Build Guide](https://github.com/bitcoin/bitcoin/blob/master/doc/build-osx.md#2-homebrew-package-manager) I am using the `llvm` installed from brew and in my `.zshrc` I have an export to override the default macos llvm `export PATH="$(brew --prefix)/opt/llvm/bin:$PATH"`.

Using the `which` command with the missing tools, I get the corresponding tool paths that are of course not the ones in the `usr/bin/` directory.

```
% which llvm-ranlib
/opt/homebrew/opt/llvm/bin/llv
...