diff --git a/.github/workflows/bitcoin-tests.yml b/.github/workflows/bitcoin-tests.yml index 05da90786..36775b395 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 @@ -64,12 +65,12 @@ 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 diff --git a/docs/rpc-endpoints.md b/docs/rpc-endpoints.md index 7f7b89280..a5e920340 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 d5c858341..75f46f9ea 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/chainstate/stacks/miner.rs b/src/chainstate/stacks/miner.rs index 1ec00283b..0e656cee3 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); diff --git a/src/core/mempool.rs b/src/core/mempool.rs index 1c2b8c68c..fcea1a2c2 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, true))) + } + + /// 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,10 @@ 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(); + let mut remember_start_with_estimate = None; + loop { if start_time.elapsed().as_millis() > settings.max_walk_time_ms as u128 { debug!("Mempool iteration deadline exceeded"; @@ -684,12 +732,19 @@ impl MemPoolDB { break; } - match self.get_next_tx_to_consider()? { + 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 => { debug!("No more transactions to consider in mempool"); 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); @@ -706,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, @@ -1183,9 +1241,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 +1541,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 txid = good_tx.txid(); + let tx_bytes = good_tx.serialize_to_vec(); + let tx_fee = good_tx.get_tx_fee(); - let height = 1 + ix as u64; + let height = 1 + ix as u64; - let origin_nonce = 0; // (2 * ix + i) as u64; - let sponsor_nonce = 0; // (2 * ix + i) 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()); + 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(); + 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()); - } + 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 3669c11fb..100521f82 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 442005606..4e5bfa1da 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)]