A few small mempool fixes (#212)

* use `request.allow_orphan` when passing the rpc tx to the mempool

* minor

* include the original accepted transaction as well

* for now keep the original timestamp deviation tolerance

Author: michaelsutton

Cargo.lock

@@ -85,9 +85,7 @@ impl<const BPS: u64> Bps<BPS> {
     }
 
     pub const fn pruning_proof_m() -> u64 {
-        // Since the important levels remain logarithmically long, it seems that this
-        // constant does not need to scale with BPS.
-        // TODO: finalize this
+        // No need to scale this constant with BPS since the important block levels (higher) remain logarithmically long
         PRUNING_PROOF_M
     }
 
@@ -120,10 +118,6 @@ impl<const BPS: u64> Bps<BPS> {
     pub const fn pre_deflationary_phase_base_subsidy() -> u64 {
         50000000000 / BPS
     }
-
-    // TODO: we might need to increase max_block_level (at least for mainnet) as a function of BPS
-    // since higher BPS means easier difficulty puzzles -> less zeros in pow hash
-    // pub const fn max_block_level() -> u64 {  }
 }
 
 #[cfg(test)]

consensus/core/src/config/bps.rs

@@ -27,8 +27,8 @@ pub mod consensus {
     pub const LEGACY_TIMESTAMP_DEVIATION_TOLERANCE: u64 = 132;
 
     /// **New** timestamp deviation tolerance (seconds).
-    /// KIP-0004: 605 (~10 minutes)
-    pub const NEW_TIMESTAMP_DEVIATION_TOLERANCE: u64 = 605;
+    /// TODO: KIP-0004: 605 (~10 minutes)
+    pub const NEW_TIMESTAMP_DEVIATION_TOLERANCE: u64 = 132;
 
     /// The desired interval between samples of the median time window (seconds).
     /// KIP-0004: 10 seconds

consensus/core/src/config/constants.rs

@@ -15,6 +15,7 @@ kaspa-txscript.workspace = true
 kaspa-core.workspace = true
 kaspa-mining-errors.workspace = true
 kaspa-consensusmanager.workspace = true
+kaspa-utils.workspace = true
 thiserror.workspace = true
 serde.workspace = true
 log.workspace = true

mining/Cargo.toml

@@ -476,9 +476,9 @@ mod tests {
         let unorphaned_txs = result.unwrap();
         let (populated_txs, orphans) = mining_manager.get_all_transactions(true, true);
         assert_eq!(
-            unorphaned_txs.len(), SKIPPED_TXS,
+            unorphaned_txs.len(), SKIPPED_TXS + 1,
             "the mempool is expected to have unorphaned the remaining child transaction after the matching parent transaction was inserted into the mempool: expected: {}, got: {}",
-            unorphaned_txs.len(), SKIPPED_TXS
+            SKIPPED_TXS + 1, unorphaned_txs.len()
         );
         assert_eq!(
             SKIPPED_TXS + SKIPPED_TXS,
@@ -632,13 +632,13 @@ mod tests {
             let unorphaned_txs = result.as_ref().unwrap();
             assert_eq!(
                 test.should_unorphan,
-                !unorphaned_txs.is_empty(),
+                unorphaned_txs.len() > 1,
                 "{}: child transaction should have been {} the orphan pool",
                 test.name,
                 test.parent_insert_result()
             );
-            if !unorphaned_txs.is_empty() {
-                assert_eq!(unorphaned_txs[0].id(), child_txs[i].id(), "the unorphaned transaction should match the inserted parent");
+            if unorphaned_txs.len() > 1 {
+                assert_eq!(unorphaned_txs[1].id(), child_txs[i].id(), "the unorphaned transaction should match the inserted parent");
             }
         }
     }

mining/src/manager_tests.rs

@@ -11,6 +11,7 @@ use kaspa_consensus_core::{
     tx::{MutableTransaction, Transaction, TransactionId, TransactionOutpoint, UtxoEntry},
 };
 use kaspa_core::info;
+use kaspa_utils::vec::VecExtensions;
 
 use super::tx::{Orphan, Priority};
 
@@ -60,8 +61,10 @@ impl Mempool {
         // transaction reference and mutably for the call to process_orphans_after_accepted_transaction
         let accepted_transaction =
             self.transaction_pool.add_transaction(transaction, consensus.get_virtual_daa_score(), priority)?.mtx.tx.clone();
-        let accepted_orphans = self.process_orphans_after_accepted_transaction(consensus, &accepted_transaction)?;
-        Ok(accepted_orphans)
+        let mut accepted_transactions = self.process_orphans_after_accepted_transaction(consensus, &accepted_transaction)?;
+        // We include the original accepted transaction as well
+        accepted_transactions.swap_insert(0, accepted_transaction);
+        Ok(accepted_transactions)
     }
 
     fn validate_transaction_pre_utxo_entry(&self, transaction: &MutableTransaction) -> RuleResult<()> {
@@ -98,16 +101,12 @@ impl Mempool {
     ) -> RuleResult<Vec<Arc<Transaction>>> {
         // Rust rewrite:
         // - The function is relocated from OrphanPool into Mempool
-        let mut added_transactions = Vec::new();
-        let mut unorphaned_transactions =
-            self.get_unorphaned_transactions_after_accepted_transaction(consensus, accepted_transaction)?;
-        while !unorphaned_transactions.is_empty() {
-            let transaction = unorphaned_transactions.pop().unwrap();
-
+        let unorphaned_transactions = self.get_unorphaned_transactions_after_accepted_transaction(consensus, accepted_transaction)?;
+        let mut added_transactions = Vec::with_capacity(unorphaned_transactions.len() + 1); // +1 since some callers add the accepted tx itself
+        for transaction in unorphaned_transactions {
             // The returned transactions are leaving the mempool but must also be added to
             // the transaction pool so we clone.
             added_transactions.push(transaction.mtx.tx.clone());
-
             self.transaction_pool.add_mempool_transaction(transaction)?;
         }
         Ok(added_transactions)

mining/src/mempool/validate_and_insert_transaction.rs

@@ -404,7 +404,12 @@ impl FlowContext {
         // TODO: call a handler function or a predefined registered service
     }
 
-    pub async fn add_transaction(
+    /// Adds the rpc-submitted transaction to the mempool and propagates it to peers.
+    ///
+    /// Transactions submitted through rpc are considered high priority. This definition does not affect the tx selection algorithm
+    /// but only changes how we manage the lifetime of the tx. A high-priority tx does not expire and is repeatedly rebroadcasted to
+    /// peers
+    pub async fn submit_rpc_transaction(
         &self,
         consensus: &ConsensusProxy,
         transaction: Transaction,

protocol/flows/src/flow_context.rs

@@ -20,7 +20,7 @@ path = "./src/bin/server.rs"
 [dependencies]
 kaspa-core.workspace = true
 kaspa-consensus-core.workspace = true
-kaspa-mining.workspace = true
+kaspa-mining-errors.workspace = true
 kaspa-hashes.workspace = true
 kaspa-math.workspace = true
 kaspa-utils.workspace = true

protocol/p2p/Cargo.toml

@@ -1,6 +1,6 @@
 use crate::{convert::error::ConversionError, core::peer::PeerKey, KaspadMessagePayloadType};
 use kaspa_consensus_core::errors::{block::RuleError, consensus::ConsensusError, pruning::PruningImportError};
-use kaspa_mining::errors::MiningManagerError;
+use kaspa_mining_errors::manager::MiningManagerError;
 use std::time::Duration;
 use thiserror::Error;
 

protocol/p2p/src/common.rs

@@ -384,7 +384,11 @@ impl RpcApi for RpcCoreService {
         let transaction: Transaction = (&request.transaction).try_into()?;
         let transaction_id = transaction.id();
         let session = self.consensus_manager.consensus().session().await;
-        self.flow_context.add_transaction(&session, transaction, Orphan::Allowed).await.map_err(|err| {
+        let orphan = match request.allow_orphan {
+            true => Orphan::Allowed,
+            false => Orphan::Forbidden,
+        };
+        self.flow_context.submit_rpc_transaction(&session, transaction, orphan).await.map_err(|err| {
             let err = RpcError::RejectedTransaction(transaction_id, err.to_string());
             debug!("{err}");
             err