Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Commit

Permalink
Merge branch 'master' into bernhard-malus-fx
Browse files Browse the repository at this point in the history 
* master:
  Bump jsonrpsee-ws-client from 0.3.0 to 0.3.1 (#3931)
  fix clock drift for assignments issued before the block (#3851)
  • Loading branch information
@ordian
ordian committed Oct 3, 2021
2 parents a1b345c + 0bfcb7a commit d5cd103
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 33 deletions.
8 changes: 4 additions & 4 deletions Cargo.lock
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

41 changes: 26 additions & 15 deletions node/core/approval-voting/src/approval_checking.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,11 @@ fn filled_tranche_iterator<'a>(
/// and tick parameters. This method also returns the next tick at which a `no_show` will occur
/// amongst the set of validators that have not submitted an approval.
///
/// This also bounds the earliest tick of all assignments to be equal to the
/// block tick for the purposes of the calculation, so no assignment can be treated
/// as being received before the block itself. This is unlikely if not impossible
/// in practice, but can occur during test code.
///
/// If the returned `next_no_show` is not None, there are two possible cases for the value of
/// based on the earliest assignment `tick` of a non-approving, yet-to-be-no-show validator:
/// - if `tick` <= `clock_drift`: the value will always be `clock_drift` + `no_show_duration`.
Expand All @@ -323,13 +328,16 @@ fn count_no_shows(
assignments: &[(ValidatorIndex, Tick)],
approvals: &BitSlice<BitOrderLsb0, u8>,
clock_drift: Tick,
block_tick: Tick,
no_show_duration: Tick,
drifted_tick_now: Tick,
) -> (usize, Option<u64>) {
let mut next_no_show = None;
let no_shows = assignments
.iter()
.map(|(v_index, tick)| (v_index, tick.saturating_sub(clock_drift) + no_show_duration))
.map(|(v_index, tick)| {
(v_index, tick.max(&block_tick).saturating_sub(clock_drift) + no_show_duration)
})
.filter(|&(v_index, no_show_at)| {
let has_approved = if let Some(approved) = approvals.get(v_index.0 as usize) {
*approved
Expand Down Expand Up @@ -418,6 +426,7 @@ pub fn tranches_to_approve(
assignments,
approvals,
clock_drift,
block_tick,
no_show_duration,
drifted_tick_now,
);
Expand Down Expand Up @@ -635,7 +644,7 @@ mod tests {

#[test]
fn tranches_to_approve_everyone_present() {
let block_tick = 0;
let block_tick = 20;
let no_show_duration = 10;
let needed_approvals = 4;

Expand Down Expand Up @@ -672,7 +681,7 @@ mod tests {
needed: 1,
tolerated_missing: 0,
next_no_show: None,
last_assignment_tick: Some(1)
last_assignment_tick: Some(21)
},
);
}
Expand Down Expand Up @@ -1127,6 +1136,7 @@ mod tests {

fn test_count_no_shows(test: NoShowTest) {
let n_validators = 4;
let block_tick = 20;

let mut approvals = bitvec![BitOrderLsb0, u8; 0; n_validators];
for &v_index in &test.approvals {
Expand All @@ -1137,6 +1147,7 @@ mod tests {
&test.assignments,
&approvals,
test.clock_drift,
block_tick,
test.no_show_duration,
test.drifted_tick_now,
);
Expand All @@ -1160,13 +1171,13 @@ mod tests {
#[test]
fn count_no_shows_single_validator_is_next_no_show() {
test_count_no_shows(NoShowTest {
assignments: vec![(ValidatorIndex(1), 21)],
assignments: vec![(ValidatorIndex(1), 31)],
approvals: vec![],
clock_drift: 10,
no_show_duration: 10,
drifted_tick_now: 20,
exp_no_shows: 0,
exp_next_no_show: Some(31),
exp_next_no_show: Some(41),
})
}

Expand Down Expand Up @@ -1199,43 +1210,43 @@ mod tests {
#[test]
fn count_no_shows_two_validators_next_no_show_ordered_first() {
test_count_no_shows(NoShowTest {
assignments: vec![(ValidatorIndex(1), 21), (ValidatorIndex(2), 22)],
assignments: vec![(ValidatorIndex(1), 31), (ValidatorIndex(2), 32)],
approvals: vec![],
clock_drift: 10,
no_show_duration: 10,
drifted_tick_now: 20,
exp_no_shows: 0,
exp_next_no_show: Some(31),
exp_next_no_show: Some(41),
})
}

#[test]
fn count_no_shows_two_validators_next_no_show_ordered_last() {
test_count_no_shows(NoShowTest {
assignments: vec![(ValidatorIndex(1), 22), (ValidatorIndex(2), 21)],
assignments: vec![(ValidatorIndex(1), 32), (ValidatorIndex(2), 31)],
approvals: vec![],
clock_drift: 10,
no_show_duration: 10,
drifted_tick_now: 20,
exp_no_shows: 0,
exp_next_no_show: Some(31),
exp_next_no_show: Some(41),
})
}

#[test]
fn count_no_shows_three_validators_one_almost_late_one_no_show_one_approving() {
test_count_no_shows(NoShowTest {
assignments: vec![
(ValidatorIndex(1), 21),
(ValidatorIndex(2), 20),
(ValidatorIndex(3), 20),
(ValidatorIndex(1), 31),
(ValidatorIndex(2), 19),
(ValidatorIndex(3), 19),
],
approvals: vec![3],
clock_drift: 10,
no_show_duration: 10,
drifted_tick_now: 20,
exp_no_shows: 1,
exp_next_no_show: Some(31),
exp_next_no_show: Some(41),
})
}

Expand Down Expand Up @@ -1282,7 +1293,7 @@ mod tests {
no_show_duration: 20,
drifted_tick_now: 0,
exp_no_shows: 0,
exp_next_no_show: Some(30),
exp_next_no_show: Some(40),
})
}

Expand All @@ -1295,7 +1306,7 @@ mod tests {
no_show_duration: 20,
drifted_tick_now: 0,
exp_no_shows: 0,
exp_next_no_show: Some(30),
exp_next_no_show: Some(40),
})
}

Expand Down
25 changes: 12 additions & 13 deletions node/core/approval-voting/src/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1609,7 +1609,7 @@ fn subsystem_process_wakeup_schedules_wakeup() {
futures_timer::Delay::new(Duration::from_millis(100)).await;

// The wakeup should have been rescheduled.
assert!(clock.inner.lock().current_wakeup_is(20));
assert!(clock.inner.lock().current_wakeup_is(30));

virtual_overseer
});
Expand Down Expand Up @@ -2234,8 +2234,8 @@ fn subsystem_process_wakeup_trigger_assignment_launch_approval() {

futures_timer::Delay::new(Duration::from_millis(200)).await;

assert!(clock.inner.lock().current_wakeup_is(slot_to_tick(slot + 1)));
clock.inner.lock().wakeup_all(slot_to_tick(slot + 1));
assert!(clock.inner.lock().current_wakeup_is(slot_to_tick(slot + 2)));
clock.inner.lock().wakeup_all(slot_to_tick(slot + 2));

assert_matches!(
overseer_recv(&mut virtual_overseer).await,
Expand Down Expand Up @@ -2467,7 +2467,7 @@ fn subsystem_assignment_triggered_by_all_with_less_than_threshold() {
approvals_to_import: vec![2, 4],
ticks: vec![
2, // APPROVAL_DELAY
20, // Check for no shows
21, // Check for no shows
],
should_be_triggered: |t| t == 20,
});
Expand All @@ -2483,7 +2483,7 @@ fn subsystem_assignment_not_triggered_by_all_with_threshold() {
approvals_to_import: vec![1, 3, 5],
ticks: vec![
2, // APPROVAL_DELAY
20, // Check no shows
21, // Check no shows
],
should_be_triggered: |_| false,
});
Expand All @@ -2498,8 +2498,8 @@ fn subsystem_assignment_triggered_if_below_maximum_and_clock_is_equal() {
assignments_to_import: vec![1],
approvals_to_import: vec![],
ticks: vec![
20, // Check no shows
21, // Alice wakeup, assignment triggered
21, // Check no shows
23, // Alice wakeup, assignment triggered
],
should_be_triggered: |tick| tick >= 21,
});
Expand All @@ -2516,24 +2516,23 @@ fn subsystem_assignment_not_triggered_more_than_maximum() {
ticks: vec![
2, // APPROVAL_DELAY
13, // Alice wakeup
20, // Check no shows
30, // Check no shows
],
should_be_triggered: |_| false,
});
}

#[test]
fn subsystem_assignment_triggered_if_at_maximum() {
// TODO(ladi): is this possible?
triggers_assignment_test(TriggersAssignmentConfig {
our_assigned_tranche: 11,
our_assigned_tranche: 21,
assign_validator_tranche: |_| Ok(2),
no_show_slots: 2,
assignments_to_import: vec![1],
approvals_to_import: vec![],
ticks: vec![
12, // Bob wakeup
20, // Check no shows
30, // Check no shows
],
should_be_triggered: |_| false,
});
Expand Down Expand Up @@ -2583,7 +2582,7 @@ fn subsystem_assignment_not_triggered_if_at_maximum_but_clock_is_before_with_dri
12, // Charlie wakeup
13, // Dave wakeup
15, // Alice wakeup, noop
20, // Check no shows
30, // Check no shows
34, // Eve wakeup
],
should_be_triggered: |_| false,
Expand Down Expand Up @@ -2755,7 +2754,7 @@ fn pre_covers_dont_stall_approval() {

// Wait for the no-show timer to observe the approval from
// tranche 0 and set a wakeup for tranche 1.
clock.inner.lock().set_tick(20);
clock.inner.lock().set_tick(30);

// Sleep to ensure we get a consistent read on the database.
futures_timer::Delay::new(Duration::from_millis(100)).await;
Expand Down
2 changes: 1 addition & 1 deletion utils/staking-miner/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ tokio = { version = "1.12", features = ["macros"] }
log = "0.4.11"
env_logger = "0.9.0"
structopt = "0.3.23"
jsonrpsee-ws-client = { version = "0.3.0", default-features = false, features = ["tokio1"] }
jsonrpsee-ws-client = { version = "0.3.1", default-features = false, features = ["tokio1"] }
serde_json = "1.0"
serde = "1.0.130"
paste = "1.0.5"
Expand Down

0 comments on commit d5cd103

Please sign in to comment.