💬 l0rinc commented on pull request "test: don't throw from the destructor of DebugLogHelper":
(https://github.com/bitcoin/bitcoin/pull/33388#discussion_r2349921508)
I admit that I don't understand this well enough, why reconnect before deleting the callback? Shouldn't we delete the callback first, then call `noui_reconnect()`?

💬 l0rinc commented on pull request "test: don't throw from the destructor of DebugLogHelper":
(https://github.com/bitcoin/bitcoin/pull/33388#discussion_r2349932872)
nit: we can simplify it a bit (+ pass by value mentioned earlier):
```suggestion
explicit DebugLogHelper(std::string message, MatchFn match = [](auto){ return true; });
```

💬 l0rinc commented on pull request "test: don't throw from the destructor of DebugLogHelper":
(https://github.com/bitcoin/bitcoin/pull/33388#discussion_r2349918985)
`LogError` would flush after this line, so we could safely abort after (since `abort` doesn't call any destructors). Can you confirm that `cerr` will also flush safely?

🤔 enirox001 reviewed a pull request: "test: Prevent disk space warning during node_init_tests"
(https://github.com/bitcoin/bitcoin/pull/33391#pullrequestreview-3226283052)
utACK bdf01c6

💬 hodlinator commented on pull request "coins: warn on oversized `-dbcache`":
(https://github.com/bitcoin/bitcoin/pull/33333#discussion_r2349949588)
nit: Comment might fit better above the `CalculateDbCacheBytes` function definition?

🤔 hodlinator reviewed a pull request: "coins: warn on oversized `-dbcache`"
(https://github.com/bitcoin/bitcoin/pull/33333#pullrequestreview-3226045467)
Reviewed 9f69c646d7eb8790ec9fd6d1d5607f554cc2f4ff

💬 hodlinator commented on pull request "coins: warn on oversized `-dbcache`":
(https://github.com/bitcoin/bitcoin/pull/33333#discussion_r2350089881)
Might be overly harsh with `Assert()`? Could use `Assume()`, and/or maybe add a comment why this is worth guarding against. Noticed the original suggestion didn't even check the return value (https://github.com/bitcoin/bitcoin/pull/33333#discussion_r2344204033).

💬 hodlinator commented on pull request "coins: warn on oversized `-dbcache`":
(https://github.com/bitcoin/bitcoin/pull/33333#discussion_r2349989732)
(nit: as stated previously I prefer making simple functions like `ShouldWarnOversizedDbCache` `constexpr` and using `static_assert` instead of runtime checks, https://github.com/bitcoin/bitcoin/pull/31645#discussion_r2282516167).

💬 hodlinator commented on pull request "coins: warn on oversized `-dbcache`":
(https://github.com/bitcoin/bitcoin/pull/33333#discussion_r2349984030)
Think we should rely on the `NOMINMAX` define you found in *bitcoin/CMakeLists.txt*, also prefer `numeric_limits`. `std::clamp<uint64_t>(` with the lower bound of `0` wasn't doing much so agree with getting rid of that.

~0 on switching from separate `if`-blocks to mutating `mem`-variable.

💬 hodlinator commented on pull request "coins: warn on oversized `-dbcache`":
(https://github.com/bitcoin/bitcoin/pull/33333#discussion_r2350032125)
Stray addition of ` = ` in last commit.

Suggestion: Could remove `total_cache` completely (the name is more fitting for the original copy which is still initialized by the result of this function anyway):
```diff
--- a/src/node/caches.cpp
+++ b/src/node/caches.cpp
@@ -29,14 +29,14 @@ static constexpr size_t MAX_32BIT_DBCACHE{1024_MiB};
namespace node {
size_t CalculateDbCacheBytes(const ArgsManager& args)
{
- size_t total_cache = {DEFAULT_DB_CACHE};
if (std::optional<int64
...

🤔 furszy reviewed a pull request: "test: don't throw from the destructor of DebugLogHelper"
(https://github.com/bitcoin/bitcoin/pull/33388#pullrequestreview-3226297965)
As `DebugLogHelper` is only used within the unit test framework; could use `BOOST_REQUIRE` too.

💬 l0rinc commented on pull request "coins: warn on oversized `-dbcache`":
(https://github.com/bitcoin/bitcoin/pull/33333#discussion_r2350121851)
In that case I'd just delete it since the code already states all of this

📝 forkfury opened a pull request: "util/strencodings: guard SAFE_CHARS index and pre-reserve result"
(https://github.com/bitcoin/bitcoin/pull/33396)
- Add defensive bounds check for `rule` parameter in `SanitizeString` to prevent out-of-range access to `SAFE_CHARS` array
- Pre-reserve result capacity and cache reference to allowed charset to avoid repeated lookups
- No behavior change for valid inputs; improves robustness and minor performance

💬 instagibbs commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2350095778)
b526fd4a3ae637ffc0c13353e1f55d690209fff7

avoid the temporary variable by inlining it in the for loop directly?

💬 instagibbs commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2350119542)
65eaf2738ac88d3f6e7418734c8eebe1f40609ec

This check should be in the loop if it's meant to be a CPU DoS protection

💬 instagibbs commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2350088607)
6502d30e4d8a684b93bf631424fac06aa62bb272

> these are all calculated including the tx itself

Don't think this was correct before or after

📝 hazem19952025 opened a pull request: "Hazem19952025/Update SECURITY.md/hazem19952025/ actions"
(https://github.com/bitcoin/bitcoin/pull/33397)
Actions
Done

<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new
...

📝 hazem19952025 opened a pull request: "Hazem19952025/Update SECURITY.md/hazem19952025/ actions"
(https://github.com/bitcoin/bitcoin/pull/33398)
Done

<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests tha
...

💬 hazem19952025 commented on pull request "Hazem19952025/Update SECURITY.md/hazem19952025/ actions":
(https://github.com/bitcoin/bitcoin/pull/33398#issuecomment-3294076715)
Actions 🎬

💬 jalateras commented on pull request "doc: Add documentation explaining different build types":
(https://github.com/bitcoin/bitcoin/pull/33355#issuecomment-3294083247)
@janb84 squashed and pushed