From a26d5976c1eeec189b8faff570a36739dec280ed Mon Sep 17 00:00:00 2001 From: Chris Sosnin <48099298+slumber@users.noreply.github.com> Date: Mon, 20 Dec 2021 13:52:21 +0300 Subject: [PATCH] dispute-coordinator: fix underflow panic (#4557) --- .../src/real/ordering/mod.rs | 22 +++++++++++++++---- .../src/real/ordering/tests.rs | 12 +++++----- 2 files changed, 23 insertions(+), 11 deletions(-) diff --git a/node/core/dispute-coordinator/src/real/ordering/mod.rs b/node/core/dispute-coordinator/src/real/ordering/mod.rs index b8eea2113414..c6f1a89904af 100644 --- a/node/core/dispute-coordinator/src/real/ordering/mod.rs +++ b/node/core/dispute-coordinator/src/real/ordering/mod.rs @@ -256,12 +256,12 @@ impl OrderingProvider { ) -> Result> { 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 = { @@ -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(); @@ -295,6 +308,7 @@ impl OrderingProvider { ancestors.push(*hash); } + match hashes.last() { Some(last_hash) => { head = *last_hash; diff --git a/node/core/dispute-coordinator/src/real/ordering/tests.rs b/node/core/dispute-coordinator/src/real/ordering/tests.rs index 6b9bac0da060..6db10d7432f9 100644 --- a/node/core/dispute-coordinator/src/real/ordering/tests.rs +++ b/node/core/dispute-coordinator/src/real/ordering/tests.rs @@ -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)