💬 maflcko commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#discussion_r1858085005)
The problem is that no one will notice if this isn't run on any machine, because it will silently pass even if there is an error.

💬 vasild commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#discussion_r1858094964)
True, if it stops working on all VMs, then nobody will notice. Any ideas how to approach this?

💬 maflcko commented on pull request "ci: Skip broken Wine64 tests by default":
(https://github.com/bitcoin/bitcoin/pull/31284#issuecomment-2500071417)
... https://cirrus-ci.com/task/5957633356595200

💬 maflcko commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#discussion_r1858114741)
I'd say it is fine to ignore it by default (if you want). However, there should be one machine in the CI matrix to run the check (and fail on any error).

The cirrus workers are running in a user account, so they may not have the permissions (unless they are switched to @0xB10C's workers, which are running as root?). Alternatively, you could try with `--cap-add=...`/`--privileged`, but I haven't tried this. I guess the only task that has the required permissions right now is the ASan GHA task?
...

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