💬 hodlinator commented on pull request "test: Prove+document ConstevalFormatString/tinyformat parity":
(https://github.com/bitcoin/bitcoin/pull/30933#issuecomment-2388722857)
@maflcko
> Are you still working on this?

I'm waiting on #30928 before un-drafting this.

💬 maflcko commented on pull request "build: Improve `ccache` performance for different build directories":
(https://github.com/bitcoin/bitcoin/pull/30861#issuecomment-2388728933)
Concept ACK

💬 l0rinc commented on pull request "cmake: Avoid hardcoding Qt's major version in Find module":
(https://github.com/bitcoin/bitcoin/pull/31010#discussion_r1784608370)
It seems to me we're still hardcoding the major version - can't we change this during migration instead?

💬 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-2388768319)
> This is a good argument except we do have some strings only known at runtime, notably translations, which probably don't get full testing. Maybe there's something that automatically compares format strings for equivalence across translations though.

If translations are malformed to induce a tinyformat error where one wouldn't be present in the non-translated string, I don't think the solution would to use `Result`. This seems no different from an otherwise fully tinyformat-valid string that
...

💬 hebasto commented on pull request "cmake: Avoid hardcoding Qt's major version in Find module":
(https://github.com/bitcoin/bitcoin/pull/31010#discussion_r1784617293)
> It seems to me we're still hardcoding the major version

I meant the variable names. The full minimum required version was not supposed to change.

> can't we change this during migration instead?

It is done in https://github.com/bitcoin/bitcoin/pull/30997. This PR split from it to make the https://github.com/bitcoin/bitcoin/pull/30997 easier to review.

💬 vasild commented on pull request "net: option to disallow v1 connection on ipv4 and ipv6 peers":
(https://github.com/bitcoin/bitcoin/pull/30951#issuecomment-2388772718)
> increase the cost of detecting that they run a bitcoin node

How? Given that the spy does not need to inspect the traffic at all:

> My node will connect to publicly known bitcoin nodes and the mere fact that I have a TCP connection to an addr:port, where it is known that a bitcoin node is running, is already revealing enough.

💬 andremralves commented on pull request "contrib: skip missing binaries in gen-manpages":
(https://github.com/bitcoin/bitcoin/pull/30986#issuecomment-2388779305)
Thanks for the feedback!

>I wonder if it's not easier to minimize what is being caught in the IOError try block and just remove missing bins from the BINARIES list? This list is used later on in the script too so in my opinion that is probably the better option. Otherwise we will end up referencing non-existing binaries/manpages in the existing manpages.

I agree and have updated the commit to reflect your suggestions. I just needed to make a copy of BINARIES so that it's possible to remove
...

💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2388783616)
@fanquake
> Building [291ec5e](https://github.com/bitcoin/bitcoin/commit/291ec5e2a876622a051baf6e33a4d4d592e6568e) (on arm64):

Thanks! I explicitly disabled unused deployment tools and a few other features for the `native_qt` package.

💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2388792957)
> @hebasto now I can't build QT with depends anymore ([291ec5e](https://github.com/bitcoin/bitcoin/commit/291ec5e2a876622a051baf6e33a4d4d592e6568e)): https://gist.github.com/Sjors/e42242b27b71c98ee10d60d58a20872a
>
> (since the last time I tested I uninstalled Xcode as part of debugging #30978)

Based on the Qt source code, it appears that the build configuration may depend on Xcode. Could you please confirm whether reinstalling Xcode resolves the issue for you? If so, I will need to apply
...

💬 sipa commented on pull request "net: option to disallow v1 connection on ipv4 and ipv6 peers":
(https://github.com/bitcoin/bitcoin/pull/30951#issuecomment-2388794160)
@vasild That assumes the attacker has sufficient monitoring infrastructure in place to maintain an accurate list of Bitcoin P2P ip:ports. That's obviously not impossible, but it is a significantly larger effort than stateless packet inspection looking for the string "\xf9\xbe\xb4\xd99version\x00\x00\x00\x00\x00".

And yes, very little in BIP324 protects against an attacker deliberately targetting specific individuals - its goal is increasing costs for large-scale monitoring. From that perspect
...

💬 fanquake commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2388854676)
> Thanks! I explicitly disabled unused deployment tools and a few other features for the native_qt package.

I tried building the new branch (a91d53615a7bb1da7ccb59e7876a1e31c94013b7) and got:
```bash
make -C depends HOST=x86_64-apple-darwin qt
<snip>
-- [QtBase] Searching for tool 'Qt6::qvkgen' in package Qt6GuiTools.
-- [QtBase] Qt6::qvkgen was found at /bitcoin/depends/x86_64-apple-darwin/native/./libexec/qvkgen using package Qt6GuiTools.
-- [QtBase] Tool 'Qt6::qtpaths' was found at /
...

💬 jonatack commented on pull request "net: option to disallow v1 connection on ipv4 and ipv6 peers":
(https://github.com/bitcoin/bitcoin/pull/30951#issuecomment-2388870050)
> However, as I pointed out in the linked issue, I am somewhat concerned about a potential future where it becomes harder for other/older software to connect to the network if large swaths move to be v2-only. Because of that, I wonder if it isn't better to make this only apply to outbound connections, and advise people who want more private operation to not listen for inbound connections.

Yes. I may be potentially concept ack here if this proposal adopts the suggestions in https://github.com/
...

💬 jonatack commented on pull request "net: option to disallow v1 connection on ipv4 and ipv6 peers":
(https://github.com/bitcoin/bitcoin/pull/30951#issuecomment-2388883991)
> However, as I pointed out in the linked issue, I am somewhat concerned about a potential future where it becomes harder for other/older software to connect to the network if large swaths move to be v2-only. Because of that, I wonder if it isn't better to make this only apply to outbound connections, and advise people who want more private operation to not listen for inbound connections.

Yes. I may be potentially concept/approach ack here if this proposal adopts the suggestion above / in htt
...

💬 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#issuecomment-2388899694)
> This is a good argument except we do have some strings only known at runtime, notably translations

I chose my wording "known at compile-time" carefully for exactly this reason: they're _known_ at compile-time, we just choose not to implement the (complete) validation logic because it much more complex than just returning an error string. It's not worth arguing semantics too much, but in my view that's very much still a programming error and not something that callsites should be aware of, l
...

💬 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_r1784734716)
> I would at least want to make it clearer that format strings are separate from other parameters

I agree with that, and I'll incorporate your first suggestion (I [don't agree](https://github.com/bitcoin/bitcoin/pull/30928#issuecomment-2388899694) with the translations bit) if I have to force-push, thanks!

✅ mzumsande closed a pull request: "net: Make AddrFetch connections to fixed seeds"
(https://github.com/bitcoin/bitcoin/pull/26114)

💬 mzumsande commented on pull request "net: Make AddrFetch connections to fixed seeds":
(https://github.com/bitcoin/bitcoin/pull/26114#issuecomment-2388967707)
The refactoring commits are pretty annoying to rebase (and probably also to review) - maybe I'll open another version without the refactoring (although I think it makes sense to get the fixed seeds logic out of ThreadOpenConnections).

💬 Sjors commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2388990278)
> Could you please confirm whether reinstalling Xcode resolves the issue for you?

I tried to reinstall Xcode 16.0 from the App Store, but it keeps failing to install. So I downloaded it instead, and started it once. Just mentioning this because it could indicate some other issue with this particular machine.

And ran `make` again on the same commit as above. It built qt6.

Building Bitcoin Core fails, but I'll try that again after your next push.

💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#issuecomment-2388991602)
I've added a commit introducing a delayed set to store transactions that are being reconciled, but won't be added to the sketch should it be constructed at this time. This is to mimic the fanout trickling logic where transactions are only made available to peers if they would have been INVed to them. This was already part of @naumenkogs approach, but was scrapped out when reworking when data is added to the reconciliation set in a22b2364f5942266596c78ad306b1e32aa89aa0f

✅ Mackain closed a pull request: "doc: update IBD requirements in doc/README.md"
(https://github.com/bitcoin/bitcoin/pull/30992)