💬 maflcko commented on pull request "doc: clarify Cirrus self-hosted workers setup":
(https://github.com/bitcoin/bitcoin/pull/30314#discussion_r1648838279)
> If we think pointing elsewhere (e.g. to a place that describes the needs, but is being updated for additional reasons beyond this file) is overkill, then we don't need to change this line.
The file is self-contained regarding this line and there is no need to point elsewhere. "podman4.1+" is equivalent to "vanilla Ubuntu 23.04+" (forever), so there is no need to update it in the future.
(https://github.com/bitcoin/bitcoin/pull/30314#discussion_r1648838279)
> If we think pointing elsewhere (e.g. to a place that describes the needs, but is being updated for additional reasons beyond this file) is overkill, then we don't need to change this line.
The file is self-contained regarding this line and there is no need to point elsewhere. "podman4.1+" is equivalent to "vanilla Ubuntu 23.04+" (forever), so there is no need to update it in the future.
✅ 0xB10C closed a pull request: "tracing: network connection tracepoints"
(https://github.com/bitcoin/bitcoin/pull/25832)
(https://github.com/bitcoin/bitcoin/pull/25832)
💬 maflcko commented on pull request "doc: clarify Cirrus self-hosted workers setup":
(https://github.com/bitcoin/bitcoin/pull/30314#discussion_r1648840963)
This file `.cirrus.yml` is self-contained and only concerns Cirrus CI. It has no meaning for the outside CI system. The labels are used in this config file only, they are only required here, and they are explained here.
(https://github.com/bitcoin/bitcoin/pull/30314#discussion_r1648840963)
This file `.cirrus.yml` is self-contained and only concerns Cirrus CI. It has no meaning for the outside CI system. The labels are used in this config file only, they are only required here, and they are explained here.
💬 vasild commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1648841695)
The comment says 13.2+ but the code says 1400000
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1648841695)
The comment says 13.2+ but the code says 1400000
📝 0xB10C reopened a pull request: "tracing: network connection tracepoints"
(https://github.com/bitcoin/bitcoin/pull/25832)
This adds five new tracepoints with documentation and tests for network connections:
- established connections with `net:inbound_connection` and `net:outbound_connection`
- closed connections (both closed by us or by the peer) with `net:closed_connnection`
- inbound connections that we choose to evict with `net:evicted_inbound_connection`
- connections that are misbehaving and punished with `net:misbehaving_connection`
I've been using these tracepoints for a few months now to monitor co
...
(https://github.com/bitcoin/bitcoin/pull/25832)
This adds five new tracepoints with documentation and tests for network connections:
- established connections with `net:inbound_connection` and `net:outbound_connection`
- closed connections (both closed by us or by the peer) with `net:closed_connnection`
- inbound connections that we choose to evict with `net:evicted_inbound_connection`
- connections that are misbehaving and punished with `net:misbehaving_connection`
I've been using these tracepoints for a few months now to monitor co
...
💬 0xB10C commented on pull request "tracing: network connection tracepoints":
(https://github.com/bitcoin/bitcoin/pull/25832#issuecomment-2182585453)
Must have closed this while force pushing. That was not my intention.
I've rebased this with https://github.com/bitcoin/bitcoin/pull/29575 merged. The `net:misbehaving_connection` changed from
```c++
TRACE5(net, misbehaving_connection,
peer.m_id,
score_before,
howmuch,
message.c_str(),
discourage_threshold_exceeded);
```
to
```c++
TRACE2(net, misbehaving_connection,
peer.m_id,
message.c_str());
```
I've updates the tests and the bpftrace demo script acco
...
(https://github.com/bitcoin/bitcoin/pull/25832#issuecomment-2182585453)
Must have closed this while force pushing. That was not my intention.
I've rebased this with https://github.com/bitcoin/bitcoin/pull/29575 merged. The `net:misbehaving_connection` changed from
```c++
TRACE5(net, misbehaving_connection,
peer.m_id,
score_before,
howmuch,
message.c_str(),
discourage_threshold_exceeded);
```
to
```c++
TRACE2(net, misbehaving_connection,
peer.m_id,
message.c_str());
```
I've updates the tests and the bpftrace demo script acco
...
💬 willcl-ark commented on pull request "contrib: Fixup verify-binaries OS platform parsing":
(https://github.com/bitcoin/bitcoin/pull/30147#discussion_r1648875929)
Well, I've enjoyed learning that python has a `difflib` builtin library. I thought I'd used all the string libraries by now, but somehow never come across that one.
If you did wwant to incorporate that change here, then I'd be happy to reACK this. I did a few tests and it did for example nicely match `27.1-x64-linux` to suggest `Did you mean: x86_64-linux-gnu.tar.gz`, along with a few other typos I tried, which does seem neat.
(https://github.com/bitcoin/bitcoin/pull/30147#discussion_r1648875929)
Well, I've enjoyed learning that python has a `difflib` builtin library. I thought I'd used all the string libraries by now, but somehow never come across that one.
If you did wwant to incorporate that change here, then I'd be happy to reACK this. I did a few tests and it did for example nicely match `27.1-x64-linux` to suggest `Did you mean: x86_64-linux-gnu.tar.gz`, along with a few other typos I tried, which does seem neat.
👍 tdb3 approved a pull request: "doc: clarify Cirrus self-hosted workers setup"
(https://github.com/bitcoin/bitcoin/pull/30314#pullrequestreview-2132514843)
ACK c67f215ea5fd57cd05e5346b8cd292dd879303ff
(https://github.com/bitcoin/bitcoin/pull/30314#pullrequestreview-2132514843)
ACK c67f215ea5fd57cd05e5346b8cd292dd879303ff
👍 hebasto approved a pull request: "contrib: add R(UN)PATH check to ELF symbol-check"
(https://github.com/bitcoin/bitcoin/pull/30312#pullrequestreview-2132535919)
ACK 4289dd02cce688a69c596f7cd5e47f831b00aa1b.
My Guix build:
```
x86_64
14f1b54936f71aaf8fedb987e1c2f5642c34ac35f4856cdbd7bf7b4a9f42507c guix-build-4289dd02cce6/output/aarch64-linux-gnu/SHA256SUMS.part
b120503ac4a37c160aa1bdc662348a2a662cb9b3d0477daa177e92afacab6a27 guix-build-4289dd02cce6/output/aarch64-linux-gnu/bitcoin-4289dd02cce6-aarch64-linux-gnu-debug.tar.gz
6416e3678aa13301ba2327e65dcd83afefd15d21c96c1b574cf616a65466a260 guix-build-4289dd02cce6/output/aarch64-linux-gnu/bitcoin
...
(https://github.com/bitcoin/bitcoin/pull/30312#pullrequestreview-2132535919)
ACK 4289dd02cce688a69c596f7cd5e47f831b00aa1b.
My Guix build:
```
x86_64
14f1b54936f71aaf8fedb987e1c2f5642c34ac35f4856cdbd7bf7b4a9f42507c guix-build-4289dd02cce6/output/aarch64-linux-gnu/SHA256SUMS.part
b120503ac4a37c160aa1bdc662348a2a662cb9b3d0477daa177e92afacab6a27 guix-build-4289dd02cce6/output/aarch64-linux-gnu/bitcoin-4289dd02cce6-aarch64-linux-gnu-debug.tar.gz
6416e3678aa13301ba2327e65dcd83afefd15d21c96c1b574cf616a65466a260 guix-build-4289dd02cce6/output/aarch64-linux-gnu/bitcoin
...
💬 m3dwards commented on pull request "ci: add option for running tests without volume":
(https://github.com/bitcoin/bitcoin/pull/30310#issuecomment-2182667210)
Re-run results on GHA:
```shell
ccache version 4.9.1
Cacheable calls: 770 / 782 (98.47%)
Hits: 62 / 770 ( 8.05%)
Direct: 9 / 62 (14.52%)
Preprocessed: 53 / 62 (85.48%)
Misses: 708 / 770 (91.95%)
Uncacheable calls: 12 / 782 ( 1.53%)
Local storage:
Cache size (GB): 0.1 / 0.1 (102.4%)
Cleanups: 616
Hits: 62 / 770 ( 8.05%)
Misses: 708 / 770 (91.95%)
```
I will increase the size and see if it helps
...
(https://github.com/bitcoin/bitcoin/pull/30310#issuecomment-2182667210)
Re-run results on GHA:
```shell
ccache version 4.9.1
Cacheable calls: 770 / 782 (98.47%)
Hits: 62 / 770 ( 8.05%)
Direct: 9 / 62 (14.52%)
Preprocessed: 53 / 62 (85.48%)
Misses: 708 / 770 (91.95%)
Uncacheable calls: 12 / 782 ( 1.53%)
Local storage:
Cache size (GB): 0.1 / 0.1 (102.4%)
Cleanups: 616
Hits: 62 / 770 ( 8.05%)
Misses: 708 / 770 (91.95%)
```
I will increase the size and see if it helps
...
💬 hebasto commented on pull request "ci: add option for running tests without volume":
(https://github.com/bitcoin/bitcoin/pull/30310#issuecomment-2182672557)
> I will increase the size and see if it helps
Yes. Especially, considering too many cleanups.
(https://github.com/bitcoin/bitcoin/pull/30310#issuecomment-2182672557)
> I will increase the size and see if it helps
Yes. Especially, considering too many cleanups.
💬 brunoerg commented on pull request "contrib: asmap-tool - Compare ASMaps with respect to specific addresses":
(https://github.com/bitcoin/bitcoin/pull/30246#issuecomment-2182683989)
I haven't looked at the code yet but I just tested it with two asmap files a few months apart and with 25k addresses, looks fine:
```
78 address(es) reassigned from unassigned to AS51167
47 address(es) reassigned from AS198949 to AS15557
38 address(es) reassigned from AS45758 to AS45629
18 address(es) reassigned from AS12715 to AS12479
15 address(es) reassigned from AS57269 to AS8708
12 address(es) reassigned from AS174 to AS212238
9 address(es) reassigned from AS41378 to AS60024
7 addr
...
(https://github.com/bitcoin/bitcoin/pull/30246#issuecomment-2182683989)
I haven't looked at the code yet but I just tested it with two asmap files a few months apart and with 25k addresses, looks fine:
```
78 address(es) reassigned from unassigned to AS51167
47 address(es) reassigned from AS198949 to AS15557
38 address(es) reassigned from AS45758 to AS45629
18 address(es) reassigned from AS12715 to AS12479
15 address(es) reassigned from AS57269 to AS8708
12 address(es) reassigned from AS174 to AS212238
9 address(es) reassigned from AS41378 to AS60024
7 addr
...
💬 hebasto commented on pull request "ci: add option for running tests without volume":
(https://github.com/bitcoin/bitcoin/pull/30310#issuecomment-2182698553)
> I will increase the size and see if it helps
From my tests it follows that the `ccache` cache size is about 227 MB. So setting `CCACHE_MAXSIZE=250MB` or `CCACHE_MAXSIZE=300MB` should work.
(https://github.com/bitcoin/bitcoin/pull/30310#issuecomment-2182698553)
> I will increase the size and see if it helps
From my tests it follows that the `ccache` cache size is about 227 MB. So setting `CCACHE_MAXSIZE=250MB` or `CCACHE_MAXSIZE=300MB` should work.
🚀 fanquake merged a pull request: "doc: clarify Cirrus self-hosted workers setup"
(https://github.com/bitcoin/bitcoin/pull/30314)
(https://github.com/bitcoin/bitcoin/pull/30314)
💬 Sjors commented on pull request "depends: bump miniupnpc to 2.2.8":
(https://github.com/bitcoin/bitcoin/pull/30301#issuecomment-2182707177)
tACK 545bb6c96080
I looked at the diff between the packaged 2.2.7 and 2.2.8. There's a `MINIUPNPC_API_VERSION` bump from 17 to 18, which was addressed in #30283. I also see our dropped patches reflected in the diff. Didn't study the rest of the changes.
Tested on macOS 14.5 with `-upnp=1 -natpmp=0`.
When I turn UPnP support off in the router:
```
[mapport] No valid UPnP IGDs found
```
When I turn it on:
```
2024-06-21T12:55:33Z [mapport] UPnP: ExternalIPAddress = x.x.x.x
20
...
(https://github.com/bitcoin/bitcoin/pull/30301#issuecomment-2182707177)
tACK 545bb6c96080
I looked at the diff between the packaged 2.2.7 and 2.2.8. There's a `MINIUPNPC_API_VERSION` bump from 17 to 18, which was addressed in #30283. I also see our dropped patches reflected in the diff. Didn't study the rest of the changes.
Tested on macOS 14.5 with `-upnp=1 -natpmp=0`.
When I turn UPnP support off in the router:
```
[mapport] No valid UPnP IGDs found
```
When I turn it on:
```
2024-06-21T12:55:33Z [mapport] UPnP: ExternalIPAddress = x.x.x.x
20
...
👋 Sjors's pull request is ready for review: "Support self-hosted Cirrus workers on forks"
(https://github.com/bitcoin/bitcoin/pull/29274)
(https://github.com/bitcoin/bitcoin/pull/29274)
💬 hebasto commented on pull request "ci: Add FreeBSD GitHub Actions job":
(https://github.com/bitcoin/bitcoin/pull/30164#issuecomment-2182716043)
Rebased to resolve a conflict with https://github.com/bitcoin/bitcoin/pull/30193.
(https://github.com/bitcoin/bitcoin/pull/30164#issuecomment-2182716043)
Rebased to resolve a conflict with https://github.com/bitcoin/bitcoin/pull/30193.
👋 hebasto's pull request is ready for review: "ci: Add FreeBSD GitHub Actions job"
(https://github.com/bitcoin/bitcoin/pull/30164)
(https://github.com/bitcoin/bitcoin/pull/30164)
💬 hebasto commented on pull request "ci: Add FreeBSD GitHub Actions job":
(https://github.com/bitcoin/bitcoin/pull/30164#issuecomment-2182724573)
> I am running nighly freeBSD CI runs for years, and I haven't yet seen an issue pop up, so the benefit seems unclear
Not encountering an issue in the past does not guarantee the same in the future :)
> Maybe, if CI resources are really scarce
FWIW, the new job does not use the GHA cache storage, which is limited.
(https://github.com/bitcoin/bitcoin/pull/30164#issuecomment-2182724573)
> I am running nighly freeBSD CI runs for years, and I haven't yet seen an issue pop up, so the benefit seems unclear
Not encountering an issue in the past does not guarantee the same in the future :)
> Maybe, if CI resources are really scarce
FWIW, the new job does not use the GHA cache storage, which is limited.
💬 Sjors commented on pull request "ci: Add FreeBSD GitHub Actions job":
(https://github.com/bitcoin/bitcoin/pull/30164#issuecomment-2182758582)
> I am running nighly freeBSD CI runs for years, and I haven't yet seen an issue pop up, so the benefit seems unclear, albeit having the CI config could certainly be useful for manual testing sometimes.
We ran into a FreeBSD compilation error here: https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1610209650
Might be limited to low level networking code and other places where FreeBSD is materially different from macOS and Linux. It seems good to at least do a build.
(https://github.com/bitcoin/bitcoin/pull/30164#issuecomment-2182758582)
> I am running nighly freeBSD CI runs for years, and I haven't yet seen an issue pop up, so the benefit seems unclear, albeit having the CI config could certainly be useful for manual testing sometimes.
We ran into a FreeBSD compilation error here: https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1610209650
Might be limited to low level networking code and other places where FreeBSD is materially different from macOS and Linux. It seems good to at least do a build.
👍 vasild approved a pull request: "ci: Add FreeBSD GitHub Actions job"
(https://github.com/bitcoin/bitcoin/pull/30164#pullrequestreview-2132670813)
ACK f9f20ed001ee021a98b72e70ea18fe28f32ac6a5
(https://github.com/bitcoin/bitcoin/pull/30164#pullrequestreview-2132670813)
ACK f9f20ed001ee021a98b72e70ea18fe28f32ac6a5