💬 eriknylund commented on pull request "test: verify spend from 999-of-999 taproot multisig wallet":
(https://github.com/bitcoin/bitcoin/pull/28212#discussion_r1284383877)
I think it's very related and also my concern, is this test as good as one with K different keys. I'm hoping @Sjors can provide some insight here. My understanding of the other tests in this file is limited because they are not documented, but they seem to be doing something similar to this. On the other hand I could of course do what the previous test did, to use the "H_POINT" approach and just throw one XPRV in there somewhere. If that makes the test more real-world like or improves the test i
...

🚀 fanquake merged a pull request: "scripted-diff: Specify Python major version explicitly on Windows"
(https://github.com/bitcoin/bitcoin/pull/28213)

💬 Sun0fABeach commented on pull request "Remove arbitrary restrictions on OP_RETURN by default":
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1665591754)
> Rational: there's lots of ways for people to publish data in the Bitcoin chain, and lots of data has been published. There's no reason for us to place artificial limits on this particular method.

There is plenty of reasons to place limits on **any** method that abuses the timechain as data storage. Any relaxation of the default settings validates existing spam and is unacceptable. Give node runners strong defaults and options to mitigate current spam strategies before doing any of these twe
...

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