🤔 murchandamus reviewed a pull request: "MiniMiner: use FeeFrac in AncestorFeerateComparator"
(https://github.com/bitcoin/bitcoin/pull/30412#pullrequestreview-2172318344)
ACK 09370529fb9f6d06f6d16bdb1fb336f7a265d8ba with nits

💬 murchandamus commented on pull request "MiniMiner: use FeeFrac in AncestorFeerateComparator":
(https://github.com/bitcoin/bitcoin/pull/30412#discussion_r1674242622)
In the commit message of "MiniMiner: use FeeFrac in AncestorFeerateComparator":

```diff
- Comparing using FeeFracs is more precise, allows us to simply the
+ Comparing feerates using FeeFracs is more precise, allows us to simplify the
```

💬 murchandamus commented on pull request "MiniMiner: use FeeFrac in AncestorFeerateComparator":
(https://github.com/bitcoin/bitcoin/pull/30412#discussion_r1674364256)
In 'fuzz: mini_miner_selection fixups':

The comment in L189 threw me off. I assume that it’s meant to indicate that we are able to add the coinbase transaction to `mock_template_txids` because MiniMiner did not add it, but I first understood it to indicate that we should not be able to add a coinbase transaction to `mock_template_txids` and therefore the assert was inverse to my expectation.

Also, TIL that the return values of `emplace(…)` differ between `set` and `vector`.

💬 darosior commented on pull request "fuzz: bound some miniscript operations to avoid fuzz timeouts":
(https://github.com/bitcoin/bitcoin/pull/30197#discussion_r1674368832)
Cool, will do since i'll retouch.

💬 darosior commented on pull request "fuzz: bound some miniscript operations to avoid fuzz timeouts":
(https://github.com/bitcoin/bitcoin/pull/30197#discussion_r1674372001)
Yeah, i guess a fragment with too many sub-fragments is a too large fragment. I'll update since i'll retouch.

💬 darosior commented on pull request "fuzz: bound some miniscript operations to avoid fuzz timeouts":
(https://github.com/bitcoin/bitcoin/pull/30197#discussion_r1674373270)
Will do. Documentation good.

💬 darosior commented on pull request "fuzz: bound some miniscript operations to avoid fuzz timeouts":
(https://github.com/bitcoin/bitcoin/pull/30197#discussion_r1674376365)
> I don't know quite enough to be sure that all timeout inputs would end up in this format, but based on those two, the function as is seems good.

I can't guarantee it'd avoid all timeouts, but i'm pretty sure it would catch all timeouts which are due to too many nested wrappers.

💬 darosior commented on pull request "fuzz: bound some miniscript operations to avoid fuzz timeouts":
(https://github.com/bitcoin/bitcoin/pull/30197#discussion_r1674377322)
Arg! It slipped through. Will address as i'll retouch.

💬 darosior commented on pull request "fuzz: bound some miniscript operations to avoid fuzz timeouts":
(https://github.com/bitcoin/bitcoin/pull/30197#discussion_r1674383009)
Taproot leaves. It's to avoid overcounting if for instance you have `tr(d971308d5463ddc82addd897f6ebfa8a88e86de0664a66b9e9cf5e873459c528,{dvdvdvdvdv:1,1))`.

💬 mzumsande commented on pull request "assumeutxo: Don't load a snapshot if it's not in the best header chain":
(https://github.com/bitcoin/bitcoin/pull/30320#issuecomment-2223477561)
[b715a13 ](https://github.com/bitcoin/bitcoin/commit/b715a13d11c6ca7e928b2dee66edcc8c6c30a8d4)to [55b6d7b](https://github.com/bitcoin/bitcoin/commit/55b6d7be68a6f6c3882588ffd5b9349d885ed953): rebased

💬 stickies-v commented on pull request "fuzz: bound some miniscript operations to avoid fuzz timeouts":
(https://github.com/bitcoin/bitcoin/pull/30197#discussion_r1674385763)
Okay, thanks. No strong opinion as to which approach, but since I couldn't find any reference to it perhaps good to add this example as a docstring in this else if path?

💬 m3dwards commented on pull request "net: Allow -proxy=[::1] on nodes with IPV6 lo only":
(https://github.com/bitcoin/bitcoin/pull/30245#discussion_r1674387539)
If pursuing this approach do we think it would be prudent to reset `ai_res` to a nullptr before the second call?

💬 stickies-v commented on pull request "fuzz: bound some miniscript operations to avoid fuzz timeouts":
(https://github.com/bitcoin/bitcoin/pull/30197#discussion_r1674388962)
Or alternatively, add a separate else if branch for ignored characters (e.g. "{"), just for clarity?

💬 theuni commented on pull request "contrib: use c++ compiler rather than c compiler for binary checks":
(https://github.com/bitcoin/bitcoin/pull/30387#discussion_r1674399301)
GRRRRR!!!

💬 ismaelsadeeq commented on issue "ci: failure in `wallet_fundrawtransaction.py --legacy-wallet`":
(https://github.com/bitcoin/bitcoin/issues/30432#issuecomment-2223506811)
This issue is fixed by #30353

💬 stratospher commented on pull request "test: Fix intermittent failure in p2p_v2_misbehaving.py":
(https://github.com/bitcoin/bitcoin/pull/30420#discussion_r1674409130)
good idea, will push an update soon.

🚀 fanquake merged a pull request: "refactor: Use designated initializer in test/util/net.cpp"
(https://github.com/bitcoin/bitcoin/pull/30397)

💬 fanquake commented on issue "Intermittent issue in p2p_i2p_ports.py AssertionError: [node 0] Expected messages "['Error connecting to [...].b32.i2p:0: Cannot connect to 127.0.0.1:60000']" does not partially match log:":
(https://github.com/bitcoin/bitcoin/issues/30030#issuecomment-2223521496)
https://cirrus-ci.com/task/5777533663182848?logs=ci#L9969

💬 darosior commented on pull request "fuzz: bound some miniscript operations to avoid fuzz timeouts":
(https://github.com/bitcoin/bitcoin/pull/30197#discussion_r1674425730)
nit stack overflow: @darosior crashed and won't be able to process any further nit

💬 stickies-v commented on pull request "rpc: Use CHECK_NONFATAL over Assert":
(https://github.com/bitcoin/bitcoin/pull/30429#discussion_r1674427363)
Also, when I run this new `lint-assertions.py` on the old `rpc/blockchain.cpp` it still doesn't seem to catch the issue, I think because of the `\<` prefix? `r"(A|a)ss(ume|ert)\(",` works fine for me.