Merge #21062: refactor: return MempoolAcceptResult from ATMP

53e716ea119658c28935fee24eb50090907c500e [refactor] improve style for touched code (gzhao408)
174cb5330af4b09f3a66974d3bae783ea43b190e [refactor] const ATMPArgs and non-const Workspace (gzhao408)
f82baf0762f60c2ca5ffc339b095f9271d7c2f33 [refactor] return MempoolAcceptResult (gzhao408)
9db10a55061e09021ff8ea1d6637d99f7959035f [refactor] clean up logic in testmempoolaccept (gzhao408)

Pull request description:

  This is the first 4 commits of #20833, and does refactoring only. It should be relatively simple to review, and offers a few nice things:
  - It makes accessing values that don't make sense (e.g. fee) when the tx is invalid an error.
  - Returning `MempoolAcceptResult` from ATMP makes the interface cleaner. The caller can get a const instead of passing in a mutable "out" param.
  - We don't have to be iterating through a bunch of lists for package validation, we can just return a `std::vector<MempoolAcceptResult>`.
  - We don't have to refactor all ATMP call sites again if/when we want to return more stuff from it.

ACKs for top commit:
  MarcoFalke:
    ACK 53e716ea119658c28935fee24eb50090907c500e 💿
  jnewbery:
    Code review ACK 53e716ea119658c28935fee24eb50090907c500e
  ariard:
    Code Review ACK 53e716e, I did tweak a bit the touched paths to see if we had good test coverage. Didn't find holes.

Tree-SHA512: fa6ec324a08ad9e6e55948615cda324cba176255708bf0a0a0f37cedb7a75311aa334ac6f223be7d8df3c7379502b1081102b9589f9a9afa1713ad3d9ab3c24f

1 files changed, 42 insertions, 7 deletions

diff --git a/src/validation.h b/src/validation.h
index e85c7bbf1a..238d6009b4 100644
--- a/src/validation.h
+++ b/src/validation.h
@@ -12,6 +12,7 @@
 
 #include <amount.h>
 #include <coins.h>
+#include <consensus/validation.h>
 #include <crypto/common.h> // for ReadLE64
 #include <fs.h>
 #include <optional.h>
@@ -23,6 +24,7 @@
 #include <txdb.h>
 #include <versionbits.h>
 #include <serialize.h>
+#include <util/check.h>
 #include <util/hasher.h>
 
 #include <atomic>
@@ -46,7 +48,6 @@ class CConnman;
 class CScriptCheck;
 class CTxMemPool;
 class ChainstateManager;
-class TxValidationState;
 struct ChainTxData;
 
 struct DisconnectedBlockTransactions;
@@ -181,12 +182,46 @@ void UnlinkPrunedFiles(const std::set<int>& setFilesToPrune);
 /** Prune block files up to a given height */
 void PruneBlockFilesManual(int nManualPruneHeight);
 
-/** (try to) add transaction to memory pool
- * plTxnReplaced will be appended to with all transactions replaced from mempool
- * @param[out] fee_out optional argument to return tx fee to the caller **/
-bool AcceptToMemoryPool(CTxMemPool& pool, TxValidationState &state, const CTransactionRef &tx,
-                        std::list<CTransactionRef>* plTxnReplaced,
-                        bool bypass_limits, bool test_accept=false, CAmount* fee_out=nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
+/**
+* Validation result for a single transaction mempool acceptance.
+*/
+struct MempoolAcceptResult {
+    /** Used to indicate the results of mempool validation,
+    * including the possibility of unfinished validation.
+    */
+    enum class ResultType {
+        VALID, //!> Fully validated, valid.
+        INVALID, //!> Invalid.
+    };
+    ResultType m_result_type;
+    TxValidationState m_state;
+
+    // The following fields are only present when m_result_type = ResultType::VALID
+    /** Mempool transactions replaced by the tx per BIP 125 rules. */
+    std::optional<std::list<CTransactionRef>> m_replaced_transactions;
+    /** Raw base fees. */
+    std::optional<CAmount> m_base_fees;
+
+    /** Constructor for failure case */
+    explicit MempoolAcceptResult(TxValidationState state)
+        : m_result_type(ResultType::INVALID),
+        m_state(state), m_replaced_transactions(nullopt), m_base_fees(nullopt) {
+            Assume(!state.IsValid()); // Can be invalid or error
+        }
+
+    /** Constructor for success case */
+    explicit MempoolAcceptResult(std::list<CTransactionRef>&& replaced_txns, CAmount fees)
+        : m_result_type(ResultType::VALID), m_state(TxValidationState{}),
+        m_replaced_transactions(std::move(replaced_txns)), m_base_fees(fees) {}
+};
+
+/**
+ * (Try to) add a transaction to the memory pool.
+ * @param[in]  bypass_limits   When true, don't enforce mempool fee limits.
+ * @param[in]  test_accept     When true, run validation checks but don't submit to mempool.
+ */
+MempoolAcceptResult AcceptToMemoryPool(CTxMemPool& pool, const CTransactionRef& tx,
+                                       bool bypass_limits, bool test_accept=false) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
 
 /** Get the BIP9 state for a given deployment at the current tip. */
 ThresholdState VersionBitsTipState(const Consensus::Params& params, Consensus::DeploymentPos pos);