🚀 fanquake merged a pull request: "remove truc_policy from libbitcoin_common_a_SOURCES"
(https://github.com/bitcoin/bitcoin/pull/30427)

💬 stickies-v commented on pull request "fuzz: bound some miniscript operations to avoid fuzz timeouts":
(https://github.com/bitcoin/bitcoin/pull/30197#discussion_r1674334965)
Ooh, right, because each character _is_ a wrapper. That also answers my [question](https://github.com/bitcoin/bitcoin/pull/30197#discussion_r1668972900) about why we iterate in reverse, and why this implementation is correct.

This is probably fairly obvious to people familiar with Miniscript, but feel free to adopt this docstring anyway if it's helpful:

```cpp
auto count{0};
// A wrapper is a single character followed by a colon, e.g. `t:`.
// The colon is dropped between su
...

💬 stickies-v commented on pull request "fuzz: bound some miniscript operations to avoid fuzz timeouts":
(https://github.com/bitcoin/bitcoin/pull/30197#discussion_r1674335378)
Resolved as per https://github.com/bitcoin/bitcoin/pull/30197#discussion_r1674334965

👍 stickies-v approved a pull request: "fuzz: bound some miniscript operations to avoid fuzz timeouts"
(https://github.com/bitcoin/bitcoin/pull/30197#pullrequestreview-2172444820)
Light ACK 178a1da9c4da955c3dc78e6b03f110f687e82508

Code LGTM and approach seems reasonable, but there may be fuzzing and miniscript nuances I'm not considering.

💬 stickies-v commented on pull request "fuzz: bound some miniscript operations to avoid fuzz timeouts":
(https://github.com/bitcoin/bitcoin/pull/30197#discussion_r1674340428)
I can't find any reference to fragments starting with '{' in the [BIP](https://github.com/bitcoin/bips/blob/master/bip-0379.md), is `|| ch == '{'` necessary?

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