Bitcoin Core Github
43 subscribers
122K links
Download Telegram
💬 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)
💬 hebasto commented on pull request "ci, iwyu: Fix warnings in `src/kernel` and treat them as errors":
(https://github.com/bitcoin/bitcoin/pull/33779#issuecomment-3488404139)
There are no conflicts with other contributors' PRs.

Friendly ping @l0rinc @maflcko @ryanofsky @willcl-ark who reviewed https://github.com/bitcoin/bitcoin/pull/31308, and @TheCharlatan, the kernel expert :)
🤔 l0rinc requested changes to a pull request: "test: Enhance GetTxSigOpCost tests for coinbase transactions"
(https://github.com/bitcoin/bitcoin/pull/32840#pullrequestreview-3418794264)
I have gone through the cases, I think we should take this opportunity and unify the test to use BOOST_ checkers for better error messages, to split the big test into smaller self-contained tests (otherwise the first failure will break the remaining ones - though this will result in some setup-repetition, but we can add helpers for those).

I have applied the cleanup that I would like to see here, if you decide to accept any of it, please do it in multiple focused commits.

<details>
<summary>De
...