Skip to content

Commit

Permalink
feat(starknet_batcher): validate height matches storage on decision r…
Browse files Browse the repository at this point in the history
…eached
  • Loading branch information
ArniStarkware committed Dec 18, 2024
1 parent 825dff3 commit 41b5866
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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(())
}
Expand Down
28 changes: 15 additions & 13 deletions crates/starknet_batcher/src/batcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand All @@ -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);

Expand All @@ -418,6 +419,7 @@ impl Batcher {
error!("Failed to commit block to mempool: {}", mempool_err);
// TODO: Should we rollback the state diff and return an error?
}

Ok(())
}

Expand Down
11 changes: 7 additions & 4 deletions crates/starknet_batcher/src/batcher_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}

Expand All @@ -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));
}

Expand Down
1 change: 1 addition & 0 deletions crates/starknet_batcher_types/src/batcher_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>;

0 comments on commit 41b5866

Please sign in to comment.