Skip to content

Commit

Permalink
design fix: avoid exposing full collections as mut. This violates enc…
Browse files Browse the repository at this point in the history
…apsulation logic since collections can be completely modified externally; while in tx pools it is important to make sure various internal collections are maintained consistently (for instance the `ready_transactions` field on `TransactionsPool` needs careful maintenance)
  • Loading branch information
michaelsutton committed Oct 5, 2023
1 parent f161b1c commit 41e4a94
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 20 deletions.
6 changes: 3 additions & 3 deletions mining/src/mempool/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use self::{
};
use kaspa_consensus_core::tx::{MutableTransaction, TransactionId};
use kaspa_core::time::Stopwatch;
use std::{collections::hash_map::Entry, sync::Arc};
use std::sync::Arc;

pub(crate) mod check_transaction_standard;
pub mod config;
Expand Down Expand Up @@ -140,8 +140,8 @@ impl Mempool {
}

pub(crate) fn update_revalidated_transaction(&mut self, transaction: MutableTransaction) -> bool {
if let Entry::Occupied(mut entry) = self.transaction_pool.all_mut().entry(transaction.id()) {
entry.get_mut().mtx = transaction;
if let Some(tx) = self.transaction_pool.entry_mut(&transaction.id()) {
tx.mtx = transaction;
true
} else {
false
Expand Down
12 changes: 6 additions & 6 deletions mining/src/mempool/model/orphan_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -259,23 +259,23 @@ impl OrphanPool {
fn get_random_low_priority_orphan(&self) -> Option<&MempoolTransaction> {
self.all_orphans.values().find(|x| x.priority == Priority::Low)
}

fn chained_mut(&mut self) -> &mut TransactionsEdges {
&mut self.chained_orphans
}
}

impl Pool for OrphanPool {
fn all(&self) -> &MempoolTransactionCollection {
&self.all_orphans
}

fn all_mut(&mut self) -> &mut MempoolTransactionCollection {
&mut self.all_orphans
}

fn chained(&self) -> &TransactionsEdges {
&self.chained_orphans
}

fn chained_mut(&mut self) -> &mut TransactionsEdges {
&mut self.chained_orphans
fn entry_mut(&mut self, transaction_id: &TransactionId) -> Option<&mut MempoolTransaction> {
self.all_orphans.get_mut(transaction_id)
}

fn expire_low_priority_transactions(&mut self, virtual_daa_score: u64) -> RuleResult<()> {
Expand Down
4 changes: 2 additions & 2 deletions mining/src/mempool/model/pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,8 @@ pub(crate) type TransactionsEdges = HashMap<TransactionId, TransactionIdSet>;

pub(crate) trait Pool {
fn all(&self) -> &MempoolTransactionCollection;
fn all_mut(&mut self) -> &mut MempoolTransactionCollection;

fn chained(&self) -> &TransactionsEdges;
fn chained_mut(&mut self) -> &mut TransactionsEdges;

fn has(&self, transaction_id: &TransactionId) -> bool {
self.all().contains_key(transaction_id)
Expand All @@ -30,6 +28,8 @@ pub(crate) trait Pool {
self.all().get(transaction_id)
}

fn entry_mut(&mut self, transaction_id: &TransactionId) -> Option<&mut MempoolTransaction>;

/// Returns the number of transactions in the pool
fn len(&self) -> usize {
self.all().len()
Expand Down
12 changes: 3 additions & 9 deletions mining/src/mempool/model/transactions_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ impl TransactionsPool {
self.ready_transactions.insert(id);
}
for parent_id in parents {
let entry = self.chained_mut().entry(parent_id).or_default();
let entry = self.chained_transactions.entry(parent_id).or_default();
entry.insert(id);
}

Expand Down Expand Up @@ -305,19 +305,13 @@ impl Pool for TransactionsPool {
&self.all_transactions
}

#[inline]
fn all_mut(&mut self) -> &mut MempoolTransactionCollection {
&mut self.all_transactions
}

#[inline]
fn chained(&self) -> &TransactionsEdges {
&self.chained_transactions
}

#[inline]
fn chained_mut(&mut self) -> &mut TransactionsEdges {
&mut self.chained_transactions
fn entry_mut(&mut self, transaction_id: &TransactionId) -> Option<&mut MempoolTransaction> {
self.all_transactions.get_mut(transaction_id)
}

fn expire_low_priority_transactions(&mut self, virtual_daa_score: u64) -> RuleResult<()> {
Expand Down

0 comments on commit 41e4a94

Please sign in to comment.