Bitcoin Core Github
43 subscribers
122K links
Download Telegram
🤔 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.
💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1574612750)
Ah, forgot to remove the comment. We need to make time parameterizable across `TxOrphanage` members to test order, so not testing / saving for followup.
maflcko closed a pull request: "lint: Refactor lint checks to reuse exclusion logic; Support running individual lint checks in lint runner"
(https://github.com/bitcoin/bitcoin/pull/29744)
💬 maflcko commented on pull request "lint: Refactor lint checks to reuse exclusion logic; Support running individual lint checks in lint runner":
(https://github.com/bitcoin/bitcoin/pull/29744#issuecomment-2069237590)
Closing for now per previous discussion. Feel free to create a new pull request(s) with the behavior changes and an accurate description in your own words. Please try to avoid the use of the word "refactor" for bugfixes or behavior changes.
💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1574632413)
I think it's pretty common to name a temporary data structure based on its intended result. But ok, I've changed it to `iters` now.
💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1574634137)
Ah that was supposed to be another log, should be fixed now