Skip to content

Commit

Permalink
dispute-coordinator: fix underflow panic (paritytech#4557)
Browse files Browse the repository at this point in the history
  • Loading branch information
slumber authored and Wizdave97 committed Feb 3, 2022
1 parent 683722a commit a26d597
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 11 deletions.
22 changes: 18 additions & 4 deletions node/core/dispute-coordinator/src/real/ordering/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -256,12 +256,12 @@ impl OrderingProvider {
) -> Result<Vec<Hash>> {
let mut ancestors = Vec::new();

if self.last_observed_blocks.get(&head).is_some() {
let finalized_block_number = get_finalized_block_number(sender).await?;

if self.last_observed_blocks.get(&head).is_some() || head_number <= finalized_block_number {
return Ok(ancestors)
}

let finalized_block_number = get_finalized_block_number(sender).await?;

loop {
let (tx, rx) = oneshot::channel();
let hashes = {
Expand All @@ -279,7 +279,20 @@ impl OrderingProvider {
rx.await.or(Err(Fatal::ChainApiSenderDropped))?.map_err(Fatal::ChainApi)?
};

let earliest_block_number = head_number - hashes.len() as u32;
let earliest_block_number = match head_number.checked_sub(hashes.len() as u32) {
Some(number) => number,
None => {
// It's assumed that it's impossible to retrieve
// more than N ancestors for block number N.
tracing::error!(
target: LOG_TARGET,
"Received {} ancestors for block number {} from Chain API",
hashes.len(),
head_number,
);
return Ok(ancestors)
},
};
// The reversed order is parent, grandparent, etc. excluding the head.
let block_numbers = (earliest_block_number..head_number).rev();

Expand All @@ -295,6 +308,7 @@ impl OrderingProvider {

ancestors.push(*hash);
}

match hashes.last() {
Some(last_hash) => {
head = *last_hash;
Expand Down
12 changes: 5 additions & 7 deletions node/core/dispute-coordinator/src/real/ordering/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,13 +67,11 @@ impl TestState {
let chain = vec![get_block_number_hash(0)];

let finalized_block_number = 0;
let expected_ancestry_len = 1;
let overseer_fut = overseer_process_active_leaves_update(
&mut ctx_handle,
&chain,
finalized_block_number,
expected_ancestry_len,
);
let overseer_fut = async {
assert_finalized_block_number_request(&mut ctx_handle, finalized_block_number).await;
// No requests for ancestors since the block is already finalized.
assert_candidate_events_request(&mut ctx_handle, &chain).await;
};

let ordering_provider =
join(OrderingProvider::new(ctx.sender(), leaf.clone()), overseer_fut)
Expand Down

0 comments on commit a26d597

Please sign in to comment.