Bitcoin Core Github
44 subscribers
122K links
Download Telegram
hebasto closed an issue: "build: Qt deprecated-declarations warnings"
(https://github.com/bitcoin/bitcoin/issues/33571)
🚀 hebasto merged a pull request: "Modernize custom filtering"
(https://github.com/bitcoin-core/gui/pull/899)
📝 Mystique85 opened a pull request: "Refactor SHA256 x86 SHANI implementation"
(https://github.com/bitcoin/bitcoin/pull/33783)
Refactored the SHA256 x86 SHANI implementation for improved readability and minor optimization consistency.

Updated static constants to use constexpr for better compile-time evaluation.

Adjusted function signatures to use static ALWAYS_INLINE for consistent linkage and inlining.

Applied reinterpret_cast for pointer conversions, improving type safety and clarity.

No changes were made to the cryptographic logic or algorithmic behavior.

Verified against existing SHA256 test vectors t
...
📝 Mystique85 opened a pull request: "Improve SHA256 x86 SHANI code safety and consistency (no functional changes)"
(https://github.com/bitcoin/bitcoin/pull/33784)
Refactor and clean up the SHA256 x86 SHANI implementation to improve code safety, readability, and consistency without any functional or behavioral changes.

Motivation

This patch improves long-term maintainability and compiler compatibility of the SHANI-optimized SHA256 implementation used in Bitcoin Core.
It replaces C-style casts with reinterpret_cast to enforce type safety, ensures consistent use of static ALWAYS_INLINE linkage for internal helper functions, and converts static constan
...
💬 Mystique85 commented on pull request "Improve SHA256 x86 SHANI code safety and consistency (no functional changes)":
(https://github.com/bitcoin/bitcoin/pull/33784#issuecomment-3487953631)
This PR refactors the SHA256 x86 SHANI implementation to improve code safety, readability, and maintainability **without changing the cryptographic logic or performance**.

The main goals are:
- Use `constexpr` for compile-time constant initialization.
- Use `reinterpret_cast` instead of C-style casts for type safety.
- Apply `static ALWAYS_INLINE` consistently across helper functions.

The implementation has been verified against all SHA256 test vectors (bit-for-bit identical outputs).

...
💬 da1sychain commented on pull request "doc: document fingerprinting risk when operating node on multiple networks":
(https://github.com/bitcoin/bitcoin/pull/33750#issuecomment-3487958150)
Good catch and thank you for the review @danielabrozzoni!! I'll go ahead and fix that.
📝 maflcko opened a pull request: "util: Allow Assert() in contexts without __func__"
(https://github.com/bitcoin/bitcoin/pull/33785)
Without this, compile warnings could be hit about `__func__` being only valid inside functions.

```
warning: predefined identifier is only valid inside function [-Wpredefined-identifier-outside-function] note: expanded from macro Assert
115 | #define Assert(val) inline_assertion_check<true>(val, __FILE__, __LINE__, __func__, #val)
| ^
```

Ref https://github.com/bitcoin/bitcoin/pull/32740#discussion_r24862
...
💬 l0rinc commented on pull request "clang-tidy: Remove no longer needed NOLINT":
(https://github.com/bitcoin/bitcoin/pull/33781#issuecomment-3488018942)
I wanted to ask the same on the original PR but forgot - ACK 038849e2e09bb9f4ce1fb5a1f291745506c6a52d
💬 hebasto commented on pull request "cmake: Create subdirectories in build tree in advance":
(https://github.com/bitcoin/bitcoin/pull/32773#discussion_r2492055139)
I'd say since the following https://github.com/bitcoin/bitcoin/pull/32881 :)
💬 hebasto commented on pull request "cmake: Create subdirectories in build tree in advance":
(https://github.com/bitcoin/bitcoin/pull/32773#discussion_r2492063103)
Thanks! Fixed.
💬 hebasto commented on pull request "cmake: Create subdirectories in build tree in advance":
(https://github.com/bitcoin/bitcoin/pull/32773#issuecomment-3488036459)
The [feedback](https://github.com/bitcoin/bitcoin/pull/32773#discussion_r2491364489) from @m3dwards has been addressed.
💬 l0rinc commented on pull request "Improve SHA256 x86 SHANI code safety and consistency (no functional changes)":
(https://github.com/bitcoin/bitcoin/pull/33784#issuecomment-3488041714)
@Mystique85 the PR is lacking a purpose and it's obviously vibecoded, while it's not clear what problem it's solving and why this alternative would be any better.
Please only open PRs that are solving a real problem and make sure the solution is not this subjective.
💬 maflcko commented on pull request "util: Allow Assert() in contexts without __func__":
(https://github.com/bitcoin/bitcoin/pull/33785#issuecomment-3488042681)
Can be tested via the following diff:


```diff
diff --git a/src/logging.cpp b/src/logging.cpp
index 2ed6835197..890e461cb9 100644
--- a/src/logging.cpp
+++ b/src/logging.cpp
@@ -20,6 +20,7 @@
using util::Join;
using util::RemovePrefixView;

+const int g_FOO{Assert(false)};
const char * const DEFAULT_DEBUGLOGFILE = "debug.log";
constexpr auto MAX_USER_SETABLE_SEVERITY_LEVEL{BCLog::Level::Info};

```

Or with a function name (slight change in behavior):

```diff
diff --
...
💬 maflcko commented on pull request "refactor: Header sync optimisations & simplifications":
(https://github.com/bitcoin/bitcoin/pull/32740#discussion_r2492071472)
Fixed in https://github.com/bitcoin/bitcoin/pull/33785. You should be able to push your previous `*Assert(WITH_LOCK(...))` after it.
📝 l0rinc opened a pull request: "script: remove dead code in `CountWitnessSigOps`"
(https://github.com/bitcoin/bitcoin/pull/33786)
Found while reviewing #32840

The `nullptr` witness path was dead in normal code paths: replacing it with reference enables us deleting unreachable logic.

Code coverage proof:
https://maflcko.github.io/b-c-cov/total.coverage/src/script/interpreter.cpp.gcov.html#L2135
💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2492188555)
Took the formats, thanks!
💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2492189914)
I removed the abort early logic, so we keep going if we don't find an input. It makes the logic much simpler, but we will do some more work if we get a block mined that is double spending.
💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2492191544)
I've updated this to be an atomic_bool instead of this enum. So, since there is only a false value that is set to true, we have the main thread call `input.ready.wait(false, std::memory_order_acquire);`. This way we don't spin.
💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2492194122)
@TheCharlatan thanks for fuzzing, and the diff for the fuzzer! I have taken it, and added you as a co-author :heart_hands:.

@l0rinc it is concerning that you are getting malloc errors. Are there any other details you can share about this?
💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2492195430)
Done, this `Status` enum has been replaced. It is now just an `atomic_bool` `ready`.