Cluster size 2 package rbf #28984

instagibbs · 2023-12-01T22:12:06Z

based on #30072 , please review that one first

Allows any 2 transaction package with no in-mempool ancestors to do package RBF when directly conflicting with other mempool clusters of size two or less.

Proposed validation steps:

If the transaction package is of size 1, legacy rbf rules apply.
Otherwise the transaction package consists of a (parent, child) pair with no other in-mempool ancestors (or descendants, obviously), so it is also going to create a cluster of size 2. If larger, fail.
The package rbf may not evict more than 100 transactions from the mempool(bip125 rule 5)
The package is a single chunk
Every directly conflicted mempool transaction is connected to at most 1 other in-mempool transaction (ie the cluster size of the conflict is at most 2).
Diagram check: We ensure that the replacement is strictly superior, improving the mempool
The total fee of the package, minus the total fee of what is being evicted, is at least the minrelayfee * size of the package (equivalent to bip125 rule 3 and 4)

Post-cluster mempool this will likely be expanded to general package rbf, but this is what we can safely support today.

DrahtBot · 2023-12-01T22:12:09Z

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 ACK	murchandamus, glozow
Stale ACK	ismaelsadeeq

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:

#29680 (wallet: fix unrelated parent conflict doesn't cause child tx to be marked as conflict by Eunovo)
#29641 (scripted-diff: Use LogInfo/LogDebug over LogPrintf/LogPrint by maflcko)
#29086 (refactor: Simply include CTxMemPool::Options in CTxMemPool directly rather than duplicating definition by luke-jr)
#27432 (contrib: add tool to convert compact-serialized UTXO set to SQLite database by theStack)
#26593 (tracing: Only prepare tracepoint arguments when actually tracing by 0xB10C)

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.

glozow · 2023-12-04T04:54:20Z

Added this to the tracking issue

instagibbs · 2023-12-04T19:26:16Z

src/policy/rbf.cpp

+        const CFeeRate replacement_miner_score(replacement_fees, replacement_vsize);
+
+        for (const auto& entry : direct_conflicts) {
+            const bool conflict_is_v3{entry->GetSharedTx()->nVersion == 3};


this can't be hit yet; post-v3 it can. I can remove this, or leave a note, or both. It makes some package rbfs more useful

(edit: it's used in #29001 functional tests )

src/test/rbf_tests.cpp

sdaftuar · 2023-12-08T16:12:07Z

So thinking conceptually about this, I'm most concerned by the new CheckMinerScores() criterion, where we only check the replacement package's feerate against the ancestor feerates of the indirect conflicts.

I think there are two issues with this approach:

If you look at GetModFeesWithAncestors()/GetSizeWithAncestors(), that can be greater than the transaction's individual feerate (ie if the parent of a transaction has a higher feerate than the child you're conflicting with). So this can cause the test to be too conservative and introduce some kind of weird pinning. Granted, this is a new replacement functionality so maybe not the end of the world if it is sometimes too conservative, but it seems like a bug.
Also if you look at the ancestor feerate of a transaction, that can underestimate its mining score, such as if some low feerate ancestor is being paid for by another transaction. So that can allow for replacements that are not incentive compatible (similar to how this is possible today with our current single-transaction RBF rules).

Even though the current RBF logic is broken already, I'd prefer to avoid adding on to that while rolling out the package RBF implementation, just so that users don't get used to some kinds of replacements working that really shouldn't be.

So I was wondering, can we deal with this by just restricting the topology of what we allow a package RBF to conflict with? Let's say we allow a replacement if the conflict set consists of either (1) a single transaction with no ancestors (and of course no descendants, since any descendants would be in the conflict set as well), or (2) two transactions that are in a parent-child topology, and that have no other in-mempool ancestors (or descendants, of course).

In those situations, the maximum miner score you would have to consider is either the individual feerate of the parent tx or the ancestor feerate of the child, so the new logic should work perfectly.

instagibbs · 2023-12-08T21:48:57Z

After offline discussion with @sdaftuar , I've worked on a set of prospective changes and pushed them to this branch as follow-on commits.

Key change is that we will now only allow package RBF when the conflicted transactions are all in "clusters" of up to size 2. This allows us to calculate mining scores for each conflicted transaction, which means that post-cluster mempool, if we did this right, IIUC, the behavior should match with more general package RBF.

glozow · 2024-05-09T09:00:37Z

src/validation.cpp

+
+    // We're in package RBF context; replacement proposal must be size 2
+    if (workspaces.size() != 2) {
+        return package_state.Invalid(PackageValidationResult::PCKG_POLICY, "package RBF failed: replacing cluster not size two");


yeah, I think PCKG_POLICY is fine given 6119f76. Just need to make sure the MempoolAcceptResults are correct so that we are attributing the failures correctly

glozow · 2024-05-09T13:43:53Z

src/validation.cpp

+    // Ensure this two transaction package is a "chunk" on its own; we don't want the child
+    // to be only paying anti-DoS fees
+    if (CFeeRate(parent_ws.m_modified_fees, parent_ws.m_vsize) >=
+            CFeeRate(parent_ws.m_modified_fees + child_ws.m_modified_fees, parent_ws.m_vsize + child_ws.m_vsize)) {
+        return package_state.Invalid(PackageValidationResult::PCKG_POLICY,
+                                     "package RBF failed: parent paying for child anti-DoS", "");
+    }


in b86487c

The comments + error string are a bit contradictory ("child only paying anti-DoS" and "parent paying for child anti-DoS") unless maybe I'm misunderstanding?

Also maybe error str "package RBF failed: parent paying for child replacement" and debug str "package feerate is X while parent feerate is Y" ?

glozow · 2024-05-09T13:49:33Z

src/validation.cpp

+    // Check if it's economically rational to mine this package rather than the ones it replaces.
+    if (const auto err_tup{ImprovesFeerateDiagram(m_pool, direct_conflict_iters, m_all_conflicts, m_total_modified_fees, m_total_vsize)}) {
+        return package_state.Invalid(PackageValidationResult::PCKG_POLICY,
+                                     "package RBF failed: " + err_tup.value().second, "");
+    }


b86487c

I wonder if PaysForRBF and ImprovesFeerateDiagram should be swapped in the order of checks?

ImprovesFeerateDiagram is a more expensive, is a superset of the rules, and a less intuitive error (I'd have an easier time guessing what went wrong with "pays less fees" vs "does not improve feerate diagram". Especially looking at the functional tests which switched out the error strings due to this check being earlier.

is a superset of the rules

PaysForRBF has incremental rate on top, so ImprovesFeerateDiagram is not a strict superset fwiw.

That said, I don't think swapping the order is a problem and would indeed be cheaper.

True, it's only a superset of Rule 3 👍

swapped them and updated tests accordingly

glozow · 2024-05-09T13:52:51Z

src/validation.cpp

+    // since it would result in a cluster larger than 2
+    for (const auto& ws : workspaces) {
+        if (!ws.m_ancestors.empty()) {
+            return package_state.Invalid(PackageValidationResult::PCKG_POLICY, "package RBF failed: replacing cluster with ancestors not size two");


Suggested change

return package_state.Invalid(PackageValidationResult::PCKG_POLICY, "package RBF failed: replacing cluster with ancestors not size two");

return package_state.Invalid(PackageValidationResult::PCKG_POLICY, "package RBF failed: new transaction cannot have mempool ancestors");

glozow · 2024-05-09T13:54:56Z

src/validation.cpp

+                                     "package RBF failed: parent paying for child anti-DoS", "");
+    }
+
+    // Check if it's economically rational to mine this package rather than the ones it replaces.


Maybe you can mention that this replaces PaysMoreForConflicts function from ReplacementChecks?

glozow · 2024-05-09T13:57:39Z

src/validation.cpp

+
+    // We're in package RBF context; replacement proposal must be size 2
+    if (workspaces.size() != 2 || !Assume(IsChildWithParents(txns))) {
+        return package_state.Invalid(PackageValidationResult::PCKG_POLICY, "package RBF failed: replacing cluster not size two");


I don't think it's very clear which cluster isn't size two in this str

Suggested change

return package_state.Invalid(PackageValidationResult::PCKG_POLICY, "package RBF failed: replacing cluster not size two");

return package_state.Invalid(PackageValidationResult::PCKG_POLICY, "package RBF failed: package must be 1-parent-1-child");

glozow · 2024-05-09T14:01:06Z

doc/policy/packages.md

-   - Package RBF may be enabled in the future.
+   - More general package RBF may be enabled in the future.


Could add more detailed docs for what the rules are in package RBF?

all to-be-replaced signal (bip125 or v3)

package is 1p1c, only in clusters up to size 2

to-be-replaced all in clusters up to size 2

no more than 100 total to-be-replaced

total fees of package > all fees being replaced, at incremental relay feerate

must improve feerate diagram

parent feerate must be < package feerate

@glozow pushed some more detailed docs, thanks!

instagibbs · 2024-05-09T18:49:19Z

@glozow thanks for the review, all comments addressed

DrahtBot · 2024-05-12T12:10:30Z

🚧 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/24790654138}

instagibbs · 2024-05-12T12:30:30Z

silent merge conflict with #29939

glozow

some test review

glozow · 2024-05-10T14:32:51Z

test/functional/mempool_package_rbf.py

+        package_child = self.wallet.create_self_transfer(fee_rate=1000*DEFAULT_FEE, utxo_to_spend=package_parent["new_utxos"][0])
+
+        pkg_results = node.submitpackage([package_parent["hex"], package_child["hex"]])
+        assert_equal(f"package RBF failed: too many potential replacements, rejecting replacement {package_parent['tx'].rehash()}; too many potential replacements (102 > 100)\n", pkg_results["package_msg"])
+        self.assert_mempool_contents(expected=expected_txns)
+
+        # Make singleton tx to conflict with in next batch
+        singleton_coin = self.coins[-1]
+        del self.coins[-1]
+        singleton_tx = self.wallet.create_self_transfer(utxo_to_spend=singleton_coin)
+        node.sendrawtransaction(singleton_tx["hex"])
+        expected_txns.append(singleton_tx["tx"])
+
+        # Double-spend same set minus last, and double-spend singleton. This hits 101 evictions; should still fail.
+        # N.B. we can't RBF just a child tx in the clusters, as that would make resulting cluster of size 3.
+        double_spending_coins = parent_coins[:-1] + [singleton_coin]
+        package_parent = self.wallet.create_self_transfer_multi(utxos_to_spend=double_spending_coins, fee_per_output=10000)
+        package_child = self.wallet.create_self_transfer(fee_rate=1000*DEFAULT_FEE, utxo_to_spend=package_parent["new_utxos"][0])
+        pkg_results = node.submitpackage([package_parent["hex"], package_child["hex"]])
+        assert_equal(f"package RBF failed: too many potential replacements, rejecting replacement {package_parent['tx'].rehash()}; too many potential replacements (101 > 100)\n", pkg_results["package_msg"])
+        self.assert_mempool_contents(expected=expected_txns)
+
+        # Finally, evict MAX_REPLACEMENT_CANDIDATES
+        package_parent = self.wallet.create_self_transfer_multi(utxos_to_spend=parent_coins[:-1], fee_per_output=10000)
+        package_child = self.wallet.create_self_transfer(fee_rate=10000*DEFAULT_FEE, utxo_to_spend=package_parent["new_utxos"][0])
+        pkg_results = node.submitpackage([package_parent["hex"], package_child["hex"]], maxfeerate=0)


nit: you change the fee in the last successful one, which is a little bit sus since fee shouldn't matter in this test

Maybe create 2 constants for the parent fee_per_output and child fee/feerate, and use them for each round?

I turned off maxfeerate, jacked up the child feerates, and made constants for both

glozow · 2024-05-10T14:38:31Z

test/functional/mempool_package_rbf.py

+        node.submitpackage(package_hex1)
+        self.assert_mempool_contents(expected=package_txns1)
+        # The first two transactions have the same conflicts
+        package_duplicate_conflicts_hex = [package_hex2[0]] + package_hex3


I don't really understand what's being tested here - this doesn't have anything to do with package RBF? This isn't a topologically valid package since the transactions aren't related to each other in addition to being double spends. #30066 seems closer to what this is going for?

Agreed, it's perhaps an older badly done version of #30066, removed

glozow · 2024-05-10T14:41:34Z

test/functional/mempool_package_rbf.py

+            fee_rate=0,
+            fee=DEFAULT_FEE,


These 2 things are contradictory. AFAICT fee_rate is ignored, delete?

removed a series of these ignored args

glozow · 2024-05-10T14:54:06Z

test/functional/mempool_package_rbf.py

+        self.assert_mempool_contents(expected=expected_txns)
+
+        # Now make conflicting packages for each coin
+        package_hex1, package_txns1 = self.create_simple_package(coin1, DEFAULT_FEE, DEFAULT_FEE * 4)


nit: more varied fees for the packages for the wrong_conflict_cluster_size_* tests, could also use constant

picked a constant and used it

glozow · 2024-05-13T09:12:29Z

test/functional/mempool_package_rbf.py

+    assert_greater_than_or_equal,
+    assert_raises_rpc_error,
+    assert_equal,
+    fill_mempool,


This is now in mempool_util (= linter complaint)

glozow · 2024-05-13T09:24:52Z

src/validation.cpp

+    const auto& child_ws = workspaces[1];
+
+    // Use the child as the transaction for attributing errors to.
+    const Txid child_hash = child_ws.m_ptx->GetHash();


nit; make reference not copy

Suggested change

const Txid child_hash = child_ws.m_ptx->GetHash();

const Txid& child_hash = child_ws.m_ptx->GetHash();

glozow · 2024-05-13T09:35:32Z

src/validation.cpp

+                                     "package RBF failed: parent paying for child replacement",
+                                     strprintf("package feerate is %s while parent feerate is %s", package_feerate.ToString(), parent_feerate.ToString()));


Sorry to nit my own previous suggestion, but I think "parent paying for child replacement" might still be too specific, since it could be that the child has nothing to replace but is lower feerate (like package_txns6 in the functional test).

Maybe

Suggested change

"package RBF failed: parent paying for child replacement",

strprintf("package feerate is %s while parent feerate is %s", package_feerate.ToString(), parent_feerate.ToString()));

"package RBF failed: package feerate is less than parent feerate",

strprintf("package feerate %s <= parent feerate %s", package_feerate.ToString(), parent_feerate.ToString()));

glozow · 2024-05-13T10:01:00Z

test/functional/mempool_package_rbf.py

+        package_hex5, package_txns5 = self.create_simple_package(coin, parent_fee=DEFAULT_FEE, child_fee=DEFAULT_FEE)
+        node.submitpackage(package_hex5)
+        self.assert_mempool_contents(expected=package_txns5)
+        package_hex6, package_txns6 = self.create_simple_package(coin, parent_fee=DEFAULT_FEE + fee_delta, child_fee=DEFAULT_FEE - (fee_delta / 2))


Given that fee_delta is a multiple of DEFAULT_FEE and you're using fee_delta / 2 here... I think the general readability of this test would be improved if you created a constant base unit FEE which was equal to, say, DEFAULT_FEE / 10 (or 104 * 10 which would give most of the transactions whole number feerates), and then used FEE, 2*FEE, 8*FEE, etc. in all the tests.

I created DEFAULT_CHILD_FEE and used that pretty much everywhere it made sense

glozow · 2024-05-13T10:29:49Z

test/functional/mempool_package_rbf.py

+        self.generate(node, 1)
+
+    def test_child_conflicts_parent_mempool_ancestor(self):
+        fill_mempool(self, self.nodes[0], self.wallet)


with rebase

Suggested change

fill_mempool(self, self.nodes[0], self.wallet)

fill_mempool(self, self.nodes[0])

glozow · 2024-05-13T10:31:47Z

doc/policy/packages.md

+
+   - *Rationale*: Basic support for package RBF can be used by wallets
+     by making chains of no longer than two, then directly conflicting
+     those chains when needed. Combined with V3 transactions this can


Maybe there should be some v3 test coverage (including nice 0fee parent package RBF)?

diff --git a/test/functional/mempool_accept_v3.py b/test/functional/mempool_accept_v3.py index 8285b82c19..990d36359e 100755 --- a/test/functional/mempool_accept_v3.py +++ b/test/functional/mempool_accept_v3.py @@ -579,6 +579,32 @@ class MempoolAcceptV3(BitcoinTestFramework): ) self.check_mempool([tx_with_multi_children["txid"], tx_with_sibling3_rbf["txid"], tx_with_sibling2["txid"]]) + @cleanup(extra_args=["-acceptnonstdtxn=1"]) + def test_package_rbf(self): + self.log.info("Test package RBF: v3 0-fee parent + high-fee child replaces parent's conflicts") + node = self.nodes[0] + # Reuse the same coins so that the transactions conflict with one another. + parent_coin = self.wallet.get_utxo(confirmed_only=True) + + # package1 pays default fee on both transactions + parent1 = self.wallet.create_self_transfer(utxo_to_spend=parent_coin, version=3) + child1 = self.wallet.create_self_transfer(utxo_to_spend=parent1["new_utxo"], version=3) + package_hex1 = [parent1["hex"], child1["hex"]] + fees_package1 = parent1["fee"] + child1["fee"] + submitres1 = node.submitpackage(package_hex1) + assert_equal(submitres1["package_msg"], "success") + self.check_mempool([parent1["txid"], child1["txid"]]) + + # package2 has a 0-fee parent (conflicting with package1) and very high fee child + parent2 = self.wallet.create_self_transfer(utxo_to_spend=parent_coin, fee=0, fee_rate=0, version=3) + child2 = self.wallet.create_self_transfer(utxo_to_spend=parent2["new_utxo"], fee=fees_package1*10, version=3) + package_hex2 = [parent2["hex"], child2["hex"]] + + submitres2 = node.submitpackage(package_hex2) + assert_equal(submitres2["package_msg"], "success") + assert_equal(set(submitres2["replaced-transactions"]), set([parent1["txid"], child1["txid"]])) + self.check_mempool([parent2["txid"], child2["txid"]]) + def run_test(self): self.log.info("Generate blocks to create UTXOs") @@ -598,6 +624,7 @@ class MempoolAcceptV3(BitcoinTestFramework): self.test_reorg_2child_rbf() self.test_v3_sibling_eviction() self.test_reorg_sibling_eviction_1p2c() + self.test_package_rbf() if __name__ == "__main__":

was wondering about this case, added with some modifications for test simplicity

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.

…rbf later

No change in behavior. For single transaction acceptance, this is a simple refactor: Workspace::m_all_conflicting -> MemPoolAccept::m_all_conflicts 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. We reset these MemPoolAccept-wide fields for each subpackage evaluation to not cause state leaking, similar to temporary coins.

Relax assumptions aabout in-mempool children of in-mempool parents. With package RBF, we will allow a package of size 2 with conflicts on its parent and reconsider the parent if its fee is insufficient on its own. Consider: TxA (in mempool) <- TxB (in mempool) TxA (in mempool) <- TxB' (in package, conflicts with TxB) <- TxD (in package) If TxB' fails to RBF TxB due to insufficient feerate, the package TxB' + TxC will be considered. PackageV3Checks called on TxB' will see an in-mempool parent TxA, and see the in-mempool child TxB. We cannot assume there is no in-mempool sibling, rather detect it and fail normally. Prior to package RBF, this would have failed on the first conflict in package.

Support package RBF where the conflicting package would result in a mempool cluster of size two, and each of its direct conflicts are also part of an up-to-size-2 mempool cluster. This restricted topology allows for exact calculation of miner scores for each side of the equation, reducing the surface area for new pins, or incentive-incompatible replacements. This allows wallets to create simple CPFP packages that can fee bump other simple CPFP packages. This, leveraged with other restrictions such as V3 transactions, can create pin-resistant applications. Future package RBF relaxations can be considered when appropriate. Co-authored-by: glozow <gloriajzhao@gmail.com> Co-authored-by: Greg Sanders <gsanders87@gmail.com>

instagibbs · 2024-05-15T12:13:27Z

rebased due to conflict

t-bast · 2024-05-15T13:47:13Z

While I wouldn't trust myself to correctly code-review this PR, I'd be happy to work on e2e tests that would leverage this for lightning channels fee-bumping (based on eclair) if it can help validate the logic and get this PR merged.

I'd like to highlight again how important this feature is for lightning (and probably for many other L2 protocols on top of bitcoin today). This is the critical step that allows us to mitigate pinning of a commitment transaction, and guarantee that we're able to set the fees of our commitment package to a value that should ensure timely confirmation.

instagibbs · 2024-05-16T15:14:20Z

sorry forgot to link #30072 here, added to OP

instagibbs force-pushed the 2023-12-cluster-size2-package-rbf branch from 38ce2cb to 5b216db Compare December 1, 2023 22:13

DrahtBot added the CI failed label Dec 2, 2023

glozow added the TX fees and policy label Dec 4, 2023

glozow mentioned this pull request Dec 4, 2023

Package Relay Project Tracking #27463

Open

53 tasks

instagibbs force-pushed the 2023-12-cluster-size2-package-rbf branch 3 times, most recently from 57e5fc7 to 56848f9 Compare December 4, 2023 18:54

instagibbs marked this pull request as ready for review December 4, 2023 18:55

instagibbs force-pushed the 2023-12-cluster-size2-package-rbf branch from 56848f9 to be9d163 Compare December 4, 2023 19:14

instagibbs commented Dec 4, 2023

View reviewed changes

DrahtBot removed the CI failed label Dec 4, 2023

instagibbs commented Dec 5, 2023

View reviewed changes

src/test/rbf_tests.cpp Outdated Show resolved Hide resolved

instagibbs mentioned this pull request Dec 5, 2023

Ephemeral Anchors #29001

Draft

DrahtBot mentioned this pull request Dec 7, 2023

[WIP] Cluster mempool implementation #28676

Draft

8 tasks

instagibbs force-pushed the 2023-12-cluster-size2-package-rbf branch 2 times, most recently from 7aee728 to 1d859b0 Compare December 8, 2023 15:27

glozow mentioned this pull request Dec 11, 2023

policy: nVersion=3 and Package RBF #25038

Closed

instagibbs force-pushed the 2023-12-cluster-size2-package-rbf branch 3 times, most recently from f9ce967 to 385b6ca Compare December 11, 2023 20:04

instagibbs force-pushed the 2023-12-cluster-size2-package-rbf branch from 705fcd9 to 53ed8fd Compare May 9, 2024 13:55

instagibbs mentioned this pull request May 9, 2024

refactor prep for package rbf #30072

Open

glozow reviewed May 9, 2024

View reviewed changes

instagibbs force-pushed the 2023-12-cluster-size2-package-rbf branch from 53ed8fd to 99c0f92 Compare May 9, 2024 18:48

DrahtBot added the CI failed label May 12, 2024

glozow reviewed May 13, 2024

View reviewed changes

instagibbs force-pushed the 2023-12-cluster-size2-package-rbf branch 2 times, most recently from 722d1a5 to 7fd321d Compare May 13, 2024 16:14

DrahtBot added Needs rebase and removed CI failed labels May 14, 2024

glozow and others added 12 commits May 15, 2024 08:05

cpfp carveout is excluded in packages

cc6f89d

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.

PreChecks: tighten args checks for sibling eviction to allow package …

c7b7485

…rbf later

[test] package rbf

9a00ceb

CheckPackageMempoolAcceptResult: Check package rbf invariants

05e069c

mempool: Improve logging of replaced transactions

063451f

Add CTxMempool::GetEntry for Wtxid args

22adc98

Fuzz: pass mempool to CheckPackageMempoolAcceptResult

cd794c8

fuzz: add package RBF subpackage size assertion

850ff78

doc: update package RBF comment

ce3a662

instagibbs force-pushed the 2023-12-cluster-size2-package-rbf branch from 7fd321d to ce3a662 Compare May 15, 2024 12:13

DrahtBot removed the Needs rebase label May 15, 2024

	return package_state.Invalid(PackageValidationResult::PCKG_POLICY, "package RBF failed: replacing cluster with ancestors not size two");
	return package_state.Invalid(PackageValidationResult::PCKG_POLICY, "package RBF failed: new transaction cannot have mempool ancestors");

		- Package RBF may be enabled in the future.
		- More general package RBF may be enabled in the future.

	const Txid child_hash = child_ws.m_ptx->GetHash();
	const Txid& child_hash = child_ws.m_ptx->GetHash();

		"package RBF failed: parent paying for child replacement",
		strprintf("package feerate is %s while parent feerate is %s", package_feerate.ToString(), parent_feerate.ToString()));

	fill_mempool(self, self.nodes[0], self.wallet)
	fill_mempool(self, self.nodes[0])

Cluster size 2 package rbf #28984

Are you sure you want to change the base?

Cluster size 2 package rbf #28984

Conversation

instagibbs commented Dec 1, 2023 • edited

DrahtBot commented Dec 1, 2023 • edited

Code Coverage

Reviews

Conflicts

glozow commented Dec 4, 2023

instagibbs Dec 4, 2023 • edited

Choose a reason for hiding this comment

sdaftuar commented Dec 8, 2023

instagibbs commented Dec 8, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

instagibbs commented May 9, 2024

DrahtBot commented May 12, 2024

instagibbs commented May 12, 2024

glozow left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

instagibbs commented May 15, 2024

t-bast commented May 15, 2024

instagibbs commented May 16, 2024

instagibbs commented Dec 1, 2023 •

edited

DrahtBot commented Dec 1, 2023 •

edited

instagibbs Dec 4, 2023 •

edited