💬 stickies-v commented on pull request "fuzz: bound some miniscript operations to avoid fuzz timeouts":
(https://github.com/bitcoin/bitcoin/pull/30197#discussion_r1674344702)
nit: I think making `max_subs` and `max_wrappers` required args (and calling them with the `MAX_SUBS` and `MAX_WRAPPERS` consts) makes for a bit more intuitive code imo, since the max number is number is arbitrary.

💬 stickies-v commented on pull request "fuzz: bound some miniscript operations to avoid fuzz timeouts":
(https://github.com/bitcoin/bitcoin/pull/30197#discussion_r1674336256)
developer-notes-nit:
```suggestion
++count;
```

💬 Sjors commented on pull request "Stratum v2 Template Provider (take 3)":
(https://github.com/bitcoin/bitcoin/pull/29432#issuecomment-2223419766)
I took another look at the Template Provider to see what else the interface needs to do for an IPC sidecar to work. In addition to #30356 and #30409:

1. Expand `createNewBlock` to (optionally, only for sv2) return a vector of serialized transactions. Since the node could let go of mempool transactions at any time, and because it doesn't hold on to generated templates, we need to copy all template transactions over to the other process. This means we lose some of the advantage of having separa
...

💬 fjahr commented on pull request "assumeutxo: Add dumptxoutset height param, remove shell scripts":
(https://github.com/bitcoin/bitcoin/pull/29553#discussion_r1674351835)
Added a bit more detail in the docs improvement commit.

💬 fjahr commented on pull request "assumeutxo: Add dumptxoutset height param, remove shell scripts":
(https://github.com/bitcoin/bitcoin/pull/29553#discussion_r1674351982)
Leaving this for a follow-up for now.

💬 fjahr commented on pull request "assumeutxo: Add dumptxoutset height param, remove shell scripts":
(https://github.com/bitcoin/bitcoin/pull/29553#discussion_r1674352105)
Right, I think that makes sense and this will be called mostly with this argument than anything else. That hadn't occurred to me so far honestly because I mostly wanted this for us to create snapshots that will potentially be added to the code in the future.

I have changed this to default to the latest valid snapshot height. That makes getting the tip more cumbersome for the users but I think that's ok since this should not be what most of the want, I think.

📝 fanquake opened a pull request: "build: add `standard branch-protection` to hardening flags for aarch64-linux"
(https://github.com/bitcoin/bitcoin/pull/30433)
Use `-mbranch-protection=standard` and `-Wl,-z,pac-plt` when targetting `*aarch64-*-linux*`.
Part of #24123, but these flags can already be used on a best effort basis.

Note that these flags are also already used by default, in the toolchain, on various distros (i.e Fedora).

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