💬 fjahr commented on pull request "assumeutxo: Add dumptxoutset height param, remove shell scripts":
(https://github.com/bitcoin/bitcoin/pull/29553#discussion_r1674353019)
I need to spend some more time with this and test if this actually changed but this was added in #28618 which goes back to the conversation in the last assumeutxo PR: https://github.com/bitcoin/bitcoin/pull/27596#issuecomment-1545856135. I think nothing has changed in that regard though. I think the prune locks prevent the blocks of the snapshot from being pruned.

💬 fjahr commented on pull request "assumeutxo: Add dumptxoutset height param, remove shell scripts":
(https://github.com/bitcoin/bitcoin/pull/29553#discussion_r1674353201)
added and mentioned also that this node needs to be synced already

💬 fjahr commented on pull request "assumeutxo: Add dumptxoutset height param, remove shell scripts":
(https://github.com/bitcoin/bitcoin/pull/29553#discussion_r1674353313)
fixed

💬 fjahr commented on pull request "assumeutxo: Add dumptxoutset height param, remove shell scripts":
(https://github.com/bitcoin/bitcoin/pull/29553#discussion_r1674353598)
I have implemented the RAII class and expanded the comment. If my understanding is correct I think we would punish a peer as misbehaving if they send us a block that is connecting to an invalid block (https://github.com/bitcoin/bitcoin/blob/master/src/net_processing.cpp#L1927) so we would punish any peer sending us a normal new block. We also wouldn't accept transactions that have a locktime above the snapshot height though I am not sure if switching off network activity is getting us a better r
...

💬 fjahr commented on pull request "assumeutxo: Add dumptxoutset height param, remove shell scripts":
(https://github.com/bitcoin/bitcoin/pull/29553#discussion_r1674354358)
This is fixed now since #29668 got merged and I am using this here now.

🤔 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