TODO:
I’ll be updating to segwit variant soon, with updated BIP text.
Builds on top of #25038 for consideration of inclusion to the proposal. Requires V3, for simplicity of reasoning and usage. Implementation of idea written out at https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2022-October/021036.html
BIP text here: https://github.com/instagibbs/bips/blob/ephemeral_anchor/bip-ephemeralanchors.mediawiki
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
For detailed information about the code coverage, see the test coverage report.
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| Concept NACK | luke-jr |
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
Reviewers, this pull request conflicts with the following ones:
DEFAULT_PERMIT_BAREMULTISIG to false by Retropex)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.
OP_TRUE does not have a weight that reflects this high cost.
@luke-jr Can you elaborate more on what the potential issues are?
This policy defines that any transaction with such an output must be 0-fee, which means there’s little incentive to mine it on its own, but only as part of a CPFP spend of that output.
Assuming the PR is correct and being run on the network, there should be no way(aside from miners prioritizing the parent explicitly) to leaving the utxo in the uxto set. Prioritizing the parent could also be disallowed easily.
The main issue I see is that it has a very low weight, but rather high cost. There is no consensus rule requiring it to be spent immediately.
Couldn’t it be CPFP’d by spending another of its outputs?
@luke-jr either you do a consensus fork and everything that entails for something like tx sponsors, or your enable in policy only something like ephemeral anchors.
One option that could be taken would be to make the OP_TRUE script contain an upgradable NOP that we document as reserved to in the future treat as a pre-execution parsed flag with a consensus must-spend rule, and then we’d be able to (if there was wide adoption of ephemeral anchors and we saw utxo bloat) add a consensus rule to enforce them as must-spend.
One option that could be taken would be to make the OP_TRUE script contain an upgradable NOP that we document as reserved to in the future treat as a pre-execution parsed flag with a consensus must-spend rule, and then we’d be able to (if there was wide adoption of ephemeral anchors and we saw utxo bloat) add a consensus rule to enforce them as must-spend.
I think this would be better, since it allows a real OP_TRUE to be given a more cost-based weight in a future softfork.
more or less, you might also be able to do a min byte segwit v1 script (2 bytes?) or something else similar too.
Otherwise you end up with some “specific script template matching upgrade”, which is IMO messy & risks being uneccessarily confiscatory.
Camping an OP_NOP and preventing it from being used for any other future soft fork seems worse than camping OP_TRUE.
If in the future we have a soft fork that makes small scriptPubKeys more costly to limit utxo bloat with an exemption for ephmeral outputs that are spent in the same block, which should just apply both rules to all scriptPubKeys, whether they’re OP_TRUE, OP_2, or OP_NOP5 OP_TRUE…
What will happen at the p2p level, if the attacker manages to submit a transaction with ephemeral outputs to many nodes but without a child?
It looks like its wtxid will make it into m_recent_rejects, and then the network won’t reconsider the tx even when the child is attached?
146+ package_hex4, package_txns4 = self.create_simple_package(parent_coins=parent_coins, parent_fee=0, child_fee=DEFAULT_FEE*2, version=3, spend_anchors=2)
147+ assert_raises_rpc_error(-26, "v3-tx-nonstandard", node.submitpackage, package_hex4)
148+ self.assert_mempool_contents(expected=package_txns3, unexpected=[])
149+
150+ # All ephemeral anchors this time, make sure old child evicted
151+ package_hex5, package_txns5 = self.create_simple_package(parent_coins=parent_coins, parent_fee=0, child_fee=DEFAULT_FEE*2, version=3)
330@@ -331,11 +331,12 @@ def test_xor_rbf(self):
331 self.generate(node, 1)
332 self.assert_mempool_contents(expected=[second_package_txns[1]], unexpected=[])
333
334- # Parent can still be prioritised
335+ # Parent can't be prioritised; RPC call succeeds but is modified fees are ignored if it has EAs
406@@ -402,6 +407,11 @@ void CTxMemPoolEntry::UpdateDescendantState(int64_t modifySize, CAmount modifyFe
407
408 void CTxMemPoolEntry::UpdateAncestorState(int64_t modifySize, CAmount modifyFee, int64_t modifyCount, int64_t modifySigOps)
409 {
410+ // We don't want to encourage miners to mine these transacitons alone. Instead
Rebased on top of latest #25038
141@@ -142,8 +142,12 @@ bool IsStandardTx(const CTransaction& tx, const std::optional<unsigned>& max_dat
142 reason = "bare-multisig";
143 return false;
144 } else if (IsDust(txout, dust_relay_fee)) {
145- reason = "dust";
146- return false;
147+ // We ensure ephemeral output is spent in the same mempool package
Should a flag be threaded through to disallow any dust outputs even if they’re EA?
https://github.com/bitcoin/bitcoin/pull/26403/commits/a1982f5773690bc0c74d1bb932cb7552b00aacaf shows that current re-submission logic on node restart fails to evict ephemeral transaction.
Until there is package-aware re-submission, should these transactions just be dropped on the floor? These types of transactions are opting into fairly aggressive fee bumping patterns, maybe that’s an ok first cut.
m_recent_rejects_reconsiderable filter instead of m_recent_rejects
135 }
136
137+ /* Only allow in package feerate context */
138+ if (!package_context) {
139+ /* Allows re-evaluation in package context */
140+ return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "missing-ephemeral-spends");
should return code like TX_LOW_FEE re:BIP331 to avoid blinding relay of an otherwise valid parent
edit: does this actually matter? in the individual context we never want to reset, really. We only want to try again, per block period, if it’s sent in a package with a new combined hash?
Random thought:
OP_TRUE is txid-malleable by block maker. e.g., they could theoretically add in superfluous pushes in the scriptSig and this would not fail validation because CLEANSTACK is a policy-only rule. Miner is of course taking less in fees by doing this.
In this particular V3 context, it just means your CPFP could change txid, wouldn’t do anything else. Maybe in a more general context if we weren’t restricted to V3 topology this would be an issue? In the context of keyless anchors, I’m not sure that will ever make sense anyways.
The “fix” would be: OP_DEPTH OP_0 OP_EQUAL, making it consensus-non-malleable at the cost of 8 additional WU. Seems like a bad tradeoff, noting anyways.
0<_aj_> instagibbs: can't a blockmaker put "OP_NOP OP_NOP OP_NOP" in the scriptSig and also satisfy a scriptPubKey of "DEPTH 0 EQUAL" ?
legacy script: i cri everytim
This also means the child transaction spending the ephemeral anchor cannot be relied upon for further transaction chaining, e.g., spent in a splice-in for a LN channel, since the funding output can have its txid mutated. A soft-fork would be necessary.
@instagibbs pushed 1 commit. faeed68 nice crash bro
hacked a quick-ish fix on top of this branch here (it fails some other tests). As discussed offline, the general problem is with calling TrimToSize in the middle of package validation (meaning something can go from already-in-mempool to evicted, or just-submitted to evicted) so a coin may be cached in m_view but then it disappears from the mempool/temporary coins in m_viewmempool. Crash is in CheckInputsFromMempoolAndCache which asserts the coin exists, but it doesn’t anymore. Will open a separate PR to address this since it’s also a problem pre-v3.
Avoid calling PackageMempoolChecks() when there is only 1 transaction.
Note to reviewers: there is a slight change in the error type returned,
as shown in the txpackage_tests change. When a transaction is the last
one left in the package and its fee is too low, this returns a PCKG_TX
instead of PCKG_POLICY. This interface is clearer;
"package-fee-too-low" for 1 transaction would be a bit misleading.
After the PackageMempoolAcceptResult is returned from
AcceptMultipleTransactions, leave room for results to change due to
LimitMempool() eviction.
The behavior is not new, but this rule exits earlier than before.
Previously, a carve out could have been granted in PreChecks() but then
nullified in PackageMempoolChecks() when CheckPackageLimits() is called
with the default limits.
At this point it's not expected that there are any such transactions,
except from reorgs and possibly when loading from mempool.dat.
As long as they are otherwise paid for, i.e. through package CPFP.
If a v3 transaction loses its sponsor, we can evict them with no trouble
because it will not have other descendants or ancestors to make the
feerate assessment more complicated.
No change in behavior.
For single transaction acceptance, this is a simple refactor:
Workspace::m_all_conflicting -> MemPoolAccept::m_all_conflicts
Workspace::m_replacement_transaction -> MemPoolAccept::m_rbf
Workspace::m_conflicting_fees -> MemPoolAccept::m_conflicting_fees
Workspace::m_conflicting_size -> MemPoolAccept::m_conflicting_size
Workspace::m_replaced_transactions -> MemPoolAccept::m_replaced_transactions
And local variables m_total_vsize and m_total_modified_fees are now
MemPoolAccept members so they can be accessed from PackageMempoolChecks.
We want these to be package-wide variables because
- Transactions could conflict with the same tx (just not the same
prevout), or their conflicts could share descendants.
- We want to compare conflicts with the package fee rather than
individual transaction fee.
Avoid accepting replacements that are not more incentive compatible to
mine. For now, as a conservative estimate, require that the minimum
between the transaction's individual feerate and ancestor feerate is
higher than the individual feerates of directly conflicting transactions
and the ancestor feerates of all original transactions.
Note that a package/transaction's ancestor feerate is not perfectly
representative of its incentive compatibility; it may overestimate (some
subset of the ancestors could be included by itself if it has other
high-feerate descendants or are themselves higher feerate than this
package/transaction). This is a conservative estimate and works for now.
Co-authored-by: Suhas Daftuar <sdaftuar@chaincode.com>
Try to prevent accidental future breakage and document some assumptions
that would break package RBF if changed.
This will be used in following commits
as the flag for ephemeral anchors, allowing
dust values, but also requiring
the output to be spend in the same relay package.
Anchor outputs are not yet required
to be spent in same mempool package.
Does not have test coverage for making sure EA are spent,
and package RBF cases, neither of which exist currently.
Either in the same relay package, or by transactions RBFing
the CPFP.
🐙 This pull request conflicts with the target branch and needs rebase.
Thus, the party that had broadcast the original child transaction will be forced to construct a new spending transaction with a different TxID anyway
Canonical example here which motivated the change was that of LN channel splicing being used to cpfp bump another transaction, would could be another channel’s commitment transaction itself. This requires the ephemeral anchor-spending tx(the splice) to retain txid stability otherwise funds are locked up in the new funding output.
I understand that not all use-cases require such things, but two points:
This requires the ephemeral anchor-spending tx(the splice) to retain txid stability otherwise funds are locked up in the new funding output.
The point I was missing is that a non-malleable anchor ensures that the other output(s) do not get spent in a transaction whose TxID was not known in advance of signing. Thank you for the answer.