Skip to content

Commit

Permalink
Merge branch 'master' into ladi-logging-disputes
Browse files Browse the repository at this point in the history 
* master:
  preserve finalized block in active leaves (paritytech#3997)
  some tweaks to rococo-local (paritytech#3996)
  always broadcast tranche 0 assignments and add a delay before approval (paritytech#3904)
  Tidy up XCM errors in preparation for v2. (paritytech#3988)
  add disputes call to Rococo (paritytech#3993)
  Fix an off-by-one: revert rather than revert-to (paritytech#3991)
  add logs to relay chain selection (paritytech#3990)
  av-store: clean up StoreAvailableData message (paritytech#3984)
  add polkadot-simnet runner (paritytech#3985)
  Remove incorrect proof about Jemalloc (paritytech#3982)
  add new rococo chainspec (paritytech#3976)
  bump async-std's version to remove conflict for substrate (paritytech#3981)
  gossip-support: set last_session_index earlier (paritytech#3978)
  • Loading branch information
@ordian
ordian committed Oct 2, 2021
2 parents 06dd295 + d070d55 commit dade923
Show file tree
Hide file tree
Showing 29 changed files with 861 additions and 426 deletions.
2 changes: 1 addition & 1 deletion .gitlab-ci.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -575,4 +575,4 @@ simnet-tests:
allow_failure: true
retry: 2
tags:
- parity-simnet
- polkadot-simnet
201 changes: 138 additions & 63 deletions node/core/approval-voting/src/approval_checking.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ pub enum RequiredTranches {
/// event that there are some assignments that don't have corresponding approval votes. If this
/// is `None`, all assignments have approvals.
next_no_show: Option<Tick>,
/// The last tick at which a needed assignment was received.
last_assignment_tick: Option<Tick>,
},
}

Expand All @@ -66,26 +68,30 @@ pub enum RequiredTranches {
pub enum Check {
/// The candidate is unapproved.
Unapproved,
/// The candidate is approved, with the given amount of no-shows.
Approved(usize),
/// The candidate is approved, with the given amount of no-shows,
/// with the last counted assignment being received at the given
/// tick.
Approved(usize, Option<Tick>),
/// The candidate is approved by one third of all validators.
ApprovedOneThird,
}

impl Check {
/// Whether the candidate is approved.
pub fn is_approved(&self) -> bool {
/// Whether the candidate is approved and all relevant assignments
/// have at most the given assignment tick.
pub fn is_approved(&self, max_assignment_tick: Tick) -> bool {
match *self {
Check::Unapproved => false,
Check::Approved(_) => true,
Check::Approved(_, last_assignment_tick) =>
last_assignment_tick.map_or(true, |t| t <= max_assignment_tick),
Check::ApprovedOneThird => true,
}
}

/// The number of known no-shows in this computation.
pub fn known_no_shows(&self) -> usize {
match *self {
Check::Approved(n) => n,
Check::Approved(n, _) => n,
_ => 0,
}
}
Expand All @@ -107,7 +113,7 @@ pub fn check_approval(
match required {
RequiredTranches::Pending { .. } => Check::Unapproved,
RequiredTranches::All => Check::Unapproved,
RequiredTranches::Exact { needed, tolerated_missing, .. } => {
RequiredTranches::Exact { needed, tolerated_missing, last_assignment_tick, .. } => {
// whether all assigned validators up to `needed` less no_shows have approved.
// e.g. if we had 5 tranches and 1 no-show, we would accept all validators in
// tranches 0..=5 except for 1 approving. In that example, we also accept all
Expand All @@ -130,7 +136,7 @@ pub fn check_approval(
// that will surpass a minimum amount of checks.
// shouldn't typically go above, since all no-shows are supposed to be covered.
if n_approved + tolerated_missing >= n_assigned {
Check::Approved(tolerated_missing)
Check::Approved(tolerated_missing, last_assignment_tick)
} else {
Check::Unapproved
}
Expand Down Expand Up @@ -170,6 +176,8 @@ struct State {
uncovered: usize,
/// The next tick at which a no-show would occur, if any.
next_no_show: Option<Tick>,
/// The last tick at which a considered assignment was received.
last_assignment_tick: Option<Tick>,
}

impl State {
Expand All @@ -192,6 +200,7 @@ impl State {
needed: tranche,
tolerated_missing: self.covered,
next_no_show: self.next_no_show,
last_assignment_tick: self.last_assignment_tick,
}
}

Expand Down Expand Up @@ -226,6 +235,7 @@ impl State {
new_assignments: usize,
new_no_shows: usize,
next_no_show: Option<Tick>,
last_assignment_tick: Option<Tick>,
) -> State {
let new_covered = if self.depth == 0 {
new_assignments
Expand All @@ -246,6 +256,7 @@ impl State {
};
let uncovered = self.uncovered + new_no_shows;
let next_no_show = super::min_prefer_some(self.next_no_show, next_no_show);
let last_assignment_tick = std::cmp::max(self.last_assignment_tick, last_assignment_tick);

let (depth, covering, uncovered) = if covering == 0 {
if uncovered == 0 {
Expand All @@ -257,7 +268,15 @@ impl State {
(self.depth, covering, uncovered)
};

State { assignments, depth, covered, covering, uncovered, next_no_show }
State {
assignments,
depth,
covered,
covering,
uncovered,
next_no_show,
last_assignment_tick,
}
}
}

Expand Down Expand Up @@ -356,6 +375,7 @@ pub fn tranches_to_approve(
covering: needed_approvals,
uncovered: 0,
next_no_show: None,
last_assignment_tick: None,
};

// The `ApprovalEntry` doesn't have any data for empty tranches. We still want to iterate over
Expand Down Expand Up @@ -384,6 +404,11 @@ pub fn tranches_to_approve(
.filter(|(v_index, _)| v_index.0 < n_validators as u32)
.count();

// Get the latest tick of valid validator assignments.
let last_assignment_tick = assignments.iter()
.map(|(_, t)| *t)
.max();

// count no-shows. An assignment is a no-show if there is no corresponding approval vote
// after a fixed duration.
//
Expand All @@ -397,7 +422,7 @@ pub fn tranches_to_approve(
drifted_tick_now,
);

let s = s.advance(n_assignments, no_shows, next_no_show);
let s = s.advance(n_assignments, no_shows, next_no_show, last_assignment_tick);
let output = s.output(tranche, needed_approvals, n_validators, no_show_duration);

*state = match output {
Expand Down Expand Up @@ -459,7 +484,7 @@ mod tests {
clock_drift: 0,
},
)
.is_approved());
.is_approved(Tick::max_value()));
}

#[test]
Expand Down Expand Up @@ -502,21 +527,36 @@ mod tests {
assert!(check_approval(
&candidate,
&approval_entry,
RequiredTranches::Exact { needed: 0, tolerated_missing: 0, next_no_show: None },
RequiredTranches::Exact {
needed: 0,
tolerated_missing: 0,
next_no_show: None,
last_assignment_tick: None
},
)
.is_approved());
.is_approved(Tick::max_value()));
assert!(!check_approval(
&candidate,
&approval_entry,
RequiredTranches::Exact { needed: 1, tolerated_missing: 0, next_no_show: None },
RequiredTranches::Exact {
needed: 1,
tolerated_missing: 0,
next_no_show: None,
last_assignment_tick: None
},
)
.is_approved());
.is_approved(Tick::max_value()));
assert!(check_approval(
&candidate,
&approval_entry,
RequiredTranches::Exact { needed: 1, tolerated_missing: 2, next_no_show: None },
RequiredTranches::Exact {
needed: 1,
tolerated_missing: 2,
next_no_show: None,
last_assignment_tick: None
},
)
.is_approved());
.is_approved(Tick::max_value()));
}

#[test]
Expand Down Expand Up @@ -556,8 +596,12 @@ mod tests {
}
.into();

let exact_all =
RequiredTranches::Exact { needed: 10, tolerated_missing: 0, next_no_show: None };
let exact_all = RequiredTranches::Exact {
needed: 10,
tolerated_missing: 0,
next_no_show: None,
last_assignment_tick: None,
};

let pending_all = RequiredTranches::Pending {
considered: 5,
Expand All @@ -566,20 +610,27 @@ mod tests {
clock_drift: 12,
};

assert!(!check_approval(&candidate, &approval_entry, RequiredTranches::All,).is_approved());
assert!(!check_approval(&candidate, &approval_entry, RequiredTranches::All,)
.is_approved(Tick::max_value()));

assert!(!check_approval(&candidate, &approval_entry, exact_all.clone(),).is_approved());
assert!(!check_approval(&candidate, &approval_entry, exact_all.clone(),)
.is_approved(Tick::max_value()));

assert!(!check_approval(&candidate, &approval_entry, pending_all.clone(),).is_approved());
assert!(!check_approval(&candidate, &approval_entry, pending_all.clone(),)
.is_approved(Tick::max_value()));

// This creates a set of 4/10 approvals, which is always an approval.
candidate.mark_approval(ValidatorIndex(3));

assert!(check_approval(&candidate, &approval_entry, RequiredTranches::All,).is_approved());
assert!(check_approval(&candidate, &approval_entry, RequiredTranches::All,)
.is_approved(Tick::max_value()));

assert!(check_approval(&candidate, &approval_entry, exact_all,).is_approved());
assert!(
check_approval(&candidate, &approval_entry, exact_all,).is_approved(Tick::max_value())
);

assert!(check_approval(&candidate, &approval_entry, pending_all,).is_approved());
assert!(check_approval(&candidate, &approval_entry, pending_all,)
.is_approved(Tick::max_value()));
}

#[test]
Expand Down Expand Up @@ -617,7 +668,12 @@ mod tests {
no_show_duration,
needed_approvals,
),
RequiredTranches::Exact { needed: 1, tolerated_missing: 0, next_no_show: None },
RequiredTranches::Exact {
needed: 1,
tolerated_missing: 0,
next_no_show: None,
last_assignment_tick: Some(1)
},
);
}

Expand Down Expand Up @@ -820,6 +876,7 @@ mod tests {
needed: 1,
tolerated_missing: 0,
next_no_show: Some(block_tick + no_show_duration + 1),
last_assignment_tick: Some(block_tick + 1),
},
);

Expand All @@ -838,6 +895,7 @@ mod tests {
needed: 2,
tolerated_missing: 1,
next_no_show: Some(block_tick + 2 * no_show_duration + 2),
last_assignment_tick: Some(block_tick + no_show_duration + 2),
},
);

Expand Down Expand Up @@ -905,7 +963,12 @@ mod tests {
no_show_duration,
needed_approvals,
),
RequiredTranches::Exact { needed: 2, tolerated_missing: 1, next_no_show: None },
RequiredTranches::Exact {
needed: 2,
tolerated_missing: 1,
next_no_show: None,
last_assignment_tick: Some(block_tick + no_show_duration + 2)
},
);

// Even though tranche 2 has 2 validators, it only covers 1 no-show.
Expand Down Expand Up @@ -943,7 +1006,12 @@ mod tests {
no_show_duration,
needed_approvals,
),
RequiredTranches::Exact { needed: 3, tolerated_missing: 2, next_no_show: None },
RequiredTranches::Exact {
needed: 3,
tolerated_missing: 2,
next_no_show: None,
last_assignment_tick: Some(block_tick + no_show_duration + 2),
},
);
}

Expand Down Expand Up @@ -1256,43 +1324,50 @@ mod tests {
exp_next_no_show: None,
})
}
}

#[test]
fn depth_0_covering_not_treated_as_such() {
let state = State {
assignments: 0,
depth: 0,
covered: 0,
covering: 10,
uncovered: 0,
next_no_show: None,
};

assert_eq!(
state.output(0, 10, 10, 20),
RequiredTranches::Pending {
considered: 0,
#[test]
fn depth_0_covering_not_treated_as_such() {
let state = State {
assignments: 0,
depth: 0,
covered: 0,
covering: 10,
uncovered: 0,
next_no_show: None,
maximum_broadcast: DelayTranche::max_value(),
clock_drift: 0,
},
);
}
last_assignment_tick: None,
};

#[test]
fn depth_0_issued_as_exact_even_when_all() {
let state = State {
assignments: 10,
depth: 0,
covered: 0,
covering: 0,
uncovered: 0,
next_no_show: None,
};
assert_eq!(
state.output(0, 10, 10, 20),
RequiredTranches::Pending {
considered: 0,
next_no_show: None,
maximum_broadcast: DelayTranche::max_value(),
clock_drift: 0,
},
);
}

assert_eq!(
state.output(0, 10, 10, 20),
RequiredTranches::Exact { needed: 0, tolerated_missing: 0, next_no_show: None },
);
#[test]
fn depth_0_issued_as_exact_even_when_all() {
let state = State {
assignments: 10,
depth: 0,
covered: 0,
covering: 0,
uncovered: 0,
next_no_show: None,
last_assignment_tick: None,
};

assert_eq!(
state.output(0, 10, 10, 20),
RequiredTranches::Exact {
needed: 0,
tolerated_missing: 0,
next_no_show: None,
last_assignment_tick: None
},
);
}
}
Loading

0 comments on commit dade923

Please sign in to comment.