From 6c9582471d459bfba9e5f69be251a7bc01b68023 Mon Sep 17 00:00:00 2001
From: Krisztian Kovacs <krisztian@equilibrium.co>
Date: Mon, 5 Sep 2022 10:53:06 +0200
Subject: [PATCH] fix(sync/l2): check that downloaded blocks are L1/L2 accepted

To avoid issues like we had past week (where a REVERTED block showed up
on Starknet testnet and we only failed to commit on it because it was
missing transaction receipts) this change adds an explicit check on the
block status: we only ever want to process L1 or L2 accepted blocks.

Fixes #559.
---
 crates/pathfinder/src/sequencer/reply.rs | 15 ++++++++++
 crates/pathfinder/src/state/sync/l2.rs   | 37 ++++++++++++++++++++++--
 2 files changed, 49 insertions(+), 3 deletions(-)

diff --git a/crates/pathfinder/src/sequencer/reply.rs b/crates/pathfinder/src/sequencer/reply.rs
index c83a1025d2..6a65213832 100644
--- a/crates/pathfinder/src/sequencer/reply.rs
+++ b/crates/pathfinder/src/sequencer/reply.rs
@@ -124,6 +124,21 @@ pub enum Status {
     Aborted,
 }
 
+impl std::fmt::Display for Status {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            Status::NotReceived => write!(f, "NOT_RECEIVED"),
+            Status::Received => write!(f, "RECEIVED"),
+            Status::Pending => write!(f, "PENDING"),
+            Status::Rejected => write!(f, "REJECTED"),
+            Status::AcceptedOnL1 => write!(f, "ACCEPTED_ON_L1"),
+            Status::AcceptedOnL2 => write!(f, "ACCEPTED_ON_L2"),
+            Status::Reverted => write!(f, "REVERTED"),
+            Status::Aborted => write!(f, "ABORTED"),
+        }
+    }
+}
+
 /// Used to deserialize a reply from [ClientApi::call](crate::sequencer::ClientApi::call).
 #[derive(Clone, Debug, Deserialize, PartialEq, Eq)]
 #[serde(deny_unknown_fields)]
diff --git a/crates/pathfinder/src/state/sync/l2.rs b/crates/pathfinder/src/state/sync/l2.rs
index 219b7d2d1b..71ce4797e7 100644
--- a/crates/pathfinder/src/state/sync/l2.rs
+++ b/crates/pathfinder/src/state/sync/l2.rs
@@ -1,14 +1,14 @@
 use std::time::Duration;
 use std::{collections::HashSet, sync::Arc};
 
-use anyhow::Context;
+use anyhow::{anyhow, Context};
 use tokio::sync::{mpsc, oneshot};
 
 use crate::core::GlobalRoot;
 use crate::sequencer;
 use crate::sequencer::error::SequencerError;
 use crate::sequencer::reply::state_update::{DeployedContract, StateDiff};
-use crate::sequencer::reply::Block;
+use crate::sequencer::reply::{Block, Status};
 use crate::state::block_hash::{verify_block_hash, VerifyResult};
 use crate::state::class_hash::extract_abi_code_hash;
 use crate::state::CompressedContract;
@@ -269,7 +269,13 @@ async fn download_block(
                 let block_number = block.block_number;
                 tracing::warn!(?block_number, block_hash = ?expected_block_hash, "Block hash mismatch");
             }
-            Ok(DownloadBlock::Block(block))
+            match block.status {
+                Status::AcceptedOnL1 | Status::AcceptedOnL2 => Ok(DownloadBlock::Block(block)),
+                _ => Err(anyhow!(
+                    "Rejecting block as its status is {}, and only accepted blocks are allowed",
+                    block.status
+                )),
+            }
         }
         Ok(MaybePendingBlock::Pending(_)) => anyhow::bail!("Sequencer returned `pending` block"),
         Err(SequencerError::StarknetError(err)) if err.code == BlockNotFound => {
@@ -1002,6 +1008,31 @@ mod tests {
             }
         }
 
+        mod errors {
+            use super::*;
+            use crate::core::Chain;
+            use crate::sequencer::reply::Status;
+
+            #[tokio::test]
+            async fn invalid_block_status() {
+                let (tx_event, _rx_event) = tokio::sync::mpsc::channel(1);
+                let mut mock = MockClientApi::new();
+                let mut seq = mockall::Sequence::new();
+
+                // Block with a non-accepted status
+                let mut block = BLOCK0.clone();
+                block.status = Status::Reverted;
+                expect_block(&mut mock, &mut seq, BLOCK0_NUMBER.into(), Ok(block.into()));
+
+                let jh = tokio::spawn(sync(tx_event, mock, None, Chain::Testnet, None));
+                let error = jh.await.unwrap().unwrap_err();
+                assert_eq!(
+                    &error.to_string(),
+                    "Rejecting block as its status is REVERTED, and only accepted blocks are allowed"
+                );
+            }
+        }
+
         mod reorg {
             use super::*;
             use crate::core::Chain;