💬 Sjors commented on pull request "build: Bump clang minimum supported version to 15":
(https://github.com/bitcoin/bitcoin/pull/29165#issuecomment-2069559533)
My remark is orthogonal to #29918. macOS < 13 doesn't have clang 15. They also don't have clang 14, which was missed in #29208.

There's need to make a separate pull request for a single sentence:

At the bottom of step 3:

```md
On macOS < 13 you additionally need to install a more recent version of clang:

brew install clang
```

💬 fjahr commented on pull request "test: Fix multiprocess CI timeout in p2p_tx_download":
(https://github.com/bitcoin/bitcoin/pull/29926#discussion_r1574814671)
Had another failure, so I will add the +300 back and see if that is a permanent fix.

💬 maflcko commented on pull request "test: Fix intermittent timeout in p2p_tx_download.py":
(https://github.com/bitcoin/bitcoin/pull/29933#issuecomment-2069568540)
> I can see the `self.shutdown()` traceback in this one: https://api.cirrus-ci.com/v1/task/6740372997537792/logs/ci.log

Thanks, in this case it seems the issue is transactions relayed to the extraneous peers. I've edited the pull request description.

To see what message is the cause for the block, you can inspect the full combined log. In this example:


```
...
[0;36m test 2024-04-19T18:08:46.732000Z TestFramework.p2p (DEBUG): Received message from 127.0.0.1:17361: msg_tx(tx=CTrans
...

💬 maflcko commented on pull request "build: Bump clang minimum supported version to 15":
(https://github.com/bitcoin/bitcoin/pull/29165#issuecomment-2069591081)
> There's need to make a separate pull request for a single sentence:

I don't think this is enough. Is it not required to specify the CC and CXX to `./configure`? Something like `CC=$(brew --prefix llvm)/bin/clang CXX=$(brew --prefix llvm)/bin/clang++`? Or does brew overwrite the clang system compiler?

Another reason to fix this in a separate pull is to have the fix in before this pull is merged (which at this point is unclear when it can happen).

💬 maflcko commented on pull request "test: Fix multiprocess CI timeout in p2p_tx_download":
(https://github.com/bitcoin/bitcoin/pull/29926#issuecomment-2069610577)
> Example failure: https://cirrus-ci.com/task/5622109341220864

If I download https://api.cirrus-ci.com/v1/task/5622109341220864/logs/ci.log, I get

```
2024-04-21T00:10:09.801000Z TestFramework.utils (ERROR): wait_until() failed. Predicate: ''''
wait_until_helper_internal(lambda: not self.network_event_loop.is_running(), timeout=timeout)
'''
[node 1] Cleaning up leftover process
[node 0] Cleaning up leftover process


[1mstderr:
[0mTraceback (most recent call last):
Fi
...

📝 Sjors opened a pull request: "doc: add LLVM instruction for macOS < 13"
(https://github.com/bitcoin/bitcoin/pull/29934)
#29208 bumped clang to 14, which users of old macOS versions need to install manually. This PR adds instructions.

Xcode 14.3.1 ships clang 14.0.3:
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-release-notes#

Homebrew itself officially supports macOS 12 or later, but _may_ still work on macOS 11: https://docs.brew.s
...

💬 instagibbs commented on pull request "test: Fix intermittent timeout in p2p_tx_download.py":
(https://github.com/bitcoin/bitcoin/pull/29933#discussion_r1574874712)
Took me way too long to realize it's the `fill_mempool` function causing the sync timeout specifically. Can a note be left here to say why we're not making additional inbounds?

💬 Sjors commented on pull request "build: Bump clang minimum supported version to 15":
(https://github.com/bitcoin/bitcoin/pull/29165#issuecomment-2069707168)
Done in #29934.

Correction to version support above: clang 15.0 is shipped with Xcode 15, not 14. But its system requirements are fairly similar, macOS 13.5 or better: https://developer.apple.com/documentation/xcode-release-notes/xcode-15-release-notes

I don't have a macOS 11 or 12 machine to actually test if it still works. Given that homebrew dropped explicit support for macOS 11 I don't think we need to worry about that version, but it would be nice if someone can test if they can still
...

💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1574879185)
The carve-out, by definition, doesn't have support in (sub)package contexts. Specific suggestions to clarify are welcome of course.

💬 maflcko commented on pull request "test: Fix intermittent issue in p2p_handshake.py":
(https://github.com/bitcoin/bitcoin/pull/29898#discussion_r1574881334)
Ah, you are right. Sorry for the wrong comment.

💬 maflcko commented on pull request "test: Fix intermittent issue in p2p_handshake.py":
(https://github.com/bitcoin/bitcoin/pull/29898#issuecomment-2069717235)
lgtm ACK 6b02c11d667adff24daf611f9b14815d27963674

💬 maflcko commented on pull request "test: Fix intermittent issue in p2p_handshake.py":
(https://github.com/bitcoin/bitcoin/pull/29898#issuecomment-2069719031)
Fixes #29896

💬 Sjors commented on pull request "doc: add LLVM instruction for macOS < 13":
(https://github.com/bitcoin/bitcoin/pull/29934#issuecomment-2069719729)
I tested these instructions by installing llvm 14.0.6 on macOS 14.4.1.

I don't have a macOS 11 or 12 machine, so have not tested those. So there may be other issues, which we'll just have to find out if someone runs into them.

💬 pablomartin4btc commented on pull request "Bugfix on TransactionsView - Disable if privacy mode is set during wallet selection":
(https://github.com/bitcoin-core/gui/pull/815#issuecomment-2069731815)
> This feels like the wrong place to fix it. Why not inside `setWalletActionsEnabled` (or whatever is enabling it to begin with)?

Yeah, it makes more sense, I'll rework it. Thanks!

💬 maflcko commented on pull request "build: Bump clang minimum supported version to 15":
(https://github.com/bitcoin/bitcoin/pull/29165#issuecomment-2069737554)
Thanks, I'll rebase once the other pull is merged.

💬 BrandonOdiwuor commented on pull request "Wallet: (Refactor) GetBalance to calculate used balance":
(https://github.com/bitcoin/bitcoin/pull/29062#discussion_r1574895998)
Fixed: updated `GetBalance` to include `m_mine_used`

💬 maflcko commented on pull request "test: Fix intermittent timeout in p2p_tx_download.py":
(https://github.com/bitcoin/bitcoin/pull/29933#discussion_r1574896587)
I think the real reason would be "because they are not needed". The secondary reason would be that the test will be faster on slower hardware. The final reason would be that this avoids intermittent test failures/timeouts. However, there are other ways to avoid the intermittent test failures, so I am not sure if this is the right place to document it.

Should I just put a comment here `# Avoid inbounds because they are not needed and slow down the test`?

👍 maflcko approved a pull request: "doc: add LLVM instruction for macOS < 13"
(https://github.com/bitcoin/bitcoin/pull/29934#pullrequestreview-2015004975)
lgtm

💬 maflcko commented on pull request "doc: add LLVM instruction for macOS < 13":
(https://github.com/bitcoin/bitcoin/pull/29934#discussion_r1574898821)
I'd say to use the latest llvm. However, I presume you wanted to avoid a too recent llvm breaking on the old operating systems? Seems fine either way.

💬 Sjors commented on pull request "doc: add LLVM instruction for macOS < 13":
(https://github.com/bitcoin/bitcoin/pull/29934#discussion_r1574902679)
> a too recent llvm breaking on the old operating systems

Exactly. Either llvm itself or Homebrew's build scripts might not work for some reason, so this seems safer.