⚠️ 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`?

💬 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_r1170242796)
Nice cleanup.

💬 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_r1170250884)
373df61bc56 Style nit, unsure, would something like `local_instance` or `local_name` read better than `name` if callers use named args? Feel free to ignore.

💬 kristapsk commented on pull request "Allow configuring target block time for a signet":
(https://github.com/bitcoin/bitcoin/pull/27446#issuecomment-1513473340)
Concept ACK

💬 MarcoFalke commented on pull request "Bump python minimum version to 3.8":
(https://github.com/bitcoin/bitcoin/pull/27483#issuecomment-1513477050)
Unrelated: Looks like there are a few bugs with GCC libstdc++-9:

* https://github.com/bitcoin/bitcoin/issues/27487#issuecomment-1513353311
* clang-8: `./util/bitdeque.h:240:5: error: exception specification of explicitly defaulted move constructor does not match the calculated one`

So I worked around them.