Bitcoin Core Github
43 subscribers
122K links
Download Telegram
📝 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`.
💬 hebasto commented on pull request "guix: Use UCRT runtime for Windows release binaries":
(https://github.com/bitcoin/bitcoin/pull/33593#issuecomment-3488313837)
Reworked per feedback from @trevarj.
💬 hebasto commented on pull request "guix: Use UCRT runtime for Windows release binaries":
(https://github.com/bitcoin/bitcoin/pull/33593#discussion_r2492270996)
Thanks! [Reworked](https://github.com/bitcoin/bitcoin/pull/33593#issuecomment-3488313837).
💬 hebasto commented on pull request "guix: Use UCRT runtime for Windows release binaries":
(https://github.com/bitcoin/bitcoin/pull/33593#issuecomment-3488354051)
My Guix build:
```
aarch64
4e80325f9cba1c286a999eeaa6fb5f06bb56ba03f118cfb6eaa86c8443318ccc guix-build-dbb835956abf/output/dist-archive/bitcoin-dbb835956abf.tar.gz
7557b85092d69ac4f6ebeb4881a41fe808ee684b1b2f87efde97a616f1d1b213 guix-build-dbb835956abf/output/x86_64-w64-mingw32/SHA256SUMS.part
3cf561ff53f83f822fd451b2f83161908daad89598d286de916a641640abe747 guix-build-dbb835956abf/output/x86_64-w64-mingw32/bitcoin-dbb835956abf-win64-codesigning.tar.gz
f607587780890278ffcc9c4b346bcecc999
...
👍 darosior approved a pull request: "script: remove dead code in `CountWitnessSigOps`"
(https://github.com/bitcoin/bitcoin/pull/33786#pullrequestreview-3418942549)
Neat. utACK 24bcad3d4df59690f30c9df8ebb62f0bddd0f1c7.

The need for a pointer was removed when `CTxInWitness` was moved into `CTxIn` in f6fb7acda4aefd01b8ef6cd77063bfc0c4f4ab36.
👋 l0rinc's pull request is ready for review: "script: remove dead code in `CountWitnessSigOps`"
(https://github.com/bitcoin/bitcoin/pull/33786)
👋 hebasto's pull request is ready for review: "ci, iwyu: Fix warnings in `src/kernel` and treat them as errors"
(https://github.com/bitcoin/bitcoin/pull/33779)