Bitcoin Core Github
44 subscribers
121K links
Download Telegram
Sjors closed a pull request: "depends: suggest GNU patch for macOS <= 13"
(https://github.com/bitcoin/bitcoin/pull/29814)
👍 ryanofsky approved a pull request: "index: race fix, lock cs_main while 'm_synced' is subject to change"
(https://github.com/bitcoin/bitcoin/pull/29867#pullrequestreview-2003980602)
Code review ACK 99afb9d15a08d2f46739f4d2b66c63dbabd7a44e. I think I have the opposite opinion as andrewtoth, and that original bugfix c395b26df37b3839a9c76abbd4d5fb48e79b3208 is more direct and simple than current 99afb9d15a08d2f46739f4d2b66c63dbabd7a44e since the original fix leaves the Rewind code alone.

re: https://github.com/bitcoin/bitcoin/pull/29867#issuecomment-2059115444

> Not a blocker, but I think it would be cleaner if this PR was the following 3 commits:
>
> ```
> revert bb
...
💬 instagibbs commented on pull request "fuzz: explicitly cap the vsize of RBFs for diagram checks":
(https://github.com/bitcoin/bitcoin/pull/29879#issuecomment-2059368920)
pushed a fixup since I think it could have resulted in UB when selecting direct conflicts from the `mempool_txs` vector.
👍 stickies-v approved a pull request: "test: Fix failing univalue float test"
(https://github.com/bitcoin/bitcoin/pull/29892#pullrequestreview-2004014087)
ACK fa4c69669e079c38844ecea1ad3394aae3702ae1 , thanks for fixing!
💬 Sjors 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-2059400017)
@ryanofsky's suggestion looks good to me as well.
💬 maflcko commented on pull request "test: Fix failing univalue float test":
(https://github.com/bitcoin/bitcoin/pull/29892#issuecomment-2059403538)
For testing, one can use gcc-13 (or later) on i386.

For example, running the centos CI task on `fedora:40`:

```diff
diff --git a/ci/test/00_setup_env_i686_centos.sh b/ci/test/00_setup_env_i686_centos.sh
index 5f8391c..dc80e98 100755
--- a/ci/test/00_setup_env_i686_centos.sh
+++ b/ci/test/00_setup_env_i686_centos.sh
@@ -12,6 +12,7 @@ export CI_IMAGE_NAME_TAG="quay.io/centos/amd64:stream9"
export CI_BASE_PACKAGES="gcc-c++ glibc-devel.x86_64 libstdc++-devel.x86_64 glibc-devel.i686 libs
...
💬 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![myaddr.png](https://github.com/bitcoin/bitcoin/assets/166213649/0dc72fd5-44a6-4015-ab06-868b8c3ca302)
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
...