📝 fanquake locked a pull request: "Update README.md"
(https://github.com/bitcoin/bitcoin/pull/31369)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

📝 fanquake locked a pull request: "Update developer-notes.md"
(https://github.com/bitcoin/bitcoin/pull/31370)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

💬 mzumsande commented on issue "0.28 binds to default ports when -port or -rpcport set":
(https://github.com/bitcoin/bitcoin/issues/31372#issuecomment-2499089563)
Thanks for the report - this is known, see #31133 . An immediate fix is to use `-bind` instead of `-port`. It is still being discussed whether and how this should be fixed, see the linked issue and https://github.com/bitcoin/bitcoin/pull/31223.

💬 stickies-v commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r1852512769)
nit: `from/by` naming inconsistency, I think my preference would lie with `from` (i.e. update to `kernel_get_block_index_from_hash` and `kernel_get_block_index_from_height`)

(_technically_, could update `kernel_get_next_block_index` -> `kernel_get_block_index_from_previous` and `kernel_get_previous_block_index` -> `kernel_get_block_index_from_next`, but... `from_next` sounds weird?)

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