💬 maflcko commented on issue "Signed Integer Overflow in GetBlockSubsidy at block height 2,147,483,647 (During Epoch 10,227, halving 10,226) could increase the block subsidy, total supply, or potentially halt the network":
(https://github.com/bitcoin/bitcoin/issues/29928#issuecomment-2069002528)
> Addressing bugs only when they become operationally critical may not be the most prudent approach.

I did not say or imply this. I said that fixing an issue in a code base that is expected to stop working in 2106, for an issue that is expected to happen in thousands of years is useless at best. In practice, a fix now would most likely result in a increased runtime load or increased memory load, along with the risk of a consensus and chain split. Also, the C++ language is evolving rapidly ri
...

💬 Sjors commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#issuecomment-2069017109)
@setavenger this PR should not break your node. It just creates an `indexes/silent_payments` directory in your data dir, which you can safely delete later.

💬 Kino1994 commented on issue "Signed Integer Overflow in GetBlockSubsidy at block height 2,147,483,647 (During Epoch 10,227, halving 10,226) could increase the block subsidy, total supply, or potentially halt the network":
(https://github.com/bitcoin/bitcoin/issues/29928#issuecomment-2069021051)
So Bitcoin will cease to function in 2106 if the other issue is not fixed beforehand, making it senseless to consider this one first. Oh! Got it.

Thank you.

💬 Sjors commented on issue "CMake-based build system tracking issue":
(https://github.com/bitcoin/bitcoin/issues/28607#issuecomment-2069038272)
What's next?

It would be useful to have a link to build instructions too, not just the cheat cheat.

In noticed that in your `hebasto/cmake-staging` branch the macOS instructions still assume autotools.

💬 glozow commented on pull request "test: Fix multiprocess CI timeout in p2p_tx_download":
(https://github.com/bitcoin/bitcoin/pull/29926#discussion_r1574527033)
I haven't been able to reproduce this issue and am mostly guessing here, but if it's a `setmocktime` problem, perhaps just do a `node.setmocktime(0)` to reset it a few lines up (shouldn't impact the test except maybe make it take 2sec longer)?

🤔 glozow reviewed a pull request: "test: Fix multiprocess CI timeout in p2p_tx_download"
(https://github.com/bitcoin/bitcoin/pull/29926#pullrequestreview-2014393208)
(cc @theStack, indeed looks like it's from #29827)

💬 glozow commented on pull request "test: Fix multiprocess CI timeout in p2p_tx_download":
(https://github.com/bitcoin/bitcoin/pull/29926#discussion_r1574531190)
I'm not convinced that the +300 would make a difference since the timing for getdata isn't variable like announcements are (which was the case in #28321)

💬 glozow commented on pull request "test: Don't fetch orphan parent when parent is in orphanage":
(https://github.com/bitcoin/bitcoin/pull/29915#discussion_r1574537327)
`wait_for_parent_requests` already checks this, since it requires that the list of getdatas match exactly (nothing additional allowed)?

💬 Sjors commented on issue "Apple Clang 14.0 lacks support for `std::is_eq`":
(https://github.com/bitcoin/bitcoin/issues/29918#issuecomment-2069067883)
I was going to suggest changing `doc/dependencies.md` to clarify the minimum clang version is 14.0.3 rather than 14.0, but #29165 bumps it to 15.0 anyway.

💬 hebasto commented on issue "CMake-based build system tracking issue":
(https://github.com/bitcoin/bitcoin/issues/28607#issuecomment-2069074476)
@Sjors

> What's next?
>
> It would be useful to have a link to build instructions too, not just the cheat cheat (or just add the standard `mkdir build`, `cmake ..` incantations to the cheat cheat).
>
> In noticed that in your `hebasto/cmake-staging` branch the macOS instructions still assume autotools.

WIP :)

Your testing and reviewing https://github.com/hebasto/bitcoin/pull/163 will be much appreciated!

💬 maflcko commented on issue "Apple Clang 14.0 lacks support for `std::is_eq`":
(https://github.com/bitcoin/bitcoin/issues/29918#issuecomment-2069098524)
> I was going to suggest changing `doc/dependencies.md` to clarify the minimum clang version is 14.0.3 rather than 14.0, but #29165 bumps it to 15.0 anyway.

That would not be accurate. Clang 14 is indeed supported, see also https://godbolt.org/z/61seKv74b

The problem here is that Apple Clang 14.0.0 is apparently not based on llvm 14.0.0. I don't have further insights into Apples versioning nonsense, so I can't help here.

💬 Sjors commented on pull request "build: Bump clang minimum supported version to 15":
(https://github.com/bitcoin/bitcoin/pull/29165#issuecomment-2069100533)
Can you add a note to `doc/build-osx.md` that users of macOS < 13 need to install `clang` via homebrew?

My Intel iMac from 2017 runs macOS 13.6.6 and has clang 15.0.0. It's installed at `/usr/bin/clang` so not Homebrew.

Xcode 14.3.1 and newer ship with clang 15.0:
https://en.wikipedia.org/wiki/Xcode#Xcode_11.0_-_14.x_(since_SwiftUI_framework)

The system requirements for that is macOS Ventura 13.0 or later: https://developer.apple.com/documentation/xcode-release-notes/xcode-14_3_1-relea
...

💬 Sjors commented on issue "Apple Clang 14.0 lacks support for `std::is_eq`":
(https://github.com/bitcoin/bitcoin/issues/29918#issuecomment-2069107675)
I see, so Apple clang needs to be >=14.0.3, but for other operating systems 14.0 is fine.

💬 setavenger commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#issuecomment-2069108058)
Ok, I see. Then I'll be able to get to this a bit faster.

💬 maflcko commented on pull request "build: Bump clang minimum supported version to 15":
(https://github.com/bitcoin/bitcoin/pull/29165#issuecomment-2069109399)
> Can you add a note to `doc/build-osx.md` that users of macOS < 13 need to install `clang` via homebrew?

#https://github.com/bitcoin/bitcoin/issues/29918 is an unrelated issue existing on current master, so I think a separate pull request would be more appropriate. Also, I don't have or use macOS/Apple, so someone else creating that pull request would be beneficial.

💬 cbergqvist commented on pull request "test: adds outbound eviction functional tests, updates comment in ConsiderEviction":
(https://github.com/bitcoin/bitcoin/pull/29122#discussion_r1574574102)
nit: Noticed you renamed `test_node` -> `peer` above (based off https://github.com/bitcoin/bitcoin/pull/29122#discussion_r1531465214) but not inside `test_outbound_eviction_blocks_relay_only()`.

(Was diffing against older head (59affd0bf9042d746a6b32739edb0cd45d28d569)).

Might be worth changing once you rebase on top of #29736.

💬 glozow commented on pull request "policy: Allow non-standard scripts with -acceptnonstdtxn=1 (test nets only)":
(https://github.com/bitcoin/bitcoin/pull/29843#issuecomment-2069138726)
concept ACK fwiw

💬 willcl-ark commented on pull request "init: Add option for rpccookie permissions (replace 26088)":
(https://github.com/bitcoin/bitcoin/pull/28167#issuecomment-2069158282)
OK Luke you've persuaded me. I've re-worked this now to accept `-rpccookieperms=[user|group|others]`.

> nit: Recommend augmenting content in doc/init.md to inform of default cookie file permissions.
Example in this commit https://github.com/tdb3/bitcoin/commit/fdd8a4e70632c74c630caaff5cc36da0c309168e (welcome to cherry pick as desired).
bitcoind --help already states default permissions of 400, so the addition to init.md is author's discretion.

Thansk @tdb3, I've taken your suggestion in
...

🚀 glozow merged a pull request: "fuzz: explicitly cap the vsize of RBFs for diagram checks"
(https://github.com/bitcoin/bitcoin/pull/29879)

💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1574610556)
I copied this from the way we do `unique_parents` in orphan parent requests (see the discussion on #19596 about the dynamic memory usage and speed). However I don't mind either way and agree a `std::set` would be simpler, so happy to change if people prefer using a set.