Skip to content

Commit c87c0eb

Browse files
jferrantjferrant
authored
Merge pull request #5266 from stacks-network/fix/5251-refresh-sortition-view
feat: refresh sortition view when proposed block has mismatch
2 parents 75f7325 + 0b785f5 commit c87c0eb

File tree

4 files changed

+134
-19
lines changed

4 files changed

+134
-19
lines changed

‎stacks-signer/src/chainstate.rs

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,7 @@ impl SortitionsView {
187187
block: &NakamotoBlock,
188188
block_pk: &StacksPublicKey,
189189
reward_cycle: u64,
190+
reset_view_if_wrong_consensus_hash: bool,
190191
) -> Result<bool, SignerChainstateError> {
191192
if self
192193
.cur_sortition
@@ -236,6 +237,23 @@ impl SortitionsView {
236237
})
237238
})
238239
else {
240+
if reset_view_if_wrong_consensus_hash {
241+
info!(
242+
"Miner block proposal has consensus hash that is neither the current or last sortition. Resetting view.";
243+
"proposed_block_consensus_hash" => %block.header.consensus_hash,
244+
"current_sortition_consensus_hash" => ?self.cur_sortition.consensus_hash,
245+
"last_sortition_consensus_hash" => ?self.last_sortition.as_ref().map(|x| x.consensus_hash),
246+
);
247+
self.reset_view(client)?;
248+
return self.check_proposal(
249+
client,
250+
signer_db,
251+
block,
252+
block_pk,
253+
reward_cycle,
254+
false,
255+
);
256+
}
239257
warn!(
240258
"Miner block proposal has consensus hash that is neither the current or last sortition. Considering invalid.";
241259
"proposed_block_consensus_hash" => %block.header.consensus_hash,
@@ -624,4 +642,23 @@ impl SortitionsView {
624642
config,
625643
})
626644
}
645+
646+
/// Reset the view to the current sortition and last sortition
647+
pub fn reset_view(&mut self, client: &StacksClient) -> Result<(), ClientError> {
648+
let CurrentAndLastSortition {
649+
current_sortition,
650+
last_sortition,
651+
} = client.get_current_and_last_sortition()?;
652+
653+
let cur_sortition = SortitionState::try_from(current_sortition)?;
654+
let last_sortition = last_sortition
655+
.map(SortitionState::try_from)
656+
.transpose()
657+
.ok()
658+
.flatten();
659+
660+
self.cur_sortition = cur_sortition;
661+
self.last_sortition = last_sortition;
662+
Ok(())
663+
}
627664
}

‎stacks-signer/src/tests/chainstate.rs

Lines changed: 83 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ use blockstack_lib::chainstate::stacks::{
2626
TransactionSpendingCondition, TransactionVersion,
2727
};
2828
use blockstack_lib::net::api::get_tenures_fork_info::TenureForkingInfo;
29+
use blockstack_lib::net::api::getsortition::SortitionInfo;
2930
use clarity::types::chainstate::{BurnchainHeaderHash, SortitionId};
3031
use clarity::util::vrf::VRFProof;
3132
use libsigner::BlockProposal;
@@ -128,13 +129,13 @@ fn check_proposal_units() {
128129
setup_test_environment("check_proposal_units");
129130

130131
assert!(!view
131-
.check_proposal(&stacks_client, &mut signer_db, &block, &block_pk, 1)
132+
.check_proposal(&stacks_client, &mut signer_db, &block, &block_pk, 1, false)
132133
.unwrap());
133134

134135
view.last_sortition = None;
135136

136137
assert!(!view
137-
.check_proposal(&stacks_client, &mut signer_db, &block, &block_pk, 1)
138+
.check_proposal(&stacks_client, &mut signer_db, &block, &block_pk, 1, false)
138139
.unwrap());
139140
}
140141

@@ -150,7 +151,8 @@ fn check_proposal_miner_pkh_mismatch() {
150151
&mut signer_db,
151152
&block,
152153
&different_block_pk,
153-
1
154+
1,
155+
false,
154156
)
155157
.unwrap());
156158

@@ -161,7 +163,8 @@ fn check_proposal_miner_pkh_mismatch() {
161163
&mut signer_db,
162164
&block,
163165
&different_block_pk,
164-
1
166+
1,
167+
false,
165168
)
166169
.unwrap());
167170
}
@@ -257,7 +260,7 @@ fn reorg_timing_testing(
257260
config,
258261
} = MockServerClient::new();
259262
let h = std::thread::spawn(move || {
260-
view.check_proposal(&client, &mut signer_db, &block, &block_pk, 1)
263+
view.check_proposal(&client, &mut signer_db, &block, &block_pk, 1, false)
261264
});
262265
header_clone.chain_length -= 1;
263266
let response = crate::client::tests::build_get_tenure_tip_response(
@@ -294,16 +297,16 @@ fn check_proposal_invalid_status() {
294297
setup_test_environment("invalid_status");
295298
block.header.consensus_hash = view.cur_sortition.consensus_hash;
296299
assert!(view
297-
.check_proposal(&stacks_client, &mut signer_db, &block, &block_pk, 1)
300+
.check_proposal(&stacks_client, &mut signer_db, &block, &block_pk, 1, false)
298301
.unwrap());
299302
view.cur_sortition.miner_status = SortitionMinerStatus::InvalidatedAfterFirstBlock;
300303
assert!(!view
301-
.check_proposal(&stacks_client, &mut signer_db, &block, &block_pk, 1)
304+
.check_proposal(&stacks_client, &mut signer_db, &block, &block_pk, 1, false)
302305
.unwrap());
303306

304307
block.header.consensus_hash = view.last_sortition.as_ref().unwrap().consensus_hash;
305308
assert!(!view
306-
.check_proposal(&stacks_client, &mut signer_db, &block, &block_pk, 1)
309+
.check_proposal(&stacks_client, &mut signer_db, &block, &block_pk, 1, false)
307310
.unwrap());
308311

309312
view.cur_sortition.miner_status = SortitionMinerStatus::InvalidatedBeforeFirstBlock;
@@ -314,7 +317,7 @@ fn check_proposal_invalid_status() {
314317
// parent blocks have been seen before, while the signer state checks are only reasoning about
315318
// stacks blocks seen by the signer, which may be a subset)
316319
assert!(view
317-
.check_proposal(&stacks_client, &mut signer_db, &block, &block_pk, 1)
320+
.check_proposal(&stacks_client, &mut signer_db, &block, &block_pk, 1, false)
318321
.unwrap());
319322
}
320323

@@ -363,7 +366,7 @@ fn check_proposal_tenure_extend_invalid_conditions() {
363366
let tx = make_tenure_change_tx(extend_payload);
364367
block.txs = vec![tx];
365368
assert!(!view
366-
.check_proposal(&stacks_client, &mut signer_db, &block, &block_pk, 1)
369+
.check_proposal(&stacks_client, &mut signer_db, &block, &block_pk, 1, false)
367370
.unwrap());
368371

369372
let mut extend_payload = make_tenure_change_payload();
@@ -373,7 +376,7 @@ fn check_proposal_tenure_extend_invalid_conditions() {
373376
let tx = make_tenure_change_tx(extend_payload);
374377
block.txs = vec![tx];
375378
assert!(view
376-
.check_proposal(&stacks_client, &mut signer_db, &block, &block_pk, 1)
379+
.check_proposal(&stacks_client, &mut signer_db, &block, &block_pk, 1, false)
377380
.unwrap());
378381
}
379382

@@ -400,7 +403,8 @@ fn check_block_proposal_timeout() {
400403
&mut signer_db,
401404
&curr_sortition_block,
402405
&block_pk,
403-
1
406+
1,
407+
false,
404408
)
405409
.unwrap());
406410

@@ -410,7 +414,8 @@ fn check_block_proposal_timeout() {
410414
&mut signer_db,
411415
&last_sortition_block,
412416
&block_pk,
413-
1
417+
1,
418+
false,
414419
)
415420
.unwrap());
416421

@@ -422,7 +427,8 @@ fn check_block_proposal_timeout() {
422427
&mut signer_db,
423428
&curr_sortition_block,
424429
&block_pk,
425-
1
430+
1,
431+
false,
426432
)
427433
.unwrap());
428434

@@ -432,7 +438,8 @@ fn check_block_proposal_timeout() {
432438
&mut signer_db,
433439
&last_sortition_block,
434440
&block_pk,
435-
1
441+
1,
442+
false,
436443
)
437444
.unwrap());
438445
}
@@ -513,3 +520,64 @@ fn check_sortition_timeout() {
513520
.is_timed_out(Duration::from_secs(1), &signer_db)
514521
.unwrap());
515522
}
523+
524+
/// Test that the sortition info is refreshed once
525+
/// when `check_proposal` is called with a sortition view
526+
/// that doesn't match the block proposal
527+
#[test]
528+
fn check_proposal_refresh() {
529+
let (stacks_client, mut signer_db, block_pk, mut view, mut block) =
530+
setup_test_environment("check_proposal_refresh");
531+
block.header.consensus_hash = view.cur_sortition.consensus_hash;
532+
assert!(view
533+
.check_proposal(&stacks_client, &mut signer_db, &block, &block_pk, 1, false)
534+
.unwrap());
535+
536+
let MockServerClient {
537+
server,
538+
client,
539+
config: _,
540+
} = MockServerClient::new();
541+
542+
let last_sortition = view.last_sortition.as_ref().unwrap();
543+
544+
let expected_result = vec![
545+
SortitionInfo {
546+
burn_block_hash: last_sortition.burn_block_hash,
547+
burn_block_height: 2,
548+
sortition_id: SortitionId([2; 32]),
549+
parent_sortition_id: SortitionId([1; 32]),
550+
consensus_hash: block.header.consensus_hash,
551+
was_sortition: true,
552+
burn_header_timestamp: 2,
553+
miner_pk_hash160: Some(view.cur_sortition.miner_pkh),
554+
stacks_parent_ch: Some(view.cur_sortition.parent_tenure_id),
555+
last_sortition_ch: Some(view.cur_sortition.parent_tenure_id),
556+
committed_block_hash: None,
557+
},
558+
SortitionInfo {
559+
burn_block_hash: BurnchainHeaderHash([128; 32]),
560+
burn_block_height: 1,
561+
sortition_id: SortitionId([1; 32]),
562+
parent_sortition_id: SortitionId([0; 32]),
563+
consensus_hash: view.cur_sortition.parent_tenure_id,
564+
was_sortition: true,
565+
burn_header_timestamp: 1,
566+
miner_pk_hash160: Some(view.cur_sortition.miner_pkh),
567+
stacks_parent_ch: Some(view.cur_sortition.parent_tenure_id),
568+
last_sortition_ch: Some(view.cur_sortition.parent_tenure_id),
569+
committed_block_hash: None,
570+
},
571+
];
572+
573+
view.cur_sortition.consensus_hash = ConsensusHash([128; 20]);
574+
let h = std::thread::spawn(move || {
575+
view.check_proposal(&client, &mut signer_db, &block, &block_pk, 1, true)
576+
});
577+
crate::client::tests::write_response(
578+
server,
579+
format!("HTTP/1.1 200 Ok\n\n{}", serde_json::json!(expected_result)).as_bytes(),
580+
);
581+
let result = h.join().unwrap();
582+
assert!(result.unwrap());
583+
}

‎stacks-signer/src/v0/signer.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -379,6 +379,7 @@ impl Signer {
379379
&block_proposal.block,
380380
miner_pubkey,
381381
self.reward_cycle,
382+
true,
382383
) {
383384
// Error validating block
384385
Err(e) => {

‎testnet/stacks-node/src/tests/nakamoto_integrations.rs

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6006,6 +6006,7 @@ fn signer_chainstate() {
60066006
prior_tenure_first,
60076007
miner_pk,
60086008
reward_cycle,
6009+
true,
60096010
)
60106011
.unwrap();
60116012
assert!(
@@ -6020,6 +6021,7 @@ fn signer_chainstate() {
60206021
block,
60216022
miner_pk,
60226023
reward_cycle,
6024+
true,
60236025
)
60246026
.unwrap();
60256027
assert!(
@@ -6056,6 +6058,7 @@ fn signer_chainstate() {
60566058
&proposal.0,
60576059
&proposal.1,
60586060
reward_cycle,
6061+
true,
60596062
)
60606063
.unwrap();
60616064

@@ -6105,6 +6108,7 @@ fn signer_chainstate() {
61056108
&proposal_interim.0,
61066109
&proposal_interim.1,
61076110
reward_cycle,
6111+
true,
61086112
)
61096113
.unwrap();
61106114

@@ -6134,6 +6138,7 @@ fn signer_chainstate() {
61346138
&proposal_interim.0,
61356139
&proposal_interim.1,
61366140
reward_cycle,
6141+
true,
61376142
)
61386143
.unwrap();
61396144

@@ -6209,7 +6214,8 @@ fn signer_chainstate() {
62096214
&mut signer_db,
62106215
&sibling_block,
62116216
&miner_pk,
6212-
reward_cycle
6217+
reward_cycle,
6218+
false,
62136219
)
62146220
.unwrap(),
62156221
"A sibling of a previously approved block must be rejected."
@@ -6266,7 +6272,8 @@ fn signer_chainstate() {
62666272
&mut signer_db,
62676273
&sibling_block,
62686274
&miner_pk,
6269-
reward_cycle
6275+
reward_cycle,
6276+
false,
62706277
)
62716278
.unwrap(),
62726279
"A sibling of a previously approved block must be rejected."
@@ -6329,7 +6336,8 @@ fn signer_chainstate() {
63296336
&mut signer_db,
63306337
&sibling_block,
63316338
&miner_pk,
6332-
reward_cycle
6339+
reward_cycle,
6340+
false,
63336341
)
63346342
.unwrap(),
63356343
"A sibling of a previously approved block must be rejected."
@@ -6394,7 +6402,8 @@ fn signer_chainstate() {
63946402
&mut signer_db,
63956403
&sibling_block,
63966404
&miner_pk,
6397-
reward_cycle
6405+
reward_cycle,
6406+
false,
63986407
)
63996408
.unwrap(),
64006409
"A sibling of a previously approved block must be rejected."

0 commit comments

Comments
 (0)