📝 BrandonOdiwuor opened a pull request: "test: Check object hashes in wait_for_getheaders"
(https://github.com/bitcoin/bitcoin/pull/29318)
Fixes https://github.com/bitcoin/bitcoin/issues/18614

Previously, wait_for_getheaders only looked for the presence of a recent "getheaders" message. Additionally checking the hashstop inside the message should make tests involving wait_for_getheaders more robust.

This PR is progress towards closing issue: https://github.com/bitcoin/bitcoin/issues/18614 after PR: https://github.com/bitcoin/bitcoin/pull/18690 (merged) which introduced this check for wait_for_getdata

👍 fanquake approved a pull request: "build: Pass sanitize flags to instrument `libsecp256k1` code"
(https://github.com/bitcoin/bitcoin/pull/28875#pullrequestreview-1844052794)
ACK cbea49c0d32badb975fbf22d44f8e25cc7972af7

💬 mzumsande commented on pull request "validation: fix misleading checkblockindex comments":
(https://github.com/bitcoin/bitcoin/pull/29299#issuecomment-1910472518)
Changed the title as suggested.

> I also think it would be good to split up the assert and make it stricter, so feel free to use the code from [#29261 (comment)](https://github.com/bitcoin/bitcoin/issues/29261#issuecomment-1908513362)

The suggestion looks good to me just by reading the code, but I want to test it more before including it. Will either update the PR in a few days, or alternatively (I don't mind either way) that could be a follow-up to this trivial doc-only PR.

💬 stratospher commented on pull request "test: p2p: check disconnect due to lack of desirable service flags":
(https://github.com/bitcoin/bitcoin/pull/29279#discussion_r1466564903)
358561e: nit: s/0/`NODE_NONE`

🤔 stratospher reviewed a pull request: "test: p2p: check disconnect due to lack of desirable service flags"
(https://github.com/bitcoin/bitcoin/pull/29279#pullrequestreview-1844051383)
Concept ACK! i think flaky tests are possible because disconnection could happen before [`p2p_conn.wait_for_connect()`](https://github.com/bitcoin/bitcoin/blob/7699a1aab8fa2a7a602391025f3cfcde29ff2613/test/functional/test_framework/test_node.py#L728) in `add_outbound_connection()`?

🚀 fanquake merged a pull request: "depends: Do not override `CFLAGS` when building SQLite with `DEBUG=1`"
(https://github.com/bitcoin/bitcoin/pull/29287)

💬 Sjors commented on pull request "Support self-hosted Cirrus workers on forks (and multi-user)":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1466573331)
I installed Podman 4.8.3 from source on my machine. However I don't understand what you mean by `run --replace`. It seems we're only using podman in the `RESTART_CI_DOCKER_BEFORE_RUN`, but still call `docker run` everywhere. Can we just swap `docker run` with `podman run` or is it more complicated?

💬 fanquake commented on pull request "Support self-hosted Cirrus workers on forks (and multi-user)":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1466579076)
I don't have docker installed on any machine I use for CI. Only podman, and a docker -> podman alias (podman-docker).

💬 fanquake commented on pull request "ci: Use DEBUG=1 in depends for MSAN jobs":
(https://github.com/bitcoin/bitcoin/pull/27495#discussion_r1466585572)
Pushed `release/18.x` now to see what breaks. Will switch to the tag once it's available.

💬 ryanofsky commented on pull request "wallet: guard against dangling to-be-reverted db transactions":
(https://github.com/bitcoin/bitcoin/pull/29253#issuecomment-1910522489)
> if there is a failure during the WalletBatch destruction (independently on if it is due to a fatal or transient and recoverable error, like a busy db or a loss of the db connection)

Yes I've started reviewing both PR's and the code changes seem reasonable, I just don't understand what motivated this PR, because "failure during the WalletBatch destruction" seems like something I would never expect to happen unless someone modified the code or manually corrupted the data on disk. So I'm wond
...

💬 real-or-random commented on pull request "build: Pass sanitize flags to instrument `libsecp256k1` code":
(https://github.com/bitcoin/bitcoin/pull/28875#discussion_r1466598461)
My take on `SECP_CFLAGS` is that it's an internal of the libsecp256k1 build system, and users are not supposed to set it (and are not guaranteed that it does anything meaningful). The proper user variable is just `CFLAGS`.

You could either export `CFLAGS="-g $SANITIZER_CFLAGS"` temporarily, or perhaps add a CFLAGS arg to `ac_configure_args`.

We could also expose the `SECP_CFLAGS` "officially" in the lib. Then it should probably be renamed to `SECP256K1_CFLAGS`. But I think it's a bit arbi
...

💬 real-or-random commented on pull request "build: Pass sanitize flags to instrument `libsecp256k1` code":
(https://github.com/bitcoin/bitcoin/pull/28875#discussion_r1466601203)
Serious question: What is this line supposed to print, given that there are also `ZMQ_CFLAGS`, `BDB_CFLAGS` and `EVENT_CFLAGS`? One could say that the libsecp256k1 configure output already prints its CFLAGS, so there's no need to cover that here, but I don't know because I don't know what the purpose of this line is. Where else do we need CFLAGS?

💬 fanquake commented on pull request "build: Pass sanitize flags to instrument `libsecp256k1` code":
(https://github.com/bitcoin/bitcoin/pull/28875#discussion_r1466607662)
I was thinking about the best way to handle this, and my current take was that given this is only when we are using sanitizers, and we are a somewhat non-standard/more-tightly-coupled users of secp in any case, I wasn't too worried. Happy for any other approach that achieves the same thing, but also don't want secp to expose additional (non-standard) flags just for us.

⚠️ sdaftuar opened an issue: "Cluster mempool, CPFP carveout, and V3 transaction policy"
(https://github.com/bitcoin/bitcoin/issues/29319)
Opening an issue for high-level discussion, as the PR that implements this has gotten difficult to follow.

### Cluster mempool

Work is underway to [redesign the mempool](https://delvingbitcoin.org/t/an-overview-of-the-cluster-mempool-proposal/393/1) with different topology constraints on the transaction graph than exist today. I originally described this proposal in a github issue (#27677), and have shared a draft implementation (#28676). In brief, with a new mempool design we could simu
...

💬 hebasto commented on issue "Unable to open wallet UI with ubuntu 23.10":
(https://github.com/bitcoin/bitcoin/issues/29311#issuecomment-1910533393)
On Ubuntu 23.10, I've installed the Bitcoin Core 26.0 from the "latest/stable" Snap channel.

> The terminal output of the command line
> `Fontconfig warning: FcPattern object weight does not accept value [0 205) [1] 824648 segmentation fault (core dumped) env /snap/bin/bitcoin-core.qt %u`

Cannot reproduce it.

> Hello Thank you this command line works [keshavbhatt/olivia#95 (comment)](https://github.com/keshavbhatt/olivia/issues/95#issuecomment-774747492)

Indeed.

> Close or move
...

💬 sdaftuar commented on issue "Cluster mempool, CPFP carveout, and V3 transaction policy":
(https://github.com/bitcoin/bitcoin/issues/29319#issuecomment-1910540024)
cc: @TheBlueMatt @rustyrussell @Roasbeef @t-bast

💬 real-or-random commented on pull request "build: Pass sanitize flags to instrument `libsecp256k1` code":
(https://github.com/bitcoin/bitcoin/pull/28875#discussion_r1466616567)
> I wasn't too worried.

Yep, I agree, any approach is ok in the end. My feeling is just that setting `SECP_CFLAGS` is not the cleanest choice and more likely than my other suggestions to cause breaks in the future.

💬 Retropex commented on pull request "Add a `-permitbarepubkey` option":
(https://github.com/bitcoin/bitcoin/pull/29309#issuecomment-1910542474)
tACK https://github.com/vostrnad/bitcoin/commit/8c1114aa61c46e58efb44368b5428d3ccb65f9d0

Steps to test:

1. Build [vostrnad branch](https://github.com/vostrnad/bitcoin/tree/permitbarepubkey)
1. Start a regtest with `-chain=regtest`
1. Create a wallet with `bitcoin-cli createwallet`
1. Get address with `bitcoin-cli getnewaddress`
1. Generate blocks to get bitcoins `generatetoaddress 500 <address obtained previously>`
1. Create a P2PK tx, this [repository](https://github.com/dianerey/bec
...

💬 Sjors commented on pull request "Support self-hosted Cirrus workers on forks (and multi-user)":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1466618909)
Mmm, I tried something similar in de26cbc79230f64d7747b8b88f4dee71ebd37e1b by adding a `DOCKER_RUN` variable and setting it to `podman run --replace`. But this fails with `Error: short-name resolution enforced but cannot prompt without a TTY`. Are you passing any other arguments in your alias?

🤔 pablomartin4btc reviewed a pull request: "debugwindow: update session ID tooltip"
(https://github.com/bitcoin-core/gui/pull/788#pullrequestreview-1844140068)
Concept ACK

nit: @MarnixCroes, maybe you can add the screenshot for transport v1 and session ID is empty/ not shown, if possible, even this PR doesn't affect that logic.