Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 Raimo33 commented on pull request "miner: empty mempool special case for waitNext()":
(https://github.com/bitcoin/bitcoin/pull/33566#issuecomment-3398044812)
Concept ACK. this is a subtle bug which would result in an infinite loop.

But allow me to propose a more elegant and modern solution: c23a65414f239c0d867d13f1be02a232096c1f9d
💬 kevkevinpal commented on pull request "test: change log rate limit version gate from 299900 to 289900":
(https://github.com/bitcoin/bitcoin/pull/33612#issuecomment-3398044892)
ACK [45dda1b](https://github.com/bitcoin/bitcoin/pull/33612/commits/45dda1b6c202c77622ad9519765efa7df9b8629e)

pretty straightforward and according to https://github.com/bitcoin/bitcoin/pull/33225#discussion_r2423464895 seems like we can still open to master and backport this change
💬 furszy commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#issuecomment-3398073492)
Haven't read the latest comments yet (will do soon), but I’ve been working on an overhaul of the approach this weekend. I got enlightened after last week’s review.

The latest code on Signet, syncing the block filter index up to block height 240593 on a MacBook M1 Pro, running on an SSD (following the testing steps in the PR description). Results:

Master branch (64a7c7cbb975cb3c3f25a3f784779f32cd95ebaa) sync time: ~50 minutes.
Current PR (f130332) sync time: ~5 minutes (with 5 worker threa
...
📝 fanquake opened a pull request: "[28.x] Backport & finalise 28.3"
(https://github.com/bitcoin/bitcoin/pull/33613)
Backports:
* #33581

Plus the changes to finalise `v28.3`
💬 fanquake commented on pull request "ci: Properly include $FILE_ENV in DEPENDS_HASH":
(https://github.com/bitcoin/bitcoin/pull/33581#issuecomment-3398132819)
Backported to `28.x` in #33613.
💬 m3dwards commented on pull request "ci: fix buildx gha cache authentication on forks":
(https://github.com/bitcoin/bitcoin/pull/33508#issuecomment-3398197163)
@maflcko hopefully this should sort it: https://github.com/m3dwards/bitcoin-core-with-ci/commit/5131564636ae971fb1ab68aeb980824af432fdf4
💬 Sjors commented on pull request "miner: empty mempool special case for waitNext()":
(https://github.com/bitcoin/bitcoin/pull/33566#issuecomment-3398246786)
Great idea, I integrated @Raimo33's approach.
💬 Raimo33 commented on pull request "miner: fix empty mempool case for waitNext()":
(https://github.com/bitcoin/bitcoin/pull/33566#issuecomment-3398250482)
> Great idea, I integrated @Raimo33's approach.

please add me as co-author or cherry-pick my commit
💬 hebasto commented on pull request "Bugfix: Only use git for build info if the repository is actually the right one":
(https://github.com/bitcoin/bitcoin/pull/32217#discussion_r2426835503)
This comment is still unaddressed.
💬 hebasto commented on pull request "Bugfix: Only use git for build info if the repository is actually the right one":
(https://github.com/bitcoin/bitcoin/pull/32217#issuecomment-3398257517)
> Also seems like the obvious time to document `BITCOIN_GENBUILD_NO_GIT` (#31999)

This is still unaddressed.
💬 Sjors commented on pull request "miner: fix empty mempool case for waitNext()":
(https://github.com/bitcoin/bitcoin/pull/33566#issuecomment-3398259043)
@Raimo33 I did, see `Co-authored-by` in commit message.
📝 waketraindev opened a pull request: "rpc, test: fix "internal" label check in importdescriptors and extend test"
(https://github.com/bitcoin/bitcoin/pull/33614)

Fixes a logic issue in `ProcessDescriptorImport()` where descriptors with `"internal": false` and a label were incorrectly rejected.
The check now uses `internal.value_or(false)` so labels are only disallowed when `internal` is `true`.

Tests updated to cover both cases
👋 waketraindev's pull request is ready for review: "rpc, test: fix "internal" label check in importdescriptors and extend test"
(https://github.com/bitcoin/bitcoin/pull/33614)
⚠️ Crypt-iQ opened an issue: "ASAN use-after-free in `m_reconnections`"
(https://github.com/bitcoin/bitcoin/issues/33615)
I ran into this while shutting my node down during IBD. I think it's benign.

What happens:
- `CConnman` is started [here](https://github.com/bitcoin/bitcoin/blob/64a7c7cbb975cb3c3f25a3f784779f32cd95ebaa/src/init.cpp#L2183).
- This initializes `semOutbound` [here](https://github.com/bitcoin/bitcoin/blob/64a7c7cbb975cb3c3f25a3f784779f32cd95ebaa/src/net.cpp#L3361).
- While `CConnman` is active, `m_reconnections` is added to in `DisconnectNodes` [here](https://github.com/bitcoin/bitcoin/blob/64a7
...
📝 instagibbs opened a pull request: "2025 10 bypass checkephemeral"
(https://github.com/bitcoin/bitcoin/pull/33616)
Similar reasoning to https://github.com/bitcoin/bitcoin/pull/33504

During a deeper reorg it's possible that a long sequence of dust-having transactions that are connected in a linear fashion, and currently each subsequent "generation" will be rejected on reorg, even though the dust would be spent usually by another child transaction. This could evict a large amount of competitive fees via normal means.

PreCheck based `PreCheckEphemeralSpends` is left in place because we wouldn't have relay
...
💬 fjahr commented on pull request "refactor: Split multithreaded case out of CheckInputScripts":
(https://github.com/bitcoin/bitcoin/pull/32575#issuecomment-3398490065)
Turning it off and on again finally worked :p
💬 janb84 commented on pull request "build: Bump clang minimum supported version to 17":
(https://github.com/bitcoin/bitcoin/pull/33555#issuecomment-3398591114)
ConceptACK fa0fa0f70087d08fe5a54832b96799bd14293279

PR Sets the clang minimum supported version to 17 and drop some work arounds for clang 16. Given that supported OS-es have already a minimal clang version of 17, this should not be a problem.

Would like to see confirmation that the fuzz test works as expected, by a fuzz specialist.

- code review
- GUIX builds
- normal build
- ran tests

Ps. NixOS 25.05 has Clang version 19 as default.

<details><summary> Guix Build O
...
👍 stickies-v approved a pull request: "refactor: Construct g_verify_flag_names on first use"
(https://github.com/bitcoin/bitcoin/pull/33600#pullrequestreview-3332643263)
ACK faa9d10c84bc6b465cbca266468990cc716b4300

I would prefer the `constexpr` approach, but either approach is acceptable to me.
💬 stickies-v commented on pull request "refactor: Construct g_verify_flag_names on first use":
(https://github.com/bitcoin/bitcoin/pull/33600#discussion_r2427048878)
An alternative approach would be to use a `constexpr` array and make the whole thing compile-time? As a nice benefit, removes the `FLAG_NAME` macro which imo doesn't simplify things enough to be worth it.

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

```diff
diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp
index abd99fc365..731c0a070f 100644
--- a/src/script/interpreter.cpp
+++ b/src/script/interpreter.cpp
@@ -2163,36 +2163,6 @@ size_t CountWitnessSigOps(con
...
💬 janb84 commented on pull request "rpc, test: fix "internal" label check in importdescriptors and extend test":
(https://github.com/bitcoin/bitcoin/pull/33614#discussion_r2427075461)
NIT: would rather see `internal == true` imho better readability.

```suggestion
if (internal == true && data.exists("label")) {
```