test: Add package RBF test with deduplicated parent

HassanFleyah commented at 8:02 pm on February 11, 2026: none

This PR adds a unit test scenario to txpackage_tests.cpp covering a specific edge case: “Package RBF with a deduplicated parent”.

The test simulates a package {Parent, Child} where:

The Parent transaction is already in the mempool (result: MEMPOOL_ENTRY).
The Child transaction conflicts with another in-mempool transaction.
The Child pays sufficient fees to effect RBF.

This ensures that the package validation logic correctly handles RBF calculations for the child even when the parent is deduplicated and not re-validated.

HassanFleyah requested review from Copilot on Feb 11, 2026

DrahtBot added the label Tests on Feb 11, 2026

DrahtBot commented at 8:03 pm on February 11, 2026: contributor

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

Reviews

See the guideline for information on the review process.

Type	Reviewers
Stale ACK	HowHsu

If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.

Copilot commented at 8:18 pm on February 11, 2026: none

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

in src/test/txpackage_tests.cpp:1281 in ace5b66a73

1276+    keystore.AddKey(coinbaseKey);
1277+
1278+    for (int i = 0; i < 2; ++i) {
1279+        SignatureData sigdata;
1280+        // Fix 1: Use 'keystore' instead of '*m_node.chainman'
1281+        // Fix 2: Remove '&' from mtx_child (as applied before)

HowHsu commented at 6:53 am on February 12, 2026:

nit: These comments look like they might be leftover development notes from debugging. Would it make sense to remove them?

HassanFleyah commented at 8:06 am on February 12, 2026:

done, thanks

in src/test/txpackage_tests.cpp:1297 in ace5b66a73 outdated

1292+    const auto result = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool,
1293+                                          package_rbf_dedup, false, {});
1294+
1295+    // 5. Validations
1296+    auto it_parent = result.m_tx_results.find(tx_parent->GetWitnessHash());
1297+    auto it_child = result.m_tx_results.find(tx_child->GetWitnessHash());

HowHsu commented at 6:54 am on February 12, 2026:

It might be worth adding iterator validity checks before dereferencing, just to be safe. If find() were to return end(), the dereference would be undefined behavior.

Something like: BOOST_CHECK(it_parent != result.m_tx_results.end()); BOOST_CHECK(it_child != result.m_tx_results.end());

HassanFleyah commented at 8:04 am on February 12, 2026:

Fixed.

in src/test/txpackage_tests.cpp:1301 in ace5b66a73 outdated

1296+    auto it_parent = result.m_tx_results.find(tx_parent->GetWitnessHash());
1297+    auto it_child = result.m_tx_results.find(tx_child->GetWitnessHash());
1298+    BOOST_CHECK(it_parent->second.m_result_type == MempoolAcceptResult::ResultType::MEMPOOL_ENTRY);
1299+    BOOST_CHECK(it_child->second.m_result_type == MempoolAcceptResult::ResultType::VALID);
1300+    BOOST_CHECK(!m_node.mempool->exists(tx_conflict->GetHash()));
1301+}

HowHsu commented at 6:55 am on February 12, 2026:

nit: A blank line before BOOST_AUTO_TEST_SUITE_END() would match the style of the rest of the file.

HowHsu commented at 8:40 am on February 12, 2026: none

Can you squash this commit into the main one

maflcko commented at 8:47 am on February 12, 2026: member

What test coverage is this adding, that does not already exist? What is the code mutant that this is killing?

maflcko commented at 8:50 am on February 12, 2026: member

Is this LLM generated?

HowHsu commented at 9:15 am on February 12, 2026: none

What test coverage is this adding, that does not already exist? What is the code mutant that this is killing?

https://github.com/bitcoin/bitcoin/blob/master/src/test/txpackage_tests.cpp#L1078 seems it is already covered.

HassanFleyah commented at 10:53 am on February 12, 2026: none

Thanks for the link. I checked package_rbf_tests, but that seems to cover sibling eviction (two children competing for the same parent output).

This test targets a different edge case: RBF against an external conflict while the parent is deduplicated. Basically, the child spends {Parent output + External UTXO}, and the conflict is on the External UTXO. I wanted to verify fee calculations when the conflict isn’t a sibling.

HowHsu commented at 11:08 am on February 12, 2026: none

Thanks for the link. I checked package_rbf_tests, but that seems to cover sibling eviction (two children competing for the same parent output).

This test targets a different edge case: RBF against an external conflict while the parent is deduplicated. Basically, the child spends {Parent output + External UTXO}, and the conflict is on the External UTXO. I wanted to verify fee calculations when the conflict isn’t a sibling.

I see, it is a little bit different with the de-duplication one in package_rbf_tests. Correct me if I’m wrong, the only difference is the confict object, which in your case is an External UTXO, and in the package_rbf_tests is the parent. Maybe it’s better to add the case to package_rbf_tests rather than create a new test if the new one is really needed.

HassanFleyah force-pushed on Feb 12, 2026

HassanFleyah commented at 11:45 am on February 12, 2026: none

Thanks for the link. I checked package_rbf_tests, but that seems to cover sibling eviction (two children competing for the same parent output). This test targets a different edge case: RBF against an external conflict while the parent is deduplicated. Basically, the child spends {Parent output + External UTXO}, and the conflict is on the External UTXO. I wanted to verify fee calculations when the conflict isn’t a sibling.

I see, it is a little bit different with the de-duplication one in package_rbf_tests. Correct me if I’m wrong, the only difference is the confict object, which in your case is an External UTXO, and in the package_rbf_tests is the parent. Maybe it’s better to add the case to package_rbf_tests rather than create a new test if the new one is really needed.

Fair point. I’ve moved the logic into a new block inside package_rbf_tests as suggested, and squashed the commits.

HowHsu commented at 6:00 am on February 13, 2026: none

ACK e18d2c2e8d866f318913ba96cf253248a8c6f5e3

DrahtBot added the label CI failed on Feb 14, 2026

DrahtBot commented at 2:00 pm on February 14, 2026: contributor

🚧 At least one of the CI tasks failed. Task lint: https://github.com/bitcoin/bitcoin/actions/runs/21945205686/job/63620524142 LLM reason (✨ experimental): Lint failure: trailing whitespace detected in src/test/txpackage_tests.cpp (line 1284).

Try to run the tests locally, according to the documentation. However, a CI failure may still 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.

test: Add package RBF test with deduplicated parent d5546c1d9d

HassanFleyah force-pushed on Feb 14, 2026

HassanFleyah commented at 4:54 pm on February 14, 2026: none

Fixed the trailing whitespace. Could you please trigger the CI workflows?

DrahtBot removed the label CI failed on Feb 16, 2026

test: Add package RBF test with deduplicated parent #34567

Reviews