Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(composer)!: update to work with appside mempool #1643

Merged
merged 21 commits into from
Oct 14, 2024
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion charts/composer/Chart.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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: 0.1.4
version: 0.1.5

# 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
Expand Down
3 changes: 2 additions & 1 deletion charts/composer/templates/configmap.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ data:
ASTRIA_COMPOSER_API_LISTEN_ADDR: "0.0.0.0:{{ .Values.ports.healthApi }}"
ASTRIA_COMPOSER_GRPC_ADDR: "0.0.0.0:{{ .Values.ports.grpc }}"
ASTRIA_COMPOSER_SEQUENCER_CHAIN_ID: "{{ tpl .Values.config.sequencerChainId . }}"
ASTRIA_COMPOSER_SEQUENCER_URL: "{{ tpl .Values.config.sequencerRpc . }}"
ASTRIA_COMPOSER_COMETBFT_ENDPOINT: "{{ tpl .Values.config.cometbftEndpoint . }}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should move under global.dev statements, marking a breaking change.

ASTRIA_COMPOSER_SEQUENCER_GRPC_ENDPOINT: "{{ tpl .Values.config.sequencerGrpcEndpoint . }}"
ASTRIA_COMPOSER_ROLLUPS: "{{ include "composer.rollups" . }}"
ASTRIA_COMPOSER_PRIVATE_KEY_FILE: "/var/secrets/{{ .Values.config.privateKey.secret.filename }}"
ASTRIA_COMPOSER_MAX_BYTES_PER_BUNDLE: "{{ .Values.config.maxBytesPerBundle }}"
Expand Down
3 changes: 2 additions & 1 deletion charts/composer/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ config:
maxSubmitInterval: 2000
sequencerAddressPrefix: astria
sequencerNativeAssetBaseDenomination: "nria"
sequencerRpc: ""
cometbftEndpoint: "http://node0-sequencer-rpc-service.astria-dev-cluster.svc.cluster.local:26657"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should template from evm-stack chart, set value to "" in composer chart, use global values of the evm-stack to override, similar to other values.

sequencerGrpcEndpoint: "http://node0-sequencer-grpc-service.astria-dev-cluster.svc.cluster.local:8080"
sequencerChainId: ""
privateKey:
devContent: ""
Expand Down
6 changes: 3 additions & 3 deletions charts/evm-stack/Chart.lock
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ dependencies:
version: 0.27.5
- name: composer
repository: file://../composer
version: 0.1.4
version: 0.1.5
- name: evm-faucet
repository: file://../evm-faucet
version: 0.1.2
Expand All @@ -20,5 +20,5 @@ dependencies:
- name: blockscout-stack
repository: https://blockscout.github.io/helm-charts
version: 1.6.2
digest: sha256:2d7e2f23cd9bbdb43b7cf42112db9ede0a7d5eee9e426b0b2344e43fcf52e1b1
generated: "2024-10-02T09:43:51.238571-04:00"
digest: sha256:84cd9e57ee284191d1637ce8fac850a3403932521c0d23a18f0beb0eb27dd398
generated: "2024-10-08T18:13:14.539588+02:00"
4 changes: 2 additions & 2 deletions charts/evm-stack/Chart.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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: 0.6.2
version: 0.6.3

dependencies:
- name: celestia-node
Expand All @@ -26,7 +26,7 @@ dependencies:
version: 0.27.5
repository: "file://../evm-rollup"
- name: composer
version: 0.1.4
version: 0.1.5
repository: "file://../composer"
condition: composer.enabled
- name: evm-faucet
Expand Down
9 changes: 8 additions & 1 deletion crates/astria-composer/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,14 @@ homepage = "https://astria.org"
name = "astria-composer"

[dependencies]
http = "0.2.9"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is to work with tonic::transport::Endpoint? If yes, use tonic::transport::Uri, which is a reexport.


astria-build-info = { path = "../astria-build-info", features = ["runtime"] }
astria-core = { path = "../astria-core", features = ["serde", "server"] }
astria-core = { path = "../astria-core", features = [
"client",
"serde",
"server",
] }
astria-eyre = { path = "../astria-eyre" }
config = { package = "astria-config", path = "../astria-config" }
telemetry = { package = "astria-telemetry", path = "../astria-telemetry", features = [
Expand Down Expand Up @@ -59,6 +65,7 @@ path = "../astria-sequencer-client"
features = ["http"]

[dev-dependencies]
astria-grpc-mock = { path = "../astria-grpc-mock" }
config = { package = "astria-config", path = "../astria-config", features = [
"tests",
] }
Expand Down
5 changes: 4 additions & 1 deletion crates/astria-composer/local.env.example
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,10 @@ NO_COLOR=
ASTRIA_COMPOSER_API_LISTEN_ADDR="0.0.0.0:0"

# Address of the RPC server for the sequencer chain
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# Address of the RPC server for the sequencer chain
# Address of the ABCI server for the sequencer chain

ASTRIA_COMPOSER_SEQUENCER_URL="http://127.0.0.1:26657"
ASTRIA_COMPOSER_COMETBFT_ENDPOINT="http://127.0.0.1:26657"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
ASTRIA_COMPOSER_COMETBFT_ENDPOINT="http://127.0.0.1:26657"
ASTRIA_COMPOSER_SEQUENCER_ABCI_ENDPOINT="http://127.0.0.1:26657"


# Address of the RPC server for the sequencer chain
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# Address of the RPC server for the sequencer chain
# Address of the gRPC server for the sequencer chain

ASTRIA_COMPOSER_SEQUENCER_GRPC_ENDPOINT="http://127.0.0.1:8080"

# Chain ID of the sequencer chain which transactions are submitted to.
ASTRIA_COMPOSER_SEQUENCER_CHAIN_ID="astria-dev-1"
Expand Down
3 changes: 2 additions & 1 deletion crates/astria-composer/src/composer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,8 @@ impl Composer {
let shutdown_token = CancellationToken::new();

let (executor, executor_handle) = executor::Builder {
sequencer_url: cfg.sequencer_url.clone(),
cometbft_endpoint: cfg.cometbft_endpoint.clone(),
sequencer_grpc_endpoint: cfg.sequencer_grpc_endpoint.clone(),
sequencer_chain_id: cfg.sequencer_chain_id.clone(),
private_key_file: cfg.private_key_file.clone(),
sequencer_address_prefix: cfg.sequencer_address_prefix.clone(),
Expand Down
5 changes: 4 additions & 1 deletion crates/astria-composer/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,10 @@ pub struct Config {
pub api_listen_addr: SocketAddr,

/// Address of the RPC server for the sequencer chain
pub sequencer_url: String,
pub cometbft_endpoint: String,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As for the comment on the env.example - to be consistent we should call this sequencer_abci_endpoint to distinguish between it and the grpc endpoint.


/// Address of the GRPC server for the sequencer chain
pub sequencer_grpc_endpoint: String,

/// The chain ID of the sequencer chain
pub sequencer_chain_id: String,
Expand Down
32 changes: 27 additions & 5 deletions crates/astria-composer/src/executor/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use std::{

use astria_core::{
crypto::SigningKey,
generated::sequencerblock::v1alpha1::sequencer_service_client::SequencerServiceClient,
primitive::v1::Address,
protocol::transaction::v1alpha1::action::Sequence,
};
Expand All @@ -14,6 +15,7 @@ use astria_eyre::eyre::{
eyre,
WrapErr as _,
};
use http::Uri;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mentioned in the toml: remove this (see also my other comment on fn connect_sequencer_grpc further down).

use tokio::sync::watch;
use tokio_util::sync::CancellationToken;

Expand All @@ -24,7 +26,8 @@ use crate::{
};

pub(crate) struct Builder {
pub(crate) sequencer_url: String,
pub(crate) cometbft_endpoint: String,
pub(crate) sequencer_grpc_endpoint: String,
pub(crate) sequencer_chain_id: String,
pub(crate) private_key_file: String,
pub(crate) sequencer_address_prefix: String,
Expand All @@ -38,7 +41,8 @@ pub(crate) struct Builder {
impl Builder {
pub(crate) fn build(self) -> eyre::Result<(super::Executor, executor::Handle)> {
let Self {
sequencer_url,
cometbft_endpoint,
sequencer_grpc_endpoint,
sequencer_chain_id,
private_key_file,
sequencer_address_prefix,
Expand All @@ -48,8 +52,14 @@ impl Builder {
shutdown_token,
metrics,
} = self;
let sequencer_client = sequencer_client::HttpClient::new(sequencer_url.as_str())
.wrap_err("failed constructing sequencer client")?;
let cometbft_client = sequencer_client::HttpClient::new(cometbft_endpoint.as_str())
.wrap_err("failed constructing sequencer http client")?;

let sequencer_grpc_client = connect_sequencer_grpc(sequencer_grpc_endpoint.as_str())
.wrap_err_with(|| {
format!("failed to connect to sequencer over gRPC at `{sequencer_grpc_endpoint}`")
})?;

let (status, _) = watch::channel(Status::new());

let sequencer_key = read_signing_key_from_file(&private_key_file).wrap_err_with(|| {
Expand All @@ -69,7 +79,8 @@ impl Builder {
super::Executor {
status,
serialized_rollup_transactions: serialized_rollup_transaction_rx,
sequencer_client,
cometbft_client,
sequencer_grpc_client,
sequencer_chain_id,
sequencer_key,
address: sequencer_address,
Expand All @@ -91,3 +102,14 @@ fn read_signing_key_from_file<P: AsRef<Path>>(path: P) -> eyre::Result<SigningKe
.map_err(|_| eyre!("invalid private key length; must be 32 bytes"))?;
Ok(SigningKey::from(private_key_bytes))
}

fn connect_sequencer_grpc(
sequencer_grpc_endpoint: &str,
) -> eyre::Result<SequencerServiceClient<tonic::transport::Channel>> {
let uri: Uri = sequencer_grpc_endpoint
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just qualify this as let uri: tonic::transport::Uri. No need to import it in the module header (and also don't use http::Uri)

.parse()
.wrap_err("failed to parse endpoint as URI")?;
Ok(SequencerServiceClient::new(
tonic::transport::Endpoint::from(uri).connect_lazy(),
))
}
Loading
Loading