Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A few small mempool fixes #212

Merged
merged 4 commits into from
Jun 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 1 addition & 7 deletions consensus/core/src/config/bps.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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)]
Expand Down
4 changes: 2 additions & 2 deletions consensus/core/src/config/constants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions mining/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 5 additions & 5 deletions mining/src/manager_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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");
}
}
}
Expand Down
17 changes: 8 additions & 9 deletions mining/src/mempool/validate_and_insert_transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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<()> {
Expand Down Expand Up @@ -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)
Expand Down
7 changes: 6 additions & 1 deletion protocol/flows/src/flow_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion protocol/p2p/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion protocol/p2p/src/common.rs
Original file line number Diff line number Diff line change
@@ -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;

Expand Down
6 changes: 5 additions & 1 deletion rpc/service/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 9 additions & 0 deletions utils/src/vec.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
pub trait VecExtensions<T> {
/// Pushes the provided value to the container if the container is empty
fn push_if_empty(self, value: T) -> Self;

/// Inserts the provided `value` at `index` while swapping the item at index to the end of the container
fn swap_insert(&mut self, index: usize, value: T);
}

impl<T> VecExtensions<T> for Vec<T> {
Expand All @@ -10,4 +13,10 @@ impl<T> VecExtensions<T> for Vec<T> {
}
self
}

fn swap_insert(&mut self, index: usize, value: T) {
self.push(value);
let loc = self.len() - 1;
self.swap(index, loc);
}
}