From d8b97784469c2b930e259ca34c97969d5fc71cdb Mon Sep 17 00:00:00 2001 From: ethanoroshiba Date: Tue, 3 Dec 2024 09:23:40 -0600 Subject: [PATCH 01/27] proto changes, add stop/restart logic --- charts/evm-rollup/templates/configmap.yaml | 2 - charts/evm-rollup/values.yaml | 4 - crates/astria-conductor/local.env.example | 6 - .../astria-conductor/src/celestia/builder.rs | 6 - crates/astria-conductor/src/celestia/mod.rs | 34 ++--- .../astria-conductor/src/conductor/inner.rs | 20 ++- crates/astria-conductor/src/config.rs | 6 - crates/astria-conductor/src/executor/mod.rs | 58 ++++++- crates/astria-conductor/src/executor/state.rs | 50 ++++-- crates/astria-conductor/src/executor/tests.rs | 5 +- .../astria-conductor/src/sequencer/builder.rs | 3 - crates/astria-conductor/src/sequencer/mod.rs | 12 +- .../tests/blackbox/firm_only.rs | 142 +++++++++++++++++- .../tests/blackbox/helpers/macros.rs | 38 ++++- .../tests/blackbox/helpers/mod.rs | 5 +- .../tests/blackbox/soft_and_firm.rs | 12 +- .../tests/blackbox/soft_only.rs | 127 +++++++++++++++- crates/astria-core/src/execution/v1/mod.rs | 59 ++++++-- .../src/generated/astria.execution.v1.rs | 9 +- .../generated/astria.execution.v1.serde.rs | 82 ++++++++-- .../astria/execution/v1/execution.proto | 6 +- 21 files changed, 561 insertions(+), 125 deletions(-) diff --git a/charts/evm-rollup/templates/configmap.yaml b/charts/evm-rollup/templates/configmap.yaml index 5a5d52ae57..7e1a466c9e 100644 --- a/charts/evm-rollup/templates/configmap.yaml +++ b/charts/evm-rollup/templates/configmap.yaml @@ -6,14 +6,12 @@ metadata: data: ASTRIA_CONDUCTOR_LOG: "astria_conductor={{ .Values.config.logLevel }}" ASTRIA_CONDUCTOR_CELESTIA_NODE_HTTP_URL: "{{ .Values.config.celestia.rpc }}" - ASTRIA_CONDUCTOR_EXPECTED_CELESTIA_CHAIN_ID: "{{ tpl .Values.config.conductor.celestiaChainId . }}" ASTRIA_CONDUCTOR_CELESTIA_BEARER_TOKEN: "{{ .Values.config.celestia.token }}" ASTRIA_CONDUCTOR_CELESTIA_BLOCK_TIME_MS: "12000" ASTRIA_CONDUCTOR_EXECUTION_RPC_URL: "http://127.0.0.1:{{ .Values.ports.executionGRPC }}" ASTRIA_CONDUCTOR_EXECUTION_COMMIT_LEVEL: "{{ .Values.config.conductor.executionCommitLevel }}" ASTRIA_CONDUCTOR_SEQUENCER_GRPC_URL: "{{ tpl .Values.config.conductor.sequencerGrpc . }}" ASTRIA_CONDUCTOR_SEQUENCER_COMETBFT_URL: "{{ tpl .Values.config.conductor.sequencerRpc . }}" - ASTRIA_CONDUCTOR_EXPECTED_SEQUENCER_CHAIN_ID: "{{ tpl .Values.config.conductor.sequencerChainId . }}" ASTRIA_CONDUCTOR_SEQUENCER_BLOCK_TIME_MS: "{{ .Values.config.conductor.sequencerBlockTimeMs }}" ASTRIA_CONDUCTOR_NO_METRICS: "{{ not .Values.metrics.enabled }}" ASTRIA_CONDUCTOR_METRICS_HTTP_LISTENER_ADDR: "0.0.0.0:{{ .Values.ports.conductorMetrics }}" diff --git a/charts/evm-rollup/values.yaml b/charts/evm-rollup/values.yaml index 1f2472c022..9bec13f48b 100644 --- a/charts/evm-rollup/values.yaml +++ b/charts/evm-rollup/values.yaml @@ -167,8 +167,6 @@ config: # - "FirmOnly" -> blocks are only pulled from DA # - "SoftAndFirm" -> blocks are pulled from both the sequencer and DA executionCommitLevel: 'SoftAndFirm' - # The chain id of the Astria sequencer chain conductor communicates with - sequencerChainId: "" # The expected fastest block time possible from sequencer, determines polling # rate. sequencerBlockTimeMs: 2000 @@ -178,8 +176,6 @@ config: sequencerGrpc: "" # The maximum number of requests to make to the sequencer per second sequencerRequestsPerSecond: 500 - # The chain id of the celestia network the conductor communicates with - celestiaChainId: "" celestia: # if config.rollup.executionLevel is NOT 'SoftOnly' AND celestia-node is not enabled diff --git a/crates/astria-conductor/local.env.example b/crates/astria-conductor/local.env.example index 9a1eb3e43d..6237d3109b 100644 --- a/crates/astria-conductor/local.env.example +++ b/crates/astria-conductor/local.env.example @@ -73,12 +73,6 @@ ASTRIA_CONDUCTOR_SEQUENCER_BLOCK_TIME_MS=2000 # CometBFT node. ASTRIA_CONDUCTOR_SEQUENCER_REQUESTS_PER_SECOND=500 -# The chain ID of the sequencer network the conductor should be communicating with. -ASTRIA_CONDUCTOR_EXPECTED_SEQUENCER_CHAIN_ID="test-sequencer-1000" - -# The chain ID of the Celestia network the conductor should be communicating with. -ASTRIA_CONDUCTOR_EXPECTED_CELESTIA_CHAIN_ID="test-celestia-1000" - # Set to true to enable prometheus metrics. ASTRIA_CONDUCTOR_NO_METRICS=true diff --git a/crates/astria-conductor/src/celestia/builder.rs b/crates/astria-conductor/src/celestia/builder.rs index ef33a28cbd..7860d439ee 100644 --- a/crates/astria-conductor/src/celestia/builder.rs +++ b/crates/astria-conductor/src/celestia/builder.rs @@ -23,8 +23,6 @@ pub(crate) struct Builder { pub(crate) executor: executor::Handle, pub(crate) sequencer_cometbft_client: SequencerClient, pub(crate) sequencer_requests_per_second: u32, - pub(crate) expected_celestia_chain_id: String, - pub(crate) expected_sequencer_chain_id: String, pub(crate) shutdown: CancellationToken, pub(crate) metrics: &'static Metrics, } @@ -39,8 +37,6 @@ impl Builder { executor, sequencer_cometbft_client, sequencer_requests_per_second, - expected_celestia_chain_id, - expected_sequencer_chain_id, shutdown, metrics, } = self; @@ -54,8 +50,6 @@ impl Builder { executor, sequencer_cometbft_client, sequencer_requests_per_second, - expected_celestia_chain_id, - expected_sequencer_chain_id, shutdown, metrics, }) diff --git a/crates/astria-conductor/src/celestia/mod.rs b/crates/astria-conductor/src/celestia/mod.rs index 85146dc645..25c384eb38 100644 --- a/crates/astria-conductor/src/celestia/mod.rs +++ b/crates/astria-conductor/src/celestia/mod.rs @@ -141,12 +141,6 @@ pub(crate) struct Reader { /// (usually to verify block data retrieved from Celestia blobs). sequencer_requests_per_second: u32, - /// The chain ID of the Celestia network the reader should be communicating with. - expected_celestia_chain_id: String, - - /// The chain ID of the Sequencer the reader should be communicating with. - expected_sequencer_chain_id: String, - /// Token to listen for Conductor being shut down. shutdown: CancellationToken, @@ -178,45 +172,45 @@ impl Reader { async fn initialize( &mut self, ) -> eyre::Result<((), executor::Handle, tendermint::chain::Id)> { + let executor = self + .executor + .wait_for_init() + .await + .wrap_err("handle to executor failed while waiting for it being initialized")?; + let validate_celestia_chain_id = async { let actual_celestia_chain_id = get_celestia_chain_id(&self.celestia_client) .await .wrap_err("failed to fetch Celestia chain ID")?; - let expected_celestia_chain_id = &self.expected_celestia_chain_id; + let expected_celestia_chain_id = executor.celestia_chain_id(); ensure!( - self.expected_celestia_chain_id == actual_celestia_chain_id.as_str(), + expected_celestia_chain_id == actual_celestia_chain_id.as_str(), "expected Celestia chain id `{expected_celestia_chain_id}` does not match actual: \ `{actual_celestia_chain_id}`" ); Ok(()) }; - let wait_for_init_executor = async { - self.executor - .wait_for_init() - .await - .wrap_err("handle to executor failed while waiting for it being initialized") - }; - let get_and_validate_sequencer_chain_id = async { let actual_sequencer_chain_id = get_sequencer_chain_id(self.sequencer_cometbft_client.clone()) .await .wrap_err("failed to get sequencer chain ID")?; - let expected_sequencer_chain_id = &self.expected_sequencer_chain_id; + let expected_sequencer_chain_id = executor.sequencer_chain_id(); ensure!( - self.expected_sequencer_chain_id == actual_sequencer_chain_id.to_string(), + expected_sequencer_chain_id == actual_sequencer_chain_id.to_string(), "expected Celestia chain id `{expected_sequencer_chain_id}` does not match \ actual: `{actual_sequencer_chain_id}`" ); Ok(actual_sequencer_chain_id) }; - try_join!( + let ((), sequencer_chain_id) = try_join!( validate_celestia_chain_id, - wait_for_init_executor, get_and_validate_sequencer_chain_id - ) + )?; + + Ok(((), executor, sequencer_chain_id)) } } diff --git a/crates/astria-conductor/src/conductor/inner.rs b/crates/astria-conductor/src/conductor/inner.rs index c52bd32536..c16aab5e96 100644 --- a/crates/astria-conductor/src/conductor/inner.rs +++ b/crates/astria-conductor/src/conductor/inner.rs @@ -28,6 +28,7 @@ use tracing::{ warn, }; +use self::executor::StopHeightExceded; use crate::{ celestia, executor, @@ -133,7 +134,6 @@ impl ConductorInner { sequencer_grpc_client, sequencer_cometbft_client: sequencer_cometbft_client.clone(), sequencer_block_time: Duration::from_millis(cfg.sequencer_block_time_ms), - expected_sequencer_chain_id: cfg.expected_sequencer_chain_id.clone(), shutdown: shutdown_token.clone(), executor: executor_handle.clone(), } @@ -155,8 +155,6 @@ impl ConductorInner { executor: executor_handle.clone(), sequencer_cometbft_client: sequencer_cometbft_client.clone(), sequencer_requests_per_second: cfg.sequencer_requests_per_second, - expected_celestia_chain_id: cfg.expected_celestia_chain_id, - expected_sequencer_chain_id: cfg.expected_sequencer_chain_id, shutdown: shutdown_token.clone(), metrics, } @@ -311,6 +309,8 @@ fn check_for_restart(name: &str, err: &eyre::Report) -> bool { if status.code() == tonic::Code::PermissionDenied { return true; } + } else if err.downcast_ref::().is_some() { + return true; } current = err.source(); } @@ -329,5 +329,19 @@ mod tests { let err = err.wrap_err("wrapper_2"); let err = err.wrap_err("wrapper_3"); assert!(super::check_for_restart("executor", &err.unwrap_err())); + + let celestia_height_error: Result<&str, super::StopHeightExceded> = + Err(super::StopHeightExceded::Celestia); + let err = celestia_height_error.wrap_err("wrapper_1"); + let err = err.wrap_err("wrapper_2"); + let err = err.wrap_err("wrapper_3"); + assert!(super::check_for_restart("executor", &err.unwrap_err())); + + let sequencer_height_error: Result<&str, super::StopHeightExceded> = + Err(super::StopHeightExceded::Sequencer); + let err = sequencer_height_error.wrap_err("wrapper_1"); + let err = err.wrap_err("wrapper_2"); + let err = err.wrap_err("wrapper_3"); + assert!(super::check_for_restart("executor", &err.unwrap_err())); } } diff --git a/crates/astria-conductor/src/config.rs b/crates/astria-conductor/src/config.rs index e8211b1714..0cb8c0969d 100644 --- a/crates/astria-conductor/src/config.rs +++ b/crates/astria-conductor/src/config.rs @@ -63,12 +63,6 @@ pub struct Config { /// The number of requests per second that will be sent to Sequencer. pub sequencer_requests_per_second: u32, - /// The chain ID of the sequencer network the conductor should be communiacting with. - pub expected_sequencer_chain_id: String, - - /// The chain ID of the Celestia network the conductor should be communicating with. - pub expected_celestia_chain_id: String, - /// Address of the RPC server for execution pub execution_rpc_url: String, diff --git a/crates/astria-conductor/src/executor/mod.rs b/crates/astria-conductor/src/executor/mod.rs index 8c3155ca8b..8bee05f991 100644 --- a/crates/astria-conductor/src/executor/mod.rs +++ b/crates/astria-conductor/src/executor/mod.rs @@ -67,6 +67,14 @@ pub(crate) struct StateNotInit; #[derive(Clone, Debug)] pub(crate) struct StateIsInit; +#[derive(Debug, thiserror::Error)] +pub(crate) enum StopHeightExceded { + #[error("firm height exceeded sequencer stop height")] + Celestia, + #[error("soft height met or exceeded sequencer stop height")] + Sequencer, +} + #[derive(Debug, thiserror::Error)] pub(crate) enum FirmSendError { #[error("executor was configured without firm commitments")] @@ -214,6 +222,14 @@ impl Handle { pub(crate) fn celestia_block_variance(&mut self) -> u64 { self.state.celestia_block_variance() } + + pub(crate) fn sequencer_chain_id(&self) -> String { + self.state.sequencer_chain_id() + } + + pub(crate) fn celestia_chain_id(&self) -> String { + self.state.celestia_chain_id() + } } pub(crate) struct Executor { @@ -396,6 +412,32 @@ impl Executor { // TODO(https://github.com/astriaorg/astria/issues/624): add retry logic before failing hard. let executable_block = ExecutableBlock::from_sequencer(block, self.state.rollup_id()); + // Stop executing soft blocks at the sequencer stop block height (exclusive). If we are also + // executing firm blocks, we let execution continue since one more firm block will be + // executed before `execute_firm` initiates a restart. If we are in soft-only mode, we + if executable_block.height >= self.state.sequencer_stop_block_height() { + let res = if self.mode.is_with_firm() { + info!( + height = %executable_block.height, + "received soft block whose height is greater than or equal to stop block height in {} mode. \ + dropping soft block and waiting for next firm block before attempting restart", + self.mode, + ); + Ok(()) + } else { + info!( + height = %executable_block.height, + "received soft block whose height is greater than or equal to stop block \ + height in soft only mode. shutting down and attempting restart", + ); + Err(StopHeightExceded::Sequencer).wrap_err( + "soft height met or exceeded sequencer stop height, attempting restart with \ + new genesis info", + ) + }; + return res; + } + let expected_height = self.state.next_expected_soft_sequencer_height(); match executable_block.height.cmp(&expected_height) { std::cmp::Ordering::Less => { @@ -413,7 +455,7 @@ impl Executor { std::cmp::Ordering::Equal => {} } - let genesis_height = self.state.sequencer_genesis_block_height(); + let genesis_height = self.state.sequencer_start_block_height(); let block_height = executable_block.height; let Some(block_number) = state::map_sequencer_height_to_rollup_height(genesis_height, block_height) @@ -457,6 +499,18 @@ impl Executor { err, ))] async fn execute_firm(&mut self, block: ReconstructedBlock) -> eyre::Result<()> { + if block.header.height() > self.state.sequencer_stop_block_height() { + info!( + height = %block.header.height(), + "received firm block whose height is greater than stop block height. \ + exiting and attempting restart with new genesis info", + ); + return Err(StopHeightExceded::Celestia).wrap_err( + "firm height exceeded sequencer stop height, attempting restart with new genesis \ + info", + ); + } + let celestia_height = block.celestia_height; let executable_block = ExecutableBlock::from_reconstructed(block); let expected_height = self.state.next_expected_firm_sequencer_height(); @@ -466,7 +520,7 @@ impl Executor { "expected block at sequencer height {expected_height}, but got {block_height}", ); - let genesis_height = self.state.sequencer_genesis_block_height(); + let genesis_height = self.state.sequencer_start_block_height(); let Some(block_number) = state::map_sequencer_height_to_rollup_height(genesis_height, block_height) else { diff --git a/crates/astria-conductor/src/executor/state.rs b/crates/astria-conductor/src/executor/state.rs index bfe794e6c1..1b48087864 100644 --- a/crates/astria-conductor/src/executor/state.rs +++ b/crates/astria-conductor/src/executor/state.rs @@ -98,10 +98,10 @@ pub(super) struct StateSender { } fn can_map_firm_to_sequencer_height( - genesis_info: GenesisInfo, + genesis_info: &GenesisInfo, commitment_state: &CommitmentState, ) -> Result<(), InvalidState> { - let sequencer_genesis_height = genesis_info.sequencer_genesis_block_height(); + let sequencer_genesis_height = genesis_info.sequencer_start_block_height(); let rollup_number = commitment_state.firm().number(); if map_rollup_number_to_sequencer_height(sequencer_genesis_height, rollup_number).is_none() { Err(InvalidState { @@ -115,10 +115,10 @@ fn can_map_firm_to_sequencer_height( } fn can_map_soft_to_sequencer_height( - genesis_info: GenesisInfo, + genesis_info: &GenesisInfo, commitment_state: &CommitmentState, ) -> Result<(), InvalidState> { - let sequencer_genesis_height = genesis_info.sequencer_genesis_block_height(); + let sequencer_genesis_height = genesis_info.sequencer_start_block_height(); let rollup_number = commitment_state.soft().number(); if map_rollup_number_to_sequencer_height(sequencer_genesis_height, rollup_number).is_none() { Err(InvalidState { @@ -137,8 +137,8 @@ impl StateSender { genesis_info: GenesisInfo, commitment_state: CommitmentState, ) -> Result<(), InvalidState> { - can_map_firm_to_sequencer_height(genesis_info, &commitment_state)?; - can_map_soft_to_sequencer_height(genesis_info, &commitment_state)?; + can_map_firm_to_sequencer_height(&genesis_info, &commitment_state)?; + can_map_soft_to_sequencer_height(&genesis_info, &commitment_state)?; self.inner.send_modify(move |state| { let old_state = state.replace(State::new(genesis_info, commitment_state)); assert!( @@ -154,8 +154,8 @@ impl StateSender { commitment_state: CommitmentState, ) -> Result<(), InvalidState> { let genesis_info = self.genesis_info(); - can_map_firm_to_sequencer_height(genesis_info, &commitment_state)?; - can_map_soft_to_sequencer_height(genesis_info, &commitment_state)?; + can_map_firm_to_sequencer_height(&genesis_info, &commitment_state)?; + can_map_soft_to_sequencer_height(&genesis_info, &commitment_state)?; self.inner.send_modify(move |state| { state .as_mut() @@ -222,8 +222,9 @@ forward_impls!( [soft_hash -> Bytes], [celestia_block_variance -> u64], [rollup_id -> RollupId], - [sequencer_genesis_block_height -> SequencerHeight], + [sequencer_start_block_height -> SequencerHeight], [celestia_base_block_height -> u64], + [sequencer_stop_block_height -> SequencerHeight], ); forward_impls!( @@ -231,6 +232,8 @@ forward_impls!( [celestia_base_block_height -> u64], [celestia_block_variance -> u64], [rollup_id -> RollupId], + [sequencer_chain_id -> String], + [celestia_chain_id -> String], ); /// `State` tracks the genesis info and commitment state of the remote rollup node. @@ -253,8 +256,8 @@ impl State { self.commitment_state = commitment_state; } - fn genesis_info(&self) -> GenesisInfo { - self.genesis_info + fn genesis_info(&self) -> &GenesisInfo { + &self.genesis_info } fn firm(&self) -> &Block { @@ -289,8 +292,20 @@ impl State { self.genesis_info.celestia_block_variance() } - fn sequencer_genesis_block_height(&self) -> SequencerHeight { - self.genesis_info.sequencer_genesis_block_height() + fn sequencer_start_block_height(&self) -> SequencerHeight { + self.genesis_info.sequencer_start_block_height() + } + + fn sequencer_stop_block_height(&self) -> SequencerHeight { + self.genesis_info.sequencer_stop_block_height() + } + + fn sequencer_chain_id(&self) -> String { + self.genesis_info.sequencer_chain_id().to_string() + } + + fn celestia_chain_id(&self) -> String { + self.genesis_info.celestia_chain_id().to_string() } fn rollup_id(&self) -> RollupId { @@ -299,14 +314,14 @@ impl State { fn next_expected_firm_sequencer_height(&self) -> Option { map_rollup_number_to_sequencer_height( - self.sequencer_genesis_block_height(), + self.sequencer_start_block_height(), self.firm_number().saturating_add(1), ) } fn next_expected_soft_sequencer_height(&self) -> Option { map_rollup_number_to_sequencer_height( - self.sequencer_genesis_block_height(), + self.sequencer_start_block_height(), self.soft_number().saturating_add(1), ) } @@ -384,8 +399,11 @@ mod tests { let rollup_id = RollupId::new([24; 32]); GenesisInfo::try_from_raw(raw::GenesisInfo { rollup_id: Some(rollup_id.to_raw()), - sequencer_genesis_block_height: 10, + sequencer_start_block_height: 10, + sequencer_stop_block_height: 100, celestia_block_variance: 0, + sequencer_chain_id: "test-sequencer-0".to_string(), + celestia_chain_id: "test-celestia-0".to_string(), }) .unwrap() } diff --git a/crates/astria-conductor/src/executor/tests.rs b/crates/astria-conductor/src/executor/tests.rs index 164d8b0a20..4f97a02980 100644 --- a/crates/astria-conductor/src/executor/tests.rs +++ b/crates/astria-conductor/src/executor/tests.rs @@ -47,8 +47,11 @@ fn make_state( ) -> (StateSender, StateReceiver) { let genesis_info = GenesisInfo::try_from_raw(raw::GenesisInfo { rollup_id: Some(ROLLUP_ID.to_raw()), - sequencer_genesis_block_height: 1, + sequencer_start_block_height: 1, + sequencer_stop_block_height: 100, celestia_block_variance: 1, + sequencer_chain_id: "test_sequencer-0".to_string(), + celestia_chain_id: "test_celestia-0".to_string(), }) .unwrap(); let commitment_state = CommitmentState::try_from_raw(raw::CommitmentState { diff --git a/crates/astria-conductor/src/sequencer/builder.rs b/crates/astria-conductor/src/sequencer/builder.rs index c71aa0e7d8..2cdaf4bf62 100644 --- a/crates/astria-conductor/src/sequencer/builder.rs +++ b/crates/astria-conductor/src/sequencer/builder.rs @@ -10,7 +10,6 @@ pub(crate) struct Builder { pub(crate) sequencer_grpc_client: SequencerGrpcClient, pub(crate) sequencer_cometbft_client: sequencer_client::HttpClient, pub(crate) sequencer_block_time: Duration, - pub(crate) expected_sequencer_chain_id: String, pub(crate) shutdown: CancellationToken, } @@ -21,7 +20,6 @@ impl Builder { sequencer_grpc_client, sequencer_cometbft_client, sequencer_block_time, - expected_sequencer_chain_id, shutdown, } = self; super::Reader { @@ -29,7 +27,6 @@ impl Builder { sequencer_grpc_client, sequencer_cometbft_client, sequencer_block_time, - expected_sequencer_chain_id, shutdown, } } diff --git a/crates/astria-conductor/src/sequencer/mod.rs b/crates/astria-conductor/src/sequencer/mod.rs index df9d11b5a1..1d399b84b5 100644 --- a/crates/astria-conductor/src/sequencer/mod.rs +++ b/crates/astria-conductor/src/sequencer/mod.rs @@ -78,9 +78,6 @@ pub(crate) struct Reader { /// height. sequencer_block_time: Duration, - /// The chain ID of the sequencer network the reader should be communicating with. - expected_sequencer_chain_id: String, - /// Token to listen for Conductor being shut down. shutdown: CancellationToken, } @@ -107,9 +104,14 @@ impl Reader { get_sequencer_chain_id(self.sequencer_cometbft_client.clone()) .await .wrap_err("failed to get chain ID from Sequencer")?; - let expected_sequencer_chain_id = &self.expected_sequencer_chain_id; + let expected_sequencer_chain_id = self + .executor + .wait_for_init() + .await + .wrap_err("handle to executor failed while waiting for it being initialized")? + .sequencer_chain_id(); ensure!( - self.expected_sequencer_chain_id == actual_sequencer_chain_id.as_str(), + expected_sequencer_chain_id == actual_sequencer_chain_id.as_str(), "expected chain id `{expected_sequencer_chain_id}` does not match actual: \ `{actual_sequencer_chain_id}`" ); diff --git a/crates/astria-conductor/tests/blackbox/firm_only.rs b/crates/astria-conductor/tests/blackbox/firm_only.rs index 2fb2c43148..e1e8f7ec45 100644 --- a/crates/astria-conductor/tests/blackbox/firm_only.rs +++ b/crates/astria-conductor/tests/blackbox/firm_only.rs @@ -54,7 +54,8 @@ async fn simple() { mount_get_genesis_info!( test_conductor, - sequencer_genesis_block_height: 1, + sequencer_start_block_height: 1, + sequencer_stop_block_height: 10, celestia_block_variance: 10, ); @@ -135,7 +136,8 @@ async fn submits_two_heights_in_succession() { mount_get_genesis_info!( test_conductor, - sequencer_genesis_block_height: 1, + sequencer_start_block_height: 1, + sequencer_stop_block_height: 10, celestia_block_variance: 10, ); @@ -236,7 +238,7 @@ async fn submits_two_heights_in_succession() { ) .await .expect( - "conductor should have executed the soft block and updated the soft commitment state \ + "conductor should have executed the firm block and updated the firm commitment state \ within 2000ms", ); } @@ -247,7 +249,8 @@ async fn skips_already_executed_heights() { mount_get_genesis_info!( test_conductor, - sequencer_genesis_block_height: 1, + sequencer_start_block_height: 1, + sequencer_stop_block_height: 10, celestia_block_variance: 10, ); @@ -320,7 +323,7 @@ async fn skips_already_executed_heights() { ) .await .expect( - "conductor should have executed the soft block and updated the soft commitment state \ + "conductor should have executed the firm block and updated the firm commitment state \ within 1000ms", ); } @@ -331,7 +334,8 @@ async fn fetch_from_later_celestia_height() { mount_get_genesis_info!( test_conductor, - sequencer_genesis_block_height: 1, + sequencer_start_block_height: 1, + sequencer_stop_block_height: 10, celestia_block_variance: 10, ); @@ -447,7 +451,8 @@ async fn exits_on_celestia_chain_id_mismatch() { matcher::message_type::(), ) .respond_with(GrpcResponse::constant_response( - genesis_info!(sequencer_genesis_block_height: 1, + genesis_info!(sequencer_start_block_height: 1, + sequencer_stop_block_height: 10, celestia_block_variance: 10,), )) .expect(0..) @@ -513,3 +518,126 @@ async fn exits_on_celestia_chain_id_mismatch() { } } } + +#[expect(clippy::too_many_lines, reason = "All lines reasonably necessary")] +#[tokio::test(flavor = "multi_thread", worker_threads = 1)] +async fn conductor_restarts_after_reaching_stop_block_height() { + let test_conductor = spawn_conductor(CommitLevel::FirmOnly).await; + + mount_get_genesis_info!( + test_conductor, + sequencer_start_block_height: 1, + sequencer_stop_block_height: 3, + celestia_block_variance: 10, + ); + + mount_get_commitment_state!( + test_conductor, + firm: ( + number: 1, + hash: [1; 64], + parent: [0; 64], + ), + soft: ( + number: 1, + hash: [1; 64], + parent: [0; 64], + ), + base_celestia_height: 1, + ); + + mount_sequencer_genesis!(test_conductor); + + mount_celestia_header_network_head!( + test_conductor, + height: 2u32, + ); + + mount_celestia_blobs!( + test_conductor, + celestia_height: 1, + sequencer_heights: [3, 4], + ); + + mount_sequencer_commit!( + test_conductor, + height: 3u32, + ); + + mount_sequencer_validator_set!(test_conductor, height: 2u32); + + // Mount height above stop block height so that the executor will initiate a restart + mount_sequencer_commit!( + test_conductor, + height: 4u32, + ); + + mount_sequencer_validator_set!(test_conductor, height: 3u32); + + let execute_block = mount_executed_block!( + test_conductor, + mock_name: "execute_block_twice", + number: 2, + hash: [2; 64], + parent: [1; 64], + expected_calls: 2, // Expected to be called again after restart + ); + + let update_commitment_state = mount_update_commitment_state!( + test_conductor, + mock_name: "update_commitment_state_twice", + firm: ( + number: 2, + hash: [2; 64], + parent: [1; 64], + ), + soft: ( + number: 2, + hash: [2; 64], + parent: [1; 64], + ), + base_celestia_height: 1, + expected_calls: 2, // Expected to be called again after restart + ); + + // Will be validated for no calls upon dropping + let _no_execute_block = mount_executed_block!( + test_conductor, + mock_name: "should_not_execute_this_block", + number: 3, + hash: [3; 64], + parent: [2; 64], + expected_calls: 0, + ); + + // Will be validated for no calls upon dropping + let _no_update_commitment_state = mount_update_commitment_state!( + test_conductor, + mock_name: "should_not_update_this_commitment_state", + firm: ( + number: 3, + hash: [3; 64], + parent: [2; 64], + ), + soft: ( + number: 3, + hash: [3; 64], + parent: [2; 64], + ), + base_celestia_height: 1, + expected_calls: 0, + ); + + timeout( + Duration::from_millis(1000), + join( + execute_block.wait_until_satisfied(), + update_commitment_state.wait_until_satisfied(), + ), + ) + .await + .expect( + "conductor should have executed the firm block and updated the firm commitment state \ + twice within 1000ms", + ); +} diff --git a/crates/astria-conductor/tests/blackbox/helpers/macros.rs b/crates/astria-conductor/tests/blackbox/helpers/macros.rs index 4119fe1bf6..4bfd3b7d79 100644 --- a/crates/astria-conductor/tests/blackbox/helpers/macros.rs +++ b/crates/astria-conductor/tests/blackbox/helpers/macros.rs @@ -94,14 +94,18 @@ macro_rules! filtered_sequencer_block { #[macro_export] macro_rules! genesis_info { ( - sequencer_genesis_block_height: - $sequencer_height:expr,celestia_block_variance: + sequencer_start_block_height: + $start_height:expr,sequencer_stop_block_height: + $stop_height:expr,celestia_block_variance: $variance:expr $(,)? ) => { ::astria_core::generated::execution::v1::GenesisInfo { rollup_id: Some($crate::ROLLUP_ID.to_raw()), - sequencer_genesis_block_height: $sequencer_height, + sequencer_start_block_height: $start_height, + sequencer_stop_block_height: $stop_height, celestia_block_variance: $variance, + sequencer_chain_id: $crate::SEQUENCER_CHAIN_ID.to_string(), + celestia_chain_id: $crate::helpers::CELESTIA_CHAIN_ID.to_string(), } }; } @@ -274,7 +278,8 @@ macro_rules! mount_executed_block { mock_name: $mock_name:expr, number: $number:expr, hash: $hash:expr, - parent: $parent:expr $(,)? + parent: $parent:expr, + expected_calls: $expected_calls:expr $(,)? ) => {{ use ::base64::prelude::*; $test_env.mount_execute_block( @@ -287,10 +292,27 @@ macro_rules! mount_executed_block { number: $number, hash: $hash, parent: $parent, - ) + ), + $expected_calls, ) .await }}; + ( + $test_env:ident, + mock_name: $mock_name:expr, + number: $number:expr, + hash: $hash:expr, + parent: $parent:expr, + ) => { + mount_executed_block!( + $test_env, + mock_name: None, + number: $number, + hash: $hash, + parent: $parent, + expected_calls: 1, + ) + }; ( $test_env:ident, number: $number:expr, @@ -334,13 +356,15 @@ macro_rules! mount_get_filtered_sequencer_block { macro_rules! mount_get_genesis_info { ( $test_env:ident, - sequencer_genesis_block_height: $sequencer_height:expr, + sequencer_start_block_height: $start_height:expr, + sequencer_stop_block_height: $stop_height:expr, celestia_block_variance: $variance:expr $(,)? ) => { $test_env.mount_get_genesis_info( $crate::genesis_info!( - sequencer_genesis_block_height: $sequencer_height, + sequencer_start_block_height: $start_height, + sequencer_stop_block_height: $stop_height, celestia_block_variance: $variance, ) ).await; diff --git a/crates/astria-conductor/tests/blackbox/helpers/mod.rs b/crates/astria-conductor/tests/blackbox/helpers/mod.rs index da7dad5061..ee2711c262 100644 --- a/crates/astria-conductor/tests/blackbox/helpers/mod.rs +++ b/crates/astria-conductor/tests/blackbox/helpers/mod.rs @@ -337,6 +337,7 @@ impl TestConductor { mock_name: Option<&str>, expected_pbjson: S, response: Block, + expected_calls: u64, ) -> astria_grpc_mock::MockGuard { use astria_grpc_mock::{ matcher::message_partial_pbjson, @@ -349,7 +350,7 @@ impl TestConductor { if let Some(name) = mock_name { mock = mock.with_name(name); } - mock.expect(1) + mock.expect(expected_calls) .mount_as_scoped(&self.mock_grpc.mock_server) .await } @@ -522,8 +523,6 @@ pub(crate) fn make_config() -> Config { sequencer_cometbft_url: "http://127.0.0.1:26657".into(), sequencer_requests_per_second: 500, sequencer_block_time_ms: 2000, - expected_celestia_chain_id: CELESTIA_CHAIN_ID.into(), - expected_sequencer_chain_id: SEQUENCER_CHAIN_ID.into(), execution_rpc_url: "http://127.0.0.1:50051".into(), log: "info".into(), execution_commit_level: astria_conductor::config::CommitLevel::SoftAndFirm, diff --git a/crates/astria-conductor/tests/blackbox/soft_and_firm.rs b/crates/astria-conductor/tests/blackbox/soft_and_firm.rs index 1054c43f80..b87cf98a7a 100644 --- a/crates/astria-conductor/tests/blackbox/soft_and_firm.rs +++ b/crates/astria-conductor/tests/blackbox/soft_and_firm.rs @@ -40,7 +40,8 @@ async fn executes_soft_first_then_updates_firm() { mount_get_genesis_info!( test_conductor, - sequencer_genesis_block_height: 1, + sequencer_start_block_height: 1, + sequencer_stop_block_height: 10, celestia_block_variance: 10, ); @@ -174,7 +175,8 @@ async fn executes_firm_then_soft_at_next_height() { mount_get_genesis_info!( test_conductor, - sequencer_genesis_block_height: 1, + sequencer_start_block_height: 1, + sequencer_stop_block_height: 10, celestia_block_variance: 10, ); @@ -333,7 +335,8 @@ async fn missing_block_is_fetched_for_updating_firm_commitment() { mount_get_genesis_info!( test_conductor, - sequencer_genesis_block_height: 1, + sequencer_start_block_height: 1, + sequencer_stop_block_height: 10, celestia_block_variance: 10, ); @@ -462,7 +465,8 @@ async fn conductor_restarts_on_permission_denied() { mount_get_genesis_info!( test_conductor, - sequencer_genesis_block_height: 1, + sequencer_start_block_height: 1, + sequencer_stop_block_height: 10, celestia_block_variance: 10, ); diff --git a/crates/astria-conductor/tests/blackbox/soft_only.rs b/crates/astria-conductor/tests/blackbox/soft_only.rs index e82793c638..51477b5d0f 100644 --- a/crates/astria-conductor/tests/blackbox/soft_only.rs +++ b/crates/astria-conductor/tests/blackbox/soft_only.rs @@ -41,7 +41,8 @@ async fn simple() { mount_get_genesis_info!( test_conductor, - sequencer_genesis_block_height: 1, + sequencer_start_block_height: 1, + sequencer_stop_block_height: 10, celestia_block_variance: 10, ); @@ -114,7 +115,8 @@ async fn submits_two_heights_in_succession() { mount_get_genesis_info!( test_conductor, - sequencer_genesis_block_height: 1, + sequencer_start_block_height: 1, + sequencer_stop_block_height: 10, celestia_block_variance: 10, ); @@ -220,7 +222,8 @@ async fn skips_already_executed_heights() { mount_get_genesis_info!( test_conductor, - sequencer_genesis_block_height: 1, + sequencer_start_block_height: 1, + sequencer_stop_block_height: 10, celestia_block_variance: 10, ); @@ -293,7 +296,8 @@ async fn requests_from_later_genesis_height() { mount_get_genesis_info!( test_conductor, - sequencer_genesis_block_height: 10, + sequencer_start_block_height: 10, + sequencer_stop_block_height: 20, celestia_block_variance: 10, ); @@ -401,7 +405,8 @@ async fn exits_on_sequencer_chain_id_mismatch() { matcher::message_type::(), ) .respond_with(GrpcResponse::constant_response( - genesis_info!(sequencer_genesis_block_height: 1, + genesis_info!(sequencer_start_block_height: 1, + sequencer_stop_block_height: 10, celestia_block_variance: 10,), )) .expect(0..) @@ -452,3 +457,115 @@ async fn exits_on_sequencer_chain_id_mismatch() { } } } + +#[tokio::test(flavor = "multi_thread", worker_threads = 1)] +async fn conductor_restarts_after_reaching_stop_block_height() { + let test_conductor = spawn_conductor(CommitLevel::SoftOnly).await; + + mount_get_genesis_info!( + test_conductor, + sequencer_start_block_height: 1, + sequencer_stop_block_height: 4, + celestia_block_variance: 10, + ); + + mount_get_commitment_state!( + test_conductor, + firm: ( + number: 1, + hash: [1; 64], + parent: [0; 64], + ), + soft: ( + number: 1, + hash: [1; 64], + parent: [0; 64], + ), + base_celestia_height: 1, + ); + + mount_sequencer_genesis!(test_conductor); + + mount_abci_info!( + test_conductor, + latest_sequencer_height: 4, + ); + + mount_get_filtered_sequencer_block!( + test_conductor, + sequencer_height: 3, + ); + + // Mount a block at the stop block height so the executor will initiate a restart + mount_get_filtered_sequencer_block!( + test_conductor, + sequencer_height: 4, + ); + + let execute_block = mount_executed_block!( + test_conductor, + mock_name: "execute_block_twice", + number: 2, + hash: [2; 64], + parent: [1; 64], + expected_calls: 2, // Expected to be called again after restart + ); + + let update_commitment_state = mount_update_commitment_state!( + test_conductor, + mock_name: "update_commitment_state_twice", + firm: ( + number: 1, + hash: [1; 64], + parent: [0; 64], + ), + soft: ( + number: 2, + hash: [2; 64], + parent: [1; 64], + ), + base_celestia_height: 1, + expected_calls: 2, // Expected to be called again after restart + ); + + // Will be validated for no calls upon dropping + let _no_execute_block = mount_executed_block!( + test_conductor, + mock_name: "should_not_execute_this_block", + number: 3, + hash: [3; 64], + parent: [2; 64], + expected_calls: 0, + ); + + // Will be validated for no calls upon dropping + let _no_update_commitment_state = mount_update_commitment_state!( + test_conductor, + mock_name: "should_not_update_this_commitment_state", + firm: ( + number: 1, + hash: [1; 64], + parent: [0; 64], + ), + soft: ( + number: 3, + hash: [3; 64], + parent: [2; 64], + ), + base_celestia_height: 1, + expected_calls: 0, + ); + + timeout( + Duration::from_millis(1000), + join( + execute_block.wait_until_satisfied(), + update_commitment_state.wait_until_satisfied(), + ), + ) + .await + .expect( + "conductor should have executed the soft block and updated the soft commitment state \ + within 1000ms", + ); +} diff --git a/crates/astria-core/src/execution/v1/mod.rs b/crates/astria-core/src/execution/v1/mod.rs index 6daca96a42..9359de2891 100644 --- a/crates/astria-core/src/execution/v1/mod.rs +++ b/crates/astria-core/src/execution/v1/mod.rs @@ -39,7 +39,7 @@ enum GenesisInfoErrorKind { /// /// Usually constructed its [`Protobuf`] implementation from a /// [`raw::GenesisInfo`]. -#[derive(Clone, Copy, Debug)] +#[derive(Clone, Debug)] #[cfg_attr(feature = "serde", derive(serde::Serialize))] #[cfg_attr( feature = "serde", @@ -49,9 +49,15 @@ pub struct GenesisInfo { /// The rollup id which is used to identify the rollup txs. rollup_id: RollupId, /// The Sequencer block height which contains the first block of the rollup. - sequencer_genesis_block_height: tendermint::block::Height, + sequencer_start_block_height: tendermint::block::Height, + /// The Sequencer block height to stop at. + sequencer_stop_block_height: tendermint::block::Height, /// The allowed variance in the block height of celestia when looking for sequencer blocks. celestia_block_variance: u64, + /// The chain ID of the sequencer network. + sequencer_chain_id: String, + /// The chain ID of the celestia network. + celestia_chain_id: String, } impl GenesisInfo { @@ -61,14 +67,29 @@ impl GenesisInfo { } #[must_use] - pub fn sequencer_genesis_block_height(&self) -> tendermint::block::Height { - self.sequencer_genesis_block_height + pub fn sequencer_start_block_height(&self) -> tendermint::block::Height { + self.sequencer_start_block_height + } + + #[must_use] + pub fn sequencer_stop_block_height(&self) -> tendermint::block::Height { + self.sequencer_stop_block_height } #[must_use] pub fn celestia_block_variance(&self) -> u64 { self.celestia_block_variance } + + #[must_use] + pub fn sequencer_chain_id(&self) -> &str { + &self.sequencer_chain_id + } + + #[must_use] + pub fn celestia_chain_id(&self) -> &str { + &self.celestia_chain_id + } } impl From for raw::GenesisInfo { @@ -84,8 +105,11 @@ impl Protobuf for GenesisInfo { fn try_from_raw_ref(raw: &Self::Raw) -> Result { let raw::GenesisInfo { rollup_id, - sequencer_genesis_block_height, + sequencer_start_block_height, + sequencer_stop_block_height, celestia_block_variance, + sequencer_chain_id, + celestia_chain_id, } = raw; let Some(rollup_id) = rollup_id else { return Err(Self::Error::no_rollup_id()); @@ -95,27 +119,42 @@ impl Protobuf for GenesisInfo { Ok(Self { rollup_id, - sequencer_genesis_block_height: (*sequencer_genesis_block_height).into(), + sequencer_start_block_height: (*sequencer_start_block_height).into(), + sequencer_stop_block_height: (*sequencer_stop_block_height).into(), celestia_block_variance: *celestia_block_variance, + sequencer_chain_id: sequencer_chain_id.clone(), + celestia_chain_id: celestia_chain_id.clone(), }) } fn to_raw(&self) -> Self::Raw { let Self { rollup_id, - sequencer_genesis_block_height, + sequencer_start_block_height, + sequencer_stop_block_height, celestia_block_variance, + sequencer_chain_id, + celestia_chain_id, } = self; - let sequencer_genesis_block_height: u32 = - (*sequencer_genesis_block_height).value().try_into().expect( + let sequencer_start_block_height: u32 = + (*sequencer_start_block_height).value().try_into().expect( + "block height overflow, this should not happen since tendermint heights are i64 \ + under the hood", + ); + let sequencer_stop_block_height: u32 = + (*sequencer_stop_block_height).value().try_into().expect( "block height overflow, this should not happen since tendermint heights are i64 \ under the hood", ); + Self::Raw { rollup_id: Some(rollup_id.to_raw()), - sequencer_genesis_block_height, + sequencer_start_block_height, + sequencer_stop_block_height, celestia_block_variance: *celestia_block_variance, + sequencer_chain_id: sequencer_chain_id.clone(), + celestia_chain_id: celestia_chain_id.clone(), } } } diff --git a/crates/astria-core/src/generated/astria.execution.v1.rs b/crates/astria-core/src/generated/astria.execution.v1.rs index 35b5d20480..c4e95364fd 100644 --- a/crates/astria-core/src/generated/astria.execution.v1.rs +++ b/crates/astria-core/src/generated/astria.execution.v1.rs @@ -10,10 +10,17 @@ pub struct GenesisInfo { pub rollup_id: ::core::option::Option, /// The first block height of sequencer chain to use for rollup transactions. #[prost(uint32, tag = "2")] - pub sequencer_genesis_block_height: u32, + pub sequencer_start_block_height: u32, + /// The last block height of sequencer chain to use for rollup transactions. + #[prost(uint32, tag = "3")] + pub sequencer_stop_block_height: u32, /// The allowed variance in celestia for sequencer blocks to have been posted. #[prost(uint64, tag = "4")] pub celestia_block_variance: u64, + #[prost(string, tag = "5")] + pub sequencer_chain_id: ::prost::alloc::string::String, + #[prost(string, tag = "6")] + pub celestia_chain_id: ::prost::alloc::string::String, } impl ::prost::Name for GenesisInfo { const NAME: &'static str = "GenesisInfo"; diff --git a/crates/astria-core/src/generated/astria.execution.v1.serde.rs b/crates/astria-core/src/generated/astria.execution.v1.serde.rs index 970146cc46..31d9b2aef6 100644 --- a/crates/astria-core/src/generated/astria.execution.v1.serde.rs +++ b/crates/astria-core/src/generated/astria.execution.v1.serde.rs @@ -710,23 +710,41 @@ impl serde::Serialize for GenesisInfo { if self.rollup_id.is_some() { len += 1; } - if self.sequencer_genesis_block_height != 0 { + if self.sequencer_start_block_height != 0 { + len += 1; + } + if self.sequencer_stop_block_height != 0 { len += 1; } if self.celestia_block_variance != 0 { len += 1; } + if !self.sequencer_chain_id.is_empty() { + len += 1; + } + if !self.celestia_chain_id.is_empty() { + len += 1; + } let mut struct_ser = serializer.serialize_struct("astria.execution.v1.GenesisInfo", len)?; if let Some(v) = self.rollup_id.as_ref() { struct_ser.serialize_field("rollupId", v)?; } - if self.sequencer_genesis_block_height != 0 { - struct_ser.serialize_field("sequencerGenesisBlockHeight", &self.sequencer_genesis_block_height)?; + if self.sequencer_start_block_height != 0 { + struct_ser.serialize_field("sequencerStartBlockHeight", &self.sequencer_start_block_height)?; + } + if self.sequencer_stop_block_height != 0 { + struct_ser.serialize_field("sequencerStopBlockHeight", &self.sequencer_stop_block_height)?; } if self.celestia_block_variance != 0 { #[allow(clippy::needless_borrow)] struct_ser.serialize_field("celestiaBlockVariance", ToString::to_string(&self.celestia_block_variance).as_str())?; } + if !self.sequencer_chain_id.is_empty() { + struct_ser.serialize_field("sequencerChainId", &self.sequencer_chain_id)?; + } + if !self.celestia_chain_id.is_empty() { + struct_ser.serialize_field("celestiaChainId", &self.celestia_chain_id)?; + } struct_ser.end() } } @@ -739,17 +757,26 @@ impl<'de> serde::Deserialize<'de> for GenesisInfo { const FIELDS: &[&str] = &[ "rollup_id", "rollupId", - "sequencer_genesis_block_height", - "sequencerGenesisBlockHeight", + "sequencer_start_block_height", + "sequencerStartBlockHeight", + "sequencer_stop_block_height", + "sequencerStopBlockHeight", "celestia_block_variance", "celestiaBlockVariance", + "sequencer_chain_id", + "sequencerChainId", + "celestia_chain_id", + "celestiaChainId", ]; #[allow(clippy::enum_variant_names)] enum GeneratedField { RollupId, - SequencerGenesisBlockHeight, + SequencerStartBlockHeight, + SequencerStopBlockHeight, CelestiaBlockVariance, + SequencerChainId, + CelestiaChainId, } impl<'de> serde::Deserialize<'de> for GeneratedField { fn deserialize(deserializer: D) -> std::result::Result @@ -772,8 +799,11 @@ impl<'de> serde::Deserialize<'de> for GenesisInfo { { match value { "rollupId" | "rollup_id" => Ok(GeneratedField::RollupId), - "sequencerGenesisBlockHeight" | "sequencer_genesis_block_height" => Ok(GeneratedField::SequencerGenesisBlockHeight), + "sequencerStartBlockHeight" | "sequencer_start_block_height" => Ok(GeneratedField::SequencerStartBlockHeight), + "sequencerStopBlockHeight" | "sequencer_stop_block_height" => Ok(GeneratedField::SequencerStopBlockHeight), "celestiaBlockVariance" | "celestia_block_variance" => Ok(GeneratedField::CelestiaBlockVariance), + "sequencerChainId" | "sequencer_chain_id" => Ok(GeneratedField::SequencerChainId), + "celestiaChainId" | "celestia_chain_id" => Ok(GeneratedField::CelestiaChainId), _ => Err(serde::de::Error::unknown_field(value, FIELDS)), } } @@ -794,8 +824,11 @@ impl<'de> serde::Deserialize<'de> for GenesisInfo { V: serde::de::MapAccess<'de>, { let mut rollup_id__ = None; - let mut sequencer_genesis_block_height__ = None; + let mut sequencer_start_block_height__ = None; + let mut sequencer_stop_block_height__ = None; let mut celestia_block_variance__ = None; + let mut sequencer_chain_id__ = None; + let mut celestia_chain_id__ = None; while let Some(k) = map_.next_key()? { match k { GeneratedField::RollupId => { @@ -804,11 +837,19 @@ impl<'de> serde::Deserialize<'de> for GenesisInfo { } rollup_id__ = map_.next_value()?; } - GeneratedField::SequencerGenesisBlockHeight => { - if sequencer_genesis_block_height__.is_some() { - return Err(serde::de::Error::duplicate_field("sequencerGenesisBlockHeight")); + GeneratedField::SequencerStartBlockHeight => { + if sequencer_start_block_height__.is_some() { + return Err(serde::de::Error::duplicate_field("sequencerStartBlockHeight")); } - sequencer_genesis_block_height__ = + sequencer_start_block_height__ = + Some(map_.next_value::<::pbjson::private::NumberDeserialize<_>>()?.0) + ; + } + GeneratedField::SequencerStopBlockHeight => { + if sequencer_stop_block_height__.is_some() { + return Err(serde::de::Error::duplicate_field("sequencerStopBlockHeight")); + } + sequencer_stop_block_height__ = Some(map_.next_value::<::pbjson::private::NumberDeserialize<_>>()?.0) ; } @@ -820,12 +861,27 @@ impl<'de> serde::Deserialize<'de> for GenesisInfo { Some(map_.next_value::<::pbjson::private::NumberDeserialize<_>>()?.0) ; } + GeneratedField::SequencerChainId => { + if sequencer_chain_id__.is_some() { + return Err(serde::de::Error::duplicate_field("sequencerChainId")); + } + sequencer_chain_id__ = Some(map_.next_value()?); + } + GeneratedField::CelestiaChainId => { + if celestia_chain_id__.is_some() { + return Err(serde::de::Error::duplicate_field("celestiaChainId")); + } + celestia_chain_id__ = Some(map_.next_value()?); + } } } Ok(GenesisInfo { rollup_id: rollup_id__, - sequencer_genesis_block_height: sequencer_genesis_block_height__.unwrap_or_default(), + sequencer_start_block_height: sequencer_start_block_height__.unwrap_or_default(), + sequencer_stop_block_height: sequencer_stop_block_height__.unwrap_or_default(), celestia_block_variance: celestia_block_variance__.unwrap_or_default(), + sequencer_chain_id: sequencer_chain_id__.unwrap_or_default(), + celestia_chain_id: celestia_chain_id__.unwrap_or_default(), }) } } diff --git a/proto/executionapis/astria/execution/v1/execution.proto b/proto/executionapis/astria/execution/v1/execution.proto index e68083db4d..068a4b7f6d 100644 --- a/proto/executionapis/astria/execution/v1/execution.proto +++ b/proto/executionapis/astria/execution/v1/execution.proto @@ -14,9 +14,13 @@ message GenesisInfo { // The rollup_id is the unique identifier for the rollup chain. astria.primitive.v1.RollupId rollup_id = 1; // The first block height of sequencer chain to use for rollup transactions. - uint32 sequencer_genesis_block_height = 2; + uint32 sequencer_start_block_height = 2; + // The last block height of sequencer chain to use for rollup transactions. + uint32 sequencer_stop_block_height = 3; // The allowed variance in celestia for sequencer blocks to have been posted. uint64 celestia_block_variance = 4; + string sequencer_chain_id = 5; + string celestia_chain_id = 6; } // The set of information which deterministic driver of block production From fdbe302830ff5e7471a94bcd81d9f8205f5bc9e8 Mon Sep 17 00:00:00 2001 From: ethanoroshiba Date: Tue, 3 Dec 2024 13:01:21 -0600 Subject: [PATCH 02/27] add soft and firm test, update change log and valus --- charts/evm-stack/values.yaml | 3 - crates/astria-conductor/CHANGELOG.md | 2 + .../tests/blackbox/soft_and_firm.rs | 185 ++++++++++++++++++ 3 files changed, 187 insertions(+), 3 deletions(-) diff --git a/charts/evm-stack/values.yaml b/charts/evm-stack/values.yaml index e18df9b10c..6729a570dd 100644 --- a/charts/evm-stack/values.yaml +++ b/charts/evm-stack/values.yaml @@ -13,7 +13,6 @@ global: rollupName: "" evmChainId: "" sequencerChainId: "" - celestiaChainId: "" otel: endpoint: "" tracesEndpoint: "" @@ -29,8 +28,6 @@ evm-rollup: chainId: "{{ .Values.global.evmChainId }}" config: conductor: - sequencerChainId: "{{ .Values.global.sequencerChainId }}" - celestiaChainId: "{{ .Values.global.celestiaChainId }}" sequencerRpc: "{{ .Values.global.sequencerRpc }}" sequencerGrpc: "{{ .Values.global.sequencerGrpc }}" otel: diff --git a/crates/astria-conductor/CHANGELOG.md b/crates/astria-conductor/CHANGELOG.md index eb75e304d5..c9076dca32 100644 --- a/crates/astria-conductor/CHANGELOG.md +++ b/crates/astria-conductor/CHANGELOG.md @@ -13,6 +13,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- Add stop height logic, remove chain id env vars, accomodate new genesis info +shape [#1843](https://github.com/astriaorg/astria/pull/1843). - Bump penumbra dependencies [#1740](https://github.com/astriaorg/astria/pull/1740). ## [1.0.0-rc.2] - 2024-10-23 diff --git a/crates/astria-conductor/tests/blackbox/soft_and_firm.rs b/crates/astria-conductor/tests/blackbox/soft_and_firm.rs index b87cf98a7a..dc518c755d 100644 --- a/crates/astria-conductor/tests/blackbox/soft_and_firm.rs +++ b/crates/astria-conductor/tests/blackbox/soft_and_firm.rs @@ -582,3 +582,188 @@ async fn conductor_restarts_on_permission_denied() { within 1000ms", ); } + +/// Tests if the conductor correctly stops and procedes to restart after reaching the sequencer stop +/// height (from genesis info provided by rollup). In `SoftAndFirm` mode executor should not call +/// `execute_block` or `update_commitment_state` for any soft blocks at or above the stop height. +/// The conductor DOES call these on the firm block at the stop height, then proceeds to restart. +/// +/// This test consists of the following steps: +/// 1. Mount genesis info with a sequencer stop height of 3. +/// 2. Mount commitment state. +/// 3. Mount ABCI info and sequencer (soft blocks) for heights 3 and 4. +/// 4. Mount Celestia network head and sequencer genesis. +/// 5. Create a mount for `execute_block` at height 3, which should be called twice: once for the +/// firm block prior to restart, and once for the firm block after restart. +/// 6. Create `update_commitment_state` mounts for soft blocks at heights 3 and 4, neither of which +/// should be called (will be validated upon dropping). +/// 7. Mount firm blocks at heights 3 and 4, with corresponding `update_commitment_state` mounts. +/// The latter should not be executed, since it is above the stop height. +/// 8. Validate that `execute_block` and `update_commitment_state` for the firm block at height 3 +/// exactly twice. +/// 9. Validate that none of the other mounts were called. +#[expect( + clippy::too_many_lines, + reason = "All lines reasonably necessary for the thoroughness of this test" +)] +#[tokio::test(flavor = "multi_thread", worker_threads = 1)] +async fn conductor_restarts_after_reaching_stop_height() { + let test_conductor = spawn_conductor(CommitLevel::SoftAndFirm).await; + + mount_get_genesis_info!( + test_conductor, + sequencer_start_block_height: 1, + sequencer_stop_block_height: 3, + celestia_block_variance: 10, + ); + + // Regular test mounts + mount_get_commitment_state!( + test_conductor, + firm: ( + number: 1, + hash: [1; 64], + parent: [0; 64], + ), + soft: ( + number: 1, + hash: [1; 64], + parent: [0; 64], + ), + base_celestia_height: 1, + ); + mount_sequencer_genesis!(test_conductor); + mount_celestia_header_network_head!( + test_conductor, + height: 1u32, + ); + + // Note that the latest height is 4, even though the stop height is 3. We want to give the + // conductor the opportunity to err if it isn't working correctly. + mount_abci_info!( + test_conductor, + latest_sequencer_height: 4, + ); + + // Neither of the following soft blocks should be executed. The first is at the stop height, the + // next just above to ensure blocks after the stop height are also not executed. + mount_get_filtered_sequencer_block!( + test_conductor, + sequencer_height: 3, + ); + mount_get_filtered_sequencer_block!( + test_conductor, + sequencer_height: 4, + ); + + let execute_block = mount_executed_block!( + test_conductor, + mock_name: "execute_block", + number: 2, + hash: [2; 64], + parent: [1; 64], + expected_calls: 2, // We expect this to be called twice, both times by executing the firm block, not the soft block + ); + + // This should not be called, since it is at the sequencer stop height + let _update_commitment_state_soft_1 = mount_update_commitment_state!( + test_conductor, + mock_name: "update_commitment_state_soft_1", + firm: ( + number: 1, + hash: [1; 64], + parent: [0; 64], + ), + soft: ( + number: 2, + hash: [2; 64], + parent: [1; 64], + ), + base_celestia_height: 1, + expected_calls: 0, + ); + + // This is the update commitment state for the next soft block, which should not be called + // either + let _update_commitment_state_soft_2 = mount_update_commitment_state!( + test_conductor, + mock_name: "update_commitment_state_soft_2", + firm: ( + number: 2, + hash: [2; 64], + parent: [1; 64], + ), + soft: ( + number: 3, + hash: [3; 64], + parent: [2; 64], + ), + base_celestia_height: 1, + expected_calls: 0, + ); + + mount_celestia_blobs!( + test_conductor, + celestia_height: 1, + sequencer_heights: [3, 4], + ); + + // Mount the firm block at the stop height, which should be executed + mount_sequencer_commit!( + test_conductor, + height: 3u32, + ); + mount_sequencer_validator_set!(test_conductor, height: 2u32); + + // Mount the next height as well, which should not be executed + mount_sequencer_commit!( + test_conductor, + height: 4u32, + ); + mount_sequencer_validator_set!(test_conductor, height: 3u32); + + let update_commitment_state_firm_1 = mount_update_commitment_state!( + test_conductor, + mock_name: "update_commitment_state_firm_1", + firm: ( + number: 2, + hash: [2; 64], + parent: [1; 64], + ), + soft: ( + number: 2, + hash: [2; 64], + parent: [1; 64], + ), + base_celestia_height: 1, + expected_calls: 2, // Expected to be called again after restart + ); + + // Should not be called, since it is above the stop height + let _update_commitment_state_firm_2 = mount_update_commitment_state!( + test_conductor, + mock_name: "update_commitment_state_firm_2", + firm: ( + number: 3, + hash: [3; 64], + parent: [2; 64], + ), + soft: ( + number: 3, + hash: [3; 64], + parent: [2; 64], + ), + base_celestia_height: 1, + expected_calls: 0, + ); + + timeout( + Duration::from_millis(1000), + join( + execute_block.wait_until_satisfied(), + update_commitment_state_firm_1.wait_until_satisfied(), + ), + ) + .await + .expect("conductor should have updated the firm commitment state within 1000ms"); +} From 30bd91534e4c0bdc4cfdc8bd506c4b944e7543c0 Mon Sep 17 00:00:00 2001 From: ethanoroshiba Date: Tue, 3 Dec 2024 13:24:29 -0600 Subject: [PATCH 03/27] version updates --- Cargo.lock | 2 +- charts/evm-rollup/Chart.yaml | 4 ++-- charts/evm-rollup/values.yaml | 2 +- charts/evm-stack/Chart.yaml | 4 ++-- crates/astria-conductor/Cargo.toml | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4e443b7b9c..ecc9200d97 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -637,7 +637,7 @@ dependencies = [ [[package]] name = "astria-conductor" -version = "1.0.0" +version = "1.0.1" dependencies = [ "astria-build-info", "astria-config", diff --git a/charts/evm-rollup/Chart.yaml b/charts/evm-rollup/Chart.yaml index c55081c828..7ca3fcb1bd 100644 --- a/charts/evm-rollup/Chart.yaml +++ b/charts/evm-rollup/Chart.yaml @@ -15,13 +15,13 @@ type: application # This is the chart version. This version number should be incremented each time you make changes # to the chart and its templates, including the app version. # Versions are expected to follow Semantic Versioning (https://semver.org/) -version: 1.0.0 +version: 1.0.1 # This is the version number of the application being deployed. This version number should be # incremented each time you make changes to the application. Versions are not expected to # follow Semantic Versioning. They should reflect the version the application is using. # It is recommended to use it with quotes. -appVersion: "1.0.0" +appVersion: "1.0.1" maintainers: - name: wafflesvonmaple diff --git a/charts/evm-rollup/values.yaml b/charts/evm-rollup/values.yaml index 9bec13f48b..4b9bbc1dbb 100644 --- a/charts/evm-rollup/values.yaml +++ b/charts/evm-rollup/values.yaml @@ -16,7 +16,7 @@ images: conductor: repo: ghcr.io/astriaorg/conductor pullPolicy: IfNotPresent - tag: 1.0.0 + tag: 1.0.1 devTag: latest diff --git a/charts/evm-stack/Chart.yaml b/charts/evm-stack/Chart.yaml index 990a8c4c53..b790ec5af6 100644 --- a/charts/evm-stack/Chart.yaml +++ b/charts/evm-stack/Chart.yaml @@ -15,7 +15,7 @@ type: application # This is the chart version. This version number should be incremented each time you make changes # to the chart and its templates, including the app version. # Versions are expected to follow Semantic Versioning (https://semver.org/) -version: 1.0.3 +version: 1.0.4 dependencies: - name: celestia-node @@ -23,7 +23,7 @@ dependencies: repository: "file://../celestia-node" condition: celestia-node.enabled - name: evm-rollup - version: 1.0.0 + version: 1.0.1 repository: "file://../evm-rollup" - name: composer version: 1.0.0 diff --git a/crates/astria-conductor/Cargo.toml b/crates/astria-conductor/Cargo.toml index 8dc3f608a8..d69d9b6558 100644 --- a/crates/astria-conductor/Cargo.toml +++ b/crates/astria-conductor/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "astria-conductor" -version = "1.0.0" +version = "1.0.1" edition = "2021" rust-version = "1.81.0" license = "MIT OR Apache-2.0" From ff9ad8c9010d3bf8fae183c7c78382ae218ab3d9 Mon Sep 17 00:00:00 2001 From: ethanoroshiba Date: Tue, 3 Dec 2024 13:26:39 -0600 Subject: [PATCH 04/27] update chart.lock --- charts/evm-stack/Chart.lock | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/charts/evm-stack/Chart.lock b/charts/evm-stack/Chart.lock index 142c15567e..b7435eee3e 100644 --- a/charts/evm-stack/Chart.lock +++ b/charts/evm-stack/Chart.lock @@ -4,7 +4,7 @@ dependencies: version: 0.4.0 - name: evm-rollup repository: file://../evm-rollup - version: 1.0.0 + version: 1.0.1 - name: composer repository: file://../composer version: 1.0.0 @@ -20,5 +20,5 @@ dependencies: - name: blockscout-stack repository: https://blockscout.github.io/helm-charts version: 1.6.8 -digest: sha256:618d0978ce1fa169bffa360010e8afeb06f21ffb7574e8a298d1d561bbcee05b -generated: "2024-11-11T13:27:42.868678+02:00" +digest: sha256:b24083a75d7242a95b7b98176e67e429c30fb33192769252f40555077034e244 +generated: "2024-12-03T13:26:17.431846-06:00" From af132a1e48e8d2560693aa90fa3de76d366fff60 Mon Sep 17 00:00:00 2001 From: ethanoroshiba Date: Wed, 4 Dec 2024 14:02:02 -0600 Subject: [PATCH 05/27] add rollup_start_block_height field to GenesisInfo --- crates/astria-conductor/src/executor/mod.rs | 18 ++++++++++------ crates/astria-conductor/src/executor/state.rs | 8 +++++++ crates/astria-conductor/src/executor/tests.rs | 1 + .../tests/blackbox/firm_only.rs | 8 ++++++- .../tests/blackbox/helpers/macros.rs | 8 +++++-- .../tests/blackbox/soft_and_firm.rs | 5 +++++ .../tests/blackbox/soft_only.rs | 8 ++++++- crates/astria-core/src/execution/v1/mod.rs | 11 ++++++++++ .../src/generated/astria.execution.v1.rs | 7 +++++-- .../generated/astria.execution.v1.serde.rs | 21 +++++++++++++++++++ .../astria/execution/v1/execution.proto | 6 ++++-- 11 files changed, 87 insertions(+), 14 deletions(-) diff --git a/crates/astria-conductor/src/executor/mod.rs b/crates/astria-conductor/src/executor/mod.rs index 8bee05f991..061a7b5158 100644 --- a/crates/astria-conductor/src/executor/mod.rs +++ b/crates/astria-conductor/src/executor/mod.rs @@ -457,9 +457,12 @@ impl Executor { let genesis_height = self.state.sequencer_start_block_height(); let block_height = executable_block.height; - let Some(block_number) = - state::map_sequencer_height_to_rollup_height(genesis_height, block_height) - else { + let rollup_start_block_height = self.state.rollup_start_block_height(); + let Some(block_number) = state::map_sequencer_height_to_rollup_height( + genesis_height, + block_height, + rollup_start_block_height, + ) else { bail!( "failed to map block height rollup number. This means the operation `sequencer_height - sequencer_genesis_height` underflowed or was not a valid @@ -515,15 +518,18 @@ impl Executor { let executable_block = ExecutableBlock::from_reconstructed(block); let expected_height = self.state.next_expected_firm_sequencer_height(); let block_height = executable_block.height; + let rollup_start_block_height = self.state.rollup_start_block_height(); ensure!( block_height == expected_height, "expected block at sequencer height {expected_height}, but got {block_height}", ); let genesis_height = self.state.sequencer_start_block_height(); - let Some(block_number) = - state::map_sequencer_height_to_rollup_height(genesis_height, block_height) - else { + let Some(block_number) = state::map_sequencer_height_to_rollup_height( + genesis_height, + block_height, + rollup_start_block_height, + ) else { bail!( "failed to map block height rollup number. This means the operation `sequencer_height - sequencer_genesis_height` underflowed or was not a valid diff --git a/crates/astria-conductor/src/executor/state.rs b/crates/astria-conductor/src/executor/state.rs index 1b48087864..7e5a7d3084 100644 --- a/crates/astria-conductor/src/executor/state.rs +++ b/crates/astria-conductor/src/executor/state.rs @@ -225,6 +225,7 @@ forward_impls!( [sequencer_start_block_height -> SequencerHeight], [celestia_base_block_height -> u64], [sequencer_stop_block_height -> SequencerHeight], + [rollup_start_block_height -> u64] ); forward_impls!( @@ -312,6 +313,10 @@ impl State { self.genesis_info.rollup_id() } + fn rollup_start_block_height(&self) -> u64 { + self.genesis_info.rollup_start_block_height() + } + fn next_expected_firm_sequencer_height(&self) -> Option { map_rollup_number_to_sequencer_height( self.sequencer_start_block_height(), @@ -348,10 +353,12 @@ fn map_rollup_number_to_sequencer_height( pub(super) fn map_sequencer_height_to_rollup_height( sequencer_genesis_height: SequencerHeight, sequencer_height: SequencerHeight, + rollup_start_block_height: u64, ) -> Option { sequencer_height .value() .checked_sub(sequencer_genesis_height.value())? + .checked_add(rollup_start_block_height)? .try_into() .ok() } @@ -402,6 +409,7 @@ mod tests { sequencer_start_block_height: 10, sequencer_stop_block_height: 100, celestia_block_variance: 0, + rollup_start_block_height: 1, sequencer_chain_id: "test-sequencer-0".to_string(), celestia_chain_id: "test-celestia-0".to_string(), }) diff --git a/crates/astria-conductor/src/executor/tests.rs b/crates/astria-conductor/src/executor/tests.rs index 4f97a02980..6e4ac5d9cc 100644 --- a/crates/astria-conductor/src/executor/tests.rs +++ b/crates/astria-conductor/src/executor/tests.rs @@ -50,6 +50,7 @@ fn make_state( sequencer_start_block_height: 1, sequencer_stop_block_height: 100, celestia_block_variance: 1, + rollup_start_block_height: 1, sequencer_chain_id: "test_sequencer-0".to_string(), celestia_chain_id: "test_celestia-0".to_string(), }) diff --git a/crates/astria-conductor/tests/blackbox/firm_only.rs b/crates/astria-conductor/tests/blackbox/firm_only.rs index e1e8f7ec45..d92f333ccc 100644 --- a/crates/astria-conductor/tests/blackbox/firm_only.rs +++ b/crates/astria-conductor/tests/blackbox/firm_only.rs @@ -57,6 +57,7 @@ async fn simple() { sequencer_start_block_height: 1, sequencer_stop_block_height: 10, celestia_block_variance: 10, + rollup_start_block_height: 0, ); mount_get_commitment_state!( @@ -139,6 +140,7 @@ async fn submits_two_heights_in_succession() { sequencer_start_block_height: 1, sequencer_stop_block_height: 10, celestia_block_variance: 10, + rollup_start_block_height: 0, ); mount_get_commitment_state!( @@ -252,6 +254,7 @@ async fn skips_already_executed_heights() { sequencer_start_block_height: 1, sequencer_stop_block_height: 10, celestia_block_variance: 10, + rollup_start_block_height: 0, ); mount_get_commitment_state!( @@ -337,6 +340,7 @@ async fn fetch_from_later_celestia_height() { sequencer_start_block_height: 1, sequencer_stop_block_height: 10, celestia_block_variance: 10, + rollup_start_block_height: 0, ); mount_get_commitment_state!( @@ -453,7 +457,8 @@ async fn exits_on_celestia_chain_id_mismatch() { .respond_with(GrpcResponse::constant_response( genesis_info!(sequencer_start_block_height: 1, sequencer_stop_block_height: 10, - celestia_block_variance: 10,), + celestia_block_variance: 10, + rollup_start_block_height: 0,), )) .expect(0..) .mount(&mock_grpc.mock_server) @@ -529,6 +534,7 @@ async fn conductor_restarts_after_reaching_stop_block_height() { sequencer_start_block_height: 1, sequencer_stop_block_height: 3, celestia_block_variance: 10, + rollup_start_block_height: 0, ); mount_get_commitment_state!( diff --git a/crates/astria-conductor/tests/blackbox/helpers/macros.rs b/crates/astria-conductor/tests/blackbox/helpers/macros.rs index 4bfd3b7d79..6870ce460c 100644 --- a/crates/astria-conductor/tests/blackbox/helpers/macros.rs +++ b/crates/astria-conductor/tests/blackbox/helpers/macros.rs @@ -97,13 +97,15 @@ macro_rules! genesis_info { sequencer_start_block_height: $start_height:expr,sequencer_stop_block_height: $stop_height:expr,celestia_block_variance: - $variance:expr $(,)? + $variance:expr,rollup_start_block_height: + $rollup_start_block_height:expr $(,)? ) => { ::astria_core::generated::execution::v1::GenesisInfo { rollup_id: Some($crate::ROLLUP_ID.to_raw()), sequencer_start_block_height: $start_height, sequencer_stop_block_height: $stop_height, celestia_block_variance: $variance, + rollup_start_block_height: $rollup_start_block_height, sequencer_chain_id: $crate::SEQUENCER_CHAIN_ID.to_string(), celestia_chain_id: $crate::helpers::CELESTIA_CHAIN_ID.to_string(), } @@ -358,7 +360,8 @@ macro_rules! mount_get_genesis_info { $test_env:ident, sequencer_start_block_height: $start_height:expr, sequencer_stop_block_height: $stop_height:expr, - celestia_block_variance: $variance:expr + celestia_block_variance: $variance:expr, + rollup_start_block_height: $rollup_start_block_height:expr $(,)? ) => { $test_env.mount_get_genesis_info( @@ -366,6 +369,7 @@ macro_rules! mount_get_genesis_info { sequencer_start_block_height: $start_height, sequencer_stop_block_height: $stop_height, celestia_block_variance: $variance, + rollup_start_block_height: $rollup_start_block_height, ) ).await; }; diff --git a/crates/astria-conductor/tests/blackbox/soft_and_firm.rs b/crates/astria-conductor/tests/blackbox/soft_and_firm.rs index dc518c755d..6b9f0ec04a 100644 --- a/crates/astria-conductor/tests/blackbox/soft_and_firm.rs +++ b/crates/astria-conductor/tests/blackbox/soft_and_firm.rs @@ -43,6 +43,7 @@ async fn executes_soft_first_then_updates_firm() { sequencer_start_block_height: 1, sequencer_stop_block_height: 10, celestia_block_variance: 10, + rollup_start_block_height: 0, ); mount_get_commitment_state!( @@ -178,6 +179,7 @@ async fn executes_firm_then_soft_at_next_height() { sequencer_start_block_height: 1, sequencer_stop_block_height: 10, celestia_block_variance: 10, + rollup_start_block_height: 0, ); mount_get_commitment_state!( @@ -338,6 +340,7 @@ async fn missing_block_is_fetched_for_updating_firm_commitment() { sequencer_start_block_height: 1, sequencer_stop_block_height: 10, celestia_block_variance: 10, + rollup_start_block_height: 0, ); mount_get_commitment_state!( @@ -468,6 +471,7 @@ async fn conductor_restarts_on_permission_denied() { sequencer_start_block_height: 1, sequencer_stop_block_height: 10, celestia_block_variance: 10, + rollup_start_block_height: 0, ); mount_get_commitment_state!( @@ -615,6 +619,7 @@ async fn conductor_restarts_after_reaching_stop_height() { sequencer_start_block_height: 1, sequencer_stop_block_height: 3, celestia_block_variance: 10, + rollup_start_block_height: 0, ); // Regular test mounts diff --git a/crates/astria-conductor/tests/blackbox/soft_only.rs b/crates/astria-conductor/tests/blackbox/soft_only.rs index 51477b5d0f..8af394c214 100644 --- a/crates/astria-conductor/tests/blackbox/soft_only.rs +++ b/crates/astria-conductor/tests/blackbox/soft_only.rs @@ -44,6 +44,7 @@ async fn simple() { sequencer_start_block_height: 1, sequencer_stop_block_height: 10, celestia_block_variance: 10, + rollup_start_block_height: 0, ); mount_get_commitment_state!( @@ -118,6 +119,7 @@ async fn submits_two_heights_in_succession() { sequencer_start_block_height: 1, sequencer_stop_block_height: 10, celestia_block_variance: 10, + rollup_start_block_height: 0, ); mount_get_commitment_state!( @@ -225,6 +227,7 @@ async fn skips_already_executed_heights() { sequencer_start_block_height: 1, sequencer_stop_block_height: 10, celestia_block_variance: 10, + rollup_start_block_height: 0, ); mount_get_commitment_state!( @@ -299,6 +302,7 @@ async fn requests_from_later_genesis_height() { sequencer_start_block_height: 10, sequencer_stop_block_height: 20, celestia_block_variance: 10, + rollup_start_block_height: 0, ); mount_get_commitment_state!( @@ -407,7 +411,8 @@ async fn exits_on_sequencer_chain_id_mismatch() { .respond_with(GrpcResponse::constant_response( genesis_info!(sequencer_start_block_height: 1, sequencer_stop_block_height: 10, - celestia_block_variance: 10,), + celestia_block_variance: 10, + rollup_start_block_height: 0,), )) .expect(0..) .mount(&mock_grpc.mock_server) @@ -467,6 +472,7 @@ async fn conductor_restarts_after_reaching_stop_block_height() { sequencer_start_block_height: 1, sequencer_stop_block_height: 4, celestia_block_variance: 10, + rollup_start_block_height: 0, ); mount_get_commitment_state!( diff --git a/crates/astria-core/src/execution/v1/mod.rs b/crates/astria-core/src/execution/v1/mod.rs index 9359de2891..6b76772e91 100644 --- a/crates/astria-core/src/execution/v1/mod.rs +++ b/crates/astria-core/src/execution/v1/mod.rs @@ -54,6 +54,8 @@ pub struct GenesisInfo { sequencer_stop_block_height: tendermint::block::Height, /// The allowed variance in the block height of celestia when looking for sequencer blocks. celestia_block_variance: u64, + /// The rollup block number to map to the sequencer start block height. + rollup_start_block_height: u64, /// The chain ID of the sequencer network. sequencer_chain_id: String, /// The chain ID of the celestia network. @@ -90,6 +92,11 @@ impl GenesisInfo { pub fn celestia_chain_id(&self) -> &str { &self.celestia_chain_id } + + #[must_use] + pub fn rollup_start_block_height(&self) -> u64 { + self.rollup_start_block_height + } } impl From for raw::GenesisInfo { @@ -108,6 +115,7 @@ impl Protobuf for GenesisInfo { sequencer_start_block_height, sequencer_stop_block_height, celestia_block_variance, + rollup_start_block_height, sequencer_chain_id, celestia_chain_id, } = raw; @@ -122,6 +130,7 @@ impl Protobuf for GenesisInfo { sequencer_start_block_height: (*sequencer_start_block_height).into(), sequencer_stop_block_height: (*sequencer_stop_block_height).into(), celestia_block_variance: *celestia_block_variance, + rollup_start_block_height: *rollup_start_block_height, sequencer_chain_id: sequencer_chain_id.clone(), celestia_chain_id: celestia_chain_id.clone(), }) @@ -133,6 +142,7 @@ impl Protobuf for GenesisInfo { sequencer_start_block_height, sequencer_stop_block_height, celestia_block_variance, + rollup_start_block_height, sequencer_chain_id, celestia_chain_id, } = self; @@ -153,6 +163,7 @@ impl Protobuf for GenesisInfo { sequencer_start_block_height, sequencer_stop_block_height, celestia_block_variance: *celestia_block_variance, + rollup_start_block_height: *rollup_start_block_height, sequencer_chain_id: sequencer_chain_id.clone(), celestia_chain_id: celestia_chain_id.clone(), } diff --git a/crates/astria-core/src/generated/astria.execution.v1.rs b/crates/astria-core/src/generated/astria.execution.v1.rs index c4e95364fd..3de5b64bc8 100644 --- a/crates/astria-core/src/generated/astria.execution.v1.rs +++ b/crates/astria-core/src/generated/astria.execution.v1.rs @@ -17,9 +17,12 @@ pub struct GenesisInfo { /// The allowed variance in celestia for sequencer blocks to have been posted. #[prost(uint64, tag = "4")] pub celestia_block_variance: u64, - #[prost(string, tag = "5")] - pub sequencer_chain_id: ::prost::alloc::string::String, + /// The rollup block number to map to the sequencer start block height. + #[prost(uint64, tag = "5")] + pub rollup_start_block_height: u64, #[prost(string, tag = "6")] + pub sequencer_chain_id: ::prost::alloc::string::String, + #[prost(string, tag = "7")] pub celestia_chain_id: ::prost::alloc::string::String, } impl ::prost::Name for GenesisInfo { diff --git a/crates/astria-core/src/generated/astria.execution.v1.serde.rs b/crates/astria-core/src/generated/astria.execution.v1.serde.rs index 31d9b2aef6..5454cac369 100644 --- a/crates/astria-core/src/generated/astria.execution.v1.serde.rs +++ b/crates/astria-core/src/generated/astria.execution.v1.serde.rs @@ -719,6 +719,9 @@ impl serde::Serialize for GenesisInfo { if self.celestia_block_variance != 0 { len += 1; } + if self.rollup_start_block_height != 0 { + len += 1; + } if !self.sequencer_chain_id.is_empty() { len += 1; } @@ -739,6 +742,10 @@ impl serde::Serialize for GenesisInfo { #[allow(clippy::needless_borrow)] struct_ser.serialize_field("celestiaBlockVariance", ToString::to_string(&self.celestia_block_variance).as_str())?; } + if self.rollup_start_block_height != 0 { + #[allow(clippy::needless_borrow)] + struct_ser.serialize_field("rollupStartBlockHeight", ToString::to_string(&self.rollup_start_block_height).as_str())?; + } if !self.sequencer_chain_id.is_empty() { struct_ser.serialize_field("sequencerChainId", &self.sequencer_chain_id)?; } @@ -763,6 +770,8 @@ impl<'de> serde::Deserialize<'de> for GenesisInfo { "sequencerStopBlockHeight", "celestia_block_variance", "celestiaBlockVariance", + "rollup_start_block_height", + "rollupStartBlockHeight", "sequencer_chain_id", "sequencerChainId", "celestia_chain_id", @@ -775,6 +784,7 @@ impl<'de> serde::Deserialize<'de> for GenesisInfo { SequencerStartBlockHeight, SequencerStopBlockHeight, CelestiaBlockVariance, + RollupStartBlockHeight, SequencerChainId, CelestiaChainId, } @@ -802,6 +812,7 @@ impl<'de> serde::Deserialize<'de> for GenesisInfo { "sequencerStartBlockHeight" | "sequencer_start_block_height" => Ok(GeneratedField::SequencerStartBlockHeight), "sequencerStopBlockHeight" | "sequencer_stop_block_height" => Ok(GeneratedField::SequencerStopBlockHeight), "celestiaBlockVariance" | "celestia_block_variance" => Ok(GeneratedField::CelestiaBlockVariance), + "rollupStartBlockHeight" | "rollup_start_block_height" => Ok(GeneratedField::RollupStartBlockHeight), "sequencerChainId" | "sequencer_chain_id" => Ok(GeneratedField::SequencerChainId), "celestiaChainId" | "celestia_chain_id" => Ok(GeneratedField::CelestiaChainId), _ => Err(serde::de::Error::unknown_field(value, FIELDS)), @@ -827,6 +838,7 @@ impl<'de> serde::Deserialize<'de> for GenesisInfo { let mut sequencer_start_block_height__ = None; let mut sequencer_stop_block_height__ = None; let mut celestia_block_variance__ = None; + let mut rollup_start_block_height__ = None; let mut sequencer_chain_id__ = None; let mut celestia_chain_id__ = None; while let Some(k) = map_.next_key()? { @@ -861,6 +873,14 @@ impl<'de> serde::Deserialize<'de> for GenesisInfo { Some(map_.next_value::<::pbjson::private::NumberDeserialize<_>>()?.0) ; } + GeneratedField::RollupStartBlockHeight => { + if rollup_start_block_height__.is_some() { + return Err(serde::de::Error::duplicate_field("rollupStartBlockHeight")); + } + rollup_start_block_height__ = + Some(map_.next_value::<::pbjson::private::NumberDeserialize<_>>()?.0) + ; + } GeneratedField::SequencerChainId => { if sequencer_chain_id__.is_some() { return Err(serde::de::Error::duplicate_field("sequencerChainId")); @@ -880,6 +900,7 @@ impl<'de> serde::Deserialize<'de> for GenesisInfo { sequencer_start_block_height: sequencer_start_block_height__.unwrap_or_default(), sequencer_stop_block_height: sequencer_stop_block_height__.unwrap_or_default(), celestia_block_variance: celestia_block_variance__.unwrap_or_default(), + rollup_start_block_height: rollup_start_block_height__.unwrap_or_default(), sequencer_chain_id: sequencer_chain_id__.unwrap_or_default(), celestia_chain_id: celestia_chain_id__.unwrap_or_default(), }) diff --git a/proto/executionapis/astria/execution/v1/execution.proto b/proto/executionapis/astria/execution/v1/execution.proto index 068a4b7f6d..62a108a7d3 100644 --- a/proto/executionapis/astria/execution/v1/execution.proto +++ b/proto/executionapis/astria/execution/v1/execution.proto @@ -19,8 +19,10 @@ message GenesisInfo { uint32 sequencer_stop_block_height = 3; // The allowed variance in celestia for sequencer blocks to have been posted. uint64 celestia_block_variance = 4; - string sequencer_chain_id = 5; - string celestia_chain_id = 6; + // The rollup block number to map to the sequencer start block height. + uint64 rollup_start_block_height = 5; + string sequencer_chain_id = 6; + string celestia_chain_id = 7; } // The set of information which deterministic driver of block production From 59456a4c1197b7e223813d725fee71a48e519d41 Mon Sep 17 00:00:00 2001 From: ethanoroshiba Date: Wed, 4 Dec 2024 15:53:43 -0600 Subject: [PATCH 06/27] update height mapping calculation, flush out tests --- crates/astria-conductor/src/executor/state.rs | 1 + .../tests/blackbox/firm_only.rs | 119 +++++++++---- .../tests/blackbox/helpers/macros.rs | 39 ++++- .../tests/blackbox/helpers/mod.rs | 10 +- .../tests/blackbox/soft_and_firm.rs | 165 ++++++++++++------ .../tests/blackbox/soft_only.rs | 113 +++++++++--- 6 files changed, 326 insertions(+), 121 deletions(-) diff --git a/crates/astria-conductor/src/executor/state.rs b/crates/astria-conductor/src/executor/state.rs index 7e5a7d3084..2dd51d400c 100644 --- a/crates/astria-conductor/src/executor/state.rs +++ b/crates/astria-conductor/src/executor/state.rs @@ -359,6 +359,7 @@ pub(super) fn map_sequencer_height_to_rollup_height( .value() .checked_sub(sequencer_genesis_height.value())? .checked_add(rollup_start_block_height)? + .checked_sub(1)? // offset rollup start block height value .try_into() .ok() } diff --git a/crates/astria-conductor/tests/blackbox/firm_only.rs b/crates/astria-conductor/tests/blackbox/firm_only.rs index d92f333ccc..6ee2265ce1 100644 --- a/crates/astria-conductor/tests/blackbox/firm_only.rs +++ b/crates/astria-conductor/tests/blackbox/firm_only.rs @@ -15,7 +15,10 @@ use futures::future::{ }; use serde_json::json; use telemetry::metrics; -use tokio::time::timeout; +use tokio::time::{ + sleep, + timeout, +}; use wiremock::{ matchers::{ body_partial_json, @@ -57,7 +60,7 @@ async fn simple() { sequencer_start_block_height: 1, sequencer_stop_block_height: 10, celestia_block_variance: 10, - rollup_start_block_height: 0, + rollup_start_block_height: 1, ); mount_get_commitment_state!( @@ -140,7 +143,7 @@ async fn submits_two_heights_in_succession() { sequencer_start_block_height: 1, sequencer_stop_block_height: 10, celestia_block_variance: 10, - rollup_start_block_height: 0, + rollup_start_block_height: 1, ); mount_get_commitment_state!( @@ -254,7 +257,7 @@ async fn skips_already_executed_heights() { sequencer_start_block_height: 1, sequencer_stop_block_height: 10, celestia_block_variance: 10, - rollup_start_block_height: 0, + rollup_start_block_height: 1, ); mount_get_commitment_state!( @@ -340,7 +343,7 @@ async fn fetch_from_later_celestia_height() { sequencer_start_block_height: 1, sequencer_stop_block_height: 10, celestia_block_variance: 10, - rollup_start_block_height: 0, + rollup_start_block_height: 1, ); mount_get_commitment_state!( @@ -458,7 +461,7 @@ async fn exits_on_celestia_chain_id_mismatch() { genesis_info!(sequencer_start_block_height: 1, sequencer_stop_block_height: 10, celestia_block_variance: 10, - rollup_start_block_height: 0,), + rollup_start_block_height: 1,), )) .expect(0..) .mount(&mock_grpc.mock_server) @@ -524,6 +527,22 @@ async fn exits_on_celestia_chain_id_mismatch() { } } +/// Tests that the conductor correctly stops at the stop block height and executes the firm block +/// for that height before restarting and continuing after fetching new genesis info and commitment +/// state. +/// +/// It consists of the following steps: +/// 1. Mount commitment state and genesis info with a stop height of 3 for the first height, only +/// responding up to 1 time so that the same info is not provided after conductor restart. +/// 2. Mount sequencer genesis and celestia header network head. +/// 3. Mount firm blocks for heights 3 and 4. +/// 4. Mount `execute_block` and `update_commitment_state` for firm block 3, expecting only one call +/// since they should not be called after restarting. +/// 5. Wait ample time for conductor to restart before performing the next set of mounts. +/// 6. Mount new genesis info and updated commitment state with rollup start block height of 2 to +/// reflect that the first block has already been executed. +/// 7. Mount `execute_block` and `update_commitment_state` for firm block 4, awaiting their +/// satisfaction. #[expect(clippy::too_many_lines, reason = "All lines reasonably necessary")] #[tokio::test(flavor = "multi_thread", worker_threads = 1)] async fn conductor_restarts_after_reaching_stop_block_height() { @@ -534,7 +553,8 @@ async fn conductor_restarts_after_reaching_stop_block_height() { sequencer_start_block_height: 1, sequencer_stop_block_height: 3, celestia_block_variance: 10, - rollup_start_block_height: 0, + rollup_start_block_height: 1, + up_to_n_times: 1, // Only respond once, since updated information is needed after restart. ); mount_get_commitment_state!( @@ -550,10 +570,10 @@ async fn conductor_restarts_after_reaching_stop_block_height() { parent: [0; 64], ), base_celestia_height: 1, + up_to_n_times: 1, ); mount_sequencer_genesis!(test_conductor); - mount_celestia_header_network_head!( test_conductor, height: 2u32, @@ -564,34 +584,29 @@ async fn conductor_restarts_after_reaching_stop_block_height() { celestia_height: 1, sequencer_heights: [3, 4], ); - mount_sequencer_commit!( test_conductor, height: 3u32, ); - - mount_sequencer_validator_set!(test_conductor, height: 2u32); - - // Mount height above stop block height so that the executor will initiate a restart mount_sequencer_commit!( test_conductor, height: 4u32, ); - + mount_sequencer_validator_set!(test_conductor, height: 2u32); mount_sequencer_validator_set!(test_conductor, height: 3u32); - let execute_block = mount_executed_block!( + let execute_block_1 = mount_executed_block!( test_conductor, - mock_name: "execute_block_twice", + mock_name: "execute_block_1", number: 2, hash: [2; 64], parent: [1; 64], - expected_calls: 2, // Expected to be called again after restart + expected_calls: 1, // should not be called again upon restart ); - let update_commitment_state = mount_update_commitment_state!( + let update_commitment_state_1 = mount_update_commitment_state!( test_conductor, - mock_name: "update_commitment_state_twice", + mock_name: "update_commitment_state_1", firm: ( number: 2, hash: [2; 64], @@ -603,23 +618,61 @@ async fn conductor_restarts_after_reaching_stop_block_height() { parent: [1; 64], ), base_celestia_height: 1, - expected_calls: 2, // Expected to be called again after restart + expected_calls: 1, // should not be called again upon restart + ); + + timeout( + Duration::from_millis(1000), + join( + execute_block_1.wait_until_satisfied(), + update_commitment_state_1.wait_until_satisfied(), + ), + ) + .await + .expect( + "conductor should have executed the first firm block and updated the first firm \ + commitment state twice within 1000ms", ); - // Will be validated for no calls upon dropping - let _no_execute_block = mount_executed_block!( + // Wait for conductor to restart before performing the next set of mounts + sleep(Duration::from_millis(1000)).await; + + // Mount new genesis info and commitment state with updated heights + mount_get_genesis_info!( test_conductor, - mock_name: "should_not_execute_this_block", + sequencer_start_block_height: 1, + sequencer_stop_block_height: 10, + celestia_block_variance: 10, + rollup_start_block_height: 2, + ); + + mount_get_commitment_state!( + test_conductor, + firm: ( + number: 2, + hash: [2; 64], + parent: [1; 64], + ), + soft: ( + number: 2, + hash: [2; 64], + parent: [1; 64], + ), + base_celestia_height: 1, + ); + + let execute_block_2 = mount_executed_block!( + test_conductor, + mock_name: "execute_block_2", number: 3, hash: [3; 64], parent: [2; 64], - expected_calls: 0, + expected_calls: 1, ); - // Will be validated for no calls upon dropping - let _no_update_commitment_state = mount_update_commitment_state!( + let update_commitment_state_2 = mount_update_commitment_state!( test_conductor, - mock_name: "should_not_update_this_commitment_state", + mock_name: "update_commitment_state_2", firm: ( number: 3, hash: [3; 64], @@ -631,19 +684,19 @@ async fn conductor_restarts_after_reaching_stop_block_height() { parent: [2; 64], ), base_celestia_height: 1, - expected_calls: 0, + expected_calls: 1, ); timeout( - Duration::from_millis(1000), + Duration::from_millis(2000), join( - execute_block.wait_until_satisfied(), - update_commitment_state.wait_until_satisfied(), + execute_block_2.wait_until_satisfied(), + update_commitment_state_2.wait_until_satisfied(), ), ) .await .expect( - "conductor should have executed the firm block and updated the firm commitment state \ - twice within 1000ms", + "conductor should have executed the second firm block and updated the second firm \ + commitment state twice within 2000ms", ); } diff --git a/crates/astria-conductor/tests/blackbox/helpers/macros.rs b/crates/astria-conductor/tests/blackbox/helpers/macros.rs index 6870ce460c..2b339adeb5 100644 --- a/crates/astria-conductor/tests/blackbox/helpers/macros.rs +++ b/crates/astria-conductor/tests/blackbox/helpers/macros.rs @@ -181,6 +181,22 @@ macro_rules! mount_get_commitment_state { soft: ( number: $soft_number:expr, hash: $soft_hash:expr, parent: $soft_parent:expr$(,)? ), base_celestia_height: $base_celestia_height:expr $(,)? + ) => { + mount_get_commitment_state!( + $test_env, + firm: ( number: $firm_number, hash: $firm_hash, parent: $firm_parent, ), + soft: ( number: $soft_number, hash: $soft_hash, parent: $soft_parent, ), + base_celestia_height: $base_celestia_height, + up_to_n_times: 1, + ) + }; + ( + $test_env:ident, + firm: ( number: $firm_number:expr, hash: $firm_hash:expr, parent: $firm_parent:expr$(,)? ), + soft: ( number: $soft_number:expr, hash: $soft_hash:expr, parent: $soft_parent:expr$(,)? ), + base_celestia_height: $base_celestia_height:expr, + up_to_n_times: $up_to_n_times:expr + $(,)? ) => { $test_env .mount_get_commitment_state($crate::commitment_state!( @@ -195,7 +211,7 @@ macro_rules! mount_get_commitment_state { parent: $soft_parent, ), base_celestia_height: $base_celestia_height, - )) + ), $up_to_n_times) .await }; } @@ -363,6 +379,24 @@ macro_rules! mount_get_genesis_info { celestia_block_variance: $variance:expr, rollup_start_block_height: $rollup_start_block_height:expr $(,)? + ) => { + mount_get_genesis_info!( + $test_env, + sequencer_start_block_height: $start_height, + sequencer_stop_block_height: $stop_height, + celestia_block_variance: $variance, + rollup_start_block_height: $rollup_start_block_height, + up_to_n_times: 1, + ) + }; + ( + $test_env:ident, + sequencer_start_block_height: $start_height:expr, + sequencer_stop_block_height: $stop_height:expr, + celestia_block_variance: $variance:expr, + rollup_start_block_height: $rollup_start_block_height:expr, + up_to_n_times: $up_to_n_times:expr + $(,)? ) => { $test_env.mount_get_genesis_info( $crate::genesis_info!( @@ -370,7 +404,8 @@ macro_rules! mount_get_genesis_info { sequencer_stop_block_height: $stop_height, celestia_block_variance: $variance, rollup_start_block_height: $rollup_start_block_height, - ) + ), + $up_to_n_times, ).await; }; } diff --git a/crates/astria-conductor/tests/blackbox/helpers/mod.rs b/crates/astria-conductor/tests/blackbox/helpers/mod.rs index ee2711c262..26ccb9d322 100644 --- a/crates/astria-conductor/tests/blackbox/helpers/mod.rs +++ b/crates/astria-conductor/tests/blackbox/helpers/mod.rs @@ -305,19 +305,24 @@ impl TestConductor { mount_genesis(&self.mock_http, chain_id).await; } - pub async fn mount_get_genesis_info(&self, genesis_info: GenesisInfo) { + pub async fn mount_get_genesis_info(&self, genesis_info: GenesisInfo, up_to_n_times: u64) { use astria_core::generated::execution::v1::GetGenesisInfoRequest; astria_grpc_mock::Mock::for_rpc_given( "get_genesis_info", astria_grpc_mock::matcher::message_type::(), ) .respond_with(astria_grpc_mock::response::constant_response(genesis_info)) + .up_to_n_times(up_to_n_times) .expect(1..) .mount(&self.mock_grpc.mock_server) .await; } - pub async fn mount_get_commitment_state(&self, commitment_state: CommitmentState) { + pub async fn mount_get_commitment_state( + &self, + commitment_state: CommitmentState, + up_to_n_times: u64, + ) { use astria_core::generated::execution::v1::GetCommitmentStateRequest; astria_grpc_mock::Mock::for_rpc_given( @@ -327,6 +332,7 @@ impl TestConductor { .respond_with(astria_grpc_mock::response::constant_response( commitment_state, )) + .up_to_n_times(up_to_n_times) .expect(1..) .mount(&self.mock_grpc.mock_server) .await; diff --git a/crates/astria-conductor/tests/blackbox/soft_and_firm.rs b/crates/astria-conductor/tests/blackbox/soft_and_firm.rs index 6b9f0ec04a..501084ebd3 100644 --- a/crates/astria-conductor/tests/blackbox/soft_and_firm.rs +++ b/crates/astria-conductor/tests/blackbox/soft_and_firm.rs @@ -5,7 +5,10 @@ use futures::future::{ join, join3, }; -use tokio::time::timeout; +use tokio::time::{ + sleep, + timeout, +}; use crate::{ helpers::spawn_conductor, @@ -43,7 +46,7 @@ async fn executes_soft_first_then_updates_firm() { sequencer_start_block_height: 1, sequencer_stop_block_height: 10, celestia_block_variance: 10, - rollup_start_block_height: 0, + rollup_start_block_height: 1, ); mount_get_commitment_state!( @@ -179,7 +182,7 @@ async fn executes_firm_then_soft_at_next_height() { sequencer_start_block_height: 1, sequencer_stop_block_height: 10, celestia_block_variance: 10, - rollup_start_block_height: 0, + rollup_start_block_height: 1, ); mount_get_commitment_state!( @@ -340,7 +343,7 @@ async fn missing_block_is_fetched_for_updating_firm_commitment() { sequencer_start_block_height: 1, sequencer_stop_block_height: 10, celestia_block_variance: 10, - rollup_start_block_height: 0, + rollup_start_block_height: 1, ); mount_get_commitment_state!( @@ -471,7 +474,8 @@ async fn conductor_restarts_on_permission_denied() { sequencer_start_block_height: 1, sequencer_stop_block_height: 10, celestia_block_variance: 10, - rollup_start_block_height: 0, + rollup_start_block_height: 1, + up_to_n_times: 2, ); mount_get_commitment_state!( @@ -487,6 +491,7 @@ async fn conductor_restarts_on_permission_denied() { parent: [0; 64], ), base_celestia_height: 1, + up_to_n_times: 2, ); mount_sequencer_genesis!(test_conductor); @@ -593,19 +598,24 @@ async fn conductor_restarts_on_permission_denied() { /// The conductor DOES call these on the firm block at the stop height, then proceeds to restart. /// /// This test consists of the following steps: -/// 1. Mount genesis info with a sequencer stop height of 3. -/// 2. Mount commitment state. +/// 1. Mount commitment state and genesis info with a sequencer stop height of 3, only responding up +/// to 1 time so that Conductor will not receive the same response after restart. +/// 2. Mount Celestia network head and sequencer genesis. /// 3. Mount ABCI info and sequencer (soft blocks) for heights 3 and 4. -/// 4. Mount Celestia network head and sequencer genesis. -/// 5. Create a mount for `execute_block` at height 3, which should be called twice: once for the -/// firm block prior to restart, and once for the firm block after restart. -/// 6. Create `update_commitment_state` mounts for soft blocks at heights 3 and 4, neither of which -/// should be called (will be validated upon dropping). -/// 7. Mount firm blocks at heights 3 and 4, with corresponding `update_commitment_state` mounts. -/// The latter should not be executed, since it is above the stop height. -/// 8. Validate that `execute_block` and `update_commitment_state` for the firm block at height 3 -/// exactly twice. -/// 9. Validate that none of the other mounts were called. +/// 4. Mount firm blocks at heights 3 and 4, with corresponding `update_commitment_state` mounts, +/// which should both be called. These are mounted with a slight delay to ensure that the soft +/// block arrives first after restart. +/// 5. Mount `execute_block` and `update_commitment_state` for both soft and firm blocks at height +/// 3. The soft block should not be executed since it is at the stop height, but the firm should +/// be. +/// 6. Await satisfaction of the `execute_block` and `update_commitment_state` for the firm block at +/// height 3 with a timeout of 1000ms. The test sleeps during this time, so that the following +/// mounts do not occur before the conductor restarts. +/// 7. Mount new genesis info with a sequencer stop height of 10 and a rollup start block height of +/// 2, along with corresponding commitment state, reflecting that block 1 has already been +/// executed and the commitment state updated. +/// 8. Mount `execute_block` and `update_commitment_state` for both soft and firm blocks at height 4 +/// and await their satisfaction. #[expect( clippy::too_many_lines, reason = "All lines reasonably necessary for the thoroughness of this test" @@ -619,10 +629,10 @@ async fn conductor_restarts_after_reaching_stop_height() { sequencer_start_block_height: 1, sequencer_stop_block_height: 3, celestia_block_variance: 10, - rollup_start_block_height: 0, + rollup_start_block_height: 1, + up_to_n_times: 1, // We only respond once since this needs to be updated after restart ); - // Regular test mounts mount_get_commitment_state!( test_conductor, firm: ( @@ -636,22 +646,20 @@ async fn conductor_restarts_after_reaching_stop_height() { parent: [0; 64], ), base_celestia_height: 1, + up_to_n_times: 1, // We only respond once since this needs to be updated after restart ); + mount_sequencer_genesis!(test_conductor); mount_celestia_header_network_head!( test_conductor, height: 1u32, ); - - // Note that the latest height is 4, even though the stop height is 3. We want to give the - // conductor the opportunity to err if it isn't working correctly. mount_abci_info!( test_conductor, latest_sequencer_height: 4, ); - // Neither of the following soft blocks should be executed. The first is at the stop height, the - // next just above to ensure blocks after the stop height are also not executed. + // Mount soft blocks for heights 3 and 4 mount_get_filtered_sequencer_block!( test_conductor, sequencer_height: 3, @@ -661,13 +669,31 @@ async fn conductor_restarts_after_reaching_stop_height() { sequencer_height: 4, ); - let execute_block = mount_executed_block!( + // Mount firm blocks for heights 3 and 4 + mount_celestia_blobs!( test_conductor, - mock_name: "execute_block", + celestia_height: 1, + sequencer_heights: [3, 4], + delay: Some(Duration::from_millis(200)) // short delay to ensure soft block at height 4 gets executed first after restart + ); + mount_sequencer_commit!( + test_conductor, + height: 3u32, + ); + mount_sequencer_commit!( + test_conductor, + height: 4u32, + ); + mount_sequencer_validator_set!(test_conductor, height: 2u32); + mount_sequencer_validator_set!(test_conductor, height: 3u32); + + let execute_block_1 = mount_executed_block!( + test_conductor, + mock_name: "execute_block_1", number: 2, hash: [2; 64], parent: [1; 64], - expected_calls: 2, // We expect this to be called twice, both times by executing the firm block, not the soft block + expected_calls: 1, // This should not be called again after restart ); // This should not be called, since it is at the sequencer stop height @@ -688,64 +714,86 @@ async fn conductor_restarts_after_reaching_stop_height() { expected_calls: 0, ); - // This is the update commitment state for the next soft block, which should not be called - // either - let _update_commitment_state_soft_2 = mount_update_commitment_state!( + let update_commitment_state_firm_1 = mount_update_commitment_state!( test_conductor, - mock_name: "update_commitment_state_soft_2", + mock_name: "update_commitment_state_firm_1", firm: ( number: 2, hash: [2; 64], parent: [1; 64], ), soft: ( - number: 3, - hash: [3; 64], - parent: [2; 64], + number: 2, + hash: [2; 64], + parent: [1; 64], ), base_celestia_height: 1, - expected_calls: 0, + expected_calls: 1, // Should not be called again after restart ); - mount_celestia_blobs!( + timeout( + Duration::from_millis(1000), + join( + execute_block_1.wait_until_satisfied(), + update_commitment_state_firm_1.wait_until_satisfied(), + ), + ) + .await + .expect("conductor should have updated the firm commitment state within 1000ms"); + + // Wait until conductor is restarted before performing next set of mounts + sleep(Duration::from_millis(1000)).await; + + mount_get_genesis_info!( test_conductor, - celestia_height: 1, - sequencer_heights: [3, 4], + sequencer_start_block_height: 1, + sequencer_stop_block_height: 10, + celestia_block_variance: 10, + rollup_start_block_height: 2, ); - // Mount the firm block at the stop height, which should be executed - mount_sequencer_commit!( + mount_get_commitment_state!( test_conductor, - height: 3u32, + firm: ( + number: 2, + hash: [2; 64], + parent: [1; 64], + ), + soft: ( + number: 2, + hash: [2; 64], + parent: [1; 64], + ), + base_celestia_height: 1, ); - mount_sequencer_validator_set!(test_conductor, height: 2u32); - // Mount the next height as well, which should not be executed - mount_sequencer_commit!( + let execute_block_2 = mount_executed_block!( test_conductor, - height: 4u32, + mock_name: "execute_block_2", + number: 3, + hash: [3; 64], + parent: [2; 64], + expected_calls: 1, ); - mount_sequencer_validator_set!(test_conductor, height: 3u32); - let update_commitment_state_firm_1 = mount_update_commitment_state!( + let update_commitment_state_soft_2 = mount_update_commitment_state!( test_conductor, - mock_name: "update_commitment_state_firm_1", + mock_name: "update_commitment_state_soft_2", firm: ( number: 2, hash: [2; 64], parent: [1; 64], ), soft: ( - number: 2, - hash: [2; 64], - parent: [1; 64], + number: 3, + hash: [3; 64], + parent: [2; 64], ), base_celestia_height: 1, - expected_calls: 2, // Expected to be called again after restart + expected_calls: 1, ); - // Should not be called, since it is above the stop height - let _update_commitment_state_firm_2 = mount_update_commitment_state!( + let update_commitment_state_firm_2 = mount_update_commitment_state!( test_conductor, mock_name: "update_commitment_state_firm_2", firm: ( @@ -759,14 +807,15 @@ async fn conductor_restarts_after_reaching_stop_height() { parent: [2; 64], ), base_celestia_height: 1, - expected_calls: 0, + expected_calls: 1, ); timeout( Duration::from_millis(1000), - join( - execute_block.wait_until_satisfied(), - update_commitment_state_firm_1.wait_until_satisfied(), + join3( + execute_block_2.wait_until_satisfied(), + update_commitment_state_soft_2.wait_until_satisfied(), + update_commitment_state_firm_2.wait_until_satisfied(), ), ) .await diff --git a/crates/astria-conductor/tests/blackbox/soft_only.rs b/crates/astria-conductor/tests/blackbox/soft_only.rs index 8af394c214..09f3143755 100644 --- a/crates/astria-conductor/tests/blackbox/soft_only.rs +++ b/crates/astria-conductor/tests/blackbox/soft_only.rs @@ -14,7 +14,10 @@ use futures::future::{ join4, }; use telemetry::metrics; -use tokio::time::timeout; +use tokio::time::{ + sleep, + timeout, +}; use crate::{ commitment_state, @@ -44,7 +47,7 @@ async fn simple() { sequencer_start_block_height: 1, sequencer_stop_block_height: 10, celestia_block_variance: 10, - rollup_start_block_height: 0, + rollup_start_block_height: 1, ); mount_get_commitment_state!( @@ -119,7 +122,7 @@ async fn submits_two_heights_in_succession() { sequencer_start_block_height: 1, sequencer_stop_block_height: 10, celestia_block_variance: 10, - rollup_start_block_height: 0, + rollup_start_block_height: 1, ); mount_get_commitment_state!( @@ -227,7 +230,7 @@ async fn skips_already_executed_heights() { sequencer_start_block_height: 1, sequencer_stop_block_height: 10, celestia_block_variance: 10, - rollup_start_block_height: 0, + rollup_start_block_height: 1, ); mount_get_commitment_state!( @@ -302,7 +305,7 @@ async fn requests_from_later_genesis_height() { sequencer_start_block_height: 10, sequencer_stop_block_height: 20, celestia_block_variance: 10, - rollup_start_block_height: 0, + rollup_start_block_height: 1, ); mount_get_commitment_state!( @@ -412,7 +415,7 @@ async fn exits_on_sequencer_chain_id_mismatch() { genesis_info!(sequencer_start_block_height: 1, sequencer_stop_block_height: 10, celestia_block_variance: 10, - rollup_start_block_height: 0,), + rollup_start_block_height: 1,), )) .expect(0..) .mount(&mock_grpc.mock_server) @@ -463,6 +466,26 @@ async fn exits_on_sequencer_chain_id_mismatch() { } } +/// Tests that the conductor correctly stops at the sequencer stop block height in soft only mode, +/// not executing the soft block at that height. Then, tests that the conductor correctly restarts +/// and continues executing soft blocks after receiving updated genesis info and commitment state. +/// +/// It consists of the following steps: +/// 1. Mount commitment state and genesis info with a stop height of 4, responding only up to 1 time +/// so that the same information is not retrieved after restarting. +/// 2. Mount sequencer genesis, ABCI info, and sequencer blocks for heights 3 and 4. +/// 3. Mount `execute_block` and `update_commitment_state` mocks for the soft block at height 3, +/// expecting only 1 call and timing out after 1000ms. During this time, the test sleeps so that +/// the following mounts are not performed before the conductor restarts. +/// 4. Mount updated commitment state and genesis info with a stop height of 10 (more than high +/// enough) and a rollup start block height of 2, reflecting that the first block has already +/// been executed. +/// 5. Mount `execute_block` and `update_commitment_state` mocks for the soft block at height 4, +/// awaiting their satisfaction. +#[expect( + clippy::too_many_lines, + reason = "All lines reasonably necessary for the thoroughness of this test" +)] #[tokio::test(flavor = "multi_thread", worker_threads = 1)] async fn conductor_restarts_after_reaching_stop_block_height() { let test_conductor = spawn_conductor(CommitLevel::SoftOnly).await; @@ -472,7 +495,8 @@ async fn conductor_restarts_after_reaching_stop_block_height() { sequencer_start_block_height: 1, sequencer_stop_block_height: 4, celestia_block_variance: 10, - rollup_start_block_height: 0, + rollup_start_block_height: 1, + up_to_n_times: 1, // We need to mount a new genesis info after restart ); mount_get_commitment_state!( @@ -488,6 +512,7 @@ async fn conductor_restarts_after_reaching_stop_block_height() { parent: [0; 64], ), base_celestia_height: 1, + up_to_n_times: 1, // We need to mount a new commitment state after restart ); mount_sequencer_genesis!(test_conductor); @@ -502,24 +527,23 @@ async fn conductor_restarts_after_reaching_stop_block_height() { sequencer_height: 3, ); - // Mount a block at the stop block height so the executor will initiate a restart mount_get_filtered_sequencer_block!( test_conductor, sequencer_height: 4, ); - let execute_block = mount_executed_block!( + let execute_block_1 = mount_executed_block!( test_conductor, - mock_name: "execute_block_twice", + mock_name: "execute_block_1", number: 2, hash: [2; 64], parent: [1; 64], - expected_calls: 2, // Expected to be called again after restart + expected_calls: 1, ); - let update_commitment_state = mount_update_commitment_state!( + let update_commitment_state_1 = mount_update_commitment_state!( test_conductor, - mock_name: "update_commitment_state_twice", + mock_name: "update_commitment_state_1", firm: ( number: 1, hash: [1; 64], @@ -531,23 +555,60 @@ async fn conductor_restarts_after_reaching_stop_block_height() { parent: [1; 64], ), base_celestia_height: 1, - expected_calls: 2, // Expected to be called again after restart + expected_calls: 1, + ); + + timeout( + Duration::from_millis(1000), + join( + execute_block_1.wait_until_satisfied(), + update_commitment_state_1.wait_until_satisfied(), + ), + ) + .await + .expect( + "conductor should have executed the first soft block and updated the first soft \ + commitment state within 1000ms", ); - // Will be validated for no calls upon dropping - let _no_execute_block = mount_executed_block!( + // Wait until conductor is restarted before performing next set of mounts + sleep(Duration::from_millis(1000)).await; + + mount_get_genesis_info!( test_conductor, - mock_name: "should_not_execute_this_block", + sequencer_start_block_height: 1, + sequencer_stop_block_height: 10, + celestia_block_variance: 10, + rollup_start_block_height: 2, + ); + + mount_get_commitment_state!( + test_conductor, + firm: ( + number: 1, + hash: [1; 64], + parent: [0; 64], + ), + soft: ( + number: 2, + hash: [2; 64], + parent: [1; 64], + ), + base_celestia_height: 1, + ); + + let execute_block_2 = mount_executed_block!( + test_conductor, + mock_name: "execute_block_2", number: 3, hash: [3; 64], parent: [2; 64], - expected_calls: 0, + expected_calls: 1, ); - // Will be validated for no calls upon dropping - let _no_update_commitment_state = mount_update_commitment_state!( + let update_commitment_state_2 = mount_update_commitment_state!( test_conductor, - mock_name: "should_not_update_this_commitment_state", + mock_name: "update_commitment_state_2", firm: ( number: 1, hash: [1; 64], @@ -559,19 +620,19 @@ async fn conductor_restarts_after_reaching_stop_block_height() { parent: [2; 64], ), base_celestia_height: 1, - expected_calls: 0, + expected_calls: 1, ); timeout( Duration::from_millis(1000), join( - execute_block.wait_until_satisfied(), - update_commitment_state.wait_until_satisfied(), + execute_block_2.wait_until_satisfied(), + update_commitment_state_2.wait_until_satisfied(), ), ) .await .expect( - "conductor should have executed the soft block and updated the soft commitment state \ - within 1000ms", + "conductor should have executed the second soft block and updated the second soft \ + commitment state within 1000ms", ); } From afc249999029ce1240a3018f6e33c18201deeecc Mon Sep 17 00:00:00 2001 From: ethanoroshiba Date: Thu, 5 Dec 2024 12:30:22 -0600 Subject: [PATCH 07/27] geth devtag --- charts/evm-rollup/values.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/charts/evm-rollup/values.yaml b/charts/evm-rollup/values.yaml index 4b9bbc1dbb..6855a776d2 100644 --- a/charts/evm-rollup/values.yaml +++ b/charts/evm-rollup/values.yaml @@ -11,7 +11,7 @@ images: repo: ghcr.io/astriaorg/astria-geth pullPolicy: IfNotPresent tag: 1.0.0 - devTag: latest + devTag: pr-59 overrideTag: "" conductor: repo: ghcr.io/astriaorg/conductor From 90b1f11baf4145c6d9025f4e2ff4ef156aead236 Mon Sep 17 00:00:00 2001 From: ethanoroshiba Date: Mon, 9 Dec 2024 16:57:39 -0600 Subject: [PATCH 08/27] fix height logic --- crates/astria-conductor/src/executor/mod.rs | 1 + crates/astria-conductor/src/executor/state.rs | 61 ++++++++++++------- .../tests/blackbox/firm_only.rs | 14 ++--- .../tests/blackbox/soft_and_firm.rs | 16 ++--- .../tests/blackbox/soft_only.rs | 14 ++--- 5 files changed, 60 insertions(+), 46 deletions(-) diff --git a/crates/astria-conductor/src/executor/mod.rs b/crates/astria-conductor/src/executor/mod.rs index 061a7b5158..9083fcf30d 100644 --- a/crates/astria-conductor/src/executor/mod.rs +++ b/crates/astria-conductor/src/executor/mod.rs @@ -415,6 +415,7 @@ impl Executor { // Stop executing soft blocks at the sequencer stop block height (exclusive). If we are also // executing firm blocks, we let execution continue since one more firm block will be // executed before `execute_firm` initiates a restart. If we are in soft-only mode, we + // return a `StopHeightExceded::Sequencer` error to signal a restart. if executable_block.height >= self.state.sequencer_stop_block_height() { let res = if self.mode.is_with_firm() { info!( diff --git a/crates/astria-conductor/src/executor/state.rs b/crates/astria-conductor/src/executor/state.rs index e0d15e8373..d49c4fda73 100644 --- a/crates/astria-conductor/src/executor/state.rs +++ b/crates/astria-conductor/src/executor/state.rs @@ -35,13 +35,13 @@ pub(super) fn channel() -> (StateSender, StateReceiver) { #[derive(Debug, thiserror::Error)] #[error( - "adding sequencer genesis height `{sequencer_genesis_height}` and `{commitment_type}` rollup \ - number `{rollup_number}` overflowed unsigned u32::MAX, the maximum permissible cometbft \ - height" + "adding sequencer genesis height `{sequencer_start_block_height}` and `{commitment_type}` \ + rollup number `{rollup_number}` overflowed unsigned u32::MAX, the maximum permissible \ + cometbft height" )] pub(super) struct InvalidState { commitment_type: &'static str, - sequencer_genesis_height: u64, + sequencer_start_block_height: u64, rollup_number: u64, } @@ -101,12 +101,19 @@ fn can_map_firm_to_sequencer_height( genesis_info: &GenesisInfo, commitment_state: &CommitmentState, ) -> Result<(), InvalidState> { - let sequencer_genesis_height = genesis_info.sequencer_start_block_height(); + let sequencer_start_block_height = genesis_info.sequencer_start_block_height(); let rollup_number = commitment_state.firm().number(); - if map_rollup_number_to_sequencer_height(sequencer_genesis_height, rollup_number).is_none() { + let rollup_start_height = genesis_info.rollup_start_block_height(); + if map_rollup_number_to_sequencer_height( + sequencer_start_block_height, + rollup_number, + rollup_start_height, + ) + .is_none() + { Err(InvalidState { commitment_type: "firm", - sequencer_genesis_height: sequencer_genesis_height.value(), + sequencer_start_block_height: sequencer_start_block_height.value(), rollup_number: rollup_number.into(), }) } else { @@ -118,12 +125,19 @@ fn can_map_soft_to_sequencer_height( genesis_info: &GenesisInfo, commitment_state: &CommitmentState, ) -> Result<(), InvalidState> { - let sequencer_genesis_height = genesis_info.sequencer_start_block_height(); + let sequencer_start_block_height = genesis_info.sequencer_start_block_height(); let rollup_number = commitment_state.soft().number(); - if map_rollup_number_to_sequencer_height(sequencer_genesis_height, rollup_number).is_none() { + let rollup_start_height = genesis_info.rollup_start_block_height(); + if map_rollup_number_to_sequencer_height( + sequencer_start_block_height, + rollup_number, + rollup_start_height, + ) + .is_none() + { Err(InvalidState { commitment_type: "soft", - sequencer_genesis_height: sequencer_genesis_height.value(), + sequencer_start_block_height: sequencer_start_block_height.value(), rollup_number: rollup_number.into(), }) } else { @@ -321,6 +335,7 @@ impl State { map_rollup_number_to_sequencer_height( self.sequencer_start_block_height(), self.firm_number().saturating_add(1), + self.rollup_start_block_height(), ) } @@ -328,38 +343,40 @@ impl State { map_rollup_number_to_sequencer_height( self.sequencer_start_block_height(), self.soft_number().saturating_add(1), + self.rollup_start_block_height(), ) } } /// Maps a rollup height to a sequencer height. /// -/// Returns `None` if `sequencer_genesis_height + rollup_number` overflows -/// `u32::MAX`. +/// Returns `None` if `sequencer_start_block_height + rollup_number - rollup_start_block_height + 1` +/// overflows `u32::MAX`. fn map_rollup_number_to_sequencer_height( - sequencer_genesis_height: SequencerHeight, + sequencer_start_block_height: SequencerHeight, rollup_number: u32, + rollup_start_block_height: u64, ) -> Option { - let sequencer_genesis_height = sequencer_genesis_height.value(); + let sequencer_start_block_height = sequencer_start_block_height.value(); let rollup_number: u64 = rollup_number.into(); - let sequencer_height = sequencer_genesis_height.checked_add(rollup_number)?; + let sequencer_height = sequencer_start_block_height + .checked_add(rollup_number)? + .checked_sub(rollup_start_block_height)?; sequencer_height.try_into().ok() } /// Maps a sequencer height to a rollup height. /// -/// Returns `None` if `sequencer_height - sequencer_genesis_height` underflows or if -/// the result does not fit in `u32`. +/// Returns `None` if `sequencer_height - sequencer_start_block_height + rollup_start_block_height` underflows or if the result does not fit in `u32`. pub(super) fn map_sequencer_height_to_rollup_height( - sequencer_genesis_height: SequencerHeight, + sequencer_start_height: SequencerHeight, sequencer_height: SequencerHeight, rollup_start_block_height: u64, ) -> Option { sequencer_height .value() - .checked_sub(sequencer_genesis_height.value())? + .checked_sub(sequencer_start_height.value())? .checked_add(rollup_start_block_height)? - .checked_sub(1)? // offset rollup start block height value .try_into() .ok() } @@ -410,7 +427,7 @@ mod tests { sequencer_start_block_height: 10, sequencer_stop_block_height: 100, celestia_block_variance: 0, - rollup_start_block_height: 1, + rollup_start_block_height: 0, sequencer_chain_id: "test-sequencer-0".to_string(), celestia_chain_id: "test-celestia-0".to_string(), }) @@ -446,7 +463,7 @@ mod tests { fn assert_height_is_correct(left: u32, right: u32, expected: u32) { assert_eq!( SequencerHeight::from(expected), - map_rollup_number_to_sequencer_height(SequencerHeight::from(left), right) + map_rollup_number_to_sequencer_height(SequencerHeight::from(left), right, 0) .expect("left + right is so small, they should never overflow"), ); } diff --git a/crates/astria-conductor/tests/blackbox/firm_only.rs b/crates/astria-conductor/tests/blackbox/firm_only.rs index 184fb9c7e7..0cc2779a30 100644 --- a/crates/astria-conductor/tests/blackbox/firm_only.rs +++ b/crates/astria-conductor/tests/blackbox/firm_only.rs @@ -60,7 +60,7 @@ async fn simple() { sequencer_start_block_height: 1, sequencer_stop_block_height: 10, celestia_block_variance: 10, - rollup_start_block_height: 1, + rollup_start_block_height: 0, ); mount_get_commitment_state!( @@ -143,7 +143,7 @@ async fn submits_two_heights_in_succession() { sequencer_start_block_height: 1, sequencer_stop_block_height: 10, celestia_block_variance: 10, - rollup_start_block_height: 1, + rollup_start_block_height: 0, ); mount_get_commitment_state!( @@ -257,7 +257,7 @@ async fn skips_already_executed_heights() { sequencer_start_block_height: 1, sequencer_stop_block_height: 10, celestia_block_variance: 10, - rollup_start_block_height: 1, + rollup_start_block_height: 0, ); mount_get_commitment_state!( @@ -343,7 +343,7 @@ async fn fetch_from_later_celestia_height() { sequencer_start_block_height: 1, sequencer_stop_block_height: 10, celestia_block_variance: 10, - rollup_start_block_height: 1, + rollup_start_block_height: 0, ); mount_get_commitment_state!( @@ -461,7 +461,7 @@ async fn exits_on_celestia_chain_id_mismatch() { genesis_info!(sequencer_start_block_height: 1, sequencer_stop_block_height: 10, celestia_block_variance: 10, - rollup_start_block_height: 1,), + rollup_start_block_height: 0,), )) .expect(0..) .mount(&mock_grpc.mock_server) @@ -553,7 +553,7 @@ async fn conductor_restarts_after_reaching_stop_block_height() { sequencer_start_block_height: 1, sequencer_stop_block_height: 3, celestia_block_variance: 10, - rollup_start_block_height: 1, + rollup_start_block_height: 0, up_to_n_times: 1, // Only respond once, since updated information is needed after restart. ); @@ -643,7 +643,7 @@ async fn conductor_restarts_after_reaching_stop_block_height() { sequencer_start_block_height: 1, sequencer_stop_block_height: 10, celestia_block_variance: 10, - rollup_start_block_height: 2, + rollup_start_block_height: 1, ); mount_get_commitment_state!( diff --git a/crates/astria-conductor/tests/blackbox/soft_and_firm.rs b/crates/astria-conductor/tests/blackbox/soft_and_firm.rs index 501084ebd3..85ac6afaff 100644 --- a/crates/astria-conductor/tests/blackbox/soft_and_firm.rs +++ b/crates/astria-conductor/tests/blackbox/soft_and_firm.rs @@ -46,7 +46,7 @@ async fn executes_soft_first_then_updates_firm() { sequencer_start_block_height: 1, sequencer_stop_block_height: 10, celestia_block_variance: 10, - rollup_start_block_height: 1, + rollup_start_block_height: 0, ); mount_get_commitment_state!( @@ -182,7 +182,7 @@ async fn executes_firm_then_soft_at_next_height() { sequencer_start_block_height: 1, sequencer_stop_block_height: 10, celestia_block_variance: 10, - rollup_start_block_height: 1, + rollup_start_block_height: 0, ); mount_get_commitment_state!( @@ -343,7 +343,7 @@ async fn missing_block_is_fetched_for_updating_firm_commitment() { sequencer_start_block_height: 1, sequencer_stop_block_height: 10, celestia_block_variance: 10, - rollup_start_block_height: 1, + rollup_start_block_height: 0, ); mount_get_commitment_state!( @@ -474,7 +474,7 @@ async fn conductor_restarts_on_permission_denied() { sequencer_start_block_height: 1, sequencer_stop_block_height: 10, celestia_block_variance: 10, - rollup_start_block_height: 1, + rollup_start_block_height: 0, up_to_n_times: 2, ); @@ -629,7 +629,7 @@ async fn conductor_restarts_after_reaching_stop_height() { sequencer_start_block_height: 1, sequencer_stop_block_height: 3, celestia_block_variance: 10, - rollup_start_block_height: 1, + rollup_start_block_height: 0, up_to_n_times: 1, // We only respond once since this needs to be updated after restart ); @@ -664,10 +664,6 @@ async fn conductor_restarts_after_reaching_stop_height() { test_conductor, sequencer_height: 3, ); - mount_get_filtered_sequencer_block!( - test_conductor, - sequencer_height: 4, - ); // Mount firm blocks for heights 3 and 4 mount_celestia_blobs!( @@ -749,7 +745,7 @@ async fn conductor_restarts_after_reaching_stop_height() { sequencer_start_block_height: 1, sequencer_stop_block_height: 10, celestia_block_variance: 10, - rollup_start_block_height: 2, + rollup_start_block_height: 1, ); mount_get_commitment_state!( diff --git a/crates/astria-conductor/tests/blackbox/soft_only.rs b/crates/astria-conductor/tests/blackbox/soft_only.rs index f91a222b07..cda9e431b7 100644 --- a/crates/astria-conductor/tests/blackbox/soft_only.rs +++ b/crates/astria-conductor/tests/blackbox/soft_only.rs @@ -47,7 +47,7 @@ async fn simple() { sequencer_start_block_height: 1, sequencer_stop_block_height: 10, celestia_block_variance: 10, - rollup_start_block_height: 1, + rollup_start_block_height: 0, ); mount_get_commitment_state!( @@ -122,7 +122,7 @@ async fn submits_two_heights_in_succession() { sequencer_start_block_height: 1, sequencer_stop_block_height: 10, celestia_block_variance: 10, - rollup_start_block_height: 1, + rollup_start_block_height: 0, ); mount_get_commitment_state!( @@ -230,7 +230,7 @@ async fn skips_already_executed_heights() { sequencer_start_block_height: 1, sequencer_stop_block_height: 10, celestia_block_variance: 10, - rollup_start_block_height: 1, + rollup_start_block_height: 0, ); mount_get_commitment_state!( @@ -305,7 +305,7 @@ async fn requests_from_later_genesis_height() { sequencer_start_block_height: 10, sequencer_stop_block_height: 20, celestia_block_variance: 10, - rollup_start_block_height: 1, + rollup_start_block_height: 0, ); mount_get_commitment_state!( @@ -415,7 +415,7 @@ async fn exits_on_sequencer_chain_id_mismatch() { genesis_info!(sequencer_start_block_height: 1, sequencer_stop_block_height: 10, celestia_block_variance: 10, - rollup_start_block_height: 1,), + rollup_start_block_height: 0,), )) .expect(0..) .mount(&mock_grpc.mock_server) @@ -495,7 +495,7 @@ async fn conductor_restarts_after_reaching_stop_block_height() { sequencer_start_block_height: 1, sequencer_stop_block_height: 4, celestia_block_variance: 10, - rollup_start_block_height: 1, + rollup_start_block_height: 0, up_to_n_times: 1, // We need to mount a new genesis info after restart ); @@ -579,7 +579,7 @@ async fn conductor_restarts_after_reaching_stop_block_height() { sequencer_start_block_height: 1, sequencer_stop_block_height: 10, celestia_block_variance: 10, - rollup_start_block_height: 2, + rollup_start_block_height: 1, ); mount_get_commitment_state!( From 2a3ca48140b4f9f928c2d9f3218f50c6a25919fc Mon Sep 17 00:00:00 2001 From: ethanoroshiba Date: Mon, 9 Dec 2024 17:06:36 -0600 Subject: [PATCH 09/27] rustfmt --- crates/astria-conductor/src/executor/state.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/astria-conductor/src/executor/state.rs b/crates/astria-conductor/src/executor/state.rs index d49c4fda73..4835522e9d 100644 --- a/crates/astria-conductor/src/executor/state.rs +++ b/crates/astria-conductor/src/executor/state.rs @@ -367,7 +367,8 @@ fn map_rollup_number_to_sequencer_height( /// Maps a sequencer height to a rollup height. /// -/// Returns `None` if `sequencer_height - sequencer_start_block_height + rollup_start_block_height` underflows or if the result does not fit in `u32`. +/// Returns `None` if `sequencer_height - sequencer_start_block_height + rollup_start_block_height` +/// underflows or if the result does not fit in `u32`. pub(super) fn map_sequencer_height_to_rollup_height( sequencer_start_height: SequencerHeight, sequencer_height: SequencerHeight, From 581424f8884be55260708a435baa117bc8dd972e Mon Sep 17 00:00:00 2001 From: ethanoroshiba Date: Tue, 10 Dec 2024 11:10:45 -0600 Subject: [PATCH 10/27] charts changes to reflect changes in geth --- .../files/genesis/geth-genesis.json | 38 ++++++-- charts/evm-rollup/values.yaml | 94 +++++++++++-------- dev/values/rollup/dev.yaml | 92 ++++++++++-------- 3 files changed, 135 insertions(+), 89 deletions(-) diff --git a/charts/evm-rollup/files/genesis/geth-genesis.json b/charts/evm-rollup/files/genesis/geth-genesis.json index 68027198d0..4e0690b818 100644 --- a/charts/evm-rollup/files/genesis/geth-genesis.json +++ b/charts/evm-rollup/files/genesis/geth-genesis.json @@ -27,20 +27,38 @@ {{- range $key, $value := .Values.genesis.extra }} "{{ $key }}": {{ toPrettyJson $value | indent 8 | trim }}, {{- end }} - {{- if .Values.genesis.extraDataOverride }} - "astriaExtraDataOverride": "{{ .Values.genesis.extraDataOverride }}", - {{- end }} "astriaOverrideGenesisExtraData": {{ .Values.genesis.overrideGenesisExtraData }}, - "astriaSequencerInitialHeight": {{ toString .Values.genesis.sequencerInitialHeight | replace "\"" "" }}, "astriaRollupName": "{{ tpl .Values.genesis.rollupName . }}", - "astriaCelestiaInitialHeight": {{ toString .Values.genesis.celestiaInitialHeight | replace "\"" "" }}, - "astriaCelestiaHeightVariance": {{ toString .Values.genesis.celestiaHeightVariance | replace "\"" "" }}, - "astriaBridgeAddresses": {{ toPrettyJson .Values.genesis.bridgeAddresses | indent 8 | trim }}, - "astriaFeeCollectors": {{ toPrettyJson .Values.genesis.feeCollectors | indent 8 | trim }}, - "astriaEIP1559Params": {{ toPrettyJson .Values.genesis.eip1559Params | indent 8 | trim }}, - "astriaSequencerAddressPrefix": "{{ .Values.genesis.sequencerAddressPrefix }}" {{- if not .Values.global.dev }} {{- else }} + "astriaForks": { + "genesis": { + "height": {{ toString .Values.genesis.forks.genesis.height | replace "\"" "" }}, + "halt": {{ toString .Values.genesis.forks.genesis.halt | replace "\"" "" }}, + "snapshotChecksum": {{ toString .Values.genesis.forks.genesis.snapshotChecksum | replace "\"" "" }}, + {{- if .Values.genesis.forks.genesis.extraDataOverride }} + "astriaExtraDataOverride": {{ toString .Values.genesis.forks.genesis.extraDataOverride }}, + {{- end }} + "feeCollector": {{ toString .Values.genesis.forks.genesis.feeCollector | replace "\"" "" }}, + "EIP1559Params": {{ toPrettyJson .Values.genesis.forks.genesis.eip1559Params | indent 8 | trim }}, + "sequencer": { + "chainId": {{ toString .Values.genesis.forks.genesis.sequencerChainId | replace "\"" "" }}, + "addressPrefix": {{ toString .Values.genesis.forks.genesis.sequencerAddressPrefix | replace "\"" "" }}, + "startHeight": {{ toString .Values.genesis.forks.genesis.sequencerStartHeight | replace "\"" "" }}, + "stopHeight": {{ toString .Values.genesis.forks.genesis.sequencerStopHeight | replace "\"" "" }}, + "rollupStartHeight": {{ toString .Values.genesis.forks.genesis.rollupStartHeight | replace "\"" "" }}, + }, + "celestia": { + "chainId": {{ toString .Values.genesis.forks.genesis.celestiaChainId | replace "\"" "" }}, + "startHeight": {{ toString .Values.genesis.forks.genesis.celestiaStartHeight | replace "\"" "" }}, + "heightVariance": {{ toString .Values.genesis.forks.genesis.celestiaHeightVariance | replace "\"" "" }}, + }, + "bridgeAddresses": {{ toPrettyJson .Values.genesis.forks.genesisbridgeAddresses | indent 8 | trim }}, + } + {{- if .Values.genesis.forks.additionalForks }} + {{ toPrettyJson .Values.genesis.forks.additionalForks | indent 8 | trim }} + {{- end }} + }, {{- end }} }, "difficulty": "0", diff --git a/charts/evm-rollup/values.yaml b/charts/evm-rollup/values.yaml index 6855a776d2..6e15b8f72e 100644 --- a/charts/evm-rollup/values.yaml +++ b/charts/evm-rollup/values.yaml @@ -21,20 +21,63 @@ images: genesis: - ## These values are used to configure the genesis block of the rollup chain - ## no defaults as they are unique to each chain - # The name of the rollup chain, used to generate the Rollup ID rollupName: "" - # Block height to start syncing rollup from, lowest possible is 2 - sequencerInitialHeight: "" - # The first Celestia height to utilize when looking for rollup data - celestiaInitialHeight: "" - # The variance in Celestia height to allow before halting the chain - celestiaHeightVariance: "" - # Will fill the extra data in each block, can be left empty - # can also fill with something unique for your chain. - extraDataOverride: "" + + # The "forks" for upgrading the chain. Contains necessary information for starting + # and, if desired, restarting the chain at a given height. The necessary fields + # for the genesis fork are provided, and additional forks can be added as needed. + forks: + ## These values are used to configure the genesis block of the rollup chain + ## no defaults as they are unique to each chain + genesis: + # 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 + halt: "false" + # Checksum of the snapshot to use upon restart + snapshotChecksum: "" + # Will fill the extra data in each block, can be left empty + # can also fill with something unique for your chain. + extraDataOverride: "" + # Configure the fee collector for the evm tx fees, activated at block heights. + # If not configured, all tx fees will be burned. + feeCollector: "" + # 1: "0xaC21B97d35Bf75A7dAb16f35b111a50e78A72F30" + # Configure EIP-1559 params, activated at block heights. + eip1559Params: {} + # 1: + # minBaseFee: 0 + # elasticityMultiplier: 2 + # baseFeeChangeDenominator: 8 + # The chain id of the sequencer chain + sequencerChainId: "" + # The hrp for bech32m addresses, unlikely to be changed + sequencerAddressPrefix: "astria" + # Block height to start syncing rollup from, lowest possible is 2 + sequencerStartHeight: "" + # Block height (on sequencer) to stop syncing rollup at, continuing to next configuration fork + sequencerStopHeight: "" + # The height to start the rollup chain at, 0 for genesis + rollupStartHeight: "0" + # The chain id of the celestia chain + celestiaChainId: "" + # The first Celestia height to utilize when looking for rollup data + celestiaStartHeight: "" + # The variance in Celestia height to allow before halting the chain + celestiaHeightVariance: "" + # Configure the sequencer bridge addresses and allowed assets if using + # the astria canonical bridge. Recommend removing alloc values if so. + bridgeAddresses: [] + # - address: "684ae50c49a434199199c9c698115391152d7b3f" + # startHeight: 1 + # assetDenom: "nria" + # senderAddress: "0x0000000000000000000000000000000000000000" + # assetPrecision: 9 + # JSON object with any additional desired forks. Can be used with any or all + # of the above fields. Any fields left empty will be populated with the previous + # fork's values. + additionalForks: {} ## These are general configuration values with some recommended defaults @@ -42,34 +85,7 @@ genesis: gasLimit: "50000000" # If set to true the genesis block will contain extra data overrideGenesisExtraData: true - # The hrp for bech32m addresses, unlikely to be changed - sequencerAddressPrefix: "astria" - - ## These values are used to configure astria native bridging - ## Many of the fields have commented out example fields - - # Configure the sequencer bridge addresses and allowed assets if using - # the astria canonical bridge. Recommend removing alloc values if so. - bridgeAddresses: [] - # - address: "684ae50c49a434199199c9c698115391152d7b3f" - # startHeight: 1 - # assetDenom: "nria" - # senderAddress: "0x0000000000000000000000000000000000000000" - # assetPrecision: 9 - - - ## Fee configuration - # Configure the fee collector for the evm tx fees, activated at block heights. - # If not configured, all tx fees will be burned. - feeCollectors: {} - # 1: "0xaC21B97d35Bf75A7dAb16f35b111a50e78A72F30" - # Configure EIP-1559 params, activated at block heights - eip1559Params: {} - # 1: - # minBaseFee: 0 - # elasticityMultiplier: 2 - # baseFeeChangeDenominator: 8 ## Standard Eth Genesis config values # An EVM chain number id, different from the astria rollup name diff --git a/dev/values/rollup/dev.yaml b/dev/values/rollup/dev.yaml index 0d9be85422..dfdb3f9485 100644 --- a/dev/values/rollup/dev.yaml +++ b/dev/values/rollup/dev.yaml @@ -10,18 +10,58 @@ global: evm-rollup: genesis: - ## These values are used to configure the genesis block of the rollup chain - ## no defaults as they are unique to each chain - - # Block height to start syncing rollup from, lowest possible is 2 - sequencerInitialHeight: 2 - # The first Celestia height to utilize when looking for rollup data - celestiaInitialHeight: 2 - # The variance in Celestia height to allow before halting the chain - celestiaHeightVariance: 10 - # Will fill the extra data in each block, can be left empty - # can also fill with something unique for your chain. - extraDataOverride: "" + # The name of the rollup chain, used to generate the Rollup ID + rollupName: "{{ tpl .dev.global.rollupName . }}" + + # The "forks" for upgrading the chain. Contains necessary information for starting + # and, if desired, restarting the chain at a given height. The necessary fields + # for the genesis fork are provided, and additional forks can be added as needed. + forks: + ## These values are used to configure the genesis block of the rollup chain + ## no defaults as they are unique to each chain + genesis: + # 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 + halt: false + # Checksum of the snapshot to use upon restart + snapshotChecksum: "" + # Will fill the extra data in each block, can be left empty + # can also fill with something unique for your chain. + extraDataOverride: "" + # Configure the fee collector for the evm tx fees, activated at block heights. + # If not configured, all tx fees will be burned. + feeCollector: "0xaC21B97d35Bf75A7dAb16f35b111a50e78A72F30" + # Configure EIP-1559 params, activated at block heights. + eip1559Params: {} + # The chain id of the sequencer chain + sequencerChainId: "{{ .dev.global.sequencerChainId }}" + # The hrp for bech32m addresses, unlikely to be changed + sequencerAddressPrefix: "astria" + # Block height to start syncing rollup from, lowest possible is 2 + sequencerStartHeight: 2 + # Block height (on sequencer) to stop syncing rollup at, continuing to next configuration fork + sequencerStopHeight: "" + # The height to start the rollup chain at, 0 for genesis + rollupStartHeight: 0 + # The chain id of the celestia chain + celestiaChainId: "{{ .dev.global.celestiaChainId }}" + # The first Celestia height to utilize when looking for rollup data + celestiaStartHeight: 2 + # The variance in Celestia height to allow before halting the chain + celestiaHeightVariance: 10 + # Configure the sequencer bridge addresses and allowed assets if using + # the astria canonical bridge. Recommend removing alloc values if so. + bridgeAddresses: + - bridgeAddress: "astria13ahqz4pjqfmynk9ylrqv4fwe4957x2p0h5782u" + startHeight: 1 + senderAddress: "0x0000000000000000000000000000000000000000" + assetDenom: "nria" + assetPrecision: 9 + # JSON object with any additional desired forks. Can be used with any or all + # of the above fields. Any fields left empty will be populated with the previous + # fork's values. + additionalForks: {} ## These are general configuration values with some recommended defaults @@ -29,34 +69,6 @@ evm-rollup: gasLimit: "50000000" # If set to true the genesis block will contain extra data overrideGenesisExtraData: true - # The hrp for bech32m addresses, unlikely to be changed - sequencerAddressPrefix: "astria" - - ## These values are used to configure astria native bridging - ## Many of the fields have commented out example fields - - # Configure the sequencer bridge addresses and allowed assets if using - # the astria canonical bridge. Recommend removing alloc values if so. - bridgeAddresses: - - bridgeAddress: "astria13ahqz4pjqfmynk9ylrqv4fwe4957x2p0h5782u" - startHeight: 1 - senderAddress: "0x0000000000000000000000000000000000000000" - assetDenom: "nria" - assetPrecision: 9 - - - ## Fee configuration - - # Configure the fee collector for the evm tx fees, activated at block heights. - # If not configured, all tx fees will be burned. - feeCollectors: - 1: "0xaC21B97d35Bf75A7dAb16f35b111a50e78A72F30" - # Configure EIP-1559 params, activated at block heights - eip1559Params: {} - # 1: - # minBaseFee: 0 - # elasticityMultiplier: 2 - # baseFeeChangeDenominator: 8 ## Standard Eth Genesis config values # Configuration of Eth forks, setting to 0 will enable from height, From 36572d01d8896fb7e9f31e6accd3de91d622b661 Mon Sep 17 00:00:00 2001 From: ethanoroshiba Date: Tue, 10 Dec 2024 11:27:55 -0600 Subject: [PATCH 11/27] fix extraDataOverride naming --- charts/evm-rollup/files/genesis/geth-genesis.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/charts/evm-rollup/files/genesis/geth-genesis.json b/charts/evm-rollup/files/genesis/geth-genesis.json index 4e0690b818..425d8e5319 100644 --- a/charts/evm-rollup/files/genesis/geth-genesis.json +++ b/charts/evm-rollup/files/genesis/geth-genesis.json @@ -37,7 +37,7 @@ "halt": {{ toString .Values.genesis.forks.genesis.halt | replace "\"" "" }}, "snapshotChecksum": {{ toString .Values.genesis.forks.genesis.snapshotChecksum | replace "\"" "" }}, {{- if .Values.genesis.forks.genesis.extraDataOverride }} - "astriaExtraDataOverride": {{ toString .Values.genesis.forks.genesis.extraDataOverride }}, + "extraDataOverride": {{ toString .Values.genesis.forks.genesis.extraDataOverride }}, {{- end }} "feeCollector": {{ toString .Values.genesis.forks.genesis.feeCollector | replace "\"" "" }}, "EIP1559Params": {{ toPrettyJson .Values.genesis.forks.genesis.eip1559Params | indent 8 | trim }}, From 6e790c1552f829a8a20791cddc45466b42521f6e Mon Sep 17 00:00:00 2001 From: ethanoroshiba Date: Tue, 10 Dec 2024 13:11:53 -0600 Subject: [PATCH 12/27] remove rollup start height from geth config --- charts/evm-rollup/files/genesis/geth-genesis.json | 1 - charts/evm-rollup/values.yaml | 2 -- dev/values/rollup/dev.yaml | 2 -- 3 files changed, 5 deletions(-) diff --git a/charts/evm-rollup/files/genesis/geth-genesis.json b/charts/evm-rollup/files/genesis/geth-genesis.json index 425d8e5319..abac182dbe 100644 --- a/charts/evm-rollup/files/genesis/geth-genesis.json +++ b/charts/evm-rollup/files/genesis/geth-genesis.json @@ -46,7 +46,6 @@ "addressPrefix": {{ toString .Values.genesis.forks.genesis.sequencerAddressPrefix | replace "\"" "" }}, "startHeight": {{ toString .Values.genesis.forks.genesis.sequencerStartHeight | replace "\"" "" }}, "stopHeight": {{ toString .Values.genesis.forks.genesis.sequencerStopHeight | replace "\"" "" }}, - "rollupStartHeight": {{ toString .Values.genesis.forks.genesis.rollupStartHeight | replace "\"" "" }}, }, "celestia": { "chainId": {{ toString .Values.genesis.forks.genesis.celestiaChainId | replace "\"" "" }}, diff --git a/charts/evm-rollup/values.yaml b/charts/evm-rollup/values.yaml index 6e15b8f72e..00f583fef4 100644 --- a/charts/evm-rollup/values.yaml +++ b/charts/evm-rollup/values.yaml @@ -58,8 +58,6 @@ genesis: sequencerStartHeight: "" # Block height (on sequencer) to stop syncing rollup at, continuing to next configuration fork sequencerStopHeight: "" - # The height to start the rollup chain at, 0 for genesis - rollupStartHeight: "0" # The chain id of the celestia chain celestiaChainId: "" # The first Celestia height to utilize when looking for rollup data diff --git a/dev/values/rollup/dev.yaml b/dev/values/rollup/dev.yaml index dfdb3f9485..9aa455db59 100644 --- a/dev/values/rollup/dev.yaml +++ b/dev/values/rollup/dev.yaml @@ -42,8 +42,6 @@ evm-rollup: sequencerStartHeight: 2 # Block height (on sequencer) to stop syncing rollup at, continuing to next configuration fork sequencerStopHeight: "" - # The height to start the rollup chain at, 0 for genesis - rollupStartHeight: 0 # The chain id of the celestia chain celestiaChainId: "{{ .dev.global.celestiaChainId }}" # The first Celestia height to utilize when looking for rollup data From 5e562e43ff5b8dada8c8e603f8d576362654efa8 Mon Sep 17 00:00:00 2001 From: ethanoroshiba Date: Thu, 12 Dec 2024 13:42:05 -0600 Subject: [PATCH 13/27] fix erroneous doc comment --- crates/astria-conductor/src/executor/state.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/astria-conductor/src/executor/state.rs b/crates/astria-conductor/src/executor/state.rs index 4835522e9d..2af15654b4 100644 --- a/crates/astria-conductor/src/executor/state.rs +++ b/crates/astria-conductor/src/executor/state.rs @@ -350,7 +350,7 @@ impl State { /// Maps a rollup height to a sequencer height. /// -/// Returns `None` if `sequencer_start_block_height + rollup_number - rollup_start_block_height + 1` +/// Returns `None` if `sequencer_start_block_height + rollup_number - rollup_start_block_height` /// overflows `u32::MAX`. fn map_rollup_number_to_sequencer_height( sequencer_start_block_height: SequencerHeight, From d971d5c3cd0edf1bf75fdfccbe8438e5339e47fa Mon Sep 17 00:00:00 2001 From: Josh Dechant Date: Fri, 13 Dec 2024 09:24:18 -0500 Subject: [PATCH 14/27] stop height of 0 means do not stop --- crates/astria-conductor/src/executor/mod.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/crates/astria-conductor/src/executor/mod.rs b/crates/astria-conductor/src/executor/mod.rs index 9083fcf30d..cce6713649 100644 --- a/crates/astria-conductor/src/executor/mod.rs +++ b/crates/astria-conductor/src/executor/mod.rs @@ -416,7 +416,9 @@ impl Executor { // executing firm blocks, we let execution continue since one more firm block will be // executed before `execute_firm` initiates a restart. If we are in soft-only mode, we // return a `StopHeightExceded::Sequencer` error to signal a restart. - if executable_block.height >= self.state.sequencer_stop_block_height() { + if self.state.sequencer_stop_block_height().value() > 0 + && executable_block.height >= self.state.sequencer_stop_block_height() + { let res = if self.mode.is_with_firm() { info!( height = %executable_block.height, @@ -503,7 +505,9 @@ impl Executor { err, ))] async fn execute_firm(&mut self, block: ReconstructedBlock) -> eyre::Result<()> { - if block.header.height() > self.state.sequencer_stop_block_height() { + if self.state.sequencer_stop_block_height().value() > 0 + && block.header.height() > self.state.sequencer_stop_block_height() + { info!( height = %block.header.height(), "received firm block whose height is greater than stop block height. \ From fa7af2a07e735e20b319d41ae3b56b16d3dc3b91 Mon Sep 17 00:00:00 2001 From: ethanoroshiba Date: Fri, 13 Dec 2024 08:32:54 -0600 Subject: [PATCH 15/27] charts updates --- charts/evm-rollup/Chart.yaml | 4 ++-- charts/evm-stack/Chart.lock | 6 +++--- charts/evm-stack/Chart.yaml | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/charts/evm-rollup/Chart.yaml b/charts/evm-rollup/Chart.yaml index 7ca3fcb1bd..ab83b13991 100644 --- a/charts/evm-rollup/Chart.yaml +++ b/charts/evm-rollup/Chart.yaml @@ -15,13 +15,13 @@ type: application # This is the chart version. This version number should be incremented each time you make changes # to the chart and its templates, including the app version. # Versions are expected to follow Semantic Versioning (https://semver.org/) -version: 1.0.1 +version: 1.0.2 # This is the version number of the application being deployed. This version number should be # incremented each time you make changes to the application. Versions are not expected to # follow Semantic Versioning. They should reflect the version the application is using. # It is recommended to use it with quotes. -appVersion: "1.0.1" +appVersion: "1.0.2" maintainers: - name: wafflesvonmaple diff --git a/charts/evm-stack/Chart.lock b/charts/evm-stack/Chart.lock index 7de99729cf..e2bc6c505f 100644 --- a/charts/evm-stack/Chart.lock +++ b/charts/evm-stack/Chart.lock @@ -4,7 +4,7 @@ dependencies: version: 0.4.0 - name: evm-rollup repository: file://../evm-rollup - version: 1.0.1 + version: 1.0.2 - name: composer repository: file://../composer version: 1.0.0 @@ -20,5 +20,5 @@ dependencies: - name: blockscout-stack repository: https://blockscout.github.io/helm-charts version: 1.6.8 -digest: sha256:4715e557b6ceb0fa85c9efe86f5b26d665783f0be9162728efe808fa3a35d727 -generated: "2024-12-12T19:52:24.992658+02:00" +digest: sha256:cabd3646c4478bb720c86bdc692243c20717d667ad8230154656d9c965197c6a +generated: "2024-12-13T08:32:31.235575-06:00" diff --git a/charts/evm-stack/Chart.yaml b/charts/evm-stack/Chart.yaml index dea0641cf7..a1990ea05e 100644 --- a/charts/evm-stack/Chart.yaml +++ b/charts/evm-stack/Chart.yaml @@ -15,7 +15,7 @@ type: application # This is the chart version. This version number should be incremented each time you make changes # to the chart and its templates, including the app version. # Versions are expected to follow Semantic Versioning (https://semver.org/) -version: 1.0.6 +version: 1.0.7 dependencies: - name: celestia-node @@ -23,7 +23,7 @@ dependencies: repository: "file://../celestia-node" condition: celestia-node.enabled - name: evm-rollup - version: 1.0.1 + version: 1.0.2 repository: "file://../evm-rollup" - name: composer version: 1.0.0 From ea80fbbc623f05ff1d990310656c078887b25d43 Mon Sep 17 00:00:00 2001 From: ethanoroshiba Date: Fri, 13 Dec 2024 10:42:43 -0600 Subject: [PATCH 16/27] fix charts --- .../files/genesis/geth-genesis.json | 48 +++++++++++-------- dev/values/rollup/dev.yaml | 23 ++------- 2 files changed, 32 insertions(+), 39 deletions(-) diff --git a/charts/evm-rollup/files/genesis/geth-genesis.json b/charts/evm-rollup/files/genesis/geth-genesis.json index abac182dbe..0a562717a7 100644 --- a/charts/evm-rollup/files/genesis/geth-genesis.json +++ b/charts/evm-rollup/files/genesis/geth-genesis.json @@ -15,10 +15,10 @@ {{- if .Values.genesis.cancunTime }} "cancunTime": {{ toString .Values.genesis.cancunTime | replace "\"" "" }}, {{- end }} - {{- if .Values.genesis.cancunTime }} + {{- if .Values.genesis.pragueTime }} "pragueTime": {{ toString .Values.genesis.pragueTime | replace "\"" "" }}, {{- end }} - {{- if .Values.genesis.cancunTime }} + {{- if .Values.genesis.verkleTime }} "verkleTime": {{ toString .Values.genesis.verkleTime | replace "\"" "" }}, {{- end }} "terminalTotalDifficulty": 0, @@ -29,35 +29,41 @@ {{- end }} "astriaOverrideGenesisExtraData": {{ .Values.genesis.overrideGenesisExtraData }}, "astriaRollupName": "{{ tpl .Values.genesis.rollupName . }}", - {{- if not .Values.global.dev }} - {{- else }} "astriaForks": { "genesis": { - "height": {{ toString .Values.genesis.forks.genesis.height | replace "\"" "" }}, - "halt": {{ toString .Values.genesis.forks.genesis.halt | replace "\"" "" }}, - "snapshotChecksum": {{ toString .Values.genesis.forks.genesis.snapshotChecksum | replace "\"" "" }}, - {{- if .Values.genesis.forks.genesis.extraDataOverride }} - "extraDataOverride": {{ toString .Values.genesis.forks.genesis.extraDataOverride }}, + "height": {{ toString .Values.genesis.forks.launch.height | replace "\"" "" }}, + {{- if .Values.genesis.forks.launch.halt }} + "halt": {{ toString .Values.genesis.forks.launch.halt | replace "\"" "" }}, + {{- end }} + {{- if .Values.genesis.forks.launch.snapshotChecksum }} + "snapshotChecksum": {{ toString .Values.genesis.forks.launch.snapshotChecksum }}, + {{- end }} + {{- if .Values.genesis.forks.launch.extraDataOverride }} + "extraDataOverride": {{ toString .Values.genesis.forks.launch.extraDataOverride }}, + {{- end }} + "feeCollector": "{{ .Values.genesis.forks.launch.feeCollector }}", + {{- if .Values.genesis.forks.launch.eip1559Params }} + "eip1559Params": {{ toPrettyJson .Values.genesis.forks.launch.eip1559Params | indent 8 | trim }}, {{- end }} - "feeCollector": {{ toString .Values.genesis.forks.genesis.feeCollector | replace "\"" "" }}, - "EIP1559Params": {{ toPrettyJson .Values.genesis.forks.genesis.eip1559Params | indent 8 | trim }}, "sequencer": { - "chainId": {{ toString .Values.genesis.forks.genesis.sequencerChainId | replace "\"" "" }}, - "addressPrefix": {{ toString .Values.genesis.forks.genesis.sequencerAddressPrefix | replace "\"" "" }}, - "startHeight": {{ toString .Values.genesis.forks.genesis.sequencerStartHeight | replace "\"" "" }}, - "stopHeight": {{ toString .Values.genesis.forks.genesis.sequencerStopHeight | replace "\"" "" }}, + "chainId": "{{ tpl .Values.genesis.forks.launch.sequencerChainId . }}", + "addressPrefix": "{{ .Values.genesis.forks.launch.sequencerAddressPrefix }}", + "startHeight": {{ toString .Values.genesis.forks.launch.sequencerStartHeight | replace "\"" "" }}, + "stopHeight": {{ toString .Values.genesis.forks.launch.sequencerStopHeight | replace "\"" "" }} }, "celestia": { - "chainId": {{ toString .Values.genesis.forks.genesis.celestiaChainId | replace "\"" "" }}, - "startHeight": {{ toString .Values.genesis.forks.genesis.celestiaStartHeight | replace "\"" "" }}, - "heightVariance": {{ toString .Values.genesis.forks.genesis.celestiaHeightVariance | replace "\"" "" }}, + "chainId": "{{ tpl .Values.genesis.forks.launch.celestiaChainId . }}", + "startHeight": {{ toString .Values.genesis.forks.launch.celestiaStartHeight | replace "\"" "" }}, + "heightVariance": {{ toString .Values.genesis.forks.launch.celestiaHeightVariance | replace "\"" "" }} }, - "bridgeAddresses": {{ toPrettyJson .Values.genesis.forks.genesisbridgeAddresses | indent 8 | trim }}, + "bridgeAddresses": {{ toPrettyJson .Values.genesis.forks.launch.bridgeAddresses | indent 8 | trim }} } {{- if .Values.genesis.forks.additionalForks }} - {{ toPrettyJson .Values.genesis.forks.additionalForks | indent 8 | trim }} + , {{ toPrettyJson .Values.genesis.forks.additionalForks | indent 8 | trim }} {{- end }} - }, + } + {{- if not .Values.global.dev }} + {{- else }} {{- end }} }, "difficulty": "0", diff --git a/dev/values/rollup/dev.yaml b/dev/values/rollup/dev.yaml index 9aa455db59..7f7af87cd9 100644 --- a/dev/values/rollup/dev.yaml +++ b/dev/values/rollup/dev.yaml @@ -11,7 +11,7 @@ global: evm-rollup: genesis: # The name of the rollup chain, used to generate the Rollup ID - rollupName: "{{ tpl .dev.global.rollupName . }}" + rollupName: "{{ .Values.global.rollupName }}" # The "forks" for upgrading the chain. Contains necessary information for starting # and, if desired, restarting the chain at a given height. The necessary fields @@ -19,31 +19,22 @@ evm-rollup: forks: ## These values are used to configure the genesis block of the rollup chain ## no defaults as they are unique to each chain - genesis: + 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 - halt: false - # Checksum of the snapshot to use upon restart - snapshotChecksum: "" - # Will fill the extra data in each block, can be left empty - # can also fill with something unique for your chain. - extraDataOverride: "" # Configure the fee collector for the evm tx fees, activated at block heights. # If not configured, all tx fees will be burned. feeCollector: "0xaC21B97d35Bf75A7dAb16f35b111a50e78A72F30" - # Configure EIP-1559 params, activated at block heights. - eip1559Params: {} # The chain id of the sequencer chain - sequencerChainId: "{{ .dev.global.sequencerChainId }}" + sequencerChainId: "{{ .Values.global.sequencerChainId }}" # The hrp for bech32m addresses, unlikely to be changed sequencerAddressPrefix: "astria" # Block height to start syncing rollup from, lowest possible is 2 sequencerStartHeight: 2 # Block height (on sequencer) to stop syncing rollup at, continuing to next configuration fork - sequencerStopHeight: "" + sequencerStopHeight: 0 # The chain id of the celestia chain - celestiaChainId: "{{ .dev.global.celestiaChainId }}" + celestiaChainId: "{{ .Values.global.celestiaChainId }}" # The first Celestia height to utilize when looking for rollup data celestiaStartHeight: 2 # The variance in Celestia height to allow before halting the chain @@ -56,10 +47,6 @@ evm-rollup: senderAddress: "0x0000000000000000000000000000000000000000" assetDenom: "nria" assetPrecision: 9 - # JSON object with any additional desired forks. Can be used with any or all - # of the above fields. Any fields left empty will be populated with the previous - # fork's values. - additionalForks: {} ## These are general configuration values with some recommended defaults From 22edac8a702550e8ac80033092eb6c665dc46d68 Mon Sep 17 00:00:00 2001 From: ethanoroshiba Date: Fri, 13 Dec 2024 10:46:58 -0600 Subject: [PATCH 17/27] Fix values --- charts/evm-rollup/values.yaml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/charts/evm-rollup/values.yaml b/charts/evm-rollup/values.yaml index 5067de8185..2a30d9af4b 100644 --- a/charts/evm-rollup/values.yaml +++ b/charts/evm-rollup/values.yaml @@ -30,7 +30,7 @@ genesis: forks: ## These values are used to configure the genesis block of the rollup chain ## no defaults as they are unique to each chain - genesis: + 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 @@ -56,7 +56,8 @@ genesis: sequencerAddressPrefix: "astria" # Block height to start syncing rollup from, lowest possible is 2 sequencerStartHeight: "" - # Block height (on sequencer) to stop syncing rollup at, continuing to next configuration fork + # Block height (on sequencer) to stop syncing rollup at, continuing to next configuration fork. + # A height of 0 indicates no stop height. sequencerStopHeight: "" # The chain id of the celestia chain celestiaChainId: "" From 2f7ed7377b1b31d056eb63df675aaa73954b4bdc Mon Sep 17 00:00:00 2001 From: ethanoroshiba Date: Fri, 13 Dec 2024 15:07:31 -0600 Subject: [PATCH 18/27] requested changes and fix ibc tests --- crates/astria-conductor/src/celestia/mod.rs | 3 +- .../astria-conductor/src/conductor/inner.rs | 10 ++- crates/astria-conductor/src/executor/mod.rs | 89 ++++++++++++------- crates/astria-conductor/src/executor/state.rs | 21 +++-- crates/astria-core/src/execution/v1/mod.rs | 44 ++++++--- dev/values/rollup/ibc-bridge-test.yaml | 44 +++++++-- 6 files changed, 146 insertions(+), 65 deletions(-) diff --git a/crates/astria-conductor/src/celestia/mod.rs b/crates/astria-conductor/src/celestia/mod.rs index 25c384eb38..0a0a0df9d7 100644 --- a/crates/astria-conductor/src/celestia/mod.rs +++ b/crates/astria-conductor/src/celestia/mod.rs @@ -168,6 +168,7 @@ impl Reader { .await } + // TODO(https://github.com/astriaorg/astria/issues/1879): refactor to not return an empty tuple #[instrument(skip_all, err)] async fn initialize( &mut self, @@ -198,7 +199,7 @@ impl Reader { .wrap_err("failed to get sequencer chain ID")?; let expected_sequencer_chain_id = executor.sequencer_chain_id(); ensure!( - expected_sequencer_chain_id == actual_sequencer_chain_id.to_string(), + expected_sequencer_chain_id == actual_sequencer_chain_id.as_str(), "expected Celestia chain id `{expected_sequencer_chain_id}` does not match \ actual: `{actual_sequencer_chain_id}`" ); diff --git a/crates/astria-conductor/src/conductor/inner.rs b/crates/astria-conductor/src/conductor/inner.rs index c16aab5e96..8f6c0c467d 100644 --- a/crates/astria-conductor/src/conductor/inner.rs +++ b/crates/astria-conductor/src/conductor/inner.rs @@ -331,14 +331,20 @@ mod tests { assert!(super::check_for_restart("executor", &err.unwrap_err())); let celestia_height_error: Result<&str, super::StopHeightExceded> = - Err(super::StopHeightExceded::Celestia); + Err(super::StopHeightExceded::Celestia { + firm_height: 1, + stop_height: 1, + }); let err = celestia_height_error.wrap_err("wrapper_1"); let err = err.wrap_err("wrapper_2"); let err = err.wrap_err("wrapper_3"); assert!(super::check_for_restart("executor", &err.unwrap_err())); let sequencer_height_error: Result<&str, super::StopHeightExceded> = - Err(super::StopHeightExceded::Sequencer); + Err(super::StopHeightExceded::Sequencer { + soft_height: 1, + stop_height: 1, + }); let err = sequencer_height_error.wrap_err("wrapper_1"); let err = err.wrap_err("wrapper_2"); let err = err.wrap_err("wrapper_3"); diff --git a/crates/astria-conductor/src/executor/mod.rs b/crates/astria-conductor/src/executor/mod.rs index cce6713649..4277adc843 100644 --- a/crates/astria-conductor/src/executor/mod.rs +++ b/crates/astria-conductor/src/executor/mod.rs @@ -68,11 +68,27 @@ pub(crate) struct StateNotInit; pub(crate) struct StateIsInit; #[derive(Debug, thiserror::Error)] -pub(crate) enum StopHeightExceded { - #[error("firm height exceeded sequencer stop height")] - Celestia, - #[error("soft height met or exceeded sequencer stop height")] - Sequencer, +pub(super) enum StopHeightExceded { + #[error("firm height {firm_height} exceeded sequencer stop height {stop_height}")] + Celestia { firm_height: u64, stop_height: u64 }, + #[error("soft height {soft_height} met or exceeded sequencer stop height {stop_height}")] + Sequencer { soft_height: u64, stop_height: u64 }, +} + +impl StopHeightExceded { + fn celestia(firm_height: u64, stop_height: u64) -> Self { + StopHeightExceded::Celestia { + firm_height, + stop_height, + } + } + + fn sequencer(soft_height: u64, stop_height: u64) -> Self { + StopHeightExceded::Sequencer { + soft_height, + stop_height, + } + } } #[derive(Debug, thiserror::Error)] @@ -416,29 +432,24 @@ impl Executor { // executing firm blocks, we let execution continue since one more firm block will be // executed before `execute_firm` initiates a restart. If we are in soft-only mode, we // return a `StopHeightExceded::Sequencer` error to signal a restart. - if self.state.sequencer_stop_block_height().value() > 0 - && executable_block.height >= self.state.sequencer_stop_block_height() - { - let res = if self.mode.is_with_firm() { - info!( - height = %executable_block.height, - "received soft block whose height is greater than or equal to stop block height in {} mode. \ - dropping soft block and waiting for next firm block before attempting restart", - self.mode, - ); - Ok(()) - } else { + if self.is_soft_block_height_exceded(&executable_block) { + if self.mode.is_with_firm() { info!( height = %executable_block.height, - "received soft block whose height is greater than or equal to stop block \ - height in soft only mode. shutting down and attempting restart", + "received soft block whose height is greater than or equal to stop block height. \ + dropping soft block and waiting for next firm block before attempting restart. \ + will continue logging info of soft blocks until restart", ); - Err(StopHeightExceded::Sequencer).wrap_err( - "soft height met or exceeded sequencer stop height, attempting restart with \ - new genesis info", - ) - }; - return res; + return Ok(()); + } + return Err(StopHeightExceded::sequencer( + executable_block.height.value(), + self.state.sequencer_stop_block_height(), + )) + .wrap_err( + "soft height met or exceeded sequencer stop height, attempting restart with new \ + genesis info", + ); } let expected_height = self.state.next_expected_soft_sequencer_height(); @@ -505,15 +516,12 @@ impl Executor { err, ))] async fn execute_firm(&mut self, block: ReconstructedBlock) -> eyre::Result<()> { - if self.state.sequencer_stop_block_height().value() > 0 - && block.header.height() > self.state.sequencer_stop_block_height() - { - info!( - height = %block.header.height(), - "received firm block whose height is greater than stop block height. \ - exiting and attempting restart with new genesis info", - ); - return Err(StopHeightExceded::Celestia).wrap_err( + if self.is_firm_block_height_exceded(&block) { + return Err(StopHeightExceded::celestia( + block.header.height().value(), + self.state.sequencer_stop_block_height(), + )) + .wrap_err( "firm height exceeded sequencer stop height, attempting restart with new genesis \ info", ); @@ -731,6 +739,19 @@ impl Executor { self.mode, ) } + + /// Returns whether the height of the soft block is greater than or equal to the stop block + /// height. + fn is_soft_block_height_exceded(&self, block: &ExecutableBlock) -> bool { + let stop_height = self.state.sequencer_stop_block_height(); + stop_height > 0 && block.height.value() >= stop_height + } + + /// Returns whether the height of the firm block is greater than the stop block height. + fn is_firm_block_height_exceded(&self, block: &ReconstructedBlock) -> bool { + let stop_height = self.state.sequencer_stop_block_height(); + stop_height > 0 && block.header.height().value() > stop_height + } } #[instrument(skip_all)] diff --git a/crates/astria-conductor/src/executor/state.rs b/crates/astria-conductor/src/executor/state.rs index 2af15654b4..a9337b6d0c 100644 --- a/crates/astria-conductor/src/executor/state.rs +++ b/crates/astria-conductor/src/executor/state.rs @@ -113,7 +113,7 @@ fn can_map_firm_to_sequencer_height( { Err(InvalidState { commitment_type: "firm", - sequencer_start_block_height: sequencer_start_block_height.value(), + sequencer_start_block_height, rollup_number: rollup_number.into(), }) } else { @@ -137,7 +137,7 @@ fn can_map_soft_to_sequencer_height( { Err(InvalidState { commitment_type: "soft", - sequencer_start_block_height: sequencer_start_block_height.value(), + sequencer_start_block_height, rollup_number: rollup_number.into(), }) } else { @@ -236,9 +236,9 @@ forward_impls!( [soft_hash -> Bytes], [celestia_block_variance -> u64], [rollup_id -> RollupId], - [sequencer_start_block_height -> SequencerHeight], + [sequencer_start_block_height -> u64], [celestia_base_block_height -> u64], - [sequencer_stop_block_height -> SequencerHeight], + [sequencer_stop_block_height -> u64], [rollup_start_block_height -> u64] ); @@ -307,11 +307,11 @@ impl State { self.genesis_info.celestia_block_variance() } - fn sequencer_start_block_height(&self) -> SequencerHeight { + fn sequencer_start_block_height(&self) -> u64 { self.genesis_info.sequencer_start_block_height() } - fn sequencer_stop_block_height(&self) -> SequencerHeight { + fn sequencer_stop_block_height(&self) -> u64 { self.genesis_info.sequencer_stop_block_height() } @@ -353,11 +353,10 @@ impl State { /// Returns `None` if `sequencer_start_block_height + rollup_number - rollup_start_block_height` /// overflows `u32::MAX`. fn map_rollup_number_to_sequencer_height( - sequencer_start_block_height: SequencerHeight, + sequencer_start_block_height: u64, rollup_number: u32, rollup_start_block_height: u64, ) -> Option { - let sequencer_start_block_height = sequencer_start_block_height.value(); let rollup_number: u64 = rollup_number.into(); let sequencer_height = sequencer_start_block_height .checked_add(rollup_number)? @@ -370,13 +369,13 @@ fn map_rollup_number_to_sequencer_height( /// Returns `None` if `sequencer_height - sequencer_start_block_height + rollup_start_block_height` /// underflows or if the result does not fit in `u32`. pub(super) fn map_sequencer_height_to_rollup_height( - sequencer_start_height: SequencerHeight, + sequencer_start_height: u64, sequencer_height: SequencerHeight, rollup_start_block_height: u64, ) -> Option { sequencer_height .value() - .checked_sub(sequencer_start_height.value())? + .checked_sub(sequencer_start_height)? .checked_add(rollup_start_block_height)? .try_into() .ok() @@ -464,7 +463,7 @@ mod tests { fn assert_height_is_correct(left: u32, right: u32, expected: u32) { assert_eq!( SequencerHeight::from(expected), - map_rollup_number_to_sequencer_height(SequencerHeight::from(left), right, 0) + map_rollup_number_to_sequencer_height(left.into(), right, 0) .expect("left + right is so small, they should never overflow"), ); } diff --git a/crates/astria-core/src/execution/v1/mod.rs b/crates/astria-core/src/execution/v1/mod.rs index 7ad814638e..7bfa96e4b4 100644 --- a/crates/astria-core/src/execution/v1/mod.rs +++ b/crates/astria-core/src/execution/v1/mod.rs @@ -23,6 +23,14 @@ impl GenesisInfoError { fn no_rollup_id() -> Self { Self(GenesisInfoErrorKind::NoRollupId) } + + fn invalid_sequencer_id(source: tendermint::Error) -> Self { + Self(GenesisInfoErrorKind::InvalidSequencerId(source)) + } + + fn invalid_celestia_id(source: celestia_tendermint::Error) -> Self { + Self(GenesisInfoErrorKind::InvalidCelestiaId(source)) + } } #[derive(Debug, thiserror::Error)] @@ -31,6 +39,10 @@ enum GenesisInfoErrorKind { IncorrectRollupIdLength(IncorrectRollupIdLength), #[error("`rollup_id` was not set")] NoRollupId, + #[error("invalid tendermint chain ID for sequencer")] + InvalidSequencerId(#[source] tendermint::Error), + #[error("invalid celestia-tendermint chain ID for celestia")] + InvalidCelestiaId(#[source] celestia_tendermint::Error), } /// Genesis Info required from a rollup to start an execution client. @@ -57,9 +69,9 @@ pub struct GenesisInfo { /// The rollup block number to map to the sequencer start block height. rollup_start_block_height: u64, /// The chain ID of the sequencer network. - sequencer_chain_id: String, + sequencer_chain_id: tendermint::chain::Id, /// The chain ID of the celestia network. - celestia_chain_id: String, + celestia_chain_id: celestia_tendermint::chain::Id, } impl GenesisInfo { @@ -69,13 +81,13 @@ impl GenesisInfo { } #[must_use] - pub fn sequencer_start_block_height(&self) -> tendermint::block::Height { - self.sequencer_start_block_height + pub fn sequencer_start_block_height(&self) -> u64 { + self.sequencer_start_block_height.into() } #[must_use] - pub fn sequencer_stop_block_height(&self) -> tendermint::block::Height { - self.sequencer_stop_block_height + pub fn sequencer_stop_block_height(&self) -> u64 { + self.sequencer_stop_block_height.into() } #[must_use] @@ -84,13 +96,13 @@ impl GenesisInfo { } #[must_use] - pub fn sequencer_chain_id(&self) -> &str { + pub fn sequencer_chain_id(&self) -> &tendermint::chain::Id { &self.sequencer_chain_id } #[must_use] pub fn celestia_chain_id(&self) -> &str { - &self.celestia_chain_id + self.celestia_chain_id.as_str() } #[must_use] @@ -124,6 +136,14 @@ impl Protobuf for GenesisInfo { }; let rollup_id = RollupId::try_from_raw_ref(rollup_id) .map_err(Self::Error::incorrect_rollup_id_length)?; + let sequencer_chain_id = sequencer_chain_id + .clone() + .try_into() + .map_err(Self::Error::invalid_sequencer_id)?; + let celestia_chain_id = celestia_chain_id + .clone() + .try_into() + .map_err(Self::Error::invalid_celestia_id)?; Ok(Self { rollup_id, @@ -131,8 +151,8 @@ impl Protobuf for GenesisInfo { sequencer_stop_block_height: (*sequencer_stop_block_height).into(), celestia_block_variance: *celestia_block_variance, rollup_start_block_height: *rollup_start_block_height, - sequencer_chain_id: sequencer_chain_id.clone(), - celestia_chain_id: celestia_chain_id.clone(), + sequencer_chain_id, + celestia_chain_id, }) } @@ -164,8 +184,8 @@ impl Protobuf for GenesisInfo { sequencer_stop_block_height, celestia_block_variance: *celestia_block_variance, rollup_start_block_height: *rollup_start_block_height, - sequencer_chain_id: sequencer_chain_id.clone(), - celestia_chain_id: celestia_chain_id.clone(), + sequencer_chain_id: sequencer_chain_id.to_string(), + celestia_chain_id: celestia_chain_id.to_string(), } } } diff --git a/dev/values/rollup/ibc-bridge-test.yaml b/dev/values/rollup/ibc-bridge-test.yaml index b5f198927c..9d546a37a2 100644 --- a/dev/values/rollup/ibc-bridge-test.yaml +++ b/dev/values/rollup/ibc-bridge-test.yaml @@ -1,12 +1,46 @@ # this file contains overrides that are used for the ibc bridge tests +global: + rollupName: astria + sequencerChainId: sequencer-test-chain-0 + celestiaChainId: celestia-local-0 evm-rollup: genesis: - bridgeAddresses: - - bridgeAddress: "astria1d7zjjljc0dsmxa545xkpwxym86g8uvvwhtezcr" - startHeight: 1 - assetDenom: "transfer/channel-0/utia" - assetPrecision: 6 + # The name of the rollup chain, used to generate the Rollup ID + rollupName: "{{ .Values.global.rollupName }}" + + # The "forks" for upgrading the chain. Contains necessary information for starting + # and, if desired, restarting the chain at a given height. The necessary fields + # for the genesis fork are provided, and additional forks can be added as needed. + forks: + ## These values are used to configure the genesis block of the rollup chain + ## no defaults as they are unique to each chain + launch: + # The block height to start the chain at, 0 for genesis + height: 0 + # Configure the fee collector for the evm tx fees, activated at block heights. + # If not configured, all tx fees will be burned. + feeCollector: "0xaC21B97d35Bf75A7dAb16f35b111a50e78A72F30" + # The chain id of the sequencer chain + sequencerChainId: "{{ .Values.global.sequencerChainId }}" + # The hrp for bech32m addresses, unlikely to be changed + sequencerAddressPrefix: "astria" + # Block height to start syncing rollup from, lowest possible is 2 + sequencerStartHeight: 2 + # Block height (on sequencer) to stop syncing rollup at, continuing to next configuration fork + sequencerStopHeight: 0 + # The chain id of the celestia chain + celestiaChainId: "{{ .Values.global.celestiaChainId }}" + # The first Celestia height to utilize when looking for rollup data + celestiaStartHeight: 2 + # The variance in Celestia height to allow before halting the chain + celestiaHeightVariance: 10 + bridgeAddresses: + - bridgeAddress: "astria1d7zjjljc0dsmxa545xkpwxym86g8uvvwhtezcr" + startHeight: 1 + assetDenom: "transfer/channel-0/utia" + assetPrecision: 6 + alloc: - address: "0x4e59b44847b379578588920cA78FbF26c0B4956C" value: From 9f0deb687172b3a913b6c230792253b547cc8509 Mon Sep 17 00:00:00 2001 From: ethanoroshiba Date: Mon, 16 Dec 2024 11:05:41 -0600 Subject: [PATCH 19/27] genesis improvements, new smoke test --- .github/workflows/docker-build.yml | 31 +++++ charts/deploy.just | 5 +- .../files/genesis/geth-genesis.json | 113 +++++++++++++----- charts/evm-rollup/values.yaml | 40 +++---- dev/values/rollup/dev.yaml | 34 +++--- dev/values/rollup/ibc-bridge-test.yaml | 30 ++--- 6 files changed, 168 insertions(+), 85 deletions(-) diff --git a/.github/workflows/docker-build.yml b/.github/workflows/docker-build.yml index 232ae7c07c..e4a56925da 100644 --- a/.github/workflows/docker-build.yml +++ b/.github/workflows/docker-build.yml @@ -165,6 +165,37 @@ jobs: run: | TAG=sha-$(git rev-parse --short HEAD) just run-smoke-test $TAG + smoke-test-evm-rollup-restart: + needs: [run_checker, composer, conductor, sequencer, sequencer-relayer, evm-bridge-withdrawer, cli] + if: (github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == 'astriaorg/astria') && (github.event_name == 'merge_group' || needs.run_checker.outputs.run_docker == 'true') + runs-on: buildjet-8vcpu-ubuntu-2204 + steps: + - uses: actions/checkout@v4 + - name: Install just + uses: taiki-e/install-action@just + - name: Install kind + uses: helm/kind-action@v1 + with: + install_only: true + - name: Log in to GHCR + uses: docker/login-action@v2 + with: + registry: ghcr.io + username: ${{ github.actor }} + password: ${{ secrets.GITHUB_TOKEN }} + - name: Setup Smoke Test Environment + timeout-minutes: 10 + run: | + TAG=sha-$(git rev-parse --short HEAD) + just deploy cluster + kubectl create secret generic regcred --from-file=.dockerconfigjson=$HOME/.docker/config.json --type=kubernetes.io/dockerconfigjson + echo -e "\n\nDeploying with astria images tagged $TAG" + just --set defaultValues "dev/values/rollup/evm-restart-test.yaml" deploy smoke-test $TAG + - name: Run Smoke test + timeout-minutes: 3 + run: | + TAG=sha-$(git rev-parse --short HEAD) + just run-smoke-test $TAG smoke-cli: needs: [run_checker, composer, conductor, sequencer, sequencer-relayer, evm-bridge-withdrawer, cli] diff --git a/charts/deploy.just b/charts/deploy.just index 301620431e..279434c685 100644 --- a/charts/deploy.just +++ b/charts/deploy.just @@ -170,8 +170,9 @@ clean-persisted-data: deploy-local-metrics: kubectl apply -f kubernetes/metrics-server-local.yml +defaultValues := "dev/values/rollup/dev.yaml" defaultTag := "" -deploy-smoke-test tag=defaultTag: +deploy-smoke-test tag=defaultTag values=defaultValues: @echo "Deploying ingress controller..." && just deploy ingress-controller > /dev/null @just wait-for-ingress-controller > /dev/null @echo "Deploying local celestia instance..." && just deploy celestia-local > /dev/null @@ -184,7 +185,7 @@ deploy-smoke-test tag=defaultTag: {{ if tag != '' { replace('--set images.sequencer.devTag=# --set sequencer-relayer.images.sequencerRelayer.devTag=#', '#', tag) } else { '' } }} \ --create-namespace > /dev/null @just wait-for-sequencer > /dev/null - @echo "Starting EVM rollup..." && helm install -n astria-dev-cluster astria-chain-chart ./charts/evm-stack -f dev/values/rollup/dev.yaml \ + @echo "Starting EVM rollup..." && helm install -n astria-dev-cluster astria-chain-chart ./charts/evm-stack -f {{ values }} \ {{ if tag != '' { replace('--set evm-rollup.images.conductor.devTag=# --set composer.images.composer.devTag=# --set evm-bridge-withdrawer.images.evmBridgeWithdrawer.devTag=#', '#', tag) } else { '' } }} \ --set blockscout-stack.enabled=false \ --set postgresql.enabled=false \ diff --git a/charts/evm-rollup/files/genesis/geth-genesis.json b/charts/evm-rollup/files/genesis/geth-genesis.json index 0a562717a7..83618bf3ce 100644 --- a/charts/evm-rollup/files/genesis/geth-genesis.json +++ b/charts/evm-rollup/files/genesis/geth-genesis.json @@ -30,42 +30,95 @@ "astriaOverrideGenesisExtraData": {{ .Values.genesis.overrideGenesisExtraData }}, "astriaRollupName": "{{ tpl .Values.genesis.rollupName . }}", "astriaForks": { - "genesis": { - "height": {{ toString .Values.genesis.forks.launch.height | replace "\"" "" }}, - {{- if .Values.genesis.forks.launch.halt }} - "halt": {{ toString .Values.genesis.forks.launch.halt | replace "\"" "" }}, + {{- $forks := .Values.genesis.forks }} + {{- $index := 0 }} + {{- $lastIndex := sub (len $forks) 1 }} + {{- range $key, $value := .Values.genesis.forks }} + "{{ $key }}": { + {{- $fields := list }} + {{- with $value }} + + {{- if .height }} + {{- $fields = append $fields (printf "\"height\": %s" (toString .height | replace "\"" "")) }} {{- end }} - {{- if .Values.genesis.forks.launch.snapshotChecksum }} - "snapshotChecksum": {{ toString .Values.genesis.forks.launch.snapshotChecksum }}, + + {{- if .halt }} + {{- $fields = append $fields (printf "\"halt\": %s" (toString .halt | replace "\"" "")) }} {{- end }} - {{- if .Values.genesis.forks.launch.extraDataOverride }} - "extraDataOverride": {{ toString .Values.genesis.forks.launch.extraDataOverride }}, + + {{- if .snapshotChecksum }} + {{- $fields = append $fields (printf "\"snapshotChecksum\": %s" (toString .snapshotChecksum)) }} {{- end }} - "feeCollector": "{{ .Values.genesis.forks.launch.feeCollector }}", - {{- if .Values.genesis.forks.launch.eip1559Params }} - "eip1559Params": {{ toPrettyJson .Values.genesis.forks.launch.eip1559Params | indent 8 | trim }}, + + {{- if .extraDataOverride }} + {{- $fields = append $fields (printf "\"extraDataOverride\": %s" (toString .extraDataOverride)) }} + {{- end }} + + {{- if .feeCollector }} + {{- $fields = append $fields (printf "\"feeCollector\": \"%s\"" (toString .feeCollector)) }} + {{- end }} + + {{- if .eip1559Params }} + {{- $fields = append $fields (printf "\"eip1559Params\": %s" (toPrettyJson .eip1559Params | indent 8 | trim)) }} + {{- end }} + + {{- if .sequencer }} + {{- $sequencerFields := list }} + + {{- if .sequencer.chainId }} + {{- $sequencerFields = append $sequencerFields (printf "\"chainId\": \"%s\"" (tpl .sequencer.chainId .)) }} + {{- end }} + + {{- if .sequencer.addressPrefix }} + {{- $sequencerFields = append $sequencerFields (printf "\"addressPrefix\": \"%s\"" .sequencer.addressPrefix) }} + {{- end }} + + {{- if .sequencer.startHeight }} + {{- $sequencerFields = append $sequencerFields (printf "\"startHeight\": %s" (toString .sequencer.startHeight | replace "\"" "")) }} + {{- end }} + + {{- if .sequencer.stopHeight }} + {{- $sequencerFields = append $sequencerFields (printf "\"stopHeight\": %s" (toString .sequencer.stopHeight | replace "\"" "")) }} + {{- end }} + + {{- $fields = append $fields (printf "\"sequencer\": {\n%s\n}" (join ",\n" $sequencerFields | indent 4)) }} + {{- end }} + + {{- if .celestia }} + {{- $celestiaFields := list }} + + {{- if .celestia.chainId }} + {{- $celestiaFields = append $celestiaFields (printf "\"chainId\": \"%s\"" (tpl .celestia.chainId .)) }} + {{- end }} + + {{- if .celestia.startHeight }} + {{- $celestiaFields = append $celestiaFields (printf "\"startHeight\": %s" (toString .celestia.startHeight | replace "\"" "")) }} + {{- end }} + + {{- if .celestia.heightVariance }} + {{- $celestiaFields = append $celestiaFields (printf "\"heightVariance\": %s" (toString .celestia.heightVariance | replace "\"" "")) }} + {{- end }} + + {{- if $celestiaFields | len }} + {{- $fields = append $fields (printf "\"celestia\": {\n%s\n}" (join ",\n" $celestiaFields | indent 4)) }} + {{- end }} + {{- end }} + + {{- if .bridgeAddresses }} + {{- $fields = append $fields (printf "\"bridgeAddresses\": %s" (toPrettyJson .bridgeAddresses | indent 4 | trim)) }} + {{- end }} + + {{- join ",\n" $fields | indent 16 }} + } + {{- if ne $index $lastIndex }},{{ end }} + {{- $index := add $index 1 }} {{- end }} - "sequencer": { - "chainId": "{{ tpl .Values.genesis.forks.launch.sequencerChainId . }}", - "addressPrefix": "{{ .Values.genesis.forks.launch.sequencerAddressPrefix }}", - "startHeight": {{ toString .Values.genesis.forks.launch.sequencerStartHeight | replace "\"" "" }}, - "stopHeight": {{ toString .Values.genesis.forks.launch.sequencerStopHeight | replace "\"" "" }} - }, - "celestia": { - "chainId": "{{ tpl .Values.genesis.forks.launch.celestiaChainId . }}", - "startHeight": {{ toString .Values.genesis.forks.launch.celestiaStartHeight | replace "\"" "" }}, - "heightVariance": {{ toString .Values.genesis.forks.launch.celestiaHeightVariance | replace "\"" "" }} - }, - "bridgeAddresses": {{ toPrettyJson .Values.genesis.forks.launch.bridgeAddresses | indent 8 | trim }} - } - {{- if .Values.genesis.forks.additionalForks }} - , {{ toPrettyJson .Values.genesis.forks.additionalForks | indent 8 | trim }} {{- end }} - } - {{- if not .Values.global.dev }} - {{- else }} - {{- end }} + } }, + {{- if not .Values.global.dev }} + {{- else }} + {{- end }} "difficulty": "0", "gasLimit": "{{ toString .Values.genesis.gasLimit | replace "\"" "" }}", "alloc": { diff --git a/charts/evm-rollup/values.yaml b/charts/evm-rollup/values.yaml index 2a30d9af4b..46f19705a1 100644 --- a/charts/evm-rollup/values.yaml +++ b/charts/evm-rollup/values.yaml @@ -17,7 +17,7 @@ images: repo: ghcr.io/astriaorg/conductor pullPolicy: IfNotPresent tag: 1.0.1 - devTag: latest + devTag: local genesis: @@ -28,8 +28,6 @@ genesis: # and, if desired, restarting the chain at a given height. The necessary fields # for the genesis fork are provided, and additional forks can be added as needed. forks: - ## These values are used to configure the genesis block of the rollup chain - ## no defaults as they are unique to each chain launch: # The block height to start the chain at, 0 for genesis height: "0" @@ -50,21 +48,23 @@ genesis: # minBaseFee: 0 # elasticityMultiplier: 2 # baseFeeChangeDenominator: 8 - # The chain id of the sequencer chain - sequencerChainId: "" - # The hrp for bech32m addresses, unlikely to be changed - sequencerAddressPrefix: "astria" - # Block height to start syncing rollup from, lowest possible is 2 - sequencerStartHeight: "" - # Block height (on sequencer) to stop syncing rollup at, continuing to next configuration fork. - # A height of 0 indicates no stop height. - sequencerStopHeight: "" - # The chain id of the celestia chain - celestiaChainId: "" - # The first Celestia height to utilize when looking for rollup data - celestiaStartHeight: "" - # The variance in Celestia height to allow before halting the chain - celestiaHeightVariance: "" + sequencer: + # The chain id of the sequencer chain + chainId: "" + # The hrp for bech32m addresses, unlikely to be changed + addressPrefix: "astria" + # Block height to start syncing rollup from, 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: "" + # The first Celestia height to utilize when looking for rollup data + startHeight: "" + # The variance in Celestia height to allow before halting the chain + heightVariance: "" # Configure the sequencer bridge addresses and allowed assets if using # the astria canonical bridge. Recommend removing alloc values if so. bridgeAddresses: [] @@ -73,10 +73,6 @@ genesis: # assetDenom: "nria" # senderAddress: "0x0000000000000000000000000000000000000000" # assetPrecision: 9 - # JSON object with any additional desired forks. Can be used with any or all - # of the above fields. Any fields left empty will be populated with the previous - # fork's values. - additionalForks: {} ## These are general configuration values with some recommended defaults diff --git a/dev/values/rollup/dev.yaml b/dev/values/rollup/dev.yaml index 7f7af87cd9..a8a0ef2481 100644 --- a/dev/values/rollup/dev.yaml +++ b/dev/values/rollup/dev.yaml @@ -21,26 +21,26 @@ evm-rollup: ## no defaults as they are unique to each chain launch: # The block height to start the chain at, 0 for genesis - height: 0 + height: "0" # Configure the fee collector for the evm tx fees, activated at block heights. # If not configured, all tx fees will be burned. feeCollector: "0xaC21B97d35Bf75A7dAb16f35b111a50e78A72F30" - # The chain id of the sequencer chain - sequencerChainId: "{{ .Values.global.sequencerChainId }}" - # The hrp for bech32m addresses, unlikely to be changed - sequencerAddressPrefix: "astria" - # Block height to start syncing rollup from, lowest possible is 2 - sequencerStartHeight: 2 - # Block height (on sequencer) to stop syncing rollup at, continuing to next configuration fork - sequencerStopHeight: 0 - # The chain id of the celestia chain - celestiaChainId: "{{ .Values.global.celestiaChainId }}" - # The first Celestia height to utilize when looking for rollup data - celestiaStartHeight: 2 - # The variance in Celestia height to allow before halting the chain - celestiaHeightVariance: 10 - # Configure the sequencer bridge addresses and allowed assets if using - # the astria canonical bridge. Recommend removing alloc values if so. + sequencer: + # The chain id of the sequencer chain + chainId: "sequencer-test-chain-0" + # The hrp for bech32m addresses, unlikely to be changed + addressPrefix: "astria" + # Block height to start syncing rollup from, lowest possible is 2 + startHeight: 2 + celestia: + # The chain id of the celestia chain + chainId: "celestia-local-0" + # The first Celestia height to utilize when looking for rollup data + startHeight: 2 + # The variance in Celestia height to allow before halting the chain + heightVariance: 10 + # Configure the sequencer bridge addresses and allowed assets if using + # the astria canonical bridge. Recommend removing alloc values if so. bridgeAddresses: - bridgeAddress: "astria13ahqz4pjqfmynk9ylrqv4fwe4957x2p0h5782u" startHeight: 1 diff --git a/dev/values/rollup/ibc-bridge-test.yaml b/dev/values/rollup/ibc-bridge-test.yaml index 9d546a37a2..c75841bccb 100644 --- a/dev/values/rollup/ibc-bridge-test.yaml +++ b/dev/values/rollup/ibc-bridge-test.yaml @@ -21,20 +21,22 @@ evm-rollup: # Configure the fee collector for the evm tx fees, activated at block heights. # If not configured, all tx fees will be burned. feeCollector: "0xaC21B97d35Bf75A7dAb16f35b111a50e78A72F30" - # The chain id of the sequencer chain - sequencerChainId: "{{ .Values.global.sequencerChainId }}" - # The hrp for bech32m addresses, unlikely to be changed - sequencerAddressPrefix: "astria" - # Block height to start syncing rollup from, lowest possible is 2 - sequencerStartHeight: 2 - # Block height (on sequencer) to stop syncing rollup at, continuing to next configuration fork - sequencerStopHeight: 0 - # The chain id of the celestia chain - celestiaChainId: "{{ .Values.global.celestiaChainId }}" - # The first Celestia height to utilize when looking for rollup data - celestiaStartHeight: 2 - # The variance in Celestia height to allow before halting the chain - celestiaHeightVariance: 10 + sequencer: + # The chain id of the sequencer chain + chainId: "{{ $.Values.global.sequencerChainId }}" + # The hrp for bech32m addresses, unlikely to be changed + addressPrefix: "astria" + # Block height to start syncing rollup from, lowest possible is 2 + startHeight: 2 + # Block height (on sequencer) to stop syncing rollup at, continuing to next configuration fork + stopHeight: 0 + celestia: + # The chain id of the celestia chain + chainId: "{{ $.Values.global.celestiaChainId }}" + # The first Celestia height to utilize when looking for rollup data + startHeight: 2 + # The variance in Celestia height to allow before halting the chain + heightVariance: 10 bridgeAddresses: - bridgeAddress: "astria1d7zjjljc0dsmxa545xkpwxym86g8uvvwhtezcr" startHeight: 1 From 1b93dfb464a9db25f32e34e5588bcd8a749c2357 Mon Sep 17 00:00:00 2001 From: ethanoroshiba Date: Mon, 16 Dec 2024 11:12:21 -0600 Subject: [PATCH 20/27] fix values.yaml --- dev/values/rollup/ibc-bridge-test.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dev/values/rollup/ibc-bridge-test.yaml b/dev/values/rollup/ibc-bridge-test.yaml index c75841bccb..9302310b47 100644 --- a/dev/values/rollup/ibc-bridge-test.yaml +++ b/dev/values/rollup/ibc-bridge-test.yaml @@ -23,7 +23,7 @@ evm-rollup: feeCollector: "0xaC21B97d35Bf75A7dAb16f35b111a50e78A72F30" sequencer: # The chain id of the sequencer chain - chainId: "{{ $.Values.global.sequencerChainId }}" + chainId: "sequencer-test-chain-0" # The hrp for bech32m addresses, unlikely to be changed addressPrefix: "astria" # Block height to start syncing rollup from, lowest possible is 2 @@ -32,7 +32,7 @@ evm-rollup: stopHeight: 0 celestia: # The chain id of the celestia chain - chainId: "{{ $.Values.global.celestiaChainId }}" + chainId: "celestia-local-0" # The first Celestia height to utilize when looking for rollup data startHeight: 2 # The variance in Celestia height to allow before halting the chain From 228367227f43f4837e697d1e4fd05b9d2890fc61 Mon Sep 17 00:00:00 2001 From: ethanoroshiba Date: Mon, 16 Dec 2024 11:16:08 -0600 Subject: [PATCH 21/27] add values --- dev/values/rollup/evm-restart-test.yaml | 269 ++++++++++++++++++++++++ 1 file changed, 269 insertions(+) create mode 100644 dev/values/rollup/evm-restart-test.yaml diff --git a/dev/values/rollup/evm-restart-test.yaml b/dev/values/rollup/evm-restart-test.yaml new file mode 100644 index 0000000000..177c2297ee --- /dev/null +++ b/dev/values/rollup/evm-restart-test.yaml @@ -0,0 +1,269 @@ +global: + useTTY: true + dev: true + evmChainId: 1337 + rollupName: astria + sequencerRpc: http://node0-sequencer-rpc-service.astria-dev-cluster.svc.cluster.local:26657 + sequencerGrpc: http://node0-sequencer-grpc-service.astria-dev-cluster.svc.cluster.local:8080 + sequencerChainId: sequencer-test-chain-0 + celestiaChainId: celestia-local-0 + +evm-rollup: + genesis: + # The name of the rollup chain, used to generate the Rollup ID + rollupName: "{{ Values.global.rollupName }}" + + # The "forks" for upgrading the chain. Contains necessary information for starting + # and, if desired, restarting the chain at a given height. The necessary fields + # for the genesis fork are provided, and additional forks can be added as needed. + launch: + ## These values are used to configure the genesis block of the rollup chain + ## no defaults as they are unique to each chain + genesis: + # 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 + halt: false + # Checksum of the snapshot to use upon restart + snapshotChecksum: "" + # Will fill the extra data in each block, can be left empty + # can also fill with something unique for your chain. + extraDataOverride: "" + # Configure the fee collector for the evm tx fees, activated at block heights. + # If not configured, all tx fees will be burned. + feeCollector: "0xaC21B97d35Bf75A7dAb16f35b111a50e78A72F30" + # Configure EIP-1559 params, activated at block heights. + eip1559Params: {} + sequencer: + # The chain id of the sequencer chain + chainId: "sequencer-test-chain-0" + # The hrp for bech32m addresses, unlikely to be changed + addressPrefix: "astria" + # Block height to start syncing rollup from, lowest possible is 2 + startHeight: 2 + # Block height (on sequencer) to stop syncing rollup at, continuing to next configuration fork + stopHeight: 20 + celestia: + # The chain id of the celestia chain + chainId: "celestia-local-0" + # The first Celestia height to utilize when looking for rollup data + startHeight: 2 + # The variance in Celestia height to allow before halting the chain + heightVariance: 10 + # Configure the sequencer bridge addresses and allowed assets if using + # the astria canonical bridge. Recommend removing alloc values if so. + bridgeAddresses: + - bridgeAddress: "astria13ahqz4pjqfmynk9ylrqv4fwe4957x2p0h5782u" + startHeight: 1 + senderAddress: "0x0000000000000000000000000000000000000000" + assetDenom: "nria" + assetPrecision: 9 + restart: + height: 18 + halt: true + sequencer: + addressPrefix: "astria" + startHeight: 20 + stopHeight: 1000 + celestia: + startHeight: 20 + + ## These are general configuration values with some recommended defaults + + # Configure the gas Limit + gasLimit: "50000000" + # If set to true the genesis block will contain extra data + overrideGenesisExtraData: true + + ## Standard Eth Genesis config values + # Configuration of Eth forks, setting to 0 will enable from height, + # left as is these forks will not activate. + cancunTime: "" + pragueTime: "" + verkleTime: "" + # Can configure the genesis allocs for the chain + alloc: + # Deploying the deterministic deploy proxy contract in genesis + # Forge and other tools use this for their CREATE2 usage, but + # can only be included through the genesis block after EIP-155 + # https://github.com/Arachnid/deterministic-deployment-proxy + - address: "0x4e59b44847b379578588920cA78FbF26c0B4956C" + value: + balance: "0" + code: "0x7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffe03601600081602082378035828234f58015156039578182fd5b8082525050506014600cf3" + - address: "0xA58639fB5458e65E4fA917FF951C390292C24A15" + value: + balance: "0" + code: "0x6080604052600436106100f35760003560e01c8063b6476c7e1161008a578063e74b981b11610059578063e74b981b1461027b578063ebd090541461029b578063f2fde38b146102bb578063fc88d31b146102db57600080fd5b8063b6476c7e1461021c578063bab916d01461023e578063d294f09314610251578063db97dc981461026657600080fd5b80638da5cb5b116100c65780638da5cb5b146101a1578063a7eaa739146101d3578063a996e020146101f3578063ad2282471461020657600080fd5b80636f46384a146100f8578063715018a6146101215780637eb6dec7146101385780638897397914610181575b600080fd5b34801561010457600080fd5b5061010e60035481565b6040519081526020015b60405180910390f35b34801561012d57600080fd5b506101366102f1565b005b34801561014457600080fd5b5061016c7f000000000000000000000000000000000000000000000000000000000000000981565b60405163ffffffff9091168152602001610118565b34801561018d57600080fd5b5061013661019c3660046107a6565b610305565b3480156101ad57600080fd5b506000546001600160a01b03165b6040516001600160a01b039091168152602001610118565b3480156101df57600080fd5b506101366101ee3660046107a6565b610312565b610136610201366004610808565b61031f565b34801561021257600080fd5b5061010e60065481565b34801561022857600080fd5b50610231610414565b6040516101189190610874565b61013661024c3660046108c3565b6104a2565b34801561025d57600080fd5b50610136610588565b34801561027257600080fd5b506102316106b4565b34801561028757600080fd5b50610136610296366004610905565b6106c1565b3480156102a757600080fd5b506005546101bb906001600160a01b031681565b3480156102c757600080fd5b506101366102d6366004610905565b6106eb565b3480156102e757600080fd5b5061010e60045481565b6102f9610729565b6103036000610756565b565b61030d610729565b600455565b61031a610729565b600355565b3460045480821161034b5760405162461bcd60e51b815260040161034290610935565b60405180910390fd5b60007f000000000000000000000000000000000000000000000000000000003b9aca006103788385610998565b61038291906109b1565b1161039f5760405162461bcd60e51b8152600401610342906109d3565b600454600660008282546103b39190610a61565b90915550506004546103c59034610998565b336001600160a01b03167f0c64e29a5254a71c7f4e52b3d2d236348c80e00a00ba2e1961962bd2827c03fb888888886040516104049493929190610a9d565b60405180910390a3505050505050565b6002805461042190610acf565b80601f016020809104026020016040519081016040528092919081815260200182805461044d90610acf565b801561049a5780601f1061046f5761010080835404028352916020019161049a565b820191906000526020600020905b81548152906001019060200180831161047d57829003601f168201915b505050505081565b346003548082116104c55760405162461bcd60e51b815260040161034290610935565b60007f000000000000000000000000000000000000000000000000000000003b9aca006104f28385610998565b6104fc91906109b1565b116105195760405162461bcd60e51b8152600401610342906109d3565b6003546006600082825461052d9190610a61565b909155505060035461053f9034610998565b336001600160a01b03167f0f4961cab7530804898499aa89f5ec81d1a73102e2e4a1f30f88e5ae3513ba2a868660405161057a929190610b09565b60405180910390a350505050565b6005546001600160a01b031633146105f45760405162461bcd60e51b815260206004820152602960248201527f41737472696142726964676561626c6545524332303a206f6e6c7920666565206044820152681c9958da5c1a595b9d60ba1b6064820152608401610342565b6005546006546040516000926001600160a01b031691908381818185875af1925050503d8060008114610643576040519150601f19603f3d011682016040523d82523d6000602084013e610648565b606091505b50509050806106ac5760405162461bcd60e51b815260206004820152602a60248201527f41737472696142726964676561626c6545524332303a20666565207472616e7360448201526919995c8819985a5b195960b21b6064820152608401610342565b506000600655565b6001805461042190610acf565b6106c9610729565b600580546001600160a01b0319166001600160a01b0392909216919091179055565b6106f3610729565b6001600160a01b03811661071d57604051631e4fbdf760e01b815260006004820152602401610342565b61072681610756565b50565b6000546001600160a01b031633146103035760405163118cdaa760e01b8152336004820152602401610342565b600080546001600160a01b038381166001600160a01b0319831681178455604051919092169283917f8be0079c531659141344cd1fd0a4f28419497f9722a3daafe3b4186f6b6457e09190a35050565b6000602082840312156107b857600080fd5b5035919050565b60008083601f8401126107d157600080fd5b50813567ffffffffffffffff8111156107e957600080fd5b60208301915083602082850101111561080157600080fd5b9250929050565b6000806000806040858703121561081e57600080fd5b843567ffffffffffffffff8082111561083657600080fd5b610842888389016107bf565b9096509450602087013591508082111561085b57600080fd5b50610868878288016107bf565b95989497509550505050565b60006020808352835180602085015260005b818110156108a257858101830151858201604001528201610886565b506000604082860101526040601f19601f8301168501019250505092915050565b600080602083850312156108d657600080fd5b823567ffffffffffffffff8111156108ed57600080fd5b6108f9858286016107bf565b90969095509350505050565b60006020828403121561091757600080fd5b81356001600160a01b038116811461092e57600080fd5b9392505050565b6020808252602d908201527f417374726961576974686472617765723a20696e73756666696369656e74207760408201526c69746864726177616c2066656560981b606082015260800190565b634e487b7160e01b600052601160045260246000fd5b818103818111156109ab576109ab610982565b92915050565b6000826109ce57634e487b7160e01b600052601260045260246000fd5b500490565b60208082526062908201527f417374726961576974686472617765723a20696e73756666696369656e74207660408201527f616c75652c206d7573742062652067726561746572207468616e203130202a2a60608201527f20283138202d20424153455f434841494e5f41535345545f505245434953494f6080820152614e2960f01b60a082015260c00190565b808201808211156109ab576109ab610982565b81835281816020850137506000828201602090810191909152601f909101601f19169091010190565b604081526000610ab1604083018688610a74565b8281036020840152610ac4818587610a74565b979650505050505050565b600181811c90821680610ae357607f821691505b602082108103610b0357634e487b7160e01b600052602260045260246000fd5b50919050565b602081526000610b1d602083018486610a74565b94935050505056fea2646970667358221220842bd8104ffc1c611919341f64a8277f2fc808138b97720a6dc1382e5670099064736f6c63430008190033" + + + config: + # The level at which core astria components will log out + # Options are: error, warn, info, and debug + logLevel: "debug" + + conductor: + # Determines what will drive block execution, options are: + # - "SoftOnly" -> blocks are only pulled from the sequencer + # - "FirmOnly" -> blocks are only pulled from DA + # - "SoftAndFirm" -> blocks are pulled from both the sequencer and DA + executionCommitLevel: 'SoftAndFirm' + # The expected fastest block time possible from sequencer, determines polling + # rate. + sequencerBlockTimeMs: 2000 + # The maximum number of requests to make to the sequencer per second + sequencerRequestsPerSecond: 500 + + celestia: + rpc: "http://celestia-service.astria-dev-cluster.svc.cluster.local:26658" + token: "" + + resources: + conductor: + requests: + cpu: 0.01 + memory: 1Mi + limits: + cpu: 0.1 + memory: 20Mi + geth: + requests: + cpu: 0.25 + memory: 256Mi + limits: + cpu: 2 + memory: 1Gi + + storage: + enabled: false + + ingress: + enabled: true + services: + rpc: + enabled: true + ws: + enabled: true + +celestia-node: + enabled: false + +composer: + enabled: true + config: + privateKey: + devContent: "2bd806c97f0e00af1a1fc3328fa763a9269723c8db8fac4f93af71db186d6e90" + +evm-bridge-withdrawer: + enabled: true + config: + minExpectedFeeAssetBalance: "0" + sequencerBridgeAddress: "astria13ahqz4pjqfmynk9ylrqv4fwe4957x2p0h5782u" + feeAssetDenom: "nria" + rollupAssetDenom: "nria" + evmContractAddress: "0xA58639fB5458e65E4fA917FF951C390292C24A15" + sequencerPrivateKey: + devContent: "dfa7108e38ab71f89f356c72afc38600d5758f11a8c337164713e4471411d2e0" + +evm-faucet: + enabled: true + ingress: + enabled: true + config: + privateKey: + devContent: "8b3a7999072c9c9314c084044fe705db11714c6c4ed7cddb64da18ea270dd203" + +postgresql: + enabled: true + nameOverride: blockscout-postegres + primary: + persistence: + enabled: false + resourcesPreset: "medium" + auth: + enablePostgresUser: true + postgresPassword: bigsecretpassword + username: blockscout + password: blockscout + database: blockscout + audit: + logHostname: true + logConnections: true + logDisconnections: true +blockscout-stack: + enabled: true + config: + network: + id: 1337 + name: Astria + shortname: Astria + currency: + name: RIA + symbol: RIA + decimals: 18 + testnet: true + prometheus: + enabled: false + blockscout: + extraEnv: + - name: ECTO_USE_SSL + value: "false" + - name: DATABASE_URL + value: "postgres://postgres:bigsecretpassword@astria-chain-chart-blockscout-postegres.astria-dev-cluster.svc.cluster.local:5432/blockscout" + - name: ETHEREUM_JSONRPC_VARIANT + value: "geth" + - name: ETHEREUM_JSONRPC_HTTP_URL + value: "http://astria-evm-service.astria-dev-cluster.svc.cluster.local:8545/" + - name: ETHEREUM_JSONRPC_INSECURE + value: "true" + - name: ETHEREUM_JSONRPC_WS_URL + value: "ws://astria-evm-service.astria-dev-cluster.svc.cluster.local:8546/" + - name: INDEXER_DISABLE_BEACON_BLOB_FETCHER + value: "true" + - name: NETWORK + value: "Astria" + - name: SUBNETWORK + value: "Local" + - name: CONTRACT_VERIFICATION_ALLOWED_SOLIDITY_EVM_VERSIONS + value: "homestead,tangerineWhistle,spuriousDragon,byzantium,constantinople,petersburg,istanbul,berlin,london,paris,shanghai,default" + - name: CONTRACT_VERIFICATION_ALLOWED_VYPER_EVM_VERSIONS + value: "byzantium,constantinople,petersburg,istanbul,berlin,paris,shanghai,default" + - name: DISABLE_EXCHANGE_RATES + value: "true" + + ingress: + enabled: true + hostname: explorer.astria.localdev.me + paths: + - path: /api + pathType: Prefix + - path: /socket + pathType: Prefix + - path: /sitemap.xml + pathType: ImplementationSpecific + - path: /public-metrics + pathType: Prefix + - path: /auth/auth0 + pathType: Exact + - path: /auth/auth0/callback + pathType: Exact + - path: /auth/logout + pathType: Exact + + frontend: + extraEnv: + - name: NEXT_PUBLIC_NETWORK_VERIFICATION_TYPE + value: "validation" + - name: NEXT_PUBLIC_AD_BANNER_PROVIDER + value: "none" + - name: NEXT_PUBLIC_API_PROTOCOL + value: "http" + - name: NEXT_PUBLIC_API_WEBSOCKET_PROTOCOL + value: "ws" + - name: NEXT_PUBLIC_NETWORK_CURRENCY_WEI_NAME + value: "aRia" + - name: NEXT_PUBLIC_AD_TEXT_PROVIDER + value: "none" + ingress: + enabled: true + hostname: explorer.astria.localdev.me From 3a73489055d0f4cc9f872cf438d7eecac6e1010a Mon Sep 17 00:00:00 2001 From: ethanoroshiba Date: Mon, 16 Dec 2024 11:27:19 -0600 Subject: [PATCH 22/27] fix devTag --- charts/evm-rollup/values.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/charts/evm-rollup/values.yaml b/charts/evm-rollup/values.yaml index 46f19705a1..bb494e5dbf 100644 --- a/charts/evm-rollup/values.yaml +++ b/charts/evm-rollup/values.yaml @@ -17,7 +17,7 @@ images: repo: ghcr.io/astriaorg/conductor pullPolicy: IfNotPresent tag: 1.0.1 - devTag: local + devTag: latest genesis: From 483aa39be97de6ec27410728bc03bf349c37a702 Mon Sep 17 00:00:00 2001 From: ethanoroshiba Date: Fri, 3 Jan 2025 08:58:16 -0600 Subject: [PATCH 23/27] update specs --- specs/conductor.md | 5 +++++ specs/execution-api.md | 40 +++++++++++++++++++++++++++++++++++++--- 2 files changed, 42 insertions(+), 3 deletions(-) diff --git a/specs/conductor.md b/specs/conductor.md index a2075922d9..6a6a1b364d 100644 --- a/specs/conductor.md +++ b/specs/conductor.md @@ -126,3 +126,8 @@ only `CommitmentState.firm.number += 1` is advanced). Soft being ahead of firm is the expected operation. In certain rare situations the numbers can match exactly, and step `firm-only.10` and `firm-only.11` are executed as written. + +## Startup, Restarts, Execution, and Commitments + +See [`astria.execution.v1alpha2` API documentation](./execution-api.md) for more +information on Conductor startup, restart, execution, and commitment logic. diff --git a/specs/execution-api.md b/specs/execution-api.md index c765229007..1b269d8169 100644 --- a/specs/execution-api.md +++ b/specs/execution-api.md @@ -29,6 +29,31 @@ previous block data, Conductor must also track the block hash of any blocks between commitments, it will call `BatchGetBlocks` to get block information between commitments. +### Restart + +The conductor is able to gracefully restart under two scenarios: + +1. Conductor recieves a `PermissionDenied` status from the execution layer when +calling `ExecuteBlock`. This is meant to function seamlessly with [`astria-geth`](https://github.com/astriaorg/astria-geth)'s +behavior upon an unexpected restart. If `geth` receives a `ExecuteBlock` request +before receving *both* `GetGenesisInfo` and `GetCommitmentState` (which will be +the case if the execution layer has restarted), it responds with a `PremissionDenied` +status, prompting the conductor to restart. +2. `GenesisInfo` contains a `sequencer_stop_block_height`, indicating a planned +restart at a given sequencer block height. If the conductor reaches the stop height, +it will perform a restart in one of the following ways corresponding to its mode: + + - **Firm Only Mode**: Once the stop height is reached for the firm block stream, + the firm block at this height is executed (and commitment state updated) before + restarting the conductor, prompting the rollup for a new `GenesisInfo` with + new start/stop heights (and potentially chain IDs). + - **Soft and Firm Mode**: Once the stop height is reached for the soft block + stream, no more soft blocks are executed (including the one at the stop height). + The firm block is still executed for this height, and then Conductor restarts. + - **Soft Only Mode**: Once the stop height is reached for the soft block stream, + no more soft blocks are executed (including the one at the stop height). Conductor + is then restarted. + ### Execution & Commitments From the perspective of the sequencer: @@ -74,9 +99,18 @@ Note: For our EVM rollup, we map the `CommitmentState` to the `ForkchoiceRule`: ### GetGenesisInfo `GetGenesisInfo` returns information which is definitional to the rollup with -regards to how it serves data from the sequencer & celestia networks. This RPC -should ALWAYS succeed. The API is agnostic as to how the information is defined -in a rollups genesis, and used by the conductor as configuration on startup. +regards to how it serves data from the sequencer & celestia networks, along with +optional block heights for initiating a [conductor restart](#restart). This RPC +should **ALWAYS** succeed. The API is agnostic as to how the information is defined +in a rollup's genesis, and used by the conductor as configuration on startup *or* +upon a restart. + +If the `GenesisInfo` provided by this RPC contains a `sequencer_stop_block_height`, +the rollup should be prepared to provide an updated response when the conductor +restarts, including, at minimum, a new `rollup_start_block_height` and `sequencer_start_block_height`. +The updated response can also contain an updated `sequencer_stop_block_height` (if +another restart is desired), `celestia_chain_id`, and/or `sequencer_chain_id` (to +facilitate network migration). ### ExecuteBlock From 680d984cd084bf154a0410a7571bea493fe9112c Mon Sep 17 00:00:00 2001 From: ethanoroshiba Date: Mon, 6 Jan 2025 10:26:01 -0600 Subject: [PATCH 24/27] add preliminary to GenesisInfo --- crates/astria-conductor/src/executor/state.rs | 1 + crates/astria-conductor/src/executor/tests.rs | 1 + .../tests/blackbox/helpers/macros.rs | 43 ++++++- .../tests/blackbox/helpers/mod.rs | 9 +- .../tests/blackbox/soft_and_firm.rs | 114 ++++++++++++++++++ crates/astria-core/src/execution/v1/mod.rs | 6 + .../src/generated/astria.execution.v1.rs | 3 + .../generated/astria.execution.v1.serde.rs | 18 +++ .../astria/execution/v1/execution.proto | 2 + 9 files changed, 194 insertions(+), 3 deletions(-) diff --git a/crates/astria-conductor/src/executor/state.rs b/crates/astria-conductor/src/executor/state.rs index a9337b6d0c..c2131a20c0 100644 --- a/crates/astria-conductor/src/executor/state.rs +++ b/crates/astria-conductor/src/executor/state.rs @@ -430,6 +430,7 @@ mod tests { rollup_start_block_height: 0, sequencer_chain_id: "test-sequencer-0".to_string(), celestia_chain_id: "test-celestia-0".to_string(), + halt_at_stop_height: false, }) .unwrap() } diff --git a/crates/astria-conductor/src/executor/tests.rs b/crates/astria-conductor/src/executor/tests.rs index 0d9f4f6409..85d0aa95d4 100644 --- a/crates/astria-conductor/src/executor/tests.rs +++ b/crates/astria-conductor/src/executor/tests.rs @@ -53,6 +53,7 @@ fn make_state( rollup_start_block_height: 1, sequencer_chain_id: "test_sequencer-0".to_string(), celestia_chain_id: "test_celestia-0".to_string(), + halt_at_stop_height: false, }) .unwrap(); let commitment_state = CommitmentState::try_from_raw(raw::CommitmentState { diff --git a/crates/astria-conductor/tests/blackbox/helpers/macros.rs b/crates/astria-conductor/tests/blackbox/helpers/macros.rs index 3594db488b..a10e56438e 100644 --- a/crates/astria-conductor/tests/blackbox/helpers/macros.rs +++ b/crates/astria-conductor/tests/blackbox/helpers/macros.rs @@ -98,7 +98,23 @@ macro_rules! genesis_info { $start_height:expr,sequencer_stop_block_height: $stop_height:expr,celestia_block_variance: $variance:expr,rollup_start_block_height: - $rollup_start_block_height:expr $(,)? + $rollup_start_block_height:expr, + ) => { + genesis_info!( + sequencer_start_block_height: $start_height, + sequencer_stop_block_height: $stop_height, + celestia_block_variance: $variance, + rollup_start_block_height: $rollup_start_block_height, + halt_at_stop_height: false, + ) + }; + ( + sequencer_start_block_height: + $start_height:expr,sequencer_stop_block_height: + $stop_height:expr,celestia_block_variance: + $variance:expr,rollup_start_block_height: + $rollup_start_block_height:expr, + halt_at_stop_height: $halt_at_stop_height:expr $(,)? ) => { ::astria_core::generated::astria::execution::v1::GenesisInfo { rollup_id: Some($crate::ROLLUP_ID.to_raw()), @@ -108,6 +124,7 @@ macro_rules! genesis_info { rollup_start_block_height: $rollup_start_block_height, sequencer_chain_id: $crate::SEQUENCER_CHAIN_ID.to_string(), celestia_chain_id: $crate::helpers::CELESTIA_CHAIN_ID.to_string(), + halt_at_stop_height: $halt_at_stop_height, } }; } @@ -397,6 +414,28 @@ macro_rules! mount_get_genesis_info { rollup_start_block_height: $rollup_start_block_height:expr, up_to_n_times: $up_to_n_times:expr $(,)? + ) => { + mount_get_genesis_info!( + $test_env, + sequencer_start_block_height: $start_height, + sequencer_stop_block_height: $stop_height, + celestia_block_variance: $variance, + rollup_start_block_height: $rollup_start_block_height, + up_to_n_times: $up_to_n_times, + halt_at_stop_height: false, + expected_calls: 1, + ) + }; + ( + $test_env:ident, + sequencer_start_block_height: $start_height:expr, + sequencer_stop_block_height: $stop_height:expr, + celestia_block_variance: $variance:expr, + rollup_start_block_height: $rollup_start_block_height:expr, + up_to_n_times: $up_to_n_times:expr, + halt_at_stop_height: $halt_at_stop_height:expr, + expected_calls: $expected_calls:expr + $(,)? ) => { $test_env.mount_get_genesis_info( $crate::genesis_info!( @@ -404,8 +443,10 @@ macro_rules! mount_get_genesis_info { sequencer_stop_block_height: $stop_height, celestia_block_variance: $variance, rollup_start_block_height: $rollup_start_block_height, + halt_at_stop_height: $halt_at_stop_height, ), $up_to_n_times, + $expected_calls, ).await; }; } diff --git a/crates/astria-conductor/tests/blackbox/helpers/mod.rs b/crates/astria-conductor/tests/blackbox/helpers/mod.rs index ceede6e553..2095218a13 100644 --- a/crates/astria-conductor/tests/blackbox/helpers/mod.rs +++ b/crates/astria-conductor/tests/blackbox/helpers/mod.rs @@ -305,7 +305,12 @@ impl TestConductor { mount_genesis(&self.mock_http, chain_id).await; } - pub async fn mount_get_genesis_info(&self, genesis_info: GenesisInfo, up_to_n_times: u64) { + pub async fn mount_get_genesis_info( + &self, + genesis_info: GenesisInfo, + up_to_n_times: u64, + expected_calls: u64, + ) { use astria_core::generated::astria::execution::v1::GetGenesisInfoRequest; astria_grpc_mock::Mock::for_rpc_given( "get_genesis_info", @@ -313,7 +318,7 @@ impl TestConductor { ) .respond_with(astria_grpc_mock::response::constant_response(genesis_info)) .up_to_n_times(up_to_n_times) - .expect(1..) + .expect(expected_calls) .mount(&self.mock_grpc.mock_server) .await; } diff --git a/crates/astria-conductor/tests/blackbox/soft_and_firm.rs b/crates/astria-conductor/tests/blackbox/soft_and_firm.rs index 85ac6afaff..f5c55b558f 100644 --- a/crates/astria-conductor/tests/blackbox/soft_and_firm.rs +++ b/crates/astria-conductor/tests/blackbox/soft_and_firm.rs @@ -476,6 +476,8 @@ async fn conductor_restarts_on_permission_denied() { celestia_block_variance: 10, rollup_start_block_height: 0, up_to_n_times: 2, + halt_at_stop_height: false, + expected_calls: 2, ); mount_get_commitment_state!( @@ -817,3 +819,115 @@ async fn conductor_restarts_after_reaching_stop_height() { .await .expect("conductor should have updated the firm commitment state within 1000ms"); } + +// ** Disabled until logic to handle this is implemented ** +// /// Tests if the conductor correctly stops and does not restart after reaching the sequencer stop +// /// height if genesis info's `halt_at_stop_height` is `true`. +// /// +// /// This test consists of the following steps: +// /// 1. Mount commitment state and genesis info with a sequencer stop height of 3, expecting only +// 1 /// response. +// /// 2. Mount Celestia network head and sequencer genesis. +// /// 3. Mount ABCI info and sequencer (soft blocks) for height 3. +// /// 4. Mount firm blocks at height 3, with corresponding `update_commitment_state` mount. +// /// 5. Mount `execute_block` and `update_commitment_state` for firm block at height +// /// 3. The soft block should not be executed since it is at the stop height, but the firm +// should /// be. +// /// 6. Await satisfaction of the `execute_block` and `update_commitment_state` for the firm block +// at /// height 3 with a timeout of 1000ms. +// /// 7. Allow ample time for the conductor to potentially restart erroneously. +// #[tokio::test(flavor = "multi_thread", worker_threads = 1)] +// async fn conductor_stops_at_stop_height() { +// let test_conductor = spawn_conductor(CommitLevel::SoftAndFirm).await; + +// mount_get_genesis_info!( +// test_conductor, +// sequencer_start_block_height: 1, +// sequencer_stop_block_height: 3, +// celestia_block_variance: 10, +// rollup_start_block_height: 0, +// up_to_n_times: 2, // allow for calls after an potential erroneous restart +// halt_at_stop_height: true, +// expected_calls: 1, +// ); + +// mount_get_commitment_state!( +// test_conductor, +// firm: ( +// number: 1, +// hash: [1; 64], +// parent: [0; 64], +// ), +// soft: ( +// number: 1, +// hash: [1; 64], +// parent: [0; 64], +// ), +// base_celestia_height: 1, +// ); + +// mount_sequencer_genesis!(test_conductor); +// mount_celestia_header_network_head!( +// test_conductor, +// height: 1u32, +// ); +// mount_abci_info!( +// test_conductor, +// latest_sequencer_height: 3, +// ); + +// // Mount soft blocks for height 3 +// mount_get_filtered_sequencer_block!( +// test_conductor, +// sequencer_height: 3, +// ); + +// // Mount firm blocks for height 3 +// mount_celestia_blobs!( +// test_conductor, +// celestia_height: 1, +// sequencer_heights: [3], +// ); +// mount_sequencer_commit!( +// test_conductor, +// height: 3u32, +// ); +// mount_sequencer_validator_set!(test_conductor, height: 2u32); + +// let execute_block_1 = mount_executed_block!( +// test_conductor, +// mock_name: "execute_block_1", +// number: 2, +// hash: [2; 64], +// parent: [1; 64], +// ); + +// let update_commitment_state_firm_1 = mount_update_commitment_state!( +// test_conductor, +// mock_name: "update_commitment_state_firm_1", +// firm: ( +// number: 2, +// hash: [2; 64], +// parent: [1; 64], +// ), +// soft: ( +// number: 2, +// hash: [2; 64], +// parent: [1; 64], +// ), +// base_celestia_height: 1, +// ); + +// timeout( +// Duration::from_millis(1000), +// join( +// execute_block_1.wait_until_satisfied(), +// update_commitment_state_firm_1.wait_until_satisfied(), +// ), +// ) +// .await +// .expect("conductor should have updated the firm commitment state within 1000ms"); + +// // Allow time for a potential erroneous restart +// sleep(Duration::from_millis(1000)).await; +// } diff --git a/crates/astria-core/src/execution/v1/mod.rs b/crates/astria-core/src/execution/v1/mod.rs index 7bfa96e4b4..80ebe83c31 100644 --- a/crates/astria-core/src/execution/v1/mod.rs +++ b/crates/astria-core/src/execution/v1/mod.rs @@ -72,6 +72,8 @@ pub struct GenesisInfo { sequencer_chain_id: tendermint::chain::Id, /// The chain ID of the celestia network. celestia_chain_id: celestia_tendermint::chain::Id, + /// Whether the conductor halt at the stop height, or otherwise attempt restart. + halt_at_stop_height: bool, } impl GenesisInfo { @@ -130,6 +132,7 @@ impl Protobuf for GenesisInfo { rollup_start_block_height, sequencer_chain_id, celestia_chain_id, + halt_at_stop_height, } = raw; let Some(rollup_id) = rollup_id else { return Err(Self::Error::no_rollup_id()); @@ -153,6 +156,7 @@ impl Protobuf for GenesisInfo { rollup_start_block_height: *rollup_start_block_height, sequencer_chain_id, celestia_chain_id, + halt_at_stop_height: *halt_at_stop_height, }) } @@ -165,6 +169,7 @@ impl Protobuf for GenesisInfo { rollup_start_block_height, sequencer_chain_id, celestia_chain_id, + halt_at_stop_height, } = self; let sequencer_start_block_height: u32 = @@ -186,6 +191,7 @@ impl Protobuf for GenesisInfo { rollup_start_block_height: *rollup_start_block_height, sequencer_chain_id: sequencer_chain_id.to_string(), celestia_chain_id: celestia_chain_id.to_string(), + halt_at_stop_height: *halt_at_stop_height, } } } diff --git a/crates/astria-core/src/generated/astria.execution.v1.rs b/crates/astria-core/src/generated/astria.execution.v1.rs index 3de5b64bc8..3c3538c0c6 100644 --- a/crates/astria-core/src/generated/astria.execution.v1.rs +++ b/crates/astria-core/src/generated/astria.execution.v1.rs @@ -24,6 +24,9 @@ pub struct GenesisInfo { pub sequencer_chain_id: ::prost::alloc::string::String, #[prost(string, tag = "7")] pub celestia_chain_id: ::prost::alloc::string::String, + /// True if the conductor should halt at the stop height instead of attempting restart. + #[prost(bool, tag = "8")] + pub halt_at_stop_height: bool, } impl ::prost::Name for GenesisInfo { const NAME: &'static str = "GenesisInfo"; diff --git a/crates/astria-core/src/generated/astria.execution.v1.serde.rs b/crates/astria-core/src/generated/astria.execution.v1.serde.rs index 5454cac369..6e0defa094 100644 --- a/crates/astria-core/src/generated/astria.execution.v1.serde.rs +++ b/crates/astria-core/src/generated/astria.execution.v1.serde.rs @@ -728,6 +728,9 @@ impl serde::Serialize for GenesisInfo { if !self.celestia_chain_id.is_empty() { len += 1; } + if self.halt_at_stop_height { + len += 1; + } let mut struct_ser = serializer.serialize_struct("astria.execution.v1.GenesisInfo", len)?; if let Some(v) = self.rollup_id.as_ref() { struct_ser.serialize_field("rollupId", v)?; @@ -752,6 +755,9 @@ impl serde::Serialize for GenesisInfo { if !self.celestia_chain_id.is_empty() { struct_ser.serialize_field("celestiaChainId", &self.celestia_chain_id)?; } + if self.halt_at_stop_height { + struct_ser.serialize_field("haltAtStopHeight", &self.halt_at_stop_height)?; + } struct_ser.end() } } @@ -776,6 +782,8 @@ impl<'de> serde::Deserialize<'de> for GenesisInfo { "sequencerChainId", "celestia_chain_id", "celestiaChainId", + "halt_at_stop_height", + "haltAtStopHeight", ]; #[allow(clippy::enum_variant_names)] @@ -787,6 +795,7 @@ impl<'de> serde::Deserialize<'de> for GenesisInfo { RollupStartBlockHeight, SequencerChainId, CelestiaChainId, + HaltAtStopHeight, } impl<'de> serde::Deserialize<'de> for GeneratedField { fn deserialize(deserializer: D) -> std::result::Result @@ -815,6 +824,7 @@ impl<'de> serde::Deserialize<'de> for GenesisInfo { "rollupStartBlockHeight" | "rollup_start_block_height" => Ok(GeneratedField::RollupStartBlockHeight), "sequencerChainId" | "sequencer_chain_id" => Ok(GeneratedField::SequencerChainId), "celestiaChainId" | "celestia_chain_id" => Ok(GeneratedField::CelestiaChainId), + "haltAtStopHeight" | "halt_at_stop_height" => Ok(GeneratedField::HaltAtStopHeight), _ => Err(serde::de::Error::unknown_field(value, FIELDS)), } } @@ -841,6 +851,7 @@ impl<'de> serde::Deserialize<'de> for GenesisInfo { let mut rollup_start_block_height__ = None; let mut sequencer_chain_id__ = None; let mut celestia_chain_id__ = None; + let mut halt_at_stop_height__ = None; while let Some(k) = map_.next_key()? { match k { GeneratedField::RollupId => { @@ -893,6 +904,12 @@ impl<'de> serde::Deserialize<'de> for GenesisInfo { } celestia_chain_id__ = Some(map_.next_value()?); } + GeneratedField::HaltAtStopHeight => { + if halt_at_stop_height__.is_some() { + return Err(serde::de::Error::duplicate_field("haltAtStopHeight")); + } + halt_at_stop_height__ = Some(map_.next_value()?); + } } } Ok(GenesisInfo { @@ -903,6 +920,7 @@ impl<'de> serde::Deserialize<'de> for GenesisInfo { rollup_start_block_height: rollup_start_block_height__.unwrap_or_default(), sequencer_chain_id: sequencer_chain_id__.unwrap_or_default(), celestia_chain_id: celestia_chain_id__.unwrap_or_default(), + halt_at_stop_height: halt_at_stop_height__.unwrap_or_default(), }) } } diff --git a/proto/executionapis/astria/execution/v1/execution.proto b/proto/executionapis/astria/execution/v1/execution.proto index 62a108a7d3..fe7cd76154 100644 --- a/proto/executionapis/astria/execution/v1/execution.proto +++ b/proto/executionapis/astria/execution/v1/execution.proto @@ -23,6 +23,8 @@ message GenesisInfo { uint64 rollup_start_block_height = 5; string sequencer_chain_id = 6; string celestia_chain_id = 7; + // True if the conductor should halt at the stop height instead of attempting restart. + bool halt_at_stop_height = 8; } // The set of information which deterministic driver of block production From 00b10a42d9993f3d27fe35cad1b0c11f0ab92598 Mon Sep 17 00:00:00 2001 From: ethanoroshiba Date: Mon, 6 Jan 2025 13:19:31 -0600 Subject: [PATCH 25/27] use enum to propagate exit reason instead of errors --- crates/astria-conductor/src/celestia/mod.rs | 13 +- .../astria-conductor/src/conductor/inner.rs | 64 +++++----- crates/astria-conductor/src/conductor/mod.rs | 1 + crates/astria-conductor/src/executor/mod.rs | 111 ++++++++++++------ crates/astria-conductor/src/sequencer/mod.rs | 15 +-- 5 files changed, 124 insertions(+), 80 deletions(-) diff --git a/crates/astria-conductor/src/celestia/mod.rs b/crates/astria-conductor/src/celestia/mod.rs index 0a0a0df9d7..e0e1bfa0f7 100644 --- a/crates/astria-conductor/src/celestia/mod.rs +++ b/crates/astria-conductor/src/celestia/mod.rs @@ -59,6 +59,7 @@ use tracing::{ use crate::{ block_cache::GetSequencerHeight, + conductor::ExitReason, executor::{ FirmSendError, FirmTrySendError, @@ -148,13 +149,13 @@ pub(crate) struct Reader { } impl Reader { - pub(crate) async fn run_until_stopped(mut self) -> eyre::Result<()> { + pub(crate) async fn run_until_stopped(mut self) -> eyre::Result { let ((), executor, sequencer_chain_id) = select!( () = self.shutdown.clone().cancelled_owned() => { info_span!("conductor::celestia::Reader::run_until_stopped").in_scope(|| info!("received shutdown signal while waiting for Celestia reader task to initialize") ); - return Ok(()); + return Ok(ExitReason::ShutdownSignal); } res = self.initialize() => { @@ -362,7 +363,7 @@ impl RunningReader { }) } - async fn run_until_stopped(mut self) -> eyre::Result<()> { + async fn run_until_stopped(mut self) -> eyre::Result { info_span!("conductor::celestia::RunningReader::run_until_stopped").in_scope(|| { info!( initial_celestia_height = self.celestia_next_height, @@ -383,7 +384,7 @@ impl RunningReader { biased; () = self.shutdown.cancelled() => { - break Ok("received shutdown signal"); + break Ok(ExitReason::ShutdownSignal); } res = &mut self.enqueued_block, if self.waiting_for_executor_capacity() => { @@ -699,11 +700,11 @@ fn max_permitted_celestia_height(reference: u64, variance: u64) -> u64 { } #[instrument(skip_all)] -fn report_exit(exit_reason: eyre::Result<&str>, message: &str) -> eyre::Result<()> { +fn report_exit(exit_reason: eyre::Result, message: &str) -> eyre::Result { match exit_reason { Ok(reason) => { info!(%reason, message); - Ok(()) + Ok(reason) } Err(reason) => { error!(%reason, message); diff --git a/crates/astria-conductor/src/conductor/inner.rs b/crates/astria-conductor/src/conductor/inner.rs index 8f6c0c467d..67a9194669 100644 --- a/crates/astria-conductor/src/conductor/inner.rs +++ b/crates/astria-conductor/src/conductor/inner.rs @@ -5,7 +5,6 @@ use std::{ use astria_eyre::eyre::{ self, - eyre, Result, WrapErr as _, }; @@ -45,12 +44,30 @@ pub(super) enum RestartOrShutdown { Shutdown, } -enum ExitReason { +pub(crate) enum ExitReason { ShutdownSignal, TaskFailed { name: &'static str, error: eyre::Report, }, + StopHeightExceded(StopHeightExceded), + ChannelsClosed, +} + +impl std::fmt::Display for ExitReason { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + ExitReason::ShutdownSignal => write!(f, "received shutdown signal"), + ExitReason::TaskFailed { + name, + error, + } => { + write!(f, "task `{name}` failed: {error}") + } + ExitReason::StopHeightExceded(reason) => write!(f, "{reason}"), + ExitReason::ChannelsClosed => write!(f, "firm and soft block channels are both closed"), + } + } } pin_project! { @@ -83,7 +100,7 @@ pub(super) struct ConductorInner { shutdown_token: CancellationToken, /// The different long-running tasks that make up the conductor; - tasks: JoinMap<&'static str, eyre::Result<()>>, + tasks: JoinMap<&'static str, eyre::Result>, } impl ConductorInner { @@ -186,7 +203,7 @@ impl ConductorInner { Some((name, res)) = self.tasks.join_next() => { match flatten(res) { - Ok(()) => ExitReason::TaskFailed{name, error: eyre!("task `{name}` exited unexpectedly")}, + Ok(exit_reason) => exit_reason, Err(err) => ExitReason::TaskFailed{name, error: err.wrap_err(format!("task `{name}` failed"))}, } } @@ -237,6 +254,12 @@ impl ConductorInner { restart_or_shutdown = RestartOrShutdown::Restart; } } + ExitReason::StopHeightExceded(_) => { + restart_or_shutdown = RestartOrShutdown::Restart; + } + ExitReason::ChannelsClosed => { + info!("firm and soft block channels are both closed, shutting down"); + } } info!("signalled all tasks to shut down; waiting for 25 seconds to exit"); @@ -245,8 +268,8 @@ impl ConductorInner { while let Some((name, res)) = self.tasks.join_next().await { let message = "task shut down"; match flatten(res) { - Ok(()) => { - info!(name, message); + Ok(exit_reason) => { + info!(name, message, %exit_reason); } Err(error) => { if check_for_restart(name, &error) @@ -295,6 +318,13 @@ fn report_exit(exit_reason: &ExitReason, message: &str) { name: task, error: reason, } => error!(%reason, %task, message), + ExitReason::StopHeightExceded(stop_height_exceded) => { + info!(%stop_height_exceded, "restarting"); + } + ExitReason::ChannelsClosed => info!( + reason = "firm and soft block channels are both closed", + message + ), } } @@ -309,8 +339,6 @@ fn check_for_restart(name: &str, err: &eyre::Report) -> bool { if status.code() == tonic::Code::PermissionDenied { return true; } - } else if err.downcast_ref::().is_some() { - return true; } current = err.source(); } @@ -329,25 +357,5 @@ mod tests { let err = err.wrap_err("wrapper_2"); let err = err.wrap_err("wrapper_3"); assert!(super::check_for_restart("executor", &err.unwrap_err())); - - let celestia_height_error: Result<&str, super::StopHeightExceded> = - Err(super::StopHeightExceded::Celestia { - firm_height: 1, - stop_height: 1, - }); - let err = celestia_height_error.wrap_err("wrapper_1"); - let err = err.wrap_err("wrapper_2"); - let err = err.wrap_err("wrapper_3"); - assert!(super::check_for_restart("executor", &err.unwrap_err())); - - let sequencer_height_error: Result<&str, super::StopHeightExceded> = - Err(super::StopHeightExceded::Sequencer { - soft_height: 1, - stop_height: 1, - }); - let err = sequencer_height_error.wrap_err("wrapper_1"); - let err = err.wrap_err("wrapper_2"); - let err = err.wrap_err("wrapper_3"); - assert!(super::check_for_restart("executor", &err.unwrap_err())); } } diff --git a/crates/astria-conductor/src/conductor/mod.rs b/crates/astria-conductor/src/conductor/mod.rs index f7450d0668..b9c28c09f5 100644 --- a/crates/astria-conductor/src/conductor/mod.rs +++ b/crates/astria-conductor/src/conductor/mod.rs @@ -9,6 +9,7 @@ use astria_eyre::eyre::{ self, Result, }; +pub(crate) use inner::ExitReason; use inner::{ ConductorInner, InnerHandle, diff --git a/crates/astria-conductor/src/executor/mod.rs b/crates/astria-conductor/src/executor/mod.rs index 4277adc843..d6d38223db 100644 --- a/crates/astria-conductor/src/executor/mod.rs +++ b/crates/astria-conductor/src/executor/mod.rs @@ -40,6 +40,7 @@ use tracing::{ use crate::{ celestia::ReconstructedBlock, + conductor::ExitReason, config::CommitLevel, metrics::Metrics, }; @@ -67,11 +68,9 @@ pub(crate) struct StateNotInit; #[derive(Clone, Debug)] pub(crate) struct StateIsInit; -#[derive(Debug, thiserror::Error)] -pub(super) enum StopHeightExceded { - #[error("firm height {firm_height} exceeded sequencer stop height {stop_height}")] +#[derive(Debug)] +pub(crate) enum StopHeightExceded { Celestia { firm_height: u64, stop_height: u64 }, - #[error("soft height {soft_height} met or exceeded sequencer stop height {stop_height}")] Sequencer { soft_height: u64, stop_height: u64 }, } @@ -91,6 +90,27 @@ impl StopHeightExceded { } } +impl std::fmt::Display for StopHeightExceded { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + StopHeightExceded::Celestia { + firm_height, + stop_height, + } => write!( + f, + "firm block height {firm_height} exceded stop height {stop_height}", + ), + StopHeightExceded::Sequencer { + soft_height, + stop_height, + } => write!( + f, + "soft block height {soft_height} exceded stop height {stop_height}", + ), + } + } +} + #[derive(Debug, thiserror::Error)] pub(crate) enum FirmSendError { #[error("executor was configured without firm commitments")] @@ -284,13 +304,12 @@ pub(crate) struct Executor { } impl Executor { - pub(crate) async fn run_until_stopped(mut self) -> eyre::Result<()> { + pub(crate) async fn run_until_stopped(mut self) -> eyre::Result { select!( () = self.shutdown.clone().cancelled_owned() => { return report_exit(Ok( - "received shutdown signal while initializing task; \ - aborting intialization and exiting" - ), ""); + ExitReason::ShutdownSignal + ), "aborting initialization due to shutdown signal"); } res = self.init() => { res.wrap_err("initialization failed")?; @@ -298,6 +317,10 @@ impl Executor { ); let reason = loop { + if self.firm_blocks.is_none() && self.soft_blocks.is_none() { + break Ok(ExitReason::ChannelsClosed); + } + let spread_not_too_large = !self.is_spread_too_large(); if spread_not_too_large { if let Some(channel) = self.soft_blocks.as_mut() { @@ -309,7 +332,7 @@ impl Executor { biased; () = self.shutdown.cancelled() => { - break Ok("received shutdown signal"); + break Ok(ExitReason::ShutdownSignal); } Some(block) = async { self.firm_blocks.as_mut().unwrap().recv().await }, @@ -320,8 +343,14 @@ impl Executor { block.hash = %telemetry::display::base64(&block.block_hash), "received block from celestia reader", )); - if let Err(error) = self.execute_firm(block).await { - break Err(error).wrap_err("failed executing firm block"); + match self.execute_firm(block).await { + Ok(None) => {}, + Ok(Some(stop_height_exceded)) => { + break Ok(ExitReason::StopHeightExceded(stop_height_exceded)); + } + Err(error) => { + break Err(error).wrap_err("failed executing firm block"); + } } } @@ -333,8 +362,14 @@ impl Executor { block.hash = %telemetry::display::base64(&block.block_hash()), "received block from sequencer reader", )); - if let Err(error) = self.execute_soft(block).await { - break Err(error).wrap_err("failed executing soft block"); + match self.execute_soft(block).await { + Ok(None) => {}, + Ok(Some(stop_height_exceded)) => { + break Ok(ExitReason::StopHeightExceded(stop_height_exceded)); + } + Err(error) => { + break Err(error).wrap_err("failed executing soft block"); + } } } ); @@ -424,7 +459,10 @@ impl Executor { block.height = block.height().value(), err, ))] - async fn execute_soft(&mut self, block: FilteredSequencerBlock) -> eyre::Result<()> { + async fn execute_soft( + &mut self, + block: FilteredSequencerBlock, + ) -> eyre::Result> { // TODO(https://github.com/astriaorg/astria/issues/624): add retry logic before failing hard. let executable_block = ExecutableBlock::from_sequencer(block, self.state.rollup_id()); @@ -434,22 +472,18 @@ impl Executor { // return a `StopHeightExceded::Sequencer` error to signal a restart. if self.is_soft_block_height_exceded(&executable_block) { if self.mode.is_with_firm() { - info!( - height = %executable_block.height, - "received soft block whose height is greater than or equal to stop block height. \ - dropping soft block and waiting for next firm block before attempting restart. \ - will continue logging info of soft blocks until restart", - ); - return Ok(()); + // If we are continuing to execute firm blocks, we should close the soft block + // channel so as to not keep hitting the sequencer RPC endpoint. + if let Some(channel) = self.soft_blocks.take() { + drop(channel); + } + self.soft_blocks = None; + return Ok(None); } - return Err(StopHeightExceded::sequencer( + return Ok(Some(StopHeightExceded::sequencer( executable_block.height.value(), self.state.sequencer_stop_block_height(), - )) - .wrap_err( - "soft height met or exceeded sequencer stop height, attempting restart with new \ - genesis info", - ); + ))); } let expected_height = self.state.next_expected_soft_sequencer_height(); @@ -459,7 +493,7 @@ impl Executor { expected_height.sequencer_block = %expected_height, "block received was stale because firm blocks were executed first; dropping", ); - return Ok(()); + return Ok(None); } std::cmp::Ordering::Greater => bail!( "block received was out-of-order; was a block skipped? expected: \ @@ -507,7 +541,7 @@ impl Executor { self.metrics .absolute_set_executed_soft_block_number(block_number); - Ok(()) + Ok(None) } #[instrument(skip_all, fields( @@ -515,16 +549,15 @@ impl Executor { block.height = block.sequencer_height().value(), err, ))] - async fn execute_firm(&mut self, block: ReconstructedBlock) -> eyre::Result<()> { + async fn execute_firm( + &mut self, + block: ReconstructedBlock, + ) -> eyre::Result> { if self.is_firm_block_height_exceded(&block) { - return Err(StopHeightExceded::celestia( + return Ok(Some(StopHeightExceded::celestia( block.header.height().value(), self.state.sequencer_stop_block_height(), - )) - .wrap_err( - "firm height exceeded sequencer stop height, attempting restart with new genesis \ - info", - ); + ))); } let celestia_height = block.celestia_height; @@ -597,7 +630,7 @@ impl Executor { self.metrics .absolute_set_executed_soft_block_number(block_number); - Ok(()) + Ok(None) } /// Executes `block` on top of its `parent_hash`. @@ -755,11 +788,11 @@ impl Executor { } #[instrument(skip_all)] -fn report_exit(reason: eyre::Result<&str>, message: &str) -> eyre::Result<()> { +fn report_exit(reason: eyre::Result, message: &str) -> eyre::Result { match reason { Ok(reason) => { info!(%reason, message); - Ok(()) + Ok(reason) } Err(error) => { error!(%error, message); diff --git a/crates/astria-conductor/src/sequencer/mod.rs b/crates/astria-conductor/src/sequencer/mod.rs index 1d399b84b5..ddecb87703 100644 --- a/crates/astria-conductor/src/sequencer/mod.rs +++ b/crates/astria-conductor/src/sequencer/mod.rs @@ -40,6 +40,7 @@ use tracing::{ use crate::{ block_cache::BlockCache, + conductor::ExitReason, executor::{ self, SoftSendError, @@ -83,10 +84,10 @@ pub(crate) struct Reader { } impl Reader { - pub(crate) async fn run_until_stopped(mut self) -> eyre::Result<()> { + pub(crate) async fn run_until_stopped(mut self) -> eyre::Result { let executor = select!( () = self.shutdown.clone().cancelled_owned() => { - return report_exit(Ok("received shutdown signal while waiting for Sequencer reader task to initialize"), ""); + return report_exit(Ok(ExitReason::ShutdownSignal), "received shutdown signal while waiting for Sequencer reader task to initialize"); } res = self.initialize() => { res? @@ -189,7 +190,7 @@ impl RunningReader { }) } - async fn run_until_stopped(mut self) -> eyre::Result<()> { + async fn run_until_stopped(mut self) -> eyre::Result { let stop_reason = self.run_loop().await; // XXX: explicitly setting the message (usually implicitly set by tracing) @@ -197,7 +198,7 @@ impl RunningReader { report_exit(stop_reason, message) } - async fn run_loop(&mut self) -> eyre::Result<&'static str> { + async fn run_loop(&mut self) -> eyre::Result { use futures::future::FusedFuture as _; loop { @@ -205,7 +206,7 @@ impl RunningReader { biased; () = self.shutdown.cancelled() => { - return Ok("received shutdown signal"); + return Ok(ExitReason::ShutdownSignal); } // Process block execution which was enqueued due to executor channel being full. @@ -318,11 +319,11 @@ impl RunningReader { } #[instrument(skip_all)] -fn report_exit(reason: eyre::Result<&str>, message: &str) -> eyre::Result<()> { +fn report_exit(reason: eyre::Result, message: &str) -> eyre::Result { match reason { Ok(reason) => { info!(%reason, message); - Ok(()) + Ok(reason) } Err(reason) => { error!(%reason, message); From 02d04d77f89732f3d36597d9df0d6bba211b02ed Mon Sep 17 00:00:00 2001 From: ethanoroshiba Date: Mon, 6 Jan 2025 13:44:03 -0600 Subject: [PATCH 26/27] add halt logic --- .../astria-conductor/src/conductor/inner.rs | 16 +- crates/astria-conductor/src/executor/mod.rs | 54 ++++- .../tests/blackbox/soft_and_firm.rs | 221 +++++++++--------- crates/astria-core/src/execution/v1/mod.rs | 5 + 4 files changed, 166 insertions(+), 130 deletions(-) diff --git a/crates/astria-conductor/src/conductor/inner.rs b/crates/astria-conductor/src/conductor/inner.rs index 67a9194669..6f99514b80 100644 --- a/crates/astria-conductor/src/conductor/inner.rs +++ b/crates/astria-conductor/src/conductor/inner.rs @@ -250,12 +250,14 @@ impl ConductorInner { name, error, } => { - if check_for_restart(name, error) { + if check_err_for_restart(name, error) { restart_or_shutdown = RestartOrShutdown::Restart; } } - ExitReason::StopHeightExceded(_) => { - restart_or_shutdown = RestartOrShutdown::Restart; + ExitReason::StopHeightExceded(stop_height_exceded) => { + if !stop_height_exceded.halt() { + restart_or_shutdown = RestartOrShutdown::Restart; + } } ExitReason::ChannelsClosed => { info!("firm and soft block channels are both closed, shutting down"); @@ -272,7 +274,7 @@ impl ConductorInner { info!(name, message, %exit_reason); } Err(error) => { - if check_for_restart(name, &error) + if check_err_for_restart(name, &error) && !matches!(exit_reason, ExitReason::ShutdownSignal) { restart_or_shutdown = RestartOrShutdown::Restart; @@ -329,7 +331,7 @@ fn report_exit(exit_reason: &ExitReason, message: &str) { } #[instrument(skip_all)] -fn check_for_restart(name: &str, err: &eyre::Report) -> bool { +fn check_err_for_restart(name: &str, err: &eyre::Report) -> bool { if name != ConductorInner::EXECUTOR { return false; } @@ -350,12 +352,12 @@ mod tests { use astria_eyre::eyre::WrapErr as _; #[test] - fn check_for_restart_ok() { + fn check_err_for_restart_ok() { let tonic_error: Result<&str, tonic::Status> = Err(tonic::Status::new(tonic::Code::PermissionDenied, "error")); let err = tonic_error.wrap_err("wrapper_1"); let err = err.wrap_err("wrapper_2"); let err = err.wrap_err("wrapper_3"); - assert!(super::check_for_restart("executor", &err.unwrap_err())); + assert!(super::check_err_for_restart("executor", &err.unwrap_err())); } } diff --git a/crates/astria-conductor/src/executor/mod.rs b/crates/astria-conductor/src/executor/mod.rs index d6d38223db..5c12bb04ac 100644 --- a/crates/astria-conductor/src/executor/mod.rs +++ b/crates/astria-conductor/src/executor/mod.rs @@ -70,22 +70,40 @@ pub(crate) struct StateIsInit; #[derive(Debug)] pub(crate) enum StopHeightExceded { - Celestia { firm_height: u64, stop_height: u64 }, - Sequencer { soft_height: u64, stop_height: u64 }, + Celestia { + firm_height: u64, + stop_height: u64, + halt: bool, + }, + Sequencer { + soft_height: u64, + stop_height: u64, + halt: bool, + }, } impl StopHeightExceded { - fn celestia(firm_height: u64, stop_height: u64) -> Self { + fn celestia(firm_height: u64, stop_height: u64, halt: bool) -> Self { StopHeightExceded::Celestia { firm_height, stop_height, + halt, } } - fn sequencer(soft_height: u64, stop_height: u64) -> Self { + fn sequencer(soft_height: u64, stop_height: u64, halt: bool) -> Self { StopHeightExceded::Sequencer { soft_height, stop_height, + halt, + } + } + + /// Returns whether the conductor should halt at the stop height. + pub(crate) fn halt(&self) -> bool { + match self { + StopHeightExceded::Celestia { halt, .. } + | StopHeightExceded::Sequencer { halt, .. } => *halt, } } } @@ -96,17 +114,27 @@ impl std::fmt::Display for StopHeightExceded { StopHeightExceded::Celestia { firm_height, stop_height, - } => write!( - f, - "firm block height {firm_height} exceded stop height {stop_height}", - ), + halt, + } => { + let halt_msg = if *halt { "halting" } else { "restarting" }; + write!( + f, + "firm block height {firm_height} exceded stop height {stop_height}, \ + {halt_msg}...", + ) + } StopHeightExceded::Sequencer { soft_height, stop_height, - } => write!( - f, - "soft block height {soft_height} exceded stop height {stop_height}", - ), + halt, + } => { + let halt_msg = if *halt { "halting" } else { "restarting" }; + write!( + f, + "soft block height {soft_height} exceded stop height {stop_height}, \ + {halt_msg}...", + ) + } } } } @@ -483,6 +511,7 @@ impl Executor { return Ok(Some(StopHeightExceded::sequencer( executable_block.height.value(), self.state.sequencer_stop_block_height(), + self.state.genesis_info().halt_at_stop_height(), ))); } @@ -557,6 +586,7 @@ impl Executor { return Ok(Some(StopHeightExceded::celestia( block.header.height().value(), self.state.sequencer_stop_block_height(), + self.state.genesis_info().halt_at_stop_height(), ))); } diff --git a/crates/astria-conductor/tests/blackbox/soft_and_firm.rs b/crates/astria-conductor/tests/blackbox/soft_and_firm.rs index f5c55b558f..11e7e67607 100644 --- a/crates/astria-conductor/tests/blackbox/soft_and_firm.rs +++ b/crates/astria-conductor/tests/blackbox/soft_and_firm.rs @@ -820,114 +820,113 @@ async fn conductor_restarts_after_reaching_stop_height() { .expect("conductor should have updated the firm commitment state within 1000ms"); } -// ** Disabled until logic to handle this is implemented ** -// /// Tests if the conductor correctly stops and does not restart after reaching the sequencer stop -// /// height if genesis info's `halt_at_stop_height` is `true`. -// /// -// /// This test consists of the following steps: -// /// 1. Mount commitment state and genesis info with a sequencer stop height of 3, expecting only -// 1 /// response. -// /// 2. Mount Celestia network head and sequencer genesis. -// /// 3. Mount ABCI info and sequencer (soft blocks) for height 3. -// /// 4. Mount firm blocks at height 3, with corresponding `update_commitment_state` mount. -// /// 5. Mount `execute_block` and `update_commitment_state` for firm block at height -// /// 3. The soft block should not be executed since it is at the stop height, but the firm -// should /// be. -// /// 6. Await satisfaction of the `execute_block` and `update_commitment_state` for the firm block -// at /// height 3 with a timeout of 1000ms. -// /// 7. Allow ample time for the conductor to potentially restart erroneously. -// #[tokio::test(flavor = "multi_thread", worker_threads = 1)] -// async fn conductor_stops_at_stop_height() { -// let test_conductor = spawn_conductor(CommitLevel::SoftAndFirm).await; - -// mount_get_genesis_info!( -// test_conductor, -// sequencer_start_block_height: 1, -// sequencer_stop_block_height: 3, -// celestia_block_variance: 10, -// rollup_start_block_height: 0, -// up_to_n_times: 2, // allow for calls after an potential erroneous restart -// halt_at_stop_height: true, -// expected_calls: 1, -// ); - -// mount_get_commitment_state!( -// test_conductor, -// firm: ( -// number: 1, -// hash: [1; 64], -// parent: [0; 64], -// ), -// soft: ( -// number: 1, -// hash: [1; 64], -// parent: [0; 64], -// ), -// base_celestia_height: 1, -// ); - -// mount_sequencer_genesis!(test_conductor); -// mount_celestia_header_network_head!( -// test_conductor, -// height: 1u32, -// ); -// mount_abci_info!( -// test_conductor, -// latest_sequencer_height: 3, -// ); - -// // Mount soft blocks for height 3 -// mount_get_filtered_sequencer_block!( -// test_conductor, -// sequencer_height: 3, -// ); - -// // Mount firm blocks for height 3 -// mount_celestia_blobs!( -// test_conductor, -// celestia_height: 1, -// sequencer_heights: [3], -// ); -// mount_sequencer_commit!( -// test_conductor, -// height: 3u32, -// ); -// mount_sequencer_validator_set!(test_conductor, height: 2u32); - -// let execute_block_1 = mount_executed_block!( -// test_conductor, -// mock_name: "execute_block_1", -// number: 2, -// hash: [2; 64], -// parent: [1; 64], -// ); - -// let update_commitment_state_firm_1 = mount_update_commitment_state!( -// test_conductor, -// mock_name: "update_commitment_state_firm_1", -// firm: ( -// number: 2, -// hash: [2; 64], -// parent: [1; 64], -// ), -// soft: ( -// number: 2, -// hash: [2; 64], -// parent: [1; 64], -// ), -// base_celestia_height: 1, -// ); - -// timeout( -// Duration::from_millis(1000), -// join( -// execute_block_1.wait_until_satisfied(), -// update_commitment_state_firm_1.wait_until_satisfied(), -// ), -// ) -// .await -// .expect("conductor should have updated the firm commitment state within 1000ms"); - -// // Allow time for a potential erroneous restart -// sleep(Duration::from_millis(1000)).await; -// } +/// Tests if the conductor correctly stops and does not restart after reaching the sequencer stop +/// height if genesis info's `halt_at_stop_height` is `true`. +/// +/// This test consists of the following steps: +/// 1. Mount commitment state and genesis info with a sequencer stop height of 3, expecting only 1 +/// response. +/// 2. Mount Celestia network head and sequencer genesis. +/// 3. Mount ABCI info and sequencer (soft blocks) for height 3. +/// 4. Mount firm blocks at height 3, with corresponding `update_commitment_state` mount. +/// 5. Mount `execute_block` and `update_commitment_state` for firm block at height +/// 3. The soft block should not be executed since it is at the stop height, but the firmshould +/// be. +/// 6. Await satisfaction of the `execute_block` and `update_commitment_state` for the firm block +/// height 3 with a timeout of 1000ms. +/// 7. Allow ample time for the conductor to potentially restart erroneously. +#[tokio::test(flavor = "multi_thread", worker_threads = 1)] +async fn conductor_stops_at_stop_height() { + let test_conductor = spawn_conductor(CommitLevel::SoftAndFirm).await; + + mount_get_genesis_info!( + test_conductor, + sequencer_start_block_height: 1, + sequencer_stop_block_height: 3, + celestia_block_variance: 10, + rollup_start_block_height: 0, + up_to_n_times: 2, // allow for calls after an potential erroneous restart + halt_at_stop_height: true, + expected_calls: 1, + ); + + mount_get_commitment_state!( + test_conductor, + firm: ( + number: 1, + hash: [1; 64], + parent: [0; 64], + ), + soft: ( + number: 1, + hash: [1; 64], + parent: [0; 64], + ), + base_celestia_height: 1, + ); + + mount_sequencer_genesis!(test_conductor); + mount_celestia_header_network_head!( + test_conductor, + height: 1u32, + ); + mount_abci_info!( + test_conductor, + latest_sequencer_height: 3, + ); + + // Mount soft blocks for height 3 + mount_get_filtered_sequencer_block!( + test_conductor, + sequencer_height: 3, + ); + + // Mount firm blocks for height 3 + mount_celestia_blobs!( + test_conductor, + celestia_height: 1, + sequencer_heights: [3], + ); + mount_sequencer_commit!( + test_conductor, + height: 3u32, + ); + mount_sequencer_validator_set!(test_conductor, height: 2u32); + + let execute_block_1 = mount_executed_block!( + test_conductor, + mock_name: "execute_block_1", + number: 2, + hash: [2; 64], + parent: [1; 64], + ); + + let update_commitment_state_firm_1 = mount_update_commitment_state!( + test_conductor, + mock_name: "update_commitment_state_firm_1", + firm: ( + number: 2, + hash: [2; 64], + parent: [1; 64], + ), + soft: ( + number: 2, + hash: [2; 64], + parent: [1; 64], + ), + base_celestia_height: 1, + ); + + timeout( + Duration::from_millis(1000), + join( + execute_block_1.wait_until_satisfied(), + update_commitment_state_firm_1.wait_until_satisfied(), + ), + ) + .await + .expect("conductor should have updated the firm commitment state within 1000ms"); + + // Allow time for a potential erroneous restart + sleep(Duration::from_millis(1000)).await; +} diff --git a/crates/astria-core/src/execution/v1/mod.rs b/crates/astria-core/src/execution/v1/mod.rs index 80ebe83c31..ac8dc15a7c 100644 --- a/crates/astria-core/src/execution/v1/mod.rs +++ b/crates/astria-core/src/execution/v1/mod.rs @@ -111,6 +111,11 @@ impl GenesisInfo { pub fn rollup_start_block_height(&self) -> u64 { self.rollup_start_block_height } + + #[must_use] + pub fn halt_at_stop_height(&self) -> bool { + self.halt_at_stop_height + } } impl From for raw::GenesisInfo { From af7ebd0f2e05c77aafd276c1f89a6681343c7190 Mon Sep 17 00:00:00 2001 From: ethanoroshiba Date: Mon, 6 Jan 2025 13:46:23 -0600 Subject: [PATCH 27/27] rustfmt --- crates/astria-conductor/src/executor/mod.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/crates/astria-conductor/src/executor/mod.rs b/crates/astria-conductor/src/executor/mod.rs index 5c12bb04ac..7acad76aad 100644 --- a/crates/astria-conductor/src/executor/mod.rs +++ b/crates/astria-conductor/src/executor/mod.rs @@ -102,8 +102,12 @@ impl StopHeightExceded { /// Returns whether the conductor should halt at the stop height. pub(crate) fn halt(&self) -> bool { match self { - StopHeightExceded::Celestia { halt, .. } - | StopHeightExceded::Sequencer { halt, .. } => *halt, + StopHeightExceded::Celestia { + halt, .. + } + | StopHeightExceded::Sequencer { + halt, .. + } => *halt, } } }