📝 fanquake opened a pull request: "[24.1] Bump version to 24.1rc2"
(https://github.com/bitcoin/bitcoin/pull/27486)
rc1 (tagged > a month ago) was mostly a no-op, due to lack of binaries. While some are up now, [albeit at the wrong URL](https://bitcoincore.org/bin/bitcoin-24.1rc1/), there seems no point continuing the rc1 cycle, when we already have additional changes backported (#27474), and we've also received no bug reports/feedback from anyone who did test.

So bump the version, and cut and rc2.

💬 TheCharlatan commented on pull request "refactor, kernel: Decouple ArgsManager from blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27125#issuecomment-1513335630)
Thank you for the notice,

Rebased 09f950e1f3608e3019e97236969b152e9252a220 -> 908d95e46f15f6b4def9534bc09fcdb3c0a5ef54 ([removeBlockstorageArgs_12](https://github.com/TheCharlatan/bitcoin/tree/removeBlockstorageArgs_12) -> [removeBlockstorageArgs_13](https://github.com/TheCharlatan/bitcoin/tree/removeBlockstorageArgs_13), [compare](https://github.com/TheCharlatan/bitcoin/compare/removeBlockstorageArgs_12..removeBlockstorageArgs_13))

💬 hebasto commented on issue "Replace all of the RecursiveMutex instances with the Mutex ones":
(https://github.com/bitcoin/bitcoin/issues/19303#issuecomment-1513337566)
A shortlist of remaining class members recursive mutexes at 2fa7344aa9bbce8069ebd6bef5f6a22f0b7c5c56:

- [ ] `BanMan::m_cs_banned` in `banman.h` -- #24097

- [ ] `CConnman::m_nodes_mutex` in `net.h`

- [ ] `TxRelay::m_bloom_filter_mutex` in `net_processing.cpp`

- [ ] `TxRelay::m_tx_inventory_mutex` in `net_processing.cpp` -- #24125

- [ ] `BlockManager::cs_LastBlockFile` in `node/blockstorage.h`

- [ ] `FillableSigningProvider::cs_KeyStore` in `script/signingprovider.h`

- [ ] `Ar
...

💬 TheCharlatan commented on pull request "refactor (tidy): Fixes after enable-debug configure":
(https://github.com/bitcoin/bitcoin/pull/27353#issuecomment-1513345473)
Updated 73afe2c3ac87049887e0633bd25c20f809e8090c -> efa1d5bb477dc2093bc06ffded382fdce6bc7703 ([clangTidyDebug_0](https://github.com/TheCharlatan/bitcoin/commits/clangTidyDebug_0) -> [clangTidyDebug_1](https://github.com/TheCharlatan/bitcoin/commits/clangTidyDebug_1), [compare](https://github.com/TheCharlatan/bitcoin/compare/clangTidyDebug_0..clangTidyDebug_1)).

* Addressed @hebasto 's [suggestion](https://github.com/bitcoin/bitcoin/pull/27353#issuecomment-1494267607) for adding the flag to th
...

⚠️ MarcoFalke opened an issue: "Bitcoin Core compiled with Ubuntu Focal gcc-8 immediately segfaults"
(https://github.com/bitcoin/bitcoin/issues/27487)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

segfault

### Expected behaviour

no segfault

### Steps to reproduce

* Fresh install of Ubuntu Focal
* `export DEBIAN_FRONTEND=noninteractive && apt update && apt install curl wget htop git vim ccache -y && git clone https://github.com/bitcoin/bitcoin.git --depth=1 ./bitcoin-core && cd bitcoin-core && apt install build-essential libtool autotools-dev automake pkg-config bsdmainutils py
...

💬 jonatack commented on pull request "sync: introduce a thread-safe generic container and use it to remove a bunch of "GlobalMutex"es":
(https://github.com/bitcoin/bitcoin/pull/25390#issuecomment-1513345887)
Light Approach ACK after reading the code and the excellent review discussions. It looks like this has been progressively honed and improved quite a bit. Will test and review.

💬 MarcoFalke commented on issue "Bitcoin Core compiled with Ubuntu Focal gcc-8 immediately segfaults":
(https://github.com/bitcoin/bitcoin/issues/27487#issuecomment-1513352143)
For reference, it seems to work on Debian Buster (https://packages.debian.org/buster/g++, 8.3) and CentOS 8 ( https://centos.pkgs.org/8/centos-appstream-x86_64/gcc-8.5.0-4.el8_5.x86_64.rpm.html, 8.5). Focal is on 8.4 (https://packages.ubuntu.com/focal/g++-8)

💬 fanquake commented on issue "Bitcoin Core compiled with Ubuntu Focal gcc-8 immediately segfaults":
(https://github.com/bitcoin/bitcoin/issues/27487#issuecomment-1513353311)
Need to double check, but iirc this is due to an issue with the `libstdc++` shipped with Focal being incompatible with GCC 8 (for some `std::filesystem` usage), becuase it's actually from GCC 9.

👍 hebasto approved a pull request: "[24.1] Bump version to 24.1rc2"
(https://github.com/bitcoin/bitcoin/pull/27486#pullrequestreview-1390379134)
ACK 03a16a1da09f45721c799c719e48350dfe12ae88, I have reviewed the code and it looks OK.

> there seems no point continuing the rc1 cycle, when we already have additional changes backported

Agree.

💬 fanquake commented on pull request "refactor (tidy): Fixes after enable-debug configure":
(https://github.com/bitcoin/bitcoin/pull/27353#issuecomment-1513355962)
> Maybe just provide CPPFLAGS=-DBOOST_MULTI_INDEX_ENABLE_SAFE_MODE?

Concept ~0 on making our clang-tidy usage dependant on some Boost define.

💬 MarcoFalke commented on issue "Bitcoin Core compiled with Ubuntu Focal gcc-8 immediately segfaults":
(https://github.com/bitcoin/bitcoin/issues/27487#issuecomment-1513361955)
Thanks

✅ MarcoFalke closed an issue: "Bitcoin Core compiled with Ubuntu Focal gcc-8 immediately segfaults"
(https://github.com/bitcoin/bitcoin/issues/27487)

👍 pinheadmz approved a pull request: "doc: Improve documentation of rpcallowip"
(https://github.com/bitcoin/bitcoin/pull/27480#pullrequestreview-1390394048)
utACK

💬 pinheadmz commented on pull request "doc: Improve documentation of rpcallowip":
(https://github.com/bitcoin/bitcoin/pull/27480#discussion_r1170212348)
nit

```suggestion
argsman.AddArg("-rpcallowip=<ip>", "Allow JSON-RPC connections from specified source. Valid for <ip> are a single IP (e.g. 1.2.3.4), a network/netmask (e.g. 1.2.3.4/255.255.255.0), a network/CIDR (e.g. 1.2.3.4/24), all ipv4 (0.0.0.0/0), or all ipv6 (::/0). This option can be specified multiple times", ArgsManager::ALLOW_ANY, OptionsCategory::RPC);
```

💬 MarcoFalke commented on issue "Replace all of the RecursiveMutex instances with the Mutex ones":
(https://github.com/bitcoin/bitcoin/issues/19303#issuecomment-1513375299)
This may also fix the false-positive https://github.com/bitcoin/bitcoin/blob/2fa7344aa9bbce8069ebd6bef5f6a22f0b7c5c56/test/sanitizer_suppressions/tsan#L15-L16

👍 hebasto approved a pull request: "refactor (tidy): Fixes after enable-debug configure"
(https://github.com/bitcoin/bitcoin/pull/27353#pullrequestreview-1390422025)
ACK 3e76aff9d4709f44d6439cd0cbc2fd6c90cae6ab

> Concept ~0 on making our clang-tidy usage dependant on some Boost define.

What are possible drawbacks here?

In this case the `-DBOOST_MULTI_INDEX_ENABLE_SAFE_MODE` is just a subset of flags being used for debug builds.

💬 mzumsande commented on pull request "blockstorage: Adjust fastprune limit if block exceeds blockfile size":
(https://github.com/bitcoin/bitcoin/pull/27191#discussion_r1170234364)
I think it is necessary, as long as the while condition says
`while (m_blockfile_info[nFile].nSize + nAddSize >= max_blockfile_size)`
it won't terminate if `nAddSize == max_blockfile_size`, because `m_blockfile_info[nFile].nSize` will be 0 if we skip to the next block.

Maybe we could lose the `+ 1` if we'd change the `>=` from the while loop into a `>`, however I really don't want to change this critical code in this PR because it would mean we'd try to squeeze another byte into all block
...

🤔 jonatack reviewed a pull request: "sync: introduce a thread-safe generic container and use it to remove a bunch of "GlobalMutex"es"
(https://github.com/bitcoin/bitcoin/pull/25390#pullrequestreview-1390434676)
Light initial ACK ebfe47e5942fa5e0ce21b44511258b6dfa4cf621 modulo poking around with the thread safety coverage/guarantees to break them. Compiled and ran unit tests at each commit with ARM64 clang 16 (except the scripted-diff ones).

Feel free to ignore the comments below unless you need to repush.

💬 jonatack commented on pull request "sync: introduce a thread-safe generic container and use it to remove a bunch of "GlobalMutex"es":
(https://github.com/bitcoin/bitcoin/pull/25390#discussion_r1170240470)
Style nit, unsure, would it be a good idea to use named args in the `WITH_SYNCED_LOCK` and `SYNCED_LOCK` calls?

```suggestion
WITH_SYNCED_LOCK(g_my_net_addr, /*name=*/p, /*code=*/p->erase(addr));
```

💬 jonatack commented on pull request "sync: introduce a thread-safe generic container and use it to remove a bunch of "GlobalMutex"es":
(https://github.com/bitcoin/bitcoin/pull/25390#discussion_r1170236041)
a49907318fff6d Naming nit, would this be better plural i.e. `g_my_net_addrs`, `g_local_addrs` or `g_my_local_addrs`?