📝 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
...

💬 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

💬 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.

💬 hebasto commented on pull request "build, docs: Fix Boost-related issues on NetBSD":
(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
...

💬 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?

💬 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.

💬 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.

💬 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...

💬 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.

💬 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.

💬 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?

💬 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
...

💬 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.

💬 maflcko commented on pull request "rest: rename `strURIPart` to `uri_part`":
(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.

🤔 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

💬 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_r2174818751)
Would be nice to keep the [length of lines in commit messages shorter](https://dev.to/noelworden/improving-your-commit-message-with-the-50-72-rule-3g79).

💬 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_r2174824246)
The testing framework will add `-bind=0.0.0.0` we want to test this. But we also want to test if no `-bind` is given at all. Here is a patch to do that and also to test `-whitebind=0.0.0.0`:

```diff
--- i/test/functional/feature_bind_port_discover.py
+++ w/test/functional/feature_bind_port_discover.py
@@ -26,23 +26,47 @@ from test_framework.util import (
ADDR1 = '1.1.1.1'
ADDR2 = '2.2.2.2'

class BindPortDiscoverTest(BitcoinTestFramework):
def set_test_params(self):

...

👍 theStack approved a pull request: "test: Add `msgtype` to `msg_generic` slots"
(https://github.com/bitcoin/bitcoin/pull/32833#pullrequestreview-2970865694)
ACK 7dc43ea503a2c145ffd4fe14b794300bfc2bcdee