💬 TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3018588171)
Updated d9e030d56343bb452d86169f77ddfb64f7160235 -> 690a5dac223ed18a65c9d9e6c535466cc3ad4511 ([kernelApi_41](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_41) -> [kernelApi_42](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_42), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelApi_41..kernelApi_42))
* Addressed @stringintech's [comment](https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2173853678), using the correct name for the kernel component. This w
...
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3018588171)
Updated d9e030d56343bb452d86169f77ddfb64f7160235 -> 690a5dac223ed18a65c9d9e6c535466cc3ad4511 ([kernelApi_41](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_41) -> [kernelApi_42](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_42), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelApi_41..kernelApi_42))
* Addressed @stringintech's [comment](https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2173853678), using the correct name for the kernel component. This w
...
⚠️ saravadeanil reopened an issue: "[signet v22.0.0] Unable to sync node from scratch"
(https://github.com/bitcoin/bitcoin/issues/32762)
Hi, I am trying to run bitcoin signet node with below configurations:-
- Bitcore release:- `v22.0.0`
- Parameters:-
```
- '-chain=signet'
- '-rest'
- '-txindex'
- '-disablewallet'
- '-server'
- '-rpcbind=0.0.0.0'
- '-rpcport=38332'
- '-rpcallowip=0.0.0.0/0'
- '-rpcthreads=512'
- '-rpcworkqueue=4096'
- '-rpcuser=xxxxxxxxxx'
- '-rpcpassword=xxxxxxxxxx'
...
(https://github.com/bitcoin/bitcoin/issues/32762)
Hi, I am trying to run bitcoin signet node with below configurations:-
- Bitcore release:- `v22.0.0`
- Parameters:-
```
- '-chain=signet'
- '-rest'
- '-txindex'
- '-disablewallet'
- '-server'
- '-rpcbind=0.0.0.0'
- '-rpcport=38332'
- '-rpcallowip=0.0.0.0/0'
- '-rpcthreads=512'
- '-rpcworkqueue=4096'
- '-rpcuser=xxxxxxxxxx'
- '-rpcpassword=xxxxxxxxxx'
...
✅ saravadeanil closed an issue: "[signet v22.0.0] Unable to sync node from scratch"
(https://github.com/bitcoin/bitcoin/issues/32762)
(https://github.com/bitcoin/bitcoin/issues/32762)
📝 dergoegge opened a pull request: "test: Add `msgtype` to `msg_generic` slots"
(https://github.com/bitcoin/bitcoin/pull/32833)
`msg_generic` can't be used unless `msgtype` is listed in `__slots__`
Example from a [custom test](https://github.com/dergoegge/bitcoin/blob/6329ce979f63b396aa036a1ad39798bb83fa4ade/test/functional/p2p_bug28676.py):
```
2025-06-30T10:14:55.418000Z TestFramework (INFO): PRNG seed is: 3137163719543762151
2025-06-30T10:14:55.418000Z TestFramework (INFO): Initializing test directory /tmp/nix-shell-110135-0/bitcoin_func_test_7lmiemmp
2025-06-30T10:14:55.675000Z TestFramework (INFO): Setting
...
(https://github.com/bitcoin/bitcoin/pull/32833)
`msg_generic` can't be used unless `msgtype` is listed in `__slots__`
Example from a [custom test](https://github.com/dergoegge/bitcoin/blob/6329ce979f63b396aa036a1ad39798bb83fa4ade/test/functional/p2p_bug28676.py):
```
2025-06-30T10:14:55.418000Z TestFramework (INFO): PRNG seed is: 3137163719543762151
2025-06-30T10:14:55.418000Z TestFramework (INFO): Initializing test directory /tmp/nix-shell-110135-0/bitcoin_func_test_7lmiemmp
2025-06-30T10:14:55.675000Z TestFramework (INFO): Setting
...
💬 maflcko commented on pull request "test: Add `msgtype` to `msg_generic` slots":
(https://github.com/bitcoin/bitcoin/pull/32833#issuecomment-3018612787)
lgtm ACK 7dc43ea503a2c145ffd4fe14b794300bfc2bcdee
Could add a small smoke test, maybe in `./test/functional/p2p_ping.py`, but seems fine either way
(https://github.com/bitcoin/bitcoin/pull/32833#issuecomment-3018612787)
lgtm ACK 7dc43ea503a2c145ffd4fe14b794300bfc2bcdee
Could add a small smoke test, maybe in `./test/functional/p2p_ping.py`, but seems fine either way
💬 fanquake commented on pull request "build, docs: Fix Boost-related issues on NetBSD":
(https://github.com/bitcoin/bitcoin/pull/32828#discussion_r2174748759)
This should link to the relevant issue.
(https://github.com/bitcoin/bitcoin/pull/32828#discussion_r2174748759)
This should link to the relevant issue.
💬 hebasto commented on pull request "build, docs: Fix Boost-related issues on NetBSD":
(https://github.com/bitcoin/bitcoin/pull/32828#discussion_r2174762553)
Sure! Added.
(https://github.com/bitcoin/bitcoin/pull/32828#discussion_r2174762553)
Sure! Added.
💬 Sjors commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#issuecomment-3018644831)
> `AreInputsStandard` is always called with a warm cache. The benchmark is not.
The `bench.run` call does not clear `coins`, so it's warm after the first iteration. However it's not measuring loading coins from disk. If that's only determined by the disk seek time [0], you're looking at 80 to 160 nanoseconds. My understanding is that our UTXO cache is mostly there to prevent writes, which are much slower than reads.
[0] https://en.wikipedia.org/wiki/Hard_disk_drive_performance_characterist
...
(https://github.com/bitcoin/bitcoin/pull/32521#issuecomment-3018644831)
> `AreInputsStandard` is always called with a warm cache. The benchmark is not.
The `bench.run` call does not clear `coins`, so it's warm after the first iteration. However it's not measuring loading coins from disk. If that's only determined by the disk seek time [0], you're looking at 80 to 160 nanoseconds. My understanding is that our UTXO cache is mostly there to prevent writes, which are much slower than reads.
[0] https://en.wikipedia.org/wiki/Hard_disk_drive_performance_characterist
...
💬 maflcko commented on issue "[IBD] Raspberry Pi: 90% CPU time for 1.5% of block processing":
(https://github.com/bitcoin/bitcoin/issues/32832#issuecomment-3018646771)
> ((900000-[886157](https://mempool.space/block/00000000000000000001b658dd1120e82e66d2790811f89ede9742ada3ed6d77))/900000=1.5%)
Could clarify in the title or in the text that this is for blocks after assumevalid?
(https://github.com/bitcoin/bitcoin/issues/32832#issuecomment-3018646771)
> ((900000-[886157](https://mempool.space/block/00000000000000000001b658dd1120e82e66d2790811f89ede9742ada3ed6d77))/900000=1.5%)
Could clarify in the title or in the text that this is for blocks after assumevalid?
💬 hebasto commented on pull request "build, docs: Fix Boost-related issues on NetBSD":
(https://github.com/bitcoin/bitcoin/pull/32828#issuecomment-3018658040)
> > The boost-headers package does not provide CMake configuration files
>
> Is there an upstream issue for this?
I've updated the PR description.
(https://github.com/bitcoin/bitcoin/pull/32828#issuecomment-3018658040)
> > The boost-headers package does not provide CMake configuration files
>
> Is there an upstream issue for this?
I've updated the PR description.
💬 Sjors commented on pull request "p2p: add more bad ports":
(https://github.com/bitcoin/bitcoin/pull/32826#issuecomment-3018661110)
utACK 6967e8e8abbc35ac98e8e3745a8bbed56e77526f
I checked that the ports make sense.
(https://github.com/bitcoin/bitcoin/pull/32826#issuecomment-3018661110)
utACK 6967e8e8abbc35ac98e8e3745a8bbed56e77526f
I checked that the ports make sense.
💬 l0rinc commented on pull request "mempool: Avoid needless vtx iteration in `removeForBlock` during IBD":
(https://github.com/bitcoin/bitcoin/pull/32827#discussion_r2174789941)
That's *the* question, this is what I was referring to in the PR description:
> Draft until I get feedback on whether `MempoolTransactionsRemovedForBlock` [should still be called for empty vectors](https://github.com/bitcoin/bitcoin/pull/32730#discussion_r2140931183), given that `feature_fee_estimation` is failing if we skip it... Would be cool if we could avoid adding another callback into the validation queue...
(https://github.com/bitcoin/bitcoin/pull/32827#discussion_r2174789941)
That's *the* question, this is what I was referring to in the PR description:
> Draft until I get feedback on whether `MempoolTransactionsRemovedForBlock` [should still be called for empty vectors](https://github.com/bitcoin/bitcoin/pull/32730#discussion_r2140931183), given that `feature_fee_estimation` is failing if we skip it... Would be cool if we could avoid adding another callback into the validation queue...
💬 optout21 commented on pull request "mempool: Avoid needless vtx iteration in `removeForBlock` during IBD":
(https://github.com/bitcoin/bitcoin/pull/32827#discussion_r2174806312)
I think it's a different question: does it need to be called ...
1. when the mempool is empty, or
2. when the list of transactions to be removed is empty.
(https://github.com/bitcoin/bitcoin/pull/32827#discussion_r2174806312)
I think it's a different question: does it need to be called ...
1. when the mempool is empty, or
2. when the list of transactions to be removed is empty.
💬 maflcko commented on pull request "mempool: Avoid needless vtx iteration in `removeForBlock` during IBD":
(https://github.com/bitcoin/bitcoin/pull/32827#discussion_r2174809229)
Probably only if `firstRecordedHeight` is zero, which would need to be checked inside.
(https://github.com/bitcoin/bitcoin/pull/32827#discussion_r2174809229)
Probably only if `firstRecordedHeight` is zero, which would need to be checked inside.
💬 maflcko commented on pull request "mempool: Avoid needless vtx iteration in `removeForBlock` during IBD":
(https://github.com/bitcoin/bitcoin/pull/32827#issuecomment-3018723953)
> Similarly to [#32730 (comment)](https://github.com/bitcoin/bitcoin/pull/32730#discussion_r2140691354),
Could make sense to include that change here as well?
(https://github.com/bitcoin/bitcoin/pull/32827#issuecomment-3018723953)
> Similarly to [#32730 (comment)](https://github.com/bitcoin/bitcoin/pull/32730#discussion_r2140691354),
Could make sense to include that change here as well?
💬 vasild commented on pull request "net: Fix Discover() not running when using -bind=0.0.0.0:port":
(https://github.com/bitcoin/bitcoin/pull/32757#discussion_r2174811056)
> its already checked with `bind_on_any` bool.
I do not think so - the `bind_on_any` bool will be true if `-whitebind` and `-bind` are not given. The current patch will not run `Discover()` if `-whitebind=0.0.0.0` is used but it should. Here is an example patch to fix that:
```diff
--- i/src/init.cpp
+++ w/src/init.cpp
@@ -2030,12 +2030,20 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
if (bind_addr.IsBindAny()) {
shou
...
(https://github.com/bitcoin/bitcoin/pull/32757#discussion_r2174811056)
> its already checked with `bind_on_any` bool.
I do not think so - the `bind_on_any` bool will be true if `-whitebind` and `-bind` are not given. The current patch will not run `Discover()` if `-whitebind=0.0.0.0` is used but it should. Here is an example patch to fix that:
```diff
--- i/src/init.cpp
+++ w/src/init.cpp
@@ -2030,12 +2030,20 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
if (bind_addr.IsBindAny()) {
shou
...
💬 hebasto commented on pull request "build, docs: Fix Boost-related issues on NetBSD":
(https://github.com/bitcoin/bitcoin/pull/32828#issuecomment-3018727545)
> > The boost-headers package does not provide CMake configuration files
>
> Is there an upstream issue for this?
https://github.com/NetBSD/pkgsrc/issues/168.
(https://github.com/bitcoin/bitcoin/pull/32828#issuecomment-3018727545)
> > The boost-headers package does not provide CMake configuration files
>
> Is there an upstream issue for this?
https://github.com/NetBSD/pkgsrc/issues/168.
💬 maflcko commented on pull request "rest: rename `strURIPart` to `uri_part`":
(https://github.com/bitcoin/bitcoin/pull/32825#issuecomment-3018731998)
lgtm ACK 856f4235b1ae56540e1d2279c27405d44a5c7b34
(https://github.com/bitcoin/bitcoin/pull/32825#issuecomment-3018731998)
lgtm ACK 856f4235b1ae56540e1d2279c27405d44a5c7b34
👍 l0rinc approved a pull request: "p2p: add more bad ports"
(https://github.com/bitcoin/bitcoin/pull/32826#pullrequestreview-2970827023)
ACK 6967e8e8abbc35ac98e8e3745a8bbed56e77526f
Changes since my last review are comment updates and namespace inlines.
My only remaining concern is the unnecessary heavy memory usage of the test - but it's not a blocker.
(https://github.com/bitcoin/bitcoin/pull/32826#pullrequestreview-2970827023)
ACK 6967e8e8abbc35ac98e8e3745a8bbed56e77526f
Changes since my last review are comment updates and namespace inlines.
My only remaining concern is the unnecessary heavy memory usage of the test - but it's not a blocker.
🤔 vasild reviewed a pull request: "net: Fix Discover() not running when using -bind=0.0.0.0:port"
(https://github.com/bitcoin/bitcoin/pull/32757#pullrequestreview-2970826081)
Approach ACK 776b7ee585
(https://github.com/bitcoin/bitcoin/pull/32757#pullrequestreview-2970826081)
Approach ACK 776b7ee585