🤔 furszy reviewed a pull request: "net: do not apply whitelist permissions to onion inbounds"
(https://github.com/bitcoin/bitcoin/pull/33395#pullrequestreview-3226709131)
Code review ACK e46a7a547371317a4d116b9b1a314917508ea480

🤔 furszy reviewed a pull request: "key: use static context for libsecp256k1 calls where applicable"
(https://github.com/bitcoin/bitcoin/pull/33399#pullrequestreview-3226718930)
Concept ACK

💬 furszy commented on pull request "util/strencodings: guard SAFE_CHARS index and pre-reserve result":
(https://github.com/bitcoin/bitcoin/pull/33396#discussion_r2350414646)
what about changing the `SafeChars` enum to be an enum class instead? (enums auto-increment their value so you would just need to cast it to int here)

💬 achow101 commented on pull request "coins: warn on oversized `-dbcache`":
(https://github.com/bitcoin/bitcoin/pull/33333#issuecomment-3294473221)
I think it would be better to use an `InitWarning` rather than just logging since people are unlikely notice

🤔 l0rinc requested changes to a pull request: "util/strencodings: guard SAFE_CHARS index and pre-reserve result"
(https://github.com/bitcoin/bitcoin/pull/33396#pullrequestreview-3226721389)
Not sure if this is an LLM generated patch or not, it's clear from the usages that the `int rule` doesn't need bounds check - but the reserve does make sense and we can swap the int param to the enum which could make sense.

💬 l0rinc commented on pull request "util/strencodings: guard SAFE_CHARS index and pre-reserve result":
(https://github.com/bitcoin/bitcoin/pull/33396#discussion_r2350425653)
There's no point in extracting this, the compilers are already smart enough to optimize this, see: https://godbolt.org/z/9rej918W7 (compare the SanitizeString_Direct and SanitizeString_Reference assemblies, they're the same).

But the string reserve that still make sense and we can change the parameter to the enum instead, e.g.
```patch
diff --git a/src/util/strencodings.cpp b/src/util/strencodings.cpp
index 289f49d46a..d1b99be943 100644
--- a/src/util/strencodings.cpp
+++ b/src/util/strencoding
...

💬 l0rinc commented on pull request "util/strencodings: guard SAFE_CHARS index and pre-reserve result":
(https://github.com/bitcoin/bitcoin/pull/33396#discussion_r2350409012)
the `rule` is an enum in reality, what's the reason for this addition?

💬 achow101 commented on pull request "test: Prevent disk space warning during node_init_tests":
(https://github.com/bitcoin/bitcoin/pull/33391#issuecomment-3294493195)
ACK bdf01c6f61262cd6e211ead3c0dbc66ccb48b32f

🚀 achow101 merged a pull request: "test: Prevent disk space warning during node_init_tests"
(https://github.com/bitcoin/bitcoin/pull/33391)

🤔 l0rinc reviewed a pull request: "key: use static context for libsecp256k1 calls where applicable"
(https://github.com/bitcoin/bitcoin/pull/33399#pullrequestreview-3226753026)
Thanks for fixing these.
Is there a way to separate these two usages by type somehow - I don't claim to fully understand when the state is needed and when a static can be used.
And would it make sense to rename the old `secp256k1_context_sign` references while we're here?

nit: typo in description `for the context paramter`

💬 l0rinc commented on pull request "key: use static context for libsecp256k1 calls where applicable":
(https://github.com/bitcoin/bitcoin/pull/33399#discussion_r2350433601)
if we still need this, we can move it to the scope where it's used (with the secp256k1_context_destroy call)

💬 l0rinc commented on pull request "key: use static context for libsecp256k1 calls where applicable":
(https://github.com/bitcoin/bitcoin/pull/33399#discussion_r2350455433)
Not my area of expertise, but is `secp256k1_context_sign` even the correct name here, wasn't that deprecated in favor of [`SECP256K1_CONTEXT_NONE`](https://github.com/bitcoin-core/secp256k1/blob/master/include/secp256k1.h#L218)?

💬 l0rinc commented on pull request "coins: warn on oversized `-dbcache`":
(https://github.com/bitcoin/bitcoin/pull/33333#issuecomment-3294609537)
[Changed](https://github.com/bitcoin/bitcoin/compare/ec92151b538473792846db7d5bcabe4d6ae83c82..13fa0d0a433722f294854001b0561c079db12dbc) it to `InitWarning`, so now the warnings look like:
```
2025-09-16T02:17:00Z [warning] A 60000 MiB dbcache may be too large for a system memory of only 65536 MiB.
Warning: A 60000 MiB dbcache may be too large for a system memory of only 65536 MiB.
```
Thanks for the hint!

💬 adamandrews1 commented on pull request "rpc: combinerawtransaction now rejects unmergeable transactions":
(https://github.com/bitcoin/bitcoin/pull/31298#issuecomment-3294736165)
@achow101 I've rebased this and updated the comment. No other changes.

💬 ajtowns commented on issue "RFC: Bitcoin Core Node `BlockTemplateManager`":
(https://github.com/bitcoin/bitcoin/issues/33389#issuecomment-3295505510)
> [net: Provide block templates to peers on request #33191](https://github.com/bitcoin/bitcoin/pull/33191) will also fit in nicely; see: [ismaelsadeeq@10bd892](https://github.com/ismaelsadeeq/bitcoin/commit/10bd8920fd259aa47d4b96037a2302d7ae6ea41f)

I don't understand this -- peer template sharing seems a really bad fit for a template manager for three reasons:

* the shared templates are probably oversized (2MvB instead of 1MvB) so aren't directly useful to reuse in other contexts (and would b
...

🤔 w0xlt reviewed a pull request: "log: always print initial signature verification state"
(https://github.com/bitcoin/bitcoin/pull/33336#pullrequestreview-3227507782)
reACK https://github.com/bitcoin/bitcoin/pull/33336/commits/9b04e4f96200daf7516d1b8b6b0dfc4c077837e8

🤔 w0xlt reviewed a pull request: "coins: warn on oversized `-dbcache`"
(https://github.com/bitcoin/bitcoin/pull/33333#pullrequestreview-3227651723)
ACK https://github.com/bitcoin/bitcoin/pull/33333/commits/13fa0d0a433722f294854001b0561c079db12dbc

BSD operating system support can be a follow-up.

💬 fanquake commented on pull request "key: use static context for libsecp256k1 calls where applicable":
(https://github.com/bitcoin/bitcoin/pull/33399#issuecomment-3296148168)
cc @jonasnick @real-or-random

💬 rkrux commented on pull request "wallet/rpc: fix listdescriptors RPC fails to return descriptors with private key information when wallet contains descriptors missing any key":
(https://github.com/bitcoin/bitcoin/pull/32471#discussion_r2351213344)
`has_priv_key` is used only for `PRIVATE` type, consider the following that limits its usage to avoid it being considered a misnomer - related unit/functional tests pass.

```diff
diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp
index f29e73e2e6..ef977a5912 100644
--- a/src/script/descriptor.cpp
+++ b/src/script/descriptor.cpp
@@ -891,7 +891,7 @@ public:
std::string tmp;
bool subscript_res{scriptarg->ToStringHelper(arg, tmp, type, cache)};

...

💬 real-or-random commented on pull request "key: use static context for libsecp256k1 calls where applicable":
(https://github.com/bitcoin/bitcoin/pull/33399#issuecomment-3296261604)
> I don't claim to fully understand when the state is needed and when a static can be used.

Whenever the generator point G is multiplied by a secret scalar. Or, roughly speaking, whenever a secret key is involved, i.e., key generation or signing. The exceptions to this rule are the computation of the shared secret in ECDH and EllSwift.

> And would it make sense to rename the old `secp256k1_context_sign` references while we're here?

I'm not sure. `SECP256K1_CONTEXT_SIGN` was deprecated,
...