💬 Sjors commented on pull request "Improve display address handling for external signer":
(https://github.com/bitcoin/bitcoin/pull/24313#issuecomment-2059436483)
Rebased after #28981 (I previously already tested on top of that).

Briefly tested with HWI 3.0 as well on a Ledger Nano X.

🤔 tdb3 reviewed a pull request: "netbase: clean up Proxy logging"
(https://github.com/bitcoin/bitcoin/pull/29882#pullrequestreview-2004067074)
CR ACK for fb4cc5f423ce587c1e97377e8afdf92fb4850f59

💬 andrewtoth commented on pull request "index: race fix, lock cs_main while 'm_synced' is subject to change":
(https://github.com/bitcoin/bitcoin/pull/29867#issuecomment-2059451959)
> It is still important to set m_synced = true; after calling Commit to avoid the bug reported https://github.com/bitcoin/bitcoin/issues/29767.

Ahh, I see yes `BlockConnected` is not guarded by `cs_main`, so that could still be called even if we hold `cs_main` inside `Sync`. Thank you for clarifying this for me.

💬 ryanofsky commented on issue "Intermittent issue in test/ipc_tests.cpp Fatal glibc error: pthread_mutex_lock.c:450 (__pthread_mutex_lock_full): assertion failed: e != ESRCH || !robust":
(https://github.com/bitcoin/bitcoin/issues/29889#issuecomment-2059515027)
I wonder if there's any way to get a stack trace. Seeing log output could also be helpful as the test should be logging everything with LogPrintf.

💬 maflcko commented on issue "Intermittent issue in test/ipc_tests.cpp Fatal glibc error: pthread_mutex_lock.c:450 (__pthread_mutex_lock_full): assertion failed: e != ESRCH || !robust":
(https://github.com/bitcoin/bitcoin/issues/29889#issuecomment-2059520640)
I threw away the pod, so the logs are gone, but I wonder why they were not printed to the screen.

Otherwise, if this issue happens on Cirrus or GHA, the logs won't be available either.

📝 mzumsande opened a pull request: "test: fix intermittent failure in p2p_compactblocks_hb.py"
(https://github.com/bitcoin/bitcoin/pull/29893)
Fixes #29860

As a result of node1 receiving a block, it sends out SENDCMPCT messages to some of its peers to update the high-bandwidth status. We need to wait until those are received and processed by the peers to avoid intermittent failures. Before, we'd only wait until all peers have synced with the new block (within `generate`) which is not sufficient.

I could reproduce the failure by adding a `std::this_thread::sleep_for(std::chrono::milliseconds(1000));` sleep to the [net_processing c
...

💬 mzumsande commented on issue "test: Intermittent issue in p2p_compactblocks_hb.py" in relay_block_through assert_equal(status_to, status_from) AssertionError: not([True, False, True, True] == [True, True, True, True])":
(https://github.com/bitcoin/bitcoin/issues/29860#issuecomment-2059535056)
see #29893 for an explanation / proposed fix.

🚀 ryanofsky merged a pull request: "assumeutxo: Fix -reindex before snapshot was validated"
(https://github.com/bitcoin/bitcoin/pull/29726)

👍 instagibbs approved a pull request: "test: fix intermittent failure in p2p_compactblocks_hb.py"
(https://github.com/bitcoin/bitcoin/pull/29893#pullrequestreview-2004196371)
ACK 1ae5b208d339fa984d9caf4fab89b0b2ba9cc197

⚠️ Bolits95 opened an issue: "Pago"
(https://github.com/bitcoin/bitcoin/issues/29894)
ethereum:0x249cA82617eC3DfB2589c4c17ab7EC9765350a18@1/transfer?address=0x8FBA23961A2DeE9628366d3Bc91574Bdad4d9740

✅ fanquake closed an issue: "Pago"
(https://github.com/bitcoin/bitcoin/issues/29894)

:lock: fanquake locked an issue: "Pago"
(https://github.com/bitcoin/bitcoin/issues/29894)

📝 fanquake opened a pull request: "[RFC] guix: remove xz from deps"
(https://github.com/bitcoin/bitcoin/pull/29895)
This isn't (easily) doable:
* Qt only officially ships `.tar.xz` or `.zip`. However it's `.zip` artifacts seem to have been generated on a Windows machine? Which means they are polluted with Windows line endings/similar, which break all of our patches. This could possibly be worked around, but is messy / annoying. The commit here is WIP in that it needs extra handling for all of the tag tarballs being named the same thing. (You can Guix build with `NO_QT=1` to test).
* libxkbcommon has no alt
...

⚠️ maflcko opened an issue: "Intermittent issue in p2p_handshake.py", line 75, in run_test self.test_desirable_service_flags(node, [NODE_NETWORK | NODE_WITNESS], not true after 2400.0 seconds"
(https://github.com/bitcoin/bitcoin/issues/29896)
https://cirrus-ci.com/task/5875023322284032?logs=ci#L3873

```
test 2024-04-16T12:25:11.924000Z TestFramework.p2p (DEBUG): Received message from 0:0: msg_getheaders(locator=CBlockLocator(vHave=[25904390216175313194312162455780078753009692101200952252992871177711616225820, 10439194800033568075353954394040901046437382900627609508673849620975391395575, 40446225147528968444488730747006358517695899943732003474923480031726915305221, 3288163205360417724924785454586033509877330136421489177587429566
...

💬 ryanofsky commented on pull request "[refactor] Check CTxMemPool options in ctor":
(https://github.com/bitcoin/bitcoin/pull/28830#issuecomment-2059591077)
This seems like an improvement, but I think the original suggestion of just adding a `std::error& error` output parameter to the CTxMemPool constructor (or a `util::Result<void>& result` output parameter) would provide a less rigid API than making the CTxMemPool constructor private and requiring every instance to be allocated on the heap and owned by a unique_ptr. This would also allow keeping the fuzz test behavior unchanged. The new fuzz test behavior is probably better, but maybe fuzz testing
...

💬 laanwj commented on pull request "build: add `-Wundef`":
(https://github.com/bitcoin/bitcoin/pull/29876#discussion_r1567731932)
Would it be possible to change these to values instead of defined/undefined? It seems to me that changing these to `ifdef` makes it less safe because you lose the protection added by the warning in the first place. (say, when you forget to include bitcoin-config)

👍 alfonsoromanz approved a pull request: "test: fix intermittent failure in p2p_compactblocks_hb.py"
(https://github.com/bitcoin/bitcoin/pull/29893#pullrequestreview-2004262314)
Tested ACK 1ae5b208d339fa984d9caf4fab89b0b2ba9cc197

⚠️ maflcko opened an issue: "Intermittent issue in p2p_unrequested_blocks.py", line 98, in run_test test_node.send_and_ping(msg_block(blocks_h2[0])) assert self.is_connected AssertionError / AcceptBlock FAILED (time-too-old, block's timestamp is too early) "
(https://github.com/bitcoin/bitcoin/issues/29897)
https://github.com/bitcoin/bitcoin/actions/runs/8689751788/job/23828088498?pr=29877#step:27:584

```
node0 2024-04-15T13:27:36.008782Z [D:\a\bitcoin\bitcoin\src\net_processing.cpp:4743] [ProcessMessage] [net] received block 20076ff9e9d9bace59d7621747b34dfb4af9b2f75ff499017397854f9fefb930 peer=0
node0 2024-04-15T13:27:36.008822Z [D:\a\bitcoin\bitcoin\src\validation.cpp:4145] [AcceptBlockHeader] [validation] AcceptBlockHeader: Consensus::ContextualCheckBlockHeader: 20076ff9e9d9bace59d7621747
...

💬 instagibbs commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1567724841)
would be good to explicitly test that the parent-giver isn't punished in this scenario, and test if parent is consensus-bad it results in something expected.

```
diff --git a/test/functional/p2p_opportunistic_1p1c.py b/test/functional/p2p_opportunistic_1p1c.py
index 603cbf08a9..1c887289dc 100755
--- a/test/functional/p2p_opportunistic_1p1c.py
+++ b/test/functional/p2p_opportunistic_1p1c.py
@@ -164,78 +164,129 @@ class PackageRelayTest(BitcoinTestFramework):
# itself and with a
...

💬 instagibbs commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1567718964)
```Suggestion
# 3. Sender relays the parent. Parent+Child are evaluated as a package and rejected.
```