📝 maflcko opened a pull request: "fuzz: Fix wallet_bdb_parser stdlib error matching"
(https://github.com/bitcoin/bitcoin/pull/30169)
The stdlib error string is an implementation detail and can not be relied upon.

Ref: `libc++abi: terminating due to uncaught exception of type std::runtime_error: AutoFile::read: end of file: unspecified iostream_category error`

📝 maflcko opened a pull request: "refactor: Use type-safe time in txorphanage"
(https://github.com/bitcoin/bitcoin/pull/30170)
This allows to remove manual conversions like multiplication by `60`, and uses a type-safe type instead of a raw `int64_t`.

💬 maflcko commented on pull request "[PoC] ci: Add FreeBSD GitHub Actions job":
(https://github.com/bitcoin/bitcoin/pull/30164#issuecomment-2129420150)
@Sjors According to https://github.com/vmactions/freebsd-vm?tab=readme-ov-file#under-the-hood you can use `with_release` to pick one.

💬 Sjors commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2129421039)
The first two commits of https://github.com/bitcoin/bitcoin/pull/26812 would make it possible to test and fuzz how this code interacts with a router.

💬 apulsifer commented on issue "LevelDB read failure: Corruption: block checksum mismatch":
(https://github.com/bitcoin/bitcoin/issues/30159#issuecomment-2129447054)
Took about 20 minutes. They all match.

Fri May 24 12:12:33 UTC 2024
{
"height": 844917,
"bestblock": "000000000000000000017dd5f59b73629f6f88797e90017a9df39c5e435296bf",
"txouts": 181984254,
"bogosize": 13994337434,
"muhash": "b97a3fb13a61e8c889668d064f7ae5408a78ee7e4c6a4fdebedb30ecb2f23378",
"total_amount": 19702649.24256659,
"transactions": 125720653,
"disk_size": 12220356091
}
Fri May 24 12:39:58 UTC 2024

💬 TheCharlatan commented on pull request "indexes: Don't wipe indexes again when continuing a prior reindex":
(https://github.com/bitcoin/bitcoin/pull/30132#issuecomment-2129452748)
Thanks for the reviews @maflcko,

891784ce73a1d911d33b125b66f3856fe6cda56b -> eeea0818c1a20adc5225b98b185953d386c033e0 ([preserveIndexOnRestart_3](https://github.com/TheCharlatan/bitcoin/tree/preserveIndexOnRestart_3) -> [preserveIndexOnRestart_4](https://github.com/TheCharlatan/bitcoin/tree/preserveIndexOnRestart_4), [compare](https://github.com/TheCharlatan/bitcoin/compare/preserveIndexOnRestart_3..preserveIndexOnRestart_4))

* Addressed @maflcko's [comment](https://github.com/bitcoin/bitc
...

✅ zefir-k closed an issue: "prune shall not delete blocks it did not download"
(https://github.com/bitcoin/bitcoin/issues/30163)

💬 TheCharlatan commented on pull request "indexes: Don't wipe indexes again when continuing a prior reindex":
(https://github.com/bitcoin/bitcoin/pull/30132#discussion_r1613414830)
Yeah, I was thinking this might be used for some more regresssion tests, but as it is right now, it is useless. Will remove.

💬 fanquake commented on pull request "[PoC] ci: Add FreeBSD GitHub Actions job":
(https://github.com/bitcoin/bitcoin/pull/30164#issuecomment-2129492145)
> I believe, though we'll have to install clang 15 on it.

Then we should pick something newer, or which won't be a blocker to using a newer Clang. Because soon our minimum Clang will be 16.

🤔 vasild reviewed a pull request: "init: fixes file descriptor accounting"
(https://github.com/bitcoin/bitcoin/pull/30065#pullrequestreview-2076548666)
Approach ACK 55f16f56f4

Might as well squash the last commit 55f16f56f4 `init: Remove FreeBSD workaround to #2695` into some of the previous ones that touch that line.

💬 vasild commented on pull request "init: fixes file descriptor accounting":
(https://github.com/bitcoin/bitcoin/pull/30065#discussion_r1613282956)
```suggestion
// If we are using select instead of poll, our actual limit may be even smaller
```

💬 vasild commented on pull request "init: fixes file descriptor accounting":
(https://github.com/bitcoin/bitcoin/pull/30065#discussion_r1613272500)
In the commit message of a44da681f5 `init: fixes fd accounting regarding poll/select`:

> ... file descriptions limits ...

s/descriptions/descriptors/

💬 vasild commented on pull request "init: fixes file descriptor accounting":
(https://github.com/bitcoin/bitcoin/pull/30065#discussion_r1613459411)
snake_case for variables. I think `available` or `available_fds` is a good name:

```suggestion
available_fds = RaiseFileDescriptorLimit(nMaxConnections + nMinRequiredFds);
```

💬 vasild commented on pull request "init: fixes file descriptor accounting":
(https://github.com/bitcoin/bitcoin/pull/30065#discussion_r1613376297)
An alternative to `max(min())` is [`clamp()`](https://en.cppreference.com/w/cpp/algorithm/clamp):

```suggestion
nMaxConnections = std::clamp(nFD - nMinRequiredFds, 0, nMaxConnections);
```

(feel free to ignore if you don't find it more readable)

💬 vasild commented on pull request "init: fixes file descriptor accounting":
(https://github.com/bitcoin/bitcoin/pull/30065#discussion_r1613414626)
What about giving an `InitError` if the provided value is negative instead of silently clamping it to 0? Surely `-maxconnections=-50` looks like a typo and probably the intention is not to end up with `-maxconnections=0`.

💬 vasild commented on pull request "init: fixes file descriptor accounting":
(https://github.com/bitcoin/bitcoin/pull/30065#discussion_r1613392850)
```suggestion
// Maximum number of connections with other nodes, this accounts for all types of outbounds and inbounds except for manual
```

💬 vasild commented on pull request "init: fixes file descriptor accounting":
(https://github.com/bitcoin/bitcoin/pull/30065#discussion_r1613422626)
according to `doc/developer-notes.md` variables should use snake_case and no need to denote the type in the variable name (`n` for integer): `min_required_fds`.

But more importantly, the name `nMinRequiredFds` is misleading because we accept a smaller number - a bit below we check `if (nFD < MIN_CORE_FDS) return error...`, so it is not "minimum required". I think this variable here should be: `min_required_fds = MIN_CORE_FDS + nBind`, that check should be `if (nFD < min_required_fds) return e
...

🤔 stickies-v reviewed a pull request: "[27.x] Backports and rc1"
(https://github.com/bitcoin/bitcoin/pull/30092#pullrequestreview-2076907949)
ACK cb6def3855427b613357696ba16a431c7964dbcc modulo small release notes fix

All backport commits are clean, except for:
- 77b2321ca03dcbd5f77060510dc8a19e7f4fdfa2 backported from 21b8a14d37c19ce292d5529597e0d52338db48a9: version bumped in https://github.com/bitcoin/bitcoin/pull/29707/ - LGTM

No diff with my local manpage generation.

💬 stickies-v commented on pull request "[27.x] Backports and rc1":
(https://github.com/bitcoin/bitcoin/pull/30092#discussion_r1613471306)
```suggestion
Bitcoin Core version 27.1rc1 is now available from:

<https://bitcoincore.org/bin/bitcoin-core-27.1/test.rc1/>
```

💬 stickies-v commented on pull request "refactor: Use type-safe time in txorphanage":
(https://github.com/bitcoin/bitcoin/pull/30170#issuecomment-2129540582)
Concept ACK