diff --git a/Cargo.lock b/Cargo.lock index 130b3bff21..7c746f9f02 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -863,6 +863,7 @@ dependencies = [ "tower-actor", "tower-http", "tracing", + "url", ] [[package]] diff --git a/charts/sequencer/Chart.yaml b/charts/sequencer/Chart.yaml index 832ce7df76..5107f393d6 100644 --- a/charts/sequencer/Chart.yaml +++ b/charts/sequencer/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.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. diff --git a/charts/sequencer/files/cometbft/config/config.toml b/charts/sequencer/files/cometbft/config/config.toml index b6d8ae68e1..586227f6e9 100644 --- a/charts/sequencer/files/cometbft/config/config.toml +++ b/charts/sequencer/files/cometbft/config/config.toml @@ -16,7 +16,7 @@ version = "0.38.8" # TCP or UNIX socket address of the ABCI application, # or the name of an ABCI application compiled in with the CometBFT binary -proxy_app = "tcp://127.0.0.1:{{ .Values.ports.sequencerABCI }}" +proxy_app = "{{ include "sequencer.abci_url" . }}" # A custom human readable name for this node moniker = "{{ .Values.moniker }}" diff --git a/charts/sequencer/files/scripts/init-cometbft.sh b/charts/sequencer/files/scripts/init-cometbft.sh index 9cc643f81b..800f7daeeb 100644 --- a/charts/sequencer/files/scripts/init-cometbft.sh +++ b/charts/sequencer/files/scripts/init-cometbft.sh @@ -13,5 +13,3 @@ if [ ! -d "/cometbft/config" ]; then else cp /config/* /cometbft/config/ fi - -chmod -R 0777 /cometbft diff --git a/charts/sequencer/templates/_helpers.tpl b/charts/sequencer/templates/_helpers.tpl index 2dc1da25ab..109bf5cd2a 100644 --- a/charts/sequencer/templates/_helpers.tpl +++ b/charts/sequencer/templates/_helpers.tpl @@ -69,9 +69,23 @@ name: {{ .Values.moniker }}-sequencer-metrics {{- end }} {{/* New sequencer address */}} -{{- define "sequencer.address"}}{ "bech32m": "{{ . }}" } +{{- define "sequencer.address" -}} +{ "bech32m": "{{ . }}" } {{- end }} {{/* uint64 fee converted to a astria proto Uint128 with only lo set */}} -{{- define "sequencer.toUint128Proto"}}{ "lo": {{ . }} } +{{- define "sequencer.toUint128Proto" -}} +{ "lo": {{ . }} } +{{- end }} + +{{- define "sequencer.socket_directory" -}} +/sockets/ +{{- end }} + +{{- define "sequencer.abci_url" -}} +{{- if and .Values.global.dev .Values.sequencer.abciUDS -}} +unix://{{- include "sequencer.socket_directory" . }}abci.sock +{{- else -}} +tcp://127.0.0.1:{{ .Values.ports.sequencerABCI }} +{{- end }} {{- end }} diff --git a/charts/sequencer/templates/configmaps.yaml b/charts/sequencer/templates/configmaps.yaml index 89f9deedea..1b4b994459 100644 --- a/charts/sequencer/templates/configmaps.yaml +++ b/charts/sequencer/templates/configmaps.yaml @@ -54,8 +54,7 @@ metadata: name: {{ .Values.moniker }}-sequencer-env namespace: {{ include "sequencer.namespace" . }} data: - ASTRIA_SEQUENCER_LOG: "astria_sequencer=debug" - ASTRIA_SEQUENCER_LISTEN_ADDR: "127.0.0.1:{{ .Values.ports.sequencerABCI }}" + ASTRIA_SEQUENCER_LOG: "info" ASTRIA_SEQUENCER_DB_FILEPATH: "/sequencer/penumbra.db" ASTRIA_SEQUENCER_MEMPOOL_PARKED_MAX_TX_COUNT: "{{ .Values.sequencer.mempool.parked.maxTxCount }}" # Socket address for GRPC server @@ -74,6 +73,8 @@ data: OTEL_EXPORTER_OTLP_TRACE_HEADERS: "{{ .Values.sequencer.otel.traceHeaders }}" OTEL_SERVICE_NAME: "{{ tpl .Values.sequencer.otel.serviceName . }}" {{- if not .Values.global.dev }} + ASTRIA_SEQUENCER_LISTEN_ADDR: "127.0.0.1:{{ .Values.ports.sequencerABCI }}" {{- else }} + ASTRIA_SEQUENCER_ABCI_LISTEN_URL: "{{ include "sequencer.abci_url" . }}" {{- end }} --- diff --git a/charts/sequencer/templates/statefulsets.yaml b/charts/sequencer/templates/statefulsets.yaml index 362a410f61..097a7fc4a5 100644 --- a/charts/sequencer/templates/statefulsets.yaml +++ b/charts/sequencer/templates/statefulsets.yaml @@ -16,6 +16,9 @@ spec: labels: app: {{ .Values.moniker }}-sequencer spec: + securityContext: + runAsUser: 1000 + fsGroup: 2000 initContainers: - command: [ "/scripts/init-cometbft.sh" ] name: config-cometbft @@ -45,6 +48,8 @@ spec: - mountPath: /sequencer name: sequencer-shared-storage-vol subPath: {{ .Values.moniker }}/sequencer + - mountPath: {{ include "sequencer.socket_directory" . }} + name: socket-volume ports: - containerPort: {{ .Values.ports.sequencerABCI }} name: sequencer-abci @@ -78,6 +83,8 @@ spec: - mountPath: /secrets readOnly: true name: sequencer-secret-keys-vol + - mountPath: {{ include "sequencer.socket_directory" . }} + name: socket-volume ports: - containerPort: {{ .Values.ports.cometbftP2P }} name: cometbft-p2p @@ -95,6 +102,8 @@ spec: cpu: {{ .Values.resources.cometbft.limits.cpu }} memory: {{ .Values.resources.cometbft.limits.memory }} volumes: + - name: socket-volume + emptyDir: {} - name: cometbft-config-volume configMap: name: {{ .Values.moniker }}-cometbft-config diff --git a/charts/sequencer/values.yaml b/charts/sequencer/values.yaml index 80bd795258..45b89dbf95 100644 --- a/charts/sequencer/values.yaml +++ b/charts/sequencer/values.yaml @@ -24,7 +24,7 @@ images: repo: ghcr.io/astriaorg/sequencer pullPolicy: IfNotPresent tag: 1.0.0 - devTag: latest + devTag: local moniker: "" genesis: @@ -105,6 +105,7 @@ genesis: # pubKey: lV57+rGs2vac7mvkGHP1oBFGHPJM3a+WoAzeFDCJDNU= sequencer: + abciUDS: true mempool: parked: maxTxCount: 200 @@ -311,7 +312,7 @@ storage: local: true entities: sequencerSharedStorage: - size: "5Gi" + size: "50Gi" persistentVolumeName: "sequencer-shared-storage" path: "/data/sequencer-data" diff --git a/crates/astria-sequencer/CHANGELOG.md b/crates/astria-sequencer/CHANGELOG.md index c80e93586f..df65163a0e 100644 --- a/crates/astria-sequencer/CHANGELOG.md +++ b/crates/astria-sequencer/CHANGELOG.md @@ -16,6 +16,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Ensure all deposit assets are trace prefixed [#1807](https://github.com/astriaorg/astria/pull/1807). - Update `idna` dependency to resolve cargo audit warning [#1869](https://github.com/astriaorg/astria/pull/1869). +### Removed + +- Remove ASTRIA_SEQUENCER_LISTEN_ADDR config variable [#1877](https://github.com/astriaorg/astria/pull/1877) + +### Added + +- Add ASTRIA_SEQUENCER_ABCI_LISTEN_URL config variable [#1877](https://github.com/astriaorg/astria/pull/1877) + ## [1.0.0] - 2024-10-25 ### Changed diff --git a/crates/astria-sequencer/Cargo.toml b/crates/astria-sequencer/Cargo.toml index 56014df011..f46316c75c 100644 --- a/crates/astria-sequencer/Cargo.toml +++ b/crates/astria-sequencer/Cargo.toml @@ -42,6 +42,7 @@ tower = "0.4" tower-abci = "0.12.0" tower-actor = "0.1.0" tower-http = { version = "0.4", features = ["cors"] } +url = "2.5.4" async-trait = { workspace = true } base64 = { workspace = true } diff --git a/crates/astria-sequencer/local.env.example b/crates/astria-sequencer/local.env.example index 7a794e616a..23223b5303 100644 --- a/crates/astria-sequencer/local.env.example +++ b/crates/astria-sequencer/local.env.example @@ -1,7 +1,9 @@ # Socket address to listen for ABCI requests from cometbft. -# This address corresponds to the `--proxy_app "tcp://"`, -# where `tcp://127.0.0.1:26658` is comebft's default. -ASTRIA_SEQUENCER_LISTEN_ADDR="127.0.0.1:26658" +# This address corresponds to the `--proxy_app ""`, +# where `tcp://127.0.0.1:26658` is comebft's default. Can also be configured to +# use a unix address ie `unix:///socket/astria_abci.sock`. Generally will see +# much higher performance with a unix socket. +ASTRIA_SEQUENCER_ABCI_LISTEN_URL="tcp://127.0.0.1:26658" # Path to rocksdb ASTRIA_SEQUENCER_DB_FILEPATH="/tmp/astria_db" diff --git a/crates/astria-sequencer/src/config.rs b/crates/astria-sequencer/src/config.rs index 00db5f637b..40b4423a18 100644 --- a/crates/astria-sequencer/src/config.rs +++ b/crates/astria-sequencer/src/config.rs @@ -1,9 +1,14 @@ -use std::path::PathBuf; +use std::{ + net::SocketAddr, + path::PathBuf, + str::FromStr, +}; use serde::{ Deserialize, Serialize, }; +use url::Url; #[expect( clippy::struct_excessive_bools, @@ -13,7 +18,7 @@ use serde::{ #[derive(Debug, Deserialize, Serialize)] pub struct Config { /// The endpoint on which Sequencer will listen for ABCI requests - pub listen_addr: String, + pub abci_listen_url: AbciListenUrl, /// The path to penumbra storage db. pub db_filepath: PathBuf, /// Log level: debug, info, warn, or error @@ -38,14 +43,178 @@ impl config::Config for Config { const PREFIX: &'static str = "ASTRIA_SEQUENCER_"; } +#[derive(Debug)] +pub enum AbciListenUrl { + Tcp(SocketAddr), + Unix(PathBuf), +} + +impl std::fmt::Display for AbciListenUrl { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + AbciListenUrl::Tcp(socket_addr) => write!(f, "tcp://{socket_addr}"), + AbciListenUrl::Unix(path) => write!(f, "unix://{}", path.display()), + } + } +} + +#[derive(Debug, thiserror::Error)] +pub enum AbciListenUrlParseError { + #[error( + "parsed input as a tcp address `{parsed}`, but could not turn it into a socket address" + )] + TcpButBadSocketAddr { parsed: Url, source: std::io::Error }, + #[error( + "parsed input as a unix domain socket URL `{parsed}`, but could not turn it into a path" + )] + UnixButBadPath { parsed: Url }, + #[error( + "parsed input as `{parsed}`, but scheme `scheme` is not suppported; supported schemes are \ + tcp, unix" + )] + UnsupportedScheme { parsed: Url, scheme: String }, + #[error("failed parsing input as URL")] + Url { + #[from] + source: url::ParseError, + }, +} + +impl FromStr for AbciListenUrl { + type Err = AbciListenUrlParseError; + + fn from_str(s: &str) -> std::result::Result { + let abci_url = Url::parse(s)?; + + match abci_url.scheme() { + "tcp" => match abci_url.socket_addrs(|| None) { + Ok(mut socket_addrs) => { + let socket_addr = socket_addrs.pop().expect( + "the url crate is guaranteed to return vec with exactly one element \ + because it relies on std::net::ToSocketAddrs::to_socket_addr; if this is \ + no longer the case there was a breaking change in the url crate", + ); + Ok(Self::Tcp(socket_addr)) + } + Err(source) => Err(Self::Err::TcpButBadSocketAddr { + parsed: abci_url, + source, + }), + }, + "unix" => { + if let Ok(path) = abci_url.to_file_path() { + Ok(Self::Unix(path)) + } else { + Err(Self::Err::UnixButBadPath { + parsed: abci_url, + }) + } + } + // If more options are added here will also need to update the server startup + // immediately below to support more than two protocols. + other => Err(Self::Err::UnsupportedScheme { + parsed: abci_url.clone(), + scheme: other.to_string(), + }), + } + } +} + +impl<'de> Deserialize<'de> for AbciListenUrl { + fn deserialize(deserializer: D) -> std::result::Result + where + D: serde::Deserializer<'de>, + { + let s = std::borrow::Cow::<'_, str>::deserialize(deserializer)?; + FromStr::from_str(&s).map_err(serde::de::Error::custom) + } +} + +impl Serialize for AbciListenUrl { + fn serialize(&self, serializer: S) -> std::result::Result + where + S: serde::Serializer, + { + serializer.collect_str(self) + } +} + #[cfg(test)] mod tests { use super::Config; const EXAMPLE_ENV: &str = include_str!("../local.env.example"); + use super::AbciListenUrl; + #[test] fn example_env_config_is_up_to_date() { config::tests::example_env_config_is_up_to_date::(EXAMPLE_ENV); } + + #[test] + fn unix_input_is_parsed_as_abci_listen_url() { + let expected = "/path/to/unix.sock"; + #[expect( + clippy::match_wildcard_for_single_variants, + reason = "intended to match all future variants because the test is only valid for a \ + single variant" + )] + match format!("unix://{expected}") + .parse::() + .unwrap() + { + AbciListenUrl::Unix(actual) => { + assert_eq!(AsRef::::as_ref(expected), actual.as_path(),); + } + other => panic!("expected AbciListenUrl::Unix, got {other:?}"), + } + } + + #[test] + fn tcp_input_is_parsed_as_abci_listen_url() { + let expected = "127.0.0.1:0"; + #[expect( + clippy::match_wildcard_for_single_variants, + reason = "intended to match all future variants because the test is only valid for a \ + single variant" + )] + match format!("tcp://{expected}") + .parse::() + .unwrap() + { + AbciListenUrl::Tcp(actual) => { + assert_eq!(expected, actual.to_string()); + } + other => panic!("expected AbciListenUrl, got {other:?}"), + } + } + + #[test] + fn tcp_listen_addr_format() { + assert_eq!( + "tcp://127.0.0.1:0", + &AbciListenUrl::Tcp(([127, 0, 0, 1], 0).into()).to_string() + ); + } + + #[test] + fn unix_listen_addr_format() { + assert_eq!( + "unix:///path/to/unix.sock", + &AbciListenUrl::Unix("/path/to/unix.sock".into()).to_string(), + ); + } + + // NOTE: the only genuine new error variant is AbciListenUrl. Tests for other error paths are + // not provided because they are fundamentally wrappers of url crate errors. + #[test] + fn http_is_not_valid_abci_listen_scheme() { + match "http://astria.org".parse::().unwrap_err() { + super::AbciListenUrlParseError::UnsupportedScheme { + scheme, .. + } => assert_eq!("http", scheme), + other => panic!("expected AbciListenUrlParseError::UnsupportedScheme, got `{other:?}`"), + } + } } diff --git a/crates/astria-sequencer/src/sequencer.rs b/crates/astria-sequencer/src/sequencer.rs index 92df49d470..eec1a10ee1 100644 --- a/crates/astria-sequencer/src/sequencer.rs +++ b/crates/astria-sequencer/src/sequencer.rs @@ -2,6 +2,7 @@ use astria_core::generated::astria::sequencerblock::v1::sequencer_service_server use astria_eyre::{ anyhow_to_eyre, eyre::{ + self, eyre, OptionExt as _, Result, @@ -36,7 +37,10 @@ use tracing::{ use crate::{ app::App, - config::Config, + config::{ + AbciListenUrl, + Config, + }, grpc::sequencer::SequencerServer, ibc::host_interface::AstriaHost, mempool::Mempool, @@ -95,26 +99,7 @@ impl Sequencer { .await .wrap_err("failed to initialize app")?; - let consensus_service = tower::ServiceBuilder::new() - .layer(request_span::layer(|req: &ConsensusRequest| { - req.create_span() - })) - .service(tower_actor::Actor::new(10, |queue: _| { - let storage = storage.clone(); - async move { service::Consensus::new(storage, app, queue).run().await } - })); let mempool_service = service::Mempool::new(storage.clone(), mempool.clone(), metrics); - let info_service = - service::Info::new(storage.clone()).wrap_err("failed initializing info service")?; - let snapshot_service = service::Snapshot; - - let server = Server::builder() - .consensus(consensus_service) - .info(info_service) - .mempool(mempool_service) - .snapshot(snapshot_service) - .finish() - .ok_or_eyre("server builder didn't return server; are all fields set?")?; let (shutdown_tx, shutdown_rx) = tokio::sync::oneshot::channel(); let (server_exit_tx, server_exit_rx) = tokio::sync::oneshot::channel(); @@ -125,20 +110,15 @@ impl Sequencer { .wrap_err("failed to parse grpc_addr address")?; let grpc_server_handle = start_grpc_server(&storage, mempool, grpc_addr, shutdown_rx); - span.in_scope(|| info!(config.listen_addr, "starting sequencer")); - let server_handle = tokio::spawn(async move { - match server.listen_tcp(&config.listen_addr).await { - Ok(()) => { - // this shouldn't happen, as there isn't a way for the ABCI server to exit - info_span!("abci_server").in_scope(|| info!("ABCI server exited successfully")); - } - Err(e) => { - error_span!("abci_server") - .in_scope(|| error!(err = e.as_ref(), "ABCI server exited with error")); - } - } - let _ = server_exit_tx.send(()); - }); + span.in_scope(|| info!(%config.abci_listen_url, "starting abci sequencer")); + let abci_server_handle = start_abci_server( + &storage, + app, + mempool_service, + config.abci_listen_url, + server_exit_tx, + ) + .wrap_err("failed to start ABCI server")?; select! { _ = signals.stop_rx.changed() => { @@ -157,7 +137,7 @@ impl Sequencer { .await .wrap_err("grpc server task failed")? .wrap_err("grpc server failed")?; - server_handle.abort(); + abci_server_handle.abort(); Ok(()) } } @@ -210,6 +190,54 @@ fn start_grpc_server( ) } +fn start_abci_server( + storage: &cnidarium::Storage, + app: App, + mempool_service: service::Mempool, + listen_url: AbciListenUrl, + server_exit_tx: oneshot::Sender<()>, +) -> eyre::Result> { + let consensus_service = tower::ServiceBuilder::new() + .layer(request_span::layer(|req: &ConsensusRequest| { + req.create_span() + })) + .service(tower_actor::Actor::new(10, |queue: _| { + let storage = storage.clone(); + async move { service::Consensus::new(storage, app, queue).run().await } + })); + let info_service = + service::Info::new(storage.clone()).wrap_err("failed initializing info service")?; + let snapshot_service = service::Snapshot; + + let server = Server::builder() + .consensus(consensus_service) + .info(info_service) + .mempool(mempool_service) + .snapshot(snapshot_service) + .finish() + .ok_or_eyre("server builder didn't return server; are all fields set?")?; + + let server_handle = tokio::spawn(async move { + let server_listen_result = match listen_url { + AbciListenUrl::Tcp(socket_addr) => server.listen_tcp(socket_addr).await, + AbciListenUrl::Unix(path) => server.listen_unix(path).await, + }; + match server_listen_result { + Ok(()) => { + // this shouldn't happen, as there isn't a way for the ABCI server to exit + info_span!("abci_server").in_scope(|| info!("ABCI server exited successfully")); + } + Err(e) => { + error_span!("abci_server") + .in_scope(|| error!(err = e.as_ref(), "ABCI server exited with error")); + } + } + let _ = server_exit_tx.send(()); + }); + + Ok(server_handle) +} + struct SignalReceiver { stop_rx: watch::Receiver<()>, }