📝 maflcko converted_to_draft a pull request: "test: Non-Shy version sender"
(https://github.com/bitcoin/bitcoin/pull/30453)
After `add_outbound_p2p_connection`, the test framework normally sends a version message only in reply to a received version. This is fine, but the protocol does not require this and tolerates a version to be sent earlier.

Add a test to check this is indeed tolerated.

💬 instagibbs commented on pull request "locks: introduce mutex for tx download, flush rejection filters once per tip change":
(https://github.com/bitcoin/bitcoin/pull/30111#issuecomment-2230965781)
reACK c85accecafc20f6a6ae94bdf6cdd3ba9747218fd

reviewed via `git range-diff master 17d41e7 c85acce`

💬 maflcko commented on pull request "test: Non-Shy version sender":
(https://github.com/bitcoin/bitcoin/pull/30453#issuecomment-2230968900)
Made a draft for now, due to failing CI. Also, edited OP to remove the sentence that implied this was a regression test for a known bug. (I don't think this checks for a previously known bug)

<!--

Arguably, https://github.com/bitcoin/bitcoin/pull/30453#issuecomment-2230681886 just shows a bug in the test framework, because `p2p_conn.wait_for_verack()` should also wait on the version msg being received from the node first.

So what the test checks is that the outbound version is sent bef
...

🤔 furszy reviewed a pull request: "index: Check all necessary block data is available before starting to sync"
(https://github.com/bitcoin/bitcoin/pull/29770#pullrequestreview-2180361000)
> > It would be nice to implement [81638f5](https://github.com/bitcoin/bitcoin/commit/81638f5d42b841fb327393974320ca47db39eb09) differently, without adding the `<chain.h>` dependency to all index headers. This dependency provides access to node internal types that indexes running on other processes (in the future) will not know about.
>
> Should not be too complicated to get rid of the dependency. I will draft something, maybe I'll just give @maflcko s [suggestion](https://github.com/bitcoin/
...

🚀 ryanofsky merged a pull request: "kernel: De-globalize static validation variables"
(https://github.com/bitcoin/bitcoin/pull/30425)

💬 Sjors commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#issuecomment-2231054406)
Getting a mismatch again. This time for block 716120 (00000000000000000009bc9db59a266e367f54fdd4cda7b5ddb31f6ae9723083).

```
bitcoin-cli getsilentpaymentblockdata 00000000000000000009bc9db59a266e367f54fdd4cda7b5ddb31f6ae9723083 | jq '.bip352_tweaks | sort'
[
"031799df7770cfdabe460e47f4571fe4ded7d7a7922afa0f5ad91753473269cdbb",
"0323fefb4e4bf637d7819c2030996dbe88d97c6a2f5708841df0e65fafd32c5a0a",
"03b2bda3a513123fe810cb1e65cd8a71a2495ea9229c50e4acc8d4b5baa51b4147",
"03cdc8c9f07d
...

🤔 hebasto reviewed a pull request: "build: use `-no_exported_symbols` on macOS"
(https://github.com/bitcoin/bitcoin/pull/29072#pullrequestreview-2180434936)
Tested 81d4dc8e8739df0e9a8e92f55071733f6500617b on macOS 14.5 Sonoma (Apple Silicon):

- the master branch @ 9c5cdf07f30f816cd134e2cd2dca9c27ef7067a5:
```
% nm --extern-only src/bitcoind | wc -l
579
```
- this PR:
```
% nm --extern-only src/bitcoind | wc -l
537
```

I don't know the reasons, but `bitcoind` still exports
```
0000000100000000 T __mh_execute_header
```

The new `-no_exported_symbols` linker flag is
...

💬 theStack commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#discussion_r1679523217)
> Can you retest after todays rebase?

Sure. Retested on commit d0019f290ab67ab4190274bb325bfc29bbfcea56, can confirm that the BIP352 indexer on signet runs through (up to current block 204598) now without crashing.

💬 fanquake commented on pull request "build: use `-no_exported_symbols` on macOS":
(https://github.com/bitcoin/bitcoin/pull/29072#issuecomment-2231067816)
> I don't know the reasons, but bitcoind still exports

That is expected. See the PR description.

> The new -no_exported_symbols linker flag is not printed in the configure summary.

That's expected. We don't print the RE LD flags.

💬 mzumsande commented on pull request "net: fix race condition in self-connect detection":
(https://github.com/bitcoin/bitcoin/pull/30394#issuecomment-2231073104)
I didn't find a reference in a BIP (there isn't any for the general version message flow as far as I can see) and maybe "violation" is too strong, but any message received before the version would be ignored by bitcoin core peers:
https://github.com/bitcoin/bitcoin/blob/46878326808f643f08ec3f69dcec5c8fafeb7b59/src/net_processing.cpp#L3889-L3893
So this is clearly not the intended way the protocol should work.

👍 theuni approved a pull request: "build: use `-no_exported_symbols` on macOS"
(https://github.com/bitcoin/bitcoin/pull/29072#pullrequestreview-2180454092)
utACK 81d4dc8e8739df0e9a8e92f55071733f6500617b

👍 hebasto approved a pull request: "build: use `-no_exported_symbols` on macOS"
(https://github.com/bitcoin/bitcoin/pull/29072#pullrequestreview-2180461920)
ACK 81d4dc8e8739df0e9a8e92f55071733f6500617b.

> > The new -no_exported_symbols linker flag is not printed in the configure summary.
>
> That's expected. We don't print the RE LD flags.

Not related to this PR changes, but that's odd as it undermines the summary's goal.

🚀 fanquake merged a pull request: "build: use `-no_exported_symbols` on macOS"
(https://github.com/bitcoin/bitcoin/pull/29072)

💬 mzumsande commented on pull request "locks: introduce mutex for tx download, flush rejection filters once per tip change":
(https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1679560788)
Makes sense! In `net.cpp`, the `AssertLockNotHeld(x)` / `LOCK(x)` pattern is so common that I didn't even consider the possibility that this could be on purpose.

📝 hebasto opened a pull request: "build: Introduce CMake-based buid system"
(https://github.com/bitcoin/bitcoin/pull/30454)
This PR introduces a new CMake-based build system, which is a drop-in replacement for the current Autotools-based build system.

As discussed during the recent CoreDev meetup in April, the switch from Autotools to CMake is intended to happen as soon as possible after branching 28.x off, which means that 29.0 will be built using CMake.

This PR branch is essentially the [staging branch](https://github.com/hebasto/bitcoin/tree/cmake-staging), with every change reviewed and tested by a group of
...

✅ hebasto closed a pull request: "[DO NOT MERGE] cmake: Migrate CI scripts to CMake-based build system -- WIP"
(https://github.com/bitcoin/bitcoin/pull/29790)

💬 hebasto commented on pull request "[DO NOT MERGE] cmake: Migrate CI scripts to CMake-based build system -- WIP":
(https://github.com/bitcoin/bitcoin/pull/29790#issuecomment-2231130195)
Closing. See https://github.com/bitcoin/bitcoin/pull/30454.

💬 maflcko commented on pull request "build: Introduce CMake-based buid system":
(https://github.com/bitcoin/bitcoin/pull/30454#issuecomment-2231166194)
> The only differences from the staging branch are:

I'd say the section can be removed (or moved to a separate comment), because when this will be merged, I presume many more differences will accumulate. Even looking at the outstanding ports (https://github.com/bitcoin/bitcoin/issues?q=label%3A%22Needs+CMake+port%22+is%3Aclosed) right now, there are some.

💬 theStack commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#issuecomment-2231180251)
> Getting a mismatch again. This time for block 716120 (00000000000000000009bc9db59a266e367f54fdd4cda7b5ddb31f6ae9723083).

Is there some easy way to find out the exact txs for which the tweak mismatches happen? (`getsilentpaymentblockdata` doesn't seem to return txids...) Would interesting to see if there is anything special/different in those (e.g. uncommon spending types).

💬 maflcko commented on issue "Fuzzing related configuration/build options can be improved":
(https://github.com/bitcoin/bitcoin/issues/30318#issuecomment-2231182809)
However, it would be good, to implement this before https://github.com/bitcoin/bitcoin/pull/30454 is merged. Otherwise there will be two breaking changes for each fuzz infra deployment.