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

Replace by fee on mempool #499

Merged
merged 30 commits into from
Jul 22, 2024
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
621ab30
Replace by fee on mempool with tests
KaffinPX Jul 1, 2024
28c2ecc
Add a custom RemovalReason -- ReplacedByFee
KaffinPX Jul 1, 2024
1430039
Let `MinerManager` handle replace by fee (RBF) for RPC and P2P
tiram88 Jul 4, 2024
7ef25be
Refines success conditions and process of RBF policies
tiram88 Jul 4, 2024
4c83d0f
Add an RPC `submit_transaction_replacement` method
tiram88 Jul 5, 2024
bf662d7
fix fmt
tiram88 Jul 5, 2024
13676ef
Fix CLI Build WASM32 error
tiram88 Jul 5, 2024
3b4fde9
Avoid breaking wRPC
tiram88 Jul 5, 2024
d6970f8
Extend some tests coverage to all priority, orphan & RBF policy combi…
tiram88 Jul 6, 2024
33b2295
Let RBF fail early or at least before checking transaction scripts in…
tiram88 Jul 8, 2024
d95dfdb
Cleaning
tiram88 Jul 8, 2024
8cce2b4
More cleaning
tiram88 Jul 8, 2024
ba3b75e
Use contextual instead of compute mass in RBF eviction rule
tiram88 Jul 9, 2024
31b20a7
Avoid collision with another PR
tiram88 Jul 9, 2024
ce2f76d
Avoid collision with another PR (2)
tiram88 Jul 9, 2024
335f3d6
Extended test coverage of RBF
tiram88 Jul 9, 2024
4139e14
Extended test coverage of RBF (2)
tiram88 Jul 9, 2024
e4e2355
Rename `TransactionBatchValidationArgs` to `TransactionValidationBatc…
tiram88 Jul 11, 2024
9147abb
Add comments
tiram88 Jul 11, 2024
96df37e
Assert instead of condition
tiram88 Jul 11, 2024
0cd7c10
Add an `RbfPolicy` parameter to mining manager tx validate_and_insert…
tiram88 Jul 12, 2024
b6730af
Infer RBF policy from unorphaned transaction
tiram88 Jul 12, 2024
6971302
Apply the RBF policy to all orphan-related cases
tiram88 Jul 12, 2024
9fbd741
In Rbf allowed mode, check feerate threshold vs all double spends (i.…
michaelsutton Jul 22, 2024
1fcb264
Minor hashset optimization
michaelsutton Jul 22, 2024
4679a8d
Rename: fee_per_mass -> feerate
michaelsutton Jul 22, 2024
8093ef6
Use rbf policy arg for post processing step as well
michaelsutton Jul 22, 2024
dc46102
Renames and comments
michaelsutton Jul 22, 2024
49d584b
Relaxation: fail gracefully if rbf replaced tx is missing (also fixes…
michaelsutton Jul 22, 2024
c4ee735
Tx id is appended by the caller anyway
michaelsutton Jul 22, 2024
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
47 changes: 47 additions & 0 deletions consensus/core/src/api/args.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
use std::collections::HashMap;

use crate::tx::TransactionId;

/// A struct provided to consensus for transaction validation processing calls
#[derive(Clone, Debug, Default)]
pub struct TransactionValidationArgs {
/// Optional fee/mass threshold above which a bound transaction in not rejected
pub fee_per_mass_threshold: Option<f64>,
}

impl TransactionValidationArgs {
pub fn new(fee_per_mass_threshold: Option<f64>) -> Self {
Self { fee_per_mass_threshold }
}
}

/// A struct provided to consensus for transactions validation processing calls
tiram88 marked this conversation as resolved.
Show resolved Hide resolved
pub struct TransactionBatchValidationArgs {
tiram88 marked this conversation as resolved.
Show resolved Hide resolved
tx_args: HashMap<TransactionId, TransactionValidationArgs>,
}

impl TransactionBatchValidationArgs {
const DEFAULT_ARGS: TransactionValidationArgs = TransactionValidationArgs { fee_per_mass_threshold: None };

pub fn new() -> Self {
Self { tx_args: HashMap::new() }
}

/// Set some fee/mass threshold for transaction `transaction_id`.
pub fn set_fee_per_mass_threshold(&mut self, transaction_id: TransactionId, threshold: f64) {
self.tx_args
.entry(transaction_id)
.and_modify(|x| x.fee_per_mass_threshold = Some(threshold))
.or_insert(TransactionValidationArgs::new(Some(threshold)));
}

pub fn get(&self, transaction_id: &TransactionId) -> &TransactionValidationArgs {
self.tx_args.get(transaction_id).unwrap_or(&Self::DEFAULT_ARGS)
}
}

impl Default for TransactionBatchValidationArgs {
fn default() -> Self {
Self::new()
}
}
14 changes: 10 additions & 4 deletions consensus/core/src/api/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use std::sync::Arc;

use crate::{
acceptance_data::AcceptanceData,
api::args::{TransactionBatchValidationArgs, TransactionValidationArgs},
block::{Block, BlockTemplate, TemplateBuildMode, TemplateTransactionSelector, VirtualStateApproxId},
blockstatus::BlockStatus,
coinbase::MinerData,
Expand All @@ -25,6 +26,7 @@ use kaspa_hashes::Hash;

pub use self::stats::{BlockCount, ConsensusStats};

pub mod args;
pub mod counters;
pub mod stats;

Expand Down Expand Up @@ -62,14 +64,18 @@ pub trait ConsensusApi: Send + Sync {
}

/// Populates the mempool transaction with maximally found UTXO entry data and proceeds to full transaction
/// validation if all are found. If validation is successful, also [`transaction.calculated_fee`] is expected to be populated.
fn validate_mempool_transaction(&self, transaction: &mut MutableTransaction) -> TxResult<()> {
/// validation if all are found. If validation is successful, also `transaction.calculated_fee` is expected to be populated.
fn validate_mempool_transaction(&self, transaction: &mut MutableTransaction, args: &TransactionValidationArgs) -> TxResult<()> {
unimplemented!()
}

/// Populates the mempool transactions with maximally found UTXO entry data and proceeds to full transactions
/// validation if all are found. If validation is successful, also [`transaction.calculated_fee`] is expected to be populated.
fn validate_mempool_transactions_in_parallel(&self, transactions: &mut [MutableTransaction]) -> Vec<TxResult<()>> {
/// validation if all are found. If validation is successful, also `transaction.calculated_fee` is expected to be populated.
fn validate_mempool_transactions_in_parallel(
&self,
transactions: &mut [MutableTransaction],
args: &TransactionBatchValidationArgs,
) -> Vec<TxResult<()>> {
unimplemented!()
}

Expand Down
3 changes: 3 additions & 0 deletions consensus/core/src/errors/tx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,9 @@ pub enum TxRuleError {

#[error("calculated contextual mass (including storage mass) {0} is not equal to the committed mass field {1}")]
WrongMass(u64, u64),

#[error("fee per compute mass ratio is not greater than the threshold")]
tiram88 marked this conversation as resolved.
Show resolved Hide resolved
FeePerMassTooLow,
}

pub type TxResult<T> = std::result::Result<T, TxRuleError>;
7 changes: 7 additions & 0 deletions consensus/core/src/tx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,13 @@ impl<T: AsRef<Transaction>> MutableTransaction<T> {
*entry = None;
}
}

pub fn calculated_fee_per_compute_mass(&self) -> Option<f64> {
match (self.calculated_fee, self.calculated_compute_mass) {
(Some(fee), Some(compute_mass)) => Some(fee as f64 / compute_mass as f64),
_ => None,
}
}
}

impl<T: AsRef<Transaction>> AsRef<Transaction> for MutableTransaction<T> {
Expand Down
21 changes: 15 additions & 6 deletions consensus/src/consensus/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,11 @@ use crate::{
};
use kaspa_consensus_core::{
acceptance_data::AcceptanceData,
api::{stats::BlockCount, BlockValidationFutures, ConsensusApi, ConsensusStats},
api::{
args::{TransactionBatchValidationArgs, TransactionValidationArgs},
stats::BlockCount,
BlockValidationFutures, ConsensusApi, ConsensusStats,
},
block::{Block, BlockTemplate, TemplateBuildMode, TemplateTransactionSelector, VirtualStateApproxId},
blockhash::BlockHashExtensions,
blockstatus::BlockStatus,
Expand All @@ -49,9 +53,10 @@ use kaspa_consensus_core::{
errors::{
coinbase::CoinbaseResult,
consensus::{ConsensusError, ConsensusResult},
difficulty::DifficultyError,
pruning::PruningImportError,
tx::TxResult,
},
errors::{difficulty::DifficultyError, pruning::PruningImportError},
header::Header,
muhash::MuHashExtensions,
network::NetworkType,
Expand Down Expand Up @@ -418,13 +423,17 @@ impl ConsensusApi for Consensus {
BlockValidationFutures { block_task: Box::pin(block_task), virtual_state_task: Box::pin(virtual_state_task) }
}

fn validate_mempool_transaction(&self, transaction: &mut MutableTransaction) -> TxResult<()> {
self.virtual_processor.validate_mempool_transaction(transaction)?;
fn validate_mempool_transaction(&self, transaction: &mut MutableTransaction, args: &TransactionValidationArgs) -> TxResult<()> {
self.virtual_processor.validate_mempool_transaction(transaction, args)?;
Ok(())
}

fn validate_mempool_transactions_in_parallel(&self, transactions: &mut [MutableTransaction]) -> Vec<TxResult<()>> {
self.virtual_processor.validate_mempool_transactions_in_parallel(transactions)
fn validate_mempool_transactions_in_parallel(
&self,
transactions: &mut [MutableTransaction],
args: &TransactionBatchValidationArgs,
) -> Vec<TxResult<()>> {
self.virtual_processor.validate_mempool_transactions_in_parallel(transactions, args)
}

fn populate_mempool_transaction(&self, transaction: &mut MutableTransaction) -> TxResult<()> {
Expand Down
22 changes: 17 additions & 5 deletions consensus/src/pipeline/virtual_processor/processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ use crate::{
};
use kaspa_consensus_core::{
acceptance_data::AcceptanceData,
api::args::{TransactionBatchValidationArgs, TransactionValidationArgs},
block::{BlockTemplate, MutableBlock, TemplateBuildMode, TemplateTransactionSelector},
blockstatus::BlockStatus::{StatusDisqualifiedFromChain, StatusUTXOValid},
coinbase::MinerData,
Expand Down Expand Up @@ -757,23 +758,28 @@ impl VirtualStateProcessor {
virtual_utxo_view: &impl UtxoView,
virtual_daa_score: u64,
virtual_past_median_time: u64,
args: &TransactionValidationArgs,
) -> TxResult<()> {
self.transaction_validator.validate_tx_in_isolation(&mutable_tx.tx)?;
self.transaction_validator.utxo_free_tx_validation(&mutable_tx.tx, virtual_daa_score, virtual_past_median_time)?;
self.validate_mempool_transaction_in_utxo_context(mutable_tx, virtual_utxo_view, virtual_daa_score)?;
self.validate_mempool_transaction_in_utxo_context(mutable_tx, virtual_utxo_view, virtual_daa_score, args)?;
Ok(())
}

pub fn validate_mempool_transaction(&self, mutable_tx: &mut MutableTransaction) -> TxResult<()> {
pub fn validate_mempool_transaction(&self, mutable_tx: &mut MutableTransaction, args: &TransactionValidationArgs) -> TxResult<()> {
let virtual_read = self.virtual_stores.read();
let virtual_state = virtual_read.state.get().unwrap();
let virtual_utxo_view = &virtual_read.utxo_set;
let virtual_daa_score = virtual_state.daa_score;
let virtual_past_median_time = virtual_state.past_median_time;
self.validate_mempool_transaction_impl(mutable_tx, virtual_utxo_view, virtual_daa_score, virtual_past_median_time)
self.validate_mempool_transaction_impl(mutable_tx, virtual_utxo_view, virtual_daa_score, virtual_past_median_time, args)
}

pub fn validate_mempool_transactions_in_parallel(&self, mutable_txs: &mut [MutableTransaction]) -> Vec<TxResult<()>> {
pub fn validate_mempool_transactions_in_parallel(
&self,
mutable_txs: &mut [MutableTransaction],
args: &TransactionBatchValidationArgs,
) -> Vec<TxResult<()>> {
let virtual_read = self.virtual_stores.read();
let virtual_state = virtual_read.state.get().unwrap();
let virtual_utxo_view = &virtual_read.utxo_set;
Expand All @@ -784,7 +790,13 @@ impl VirtualStateProcessor {
mutable_txs
.par_iter_mut()
.map(|mtx| {
self.validate_mempool_transaction_impl(mtx, &virtual_utxo_view, virtual_daa_score, virtual_past_median_time)
self.validate_mempool_transaction_impl(
mtx,
&virtual_utxo_view,
virtual_daa_score,
virtual_past_median_time,
args.get(&mtx.id()),
)
})
.collect::<Vec<TxResult<()>>>()
})
Expand Down
7 changes: 6 additions & 1 deletion consensus/src/pipeline/virtual_processor/utxo_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use crate::{
};
use kaspa_consensus_core::{
acceptance_data::{AcceptedTxEntry, MergesetBlockAcceptanceData},
api::args::TransactionValidationArgs,
coinbase::*,
hashing,
header::Header,
Expand Down Expand Up @@ -248,7 +249,8 @@ impl VirtualStateProcessor {
}
}
let populated_tx = PopulatedTransaction::new(transaction, entries);
let res = self.transaction_validator.validate_populated_transaction_and_get_fee(&populated_tx, pov_daa_score, flags);
let res =
self.transaction_validator.validate_populated_transaction_and_get_fee(&populated_tx, pov_daa_score, flags, None, None);
match res {
Ok(calculated_fee) => Ok(ValidatedTransaction::new(populated_tx, calculated_fee)),
Err(tx_rule_error) => {
Expand Down Expand Up @@ -290,6 +292,7 @@ impl VirtualStateProcessor {
mutable_tx: &mut MutableTransaction,
utxo_view: &impl UtxoView,
pov_daa_score: u64,
args: &TransactionValidationArgs,
) -> TxResult<()> {
self.populate_mempool_transaction_in_utxo_context(mutable_tx, utxo_view)?;

Expand All @@ -312,6 +315,8 @@ impl VirtualStateProcessor {
&mutable_tx.as_verifiable(),
pov_daa_score,
TxValidationFlags::SkipMassCheck, // we can skip the mass check since we just set it
mutable_tx.calculated_compute_mass,
michaelsutton marked this conversation as resolved.
Show resolved Hide resolved
args.fee_per_mass_threshold,
)?;
mutable_tx.calculated_fee = Some(calculated_fee);
Ok(())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,13 @@ impl TransactionValidator {
tx: &impl VerifiableTransaction,
pov_daa_score: u64,
flags: TxValidationFlags,
compute_mass: Option<u64>,
fee_per_mass_threshold: Option<f64>,
) -> TxResult<u64> {
self.check_transaction_coinbase_maturity(tx, pov_daa_score)?;
let total_in = self.check_transaction_input_amounts(tx)?;
let total_out = Self::check_transaction_output_values(tx, total_in)?;
let fee = total_in - total_out;
if flags != TxValidationFlags::SkipMassCheck && pov_daa_score > self.storage_mass_activation_daa_score {
// Storage mass hardfork was activated
self.check_mass_commitment(tx)?;
Expand All @@ -40,14 +43,26 @@ impl TransactionValidator {
}
}
Self::check_sequence_lock(tx, pov_daa_score)?;
Self::check_fee_per_mass(fee, compute_mass, fee_per_mass_threshold)?;
match flags {
TxValidationFlags::Full | TxValidationFlags::SkipMassCheck => {
Self::check_sig_op_counts(tx)?;
self.check_scripts(tx)?;
}
TxValidationFlags::SkipScriptChecks => {}
}
Ok(total_in - total_out)
Ok(fee)
}

tiram88 marked this conversation as resolved.
Show resolved Hide resolved
fn check_fee_per_mass(fee: u64, mass: Option<u64>, threshold: Option<f64>) -> TxResult<()> {
// An actual check can only occur if both some mass and some threshold are provided,
// otherwise, the check does not verify anything and exits successfully.
if let (Some(compute_mass), Some(threshold)) = (mass, threshold) {
if fee as f64 / compute_mass as f64 <= threshold {
return Err(TxRuleError::FeePerMassTooLow);
}
}
Ok(())
}

fn check_transaction_coinbase_maturity(&self, tx: &impl VerifiableTransaction, pov_daa_score: u64) -> TxResult<()> {
Expand Down
12 changes: 9 additions & 3 deletions mining/errors/src/mempool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use kaspa_consensus_core::{
};
use thiserror::Error;

#[derive(Error, Debug, Clone)]
#[derive(Error, Debug, Clone, PartialEq, Eq)]
pub enum RuleError {
/// A consensus transaction rule error
///
Expand All @@ -24,9 +24,15 @@ pub enum RuleError {
#[error("transaction {0} is already in the mempool")]
RejectDuplicate(TransactionId),

#[error("output {0} already spent by transaction {1} in the memory pool")]
#[error("output {0} already spent by transaction {1} in the mempool")]
RejectDoubleSpendInMempool(TransactionOutpoint, TransactionId),

#[error("replace by fee found no double spending transaction in the mempool")]
RejectRbfNoDoubleSpend,

#[error("replace by fee found more than one double spending transaction in the mempool")]
RejectRbfTooManyDoubleSpendingTransactions,

/// New behavior: a transaction is rejected if the mempool is full
#[error("number of high-priority transactions in mempool ({0}) has reached the maximum allowed ({1})")]
RejectMempoolIsFull(usize, u64),
Expand Down Expand Up @@ -95,7 +101,7 @@ impl From<TxRuleError> for RuleError {

pub type RuleResult<T> = std::result::Result<T, RuleError>;

#[derive(Error, Debug, Clone)]
#[derive(Error, Debug, Clone, PartialEq, Eq)]
pub enum NonStandardError {
#[error("transaction version {1} is not in the valid range of {2}-{3}")]
RejectVersion(TransactionId, u16, u16, u16),
Expand Down
Loading