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.

Sign up for GitHub

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

Jump to bottom

Fix: update microblock mining behavior #2981

Merged
merged 11 commits into from
Feb 8, 2022
Merged

Fix: update microblock mining behavior #2981

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
c77c5a8
Fix: update microblock mining behavior to take block limit hit into a…
pavitthrap Dec 22, 2021
2ef936c
WIP: block_limit integration test times out during block building
pavitthrap Jan 5, 2022
36f01f3
block limit test passing
pavitthrap Jan 7, 2022
862d24e
Added fourth test: microblock limit behavior
pavitthrap Jan 10, 2022
31a538d
Fixes: removed logs, updated test names
pavitthrap Jan 11, 2022
abb0a94
Merge branch 'feat/miner-structured-logging' into fix/update-microblo…
pavitthrap Jan 11, 2022
19e39e5
nit: fix to adding tx to the considered set
pavitthrap Feb 1, 2022
8e76923
Merge branch 'develop' into fix/update-microblock-builder-logic
pavitthrap Feb 1, 2022
e35c9d7
Merge branch 'develop' into fix/update-microblock-builder-logic
pavitthrap Feb 7, 2022
b2ee7ad
logs: changed format
pavitthrap Feb 7, 2022
1b9441a
tests: removed test that tests old mining behavior where limit behavi…
pavitthrap Feb 7, 2022
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
4 changes: 4 additions & 0 deletions .github/workflows/bitcoin-tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,10 @@ jobs:
- 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
- tests::neon_integrations::microblock_large_tx_integration_test
- tests::neon_integrations::block_large_tx_integration_test
- tests::neon_integrations::microblock_limit_hit_integration_test
- tests::neon_integrations::block_limit_hit_integration_test
- tests::neon_integrations::fuzzed_median_fee_rate_estimation_test_window5
- tests::neon_integrations::fuzzed_median_fee_rate_estimation_test_window10
- tests::neon_integrations::use_latest_tip_integration_test
Expand Down
170 changes: 126 additions & 44 deletions src/chainstate/stacks/miner.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -611,8 +611,8 @@ impl<'a> StacksMicroblockBuilder<'a> {
clarity_tx: &mut ClarityTx<'a>,
tx: StacksTransaction,
tx_len: u64,
considered: &mut HashSet<Txid>,
bytes_so_far: u64,
limit_behavior: &BlockLimitFunction,
) -> Result<TransactionResult, Error> {
if tx.anchor_mode != TransactionAnchorMode::OffChainOnly
&& tx.anchor_mode != TransactionAnchorMode::Any
Expand All @@ -625,14 +625,7 @@ impl<'a> StacksMicroblockBuilder<'a> {
),
));
}
if considered.contains(&tx.txid()) {
return Ok(TransactionResult::skipped(
&tx,
"Already considered.".to_string(),
));
} else {
considered.insert(tx.txid());
}

if bytes_so_far + tx_len >= MAX_EPOCH_SIZE.into() {
info!(
"Adding microblock tx {} would exceed epoch data size",
Expand All @@ -643,6 +636,37 @@ impl<'a> StacksMicroblockBuilder<'a> {
Error::BlockTooBigError,
));
}
match limit_behavior {
BlockLimitFunction::CONTRACT_LIMIT_HIT => {
match &tx.payload {
TransactionPayload::ContractCall(cc) => {
// 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(TransactionResult::skipped(
&tx,
"BlockLimitFunction::CONTRACT_LIMIT_HIT".to_string(),
));
}
}
TransactionPayload::SmartContract(_) => {
return Ok(TransactionResult::skipped(
&tx,
"BlockLimitFunction::CONTRACT_LIMIT_HIT".to_string(),
));
}
_ => {}
}
}
BlockLimitFunction::LIMIT_REACHED => {
return Ok(TransactionResult::skipped(
&tx,
"BlockLimitFunction::LIMIT_REACHED".to_string(),
))
}
BlockLimitFunction::NO_LIMIT_HIT => {}
};

let quiet = !cfg!(test);
match StacksChainState::process_transaction(clarity_tx, &tx, quiet) {
Ok((fee, receipt)) => Ok(TransactionResult::success(&tx, fee, receipt)),
Expand Down Expand Up @@ -706,15 +730,22 @@ impl<'a> StacksMicroblockBuilder<'a> {
let mut bytes_so_far = self.runtime.bytes_so_far;
let mut num_txs = self.runtime.num_mined;
let mut tx_events = Vec::new();
let mut block_limit_hit = BlockLimitFunction::NO_LIMIT_HIT;

let mut result = Ok(());
for (tx, tx_len) in txs_and_lens.into_iter() {
if considered.contains(&tx.txid()) {
continue;
} else {
considered.insert(tx.txid());
}

match StacksMicroblockBuilder::mine_next_transaction(
&mut clarity_tx,
tx.clone(),
tx_len,
&mut considered,
bytes_so_far,
&block_limit_hit,
) {
Ok(tx_result) => {
tx_events.push(tx_result.convert_to_event());
Expand All @@ -725,8 +756,29 @@ impl<'a> StacksMicroblockBuilder<'a> {
num_txs += 1;
txs_included.push(tx);
}
TransactionResult::Skipped(..) | TransactionResult::ProcessingError(..) => {
TransactionResult::Skipped(TransactionSkipped { error, .. })
| TransactionResult::ProcessingError(TransactionError { error, .. }) => {
test_debug!("Exclude tx {} from microblock", tx.txid());
match &error {
Error::BlockTooBigError => {
// done mining -- our execution budget is exceeded.
// Make the block from the transactions we did manage to get
test_debug!("Block budget exceeded on tx {}", &tx.txid());
Copy link
Member

@jcnelson jcnelson Jan 31, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this test_debug! statement be merged with the ones in either if branch, so that there's just one debug line emitted here? Makes it easier to grep for this event in test output. Same below.

if block_limit_hit == BlockLimitFunction::NO_LIMIT_HIT {
test_debug!("Switch to mining stx-transfers only");
block_limit_hit = BlockLimitFunction::CONTRACT_LIMIT_HIT;
} else if block_limit_hit
== BlockLimitFunction::CONTRACT_LIMIT_HIT
{
test_debug!(
"Stop mining anchored block due to limit exceeded"
);
block_limit_hit = BlockLimitFunction::LIMIT_REACHED;
break;
}
}
_ => {}
}
continue;
}
}
Expand Down Expand Up @@ -784,11 +836,14 @@ impl<'a> StacksMicroblockBuilder<'a> {
.take()
.expect("Microblock already open and processing");

let mut invalidated_txs = vec![];

let mut bytes_so_far = self.runtime.bytes_so_far;
let mut num_txs = self.runtime.num_mined;
let mut num_selected = 0;
let mut tx_events = Vec::new();
let deadline = get_epoch_time_ms() + (self.settings.max_miner_time_ms as u128);
let mut block_limit_hit = BlockLimitFunction::NO_LIMIT_HIT;

mem_pool.reset_last_known_nonces()?;
let stacks_epoch_id = clarity_tx.get_epoch();
Expand Down Expand Up @@ -821,12 +876,18 @@ impl<'a> StacksMicroblockBuilder<'a> {
return Ok(false);
}

if considered.contains(&mempool_tx.tx.txid()) {
return Ok(true);
} else {
considered.insert(mempool_tx.tx.txid());
}

match StacksMicroblockBuilder::mine_next_transaction(
clarity_tx,
mempool_tx.tx.clone(),
mempool_tx.metadata.len,
&mut considered,
bytes_so_far,
&block_limit_hit,
) {
Ok(tx_result) => {
tx_events.push(tx_result.convert_to_event());
Expand Down Expand Up @@ -861,8 +922,40 @@ impl<'a> StacksMicroblockBuilder<'a> {
num_selected += 1;
Ok(true)
}
TransactionResult::Skipped(..)
| TransactionResult::ProcessingError(..) => Ok(true), // keep iterating
TransactionResult::Skipped(TransactionSkipped {
error,
..
})
| TransactionResult::ProcessingError(TransactionError {
error,
..
}) => {
match &error {
Error::BlockTooBigError => {
// done mining -- our execution budget is exceeded.
// Make the block from the transactions we did manage to get
debug!("Block budget exceeded on tx {}", &mempool_tx.tx.txid());
if block_limit_hit == BlockLimitFunction::NO_LIMIT_HIT {
debug!("Switch to mining stx-transfers only");
block_limit_hit =
BlockLimitFunction::CONTRACT_LIMIT_HIT;
} else if block_limit_hit
== BlockLimitFunction::CONTRACT_LIMIT_HIT
{
debug!(
"Stop mining anchored block due to limit exceeded"
);
block_limit_hit = BlockLimitFunction::LIMIT_REACHED;
return Ok(false);
}
pavitthrap marked this conversation as resolved.
Show resolved Hide resolved
}
Error::TransactionTooBigError => {
invalidated_txs.push(mempool_tx.metadata.txid);
}
_ => {}
}
return Ok(true)
}
}
}
Err(e) => Err(e),
Expand Down Expand Up @@ -900,13 +993,13 @@ impl<'a> StacksMicroblockBuilder<'a> {
self.runtime.considered.replace(considered);
self.runtime.num_mined = num_txs;

mem_pool.drop_txs(&invalidated_txs)?;
event_dispatcher.mempool_txs_dropped(invalidated_txs, MemPoolDropReason::TOO_EXPENSIVE);

match result {
Ok(_) => {}
Err(Error::BlockTooBigError) => {
info!("Block size budget reached with microblocks");
}
Err(e) => {
warn!("Error producing microblock: {}", e);
warn!("Failure building microblock: {}", e);
return Err(e);
}
}
Expand Down Expand Up @@ -1190,11 +1283,11 @@ impl StacksBlockBuilder {
< TX_BLOCK_LIMIT_PROPORTION_HEURISTIC
{
warn!(
"Transaction {} consumed over {}% of block budget, marking as invalid; budget was {}",
tx.txid(),
100 - TX_BLOCK_LIMIT_PROPORTION_HEURISTIC,
&total_budget
);
"Transaction {} consumed over {}% of block budget, marking as invalid; budget was {}",
tx.txid(),
100 - TX_BLOCK_LIMIT_PROPORTION_HEURISTIC,
&total_budget
);
return TransactionResult::error(&tx, Error::TransactionTooBigError);
} else {
warn!(
Expand Down Expand Up @@ -1938,6 +2031,15 @@ impl StacksBlockBuilder {
"error" => ?e);
}
}
mined_origin_nonces.insert(
txinfo.tx.origin_address(),
txinfo.tx.get_origin_nonce(),
);
if let (Some(sponsor_addr), Some(sponsor_nonce)) =
(txinfo.tx.sponsor_address(), txinfo.tx.get_sponsor_nonce())
{
mined_sponsor_nonces.insert(sponsor_addr, sponsor_nonce);
}
}
TransactionResult::Skipped(TransactionSkipped { error, .. })
| TransactionResult::ProcessingError(TransactionError {
Expand Down Expand Up @@ -1965,19 +2067,6 @@ impl StacksBlockBuilder {
}
Error::TransactionTooBigError => {
invalidated_txs.push(txinfo.metadata.txid);
if block_limit_hit == BlockLimitFunction::NO_LIMIT_HIT {
block_limit_hit =
BlockLimitFunction::CONTRACT_LIMIT_HIT;
debug!("Switch to mining stx-transfers only");
} else if block_limit_hit
== BlockLimitFunction::CONTRACT_LIMIT_HIT
{
debug!(
"Stop mining anchored block due to limit exceeded"
);
block_limit_hit = BlockLimitFunction::LIMIT_REACHED;
return Ok(false);
}
}
Error::InvalidStacksTransaction(_, true) => {
// if we have an invalid transaction that was quietly ignored, don't warn here either
Expand All @@ -1990,14 +2079,6 @@ impl StacksBlockBuilder {
}
}

mined_origin_nonces
.insert(txinfo.tx.origin_address(), txinfo.tx.get_origin_nonce());
if let (Some(sponsor_addr), Some(sponsor_nonce)) =
(txinfo.tx.sponsor_address(), txinfo.tx.get_sponsor_nonce())
{
mined_sponsor_nonces.insert(sponsor_addr, sponsor_nonce);
}

Ok(true)
},
);
Expand All @@ -2015,6 +2096,7 @@ impl StacksBlockBuilder {
};

mempool.drop_txs(&invalidated_txs)?;

if let Some(observer) = event_observer {
observer.mempool_txs_dropped(invalidated_txs, MemPoolDropReason::TOO_EXPENSIVE);
}
Expand Down
Loading