💬 luke-jr commented on pull request "Remove arbitrary restrictions on OP_RETURN by default":
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1665602678)
> the idea is not to store a video on bitcoin but indeed a reference to it like bittorrent

You can already do that within the current datacarriersize. Removing the limit has no purpose other than gratuitious abuse.

>As different people explained this change is really needed and really necessary for the future of bitcoin

This is completely false and nonsensical. It does literally nothing good for Bitcoin, only harms Bitcoin.

> Regarding the 80 B number I don t understand where it com
...

💬 furszy commented on pull request "fuzz: improve `coinselection`":
(https://github.com/bitcoin/bitcoin/pull/27585#discussion_r1284431053)
I think that it worth. One of the fuzzing goals is to assert expected behavior externally. Black-box style. So if the internal implementation changes by mistake (or if there is a way to by-pass the expected behavior that we aren't aware of), the fuzzer catches it.

Also, the internal BnB `ComputeSetAndWaste` call checks that the final selection is within a certain range: `target <= value < target + change cost`. It is indirectly checking `GetChange()` result but I don't think that we should r
...

💬 brunoerg commented on pull request "fuzz: improve `coinselection`":
(https://github.com/bitcoin/bitcoin/pull/27585#discussion_r1284434687)
Fair enough, gonna address it.

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