💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1669189198)
This code is gone.

💬 instagibbs commented on pull request "policy: Add PayToAnchor(P2A), `OP_TRUE <0x4e73>` as a standard output script for spending":
(https://github.com/bitcoin/bitcoin/pull/30352#issuecomment-2215089061)

> What is the 0x4e73 for?

To be segwit, it needs to be a witness version + witness program(or p2sh nested segwit script). A witness program is 2 to 40 bytes long push. For this use-case we simply use size two. The specific bytes chosen are simply "cutesy", as they cannot be 0 so something else has to be chosen and have an associated address. As far as I know this is the only proposed size 2 witness program use case so far.

> These outputs bloat the UTXO set, without sufficient value to i
...

🤔 mzumsande reviewed a pull request: "Assumeutxo: bugfix on loadtxoutset with a divergent chain + test"
(https://github.com/bitcoin/bitcoin/pull/29996#pullrequestreview-2164290570)
Code Review ACK 5b7f70ba2661a194a3c476b81e03785feddb4e1c

🤔 fjahr reviewed a pull request: "assumeutxo: Don't load a snapshot if it's not in the best header chain"
(https://github.com/bitcoin/bitcoin/pull/30320#pullrequestreview-2164298309)
re-ACK ab478c5fa16427b496e8a93e4780770d4c270214

Confirmed only changes since last review were the rebase and addressing comments (per `git range-diff master 29ab88dfe1c43af3620480c99cba844aa414023c ab478c5fa16427b496e8a93e4780770d4c270214`).

💬 fjahr 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#discussion_r1669238451)
nit: seems the "snapshot" added before is back out, still not a block for me though

💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#issuecomment-2215221618)
rebased on master, and s/anchor/dust/ everywhere

💬 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#discussion_r1669272313)
ah, sorry, am on a different computer now and forgot to update the branch... Fixed now.

💬 fjahr 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-2215232338)
re-ACK b715a13d11c6ca7e928b2dee66edcc8c6c30a8d4

Just addressed my latest re-nit.

💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1669280166)
I've changed it to "Find the best (highest-feerate, smallest among those in case of a tie) ancestor set among the remaining transactions.", because "best" is a bit more specific than highest-feerate.

Regarding the side note: that rule in `BlockAssembler` is an optimization, as it avoids looking at child transactions with higher ancestor feerate, because the ancestors will have been picked earlier anyway. The same optimization could be made in `AncestorCandidateFinder`, but because it's an $\m
...

💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1669281361)
I've added a comment that inc needs to be topological (a term which is defined in the `WorkItem` definition above).

💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1669281468)
Fixed.

💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1669282068)
I've added a short comment to the docstring of the function instead.

💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1669283836)
Done, good idea. Avoiding a separate copy `m_todo` in `Linearize` forced me to rewrite some of the code, which turned out to simplify the LIMO logic slightly.

💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1669288946)
@glozow Indeed. Even if this function was changed to return an explicit `optimal`, it would remain quite possible that it returns the optimal without *knowing* it's optimal (because it's possible that the `best` passed in for example was already optimal, but it requires a ton of iterations to exhaust the search space to prove nothing better exists).

@instagibbs Well I think we may want to cache in the mempool clusters whether or not the linearization for them is known to be optimal too, so th
...

💬 instagibbs commented on pull request "policy: Add PayToAnchor(P2A), `OP_TRUE <0x4e73>` as a standard output script for spending":
(https://github.com/bitcoin/bitcoin/pull/30352#issuecomment-2215368575)
@achow101 `wallet_backwards_compatibility.py --legacy` failure seems spurious? can you check?

🤔 stickies-v reviewed a pull request: "fuzz: bound some miniscript operations to avoid fuzz timeouts"
(https://github.com/bitcoin/bitcoin/pull/30197#pullrequestreview-2163854874)
Concept ACK

💬 stickies-v commented on pull request "fuzz: bound some miniscript operations to avoid fuzz timeouts":
(https://github.com/bitcoin/bitcoin/pull/30197#discussion_r1668972900)
It's not obvious to me why we need/prefer to iterate in reverse, here?

💬 stickies-v commented on pull request "fuzz: bound some miniscript operations to avoid fuzz timeouts":
(https://github.com/bitcoin/bitcoin/pull/30197#discussion_r1668971771)
note: with #30263 merged, you can now use `std::reverse::views` again (see #28687)

<details>
<summary>git diff on 178a1da9c4</summary>

```diff
diff --git a/src/test/fuzz/util/descriptor.cpp b/src/test/fuzz/util/descriptor.cpp
index 1ead54f254..d0ed6ee94c 100644
--- a/src/test/fuzz/util/descriptor.cpp
+++ b/src/test/fuzz/util/descriptor.cpp
@@ -3,8 +3,8 @@
// file COPYING or http://www.opensource.org/licenses/mit-license.php.

#include <test/fuzz/util/descriptor.h>
-#include <r
...

💬 stickies-v commented on pull request "fuzz: bound some miniscript operations to avoid fuzz timeouts":
(https://github.com/bitcoin/bitcoin/pull/30197#discussion_r1669371925)
Is this correct? I'd expect something like:

<details>
<summary>git diff on 178a1da9c4</summary>

```diff
diff --git a/src/test/fuzz/util/descriptor.cpp b/src/test/fuzz/util/descriptor.cpp
index 1ead54f254..434df93994 100644
--- a/src/test/fuzz/util/descriptor.cpp
+++ b/src/test/fuzz/util/descriptor.cpp
@@ -108,12 +108,10 @@ bool HasTooManyWrappers(const FuzzBufferType& buff, const int max_wrappers)
auto count{0};
// We iterate in reverse order to start counting when we enc
...

💬 stickies-v commented on pull request "fuzz: bound some miniscript operations to avoid fuzz timeouts":
(https://github.com/bitcoin/bitcoin/pull/30197#discussion_r1668978835)
nit: `HasTooManySubFrags` seems like a more accurate name?
```suggestion
bool HasTooManySubFrags(const FuzzBufferType& buff, const int max_subs = MAX_SUBS);
```