apollo_consensus: better panic message on voted height storage error

[ShahakShama]

No description provided.

[reviewable-StarkWare]

This change is 

[ShahakShama]

• apollo_consensus: better panic message on voted height storage error #13835 👈 (View in Graphite)
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[itamar-starkware]

Code review

Result::expect takes a &str and passes it straight to the panic formatter — no format!-style substitution. The literal text {self.height} appears as-is in the panic message. Also, MultiHeightManager has no height field; the value in scope is the local height parameter.

Demonstration:

fn main() {
    let x: Result<(), &str> = Err("underlying error");
    x.expect("Failed to write voted height {self.height} to storage");
}

Output:

thread 'main' panicked at ...:
Failed to write voted height {self.height} to storage: "underlying error"

Suggestion: switch to unwrap_or_else with panic! (which is a macro and does interpolate), and reference the local height:

self.voted_height_storage
    .lock()
    .await
    .set_prev_voted_height(height)
    .unwrap_or_else(|error| {
        panic!(
            "Failed to write voted height {height} to storage: {error}. Crashing before \
             sending vote to avoid risk of equivocation"
        )
    });

sequencer/crates/apollo_consensus/src/manager.rs

Lines 1020 to 1028 in d32871d

	}
	SMRequest::BroadcastVote(vote) => {
	trace!("Writing voted height {} to storage", height);
	self.voted_height_storage.lock().await.set_prev_voted_height(height).expect(
	"Failed to write voted height {self.height} to storage. Crashing before \
	sending vote to avoid risk of equivocation",
	);
	context.broadcast(vote.clone()).await?;
	// Schedule a rebroadcast after the appropriate timeout.

Reference: Result::expect.

🤖 Generated with Claude Code

_{- If this code review was useful, please react with 👍. Otherwise, react with 👎.}

[itamar-starkware]

.expect(&format!("Failed to write voted height {height} to storage"));
Will also work.

[itamar-starkware]

crates/apollo_consensus/src/manager.rs line 1023 at r1 (raw file):

            SMRequest::BroadcastVote(vote) => {
                trace!("Writing voted height {} to storage", height);
                self.voted_height_storage.lock().await.set_prev_voted_height(height).expect(

Suggestion:

.expect(&format!("Failed to write voted height {height} to storage. Crashing..."));

[itamar-starkware]

@itamar-starkware reviewed 1 file and all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on ShahakShama).

[ShahakShama]

@ShahakShama made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on itamar-starkware).

---

crates/apollo_consensus/src/manager.rs line 1023 at r1 (raw file):

Previously, itamar-starkware wrote…

Suggestion:

.expect(&format!("Failed to write voted height {height} to storage. Crashing..."));

Done.