💬 dergoegge commented on pull request "ci: Split out native fuzz jobs for macOS and windows (take 2)":
(https://github.com/bitcoin/bitcoin/pull/31221#issuecomment-2500094356)
rfm?

💬 Sjors commented on issue "Stratum v2 via IPC Mining Interface tracking issue":
(https://github.com/bitcoin/bitcoin/issues/31098#issuecomment-2500137804)
In both versions `-sv2interval` specifies the _minimum_ time before sending a fee update, and it currently can't set lower than 1 second. This minimum is not related to polling between two processes.

It's there because determining the new fees requires the node to construct a whole new block template under the hood. This is currently inefficient so the node waits one second between each template generation. But once it finds a better template the node _pushses_ it to the `bitcoin-mine` proces
...

💬 TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r1858149291)
What do you think of doing something like this instead: https://github.com/TheCharlatan/bitcoin/commit/6323d7b072de5b13ab25aaa29e02332c44808b62

💬 maflcko commented on pull request "refactor: Clamp worker threads in ChainstateManager constructor":
(https://github.com/bitcoin/bitcoin/pull/31313#issuecomment-2500141836)
ACK 8f85d36d68ab33ba237407a2ed16667eb149d61f 🛳

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: ACK 8f85d36d68ab33ba237407a2ed
...

💬 maflcko commented on pull request "interpreter: Use the same type for SignatureHash in the definition":
(https://github.com/bitcoin/bitcoin/pull/31365#issuecomment-2500148749)
review ACK c288c790cd9abe91e53164aba5d975ef1e26ee3f 🐺

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: review ACK c288c790cd9a
...

👍 TheCharlatan approved a pull request: "cmake: Check `-Wno-*` compiler options for `leveldb` target"
(https://github.com/bitcoin/bitcoin/pull/31366#pullrequestreview-2460955639)
ACK 9e4a4b4832219d9d11da441779ab8a3b1304bd8b

💬 maflcko commented on pull request "refactor: Fix remaining clang-tidy performance-unnecessary-copy-initialization errors":
(https://github.com/bitcoin/bitcoin/pull/31364#issuecomment-2500166446)
review ACK 3305972f7bfd78181566e4297891c2dd7cae0f2 🏀

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: review ACK 3305972f7bfd7
...

💬 dergoegge commented on pull request "ci: limit max stack size to 512 KiB":
(https://github.com/bitcoin/bitcoin/pull/31367#issuecomment-2500175006)
Looks like there are some things to fix if we want to do this:

```
[11:46:11.067] Run str_printf with args ['/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/src/test/fuzz/fuzz', '-runs=1', PosixPath('/ci_container_base/ci/scratch/qa-assets/fuzz_corpora/str_printf')]INFO: Running with entropic power schedule (0xFF, 100).
[11:46:11.067] INFO: Seed: 2972386372
[11:46:11.067] INFO: Loaded 1 modules (627947 inline 8-bit counters): 627947 [0x55cf2473c058, 0x55cf247d5543),
[11:46:11.0
...

💬 maflcko commented on pull request "refactor: Fix remaining clang-tidy performance-inefficient-vector errors":
(https://github.com/bitcoin/bitcoin/pull/31305#issuecomment-2500182183)
review ACK 11f3bc229ccd4b20191855fb1df882cfa6145264 🎦

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: review ACK 11f3bc229ccd
...

💬 i-am-yuvi commented on issue "Prioritize processing of peers based on their CPU usage":
(https://github.com/bitcoin/bitcoin/issues/31033#issuecomment-2500190904)
> Good questions that I do not have an answer to.
>
> I guess this should start with planting some metrics - CPU time used by each peer in the last 1, 5 and 15 minutes and assessing what a "usual" situation looks like.
>
> Specific to CPU DoS via invalid transactions: for peers that send us such transactions, maybe multiply the CPU time by a factor e.g. 10x or 100x. Or the other way around - nullify CPU time spend on definitely-good-and-positive things like new blocks or new transactions t
...

💬 maflcko commented on pull request "ci: limit max stack size to 512 KiB":
(https://github.com/bitcoin/bitcoin/pull/31367#issuecomment-2500191918)
It looks like `DEBUG=1` has an effect on this?

💬 maflcko commented on pull request "ci: limit max stack size to 512 KiB":
(https://github.com/bitcoin/bitcoin/pull/31367#issuecomment-2500196258)
> tinyformat

Pretty sure this can be ignored, as in this codebase it is not relevant, see https://github.com/c42f/tinyformat/issues/70#issuecomment-579391305

💬 vasild commented on issue "Prioritize processing of peers based on their CPU usage":
(https://github.com/bitcoin/bitcoin/issues/31033#issuecomment-2500225499)
We already keep stats for each and every peer, displayed in the `getpeerinfo` RPC, e.g. `bitcoin-cli getpeerinfo`.

💬 hodlinator commented on pull request "test: Shut down framework cleanly on RPC connection failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r1858229897)
I am satisfied with the PR in it's latest revision, but if you don't consider this a nit, but rather a blocker, let me know.

💬 evgeniytar commented on issue "Discover() will not run if listening on any address with an explicit bind=0.0.0.0":
(https://github.com/bitcoin/bitcoin/issues/31293#issuecomment-2500256851)
Applied patch provided by @vasild and test run. But seems like `Discover()` indeed doesn't see IPv4. I printed `self.nodes[0].getnetworkinfo()['localaddresses']` before first assertion and it shows only IPv6 addresses.

```python3
[{'address': '2a0d:3344:34da:6410::fc5', 'port': 31001, 'score': 1}, {'address': '2a0d:3344:34da:6410:80:af5f:cf76:f4b9', 'port': 31001, 'score': 1}, {'address': '2a0d:3344:34da:6410:8b8:ca87:43b2:565f', 'port': 31001, 'score': 1}]
```

Second node displays empt
...

💬 hebasto commented on pull request "refactor: Fix remaining clang-tidy performance-unnecessary-copy-initialization errors":
(https://github.com/bitcoin/bitcoin/pull/31364#discussion_r1858230932)
The `prev_spk` variable is passed to the `CTxOut` ctor, which takes a copy rather than a const reference.

This raises a concern about the correctness of the change, doesn't it?

👍 rkrux approved a pull request: "doc, test: more ephemeral dust follow-ups"
(https://github.com/bitcoin/bitcoin/pull/31371#pullrequestreview-2461044048)
tACK 160799d9135528dbdea40690f0bb0d56c6c4803a

I agree with this change, gets rid of duplication. Successful make and functional tests.

I noticed 3 call sites at the end where the returned `sweep_tx` is not used, though it seems redundant but I don't suppose it's a big deal, just noteworthy.

Left few minor suggestions.

💬 rkrux commented on pull request "doc, test: more ephemeral dust follow-ups":
(https://github.com/bitcoin/bitcoin/pull/31371#discussion_r1858216903)
```
fee_per_output=dust_tx_fee
```

Since technically it's the fee per output, let's call it `dust_tx_fee_per_output`? Otherwise reading the function signatures gives an impression that `dust_tx_fee` is for the whole tx.

💬 rkrux commented on pull request "doc, test: more ephemeral dust follow-ups":
(https://github.com/bitcoin/bitcoin/pull/31371#discussion_r1858219028)
```diff
- def create_ephemeral_dust_package(self, *, tx_version, dust_tx_fee=0, dust_value=0, num_dust_outputs=1, extra_sponsors=None):
+ def create_ephemeral_dust_package(self, *, tx_version=3, dust_tx_fee=0, dust_value=0, num_dust_outputs=1, extra_sponsors=None):
```

Because v3 is the most common in the file.

💬 rkrux commented on pull request "doc, test: more ephemeral dust follow-ups":
(https://github.com/bitcoin/bitcoin/pull/31371#discussion_r1858220783)
```diff
- def create_ephemeral_dust_package(self, *, tx_version, dust_tx_fee=0, dust_value=0, num_dust_outputs=1, extra_sponsors=None):
+ def create_ephemeral_dust_package(self, *, tx_version, dust_tx_fee=0, dust_value=0, num_dust_outputs=1, extra_sponsor_coins=None):
```

Nit for verbosity in function signature.