Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 stickies-v commented on pull request "refactor: Convert GenTxid to `std::variant`":
(https://github.com/bitcoin/bitcoin/pull/32631#discussion_r2179755821)
In 7c4eb58fb0:

I don't really see the point of using `util::Overloaded` here when we're always doing a `Wtxid` query?

This looks more straightforward to me:

<details>
<summary>git diff on 7c4eb58fb0</summary>

```diff
diff --git a/src/node/txdownloadman_impl.cpp b/src/node/txdownloadman_impl.cpp
index a16490ceb9..87a47b30ee 100644
--- a/src/node/txdownloadman_impl.cpp
+++ b/src/node/txdownloadman_impl.cpp
@@ -9,7 +9,6 @@
#include <consensus/validation.h>
#include <logging.h>
...
💬 fjahr commented on pull request "index: Deduplicate HashKey / HeightKey handling":
(https://github.com/bitcoin/bitcoin/pull/32997#discussion_r2216982231)
nit: could do IWYU I guess, even though this wasn't followed that well in the index cpp files.

```
#include <dbwrapper.h>
#include <interfaces/types.h>
```
⚠️ totdking opened an issue: "A missing import to the src/chainparamsbase.h"
(https://github.com/bitcoin/bitcoin/issues/33019)
Tried running the make -j $CORES as the bitcoin repo is important to what I'm working with.

The import that is missing is the ```#include <cstdint>```, it gave a plethora of errors.

This is not a bug but i just wanted the community to have a peaceful and seamless integration while building the repo
📝 hebasto opened a pull request: "refactor: Move `FreespaceChecker` class into its own module"
(https://github.com/bitcoin-core/gui/pull/881)
The MOC compiler in older versions of Qt 6 fails to parse `qt/intro.cpp`, as noted in [this comment](https://github.com/bitcoin/bitcoin/pull/32998#issuecomment-3082011233).

This PR proposes a move-only refactoring to simplify the source structure by eliminating the need for the inline `#include <qt/intro.moc>`, thereby effectively working around the issue.
💬 ajtowns commented on pull request "refactor: Move `FreespaceChecker` class into its own module":
(https://github.com/bitcoin-core/gui/pull/881#issuecomment-3130725962)
ACK dd392a64bb0608847f771f8b1f09c2fcae146923

Shouldn't the copyright dates in these files be updated? `git blame` has some lines from 2022 in intro.h and 2024/25 in intro.cpp.

You could avoid the circularity with something like:

```diff
diff --git a/src/qt/freespacechecker.h b/src/qt/freespacechecker.h
index d3a61a11571..214324be7c8 100644
--- a/src/qt/freespacechecker.h
+++ b/src/qt/freespacechecker.h
@@ -9,8 +9,6 @@
#include <QString>
#include <QtGlobal>

-class Intro;
-

...
💬 hodlinator commented on pull request "test,refactor: extract script template helpers & widen sigop count coverage":
(https://github.com/bitcoin/bitcoin/pull/32729#discussion_r2260374450)
Since we're adding the *secp256k1.h* `#include` in commit 17922e88153b07f6146f5740dde69e8a3587220a, why not use `SECP256K1_TAG_PUBKEY_EVEN`/`..._ODD` directly instead of changing to them in 90df1ccfca9c0adfd1c61c1b07d8911ab632bc6a?
💬 stickies-v commented on pull request "refactor: Convert uint256 to Txid":
(https://github.com/bitcoin/bitcoin/pull/33116#discussion_r2262703752)
nit: could modernize

<details>
<summary>git diff on a20724d926</summary>

```diff
diff --git a/src/node/mini_miner.cpp b/src/node/mini_miner.cpp
index c26355d832..a1c2edcb7b 100644
--- a/src/node/mini_miner.cpp
+++ b/src/node/mini_miner.cpp
@@ -16,6 +16,7 @@

#include <algorithm>
#include <numeric>
+#include <ranges>
#include <utility>

namespace node {
@@ -61,12 +62,8 @@ MiniMiner::MiniMiner(const CTxMemPool& mempool, const std::vector<COutPoint>& ou
if (m_request
...
💬 ryanofsky commented on issue "depends: `native_libmultiprocess` fails to build on OpenBSD":
(https://github.com/bitcoin/bitcoin/issues/33219#issuecomment-3202305628)
Thanks for the report and clear reproduction steps. Following change fixes the problem for me, and I guess this was caused by the IWYU commit in https://github.com/bitcoin-core/libmultiprocess/pull/184,

```
--- a/src/ipc/libmultiprocess/src/mp/util.cpp
+++ b/src/ipc/libmultiprocess/src/mp/util.cpp
@@ -16,6 +16,7 @@
#include <sys/socket.h>
#include <sys/wait.h>
#include <system_error>
+#include <thread>
#include <unistd.h>
#include <utility>
#include <vector>
```

An alternate change also
...
🤔 l0rinc requested changes to a pull request: "bench: Add more realistic Coin Selection Bench"
(https://github.com/bitcoin/bitcoin/pull/33160#pullrequestreview-3157264436)
Concept ACK, the existing test were all quite trivial compared to this new one.
I left a ton of comments, for simplicity here's the final code that I'm suggesting, feel free to pick and choose from it.

<details>
<summary>Details</summary>

```C++
// Copyright (c) 2012-2022 The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.

#include <bench/bench.h>
#include <consensus/
...
💬 maflcko commented on pull request "p2p: add `DifferenceFormatter` fuzz target and invariant check":
(https://github.com/bitcoin/bitcoin/pull/33252#discussion_r2309281175)
It is less code this way:

```diff
diff --git a/src/test/fuzz/difference_formatter.cpp b/src/test/fuzz/difference_formatter.cpp
index 49e859ffdc..38b54352c1 100644
--- a/src/test/fuzz/difference_formatter.cpp
+++ b/src/test/fuzz/difference_formatter.cpp
@@ -4,25 +4,13 @@

#include <blockencodings.h>
#include <streams.h>
-#include <test/fuzz/FuzzedDataProvider.h>
+#include <random.h>
#include <test/fuzz/fuzz.h>

#include <vector>

namespace {
-// Test struct for VectorFo
...