feat(consensus): set active/obserever height based on batcher height

[asmaastarkware]

No description provided.

[reviewable-StarkWare]

This change is 

[github-actions]

Artifacts upload workflows:

• Started at 2024-12-17T15:27:20Z
• Started at 2024-12-18T08:06:43Z

[github-actions]

Benchmark movements:
full_committer_flow performance regressed!
full_committer_flow time: [30.370 ms 30.427 ms 30.491 ms]
change: [+1.1154% +1.4592% +1.7638%] (p = 0.00 < 0.05)
Performance has regressed.
Found 2 outliers among 100 measurements (2.00%)
1 (1.00%) high mild
1 (1.00%) high severe

[matan-starkware]

Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @asmaastarkware)

---

crates/starknet_consensus_manager/src/consensus_manager.rs line 72 at r1 (raw file):

                ConsensusError::InternalNetworkError(
                    "Failed to get height from batcher".to_string(),
                )

This is not a network error. I'm fine with just defining an Other(String) error type. Another option is for us to define another error type in this crate and have it inherit from ConsensusError and then add BatcherError. That feels more "proper," but not sure how much I care. What do you prefer?

Code quote:

                ConsensusError::InternalNetworkError(
                    "Failed to get height from batcher".to_string(),
                )

---

crates/starknet_consensus_manager/src/consensus_manager.rs line 78 at r1 (raw file):

        } else {
            BlockNumber(observer_height.0 + 1)
        };

Suggestion:

        let active_height = if self.config.consensus_config.start_height == observer_height {
            // Setting `start_height` is only used to enable consensus starting immediately without observing
            // the first height. This means consensus may return to a height it already voted in, risking
            // equivocation. This is only safe to do if we restart all nodes at this height.
            observer_height
        } else {
            BlockNumber(observer_height.0 + 1)
        };

[asmaastarkware]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @matan-starkware)

---

crates/starknet_consensus_manager/src/consensus_manager.rs line 72 at r1 (raw file):

Previously, matan-starkware wrote…

This is not a network error. I'm fine with just defining an Other(String) error type. Another option is for us to define another error type in this crate and have it inherit from ConsensusError and then add BatcherError. That feels more "proper," but not sure how much I care. What do you prefer?

went with Other(String), but can revisit if needed..