Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

fix: update miner mempool iterator query to consider both nonces and fee rates #5541

Open
wants to merge 83 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
83 commits
Select commit Hold shift + click to select a range
b0705d0
fix: new query
rafaelcr Dec 7, 2024
a0600b6
chore: remove dev comment
rafaelcr Dec 7, 2024
3719188
style: lint fixes
rafaelcr Dec 11, 2024
2cd116f
fix: add simulated fee rates for null
rafaelcr Dec 13, 2024
685924c
fix: indexes
rafaelcr Dec 13, 2024
e1dac9d
fix: remove tx_fee column
rafaelcr Dec 13, 2024
c587c53
Merge branch 'fix/mempool-query' of github.com:rafaelcr/stacks-core i…
rafaelcr Dec 13, 2024
dd9729c
test: correct tx order
rafaelcr Dec 15, 2024
2406d25
Merge branch 'develop' into fix/mempool-query
rafaelcr Dec 15, 2024
7702f67
fix: nonce ordering
rafaelcr Dec 16, 2024
89d1078
Merge branch 'fix/mempool-query' of github.com:rafaelcr/stacks-core i…
rafaelcr Dec 16, 2024
d0d1f8d
fix: remove now unneccessary index
rafaelcr Dec 16, 2024
cf6c37b
chore: config strategy
rafaelcr Dec 17, 2024
b8b9b89
chore: strategy selection draft
rafaelcr Dec 17, 2024
ea49875
fix: correct tx confirmation order
rafaelcr Dec 17, 2024
a56baef
test: success
rafaelcr Dec 18, 2024
a854f3b
test: missing nonces from table
rafaelcr Dec 18, 2024
c24c2ad
chore: merge upstream develop
rafaelcr Dec 18, 2024
07cf97f
style: fixes
rafaelcr Dec 18, 2024
0b0b821
style: error transforms
rafaelcr Dec 18, 2024
635cfe3
fix: style
rafaelcr Dec 19, 2024
5038c4d
Merge branch 'develop' into fix/mempool-query
rafaelcr Dec 19, 2024
8b4ec39
feat: redesign nonce cache
obycode Dec 19, 2024
e7eb1cc
chore: remove debug print
obycode Dec 20, 2024
9eb3921
chore: small changes from code review
obycode Dec 20, 2024
5269ed5
chore: fix clippy warning
obycode Jan 3, 2025
3a433b4
Merge branch 'develop' into fix/mempool-query
obycode Mar 5, 2025
70bb65d
Merge branch 'feat/persist-nonce-cache-dev' into fix/mempool-query
obycode Mar 6, 2025
f251a4a
fix: remove the `candidate_cache` during mempool iteration
obycode Mar 6, 2025
6893a2a
fix: resolve DB lock issue
obycode Mar 7, 2025
e9cc50c
chore: remove candidate cache
obycode Mar 10, 2025
635cd48
test: add `large_mempool` integration test
obycode Mar 11, 2025
64d85c2
chore: cleanup
obycode Mar 7, 2025
ebadadb
test: insert transactions directly into the mempool
obycode Mar 12, 2025
967b6d1
feat: improve `NextNonceWithHighestFeeRate` algorithm
obycode Mar 12, 2025
3274f08
feat: further mempool iteration improvements and testing
obycode Mar 14, 2025
3709e96
test: use random fees for `larger_mempool` test
obycode Mar 14, 2025
db4dc8c
refactor: move testing utilities to a common location
obycode Mar 14, 2025
031a824
test: add test for mempool walk with large mempool
obycode Mar 14, 2025
6fd9d07
Merge branch 'develop' into fix/mempool-query
obycode Mar 14, 2025
97c3807
chore: remove unused import
obycode Mar 14, 2025
7f93806
test: fix `large_mempool_random_fee`
obycode Mar 15, 2025
36f2cff
chore: adjust `LIMIT` in mempool query
obycode Mar 15, 2025
fcaf8c5
test: add `tests::signer::v0::larger_mempool`
obycode Mar 17, 2025
0b25c2c
chore: fix merge conflict
obycode Mar 17, 2025
38b8464
chore: put candidate cache back for `GlobalFeeRate` strategy
obycode Mar 17, 2025
0b547a8
fix: resolve issue with `GlobalFeeRate` strategy
obycode Mar 17, 2025
c86ff5a
test: fix Bitcoin test exclusions
obycode Mar 17, 2025
66f7b4c
test: update mempool unit test for latest algorithm
obycode Mar 17, 2025
c757dbe
test: refactor and clean up tests
obycode Mar 18, 2025
ab22c32
test: only assert transaction count for new strategy
obycode Mar 18, 2025
67cc828
chore: clarify order in test
obycode Mar 18, 2025
766278f
fix: only open `nonce_conn` once in `iterate_candidates`
obycode Mar 18, 2025
b2503f2
fix: error in LruCache when evicting clean value
obycode Mar 19, 2025
d9ff63c
fix: reset nonce cache when assembled block is not accepted
obycode Mar 20, 2025
883fbac
fix: nonce cache reset logic
obycode Mar 20, 2025
e4271b3
test: fix large mempool test logic
obycode Mar 20, 2025
1c4303c
Merge branch 'develop' into fix/mempool-query
obycode Mar 20, 2025
94b55c5
feat: add `considered_txs` table
obycode Mar 21, 2025
48ca4e5
test: reduce test flakiness by ignoring phantom transactions
obycode Mar 21, 2025
5abe94d
Merge branch 'develop' into fix/mempool-query
obycode Mar 22, 2025
b6d9604
refactor: improvements to nonce cache from review
obycode Mar 24, 2025
2e683a3
test: add proptests for `LruCache`
obycode Mar 25, 2025
54d476f
feat: make `LruCache` methods fallible
obycode Mar 25, 2025
fdb5348
feat: handle fallible `LruCache` in `NonceCache`
obycode Mar 25, 2025
e6775d1
feat: add key check for cache safety
obycode Mar 25, 2025
2b79d70
test: update proptests for fallible operations
obycode Mar 25, 2025
4831d47
test: add LRU proptest
obycode Mar 25, 2025
0fcc361
test: improve proptests for `LruCache`
obycode Mar 25, 2025
fc8adfc
refactor: reorganize the `LruCache` implementation
obycode Mar 25, 2025
d7df7c9
refactor: move core::util to core::test_util
obycode Mar 25, 2025
9756fb0
feat: simplify `to_addr` implementation
obycode Mar 25, 2025
2f7cdb2
refactor: finish rename of core::util to core::test_util
obycode Mar 25, 2025
0db4115
chore: minor changes from review
obycode Mar 25, 2025
30a68d8
docs: add more comments to mempool iteration algorithm
obycode Mar 25, 2025
cc234be
Merge branch 'fix/mempool-query' into fix/mempool-query-errors
obycode Mar 26, 2025
8754877
chore: update copyright date
obycode Mar 26, 2025
af945f2
fix: merge error
obycode Mar 26, 2025
75b913b
Merge branch 'develop' into fix/mempool-query
obycode Mar 26, 2025
39610a8
fix: merge error
obycode Mar 26, 2025
3fc8e23
Merge pull request #1 from obycode/fix/mempool-query-errors
obycode Mar 27, 2025
f71e0ec
feat: use a proper error type in `LruCache`
obycode Mar 27, 2025
1035f7b
Merge branch 'develop' into fix/mempool-query
obycode Mar 27, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
245 changes: 245 additions & 0 deletions stackslib/src/chainstate/stacks/tests/block_construction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ use clarity::vm::costs::LimitedCostTracker;
use clarity::vm::database::ClarityDatabase;
use clarity::vm::test_util::TEST_BURN_STATE_DB;
use clarity::vm::types::*;
use mempool::MemPoolWalkStrategy;
use rand::seq::SliceRandom;
use rand::{thread_rng, Rng};
use rusqlite::params;
Expand Down Expand Up @@ -5087,3 +5088,247 @@ fn paramaterized_mempool_walk_test(
},
);
}

#[test]
/// Test that the mempool walk query ignores old nonces and prefers next possible nonces before higher global fees.
fn mempool_walk_test_next_nonce_with_highest_fee_rate_strategy() {
let key_address_pairs: Vec<(Secp256k1PrivateKey, StacksAddress)> = (0..7)
.map(|_user_index| {
let privk = StacksPrivateKey::new();
let addr = StacksAddress::from_public_keys(
C32_ADDRESS_VERSION_TESTNET_SINGLESIG,
&AddressHashMode::SerializeP2PKH,
1,
&vec![StacksPublicKey::from_private(&privk)],
)
.unwrap();
(privk, addr)
})
.collect();
let accounts: Vec<String> = key_address_pairs
.iter()
.map(|(_, b)| b.to_string())
.collect();
let address_0 = accounts[0].to_string();
let address_1 = accounts[1].to_string();
let address_2 = accounts[2].to_string();
let address_3 = accounts[3].to_string();
let address_4 = accounts[4].to_string();
let address_5 = accounts[5].to_string();
let address_6 = accounts[6].to_string();

let test_name = function_name!();
let mut peer_config = TestPeerConfig::new(&test_name, 0, 0);
peer_config.initial_balances = vec![];
for (privk, addr) in &key_address_pairs {
peer_config
.initial_balances
.push((addr.to_account_principal(), 1000000000));
}

let recipient =
StacksAddress::from_string("ST1RFD5Q2QPK3E0F08HG9XDX7SSC7CNRS0QR0SGEV").unwrap();

let mut chainstate =
instantiate_chainstate_with_balances(false, 0x80000000, &test_name, vec![]);
let chainstate_path = chainstate_path(&test_name);
let mut mempool = MemPoolDB::open_test(false, 0x80000000, &chainstate_path).unwrap();
let b_1 = make_block(
&mut chainstate,
ConsensusHash([0x1; 20]),
&(
FIRST_BURNCHAIN_CONSENSUS_HASH.clone(),
FIRST_STACKS_BLOCK_HASH.clone(),
),
1,
1,
);
let b_2 = make_block(&mut chainstate, ConsensusHash([0x2; 20]), &b_1, 2, 2);

let mut tx_events = Vec::new();

// Simulate next possible nonces for **some** addresses. Leave some blank so we can test the case where the nonce cannot be
// found on the db table and has to be pulled from the MARF.
let mempool_tx = mempool.tx_begin().unwrap();
mempool_tx
.execute(
"INSERT INTO nonces (address, nonce) VALUES (?, ?), (?, ?), (?, ?), (?, ?), (?, ?)",
params![address_0, 2, address_1, 1, address_2, 6, address_4, 1, address_5, 0],
)
.unwrap();
mempool_tx.commit().unwrap();

// Test transactions with a wide variety of origin/sponsor configurations and fee rate values. Some transactions do not have a
// sponsor, some others do, and some others are sponsored by other sponsors. All will be in flight at the same time.
//
// tuple shape: (origin_address_index, origin_nonce, sponsor_address_index, sponsor_nonce, fee_rate)
let test_vectors = vec![
(0, 0, 0, 0, 100.0), // Old origin nonce - ignored
(0, 1, 0, 1, 200.0), // Old origin nonce - ignored
(0, 2, 0, 2, 300.0),
(0, 3, 0, 3, 400.0),
(0, 4, 3, 0, 500.0), // Nonce 0 for address 3 is not in the table but will be valid on MARF
(1, 0, 1, 0, 400.0), // Old origin nonce - ignored
(1, 1, 3, 1, 600.0),
(1, 2, 3, 2, 700.0),
(1, 3, 3, 3, 800.0),
(1, 4, 1, 4, 1200.0),
(2, 3, 2, 3, 9000.0), // Old origin nonce - ignored
(2, 4, 2, 4, 9000.0), // Old origin nonce - ignored
(2, 5, 2, 5, 9000.0), // Old origin nonce - ignored
(2, 6, 4, 0, 900.0), // Old sponsor nonce - ignored
(2, 6, 4, 1, 1000.0),
(2, 7, 4, 2, 800.0),
(2, 8, 2, 8, 1000.0),
(2, 9, 3, 5, 1000.0),
(2, 10, 3, 6, 1500.0),
(3, 4, 3, 4, 100.0),
(4, 3, 5, 2, 500.0),
(5, 0, 5, 0, 500.0),
(5, 1, 5, 1, 500.0),
(5, 3, 4, 4, 2000.0),
(5, 4, 4, 5, 2000.0),
(6, 2, 6, 2, 1000.0), // Address has nonce 0 in MARF - ignored
];
for (origin_index, origin_nonce, sponsor_index, sponsor_nonce, fee_rate) in
test_vectors.into_iter()
{
// Create tx, either standard or sponsored
let mut tx = if origin_index != sponsor_index {
let payload = TransactionPayload::TokenTransfer(
recipient.to_account_principal(),
1,
TokenTransferMemo([0; 34]),
);
sign_sponsored_singlesig_tx(
payload.into(),
&key_address_pairs[origin_index].0,
&key_address_pairs[sponsor_index].0,
origin_nonce,
sponsor_nonce,
200,
)
} else {
make_user_stacks_transfer(
&key_address_pairs[origin_index].0,
origin_nonce,
200,
&recipient.to_account_principal(),
1,
)
};

let mut mempool_tx = mempool.tx_begin().unwrap();

let origin_address = tx.origin_address();
let sponsor_address = tx.sponsor_address().unwrap_or(origin_address);
tx.set_tx_fee(fee_rate as u64);
let txid = tx.txid();
let tx_bytes = tx.serialize_to_vec();
let tx_fee = tx.get_tx_fee();
let height = 100;
MemPoolDB::try_add_tx(
&mut mempool_tx,
&mut chainstate,
&b_1.0,
&b_1.1,
true,
txid,
tx_bytes,
tx_fee,
height,
&origin_address,
origin_nonce,
&sponsor_address,
sponsor_nonce,
None,
)
.unwrap();
mempool_tx
.execute(
"UPDATE mempool SET fee_rate = ? WHERE txid = ?",
params![Some(fee_rate), &txid],
)
.unwrap();

mempool_tx.commit().unwrap();
}

// Visit transactions using the `NextNonceWithHighestFeeRate` strategy. Keep a record of the order of visits so we can compare
// at the end.
let mut mempool_settings = MemPoolWalkSettings::default();
mempool_settings.strategy = MemPoolWalkStrategy::NextNonceWithHighestFeeRate;
let mut considered_txs = vec![];
let deadline = get_epoch_time_ms() + 30000;
chainstate.with_read_only_clarity_tx(
&TEST_BURN_STATE_DB,
&StacksBlockHeader::make_index_block_hash(&b_2.0, &b_2.1),
|clarity_conn| {
loop {
if mempool
.iterate_candidates::<_, ChainstateError, _>(
clarity_conn,
&mut tx_events,
mempool_settings.clone(),
|_, available_tx, _| {
considered_txs.push((
available_tx.tx.metadata.origin_address.to_string(),
available_tx.tx.metadata.origin_nonce,
available_tx.tx.metadata.sponsor_address.to_string(),
available_tx.tx.metadata.sponsor_nonce,
available_tx.tx.metadata.tx_fee,
));
Ok(Some(
// Generate any success result
TransactionResult::success(
&available_tx.tx.tx,
available_tx.tx.metadata.tx_fee,
StacksTransactionReceipt::from_stx_transfer(
available_tx.tx.tx.clone(),
vec![],
Value::okay(Value::Bool(true)).unwrap(),
ExecutionCost::ZERO,
),
)
.convert_to_event(),
))
},
)
.unwrap()
.0
== 0
{
break;
}
assert!(get_epoch_time_ms() < deadline, "test timed out");
}

// Expected transaction consideration order, sorted by mineable first (next origin+sponsor nonces, highest fee).
// Ignores old and very future nonces.
let expected_tx_order = vec![
(address_2.clone(), 6, address_4.clone(), 1, 1000),
(address_2.clone(), 7, address_4.clone(), 2, 800),
(address_2.clone(), 8, address_2.clone(), 8, 1000),
(address_5.clone(), 0, address_5.clone(), 0, 500),
(address_5.clone(), 1, address_5.clone(), 1, 500),
(address_4.clone(), 3, address_5.clone(), 2, 500),
(address_5.clone(), 3, address_4.clone(), 4, 2000),
(address_5.clone(), 4, address_4.clone(), 5, 2000),
(address_0.clone(), 2, address_0.clone(), 2, 300),
(address_0.clone(), 3, address_0.clone(), 3, 400),
(address_0.clone(), 4, address_3.clone(), 0, 500),
(address_1.clone(), 1, address_3.clone(), 1, 600),
(address_1.clone(), 2, address_3.clone(), 2, 700),
(address_1.clone(), 3, address_3.clone(), 3, 800),
(address_1.clone(), 4, address_1.clone(), 4, 1200),
(address_3.clone(), 4, address_3.clone(), 4, 100),
(address_2.clone(), 9, address_3.clone(), 5, 1000),
(address_2.clone(), 10, address_3.clone(), 6, 1500),
];
assert_eq!(
considered_txs, expected_tx_order,
"Mempool should visit transactions in the correct order while ignoring past nonces",
);
},
);
}
31 changes: 31 additions & 0 deletions stackslib/src/chainstate/stacks/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1396,6 +1396,37 @@ pub fn sign_standard_singlesig_tx(
tx_signer.get_tx().unwrap()
}

pub fn sign_sponsored_singlesig_tx(
payload: TransactionPayload,
origin: &StacksPrivateKey,
sponsor: &StacksPrivateKey,
origin_nonce: u64,
sponsor_nonce: u64,
tx_fee: u64,
) -> StacksTransaction {
let mut origin_spending_condition =
TransactionSpendingCondition::new_singlesig_p2pkh(StacksPublicKey::from_private(origin))
.expect("Failed to create p2pkh spending condition from public key.");
origin_spending_condition.set_nonce(origin_nonce);
origin_spending_condition.set_tx_fee(tx_fee);
let mut sponsored_spending_condition =
TransactionSpendingCondition::new_singlesig_p2pkh(StacksPublicKey::from_private(sponsor))
.expect("Failed to create p2pkh spending condition from public key.");
sponsored_spending_condition.set_nonce(sponsor_nonce);
sponsored_spending_condition.set_tx_fee(tx_fee);
let auth = TransactionAuth::Sponsored(origin_spending_condition, sponsored_spending_condition);
let mut unsigned_tx = StacksTransaction::new(TransactionVersion::Testnet, auth, payload);

unsigned_tx.chain_id = 0x80000000;
unsigned_tx.post_condition_mode = TransactionPostConditionMode::Allow;

let mut tx_signer = StacksTransactionSigner::new(&unsigned_tx);
tx_signer.sign_origin(origin).unwrap();
tx_signer.sign_sponsor(sponsor).unwrap();

tx_signer.get_tx().unwrap()
}

pub fn get_stacks_account(peer: &mut TestPeer, addr: &PrincipalData) -> StacksAccount {
let account = peer
.with_db_state(|ref mut sortdb, ref mut chainstate, _, _| {
Expand Down
20 changes: 19 additions & 1 deletion stackslib/src/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ use crate::chainstate::stacks::index::storage::TrieHashCalculationMode;
use crate::chainstate::stacks::miner::{BlockBuilderSettings, MinerStatus};
use crate::chainstate::stacks::MAX_BLOCK_LEN;
use crate::config::chain_data::MinerStats;
use crate::core::mempool::{MemPoolWalkSettings, MemPoolWalkTxTypes};
use crate::core::mempool::{MemPoolWalkSettings, MemPoolWalkStrategy, MemPoolWalkTxTypes};
use crate::core::{
MemPoolDB, StacksEpoch, StacksEpochExtension, StacksEpochId,
BITCOIN_TESTNET_FIRST_BLOCK_HEIGHT, BITCOIN_TESTNET_STACKS_25_BURN_HEIGHT,
Expand Down Expand Up @@ -1064,6 +1064,7 @@ impl Config {
BlockBuilderSettings {
max_miner_time_ms: miner_config.nakamoto_attempt_time_ms,
mempool_settings: MemPoolWalkSettings {
strategy: miner_config.mempool_walk_strategy,
max_walk_time_ms: miner_config.nakamoto_attempt_time_ms,
consider_no_estimate_tx_prob: miner_config.probability_pick_no_estimate_tx,
nonce_cache_size: miner_config.nonce_cache_size,
Expand Down Expand Up @@ -1107,6 +1108,7 @@ impl Config {
// second or later attempt to mine a block -- give it some time
miner_config.subsequent_attempt_time_ms
},
strategy: miner_config.mempool_walk_strategy,
consider_no_estimate_tx_prob: miner_config.probability_pick_no_estimate_tx,
nonce_cache_size: miner_config.nonce_cache_size,
candidate_retry_cache_size: miner_config.candidate_retry_cache_size,
Expand Down Expand Up @@ -2100,6 +2102,8 @@ pub struct MinerConfig {
pub microblock_attempt_time_ms: u64,
/// Max time to assemble Nakamoto block
pub nakamoto_attempt_time_ms: u64,
/// Strategy to follow when picking next mempool transactions to consider.
pub mempool_walk_strategy: MemPoolWalkStrategy,
pub probability_pick_no_estimate_tx: u8,
pub block_reward_recipient: Option<PrincipalData>,
/// If possible, mine with a p2wpkh address
Expand Down Expand Up @@ -2180,6 +2184,7 @@ impl Default for MinerConfig {
activated_vrf_key_path: None,
fast_rampup: false,
underperform_stop_threshold: None,
mempool_walk_strategy: MemPoolWalkStrategy::GlobalFeeRate,
txs_to_consider: MemPoolWalkTxTypes::all(),
filter_origins: HashSet::new(),
max_reorg_depth: 3,
Expand Down Expand Up @@ -2553,6 +2558,7 @@ pub struct MinerConfigFile {
pub subsequent_attempt_time_ms: Option<u64>,
pub microblock_attempt_time_ms: Option<u64>,
pub nakamoto_attempt_time_ms: Option<u64>,
pub mempool_walk_strategy: Option<String>,
pub probability_pick_no_estimate_tx: Option<u8>,
pub block_reward_recipient: Option<String>,
pub segwit: Option<bool>,
Expand Down Expand Up @@ -2670,6 +2676,18 @@ impl MinerConfigFile {
activated_vrf_key_path: self.activated_vrf_key_path.clone(),
fast_rampup: self.fast_rampup.unwrap_or(miner_default_config.fast_rampup),
underperform_stop_threshold: self.underperform_stop_threshold,
mempool_walk_strategy: {
if let Some(mempool_walk_strategy) = &self.mempool_walk_strategy {
match str::parse(&mempool_walk_strategy) {
Ok(strategy) => strategy,
Err(e) => {
panic!("could not parse '{mempool_walk_strategy}': {e}");
},
}
} else {
MemPoolWalkStrategy::GlobalFeeRate
}
},
txs_to_consider: {
if let Some(txs_to_consider) = &self.txs_to_consider {
txs_to_consider
Expand Down
Loading