💬 w0xlt commented on pull request "net: Prevent node from binding to the same `CService`":
(https://github.com/bitcoin/bitcoin/pull/33231#discussion_r2305020054)
The execution will be stopped either way (in the master branch or in this PR).
This PR simply prevents the node from attempting to connect twice to the same address and port, and instead terminates it with a clear message — rather than the confusing one currently shown in the master branch.
> I guess the first err msg is from logging while the second is from init.cpp
Yes, that makes sense. I'll check again.
(https://github.com/bitcoin/bitcoin/pull/33231#discussion_r2305020054)
The execution will be stopped either way (in the master branch or in this PR).
This PR simply prevents the node from attempting to connect twice to the same address and port, and instead terminates it with a clear message — rather than the confusing one currently shown in the master branch.
> I guess the first err msg is from logging while the second is from init.cpp
Yes, that makes sense. I'll check again.
💬 l0rinc commented on pull request "net: Prevent node from binding to the same `CService`":
(https://github.com/bitcoin/bitcoin/pull/33231#discussion_r2305027502)
it just continues IBD on my side after the warning, it doesn't seem to stop after the error
(https://github.com/bitcoin/bitcoin/pull/33231#discussion_r2305027502)
it just continues IBD on my side after the warning, it doesn't seem to stop after the error
🤔 hodlinator reviewed a pull request: "clang-format: align brace-after-struct and *-class formatting"
(https://github.com/bitcoin/bitcoin/pull/32813#pullrequestreview-3161358325)
ACK 7c9b3e1eae8f206753457149f1b1c837f6627d6d (first commit)
Ran `clang-format -dump-config -style=file:src/.clang-format > foo` on that commit and diffed *foo* against *src/.clang-format*. Only differences where added/elaborated settings from me running Clang-format 19. Makes sense to keep the format at the minimum version we support.
(https://github.com/bitcoin/bitcoin/pull/32813#pullrequestreview-3161358325)
ACK 7c9b3e1eae8f206753457149f1b1c837f6627d6d (first commit)
Ran `clang-format -dump-config -style=file:src/.clang-format > foo` on that commit and diffed *foo* against *src/.clang-format*. Only differences where added/elaborated settings from me running Clang-format 19. Makes sense to keep the format at the minimum version we support.
💬 hodlinator commented on pull request "clang-format: align brace-after-struct and *-class formatting":
(https://github.com/bitcoin/bitcoin/pull/32813#discussion_r2305026004)
Ran `clang-format -i -- **.cpp **.h **.hpp` in *src/* for both `AfterStruct` true & false, and compared diffs. Man was it close.
| true (94364a7d5403d9d480ebc065f8709c6dd21e543f) | false |
| -------- | -------- |
| `struct\n{` | `struct {` |
| 85'849 lines | 85'659 lines |
Even though `struct` & `class` can be used interchangeably in C++, only with different defaults for public/private, I think we might as well benefit from the fact that we have 2 concepts and use them to communicate whic
...
(https://github.com/bitcoin/bitcoin/pull/32813#discussion_r2305026004)
Ran `clang-format -i -- **.cpp **.h **.hpp` in *src/* for both `AfterStruct` true & false, and compared diffs. Man was it close.
| true (94364a7d5403d9d480ebc065f8709c6dd21e543f) | false |
| -------- | -------- |
| `struct\n{` | `struct {` |
| 85'849 lines | 85'659 lines |
Even though `struct` & `class` can be used interchangeably in C++, only with different defaults for public/private, I think we might as well benefit from the fact that we have 2 concepts and use them to communicate whic
...
💬 w0xlt commented on pull request "net: Prevent node from binding to the same `CService`":
(https://github.com/bitcoin/bitcoin/pull/33231#discussion_r2305179063)
If you are running in the foreground, `InitError()` prints the message to the console. In `debug.log`, only a single entry is recorded.
This behavior applies to all `InitError()` calls.
https://github.com/bitcoin/bitcoin/blob/6ca6f3b37b992591726bd13b494369bee3bd6468/src/node/interface_ui.cpp#L64
> it just continues IBD on my side after the warning, it doesn't seem to stop after the error
Could you please check again? `InitError()` returns false, which should stop execution.
https://gith
...
(https://github.com/bitcoin/bitcoin/pull/33231#discussion_r2305179063)
If you are running in the foreground, `InitError()` prints the message to the console. In `debug.log`, only a single entry is recorded.
This behavior applies to all `InitError()` calls.
https://github.com/bitcoin/bitcoin/blob/6ca6f3b37b992591726bd13b494369bee3bd6468/src/node/interface_ui.cpp#L64
> it just continues IBD on my side after the warning, it doesn't seem to stop after the error
Could you please check again? `InitError()` returns false, which should stop execution.
https://gith
...
💬 achow101 commented on pull request "test, refactor: Embedded ASmap selected preparatory work":
(https://github.com/bitcoin/bitcoin/pull/33026#issuecomment-3229648177)
ACK 734737b5930df7cebab83cf0dbe5fd390143f2be
(https://github.com/bitcoin/bitcoin/pull/33026#issuecomment-3229648177)
ACK 734737b5930df7cebab83cf0dbe5fd390143f2be
💬 l0rinc commented on pull request "net: Prevent node from binding to the same `CService`":
(https://github.com/bitcoin/bitcoin/pull/33231#discussion_r2305302493)
I tried it again:
```bash
git log -1 | head
commit 28fb7f74c4ae66dcb0f918774d644ac3c393f25b
Author: woltx <94266259+w0xlt@users.noreply.github.com>
Date: Mon Aug 25 20:22:33 2025 -0700
net: Prevent node from binding to the same CService
```
and did:
```bash
rm -rfd build && \
cmake -B build -G Ninja -DCMAKE_BUILD_TYPE=Release && \
ninja -C build && \
build/bin/bitcoind -bind=127.0.0.1:11012 -bind=127.0.0.1:11012
```
which resulted in:
```
...
2025-08-27T21:07:33Z ini
...
(https://github.com/bitcoin/bitcoin/pull/33231#discussion_r2305302493)
I tried it again:
```bash
git log -1 | head
commit 28fb7f74c4ae66dcb0f918774d644ac3c393f25b
Author: woltx <94266259+w0xlt@users.noreply.github.com>
Date: Mon Aug 25 20:22:33 2025 -0700
net: Prevent node from binding to the same CService
```
and did:
```bash
rm -rfd build && \
cmake -B build -G Ninja -DCMAKE_BUILD_TYPE=Release && \
ninja -C build && \
build/bin/bitcoind -bind=127.0.0.1:11012 -bind=127.0.0.1:11012
```
which resulted in:
```
...
2025-08-27T21:07:33Z ini
...
💬 l0rinc commented on pull request "clang-format: align brace-after-struct and *-class formatting":
(https://github.com/bitcoin/bitcoin/pull/32813#discussion_r2305316780)
I would be fine with both as long as it's explicit. I still find the rationale that `struct` and `class` should be formatted similarly, it's harder for me to explain why `struct` is more like a for loop than a `class`...
> 85'849 lines
I have no idea what these numbers mean, I checked same-line opening braces with
```bash
% grep -E '^\s*\bstruct\b\s+[^;{]*\{' --include="*.h" --include="*.cpp" -r src/ | wc -l
393
```
and quickly vibe coded a script that does newlines:
<detai
...
(https://github.com/bitcoin/bitcoin/pull/32813#discussion_r2305316780)
I would be fine with both as long as it's explicit. I still find the rationale that `struct` and `class` should be formatted similarly, it's harder for me to explain why `struct` is more like a for loop than a `class`...
> 85'849 lines
I have no idea what these numbers mean, I checked same-line opening braces with
```bash
% grep -E '^\s*\bstruct\b\s+[^;{]*\{' --include="*.h" --include="*.cpp" -r src/ | wc -l
393
```
and quickly vibe coded a script that does newlines:
<detai
...
💬 w0xlt commented on pull request "net: Prevent node from binding to the same `CService`":
(https://github.com/bitcoin/bitcoin/pull/33231#discussion_r2305337219)
That's strange. On my machine, it's interrupted.
I tried running it by pointing the `-datadir` to an empty folder to see if that was the reason, but it also stopped (shown below).
Wouldn't the `feature_bind_extra` test fail if the node didn't stop?
Are you using a configuration file? Could some settings be affecting this?
```
2025-08-27T21:27:45Z Loaded best chain: hashBestChain=000000000019d6689c085ae165831e934ff763ae46a2a6c172b3f1b60a8ce26f height=0 date=2009-01-03T18:15:05Z progres
...
(https://github.com/bitcoin/bitcoin/pull/33231#discussion_r2305337219)
That's strange. On my machine, it's interrupted.
I tried running it by pointing the `-datadir` to an empty folder to see if that was the reason, but it also stopped (shown below).
Wouldn't the `feature_bind_extra` test fail if the node didn't stop?
Are you using a configuration file? Could some settings be affecting this?
```
2025-08-27T21:27:45Z Loaded best chain: hashBestChain=000000000019d6689c085ae165831e934ff763ae46a2a6c172b3f1b60a8ce26f height=0 date=2009-01-03T18:15:05Z progres
...
💬 murchandamus commented on pull request "coinselection: Tiebreak SRD eviction by weight":
(https://github.com/bitcoin/bitcoin/pull/33223#issuecomment-3229819476)
I want to use the opportunity to move the SRD tests to the coinselection_tests, but haven’t gotten around to it yet.
(https://github.com/bitcoin/bitcoin/pull/33223#issuecomment-3229819476)
I want to use the opportunity to move the SRD tests to the coinselection_tests, but haven’t gotten around to it yet.
💬 l0rinc commented on pull request "net: Prevent node from binding to the same `CService`":
(https://github.com/bitcoin/bitcoin/pull/33231#discussion_r2305346479)
`feature_bind_extra` is skipped on a Mac
(https://github.com/bitcoin/bitcoin/pull/33231#discussion_r2305346479)
`feature_bind_extra` is skipped on a Mac
💬 w0xlt commented on pull request "net: Prevent node from binding to the same `CService`":
(https://github.com/bitcoin/bitcoin/pull/33231#discussion_r2305363666)
I’m also testing on a Mac right now.
On both Linux and macOS ARM (Sequoia 15.5), the node stops as expected.
It’s curious that on your machine `InitError()` prints and logs the error but doesn’t actually stop the node.
(https://github.com/bitcoin/bitcoin/pull/33231#discussion_r2305363666)
I’m also testing on a Mac right now.
On both Linux and macOS ARM (Sequoia 15.5), the node stops as expected.
It’s curious that on your machine `InitError()` prints and logs the error but doesn’t actually stop the node.
💬 davidgumberg commented on pull request "test: fix p2p_leak_tx.py":
(https://github.com/bitcoin/bitcoin/pull/33121#issuecomment-3229951194)
Ideally, `p2p_leak_tx.py` would be rewritten so that it would deterministically exercise both branches, my impression is the that we only really care about the first case, that tx'es that haven't been inv'ed don't get leaked, but exercising the second case is a good validation of the assumptions of the test, and would have caught that the existing test was broken.
One solution is to set a mocktime as suggested by @enirox001 above so that no time passes and no inv's can be sent and the `getdat
...
(https://github.com/bitcoin/bitcoin/pull/33121#issuecomment-3229951194)
Ideally, `p2p_leak_tx.py` would be rewritten so that it would deterministically exercise both branches, my impression is the that we only really care about the first case, that tx'es that haven't been inv'ed don't get leaked, but exercising the second case is a good validation of the assumptions of the test, and would have caught that the existing test was broken.
One solution is to set a mocktime as suggested by @enirox001 above so that no time passes and no inv's can be sent and the `getdat
...
💬 davidgumberg commented on pull request "ci: return to using dash in CentOS job":
(https://github.com/bitcoin/bitcoin/pull/33261#issuecomment-3229956996)
ACK https://github.com/bitcoin/bitcoin/commit/509ffea40abbc706ef8b8fc449b7de8677fc5096
(https://github.com/bitcoin/bitcoin/pull/33261#issuecomment-3229956996)
ACK https://github.com/bitcoin/bitcoin/commit/509ffea40abbc706ef8b8fc449b7de8677fc5096
💬 achow101 commented on pull request "Wallet: Add `maxfeerate` wallet startup option":
(https://github.com/bitcoin/bitcoin/pull/29278#discussion_r2305442956)
In 0d9f0888542081b1792705594b570ae748a9f839 "[wallet]: add `maxfeerate` wallet startup option"
`-maxtxfee` adds a warning if the max fee is too high. I think it would make sense to have a similar warning for `-maxfeerate`.
(https://github.com/bitcoin/bitcoin/pull/29278#discussion_r2305442956)
In 0d9f0888542081b1792705594b570ae748a9f839 "[wallet]: add `maxfeerate` wallet startup option"
`-maxtxfee` adds a warning if the max fee is too high. I think it would make sense to have a similar warning for `-maxfeerate`.
💬 achow101 commented on pull request "Wallet: Add `maxfeerate` wallet startup option":
(https://github.com/bitcoin/bitcoin/pull/29278#discussion_r2305470201)
In 99b45844cab7896e206dd0b8e24355586bc1a84d "[wallet]: warn user if 1kvb tx with `-maxtxfee` base fee has fee rate less than `minrelaytxfee`"
This check does not make sense to me. This converts the max tx fee back into a feerate just for this check, even though `-maxfeerate` has taken over that check. I don't think we need to have this check here.
(https://github.com/bitcoin/bitcoin/pull/29278#discussion_r2305470201)
In 99b45844cab7896e206dd0b8e24355586bc1a84d "[wallet]: warn user if 1kvb tx with `-maxtxfee` base fee has fee rate less than `minrelaytxfee`"
This check does not make sense to me. This converts the max tx fee back into a feerate just for this check, even though `-maxfeerate` has taken over that check. I don't think we need to have this check here.
💬 mzumsande commented on pull request "test: fix p2p_leak_tx.py":
(https://github.com/bitcoin/bitcoin/pull/33121#issuecomment-3229982021)
Thanks for the reviews and suggestions - I was busy the last couple of weeks, but I'll look at them in detail / update the PR next week!
(https://github.com/bitcoin/bitcoin/pull/33121#issuecomment-3229982021)
Thanks for the reviews and suggestions - I was busy the last couple of weeks, but I'll look at them in detail / update the PR next week!
🤔 luke-jr reviewed a pull request: "rpc, logging: add backgroundvalidation to getblockchaininfo"
(https://github.com/bitcoin/bitcoin/pull/33259#pullrequestreview-3162020387)
Concept ACK. Would be nice to test the active state.
(https://github.com/bitcoin/bitcoin/pull/33259#pullrequestreview-3162020387)
Concept ACK. Would be nice to test the active state.
💬 lucifermmmenriquejr commented on pull request "rpc, logging: add backgroundvalidation to getblockchaininfo":
(https://github.com/bitcoin/bitcoin/pull/33259#issuecomment-3230000566)
Thanks for asking me to reply lol 😘
EnriqueRamirez
On Wed, 27 Aug 2025, 4:50 pm Luke Dashjr, ***@***.***> wrote:
> ***@***.**** commented on this pull request.
>
> Concept ACK. Would be nice to test the active state.
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/bitcoin/bitcoin/pull/33259#pullrequestreview-3162020387>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/BSKJHMLB5H7PZXGL3ULONU33PYY4FAVCNFSM6AAAAACE4PAMZ2VHI2DSMVQ
...
(https://github.com/bitcoin/bitcoin/pull/33259#issuecomment-3230000566)
Thanks for asking me to reply lol 😘
EnriqueRamirez
On Wed, 27 Aug 2025, 4:50 pm Luke Dashjr, ***@***.***> wrote:
> ***@***.**** commented on this pull request.
>
> Concept ACK. Would be nice to test the active state.
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/bitcoin/bitcoin/pull/33259#pullrequestreview-3162020387>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/BSKJHMLB5H7PZXGL3ULONU33PYY4FAVCNFSM6AAAAACE4PAMZ2VHI2DSMVQ
...
💬 l0rinc commented on pull request "net: Prevent node from binding to the same `CService`":
(https://github.com/bitcoin/bitcoin/pull/33231#discussion_r2305521667)
Tried the same on Linux:
```bash
# build/bin/bitcoind -datadir=/mnt/my_storage/BitcoinData -bind=127.0.0.1:11012 -bind=127.0.0.1:11012
2025-08-27T23:09:10Z Bitcoin Core version v29.99.0-28fb7f74c4ae (release build)
2025-08-27T23:09:10Z parameter interaction: -bind set -> setting -listen=1
...
(https://github.com/bitcoin/bitcoin/pull/33231#discussion_r2305521667)
Tried the same on Linux:
```bash
# build/bin/bitcoind -datadir=/mnt/my_storage/BitcoinData -bind=127.0.0.1:11012 -bind=127.0.0.1:11012
2025-08-27T23:09:10Z Bitcoin Core version v29.99.0-28fb7f74c4ae (release build)
2025-08-27T23:09:10Z parameter interaction: -bind set -> setting -listen=1
...