💬 hebasto commented on pull request "cmake: Get rid of undocumented `BITCOIN_GENBUILD_NO_GIT` environment variable":
(https://github.com/bitcoin/bitcoin/pull/32220#issuecomment-3398016074)
Rebased to refresh the CI.
(https://github.com/bitcoin/bitcoin/pull/32220#issuecomment-3398016074)
Rebased to refresh the CI.
💬 Waytoogo commented on issue "Version 30 `-datadir=` not working":
(https://github.com/bitcoin/bitcoin/issues/33608#issuecomment-3398016529)
> Closing for now, but discussion can continue. Also, this issue can be re-opened, when there is need.
I'm sorry for all the fuss, but this was very confusing for me. Thank you for your help.
(https://github.com/bitcoin/bitcoin/issues/33608#issuecomment-3398016529)
> Closing for now, but discussion can continue. Also, this issue can be re-opened, when there is need.
I'm sorry for all the fuss, but this was very confusing for me. Thank you for your help.
💬 pablomartin4btc commented on pull request "cli: Improve error message on multiwallet cli-side commands":
(https://github.com/bitcoin/bitcoin/pull/26990#issuecomment-3398037313)
> > > Removed validation commit which I'll added in a separate PR as [#26990 (comment)](https://github.com/bitcoin/bitcoin/pull/26990#issuecomment-2354091658) by @jonatack.
> >
> >
> > Does this exist?
>
> So, this was the 2nd commit [3d63fc9](https://github.com/bitcoin/bitcoin/commit/3d63fc976d616436d64335b15a918ffba1883b9a) ...
>
> ... other 2 validations...
> - (a) no params starting with slash "-" will be accepted after a cli-command...
> - (b) duplication of cli-command (and o
...
(https://github.com/bitcoin/bitcoin/pull/26990#issuecomment-3398037313)
> > > Removed validation commit which I'll added in a separate PR as [#26990 (comment)](https://github.com/bitcoin/bitcoin/pull/26990#issuecomment-2354091658) by @jonatack.
> >
> >
> > Does this exist?
>
> So, this was the 2nd commit [3d63fc9](https://github.com/bitcoin/bitcoin/commit/3d63fc976d616436d64335b15a918ffba1883b9a) ...
>
> ... other 2 validations...
> - (a) no params starting with slash "-" will be accepted after a cli-command...
> - (b) duplication of cli-command (and o
...
💬 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
(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
(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
...
(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`
(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.
(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
(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.
(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
(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.
(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.
(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.
(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
(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)
(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
...
(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
...
(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
(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
...
(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
...