💬 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

💬 mzumsande commented on pull request "p2p: Advance pindexLastCommonBlock early after connecting blocks":
(https://github.com/bitcoin/bitcoin/pull/32180#discussion_r2350222343)
That was not on purpose - sorry. Fixed now.

💬 mzumsande commented on pull request "p2p: Advance pindexLastCommonBlock early after connecting blocks":
(https://github.com/bitcoin/bitcoin/pull/32180#issuecomment-3294127975)
[3bdff9a](https://github.com/bitcoin/bitcoin/commit/3bdff9a154733f8f9f379448f5595a2e90474bc6) to [b2c6cb7](https://github.com/bitcoin/bitcoin/commit/b2c6cb7f026ff704c227e632f8c5c2f09e6f058a):
- Reintroduced test improvement that I unintentionally dropped with the last rebase (https://github.com/bitcoin/bitcoin/pull/32180#discussion_r2344157017)
- changed commit message of "p2p: remove extra logic for assumeutxo"

💬 mzumsande commented on pull request "p2p: Advance pindexLastCommonBlock early after connecting blocks":
(https://github.com/bitcoin/bitcoin/pull/32180#discussion_r2350232732)
About the first, I didn't have a strong opinion. Seems like if we do it for `ActiveTip()`, we should also do it for `ActiveHeight()`.

> Also, this isn’t introduced by this PR, but the second check has always puzzled me a bit. It would be nice to have some util function like IsBlockInChain(const CBlockIndex* block_to_test, const CBlockIndex* chain_tip).

I'd say that a name like `CChain::IsAncestorOf(const CBlockIndex* possible_ancestor, const CBlockIndex* predecessor)` would be more precise
...

💬 D33r-Gee commented on pull request "Interfaces: Expose UTXO Snapshot Loading and Add Progress Notifications":
(https://github.com/bitcoin/bitcoin/pull/33117#issuecomment-3294141976)
with [94d68d1](https://github.com/bitcoin/bitcoin/commit/94d68d12a6c19060a4c2447a414f51049906f965) RPC Error handling has been restored through the interface

💬 achow101 commented on pull request "rpc: combinerawtransaction now rejects unmergeable transactions":
(https://github.com/bitcoin/bitcoin/pull/31298#issuecomment-3294202869)
Are you still working on this?

💬 achow101 commented on pull request "net: do not apply whitelist permissions to onion inbounds":
(https://github.com/bitcoin/bitcoin/pull/33395#issuecomment-3294292053)
ACK e46a7a547371317a4d116b9b1a314917508ea480

📝 theStack opened a pull request: "key: use static context for libsecp256k1 calls where applicable"
(https://github.com/bitcoin/bitcoin/pull/33399)
The dynamically created [signing context](https://github.com/bitcoin/bitcoin/blob/2d6a0c464912c325faf35d4ad28b1990e828b414/src/key.cpp#L19) for libsecp256k1 calls is only needed for functions that involve generator point multiplication with a secret key, i.e. different variants of public key creation and signing. The API docs hint to those by stating "[(not secp256k1_context_static)](https://github.com/bitcoin-core/secp256k1/blob/b4756543028065b3ae6f30e9e6d7f1ecf2bb08c6/include/secp256k1.h#L645)
...

⚠️ kelvin7891 opened an issue: "Transfer issues on myProject I need some ideas"
(https://github.com/bitcoin/bitcoin/issues/33400)