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

Miner can continue mining after an empty tenure followed by empty sortition #5411

Merged
merged 21 commits into from
Nov 8, 2024
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
559b1d6
dx: add set/get helper to TestFlag
hstove Oct 31, 2024
f5c8b03
feat: setup test to repro #5400
hstove Oct 31, 2024
92528c7
Merge branch 'develop' of https://github.com/stacks-network/stacks-co…
jferrant Nov 5, 2024
1aa6793
Apply clippy
jferrant Nov 5, 2024
d0355cb
Use multiple miners to trigger weird chain stall
jferrant Nov 5, 2024
e459b8c
Update test description
jferrant Nov 5, 2024
25cd9d9
Update test to be more streamlined and ensure no extra commits make i…
jferrant Nov 5, 2024
cf1d13c
Add fast blocks test to CI
jferrant Nov 6, 2024
80fbce7
BUGFIX: use the block election snapshot when creating a tenure extend…
jferrant Nov 6, 2024
b9438fa
Cargo fmt
jferrant Nov 6, 2024
514c0d0
Fix VRF proof calculation and update test to be more expansive
jferrant Nov 7, 2024
a28a6dd
Cleanup tests
jferrant Nov 7, 2024
80c6a72
Merge branch 'develop' of https://github.com/stacks-network/stacks-co…
jferrant Nov 7, 2024
f2a8d3e
Fix tenure extend to build off the chain tip correclty
jferrant Nov 7, 2024
8c9d129
Remove TODO comment
jferrant Nov 7, 2024
f42bc48
Fix miner forking by being strict about sortition winners
jferrant Nov 7, 2024
2dced24
CRC: when reissuing a tenure change payload use the winning stacks bl…
jferrant Nov 8, 2024
ebd7904
Cleanup a wait
jferrant Nov 8, 2024
d2557f3
Merge pull request #5435 from stacks-network/chore/rewrite-miner-forking
jcnelson Nov 8, 2024
6ec21f5
Merge branch 'develop' into fix/fast-block-test
jcnelson Nov 8, 2024
7e70420
Merge branch 'develop' of https://github.com/stacks-network/stacks-co…
jferrant Nov 8, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/bitcoin-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ jobs:
- tests::signer::v0::signing_in_0th_tenure_of_reward_cycle
- tests::signer::v0::continue_after_tenure_extend
- tests::signer::v0::multiple_miners_with_custom_chain_id
- tests::signer::v0::continue_after_fast_block_no_sortition
- tests::nakamoto_integrations::burn_ops_integration_test
- tests::nakamoto_integrations::check_block_heights
- tests::nakamoto_integrations::clarity_burn_state
Expand Down
20 changes: 13 additions & 7 deletions testnet/stacks-node/src/nakamoto_node/miner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,8 @@ pub enum MinerReason {
/// sortition.
burn_view_consensus_hash: ConsensusHash,
},
/// The miner thread was spawned to initialize a prior empty tenure
EmptyTenure,
}

impl std::fmt::Display for MinerReason {
Expand All @@ -121,6 +123,7 @@ impl std::fmt::Display for MinerReason {
f,
"Extended: burn_view_consensus_hash = {burn_view_consensus_hash:?}",
),
MinerReason::EmptyTenure => write!(f, "EmptyTenure"),
}
}
}
Expand Down Expand Up @@ -919,22 +922,25 @@ impl BlockMinerThread {
fn make_vrf_proof(&mut self) -> Option<VRFProof> {
// if we're a mock miner, then make sure that the keychain has a keypair for the mocked VRF
// key
let sortition_hash = if matches!(self.reason, MinerReason::EmptyTenure) {
self.burn_election_block.sortition_hash
} else {
self.burn_block.sortition_hash
};
jferrant marked this conversation as resolved.
Show resolved Hide resolved
let vrf_proof = if self.config.get_node_config(false).mock_mining {
self.keychain.generate_proof(
VRF_MOCK_MINER_KEY,
self.burn_block.sortition_hash.as_bytes(),
)
self.keychain
.generate_proof(VRF_MOCK_MINER_KEY, sortition_hash.as_bytes())
} else {
self.keychain.generate_proof(
self.registered_key.target_block_height,
self.burn_block.sortition_hash.as_bytes(),
sortition_hash.as_bytes(),
)
};

debug!(
"Generated VRF Proof: {} over {} ({},{}) with key {}",
vrf_proof.to_hex(),
&self.burn_block.sortition_hash,
&sortition_hash,
&self.burn_block.block_height,
&self.burn_block.burn_header_hash,
&self.registered_key.vrf_public_key.to_hex()
Expand Down Expand Up @@ -1155,7 +1161,7 @@ impl BlockMinerThread {
};

let (tenure_change_tx, coinbase_tx) = match &self.reason {
MinerReason::BlockFound => {
MinerReason::BlockFound | MinerReason::EmptyTenure => {
let tenure_change_tx =
self.generate_tenure_change_tx(current_miner_nonce, payload)?;
let coinbase_tx =
Expand Down
65 changes: 40 additions & 25 deletions testnet/stacks-node/src/nakamoto_node/relayer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -880,23 +880,14 @@ impl RelayerThread {
SortitionDB::get_canonical_stacks_chain_tip_hash(self.sortdb.conn()).unwrap();
let canonical_stacks_tip =
StacksBlockId::new(&canonical_stacks_tip_ch, &canonical_stacks_tip_bh);
let block_election_snapshot =
SortitionDB::get_block_snapshot_consensus(self.sortdb.conn(), &canonical_stacks_tip_ch)
.map_err(|e| {
error!("Relayer: failed to get block snapshot for canonical tip: {e:?}");
NakamotoNodeError::SnapshotNotFoundForChainTip
})?
.ok_or_else(|| {
error!("Relayer: failed to get block snapshot for canonical tip");
NakamotoNodeError::SnapshotNotFoundForChainTip
})?;

let Some(ref mining_key) = self.config.miner.mining_key else {
return Ok(());
};
let mining_pkh = Hash160::from_node_public_key(&StacksPublicKey::from_private(mining_key));

let last_winner_snapshot = {
// If we won the last sortition, then we should start a new tenure off of it.
jferrant marked this conversation as resolved.
Show resolved Hide resolved
let last_block_election_snapshot = {
let ih = self.sortdb.index_handle(&burn_tip.sortition_id);
ih.get_last_snapshot_with_sortition(burn_tip.block_height)
.map_err(|e| {
Expand All @@ -905,29 +896,59 @@ impl RelayerThread {
})?
};

let won_last_sortition = last_winner_snapshot.miner_pk_hash == Some(mining_pkh);
let won_last_sortition = last_block_election_snapshot.miner_pk_hash == Some(mining_pkh);
debug!(
"Relayer: Current burn block had no sortition. Checking for tenure continuation.";
"won_last_sortition" => won_last_sortition,
"current_mining_pkh" => %mining_pkh,
"last_winner_snapshot.miner_pk_hash" => ?last_winner_snapshot.miner_pk_hash,
"last_block_election_snapshot.consensus_hash" => %last_block_election_snapshot.consensus_hash,
"last_block_election_snapshot.miner_pk_hash" => ?last_block_election_snapshot.miner_pk_hash,
"canonical_stacks_tip_id" => %canonical_stacks_tip,
"canonical_stacks_tip_ch" => %canonical_stacks_tip_ch,
"block_election_ch" => %block_election_snapshot.consensus_hash,
"burn_view_ch" => %new_burn_view,
);

if !won_last_sortition {
return Ok(());
}

let last_non_empty_sortition_snapshot =
SortitionDB::get_block_snapshot_consensus(self.sortdb.conn(), &canonical_stacks_tip_ch)
jferrant marked this conversation as resolved.
Show resolved Hide resolved
.map_err(|e| {
error!("Relayer: failed to get block snapshot for canonical tip: {e:?}");
NakamotoNodeError::SnapshotNotFoundForChainTip
})?
.ok_or_else(|| {
error!("Relayer: failed to get block snapshot for canonical tip");
NakamotoNodeError::SnapshotNotFoundForChainTip
})?;

let won_last_non_empty_sortition_snapshot =
last_non_empty_sortition_snapshot.miner_pk_hash == Some(mining_pkh);

let (parent_tenure_start, block_election_snapshot, reason) =
if !won_last_non_empty_sortition_snapshot {
debug!("Relayer: Failed to issue a tenure change payload in our last tenure. Issue a new tenure change payload.");
(
canonical_stacks_tip,
jferrant marked this conversation as resolved.
Show resolved Hide resolved
last_block_election_snapshot,
MinerReason::EmptyTenure,
)
} else {
debug!("Relayer: Successfully issued a tenure change payload in its tenure. Issue a continue extend from the chain tip.");
(
canonical_stacks_tip, //For tenure extend, we sould be extending off the canonical tip
jferrant marked this conversation as resolved.
Show resolved Hide resolved
last_non_empty_sortition_snapshot,
MinerReason::Extended {
burn_view_consensus_hash: new_burn_view,
},
)
};
match self.start_new_tenure(
canonical_stacks_tip, // For tenure extend, we should be extending off the canonical tip
parent_tenure_start,
block_election_snapshot,
burn_tip,
MinerReason::Extended {
burn_view_consensus_hash: new_burn_view,
},
reason,
) {
Ok(()) => {
debug!("Relayer: successfully started new tenure.");
Expand Down Expand Up @@ -1000,13 +1021,7 @@ impl RelayerThread {

#[cfg(test)]
fn fault_injection_skip_block_commit(&self) -> bool {
self.globals
.counters
.naka_skip_commit_op
.0
.lock()
.unwrap()
.unwrap_or(false)
self.globals.counters.naka_skip_commit_op.get()
}

#[cfg(not(test))]
Expand Down
15 changes: 14 additions & 1 deletion testnet/stacks-node/src/run_loop/neon.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,19 @@ impl Default for TestFlag {
}
}

#[cfg(test)]
impl TestFlag {
/// Set the test flag to the given value
pub fn set(&self, value: bool) {
*self.0.lock().unwrap() = Some(value);
}

/// Get the test flag value. Defaults to false if the flag is not set.
pub fn get(&self) -> bool {
self.0.lock().unwrap().unwrap_or(false)
}
}

#[derive(Clone, Default)]
pub struct Counters {
pub blocks_processed: RunLoopCounter,
Expand Down Expand Up @@ -1026,7 +1039,7 @@ impl RunLoop {
/// This function will block by looping infinitely.
/// It will start the burnchain (separate thread), set-up a channel in
/// charge of coordinating the new blocks coming from the burnchain and
/// the nodes, taking turns on tenures.
/// the nodes, taking turns on tenures.
///
/// Returns `Option<NeonGlobals>` so that data can be passed to `NakamotoNode`
pub fn start(
Expand Down
8 changes: 4 additions & 4 deletions testnet/stacks-node/src/tests/nakamoto_integrations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4985,7 +4985,7 @@ fn forked_tenure_is_ignored() {

// Unpause the broadcast of Tenure B's block, do not submit commits, and do not allow blocks to
// be processed
test_skip_commit_op.0.lock().unwrap().replace(true);
test_skip_commit_op.set(true);
TEST_BROADCAST_STALL.lock().unwrap().replace(false);

// Wait for a stacks block to be broadcasted.
Expand Down Expand Up @@ -5038,7 +5038,7 @@ fn forked_tenure_is_ignored() {
.expect("Mutex poisoned")
.get_stacks_blocks_processed();
next_block_and(&mut btc_regtest_controller, 60, || {
test_skip_commit_op.0.lock().unwrap().replace(false);
test_skip_commit_op.set(false);
TEST_BLOCK_ANNOUNCE_STALL.lock().unwrap().replace(false);
let commits_count = commits_submitted.load(Ordering::SeqCst);
let blocks_count = mined_blocks.load(Ordering::SeqCst);
Expand Down Expand Up @@ -6973,7 +6973,7 @@ fn continue_tenure_extend() {
.get_stacks_blocks_processed();

info!("Pausing commit ops to trigger a tenure extend.");
test_skip_commit_op.0.lock().unwrap().replace(true);
test_skip_commit_op.set(true);

next_block_and(&mut btc_regtest_controller, 60, || Ok(true)).unwrap();

Expand Down Expand Up @@ -7072,7 +7072,7 @@ fn continue_tenure_extend() {
}

info!("Resuming commit ops to mine regular tenures.");
test_skip_commit_op.0.lock().unwrap().replace(false);
test_skip_commit_op.set(false);

// Mine 15 more regular nakamoto tenures
for _i in 0..15 {
Expand Down
Loading