💬 mzumsande commented on pull request "init: Remove retry for loop":
(https://github.com/bitcoin/bitcoin/pull/30968#discussion_r1779222847)
nit: might use "retry with do_reindex set". Otherwise it might sounds a bit like "try turning it off and on again".

💬 ryanofsky commented on pull request "init: Remove retry for loop":
(https://github.com/bitcoin/bitcoin/pull/30968#issuecomment-2380253538)
> I think that one minor change in behavior is that with the old code we could retry many times if, for some reason, we'd repeatedly get `ChainstateLoadStatus::FAILURE` messages (which shouldn't happen normally, but might in some exotic failure scenarios?)

I think it wouldn't retry multiple times because the old code sets `do_reindex = true`

https://github.com/bitcoin/bitcoin/blob/d812cf11896a2214467b6fa72d7b763bac6077c5/src/init.cpp#L1643

At least that was my interpretation reviewing t
...

💬 jonatack commented on pull request "Disable RBF Rule 2":
(https://github.com/bitcoin/bitcoin/pull/30964#issuecomment-2380259736)
In this context, concept ack with waiting for a more general solution rather than iterative piecemeal changes, as long as achieving it (via cluster mempool) looks reasonably attainable.

💬 andrewtoth commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1779311123)
Since #30921, we can now make these `constexpr string_view`.

💬 mzumsande commented on pull request "init: Remove retry for loop":
(https://github.com/bitcoin/bitcoin/pull/30968#issuecomment-2380328047)
> At least that was my interpretation reviewing this earlier (could be mistaken)

Yes, you are right. I thought that maybe there could be some scenarios where even with `do_reindex == true` we could maybe get `ChainstateLoadStatus::FAILURE` again, but after looking more closely I don't see how this could be the case.

📝 RandyMcMillan opened a pull request: "doc/build-osx.md:brew relinking note"
(https://github.com/bitcoin/bitcoin/pull/30993)

💬 RandyMcMillan commented on pull request "docs: Add instructions on how to self-sign bitcoin-core binaries for macOS":
(https://github.com/bitcoin/bitcoin/pull/30982#issuecomment-2380384258)
Concept ACK

💬 vasild commented on pull request "net: option to disallow v1 connection on ipv4 and ipv6 peers":
(https://github.com/bitcoin/bitcoin/pull/30951#issuecomment-2380400982)
> For example if you are concerned about someone inspecting the traffic to your node it really doesn't matter how many BIP324 peers you have if you have just one unencrypted connection.

True. Lets dive a bit deeper - what are the reasons to be concerned about somebody inspecting the traffic to my node?

💬 vasild commented on pull request "[WIP] net: return result from addnode RPC":
(https://github.com/bitcoin/bitcoin/pull/30381#issuecomment-2380402219)
Concept ACK

When `addnode` exits silently I have always found it annoying to have to go to the debug log to find out what happened.

Isn't one of the `addnode` variants asynchronous?

💬 vasild commented on pull request "test: add end-to-end tests for CConnman and PeerManager":
(https://github.com/bitcoin/bitcoin/pull/26812#issuecomment-2380411313)
`6d0c5247eb...752fbab26a`: rebase due to conflicts

💬 vasild commented on pull request "util: explicitly close all AutoFiles that have been written":
(https://github.com/bitcoin/bitcoin/pull/29307#issuecomment-2380413586)
`489c2477e7...dba7835386`: rebase due to conflicts

💬 l0rinc commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1779377188)
Made it a `constexpr auto` (I don't want non-owning fields) https://github.com/bitcoin/bitcoin/compare/1dd79a32c9c6e0c6cf4ecef00b56972574babced..cb9be6729c4e27bf2d6d703c0d454a22cbcdb6e1

💬 hebasto commented on pull request "doc/build-osx.md:brew relinking note":
(https://github.com/bitcoin/bitcoin/pull/30993#discussion_r1779449856)
Is this issue CMake specific?

📝 itornaza opened a pull request: "depends: fix for llvm-ranlib (etc): 'No such file or directory' macOS 15.0"
(https://github.com/bitcoin/bitcoin/pull/30994)
This is an attempt to fix the issue https://github.com/bitcoin/bitcoin/issues/30978 by conditionally defining the necessary tools when building on macos and is based on the Solution 3 described in https://github.com/bitcoin/bitcoin/issues/30978#issuecomment-2379782629.

💬 jonatack commented on pull request "[WIP] net: return result from addnode RPC":
(https://github.com/bitcoin/bitcoin/pull/30381#issuecomment-2380636483)
> make getpeerinfo accept a peer id param to just investigate that one

Sounds useful.

Didn't re-read the discussion, but I'm not sure that no connection should throw, in favor of returning an "error" field (or "errors" object).

@willcl-ark do you plan to update here soon?

💬 jonatack commented on pull request "net: option to disallow v1 connection on ipv4 and ipv6 peers":
(https://github.com/bitcoin/bitcoin/pull/30951#discussion_r1779484434)
I might be too focused on the risks rather than benefits, but this is unfortunately only one brigading campaign by one or more prominent bitcoiners on social media away from being potentially adopted by many nodes.

With -onlynet, the network partition risk is in practice opt-in, i.e. chosen by a minority subset for themselves. Whereas here, the network partition risk could be imposed on a future minority of users that are running earlier versions of Bitcoin Core. This seems at odds with main
...

💬 jonatack commented on pull request "net: option to disallow v1 connection on ipv4 and ipv6 peers":
(https://github.com/bitcoin/bitcoin/pull/30951#discussion_r1779484507)
```suggestion
* connections on Tor/I2P/CJDNS can be v1 or v2 connections.
```

💬 jonatack commented on pull request "net: option to disallow v1 connection on ipv4 and ipv6 peers":
(https://github.com/bitcoin/bitcoin/pull/30951#discussion_r1779484571)
```suggestion
} else if {
(LookupHost(host, true).has_value()) {
```

👍 itornaza approved a pull request: "docs: Add instructions on how to self-sign bitcoin-core binaries for macOS"
(https://github.com/bitcoin/bitcoin/pull/30982#pullrequestreview-2335026045)
Concept ACK.

I can also verify that the steps described in the pr work properly on macOS Sequoia 15.0 (24A335).

> I think it might be good to also include instructions for signing a downloaded `Bitcoin-Qt.app`, as I would think that might be what a lot (majority?) of macOS users are downloading.

In my view, users who download the app directly can always be referenced to the standard Apple procedure for installing unknown developer apps https://support.apple.com/guide/mac-help/open-a-mac
...

💬 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_r1779498782)
I am not sure about "partial validation". Either the string is fully validated, or not at all. Prepending a valid string to an invalid string doesn't make it valid.