diff --git a/Cargo.lock b/Cargo.lock index b48405d4330b..a77545269a95 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3067,9 +3067,9 @@ dependencies = [ [[package]] name = "jsonrpsee-types" -version = "0.3.0" +version = "0.3.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d67724d368c59e08b557a516cf8fcc51100e7a708850f502e1044b151fe89788" +checksum = "4cc738fd55b676ada3271ef7c383a14a0867a2a88b0fa941311bf5fc0a29d498" dependencies = [ "async-trait", "beef", @@ -3085,9 +3085,9 @@ dependencies = [ [[package]] name = "jsonrpsee-ws-client" -version = "0.3.0" +version = "0.3.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8e2834b6e7f57ce9a4412ed4d6dc95125d2c8612e68f86b9d9a07369164e4198" +checksum = "9841352dbecf4c2ed5dc71698df9f1660262ae4e0b610e968602529bdbcf7b30" dependencies = [ "async-trait", "fnv", diff --git a/node/core/approval-voting/src/approval_checking.rs b/node/core/approval-voting/src/approval_checking.rs index 644d820f38e5..d681e67eb853 100644 --- a/node/core/approval-voting/src/approval_checking.rs +++ b/node/core/approval-voting/src/approval_checking.rs @@ -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`. @@ -323,13 +328,16 @@ fn count_no_shows( assignments: &[(ValidatorIndex, Tick)], approvals: &BitSlice, clock_drift: Tick, + block_tick: Tick, no_show_duration: Tick, drifted_tick_now: Tick, ) -> (usize, Option) { 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 @@ -418,6 +426,7 @@ pub fn tranches_to_approve( assignments, approvals, clock_drift, + block_tick, no_show_duration, drifted_tick_now, ); @@ -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; @@ -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) }, ); } @@ -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 { @@ -1137,6 +1147,7 @@ mod tests { &test.assignments, &approvals, test.clock_drift, + block_tick, test.no_show_duration, test.drifted_tick_now, ); @@ -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), }) } @@ -1199,26 +1210,26 @@ 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), }) } @@ -1226,16 +1237,16 @@ mod tests { 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), }) } @@ -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), }) } @@ -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), }) } diff --git a/node/core/approval-voting/src/tests.rs b/node/core/approval-voting/src/tests.rs index 2582db301a60..af821039324f 100644 --- a/node/core/approval-voting/src/tests.rs +++ b/node/core/approval-voting/src/tests.rs @@ -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 }); @@ -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, @@ -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, }); @@ -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, }); @@ -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, }); @@ -2516,7 +2516,7 @@ 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, }); @@ -2524,16 +2524,15 @@ fn subsystem_assignment_not_triggered_more_than_maximum() { #[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, }); @@ -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, @@ -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; diff --git a/utils/staking-miner/Cargo.toml b/utils/staking-miner/Cargo.toml index 296240a705d3..593d1a1bf2cd 100644 --- a/utils/staking-miner/Cargo.toml +++ b/utils/staking-miner/Cargo.toml @@ -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"