Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 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)
pinheadmz closed an issue: "Transfer issues on myProject I need some ideas"
(https://github.com/bitcoin/bitcoin/issues/33400)
🤔 l0rinc reviewed a pull request: "coins: warn on oversized `-dbcache`"
(https://github.com/bitcoin/bitcoin/pull/33333#pullrequestreview-3226673018)
Thanks for the reviews, keep them coming :)
💬 l0rinc commented on pull request "coins: warn on oversized `-dbcache`":
(https://github.com/bitcoin/bitcoin/pull/33333#discussion_r2350374604)
Done
💬 l0rinc commented on pull request "coins: warn on oversized `-dbcache`":
(https://github.com/bitcoin/bitcoin/pull/33333#discussion_r2350374637)
Changed to `Assume`, but I don't find comments useful here.
💬 l0rinc commented on pull request "coins: warn on oversized `-dbcache`":
(https://github.com/bitcoin/bitcoin/pull/33333#discussion_r2350394510)
I don't like burdening the compiler this - and it makes errors and debugging uniform if we do it in tests - but I did make it `constexpr`, thanks for the hint
💬 l0rinc commented on pull request "coins: warn on oversized `-dbcache`":
(https://github.com/bitcoin/bitcoin/pull/33333#discussion_r2350374491)
Ok, I have applied something similar and updated the godbolt reproducer, thanks for the feedback
🤔 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)