Skip to content

Commit

Permalink
fix(sync/l2): check that downloaded blocks are L1/L2 accepted
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
kkovaacs committed Sep 5, 2022
1 parent 56adca0 commit 6c95824
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 3 deletions.
15 changes: 15 additions & 0 deletions crates/pathfinder/src/sequencer/reply.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
37 changes: 34 additions & 3 deletions crates/pathfinder/src/state/sync/l2.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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 => {
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit 6c95824

Please sign in to comment.