💬 mzumsande commented on pull request "net: check for empty header before calling FillBlock":
(https://github.com/bitcoin/bitcoin/pull/33296#discussion_r2323690854)
I think this will change behavior in non-debug builds such that repeated `blocktxn` messages will now be ignored (whereas they would result in `READ_STATUS_INVALID` / disconnection before). Shouldn't we rather keep on disconnecting the peer?
(https://github.com/bitcoin/bitcoin/pull/33296#discussion_r2323690854)
I think this will change behavior in non-debug builds such that repeated `blocktxn` messages will now be ignored (whereas they would result in `READ_STATUS_INVALID` / disconnection before). Shouldn't we rather keep on disconnecting the peer?
💬 hebasto commented on issue "cmake: Errors not actually errors?":
(https://github.com/bitcoin/bitcoin/issues/33153#issuecomment-3256075142)
I agree that this issue needs to be fixed.
However, considering:
- the limited time left before branching off,
- the specific conditions (e.g., `-DCMAKE_INTERPROCEDURAL_OPTIMIZATION=ON`) under which the issue manifests,
- the lack of consensus on how to fix it, and
- the absence of an open PR with a suggested fix,
I propose bumping the milestone for this PR from 30.0 to 31.0.
(https://github.com/bitcoin/bitcoin/issues/33153#issuecomment-3256075142)
I agree that this issue needs to be fixed.
However, considering:
- the limited time left before branching off,
- the specific conditions (e.g., `-DCMAKE_INTERPROCEDURAL_OPTIMIZATION=ON`) under which the issue manifests,
- the lack of consensus on how to fix it, and
- the absence of an open PR with a suggested fix,
I propose bumping the milestone for this PR from 30.0 to 31.0.
💬 davidgumberg commented on pull request "build: suggest -DENABLE_IPC=OFF when missing capnp":
(https://github.com/bitcoin/bitcoin/pull/33290#issuecomment-3256100512)
Conceptual question:
Why not print a warning *and* `set(ENABLE_IPC OFF)` when missing capnp? This was generally the approach take in the old autotools build system. Is it better to force the user to confront the warning and change their configuration than to just try to build something reasonable based on what dependencies the user has?
(https://github.com/bitcoin/bitcoin/pull/33290#issuecomment-3256100512)
Conceptual question:
Why not print a warning *and* `set(ENABLE_IPC OFF)` when missing capnp? This was generally the approach take in the old autotools build system. Is it better to force the user to confront the warning and change their configuration than to just try to build something reasonable based on what dependencies the user has?
💬 sipa commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2323700219)
Ok, dropped this piece since this code basically never did anything at all, and adding a test that works with the wallet needs more work than I'm willing to put in right now, I'm going to leave that for all follow-up instead.
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2323700219)
Ok, dropped this piece since this code basically never did anything at all, and adding a test that works with the wallet needs more work than I'm willing to put in right now, I'm going to leave that for all follow-up instead.
💬 sipa commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2323701264)
Actually, fixed.
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2323701264)
Actually, fixed.
💬 sipa commented on pull request "build: suggest -DENABLE_IPC=OFF when missing capnp":
(https://github.com/bitcoin/bitcoin/pull/33290#issuecomment-3256128656)
@davidgumberg I believe that's a philosophical difference between cmakeland and autotoolsland, and I think in the case of optional build dependencies, it's better not to autodetect them, to avoid build system configuration leaking into the build (which then might not work on the system you actually want to run it on).
(https://github.com/bitcoin/bitcoin/pull/33290#issuecomment-3256128656)
@davidgumberg I believe that's a philosophical difference between cmakeland and autotoolsland, and I think in the case of optional build dependencies, it's better not to autodetect them, to avoid build system configuration leaking into the build (which then might not work on the system you actually want to run it on).
💬 krud491 commented on issue "natpmp: quiet down unconditional logging":
(https://github.com/bitcoin/bitcoin/issues/33301#issuecomment-3256415998)
Use markdown for format
(https://github.com/bitcoin/bitcoin/issues/33301#issuecomment-3256415998)
Use markdown for format
📝 l0rinc opened a pull request: "test/refactor: use test deque to avoid quadratic iteration"
(https://github.com/bitcoin/bitcoin/pull/33313)
Extracted from https://github.com/bitcoin/bitcoin/pull/33141#discussion_r2323012972.
-----
In Python, [list `pop(0)` is linear](https://docs.python.org/3/tutorial/datastructures.html#using-lists-as-queues), so consuming all items in the test results in quadratic iteration.
Switching to `collections.deque` with `popleft()` expresses FIFO intent and avoids the O(n^2) path.
Behavior is unchanged - for a few hundred items the perf impact is likely negligible.
(https://github.com/bitcoin/bitcoin/pull/33313)
Extracted from https://github.com/bitcoin/bitcoin/pull/33141#discussion_r2323012972.
-----
In Python, [list `pop(0)` is linear](https://docs.python.org/3/tutorial/datastructures.html#using-lists-as-queues), so consuming all items in the test results in quadratic iteration.
Switching to `collections.deque` with `popleft()` expresses FIFO intent and avoids the O(n^2) path.
Behavior is unchanged - for a few hundred items the perf impact is likely negligible.
💬 l0rinc commented on pull request "test: Remove polling loop from test_runner (take 2)":
(https://github.com/bitcoin/bitcoin/pull/33141#discussion_r2323760016)
Pushed to https://github.com/bitcoin/bitcoin/pull/33313, please resolve the comment
(https://github.com/bitcoin/bitcoin/pull/33141#discussion_r2323760016)
Pushed to https://github.com/bitcoin/bitcoin/pull/33313, please resolve the comment
💬 fanquake commented on pull request "doc: fix `LIBRARY_PATH` comment":
(https://github.com/bitcoin/bitcoin/pull/33308#discussion_r2323762474)
I think it's clear it applies to both, given it's the same code?
(https://github.com/bitcoin/bitcoin/pull/33308#discussion_r2323762474)
I think it's clear it applies to both, given it's the same code?
💬 fanquake commented on issue "cmake: Errors not actually errors?":
(https://github.com/bitcoin/bitcoin/issues/33153#issuecomment-3256452263)
> the limited time left before branching off,
I don't see how that's relevant. Any fix merged into master over the next few weeks can be backported to 30.x.
(https://github.com/bitcoin/bitcoin/issues/33153#issuecomment-3256452263)
> the limited time left before branching off,
I don't see how that's relevant. Any fix merged into master over the next few weeks can be backported to 30.x.
⚠️ www222fff opened an issue: "Is there way to sort balance address functionality just like btcrank.site showing"
(https://github.com/bitcoin/bitcoin/issues/33314)
### Please describe the feature you'd like to see added.
Is there way to sort balance address functionality just like btcrank.site showing,
Some invest want to get the real time live balance address with sort.
### Is your feature related to a problem, if so please describe it.
_No response_
### Describe the solution you'd like
_No response_
### Describe any alternatives you've considered
_No response_
### Please leave any additional context
_No response_
(https://github.com/bitcoin/bitcoin/issues/33314)
### Please describe the feature you'd like to see added.
Is there way to sort balance address functionality just like btcrank.site showing,
Some invest want to get the real time live balance address with sort.
### Is your feature related to a problem, if so please describe it.
_No response_
### Describe the solution you'd like
_No response_
### Describe any alternatives you've considered
_No response_
### Please leave any additional context
_No response_
🤔 yuvicc reviewed a pull request: "net: Prevent node from binding to the same `CService`"
(https://github.com/bitcoin/bitcoin/pull/33231#pullrequestreview-3158265913)
ACK 1c40b32597712059d5d809925da0e9adccac0fb3
`bitcoind` stops with error message for duplicate bind options.
```
Error: Duplicate binding configuration for address 0.0.0.0:8333. Please check your -bind, -bind=...=onion and -whitebind settings.
2025-09-05T05:15:05Z torcontrol thread start
2025-09-05T05:15:05Z Loading 0 mempool transactions from file...
2025-09-05T05:15:05Z Imported mempool transactions from file: 0 succeeded, 0 failed, 0 expired, 0 already there, 0 waiting for initial br
...
(https://github.com/bitcoin/bitcoin/pull/33231#pullrequestreview-3158265913)
ACK 1c40b32597712059d5d809925da0e9adccac0fb3
`bitcoind` stops with error message for duplicate bind options.
```
Error: Duplicate binding configuration for address 0.0.0.0:8333. Please check your -bind, -bind=...=onion and -whitebind settings.
2025-09-05T05:15:05Z torcontrol thread start
2025-09-05T05:15:05Z Loading 0 mempool transactions from file...
2025-09-05T05:15:05Z Imported mempool transactions from file: 0 succeeded, 0 failed, 0 expired, 0 already there, 0 waiting for initial br
...
💬 yuvicc commented on pull request "net: Prevent node from binding to the same `CService`":
(https://github.com/bitcoin/bitcoin/pull/33231#discussion_r2302839967)
```suggestion
std::optional<CService> CheckBindingConflicts(const CConnman::Options& conn_options)
```
nit: as per the [guidelines](https://github.com/bitcoin/bitcoin/blob/6ca6f3b37b992591726bd13b494369bee3bd6468/doc/developer-notes.md?plain=1#L38), function arguments should be `snake_case`.
(https://github.com/bitcoin/bitcoin/pull/33231#discussion_r2302839967)
```suggestion
std::optional<CService> CheckBindingConflicts(const CConnman::Options& conn_options)
```
nit: as per the [guidelines](https://github.com/bitcoin/bitcoin/blob/6ca6f3b37b992591726bd13b494369bee3bd6468/doc/developer-notes.md?plain=1#L38), function arguments should be `snake_case`.
💬 l0rinc commented on pull request "net: Prevent node from binding to the same `CService`":
(https://github.com/bitcoin/bitcoin/pull/33231#discussion_r2324140790)
@yuvicc, this still needs fixing
(https://github.com/bitcoin/bitcoin/pull/33231#discussion_r2324140790)
@yuvicc, this still needs fixing
💬 yuvicc commented on issue "InitError() doesn't always halt node startup when blockchain state exists":
(https://github.com/bitcoin/bitcoin/issues/33276#issuecomment-3257139172)
Tried on my local system, I was able to reproduce that.
```
2025-09-05T05:39:58Z UpdateTip: new best=000000004b6da5bbb89d6602b47021c817837330c94c92ffdf0fe7a3fc65fa57 height=1057 version=0x00000001 log2_work=42.047146 tx=1079 date='2009-01-19T19:36:59Z' progress=0.000001 cache=0.3MiB(74txo)
2025-09-05T05:39:58Z UpdateTip: new best=00000000d357309918e1e2586ca15141650f39b8097169274ccdba05991a87b3 height=1058 version=0x00000001 log2_work=42.048509 tx=1080 date='2009-01-19T19:43:41Z' progress=0.0000
...
(https://github.com/bitcoin/bitcoin/issues/33276#issuecomment-3257139172)
Tried on my local system, I was able to reproduce that.
```
2025-09-05T05:39:58Z UpdateTip: new best=000000004b6da5bbb89d6602b47021c817837330c94c92ffdf0fe7a3fc65fa57 height=1057 version=0x00000001 log2_work=42.047146 tx=1079 date='2009-01-19T19:36:59Z' progress=0.000001 cache=0.3MiB(74txo)
2025-09-05T05:39:58Z UpdateTip: new best=00000000d357309918e1e2586ca15141650f39b8097169274ccdba05991a87b3 height=1058 version=0x00000001 log2_work=42.048509 tx=1080 date='2009-01-19T19:43:41Z' progress=0.0000
...
💬 kannapoix commented on pull request "doc: update multisig tutorial to use multipath descriptors":
(https://github.com/bitcoin/bitcoin/pull/33286#issuecomment-3257174144)
ACK 226836e70501abf9ac2434900e74eec31edd716e
I followed the new instruction and successfully spend from multisig wallet: https://mempool.space/signet/tx/6b5b7922b68b6becfc3aea5a541ce700837d5105d332889929b46cf1428df4a4
Although not directly related to this PR, I have a minor suggestion: it might be helpful to add a loadwallet command for multisig_wallet_01, not just for participant wallets.
````diff
diff --git a/doc/multisig-tutorial.md b/doc/multisig-tutorial.md
index fa6366df91d..352
...
(https://github.com/bitcoin/bitcoin/pull/33286#issuecomment-3257174144)
ACK 226836e70501abf9ac2434900e74eec31edd716e
I followed the new instruction and successfully spend from multisig wallet: https://mempool.space/signet/tx/6b5b7922b68b6becfc3aea5a541ce700837d5105d332889929b46cf1428df4a4
Although not directly related to this PR, I have a minor suggestion: it might be helpful to add a loadwallet command for multisig_wallet_01, not just for participant wallets.
````diff
diff --git a/doc/multisig-tutorial.md b/doc/multisig-tutorial.md
index fa6366df91d..352
...
💬 TheCharlatan commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2324220680)
That seems fine, though it would be good to get the fee bumping functionality tested here (in a follow up).
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2324220680)
That seems fine, though it would be good to get the fee bumping functionality tested here (in a follow up).
💬 Sjors commented on pull request "build: suggest -DENABLE_IPC=OFF when missing capnp":
(https://github.com/bitcoin/bitcoin/pull/33290#discussion_r2324239243)
Let's go for that approach then.
(https://github.com/bitcoin/bitcoin/pull/33290#discussion_r2324239243)
Let's go for that approach then.
✅ Sjors closed a pull request: "build: suggest -DENABLE_IPC=OFF when missing capnp"
(https://github.com/bitcoin/bitcoin/pull/33290)
(https://github.com/bitcoin/bitcoin/pull/33290)