💬 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.

💬 rebroad commented on pull request "p2p: When close to the tip, download blocks in parallel from additional peers to prevent stalling":
(https://github.com/bitcoin/bitcoin/pull/29664#issuecomment-2380661215)
Rather than the crude 10 minute timeout - why not actually check if the block is being downloaded from the chosen peer - i.e. work out the throughput and make a decision based on this (compared to typical throughput from other peers)?

💬 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_r1779584565)
Weren't we already doing partial validation?
Doing a full one for these examples wouldn't be *that* difficult - do you suggest we do that instead?

💬 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_r1779656652)
Got it, thanks for clarifying!

💬 naiyoma commented on pull request "test: addrman: tried 3 times and never a success so `isTerrible=true`":
(https://github.com/bitcoin/bitcoin/pull/30445#issuecomment-2380717731)
Concept ACK: additional test coverage to check that an address is marked as "terrible" after 3 or more unsuccessful connection attempts.

💬 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_r1779666783)
Alternatively, if that's not desired, what if we replaced the dynamic widths with explicit `PadLeft`/`PadRight` calls, e.g.
```C++
result += tfm::format("<-> type net v mping ping send recv txn blk hb %s%s%s ",
PadRight("addrp", m_max_addr_processed_length),
PadRight("addrl", m_max_addr_rate_limited_length),
PadRight("age", m_max_age_length));
```
and
```C++
result += tfm::format("%s %s%s\n",

...

📝 Jitphanu-051 opened a pull request: "Initial commit"
(https://github.com/bitcoin/bitcoin/pull/30995)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

✅ hebasto closed a pull request: "Initial commit"
(https://github.com/bitcoin/bitcoin/pull/30995)

📝 hebasto locked a pull request: "."
(https://github.com/bitcoin/bitcoin/pull/30995)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

💬 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_r1779720533)
do we need to keep the `strprintf("%s%s"` on line 563?

🤔 glozow reviewed a pull request: "test: switch MiniWallet padding unit from weight to vsize"
(https://github.com/bitcoin/bitcoin/pull/30718#pullrequestreview-2335386388)
ACK 7d1c8d0aa3ebe30d7633caf9bc4765927e4d6e08, lmk if you want to update the second commit

💬 glozow commented on pull request "test: switch MiniWallet padding unit from weight to vsize":
(https://github.com/bitcoin/bitcoin/pull/30718#discussion_r1779726643)
a few more 1000s not changed:

L104 `assert_greater_than_or_equal(tx_v3_child_almost_heavy["tx"].get_vsize() + tx_v3_child_almost_heavy_rbf["tx"].get_vsize(), 1000)`

L194 `assert_greater_than(node.getmempoolentry(tx_v3_child_large["txid"])["vsize"], 1000)`

💬 RandyMcMillan commented on pull request "doc/build-osx.md:brew relinking note":
(https://github.com/bitcoin/bitcoin/pull/30993#discussion_r1779729009)
good point - i will move it to the end of the "brew install" list and make a more general instruction.

-thanks

💬 torkelrogstad commented on pull request "Extend signetchallenge to set target block spacing":
(https://github.com/bitcoin/bitcoin/pull/29365#issuecomment-2380864234)
Concept ACK

📝 torkelrogstad opened a pull request: "doc: update signet documentation related to build directories"
(https://github.com/bitcoin/bitcoin/pull/30996)
While setting up my own signet I noticed that the binary paths in the documentation for this is out of date, after build artifacts moved to the `build` directory. This PR mimics what happened in #30741