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 multiple_miners* tests #5237

Merged
merged 23 commits into from
Oct 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
8ddddae
Add positive integer for pox_sync_sample_secs and wait_on_interim_blo…
jferrant Sep 24, 2024
082fc51
Merge branch 'develop' into chore/fix-multiple-miners
jcnelson Sep 24, 2024
9360397
Do not use a test_observer in boot_to_epoch_3 to enable use with mult…
jferrant Sep 25, 2024
369bc78
Merge branch 'develop' of https://github.com/stacks-network/stacks-co…
jferrant Sep 25, 2024
257e7ec
Merge branch 'chore/fix-multiple-miners' of https://github.com/stacks…
jferrant Sep 25, 2024
1f53fac
Do not blanket set pox_sync_sample_secs to a postiive integer
jferrant Sep 25, 2024
b126d17
Merge branch 'develop' of https://github.com/stacks-network/stacks-co…
jferrant Sep 25, 2024
cd49977
Do not advance unless the bootstrapped or follower node also hits epo…
jferrant Sep 26, 2024
db564e9
Merge branch 'develop' of https://github.com/stacks-network/stacks-co…
jferrant Sep 26, 2024
c60e4a6
Test: try increasing a timeout to see what CI does
jferrant Sep 27, 2024
28d06b7
Merge branch 'develop' of https://github.com/stacks-network/stacks-co…
jferrant Sep 30, 2024
9c89e46
Do not wait for an exact number of block rejections
jferrant Sep 30, 2024
065a89b
Merge branch 'develop' of https://github.com/stacks-network/stacks-co…
jferrant Sep 30, 2024
fdcfcdf
Add some logging to bitcoind test
jferrant Sep 30, 2024
51d6000
Fix microblocks disabled test to allow at least one rather than stric…
jferrant Sep 30, 2024
4c311bb
Convert logs to info in test
jferrant Sep 30, 2024
836a97a
CRC: remove dead code
jferrant Oct 1, 2024
4d1f9a9
Merge branch 'develop' of https://github.com/stacks-network/stacks-co…
jferrant Oct 1, 2024
61eab90
Change vec to hashset in wait_for_block_rejections
jferrant Oct 1, 2024
4752a90
Do not attempt to process a block validation response for an already …
jferrant Oct 1, 2024
b9d3b90
Merge branch 'develop' of https://github.com/stacks-network/stacks-co…
jferrant Oct 1, 2024
9eb4e05
test: remove `signer_vote_if_needed`
obycode Oct 1, 2024
db105e0
Increase pox_sync_sample_secs to 5 to be on the safe side when waitin…
jferrant Oct 2, 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
3 changes: 0 additions & 3 deletions stacks-signer/src/client/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,6 @@ pub enum ClientError {
/// Invalid response from the stacks node
#[error("Invalid response from the stacks node: {0}")]
InvalidResponse(String),
/// A successful sortition has not occurred yet
#[error("The Stacks chain has not processed any successful sortitions yet")]
NoSortitionOnChain,
/// A successful sortition's info response should be parseable into a SortitionState
#[error("A successful sortition's info response should be parseable into a SortitionState")]
UnexpectedSortitionInfo,
Expand Down
16 changes: 6 additions & 10 deletions stacks-signer/src/client/stacks_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ use blockstack_lib::net::api::get_tenures_fork_info::{
use blockstack_lib::net::api::getaccount::AccountEntryResponse;
use blockstack_lib::net::api::getpoxinfo::RPCPoxInfoData;
use blockstack_lib::net::api::getsortition::{SortitionInfo, RPC_SORTITION_INFO_PATH};
use blockstack_lib::net::api::getstackers::{GetStackersErrors, GetStackersResponse};
use blockstack_lib::net::api::getstackers::GetStackersResponse;
use blockstack_lib::net::api::postblock::StacksBlockAcceptedData;
use blockstack_lib::net::api::postblock_proposal::NakamotoBlockProposal;
use blockstack_lib::net::api::postblock_v3;
Expand Down Expand Up @@ -78,7 +78,6 @@ pub struct StacksClient {

#[derive(Deserialize)]
struct GetStackersErrorResp {
err_type: String,
err_msg: String,
}

Expand Down Expand Up @@ -483,14 +482,11 @@ impl StacksClient {
warn!("Failed to parse the GetStackers error response: {e}");
backoff::Error::permanent(e.into())
})?;
if error_data.err_type == GetStackersErrors::NOT_AVAILABLE_ERR_TYPE {
Err(backoff::Error::permanent(ClientError::NoSortitionOnChain))
} else {
warn!("Got error response ({status}): {}", error_data.err_msg);
Err(backoff::Error::permanent(ClientError::RequestFailure(
status,
)))
}

warn!("Got error response ({status}): {}", error_data.err_msg);
Err(backoff::Error::permanent(ClientError::RequestFailure(
status,
)))
};
let stackers_response =
retry_with_exponential_backoff::<_, ClientError, GetStackersResponse>(send_request)?;
Expand Down
11 changes: 10 additions & 1 deletion stacks-signer/src/v0/signer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -530,7 +530,16 @@ impl Signer {
.signer_db
.block_lookup(self.reward_cycle, &signer_signature_hash)
{
Ok(Some(block_info)) => block_info,
Ok(Some(block_info)) => {
if block_info.state == BlockState::GloballyRejected
|| block_info.state == BlockState::GloballyAccepted
{
debug!("{self}: Received block validation for a block that is already marked as {}. Ignoring...", block_info.state);
return None;
} else {
block_info
}
}
Ok(None) => {
// We have not seen this block before. Why are we getting a response for it?
debug!("{self}: Received a block validate response for a block we have not seen before. Ignoring...");
Expand Down
13 changes: 7 additions & 6 deletions testnet/stacks-node/src/tests/epoch_25.rs
Original file line number Diff line number Diff line change
Expand Up @@ -211,11 +211,12 @@ fn microblocks_disabled() {
);
assert_eq!(account.nonce, 1);

info!(
"Microblocks assembled: {}",
test_observer::get_microblocks().len()
let microblocks_assembled = test_observer::get_microblocks().len();
jferrant marked this conversation as resolved.
Show resolved Hide resolved
info!("Microblocks assembled: {microblocks_assembled}",);
assert!(
microblocks_assembled > 0,
"There should be at least 1 microblock assembled"
);
assert_eq!(test_observer::get_microblocks().len(), 1);

let miner_nonce_before_microblock_assembly = get_account(&http_origin, &miner_account).nonce;

Expand Down Expand Up @@ -244,8 +245,8 @@ fn microblocks_disabled() {
);
assert_eq!(account.nonce, 1);

// but we should have assembled and announced at least 1 to the observer
assert!(test_observer::get_microblocks().len() >= 2);
// but we should have assembled and announced at least 1 more block to the observer
assert!(test_observer::get_microblocks().len() > microblocks_assembled);
info!(
"Microblocks assembled: {}",
test_observer::get_microblocks().len()
Expand Down
155 changes: 1 addition & 154 deletions testnet/stacks-node/src/tests/nakamoto_integrations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1155,69 +1155,6 @@ pub fn is_key_set_for_cycle(
Ok(key.is_some())
}

fn signer_vote_if_needed(
btc_regtest_controller: &BitcoinRegtestController,
naka_conf: &Config,
signer_sks: &[StacksPrivateKey], // TODO: Is there some way to get this from the TestSigners?
signers: &TestSigners,
) {
// When we reach the next prepare phase, submit new voting transactions
let block_height = btc_regtest_controller.get_headers_height();
let reward_cycle = btc_regtest_controller
.get_burnchain()
.block_height_to_reward_cycle(block_height)
.unwrap();
let prepare_phase_start = btc_regtest_controller
.get_burnchain()
.pox_constants
.prepare_phase_start(
btc_regtest_controller.get_burnchain().first_block_height,
reward_cycle,
);

if block_height >= prepare_phase_start {
// If the key is already set, do nothing.
if is_key_set_for_cycle(
reward_cycle + 1,
naka_conf.is_mainnet(),
&naka_conf.node.rpc_bind,
)
.unwrap_or(false)
{
return;
}

// If we are self-signing, then we need to vote on the aggregate public key
let http_origin = format!("http://{}", &naka_conf.node.rpc_bind);

// Get the aggregate key
let aggregate_key = signers.clone().generate_aggregate_key(reward_cycle + 1);
let aggregate_public_key = clarity::vm::Value::buff_from(aggregate_key)
.expect("Failed to serialize aggregate public key");

for (i, signer_sk) in signer_sks.iter().enumerate() {
let signer_nonce = get_account(&http_origin, &to_addr(signer_sk)).nonce;

// Vote on the aggregate public key
let voting_tx = tests::make_contract_call(
&signer_sk,
signer_nonce,
300,
&StacksAddress::burn_address(false),
SIGNERS_VOTING_NAME,
"vote-for-aggregate-public-key",
&[
clarity::vm::Value::UInt(i as u128),
aggregate_public_key.clone(),
clarity::vm::Value::UInt(0),
clarity::vm::Value::UInt(reward_cycle as u128 + 1),
],
);
submit_tx(&http_origin, &voting_tx);
}
}
}

pub fn setup_epoch_3_reward_set(
naka_conf: &Config,
blocks_processed: &Arc<AtomicU64>,
Expand Down Expand Up @@ -1553,13 +1490,6 @@ fn simple_neon_integration() {
&commits_submitted,
)
.unwrap();

signer_vote_if_needed(
&btc_regtest_controller,
&naka_conf,
&[sender_signer_sk],
&signers,
);
}

// Submit a TX
Expand Down Expand Up @@ -1595,13 +1525,6 @@ fn simple_neon_integration() {
&commits_submitted,
)
.unwrap();

signer_vote_if_needed(
&btc_regtest_controller,
&naka_conf,
&[sender_signer_sk],
&signers,
);
}

// load the chain tip, and assert that it is a nakamoto block and at least 30 blocks have advanced in epoch 3
Expand Down Expand Up @@ -1805,13 +1728,6 @@ fn simple_neon_integration_with_flash_blocks_on_epoch_3() {
&commits_submitted,
)
.unwrap();

signer_vote_if_needed(
&btc_regtest_controller,
&naka_conf,
&[sender_signer_sk],
&signers,
);
}

// Submit a TX
Expand Down Expand Up @@ -1847,13 +1763,6 @@ fn simple_neon_integration_with_flash_blocks_on_epoch_3() {
&commits_submitted,
)
.unwrap();

signer_vote_if_needed(
&btc_regtest_controller,
&naka_conf,
&[sender_signer_sk],
&signers,
);
}

// load the chain tip, and assert that it is a nakamoto block and at least 30 blocks have advanced in epoch 3
Expand Down Expand Up @@ -2144,6 +2053,7 @@ fn multiple_miners() {
let node_2_p2p = 51025;
let http_origin = format!("http://{}", &naka_conf.node.rpc_bind);
naka_conf.miner.wait_on_interim_blocks = Duration::from_secs(1);
naka_conf.node.pox_sync_sample_secs = 5;
let sender_sk = Secp256k1PrivateKey::new();
let sender_signer_sk = Secp256k1PrivateKey::new();
let sender_signer_addr = tests::to_addr(&sender_signer_sk);
Expand Down Expand Up @@ -2579,13 +2489,6 @@ fn correct_burn_outs() {
&naka_conf,
);

signer_vote_if_needed(
&btc_regtest_controller,
&naka_conf,
&[sender_signer_sk],
&signers,
);

run_until_burnchain_height(
&mut btc_regtest_controller,
&blocks_processed,
Expand Down Expand Up @@ -2645,13 +2548,6 @@ fn correct_burn_outs() {
tip_sn.block_height > prior_tip,
"The new burnchain tip must have been processed"
);

signer_vote_if_needed(
&btc_regtest_controller,
&naka_conf,
&[sender_signer_sk],
&signers,
);
}

coord_channel
Expand Down Expand Up @@ -4751,13 +4647,6 @@ fn forked_tenure_is_ignored() {
})
.unwrap();

signer_vote_if_needed(
&btc_regtest_controller,
&naka_conf,
&[sender_signer_sk],
&signers,
);

info!("Commit op is submitted; unpause Tenure B's block");

// Unpause the broadcast of Tenure B's block, do not submit commits, and do not allow blocks to
Expand Down Expand Up @@ -6198,13 +6087,6 @@ fn signer_chainstate() {
make_stacks_transfer(&sender_sk, sender_nonce, send_fee, &recipient, send_amt);
submit_tx(&http_origin, &transfer_tx);

signer_vote_if_needed(
&btc_regtest_controller,
&naka_conf,
&[sender_signer_sk],
&signers,
);

let timer = Instant::now();
while proposals_submitted.load(Ordering::SeqCst) <= before {
thread::sleep(Duration::from_millis(5));
Expand Down Expand Up @@ -6681,13 +6563,6 @@ fn continue_tenure_extend() {
)
.unwrap();

signer_vote_if_needed(
&btc_regtest_controller,
&naka_conf,
&[sender_signer_sk],
&signers,
);

wait_for(5, || {
let blocks_processed = coord_channel
.lock()
Expand All @@ -6707,13 +6582,6 @@ fn continue_tenure_extend() {

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

signer_vote_if_needed(
&btc_regtest_controller,
&naka_conf,
&[sender_signer_sk],
&signers,
);

wait_for(5, || {
let blocks_processed = coord_channel
.lock()
Expand Down Expand Up @@ -6755,13 +6623,6 @@ fn continue_tenure_extend() {
next_block_and_process_new_stacks_block(&mut btc_regtest_controller, 60, &coord_channel)
.unwrap();

signer_vote_if_needed(
&btc_regtest_controller,
&naka_conf,
&[sender_signer_sk],
&signers,
);

wait_for(5, || {
let blocks_processed = coord_channel
.lock()
Expand All @@ -6778,13 +6639,6 @@ fn continue_tenure_extend() {

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

signer_vote_if_needed(
&btc_regtest_controller,
&naka_conf,
&[sender_signer_sk],
&signers,
);

wait_for(5, || {
let blocks_processed = coord_channel
.lock()
Expand All @@ -6810,13 +6664,6 @@ fn continue_tenure_extend() {
})
.unwrap();

signer_vote_if_needed(
&btc_regtest_controller,
&naka_conf,
&[sender_signer_sk],
&signers,
);

wait_for(5, || {
let blocks_processed = coord_channel
.lock()
Expand Down
Loading