Ephemeral Dust #30239

pull instagibbs wants to merge 8 commits into bitcoin:master from instagibbs:2024-03-general-ephemeral changing 19 files +1139 −4
  1. instagibbs commented at 5:31 pm on June 6, 2024: member

    A replacement for #29001

    Now that we have 1P1C relay, TRUC transactions and sibling eviction, it makes sense to retarget this feature more narrowly by not introducing a new output type for now, and simple focusing on the feature of allowing temporary dust in the mempool.

    Users of this can immediately use dust outputs as:

    1. Single keyed anchor (can be shared by multiple parties)
    2. Single unkeyed anchor, ala P2A

    Which is useful when the parent transaction cannot have fees for technical or accounting reasons.

    What I’m calling “keyed” anchors would be used anytime you don’t want a third party to be able to run off with the utxo. As a motivating example, in Ark there is the concept of a “forfeit transaction” which spends a “connector output”. The connector output would ideally be 0-value, but you would not want that utxo spend by anyone, because this would cause financial loss for the coordinator of the service: https://arkdev.info/docs/learn/concepts#forfeit-transaction

    Note that this specific use-case likely doesn’t work as it involves a tree of dust, but the connector idea in general demonstrates how it could be used.

    Another related example is connector outputs in BitVM2: https://bitvm.org/bitvm2.html .

    Note that non-TRUC usage will be impractical unless the minrelay requirement on individual transactions are dropped in general, which should happen post-cluster mempool.

  2. DrahtBot commented at 5:31 pm on June 6, 2024: contributor

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    Code Coverage

    For detailed information about the code coverage, see the test coverage report.

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    Concept NACK petertodd
    Concept ACK glozow, t-bast

    If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #30232 (refactor: policy: Pass kernel::MemPoolOptions to IsStandard[Tx] rather than long list of individual options by luke-jr)
    • #29247 (CAT in Tapscript (BIP-347) by 0xBEEFCAF3)
    • #28690 (build: Introduce internal kernel library by TheCharlatan)
    • #28676 ([WIP] Cluster mempool implementation by sdaftuar)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

  3. instagibbs force-pushed on Jun 6, 2024
  4. DrahtBot added the label CI failed on Jun 6, 2024
  5. DrahtBot commented at 5:39 pm on June 6, 2024: contributor

    🚧 At least one of the CI tasks failed. Make sure to run all tests locally, according to the documentation.

    Possibly this is due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

    Leave a comment here, if you need help tracking down a confusing failure.

    Debug: https://github.com/bitcoin/bitcoin/runs/25906194302

  6. instagibbs commented at 6:44 pm on June 6, 2024: member
    going to put this into draft until #29496 is merged, since dust checks are completely off when -acceptnonstdtxn=1. which is required for TRUC currently.
  7. instagibbs marked this as a draft on Jun 6, 2024
  8. instagibbs force-pushed on Jun 7, 2024
  9. instagibbs force-pushed on Jun 7, 2024
  10. instagibbs marked this as ready for review on Jun 7, 2024
  11. DrahtBot removed the label CI failed on Jun 8, 2024
  12. theStack commented at 5:22 pm on June 9, 2024: contributor
    Is it an option to be more restrictive and only allow zero-value outputs as ephemeral anchors, for not having to deal with the concept of dust at all? Or, asked differently: what would be the motivation/benefit for users to ever create an anchor output with nValue > 0? (Note that I haven’t looked too deep into the concept of ephemeral anchors and the predecessor PR #29001, so very likely I’m missing something obvious here, but I guess the answer could be interesting for other potential reviewers too.)
  13. instagibbs commented at 1:39 pm on June 10, 2024: member

    @theStack the primary motivation is to cover cases where non-0 value is attached to handle cases where a smart contract may want to “throw away” a few sats to fees, but otherwise cannot because of the 0-fee requirement of this PR for transactions with ephemeral anchors. If the ephemeral anchor-having transaction had non-0-fee, that would allow endogenous incentives to get it mined on its own, leaving the dust in the utxo set. As an example from the LN spec, [trimmed outputs(https://github.com/lightning/bolts/blob/master/03-transactions.md#trimmed-outputs) are directly added to the commitment transaction fee. Instead, spec writers could have the value flow to the ephemeral anchor, which is then spent to fees by the child transaction. Example spec here: https://github.com/instagibbs/bolts/commits/zero_fee_commitment

    It’s a fairly narrow motivation, and honestly I don’t love the timmed output to fees scheme, but also doesn’t make the code anymore complicated, so I think it’s worth consideration.

  14. DrahtBot added the label CI failed on Jun 16, 2024
  15. DrahtBot commented at 8:07 pm on June 16, 2024: contributor

    🚧 At least one of the CI tasks failed. Make sure to run all tests locally, according to the documentation.

    Possibly this is due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

    Leave a comment here, if you need help tracking down a confusing failure.

    Debug: https://github.com/bitcoin/bitcoin/runs/25953908933

  16. bitcoin deleted a comment on Jun 16, 2024
  17. instagibbs force-pushed on Jun 17, 2024
  18. instagibbs commented at 2:54 pm on June 17, 2024: member
    rebased due to s/nVersion/version/ change for transactions causing a silent merge conflict
  19. DrahtBot removed the label CI failed on Jun 17, 2024
  20. instagibbs force-pushed on Jun 20, 2024
  21. DrahtBot added the label CI failed on Jul 2, 2024
  22. instagibbs renamed this:
    Ephemeral Anchors, take 2
    Ephemeral Dust
    on Jul 2, 2024
  23. DrahtBot added the label Needs rebase on Jul 2, 2024
  24. DrahtBot removed the label CI failed on Jul 3, 2024
  25. instagibbs force-pushed on Jul 8, 2024
  26. instagibbs commented at 8:30 pm on July 8, 2024: member
    rebased on master, and s/anchor/dust/ everywhere
  27. DrahtBot removed the label Needs rebase on Jul 8, 2024
  28. glozow commented at 10:08 am on July 15, 2024: member
    concept ACK
  29. petertodd commented at 2:40 pm on July 18, 2024: contributor

    Concept NACK

    There’s no reason to limit this behavior to TRUC/V3 transactions. The implementation should not ship with this restriction.

  30. instagibbs commented at 7:09 pm on July 18, 2024: member

    @petertodd I’ll give a non-TRUC version another try and report back. I may have been over-thinking things. If it works out it will be very similar in code structure…

    edit: oh of course I already noted in OP: We don’t allow individual transactions with 0-fee outside of TRUC, for now due to trimming computation concerns. Cluster mempool does away with this restriction entirely. I’ll rework the code to upgrade to non-TRUC usage once cluster mempool hits.

  31. petertodd commented at 1:09 am on July 19, 2024: contributor

    @petertodd I’ll give a non-TRUC version another try and report back. I may have been over-thinking things. If it works out it will be very similar in code structure…

    edit: oh of course I already noted in OP: We don’t allow individual transactions with 0-fee outside of TRUC, for now due to trimming computation concerns. Cluster mempool does away with this restriction entirely. I’ll rework the code to upgrade to non-TRUC usage once cluster mempool hits.

    I think the worthwhile question here is why are 0-fee CPFP’s required to signal TRUC/V3? I don’t see any reason why transaction creators should be forced to adhere to the restrictive TRUC/V3 rules if they do not wish to.

  32. t-bast commented at 9:29 am on July 19, 2024: contributor
    Concept ACK :+1:
  33. petertodd commented at 2:59 pm on July 19, 2024: contributor

    I’m also going to point out that restricting ephemeral dust transactions to zero fee seems to create protocol design problems around HTLCs: https://delvingbitcoin.org/t/ephemeral-anchors-and-mevil/383

    The simplest solution with dust HTLCs is to just put the value towards mining fees. Forcing them to be put towards the keyless ephemeral output potentially creates incentive problems around stealing that output. If ephemeral dust transactions have fewer rules around them, that kind of design problem goes away and protocols can do what makes sense for them without trying to meet TRUC/V3 requirements.

  34. t-bast commented at 3:50 pm on July 19, 2024: contributor

    Forcing them to be put towards the keyless ephemeral output potentially creates incentive problems around stealing that output.

    Not really, because it will always be the miner that will steal it (since they can replace any spend by a spend to themselves). As was detailed in the delving post you’re referencing, putting HTLC values in the keyless ephemeral output ensures that it’s spent and doesn’t pollute the utxo set, while putting the fees directly to the commitment transaction doesn’t incentivize spending that output?

  35. petertodd commented at 4:10 pm on July 19, 2024: contributor

    Stealing an ephemeral anchor output can only be done in certain conditions, at specific ranges of values. It is not always profitable due to overheads, and you can reduce those overheads by doing complex things.

    The game theory is much simpler if the dust HTLC funds go directly to fees.

    On July 19, 2024 5:50:49 PM GMT+02:00, Bastien Teinturier @.***> wrote:

    Forcing them to be put towards the keyless ephemeral output potentially creates incentive problems around stealing that output.

    Not really, because it will always be the miner that will steal it (since they can replace any spend by a spend to themselves). As was detailed in the delving post you’re referencing, putting HTLC values in the keyless ephemeral output ensures that it’s spent and doesn’t pollute the utxo set, while putting the fees directly to the commitment transaction doesn’t incentivize spending that output?

  36. instagibbs force-pushed on Jul 19, 2024
  37. instagibbs commented at 5:32 pm on July 19, 2024: member

    Ok, the rework ended up being quite easy.

    Pushed an update that generalizes to non-TRUC transactions, but as noted earlier, individual transactions have to meet minrelay for now unless they are TRUC, see/divert discussion here for that specific point: #26933 . If this restriction goes away, it will be usable(see tests I added with v2 transactions at the bottom where I checked it). Note that we are still constrained to 1P1C relay until we get general package relay.

    re:0-fee, yes it’s obviously sub-optimal for transaction creation for things like trimmed values. There’s no simple way that I can think of that ensures that dust isn’t mined. E.g., TxA with dust, followed by TxB spending the dust. Both enter mempool. Then TxB' replaces TxB, but doesn’t spend the dust output from TxA. If TxA has fees at or above minrelay, it may well be mined on its own.

    I don’t have any clean ways of doing that at present. If we figure it out, it’s another clean relaxation that should have easy logic to follow. @glozow had suggested long ago that we could allow any fee under minrelay for ephemeral tx, but this is still pretty limited in what amounts are allowed for fees. I’m not convinced it’s worth it vs thinking harder about figuring out how to allow unbounded feerates.

  38. t-bast commented at 8:25 am on July 22, 2024: contributor

    Stealing an ephemeral anchor output can only be done in certain conditions, at specific ranges of values. It is not always profitable due to overheads, and you can reduce those overheads by doing complex things.

    I don’t understand this hand-wavy comment, can you detail? I don’t see any complexity here. Whenever a miner would include a transaction spending an ephemeral output in a block:

    • if that ephemeral output is economical, the miner replaces the spending transaction by one that sends funds to themselves
    • otherwise, they keep the existing transaction (which in most cases will contain an unrelated input to bring fees)

    Sounds trivial enough?

  39. DrahtBot added the label CI failed on Jul 22, 2024
  40. DrahtBot commented at 4:54 pm on July 22, 2024: contributor

    🚧 At least one of the CI tasks failed. Debug: https://github.com/bitcoin/bitcoin/runs/27674254741

    Make sure to run all tests locally, according to the documentation.

    The failure may happen due to a number of reasons, for example:

    • Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

    • A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

    • An intermittent issue.

    Leave a comment here, if you need help tracking down a confusing failure.

  41. instagibbs force-pushed on Jul 22, 2024
  42. Mahmoud198425 approved
  43. in src/policy/ephemeral_policy.cpp:80 in 6eca593c83 outdated 
    75+
76+        // Process new dust
77+        for (uint32_t i=0; i<tx->vout.size(); i++) {
78+            if (IsDust(tx->vout[i], dust_relay_rate)) {
79+                // CheckValidEphemeralTx should disallow multiples
80+                Assume(map_tx_dust.count(tx->GetHash()) == 0);
    achow101 commented at 7:44 pm on July 22, 2024:

    In 872c06dacde6c45c2add69c8c15c94571b08119d “policy: Allow dust in transactions, spent in-mempool”

    nit: Use contains, here and elsewhere.

    0                Assume(!map_tx_dust.contains(tx->GetHash()));
    instagibbs commented at 2:08 am on July 23, 2024:
    done
  44. in src/policy/ephemeral_policy.h:23 in 6eca593c83 outdated 
    18+ * Otherwise it returns true.
19+ */
20+bool CheckValidEphemeralTx(const CTransaction& tx, CFeeRate dust_relay_fee, CAmount txfee, TxValidationState& state);
21+
22+/** Checks that all dust in package ends up spent by direct children. Assumes package is well-formed and sorted. */
23+std::optional<uint256> CheckEphemeralSpends(const Package& package, CFeeRate dust_relay_rate);
    achow101 commented at 8:05 pm on July 22, 2024:

    In 872c06dacde6c45c2add69c8c15c94571b08119d “policy: Allow dust in transactions, spent in-mempool”

    Could you document what the return value is?

    instagibbs commented at 2:08 am on July 23, 2024:
    documented
  45. in src/policy/ephemeral_policy.cpp:58 in 6eca593c83 outdated 
    68+            // We want to detect dangling dust too
69+            unspent_dust.erase(tx_input.prevout);
70+        }
71+
72+        if (!child_unspent_dust.empty()) {
73+            return tx->GetHash();
    achow101 commented at 8:08 pm on July 22, 2024:

    In 872c06dacde6c45c2add69c8c15c94571b08119d “policy: Allow dust in transactions, spent in-mempool”

    I’m a little confused about this return value. The usage of this function suggests that the return value is the wtxid of a parent transaction that has unspent dust. However, this appears to be returning the child txid.

    instagibbs commented at 2:08 am on July 23, 2024:
    switched to txid since that’s how it’s being tracked
  46. in src/validation.cpp:1606 in 6eca593c83 outdated 
    1599@@ -1560,6 +1600,19 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std::
1600         }
1601     }
1602 
1603+    // Run package-based dust spentness checks
1604+    if (m_pool.m_opts.require_standard) {
1605+        if (const auto ephemeral_violation{CheckEphemeralSpends(txns, m_pool.m_opts.dust_relay_feerate)}) {
1606+            const auto parent_wtxid = ephemeral_violation.value();
    achow101 commented at 8:09 pm on July 22, 2024:

    In 872c06dacde6c45c2add69c8c15c94571b08119d “policy: Allow dust in transactions, spent in-mempool”

    CheckEphemeralSpends appears to only return txids, not wtxid, suggest renaming this variable:

    0            const auto parent_txid = ephemeral_violation.value();
    instagibbs commented at 2:08 am on July 23, 2024:
    done
  47. in src/policy/ephemeral_policy.cpp:55 in 6eca593c83 outdated 
    65+                 child_unspent_dust.erase(prevout.hash);
66+            }
67+
68+            // We want to detect dangling dust too
69+            unspent_dust.erase(tx_input.prevout);
70+        }
    achow101 commented at 8:17 pm on July 22, 2024:

    In 872c06dacde6c45c2add69c8c15c94571b08119d “policy: Allow dust in transactions, spent in-mempool”

    It’s not clear to me why there is a rule that a descendant must also spend the dust if they spent any other output from an ancestor. This rule does not seem to be mentioned anywhere in the PR description.

    ISTM it would be easier and just as effective to check that all dust outputs end up being spent rather than this slightly convoluted check.

    instagibbs commented at 8:58 pm on July 22, 2024:

    Imagine three transactions:

    TxA, 0-fee with two outputs, one non-dust, one dust TxB, spends TxA’s non-dust TxC, spends TxA’s dust

    All the dust is spent if TxA+TxB+TxC is accepted, but the mining template may just pick up TxA+TxB rather than the three “legal configurations: 0) None

    1. TxA+TxB+TxC
    2. TxA+TxC

    By requiring the child transaction to sweep any dust from the parent txn, we ensure that there is a single child only, and this child is the only transaction possible for bringing fees, or itself being spent by another child, and so on.

    instagibbs commented at 9:12 pm on July 22, 2024:
    I can update the documentation reflecting this if you find the explanation compelling.
    achow101 commented at 9:23 pm on July 22, 2024:

    Ah, I see. Any child must spend the dust output, and I see that this is being enforced in the other variant too.

    Additional documentation would be appreciated.

    instagibbs commented at 2:08 am on July 23, 2024:
    added some documentation
  48. DrahtBot removed the label CI failed on Jul 22, 2024
  49. instagibbs force-pushed on Jul 23, 2024
  50. instagibbs force-pushed on Jul 23, 2024
  51. instagibbs force-pushed on Jul 24, 2024
  52. instagibbs commented at 8:29 pm on July 24, 2024: member
    added fuzz and unit tests which cover more than the functional tests can alone
  53. instagibbs force-pushed on Jul 24, 2024
  54. DrahtBot added the label CI failed on Jul 24, 2024
  55. DrahtBot commented at 8:32 pm on July 24, 2024: contributor

    🚧 At least one of the CI tasks failed. Debug: https://github.com/bitcoin/bitcoin/runs/27880415572

    Make sure to run all tests locally, according to the documentation.

    The failure may happen due to a number of reasons, for example:

    • Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

    • A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

    • An intermittent issue.

    Leave a comment here, if you need help tracking down a confusing failure.

  56. DrahtBot removed the label CI failed on Jul 25, 2024
  57. instagibbs force-pushed on Jul 26, 2024
  58. instagibbs force-pushed on Jul 27, 2024
  59. instagibbs force-pushed on Jul 29, 2024
  60. petertodd commented at 7:53 am on August 2, 2024: contributor

    It looks like we’ve created a new, rare, pinning vector here: in the event that a dust-generating transaction is reorged back into the mempool, transactions that otherwise could have spent an ordinary output of that transaction alone now can’t be accepted into the mempool because they fail the “spends all parent dust” check.

    For example, imagine a coinjoin is funded by a transaction A containing a dust output. After a reorg, the transaction A gets forced back into the mempool. Now the coinjoin (or double-spends of it) fails the spends all parents dust check, because it doesn’t spend the dust of A, and A may never get mined due to fee-rates increasing.

  61. instagibbs commented at 2:01 pm on August 2, 2024: member

    @petertodd I believe that’s incorrect, and looks like I wrote a test case for this: https://github.com/bitcoin/bitcoin/pull/30239/commits/63e097092253b25fa9d72af167d83d911c805c9c#diff-f3bb3623abbd14a7140e405175278c9187eb8c2cf6ff0279700ac507c5f4180cR334

    I show that a child spend of parent tx gets into mempool even if it has dust of its own, and fails to spend parent’s dust.

    Reorgs should hopefully be expensive, so we simply allow things back in mempool even if the spentness is violated.

    You can glance at the new usages of bypass_limits in the code changes to see. I should probably add a test showing that a tx with two dust or fee-having can’t use bypass_limits.

  62. instagibbs force-pushed on Aug 2, 2024
  63. glozow referenced this in commit 2aff9a36c3 on Aug 2, 2024
  64. instagibbs force-pushed on Aug 2, 2024
  65. instagibbs commented at 5:06 pm on August 2, 2024: member
    rebased on top of master to pick up PayToAnchor support for those who would like to test them together
  66. petertodd commented at 7:38 pm on August 2, 2024: contributor

    @petertodd I believe that’s incorrect, and looks like I wrote a test case for this: 63e0970#diff-f3bb3623abbd14a7140e405175278c9187eb8c2cf6ff0279700ac507c5f4180cR334

    I show that a child spend of parent tx gets into mempool even if it has dust of its own, and fails to spend parent’s dust.

    Reorgs should hopefully be expensive, so we simply allow things back in mempool even if the spentness is violated.

    You can glance at the new usages of bypass_limits in the code changes to see. I should probably add a test showing that a tx with two dust or fee-having can’t use bypass_limits.

    I think you’re misunderstanding the scenario I’m talking about. I’m not saying that transactions in a block won’t get back into your mempool when your node reorgs the block - they of course do get it due to the bypass_limits flag. What I’m saying is that subsequent transactions won’t be accepted, resulting in a pin.

    Concretely, imagine that the top block contains transaction A, which contains a dust output. Transaction B, which spends an output of A, can be accepted into your mempool as A is mined, and we can double-spend B as usual with a higher fee-rate.

    Now let’s reorg out the top block. Transaction A is forced into the mempool using bypass_limits; transaction B remains in the mempool. However, we can no longer double-spend B with a higher fee-rate, as any B2 is rejected on the basis that it does not spend the dust output of A. Thus, B is pinned by the existence of A in your mempool.

    While this is a relatively rare scenario, it’s not that rare. And there’s probably circumstances where attackers could take advantage of this opportunistically by waiting until they happened to have a transaction in the right state to use in an attack.

  67. petertodd commented at 7:51 pm on August 2, 2024: contributor
    I should point out that the TRUC code may suffer from this same category of problem around reorgs too…
  68. instagibbs commented at 8:06 pm on August 2, 2024: member

    Got it. Normally a reorg like this would result in an effective feerate reduction, as the entire package is now having to be paid for. With the dust spending restriction, it makes it impossible to replace later, exactly like a package limit pin.

    I should point out that the TRUC code may suffer from this same category of problem around reorgs too…

    Depends on the topo but yep, post-reorg package limits may be hit unexpectedly. Parent->child->grandchild, where parent was formerly confirmed, as an example. Regular chain limits are also possible to hit, f.e. if the re-entered parent was 100kvB, or full chain limits hit.

    Circling back to dust, I suppose bypass-ness of a transaction entry(the re-entered parent in this context) could be tracked and spenders could bypass the dust spentness check as well? Will mull it over.

  69. petertodd commented at 6:37 pm on August 3, 2024: contributor

    Got it. Normally a reorg like this would result in an effective feerate reduction, as the entire package is now having to be paid for. With the dust spending restriction, it makes it impossible to replace later, exactly like a package limit pin.

    Exactly. And unlike the inherent pin possible by the fact that the reorged transaction could simply be big, this variant works even if the transaction is small, making pulling it off potentially a lot easier.

    Personally I’d consider removing the restrictions on spending outputs of ephemeral dust transactions. For what Core is trying to accomplish here, I think it’s enough that the transaction was accepted into your mempool with the dust being spent. That’s enough of a nudge to get the vast majority of use-cases to do what we want them to do. It’s more important that this feature can be used securely by L2 implementations, without having to worry about security problems from complex edge cases.

    People who want to actually create dust intentionally can do so pretty easily anyway by just sending their transactions to one of the pools that doesn’t enforce the dust limit; most of the recent growth is probably due to intentional token protocols doing things for which rules like this are irrelevant.

    Suppose we do remove that restriction. You would be able to create an unspent dust UTXO by broadcasting a transaction package creating the UTXO in A, a transaction with a dust output and a non-dust output, and then spending it with a two input transaction B spending both those outputs. Then you double-spend B to remove the dust input while still providing miners an incentive to mine A.

    Thing is, to do this you have to spend about as much money in fees as it would have cost you in lost sats to create an unspent non-dust output instead. Probably more, because the dust limit is based on an unrealistically low 1sat/vb fee-rate. This isn’t actually any worse than the status quo for an attacker trying to create dust.

    Similarly, I don’t think we actually need the restriction only having one dust output per transaction. If they’ve all been spent to get the transaction into your mempool, playing games to get them unspent costs as much or more money as just creating non-dust UTXOs.

    Regular chain limits are also possible to hit, f.e. if the re-entered parent was 100kvB, or full chain limits hit.

    While I’m not sure off the top of my head if this is actually true, it should be the case that RBF’s of existing transactions should always be allowed even if package limits have been exceeded somehow. I proposed this principle awhile back as the “always replaceable invariant”: the mempool should always allow you to replace a transaction already in the mempool with a similar one at higher fee so that you can make forward progress.

    This specific issue contradicts that principal because it’s a hard restriction on the type of transaction allowed in a specific circumstance.

  70. hebasto added the label Needs CMake port on Aug 16, 2024
  71. instagibbs force-pushed on Aug 28, 2024
  72. instagibbs force-pushed on Aug 28, 2024
  73. hebasto removed the label Needs CMake port on Aug 29, 2024
  74. instagibbs force-pushed on Aug 30, 2024
  75. instagibbs force-pushed on Aug 30, 2024
  76. DrahtBot added the label Needs rebase on Sep 2, 2024
  77. instagibbs force-pushed on Sep 3, 2024
  78. DrahtBot removed the label Needs rebase on Sep 3, 2024
  79. instagibbs force-pushed on Sep 3, 2024
  80. in src/policy/ephemeral_policy.cpp:15 in 1baab61485 outdated 
    10+    bool has_dust = false;
11+    for (const CTxOut& txout : tx.vout) {
12+        if (IsDust(txout, dust_relay_fee)) {
13+            // We only allow a single dusty output
14+            if (has_dust) {
15+                return state.Invalid(TxValidationResult::TX_NOT_STANDARD, "dust");
    glozow commented at 5:52 pm on September 3, 2024: 
    0                return state.Invalid(TxValidationResult::TX_NOT_STANDARD, "dust", "tx has more than 1 dust output");
    instagibbs commented at 3:17 pm on September 5, 2024:
    never actually reachable as this is already caught in IsStandardTx, removed this error condition
  81. in src/policy/ephemeral_policy.cpp:26 in 1baab61485 outdated 
    21+    // No dust; it's complete standard already
22+    if (!has_dust) return true;
23+
24+    // We never want to give incentives to mine this alone
25+    if (txfee != 0) {
26+        return state.Invalid(TxValidationResult::TX_NOT_STANDARD, "dust");
    glozow commented at 5:52 pm on September 3, 2024: 
    0        return state.Invalid(TxValidationResult::TX_NOT_STANDARD, "dust", "tx with dust output must be 0 fee");
    instagibbs commented at 3:17 pm on September 5, 2024:
    done
  82. instagibbs commented at 5:57 pm on September 3, 2024: member

    Reworked the PR to relax IsStandardTx to allow a single dust output, and hopefully simplified the remaining checks that enforces spend of dusty parent’s dust. This should assure the same mempool invariants as before, but with more understandable and minimal changes to logic flow. @petertodd

    It’s more important that this feature can be used securely by L2 implementations, without having to worry about security problems from complex edge cases.

    I can’t speak for all wallets, but at least for CLN the fresh spend of funding outputs will be detected and the anchor spending will start anew. I think for first cut the tradeoff of having invariants that are easy to reason about is best. Given that there are actionable things wallets can do and the minimal complexity, I’m sticking with the overall current approach.

    While I’m not sure off the top of my head if this is actually true, it should be the case that RBF’s of existing transactions should always be allowed even if package limits have been exceeded somehow.

    Today’s mempool does a suboptimal job of dealing with chain limits and RBFs. Assuming cluster mempool, we still have the issue of what to do with cluster limits that are exceeded even after removing RBF conflicts, if any. This has been called “sibling eviction” and in general seems doable in a DoS-resistant way but is left to future work. It’s a problem I’m very interested in solving post-cluster mempool.

  83. instagibbs force-pushed on Sep 3, 2024
  84. DrahtBot commented at 8:46 pm on September 3, 2024: contributor

    🚧 At least one of the CI tasks failed. Debug: https://github.com/bitcoin/bitcoin/runs/29626041615

    Make sure to run all tests locally, according to the documentation.

    The failure may happen due to a number of reasons, for example:

    • Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

    • A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

    • An intermittent issue.

    Leave a comment here, if you need help tracking down a confusing failure.

  85. DrahtBot added the label CI failed on Sep 3, 2024
  86. instagibbs force-pushed on Sep 3, 2024
  87. DrahtBot removed the label CI failed on Sep 4, 2024
  88. petertodd commented at 11:39 am on September 4, 2024: contributor

    Today’s mempool does a suboptimal job of dealing with chain limits and RBFs. Assuming cluster mempool, we still have the issue of what to do with cluster limits that are exceeded even after removing RBF conflicts, if any. This has been called “sibling eviction” and in general seems doable in a DoS-resistant way but is left to future work. It’s a problem I’m very interested in solving post-cluster mempool.

    What I mean is, if you are doing an RBF replacement of one transaction with another transaction paying a higher fee-rate, with the same or smaller size, this should always be allowed even if somehow your mempool got into a state where a cluster limit was exceeded.

    Point is, the replacement isn’t making anything worse, so it should be allowed. Sibling eviction isn’t needed to support this case.

  89. in test/functional/mempool_ephemeral_dust.py:35 in c98fb6f490 outdated 
    30+        if not expected:
31+            expected = []
32+        mempool = self.nodes[0].getrawmempool(verbose=False)
33+        assert_equal(len(mempool), len(expected))
34+        for tx in expected:
35+            assert tx.rehash() in mempool
    glozow commented at 7:00 pm on September 4, 2024:
    could go in test_framework/mempool_util.py?
    instagibbs commented at 3:17 pm on September 5, 2024:
    done
  90. in test/functional/mempool_ephemeral_dust.py:177 in c98fb6f490 outdated 
    172+        assert_equal(res["package_msg"], "transaction failed")
173+        assert_equal(res["tx-results"][dusty_tx["wtxid"]]["error"], "dust")
174+        assert_equal(self.nodes[0].getrawmempool(), [])
175+
176+    def test_nonzero_dust(self):
177+        self.log.info("Test that a transaction with any value is standard as long as it's spent")
    glozow commented at 7:10 pm on September 4, 2024: 
    0        self.log.info("Test that ephemeral dust is allowed for non-0 dust values")
    instagibbs commented at 3:17 pm on September 5, 2024:
    done
  91. in test/functional/mempool_ephemeral_dust.py:372 in c98fb6f490 outdated 
    367+        # Re-connect and make sure we have same state still
368+        self.connect_nodes(0, 1)
369+        self.sync_all()
370+
371+    def test_acceptnonstd(self):
372+        self.log.info("Test that dust is still allowed when acceptnonstd=1")
    glozow commented at 7:13 pm on September 4, 2024:
    acceptnonstdtxn
    instagibbs commented at 3:17 pm on September 5, 2024:
    done
  92. in test/functional/mempool_ephemeral_dust.py:95 in c98fb6f490 outdated 
    90+
91+        # But package evaluation succeeds
92+        res = self.nodes[0].submitpackage([dusty_tx["hex"], sweep_tx["hex"]])
93+        assert_equal(res["package_msg"], "success")
94+        assert_equal(len(self.nodes[0].getrawmempool()), 2)
95+        self.assert_mempool_contents(expected=[dusty_tx["tx"], sweep_tx["tx"]])
    glozow commented at 7:14 pm on September 4, 2024:
    I think all of these assert_mempool_contents can also have a self.sync_mempools() to check that the ephemeral dust packages propagate.
    instagibbs commented at 3:17 pm on September 5, 2024:
    added an optional sync argument, only unused during reorg test
  93. in src/test/txvalidation_tests.cpp:133 in c98fb6f490 outdated 
    128+    // We first start with nothing "in the mempool", using package checks
129+
130+    // Trivial single transaction with no dust
131+    BOOST_CHECK(!CheckEphemeralSpends({dust_spend}, minrelay).has_value());
132+
133+    // Now with dust, ok because no spending transaction exists
    glozow commented at 7:28 pm on September 4, 2024: 
    0    // Now with dust, ok because the tx has no dusty parents
    instagibbs commented at 3:17 pm on September 5, 2024:
    done
  94. in src/validation.cpp:1581 in c98fb6f490 outdated 
    1574@@ -1560,6 +1575,19 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std::
1575         }
1576     }
1577 
1578+    // Run package-based dust spentness checks
1579+    if (m_pool.m_opts.require_standard) {
1580+        if (const auto ephemeral_violation{CheckEphemeralSpends(txns, m_pool.m_opts.dust_relay_feerate)}) {
1581+            const Txid parent_txid = ephemeral_violation.value();
    glozow commented at 7:29 pm on September 4, 2024: 
    0            const Txid& parent_txid = ephemeral_violation.value();
    instagibbs commented at 3:17 pm on September 5, 2024:
    done
  95. in src/policy/policy.cpp:151 in c98fb6f490 outdated 
    148+            num_dust_outputs++;
149         }
150     }
151 
152+    // Only one dust is permitted(on otherwise valid ephemeral dust)
153+    if (num_dust_outputs > 1) {
    glozow commented at 7:40 pm on September 4, 2024:
    if you make 1 a constexpr MAX_DUST_OUTPUTS_PER_TX, you can static_assert that it == 1 as an assumption in CheckEphemeralSpends
    instagibbs commented at 3:18 pm on September 5, 2024:
    great idea, added
  96. in src/test/txvalidation_tests.cpp:160 in c98fb6f490 outdated 
    155+    BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, dust_non_spend_both_parents}, minrelay).has_value());
156+
157+    auto dust_spend_both_parents = make_tx({COutPoint{dust_txid, dust_index}, COutPoint{dust_txid_2, dust_index}}, /*version=*/2);
158+    BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, dust_spend_both_parents}, minrelay).has_value());
159+
160+    // Spending all outputs is also correct
    glozow commented at 7:46 pm on September 4, 2024: 
    0    // Spending other outputs is also correct, as long as the dusty one is spent
    instagibbs commented at 3:18 pm on September 5, 2024:
    done
  97. in src/policy/ephemeral_policy.cpp:48 in c98fb6f490 outdated 
    43+
44+    for (const auto& tx : package) {
45+        std::unordered_set<Txid, SaltedTxidHasher> child_unspent_dust;
46+        for (const auto& tx_input : tx->vin) {
47+            // Parent tx had dust, child MUST be sweeping it
48+            // if it's spending any output from parent
    glozow commented at 7:50 pm on September 4, 2024:
    Could comment that tx_input.prevout.n doesn’t need to equal map_tx_dust.at(tx_input.prevout.hash); a child can spend multiple outputs of a parent.
    instagibbs commented at 3:18 pm on September 5, 2024:
    elaborated a bit, let me know if it helps
  98. glozow commented at 7:56 pm on September 4, 2024: member

    Looks good. New rules seem easier to reason about (up to 1 as long as 0-fee, must spend all of my parent’s dust no matter what).

    Can you remind me of the use cases of a keyed dust output again? It seems slightly safer to require ephemeral dust be P2A, as it would allow anybody to clean them up in case something goes wrong, but perhaps too narrow.

    Additionally, could add a release note fragment.

  99. instagibbs force-pushed on Sep 5, 2024
  100. instagibbs commented at 3:40 pm on September 5, 2024: member

    With the new changes, I’m considering disallowing entry into the mempool if either the base fee or the modified fee is non-0. If miners really want to mine things generally with 0 fee that’s one thing, but offering an interface to avoid the dust cleanup seems suboptimal. @glozow

    Can you remind me of the use cases of a keyed dust output again?

    What I’m calling “keyed” anchors would be used anytime you don’t want a third party to be able to run off with the utxo. As a motivating example, in Ark there is the concept of a “forfeit transaction” which spends a “connector output”. The connector output would ideally be 0-value, but you would not want that utxo spend by anyone, because this would cause financial loss for the coordinator of the service: https://arkdev.info/docs/learn/concepts#forfeit-transaction

    Note that this specific use-case likely doesn’t work as it involves a tree of dust, but the connector idea in general demonstrates how it could be used.

    Another related example is connector outputs in BitVM2: https://bitvm.org/bitvm2.html . You could imag

    It seems slightly safer to require ephemeral dust be P2A, as it would allow anybody to clean them up in case something goes wrong, but perhaps too narrow.

    In general. if we have 0-value dust, I find it unlikely to hope that miners will clean it up for no reason?

    I’m also pretty hesitant to guess what people will find useful in general, e.g., if they’re worried about third party replacement cycling, they can be more cautious. The can also include a tapscript branch for anyonecanspend cleanup, just like LN does today!

    I feel moderately strongly about this, but of course relaxing policy further later is also another valid choice.

    Additionally, could add a release note fragment.

    Added

  101. glozow commented at 6:50 pm on September 5, 2024: member

    I’m also pretty hesitant to guess what people will find useful in general, e.g., if they’re worried about third party replacement cycling, they can be more cautious. The can also include a tapscript branch for anyonecanspend cleanup, just like LN does today!

    Seems reasonable to want keyed. Mostly wanted the rationale to be written up here.

  102. instagibbs force-pushed on Sep 5, 2024
  103. instagibbs commented at 10:13 pm on September 5, 2024: member
    @glozow copied some of the text to the OP
  104. DrahtBot added the label CI failed on Sep 6, 2024
  105. instagibbs commented at 1:41 pm on September 6, 2024: member
    CI failures seem like spurious timeouts of random tests
  106. instagibbs force-pushed on Sep 6, 2024
  107. instagibbs force-pushed on Sep 9, 2024
  108. instagibbs force-pushed on Sep 9, 2024
  109. instagibbs force-pushed on Sep 9, 2024
  110. DrahtBot removed the label CI failed on Sep 10, 2024
  111. petertodd commented at 1:15 pm on September 12, 2024: contributor

    In general. if we have 0-value dust, I find it unlikely to hope that miners will clean it up for no reason?

    Anyone can pay fees to clean up the dust. Doesn’t have to be miners specifically.

  112. in src/policy/ephemeral_policy.cpp:8 in 1332c4a077 outdated 
    0@@ -0,0 +1,110 @@
1+// Copyright (c) 2024-present The Bitcoin Core developers
2+// Distributed under the MIT software license, see the accompanying
3+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
4+
5+#include<policy/ephemeral_policy.h>
6+#include<policy/policy.h>
7+
8+bool CheckValidEphemeralTx(const CTransaction& tx, CFeeRate dust_relay_fee, CAmount txfee, TxValidationState& state)
    glozow commented at 8:24 pm on September 12, 2024:
    Any reason to use a CTransaction& instead of a CTransactionRef?
    instagibbs commented at 2:40 pm on September 13, 2024:
    nope, changed
  113. in src/policy/ephemeral_policy.h:15 in 1332c4a077 outdated 
    10+#include <primitives/transaction.h>
11+#include <txmempool.h>
12+
13+/** These utility functions ensure that ephemeral dust is safely
14+ * created and spent without risking them entering the utxo
15+ * set.
    glozow commented at 8:28 pm on September 12, 2024:
    I think this language is a little bit too strong. It’s not risk-free, I think we’re just checking that there is no visible incentive for the dust to enter the utxo set.
    instagibbs commented at 2:40 pm on September 13, 2024:
    added a hedge word
  114. in src/policy/ephemeral_policy.h:39 in 1332c4a077 outdated 
    34+ * bringing fees, or itself being spent by another child, and so on.
35+ */
36+
37+/** Does context-less checks about a single transaction.
38+ * If it has relay dust, it returns false if tx has non-0 fee
39+ * and sets relevant invalid state.
    glozow commented at 8:34 pm on September 12, 2024: 
    0 * Returns false if the fee is non-zero and dust exists, populating state. True otherwise.
    instagibbs commented at 2:40 pm on September 13, 2024:
    taken
  115. in src/policy/ephemeral_policy.h:34 in 1332c4a077 outdated 
    29+ * 1) None
30+ * 2) TxA+TxB+TxC
31+ * 3) TxA+TxC
32+ * By requiring the child transaction to sweep any dust from the parent txn, we ensure that
33+ * there is a single child only, and this child is the only transaction possible for
34+ * bringing fees, or itself being spent by another child, and so on.
    glozow commented at 8:36 pm on September 12, 2024:
    I don’t really understand the “or itself being spent by another child, and so on” part
    instagibbs commented at 2:40 pm on September 13, 2024:
    clarified comment, let me know if that’s more clear
  116. in src/policy/ephemeral_policy.cpp:13 in 1332c4a077 outdated 
    12+        for (const CTxOut& txout : tx.vout) {
13+            if (IsDust(txout, dust_relay_fee)) {
14+                return state.Invalid(TxValidationResult::TX_NOT_STANDARD, "dust", "tx with dust output must be 0-fee");
15+             }
16+        }
17+    }
    glozow commented at 8:37 pm on September 12, 2024:

    ?

    0    if (txfee != 0 && std::any_of(tx.vout.cbegin(), tx.vout.cbegin(), [&](const auto& output) { return IsDust(output, dust_relay_fee); })) {
1        return state.Invalid(TxValidationResult::TX_NOT_STANDARD, "dust", "tx with dust output must be 0-fee");
2    }
    instagibbs commented at 2:40 pm on September 13, 2024:
    taken
  117. in src/policy/ephemeral_policy.h:56 in 1332c4a077 outdated 
    48+std::optional<Txid> CheckEphemeralSpends(const Package& package, CFeeRate dust_relay_rate);
49+
50+/** Checks that individual transactions' parents have all their dust spent by this only-child transaction.
51+ *  The function returns std::nullopt if all dust is properly spent or an error message string.
52+ */
53+std::optional<std::string> CheckEphemeralSpends(const CTransactionRef& ptx,
    glozow commented at 8:39 pm on September 12, 2024:
    Can you add documentation for when each function must be called, like we did for TRUC?
    instagibbs commented at 2:39 pm on September 13, 2024:
    added a sentence to each saying when they should be called
  118. in test/functional/mempool_ephemeral_dust.py:117 in 1332c4a077 outdated 
    111+        assert_mempool_contents(self, self.nodes[0], expected=[dusty_tx["tx"], sweep_tx["tx"]])
112+
113+        # Node restart; doesn't allow allow ephemeral transaction back in due to individual submission
114+        # resulting in 0-fee
115+        self.restart_node(0)
116+        self.restart_node(1)
    glozow commented at 8:57 pm on September 12, 2024:
    No need to restart node1?
    instagibbs commented at 2:39 pm on September 13, 2024:
    would cause the nodes to fail to sync their mempools, no? I find it easiest when the restarts are done in sync anyways.
  119. in test/functional/mempool_ephemeral_dust.py:113 in 1332c4a077 outdated 
    108+        res = self.nodes[0].submitpackage([dusty_tx["hex"], sweep_tx["hex"]])
109+        assert_equal(res["package_msg"], "success")
110+        assert_equal(len(self.nodes[0].getrawmempool()), 2)
111+        assert_mempool_contents(self, self.nodes[0], expected=[dusty_tx["tx"], sweep_tx["tx"]])
112+
113+        # Node restart; doesn't allow allow ephemeral transaction back in due to individual submission
    glozow commented at 8:58 pm on September 12, 2024:
    This isn’t really the desirable behavior, perhaps worth commenting. If we fix this through something like #27476, this should be replaced with a test for the opposite case.
    instagibbs commented at 2:39 pm on September 13, 2024:
    Yes, if it’s fixed this will fail, letting the author know to write a new case. Added a note that this isn’t desired, just descriptive,
  120. in test/functional/mempool_ephemeral_dust.py:121 in 1332c4a077 outdated 
    116+        self.restart_node(1)
117+        self.connect_nodes(0, 1)
118+        assert_mempool_contents(self, self.nodes[0], expected=[])
119+
120+    def test_fee_having_parent(self):
121+        self.log.info("Test that a transaction with ephemeral dust may not have base fee, and ignores modified")
    glozow commented at 9:01 pm on September 12, 2024:
    nit: sentence structure kind of weird. the transaction ignores modified? I know what you mean, but maybe confusing wording
    instagibbs commented at 2:39 pm on September 13, 2024:
    dropped the reference to modified as it probably isn’t clarifying
  121. in test/functional/mempool_ephemeral_dust.py:128 in 1332c4a077 outdated 
    123+        assert_equal(self.nodes[0].getrawmempool(), [])
124+
125+        sats_fee = 1
126+        dusty_tx = self.wallet.create_self_transfer_multi(fee_per_output=sats_fee, version=3)
127+        self.add_output_to_create_multi_result(dusty_tx)
128+        assert_equal(dusty_tx["tx"].vout[0].nValue, 5000000000 - sats_fee) # main output is not dust
    glozow commented at 9:02 pm on September 12, 2024:

    A little bit brittle, as it assumes the utxo selected is from a coinbase.

    0        assert_equal(int(COIN * dusty_tx["fee"]), sats_fee) # main output is not dust
    instagibbs commented at 2:39 pm on September 13, 2024:
    added suggestion and a direct assertion that the main output amount is >330 the taproot dust amount
  122. in src/validation.cpp:940 in 1332c4a077 outdated 
    934@@ -934,6 +935,13 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
935                                     fSpendsCoinbase, nSigOpsCost, lock_points.value()));
936     ws.m_vsize = entry->GetTxSize();
937 
938+    // Enforces 0 base fee for dust transactions, no incentive to be mined alone
939+    if (m_pool.m_opts.require_standard) {
940+        if (!CheckValidEphemeralTx(tx, m_pool.m_opts.dust_relay_feerate, ws.m_base_fees, state)) {
    glozow commented at 9:07 pm on September 12, 2024:
    I can’t remember if we discussed this in the past… we definitely don’t want people to be able to bypass the rule using prioritisetransaction, but have you considered adding a CheckValidEphemeralTx(tx, m_pool.m_opts.dust_relay_feerate, ws.m_modified_fees, state) in addition to this check? Basically equivalent to “you can’t prioritisetransaction at all,” I think.
    instagibbs commented at 9:45 pm on September 12, 2024:

    yes, this was my open question #30239 (comment)

    Even if we do this change it can still be prioritized for mining after entering the mempool; we would need heavier machinery to stop ever treating it as prioritised. This would mean someone would have to create a CPFP transaction then cycle it away, while also using prioritisetransaction to get it mined without the dust spent? @sdaftuar you have any thoughts?

    sdaftuar commented at 0:07 am on September 13, 2024:

    I don’t have a strong view, but I’m trying to think about what a mining pool might want the behavior to be. One guess is that some miners may want to have a way to prioritise normal-looking transactions but not inadvertently prioritise something that would create a dust output and spam the UTXO set with garbage that will never be spent.

    If that is the case, then we could try to build the heavier machinery you’re talking about: perhaps change the behavior of prioritisetransaction to produce an error by default when invoked on an in-mempool transaction that has a dust output, coupled with what I think you guys are talking about here of rejecting transactions that are prioritised already on entry. But probably someone should talk to mining pools that offer transaction acceleration to see what they would intend and if what I just described is worth implementing; if they don’t care about UTXO bloat then it’s not hard to bypass these rules regardless of what we do here.

    petertodd commented at 8:01 am on September 13, 2024:

    @sdaftuar

    Re: dust UTXO creation, MARA has told me directly that they do not enforce the dust limit at all, and IIUC F2Pool does not as well. I haven’t however personally tested this.

    instagibbs commented at 3:55 pm on September 13, 2024:

    @petertodd I know for their slipstream accelerator at a minimum they do not, and perhaps they updated all their nodes to not enforce, I’m not sure. @sdaftuar Ok that’s not much machinery, and seems to make sense combined with base and modified fee both being checked.

    I think any node runner who wants to ignore dust rules they’ll simply set -dustrelayfee=0, which works out of the box as expected in this PR, and even if we’re more controlling on when prioritisetransaction can be called.

    I pushed a commit which makes the changes and can rework history if people think the changes are agreeable. Miners can evade this by simply turning dust levels to 0, like they could before.

    petertodd commented at 9:47 am on September 14, 2024:
    @instagibbs What MARA told me was that their nodes do not enforce the dust limit. Notably, that means if Libre Relay or some other implementation removed the dust limit, transactions violating it would get to MARA and be mined.
  123. in test/functional/mempool_ephemeral_dust.py:207 in 1332c4a077 outdated 
    198+
199+        self.generate(self.nodes[0], 1)
200+        assert_mempool_contents(self, self.nodes[0], expected=[])
201+
202+    def test_non_truc(self):
203+        self.log.info("Test that v2 dust-having transaction is rejected even if spent, because of min relay requirement")
    glozow commented at 9:11 pm on September 12, 2024:
    Could add a note that this is basically checking that v2 is subject to min relay feerate requirements, and you can flip this test if that changes
    instagibbs commented at 2:39 pm on September 13, 2024:
    dropped a note
  124. in test/functional/mempool_ephemeral_dust.py:412 in 1332c4a077 outdated 
    407+
408+        self.restart_node(0, extra_args=[])
409+        self.restart_node(1, extra_args=[])
410+        self.connect_nodes(0, 1)
411+
412+        assert_equal(self.nodes[0].getrawmempool(), [])
    glozow commented at 9:25 pm on September 12, 2024:
    Thoughts on adding this test before the changes in commit 1?
    instagibbs commented at 2:39 pm on September 13, 2024:
    good idea, moved this subtest to mempool_dust.py as its own commit, switched it to testing dustrelay arg instead of standardness as I think that’s more directly testing what we want.
  125. in test/functional/mempool_ephemeral_dust.py:447 in 1332c4a077 outdated 
    442+            dusty_txs.append(self.wallet.create_self_transfer_multi(fee_per_output=0, version=2))
443+            self.add_output_to_create_multi_result(dusty_txs[-1])
444+
445+        all_parent_utxos = [utxo for tx in dusty_txs for utxo in tx["new_utxos"]]
446+
447+        # Missing one dust spend spend from a single parent, child rejected
    glozow commented at 9:29 pm on September 12, 2024:
    spend spend
    instagibbs commented at 2:39 pm on September 13, 2024:
    fixed
  126. glozow commented at 9:34 pm on September 12, 2024: member
    Finer comb of the first 2 commits
  127. functional test: Add new -dustrelayfee=0 test case 
    This test would catch regressions where ephemeral
dust checks are being erroneously applied on outputs
that are not actually dust.
    fd22f9e2f4
  128. policy: Allow dust in transactions, spent in-mempool 
    Also known as Ephemeral Dust.

We try to ensure that dust is spent in blocks by requiring:
  - ephemeral dust tx is 0-fee
child bringing fees)
  - ephemeral dust tx only has one dust output
  - the output is spent by a single child transaction
    or the transaction has no child

0-fee requirement means there is no incentive to mine
a transaction which doesn't have a child bringing its
own fees for the transaction package.
    480374ae94
  129. functional test: Add ephemeral dust tests b14de3488c
  130. test: Add CheckMempoolEphemeralInvariants e1592d7ffe
  131. fuzz: add ephemeral_package_eval harness 81066562a9
  132. test: unit test for CheckEphemeralSpends 1193751a2e
  133. Add release note for ephemeral dust e2eecf65ba
  134. instagibbs force-pushed on Sep 13, 2024
  135. policy, rpc: Disallow non-0 mod fees 
    Reject ephemeral dust entry into mempool
if base or modified fees are non-0.

prioritisetransaction throws an error
if called on a transaction in its mempool
that has dust.

This has the effect that unspent dust should
never be mined unless minrelay et al are changed
to 0 fee, or the operator simply redefines that dust
values using `-dustrelayfee` to make dust threshholds
lower.
    289811ac08


instagibbs DrahtBot theStack glozow petertodd t-bast Mahmoud198425 achow101 sdaftuar

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-09-19 19:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me