π¬ MarcoFalke commented on issue "Bitcoin Core compiled with Ubuntu Focal gcc-8 immediately segfaults":
(https://github.com/bitcoin/bitcoin/issues/27487#issuecomment-1513361955)
Thanks
(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)
(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
(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);
```
(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
(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.
(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
...
(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.
(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));
```
(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`?
(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.
(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.
(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
(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.
(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.
π¬ achow101 commented on pull request "[24.1] Bump version to 24.1rc2":
(https://github.com/bitcoin/bitcoin/pull/27486#issuecomment-1513478139)
ACK 03a16a1da09f45721c799c719e48350dfe12ae88
(https://github.com/bitcoin/bitcoin/pull/27486#issuecomment-1513478139)
ACK 03a16a1da09f45721c799c719e48350dfe12ae88
π¬ josibake commented on pull request "kernel: chainparams updates for 25.x":
(https://github.com/bitcoin/bitcoin/pull/27482#issuecomment-1513518396)
ACK https://github.com/bitcoin/bitcoin/pull/27482/commits/a2bef805c11a4ab391f4ea635bfde7dd2ec9ce81
verified that I got all the same values on my node for mainnet
(https://github.com/bitcoin/bitcoin/pull/27482#issuecomment-1513518396)
ACK https://github.com/bitcoin/bitcoin/pull/27482/commits/a2bef805c11a4ab391f4ea635bfde7dd2ec9ce81
verified that I got all the same values on my node for mainnet
π jonatack opened a pull request: "p2p: update manual tor/i2p/cjdns mainnet seeds for 25.x"
(https://github.com/bitcoin/bitcoin/pull/27488)
Update the hardcoded P2P network seeds for 25.x after updating the manual seeds (we needed more Tor and CJDNS seeds, in particular) and the generation script as necessary. Previous update was #25853.
Can be tested by following the steps in `contrib/seeds/README.md`.
Tool output:
```
$ python3 makeseeds.py -a asmap-filled.dat -s seeds_main.txt > nodes_main.txt
Loading asmap database "asmap-filled.dat"β¦Done.
Loading and parsing DNS seedsβ¦Done.
IPv4 IPv6 Onion Pass
...
(https://github.com/bitcoin/bitcoin/pull/27488)
Update the hardcoded P2P network seeds for 25.x after updating the manual seeds (we needed more Tor and CJDNS seeds, in particular) and the generation script as necessary. Previous update was #25853.
Can be tested by following the steps in `contrib/seeds/README.md`.
Tool output:
```
$ python3 makeseeds.py -a asmap-filled.dat -s seeds_main.txt > nodes_main.txt
Loading asmap database "asmap-filled.dat"β¦Done.
Loading and parsing DNS seedsβ¦Done.
IPv4 IPv6 Onion Pass
...
π¬ jonatack commented on pull request "p2p: update manual tor/i2p/cjdns mainnet seeds for 25.x":
(https://github.com/bitcoin/bitcoin/pull/27488#issuecomment-1513531812)
Briefly in draft while I run a script to update more manual seeds for reachability/uptime/service bit 1.
(https://github.com/bitcoin/bitcoin/pull/27488#issuecomment-1513531812)
Briefly in draft while I run a script to update more manual seeds for reachability/uptime/service bit 1.
π€ pablomartin4btc reviewed a pull request: "wallet: improve IBD sync time by skipping block scanning prior birth time"
(https://github.com/bitcoin/bitcoin/pull/27469#pullrequestreview-1390605733)
Concept ACK. Light code review, I'd to spend more time with it, which I'll do soon. Good stuff!
(https://github.com/bitcoin/bitcoin/pull/27469#pullrequestreview-1390605733)
Concept ACK. Light code review, I'd to spend more time with it, which I'll do soon. Good stuff!
π¬ fanquake commented on pull request "p2p: update manual tor/i2p/cjdns mainnet seeds for 25.x":
(https://github.com/bitcoin/bitcoin/pull/27488#discussion_r1170366895)
Manually bumping dates in copyright headers is never required
(https://github.com/bitcoin/bitcoin/pull/27488#discussion_r1170366895)
Manually bumping dates in copyright headers is never required