💬 aureleoules commented on pull request "Silent Payments: Implement BIP352":
(https://github.com/bitcoin/bitcoin/pull/28122#discussion_r1276888579)
65b2dec1e39ea4efb327457563d2b18d49b4bc4c: Could use an initializer list

💬 aureleoules commented on pull request "Silent Payments: Implement BIP352":
(https://github.com/bitcoin/bitcoin/pull/28122#discussion_r1276890488)
65b2dec1e39ea4efb327457563d2b18d49b4bc4c: Could use structured binding
```suggestion
for (const auto& [pubkey, amount]: m_recipient.m_outputs) {
```

💬 aureleoules commented on pull request "Silent Payments: Implement BIP352":
(https://github.com/bitcoin/bitcoin/pull/28122#discussion_r1276885882)
65b2dec1e39ea4efb327457563d2b18d49b4bc4c: Use an initializer list for data members

💬 aureleoules commented on pull request "Silent Payments: Implement BIP352":
(https://github.com/bitcoin/bitcoin/pull/28122#discussion_r1276891288)
65b2dec1e39ea4efb327457563d2b18d49b4bc4c: Could use structured binding

🤔 darosior reviewed a pull request: "fuzz: Generate with random libFuzzer settings"
(https://github.com/bitcoin/bitcoin/pull/28178#pullrequestreview-1551671337)
Concept ACK.

> Also, randomize -mutate_depth, for fun.

Could you expand?

💬 darosior commented on pull request "fuzz: Generate with random libFuzzer settings":
(https://github.com/bitcoin/bitcoin/pull/28178#discussion_r1277296384)
If we are going to set `max_len`, should we also set `len_control`?

💬 MarcoFalke commented on pull request "fuzz: Generate with random libFuzzer settings":
(https://github.com/bitcoin/bitcoin/pull/28178#discussion_r1277303930)
The default of `-len_control=100` seems fine and applicable to all values of `-max_len`, no?

💬 MarcoFalke commented on pull request "fuzz: Generate with random libFuzzer settings":
(https://github.com/bitcoin/bitcoin/pull/28178#issuecomment-1655335044)
> Concept ACK.
>
> > Also, randomize -mutate_depth, for fun.
>
> Could you expand?

Not sure. I am happy to drop this, but it was part of https://www.github.com/bitcoin/bitcoin/pull/20752/commits/1ff0dc525f051bbc7a93312dd622340ca8f4f52c, which is why I picked it up.

💬 darosior commented on pull request "fuzz: Generate with random libFuzzer settings":
(https://github.com/bitcoin/bitcoin/pull/28178#issuecomment-1655353550)
> Not sure. I am happy to drop this

No, i was just curious about the rationale. I guess i've a mild preference for sticking to defaults if there isn't any, but i don't think it hurts either.
------- Original Message -------
On Friday, July 28th, 2023 at 11:07 AM, MacrabFalke ***@***.***> wrote:

>> Concept ACK.
>>
>>> Also, randomize -mutate_depth, for fun.
>>
>> Could you expand?
>
> Not sure. I am happy to drop this, but it was part of https://www.github.com/bitcoin/bitcoin/pull/20752/commits
...

💬 darosior commented on pull request "fuzz: Generate with random libFuzzer settings":
(https://github.com/bitcoin/bitcoin/pull/28178#discussion_r1277316390)
Yes, i guess 100 minutes is long enough for the fuzzer to eventually get to try larger inputs.

💬 MarcoFalke commented on pull request "fuzz: Test headers pre-sync through p2p interface":
(https://github.com/bitcoin/bitcoin/pull/28043#discussion_r1273466446)
How is this different from CallOneOf?

💬 MarcoFalke commented on pull request "fuzz: use `ConnmanTestMsg` in `connman`":
(https://github.com/bitcoin/bitcoin/pull/28091#issuecomment-1655402712)
review ACK ecfe507e07e9bdab210e04ebd3fc3b8ae9d6a094

Didn't test anything nor compile

👍 TheCharlatan approved a pull request: "test: Add SyncWithValidationInterfaceQueue to mockscheduler RPC"
(https://github.com/bitcoin/bitcoin/pull/28118#pullrequestreview-1551760387)
ACK fabef121b0cdfac6ec1985f6c08c5685a886ba5a

Seems like the correct thing to do here anyway. Also reproduced the `feature_fee_estimation` behavior change.

👍 TheCharlatan approved a pull request: "refactor: Remove unused raw-pointer read helper from univalue"
(https://github.com/bitcoin/bitcoin/pull/28168#pullrequestreview-1551784192)
tACK fa940f41eaffa4b2a28c465a10a4c12d4b8976b8

💬 MarcoFalke commented on pull request "Fuzz: a more efficient descriptor parsing target":
(https://github.com/bitcoin/bitcoin/pull/27888#discussion_r1277370453)


Funny how `-only_ascii=1` performs worse than `-only_ascii=0`.

💬 MarcoFalke commented on pull request "Fuzz: a more efficient descriptor parsing target":
(https://github.com/bitcoin/bitcoin/pull/27888#discussion_r1277371055)
(or at least, not significantly better)

💬 darosior commented on pull request "Fuzz: a more efficient descriptor parsing target":
(https://github.com/bitcoin/bitcoin/pull/27888#discussion_r1277371969)
Maybe the target returning early on non-ASCII has basically the same effect?

💬 MarcoFalke commented on pull request "Fuzz: a more efficient descriptor parsing target":
(https://github.com/bitcoin/bitcoin/pull/27888#issuecomment-1655477388)
`-mutate_depth=3` seems to be the best so far:


💬 hebasto commented on pull request "ci: Run Windows native task on GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28173#discussion_r1277398254)
Done.

💬 hebasto commented on pull request "ci: Run Windows native task on GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28173#issuecomment-1655483540)
> Can you remove the Windows Cirrus task here, as it will need to be removed either way?

Done.

> Maybe also print the `GITHUB_TOKEN` permissions at every start of the task, just to double check they are read-only?

They are printed by default. To see them, in https://github.com/hebasto/bitcoin/actions/runs/5685393993/job/15410187833, one needs to expand "Set up job", then expand "GIHUB_TOKEN Permissions".