Skip to content

Conversation

@DashCoreAutoGuix
Copy link
Owner

@DashCoreAutoGuix DashCoreAutoGuix commented Dec 9, 2025

Backports bitcoin#17786

This PR:

  • Gets rid of the policy/fees -> txmempool -> policy/fees circular dependency
  • Moves CTxMemPoolEntry class to its own module (txmempool_entry.h)
  • Inlines the class's functions

Original Bitcoin PR: bitcoin#17786
Original commit: d0b1f61

Dash-specific adaptations:

  • Preserved Dash's size-based fee calculation (nTxSize/sigOpCount) instead of Bitcoin's weight-based (nTxWeight/sigOpCost)
  • Added Dash-specific fields to CTxMemPoolEntry: validForProTxKey, isKeyChangeProTx
  • Kept existing src/policy/rbf.cpp and src/test/fuzz/util/mempool.cpp files that were already present

Summary by CodeRabbit

Release Notes

Refactor

  • Reorganized transaction mempool entry handling into separate internal modules
  • Restructured Replace-By-Fee policy validation logic for improved maintainability
  • Eliminated circular dependency in internal module structure
  • Updated test infrastructure and sanitizer suppressions

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Dec 9, 2025

Walkthrough

This refactoring extracts the CTxMemPoolEntry class definition from src/txmempool.h into a dedicated new header src/txmempool_entry.h. Multiple source files are updated to include the new header. A new RBF (Replace-By-Fee) policy implementation file is added with several validation functions. Method implementations are adjusted accordingly.

Changes

Cohort / File(s) Summary
Header extraction and reorganization
src/txmempool_entry.h
New public header file introducing the CTxMemPoolEntry class with comprehensive mempool entry state management, including fee tracking, ancestor/descendant statistics, lock points, and mempool relationships. Also defines LockPoints struct and CompareIteratorByHash comparator.
src/txmempool.h
src/Makefile.am
RBF policy logic
src/policy/rbf.cpp
New implementation file containing eight RBF validation functions: IsRBFOptIn(), IsRBFOptInEmptyMempool(), GetEntriesForConflicts(), HasNoNewUnconfirmed(), EntriesAndTxidsDisjoint(), PaysMoreThanConflicts(), and PaysForRBF(). Functions use std::optional<std::string> for error reporting.
Mempool implementation
src/txmempool.cpp
Removed public constructor CTxMemPoolEntry::CTxMemPoolEntry() and member functions UpdateModifiedFee(), UpdateLockPoints(), and GetTxSize().
Include directive additions
src/bench/mempool_eviction.cpp, src/bench/mempool_stress.cpp, src/bench/rpc_mempool.cpp, src/net_processing.cpp, src/node/interfaces.cpp, src/rpc/mempool.cpp, src/test/util/setup_common.cpp, src/validation.cpp
Added #include <txmempool_entry.h> to access CTxMemPoolEntry type declarations.
Header replacement
src/policy/fees.cpp
Replaced #include <txmempool.h> with #include <txmempool_entry.h>.
Test utilities
src/test/fuzz/util/mempool.cpp
Added new helper function ConsumeTxMemPoolEntry() for fuzzing; generates randomized CTxMemPoolEntry instances from fuzzed data.
src/test/fuzz/policy_estimator.cpp
Build and linting
test/lint/lint-circular-dependencies.py
Removed expected circular dependency "policy/fees -> txmempool -> policy/fees".
test/sanitizer_suppressions/ubsan

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

  • src/policy/rbf.cpp: Requires detailed review of RBF validation logic, error handling paths, and feerate/fee sufficiency rules. Eight interdependent functions with optional error reporting.
  • src/txmempool_entry.h: Large new header file with comprehensive CTxMemPoolEntry class definition, multiple accessors, mutators, and member tracking. Verify correctness of ancestor/descendant state tracking and mutable field semantics.
  • Header extraction from src/txmempool.h: Verify that all moved definitions are correctly relocated and that remaining code in txmempool.h properly depends on the new external header.
  • Method removals in src/txmempool.cpp: Confirm that removal of constructor and update methods does not break existing call sites and that alternative mechanisms handle fee/size propagation.
  • Circular dependency removal: Verify that the lint change correctly reflects the architectural decoupling achieved by the header extraction.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 38.89% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main refactoring objective: eliminating the circular dependency between policy/fees and mempool by moving CTxMemPoolEntry to its own header.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch backport-0.25-batch-488-pr-17786

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants