From c1042ccefb39d92beb0af48ec58b977c5c1b50ec Mon Sep 17 00:00:00 2001 From: Aaron Blankstein Date: Thu, 23 Sep 2021 12:35:45 -0500 Subject: [PATCH 01/24] feat: add estimate evaluation and nonce tracking to mempool iteration --- src/chainstate/stacks/db/blocks.rs | 7 + src/chainstate/stacks/db/mod.rs | 18 +- src/chainstate/stacks/miner.rs | 175 +++++++++++--- src/chainstate/stacks/mod.rs | 10 + src/core/mempool.rs | 351 ++++++++++++++++++++--------- src/cost_estimates/metrics.rs | 15 ++ src/cost_estimates/mod.rs | 25 ++ src/main.rs | 7 + src/net/download.rs | 8 + src/util/db.rs | 15 ++ 10 files changed, 482 insertions(+), 149 deletions(-) diff --git a/src/chainstate/stacks/db/blocks.rs b/src/chainstate/stacks/db/blocks.rs index d45afa4eee..45b449e83e 100644 --- a/src/chainstate/stacks/db/blocks.rs +++ b/src/chainstate/stacks/db/blocks.rs @@ -5545,6 +5545,8 @@ pub mod test { use util::hash::*; use util::retry::*; + use crate::cost_estimates::metrics::UnitMetric; + use crate::cost_estimates::UnitEstimator; use crate::types::chainstate::{BlockHeaderHash, StacksWorkScore}; use super::*; @@ -9169,6 +9171,9 @@ pub mod test { let mut mempool = MemPoolDB::open(false, 0x80000000, &chainstate_path).unwrap(); let coinbase_tx = make_coinbase(miner, tenure_id); + let mut estimator = UnitEstimator; + let metric = UnitMetric; + let anchored_block = StacksBlockBuilder::build_anchored_block( chainstate, &sortdb.index_conn(), @@ -9180,6 +9185,8 @@ pub mod test { &coinbase_tx, BlockBuilderSettings::max_value(), None, + &mut estimator, + &metric, ) .unwrap(); (anchored_block.0, vec![]) diff --git a/src/chainstate/stacks/db/mod.rs b/src/chainstate/stacks/db/mod.rs index 87943caf05..ddaf566d51 100644 --- a/src/chainstate/stacks/db/mod.rs +++ b/src/chainstate/stacks/db/mod.rs @@ -237,11 +237,11 @@ impl FromRow for DBConfig { impl FromRow for StacksHeaderInfo { fn from_row<'a>(row: &'a Row) -> Result { - let block_height = u64::from_column(row, "block_height")?; + let block_height: u64 = u64::from_column(row, "block_height")?; let index_root = TrieHash::from_column(row, "index_root")?; let consensus_hash = ConsensusHash::from_column(row, "consensus_hash")?; let burn_header_hash = BurnchainHeaderHash::from_column(row, "burn_header_hash")?; - let burn_header_height = u64::from_column(row, "burn_header_height")? as u32; + let burn_header_height: u64 = u64::from_column(row, "burn_header_height")?; let burn_header_timestamp = u64::from_column(row, "burn_header_timestamp")?; let stacks_header = StacksBlockHeader::from_row(row)?; let anchored_block_size_str: String = row.get_unwrap("block_size"); @@ -256,13 +256,13 @@ impl FromRow for StacksHeaderInfo { Ok(StacksHeaderInfo { anchored_header: stacks_header, microblock_tail: None, - block_height: block_height, - index_root: index_root, - consensus_hash: consensus_hash, - burn_header_hash: burn_header_hash, - burn_header_height: burn_header_height, - burn_header_timestamp: burn_header_timestamp, - anchored_block_size: anchored_block_size, + block_height, + index_root, + consensus_hash, + burn_header_hash, + burn_header_height: burn_header_height as u32, + burn_header_timestamp, + anchored_block_size, }) } } diff --git a/src/chainstate/stacks/miner.rs b/src/chainstate/stacks/miner.rs index 90c0d0fa77..1c2a272cb0 100644 --- a/src/chainstate/stacks/miner.rs +++ b/src/chainstate/stacks/miner.rs @@ -20,6 +20,8 @@ use std::convert::From; use std::fs; use std::mem; +use crate::cost_estimates::metrics::CostMetric; +use crate::cost_estimates::CostEstimator; use crate::types::StacksPublicKeyBuffer; use burnchains::PrivateKey; use burnchains::PublicKey; @@ -322,21 +324,22 @@ impl<'a> StacksMicroblockBuilder<'a> { } /// Mine the next transaction into a microblock. - /// Returns true/false if the transaction was/was not mined into this microblock. + /// Returns Some(StacksTransactionReceipt) or None if the transaction was + /// or was not mined into this microblock. fn mine_next_transaction( clarity_tx: &mut ClarityTx<'a>, tx: StacksTransaction, tx_len: u64, considered: &mut HashSet, bytes_so_far: u64, - ) -> Result { + ) -> Result, Error> { if tx.anchor_mode != TransactionAnchorMode::OffChainOnly && tx.anchor_mode != TransactionAnchorMode::Any { - return Ok(false); + return Ok(None); } if considered.contains(&tx.txid()) { - return Ok(false); + return Ok(None); } else { considered.insert(tx.txid()); } @@ -345,11 +348,11 @@ impl<'a> StacksMicroblockBuilder<'a> { "Adding microblock tx {} would exceed epoch data size", &tx.txid() ); - return Ok(false); + return Ok(None); } let quiet = !cfg!(test); match StacksChainState::process_transaction(clarity_tx, &tx, quiet) { - Ok(_) => return Ok(true), + Ok((_, receipt)) => return Ok(Some(receipt)), Err(e) => match e { Error::CostOverflowError(cost_before, cost_after, total_budget) => { // note: this path _does_ not perform the tx block budget % heuristic, @@ -367,7 +370,7 @@ impl<'a> StacksMicroblockBuilder<'a> { } }, } - return Ok(false); + return Ok(None); } /// NOTE: this is only used in integration tests. @@ -401,12 +404,12 @@ impl<'a> StacksMicroblockBuilder<'a> { &mut considered, bytes_so_far, ) { - Ok(true) => { + Ok(Some(_)) => { bytes_so_far += tx_len; num_txs += 1; txs_included.push(tx); } - Ok(false) => { + Ok(None) => { continue; } Err(e) => { @@ -445,10 +448,12 @@ impl<'a> StacksMicroblockBuilder<'a> { return self.make_next_microblock(txs_included, miner_key); } - pub fn mine_next_microblock( + pub fn mine_next_microblock( &mut self, - mem_pool: &MemPoolDB, + mem_pool: &mut MemPoolDB, miner_key: &Secp256k1PrivateKey, + estimator: &mut CE, + metric: &CM, ) -> Result { let mut txs_included = vec![]; let mempool_settings = self.settings.mempool_settings.clone(); @@ -469,6 +474,9 @@ impl<'a> StacksMicroblockBuilder<'a> { let mut num_selected = 0; let deadline = get_epoch_time_ms() + (self.settings.max_miner_time_ms as u128); + mem_pool.reset_last_known_nonces()?; + mem_pool.estimate_tx_rates(estimator, metric, 100); + debug!( "Microblock transaction selection begins (child of {}), bytes so far: {}", &self.anchor_block, bytes_so_far @@ -481,7 +489,10 @@ impl<'a> StacksMicroblockBuilder<'a> { &mut clarity_tx, self.anchor_block_height, mempool_settings.clone(), - |clarity_tx, mempool_tx| { + |clarity_tx, to_consider| { + let mempool_tx = &to_consider.tx; + let update_estimator = to_consider.update_estimate; + if get_epoch_time_ms() >= deadline { debug!( "Microblock miner deadline exceeded ({} ms)", @@ -497,21 +508,32 @@ impl<'a> StacksMicroblockBuilder<'a> { &mut considered, bytes_so_far, ) { - Ok(true) => { + Ok(Some(receipt)) => { bytes_so_far += mempool_tx.metadata.len; + if update_estimator { + if let Err(e) = estimator.notify_event( + &mempool_tx.tx.payload, + &receipt.execution_cost, + ) { + warn!("Error updating estimator"; + "txid" => %mempool_tx.metadata.txid, + "error" => ?e); + } + } + debug!( "Include tx {} ({}) in microblock", mempool_tx.tx.txid(), mempool_tx.tx.payload.name() ); - txs_included.push(mempool_tx.tx); + txs_included.push(mempool_tx.tx.clone()); num_txs += 1; num_added += 1; num_selected += 1; Ok(true) } - Ok(false) => Ok(true), // keep iterating + Ok(None) => Ok(true), // keep iterating Err(e) => Err(e), } }, @@ -755,6 +777,7 @@ impl StacksBlockBuilder { ) -> Result<(), Error> { let tx_len = tx.tx_len(); self.try_mine_tx_with_len(clarity_tx, tx, tx_len, &BlockLimitFunction::NO_LIMIT_HIT) + .map(|_| ()) } /// Append a transaction if doing so won't exceed the epoch data size. @@ -765,7 +788,7 @@ impl StacksBlockBuilder { tx: &StacksTransaction, tx_len: u64, limit_behavior: &BlockLimitFunction, - ) -> Result<(), Error> { + ) -> Result { if self.bytes_so_far + tx_len >= MAX_EPOCH_SIZE.into() { return Err(Error::BlockTooBigError); } @@ -777,19 +800,21 @@ impl StacksBlockBuilder { // once we've hit the runtime limit once, allow boot code contract calls, but do not try to eval // other contract calls if !cc.address.is_boot_code_addr() { - return Ok(()); + return Err(Error::StacksTransactionSkipped); } } - TransactionPayload::SmartContract(_) => return Ok(()), + TransactionPayload::SmartContract(_) => { + return Err(Error::StacksTransactionSkipped) + } _ => {} } } - BlockLimitFunction::LIMIT_REACHED => return Ok(()), + BlockLimitFunction::LIMIT_REACHED => return Err(Error::StacksTransactionSkipped), BlockLimitFunction::NO_LIMIT_HIT => {} }; let quiet = !cfg!(test); - if !self.anchored_done { + let receipt = if !self.anchored_done { // building up the anchored blocks if tx.anchor_mode != TransactionAnchorMode::OnChainOnly && tx.anchor_mode != TransactionAnchorMode::Any @@ -800,7 +825,7 @@ impl StacksBlockBuilder { )); } - let (fee, _receipt) = StacksChainState::process_transaction(clarity_tx, tx, quiet) + let (fee, receipt) = StacksChainState::process_transaction(clarity_tx, tx, quiet) .map_err(|e| match e { Error::CostOverflowError(cost_before, cost_after, total_budget) => { clarity_tx.reset_cost(cost_before.clone()); @@ -833,6 +858,8 @@ impl StacksBlockBuilder { // save self.txs.push(tx.clone()); self.total_anchored_fees += fee; + + receipt } else { // building up the microblocks if tx.anchor_mode != TransactionAnchorMode::OffChainOnly @@ -844,7 +871,7 @@ impl StacksBlockBuilder { )); } - let (fee, _receipt) = StacksChainState::process_transaction(clarity_tx, tx, quiet) + let (fee, receipt) = StacksChainState::process_transaction(clarity_tx, tx, quiet) .map_err(|e| match e { Error::CostOverflowError(cost_before, cost_after, total_budget) => { clarity_tx.reset_cost(cost_before.clone()); @@ -878,10 +905,12 @@ impl StacksBlockBuilder { // save self.micro_txs.push(tx.clone()); self.total_streamed_fees += fee; - } + + receipt + }; self.bytes_so_far += tx_len; - Ok(()) + Ok(receipt) } /// Append a transaction if doing so won't exceed the epoch data size. @@ -1415,7 +1444,7 @@ impl StacksBlockBuilder { /// Given access to the mempool, mine an anchored block with no more than the given execution cost. /// returns the assembled block, and the consumed execution budget. - pub fn build_anchored_block( + pub fn build_anchored_block( chainstate_handle: &StacksChainState, // not directly used; used as a handle to open other chainstates burn_dbconn: &SortitionDBConn, mempool: &mut MemPoolDB, @@ -1426,6 +1455,8 @@ impl StacksBlockBuilder { coinbase_tx: &StacksTransaction, settings: BlockBuilderSettings, event_observer: Option<&dyn MemPoolEventDispatcher>, + estimator: &mut CE, + metric: &CM, ) -> Result<(StacksBlock, ExecutionCost, u64), Error> { let execution_budget = settings.execution_cost; let mempool_settings = settings.mempool_settings; @@ -1464,6 +1495,9 @@ impl StacksBlockBuilder { let mut epoch_tx = builder.epoch_begin(&mut chainstate, burn_dbconn)?; builder.try_mine_tx(&mut epoch_tx, coinbase_tx)?; + mempool.reset_last_known_nonces()?; + mempool.estimate_tx_rates(estimator, metric, 100); + let mut considered = HashSet::new(); // txids of all transactions we looked at let mut mined_origin_nonces: HashMap = HashMap::new(); // map addrs of mined transaction origins to the nonces we used let mut mined_sponsor_nonces: HashMap = HashMap::new(); // map addrs of mined transaction sponsors to the nonces we used @@ -1486,7 +1520,10 @@ impl StacksBlockBuilder { &mut epoch_tx, tip_height, mempool_settings.clone(), - |epoch_tx, txinfo| { + |epoch_tx, to_consider| { + let txinfo = &to_consider.tx; + let update_estimator = to_consider.update_estimate; + if block_limit_hit == BlockLimitFunction::LIMIT_REACHED { return Ok(false); } @@ -1524,9 +1561,20 @@ impl StacksBlockBuilder { txinfo.metadata.len, &block_limit_hit, ) { - Ok(_) => { + Ok(tx_receipt) => { num_txs += 1; + if update_estimator { + if let Err(e) = estimator.notify_event( + &txinfo.tx.payload, + &tx_receipt.execution_cost, + ) { + warn!("Error updating estimator"; + "txid" => %txinfo.metadata.txid, + "error" => ?e); + } + } } + Err(Error::StacksTransactionSkipped) => {} Err(Error::BlockTooBigError) => { // done mining -- our execution budget is exceeded. // Make the block from the transactions we did manage to get @@ -1659,6 +1707,8 @@ pub mod test { use util::vrf::VRFProof; use vm::types::*; + use crate::cost_estimates::metrics::UnitMetric; + use crate::cost_estimates::UnitEstimator; use crate::types::chainstate::SortitionId; use crate::util::boot::boot_code_addr; @@ -6407,6 +6457,9 @@ pub mod test { let coinbase_tx = make_coinbase(miner, tenure_id); + let mut estimator = UnitEstimator; + let metric = UnitMetric; + let anchored_block = StacksBlockBuilder::build_anchored_block( chainstate, &sortdb.index_conn(), @@ -6418,6 +6471,8 @@ pub mod test { &coinbase_tx, BlockBuilderSettings::max_value(), None, + &mut estimator, + &metric, ) .unwrap(); (anchored_block.0, vec![]) @@ -6531,6 +6586,10 @@ pub mod test { ) .unwrap(); } + + let mut estimator = UnitEstimator; + let metric = UnitMetric; + let anchored_block = StacksBlockBuilder::build_anchored_block( chainstate, &sortdb.index_conn(), @@ -6542,6 +6601,8 @@ pub mod test { &coinbase_tx, BlockBuilderSettings::max_value(), None, + &mut estimator, + &metric, ) .unwrap(); (anchored_block.0, vec![]) @@ -6668,6 +6729,10 @@ pub mod test { ) .unwrap(); } + + let mut estimator = UnitEstimator; + let metric = UnitMetric; + let anchored_block = StacksBlockBuilder::build_anchored_block( chainstate, &sortdb.index_conn(), @@ -6684,6 +6749,8 @@ pub mod test { ..BlockBuilderSettings::max_value() }, None, + &mut estimator, + &metric, ) .unwrap(); (anchored_block.0, vec![]) @@ -6830,6 +6897,9 @@ pub mod test { sender_nonce += 1; } + let mut estimator = UnitEstimator; + let metric = UnitMetric; + let anchored_block = StacksBlockBuilder::build_anchored_block( chainstate, &sortdb.index_conn(), @@ -6841,6 +6911,8 @@ pub mod test { &coinbase_tx, BlockBuilderSettings::max_value(), None, + &mut estimator, + &metric, ) .unwrap(); (anchored_block.0, vec![]) @@ -7058,6 +7130,9 @@ pub mod test { runtime: 3350, }; + let mut estimator = UnitEstimator; + let metric = UnitMetric; + let anchored_block = StacksBlockBuilder::build_anchored_block( chainstate, &sortdb.index_conn(), @@ -7069,6 +7144,8 @@ pub mod test { &coinbase_tx, BlockBuilderSettings::limited(execution_cost), None, + &mut estimator, + &metric, ) .unwrap(); (anchored_block.0, vec![]) @@ -7216,6 +7293,9 @@ pub mod test { &mut mempool }; + let mut estimator = UnitEstimator; + let metric = UnitMetric; + StacksBlockBuilder::build_anchored_block( chainstate, &sortdb.index_conn(), @@ -7227,6 +7307,8 @@ pub mod test { &coinbase_tx, BlockBuilderSettings::limited(execution_cost), None, + &mut estimator, + &metric, ) .unwrap() }; @@ -7324,6 +7406,9 @@ pub mod test { let mut mempool = MemPoolDB::open(false, 0x80000000, &chainstate_path).unwrap(); + let mut estimator = UnitEstimator; + let metric = UnitMetric; + let anchored_block = StacksBlockBuilder::build_anchored_block( chainstate, &sortdb.index_conn(), @@ -7335,6 +7420,8 @@ pub mod test { &coinbase_tx, BlockBuilderSettings::max_value(), None, + &mut estimator, + &metric, ) .unwrap(); @@ -7526,6 +7613,9 @@ pub mod test { sleep_ms(2000); } + let mut estimator = UnitEstimator; + let metric = UnitMetric; + let anchored_block = StacksBlockBuilder::build_anchored_block( chainstate, &sortdb.index_conn(), @@ -7537,6 +7627,8 @@ pub mod test { &coinbase_tx, BlockBuilderSettings::max_value(), None, + &mut estimator, + &metric, ) .unwrap(); @@ -7693,7 +7785,12 @@ pub mod test { let coinbase_tx = make_coinbase(miner, tenure_id as usize); - let mut anchored_block = StacksBlockBuilder::build_anchored_block(chainstate, &sortdb.index_conn(), &mut mempool, &parent_tip, tip.total_burn, vrf_proof, Hash160([tenure_id as u8; 20]), &coinbase_tx, BlockBuilderSettings::max_value(), None + let mut estimator = UnitEstimator; + let metric = UnitMetric; + + let mut anchored_block = StacksBlockBuilder::build_anchored_block(chainstate, &sortdb.index_conn(), &mut mempool, &parent_tip, tip.total_burn, vrf_proof, Hash160([tenure_id as u8; 20]), &coinbase_tx, BlockBuilderSettings::max_value(), None, + &mut estimator, + &metric, ).unwrap(); if tenure_id == bad_block_tenure { @@ -7955,6 +8052,9 @@ pub mod test { sleep_ms(2000); } + let mut estimator = UnitEstimator; + let metric = UnitMetric; + let anchored_block = StacksBlockBuilder::build_anchored_block( chainstate, &sortdb.index_conn(), @@ -7966,6 +8066,8 @@ pub mod test { &coinbase_tx, BlockBuilderSettings::max_value(), None, + &mut estimator, + &metric, ) .unwrap(); @@ -8223,6 +8325,10 @@ pub mod test { let mblock_pubkey_hash = Hash160::from_node_public_key(&StacksPublicKey::from_private(&mblock_privks[tenure_id])); + + let mut estimator = UnitEstimator; + let metric = UnitMetric; + let (anchored_block, block_size, block_execution_cost) = StacksBlockBuilder::build_anchored_block( chainstate, &sortdb.index_conn(), @@ -8234,6 +8340,8 @@ pub mod test { &coinbase_tx, BlockBuilderSettings::max_value(), None, + &mut estimator, + &metric, ) .unwrap(); @@ -8645,6 +8753,10 @@ pub mod test { .unwrap(); } + + let mut estimator = UnitEstimator; + let metric = UnitMetric; + let (anchored_block, block_size, block_execution_cost) = StacksBlockBuilder::build_anchored_block( chainstate, &sortdb.index_conn(), @@ -8656,6 +8768,8 @@ pub mod test { &coinbase_tx, BlockBuilderSettings::max_value(), None, + &mut estimator, + &metric, ) .unwrap(); @@ -9065,6 +9179,9 @@ pub mod test { let coinbase_tx = tx_signer.get_tx().unwrap(); + let mut estimator = UnitEstimator; + let metric = UnitMetric; + let (block, _cost, _size) = StacksBlockBuilder::build_anchored_block( &chainstate, &burndb.index_conn(), @@ -9076,6 +9193,8 @@ pub mod test { &coinbase_tx, BlockBuilderSettings::limited(execution_limit.clone()), None, + &mut estimator, + &metric, ) .unwrap(); diff --git a/src/chainstate/stacks/mod.rs b/src/chainstate/stacks/mod.rs index 07fb486904..478406dbc2 100644 --- a/src/chainstate/stacks/mod.rs +++ b/src/chainstate/stacks/mod.rs @@ -145,6 +145,11 @@ pub enum Error { InvalidStacksBlock(String), InvalidStacksMicroblock(String, BlockHeaderHash), InvalidStacksTransaction(String, bool), + /// This error indicates that the considered transaction was skipped + /// because of the current state of the block assembly algorithm, + /// but the transaction otherwise may be valid (e.g., block assembly is + /// only considering STX transfers and this tx isn't a transfer). + StacksTransactionSkipped, PostConditionFailed(String), NoSuchBlockError, InvalidChainstateDB, @@ -229,6 +234,9 @@ impl fmt::Display for Error { Error::PoxAlreadyLocked => write!(f, "Account has already locked STX for PoX"), Error::PoxInsufficientBalance => write!(f, "Not enough STX to lock"), Error::PoxNoRewardCycle => write!(f, "No such reward cycle"), + Error::StacksTransactionSkipped => { + write!(f, "Stacks transaction skipped during assembly") + } } } } @@ -261,6 +269,7 @@ impl error::Error for Error { Error::PoxAlreadyLocked => None, Error::PoxInsufficientBalance => None, Error::PoxNoRewardCycle => None, + Error::StacksTransactionSkipped => None, } } } @@ -293,6 +302,7 @@ impl Error { Error::PoxAlreadyLocked => "PoxAlreadyLocked", Error::PoxInsufficientBalance => "PoxInsufficientBalance", Error::PoxNoRewardCycle => "PoxNoRewardCycle", + Error::StacksTransactionSkipped => "StacksTransactionSkipped", } } diff --git a/src/core/mempool.rs b/src/core/mempool.rs index 36d1c2980c..166784fe9e 100644 --- a/src/core/mempool.rs +++ b/src/core/mempool.rs @@ -40,6 +40,7 @@ use chainstate::stacks::{ use core::FIRST_BURNCHAIN_CONSENSUS_HASH; use core::FIRST_STACKS_BLOCK_HASH; use monitoring::increment_stx_mempool_gc; +use std::time::Instant; use util::db::query_row_columns; use util::db::query_rows; use util::db::tx_begin_immediate; @@ -55,7 +56,11 @@ use vm::types::PrincipalData; use clarity_vm::clarity::ClarityConnection; +use crate::chainstate::stacks::events::StacksTransactionReceipt; use crate::codec::StacksMessageCodec; +use crate::cost_estimates::metrics::CostMetric; +use crate::cost_estimates::CostEstimator; +use crate::cost_estimates::EstimatorError; use crate::monitoring; use crate::types::chainstate::{BlockHeaderHash, StacksAddress, StacksBlockHeader}; @@ -103,6 +108,21 @@ pub enum MemPoolDropReason { TOO_EXPENSIVE, } +pub struct ConsiderTransaction { + /// Transaction to consider in block assembly + pub tx: MemPoolTxInfo, + /// If `update_estimator` is set, the iteration should update the estimator + /// after considering the tx. + pub update_estimate: bool, +} + +enum ConsiderTransactionResult { + NO_TRANSACTIONS, + UPDATE_NONCES(Vec), + /// This transaction should be considered for inclusion in the block. + CONSIDER(ConsiderTransaction), +} + impl std::fmt::Display for MemPoolDropReason { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { @@ -136,6 +156,8 @@ pub struct MemPoolTxMetadata { pub origin_nonce: u64, pub sponsor_address: StacksAddress, pub sponsor_nonce: u64, + pub last_known_origin_nonce: Option, + pub last_known_sponsor_nonce: Option, pub accept_time: u64, } @@ -175,26 +197,30 @@ impl FromRow for MemPoolTxMetadata { let consensus_hash = ConsensusHash::from_column(row, "consensus_hash")?; let block_header_hash = BlockHeaderHash::from_column(row, "block_header_hash")?; let tx_fee = u64::from_column(row, "tx_fee")?; - let height = u64::from_column(row, "height")?; + let block_height = u64::from_column(row, "height")?; let len = u64::from_column(row, "length")?; - let ts = u64::from_column(row, "accept_time")?; + let accept_time = u64::from_column(row, "accept_time")?; let origin_address = StacksAddress::from_column(row, "origin_address")?; let origin_nonce = u64::from_column(row, "origin_nonce")?; let sponsor_address = StacksAddress::from_column(row, "sponsor_address")?; let sponsor_nonce = u64::from_column(row, "sponsor_nonce")?; + let last_known_sponsor_nonce = u64::from_column(row, "last_known_sponsor_nonce")?; + let last_known_origin_nonce = u64::from_column(row, "last_known_origin_nonce")?; Ok(MemPoolTxMetadata { - txid: txid, - tx_fee: tx_fee, - len: len, - consensus_hash: consensus_hash, - block_header_hash: block_header_hash, - block_height: height, - accept_time: ts, - origin_address: origin_address, - origin_nonce: origin_nonce, - sponsor_address: sponsor_address, - sponsor_nonce: sponsor_nonce, + txid, + len, + tx_fee, + consensus_hash, + block_header_hash, + block_height, + origin_address, + origin_nonce, + sponsor_address, + sponsor_nonce, + last_known_origin_nonce, + last_known_sponsor_nonce, + accept_time, }) } } @@ -255,6 +281,24 @@ const MEMPOOL_INITIAL_SCHEMA: &'static [&'static str] = &[ "CREATE INDEX by_chaintip ON mempool(consensus_hash,block_header_hash);", ]; +const MEMPOOL_SCHEMA_2: &'static [&'static str] = &[ + r#" + CREATE TABLE fee_estimates( + txid TEXT NOT NULL, + fee_rate NUMBER, + PRIMARY KEY (txid), + FOREIGN KEY (txid) REFERENCES mempool (txid) ON DELETE CASCADE ON UPDATE CASCADE + ); + "#, + r#" + ALTER TABLE mempool ADD COLUMN last_known_origin_nonce TEXT; + "#, + r#" + ALTER TABLE mempool ADD COLUMN last_known_sponsor_nonce TEXT; + "#, + "CREATE INDEX by_txid ON mempool(txid);", +]; + pub struct MemPoolDB { db: DBConn, path: String, @@ -311,22 +355,21 @@ impl MemPoolTxInfo { }; let metadata = MemPoolTxMetadata { - txid: txid, + txid, len: tx_data.len() as u64, tx_fee: tx.get_tx_fee(), - consensus_hash: consensus_hash, - block_header_hash: block_header_hash, - block_height: block_height, - origin_address: origin_address, - origin_nonce: origin_nonce, - sponsor_address: sponsor_address, - sponsor_nonce: sponsor_nonce, + consensus_hash, + block_header_hash, + block_height, + origin_address, + origin_nonce, + sponsor_address, + sponsor_nonce, accept_time: get_epoch_time_secs(), + last_known_origin_nonce: None, + last_known_sponsor_nonce: None, }; - MemPoolTxInfo { - tx: tx, - metadata: metadata, - } + MemPoolTxInfo { tx, metadata } } } @@ -393,6 +436,8 @@ impl MemPoolDB { conn.busy_handler(Some(tx_busy_handler)) .map_err(db_error::SqliteError)?; + conn.execute_batch("PRAGMA foreign_keys = ON;")?; + if create_flag { // instantiate! MemPoolDB::instantiate_mempool_db(&mut conn)?; @@ -401,10 +446,84 @@ impl MemPoolDB { Ok(MemPoolDB { db: conn, path: db_path, - admitter: admitter, + admitter, }) } + pub fn reset_last_known_nonces(&mut self) -> Result<(), db_error> { + let sql = + "UPDATE mempool SET last_known_origin_nonce = NULL, last_known_sponsor_nonce = NULL"; + self.db.execute(sql, rusqlite::NO_PARAMS)?; + Ok(()) + } + + fn bump_last_known_nonces(&self, address: &StacksAddress) -> Result<(), db_error> { + let query_by = address.to_string(); + + let sql = "UPDATE mempool SET last_known_origin_nonce = last_known_origin_nonce + 1 + WHERE origin_address = ? AND last_known_origin_nonce IS NOT NULL"; + self.db.execute(sql, &[&query_by])?; + + let sql = "UPDATE mempool SET last_known_sponsor_nonce = last_known_sponsor_nonce + 1 + WHERE sponsor_address = ? AND last_known_sponsor_nonce IS NOT NULL"; + self.db.execute(sql, &[&query_by])?; + Ok(()) + } + + fn update_last_known_nonces( + &self, + address: &StacksAddress, + nonce: u64, + ) -> Result<(), db_error> { + let addr_str = address.to_string(); + let nonce_i64 = u64_to_sql(nonce)?; + + let sql = "UPDATE mempool SET last_known_origin_nonce = ? WHERE origin_address = ?"; + self.db + .execute(sql, rusqlite::params![nonce_i64, &addr_str])?; + + let sql = "UPDATE mempool SET last_known_sponsor_nonce = ? WHERE sponsor_address = ?"; + self.db + .execute(sql, rusqlite::params![nonce_i64, &addr_str])?; + Ok(()) + } + + fn get_next_tx_to_consider(&self) -> Result { + let select_estimate = "SELECT * FROM mempool WHERE + ((origin_nonce = last_known_origin_nonce + 1 AND + sponsor_nonce = last_known_sponsor_nonce + 1) OR (last_known_origin_nonce is NULL) OR (last_known_sponsor_nonce is NULL)) + AND estimated_fee_rate IS NOT NULL ORDER BY estimated_fee_rate DESC LIMIT 1"; + let select_no_estimate = "SELECT * FROM mempool WHERE + ((origin_nonce = last_known_origin_nonce + 1 AND + sponsor_nonce = last_known_sponsor_nonce + 1) OR (last_known_origin_nonce is NULL) OR (last_known_sponsor_nonce is NULL)) + AND estimated_fee_rate IS NULL ORDER BY tx_fee DESC LIMIT 1"; + let (next_tx, update_estimate): (MemPoolTxInfo, bool) = + match query_row(&self.db, select_estimate, rusqlite::NO_PARAMS)? { + Some(tx) => (tx, false), + None => match query_row(&self.db, select_no_estimate, rusqlite::NO_PARAMS)? { + Some(tx) => (tx, true), + None => return Ok(ConsiderTransactionResult::NO_TRANSACTIONS), + }, + }; + + let mut needs_nonces = vec![]; + if next_tx.metadata.last_known_origin_nonce.is_none() { + needs_nonces.push(next_tx.metadata.origin_address); + } + if next_tx.metadata.last_known_sponsor_nonce.is_none() { + needs_nonces.push(next_tx.metadata.sponsor_address); + } + + if !needs_nonces.is_empty() { + Ok(ConsiderTransactionResult::UPDATE_NONCES(needs_nonces)) + } else { + Ok(ConsiderTransactionResult::CONSIDER(ConsiderTransaction { + tx: next_tx, + update_estimate, + })) + } + } + /// Find the origin addresses who have sent the highest-fee transactions fn find_origin_addresses_by_descending_fees( &self, @@ -414,7 +533,8 @@ impl MemPoolDB { offset: u32, count: u32, ) -> Result, db_error> { - let sql = "SELECT DISTINCT origin_address FROM mempool WHERE height > ?1 AND height <= ?2 AND tx_fee >= ?3 ORDER BY tx_fee DESC LIMIT ?4 OFFSET ?5"; + let sql = "SELECT DISTINCT origin_address FROM mempool WHERE height > ?1 AND height <= ?2 AND tx_fee >= ?3 + ORDER BY tx_fee DESC LIMIT ?4 OFFSET ?5"; let args: &[&dyn ToSql] = &[ &start_height, &end_height, @@ -425,6 +545,53 @@ impl MemPoolDB { query_row_columns(self.conn(), sql, args, "origin_address") } + /// Add estimated fee rates to the mempool rate table using + /// the supplied `CostMetric` and `CostEstimator`. Will update + /// at most `max_updates` entries in the database before returning. + /// + /// Returns `Ok(number_updated)` on success + pub fn estimate_tx_rates( + &mut self, + estimator: &CE, + metric: &CM, + max_updates: u32, + ) -> Result { + let sql_tx = self.tx_begin()?; + let txs: Vec = query_rows( + &sql_tx, + "SELECT * FROM mempool as m LEFT OUTER JOIN fee_estimates as f ON + m.txid = f.txid WHERE f.fee_rate IS NULL LIMIT ?", + &[max_updates], + )?; + let mut updated = 0; + for tx_to_estimate in txs { + let txid = tx_to_estimate.tx.txid(); + let cost_estimate = match estimator.estimate_cost(&tx_to_estimate.tx.payload) { + Ok(x) => x, + Err(EstimatorError::NoEstimateAvailable) => continue, + Err(e) => { + warn!("Error while estimating mempool tx rate"; + "txid" => %txid, + "error" => ?e); + continue; + } + }; + let metric_estimate = + metric.from_cost_and_len(&cost_estimate, tx_to_estimate.tx.tx_len()); + let fee_rate_f64 = tx_to_estimate.tx.get_tx_fee() as f64 / metric_estimate as f64; + + sql_tx.execute( + "INSERT OR REPLACE INTO fee_estimates(txid, fee_rate) VALUES (?, ?)", + rusqlite::params![&txid, fee_rate_f64], + )?; + updated += 1; + } + + sql_tx.commit()?; + + Ok(updated) + } + /// /// Iterate over candidates in the mempool /// `todo` will be called once for each transaction whose origin nonce is equal @@ -437,114 +604,74 @@ impl MemPoolDB { pub fn iterate_candidates( &self, clarity_tx: &mut C, - tip_height: u64, + _tip_height: u64, settings: MemPoolWalkSettings, mut todo: F, ) -> Result where C: ClarityConnection, - F: FnMut(&mut C, MemPoolTxInfo) -> Result, + F: FnMut(&mut C, &ConsiderTransaction) -> Result, E: From + From, { - let min_height = (tip_height as i64) - .checked_sub((MEMPOOL_MAX_TRANSACTION_AGE + 1) as i64) - .unwrap_or(-1); - let max_height = tip_height as i64; - - let page_size = 1000; - let mut offset = 0; - - let min_tx_fee = settings.min_tx_fee; - - let deadline = get_epoch_time_ms() + (settings.max_walk_time_ms as u128); + let start_time = Instant::now(); let mut total_considered = 0; - let mut total_origins = 0; - test_debug!( - "Mempool walk for {}ms, min tx fee {}", - settings.max_walk_time_ms, - min_tx_fee, - ); + debug!("Mempool walk for {}ms", settings.max_walk_time_ms,); loop { - if deadline <= get_epoch_time_ms() { - debug!("Mempool iteration deadline exceeded"); - break; - } - - let origin_addresses = self.find_origin_addresses_by_descending_fees( - min_height, - max_height, - min_tx_fee, - offset * page_size, - page_size, - )?; - debug!( - "Consider {} origin addresses between {},{} with min_fee {}", - origin_addresses.len(), - min_height, - max_height, - min_tx_fee, - ); - - if origin_addresses.len() == 0 { - debug!("No more origin addresses to consider"); + if start_time.elapsed().as_millis() > settings.max_walk_time_ms as u128 { + debug!("Mempool iteration deadline exceeded"; + "deadline_ms" => settings.max_walk_time_ms); break; } - for origin_address in origin_addresses.iter() { - if deadline <= get_epoch_time_ms() { - debug!("Mempool iteration deadline exceeded"); + match self.get_next_tx_to_consider()? { + ConsiderTransactionResult::NO_TRANSACTIONS => { + debug!("No more transactions to consider in mempool"); break; } - - let min_origin_nonce = StacksChainState::get_account( - clarity_tx, - &PrincipalData::Standard(origin_address.to_owned().into()), - ) - .nonce; - - total_origins += 1; - - debug!( - "Consider mempool transactions from origin address {} nonce {}", - &origin_address, min_origin_nonce - ); - - let sql = "SELECT * FROM mempool WHERE origin_address = ?1 AND height > ?2 AND height <= ?3 AND origin_nonce = ?4 AND tx_fee >= ?5 ORDER BY sponsor_nonce ASC LIMIT 1"; - let args: &[&dyn ToSql] = &[ - &origin_address.to_string(), - &min_height, - &max_height, - &u64_to_sql(min_origin_nonce)?, - &u64_to_sql(min_tx_fee)?, - ]; - - let tx_opt = query_row::(self.conn(), sql, args)?; - if let Some(tx) = tx_opt { + ConsiderTransactionResult::UPDATE_NONCES(addresses) => { + let mut last_addr = None; + for address in addresses.into_iter() { + // do not recheck nonces if the sponsor == origin + if last_addr.as_ref() == Some(&address) { + continue; + } + let min_nonce = + StacksChainState::get_account(clarity_tx, &address.clone().into()) + .nonce; + + self.update_last_known_nonces(&address, min_nonce)?; + last_addr = Some(address) + } + } + ConsiderTransactionResult::CONSIDER(consider) => { + debug!("Consider mempool transaction"; + "txid" => %consider.tx.tx.txid(), + "origin_addr" => %consider.tx.metadata.origin_address, + "sponsor_addr" => %consider.tx.metadata.sponsor_address, + "accept_time" => consider.tx.metadata.accept_time, + "tx_fee" => consider.tx.metadata.tx_fee, + "size" => consider.tx.metadata.len); total_considered += 1; - debug!( - "Consider transaction {} from {} between heights {},{} with nonce = {} and tx_fee = {} and size = {}", - &tx.metadata.txid, - &origin_address, - min_height, - max_height, - min_origin_nonce, - tx.metadata.tx_fee, - tx.metadata.len - ); - - if !todo(clarity_tx, tx)? { - test_debug!("Mempool early return from iteration"); + + if !todo(clarity_tx, &consider)? { + debug!("Mempool iteration early exit from iterator"); break; } + + self.bump_last_known_nonces(&consider.tx.metadata.origin_address)?; + if consider.tx.tx.auth.is_sponsored() { + self.bump_last_known_nonces(&consider.tx.metadata.sponsor_address)?; + } } } - offset += 1; } + debug!( - "Mempool iteration finished; considered {} transactions across {} origin addresses", - total_considered, total_origins + "Mempool iteration finished"; + "considered_txs" => total_considered, + "elapsed_ms" => start_time.elapsed().as_millis() ); Ok(total_considered) } diff --git a/src/cost_estimates/metrics.rs b/src/cost_estimates/metrics.rs index bcdb513c01..6a79d01672 100644 --- a/src/cost_estimates/metrics.rs +++ b/src/cost_estimates/metrics.rs @@ -22,6 +22,11 @@ pub struct ProportionalDotProduct { block_size_limit: u64, } +/// This metric always returns a unit value for all execution costs and tx lengths. +/// When used, this metric will cause block assembly to consider transactions based +/// solely on their raw transaction fee, not any kind of rate estimation. +pub struct UnitMetric; + impl ProportionalDotProduct { pub fn new( block_size_limit: u64, @@ -54,3 +59,13 @@ impl CostMetric for ProportionalDotProduct { self.calculate_len_proportion(tx_len) } } + +impl CostMetric for UnitMetric { + fn from_cost_and_len(&self, _cost: &ExecutionCost, _tx_len: u64) -> u64 { + 1 + } + + fn from_len(&self, _tx_len: u64) -> u64 { + 1 + } +} diff --git a/src/cost_estimates/mod.rs b/src/cost_estimates/mod.rs index b0468c5632..7565e8df48 100644 --- a/src/cost_estimates/mod.rs +++ b/src/cost_estimates/mod.rs @@ -178,3 +178,28 @@ impl FeeEstimator for () { Err(EstimatorError::NoEstimateAvailable) } } + +/// This estimator always returns a unit estimate in all dimensions. +/// This can be paired with the UnitMetric to cause block assembly to consider +/// *only* transaction fees, not performing any kind of rate estimation. +pub struct UnitEstimator; + +impl CostEstimator for UnitEstimator { + fn notify_event( + &mut self, + _tx: &TransactionPayload, + _actual_cost: &ExecutionCost, + ) -> Result<(), EstimatorError> { + Ok(()) + } + + fn estimate_cost(&self, _tx: &TransactionPayload) -> Result { + Ok(ExecutionCost { + write_length: 1, + write_count: 1, + read_length: 1, + read_count: 1, + runtime: 1, + }) + } +} diff --git a/src/main.rs b/src/main.rs index 2f8e869f5f..01442821f2 100644 --- a/src/main.rs +++ b/src/main.rs @@ -33,6 +33,8 @@ use std::process; use std::{collections::HashMap, env}; use std::{convert::TryFrom, fs}; +use blockstack_lib::cost_estimates::UnitEstimator; +use cost_estimates::metrics::UnitMetric; use rusqlite::types::ToSql; use rusqlite::Connection; use rusqlite::OpenFlags; @@ -486,6 +488,9 @@ simulating a miner. settings.max_miner_time_ms = max_time; settings.mempool_settings.min_tx_fee = min_fee; + let mut estimator = UnitEstimator; + let metric = UnitMetric; + let result = StacksBlockBuilder::build_anchored_block( &chain_state, &sort_db.index_conn(), @@ -497,6 +502,8 @@ simulating a miner. &coinbase_tx, settings, None, + &mut estimator, + &metric, ); let stop = get_epoch_time_ms(); diff --git a/src/net/download.rs b/src/net/download.rs index 7614d64db5..90d3571865 100644 --- a/src/net/download.rs +++ b/src/net/download.rs @@ -2554,6 +2554,9 @@ pub mod test { use vm::costs::ExecutionCost; use vm::representations::*; + use crate::cost_estimates::metrics::UnitMetric; + use crate::cost_estimates::UnitEstimator; + use super::*; fn get_peer_availability( @@ -3670,6 +3673,9 @@ pub mod test { let coinbase_tx = make_coinbase_with_nonce(miner, i, (i + 2) as u64); + let mut estimator = UnitEstimator; + let metric = UnitMetric; + let (anchored_block, block_size, block_execution_cost) = StacksBlockBuilder::build_anchored_block( chainstate, @@ -3682,6 +3688,8 @@ pub mod test { &coinbase_tx, BlockBuilderSettings::max_value(), None, + &mut estimator, + &metric, ) .unwrap(); (anchored_block, vec![]) diff --git a/src/util/db.rs b/src/util/db.rs index d8fa5b4646..dde863feea 100644 --- a/src/util/db.rs +++ b/src/util/db.rs @@ -179,6 +179,21 @@ impl FromColumn for u64 { } } +impl FromColumn> for u64 { + fn from_column<'a>(row: &'a Row, column_name: &str) -> Result, Error> { + let x: Option = row.get_unwrap(column_name); + match x { + Some(x) => { + if x < 0 { + return Err(Error::ParseError); + } + Ok(Some(x as u64)) + } + None => Ok(None), + } + } +} + impl FromRow for i64 { fn from_row<'a>(row: &'a Row) -> Result { let x: i64 = row.get_unwrap(0); From d5d94cfb2d27f72cfb333d5723b98d8337ff7afd Mon Sep 17 00:00:00 2001 From: Aaron Blankstein Date: Thu, 23 Sep 2021 13:29:48 -0500 Subject: [PATCH 02/24] chore: integrate into neon_node. requires Box --- src/chainstate/stacks/miner.rs | 4 +- src/core/mempool.rs | 2 +- testnet/stacks-node/src/neon_node.rs | 76 +++++++++++++++++++++++----- testnet/stacks-node/src/tenure.rs | 6 +++ 4 files changed, 73 insertions(+), 15 deletions(-) diff --git a/src/chainstate/stacks/miner.rs b/src/chainstate/stacks/miner.rs index 1c2a272cb0..cae70b6c49 100644 --- a/src/chainstate/stacks/miner.rs +++ b/src/chainstate/stacks/miner.rs @@ -448,7 +448,7 @@ impl<'a> StacksMicroblockBuilder<'a> { return self.make_next_microblock(txs_included, miner_key); } - pub fn mine_next_microblock( + pub fn mine_next_microblock( &mut self, mem_pool: &mut MemPoolDB, miner_key: &Secp256k1PrivateKey, @@ -1444,7 +1444,7 @@ impl StacksBlockBuilder { /// Given access to the mempool, mine an anchored block with no more than the given execution cost. /// returns the assembled block, and the consumed execution budget. - pub fn build_anchored_block( + pub fn build_anchored_block( chainstate_handle: &StacksChainState, // not directly used; used as a handle to open other chainstates burn_dbconn: &SortitionDBConn, mempool: &mut MemPoolDB, diff --git a/src/core/mempool.rs b/src/core/mempool.rs index 166784fe9e..b5014e877f 100644 --- a/src/core/mempool.rs +++ b/src/core/mempool.rs @@ -550,7 +550,7 @@ impl MemPoolDB { /// at most `max_updates` entries in the database before returning. /// /// Returns `Ok(number_updated)` on success - pub fn estimate_tx_rates( + pub fn estimate_tx_rates( &mut self, estimator: &CE, metric: &CM, diff --git a/testnet/stacks-node/src/neon_node.rs b/testnet/stacks-node/src/neon_node.rs index 99097f3c70..a0805897a1 100644 --- a/testnet/stacks-node/src/neon_node.rs +++ b/testnet/stacks-node/src/neon_node.rs @@ -27,6 +27,7 @@ use stacks::chainstate::stacks::db::{ }; use stacks::chainstate::stacks::Error as ChainstateError; use stacks::chainstate::stacks::StacksPublicKey; +use stacks::chainstate::stacks::MAX_BLOCK_LEN; use stacks::chainstate::stacks::{ miner::BlockBuilderSettings, miner::StacksMicroblockBuilder, StacksBlockBuilder, }; @@ -37,6 +38,11 @@ use stacks::chainstate::stacks::{ use stacks::codec::StacksMessageCodec; use stacks::core::mempool::MemPoolDB; use stacks::core::FIRST_BURNCHAIN_CONSENSUS_HASH; +use stacks::cost_estimates::metrics::CostMetric; +use stacks::cost_estimates::metrics::ProportionalDotProduct; +use stacks::cost_estimates::metrics::UnitMetric; +use stacks::cost_estimates::CostEstimator; +use stacks::cost_estimates::UnitEstimator; use stacks::monitoring::{increment_stx_blocks_mined_counter, update_active_miners_count_gauge}; use stacks::net::{ atlas::{AtlasConfig, AtlasDB, AttachmentInstance}, @@ -60,6 +66,8 @@ use stacks::vm::costs::ExecutionCost; use stacks::{burnchains::BurnchainSigner, chainstate::stacks::db::StacksHeaderInfo}; use crate::burnchains::bitcoin_regtest_controller::BitcoinRegtestController; +use crate::config::CostEstimatorName; +use crate::config::CostMetricName; use crate::run_loop::RegisteredKey; use crate::syncctl::PoxSyncWatchdogComms; use crate::ChainTip; @@ -324,11 +332,13 @@ fn inner_generate_block_commit_op( } /// Mine and broadcast a single microblock, unconditionally. -fn mine_one_microblock( +fn mine_one_microblock( microblock_state: &mut MicroblockMinerState, sortdb: &SortitionDB, chainstate: &mut StacksChainState, - mempool: &MemPoolDB, + mempool: &mut MemPoolDB, + estimator: &mut CE, + metric: &CM, ) -> Result { debug!( "Try to mine one microblock off of {}/{} (total: {})", @@ -363,7 +373,13 @@ fn mine_one_microblock( }; let t1 = get_epoch_time_ms(); - let mblock = microblock_miner.mine_next_microblock(mempool, µblock_state.miner_key)?; + + let mblock = microblock_miner.mine_next_microblock( + mempool, + µblock_state.miner_key, + estimator, + metric, + )?; let new_cost_so_far = microblock_miner.get_cost_so_far().expect("BUG: cannot read cost so far from miner -- indicates that the underlying Clarity Tx is somehow in use still."); let t2 = get_epoch_time_ms(); @@ -399,13 +415,15 @@ fn mine_one_microblock( return Ok(mined_microblock); } -fn try_mine_microblock( +fn try_mine_microblock( config: &Config, microblock_miner_state: &mut Option, chainstate: &mut StacksChainState, sortdb: &SortitionDB, - mem_pool: &MemPoolDB, + mem_pool: &mut MemPoolDB, winning_tip: (ConsensusHash, BlockHeaderHash, Secp256k1PrivateKey), + estimator: &mut CE, + metric: &CM, ) -> Result, NetError> { let ch = winning_tip.0; let bhh = winning_tip.1; @@ -466,8 +484,14 @@ fn try_mine_microblock( get_epoch_time_secs() - 600, )?; if num_attachable == 0 { - match mine_one_microblock(&mut microblock_miner, sortdb, chainstate, &mem_pool) - { + match mine_one_microblock( + &mut microblock_miner, + sortdb, + chainstate, + mem_pool, + estimator, + metric, + ) { Ok(microblock) => { // will need to relay this next_microblock = Some(microblock); @@ -493,16 +517,18 @@ fn try_mine_microblock( Ok(next_microblock) } -fn run_microblock_tenure( +fn run_microblock_tenure( config: &Config, microblock_miner_state: &mut Option, chainstate: &mut StacksChainState, sortdb: &mut SortitionDB, - mem_pool: &MemPoolDB, + mem_pool: &mut MemPoolDB, relayer: &mut Relayer, miner_tip: (ConsensusHash, BlockHeaderHash, Secp256k1PrivateKey), microblocks_processed: BlocksProcessedCounter, event_dispatcher: &EventDispatcher, + estimator: &mut CE, + metric: &CM, ) { // TODO: this is sensitive to poll latency -- can we call this on a fixed // schedule, regardless of network activity? @@ -522,6 +548,8 @@ fn run_microblock_tenure( sortdb, mem_pool, miner_tip.clone(), + estimator, + metric, ) { Ok(x) => x, Err(e) => { @@ -839,6 +867,20 @@ fn spawn_miner_relayer( let mut last_tenure_issue_time = 0; let relayer_handle = thread::Builder::new().name("relayer".to_string()).spawn(move || { + let mut cost_estimator: Box = match config.estimation.cost_estimator { + Some(CostEstimatorName::NaivePessimistic) => + Box::new( + config.estimation + .make_pessimistic_cost_estimator(config.get_chainstate_path())), + None => Box::new(UnitEstimator), + }; + + let metric: Box = match config.estimation.cost_metric { + Some(CostMetricName::ProportionDotProduct) => + Box::new(ProportionalDotProduct::new(MAX_BLOCK_LEN as u64, config.block_limit.clone())), + None => Box::new(UnitMetric), + }; + while let Ok(mut directive) = relay_channel.recv() { match directive { RelayerDirective::HandleNetResult(ref mut net_result) => { @@ -1048,6 +1090,8 @@ fn spawn_miner_relayer( &mut bitcoin_controller, &last_mined_blocks_vec.iter().map(|(blk, _)| blk).collect(), &event_dispatcher, + cost_estimator.as_mut(), + metric.as_ref(), ); if let Some((last_mined_block, microblock_privkey)) = last_mined_block_opt { if last_mined_blocks_vec.len() == 0 { @@ -1096,11 +1140,13 @@ fn spawn_miner_relayer( &mut microblock_miner_state, &mut chainstate, &mut sortdb, - &mem_pool, + &mut mem_pool, &mut relayer, (ch, bh, mblock_pkey), microblocks_processed.clone(), - &event_dispatcher + &event_dispatcher, + cost_estimator.as_mut(), + metric.as_ref(), ); // synchronize unconfirmed tx index to p2p thread @@ -1531,7 +1577,7 @@ impl InitializedNeonNode { /// Return the assembled anchor block info and microblock private key on success. /// Return None if we couldn't build a block for whatever reason - fn relayer_run_tenure( + fn relayer_run_tenure( config: &Config, registered_key: RegisteredKey, chain_state: &mut StacksChainState, @@ -1544,6 +1590,8 @@ impl InitializedNeonNode { bitcoin_controller: &mut BitcoinRegtestController, last_mined_blocks: &Vec<&AssembledAnchorBlock>, event_observer: &EventDispatcher, + estimator: &mut CE, + metric: &CM, ) -> Option<(AssembledAnchorBlock, Secp256k1PrivateKey)> { let MiningTenureInformation { mut stacks_parent_header, @@ -1830,6 +1878,8 @@ impl InitializedNeonNode { &coinbase_tx, config.make_block_builder_settings((last_mined_blocks.len() + 1) as u64), Some(event_observer), + estimator, + metric, ) { Ok(block) => block, Err(ChainstateError::InvalidStacksMicroblock(msg, mblock_header_hash)) => { @@ -1870,6 +1920,8 @@ impl InitializedNeonNode { &coinbase_tx, config.make_block_builder_settings((last_mined_blocks.len() + 1) as u64), Some(event_observer), + estimator, + metric, ) { Ok(block) => block, Err(e) => { diff --git a/testnet/stacks-node/src/tenure.rs b/testnet/stacks-node/src/tenure.rs index bb5a336d0d..f9a70eb41b 100644 --- a/testnet/stacks-node/src/tenure.rs +++ b/testnet/stacks-node/src/tenure.rs @@ -11,6 +11,8 @@ use stacks::chainstate::stacks::{ StacksPrivateKey, StacksPublicKey, StacksTransaction, }; use stacks::core::mempool::MemPoolDB; +use stacks::cost_estimates::metrics::UnitMetric; +use stacks::cost_estimates::UnitEstimator; use stacks::types::chainstate::VRFSeed; use stacks::util::hash::Hash160; use stacks::util::vrf::VRFProof; @@ -84,6 +86,8 @@ impl<'a> Tenure { ) .unwrap(); + let mut estimator = UnitEstimator; + let metric = UnitMetric; let (anchored_block, _, _) = StacksBlockBuilder::build_anchored_block( &mut chain_state, burn_dbconn, @@ -95,6 +99,8 @@ impl<'a> Tenure { &self.coinbase_tx, BlockBuilderSettings::limited(self.config.block_limit.clone()), None, + &mut estimator, + &metric, ) .unwrap(); From 81a5afe42a408c4cae80ccd01cd9af599030a83c Mon Sep 17 00:00:00 2001 From: Aaron Blankstein Date: Fri, 24 Sep 2021 10:17:45 -0500 Subject: [PATCH 03/24] apply schema changes if necessary --- src/core/mempool.rs | 36 +++++++++++++++++++++++++++++++----- 1 file changed, 31 insertions(+), 5 deletions(-) diff --git a/src/core/mempool.rs b/src/core/mempool.rs index b5014e877f..f17078f309 100644 --- a/src/core/mempool.rs +++ b/src/core/mempool.rs @@ -281,6 +281,10 @@ const MEMPOOL_INITIAL_SCHEMA: &'static [&'static str] = &[ "CREATE INDEX by_chaintip ON mempool(consensus_hash,block_header_hash);", ]; +const MEMPOOL_SCHEMA_2_TEST: &'static str = " + SELECT name FROM sqlite_master WHERE type='table' AND name='fee_estimates' +"; + const MEMPOOL_SCHEMA_2: &'static [&'static str] = &[ r#" CREATE TABLE fee_estimates( @@ -296,7 +300,7 @@ const MEMPOOL_SCHEMA_2: &'static [&'static str] = &[ r#" ALTER TABLE mempool ADD COLUMN last_known_sponsor_nonce TEXT; "#, - "CREATE INDEX by_txid ON mempool(txid);", + "CREATE INDEX fee_by_txid ON fee_estimates(txid);", ]; pub struct MemPoolDB { @@ -443,6 +447,8 @@ impl MemPoolDB { MemPoolDB::instantiate_mempool_db(&mut conn)?; } + MemPoolDB::apply_schema_2_if_needed(&mut conn)?; + Ok(MemPoolDB { db: conn, path: db_path, @@ -450,6 +456,26 @@ impl MemPoolDB { }) } + fn apply_schema_2_if_needed(conn: &mut DBConn) -> Result<(), db_error> { + let tx = conn.transaction()?; + let needs_to_apply = tx + .query_row(MEMPOOL_SCHEMA_2_TEST, rusqlite::NO_PARAMS, |row| { + row.get::<_, String>(0) + }) + .optional()? + .is_none(); + + if needs_to_apply { + for sql_exec in MEMPOOL_SCHEMA_2 { + tx.execute_batch(sql_exec)?; + } + } + + tx.commit()?; + + Ok(()) + } + pub fn reset_last_known_nonces(&mut self) -> Result<(), db_error> { let sql = "UPDATE mempool SET last_known_origin_nonce = NULL, last_known_sponsor_nonce = NULL"; @@ -489,14 +515,14 @@ impl MemPoolDB { } fn get_next_tx_to_consider(&self) -> Result { - let select_estimate = "SELECT * FROM mempool WHERE + let select_estimate = "SELECT * FROM mempool LEFT OUTER JOIN fee_estimates as f WHERE ((origin_nonce = last_known_origin_nonce + 1 AND sponsor_nonce = last_known_sponsor_nonce + 1) OR (last_known_origin_nonce is NULL) OR (last_known_sponsor_nonce is NULL)) - AND estimated_fee_rate IS NOT NULL ORDER BY estimated_fee_rate DESC LIMIT 1"; - let select_no_estimate = "SELECT * FROM mempool WHERE + AND f.fee_rate IS NOT NULL ORDER BY f.fee_rate DESC LIMIT 1"; + let select_no_estimate = "SELECT * FROM mempool LEFT OUTER JOIN fee_estimates as f WHERE ((origin_nonce = last_known_origin_nonce + 1 AND sponsor_nonce = last_known_sponsor_nonce + 1) OR (last_known_origin_nonce is NULL) OR (last_known_sponsor_nonce is NULL)) - AND estimated_fee_rate IS NULL ORDER BY tx_fee DESC LIMIT 1"; + AND f.fee_rate IS NULL ORDER BY tx_fee DESC LIMIT 1"; let (next_tx, update_estimate): (MemPoolTxInfo, bool) = match query_row(&self.db, select_estimate, rusqlite::NO_PARAMS)? { Some(tx) => (tx, false), From 04720315161347aadc56e70dc26b0e4fa2218f9e Mon Sep 17 00:00:00 2001 From: Aaron Blankstein Date: Fri, 24 Sep 2021 12:06:44 -0500 Subject: [PATCH 04/24] various fixes to the iteration --- src/chainstate/stacks/miner.rs | 2 +- src/core/mempool.rs | 16 +++++++++------- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/chainstate/stacks/miner.rs b/src/chainstate/stacks/miner.rs index cae70b6c49..af653efd80 100644 --- a/src/chainstate/stacks/miner.rs +++ b/src/chainstate/stacks/miner.rs @@ -1496,7 +1496,7 @@ impl StacksBlockBuilder { builder.try_mine_tx(&mut epoch_tx, coinbase_tx)?; mempool.reset_last_known_nonces()?; - mempool.estimate_tx_rates(estimator, metric, 100); + mempool.estimate_tx_rates(estimator, metric, 100)?; let mut considered = HashSet::new(); // txids of all transactions we looked at let mut mined_origin_nonces: HashMap = HashMap::new(); // map addrs of mined transaction origins to the nonces we used diff --git a/src/core/mempool.rs b/src/core/mempool.rs index f17078f309..3b0ec95b7b 100644 --- a/src/core/mempool.rs +++ b/src/core/mempool.rs @@ -295,10 +295,10 @@ const MEMPOOL_SCHEMA_2: &'static [&'static str] = &[ ); "#, r#" - ALTER TABLE mempool ADD COLUMN last_known_origin_nonce TEXT; + ALTER TABLE mempool ADD COLUMN last_known_origin_nonce INTEGER; "#, r#" - ALTER TABLE mempool ADD COLUMN last_known_sponsor_nonce TEXT; + ALTER TABLE mempool ADD COLUMN last_known_sponsor_nonce INTEGER; "#, "CREATE INDEX fee_by_txid ON fee_estimates(txid);", ]; @@ -511,17 +511,18 @@ impl MemPoolDB { let sql = "UPDATE mempool SET last_known_sponsor_nonce = ? WHERE sponsor_address = ?"; self.db .execute(sql, rusqlite::params![nonce_i64, &addr_str])?; + Ok(()) } fn get_next_tx_to_consider(&self) -> Result { let select_estimate = "SELECT * FROM mempool LEFT OUTER JOIN fee_estimates as f WHERE - ((origin_nonce = last_known_origin_nonce + 1 AND - sponsor_nonce = last_known_sponsor_nonce + 1) OR (last_known_origin_nonce is NULL) OR (last_known_sponsor_nonce is NULL)) + ((origin_nonce = last_known_origin_nonce AND + sponsor_nonce = last_known_sponsor_nonce) OR (last_known_origin_nonce is NULL) OR (last_known_sponsor_nonce is NULL)) AND f.fee_rate IS NOT NULL ORDER BY f.fee_rate DESC LIMIT 1"; - let select_no_estimate = "SELECT * FROM mempool LEFT OUTER JOIN fee_estimates as f WHERE - ((origin_nonce = last_known_origin_nonce + 1 AND - sponsor_nonce = last_known_sponsor_nonce + 1) OR (last_known_origin_nonce is NULL) OR (last_known_sponsor_nonce is NULL)) + let select_no_estimate = "SELECT * FROM mempool LEFT JOIN fee_estimates as f ON mempool.txid = f.txid WHERE + ((origin_nonce = last_known_origin_nonce AND + sponsor_nonce = last_known_sponsor_nonce) OR (last_known_origin_nonce is NULL) OR (last_known_sponsor_nonce is NULL)) AND f.fee_rate IS NULL ORDER BY tx_fee DESC LIMIT 1"; let (next_tx, update_estimate): (MemPoolTxInfo, bool) = match query_row(&self.db, select_estimate, rusqlite::NO_PARAMS)? { @@ -659,6 +660,7 @@ impl MemPoolDB { ConsiderTransactionResult::UPDATE_NONCES(addresses) => { let mut last_addr = None; for address in addresses.into_iter() { + debug!("Update nonce"; "address" => %address); // do not recheck nonces if the sponsor == origin if last_addr.as_ref() == Some(&address) { continue; From 69f630c4857a855ed765845f4961189af1f2f8e2 Mon Sep 17 00:00:00 2001 From: Aaron Blankstein Date: Mon, 27 Sep 2021 13:13:40 -0500 Subject: [PATCH 05/24] fix mempool test --- src/core/mempool.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/core/mempool.rs b/src/core/mempool.rs index 3b0ec95b7b..423ca1c903 100644 --- a/src/core/mempool.rs +++ b/src/core/mempool.rs @@ -812,7 +812,9 @@ impl MemPoolDB { consensus_hash, block_header_hash, height, - accept_time + accept_time, + last_known_sponsor_nonce, + last_known_origin_nonce FROM mempool WHERE {0}_address = ?1 AND {0}_nonce = ?2", if is_origin { "origin" } else { "sponsor" } ); From 896b9cc6e4557179de91198938cf90e48531af9b Mon Sep 17 00:00:00 2001 From: Aaron Blankstein Date: Tue, 28 Sep 2021 16:23:12 -0500 Subject: [PATCH 06/24] fix mempool test --- src/core/mempool.rs | 88 ++++++++++++++++++++++++++------------------- 1 file changed, 52 insertions(+), 36 deletions(-) diff --git a/src/core/mempool.rs b/src/core/mempool.rs index 423ca1c903..8dbc50ffc3 100644 --- a/src/core/mempool.rs +++ b/src/core/mempool.rs @@ -1386,48 +1386,24 @@ mod tests { tx.set_tx_fee(123); } - let mut underfunded_txs = [ - all_txs.pop().unwrap(), - all_txs.pop().unwrap(), - all_txs.pop().unwrap(), - ]; - for tx in underfunded_txs.iter_mut() { - tx.set_tx_fee(0); - } - for ix in 0..3 { let mut mempool_tx = mempool.tx_begin().unwrap(); let block = &blocks_to_broadcast_in[ix]; let good_tx = &txs[ix]; - let underfunded_tx = &underfunded_txs[ix]; let origin_address = StacksAddress { version: 22, bytes: Hash160::from_data(&[ix as u8; 32]), }; - let underfunded_origin_address = StacksAddress { - version: 26, - bytes: Hash160::from_data(&[ix as u8; 32]), - }; let sponsor_address = StacksAddress { version: 22, bytes: Hash160::from_data(&[0x80 | (ix as u8); 32]), }; - let underfunded_sponsor_address = StacksAddress { - version: 26, - bytes: Hash160::from_data(&[0x80 | (ix as u8); 32]), - }; - for (i, (tx, (origin, sponsor))) in [good_tx, underfunded_tx] + for (i, (tx, (origin, sponsor))) in [good_tx] .iter() - .zip( - [ - (origin_address, sponsor_address), - (underfunded_origin_address, underfunded_sponsor_address), - ] - .iter(), - ) + .zip([(origin_address, sponsor_address)].iter()) .enumerate() { let txid = tx.txid(); @@ -1469,7 +1445,7 @@ mod tests { // // *'d blocks accept transactions, // try to walk at b_4, we should be able to find - // the transaction at b_1, and we should skip all underfunded transactions. + // the transaction at b_1 let mut mempool_settings = MemPoolWalkSettings::default(); mempool_settings.min_tx_fee = 10; @@ -1491,12 +1467,38 @@ mod tests { ) .unwrap(); assert_eq!( - count_txs, 2, - "Mempool should find two transactions from b_2" + count_txs, 3, + "Mempool should find three transactions from b_2" ); }, ); + // Now that the mempool has iterated over those transactions, its view of the + // nonce for the origin address should have changed. Now it should find *no* transactions. + chainstate.with_read_only_clarity_tx( + &NULL_BURN_STATE_DB, + &StacksBlockHeader::make_index_block_hash(&b_2.0, &b_2.1), + |clarity_conn| { + let mut count_txs = 0; + mempool + .iterate_candidates::<_, ChainstateError, _>( + clarity_conn, + 2, + mempool_settings.clone(), + |_, available_tx| { + count_txs += 1; + Ok(true) + }, + ) + .unwrap(); + assert_eq!(count_txs, 0, "Mempool should find no transactions"); + }, + ); + + mempool + .reset_last_known_nonces() + .expect("Should be able to reset nonces"); + chainstate.with_read_only_clarity_tx( &NULL_BURN_STATE_DB, &StacksBlockHeader::make_index_block_hash(&b_5.0, &b_5.1), @@ -1520,6 +1522,12 @@ mod tests { }, ); + mempool + .reset_last_known_nonces() + .expect("Should be able to reset nonces"); + + // The mempool iterator no longer does any consideration of what block accepted + // the transaction, so b_3 should have the same view. chainstate.with_read_only_clarity_tx( &NULL_BURN_STATE_DB, &StacksBlockHeader::make_index_block_hash(&b_3.0, &b_3.1), @@ -1537,12 +1545,16 @@ mod tests { ) .unwrap(); assert_eq!( - count_txs, 2, - "Mempool should find two transactions from b_3" + count_txs, 3, + "Mempool should find three transactions from b_3" ); }, ); + mempool + .reset_last_known_nonces() + .expect("Should be able to reset nonces"); + chainstate.with_read_only_clarity_tx( &NULL_BURN_STATE_DB, &StacksBlockHeader::make_index_block_hash(&b_4.0, &b_4.1), @@ -1566,6 +1578,10 @@ mod tests { }, ); + mempool + .reset_last_known_nonces() + .expect("Should be able to reset nonces"); + // let's test replace-across-fork while we're here. // first try to replace a tx in b_2 in b_1 - should fail because they are in the same fork let mut mempool_tx = mempool.tx_begin().unwrap(); @@ -1612,14 +1628,14 @@ mod tests { mempool_tx.commit().unwrap(); // now try replace-across-fork from b_2 to b_4 - // check that the number of transactions at b_2 and b_4 starts at 2 each + // check that the number of transactions at b_2 and b_4 starts at 1 each assert_eq!( MemPoolDB::get_num_tx_at_block(&mempool.db, &b_4.0, &b_4.1).unwrap(), - 2 + 1 ); assert_eq!( MemPoolDB::get_num_tx_at_block(&mempool.db, &b_2.0, &b_2.1).unwrap(), - 2 + 1 ); let mut mempool_tx = mempool.tx_begin().unwrap(); let block = &b_4; @@ -1668,11 +1684,11 @@ mod tests { // after replace-across-fork, tx[1] should have moved from the b_2->b_5 fork to b_4 assert_eq!( MemPoolDB::get_num_tx_at_block(&mempool.db, &b_4.0, &b_4.1).unwrap(), - 3 + 2 ); assert_eq!( MemPoolDB::get_num_tx_at_block(&mempool.db, &b_2.0, &b_2.1).unwrap(), - 1 + 0 ); } From bc1ef552e347bac4891d3616872e3a61f1c99a4a Mon Sep 17 00:00:00 2001 From: Aaron Blankstein Date: Wed, 29 Sep 2021 10:10:04 -0500 Subject: [PATCH 07/24] fix neon_integrations tests --- testnet/stacks-node/src/tests/neon_integrations.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/testnet/stacks-node/src/tests/neon_integrations.rs b/testnet/stacks-node/src/tests/neon_integrations.rs index e4b7823686..0c8df5e48a 100644 --- a/testnet/stacks-node/src/tests/neon_integrations.rs +++ b/testnet/stacks-node/src/tests/neon_integrations.rs @@ -2001,13 +2001,14 @@ fn filter_low_fee_tx_integration_test() { next_block_and_wait(&mut btc_regtest_controller, &blocks_processed); next_block_and_wait(&mut btc_regtest_controller, &blocks_processed); - // first five accounts are blank + // First five accounts have a transaction. The miner will consider low fee transactions, + // but rank by estimated fee rate. for i in 0..5 { let account = get_account(&http_origin, &spender_addrs[i]); - assert_eq!(account.nonce, 0); + assert_eq!(account.nonce, 1); } - // last five accounts have traction + // last five accounts have transaction for i in 5..10 { let account = get_account(&http_origin, &spender_addrs[i]); assert_eq!(account.nonce, 1); @@ -2134,8 +2135,8 @@ fn mining_transactions_is_fair() { txs.push(tx); } - // spender 1 sends 1 tx - let tx = make_stacks_transfer(&spender_sks[1], 0, 1000, &recipient.into(), 1000); + // spender 1 sends 1 tx, that is roughly the middle rate among the spender[0] transactions + let tx = make_stacks_transfer(&spender_sks[1], 0, 20_000, &recipient.into(), 1000); txs.push(tx); let (mut conf, _) = neon_integration_test_conf(); From ade8353a3281ff68447060ef4a8f81ba53f8e38e Mon Sep 17 00:00:00 2001 From: Aaron Blankstein Date: Wed, 29 Sep 2021 12:55:40 -0500 Subject: [PATCH 08/24] refactor the configuration logic --- src/chainstate/coordinator/mod.rs | 18 +++++------ src/chainstate/stacks/miner.rs | 2 +- src/cost_estimates/metrics.rs | 10 ++++++ testnet/stacks-node/src/config.rs | 41 ++++++++++++++++++++++++ testnet/stacks-node/src/neon_node.rs | 20 +++--------- testnet/stacks-node/src/run_loop/neon.rs | 39 +++------------------- 6 files changed, 70 insertions(+), 60 deletions(-) diff --git a/src/chainstate/coordinator/mod.rs b/src/chainstate/coordinator/mod.rs index 2bd3baa869..84b75ea9fa 100644 --- a/src/chainstate/coordinator/mod.rs +++ b/src/chainstate/coordinator/mod.rs @@ -148,8 +148,8 @@ pub struct ChainsCoordinator< T: BlockEventDispatcher, N: CoordinatorNotices, R: RewardSetProvider, - CE: CostEstimator, - FE: FeeEstimator, + CE: CostEstimator + ?Sized, + FE: FeeEstimator + ?Sized, > { canonical_sortition_tip: Option, canonical_chain_tip: Option, @@ -160,8 +160,8 @@ pub struct ChainsCoordinator< burnchain: Burnchain, attachments_tx: SyncSender>, dispatcher: Option<&'a T>, - cost_estimator: Option, - fee_estimator: Option, + cost_estimator: Option<&'a mut CE>, + fee_estimator: Option<&'a mut FE>, reward_set_provider: R, notifier: N, atlas_config: AtlasConfig, @@ -256,7 +256,7 @@ impl RewardSetProvider for OnChainRewardSetProvider { } } -impl<'a, T: BlockEventDispatcher, CE: CostEstimator, FE: FeeEstimator> +impl<'a, T: BlockEventDispatcher, CE: CostEstimator + ?Sized, FE: FeeEstimator + ?Sized> ChainsCoordinator<'a, T, ArcCounterCoordinatorNotices, OnChainRewardSetProvider, CE, FE> { pub fn run( @@ -266,8 +266,8 @@ impl<'a, T: BlockEventDispatcher, CE: CostEstimator, FE: FeeEstimator> dispatcher: &'a mut T, comms: CoordinatorReceivers, atlas_config: AtlasConfig, - cost_estimator: Option, - fee_estimator: Option, + cost_estimator: Option<&mut CE>, + fee_estimator: Option<&mut FE>, ) where T: BlockEventDispatcher, { @@ -517,8 +517,8 @@ impl< T: BlockEventDispatcher, N: CoordinatorNotices, U: RewardSetProvider, - CE: CostEstimator, - FE: FeeEstimator, + CE: CostEstimator + ?Sized, + FE: FeeEstimator + ?Sized, > ChainsCoordinator<'a, T, N, U, CE, FE> { pub fn handle_new_stacks_block(&mut self) -> Result<(), Error> { diff --git a/src/chainstate/stacks/miner.rs b/src/chainstate/stacks/miner.rs index af653efd80..e8ca688f49 100644 --- a/src/chainstate/stacks/miner.rs +++ b/src/chainstate/stacks/miner.rs @@ -475,7 +475,7 @@ impl<'a> StacksMicroblockBuilder<'a> { let deadline = get_epoch_time_ms() + (self.settings.max_miner_time_ms as u128); mem_pool.reset_last_known_nonces()?; - mem_pool.estimate_tx_rates(estimator, metric, 100); + mem_pool.estimate_tx_rates(estimator, metric, 100)?; debug!( "Microblock transaction selection begins (child of {}), bytes so far: {}", diff --git a/src/cost_estimates/metrics.rs b/src/cost_estimates/metrics.rs index 6a79d01672..bcb9840e0f 100644 --- a/src/cost_estimates/metrics.rs +++ b/src/cost_estimates/metrics.rs @@ -9,6 +9,16 @@ pub trait CostMetric { fn from_len(&self, tx_len: u64) -> u64; } +impl CostMetric for Box { + fn from_cost_and_len(&self, cost: &ExecutionCost, tx_len: u64) -> u64 { + self.as_ref().from_cost_and_len(cost, tx_len) + } + + fn from_len(&self, tx_len: u64) -> u64 { + self.as_ref().from_len(tx_len) + } +} + pub const PROPORTION_RESOLUTION: u64 = 10_000; /// This metric calculates a single dimensional value for a transaction's diff --git a/testnet/stacks-node/src/config.rs b/testnet/stacks-node/src/config.rs index ce8a484eb7..2c065791ae 100644 --- a/testnet/stacks-node/src/config.rs +++ b/testnet/stacks-node/src/config.rs @@ -8,6 +8,7 @@ use rand::RngCore; use stacks::burnchains::bitcoin::BitcoinNetworkType; use stacks::burnchains::{MagicBytes, BLOCKSTACK_MAGIC_MAINNET}; use stacks::chainstate::stacks::miner::BlockBuilderSettings; +use stacks::chainstate::stacks::MAX_BLOCK_LEN; use stacks::core::mempool::MemPoolWalkSettings; use stacks::core::{ BLOCK_LIMIT_MAINNET, CHAIN_ID_MAINNET, CHAIN_ID_TESTNET, HELIUM_BLOCK_LIMIT, @@ -15,6 +16,9 @@ use stacks::core::{ }; use stacks::cost_estimates::fee_scalar::ScalarFeeRateEstimator; use stacks::cost_estimates::metrics::CostMetric; +use stacks::cost_estimates::metrics::ProportionalDotProduct; +use stacks::cost_estimates::CostEstimator; +use stacks::cost_estimates::FeeEstimator; use stacks::cost_estimates::PessimisticEstimator; use stacks::net::connection::ConnectionOptions; use stacks::net::{Neighbor, NeighborKey, PeerAddress}; @@ -1143,6 +1147,43 @@ impl From for FeeEstimationConfig { } } +impl Config { + pub fn make_cost_estimator(&self) -> Option> { + let cost_estimator: Box = + match self.estimation.cost_estimator.as_ref()? { + CostEstimatorName::NaivePessimistic => Box::new( + self.estimation + .make_pessimistic_cost_estimator(self.get_chainstate_path()), + ), + }; + + Some(cost_estimator) + } + + pub fn make_cost_metric(&self) -> Option> { + let metric: Box = match self.estimation.cost_metric.as_ref()? { + CostMetricName::ProportionDotProduct => Box::new(ProportionalDotProduct::new( + MAX_BLOCK_LEN as u64, + self.block_limit.clone(), + )), + }; + + Some(metric) + } + + pub fn make_fee_estimator(&self) -> Option> { + let metric = self.make_cost_metric()?; + let fee_estimator: Box = match self.estimation.fee_estimator.as_ref()? { + FeeEstimatorName::ScalarFeeRate => Box::new( + self.estimation + .make_scalar_fee_estimator(self.get_chainstate_path(), metric), + ), + }; + + Some(fee_estimator) + } +} + impl FeeEstimationConfig { pub fn make_pessimistic_cost_estimator( &self, diff --git a/testnet/stacks-node/src/neon_node.rs b/testnet/stacks-node/src/neon_node.rs index a0805897a1..83576e5e28 100644 --- a/testnet/stacks-node/src/neon_node.rs +++ b/testnet/stacks-node/src/neon_node.rs @@ -27,7 +27,6 @@ use stacks::chainstate::stacks::db::{ }; use stacks::chainstate::stacks::Error as ChainstateError; use stacks::chainstate::stacks::StacksPublicKey; -use stacks::chainstate::stacks::MAX_BLOCK_LEN; use stacks::chainstate::stacks::{ miner::BlockBuilderSettings, miner::StacksMicroblockBuilder, StacksBlockBuilder, }; @@ -39,7 +38,6 @@ use stacks::codec::StacksMessageCodec; use stacks::core::mempool::MemPoolDB; use stacks::core::FIRST_BURNCHAIN_CONSENSUS_HASH; use stacks::cost_estimates::metrics::CostMetric; -use stacks::cost_estimates::metrics::ProportionalDotProduct; use stacks::cost_estimates::metrics::UnitMetric; use stacks::cost_estimates::CostEstimator; use stacks::cost_estimates::UnitEstimator; @@ -66,8 +64,6 @@ use stacks::vm::costs::ExecutionCost; use stacks::{burnchains::BurnchainSigner, chainstate::stacks::db::StacksHeaderInfo}; use crate::burnchains::bitcoin_regtest_controller::BitcoinRegtestController; -use crate::config::CostEstimatorName; -use crate::config::CostMetricName; use crate::run_loop::RegisteredKey; use crate::syncctl::PoxSyncWatchdogComms; use crate::ChainTip; @@ -867,19 +863,11 @@ fn spawn_miner_relayer( let mut last_tenure_issue_time = 0; let relayer_handle = thread::Builder::new().name("relayer".to_string()).spawn(move || { - let mut cost_estimator: Box = match config.estimation.cost_estimator { - Some(CostEstimatorName::NaivePessimistic) => - Box::new( - config.estimation - .make_pessimistic_cost_estimator(config.get_chainstate_path())), - None => Box::new(UnitEstimator), - }; + let mut cost_estimator = config.make_cost_estimator() + .unwrap_or_else(|| Box::new(UnitEstimator)); - let metric: Box = match config.estimation.cost_metric { - Some(CostMetricName::ProportionDotProduct) => - Box::new(ProportionalDotProduct::new(MAX_BLOCK_LEN as u64, config.block_limit.clone())), - None => Box::new(UnitMetric), - }; + let metric = config.make_cost_metric() + .unwrap_or_else(|| Box::new(UnitMetric)); while let Ok(mut directive) = relay_channel.recv() { match directive { diff --git a/testnet/stacks-node/src/run_loop/neon.rs b/testnet/stacks-node/src/run_loop/neon.rs index 4021db6e8e..db28239b51 100644 --- a/testnet/stacks-node/src/run_loop/neon.rs +++ b/testnet/stacks-node/src/run_loop/neon.rs @@ -15,12 +15,9 @@ use stacks::chainstate::coordinator::{ BlockEventDispatcher, ChainsCoordinator, CoordinatorCommunication, }; use stacks::chainstate::stacks::db::{ChainStateBootData, StacksChainState}; -use stacks::chainstate::stacks::MAX_BLOCK_LEN; -use stacks::cost_estimates::metrics::ProportionalDotProduct; use stacks::net::atlas::{AtlasConfig, Attachment}; use stx_genesis::GenesisData; -use crate::config::{CostEstimatorName, CostMetricName, FeeEstimatorName}; use crate::monitoring::start_serving_monitoring_metrics; use crate::node::use_test_genesis_chainstate; use crate::syncctl::PoxSyncWatchdog; @@ -248,39 +245,13 @@ impl RunLoop { let atlas_config = AtlasConfig::default(mainnet); let moved_atlas_config = atlas_config.clone(); - let moved_estimator_config = self.config.estimation.clone(); - let moved_chainstate_path = self.config.get_chainstate_path(); - let moved_block_limit = self.config.block_limit.clone(); + let moved_config = self.config.clone(); let coordinator_thread_handle = thread::Builder::new() .name("chains-coordinator".to_string()) .spawn(move || { - let cost_estimator = match moved_estimator_config.cost_estimator { - Some(CostEstimatorName::NaivePessimistic) => Some( - moved_estimator_config - .make_pessimistic_cost_estimator(moved_chainstate_path.clone()), - ), - None => None, - }; - - let metric = match moved_estimator_config.cost_metric { - Some(CostMetricName::ProportionDotProduct) => Some( - ProportionalDotProduct::new(MAX_BLOCK_LEN as u64, moved_block_limit), - ), - None => None, - }; - - let fee_estimator = match moved_estimator_config.fee_estimator { - Some(FeeEstimatorName::ScalarFeeRate) => { - let metric = metric - .expect("Configured a fee rate estimator without a configure metric."); - Some( - moved_estimator_config - .make_scalar_fee_estimator(moved_chainstate_path, metric), - ) - } - None => None, - }; + let mut cost_estimator = moved_config.make_cost_estimator(); + let mut fee_estimator = moved_config.make_fee_estimator(); ChainsCoordinator::run( chain_state_db, @@ -289,8 +260,8 @@ impl RunLoop { &mut coordinator_dispatcher, coordinator_receivers, moved_atlas_config, - cost_estimator, - fee_estimator, + cost_estimator.as_deref_mut(), + fee_estimator.as_deref_mut(), ); }) .unwrap(); From 53668067af2365a5d5de666bc6b9a011d1a5b153 Mon Sep 17 00:00:00 2001 From: Aaron Blankstein Date: Thu, 7 Oct 2021 10:55:12 -0500 Subject: [PATCH 09/24] calculate estimate on mempool admission --- src/chainstate/stacks/db/blocks.rs | 8 +- src/chainstate/stacks/miner.rs | 123 +++++++-------------------- src/core/mempool.rs | 121 ++++++++++++++++++++------ src/cost_estimates/mod.rs | 15 ++++ src/main.rs | 12 ++- src/net/download.rs | 7 +- src/net/mod.rs | 12 ++- src/net/server.rs | 11 +-- testnet/stacks-node/src/neon_node.rs | 85 ++++++++---------- testnet/stacks-node/src/node.rs | 24 +++++- testnet/stacks-node/src/tenure.rs | 6 -- 11 files changed, 222 insertions(+), 202 deletions(-) diff --git a/src/chainstate/stacks/db/blocks.rs b/src/chainstate/stacks/db/blocks.rs index 45b449e83e..d5c8583413 100644 --- a/src/chainstate/stacks/db/blocks.rs +++ b/src/chainstate/stacks/db/blocks.rs @@ -9168,12 +9168,10 @@ pub mod test { } }; - let mut mempool = MemPoolDB::open(false, 0x80000000, &chainstate_path).unwrap(); + let mut mempool = + MemPoolDB::open_test(false, 0x80000000, &chainstate_path).unwrap(); let coinbase_tx = make_coinbase(miner, tenure_id); - let mut estimator = UnitEstimator; - let metric = UnitMetric; - let anchored_block = StacksBlockBuilder::build_anchored_block( chainstate, &sortdb.index_conn(), @@ -9185,8 +9183,6 @@ pub mod test { &coinbase_tx, BlockBuilderSettings::max_value(), None, - &mut estimator, - &metric, ) .unwrap(); (anchored_block.0, vec![]) diff --git a/src/chainstate/stacks/miner.rs b/src/chainstate/stacks/miner.rs index e8ca688f49..f820b7d53a 100644 --- a/src/chainstate/stacks/miner.rs +++ b/src/chainstate/stacks/miner.rs @@ -448,12 +448,10 @@ impl<'a> StacksMicroblockBuilder<'a> { return self.make_next_microblock(txs_included, miner_key); } - pub fn mine_next_microblock( + pub fn mine_next_microblock( &mut self, mem_pool: &mut MemPoolDB, miner_key: &Secp256k1PrivateKey, - estimator: &mut CE, - metric: &CM, ) -> Result { let mut txs_included = vec![]; let mempool_settings = self.settings.mempool_settings.clone(); @@ -475,7 +473,7 @@ impl<'a> StacksMicroblockBuilder<'a> { let deadline = get_epoch_time_ms() + (self.settings.max_miner_time_ms as u128); mem_pool.reset_last_known_nonces()?; - mem_pool.estimate_tx_rates(estimator, metric, 100)?; + mem_pool.estimate_tx_rates(100)?; debug!( "Microblock transaction selection begins (child of {}), bytes so far: {}", @@ -489,7 +487,7 @@ impl<'a> StacksMicroblockBuilder<'a> { &mut clarity_tx, self.anchor_block_height, mempool_settings.clone(), - |clarity_tx, to_consider| { + |clarity_tx, to_consider, estimator| { let mempool_tx = &to_consider.tx; let update_estimator = to_consider.update_estimate; @@ -1444,7 +1442,7 @@ impl StacksBlockBuilder { /// Given access to the mempool, mine an anchored block with no more than the given execution cost. /// returns the assembled block, and the consumed execution budget. - pub fn build_anchored_block( + pub fn build_anchored_block( chainstate_handle: &StacksChainState, // not directly used; used as a handle to open other chainstates burn_dbconn: &SortitionDBConn, mempool: &mut MemPoolDB, @@ -1455,8 +1453,6 @@ impl StacksBlockBuilder { coinbase_tx: &StacksTransaction, settings: BlockBuilderSettings, event_observer: Option<&dyn MemPoolEventDispatcher>, - estimator: &mut CE, - metric: &CM, ) -> Result<(StacksBlock, ExecutionCost, u64), Error> { let execution_budget = settings.execution_cost; let mempool_settings = settings.mempool_settings; @@ -1496,7 +1492,7 @@ impl StacksBlockBuilder { builder.try_mine_tx(&mut epoch_tx, coinbase_tx)?; mempool.reset_last_known_nonces()?; - mempool.estimate_tx_rates(estimator, metric, 100)?; + mempool.estimate_tx_rates(100)?; let mut considered = HashSet::new(); // txids of all transactions we looked at let mut mined_origin_nonces: HashMap = HashMap::new(); // map addrs of mined transaction origins to the nonces we used @@ -1520,7 +1516,7 @@ impl StacksBlockBuilder { &mut epoch_tx, tip_height, mempool_settings.clone(), - |epoch_tx, to_consider| { + |epoch_tx, to_consider, estimator| { let txinfo = &to_consider.tx; let update_estimator = to_consider.update_estimate; @@ -6453,13 +6449,11 @@ pub mod test { } }; - let mut mempool = MemPoolDB::open(false, 0x80000000, &chainstate_path).unwrap(); + let mut mempool = + MemPoolDB::open_test(false, 0x80000000, &chainstate_path).unwrap(); let coinbase_tx = make_coinbase(miner, tenure_id); - let mut estimator = UnitEstimator; - let metric = UnitMetric; - let anchored_block = StacksBlockBuilder::build_anchored_block( chainstate, &sortdb.index_conn(), @@ -6471,8 +6465,6 @@ pub mod test { &coinbase_tx, BlockBuilderSettings::max_value(), None, - &mut estimator, - &metric, ) .unwrap(); (anchored_block.0, vec![]) @@ -6562,7 +6554,8 @@ pub mod test { let parent_header_hash = parent_tip.anchored_header.block_hash(); let parent_consensus_hash = parent_tip.consensus_hash.clone(); - let mut mempool = MemPoolDB::open(false, 0x80000000, &chainstate_path).unwrap(); + let mut mempool = + MemPoolDB::open_test(false, 0x80000000, &chainstate_path).unwrap(); let coinbase_tx = make_coinbase(miner, tenure_id); @@ -6587,9 +6580,6 @@ pub mod test { .unwrap(); } - let mut estimator = UnitEstimator; - let metric = UnitMetric; - let anchored_block = StacksBlockBuilder::build_anchored_block( chainstate, &sortdb.index_conn(), @@ -6601,8 +6591,6 @@ pub mod test { &coinbase_tx, BlockBuilderSettings::max_value(), None, - &mut estimator, - &metric, ) .unwrap(); (anchored_block.0, vec![]) @@ -6705,7 +6693,8 @@ pub mod test { let parent_header_hash = parent_tip.anchored_header.block_hash(); let parent_consensus_hash = parent_tip.consensus_hash.clone(); - let mut mempool = MemPoolDB::open(false, 0x80000000, &chainstate_path).unwrap(); + let mut mempool = + MemPoolDB::open_test(false, 0x80000000, &chainstate_path).unwrap(); let coinbase_tx = make_coinbase(miner, tenure_id); @@ -6730,9 +6719,6 @@ pub mod test { .unwrap(); } - let mut estimator = UnitEstimator; - let metric = UnitMetric; - let anchored_block = StacksBlockBuilder::build_anchored_block( chainstate, &sortdb.index_conn(), @@ -6749,8 +6735,6 @@ pub mod test { ..BlockBuilderSettings::max_value() }, None, - &mut estimator, - &metric, ) .unwrap(); (anchored_block.0, vec![]) @@ -6847,7 +6831,8 @@ pub mod test { let parent_header_hash = parent_tip.anchored_header.block_hash(); let parent_consensus_hash = parent_tip.consensus_hash.clone(); - let mut mempool = MemPoolDB::open(false, 0x80000000, &chainstate_path).unwrap(); + let mut mempool = + MemPoolDB::open_test(false, 0x80000000, &chainstate_path).unwrap(); let coinbase_tx = make_coinbase(miner, tenure_id); @@ -6897,9 +6882,6 @@ pub mod test { sender_nonce += 1; } - let mut estimator = UnitEstimator; - let metric = UnitMetric; - let anchored_block = StacksBlockBuilder::build_anchored_block( chainstate, &sortdb.index_conn(), @@ -6911,8 +6893,6 @@ pub mod test { &coinbase_tx, BlockBuilderSettings::max_value(), None, - &mut estimator, - &metric, ) .unwrap(); (anchored_block.0, vec![]) @@ -7046,7 +7026,8 @@ pub mod test { let parent_consensus_hash = parent_tip.consensus_hash.clone(); let coinbase_tx = make_coinbase(miner, tenure_id); - let mut mempool = MemPoolDB::open(false, 0x80000000, &chainstate_path).unwrap(); + let mut mempool = + MemPoolDB::open_test(false, 0x80000000, &chainstate_path).unwrap(); if tenure_id > 0 { let mut expensive_part = vec![]; @@ -7130,9 +7111,6 @@ pub mod test { runtime: 3350, }; - let mut estimator = UnitEstimator; - let metric = UnitMetric; - let anchored_block = StacksBlockBuilder::build_anchored_block( chainstate, &sortdb.index_conn(), @@ -7144,8 +7122,6 @@ pub mod test { &coinbase_tx, BlockBuilderSettings::limited(execution_cost), None, - &mut estimator, - &metric, ) .unwrap(); (anchored_block.0, vec![]) @@ -7208,7 +7184,8 @@ pub mod test { 1, "test_build_anchored_blocks_multiple_chaintips_blank", ); - let mut blank_mempool = MemPoolDB::open(false, 1, &blank_chainstate.root_path).unwrap(); + let mut blank_mempool = + MemPoolDB::open_test(false, 1, &blank_chainstate.root_path).unwrap(); let first_stacks_block_height = { let sn = @@ -7257,7 +7234,8 @@ pub mod test { let parent_consensus_hash = parent_tip.consensus_hash.clone(); let coinbase_tx = make_coinbase(miner, tenure_id); - let mut mempool = MemPoolDB::open(false, 0x80000000, &chainstate_path).unwrap(); + let mut mempool = + MemPoolDB::open_test(false, 0x80000000, &chainstate_path).unwrap(); if tenure_id > 0 { let contract = " @@ -7293,9 +7271,6 @@ pub mod test { &mut mempool }; - let mut estimator = UnitEstimator; - let metric = UnitMetric; - StacksBlockBuilder::build_anchored_block( chainstate, &sortdb.index_conn(), @@ -7307,8 +7282,6 @@ pub mod test { &coinbase_tx, BlockBuilderSettings::limited(execution_cost), None, - &mut estimator, - &metric, ) .unwrap() }; @@ -7404,10 +7377,8 @@ pub mod test { let parent_consensus_hash = parent_tip.consensus_hash.clone(); let coinbase_tx = make_coinbase(miner, tenure_id); - let mut mempool = MemPoolDB::open(false, 0x80000000, &chainstate_path).unwrap(); - - let mut estimator = UnitEstimator; - let metric = UnitMetric; + let mut mempool = + MemPoolDB::open_test(false, 0x80000000, &chainstate_path).unwrap(); let anchored_block = StacksBlockBuilder::build_anchored_block( chainstate, @@ -7420,8 +7391,6 @@ pub mod test { &coinbase_tx, BlockBuilderSettings::max_value(), None, - &mut estimator, - &metric, ) .unwrap(); @@ -7553,7 +7522,8 @@ pub mod test { let parent_consensus_hash = parent_tip.consensus_hash.clone(); let coinbase_tx = make_coinbase(miner, tenure_id); - let mut mempool = MemPoolDB::open(false, 0x80000000, &chainstate_path).unwrap(); + let mut mempool = + MemPoolDB::open_test(false, 0x80000000, &chainstate_path).unwrap(); if tenure_id == 2 { let contract = " @@ -7613,9 +7583,6 @@ pub mod test { sleep_ms(2000); } - let mut estimator = UnitEstimator; - let metric = UnitMetric; - let anchored_block = StacksBlockBuilder::build_anchored_block( chainstate, &sortdb.index_conn(), @@ -7627,8 +7594,6 @@ pub mod test { &coinbase_tx, BlockBuilderSettings::max_value(), None, - &mut estimator, - &metric, ) .unwrap(); @@ -7781,16 +7746,12 @@ pub mod test { miner.set_nonce(miner.get_nonce() - ((bad_block_tenure - bad_block_ancestor_tenure) as u64)); } - let mut mempool = MemPoolDB::open(false, 0x80000000, &chainstate_path).unwrap(); + let mut mempool = MemPoolDB::open_test(false, 0x80000000, &chainstate_path).unwrap(); let coinbase_tx = make_coinbase(miner, tenure_id as usize); - let mut estimator = UnitEstimator; - let metric = UnitMetric; - - let mut anchored_block = StacksBlockBuilder::build_anchored_block(chainstate, &sortdb.index_conn(), &mut mempool, &parent_tip, tip.total_burn, vrf_proof, Hash160([tenure_id as u8; 20]), &coinbase_tx, BlockBuilderSettings::max_value(), None, - &mut estimator, - &metric, + let mut anchored_block = StacksBlockBuilder::build_anchored_block( + chainstate, &sortdb.index_conn(), &mut mempool, &parent_tip, tip.total_burn, vrf_proof, Hash160([tenure_id as u8; 20]), &coinbase_tx, BlockBuilderSettings::max_value(), None, ).unwrap(); if tenure_id == bad_block_tenure { @@ -7930,7 +7891,8 @@ pub mod test { let parent_tip_ch = parent_tip.consensus_hash.clone(); let coinbase_tx = make_coinbase(miner, tenure_id); - let mut mempool = MemPoolDB::open(false, 0x80000000, &chainstate_path).unwrap(); + let mut mempool = + MemPoolDB::open_test(false, 0x80000000, &chainstate_path).unwrap(); if tenure_id == 2 { let contract = " @@ -8052,7 +8014,7 @@ pub mod test { sleep_ms(2000); } - let mut estimator = UnitEstimator; + let estimator = UnitEstimator; let metric = UnitMetric; let anchored_block = StacksBlockBuilder::build_anchored_block( @@ -8066,8 +8028,6 @@ pub mod test { &coinbase_tx, BlockBuilderSettings::max_value(), None, - &mut estimator, - &metric, ) .unwrap(); @@ -8196,7 +8156,7 @@ pub mod test { let parent_index_hash = StacksBlockHeader::make_index_block_hash(&parent_consensus_hash, &parent_header_hash); let parent_size = parent_tip.anchored_block_size; - let mut mempool = MemPoolDB::open(false, 0x80000000, &chainstate_path).unwrap(); + let mut mempool = MemPoolDB::open_test(false, 0x80000000, &chainstate_path).unwrap(); let expected_parent_microblock_opt = if tenure_id > 0 { @@ -8325,10 +8285,6 @@ pub mod test { let mblock_pubkey_hash = Hash160::from_node_public_key(&StacksPublicKey::from_private(&mblock_privks[tenure_id])); - - let mut estimator = UnitEstimator; - let metric = UnitMetric; - let (anchored_block, block_size, block_execution_cost) = StacksBlockBuilder::build_anchored_block( chainstate, &sortdb.index_conn(), @@ -8340,8 +8296,6 @@ pub mod test { &coinbase_tx, BlockBuilderSettings::max_value(), None, - &mut estimator, - &metric, ) .unwrap(); @@ -8529,7 +8483,7 @@ pub mod test { let parent_index_hash = StacksBlockHeader::make_index_block_hash(&parent_consensus_hash, &parent_header_hash); let parent_size = parent_tip.anchored_block_size; - let mut mempool = MemPoolDB::open(false, 0x80000000, &chainstate_path).unwrap(); + let mut mempool = MemPoolDB::open_test(false, 0x80000000, &chainstate_path).unwrap(); let (expected_parent_microblock_opt, fork_1, fork_2) = if tenure_id == 1 { @@ -8753,10 +8707,6 @@ pub mod test { .unwrap(); } - - let mut estimator = UnitEstimator; - let metric = UnitMetric; - let (anchored_block, block_size, block_execution_cost) = StacksBlockBuilder::build_anchored_block( chainstate, &sortdb.index_conn(), @@ -8768,8 +8718,6 @@ pub mod test { &coinbase_tx, BlockBuilderSettings::max_value(), None, - &mut estimator, - &metric, ) .unwrap(); @@ -9028,7 +8976,7 @@ pub mod test { runtime: 5_000_000_000, }; - let mut mempool = MemPoolDB::open(false, chain_id, &chainstate.root_path).unwrap(); + let mut mempool = MemPoolDB::open_test(false, chain_id, &chainstate.root_path).unwrap(); let txs_to_push = vec![ StacksTransaction::new( @@ -9179,9 +9127,6 @@ pub mod test { let coinbase_tx = tx_signer.get_tx().unwrap(); - let mut estimator = UnitEstimator; - let metric = UnitMetric; - let (block, _cost, _size) = StacksBlockBuilder::build_anchored_block( &chainstate, &burndb.index_conn(), @@ -9193,8 +9138,6 @@ pub mod test { &coinbase_tx, BlockBuilderSettings::limited(execution_limit.clone()), None, - &mut estimator, - &metric, ) .unwrap(); diff --git a/src/core/mempool.rs b/src/core/mempool.rs index 8dbc50ffc3..d70d06d6d0 100644 --- a/src/core/mempool.rs +++ b/src/core/mempool.rs @@ -58,9 +58,12 @@ use clarity_vm::clarity::ClarityConnection; use crate::chainstate::stacks::events::StacksTransactionReceipt; use crate::codec::StacksMessageCodec; +use crate::cost_estimates; use crate::cost_estimates::metrics::CostMetric; +use crate::cost_estimates::metrics::UnitMetric; use crate::cost_estimates::CostEstimator; use crate::cost_estimates::EstimatorError; +use crate::cost_estimates::UnitEstimator; use crate::monitoring; use crate::types::chainstate::{BlockHeaderHash, StacksAddress, StacksBlockHeader}; @@ -307,6 +310,8 @@ pub struct MemPoolDB { db: DBConn, path: String, admitter: MemPoolAdmitter, + cost_estimator: Box, + metric: Box, } pub struct MemPoolTx<'a> { @@ -400,12 +405,25 @@ impl MemPoolDB { .map(String::from) } + #[cfg(test)] + pub fn open_test( + mainnet: bool, + chain_id: u32, + chainstate_path: &str, + ) -> Result { + let estimator = Box::new(UnitEstimator); + let metric = Box::new(UnitMetric); + MemPoolDB::open(mainnet, chain_id, chainstate_path, estimator, metric) + } + /// Open the mempool db within the chainstate directory. /// The chainstate must be instantiated already. pub fn open( mainnet: bool, chain_id: u32, chainstate_path: &str, + cost_estimator: Box, + metric: Box, ) -> Result { match fs::metadata(chainstate_path) { Ok(md) => { @@ -453,6 +471,8 @@ impl MemPoolDB { db: conn, path: db_path, admitter, + cost_estimator, + metric, }) } @@ -573,17 +593,12 @@ impl MemPoolDB { } /// Add estimated fee rates to the mempool rate table using - /// the supplied `CostMetric` and `CostEstimator`. Will update + /// the mempool's configured `CostMetric` and `CostEstimator`. Will update /// at most `max_updates` entries in the database before returning. /// /// Returns `Ok(number_updated)` on success - pub fn estimate_tx_rates( - &mut self, - estimator: &CE, - metric: &CM, - max_updates: u32, - ) -> Result { - let sql_tx = self.tx_begin()?; + pub fn estimate_tx_rates(&mut self, max_updates: u32) -> Result { + let sql_tx = tx_begin_immediate(&mut self.db)?; let txs: Vec = query_rows( &sql_tx, "SELECT * FROM mempool as m LEFT OUTER JOIN fee_estimates as f ON @@ -593,8 +608,13 @@ impl MemPoolDB { let mut updated = 0; for tx_to_estimate in txs { let txid = tx_to_estimate.tx.txid(); - let cost_estimate = match estimator.estimate_cost(&tx_to_estimate.tx.payload) { - Ok(x) => x, + let estimator_result = cost_estimates::estimate_fee_rate( + &tx_to_estimate.tx, + self.cost_estimator.as_ref(), + self.metric.as_ref(), + ); + let fee_rate_f64 = match estimator_result { + Ok(x) => Some(x), Err(EstimatorError::NoEstimateAvailable) => continue, Err(e) => { warn!("Error while estimating mempool tx rate"; @@ -603,9 +623,6 @@ impl MemPoolDB { continue; } }; - let metric_estimate = - metric.from_cost_and_len(&cost_estimate, tx_to_estimate.tx.tx_len()); - let fee_rate_f64 = tx_to_estimate.tx.get_tx_fee() as f64 / metric_estimate as f64; sql_tx.execute( "INSERT OR REPLACE INTO fee_estimates(txid, fee_rate) VALUES (?, ?)", @@ -629,7 +646,7 @@ impl MemPoolDB { /// /// Returns the number of transactions considered on success. pub fn iterate_candidates( - &self, + &mut self, clarity_tx: &mut C, _tip_height: u64, settings: MemPoolWalkSettings, @@ -637,7 +654,7 @@ impl MemPoolDB { ) -> Result where C: ClarityConnection, - F: FnMut(&mut C, &ConsiderTransaction) -> Result, + F: FnMut(&mut C, &ConsiderTransaction, &mut dyn CostEstimator) -> Result, E: From + From, { let start_time = Instant::now(); @@ -683,7 +700,7 @@ impl MemPoolDB { "size" => consider.tx.metadata.len); total_considered += 1; - if !todo(clarity_tx, &consider)? { + if !todo(clarity_tx, &consider, self.cost_estimator.as_mut())? { debug!("Mempool iteration early exit from iterator"); break; } @@ -1045,6 +1062,7 @@ impl MemPoolDB { tx: &StacksTransaction, do_admission_checks: bool, event_observer: Option<&dyn MemPoolEventDispatcher>, + fee_rate_estimate: Option, ) -> Result<(), MemPoolRejection> { test_debug!( "Mempool submit {} at {}/{}", @@ -1112,6 +1130,13 @@ impl MemPoolDB { event_observer, )?; + mempool_tx + .execute( + "INSERT OR REPLACE INTO fee_estimates(txid, fee_rate) VALUES (?, ?)", + rusqlite::params![&txid, fee_rate_estimate], + ) + .map_err(db_error::from)?; + if let Err(e) = monitoring::mempool_accepted(&txid, &chainstate.root_path) { warn!("Failed to monitor TX receive: {:?}", e; "txid" => %txid); } @@ -1128,7 +1153,27 @@ impl MemPoolDB { tx: &StacksTransaction, event_observer: Option<&dyn MemPoolEventDispatcher>, ) -> Result<(), MemPoolRejection> { + let estimator_result = cost_estimates::estimate_fee_rate( + tx, + self.cost_estimator.as_ref(), + self.metric.as_ref(), + ); + let mut mempool_tx = self.tx_begin().map_err(MemPoolRejection::DBError)?; + + let fee_rate = match estimator_result { + Ok(x) => Some(x), + Err(EstimatorError::NoEstimateAvailable) => None, + Err(e) => { + warn!("Error while estimating mempool tx rate"; + "txid" => %tx.txid(), + "error" => ?e); + return Err(MemPoolRejection::Other( + "Failed to estimate mempool tx rate".into(), + )); + } + }; + MemPoolDB::tx_submit( &mut mempool_tx, chainstate, @@ -1137,12 +1182,15 @@ impl MemPoolDB { tx, true, event_observer, + fee_rate, )?; mempool_tx.commit().map_err(MemPoolRejection::DBError)?; Ok(()) } /// Directly submit to the mempool, and don't do any admissions checks. + /// This method is only used during testing, but because it is used by the + /// integration tests, it cannot be marked #[cfg(test)]. pub fn submit_raw( &mut self, chainstate: &mut StacksChainState, @@ -1153,7 +1201,27 @@ impl MemPoolDB { let tx = StacksTransaction::consensus_deserialize(&mut &tx_bytes[..]) .map_err(MemPoolRejection::DeserializationFailure)?; + let estimator_result = cost_estimates::estimate_fee_rate( + &tx, + self.cost_estimator.as_ref(), + self.metric.as_ref(), + ); + let mut mempool_tx = self.tx_begin().map_err(MemPoolRejection::DBError)?; + + let fee_rate = match estimator_result { + Ok(x) => Some(x), + Err(EstimatorError::NoEstimateAvailable) => None, + Err(e) => { + warn!("Error while estimating mempool tx rate"; + "txid" => %tx.txid(), + "error" => ?e); + return Err(MemPoolRejection::Other( + "Failed to estimate mempool tx rate".into(), + )); + } + }; + MemPoolDB::tx_submit( &mut mempool_tx, chainstate, @@ -1162,6 +1230,7 @@ impl MemPoolDB { &tx, false, None, + fee_rate, )?; mempool_tx.commit().map_err(MemPoolRejection::DBError)?; Ok(()) @@ -1263,7 +1332,7 @@ mod tests { fn mempool_db_init() { let _chainstate = instantiate_chainstate(false, 0x80000000, "mempool_db_init"); let chainstate_path = chainstate_path("mempool_db_init"); - let _mempool = MemPoolDB::open(false, 0x80000000, &chainstate_path).unwrap(); + let _mempool = MemPoolDB::open_test(false, 0x80000000, &chainstate_path).unwrap(); } fn make_block( @@ -1367,7 +1436,7 @@ mod tests { let b_4 = make_block(&mut chainstate, ConsensusHash([0x4; 20]), &b_3, 4, 3); let chainstate_path = chainstate_path("mempool_walk_over_fork"); - let mut mempool = MemPoolDB::open(false, 0x80000000, &chainstate_path).unwrap(); + let mut mempool = MemPoolDB::open_test(false, 0x80000000, &chainstate_path).unwrap(); let mut all_txs = codec_all_transactions( &TransactionVersion::Testnet, @@ -1460,7 +1529,7 @@ mod tests { clarity_conn, 2, mempool_settings.clone(), - |_, available_tx| { + |_, available_tx, _| { count_txs += 1; Ok(true) }, @@ -1485,7 +1554,7 @@ mod tests { clarity_conn, 2, mempool_settings.clone(), - |_, available_tx| { + |_, available_tx, _| { count_txs += 1; Ok(true) }, @@ -1509,7 +1578,7 @@ mod tests { clarity_conn, 3, mempool_settings.clone(), - |_, available_tx| { + |_, available_tx, _| { count_txs += 1; Ok(true) }, @@ -1538,7 +1607,7 @@ mod tests { clarity_conn, 2, mempool_settings.clone(), - |_, available_tx| { + |_, available_tx, _| { count_txs += 1; Ok(true) }, @@ -1565,7 +1634,7 @@ mod tests { clarity_conn, 3, mempool_settings.clone(), - |_, available_tx| { + |_, available_tx, _| { count_txs += 1; Ok(true) }, @@ -1718,7 +1787,7 @@ mod tests { let b_3 = make_block(&mut chainstate, ConsensusHash([0x3; 20]), &b_1, 1, 1); let chainstate_path = chainstate_path("mempool_do_not_replace_tx"); - let mut mempool = MemPoolDB::open(false, 0x80000000, &chainstate_path).unwrap(); + let mut mempool = MemPoolDB::open_test(false, 0x80000000, &chainstate_path).unwrap(); let mut txs = codec_all_transactions( &TransactionVersion::Testnet, @@ -1815,7 +1884,7 @@ mod tests { let mut chainstate = instantiate_chainstate(false, 0x80000000, "mempool_db_load_store_replace_tx"); let chainstate_path = chainstate_path("mempool_db_load_store_replace_tx"); - let mut mempool = MemPoolDB::open(false, 0x80000000, &chainstate_path).unwrap(); + let mut mempool = MemPoolDB::open_test(false, 0x80000000, &chainstate_path).unwrap(); let mut txs = codec_all_transactions( &TransactionVersion::Testnet, @@ -2064,7 +2133,7 @@ mod tests { fn mempool_db_test_rbf() { let mut chainstate = instantiate_chainstate(false, 0x80000000, "mempool_db_test_rbf"); let chainstate_path = chainstate_path("mempool_db_test_rbf"); - let mut mempool = MemPoolDB::open(false, 0x80000000, &chainstate_path).unwrap(); + let mut mempool = MemPoolDB::open_test(false, 0x80000000, &chainstate_path).unwrap(); // create initial transaction let mut mempool_tx = mempool.tx_begin().unwrap(); diff --git a/src/cost_estimates/mod.rs b/src/cost_estimates/mod.rs index 9db56099e6..7bcdf07db7 100644 --- a/src/cost_estimates/mod.rs +++ b/src/cost_estimates/mod.rs @@ -20,6 +20,9 @@ pub mod pessimistic; #[cfg(test)] pub mod tests; +use crate::chainstate::stacks::StacksTransaction; + +use self::metrics::CostMetric; pub use self::pessimistic::PessimisticEstimator; /// This trait is for implementation of *fee rate* estimation: estimators should @@ -86,6 +89,18 @@ impl Add for FeeRateEstimate { } } +/// Given a cost estimator and a scalar metric, estimate the fee rate for +/// the provided transaction +pub fn estimate_fee_rate( + tx: &StacksTransaction, + estimator: &CE, + metric: &CM, +) -> Result { + let cost_estimate = estimator.estimate_cost(&tx.payload)?; + let metric_estimate = metric.from_cost_and_len(&cost_estimate, tx.tx_len()); + Ok(tx.get_tx_fee() as f64 / metric_estimate as f64) +} + /// This trait is for implementation of *execution cost* estimation. CostEstimators /// provide the estimated `ExecutionCost` for a given `TransactionPayload`. /// diff --git a/src/main.rs b/src/main.rs index 01442821f2..e27d65b1b4 100644 --- a/src/main.rs +++ b/src/main.rs @@ -456,8 +456,11 @@ simulating a miner. let chain_tip = SortitionDB::get_canonical_burn_chain_tip(sort_db.conn()) .expect("Failed to get sortition chain tip"); - let mut mempool_db = - MemPoolDB::open(true, chain_id, &chain_state_path).expect("Failed to open mempool db"); + let estimator = Box::new(UnitEstimator); + let metric = Box::new(UnitMetric); + + let mut mempool_db = MemPoolDB::open(true, chain_id, &chain_state_path, estimator, metric) + .expect("Failed to open mempool db"); let stacks_block = chain_state.get_stacks_chain_tip(&sort_db).unwrap().unwrap(); let parent_header = StacksChainState::get_anchored_block_header_info( @@ -488,9 +491,6 @@ simulating a miner. settings.max_miner_time_ms = max_time; settings.mempool_settings.min_tx_fee = min_fee; - let mut estimator = UnitEstimator; - let metric = UnitMetric; - let result = StacksBlockBuilder::build_anchored_block( &chain_state, &sort_db.index_conn(), @@ -502,8 +502,6 @@ simulating a miner. &coinbase_tx, settings, None, - &mut estimator, - &metric, ); let stop = get_epoch_time_ms(); diff --git a/src/net/download.rs b/src/net/download.rs index 90d3571865..3669c11fb2 100644 --- a/src/net/download.rs +++ b/src/net/download.rs @@ -3668,14 +3668,11 @@ pub mod test { Some(microblock_stream[i - 1].header.clone()); let mut mempool = - MemPoolDB::open(false, 0x80000000, &chainstate_path) + MemPoolDB::open_test(false, 0x80000000, &chainstate_path) .unwrap(); let coinbase_tx = make_coinbase_with_nonce(miner, i, (i + 2) as u64); - let mut estimator = UnitEstimator; - let metric = UnitMetric; - let (anchored_block, block_size, block_execution_cost) = StacksBlockBuilder::build_anchored_block( chainstate, @@ -3688,8 +3685,6 @@ pub mod test { &coinbase_tx, BlockBuilderSettings::max_value(), None, - &mut estimator, - &metric, ) .unwrap(); (anchored_block, vec![]) diff --git a/src/net/mod.rs b/src/net/mod.rs index 84a413add0..a722f2fd5c 100644 --- a/src/net/mod.rs +++ b/src/net/mod.rs @@ -2235,11 +2235,15 @@ pub mod test { } impl<'a> TestPeer<'a> { - pub fn new(mut config: TestPeerConfig) -> TestPeer<'a> { - let test_path = format!( + pub fn test_path(config: &TestPeerConfig) -> String { + format!( "/tmp/blockstack-test-peer-{}-{}", &config.test_name, config.server_port - ); + ) + } + + pub fn new(mut config: TestPeerConfig) -> TestPeer<'a> { + let test_path = TestPeer::test_path(&config); match fs::metadata(&test_path) { Ok(_) => { fs::remove_dir_all(&test_path).unwrap(); @@ -2473,7 +2477,7 @@ pub mod test { peer_network.bind(&local_addr, &http_local_addr).unwrap(); let relayer = Relayer::from_p2p(&mut peer_network); - let mempool = MemPoolDB::open(false, config.network_id, &chainstate_path).unwrap(); + let mempool = MemPoolDB::open_test(false, config.network_id, &chainstate_path).unwrap(); TestPeer { config: config, diff --git a/src/net/server.rs b/src/net/server.rs index cb219869f5..47e244b23c 100644 --- a/src/net/server.rs +++ b/src/net/server.rs @@ -710,6 +710,7 @@ mod test { use net::*; use std::cell::RefCell; + use crate::chainstate::coordinator::tests::get_chainstate_path_str; use crate::types::chainstate::BurnchainHeaderHash; use burnchains::Burnchain; use burnchains::BurnchainView; @@ -765,15 +766,15 @@ mod test { let mut peer_config = TestPeerConfig::new(test_name, peer_p2p, peer_http); peer_config.connection_opts = conn_opts; - let mut peer = TestPeer::new(peer_config); - let view = peer.get_burnchain_view().unwrap(); - let (http_sx, http_rx) = sync_channel(1); + let test_path = TestPeer::test_path(&peer_config); + let network_id = peer_config.network_id; + let chainstate_path = get_chainstate_path_str(&test_path); - let network_id = peer.config.network_id; - let chainstate_path = peer.chainstate_path.clone(); + let (http_sx, http_rx) = sync_channel(1); let (num_events_sx, num_events_rx) = sync_channel(1); let http_thread = thread::spawn(move || { + let mut peer = TestPeer::new(peer_config); let view = peer.get_burnchain_view().unwrap(); loop { test_debug!("http wakeup"); diff --git a/testnet/stacks-node/src/neon_node.rs b/testnet/stacks-node/src/neon_node.rs index 83576e5e28..281720713b 100644 --- a/testnet/stacks-node/src/neon_node.rs +++ b/testnet/stacks-node/src/neon_node.rs @@ -37,9 +37,7 @@ use stacks::chainstate::stacks::{ use stacks::codec::StacksMessageCodec; use stacks::core::mempool::MemPoolDB; use stacks::core::FIRST_BURNCHAIN_CONSENSUS_HASH; -use stacks::cost_estimates::metrics::CostMetric; use stacks::cost_estimates::metrics::UnitMetric; -use stacks::cost_estimates::CostEstimator; use stacks::cost_estimates::UnitEstimator; use stacks::monitoring::{increment_stx_blocks_mined_counter, update_active_miners_count_gauge}; use stacks::net::{ @@ -328,13 +326,11 @@ fn inner_generate_block_commit_op( } /// Mine and broadcast a single microblock, unconditionally. -fn mine_one_microblock( +fn mine_one_microblock( microblock_state: &mut MicroblockMinerState, sortdb: &SortitionDB, chainstate: &mut StacksChainState, mempool: &mut MemPoolDB, - estimator: &mut CE, - metric: &CM, ) -> Result { debug!( "Try to mine one microblock off of {}/{} (total: {})", @@ -370,12 +366,7 @@ fn mine_one_microblock( let t1 = get_epoch_time_ms(); - let mblock = microblock_miner.mine_next_microblock( - mempool, - µblock_state.miner_key, - estimator, - metric, - )?; + let mblock = microblock_miner.mine_next_microblock(mempool, µblock_state.miner_key)?; let new_cost_so_far = microblock_miner.get_cost_so_far().expect("BUG: cannot read cost so far from miner -- indicates that the underlying Clarity Tx is somehow in use still."); let t2 = get_epoch_time_ms(); @@ -411,15 +402,13 @@ fn mine_one_microblock( return Ok(mined_microblock); } -fn try_mine_microblock( +fn try_mine_microblock( config: &Config, microblock_miner_state: &mut Option, chainstate: &mut StacksChainState, sortdb: &SortitionDB, mem_pool: &mut MemPoolDB, winning_tip: (ConsensusHash, BlockHeaderHash, Secp256k1PrivateKey), - estimator: &mut CE, - metric: &CM, ) -> Result, NetError> { let ch = winning_tip.0; let bhh = winning_tip.1; @@ -480,14 +469,7 @@ fn try_mine_microblock( get_epoch_time_secs() - 600, )?; if num_attachable == 0 { - match mine_one_microblock( - &mut microblock_miner, - sortdb, - chainstate, - mem_pool, - estimator, - metric, - ) { + match mine_one_microblock(&mut microblock_miner, sortdb, chainstate, mem_pool) { Ok(microblock) => { // will need to relay this next_microblock = Some(microblock); @@ -513,7 +495,7 @@ fn try_mine_microblock( Ok(next_microblock) } -fn run_microblock_tenure( +fn run_microblock_tenure( config: &Config, microblock_miner_state: &mut Option, chainstate: &mut StacksChainState, @@ -523,8 +505,6 @@ fn run_microblock_tenure( miner_tip: (ConsensusHash, BlockHeaderHash, Secp256k1PrivateKey), microblocks_processed: BlocksProcessedCounter, event_dispatcher: &EventDispatcher, - estimator: &mut CE, - metric: &CM, ) { // TODO: this is sensitive to poll latency -- can we call this on a fixed // schedule, regardless of network activity? @@ -544,8 +524,6 @@ fn run_microblock_tenure( sortdb, mem_pool, miner_tip.clone(), - estimator, - metric, ) { Ok(x) => x, Err(e) => { @@ -662,19 +640,28 @@ fn spawn_peer( ) .map_err(|e| NetError::ChainstateError(e.to_string()))?; - let mut mem_pool = MemPoolDB::open( - is_mainnet, - config.burnchain.chain_id, - &stacks_chainstate_path, - ) - .map_err(NetError::DBError)?; - // buffer up blocks to store without stalling the p2p thread let mut results_with_data = VecDeque::new(); let server_thread = thread::Builder::new() .name("p2p".to_string()) .spawn(move || { + let cost_estimator = config + .make_cost_estimator() + .unwrap_or_else(|| Box::new(UnitEstimator)); + let metric = config + .make_cost_metric() + .unwrap_or_else(|| Box::new(UnitMetric)); + + let mut mem_pool = MemPoolDB::open( + is_mainnet, + config.burnchain.chain_id, + &stacks_chainstate_path, + cost_estimator, + metric, + ) + .expect("Database failure opening mempool"); + let handler_args = RPCHandlerArgs { exit_at_block_height: exit_at_block_height.as_ref(), genesis_chainstate_hash: Sha256Sum::from_hex(stx_genesis::GENESIS_CHAINSTATE_HASH) @@ -847,9 +834,6 @@ fn spawn_miner_relayer( ) .map_err(|e| NetError::ChainstateError(e.to_string()))?; - let mut mem_pool = MemPoolDB::open(is_mainnet, chain_id, &stacks_chainstate_path) - .map_err(NetError::DBError)?; - let mut last_mined_blocks: HashMap< BurnchainHeaderHash, Vec<(AssembledAnchorBlock, Secp256k1PrivateKey)>, @@ -863,12 +847,14 @@ fn spawn_miner_relayer( let mut last_tenure_issue_time = 0; let relayer_handle = thread::Builder::new().name("relayer".to_string()).spawn(move || { - let mut cost_estimator = config.make_cost_estimator() + let cost_estimator = config.make_cost_estimator() .unwrap_or_else(|| Box::new(UnitEstimator)); - let metric = config.make_cost_metric() .unwrap_or_else(|| Box::new(UnitMetric)); + let mut mem_pool = MemPoolDB::open(is_mainnet, chain_id, &stacks_chainstate_path, cost_estimator, metric) + .expect("Database failure opening mempool"); + while let Ok(mut directive) = relay_channel.recv() { match directive { RelayerDirective::HandleNetResult(ref mut net_result) => { @@ -1078,8 +1064,6 @@ fn spawn_miner_relayer( &mut bitcoin_controller, &last_mined_blocks_vec.iter().map(|(blk, _)| blk).collect(), &event_dispatcher, - cost_estimator.as_mut(), - metric.as_ref(), ); if let Some((last_mined_block, microblock_privkey)) = last_mined_block_opt { if last_mined_blocks_vec.len() == 0 { @@ -1133,8 +1117,6 @@ fn spawn_miner_relayer( (ch, bh, mblock_pkey), microblocks_processed.clone(), &event_dispatcher, - cost_estimator.as_mut(), - metric.as_ref(), ); // synchronize unconfirmed tx index to p2p thread @@ -1295,10 +1277,19 @@ impl InitializedNeonNode { }; // force early mempool instantiation + let cost_estimator = config + .make_cost_estimator() + .unwrap_or_else(|| Box::new(UnitEstimator)); + let metric = config + .make_cost_metric() + .unwrap_or_else(|| Box::new(UnitMetric)); + let _ = MemPoolDB::open( config.is_mainnet(), config.burnchain.chain_id, &config.get_chainstate_path_str(), + cost_estimator, + metric, ) .expect("BUG: failed to instantiate mempool"); @@ -1565,7 +1556,7 @@ impl InitializedNeonNode { /// Return the assembled anchor block info and microblock private key on success. /// Return None if we couldn't build a block for whatever reason - fn relayer_run_tenure( + fn relayer_run_tenure( config: &Config, registered_key: RegisteredKey, chain_state: &mut StacksChainState, @@ -1578,8 +1569,6 @@ impl InitializedNeonNode { bitcoin_controller: &mut BitcoinRegtestController, last_mined_blocks: &Vec<&AssembledAnchorBlock>, event_observer: &EventDispatcher, - estimator: &mut CE, - metric: &CM, ) -> Option<(AssembledAnchorBlock, Secp256k1PrivateKey)> { let MiningTenureInformation { mut stacks_parent_header, @@ -1866,8 +1855,6 @@ impl InitializedNeonNode { &coinbase_tx, config.make_block_builder_settings((last_mined_blocks.len() + 1) as u64), Some(event_observer), - estimator, - metric, ) { Ok(block) => block, Err(ChainstateError::InvalidStacksMicroblock(msg, mblock_header_hash)) => { @@ -1908,8 +1895,6 @@ impl InitializedNeonNode { &coinbase_tx, config.make_block_builder_settings((last_mined_blocks.len() + 1) as u64), Some(event_observer), - estimator, - metric, ) { Ok(block) => block, Err(e) => { diff --git a/testnet/stacks-node/src/node.rs b/testnet/stacks-node/src/node.rs index 2c00d56dce..4ac41a8b06 100644 --- a/testnet/stacks-node/src/node.rs +++ b/testnet/stacks-node/src/node.rs @@ -23,6 +23,8 @@ use stacks::chainstate::stacks::{ }; use stacks::chainstate::{burn::db::sortdb::SortitionDB, stacks::db::StacksEpochReceipt}; use stacks::core::mempool::MemPoolDB; +use stacks::cost_estimates::metrics::UnitMetric; +use stacks::cost_estimates::UnitEstimator; use stacks::net::atlas::AttachmentInstance; use stacks::net::{ atlas::{AtlasConfig, AtlasDB}, @@ -194,8 +196,16 @@ fn spawn_peer( } }; - let mut mem_pool = match MemPoolDB::open(is_mainnet, chain_id, &stacks_chainstate_path) - { + let estimator = Box::new(UnitEstimator); + let metric = Box::new(UnitMetric); + + let mut mem_pool = match MemPoolDB::open( + is_mainnet, + chain_id, + &stacks_chainstate_path, + estimator, + metric, + ) { Ok(x) => x, Err(e) => { warn!("Error while connecting to mempool db in peer loop: {}", e); @@ -311,11 +321,16 @@ impl Node { ), }; + let estimator = Box::new(UnitEstimator); + let metric = Box::new(UnitMetric); + // avoid race to create condition on mempool db let _mem_pool = MemPoolDB::open( config.is_mainnet(), config.burnchain.chain_id, &chain_state.root_path, + estimator, + metric, ) .expect("FATAL: failed to initiate mempool"); @@ -651,10 +666,15 @@ impl Node { }, }; + let estimator = Box::new(UnitEstimator); + let metric = Box::new(UnitMetric); + let mem_pool = MemPoolDB::open( self.config.is_mainnet(), self.config.burnchain.chain_id, &self.chain_state.root_path, + estimator, + metric, ) .expect("FATAL: failed to open mempool"); diff --git a/testnet/stacks-node/src/tenure.rs b/testnet/stacks-node/src/tenure.rs index f9a70eb41b..bb5a336d0d 100644 --- a/testnet/stacks-node/src/tenure.rs +++ b/testnet/stacks-node/src/tenure.rs @@ -11,8 +11,6 @@ use stacks::chainstate::stacks::{ StacksPrivateKey, StacksPublicKey, StacksTransaction, }; use stacks::core::mempool::MemPoolDB; -use stacks::cost_estimates::metrics::UnitMetric; -use stacks::cost_estimates::UnitEstimator; use stacks::types::chainstate::VRFSeed; use stacks::util::hash::Hash160; use stacks::util::vrf::VRFProof; @@ -86,8 +84,6 @@ impl<'a> Tenure { ) .unwrap(); - let mut estimator = UnitEstimator; - let metric = UnitMetric; let (anchored_block, _, _) = StacksBlockBuilder::build_anchored_block( &mut chain_state, burn_dbconn, @@ -99,8 +95,6 @@ impl<'a> Tenure { &self.coinbase_tx, BlockBuilderSettings::limited(self.config.block_limit.clone()), None, - &mut estimator, - &metric, ) .unwrap(); From 14dcd4927b26e18e2ad2c5350b339304653c7933 Mon Sep 17 00:00:00 2001 From: Aaron Blankstein Date: Thu, 7 Oct 2021 12:39:51 -0500 Subject: [PATCH 10/24] fix mempool integration test --- testnet/stacks-node/src/tests/mempool.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/testnet/stacks-node/src/tests/mempool.rs b/testnet/stacks-node/src/tests/mempool.rs index 90692318d7..52b3b878f2 100644 --- a/testnet/stacks-node/src/tests/mempool.rs +++ b/testnet/stacks-node/src/tests/mempool.rs @@ -11,6 +11,8 @@ use stacks::chainstate::stacks::{ use stacks::codec::StacksMessageCodec; use stacks::core::mempool::MemPoolDB; use stacks::core::CHAIN_ID_TESTNET; +use stacks::cost_estimates::metrics::UnitMetric; +use stacks::cost_estimates::UnitEstimator; use stacks::net::Error as NetError; use stacks::types::chainstate::{ BlockHeaderHash, StacksAddress, StacksBlockHeader, StacksMicroblockHeader, @@ -197,7 +199,12 @@ fn mempool_setup_chainstate() { let chainstate_path = { CHAINSTATE_PATH.lock().unwrap().clone().unwrap() }; - let _mempool = MemPoolDB::open(false, CHAIN_ID_TESTNET, &chainstate_path).unwrap(); + let estimator = Box::new(UnitEstimator); + let metric = Box::new(UnitMetric); + + let _mempool = + MemPoolDB::open(false, CHAIN_ID_TESTNET, &chainstate_path, estimator, metric) + .unwrap(); if round == 3 { let block_header = chain_tip.metadata.clone(); From 497851fb1d3742740a19a41f587eae1275622b24 Mon Sep 17 00:00:00 2001 From: Aaron Blankstein Date: Thu, 7 Oct 2021 15:21:49 -0500 Subject: [PATCH 11/24] use Send bound on trait for http server tests --- src/cost_estimates/metrics.rs | 2 +- src/cost_estimates/mod.rs | 2 +- src/net/server.rs | 11 +++++------ 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/cost_estimates/metrics.rs b/src/cost_estimates/metrics.rs index bcb9840e0f..236ed07b4a 100644 --- a/src/cost_estimates/metrics.rs +++ b/src/cost_estimates/metrics.rs @@ -4,7 +4,7 @@ use crate::vm::costs::ExecutionCost; /// This trait defines metrics used to convert `ExecutionCost` and tx_len usage into single-dimensional /// metrics that can be used to compute a fee rate. -pub trait CostMetric { +pub trait CostMetric: Send { fn from_cost_and_len(&self, cost: &ExecutionCost, tx_len: u64) -> u64; fn from_len(&self, tx_len: u64) -> u64; } diff --git a/src/cost_estimates/mod.rs b/src/cost_estimates/mod.rs index 7bcdf07db7..9352a5b7f0 100644 --- a/src/cost_estimates/mod.rs +++ b/src/cost_estimates/mod.rs @@ -109,7 +109,7 @@ pub fn estimate_fee_rate( /// payload. `FeeRateEstimator` implementations estimate the network's current fee rate. /// Clients interested in determining the fee to be paid for a transaction must used both /// whereas miners only need to use a `CostEstimator` -pub trait CostEstimator { +pub trait CostEstimator: Send { /// This method is invoked by the `stacks-node` to update the cost estimator with a new /// cost measurement. The given `tx` had a measured cost of `actual_cost`. fn notify_event( diff --git a/src/net/server.rs b/src/net/server.rs index 47e244b23c..cb219869f5 100644 --- a/src/net/server.rs +++ b/src/net/server.rs @@ -710,7 +710,6 @@ mod test { use net::*; use std::cell::RefCell; - use crate::chainstate::coordinator::tests::get_chainstate_path_str; use crate::types::chainstate::BurnchainHeaderHash; use burnchains::Burnchain; use burnchains::BurnchainView; @@ -766,15 +765,15 @@ mod test { let mut peer_config = TestPeerConfig::new(test_name, peer_p2p, peer_http); peer_config.connection_opts = conn_opts; - let test_path = TestPeer::test_path(&peer_config); - let network_id = peer_config.network_id; - let chainstate_path = get_chainstate_path_str(&test_path); - + let mut peer = TestPeer::new(peer_config); + let view = peer.get_burnchain_view().unwrap(); let (http_sx, http_rx) = sync_channel(1); + let network_id = peer.config.network_id; + let chainstate_path = peer.chainstate_path.clone(); + let (num_events_sx, num_events_rx) = sync_channel(1); let http_thread = thread::spawn(move || { - let mut peer = TestPeer::new(peer_config); let view = peer.get_burnchain_view().unwrap(); loop { test_debug!("http wakeup"); From b9531b70f0ff85acb43432786f0d9a1d855219f3 Mon Sep 17 00:00:00 2001 From: Aaron Blankstein Date: Thu, 7 Oct 2021 16:21:53 -0500 Subject: [PATCH 12/24] enum renaming, add mempool db schema version --- src/core/mempool.rs | 66 ++++++++++++++++++++++++++++++--------------- 1 file changed, 45 insertions(+), 21 deletions(-) diff --git a/src/core/mempool.rs b/src/core/mempool.rs index d70d06d6d0..ad80bfa917 100644 --- a/src/core/mempool.rs +++ b/src/core/mempool.rs @@ -120,10 +120,10 @@ pub struct ConsiderTransaction { } enum ConsiderTransactionResult { - NO_TRANSACTIONS, - UPDATE_NONCES(Vec), + NoTransactions, + UpdateNonces(Vec), /// This transaction should be considered for inclusion in the block. - CONSIDER(ConsiderTransaction), + Consider(ConsiderTransaction), } impl std::fmt::Display for MemPoolDropReason { @@ -284,8 +284,8 @@ const MEMPOOL_INITIAL_SCHEMA: &'static [&'static str] = &[ "CREATE INDEX by_chaintip ON mempool(consensus_hash,block_header_hash);", ]; -const MEMPOOL_SCHEMA_2_TEST: &'static str = " - SELECT name FROM sqlite_master WHERE type='table' AND name='fee_estimates' +const MEMPOOL_SCHEMA_VERSIONED_TEST: &'static str = " + SELECT name FROM sqlite_master WHERE type='table' AND name='schema_version' "; const MEMPOOL_SCHEMA_2: &'static [&'static str] = &[ @@ -304,6 +304,12 @@ const MEMPOOL_SCHEMA_2: &'static [&'static str] = &[ ALTER TABLE mempool ADD COLUMN last_known_sponsor_nonce INTEGER; "#, "CREATE INDEX fee_by_txid ON fee_estimates(txid);", + r#" + CREATE TABLE schema_version (version NUMBER, PRIMARY KEY (version)); + "#, + r#" + INSERT INTO schema_version (version) VALUES (2) + "#, ]; pub struct MemPoolDB { @@ -465,7 +471,11 @@ impl MemPoolDB { MemPoolDB::instantiate_mempool_db(&mut conn)?; } - MemPoolDB::apply_schema_2_if_needed(&mut conn)?; + let version = MemPoolDB::get_schema_version(&conn)?.unwrap_or(1); + + if version < 2 { + MemPoolDB::apply_schema_2(&mut conn)?; + } Ok(MemPoolDB { db: conn, @@ -476,19 +486,33 @@ impl MemPoolDB { }) } - fn apply_schema_2_if_needed(conn: &mut DBConn) -> Result<(), db_error> { - let tx = conn.transaction()?; - let needs_to_apply = tx - .query_row(MEMPOOL_SCHEMA_2_TEST, rusqlite::NO_PARAMS, |row| { + fn get_schema_version(conn: &DBConn) -> Result, db_error> { + let is_versioned = conn + .query_row(MEMPOOL_SCHEMA_VERSIONED_TEST, rusqlite::NO_PARAMS, |row| { row.get::<_, String>(0) }) .optional()? - .is_none(); + .is_some(); + if !is_versioned { + return Ok(None); + } - if needs_to_apply { - for sql_exec in MEMPOOL_SCHEMA_2 { - tx.execute_batch(sql_exec)?; - } + let version = conn + .query_row( + "SELECT MAX(version) FROM schema_version", + rusqlite::NO_PARAMS, + |row| row.get(0), + ) + .optional()?; + + Ok(version) + } + + fn apply_schema_2(conn: &mut DBConn) -> Result<(), db_error> { + let tx = conn.transaction()?; + + for sql_exec in MEMPOOL_SCHEMA_2 { + tx.execute_batch(sql_exec)?; } tx.commit()?; @@ -549,7 +573,7 @@ impl MemPoolDB { Some(tx) => (tx, false), None => match query_row(&self.db, select_no_estimate, rusqlite::NO_PARAMS)? { Some(tx) => (tx, true), - None => return Ok(ConsiderTransactionResult::NO_TRANSACTIONS), + None => return Ok(ConsiderTransactionResult::NoTransactions), }, }; @@ -562,9 +586,9 @@ impl MemPoolDB { } if !needs_nonces.is_empty() { - Ok(ConsiderTransactionResult::UPDATE_NONCES(needs_nonces)) + Ok(ConsiderTransactionResult::UpdateNonces(needs_nonces)) } else { - Ok(ConsiderTransactionResult::CONSIDER(ConsiderTransaction { + Ok(ConsiderTransactionResult::Consider(ConsiderTransaction { tx: next_tx, update_estimate, })) @@ -670,11 +694,11 @@ impl MemPoolDB { } match self.get_next_tx_to_consider()? { - ConsiderTransactionResult::NO_TRANSACTIONS => { + ConsiderTransactionResult::NoTransactions => { debug!("No more transactions to consider in mempool"); break; } - ConsiderTransactionResult::UPDATE_NONCES(addresses) => { + ConsiderTransactionResult::UpdateNonces(addresses) => { let mut last_addr = None; for address in addresses.into_iter() { debug!("Update nonce"; "address" => %address); @@ -690,7 +714,7 @@ impl MemPoolDB { last_addr = Some(address) } } - ConsiderTransactionResult::CONSIDER(consider) => { + ConsiderTransactionResult::Consider(consider) => { debug!("Consider mempool transaction"; "txid" => %consider.tx.tx.txid(), "origin_addr" => %consider.tx.metadata.origin_address, From b960b27945306ce5dcc4574204cf48cd3e9b2092 Mon Sep 17 00:00:00 2001 From: Aaron Blankstein Date: Fri, 8 Oct 2021 14:05:14 -0500 Subject: [PATCH 13/24] enable WAL mode in estimator DBs, add TestPeer testing for block assembly changes --- src/chainstate/stacks/miner.rs | 152 +++++++++++++++++++++++++++ src/core/mempool.rs | 25 ++--- src/cost_estimates/fee_scalar.rs | 17 ++- src/cost_estimates/pessimistic.rs | 14 ++- src/net/mod.rs | 2 +- src/util/db.rs | 10 ++ testnet/stacks-node/src/tests/mod.rs | 6 +- 7 files changed, 202 insertions(+), 24 deletions(-) diff --git a/src/chainstate/stacks/miner.rs b/src/chainstate/stacks/miner.rs index f820b7d53a..d258280971 100644 --- a/src/chainstate/stacks/miner.rs +++ b/src/chainstate/stacks/miner.rs @@ -6921,6 +6921,158 @@ pub mod test { } } + #[test] + /// This test covers two different behaviors added to the block assembly logic: + /// (1) Ordering by estimated fee rate: the test peer uses the "unit" estimator + /// for costs, but this estimator still uses the fee of the transaction to order + /// the mempool. This leads to the behavior in this test where txs are included + /// like 0 -> 1 -> 2 ... -> 25 -> next origin 0 -> 1 ... + /// because the fee goes up with the nonce. + /// (2) Discovery of nonce in the mempool iteration: this behavior allows the miner + /// to consider an origin's "next" transaction immediately. Prior behavior would + /// only do so after processing any other origin's transactions. + fn test_build_anchored_blocks_incrementing_nonces() { + let private_keys: Vec<_> = (0..10).map(|_| StacksPrivateKey::new()).collect(); + let addresses: Vec<_> = private_keys + .iter() + .map(|sk| { + StacksAddress::from_public_keys( + C32_ADDRESS_VERSION_TESTNET_SINGLESIG, + &AddressHashMode::SerializeP2PKH, + 1, + &vec![StacksPublicKey::from_private(sk)], + ) + .unwrap() + }) + .collect(); + + let initial_balances: Vec<_> = addresses + .iter() + .map(|addr| (addr.to_account_principal(), 100000000000)) + .collect(); + + let mut peer_config = TestPeerConfig::new("build_anchored_incrementing_nonces", 2006, 2007); + peer_config.initial_balances = initial_balances; + + let mut peer = TestPeer::new(peer_config); + + let chainstate_path = peer.chainstate_path.clone(); + + let mut mempool = MemPoolDB::open_test(false, 0x80000000, &chainstate_path).unwrap(); + + // during the tenure, let's push transactions to the mempool + let tip = SortitionDB::get_canonical_burn_chain_tip(&peer.sortdb.as_ref().unwrap().conn()) + .unwrap(); + + let (burn_ops, stacks_block, microblocks) = peer.make_tenure( + |ref mut miner, + ref mut sortdb, + ref mut chainstate, + vrf_proof, + ref parent_opt, + ref parent_microblock_header_opt| { + let parent_tip = match parent_opt { + None => StacksChainState::get_genesis_header_info(chainstate.db()).unwrap(), + Some(block) => { + let ic = sortdb.index_conn(); + let snapshot = SortitionDB::get_block_snapshot_for_winning_stacks_block( + &ic, + &tip.sortition_id, + &block.block_hash(), + ) + .unwrap() + .unwrap(); // succeeds because we don't fork + StacksChainState::get_anchored_block_header_info( + chainstate.db(), + &snapshot.consensus_hash, + &snapshot.winning_stacks_block_hash, + ) + .unwrap() + .unwrap() + } + }; + + let parent_header_hash = parent_tip.anchored_header.block_hash(); + let parent_consensus_hash = parent_tip.consensus_hash.clone(); + let coinbase_tx = make_coinbase(miner, 0); + + let txs: Vec<_> = private_keys + .iter() + .flat_map(|privk| { + let privk = privk.clone(); + (0..25).map(move |tx_nonce| { + let contract = "(define-data-var bar int 0)"; + make_user_contract_publish( + &privk, + tx_nonce, + 200 * (tx_nonce + 1), + &format!("contract-{}", tx_nonce), + contract, + ) + }) + }) + .collect(); + + for tx in txs { + mempool + .submit( + chainstate, + &parent_consensus_hash, + &parent_header_hash, + &tx, + None, + ) + .unwrap(); + } + + let execution_cost = BLOCK_LIMIT_MAINNET; + + let anchored_block = StacksBlockBuilder::build_anchored_block( + chainstate, + &sortdb.index_conn(), + &mut mempool, + &parent_tip, + tip.total_burn, + vrf_proof, + Hash160([0 as u8; 20]), + &coinbase_tx, + BlockBuilderSettings::limited(execution_cost), + None, + ) + .unwrap(); + (anchored_block.0, vec![]) + }, + ); + + peer.next_burnchain_block(burn_ops.clone()); + peer.process_stacks_epoch_at_tip(&stacks_block, µblocks); + + // expensive transaction was not mined, but the two stx-transfers were + assert_eq!(stacks_block.txs.len(), 251); + + // block should be ordered like coinbase, nonce 0, nonce 1, .. nonce 25, nonce 0, .. + // because the tx fee for each transaction increases with the nonce + for (i, tx) in stacks_block.txs.iter().enumerate() { + if i == 0 { + let okay = if let TransactionPayload::Coinbase(..) = tx.payload { + true + } else { + false + }; + assert!(okay, "Coinbase should be first tx"); + } else { + let expected_nonce = (i - 1) % 25; + assert_eq!( + tx.get_origin_nonce(), + expected_nonce as u64, + "{}th transaction should have nonce = {}", + i, + expected_nonce + ); + } + } + } + #[test] fn test_build_anchored_blocks_skip_too_expensive() { let privk = StacksPrivateKey::from_hex( diff --git a/src/core/mempool.rs b/src/core/mempool.rs index ad80bfa917..c793351669 100644 --- a/src/core/mempool.rs +++ b/src/core/mempool.rs @@ -66,6 +66,7 @@ use crate::cost_estimates::EstimatorError; use crate::cost_estimates::UnitEstimator; use crate::monitoring; use crate::types::chainstate::{BlockHeaderHash, StacksAddress, StacksBlockHeader}; +use crate::util::db::table_exists; // maximum number of confirmations a transaction can have before it's garbage-collected pub const MEMPOOL_MAX_TRANSACTION_AGE: u64 = 256; @@ -284,10 +285,6 @@ const MEMPOOL_INITIAL_SCHEMA: &'static [&'static str] = &[ "CREATE INDEX by_chaintip ON mempool(consensus_hash,block_header_hash);", ]; -const MEMPOOL_SCHEMA_VERSIONED_TEST: &'static str = " - SELECT name FROM sqlite_master WHERE type='table' AND name='schema_version' -"; - const MEMPOOL_SCHEMA_2: &'static [&'static str] = &[ r#" CREATE TABLE fee_estimates( @@ -471,12 +468,15 @@ impl MemPoolDB { MemPoolDB::instantiate_mempool_db(&mut conn)?; } - let version = MemPoolDB::get_schema_version(&conn)?.unwrap_or(1); + let tx = conn.transaction()?; + let version = MemPoolDB::get_schema_version(&tx)?.unwrap_or(1); if version < 2 { - MemPoolDB::apply_schema_2(&mut conn)?; + MemPoolDB::apply_schema_2(&tx)?; } + tx.commit()?; + Ok(MemPoolDB { db: conn, path: db_path, @@ -487,12 +487,7 @@ impl MemPoolDB { } fn get_schema_version(conn: &DBConn) -> Result, db_error> { - let is_versioned = conn - .query_row(MEMPOOL_SCHEMA_VERSIONED_TEST, rusqlite::NO_PARAMS, |row| { - row.get::<_, String>(0) - }) - .optional()? - .is_some(); + let is_versioned = table_exists(conn, "schema_version")?; if !is_versioned { return Ok(None); } @@ -508,15 +503,11 @@ impl MemPoolDB { Ok(version) } - fn apply_schema_2(conn: &mut DBConn) -> Result<(), db_error> { - let tx = conn.transaction()?; - + fn apply_schema_2(tx: &Transaction) -> Result<(), db_error> { for sql_exec in MEMPOOL_SCHEMA_2 { tx.execute_batch(sql_exec)?; } - tx.commit()?; - Ok(()) } diff --git a/src/cost_estimates/fee_scalar.rs b/src/cost_estimates/fee_scalar.rs index 7a0822296c..acd527c8f3 100644 --- a/src/cost_estimates/fee_scalar.rs +++ b/src/cost_estimates/fee_scalar.rs @@ -20,6 +20,9 @@ use core::BLOCK_LIMIT_MAINNET; use chainstate::stacks::db::StacksEpochReceipt; use chainstate::stacks::events::TransactionOrigin; +use crate::util::db::sql_pragma; +use crate::util::db::table_exists; + use super::metrics::CostMetric; use super::FeeRateEstimate; use super::{EstimatorError, FeeEstimator}; @@ -73,8 +76,18 @@ impl ScalarFeeRateEstimator { }) } - fn instantiate_db(c: &SqlTransaction) -> Result<(), SqliteError> { - c.execute(CREATE_TABLE, rusqlite::NO_PARAMS)?; + /// Check if the SQL database was already created. Necessary to avoid races if + /// different threads open an estimator at the same time. + fn db_already_instantiated(tx: &SqlTransaction) -> Result { + table_exists(tx, "scalar_fee_estimator") + } + + fn instantiate_db(tx: &SqlTransaction) -> Result<(), SqliteError> { + if !Self::db_already_instantiated(tx)? { + tx.pragma_update(None, "journal_mode", &"WAL".to_string())?; + tx.execute(CREATE_TABLE, rusqlite::NO_PARAMS)?; + } + Ok(()) } diff --git a/src/cost_estimates/pessimistic.rs b/src/cost_estimates/pessimistic.rs index a9b683845f..422edc9c6a 100644 --- a/src/cost_estimates/pessimistic.rs +++ b/src/cost_estimates/pessimistic.rs @@ -15,6 +15,8 @@ use vm::costs::ExecutionCost; use core::BLOCK_LIMIT_MAINNET; +use crate::util::db::sql_pragma; +use crate::util::db::table_exists; use crate::util::db::tx_begin_immediate_sqlite; use super::{CostEstimator, EstimatorError}; @@ -194,8 +196,18 @@ impl PessimisticEstimator { Ok(PessimisticEstimator { db, log_error }) } + /// Check if the SQL database was already created. Necessary to avoid races if + /// different threads open an estimator at the same time. + fn db_already_instantiated(tx: &SqliteTransaction) -> Result { + table_exists(tx, "pessimistic_estimator") + } + fn instantiate_db(tx: &SqliteTransaction) -> Result<(), SqliteError> { - tx.execute(CREATE_TABLE, rusqlite::NO_PARAMS)?; + if !Self::db_already_instantiated(tx)? { + tx.pragma_update(None, "journal_mode", &"WAL".to_string())?; + tx.execute(CREATE_TABLE, rusqlite::NO_PARAMS)?; + } + Ok(()) } diff --git a/src/net/mod.rs b/src/net/mod.rs index a722f2fd5c..db37766ddb 100644 --- a/src/net/mod.rs +++ b/src/net/mod.rs @@ -2237,7 +2237,7 @@ pub mod test { impl<'a> TestPeer<'a> { pub fn test_path(config: &TestPeerConfig) -> String { format!( - "/tmp/blockstack-test-peer-{}-{}", + "/tmp/stacks-node-tests/units-test-peer/{}-{}", &config.test_name, config.server_port ) } diff --git a/src/util/db.rs b/src/util/db.rs index dde863feea..f17cbd8eac 100644 --- a/src/util/db.rs +++ b/src/util/db.rs @@ -37,6 +37,7 @@ use rusqlite::types::{ }; use rusqlite::Connection; use rusqlite::Error as sqlite_error; +use rusqlite::OptionalExtension; use rusqlite::Row; use rusqlite::Transaction; use rusqlite::TransactionBehavior; @@ -403,6 +404,15 @@ pub fn sql_pragma(conn: &Connection, pragma_stmt: &str) -> Result<(), Error> { conn.query_row_and_then(pragma_stmt, NO_PARAMS, |_row| Ok(())) } +/// Returns true if the database table `table_name` exists in the active +/// database of the provided SQLite connection. +pub fn table_exists(conn: &Connection, table_name: &str) -> Result { + let sql = "SELECT name FROM sqlite_master WHERE type='table' AND name=?"; + conn.query_row(sql, &[table_name], |row| row.get::<_, String>(0)) + .optional() + .map(|r| r.is_some()) +} + /// Set up an on-disk database with a MARF index if they don't exist yet. /// Either way, returns (db path, MARF path) pub fn db_mkdirs(path_str: &str) -> Result<(String, String), Error> { diff --git a/testnet/stacks-node/src/tests/mod.rs b/testnet/stacks-node/src/tests/mod.rs index d0cb9f923e..be060fac67 100644 --- a/testnet/stacks-node/src/tests/mod.rs +++ b/testnet/stacks-node/src/tests/mod.rs @@ -238,9 +238,9 @@ pub fn new_test_conf() -> Config { let mut conf = Config::default(); conf.node.working_dir = format!( - "/tmp/stacks-node-tests/{}-{}", - get_epoch_time_secs(), - to_hex(&buf) + "/tmp/stacks-node-tests/integrations-neon/{}-{}", + to_hex(&buf), + get_epoch_time_secs() ); conf.node.seed = hex_bytes("0000000000000000000000000000000000000000000000000000000000000000").unwrap(); From 92dd070fa14e3b16e9dce44a4459658e1bad6296 Mon Sep 17 00:00:00 2001 From: Aaron Blankstein Date: Fri, 8 Oct 2021 16:26:28 -0500 Subject: [PATCH 14/24] update test, try using matrix for parallel testing --- .../Dockerfile.bitcoin-tests | 36 ++----------------- .../Dockerfile.generic.bitcoin-tests | 15 ++++++++ .github/workflows/bitcoin-tests.yml | 20 +++++++++++ .../src/tests/neon_integrations.rs | 2 +- 4 files changed, 39 insertions(+), 34 deletions(-) create mode 100644 .github/actions/bitcoin-int-tests/Dockerfile.generic.bitcoin-tests diff --git a/.github/actions/bitcoin-int-tests/Dockerfile.bitcoin-tests b/.github/actions/bitcoin-int-tests/Dockerfile.bitcoin-tests index c0d4973283..2b7fb6a5ea 100644 --- a/.github/actions/bitcoin-int-tests/Dockerfile.bitcoin-tests +++ b/.github/actions/bitcoin-int-tests/Dockerfile.bitcoin-tests @@ -1,36 +1,6 @@ -FROM rust:bullseye - -WORKDIR /src/ - -COPY . . - -WORKDIR /src/testnet/stacks-node -RUN cargo test --no-run - -RUN cd / && wget https://bitcoin.org/bin/bitcoin-core-0.20.0/bitcoin-0.20.0-x86_64-linux-gnu.tar.gz -RUN cd / && tar -xvzf bitcoin-0.20.0-x86_64-linux-gnu.tar.gz - -RUN ln -s /bitcoin-0.20.0/bin/bitcoind /bin/ +FROM stacks-node:integrations ENV BITCOIND_TEST 1 -RUN cargo test -- --test-threads 1 --ignored tests::neon_integrations::microblock_integration_test -RUN cargo test -- --test-threads 1 --ignored tests::neon_integrations::size_check_integration_test -RUN cargo test -- --test-threads 1 --ignored tests::neon_integrations::cost_voting_integration -RUN cargo test -- --test-threads 1 --ignored tests::integrations::integration_test_get_info -RUN cargo test -- --test-threads 1 --ignored tests::neon_integrations::bitcoind_integration_test -RUN cargo test -- --test-threads 1 --ignored tests::neon_integrations::liquid_ustx_integration -RUN cargo test -- --test-threads 1 --ignored tests::neon_integrations::stx_transfer_btc_integration_test -RUN cargo test -- --test-threads 1 --ignored tests::neon_integrations::bitcoind_forking_test -RUN cargo test -- --test-threads 1 --ignored tests::neon_integrations::should_fix_2771 -RUN cargo test -- --test-threads 1 --ignored tests::neon_integrations::pox_integration_test -RUN cargo test -- --test-threads 1 --ignored tests::bitcoin_regtest::bitcoind_integration_test -RUN cargo test -- --test-threads 1 --ignored tests::should_succeed_handling_malformed_and_valid_txs -RUN cargo test -- --test-threads 1 --ignored tests::neon_integrations::size_overflow_unconfirmed_microblocks_integration_test -RUN cargo test -- --test-threads 1 --ignored tests::neon_integrations::size_overflow_unconfirmed_stream_microblocks_integration_test -RUN cargo test -- --test-threads 1 --ignored tests::neon_integrations::size_overflow_unconfirmed_invalid_stream_microblocks_integration_test -RUN cargo test -- --test-threads 1 --ignored tests::neon_integrations::runtime_overflow_unconfirmed_microblocks_integration_test -RUN cargo test -- --test-threads 1 --ignored tests::neon_integrations::antientropy_integration_test -RUN cargo test -- --test-threads 1 --ignored tests::neon_integrations::filter_low_fee_tx_integration_test -RUN cargo test -- --test-threads 1 --ignored tests::neon_integrations::filter_long_runtime_tx_integration_test -RUN cargo test -- --test-threads 1 --ignored tests::neon_integrations::mining_transactions_is_fair + +RUN cargo test -- --test-threads 1 --ignored "$TEST_NAME" diff --git a/.github/actions/bitcoin-int-tests/Dockerfile.generic.bitcoin-tests b/.github/actions/bitcoin-int-tests/Dockerfile.generic.bitcoin-tests new file mode 100644 index 0000000000..476dd3a2c5 --- /dev/null +++ b/.github/actions/bitcoin-int-tests/Dockerfile.generic.bitcoin-tests @@ -0,0 +1,15 @@ +FROM rust:bullseye + +WORKDIR /src/ + +COPY . . + +WORKDIR /src/testnet/stacks-node +RUN cargo test --no-run + +RUN cd / && wget https://bitcoin.org/bin/bitcoin-core-0.20.0/bitcoin-0.20.0-x86_64-linux-gnu.tar.gz +RUN cd / && tar -xvzf bitcoin-0.20.0-x86_64-linux-gnu.tar.gz + +RUN ln -s /bitcoin-0.20.0/bin/bitcoind /bin/ + +ENV BITCOIND_TEST 1 diff --git a/.github/workflows/bitcoin-tests.yml b/.github/workflows/bitcoin-tests.yml index e791a7a53d..b678ceb5db 100644 --- a/.github/workflows/bitcoin-tests.yml +++ b/.github/workflows/bitcoin-tests.yml @@ -6,14 +6,34 @@ on: pull_request: jobs: + build-integration-image: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - name: Build bitcoin integration testing image + env: + DOCKER_BUILDKIT: 1 + run: docker build --output type=tar,dest=docker-image.tar -f ./.github/actions/bitcoin-int-tests/Dockerfile.generic.bitcoin-tests . stacks-node:integrations + - name: Upload built docker image + uses: actions/upload-artifact@v2 + with: + name: integration-image.tar + path: docker-image.tar # Run sampled genesis tests sampled-genesis: runs-on: ubuntu-latest + strategy: + matrix: + test-name: + - tests::neon_integrations::microblock_integration_test + - tests::neon_integrations::size_check_integration_test + - tests::integrations::integration_test_get_info steps: - uses: actions/checkout@v2 - name: All integration tests with sampled genesis env: DOCKER_BUILDKIT: 1 + TEST_NAME: ${{ matrix.test-name }} run: docker build -f ./.github/actions/bitcoin-int-tests/Dockerfile.bitcoin-tests . atlas-test: # disable this job/test for now, as the atlas endpoints are currently disabled. diff --git a/testnet/stacks-node/src/tests/neon_integrations.rs b/testnet/stacks-node/src/tests/neon_integrations.rs index 0c8df5e48a..2042d7d326 100644 --- a/testnet/stacks-node/src/tests/neon_integrations.rs +++ b/testnet/stacks-node/src/tests/neon_integrations.rs @@ -2128,7 +2128,7 @@ fn mining_transactions_is_fair() { let tx = make_stacks_transfer( &spender_sks[0], i, - 2000 + (i as u64), + 2000 * (i as u64 + 1), &recipient.into(), 1000, ); From c59290ffb58fddb7b0e88b8c62741932bb3847b2 Mon Sep 17 00:00:00 2001 From: Aaron Blankstein Date: Fri, 8 Oct 2021 16:31:32 -0500 Subject: [PATCH 15/24] test: use job matrix execution for bitcoin integration tests --- .../Dockerfile.bitcoin-tests | 3 +- .github/workflows/bitcoin-tests.yml | 33 +++++++++++++++++-- 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/.github/actions/bitcoin-int-tests/Dockerfile.bitcoin-tests b/.github/actions/bitcoin-int-tests/Dockerfile.bitcoin-tests index 2b7fb6a5ea..b3a7d97ff5 100644 --- a/.github/actions/bitcoin-int-tests/Dockerfile.bitcoin-tests +++ b/.github/actions/bitcoin-int-tests/Dockerfile.bitcoin-tests @@ -1,6 +1,7 @@ FROM stacks-node:integrations +ARG test_name ENV BITCOIND_TEST 1 -RUN cargo test -- --test-threads 1 --ignored "$TEST_NAME" +RUN cargo test -- --test-threads 1 --ignored "$test_name" diff --git a/.github/workflows/bitcoin-tests.yml b/.github/workflows/bitcoin-tests.yml index b678ceb5db..10d02754a6 100644 --- a/.github/workflows/bitcoin-tests.yml +++ b/.github/workflows/bitcoin-tests.yml @@ -13,28 +13,55 @@ jobs: - name: Build bitcoin integration testing image env: DOCKER_BUILDKIT: 1 - run: docker build --output type=tar,dest=docker-image.tar -f ./.github/actions/bitcoin-int-tests/Dockerfile.generic.bitcoin-tests . stacks-node:integrations + run: docker build -f ./.github/actions/bitcoin-int-tests/Dockerfile.generic.bitcoin-tests -t stacks-node:integrations . + - name: Export docker image as tarball + run: docker save -o integration-image.tar stacks-node:integrations - name: Upload built docker image uses: actions/upload-artifact@v2 with: name: integration-image.tar - path: docker-image.tar + path: integration-image.tar # Run sampled genesis tests sampled-genesis: runs-on: ubuntu-latest + needs: + - build-integration-image strategy: matrix: test-name: - tests::neon_integrations::microblock_integration_test - tests::neon_integrations::size_check_integration_test + - tests::neon_integrations::cost_voting_integration - tests::integrations::integration_test_get_info + - tests::neon_integrations::bitcoind_integration_test + - tests::neon_integrations::liquid_ustx_integration + - tests::neon_integrations::stx_transfer_btc_integration_test + - tests::neon_integrations::bitcoind_forking_test + - tests::neon_integrations::should_fix_2771 + - tests::neon_integrations::pox_integration_test + - tests::bitcoin_regtest::bitcoind_integration_test + - tests::should_succeed_handling_malformed_and_valid_txs + - tests::neon_integrations::size_overflow_unconfirmed_microblocks_integration_test + - tests::neon_integrations::size_overflow_unconfirmed_stream_microblocks_integration_test + - tests::neon_integrations::size_overflow_unconfirmed_invalid_stream_microblocks_integration_test + - tests::neon_integrations::runtime_overflow_unconfirmed_microblocks_integration_test + - tests::neon_integrations::antientropy_integration_test + - tests::neon_integrations::filter_low_fee_tx_integration_test + - tests::neon_integrations::filter_long_runtime_tx_integration_test + - tests::neon_integrations::mining_transactions_is_fair steps: - uses: actions/checkout@v2 + - name: Download math result for job 1 + uses: actions/download-artifact@v2 + with: + name: integration-image.tar + - name: Load docker image + run: docker load -i integration-image.tar && rm integration-image.tar - name: All integration tests with sampled genesis env: DOCKER_BUILDKIT: 1 TEST_NAME: ${{ matrix.test-name }} - run: docker build -f ./.github/actions/bitcoin-int-tests/Dockerfile.bitcoin-tests . + run: docker build --build-arg test_name=${{ matrix.test-name }} -f ./.github/actions/bitcoin-int-tests/Dockerfile.bitcoin-tests . atlas-test: # disable this job/test for now, as the atlas endpoints are currently disabled. if: ${{ true }} From a4079855bdde378a0c645bae353b1e79ee4b32a1 Mon Sep 17 00:00:00 2001 From: Aaron Blankstein Date: Sat, 9 Oct 2021 13:56:56 -0500 Subject: [PATCH 16/24] chore: attempt to fix mining fairness test in GH action --- src/core/mempool.rs | 2 +- testnet/stacks-node/src/config.rs | 4 ++-- testnet/stacks-node/src/tests/neon_integrations.rs | 4 +++- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/core/mempool.rs b/src/core/mempool.rs index c793351669..1c2b8c68c9 100644 --- a/src/core/mempool.rs +++ b/src/core/mempool.rs @@ -551,7 +551,7 @@ impl MemPoolDB { } fn get_next_tx_to_consider(&self) -> Result { - let select_estimate = "SELECT * FROM mempool LEFT OUTER JOIN fee_estimates as f WHERE + let select_estimate = "SELECT * FROM mempool LEFT OUTER JOIN fee_estimates as f ON mempool.txid = f.txid WHERE ((origin_nonce = last_known_origin_nonce AND sponsor_nonce = last_known_sponsor_nonce) OR (last_known_origin_nonce is NULL) OR (last_known_sponsor_nonce is NULL)) AND f.fee_rate IS NOT NULL ORDER BY f.fee_rate DESC LIMIT 1"; diff --git a/testnet/stacks-node/src/config.rs b/testnet/stacks-node/src/config.rs index 2c065791ae..4420056064 100644 --- a/testnet/stacks-node/src/config.rs +++ b/testnet/stacks-node/src/config.rs @@ -1110,7 +1110,7 @@ impl Default for FeeEstimationConfig { cost_estimator: Some(CostEstimatorName::default()), fee_estimator: Some(FeeEstimatorName::default()), cost_metric: Some(CostMetricName::default()), - log_error: true, + log_error: false, } } } @@ -1137,7 +1137,7 @@ impl From for FeeEstimationConfig { .cost_metric .map(CostMetricName::panic_parse) .unwrap_or_default(); - let log_error = f.log_error.unwrap_or(true); + let log_error = f.log_error.unwrap_or(false); Self { cost_estimator: Some(cost_estimator), fee_estimator: Some(fee_estimator), diff --git a/testnet/stacks-node/src/tests/neon_integrations.rs b/testnet/stacks-node/src/tests/neon_integrations.rs index 2042d7d326..a46d0a8575 100644 --- a/testnet/stacks-node/src/tests/neon_integrations.rs +++ b/testnet/stacks-node/src/tests/neon_integrations.rs @@ -2147,6 +2147,9 @@ fn mining_transactions_is_fair() { }); } + // don't RBF! + conf.burnchain.max_rbf = 1; + // all transactions have high-enough fees... conf.miner.min_tx_fee = 1; conf.miner.first_attempt_time_ms = u64::max_value(); @@ -2200,7 +2203,6 @@ fn mining_transactions_is_fair() { next_block_and_wait(&mut btc_regtest_controller, &blocks_processed); next_block_and_wait(&mut btc_regtest_controller, &blocks_processed); - // the lower-fee origin should *not* be the last transaction added let blocks = test_observer::get_blocks(); let mut found_sender_1 = false; From 6b1499935a2633d142034074b20befabfea3f173 Mon Sep 17 00:00:00 2001 From: Aaron Blankstein Date: Sat, 9 Oct 2021 17:12:11 -0500 Subject: [PATCH 17/24] fix metric returning zero while dimensions near zero, add test case --- src/cost_estimates/metrics.rs | 2 +- src/cost_estimates/tests/metrics.rs | 43 +++++++++++++++++++ src/vm/costs/mod.rs | 3 +- .../src/tests/neon_integrations.rs | 11 +---- 4 files changed, 47 insertions(+), 12 deletions(-) diff --git a/src/cost_estimates/metrics.rs b/src/cost_estimates/metrics.rs index 236ed07b4a..afbdd94363 100644 --- a/src/cost_estimates/metrics.rs +++ b/src/cost_estimates/metrics.rs @@ -53,7 +53,7 @@ impl ProportionalDotProduct { // use MIN(1, self/block_limit) to guard against self > block_limit let len_proportion = PROPORTION_RESOLUTION as f64 * 1_f64.min(tx_len as f64 / 1_f64.max(self.block_size_limit as f64)); - len_proportion as u64 + cmp::max(len_proportion as u64, 1) } } diff --git a/src/cost_estimates/tests/metrics.rs b/src/cost_estimates/tests/metrics.rs index 83cfd7b391..9d5e272646 100644 --- a/src/cost_estimates/tests/metrics.rs +++ b/src/cost_estimates/tests/metrics.rs @@ -3,6 +3,49 @@ use core::BLOCK_LIMIT_MAINNET; use cost_estimates::metrics::{CostMetric, ProportionalDotProduct}; use vm::costs::ExecutionCost; +#[test] +// Test that when dimensions of the execution cost are near "zero", +// that the metric always returns a number greater than zero. +fn test_proportional_dot_product_near_zero() { + let metric = ProportionalDotProduct::new( + 12_000, + ExecutionCost { + write_length: 50_000, + write_count: 60_000, + read_length: 70_000, + read_count: 80_000, + runtime: 90_000, + }, + ); + assert_eq!( + metric.from_cost_and_len( + &ExecutionCost { + write_length: 1, + write_count: 1, + read_length: 1, + read_count: 1, + runtime: 1, + }, + 1 + ), + 6 + ); + + assert_eq!( + metric.from_cost_and_len( + &ExecutionCost { + write_length: 0, + write_count: 0, + read_length: 0, + read_count: 0, + runtime: 0, + }, + 0 + ), + 6 + ); +} + #[test] fn test_proportional_dot_product() { let metric = ProportionalDotProduct::new( diff --git a/src/vm/costs/mod.rs b/src/vm/costs/mod.rs index 0af85c314b..2b07263572 100644 --- a/src/vm/costs/mod.rs +++ b/src/vm/costs/mod.rs @@ -1018,7 +1018,8 @@ impl ExecutionCost { ] .iter() .fold(0, |acc, dim| { - acc.checked_add(*dim as u64).unwrap_or(u64::MAX) + acc.checked_add(cmp::max(*dim as u64, 1)) + .unwrap_or(u64::MAX) }) } diff --git a/testnet/stacks-node/src/tests/neon_integrations.rs b/testnet/stacks-node/src/tests/neon_integrations.rs index a46d0a8575..c7398127da 100644 --- a/testnet/stacks-node/src/tests/neon_integrations.rs +++ b/testnet/stacks-node/src/tests/neon_integrations.rs @@ -2125,13 +2125,7 @@ fn mining_transactions_is_fair() { // spender 0 sends 20 txs, at over 2000 uSTX tx fee for i in 0..20 { - let tx = make_stacks_transfer( - &spender_sks[0], - i, - 2000 * (i as u64 + 1), - &recipient.into(), - 1000, - ); + let tx = make_stacks_transfer(&spender_sks[0], i, 2000 * (21 - i), &recipient.into(), 1000); txs.push(tx); } @@ -2147,9 +2141,6 @@ fn mining_transactions_is_fair() { }); } - // don't RBF! - conf.burnchain.max_rbf = 1; - // all transactions have high-enough fees... conf.miner.min_tx_fee = 1; conf.miner.first_attempt_time_ms = u64::max_value(); From 58b03583214b2eb748b174883576112b2334da4c Mon Sep 17 00:00:00 2001 From: Aaron Blankstein Date: Mon, 11 Oct 2021 09:22:30 -0500 Subject: [PATCH 18/24] chore: update "mined anchored" log line to k-v + added total tx fee --- .github/workflows/bitcoin-tests.yml | 1 + src/chainstate/stacks/miner.rs | 23 +++++++++++++---------- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/.github/workflows/bitcoin-tests.yml b/.github/workflows/bitcoin-tests.yml index 10d02754a6..fe8a6dd95f 100644 --- a/.github/workflows/bitcoin-tests.yml +++ b/.github/workflows/bitcoin-tests.yml @@ -58,6 +58,7 @@ jobs: - name: Load docker image run: docker load -i integration-image.tar && rm integration-image.tar - name: All integration tests with sampled genesis + timeout-minutes: 30 env: DOCKER_BUILDKIT: 1 TEST_NAME: ${{ matrix.test-name }} diff --git a/src/chainstate/stacks/miner.rs b/src/chainstate/stacks/miner.rs index d258280971..1ec00283b8 100644 --- a/src/chainstate/stacks/miner.rs +++ b/src/chainstate/stacks/miner.rs @@ -1655,16 +1655,19 @@ impl StacksBlockBuilder { let ts_end = get_epoch_time_ms(); debug!( - "Miner: mined anchored block {} height {} with {} txs, parent block {}, parent microblock {} ({}), size {}, consumed {:?}, in {}ms", - block.block_hash(), - block.header.total_work.work, - block.txs.len(), - &block.header.parent_block, - &block.header.parent_microblock, - block.header.parent_microblock_sequence, - size, - &consumed, - ts_end.saturating_sub(ts_start); + "Miner: mined anchored block"; + "block_hash" => %block.block_hash(), + "height" => block.header.total_work.work, + "tx_count" => block.txs.len(), + "parent_stacks_block_hash" => %block.header.parent_block, + "parent_stacks_microblock" => %block.header.parent_microblock, + "parent_stacks_microblock_seq" => block.header.parent_microblock_sequence, + "block_size" => size, + "execution_consumed" => %consumed, + "assembly_time_ms" => ts_end.saturating_sub(ts_start), + "tx_fees_microstacks" => block.txs.iter().fold(0, |agg: u64, tx| { + agg.saturating_add(tx.get_tx_fee()) + }) ); Ok((block, consumed, size)) From 903fdda9829939d41914b8b4e1fbb691baec6df0 Mon Sep 17 00:00:00 2001 From: Aaron Blankstein Date: Fri, 15 Oct 2021 16:36:08 -0500 Subject: [PATCH 19/24] address PR feedback --- .github/workflows/bitcoin-tests.yml | 1 + docs/rpc-endpoints.md | 5 +- src/chainstate/stacks/db/blocks.rs | 3 + src/core/mempool.rs | 140 ++++++++++++++++++---------- src/net/download.rs | 3 - testnet/stacks-node/src/config.rs | 7 ++ 6 files changed, 106 insertions(+), 53 deletions(-) diff --git a/.github/workflows/bitcoin-tests.yml b/.github/workflows/bitcoin-tests.yml index fe8a6dd95f..4dc6bfaa75 100644 --- a/.github/workflows/bitcoin-tests.yml +++ b/.github/workflows/bitcoin-tests.yml @@ -27,6 +27,7 @@ jobs: needs: - build-integration-image strategy: + fail-fast: false matrix: test-name: - tests::neon_integrations::microblock_integration_test diff --git a/docs/rpc-endpoints.md b/docs/rpc-endpoints.md index 7f7b89280b..a5e920340a 100644 --- a/docs/rpc-endpoints.md +++ b/docs/rpc-endpoints.md @@ -28,6 +28,9 @@ Possible values for the "reason" field and "reason_data" field are: * `Deserialization` * The `reason_data` field will be an object containing a `message` string detailing the deserialization error +* `EstimatorError` + * The `reason_data` field will be an object containing a `message` + string detailing the error * `SignatureValidation` * The `reason_data` field will be an object containing a `message` string detailing the signature validation error @@ -343,4 +346,4 @@ object of the following form: Determine whether a given trait is implemented within the specified contract (either explicitly or implicitly). -See OpenAPI [spec](./rpc/openapi.yaml) for details. \ No newline at end of file +See OpenAPI [spec](./rpc/openapi.yaml) for details. diff --git a/src/chainstate/stacks/db/blocks.rs b/src/chainstate/stacks/db/blocks.rs index d5c8583413..75f46f9eae 100644 --- a/src/chainstate/stacks/db/blocks.rs +++ b/src/chainstate/stacks/db/blocks.rs @@ -48,6 +48,7 @@ use chainstate::stacks::{ use clarity_vm::clarity::{ClarityBlockConnection, ClarityConnection, ClarityInstance}; use core::mempool::MAXIMUM_MEMPOOL_TX_CHAINING; use core::*; +use cost_estimates::EstimatorError; use net::BlocksInvData; use net::Error as net_error; use util::db::u64_to_sql; @@ -147,6 +148,7 @@ pub enum MemPoolRejection { TransferRecipientIsSender(PrincipalData), TransferAmountMustBePositive, DBError(db_error), + EstimatorError(EstimatorError), Other(String), } @@ -212,6 +214,7 @@ impl MemPoolRejection { "actual": format!("0x{}", to_hex(&actual.to_be_bytes())) })), ), + EstimatorError(e) => ("EstimatorError", Some(json!({"message": e.to_string()}))), NoSuchContract => ("NoSuchContract", None), NoSuchPublicFunction => ("NoSuchPublicFunction", None), BadFunctionArgument(e) => ( diff --git a/src/core/mempool.rs b/src/core/mempool.rs index 1c2b8c68c9..68b08960aa 100644 --- a/src/core/mempool.rs +++ b/src/core/mempool.rs @@ -21,6 +21,8 @@ use std::ops::Deref; use std::ops::DerefMut; use std::path::{Path, PathBuf}; +use rand::distributions::Uniform; +use rand::prelude::Distribution; use rusqlite::types::ToSql; use rusqlite::Connection; use rusqlite::Error as SqliteError; @@ -172,6 +174,10 @@ pub struct MemPoolWalkSettings { /// Maximum amount of time a miner will spend walking through mempool transactions, in /// milliseconds. This is a soft deadline. pub max_walk_time_ms: u64, + /// Probability percentage to consider a transaction which has not received a cost estimate. + /// That is, with x%, when picking the next transaction to include a block, select one that + /// either failed to get a cost estimate or has not been estimated yet. + pub consider_no_estimate_tx_prob: u8, } impl MemPoolWalkSettings { @@ -179,12 +185,14 @@ impl MemPoolWalkSettings { MemPoolWalkSettings { min_tx_fee: 1, max_walk_time_ms: u64::max_value(), + consider_no_estimate_tx_prob: 5, } } pub fn zero() -> MemPoolWalkSettings { MemPoolWalkSettings { min_tx_fee: 0, max_walk_time_ms: u64::max_value(), + consider_no_estimate_tx_prob: 5, } } } @@ -550,23 +558,59 @@ impl MemPoolDB { Ok(()) } - fn get_next_tx_to_consider(&self) -> Result { - let select_estimate = "SELECT * FROM mempool LEFT OUTER JOIN fee_estimates as f ON mempool.txid = f.txid WHERE - ((origin_nonce = last_known_origin_nonce AND - sponsor_nonce = last_known_sponsor_nonce) OR (last_known_origin_nonce is NULL) OR (last_known_sponsor_nonce is NULL)) - AND f.fee_rate IS NOT NULL ORDER BY f.fee_rate DESC LIMIT 1"; + /// Select the next TX to consider from the pool of transactions without cost estimates. + /// If a transaction is found, returns Some object containing the transaction and a boolean indicating + /// whether or not the miner should propagate transaction receipts back to the estimator. + fn get_next_tx_to_consider_no_estimate( + &self, + ) -> Result, db_error> { let select_no_estimate = "SELECT * FROM mempool LEFT JOIN fee_estimates as f ON mempool.txid = f.txid WHERE ((origin_nonce = last_known_origin_nonce AND sponsor_nonce = last_known_sponsor_nonce) OR (last_known_origin_nonce is NULL) OR (last_known_sponsor_nonce is NULL)) AND f.fee_rate IS NULL ORDER BY tx_fee DESC LIMIT 1"; - let (next_tx, update_estimate): (MemPoolTxInfo, bool) = - match query_row(&self.db, select_estimate, rusqlite::NO_PARAMS)? { - Some(tx) => (tx, false), - None => match query_row(&self.db, select_no_estimate, rusqlite::NO_PARAMS)? { - Some(tx) => (tx, true), + query_row(&self.db, select_no_estimate, rusqlite::NO_PARAMS) + .map(|opt_tx| opt_tx.map(|tx| (tx, false))) + } + + /// Select the next TX to consider from the pool of transactions with cost estimates. + /// If a transaction is found, returns Some object containing the transaction and a boolean indicating + /// whether or not the miner should propagate transaction receipts back to the estimator. + fn get_next_tx_to_consider_with_estimate( + &self, + ) -> Result, db_error> { + let select_estimate = "SELECT * FROM mempool LEFT OUTER JOIN fee_estimates as f ON mempool.txid = f.txid WHERE + ((origin_nonce = last_known_origin_nonce AND + sponsor_nonce = last_known_sponsor_nonce) OR (last_known_origin_nonce is NULL) OR (last_known_sponsor_nonce is NULL)) + AND f.fee_rate IS NOT NULL ORDER BY f.fee_rate DESC LIMIT 1"; + query_row(&self.db, select_estimate, rusqlite::NO_PARAMS) + .map(|opt_tx| opt_tx.map(|tx| (tx, false))) + } + + /// * `start_with_no_estimate` - Pass `true` to make this function + /// start by considering transactions without a cost + /// estimate, and if none are found, use transactions with a cost estimate. + /// Pass `false` for the opposite behavior. + fn get_next_tx_to_consider( + &self, + start_with_no_estimate: bool, + ) -> Result { + let (next_tx, update_estimate): (MemPoolTxInfo, bool) = if start_with_no_estimate { + match self.get_next_tx_to_consider_no_estimate()? { + Some(result) => result, + None => match self.get_next_tx_to_consider_with_estimate()? { + Some(result) => result, None => return Ok(ConsiderTransactionResult::NoTransactions), }, - }; + } + } else { + match self.get_next_tx_to_consider_with_estimate()? { + Some(result) => result, + None => match self.get_next_tx_to_consider_no_estimate()? { + Some(result) => result, + None => return Ok(ConsiderTransactionResult::NoTransactions), + }, + } + }; let mut needs_nonces = vec![]; if next_tx.metadata.last_known_origin_nonce.is_none() { @@ -677,6 +721,9 @@ impl MemPoolDB { debug!("Mempool walk for {}ms", settings.max_walk_time_ms,); + let tx_consideration_sampler = Uniform::new(0, 100); + let mut rng = rand::thread_rng(); + loop { if start_time.elapsed().as_millis() > settings.max_walk_time_ms as u128 { debug!("Mempool iteration deadline exceeded"; @@ -684,7 +731,10 @@ impl MemPoolDB { break; } - match self.get_next_tx_to_consider()? { + let start_with_no_estimate = + tx_consideration_sampler.sample(&mut rng) < settings.consider_no_estimate_tx_prob; + + match self.get_next_tx_to_consider(start_with_no_estimate)? { ConsiderTransactionResult::NoTransactions => { debug!("No more transactions to consider in mempool"); break; @@ -1183,9 +1233,7 @@ impl MemPoolDB { warn!("Error while estimating mempool tx rate"; "txid" => %tx.txid(), "error" => ?e); - return Err(MemPoolRejection::Other( - "Failed to estimate mempool tx rate".into(), - )); + return Err(MemPoolRejection::EstimatorError(e)); } }; @@ -1485,41 +1533,35 @@ mod tests { bytes: Hash160::from_data(&[0x80 | (ix as u8); 32]), }; - for (i, (tx, (origin, sponsor))) in [good_tx] - .iter() - .zip([(origin_address, sponsor_address)].iter()) - .enumerate() - { - let txid = tx.txid(); - let tx_bytes = tx.serialize_to_vec(); - let tx_fee = tx.get_tx_fee(); - - let height = 1 + ix as u64; - - let origin_nonce = 0; // (2 * ix + i) as u64; - let sponsor_nonce = 0; // (2 * ix + i) as u64; - - assert!(!MemPoolDB::db_has_tx(&mempool_tx, &txid).unwrap()); - - MemPoolDB::try_add_tx( - &mut mempool_tx, - &mut chainstate, - &block.0, - &block.1, - txid, - tx_bytes, - tx_fee, - height, - &origin, - origin_nonce, - &sponsor, - sponsor_nonce, - None, - ) - .unwrap(); + let txid = good_tx.txid(); + let tx_bytes = good_tx.serialize_to_vec(); + let tx_fee = good_tx.get_tx_fee(); - assert!(MemPoolDB::db_has_tx(&mempool_tx, &txid).unwrap()); - } + let height = 1 + ix as u64; + + let origin_nonce = 0; // (2 * ix + i) as u64; + let sponsor_nonce = 0; // (2 * ix + i) as u64; + + assert!(!MemPoolDB::db_has_tx(&mempool_tx, &txid).unwrap()); + + MemPoolDB::try_add_tx( + &mut mempool_tx, + &mut chainstate, + &block.0, + &block.1, + txid, + tx_bytes, + tx_fee, + height, + &origin_address, + origin_nonce, + &sponsor_address, + sponsor_nonce, + None, + ) + .unwrap(); + + assert!(MemPoolDB::db_has_tx(&mempool_tx, &txid).unwrap()); mempool_tx.commit().unwrap(); } diff --git a/src/net/download.rs b/src/net/download.rs index 3669c11fb2..100521f82f 100644 --- a/src/net/download.rs +++ b/src/net/download.rs @@ -2554,9 +2554,6 @@ pub mod test { use vm::costs::ExecutionCost; use vm::representations::*; - use crate::cost_estimates::metrics::UnitMetric; - use crate::cost_estimates::UnitEstimator; - use super::*; fn get_peer_availability( diff --git a/testnet/stacks-node/src/config.rs b/testnet/stacks-node/src/config.rs index 4420056064..4e5bfa1da8 100644 --- a/testnet/stacks-node/src/config.rs +++ b/testnet/stacks-node/src/config.rs @@ -503,6 +503,9 @@ impl Config { subsequent_attempt_time_ms: miner .subsequent_attempt_time_ms .unwrap_or(miner_default_config.subsequent_attempt_time_ms), + probability_no_estimate_tx: miner + .probability_no_estimate_tx + .unwrap_or(miner_default_config.probability_no_estimate_tx), }, None => miner_default_config, }; @@ -865,6 +868,7 @@ impl Config { // second or later attempt to mine a block -- give it some time self.miner.subsequent_attempt_time_ms }, + consider_no_estimate_tx_prob: self.miner.probability_no_estimate_tx, }, } } @@ -1336,6 +1340,7 @@ pub struct MinerConfig { pub min_tx_fee: u64, pub first_attempt_time_ms: u64, pub subsequent_attempt_time_ms: u64, + pub probability_no_estimate_tx: u8, } impl MinerConfig { @@ -1344,6 +1349,7 @@ impl MinerConfig { min_tx_fee: 1, first_attempt_time_ms: 1_000, subsequent_attempt_time_ms: 60_000, + probability_no_estimate_tx: 5, } } } @@ -1440,6 +1446,7 @@ pub struct MinerConfigFile { pub min_tx_fee: Option, pub first_attempt_time_ms: Option, pub subsequent_attempt_time_ms: Option, + pub probability_no_estimate_tx: Option, } #[derive(Clone, Deserialize, Default)] From cc3ba5094797b43fe1fa14769544fab8a901a290 Mon Sep 17 00:00:00 2001 From: Aaron Blankstein Date: Fri, 15 Oct 2021 16:38:37 -0500 Subject: [PATCH 20/24] oops, tx_consider_no_estimate should return that the estimate must be updated --- src/core/mempool.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/mempool.rs b/src/core/mempool.rs index 68b08960aa..766f0b0f15 100644 --- a/src/core/mempool.rs +++ b/src/core/mempool.rs @@ -569,7 +569,7 @@ impl MemPoolDB { sponsor_nonce = last_known_sponsor_nonce) OR (last_known_origin_nonce is NULL) OR (last_known_sponsor_nonce is NULL)) AND f.fee_rate IS NULL ORDER BY tx_fee DESC LIMIT 1"; query_row(&self.db, select_no_estimate, rusqlite::NO_PARAMS) - .map(|opt_tx| opt_tx.map(|tx| (tx, false))) + .map(|opt_tx| opt_tx.map(|tx| (tx, true))) } /// Select the next TX to consider from the pool of transactions with cost estimates. From 36deacb4fe81b5182a0c9a49410e403a6faf3bf8 Mon Sep 17 00:00:00 2001 From: Aaron Blankstein Date: Sat, 16 Oct 2021 16:24:13 -0500 Subject: [PATCH 21/24] test: fix ports in new test case, backport atlas test updates from other branch --- .../bitcoin-int-tests/Dockerfile.atlas-test | 17 --------------- .github/workflows/bitcoin-tests.yml | 21 ++++++++++++++++--- src/chainstate/stacks/miner.rs | 2 +- 3 files changed, 19 insertions(+), 21 deletions(-) delete mode 100644 .github/actions/bitcoin-int-tests/Dockerfile.atlas-test diff --git a/.github/actions/bitcoin-int-tests/Dockerfile.atlas-test b/.github/actions/bitcoin-int-tests/Dockerfile.atlas-test deleted file mode 100644 index fc3403fbd2..0000000000 --- a/.github/actions/bitcoin-int-tests/Dockerfile.atlas-test +++ /dev/null @@ -1,17 +0,0 @@ -FROM rust:bullseye - -WORKDIR /src - -COPY . . - -RUN cargo test --no-run --workspace - -RUN cd / && wget https://bitcoin.org/bin/bitcoin-core-0.20.0/bitcoin-0.20.0-x86_64-linux-gnu.tar.gz -RUN cd / && tar -xvzf bitcoin-0.20.0-x86_64-linux-gnu.tar.gz - -RUN ln -s /bitcoin-0.20.0/bin/bitcoind /bin/ - -ENV BITCOIND_TEST 1 -WORKDIR /src/testnet/stacks-node -RUN cargo test -- --test-threads 1 --ignored tests::neon_integrations::atlas_integration_test -RUN cargo test -- --test-threads 1 --ignored tests::neon_integrations::atlas_stress_integration_test diff --git a/.github/workflows/bitcoin-tests.yml b/.github/workflows/bitcoin-tests.yml index 4dc6bfaa75..bac7431a0b 100644 --- a/.github/workflows/bitcoin-tests.yml +++ b/.github/workflows/bitcoin-tests.yml @@ -65,12 +65,27 @@ jobs: TEST_NAME: ${{ matrix.test-name }} run: docker build --build-arg test_name=${{ matrix.test-name }} -f ./.github/actions/bitcoin-int-tests/Dockerfile.bitcoin-tests . atlas-test: - # disable this job/test for now, as the atlas endpoints are currently disabled. if: ${{ true }} runs-on: ubuntu-latest + needs: + - build-integration-image + strategy: + fail-fast: false + matrix: + test-name: + - tests::neon_integrations::atlas_integration_test + - tests::neon_integrations::atlas_stress_integration_test steps: - uses: actions/checkout@v2 - - name: All integration tests with sampled genesis + - name: Download docker image + uses: actions/download-artifact@v2 + with: + name: integration-image.tar + - name: Load docker image + run: docker load -i integration-image.tar && rm integration-image.tar + - name: Atlas integration tests + timeout-minutes: 40 env: DOCKER_BUILDKIT: 1 - run: docker build -f ./.github/actions/bitcoin-int-tests/Dockerfile.atlas-test . + TEST_NAME: ${{ matrix.test-name }} + run: docker build --build-arg test_name=${{ matrix.test-name }} -f ./.github/actions/bitcoin-int-tests/Dockerfile.bitcoin-tests . diff --git a/src/chainstate/stacks/miner.rs b/src/chainstate/stacks/miner.rs index 1ec00283b8..0e656cee32 100644 --- a/src/chainstate/stacks/miner.rs +++ b/src/chainstate/stacks/miner.rs @@ -6954,7 +6954,7 @@ pub mod test { .map(|addr| (addr.to_account_principal(), 100000000000)) .collect(); - let mut peer_config = TestPeerConfig::new("build_anchored_incrementing_nonces", 2006, 2007); + let mut peer_config = TestPeerConfig::new("build_anchored_incrementing_nonces", 2030, 2031); peer_config.initial_balances = initial_balances; let mut peer = TestPeer::new(peer_config); From 3a553069250922ffb0090a5af76a556d34a0cc68 Mon Sep 17 00:00:00 2001 From: Aaron Blankstein Date: Sat, 16 Oct 2021 16:46:04 -0500 Subject: [PATCH 22/24] remember `start_with_no_estimate` value if only updated nonces --- src/core/mempool.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/core/mempool.rs b/src/core/mempool.rs index 766f0b0f15..fcea1a2c2a 100644 --- a/src/core/mempool.rs +++ b/src/core/mempool.rs @@ -723,6 +723,7 @@ impl MemPoolDB { let tx_consideration_sampler = Uniform::new(0, 100); let mut rng = rand::thread_rng(); + let mut remember_start_with_estimate = None; loop { if start_time.elapsed().as_millis() > settings.max_walk_time_ms as u128 { @@ -731,8 +732,9 @@ impl MemPoolDB { break; } - let start_with_no_estimate = - tx_consideration_sampler.sample(&mut rng) < settings.consider_no_estimate_tx_prob; + let start_with_no_estimate = remember_start_with_estimate.unwrap_or_else(|| { + tx_consideration_sampler.sample(&mut rng) < settings.consider_no_estimate_tx_prob + }); match self.get_next_tx_to_consider(start_with_no_estimate)? { ConsiderTransactionResult::NoTransactions => { @@ -740,6 +742,9 @@ impl MemPoolDB { break; } ConsiderTransactionResult::UpdateNonces(addresses) => { + // if we need to update the nonce for the considered transaction, + // use the last value of start_with_no_estimate on the next loop + remember_start_with_estimate = Some(start_with_no_estimate); let mut last_addr = None; for address in addresses.into_iter() { debug!("Update nonce"; "address" => %address); @@ -756,6 +761,9 @@ impl MemPoolDB { } } ConsiderTransactionResult::Consider(consider) => { + // if we actually consider the chosen transaction, + // compute a new start_with_no_estimate on the next loop + remember_start_with_estimate = None; debug!("Consider mempool transaction"; "txid" => %consider.tx.tx.txid(), "origin_addr" => %consider.tx.metadata.origin_address, From ee201e8e90fcb199c033a0d889a3328dbf1e8baa Mon Sep 17 00:00:00 2001 From: Aaron Blankstein Date: Mon, 18 Oct 2021 14:43:10 -0500 Subject: [PATCH 23/24] chore: update comment in gh action --- .github/workflows/bitcoin-tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/bitcoin-tests.yml b/.github/workflows/bitcoin-tests.yml index bac7431a0b..972fcd5fcf 100644 --- a/.github/workflows/bitcoin-tests.yml +++ b/.github/workflows/bitcoin-tests.yml @@ -21,7 +21,7 @@ jobs: with: name: integration-image.tar path: integration-image.tar - # Run sampled genesis tests + # Run integration tests using sampled genesis block sampled-genesis: runs-on: ubuntu-latest needs: From 600520745313e106895e066496323fb2305e5762 Mon Sep 17 00:00:00 2001 From: Aaron Blankstein Date: Tue, 19 Oct 2021 13:01:17 -0500 Subject: [PATCH 24/24] chore: add changelog entry, change config variable name --- CHANGELOG.md | 4 ++++ testnet/stacks-node/src/config.rs | 14 +++++++------- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f553f64ee2..26d9e6419a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,10 @@ and this project adheres to the versioning scheme outlined in the [README.md](RE via node configuration options. See the `README.md` for more information on the configuration. +## Changed + +- Prioritize transaction inclusion in blocks by estimated fee rates (#2859). + ## [2.0.11.3.0] This software update is a point-release to change the transaction selection diff --git a/testnet/stacks-node/src/config.rs b/testnet/stacks-node/src/config.rs index 4e5bfa1da8..93187c6f44 100644 --- a/testnet/stacks-node/src/config.rs +++ b/testnet/stacks-node/src/config.rs @@ -503,9 +503,9 @@ impl Config { subsequent_attempt_time_ms: miner .subsequent_attempt_time_ms .unwrap_or(miner_default_config.subsequent_attempt_time_ms), - probability_no_estimate_tx: miner - .probability_no_estimate_tx - .unwrap_or(miner_default_config.probability_no_estimate_tx), + probability_pick_no_estimate_tx: miner + .probability_pick_no_estimate_tx + .unwrap_or(miner_default_config.probability_pick_no_estimate_tx), }, None => miner_default_config, }; @@ -868,7 +868,7 @@ impl Config { // second or later attempt to mine a block -- give it some time self.miner.subsequent_attempt_time_ms }, - consider_no_estimate_tx_prob: self.miner.probability_no_estimate_tx, + consider_no_estimate_tx_prob: self.miner.probability_pick_no_estimate_tx, }, } } @@ -1340,7 +1340,7 @@ pub struct MinerConfig { pub min_tx_fee: u64, pub first_attempt_time_ms: u64, pub subsequent_attempt_time_ms: u64, - pub probability_no_estimate_tx: u8, + pub probability_pick_no_estimate_tx: u8, } impl MinerConfig { @@ -1349,7 +1349,7 @@ impl MinerConfig { min_tx_fee: 1, first_attempt_time_ms: 1_000, subsequent_attempt_time_ms: 60_000, - probability_no_estimate_tx: 5, + probability_pick_no_estimate_tx: 5, } } } @@ -1446,7 +1446,7 @@ pub struct MinerConfigFile { pub min_tx_fee: Option, pub first_attempt_time_ms: Option, pub subsequent_attempt_time_ms: Option, - pub probability_no_estimate_tx: Option, + pub probability_pick_no_estimate_tx: Option, } #[derive(Clone, Deserialize, Default)]