💬 stickies-v commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r1850657572)
nit: according to the `worker_threads_num`, 0 is accepted too:
> Zero means no parallel verification.

```suggestion
* used for validation. The number must not be negative. When set to zero, no parallel verification is done.
```

💬 stickies-v commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r1857036990)
I think it's confusing that this function can return `False` and have `status == kernel_SCRIPT_VERIFY_OK`. How about adding a `kernel_SCRIPT_VERIFY_ERROR` catch-all member for unspecified errors? Or alternatively, requiring the user to provide a nullptr and only setting it to `kernel_SCRIPT_VERIFY_OK` is that's actually so?

💬 stickies-v commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r1852510169)
nit: for the other `block_index` getters, `chainstate_manager` is the second argument - would keep that consistent

✅ dpc closed an issue: "0.28 binds to default ports when -port or -rpcport set"
(https://github.com/bitcoin/bitcoin/issues/31372)

💬 dpc commented on issue "0.28 binds to default ports when -port or -rpcport set":
(https://github.com/bitcoin/bitcoin/issues/31372#issuecomment-2499093566)
Thanks. Closing, since a dup.

💬 TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r1857397944)
We discussed during the last workshop that ideally we don't have any status codes here at all. But the problem is annoying to tackle. You'd probably want to pass this function a script verify object that has already passed through the required pre-checks. But then you have to either copy the objects into this object, or give ownership up to that object, which I don't think is desirable. The alternative to that is having a function with the same signature that you can call to check the arguments.
...

💬 stickies-v commented on pull request "refactor: Clamp worker threads in ChainstateManager constructor":
(https://github.com/bitcoin/bitcoin/pull/31313#discussion_r1857423512)
Ah, I hadn't considered that. I suppose there's also no real harm in default values being different when accessed from different contexts.
Can be marked as resolved.

💬 TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-2499171169)
Thanks for the review @stickies-v!

Updated 34a8429ff3a870c0caaf4c4790becd86c5acde38 -> 35f8503285c672e8ee7e98617e236b38d8ce7a7f ([kernelApi_5](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_5) -> [kernelApi_6](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_6), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelApi_5..kernelApi_6))

* Addressed @stickies-v's [comment](https://github.com/bitcoin/bitcoin/pull/30595#discussion_r1850657572), fixed worker threads docs
...

💬 theStack commented on pull request "ci: limit max stack size to 512 KiB":
(https://github.com/bitcoin/bitcoin/pull/31367#issuecomment-2499192517)
Concept ACK

💬 0xB10C commented on issue "Avoid internet traffic from tests":
(https://github.com/bitcoin/bitcoin/issues/31339#issuecomment-2499241767)
Did you consider running the docker container in CI with `--network none`?

https://docs.docker.com/engine/network/drivers/none/

💬 0xB10C commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-2499335672)
Ran this on my CI runner which has 8.8.8.8 configured as DNS server for docker.

https://cirrus-ci.com/task/5500763260059648?logs=ci#L1137

```
[00:46:26.215] + tcpdump -n -r /tmp/tcpdump_eth0 tcp or udp
[00:46:26.219] 00:42:50.052764 IP 172.18.0.2.46566 > 8.8.8.8.53: 39301+ A? x9.dummySeed.invalid. (38)
[00:46:26.219] 00:42:50.053181 IP 172.18.0.2.58686 > 8.8.8.8.53: 36487+ AAAA? x9.dummySeed.invalid. (38)
[00:46:26.219] 00:42:50.059038 IP 8.8.8.8.53 > 172.18.0.2.46566: 39301 NXDomain 0
...

💬 plebhash commented on issue "Stratum v2 via IPC Mining Interface tracking issue":
(https://github.com/bitcoin/bitcoin/issues/31098#issuecomment-2499825788)
on the pre-IPC Sv2 Template Provider that @Sjors provides [on his `sv2` branch](https://github.com/Sjors/bitcoin), we have two CLI parameters for `bitcoind`:

- `-sv2interval`
- `-sv2feedelta`

these two parameters dictated how often TP would send templates to its clients (Pool or JDC), which could happen either on a regular interval (`sv2interval`), or ad-hoc when needed (`sv2feedelta`).

it seems that the new IPC approach makes communication between `bitcoin-stratum` and `bitcoin-node`
...

💬 Sjors commented on issue "Stratum v2 via IPC Mining Interface tracking issue":
(https://github.com/bitcoin/bitcoin/issues/31098#issuecomment-2499968810)
@plebhash I pushed an update to the `bitcoin-mine` (Template Provider) application yesterday in https://github.com/Sjors/bitcoin/pull/48. It has the same `-sv2interval` and `-sv2feedelta` arguments. This uses `waitNext()` from #31283 under the hood. There's no polling involved!

💬 vasild commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#discussion_r1858069999)
On some of the VMs this produces:

```
[09:47:01.270] + tcpdump -n -i eth0 -w /tmp/tcpdump_eth0
[09:47:01.334] tcpdump: eth0: You don't have permission to perform this capture on that device
[09:47:01.335] (socket: Operation not permitted)
```

and then the CI passes because the return code is ignored. I think better not fail the CI when `tcpdump` does not work in that environment. It is ok as long as `tcpdump` works on at least one VM to catch problems.

💬 maflcko commented on issue "Avoid internet traffic from tests":
(https://github.com/bitcoin/bitcoin/issues/31339#issuecomment-2500040855)
> Edit 2: `--internal` works

Good point, but I don't think this works conceptually to turn any and all network access in the tests into a failure, because a failure to access the network in the tests may not always lead to a test failure.

💬 vasild commented on issue "Avoid internet traffic from tests":
(https://github.com/bitcoin/bitcoin/issues/31339#issuecomment-2500045928)
@0xB10C my wish is to catch and report these, not just block them silently (only) from the CI environments.

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