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

💬 TheCharlatan commented on pull request "doc: add LLVM instruction for macOS < 13":
(https://github.com/bitcoin/bitcoin/pull/29934#issuecomment-2069769652)
Will test shortly.

💬 ismaelsadeeq commented on pull request "doc: add LLVM instruction for macOS < 13":
(https://github.com/bitcoin/bitcoin/pull/29934#issuecomment-2069796578)
Concept ACK, I ran into this issue sometime ago

💬 BrandonOdiwuor commented on pull request "doc: explain what the wallet password does":
(https://github.com/bitcoin/bitcoin/pull/28974#discussion_r1574923164)
removed

💬 BrandonOdiwuor commented on pull request "doc: explain what the wallet password does":
(https://github.com/bitcoin/bitcoin/pull/28974#discussion_r1574923762)
fixed:summarised further

💬 maflcko commented on pull request "test: Fix multiprocess CI timeout in p2p_tx_download":
(https://github.com/bitcoin/bitcoin/pull/29926#issuecomment-2069822890)
See also the current test failure, which remains: https://github.com/bitcoin/bitcoin/pull/29926/checks?check_run_id=24105987877

💬 achow101 commented on pull request "ZMQ: Support UNIX domain sockets":
(https://github.com/bitcoin/bitcoin/pull/27679#issuecomment-2069827639)
ACK 21d0e6c7b7c7af7f6e54a45829b4fbfba6923b86

💬 instagibbs commented on pull request "test: Fix intermittent timeout in p2p_tx_download.py":
(https://github.com/bitcoin/bitcoin/pull/29933#discussion_r1574936864)
> Should I just put a comment here # Avoid inbounds because they are not needed and slow down the test?

not really, doesn't add anything, so leave it as is

💬 instagibbs commented on pull request "test: Fix intermittent timeout in p2p_tx_download.py":
(https://github.com/bitcoin/bitcoin/pull/29933#issuecomment-2069846036)
makes sense

ACK fa6c300a9926a1d35fdd0a80f59ea39769bd2596

💬 fjahr commented on pull request "test: Fix multiprocess CI timeout in p2p_tx_download":
(https://github.com/bitcoin/bitcoin/pull/29926#issuecomment-2069859823)
> Which indicates that the problem is during shutdown, _after_ the test, not in the test.
>
> So I don't think adding mocktime will fix it. Maybe you were running into a different issue, or are trying to fix a different bug?

Hm, I didn't see `Stopping nodes` being printed so I am not convinced it's during shutdown but maybe I am just not familiar enough with the test framework. I am closing since it seems even with the +30 it's still failing and it seems duplicate #29933 might have a bette
...

✅ fjahr closed a pull request: "test: Fix multiprocess CI timeout in p2p_tx_download"
(https://github.com/bitcoin/bitcoin/pull/29926)