feat(starknet_batcher): validate height matches storage on decision reached

crates/sequencing/papyrus_consensus_orchestrator/src/sequencer_consensus_context.rs

@@ -289,7 +289,7 @@ impl ConsensusContext for SequencerConsensusContext {
         block: ProposalContentId,
         precommits: Vec<Vote>,
     ) -> Result<(), ConsensusError> {
-        let height = precommits[0].height;
+        let height = BlockNumber(precommits[0].height);
         info!("Finished consensus for height: {height}. Agreed on block: {:#064x}", block.0);
 
         // TODO(matan): Broadcast the decision to the network.
@@ -300,10 +300,10 @@ impl ConsensusContext for SequencerConsensusContext {
                 .valid_proposals
                 .lock()
                 .expect("Lock on active proposals was poisoned due to a previous panic");
-            proposal_id = proposals.get(&BlockNumber(height)).unwrap().get(&block).unwrap().1;
-            proposals.retain(|&h, _| h > BlockNumber(height));
+            proposal_id = proposals.get(&height).unwrap().get(&block).unwrap().1;
+            proposals.retain(|&h, _| h > height);
         }
-        self.batcher.decision_reached(DecisionReachedInput { proposal_id }).await.unwrap();
+        self.batcher.decision_reached(DecisionReachedInput { proposal_id, height }).await.unwrap();
 
         Ok(())
     }

crates/starknet_batcher/src/batcher.rs

@@ -364,7 +364,7 @@ impl Batcher {
         let address_to_nonce = state_diff.nonces.iter().map(|(k, v)| (*k, *v)).collect();
         let tx_hashes = transaction_hashes.into_iter().collect();
 
-        self.commit_proposal_and_block(Some(height), state_diff, address_to_nonce, tx_hashes).await
+        self.commit_proposal_and_block(height, state_diff, address_to_nonce, tx_hashes).await
     }
 
     #[instrument(skip(self), err)]
@@ -381,29 +381,30 @@ impl Batcher {
             .map_err(|_| BatcherError::InternalError)?;
         let ProposalOutput { state_diff, nonces: address_to_nonce, tx_hashes, .. } =
             proposal_output;
+        self.commit_proposal_and_block(
+            input.height,
+            state_diff.clone(),
+            address_to_nonce,
+            tx_hashes,
+        )
+        .await?;
 
-        // TODO(Arni): Get the height from the proposal output or from the proposal id. Pass it to
-        // commit proposal and block. set the variable as non-optional.
-        self.commit_proposal_and_block(None, state_diff.clone(), address_to_nonce, tx_hashes)
-            .await?;
         Ok(DecisionReachedResponse { state_diff })
     }
 
     async fn commit_proposal_and_block(
         &mut self,
-        proposed_height: Option<BlockNumber>,
+        proposed_height: BlockNumber,
         state_diff: ThinStateDiff,
         address_to_nonce: HashMap<ContractAddress, Nonce>,
         tx_hashes: HashSet<TransactionHash>,
     ) -> BatcherResult<()> {
         let height = self.get_height_from_storage()?;
-        if let Some(proposed_height) = proposed_height {
-            if height != proposed_height {
-                panic!(
-                    "Proposed height {} does not match the current height {}.",
-                    proposed_height, height
-                );
-            }
+        if height != proposed_height {
+            panic!(
+                "Proposed height {} does not match the current height {}.",
+                proposed_height, height
+            );
         }
         info!("Committing block at height {} and notifying mempool of the block.", height);
         trace!("Transactions: {:#?}, State diff: {:#?}.", tx_hashes, state_diff);
@@ -417,6 +418,7 @@ impl Batcher {
             error!("Failed to commit block to mempool: {}", mempool_err);
             // TODO: Should we rollback the state diff and return an error?
         }
+
         Ok(())
     }
 

crates/starknet_batcher/src/batcher_test.rs

@@ -555,8 +555,10 @@ async fn decision_reached() {
 
     let mut batcher = create_batcher(mock_dependencies);
 
-    let response =
-        batcher.decision_reached(DecisionReachedInput { proposal_id: PROPOSAL_ID }).await.unwrap();
+    let response = batcher
+        .decision_reached(DecisionReachedInput { proposal_id: PROPOSAL_ID, height: INITIAL_HEIGHT })
+        .await
+        .unwrap();
     assert_eq!(response.state_diff, test_state_diff());
 }
 
@@ -573,8 +575,9 @@ async fn decision_reached_no_executed_proposal() {
         .return_once(|_| async move { None }.boxed());
 
     let mut batcher = create_batcher(MockDependencies { proposal_manager, ..Default::default() });
-    let decision_reached_result =
-        batcher.decision_reached(DecisionReachedInput { proposal_id: PROPOSAL_ID }).await;
+    let decision_reached_result = batcher
+        .decision_reached(DecisionReachedInput { proposal_id: PROPOSAL_ID, height: INITIAL_HEIGHT })
+        .await;
     assert_eq!(decision_reached_result, Err(expected_error));
 }
 

crates/starknet_batcher_types/src/batcher_types.rs

@@ -114,6 +114,7 @@ pub struct StartHeightInput {
 #[derive(Clone, Debug, Serialize, Deserialize)]
 pub struct DecisionReachedInput {
     pub proposal_id: ProposalId,
+    pub height: BlockNumber,
 }
 
 pub type BatcherResult<T> = Result<T, BatcherError>;