💬 hebasto commented on pull request "ci: Run "macOS native x86_64" job on GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28187#issuecomment-1665619844)
Updated e9b72185205812df51082b42fb8c25cbf53cbc03 -> cc26f0d8c8539094db8d95da9fa46ea1247a0d8c ([pr28187.03](https://github.com/hebasto/bitcoin/commits/pr28187.03) -> [pr28187.04](https://github.com/hebasto/bitcoin/commits/pr28187.04)):

- rebased
- squashed
- addressed @MarcoFalke's comments:
- https://github.com/bitcoin/bitcoin/pull/28187#discussion_r1279021074
- https://github.com/bitcoin/bitcoin/pull/28187#discussion_r1278964021
- https://github.com/bitcoin/bitcoin/pull/28187#disc
...

💬 hebasto commented on pull request "ci: Run "macOS native x86_64" job on GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28187#discussion_r1284436416)
[Updated](https://github.com/bitcoin/bitcoin/pull/28187#issuecomment-1665619844).

💬 hebasto commented on pull request "ci: Run "macOS native x86_64" job on GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28187#discussion_r1284436805)
Thanks! [Updated](https://github.com/bitcoin/bitcoin/pull/28187#issuecomment-1665619844).

💬 hebasto commented on pull request "ci: Run "macOS native x86_64" job on GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28187#discussion_r1284437226)
[Reworked](https://github.com/bitcoin/bitcoin/pull/28187#issuecomment-1665619844).

💬 hebasto commented on pull request "ci: Run "macOS native x86_64" job on GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28187#discussion_r1284438029)
[Updated](https://github.com/bitcoin/bitcoin/pull/28187#issuecomment-1665619844).

💬 darosior commented on pull request "fuzz: a target for the block index database":
(https://github.com/bitcoin/bitcoin/pull/28209#discussion_r1284450094)
Alright, in order to benchmark which approach was most efficient i ran both versions with `-runs=100000` on an empty folder 3 times and compared the average coverage and runtime.

- `ConsumeIntegral`-based: runtime of 75 seconds for a coverage of `1565` for all 3 runs.
- `ConsumeDeserializable`-based: average runtime of 12 seconds for a coverage of `1560`.

Given the burst in coverage at the start of the run, the clear difference in runtime and the small difference in coverage i figured i'd
...

💬 hebasto commented on pull request "ci: Run "macOS native x86_64" job on GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28187#issuecomment-1665646336)
From [IRC](https://bitcoin-irc.chaincode.com/bitcoin-core-dev/2023-08-03#952553):
> 14:22 \<fanquake\> Yea the PR description in that PR needs updating to actually explain what’s its doing (dropping support for a release platform from CI), rather than making it look like just swapping infra

The PR description has been updated.

👍 stickies-v approved a pull request: "kernel: Prune leveldb headers"
(https://github.com/bitcoin/bitcoin/pull/28186#pullrequestreview-1562868143)
ACK 3fb2dac2ce78092c1006ee082c536bea1b69a152

I would however strongly prefer to [simplify the lambda logic](https://github.com/bitcoin/bitcoin/pull/28186#discussion_r1283260447), and probably [this duplication should also be removed](https://github.com/bitcoin/bitcoin/pull/28186#discussion_r1283281451).

💬 stickies-v commented on pull request "kernel: Prune leveldb headers":
(https://github.com/bitcoin/bitcoin/pull/28186#discussion_r1284419848)
nit: missing docstring

💬 brunoerg commented on pull request "fuzz: a target for the block index database":
(https://github.com/bitcoin/bitcoin/pull/28209#discussion_r1284458654)
Yea, `ConsumeDeserializable` is faster because it will 'return' every time it gets an invalid deserialization. The coverage is similar but I believe that it's expected because the difference between both approaches will only reflect on the size of blocks and files.

💬 fiatjaf commented on pull request "Remove arbitrary restrictions on OP_RETURN by default":
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1665654704)
Concept ACK.

💬 brunoerg commented on pull request "fuzz: a target for the block index database":
(https://github.com/bitcoin/bitcoin/pull/28209#discussion_r1284461013)
You could also test both approaches by putting an assert right after the loop, something like: `assert(files.size() > 50)`. I believe that the `ConsumeDeserializable`-based will take much more time to reach it.

👍 MarcoFalke approved a pull request: "ci: Run "macOS native x86_64" job on GitHub Actions"
(https://github.com/bitcoin/bitcoin/pull/28187#pullrequestreview-1562926246)
lgtm ACK cc26f0d8c8539094db8d95da9fa46ea1247a0d8c 🖐

<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: lgtm ACK cc26f0d8c8539094
...

💬 MarcoFalke commented on pull request "ci: Run "macOS native x86_64" job on GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28187#discussion_r1284457296)
Any reason to reorder, remove libevent and libtool?

💬 MarcoFalke commented on pull request "ci: Run "macOS native x86_64" job on GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28187#discussion_r1284458404)
nit: forgot to set the timeout? What is the default?

💬 real-or-random commented on issue "ci: Future of macOS and Windows MSVC CI tasks":
(https://github.com/bitcoin/bitcoin/issues/28098#issuecomment-1665661274)
If we now have self-hosted runners for Cirrus, would it make sense (perhaps in the long-run) to make them also [self-hosted runners for GitHub Actions](https://docs.github.com/en/actions/hosting-your-own-runners/managing-self-hosted-runners/about-self-hosted-runners)? This will remove Cirrus from the loop entirely, which, I think, is a good thing. Cirrus anyway acts only as a coordination layer between GitHub and the runner.

🤔 furszy reviewed a pull request: "test: Extend stale tip test"
(https://github.com/bitcoin/bitcoin/pull/18470#pullrequestreview-1562940257)
This test extension could play well with #28170. Are you planning to rebase it?

💬 MarcoFalke commented on pull request "fuzz: a target for the block index database":
(https://github.com/bitcoin/bitcoin/pull/28209#discussion_r1284468527)
> My interpretation of these results is

I don't think you can use coverage as a metric when comparing two different code bases. The version that has higher coverage is also the version that has more code in the fuzz target, which is also counted toward "coverage".

💬 iBeizsley commented on pull request "Remove arbitrary restrictions on OP_RETURN by default":
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1665678418)
> I'm not going to war for this change but this strikes me as correct. I classified the OP_RETURN limit as the last remaining "paternalism" restriction in the codebase, since you can't even over-write how many datacarrier outputs are allowed via config.

If we are concerned about 'paternalisms', rather than changing default behaviour, which is itself making a decision for node runners, just allowing the overriding of the default by config is a better solution.

💬 darosior commented on pull request "fuzz: a target for the block index database":
(https://github.com/bitcoin/bitcoin/pull/28209#discussion_r1284480443)
Trying to maximum the coverage in 100_000 runs for each target i managed to:
- achieve `2099` of coverage with the `ConsumeDeserializable`-based target using `-max_len=10000 -len_control=1 -mutate_depth=3` (in 305 seconds).
- achieve `2088` of coverage with the `ConsumeIntegral`-based target using -max_len=8000 -len_control=0 -use_value_profile=1 -mutate_depth=1` (in 252 seconds).

------

> I don't think you can use coverage as a metric

Oh, right.. I had overlooked this. However the di
...