From 485e7803789eb8c6471b440862e3dd76b4df1fed Mon Sep 17 00:00:00 2001 From: MartinquaXD Date: Tue, 24 Sep 2024 18:16:41 +0000 Subject: [PATCH 1/6] Handle multiple new blocks without reorg --- crates/shared/src/event_handling.rs | 58 ++++++++++++++++++++++++++++- 1 file changed, 57 insertions(+), 1 deletion(-) diff --git a/crates/shared/src/event_handling.rs b/crates/shared/src/event_handling.rs index f696ad9766..8742cc4421 100644 --- a/crates/shared/src/event_handling.rs +++ b/crates/shared/src/event_handling.rs @@ -204,7 +204,29 @@ where }); } - // full range of blocks which are considered for event update + // handle special case where multiple new blocks were added (assuming no reorg + // happened) + let block_range = RangeInclusive::try_new(last_handled_block_number, current_block_number)?; + let mut new_blocks = self.block_retriever.blocks(block_range).await?; + if new_blocks.first().map(|b| b.1) == Some(last_handled_block_hash) { + // the oldest block is actually not new and was only fetched to check if a reorg + // happened + new_blocks.remove(0); + tracing::debug!( + first_new = ?new_blocks.first(), + last_new = ?new_blocks.last(), + "multiple blocks added without reorg" + ); + return Ok(EventRange { + history_range: None, + latest_blocks: new_blocks, + is_reorg: false, + }); + } + + // At this point we are sure a reorg happened somewhere. Since this happens + // very rarely we just fetch the entire block range we consider not finalized + // to look for the reorg. let block_range = RangeInclusive::try_new( last_handled_block_number.saturating_sub(MAX_REORG_BLOCK_COUNT), current_block_number, @@ -811,6 +833,40 @@ mod tests { // add logs to event handler and observe } + #[tokio::test] + #[ignore] + async fn multiple_new_blocks_but_no_reorg_test() { + tracing_subscriber::fmt::init(); + let transport = create_env_test_transport(); + let web3 = Web3::new(transport); + let contract = GPv2Settlement::deployed(&web3).await.unwrap(); + let storage = EventStorage { events: vec![] }; + let current_block = web3.eth().block_number().await.unwrap(); + + const NUMBER_OF_BLOCKS: u64 = 300; + + //get block in history (current_block - NUMBER_OF_BLOCKS) + let block = web3 + .eth() + .block( + BlockNumber::Number(current_block.saturating_sub(NUMBER_OF_BLOCKS.into())).into(), + ) + .await + .unwrap() + .unwrap(); + let block = (block.number.unwrap().as_u64(), block.hash.unwrap()); + let mut event_handler = EventHandler::new( + Arc::new(web3), + GPv2SettlementContract(contract), + storage, + Some(block), + ); + let _result = event_handler.update_events().await; + tracing::info!("wait for at least 2 blocks to see if we hit the new code path"); + tokio::time::sleep(tokio::time::Duration::from_millis(26_000)).await; + let _result = event_handler.update_events().await; + } + #[tokio::test] #[ignore] async fn optional_block_skipping() { From 696020c0073efbc327b77e69e4748193d4efcfa3 Mon Sep 17 00:00:00 2001 From: MartinquaXD Date: Tue, 24 Sep 2024 19:24:55 +0000 Subject: [PATCH 2/6] Handle history range correctly? --- crates/shared/src/event_handling.rs | 41 +++++++++++++++++++++-------- 1 file changed, 30 insertions(+), 11 deletions(-) diff --git a/crates/shared/src/event_handling.rs b/crates/shared/src/event_handling.rs index 8742cc4421..5178036548 100644 --- a/crates/shared/src/event_handling.rs +++ b/crates/shared/src/event_handling.rs @@ -204,22 +204,21 @@ where }); } - // handle special case where multiple new blocks were added (assuming no reorg - // happened) + // special case where multiple new blocks were added and no reorg happened let block_range = RangeInclusive::try_new(last_handled_block_number, current_block_number)?; - let mut new_blocks = self.block_retriever.blocks(block_range).await?; + let new_blocks = self.block_retriever.blocks(block_range).await?; if new_blocks.first().map(|b| b.1) == Some(last_handled_block_hash) { - // the oldest block is actually not new and was only fetched to check if a reorg - // happened - new_blocks.remove(0); + // first block is not actually new and was only fetched to detect a reorg + let (finalized, unfinalized) = separate_finalized_blocks(new_blocks)?; tracing::debug!( - first_new = ?new_blocks.first(), - last_new = ?new_blocks.last(), - "multiple blocks added without reorg" + history=?finalized, + first_new=?unfinalized.first(), + last_new=?unfinalized.last(), + "multiple new blocks without reorg" ); return Ok(EventRange { - history_range: None, - latest_blocks: new_blocks, + history_range: finalized, + latest_blocks: unfinalized, is_reorg: false, }); } @@ -517,6 +516,26 @@ fn split_range(range: RangeInclusive) -> (Option>, Rang } } +/// Splits a range of blocks into an optional range of historic block numbers +/// which are reorgs safe and a range of blocks which could still be reorged in +/// the future. Function assumes `blocks`: +/// - is sorted in increasing order +/// - has no gaps or duplicates (e.g. [1,2,3,4]) +/// - last block (youngest) is the current block +fn separate_finalized_blocks( + mut blocks: Vec, +) -> Result<(Option>, Vec)> { + let split_index = blocks + .len() + .saturating_sub(MAX_REORG_BLOCK_COUNT.try_into()?); + let unfinalized = blocks.split_off(split_index); + let finalized = match (blocks.first(), blocks.last()) { + (Some(first), Some(last)) => Some(RangeInclusive::try_new(first.0, last.0)?), + _ => None, + }; + Ok((finalized, unfinalized)) +} + #[async_trait::async_trait] impl Maintaining for Mutex> where From b26c08b906b55acd68afa35290b45beecd7fd831 Mon Sep 17 00:00:00 2001 From: MartinquaXD Date: Tue, 24 Sep 2024 19:41:33 +0000 Subject: [PATCH 3/6] Limit the number of blocks to fetch --- crates/shared/src/event_handling.rs | 40 +++++++++++++++-------------- 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/crates/shared/src/event_handling.rs b/crates/shared/src/event_handling.rs index 5178036548..043a07d927 100644 --- a/crates/shared/src/event_handling.rs +++ b/crates/shared/src/event_handling.rs @@ -204,28 +204,30 @@ where }); } - // special case where multiple new blocks were added and no reorg happened + // Special case where multiple new blocks were added and no reorg happened. + // Because we need to fetch the full block range we only do this if the number + // of new blocks is sufficiently small. let block_range = RangeInclusive::try_new(last_handled_block_number, current_block_number)?; - let new_blocks = self.block_retriever.blocks(block_range).await?; - if new_blocks.first().map(|b| b.1) == Some(last_handled_block_hash) { - // first block is not actually new and was only fetched to detect a reorg - let (finalized, unfinalized) = separate_finalized_blocks(new_blocks)?; - tracing::debug!( - history=?finalized, - first_new=?unfinalized.first(), - last_new=?unfinalized.last(), - "multiple new blocks without reorg" - ); - return Ok(EventRange { - history_range: finalized, - latest_blocks: unfinalized, - is_reorg: false, - }); + if block_range.end() - block_range.start() <= MAX_REORG_BLOCK_COUNT { + let new_blocks = self.block_retriever.blocks(block_range).await?; + if new_blocks.first().map(|b| b.1) == Some(last_handled_block_hash) { + // first block is not actually new and was only fetched to detect a reorg + let (finalized, unfinalized) = separate_finalized_blocks(new_blocks)?; + tracing::debug!( + history=?finalized, + first_new=?unfinalized.first(), + last_new=?unfinalized.last(), + "multiple new blocks without reorg" + ); + return Ok(EventRange { + history_range: finalized, + latest_blocks: unfinalized, + is_reorg: false, + }); + } } - // At this point we are sure a reorg happened somewhere. Since this happens - // very rarely we just fetch the entire block range we consider not finalized - // to look for the reorg. + // full range of blocks which are considered for event update let block_range = RangeInclusive::try_new( last_handled_block_number.saturating_sub(MAX_REORG_BLOCK_COUNT), current_block_number, From 9b348a3d69f64c2575501cb103163fcc5b01c00a Mon Sep 17 00:00:00 2001 From: MartinquaXD Date: Wed, 25 Sep 2024 06:21:45 +0000 Subject: [PATCH 4/6] Simplify code again after having block range limited --- crates/shared/src/event_handling.rs | 33 ++++++----------------------- 1 file changed, 6 insertions(+), 27 deletions(-) diff --git a/crates/shared/src/event_handling.rs b/crates/shared/src/event_handling.rs index 043a07d927..69e78add66 100644 --- a/crates/shared/src/event_handling.rs +++ b/crates/shared/src/event_handling.rs @@ -209,19 +209,18 @@ where // of new blocks is sufficiently small. let block_range = RangeInclusive::try_new(last_handled_block_number, current_block_number)?; if block_range.end() - block_range.start() <= MAX_REORG_BLOCK_COUNT { - let new_blocks = self.block_retriever.blocks(block_range).await?; + let mut new_blocks = self.block_retriever.blocks(block_range).await?; if new_blocks.first().map(|b| b.1) == Some(last_handled_block_hash) { // first block is not actually new and was only fetched to detect a reorg - let (finalized, unfinalized) = separate_finalized_blocks(new_blocks)?; + new_blocks.remove(0); tracing::debug!( - history=?finalized, - first_new=?unfinalized.first(), - last_new=?unfinalized.last(), + first_new=?new_blocks.first(), + last_new=?new_blocks.last(), "multiple new blocks without reorg" ); return Ok(EventRange { - history_range: finalized, - latest_blocks: unfinalized, + history_range: None, + latest_blocks: new_blocks, is_reorg: false, }); } @@ -518,26 +517,6 @@ fn split_range(range: RangeInclusive) -> (Option>, Rang } } -/// Splits a range of blocks into an optional range of historic block numbers -/// which are reorgs safe and a range of blocks which could still be reorged in -/// the future. Function assumes `blocks`: -/// - is sorted in increasing order -/// - has no gaps or duplicates (e.g. [1,2,3,4]) -/// - last block (youngest) is the current block -fn separate_finalized_blocks( - mut blocks: Vec, -) -> Result<(Option>, Vec)> { - let split_index = blocks - .len() - .saturating_sub(MAX_REORG_BLOCK_COUNT.try_into()?); - let unfinalized = blocks.split_off(split_index); - let finalized = match (blocks.first(), blocks.last()) { - (Some(first), Some(last)) => Some(RangeInclusive::try_new(first.0, last.0)?), - _ => None, - }; - Ok((finalized, unfinalized)) -} - #[async_trait::async_trait] impl Maintaining for Mutex> where From 084433d0bed1f0a9ad63dbea648c87d493f55499 Mon Sep 17 00:00:00 2001 From: MartinquaXD Date: Wed, 25 Sep 2024 12:10:05 +0000 Subject: [PATCH 5/6] Be extra careful with reorgs --- crates/shared/src/event_handling.rs | 33 +++++++++++++++-------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/crates/shared/src/event_handling.rs b/crates/shared/src/event_handling.rs index 69e78add66..c767a94886 100644 --- a/crates/shared/src/event_handling.rs +++ b/crates/shared/src/event_handling.rs @@ -207,22 +207,23 @@ where // Special case where multiple new blocks were added and no reorg happened. // Because we need to fetch the full block range we only do this if the number // of new blocks is sufficiently small. - let block_range = RangeInclusive::try_new(last_handled_block_number, current_block_number)?; - if block_range.end() - block_range.start() <= MAX_REORG_BLOCK_COUNT { - let mut new_blocks = self.block_retriever.blocks(block_range).await?; - if new_blocks.first().map(|b| b.1) == Some(last_handled_block_hash) { - // first block is not actually new and was only fetched to detect a reorg - new_blocks.remove(0); - tracing::debug!( - first_new=?new_blocks.first(), - last_new=?new_blocks.last(), - "multiple new blocks without reorg" - ); - return Ok(EventRange { - history_range: None, - latest_blocks: new_blocks, - is_reorg: false, - }); + if let Ok(block_range) = RangeInclusive::try_new(last_handled_block_number, current_block_number) { + if block_range.end() - block_range.start() <= MAX_REORG_BLOCK_COUNT { + let mut new_blocks = self.block_retriever.blocks(block_range).await?; + if new_blocks.first().map(|b| b.1) == Some(last_handled_block_hash) { + // first block is not actually new and was only fetched to detect a reorg + new_blocks.remove(0); + tracing::debug!( + first_new=?new_blocks.first(), + last_new=?new_blocks.last(), + "multiple new blocks without reorg" + ); + return Ok(EventRange { + history_range: None, + latest_blocks: new_blocks, + is_reorg: false, + }); + } } } From 8b6692dc263e389b75dd72c87682c6dd2d9abdaa Mon Sep 17 00:00:00 2001 From: MartinquaXD Date: Wed, 25 Sep 2024 12:12:03 +0000 Subject: [PATCH 6/6] cargo fmt --- crates/shared/src/event_handling.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/shared/src/event_handling.rs b/crates/shared/src/event_handling.rs index c767a94886..0570d5b64e 100644 --- a/crates/shared/src/event_handling.rs +++ b/crates/shared/src/event_handling.rs @@ -207,7 +207,9 @@ where // Special case where multiple new blocks were added and no reorg happened. // Because we need to fetch the full block range we only do this if the number // of new blocks is sufficiently small. - if let Ok(block_range) = RangeInclusive::try_new(last_handled_block_number, current_block_number) { + if let Ok(block_range) = + RangeInclusive::try_new(last_handled_block_number, current_block_number) + { if block_range.end() - block_range.start() <= MAX_REORG_BLOCK_COUNT { let mut new_blocks = self.block_retriever.blocks(block_range).await?; if new_blocks.first().map(|b| b.1) == Some(last_handled_block_hash) {