make start height the first execution height instead of one before

charts/evm-rollup/values.yaml

@@ -32,9 +32,9 @@ genesis:
   # for the genesis fork are provided, and additional forks can be added as needed.
   forks:
     launch:
-      # The block height to start the chain at, 0 for genesis
-      height: "0"
-      # Whether to halt the rollup chain at the given height. False for genesis
+      # The rollup number to start executing blocks at, lowest possible is 1
+      height: 1
+      # Whether to halt the rollup chain at the given height
       halt: "false"
       # Checksum of the snapshot to use upon restart
       snapshotChecksum: ""
@@ -56,11 +56,8 @@ genesis:
         chainId: ""
         # The hrp for bech32m addresses, unlikely to be changed
         addressPrefix: "astria"
-        # Block height to start syncing rollup from, lowest possible is 2
+        # Block height to start syncing rollup from (inclusive), lowest possible is 2
         startHeight: ""
-        # Block height (on sequencer) to stop syncing rollup at, continuing to next configuration fork.
-        # A height of 0 indicates no stop height.
-        stopHeight: ""
       celestia:
         # The chain id of the celestia chain
         chainId: ""

crates/astria-conductor/src/executor/mod.rs

@@ -215,11 +215,14 @@ impl Executor {
         let (genesis_info, commitment_state) = tokio::try_join!(genesis_info, commitment_state)?;
 
         let (state, _) = state::channel(
-            State::try_from_genesis_info_and_commitment_state(genesis_info, commitment_state)
-                .wrap_err(
-                    "failed to construct initial state gensis and commitment info received from \
-                     rollup",
-                )?,
+            State::try_from_genesis_info_and_commitment_state(
+                genesis_info,
+                commitment_state,
+                self.config.execution_commit_level,
+            )
+            .wrap_err(
+                "failed to construct initial state gensis and commitment info received from rollup",
+            )?,
         );
 
         self.metrics
@@ -544,14 +547,27 @@ impl Initialized {
             OnlySoft,
             ToSame,
         };
-        let (firm, soft, celestia_height) = match update {
-            OnlyFirm(firm, celestia_height) => (firm, self.state.soft(), celestia_height),
+
+        use crate::config::CommitLevel;
+        let (firm, soft, celestia_height, commit_level) = match update {
+            OnlyFirm(firm, celestia_height) => (
+                firm,
+                self.state.soft(),
+                celestia_height,
+                CommitLevel::FirmOnly,
+            ),
             OnlySoft(soft) => (
                 self.state.firm(),
                 soft,
                 self.state.celestia_base_block_height(),
+                CommitLevel::SoftOnly,
+            ),
+            ToSame(block, celestia_height) => (
+                block.clone(),
+                block,
+                celestia_height,
+                CommitLevel::SoftAndFirm,
             ),
-            ToSame(block, celestia_height) => (block.clone(), block, celestia_height),
         };
         let commitment_state = CommitmentState::builder()
             .firm(firm)
@@ -572,7 +588,7 @@ impl Initialized {
             "updated commitment state",
         );
         self.state
-            .try_update_commitment_state(new_state)
+            .try_update_commitment_state(new_state, commit_level)
             .wrap_err("failed updating internal state tracking rollup state; invalid?")?;
         Ok(())
     }

crates/astria-conductor/src/executor/state.rs

@@ -191,10 +191,15 @@ impl StateSender {
     pub(super) fn try_update_commitment_state(
         &mut self,
         commitment_state: CommitmentState,
+        commit_level: crate::config::CommitLevel,
     ) -> Result<(), InvalidState> {
         let genesis_info = self.genesis_info();
-        let _ = map_firm_to_sequencer_height(&genesis_info, &commitment_state)?;
-        let _ = map_soft_to_sequencer_height(&genesis_info, &commitment_state)?;
+        if commit_level.is_with_firm() {
+            let _ = map_firm_to_sequencer_height(&genesis_info, &commitment_state)?;
+        }
+        if commit_level.is_with_soft() {
+            let _ = map_soft_to_sequencer_height(&genesis_info, &commitment_state)?;
+        }
         self.inner.send_modify(move |state| {
             state.set_commitment_state(commitment_state);
         });
@@ -280,9 +285,14 @@ impl State {
     pub(crate) fn try_from_genesis_info_and_commitment_state(
         genesis_info: GenesisInfo,
         commitment_state: CommitmentState,
+        commit_level: crate::config::CommitLevel,
     ) -> Result<Self, InvalidState> {
-        let _ = map_firm_to_sequencer_height(&genesis_info, &commitment_state)?;
-        let _ = map_soft_to_sequencer_height(&genesis_info, &commitment_state)?;
+        if commit_level.is_with_firm() {
+            let _ = map_firm_to_sequencer_height(&genesis_info, &commitment_state)?;
+        }
+        if commit_level.is_with_soft() {
+            let _ = map_soft_to_sequencer_height(&genesis_info, &commitment_state)?;
+        }
         Ok(State {
             commitment_state,
             genesis_info,
@@ -402,18 +412,22 @@ impl State {
 /// Maps a rollup height to a sequencer height.
 ///
 /// Returns error if `sequencer_start_height + (rollup_number - rollup_start_block_number)`
-/// overflows `u32::MAX`.
+/// is out of range of `u32` or if `rollup_start_block_number` is more than 1 greater than
+/// `rollup_number`.
 fn map_rollup_number_to_sequencer_height(
     sequencer_start_height: u64,
     rollup_start_block_number: u64,
     rollup_number: u32,
 ) -> Result<SequencerHeight, &'static str> {
-    let delta = u64::from(rollup_number)
-        .checked_sub(rollup_start_block_number)
-        .ok_or("rollup start height exceeds rollup number")?;
+    let rollup_number = u64::from(rollup_number);
+    if rollup_start_block_number > (rollup_number.checked_add(1).ok_or("overflows u64::MAX")?) {
+        return Err("rollup start height exceeds rollup number + 1");
+    }
     let sequencer_height = sequencer_start_height
-        .checked_add(delta)
-        .ok_or("overflows u64::MAX")?;
+        .checked_add(rollup_number)
+        .ok_or("overflows u64::MAX")?
+        .checked_sub(rollup_start_block_number)
+        .ok_or("(sequencer height + rollup number - rollup start height) is negative")?;
     sequencer_height
         .try_into()
         .map_err(|_| "overflows u32::MAX, the maximum cometbft height")
@@ -456,7 +470,7 @@ mod tests {
     fn next_firm_sequencer_height_is_correct() {
         let (_, rx) = make_channel();
         assert_eq!(
-            SequencerHeight::from(12u32),
+            SequencerHeight::from(11u32),
             rx.next_expected_firm_sequencer_height(),
         );
     }
@@ -465,7 +479,7 @@ mod tests {
     fn next_soft_sequencer_height_is_correct() {
         let (_, rx) = make_channel();
         assert_eq!(
-            SequencerHeight::from(13u32),
+            SequencerHeight::from(12u32),
             rx.next_expected_soft_sequencer_height(),
         );
     }
@@ -490,8 +504,8 @@ mod tests {
 
     #[should_panic = "rollup start height exceeds rollup number"]
     #[test]
-    fn is_error_if_rollup_start_exceeds_current_number() {
-        map_rollup_number_to_sequencer_height(10, 10, 9).unwrap();
+    fn is_error_if_rollup_start_exceeds_current_number_plus_one() {
+        map_rollup_number_to_sequencer_height(10, 11, 9).unwrap();
     }
 
     #[test]

crates/astria-conductor/src/executor/tests.rs

@@ -53,8 +53,12 @@ fn make_state(
         base_celestia_height: 1,
     })
     .unwrap();
-    let state =
-        State::try_from_genesis_info_and_commitment_state(genesis_info, commitment_state).unwrap();
+    let state = State::try_from_genesis_info_and_commitment_state(
+        genesis_info,
+        commitment_state,
+        crate::config::CommitLevel::SoftAndFirm,
+    )
+    .unwrap();
     super::state::channel(state)
 }
 

crates/astria-conductor/src/test_utils.rs

@@ -42,7 +42,7 @@ pub(crate) fn make_genesis_info() -> GenesisInfo {
         rollup_id: Some(rollup_id.to_raw()),
         sequencer_start_height: 10,
         celestia_block_variance: 0,
-        rollup_start_block_number: 0,
+        rollup_start_block_number: 1,
         rollup_stop_block_number: 90,
         sequencer_chain_id: "test-sequencer-0".to_string(),
         celestia_chain_id: "test-celestia-0".to_string(),
@@ -57,5 +57,10 @@ pub(crate) fn make_rollup_state(
     let genesis_info = astria_core::execution::v2::GenesisInfo::try_from_raw(genesis_info).unwrap();
     let commitment_state =
         astria_core::execution::v2::CommitmentState::try_from_raw(commitment_state).unwrap();
-    State::try_from_genesis_info_and_commitment_state(genesis_info, commitment_state).unwrap()
+    State::try_from_genesis_info_and_commitment_state(
+        genesis_info,
+        commitment_state,
+        crate::config::CommitLevel::SoftAndFirm,
+    )
+    .unwrap()
 }

crates/astria-conductor/tests/blackbox/firm_only.rs

@@ -54,9 +54,9 @@ async fn simple() {
 
     mount_get_genesis_info!(
         test_conductor,
-        sequencer_start_height: 1,
+        sequencer_start_height: 3,
         celestia_block_variance: 10,
-        rollup_start_block_number: 0,
+        rollup_start_block_number: 2,
         rollup_stop_block_number: 9,
     );
 
@@ -137,9 +137,9 @@ async fn submits_two_heights_in_succession() {
 
     mount_get_genesis_info!(
         test_conductor,
-        sequencer_start_height: 1,
+        sequencer_start_height: 3,
         celestia_block_variance: 10,
-        rollup_start_block_number: 0,
+        rollup_start_block_number: 2,
         rollup_stop_block_number: 9,
     );
 
@@ -251,9 +251,9 @@ async fn skips_already_executed_heights() {
 
     mount_get_genesis_info!(
         test_conductor,
-        sequencer_start_height: 1,
+        sequencer_start_height: 3,
         celestia_block_variance: 10,
-        rollup_start_block_number: 0,
+        rollup_start_block_number: 2,
         rollup_stop_block_number: 9,
     );
 
@@ -337,9 +337,9 @@ async fn fetch_from_later_celestia_height() {
 
     mount_get_genesis_info!(
         test_conductor,
-        sequencer_start_height: 1,
+        sequencer_start_height: 3,
         celestia_block_variance: 10,
-        rollup_start_block_number: 0,
+        rollup_start_block_number: 2,
         rollup_stop_block_number: 9,
     );
 
@@ -455,9 +455,9 @@ async fn exits_on_celestia_chain_id_mismatch() {
         matcher::message_type::<GetGenesisInfoRequest>(),
     )
     .respond_with(GrpcResponse::constant_response(
-        genesis_info!(sequencer_start_height: 1,
+        genesis_info!(sequencer_start_height: 3,
             celestia_block_variance: 10,
-            rollup_start_block_number: 0,
+            rollup_start_block_number: 2,
             rollup_stop_block_number: 9
         ),
     ))
@@ -548,9 +548,9 @@ async fn restarts_after_reaching_stop_block_height() {
 
     mount_get_genesis_info!(
         test_conductor,
-        sequencer_start_height: 1,
+        sequencer_start_height: 3,
         celestia_block_variance: 10,
-        rollup_start_block_number: 0,
+        rollup_start_block_number: 2,
         rollup_stop_block_number: 2,
         up_to_n_times: 1, // Only respond once, since updated information is needed after restart.
     );
@@ -635,9 +635,9 @@ async fn restarts_after_reaching_stop_block_height() {
     // Mount new genesis info and commitment state with updated heights
     mount_get_genesis_info!(
         test_conductor,
-        sequencer_start_height: 2,
+        sequencer_start_height: 4,
         celestia_block_variance: 10,
-        rollup_start_block_number: 1,
+        rollup_start_block_number: 3,
         rollup_stop_block_number: 9,
     );