📝 ANIKKARAINE opened a pull request: "Test "
(https://github.com/bitcoin-core/gui/pull/782)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

💬 hebasto commented on pull request "ci: Set `HOMEBREW_NO_INSTALLED_DEPENDENTS_CHECK` to avoid unrelated failures":
(https://github.com/bitcoin/bitcoin/pull/29080#issuecomment-1855896100)
> Do we use old images?

The GHA chooses an image from a set of available images when setting up a new job.

It is not rare case when more than one images are available simultaneously. One of them is older than others.

✅ glozow closed a pull request: "Test "
(https://github.com/bitcoin-core/gui/pull/782)

💬 fanquake commented on pull request "ci: Set `HOMEBREW_NO_INSTALLED_DEPENDENTS_CHECK` to avoid unrelated failures":
(https://github.com/bitcoin/bitcoin/pull/29080#issuecomment-1855901938)
What does "older" mean here? Aren't we pinned to a specific version to avoid exactly this issue?

💬 hebasto commented on pull request "ci: Set `HOMEBREW_NO_INSTALLED_DEPENDENTS_CHECK` to avoid unrelated failures":
(https://github.com/bitcoin/bitcoin/pull/29080#issuecomment-1855914085)
> > It is not rare case when more than one images are available simultaneously. One of them is older than others.
>
> What does "older" mean here? Aren't we pinned to a specific version to avoid exactly this issue?

For example,
```
##[group]Operating System
macOS
13.6
22G120
##[endgroup]
##[group]Runner Image
Image: macos-13
Version: 20231025.2
Included Software: https://github.com/actions/runner-images/blob/macos-13/20231025.2/images/macos/macos-13-Readme.md
Image Release: http
...

💬 murchandamus commented on pull request "fuzz: coinselection, improve `min_viable_change`/`change_output_size`":
(https://github.com/bitcoin/bitcoin/pull/28372#issuecomment-1855916192)
Most of them, but not all crashed on this:

```
#626233 REDUCE cov: 1289 ft: 13823 corp: 4183/16Mb lim: 66076 exec/s: 206 rss: 88Mb L: 157/65635 MS: 1 EraseBytes-
fuzz: ../../src/wallet/test/fuzz/coinselection.cpp:131: void wallet::coinselection_fuzz_target(FuzzBufferType): Assertion `result_bnb->GetChange(coin_params.m_cost_of_change, CAmount{0}) == 0' failed.
```

I think we might have misdiagnosed the issue, when we thought it could only happen when SFFO is active

🤔 furszy reviewed a pull request: "fuzz: coinselection, improve `min_viable_change`/`change_output_size`"
(https://github.com/bitcoin/bitcoin/pull/28372#pullrequestreview-1781900268)
> I think we might have misdiagnosed the issue, when we thought it could only happen when SFFO is active

@murchandamus.The code here has not fixed the issue I mentioned https://github.com/bitcoin/bitcoin/issues/28918#issuecomment-1853853445.
Need to provide `min_viable_change` to the BnB function, not `cost_of_change`. @brunoerg

💬 brunoerg commented on pull request "fuzz: coinselection, improve `min_viable_change`/`change_output_size`":
(https://github.com/bitcoin/bitcoin/pull/28372#issuecomment-1855925603)
> Need to provide min_viable_change to the BnB function, not cost_of_change. @brunoerg

Yes, I'm addressing it.

💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#issuecomment-1855935981)
I responded here: https://delvingbitcoin.org/t/cluster-mempool-rbf-thoughts/156/24?u=instagibbs

issue 1: awaiting further details on potential diagram check integration vs suggested heuristic I gave
issue 2: known issue, wallet authors shouldn't do that until cluster mempool fixes that\

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1426780572)
Just saw your edit

> I'd just set check_fees to false if Prio had just been called before calling this, to stay simple.

I'm not sure if this helps. When debugging some previous fuzzer crashes, it would often be that a prioritisation happened in a previous iteration, and then nothing was submitted in this iteration.

fwiw my local fuzzer seems happy right now, with only calling `CheckMempoolV3Invariants` when accepted.

👋 maflcko's pull request is ready for review: "build: Bump guix time-machine to unlock riscv64 metal"
(https://github.com/bitcoin/bitcoin/pull/29078)

💬 furszy commented on pull request "fuzz: coinselection, improve `min_viable_change`/`change_output_size`":
(https://github.com/bitcoin/bitcoin/pull/28372#issuecomment-1855942122)
> Yes, I'm addressing it.

Before pushing the changes, can verify them against the issue https://github.com/bitcoin/bitcoin/issues/28918#issuecomment-1853532671.

👍 dergoegge approved a pull request: "fuzz: Limit p2p fuzz targets to MAX_PROTOCOL_MESSAGE_LENGTH"
(https://github.com/bitcoin/bitcoin/pull/29079#pullrequestreview-1781928518)
utACK fa769d3e41daec696452b8a0a8753ba511b0a4b5

I didn't think oss-fuzz would produce inputs this large

💬 maflcko commented on pull request "fuzz: Limit p2p fuzz targets to MAX_PROTOCOL_MESSAGE_LENGTH":
(https://github.com/bitcoin/bitcoin/pull/29079#issuecomment-1855953721)
> I didn't think oss-fuzz would produce inputs this large

Same. I guess it was found with `use_value_profile=1` storing inputs hitting the `MAX_PROTOCOL_MESSAGE_LENGTH` comparison. :man_shrugging:

💬 instagibbs commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1425836162)
can't we toss `CheckV3Inheritance` entirely and just use `std::all_of` check, since we've already checked that there exists one v3 already?

e.g.,

```
const bool all_v3{std::all_of(package.cbegin(), package.cend(), [](const auto& tx){ return tx->nVersion == 3; })};
// Check inheritance rules within package.
if (!all_v3) {
// We already checked there was one at least
Assume(std::any_of(package.cbegin(), package.cend(), [](const auto& tx){ return tx->nVersion
...

💬 instagibbs commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1425837509)
```Suggestion
if (package.size() > V3_ANCESTOR_LIMIT) {
```
since we already know the whole package is v3

💬 instagibbs commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1426795358)
If parent is v3 but child v2, we wouldn't check, but you added the extra `else if` which catches that. lgtm

💬 brunoerg commented on pull request "fuzz: coinselection, improve `min_viable_change`/`change_output_size`":
(https://github.com/bitcoin/bitcoin/pull/28372#issuecomment-1855957951)
Also, I think it would be more appropriate if we don't set `change_spend_size` randomly. Perhaps using `CalculateMaximumSignedInputSize` and the same approach in `CreateTransactionInternal`.

💬 murchandamus commented on pull request "fuzz: coinselection, improve `min_viable_change`/`change_output_size`":
(https://github.com/bitcoin/bitcoin/pull/28372#issuecomment-1855990090)
I think it would be fine to roll input and output sizes randomly as long as they remain greater than zero and all the related values are derived from them. E.g. it would be an issue if `cost_of_change` or `min_viable_change` were independent of `change_spend_size`

💬 fanquake commented on pull request "ci: Set `HOMEBREW_NO_INSTALLED_DEPENDENTS_CHECK` to avoid unrelated failures":
(https://github.com/bitcoin/bitcoin/pull/29080#issuecomment-1855993456)
> For example,.
> Image Release: https://github.com/actions/runner-images/releases/tag/macos-13%2F20231025.2
> vs
> Image Release: https://github.com/actions/runner-images/releases/tag/macos-13%2F20231204.4

I still don't really understand this fix, and assume it could lead to obscure breakage in the future, i.e a missing dep, or a dep trying to use an outdated sub dep. (it also just makes our CI further differ from nomal macOS / dev setups).

I don't really understand how a new image bei
...