From de1079890d3fd6d93058a4fd9a075508efc7f8bb Mon Sep 17 00:00:00 2001 From: itamar Date: Mon, 20 May 2024 18:16:21 -0400 Subject: [PATCH 01/23] start impl tx submission --- Cargo.lock | 5 + crates/astria-bridge-withdrawer/Cargo.toml | 8 + .../src/bridge/builder.rs | 38 ++- .../src/bridge/event.rs | 12 + .../src/bridge/mod.rs | 219 +++++++++++++++++- .../src/bridge/signer.rs | 37 +++ .../src/metrics_init.rs | 31 ++- 7 files changed, 331 insertions(+), 19 deletions(-) create mode 100644 crates/astria-bridge-withdrawer/src/bridge/event.rs create mode 100644 crates/astria-bridge-withdrawer/src/bridge/signer.rs diff --git a/Cargo.lock b/Cargo.lock index 6813ee7e13..b464eb41b4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -502,9 +502,13 @@ dependencies = [ "astria-core", "astria-eyre", "astria-grpc-mock", + "astria-sequencer-client", "astria-telemetry", "axum", + "ed25519-consensus", + "hex", "http", + "humantime", "hyper", "metrics", "serde", @@ -512,6 +516,7 @@ dependencies = [ "tokio", "tokio-util 0.7.10", "tracing", + "tryhard", ] [[package]] diff --git a/crates/astria-bridge-withdrawer/Cargo.toml b/crates/astria-bridge-withdrawer/Cargo.toml index 524d287329..5d5e6a7ce0 100644 --- a/crates/astria-bridge-withdrawer/Cargo.toml +++ b/crates/astria-bridge-withdrawer/Cargo.toml @@ -15,17 +15,25 @@ name = "astria-bridge-withdrawer" http = "0.2.9" axum = { workspace = true } +ed25519-consensus = { workspace = true } +hex = { workspace = true } hyper = { workspace = true } +humantime = { workspace = true } metrics = { workspace = true } serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } tracing = { workspace = true } +tryhard = { workspace = true } tokio = { workspace = true, features = ["macros", "rt-multi-thread", "signal"] } tokio-util = { workspace = true } astria-build-info = { path = "../astria-build-info", features = ["runtime"] } +astria-core = { path = "../astria-core", features = ["serde", "server"] } astria-eyre = { path = "../astria-eyre" } config = { package = "astria-config", path = "../astria-config" } +sequencer-client = { package = "astria-sequencer-client", path = "../astria-sequencer-client", features = [ + "http", +] } telemetry = { package = "astria-telemetry", path = "../astria-telemetry", features = [ "display", ] } diff --git a/crates/astria-bridge-withdrawer/src/bridge/builder.rs b/crates/astria-bridge-withdrawer/src/bridge/builder.rs index 4fe8de9a6b..6d8d8f0485 100644 --- a/crates/astria-bridge-withdrawer/src/bridge/builder.rs +++ b/crates/astria-bridge-withdrawer/src/bridge/builder.rs @@ -1,26 +1,50 @@ use std::sync::Arc; -use astria_eyre::eyre; +use astria_eyre::eyre::{ + self, + Context as _, +}; use tokio_util::sync::CancellationToken; -use super::{ - state::State, - Bridge, -}; +use super::state::State; pub(crate) struct Builder { pub(crate) shutdown_token: CancellationToken, + pub(crate) sequencer_key_path: String, + pub(crate) sequencer_chain_id: String, + pub(crate) cometbft_endpoint: String, } impl Builder { /// Instantiates a `Bridge`. - pub(crate) fn build(self) -> eyre::Result { + pub(crate) fn build(self) -> eyre::Result<(super::Bridge, super::Handle)> { let Self { shutdown_token, + sequencer_key_path, + sequencer_chain_id, + cometbft_endpoint, } = self; + let signer = super::signer::SequencerSigner::from_path(sequencer_key_path)?; + let (batches_tx, batches_rx) = tokio::sync::mpsc::channel(1); + + let sequencer_cometbft_client = sequencer_client::HttpClient::new(&*cometbft_endpoint) + .context("failed constructing cometbft http client")?; + let state = Arc::new(State::new()); - Ok(Bridge::new(state, shutdown_token)) + Ok(( + super::Bridge { + shutdown_token, + state, + batches_rx, + signer, + sequencer_chain_id, + sequencer_cometbft_client, + }, + super::Handle { + batches_tx, + }, + )) } } diff --git a/crates/astria-bridge-withdrawer/src/bridge/event.rs b/crates/astria-bridge-withdrawer/src/bridge/event.rs new file mode 100644 index 0000000000..c670a6f577 --- /dev/null +++ b/crates/astria-bridge-withdrawer/src/bridge/event.rs @@ -0,0 +1,12 @@ +use astria_core::protocol::transaction::v1alpha1::{ + action::BridgeUnlockAction, + Action, +}; + +pub(crate) struct Event; + +impl From for Action { + fn from(e: Event) -> Self { + Action::BridgeUnlock(BridgeUnlockAction::default()) + } +} diff --git a/crates/astria-bridge-withdrawer/src/bridge/mod.rs b/crates/astria-bridge-withdrawer/src/bridge/mod.rs index efbbf766ed..5c2b901357 100644 --- a/crates/astria-bridge-withdrawer/src/bridge/mod.rs +++ b/crates/astria-bridge-withdrawer/src/bridge/mod.rs @@ -1,34 +1,231 @@ -use std::sync::Arc; +use std::{ + sync::Arc, + time::Duration, +}; -use astria_eyre::eyre; +use astria_core::protocol::transaction::v1alpha1::{ + Action, + TransactionParams, + UnsignedTransaction, +}; +use astria_eyre::eyre::{ + self, + eyre, + WrapErr as _, +}; +use sequencer_client::{ + Address, + SequencerClientExt as _, + SignedTransaction, +}; +use tokio::{ + select, + sync::mpsc, + time::Instant, +}; use tokio_util::sync::CancellationToken; mod builder; +mod event; +mod signer; mod state; pub(crate) use builder::Builder; +use signer::SequencerSigner; use state::State; pub(super) use state::StateSnapshot; +use tracing::{ + debug, + error, + info, + info_span, + instrument, + warn, + Instrument as _, + Span, +}; + +use self::event::Event; + +pub(super) struct Handle { + batches_tx: mpsc::Sender>, +} pub(super) struct Bridge { - _shutdown_token: CancellationToken, + shutdown_token: CancellationToken, state: Arc, + batches_rx: mpsc::Receiver>, + sequencer_cometbft_client: sequencer_client::HttpClient, + signer: SequencerSigner, + sequencer_chain_id: String, } impl Bridge { - pub(super) fn new(state: Arc, shutdown_token: CancellationToken) -> Self { - Self { - state, - _shutdown_token: shutdown_token.clone(), - } - } - pub(super) fn subscribe_to_state(&self) -> tokio::sync::watch::Receiver { self.state.subscribe() } - pub(super) async fn run(self) -> eyre::Result<()> { + pub(super) async fn run(mut self) -> eyre::Result<()> { + // run collector + self.state.set_ready(); + + let reason = loop { + select!( + biased; + + () = self.shutdown_token.cancelled() => { + info!("received shutdown signal"); + break Ok("shutdown requested"); + } + + batch = self.batches_rx.recv() => { + let batch = match batch { + Some(batch) => batch, + None => { + info!("received None from batch channel, shutting down"); + break Err("channel closed"); + } + }; + + let actions = batch.into_iter().map(Action::from).collect::>(); + + // get nonce + let nonce = get_latest_nonce(self.sequencer_cometbft_client.clone(), self.signer.address).await?; + + let unsigned = UnsignedTransaction { + actions, + params: TransactionParams { + nonce, + chain_id: self.sequencer_chain_id.clone(), + }, + }; + + // sign + let signed = unsigned.into_signed(&self.signer.signing_key); + + // broadcast commit + } + ) + }; + // update status + + self.batches_rx.close(); + + match reason { + Ok(reason) => info!(reason, "starting shutdown process"), + Err(reason) => { + error!(%reason, "starting shutdown process") + } + } + + // handle shutdown + Ok(()) } } + +/// Queries the sequencer for the latest nonce with an exponential backoff +#[instrument(name = "get latest nonce", skip_all, fields(%address))] +async fn get_latest_nonce( + client: sequencer_client::HttpClient, + address: Address, +) -> eyre::Result { + debug!("fetching latest nonce from sequencer"); + metrics::counter!(crate::metrics_init::NONCE_FETCH_COUNT).increment(1); + let span = Span::current(); + let start = Instant::now(); + let retry_config = tryhard::RetryFutureConfig::new(1024) + .exponential_backoff(Duration::from_millis(200)) + .max_delay(Duration::from_secs(60)) + .on_retry( + |attempt, + next_delay: Option, + err: &sequencer_client::extension_trait::Error| { + metrics::counter!(crate::metrics_init::NONCE_FETCH_FAILURE_COUNT).increment(1); + + let wait_duration = next_delay + .map(humantime::format_duration) + .map(tracing::field::display); + warn!( + parent: span.clone(), + error = err as &dyn std::error::Error, + attempt, + wait_duration, + "failed getting latest nonce from sequencer; retrying after backoff", + ); + async move {} + }, + ); + let res = tryhard::retry_fn(|| { + let client = client.clone(); + let span = info_span!(parent: span.clone(), "attempt get nonce"); + async move { client.get_latest_nonce(address).await.map(|rsp| rsp.nonce) }.instrument(span) + }) + .with_config(retry_config) + .await + .wrap_err("failed getting latest nonce from sequencer after 1024 attempts"); + + metrics::histogram!(crate::metrics_init::NONCE_FETCH_LATENCY).record(start.elapsed()); + + res +} + +/// Queries the sequencer for the latest nonce with an exponential backoff +#[instrument( + name = "submit signed transaction", + skip_all, + fields( + nonce = tx.unsigned_transaction().params.nonce, + transaction.hash = hex::encode(sha256(&tx.to_raw().encode_to_vec())), + ) +)] +async fn submit_tx( + client: sequencer_client::HttpClient, + tx: SignedTransaction, +) -> eyre::Result { + let nonce = tx.unsigned_transaction().params.nonce; + metrics::gauge!(crate::metrics_init::CURRENT_NONCE).set(nonce); + + // TODO: change to info and log tx hash (to match info log in `SubmitFut`'s response handling + // logic) + let start = std::time::Instant::now(); + debug!("submitting signed transaction to sequencer"); + let span = Span::current(); + let retry_config = tryhard::RetryFutureConfig::new(1024) + .exponential_backoff(Duration::from_millis(200)) + .max_delay(Duration::from_secs(60)) + .on_retry( + |attempt, + next_delay: Option, + err: &sequencer_client::extension_trait::Error| { + metrics::counter!(crate::metrics_init::SEQUENCER_SUBMISSION_FAILURE_COUNT) + .increment(1); + + let wait_duration = next_delay + .map(humantime::format_duration) + .map(tracing::field::display); + warn!( + parent: span.clone(), + attempt, + wait_duration, + error = err as &dyn std::error::Error, + "failed sending transaction to sequencer; retrying after backoff", + ); + async move {} + }, + ); + let res = tryhard::retry_fn(|| { + let client = client.clone(); + let tx = tx.clone(); + let span = info_span!(parent: span.clone(), "attempt send"); + async move { client.submit_transaction_sync(tx).await }.instrument(span) + }) + .with_config(retry_config) + .await + .wrap_err("failed sending transaction after 1024 attempts"); + + metrics::histogram!(crate::metrics_init::SEQUENCER_SUBMISSION_LATENCY).record(start.elapsed()); + + res +} diff --git a/crates/astria-bridge-withdrawer/src/bridge/signer.rs b/crates/astria-bridge-withdrawer/src/bridge/signer.rs new file mode 100644 index 0000000000..9b8f61bc87 --- /dev/null +++ b/crates/astria-bridge-withdrawer/src/bridge/signer.rs @@ -0,0 +1,37 @@ +use std::{ + fs, + path::Path, +}; + +use astria_core::crypto::SigningKey; +use astria_eyre::eyre::{ + self, + eyre, +}; +use ed25519_consensus::VerificationKey; +use sequencer_client::Address; + +pub(super) struct SequencerSigner { + pub(super) address: Address, + pub(super) signing_key: SigningKey, + pub(super) verification_key: VerificationKey, +} + +impl SequencerSigner { + /// Construct a `SequencerSigner` from a file. + /// + /// The file should contain a hex-encoded ed25519 secret key. + pub(super) fn from_path>(path: P) -> eyre::Result { + let hex = fs::read_to_string(path)?; + let bytes: [u8; 32] = hex::decode(hex.trim())? + .try_into() + .map_err(|_| eyre!("invalid private key length; must be 32 bytes"))?; + let signing_key = SigningKey::from(bytes); + + Ok(Self { + address: Address::from_verification_key(signing_key.verification_key()), + signing_key, + verification_key: signing_key.verification_key(), + }) + } +} diff --git a/crates/astria-bridge-withdrawer/src/metrics_init.rs b/crates/astria-bridge-withdrawer/src/metrics_init.rs index e88c68efc3..fb4307c074 100644 --- a/crates/astria-bridge-withdrawer/src/metrics_init.rs +++ b/crates/astria-bridge-withdrawer/src/metrics_init.rs @@ -2,5 +2,34 @@ //! //! Registers metrics & lists constants to be used as metric names throughout crate. +use metrics::{ + describe_counter, + describe_histogram, + Unit, +}; + /// Registers all metrics used by this crate. -pub fn register() {} +pub fn register() { + describe_counter!( + NONCE_FETCH_COUNT, + Unit::Count, + "The number of times we have attempted to fetch the nonce" + ); + describe_counter!( + NONCE_FETCH_FAILURE_COUNT, + Unit::Count, + "The number of times we have failed to fetch the nonce" + ); + describe_histogram!( + NONCE_FETCH_LATENCY, + Unit::Milliseconds, + "The latency of nonce fetch" + ); +} + +pub const NONCE_FETCH_COUNT: &str = concat!(env!("CARGO_CRATE_NAME"), "_nonce_fetch_count"); + +pub const NONCE_FETCH_FAILURE_COUNT: &str = + concat!(env!("CARGO_CRATE_NAME"), "_nonce_fetch_failure_count"); + +pub const NONCE_FETCH_LATENCY: &str = concat!(env!("CARGO_CRATE_NAME"), "_nonce_fetch_latency"); From 68fc505a4049fa931c7b0b204b1177a12ca3124f Mon Sep 17 00:00:00 2001 From: itamar Date: Tue, 21 May 2024 18:22:56 -0400 Subject: [PATCH 02/23] clean up imports --- Cargo.lock | 3 ++ crates/astria-bridge-withdrawer/Cargo.toml | 3 ++ crates/astria-bridge-withdrawer/src/api.rs | 6 +-- .../src/bridge_service.rs | 51 ++++++++++--------- crates/astria-bridge-withdrawer/src/config.rs | 2 + .../src/{bridge => executor}/builder.rs | 4 +- .../src/{bridge => executor}/event.rs | 0 .../src/{bridge => executor}/mod.rs | 19 ++++--- .../src/{bridge => executor}/signer.rs | 0 .../src/{bridge => executor}/state.rs | 0 crates/astria-bridge-withdrawer/src/lib.rs | 2 +- .../src/metrics_init.rs | 22 ++++++++ 12 files changed, 75 insertions(+), 37 deletions(-) rename crates/astria-bridge-withdrawer/src/{bridge => executor}/builder.rs (91%) rename crates/astria-bridge-withdrawer/src/{bridge => executor}/event.rs (100%) rename crates/astria-bridge-withdrawer/src/{bridge => executor}/mod.rs (94%) rename crates/astria-bridge-withdrawer/src/{bridge => executor}/signer.rs (100%) rename crates/astria-bridge-withdrawer/src/{bridge => executor}/state.rs (100%) diff --git a/Cargo.lock b/Cargo.lock index b464eb41b4..182ca83309 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -511,8 +511,11 @@ dependencies = [ "humantime", "hyper", "metrics", + "prost", "serde", "serde_json", + "sha2 0.10.8", + "tendermint 0.34.1", "tokio", "tokio-util 0.7.10", "tracing", diff --git a/crates/astria-bridge-withdrawer/Cargo.toml b/crates/astria-bridge-withdrawer/Cargo.toml index 5d5e6a7ce0..499f638723 100644 --- a/crates/astria-bridge-withdrawer/Cargo.toml +++ b/crates/astria-bridge-withdrawer/Cargo.toml @@ -20,8 +20,11 @@ hex = { workspace = true } hyper = { workspace = true } humantime = { workspace = true } metrics = { workspace = true } +prost = { workspace = true } serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } +sha2 = { workspace = true } +tendermint = { workspace = true } tracing = { workspace = true } tryhard = { workspace = true } tokio = { workspace = true, features = ["macros", "rt-multi-thread", "signal"] } diff --git a/crates/astria-bridge-withdrawer/src/api.rs b/crates/astria-bridge-withdrawer/src/api.rs index ae1d1aae73..42e1ede0c1 100644 --- a/crates/astria-bridge-withdrawer/src/api.rs +++ b/crates/astria-bridge-withdrawer/src/api.rs @@ -21,11 +21,11 @@ use hyper::server::conn::AddrIncoming; use serde::Serialize; use tokio::sync::watch; -use crate::bridge; +use crate::executor; pub(crate) type ApiServer = axum::Server>; -type BridgeState = watch::Receiver; +type BridgeState = watch::Receiver; #[derive(Clone)] /// `AppState` is used for as an axum extractor in its method handlers. @@ -76,7 +76,7 @@ async fn get_readyz(State(bridge_state): State) -> Readyz { } #[allow(clippy::unused_async)] // Permit because axum handlers must be async -async fn get_status(State(bridge_state): State) -> Json { +async fn get_status(State(bridge_state): State) -> Json { Json(bridge_state.borrow().clone()) } diff --git a/crates/astria-bridge-withdrawer/src/bridge_service.rs b/crates/astria-bridge-withdrawer/src/bridge_service.rs index 72a417fd47..fc61e35263 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_service.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_service.rs @@ -24,18 +24,18 @@ use tracing::{ use crate::{ api, - bridge::{ + config::Config, + executor::{ self, - Bridge, + Executor, }, - config::Config, }; pub struct BridgeService { // Token to signal all subtasks to shut down gracefully. shutdown_token: CancellationToken, api_server: api::ApiServer, - bridge: Bridge, + executor: Executor, } impl BridgeService { @@ -47,14 +47,17 @@ impl BridgeService { // make bridge object // TODO: add more fields - let bridge = bridge::Builder { + let (executor, executor_handle) = executor::Builder { shutdown_token: shutdown_handle.token(), + cometbft_endpoint: cfg.cometbft_endpoint, + sequencer_chain_id: cfg.sequencer_chain_id, + sequencer_key_path: cfg.sequencer_key_path, } .build() .wrap_err("failed to create bridge")?; // make api server - let state_rx = bridge.subscribe_to_state(); + let state_rx = executor.subscribe_to_state(); let api_socket_addr = api_addr.parse::().wrap_err_with(|| { format!("failed to parse provided `api_addr` string as socket address: `{api_addr}`",) })?; @@ -63,7 +66,7 @@ impl BridgeService { let bridge = Self { shutdown_token: shutdown_handle.token(), api_server, - bridge, + executor, }; Ok((bridge, shutdown_handle)) @@ -73,7 +76,7 @@ impl BridgeService { let Self { shutdown_token, api_server, - bridge, + executor, } = self; // Separate the API shutdown signal from the cancellation token because we want it to live @@ -89,17 +92,17 @@ impl BridgeService { }); info!("spawned API server"); - let mut bridge_task = tokio::spawn(bridge.run()); + let mut executor_task = tokio::spawn(executor.run()); info!("spawned bridge withdrawer task"); let shutdown = select!( o = &mut api_task => { report_exit("api server", o); - Shutdown { api_task: None, bridge_task: Some(bridge_task), api_shutdown_signal, shutdown_token } + Shutdown { api_task: None, executor_task: Some(executor_task), api_shutdown_signal, shutdown_token } } - o = &mut bridge_task => { + o = &mut executor_task => { report_exit("bridge worker", o); - Shutdown { api_task: Some(api_task), bridge_task: None, api_shutdown_signal, shutdown_token } + Shutdown { api_task: Some(api_task), executor_task: None, api_shutdown_signal, shutdown_token } } ); @@ -107,10 +110,10 @@ impl BridgeService { } } -/// A handle for instructing the [`Bridge`] to shut down. +/// A handle for instructing the [`BridgeService`] to shut down. /// -/// It is returned along with its related `Bridge` from [`Bridge::new`]. The -/// `Bridge` will begin to shut down as soon as [`ShutdownHandle::shutdown`] is called or +/// It is returned along with its related `BridgeService` from [`BridgeService::new`]. The +/// `BridgeService` will begin to shut down as soon as [`ShutdownHandle::shutdown`] is called or /// when the `ShutdownHandle` is dropped. pub struct ShutdownHandle { token: CancellationToken, @@ -163,30 +166,30 @@ fn report_exit(task_name: &str, outcome: Result, JoinError>) { struct Shutdown { api_task: Option>>, - bridge_task: Option>>, + executor_task: Option>>, api_shutdown_signal: oneshot::Sender<()>, shutdown_token: CancellationToken, } impl Shutdown { const API_SHUTDOWN_TIMEOUT_SECONDS: u64 = 4; - const BRIDGE_SHUTDOWN_TIMEOUT_SECONDS: u64 = 25; + const EXECUTOR_SHUTDOWN_TIMEOUT_SECONDS: u64 = 25; async fn run(self) { let Self { api_task, - bridge_task, + executor_task, api_shutdown_signal, shutdown_token, } = self; shutdown_token.cancel(); - // Giving bridge 25 seconds to shutdown because Kubernetes issues a SIGKILL after 30. - if let Some(mut bridge_task) = bridge_task { - info!("waiting for bridge task to shut down"); - let limit = Duration::from_secs(Self::BRIDGE_SHUTDOWN_TIMEOUT_SECONDS); - match timeout(limit, &mut bridge_task).await.map(flatten_result) { + // Giving executor 25 seconds to shutdown because Kubernetes issues a SIGKILL after 30. + if let Some(mut executor_task) = executor_task { + info!("waiting for executor task to shut down"); + let limit = Duration::from_secs(Self::EXECUTOR_SHUTDOWN_TIMEOUT_SECONDS); + match timeout(limit, &mut executor_task).await.map(flatten_result) { Ok(Ok(())) => info!("bridge exited gracefully"), Ok(Err(error)) => error!(%error, "bridge exited with an error"), Err(_) => { @@ -194,7 +197,7 @@ impl Shutdown { timeout_secs = limit.as_secs(), "bridge did not shut down within timeout; killing it" ); - bridge_task.abort(); + executor_task.abort(); } } } else { diff --git a/crates/astria-bridge-withdrawer/src/config.rs b/crates/astria-bridge-withdrawer/src/config.rs index 70152db6dc..841773bf31 100644 --- a/crates/astria-bridge-withdrawer/src/config.rs +++ b/crates/astria-bridge-withdrawer/src/config.rs @@ -10,6 +10,8 @@ use serde::{ /// The single config for creating an astria-bridge service. pub struct Config { pub cometbft_endpoint: String, + pub sequencer_chain_id: String, + pub sequencer_key_path: String, // The socket address at which the bridge service will server healthz, readyz, and status // calls. pub api_addr: String, diff --git a/crates/astria-bridge-withdrawer/src/bridge/builder.rs b/crates/astria-bridge-withdrawer/src/executor/builder.rs similarity index 91% rename from crates/astria-bridge-withdrawer/src/bridge/builder.rs rename to crates/astria-bridge-withdrawer/src/executor/builder.rs index 6d8d8f0485..e49ef2daab 100644 --- a/crates/astria-bridge-withdrawer/src/bridge/builder.rs +++ b/crates/astria-bridge-withdrawer/src/executor/builder.rs @@ -17,7 +17,7 @@ pub(crate) struct Builder { impl Builder { /// Instantiates a `Bridge`. - pub(crate) fn build(self) -> eyre::Result<(super::Bridge, super::Handle)> { + pub(crate) fn build(self) -> eyre::Result<(super::Executor, super::Handle)> { let Self { shutdown_token, sequencer_key_path, @@ -34,7 +34,7 @@ impl Builder { let state = Arc::new(State::new()); Ok(( - super::Bridge { + super::Executor { shutdown_token, state, batches_rx, diff --git a/crates/astria-bridge-withdrawer/src/bridge/event.rs b/crates/astria-bridge-withdrawer/src/executor/event.rs similarity index 100% rename from crates/astria-bridge-withdrawer/src/bridge/event.rs rename to crates/astria-bridge-withdrawer/src/executor/event.rs diff --git a/crates/astria-bridge-withdrawer/src/bridge/mod.rs b/crates/astria-bridge-withdrawer/src/executor/mod.rs similarity index 94% rename from crates/astria-bridge-withdrawer/src/bridge/mod.rs rename to crates/astria-bridge-withdrawer/src/executor/mod.rs index 5c2b901357..48a2807f37 100644 --- a/crates/astria-bridge-withdrawer/src/bridge/mod.rs +++ b/crates/astria-bridge-withdrawer/src/executor/mod.rs @@ -10,14 +10,16 @@ use astria_core::protocol::transaction::v1alpha1::{ }; use astria_eyre::eyre::{ self, - eyre, WrapErr as _, }; +use prost::Message as _; use sequencer_client::{ + tendermint_rpc::endpoint::broadcast::tx_commit, Address, SequencerClientExt as _, SignedTransaction, }; +use tendermint::crypto::Sha256; use tokio::{ select, sync::mpsc, @@ -51,7 +53,7 @@ pub(super) struct Handle { batches_tx: mpsc::Sender>, } -pub(super) struct Bridge { +pub(super) struct Executor { shutdown_token: CancellationToken, state: Arc, batches_rx: mpsc::Receiver>, @@ -60,14 +62,12 @@ pub(super) struct Bridge { sequencer_chain_id: String, } -impl Bridge { +impl Executor { pub(super) fn subscribe_to_state(&self) -> tokio::sync::watch::Receiver { self.state.subscribe() } pub(super) async fn run(mut self) -> eyre::Result<()> { - // run collector - self.state.set_ready(); let reason = loop { @@ -183,7 +183,7 @@ async fn get_latest_nonce( async fn submit_tx( client: sequencer_client::HttpClient, tx: SignedTransaction, -) -> eyre::Result { +) -> eyre::Result { let nonce = tx.unsigned_transaction().params.nonce; metrics::gauge!(crate::metrics_init::CURRENT_NONCE).set(nonce); @@ -219,7 +219,7 @@ async fn submit_tx( let client = client.clone(); let tx = tx.clone(); let span = info_span!(parent: span.clone(), "attempt send"); - async move { client.submit_transaction_sync(tx).await }.instrument(span) + async move { client.submit_transaction_commit(tx).await }.instrument(span) }) .with_config(retry_config) .await @@ -229,3 +229,8 @@ async fn submit_tx( res } + +fn sha256(data: &[u8]) -> [u8; 32] { + use sha2::Sha256; + Sha256::digest(data) +} diff --git a/crates/astria-bridge-withdrawer/src/bridge/signer.rs b/crates/astria-bridge-withdrawer/src/executor/signer.rs similarity index 100% rename from crates/astria-bridge-withdrawer/src/bridge/signer.rs rename to crates/astria-bridge-withdrawer/src/executor/signer.rs diff --git a/crates/astria-bridge-withdrawer/src/bridge/state.rs b/crates/astria-bridge-withdrawer/src/executor/state.rs similarity index 100% rename from crates/astria-bridge-withdrawer/src/bridge/state.rs rename to crates/astria-bridge-withdrawer/src/executor/state.rs diff --git a/crates/astria-bridge-withdrawer/src/lib.rs b/crates/astria-bridge-withdrawer/src/lib.rs index 93b5831bae..7b77304e3e 100644 --- a/crates/astria-bridge-withdrawer/src/lib.rs +++ b/crates/astria-bridge-withdrawer/src/lib.rs @@ -1,8 +1,8 @@ pub(crate) mod api; -pub(crate) mod bridge; pub mod bridge_service; mod build_info; pub(crate) mod config; +pub(crate) mod executor; pub mod metrics_init; pub use bridge_service::BridgeService; diff --git a/crates/astria-bridge-withdrawer/src/metrics_init.rs b/crates/astria-bridge-withdrawer/src/metrics_init.rs index fb4307c074..44b039e72b 100644 --- a/crates/astria-bridge-withdrawer/src/metrics_init.rs +++ b/crates/astria-bridge-withdrawer/src/metrics_init.rs @@ -4,6 +4,7 @@ use metrics::{ describe_counter, + describe_gauge, describe_histogram, Unit, }; @@ -25,6 +26,17 @@ pub fn register() { Unit::Milliseconds, "The latency of nonce fetch" ); + describe_gauge!(CURRENT_NONCE, Unit::Count, "The current nonce"); + describe_histogram!( + SEQUENCER_SUBMISSION_LATENCY, + Unit::Milliseconds, + "The latency of submitting a transaction to the sequencer" + ); + describe_counter!( + SEQUENCER_SUBMISSION_FAILURE_COUNT, + Unit::Count, + "The number of failed transaction submissions to the sequencer" + ); } pub const NONCE_FETCH_COUNT: &str = concat!(env!("CARGO_CRATE_NAME"), "_nonce_fetch_count"); @@ -33,3 +45,13 @@ pub const NONCE_FETCH_FAILURE_COUNT: &str = concat!(env!("CARGO_CRATE_NAME"), "_nonce_fetch_failure_count"); pub const NONCE_FETCH_LATENCY: &str = concat!(env!("CARGO_CRATE_NAME"), "_nonce_fetch_latency"); + +pub const CURRENT_NONCE: &str = concat!(env!("CARGO_CRATE_NAME"), "_current_nonce"); + +pub const SEQUENCER_SUBMISSION_FAILURE_COUNT: &str = concat!( + env!("CARGO_CRATE_NAME"), + "_sequencer_submission_failure_count" +); + +pub const SEQUENCER_SUBMISSION_LATENCY: &str = + concat!(env!("CARGO_CRATE_NAME"), "_sequencer_submission_latency"); From 32258cf92f253a2f33826eccb430e3dab0913374 Mon Sep 17 00:00:00 2001 From: itamar Date: Wed, 22 May 2024 13:07:32 -0400 Subject: [PATCH 03/23] submitter state machine --- Cargo.lock | 2 + crates/astria-bridge-withdrawer/Cargo.toml | 3 + .../src/executor/builder.rs | 2 +- .../src/executor/mod.rs | 20 ++-- .../src/executor/submitter.rs | 96 +++++++++++++++++++ 5 files changed, 112 insertions(+), 11 deletions(-) create mode 100644 crates/astria-bridge-withdrawer/src/executor/submitter.rs diff --git a/Cargo.lock b/Cargo.lock index 182ca83309..883ea5756d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -506,11 +506,13 @@ dependencies = [ "astria-telemetry", "axum", "ed25519-consensus", + "futures", "hex", "http", "humantime", "hyper", "metrics", + "pin-project-lite", "prost", "serde", "serde_json", diff --git a/crates/astria-bridge-withdrawer/Cargo.toml b/crates/astria-bridge-withdrawer/Cargo.toml index 499f638723..db62c83018 100644 --- a/crates/astria-bridge-withdrawer/Cargo.toml +++ b/crates/astria-bridge-withdrawer/Cargo.toml @@ -14,8 +14,11 @@ name = "astria-bridge-withdrawer" [dependencies] http = "0.2.9" +pin-project-lite = "0.2.13" + axum = { workspace = true } ed25519-consensus = { workspace = true } +futures = { workspace = true } hex = { workspace = true } hyper = { workspace = true } humantime = { workspace = true } diff --git a/crates/astria-bridge-withdrawer/src/executor/builder.rs b/crates/astria-bridge-withdrawer/src/executor/builder.rs index e49ef2daab..4021c3c8d5 100644 --- a/crates/astria-bridge-withdrawer/src/executor/builder.rs +++ b/crates/astria-bridge-withdrawer/src/executor/builder.rs @@ -16,7 +16,7 @@ pub(crate) struct Builder { } impl Builder { - /// Instantiates a `Bridge`. + /// Instantiates an `Executor`. pub(crate) fn build(self) -> eyre::Result<(super::Executor, super::Handle)> { let Self { shutdown_token, diff --git a/crates/astria-bridge-withdrawer/src/executor/mod.rs b/crates/astria-bridge-withdrawer/src/executor/mod.rs index 48a2807f37..7717ea500c 100644 --- a/crates/astria-bridge-withdrawer/src/executor/mod.rs +++ b/crates/astria-bridge-withdrawer/src/executor/mod.rs @@ -12,6 +12,7 @@ use astria_eyre::eyre::{ self, WrapErr as _, }; +pub(crate) use builder::Builder; use prost::Message as _; use sequencer_client::{ tendermint_rpc::endpoint::broadcast::tx_commit, @@ -19,6 +20,9 @@ use sequencer_client::{ SequencerClientExt as _, SignedTransaction, }; +use signer::SequencerSigner; +use state::State; +pub(super) use state::StateSnapshot; use tendermint::crypto::Sha256; use tokio::{ select, @@ -26,16 +30,6 @@ use tokio::{ time::Instant, }; use tokio_util::sync::CancellationToken; - -mod builder; -mod event; -mod signer; -mod state; - -pub(crate) use builder::Builder; -use signer::SequencerSigner; -use state::State; -pub(super) use state::StateSnapshot; use tracing::{ debug, error, @@ -49,6 +43,12 @@ use tracing::{ use self::event::Event; +mod builder; +mod event; +mod signer; +mod state; +mod submitter; + pub(super) struct Handle { batches_tx: mpsc::Sender>, } diff --git a/crates/astria-bridge-withdrawer/src/executor/submitter.rs b/crates/astria-bridge-withdrawer/src/executor/submitter.rs new file mode 100644 index 0000000000..bb179ae533 --- /dev/null +++ b/crates/astria-bridge-withdrawer/src/executor/submitter.rs @@ -0,0 +1,96 @@ +use std::{ + pin::Pin, + task::Poll, +}; + +use astria_eyre::eyre; +use futures::{ + ready, + Future, +}; +use pin_project_lite::pin_project; +use sequencer_client::{ + tendermint_rpc::endpoint::broadcast::tx_commit, + SignedTransaction, +}; + +use super::signer::SequencerSigner; + +pin_project! { + /// TODO + pub(super) struct SubmitFut { + tx: SignedTransaction, + signer: SequencerSigner, + cometbft_client: sequencer_client::HttpClient, + nonce: u32, + sequencer_chain_id: String, + #[pin] + state: SubmitState, + } +} + +pin_project! { + #[project = SubmitStateProj] + enum SubmitState { + NotStarted, + WaitingForTxCommit { + #[pin] + fut: Pin> + Send>> + }, + WaitingForNonce { + #[pin] + fut: Pin> + Send>> + } + } +} + +impl Future for SubmitFut { + // TODO: output? + type Output = eyre::Result<()>; + + fn poll(mut self: Pin<&mut Self>, cx: &mut std::task::Context<'_>) -> Poll { + loop { + let this = self.as_mut().project(); + let new_state = match this.state.project() { + SubmitStateProj::NotStarted => { + // TODO: broadcast tx commit and change to waiting for txcommit + SubmitState::WaitingForTxCommit { + fut: todo!("broadcast tx commit"), + } + } + SubmitStateProj::WaitingForTxCommit { + fut, + } => match ready!(fut.poll(cx)) { + Ok(rsp) => { + // handle 0 code + // handle non-zero code + // if invalid nonce, change to waiting for nonce + SubmitState::WaitingForNonce { + fut: todo!("nonce fetch"), + } + } + Err(e) => { + // exit with an error + return todo!("exit with an error"); + } + }, + SubmitStateProj::WaitingForNonce { + fut, + } => match ready!(fut.poll(cx)) { + Ok(nonce) => { + // update tx nonce + // change to waiting for txcommit + SubmitState::WaitingForTxCommit { + fut: todo!("broadcast tx commit"), + } + } + Err(e) => { + // exit with an error + return todo!("exit with an error"); + } + }, + }; + self.as_mut().project().state.set(new_state); + } + } +} From ca1d8436cf4a36ad875e8541b3e121eefb03d3dc Mon Sep 17 00:00:00 2001 From: itamar Date: Wed, 22 May 2024 15:29:05 -0400 Subject: [PATCH 04/23] add state to executor --- .../src/executor/mod.rs | 49 +++++++++- .../src/executor/state.rs | 53 +++++++++- .../src/executor/submitter.rs | 96 ------------------- 3 files changed, 99 insertions(+), 99 deletions(-) delete mode 100644 crates/astria-bridge-withdrawer/src/executor/submitter.rs diff --git a/crates/astria-bridge-withdrawer/src/executor/mod.rs b/crates/astria-bridge-withdrawer/src/executor/mod.rs index 7717ea500c..f2bae158f1 100644 --- a/crates/astria-bridge-withdrawer/src/executor/mod.rs +++ b/crates/astria-bridge-withdrawer/src/executor/mod.rs @@ -10,6 +10,7 @@ use astria_core::protocol::transaction::v1alpha1::{ }; use astria_eyre::eyre::{ self, + Context, WrapErr as _, }; pub(crate) use builder::Builder; @@ -47,7 +48,6 @@ mod builder; mod event; mod signer; mod state; -mod submitter; pub(super) struct Handle { batches_tx: mpsc::Sender>, @@ -88,10 +88,14 @@ impl Executor { } }; + // TODO: get this from the batch + let rollup_height = 0; + let actions = batch.into_iter().map(Action::from).collect::>(); // get nonce - let nonce = get_latest_nonce(self.sequencer_cometbft_client.clone(), self.signer.address).await?; + let nonce = get_latest_nonce(self.sequencer_cometbft_client.clone(), self.signer.address, self.state.clone()).await?; + debug!(nonce, "got latest nonce"); let unsigned = UnsignedTransaction { actions, @@ -103,13 +107,42 @@ impl Executor { // sign let signed = unsigned.into_signed(&self.signer.signing_key); + debug!(tx_hash = ?hex::encode(sha256(&signed.to_raw().encode_to_vec())), "signed transaction"); // broadcast commit + let rsp = submit_tx(self.sequencer_cometbft_client.clone(), signed, self.state.clone()).await.context("failed to submit transaction to to cometbft")?; + if let tendermint::abci::Code::Err(check_tx_code) = rsp.check_tx.code { + // handle check_tx failure + error!(abci.code = check_tx_code, + abci.log = rsp.check_tx.log, + rollup.height = rollup_height, + "transaction failed to be included in the mempool, aborting."); + break Err("check_tx failure upon submitting transaction"); + } else if let tendermint::abci::Code::Err(deliver_tx_code) = rsp.tx_result.code { + // handle deliver_tx failure + error!(abci.code = deliver_tx_code, + abci.log = rsp.tx_result.log, + rollup.height = rollup_height, + "transaction failed to be executed in a block, aborting." + ); + } else { + info!( + sequencer.block = rsp.height.value(), + sequencer.tx_hash = ?rsp.hash, + rollup.height = rollup_height, + "withdraw batch successfully executed." + ); + self.state.set_last_rollup_batch_height(rollup_height); + self.state.set_last_sequencer_height(rsp.height.value()); + self.state.set_last_sequencer_tx_hash(rsp.hash) + } } ) }; // update status + self.state; + // close the channel to signal to batcher that the executor is shutting down self.batches_rx.close(); match reason { @@ -130,6 +163,7 @@ impl Executor { async fn get_latest_nonce( client: sequencer_client::HttpClient, address: Address, + state: Arc, ) -> eyre::Result { debug!("fetching latest nonce from sequencer"); metrics::counter!(crate::metrics_init::NONCE_FETCH_COUNT).increment(1); @@ -144,6 +178,9 @@ async fn get_latest_nonce( err: &sequencer_client::extension_trait::Error| { metrics::counter!(crate::metrics_init::NONCE_FETCH_FAILURE_COUNT).increment(1); + let state = Arc::clone(&state); + state.set_sequencer_connected(false); + let wait_duration = next_delay .map(humantime::format_duration) .map(tracing::field::display); @@ -166,6 +203,8 @@ async fn get_latest_nonce( .await .wrap_err("failed getting latest nonce from sequencer after 1024 attempts"); + state.set_sequencer_connected(res.is_ok()); + metrics::histogram!(crate::metrics_init::NONCE_FETCH_LATENCY).record(start.elapsed()); res @@ -183,6 +222,7 @@ async fn get_latest_nonce( async fn submit_tx( client: sequencer_client::HttpClient, tx: SignedTransaction, + state: Arc, ) -> eyre::Result { let nonce = tx.unsigned_transaction().params.nonce; metrics::gauge!(crate::metrics_init::CURRENT_NONCE).set(nonce); @@ -202,6 +242,9 @@ async fn submit_tx( metrics::counter!(crate::metrics_init::SEQUENCER_SUBMISSION_FAILURE_COUNT) .increment(1); + let state = Arc::clone(&state); + state.set_sequencer_connected(false); + let wait_duration = next_delay .map(humantime::format_duration) .map(tracing::field::display); @@ -225,6 +268,8 @@ async fn submit_tx( .await .wrap_err("failed sending transaction after 1024 attempts"); + state.set_sequencer_connected(res.is_ok()); + metrics::histogram!(crate::metrics_init::SEQUENCER_SUBMISSION_LATENCY).record(start.elapsed()); res diff --git a/crates/astria-bridge-withdrawer/src/executor/state.rs b/crates/astria-bridge-withdrawer/src/executor/state.rs index aa03cc916a..77edc217c4 100644 --- a/crates/astria-bridge-withdrawer/src/executor/state.rs +++ b/crates/astria-bridge-withdrawer/src/executor/state.rs @@ -21,9 +21,35 @@ impl State { } } +macro_rules! forward_setter { + ($([$fn:ident <- $val:ty]),*$(,)?) => { + impl State { + $( + pub(super) fn $fn(&self, val: $val) { + self.inner + .send_if_modified(|state| state.$fn(val)); + } + )* + } + }; +} + +forward_setter!( + [set_sequencer_connected <- bool], + [set_last_rollup_batch_height <- u64], + [set_last_sequencer_height <- u64], + [set_last_sequencer_tx_hash <- tendermint::Hash], +); + #[derive(Clone, Debug, Default, PartialEq, Eq, serde::Serialize)] pub(crate) struct StateSnapshot { ready: bool, + + sequencer_connected: bool, + + last_rollup_block: Option, + last_sequencer_block: Option, + last_sequencer_tx_hash: Option, } impl StateSnapshot { @@ -36,6 +62,31 @@ impl StateSnapshot { } pub(crate) fn is_healthy(&self) -> bool { - todo!() + self.sequencer_connected + } + + /// Sets the sequencer connection status to `connected`. + fn set_sequencer_connected(&mut self, connected: bool) -> bool { + let changed = self.sequencer_connected ^ connected; + self.sequencer_connected = connected; + changed + } + + fn set_last_rollup_batch_height(&mut self, height: u64) -> bool { + let changed = self.last_rollup_block.map_or(true, |h| h != height); + self.last_rollup_block = Some(height); + changed + } + + fn set_last_sequencer_height(&mut self, height: u64) -> bool { + let changed = self.last_sequencer_block.map_or(true, |h| h != height); + self.last_sequencer_block = Some(height); + changed + } + + fn set_last_sequencer_tx_hash(&mut self, hash: tendermint::Hash) -> bool { + let changed = self.last_sequencer_tx_hash.map_or(true, |h| h != hash); + self.last_sequencer_tx_hash = Some(hash); + changed } } diff --git a/crates/astria-bridge-withdrawer/src/executor/submitter.rs b/crates/astria-bridge-withdrawer/src/executor/submitter.rs deleted file mode 100644 index bb179ae533..0000000000 --- a/crates/astria-bridge-withdrawer/src/executor/submitter.rs +++ /dev/null @@ -1,96 +0,0 @@ -use std::{ - pin::Pin, - task::Poll, -}; - -use astria_eyre::eyre; -use futures::{ - ready, - Future, -}; -use pin_project_lite::pin_project; -use sequencer_client::{ - tendermint_rpc::endpoint::broadcast::tx_commit, - SignedTransaction, -}; - -use super::signer::SequencerSigner; - -pin_project! { - /// TODO - pub(super) struct SubmitFut { - tx: SignedTransaction, - signer: SequencerSigner, - cometbft_client: sequencer_client::HttpClient, - nonce: u32, - sequencer_chain_id: String, - #[pin] - state: SubmitState, - } -} - -pin_project! { - #[project = SubmitStateProj] - enum SubmitState { - NotStarted, - WaitingForTxCommit { - #[pin] - fut: Pin> + Send>> - }, - WaitingForNonce { - #[pin] - fut: Pin> + Send>> - } - } -} - -impl Future for SubmitFut { - // TODO: output? - type Output = eyre::Result<()>; - - fn poll(mut self: Pin<&mut Self>, cx: &mut std::task::Context<'_>) -> Poll { - loop { - let this = self.as_mut().project(); - let new_state = match this.state.project() { - SubmitStateProj::NotStarted => { - // TODO: broadcast tx commit and change to waiting for txcommit - SubmitState::WaitingForTxCommit { - fut: todo!("broadcast tx commit"), - } - } - SubmitStateProj::WaitingForTxCommit { - fut, - } => match ready!(fut.poll(cx)) { - Ok(rsp) => { - // handle 0 code - // handle non-zero code - // if invalid nonce, change to waiting for nonce - SubmitState::WaitingForNonce { - fut: todo!("nonce fetch"), - } - } - Err(e) => { - // exit with an error - return todo!("exit with an error"); - } - }, - SubmitStateProj::WaitingForNonce { - fut, - } => match ready!(fut.poll(cx)) { - Ok(nonce) => { - // update tx nonce - // change to waiting for txcommit - SubmitState::WaitingForTxCommit { - fut: todo!("broadcast tx commit"), - } - } - Err(e) => { - // exit with an error - return todo!("exit with an error"); - } - }, - }; - self.as_mut().project().state.set(new_state); - } - } -} From 116a56b81c0416d7d070655d79cf069271598b2b Mon Sep 17 00:00:00 2001 From: itamar Date: Wed, 22 May 2024 16:31:40 -0400 Subject: [PATCH 05/23] post-merge clean up --- crates/astria-bridge-withdrawer/src/main.rs | 3 ++- .../src/withdrawer/executor/event.rs | 12 ------------ .../src/withdrawer/executor/mod.rs | 3 +-- .../src/withdrawer/executor/signer.rs | 2 +- .../astria-bridge-withdrawer/src/withdrawer/mod.rs | 6 ++---- 5 files changed, 6 insertions(+), 20 deletions(-) delete mode 100644 crates/astria-bridge-withdrawer/src/withdrawer/executor/event.rs diff --git a/crates/astria-bridge-withdrawer/src/main.rs b/crates/astria-bridge-withdrawer/src/main.rs index 88f715c0ae..e1e72e6764 100644 --- a/crates/astria-bridge-withdrawer/src/main.rs +++ b/crates/astria-bridge-withdrawer/src/main.rs @@ -4,6 +4,7 @@ use astria_bridge_withdrawer::{ metrics_init, BridgeService, Config, + WithdrawerService, BUILD_INFO, }; use astria_eyre::eyre::WrapErr as _; @@ -54,7 +55,7 @@ async fn main() -> ExitCode { let mut sigterm = signal(SignalKind::terminate()) .expect("setting a SIGTERM listener should always work on Unix"); - let (bridge, shutdown_handle) = BridgeService::new(cfg) + let (bridge, shutdown_handle) = WithdrawerService::new(cfg) .await .expect("could not initialize bridge"); let bridge_handle = tokio::spawn(bridge.run()); diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/executor/event.rs b/crates/astria-bridge-withdrawer/src/withdrawer/executor/event.rs deleted file mode 100644 index c670a6f577..0000000000 --- a/crates/astria-bridge-withdrawer/src/withdrawer/executor/event.rs +++ /dev/null @@ -1,12 +0,0 @@ -use astria_core::protocol::transaction::v1alpha1::{ - action::BridgeUnlockAction, - Action, -}; - -pub(crate) struct Event; - -impl From for Action { - fn from(e: Event) -> Self { - Action::BridgeUnlock(BridgeUnlockAction::default()) - } -} diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/executor/mod.rs b/crates/astria-bridge-withdrawer/src/withdrawer/executor/mod.rs index 6c33598a47..8889f8c506 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/executor/mod.rs +++ b/crates/astria-bridge-withdrawer/src/withdrawer/executor/mod.rs @@ -11,7 +11,6 @@ use astria_core::protocol::transaction::v1alpha1::{ use astria_eyre::eyre::{ self, Context, - WrapErr as _, }; pub(crate) use builder::Builder; use prost::Message as _; @@ -48,7 +47,7 @@ mod builder; mod signer; pub(super) struct Handle { - batches_tx: mpsc::Sender>, + pub(super) batches_tx: mpsc::Sender>, } pub(super) struct Executor { diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/executor/signer.rs b/crates/astria-bridge-withdrawer/src/withdrawer/executor/signer.rs index 9b8f61bc87..5c7e15563e 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/executor/signer.rs +++ b/crates/astria-bridge-withdrawer/src/withdrawer/executor/signer.rs @@ -30,8 +30,8 @@ impl SequencerSigner { Ok(Self { address: Address::from_verification_key(signing_key.verification_key()), - signing_key, verification_key: signing_key.verification_key(), + signing_key, }) } } diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/mod.rs b/crates/astria-bridge-withdrawer/src/withdrawer/mod.rs index 4af1d0c1b4..009b33ffd2 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/mod.rs +++ b/crates/astria-bridge-withdrawer/src/withdrawer/mod.rs @@ -60,7 +60,6 @@ impl WithdrawerService { } = cfg; // make bridge object - // TODO: add more fields let (executor, executor_handle) = executor::Builder { shutdown_token: shutdown_handle.token(), cometbft_endpoint: cfg.cometbft_endpoint, @@ -68,16 +67,15 @@ impl WithdrawerService { sequencer_key_path: cfg.sequencer_key_path, } .build() - .wrap_err("failed to create bridge")?; + .wrap_err("failed to initialize executor")?; // make api server let state_rx = executor.subscribe_to_state(); // TODO: use event_rx in the sequencer submitter - let (event_tx, _event_rx) = mpsc::channel(100); let ethereum_watcher = Watcher::new( &cfg.ethereum_contract_address, &cfg.ethereum_rpc_endpoint, - event_tx.clone(), + executor_handle.batches_tx, &shutdown_handle.token(), ) .await From f81a5136bb42d32cc1e247f0168148fe5ae6cc94 Mon Sep 17 00:00:00 2001 From: itamar Date: Wed, 22 May 2024 18:11:31 -0400 Subject: [PATCH 06/23] rename to submitter --- .../src/withdrawer/ethereum/watcher.rs | 3 +- .../src/withdrawer/mod.rs | 63 +++++++++++-------- .../{executor => submitter}/builder.rs | 8 +-- .../withdrawer/{executor => submitter}/mod.rs | 6 +- .../{executor => submitter}/signer.rs | 0 5 files changed, 47 insertions(+), 33 deletions(-) rename crates/astria-bridge-withdrawer/src/withdrawer/{executor => submitter}/builder.rs (87%) rename crates/astria-bridge-withdrawer/src/withdrawer/{executor => submitter}/mod.rs (98%) rename crates/astria-bridge-withdrawer/src/withdrawer/{executor => submitter}/signer.rs (100%) diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs b/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs index d76538badc..15e1dfc5e6 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs +++ b/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs @@ -52,6 +52,7 @@ impl Watcher { ethereum_rpc_endpoint: &str, batch_tx: mpsc::Sender>, shutdown_token: &CancellationToken, + state: Arc, ) -> Result { let provider = Arc::new( Provider::::connect(ethereum_rpc_endpoint) @@ -64,7 +65,6 @@ impl Watcher { let (event_tx, event_rx) = mpsc::channel(100); let batcher = Batcher::new(event_rx, batch_tx, shutdown_token); - let state = Arc::new(State::new()); Ok(Self { contract, event_tx, @@ -296,6 +296,7 @@ mod tests { &anvil.ws_endpoint(), event_tx, &CancellationToken::new(), + Arc::new(State::new()), ) .await .unwrap(); diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/mod.rs b/crates/astria-bridge-withdrawer/src/withdrawer/mod.rs index 009b33ffd2..756a051812 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/mod.rs +++ b/crates/astria-bridge-withdrawer/src/withdrawer/mod.rs @@ -1,5 +1,7 @@ use std::{ net::SocketAddr, + ops::Sub, + sync::Arc, time::Duration, }; @@ -28,7 +30,8 @@ use tracing::{ pub(crate) use self::state::StateSnapshot; use self::{ ethereum::Watcher, - executor::Executor, + state::State, + submitter::Submitter, }; use crate::{ api, @@ -36,15 +39,16 @@ use crate::{ }; mod ethereum; -mod executor; mod state; +mod submitter; pub struct WithdrawerService { // Token to signal all subtasks to shut down gracefully. shutdown_token: CancellationToken, api_server: api::ApiServer, - executor: Executor, + submitter: Submitter, ethereum_watcher: Watcher, + state: Arc, } impl WithdrawerService { @@ -59,24 +63,28 @@ impl WithdrawerService { api_addr, .. } = cfg; + let state = Arc::new(State::new()); + // make bridge object - let (executor, executor_handle) = executor::Builder { + let (submitter, submitter_handle) = submitter::Builder { shutdown_token: shutdown_handle.token(), cometbft_endpoint: cfg.cometbft_endpoint, sequencer_chain_id: cfg.sequencer_chain_id, sequencer_key_path: cfg.sequencer_key_path, + state: state.clone(), } .build() - .wrap_err("failed to initialize executor")?; + .wrap_err("failed to initialize submitter")?; // make api server - let state_rx = executor.subscribe_to_state(); + let state_rx = state.subscribe(); // TODO: use event_rx in the sequencer submitter let ethereum_watcher = Watcher::new( &cfg.ethereum_contract_address, &cfg.ethereum_rpc_endpoint, - executor_handle.batches_tx, + submitter_handle.batches_tx, &shutdown_handle.token(), + state.clone(), ) .await .wrap_err("failed to initialize ethereum watcher")?; @@ -91,8 +99,9 @@ impl WithdrawerService { let service = Self { shutdown_token: shutdown_handle.token(), api_server, - executor, + submitter, ethereum_watcher, + state, }; Ok((service, shutdown_handle)) @@ -102,8 +111,9 @@ impl WithdrawerService { let Self { shutdown_token, api_server, - executor, + submitter, ethereum_watcher, + state, } = self; // Separate the API shutdown signal from the cancellation token because we want it to live @@ -119,8 +129,8 @@ impl WithdrawerService { }); info!("spawned API server"); - let mut executor_task = tokio::spawn(executor.run()); - info!("spawned executor task"); + let mut submitter_task = tokio::spawn(submitter.run()); + info!("spawned submitter task"); let mut ethereum_watcher_task = tokio::spawn(ethereum_watcher.run()); info!("spawned ethereum watcher task"); @@ -129,17 +139,17 @@ impl WithdrawerService { report_exit("api server", o); Shutdown { api_task: None, - executor_task: Some(executor_task), + submitter_task: Some(submitter_task), ethereum_watcher_task: None, api_shutdown_signal, shutdown_token } } - o = &mut executor_task => { + o = &mut submitter_task => { report_exit("bridge worker", o); Shutdown { api_task: Some(api_task), - executor_task: None, + submitter_task: None, ethereum_watcher_task:Some(ethereum_watcher_task), api_shutdown_signal, shutdown_token @@ -149,7 +159,7 @@ impl WithdrawerService { report_exit("ethereum watcher", o); Shutdown { api_task: Some(api_task), - executor_task: Some(executor_task), + submitter_task: Some(submitter_task), ethereum_watcher_task: None, api_shutdown_signal, shutdown_token @@ -217,7 +227,7 @@ fn report_exit(task_name: &str, outcome: Result, JoinError>) { struct Shutdown { api_task: Option>>, - executor_task: Option>>, + submitter_task: Option>>, ethereum_watcher_task: Option>>, api_shutdown_signal: oneshot::Sender<()>, shutdown_token: CancellationToken, @@ -226,12 +236,12 @@ struct Shutdown { impl Shutdown { const API_SHUTDOWN_TIMEOUT_SECONDS: u64 = 4; const ETHEREUM_WATCHER_SHUTDOWN_TIMEOUT_SECONDS: u64 = 25; - const EXECUTOR_SHUTDOWN_TIMEOUT_SECONDS: u64 = 25; + const SUBMITTER_SHUTDOWN_TIMEOUT_SECONDS: u64 = 25; async fn run(self) { let Self { api_task, - executor_task, + submitter_task, ethereum_watcher_task, api_shutdown_signal, shutdown_token: token, @@ -239,11 +249,14 @@ impl Shutdown { token.cancel(); - // Giving executor 25 seconds to shutdown because Kubernetes issues a SIGKILL after 30. - if let Some(mut executor_task) = executor_task { - info!("waiting for executor task to shut down"); - let limit = Duration::from_secs(Self::EXECUTOR_SHUTDOWN_TIMEOUT_SECONDS); - match timeout(limit, &mut executor_task).await.map(flatten_result) { + // Giving submitter 25 seconds to shutdown because Kubernetes issues a SIGKILL after 30. + if let Some(mut submitter_task) = submitter_task { + info!("waiting for submitter task to shut down"); + let limit = Duration::from_secs(Self::SUBMITTER_SHUTDOWN_TIMEOUT_SECONDS); + match timeout(limit, &mut submitter_task) + .await + .map(flatten_result) + { Ok(Ok(())) => info!("bridge exited gracefully"), Ok(Err(error)) => error!(%error, "bridge exited with an error"), Err(_) => { @@ -251,11 +264,11 @@ impl Shutdown { timeout_secs = limit.as_secs(), "watcher did not shut down within timeout; killing it" ); - executor_task.abort(); + submitter_task.abort(); } } } else { - info!("executor task was already dead"); + info!("submitter task was already dead"); } // Giving bridge 25 seconds to shutdown because Kubernetes issues a SIGKILL after 30. diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/executor/builder.rs b/crates/astria-bridge-withdrawer/src/withdrawer/submitter/builder.rs similarity index 87% rename from crates/astria-bridge-withdrawer/src/withdrawer/executor/builder.rs rename to crates/astria-bridge-withdrawer/src/withdrawer/submitter/builder.rs index 4021c3c8d5..da1b375872 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/executor/builder.rs +++ b/crates/astria-bridge-withdrawer/src/withdrawer/submitter/builder.rs @@ -13,16 +13,18 @@ pub(crate) struct Builder { pub(crate) sequencer_key_path: String, pub(crate) sequencer_chain_id: String, pub(crate) cometbft_endpoint: String, + pub(crate) state: Arc, } impl Builder { /// Instantiates an `Executor`. - pub(crate) fn build(self) -> eyre::Result<(super::Executor, super::Handle)> { + pub(crate) fn build(self) -> eyre::Result<(super::Submitter, super::Handle)> { let Self { shutdown_token, sequencer_key_path, sequencer_chain_id, cometbft_endpoint, + state, } = self; let signer = super::signer::SequencerSigner::from_path(sequencer_key_path)?; @@ -31,10 +33,8 @@ impl Builder { let sequencer_cometbft_client = sequencer_client::HttpClient::new(&*cometbft_endpoint) .context("failed constructing cometbft http client")?; - let state = Arc::new(State::new()); - Ok(( - super::Executor { + super::Submitter { shutdown_token, state, batches_rx, diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/executor/mod.rs b/crates/astria-bridge-withdrawer/src/withdrawer/submitter/mod.rs similarity index 98% rename from crates/astria-bridge-withdrawer/src/withdrawer/executor/mod.rs rename to crates/astria-bridge-withdrawer/src/withdrawer/submitter/mod.rs index 8889f8c506..1a39e63530 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/executor/mod.rs +++ b/crates/astria-bridge-withdrawer/src/withdrawer/submitter/mod.rs @@ -50,7 +50,7 @@ pub(super) struct Handle { pub(super) batches_tx: mpsc::Sender>, } -pub(super) struct Executor { +pub(super) struct Submitter { shutdown_token: CancellationToken, state: Arc, batches_rx: mpsc::Receiver>, @@ -59,7 +59,7 @@ pub(super) struct Executor { sequencer_chain_id: String, } -impl Executor { +impl Submitter { pub(super) fn subscribe_to_state(&self) -> tokio::sync::watch::Receiver { self.state.subscribe() } @@ -137,7 +137,7 @@ impl Executor { // update status self.state.set_sequencer_connected(false); - // close the channel to signal to batcher that the executor is shutting down + // close the channel to signal to batcher that the submitter is shutting down self.batches_rx.close(); match reason { diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/executor/signer.rs b/crates/astria-bridge-withdrawer/src/withdrawer/submitter/signer.rs similarity index 100% rename from crates/astria-bridge-withdrawer/src/withdrawer/executor/signer.rs rename to crates/astria-bridge-withdrawer/src/withdrawer/submitter/signer.rs From 1212ee8b49a2a92ddc14018334bc8f10f54b2302 Mon Sep 17 00:00:00 2001 From: itamar Date: Wed, 22 May 2024 18:23:52 -0400 Subject: [PATCH 07/23] clean up submitter --- crates/astria-bridge-withdrawer/src/api.rs | 24 +++++++++---------- crates/astria-bridge-withdrawer/src/main.rs | 12 +++++----- .../src/withdrawer/mod.rs | 19 +++++++-------- .../src/withdrawer/state.rs | 12 ++++++---- .../src/withdrawer/submitter/builder.rs | 4 +++- .../src/withdrawer/submitter/mod.rs | 19 +++++++++++---- 6 files changed, 51 insertions(+), 39 deletions(-) diff --git a/crates/astria-bridge-withdrawer/src/api.rs b/crates/astria-bridge-withdrawer/src/api.rs index db51445584..25e7e786f3 100644 --- a/crates/astria-bridge-withdrawer/src/api.rs +++ b/crates/astria-bridge-withdrawer/src/api.rs @@ -25,34 +25,34 @@ use crate::withdrawer::StateSnapshot; pub(crate) type ApiServer = axum::Server>; -type BridgeState = watch::Receiver; +type WithdrawerState = watch::Receiver; #[derive(Clone)] /// `AppState` is used for as an axum extractor in its method handlers. struct AppState { - bridge_state: BridgeState, + withdrawer_state: WithdrawerState, } -impl FromRef for BridgeState { +impl FromRef for WithdrawerState { fn from_ref(app_state: &AppState) -> Self { - app_state.bridge_state.clone() + app_state.withdrawer_state.clone() } } -pub(crate) fn start(socket_addr: SocketAddr, bridge_state: BridgeState) -> ApiServer { +pub(crate) fn start(socket_addr: SocketAddr, withdrawer_state: WithdrawerState) -> ApiServer { let app = Router::new() .route("/healthz", get(get_healthz)) .route("/readyz", get(get_readyz)) .route("/status", get(get_status)) .with_state(AppState { - bridge_state, + withdrawer_state, }); axum::Server::bind(&socket_addr).serve(app.into_make_service()) } #[allow(clippy::unused_async)] // Permit because axum handlers must be async -async fn get_healthz(State(bridge_state): State) -> Healthz { - if bridge_state.borrow().is_healthy() { +async fn get_healthz(State(withdrawer_state): State) -> Healthz { + if withdrawer_state.borrow().is_healthy() { Healthz::Ok } else { Healthz::Degraded @@ -66,9 +66,9 @@ async fn get_healthz(State(bridge_state): State) -> Healthz { /// + there is a current sequencer height (implying a block from sequencer was received) /// + there is a current data availability height (implying a height was received from the DA) #[allow(clippy::unused_async)] // Permit because axum handlers must be async -async fn get_readyz(State(bridge_state): State) -> Readyz { - let is_bridge_online = bridge_state.borrow().is_ready(); - if is_bridge_online { +async fn get_readyz(State(withdrawer_state): State) -> Readyz { + let is_withdrawer_online = withdrawer_state.borrow().is_ready(); + if is_withdrawer_online { Readyz::Ok } else { Readyz::NotReady @@ -76,7 +76,7 @@ async fn get_readyz(State(bridge_state): State) -> Readyz { } #[allow(clippy::unused_async)] // Permit because axum handlers must be async -async fn get_status(State(withdrawer_state): State) -> Json { +async fn get_status(State(withdrawer_state): State) -> Json { Json(withdrawer_state.borrow().clone()) } diff --git a/crates/astria-bridge-withdrawer/src/main.rs b/crates/astria-bridge-withdrawer/src/main.rs index e1e72e6764..ca2982ad5d 100644 --- a/crates/astria-bridge-withdrawer/src/main.rs +++ b/crates/astria-bridge-withdrawer/src/main.rs @@ -55,10 +55,10 @@ async fn main() -> ExitCode { let mut sigterm = signal(SignalKind::terminate()) .expect("setting a SIGTERM listener should always work on Unix"); - let (bridge, shutdown_handle) = WithdrawerService::new(cfg) + let (withdrawer, shutdown_handle) = WithdrawerService::new(cfg) .await - .expect("could not initialize bridge"); - let bridge_handle = tokio::spawn(bridge.run()); + .expect("could not initialize withdrawer"); + let withdrawer_handle = tokio::spawn(withdrawer.run()); let shutdown_token = shutdown_handle.token(); tokio::select!( @@ -73,10 +73,10 @@ async fn main() -> ExitCode { } ); - if let Err(error) = bridge_handle.await { - error!(%error, "failed to join main bridge task"); + if let Err(error) = withdrawer_handle.await { + error!(%error, "failed to join main withdrawer task"); } - info!("bridge stopped"); + info!("withdrawer stopped"); ExitCode::SUCCESS } diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/mod.rs b/crates/astria-bridge-withdrawer/src/withdrawer/mod.rs index 756a051812..c3feac5cbc 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/mod.rs +++ b/crates/astria-bridge-withdrawer/src/withdrawer/mod.rs @@ -76,8 +76,6 @@ impl WithdrawerService { .build() .wrap_err("failed to initialize submitter")?; - // make api server - let state_rx = state.subscribe(); // TODO: use event_rx in the sequencer submitter let ethereum_watcher = Watcher::new( &cfg.ethereum_contract_address, @@ -90,7 +88,7 @@ impl WithdrawerService { .wrap_err("failed to initialize ethereum watcher")?; // make api server - let state_rx = ethereum_watcher.subscribe_to_state(); + let state_rx = state.subscribe(); let api_socket_addr = api_addr.parse::().wrap_err_with(|| { format!("failed to parse provided `api_addr` string as socket address: `{api_addr}`",) })?; @@ -140,13 +138,13 @@ impl WithdrawerService { Shutdown { api_task: None, submitter_task: Some(submitter_task), - ethereum_watcher_task: None, + ethereum_watcher_task: Some(ethereum_watcher_task), api_shutdown_signal, shutdown_token } } o = &mut submitter_task => { - report_exit("bridge worker", o); + report_exit("submitter", o); Shutdown { api_task: Some(api_task), submitter_task: None, @@ -235,8 +233,8 @@ struct Shutdown { impl Shutdown { const API_SHUTDOWN_TIMEOUT_SECONDS: u64 = 4; - const ETHEREUM_WATCHER_SHUTDOWN_TIMEOUT_SECONDS: u64 = 25; - const SUBMITTER_SHUTDOWN_TIMEOUT_SECONDS: u64 = 25; + const ETHEREUM_WATCHER_SHUTDOWN_TIMEOUT_SECONDS: u64 = 5; + const SUBMITTER_SHUTDOWN_TIMEOUT_SECONDS: u64 = 20; async fn run(self) { let Self { @@ -249,7 +247,7 @@ impl Shutdown { token.cancel(); - // Giving submitter 25 seconds to shutdown because Kubernetes issues a SIGKILL after 30. + // Giving submitter 20 seconds to shutdown because Kubernetes issues a SIGKILL after 30. if let Some(mut submitter_task) = submitter_task { info!("waiting for submitter task to shut down"); let limit = Duration::from_secs(Self::SUBMITTER_SHUTDOWN_TIMEOUT_SECONDS); @@ -271,7 +269,7 @@ impl Shutdown { info!("submitter task was already dead"); } - // Giving bridge 25 seconds to shutdown because Kubernetes issues a SIGKILL after 30. + // Giving bridge 5 seconds to shutdown because Kubernetes issues a SIGKILL after 30. if let Some(mut ethereum_watcher_task) = ethereum_watcher_task { info!("waiting for watcher task to shut down"); let limit = Duration::from_secs(Self::ETHEREUM_WATCHER_SHUTDOWN_TIMEOUT_SECONDS); @@ -293,7 +291,8 @@ impl Shutdown { info!("watcher task was already dead"); } - // Giving the API task 4 seconds. 25 for watcher + 4s = 29s (out of 30s for k8s). + // Giving the API task 4 seconds. 5s for watcher + 20 for submitter + 4s = 29s (out of 30s + // for k8s). if let Some(mut api_task) = api_task { info!("sending shutdown signal to API server"); let _ = api_shutdown_signal.send(()); diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/state.rs b/crates/astria-bridge-withdrawer/src/withdrawer/state.rs index 77edc217c4..eb842c4439 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/state.rs +++ b/crates/astria-bridge-withdrawer/src/withdrawer/state.rs @@ -36,7 +36,7 @@ macro_rules! forward_setter { forward_setter!( [set_sequencer_connected <- bool], - [set_last_rollup_batch_height <- u64], + [set_last_rollup_height_submitted <- u64], [set_last_sequencer_height <- u64], [set_last_sequencer_tx_hash <- tendermint::Hash], ); @@ -47,7 +47,7 @@ pub(crate) struct StateSnapshot { sequencer_connected: bool, - last_rollup_block: Option, + last_rollup_height_submitted: Option, last_sequencer_block: Option, last_sequencer_tx_hash: Option, } @@ -72,9 +72,11 @@ impl StateSnapshot { changed } - fn set_last_rollup_batch_height(&mut self, height: u64) -> bool { - let changed = self.last_rollup_block.map_or(true, |h| h != height); - self.last_rollup_block = Some(height); + fn set_last_rollup_height_submitted(&mut self, height: u64) -> bool { + let changed = self + .last_rollup_height_submitted + .map_or(true, |h| h != height); + self.last_rollup_height_submitted = Some(height); changed } diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/submitter/builder.rs b/crates/astria-bridge-withdrawer/src/withdrawer/submitter/builder.rs index da1b375872..8295da05b4 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/submitter/builder.rs +++ b/crates/astria-bridge-withdrawer/src/withdrawer/submitter/builder.rs @@ -8,6 +8,8 @@ use tokio_util::sync::CancellationToken; use super::state::State; +const BATCH_QUEUE_SIZE: usize = 256; + pub(crate) struct Builder { pub(crate) shutdown_token: CancellationToken, pub(crate) sequencer_key_path: String, @@ -28,7 +30,7 @@ impl Builder { } = self; let signer = super::signer::SequencerSigner::from_path(sequencer_key_path)?; - let (batches_tx, batches_rx) = tokio::sync::mpsc::channel(1); + let (batches_tx, batches_rx) = tokio::sync::mpsc::channel(BATCH_QUEUE_SIZE); let sequencer_cometbft_client = sequencer_client::HttpClient::new(&*cometbft_endpoint) .context("failed constructing cometbft http client")?; diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/submitter/mod.rs b/crates/astria-bridge-withdrawer/src/withdrawer/submitter/mod.rs index 1a39e63530..7dde4343ea 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/submitter/mod.rs +++ b/crates/astria-bridge-withdrawer/src/withdrawer/submitter/mod.rs @@ -88,9 +88,9 @@ impl Submitter { // TODO: get this from the batch let rollup_height = 0; - // get nonce + // get nonce and make unsigned transaction let nonce = get_latest_nonce(self.sequencer_cometbft_client.clone(), self.signer.address, self.state.clone()).await?; - debug!(nonce, "got latest nonce"); + debug!(nonce, "fetched latest nonce"); let unsigned = UnsignedTransaction { actions, @@ -100,11 +100,11 @@ impl Submitter { }, }; - // sign + // sign transaction let signed = unsigned.into_signed(&self.signer.signing_key); debug!(tx_hash = ?hex::encode(sha256(&signed.to_raw().encode_to_vec())), "signed transaction"); - // broadcast commit + // submit transaction and handle response let rsp = submit_tx(self.sequencer_cometbft_client.clone(), signed, self.state.clone()).await.context("failed to submit transaction to to cometbft")?; if let tendermint::abci::Code::Err(check_tx_code) = rsp.check_tx.code { // handle check_tx failure @@ -121,13 +121,14 @@ impl Submitter { "transaction failed to be executed in a block, aborting." ); } else { + // update state after successful submission info!( sequencer.block = rsp.height.value(), sequencer.tx_hash = ?rsp.hash, rollup.height = rollup_height, "withdraw batch successfully executed." ); - self.state.set_last_rollup_batch_height(rollup_height); + self.state.set_last_rollup_height_submitted(rollup_height); self.state.set_last_sequencer_height(rsp.height.value()); self.state.set_last_sequencer_tx_hash(rsp.hash) } @@ -274,3 +275,11 @@ fn sha256(data: &[u8]) -> [u8; 32] { use sha2::Sha256; Sha256::digest(data) } + +#[cfg(test)] +mod tests { + #[tokio::test] + async fn submit_success() { + todo!("should this be here or in separate tests folder?") + } +} From d789bd42e2c8b72a5d0f284425820a492b3a5b99 Mon Sep 17 00:00:00 2001 From: itamar Date: Wed, 22 May 2024 18:38:31 -0400 Subject: [PATCH 08/23] add rollup height to batches --- .../src/withdrawer/ethereum/watcher.rs | 14 +++++-------- .../src/withdrawer/state.rs | 21 +++++++++++++------ .../src/withdrawer/submitter/mod.rs | 9 +++----- 3 files changed, 23 insertions(+), 21 deletions(-) diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs b/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs index 15e1dfc5e6..e15c8f9167 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs +++ b/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs @@ -50,7 +50,7 @@ impl Watcher { pub(crate) async fn new( ethereum_contract_address: &str, ethereum_rpc_endpoint: &str, - batch_tx: mpsc::Sender>, + batch_tx: mpsc::Sender<(Vec, u64)>, shutdown_token: &CancellationToken, state: Arc, ) -> Result { @@ -76,15 +76,11 @@ impl Watcher { } impl Watcher { - pub(crate) fn subscribe_to_state(&self) -> tokio::sync::watch::Receiver { - self.state.subscribe() - } - pub(crate) async fn run(mut self) -> Result<()> { let batcher = self.batcher.take().expect("batcher must be present"); tokio::task::spawn(batcher.run()); - self.state.set_ready(); + self.state.set_watcher_ready(); // start from block 1 right now // TODO: determine the last block we've seen based on the sequencer data @@ -135,14 +131,14 @@ pub(crate) struct EventWithMetadata { struct Batcher { event_rx: mpsc::Receiver<(WithdrawalFilter, LogMeta)>, - batch_tx: mpsc::Sender>, + batch_tx: mpsc::Sender<(Vec, u64)>, shutdown_token: CancellationToken, } impl Batcher { pub(crate) fn new( event_rx: mpsc::Receiver<(WithdrawalFilter, LogMeta)>, - batch_tx: mpsc::Sender>, + batch_tx: mpsc::Sender<(Vec, u64)>, shutdown_token: &CancellationToken, ) -> Self { Self { @@ -180,7 +176,7 @@ impl Batcher { // block number increased; send current batch and start a new one if !actions.is_empty() { self.batch_tx - .send(actions) + .send((actions, last_block_number.into())) .await .wrap_err("failed to send batched events; receiver dropped?")?; } diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/state.rs b/crates/astria-bridge-withdrawer/src/withdrawer/state.rs index eb842c4439..647bb489b5 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/state.rs +++ b/crates/astria-bridge-withdrawer/src/withdrawer/state.rs @@ -12,8 +12,12 @@ impl State { } } - pub(super) fn set_ready(&self) { - self.inner.send_modify(StateSnapshot::set_ready); + pub(super) fn set_watcher_ready(&self) { + self.inner.send_modify(StateSnapshot::set_watcher_ready); + } + + pub(super) fn set_submitter_ready(&self) { + self.inner.send_modify(StateSnapshot::set_submitter_ready); } pub(super) fn subscribe(&self) -> watch::Receiver { @@ -43,7 +47,8 @@ forward_setter!( #[derive(Clone, Debug, Default, PartialEq, Eq, serde::Serialize)] pub(crate) struct StateSnapshot { - ready: bool, + watcher_ready: bool, + submitter_ready: bool, sequencer_connected: bool, @@ -53,12 +58,16 @@ pub(crate) struct StateSnapshot { } impl StateSnapshot { - pub(crate) fn set_ready(&mut self) { - self.ready = true; + pub(crate) fn set_watcher_ready(&mut self) { + self.watcher_ready = true; + } + + pub(crate) fn set_submitter_ready(&mut self) { + self.submitter_ready = true; } pub(crate) fn is_ready(&self) -> bool { - self.ready + self.submitter_ready && self.watcher_ready } pub(crate) fn is_healthy(&self) -> bool { diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/submitter/mod.rs b/crates/astria-bridge-withdrawer/src/withdrawer/submitter/mod.rs index 7dde4343ea..64db8e2275 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/submitter/mod.rs +++ b/crates/astria-bridge-withdrawer/src/withdrawer/submitter/mod.rs @@ -53,7 +53,7 @@ pub(super) struct Handle { pub(super) struct Submitter { shutdown_token: CancellationToken, state: Arc, - batches_rx: mpsc::Receiver>, + batches_rx: mpsc::Receiver<(Vec, u64)>, sequencer_cometbft_client: sequencer_client::HttpClient, signer: SequencerSigner, sequencer_chain_id: String, @@ -65,7 +65,7 @@ impl Submitter { } pub(super) async fn run(mut self) -> eyre::Result<()> { - self.state.set_ready(); + self.state.set_submitter_ready(); let reason = loop { select!( @@ -76,7 +76,7 @@ impl Submitter { break Ok("shutdown requested"); } - batch = self.batches_rx.recv() => { + (batch, rollup_height) = self.batches_rx.recv() => { let actions = match batch { Some(batch) => batch, None => { @@ -85,9 +85,6 @@ impl Submitter { } }; - // TODO: get this from the batch - let rollup_height = 0; - // get nonce and make unsigned transaction let nonce = get_latest_nonce(self.sequencer_cometbft_client.clone(), self.signer.address, self.state.clone()).await?; debug!(nonce, "fetched latest nonce"); From 387c784db428077db2195d63c76236cc27fdffb0 Mon Sep 17 00:00:00 2001 From: itamar Date: Wed, 22 May 2024 19:09:06 -0400 Subject: [PATCH 09/23] convert events to actions --- crates/astria-bridge-withdrawer/src/main.rs | 1 - .../src/withdrawer/ethereum/watcher.rs | 32 ++++++++++++------- .../src/withdrawer/mod.rs | 8 ++--- .../src/withdrawer/submitter/mod.rs | 13 +++----- .../src/withdrawer/submitter/signer.rs | 3 -- 5 files changed, 27 insertions(+), 30 deletions(-) diff --git a/crates/astria-bridge-withdrawer/src/main.rs b/crates/astria-bridge-withdrawer/src/main.rs index ca2982ad5d..9dfc2d4daa 100644 --- a/crates/astria-bridge-withdrawer/src/main.rs +++ b/crates/astria-bridge-withdrawer/src/main.rs @@ -2,7 +2,6 @@ use std::process::ExitCode; use astria_bridge_withdrawer::{ metrics_init, - BridgeService, Config, WithdrawerService, BUILD_INFO, diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs b/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs index e15c8f9167..a0802464fc 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs +++ b/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs @@ -1,8 +1,15 @@ use std::sync::Arc; -use astria_core::protocol::transaction::v1alpha1::Action; +use astria_core::{ + primitive::v1::asset::Id, + protocol::transaction::v1alpha1::{ + action::BridgeUnlockAction, + Action, + }, +}; use astria_eyre::{ eyre::{ + self, eyre, WrapErr as _, }, @@ -32,10 +39,9 @@ use super::astria_withdrawer::{ astria_withdrawer::WithdrawalFilter, AstriaWithdrawer, }; -use crate::withdrawer::{ - state::State, - StateSnapshot, -}; +use crate::withdrawer::state::State; + +const FEE_ASSET_ID: &str = "fee"; /// Watches for withdrawal events emitted by the `AstriaWithdrawer` contract. pub(crate) struct Watcher { @@ -167,7 +173,7 @@ impl Batcher { block_number: meta.block_number, transaction_hash: meta.transaction_hash, }; - let action = Action::from(event_with_metadata); + let action = event_to_action(event_with_metadata)?; if meta.block_number == last_block_number { // block number was the same; add event to current batch @@ -176,7 +182,7 @@ impl Batcher { // block number increased; send current batch and start a new one if !actions.is_empty() { self.batch_tx - .send((actions, last_block_number.into())) + .send((actions, last_block_number.as_u64())) .await .wrap_err("failed to send batched events; receiver dropped?")?; } @@ -204,10 +210,14 @@ fn address_from_string(s: &str) -> Result { Ok(address.into()) } -impl From for Action { - fn from(event_with_metadata: EventWithMetadata) -> Self { - todo!("implement action conversion logic"); - } +fn event_to_action(event_with_metadata: EventWithMetadata) -> eyre::Result { + let action = BridgeUnlockAction { + to: event_with_metadata.event.sender.to_fixed_bytes().into(), + amount: event_with_metadata.event.amount.as_u128(), + memo: event_with_metadata.event.memo.to_vec(), + fee_asset_id: Id::from_denom(FEE_ASSET_ID), + }; + Ok(Action::BridgeUnlock(action)) } #[cfg(test)] diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/mod.rs b/crates/astria-bridge-withdrawer/src/withdrawer/mod.rs index c3feac5cbc..888ae2a8e4 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/mod.rs +++ b/crates/astria-bridge-withdrawer/src/withdrawer/mod.rs @@ -1,6 +1,5 @@ use std::{ net::SocketAddr, - ops::Sub, sync::Arc, time::Duration, }; @@ -11,10 +10,7 @@ use astria_eyre::eyre::{ }; use tokio::{ select, - sync::{ - mpsc, - oneshot, - }, + sync::oneshot, task::{ JoinError, JoinHandle, @@ -111,7 +107,7 @@ impl WithdrawerService { api_server, submitter, ethereum_watcher, - state, + state: _state, } = self; // Separate the API shutdown signal from the cancellation token because we want it to live diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/submitter/mod.rs b/crates/astria-bridge-withdrawer/src/withdrawer/submitter/mod.rs index 64db8e2275..e275c2f82f 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/submitter/mod.rs +++ b/crates/astria-bridge-withdrawer/src/withdrawer/submitter/mod.rs @@ -22,7 +22,6 @@ use sequencer_client::{ }; use signer::SequencerSigner; use state::State; -pub(super) use state::StateSnapshot; use tendermint::crypto::Sha256; use tokio::{ select, @@ -47,7 +46,7 @@ mod builder; mod signer; pub(super) struct Handle { - pub(super) batches_tx: mpsc::Sender>, + pub(super) batches_tx: mpsc::Sender<(Vec, u64)>, } pub(super) struct Submitter { @@ -60,10 +59,6 @@ pub(super) struct Submitter { } impl Submitter { - pub(super) fn subscribe_to_state(&self) -> tokio::sync::watch::Receiver { - self.state.subscribe() - } - pub(super) async fn run(mut self) -> eyre::Result<()> { self.state.set_submitter_ready(); @@ -76,9 +71,9 @@ impl Submitter { break Ok("shutdown requested"); } - (batch, rollup_height) = self.batches_rx.recv() => { - let actions = match batch { - Some(batch) => batch, + batch = self.batches_rx.recv() => { + let (actions, rollup_height) = match batch { + Some((actions, rollup_height)) => (actions, rollup_height), None => { info!("received None from batch channel, shutting down"); break Err("channel closed"); diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/submitter/signer.rs b/crates/astria-bridge-withdrawer/src/withdrawer/submitter/signer.rs index 5c7e15563e..0556f82e43 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/submitter/signer.rs +++ b/crates/astria-bridge-withdrawer/src/withdrawer/submitter/signer.rs @@ -8,13 +8,11 @@ use astria_eyre::eyre::{ self, eyre, }; -use ed25519_consensus::VerificationKey; use sequencer_client::Address; pub(super) struct SequencerSigner { pub(super) address: Address, pub(super) signing_key: SigningKey, - pub(super) verification_key: VerificationKey, } impl SequencerSigner { @@ -30,7 +28,6 @@ impl SequencerSigner { Ok(Self { address: Address::from_verification_key(signing_key.verification_key()), - verification_key: signing_key.verification_key(), signing_key, }) } From 3dda438469e8d656d188b2c5a86fe97d256ddc08 Mon Sep 17 00:00:00 2001 From: itamar Date: Wed, 22 May 2024 19:20:26 -0400 Subject: [PATCH 10/23] watcher test --- .../src/withdrawer/ethereum/watcher.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs b/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs index a0802464fc..f4f8b0defb 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs +++ b/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs @@ -294,7 +294,7 @@ mod tests { block_number: receipt.block_number.unwrap(), transaction_hash: receipt.transaction_hash, }; - let expected_action = Action::from(expected_event); + let expected_action = event_to_action(expected_event).unwrap(); let (event_tx, mut event_rx) = mpsc::channel(100); let watcher = Watcher::new( @@ -312,8 +312,8 @@ mod tests { // make another tx to trigger anvil to make another block send_withdraw_transaction(&contract, value).await; - let events = event_rx.recv().await.unwrap(); + let (events, rollup_height) = event_rx.recv().await.unwrap(); assert_eq!(events.len(), 1); - // assert_eq!(events[0], expected_action); + assert!(matches!(events[0], Action::BridgeUnlock { .. })); } } From bba08c5d35141919ab9dd2e34b2aa6b696dfef15 Mon Sep 17 00:00:00 2001 From: itamar Date: Wed, 22 May 2024 19:24:12 -0400 Subject: [PATCH 11/23] fix watcher test? --- .../src/withdrawer/ethereum/watcher.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs b/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs index f4f8b0defb..7865902161 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs +++ b/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs @@ -314,6 +314,9 @@ mod tests { let (events, rollup_height) = event_rx.recv().await.unwrap(); assert_eq!(events.len(), 1); - assert!(matches!(events[0], Action::BridgeUnlock { .. })); + assert!(matches!( + events[0], + Action::BridgeUnlock(BridgeUnlockAction { .. }) + )); } } From 61f225e1ed3cdfe0ef08c0ad6d299c01e9ad6c69 Mon Sep 17 00:00:00 2001 From: itamar Date: Wed, 22 May 2024 19:24:58 -0400 Subject: [PATCH 12/23] fix watcher test?? --- .../src/withdrawer/ethereum/watcher.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs b/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs index 7865902161..9098a8b2a2 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs +++ b/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs @@ -314,9 +314,6 @@ mod tests { let (events, rollup_height) = event_rx.recv().await.unwrap(); assert_eq!(events.len(), 1); - assert!(matches!( - events[0], - Action::BridgeUnlock(BridgeUnlockAction { .. }) - )); + assert!(matches!(events[0], Action::BridgeUnlock(_))); } } From 3b3199218f0a065889a5b0b126cec2db0837d0e5 Mon Sep 17 00:00:00 2001 From: itamar Date: Wed, 22 May 2024 19:30:01 -0400 Subject: [PATCH 13/23] fix env test --- crates/astria-bridge-withdrawer/local.env.example | 7 +++++++ crates/astria-bridge-withdrawer/src/config.rs | 1 - .../src/withdrawer/ethereum/watcher.rs | 4 ++-- .../src/withdrawer/submitter/mod.rs | 8 -------- 4 files changed, 9 insertions(+), 11 deletions(-) diff --git a/crates/astria-bridge-withdrawer/local.env.example b/crates/astria-bridge-withdrawer/local.env.example index 0c43abfa99..1e4959323a 100644 --- a/crates/astria-bridge-withdrawer/local.env.example +++ b/crates/astria-bridge-withdrawer/local.env.example @@ -24,6 +24,13 @@ NO_COLOR= # serves RPCs. ASTRIA_BRIDGE_WITHDRAWER_COMETBFT_ENDPOINT="http://127.0.0.1:26657" +# Chain ID of the sequencer chain which transactions are submitted to. +ASTRIA_BRIDGE_WITHDRAWER_SEQUENCER_CHAIN_ID="astria-dev-1" + +# The path to the file storing the private key for the sequencer account used for signing +# transactions. The file should contain a hex-encoded Ed25519 secret key. +ASTRIA_BRIDGE_WITHDRAWER_SEQUENCER_KEY_PATH=/path/to/priv_sequencer_key.json + ASTRIA_BRIDGE_WITHDRAWER_ETHEREUM_CONTRACT_ADDRESS="0x" ASTRIA_BRIDGE_WITHDRAWER_ETHEREUM_RPC_ENDPOINT="http://127.0.0.1:8545" diff --git a/crates/astria-bridge-withdrawer/src/config.rs b/crates/astria-bridge-withdrawer/src/config.rs index 6c0e115883..4dad440695 100644 --- a/crates/astria-bridge-withdrawer/src/config.rs +++ b/crates/astria-bridge-withdrawer/src/config.rs @@ -19,7 +19,6 @@ pub struct Config { pub ethereum_contract_address: String, // The rpc endpoint of the evm rollup. pub ethereum_rpc_endpoint: String, - // The socket address at which the bridge service will server healthz, readyz, and status // calls. pub api_addr: String, diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs b/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs index 9098a8b2a2..82bf59a55e 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs +++ b/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs @@ -294,7 +294,7 @@ mod tests { block_number: receipt.block_number.unwrap(), transaction_hash: receipt.transaction_hash, }; - let expected_action = event_to_action(expected_event).unwrap(); + let _expected_action = event_to_action(expected_event).unwrap(); let (event_tx, mut event_rx) = mpsc::channel(100); let watcher = Watcher::new( @@ -312,7 +312,7 @@ mod tests { // make another tx to trigger anvil to make another block send_withdraw_transaction(&contract, value).await; - let (events, rollup_height) = event_rx.recv().await.unwrap(); + let (events, _rollup_height) = event_rx.recv().await.unwrap(); assert_eq!(events.len(), 1); assert!(matches!(events[0], Action::BridgeUnlock(_))); } diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/submitter/mod.rs b/crates/astria-bridge-withdrawer/src/withdrawer/submitter/mod.rs index e275c2f82f..0ce445424c 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/submitter/mod.rs +++ b/crates/astria-bridge-withdrawer/src/withdrawer/submitter/mod.rs @@ -267,11 +267,3 @@ fn sha256(data: &[u8]) -> [u8; 32] { use sha2::Sha256; Sha256::digest(data) } - -#[cfg(test)] -mod tests { - #[tokio::test] - async fn submit_success() { - todo!("should this be here or in separate tests folder?") - } -} From 235e8f7fe9afd8b6bd5565afb99c389cc8560e39 Mon Sep 17 00:00:00 2001 From: elizabeth Date: Wed, 22 May 2024 20:12:01 -0400 Subject: [PATCH 14/23] update watcher unit test to do exact match --- Cargo.toml | 1 + crates/astria-bridge-withdrawer/Cargo.toml | 3 +-- .../src/withdrawer/ethereum/watcher.rs | 15 ++++++++++++--- crates/astria-composer/Cargo.toml | 2 +- crates/astria-conductor/Cargo.toml | 2 +- .../src/protocol/transaction/v1alpha1/action.rs | 2 +- crates/astria-sequencer-relayer/Cargo.toml | 2 +- 7 files changed, 18 insertions(+), 9 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index cd4ad3a39c..7f0650438c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -72,6 +72,7 @@ itertools = "0.12.1" itoa = "1.0.10" jsonrpsee = { version = "0.20" } once_cell = "1.17.1" +pin-project-lite = "0.2.13" sha2 = "0.10" serde = "1" serde_json = "1" diff --git a/crates/astria-bridge-withdrawer/Cargo.toml b/crates/astria-bridge-withdrawer/Cargo.toml index 476c53606f..bfde7151fa 100644 --- a/crates/astria-bridge-withdrawer/Cargo.toml +++ b/crates/astria-bridge-withdrawer/Cargo.toml @@ -14,8 +14,6 @@ name = "astria-bridge-withdrawer" [dependencies] http = "0.2.9" -pin-project-lite = "0.2.13" - axum = { workspace = true } ed25519-consensus = { workspace = true } futures = { workspace = true } @@ -24,6 +22,7 @@ ethers = { workspace = true, features = ["ethers-solc", "ws"] } hyper = { workspace = true } humantime = { workspace = true } metrics = { workspace = true } +pin-project-lite = { workspace = true } prost = { workspace = true } serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs b/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs index 82bf59a55e..ffc0149d96 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs +++ b/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs @@ -214,7 +214,7 @@ fn event_to_action(event_with_metadata: EventWithMetadata) -> eyre::Result Date: Wed, 22 May 2024 20:15:26 -0400 Subject: [PATCH 15/23] match statement cleanup --- .../src/withdrawer/submitter/mod.rs | 54 +++++++++---------- 1 file changed, 25 insertions(+), 29 deletions(-) diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/submitter/mod.rs b/crates/astria-bridge-withdrawer/src/withdrawer/submitter/mod.rs index 0ce445424c..60a4342d5d 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/submitter/mod.rs +++ b/crates/astria-bridge-withdrawer/src/withdrawer/submitter/mod.rs @@ -98,35 +98,33 @@ impl Submitter { // submit transaction and handle response let rsp = submit_tx(self.sequencer_cometbft_client.clone(), signed, self.state.clone()).await.context("failed to submit transaction to to cometbft")?; - if let tendermint::abci::Code::Err(check_tx_code) = rsp.check_tx.code { - // handle check_tx failure - error!(abci.code = check_tx_code, - abci.log = rsp.check_tx.log, - rollup.height = rollup_height, - "transaction failed to be included in the mempool, aborting."); - break Err("check_tx failure upon submitting transaction"); - } else if let tendermint::abci::Code::Err(deliver_tx_code) = rsp.tx_result.code { - // handle deliver_tx failure - error!(abci.code = deliver_tx_code, - abci.log = rsp.tx_result.log, - rollup.height = rollup_height, - "transaction failed to be executed in a block, aborting." - ); - } else { - // update state after successful submission - info!( - sequencer.block = rsp.height.value(), - sequencer.tx_hash = ?rsp.hash, - rollup.height = rollup_height, - "withdraw batch successfully executed." - ); - self.state.set_last_rollup_height_submitted(rollup_height); - self.state.set_last_sequencer_height(rsp.height.value()); - self.state.set_last_sequencer_tx_hash(rsp.hash) + + match rsp.check_tx.code { + tendermint::abci::Code::Ok => { + // update state after successful submission + info!( + sequencer.block = rsp.height.value(), + sequencer.tx_hash = ?rsp.hash, + rollup.height = rollup_height, + "withdraw batch successfully executed" + ); + self.state.set_last_rollup_height_submitted(rollup_height); + self.state.set_last_sequencer_height(rsp.height.value()); + self.state.set_last_sequencer_tx_hash(rsp.hash) + } + tendermint::abci::Code::Err(check_tx_code) => { + // handle check_tx failure + error!(abci.code = check_tx_code, + abci.log = rsp.check_tx.log, + rollup.height = rollup_height, + "transaction failed to be included in the mempool, aborting batch submission"); + break Err("check_tx failure upon submitting transaction"); + } } } ) }; + // update status self.state.set_sequencer_connected(false); @@ -134,14 +132,12 @@ impl Submitter { self.batches_rx.close(); match reason { - Ok(reason) => info!(reason, "starting shutdown process"), + Ok(reason) => info!(reason, "submitter shutting down"), Err(reason) => { - error!(%reason, "starting shutdown process") + error!(%reason, "submitter shutting down") } } - // handle shutdown - Ok(()) } } From 734e91259becd95d3b5ea45de4016be1406cc862 Mon Sep 17 00:00:00 2001 From: itamar Date: Thu, 23 May 2024 11:54:12 -0400 Subject: [PATCH 16/23] clean up pr comments --- .../src/withdrawer/mod.rs | 17 +-- .../src/withdrawer/submitter/builder.rs | 2 +- .../src/withdrawer/submitter/mod.rs | 133 ++++++++++-------- 3 files changed, 82 insertions(+), 70 deletions(-) diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/mod.rs b/crates/astria-bridge-withdrawer/src/withdrawer/mod.rs index 888ae2a8e4..2def72b22f 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/mod.rs +++ b/crates/astria-bridge-withdrawer/src/withdrawer/mod.rs @@ -48,7 +48,7 @@ pub struct WithdrawerService { } impl WithdrawerService { - /// Instantiates a new `BridgeService`. + /// Instantiates a new `WithdrawerService`. /// /// # Errors /// @@ -61,7 +61,7 @@ impl WithdrawerService { let state = Arc::new(State::new()); - // make bridge object + // make submitter object let (submitter, submitter_handle) = submitter::Builder { shutdown_token: shutdown_handle.token(), cometbft_endpoint: cfg.cometbft_endpoint, @@ -165,10 +165,10 @@ impl WithdrawerService { } } -/// A handle for instructing the [`BridgeService`] to shut down. +/// A handle for instructing the [`WithdrawerService`] to shut down. /// -/// It is returned along with its related `BridgeService` from [`BridgeService::new`]. The -/// `BridgeService` will begin to shut down as soon as [`ShutdownHandle::shutdown`] is called or +/// It is returned along with its related `WithdrawerService` from [`WithdrawerService::new`]. The +/// `WithdrawerService` will begin to shut down as soon as [`ShutdownHandle::shutdown`] is called or /// when the `ShutdownHandle` is dropped. pub struct ShutdownHandle { token: CancellationToken, @@ -251,8 +251,8 @@ impl Shutdown { .await .map(flatten_result) { - Ok(Ok(())) => info!("bridge exited gracefully"), - Ok(Err(error)) => error!(%error, "bridge exited with an error"), + Ok(Ok(())) => info!("withdrawer exited gracefully"), + Ok(Err(error)) => error!(%error, "withdrawer exited with an error"), Err(_) => { error!( timeout_secs = limit.as_secs(), @@ -265,7 +265,8 @@ impl Shutdown { info!("submitter task was already dead"); } - // Giving bridge 5 seconds to shutdown because Kubernetes issues a SIGKILL after 30. + // Giving ethereum watcher 5 seconds to shutdown because Kubernetes issues a SIGKILL after + // 30. if let Some(mut ethereum_watcher_task) = ethereum_watcher_task { info!("waiting for watcher task to shut down"); let limit = Duration::from_secs(Self::ETHEREUM_WATCHER_SHUTDOWN_TIMEOUT_SECONDS); diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/submitter/builder.rs b/crates/astria-bridge-withdrawer/src/withdrawer/submitter/builder.rs index 8295da05b4..51bded9efb 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/submitter/builder.rs +++ b/crates/astria-bridge-withdrawer/src/withdrawer/submitter/builder.rs @@ -19,7 +19,7 @@ pub(crate) struct Builder { } impl Builder { - /// Instantiates an `Executor`. + /// Instantiates an `Submitter`. pub(crate) fn build(self) -> eyre::Result<(super::Submitter, super::Handle)> { let Self { shutdown_token, diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/submitter/mod.rs b/crates/astria-bridge-withdrawer/src/withdrawer/submitter/mod.rs index 60a4342d5d..db8b856e35 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/submitter/mod.rs +++ b/crates/astria-bridge-withdrawer/src/withdrawer/submitter/mod.rs @@ -10,10 +10,10 @@ use astria_core::protocol::transaction::v1alpha1::{ }; use astria_eyre::eyre::{ self, + eyre, Context, }; pub(crate) use builder::Builder; -use prost::Message as _; use sequencer_client::{ tendermint_rpc::endpoint::broadcast::tx_commit, Address, @@ -22,7 +22,6 @@ use sequencer_client::{ }; use signer::SequencerSigner; use state::State; -use tendermint::crypto::Sha256; use tokio::{ select, sync::mpsc, @@ -72,54 +71,12 @@ impl Submitter { } batch = self.batches_rx.recv() => { - let (actions, rollup_height) = match batch { - Some((actions, rollup_height)) => (actions, rollup_height), - None => { - info!("received None from batch channel, shutting down"); - break Err("channel closed"); - } + let Some((actions, rollup_height)) = batch else { + info!("received None from batch channel, shutting down"); + break Err(eyre!("batch channel closed")); }; - - // get nonce and make unsigned transaction - let nonce = get_latest_nonce(self.sequencer_cometbft_client.clone(), self.signer.address, self.state.clone()).await?; - debug!(nonce, "fetched latest nonce"); - - let unsigned = UnsignedTransaction { - actions, - params: TransactionParams { - nonce, - chain_id: self.sequencer_chain_id.clone(), - }, - }; - - // sign transaction - let signed = unsigned.into_signed(&self.signer.signing_key); - debug!(tx_hash = ?hex::encode(sha256(&signed.to_raw().encode_to_vec())), "signed transaction"); - - // submit transaction and handle response - let rsp = submit_tx(self.sequencer_cometbft_client.clone(), signed, self.state.clone()).await.context("failed to submit transaction to to cometbft")?; - - match rsp.check_tx.code { - tendermint::abci::Code::Ok => { - // update state after successful submission - info!( - sequencer.block = rsp.height.value(), - sequencer.tx_hash = ?rsp.hash, - rollup.height = rollup_height, - "withdraw batch successfully executed" - ); - self.state.set_last_rollup_height_submitted(rollup_height); - self.state.set_last_sequencer_height(rsp.height.value()); - self.state.set_last_sequencer_tx_hash(rsp.hash) - } - tendermint::abci::Code::Err(check_tx_code) => { - // handle check_tx failure - error!(abci.code = check_tx_code, - abci.log = rsp.check_tx.log, - rollup.height = rollup_height, - "transaction failed to be included in the mempool, aborting batch submission"); - break Err("check_tx failure upon submitting transaction"); - } + if let Err(e) = self.process_batch(actions, rollup_height).await { + break Err(e); } } ) @@ -140,10 +97,72 @@ impl Submitter { Ok(()) } + + async fn process_batch( + &mut self, + actions: Vec, + rollup_height: u64, + ) -> eyre::Result<()> { + // get nonce and make unsigned transaction + let nonce = get_latest_nonce( + self.sequencer_cometbft_client.clone(), + self.signer.address, + self.state.clone(), + ) + .await?; + debug!(nonce, "fetched latest nonce"); + + let unsigned = UnsignedTransaction { + actions, + params: TransactionParams { + nonce, + chain_id: self.sequencer_chain_id.clone(), + }, + }; + + // sign transaction + let signed = unsigned.into_signed(&self.signer.signing_key); + debug!(tx_hash = %telemetry::display::hex(&signed.sha256_of_proto_encoding()), "signed transaction"); + + // submit transaction and handle response + let rsp = submit_tx( + self.sequencer_cometbft_client.clone(), + signed, + self.state.clone(), + ) + .await + .context("failed to submit transaction to to cometbft")?; + + match rsp.check_tx.code { + tendermint::abci::Code::Ok => { + // update state after successful submission + info!( + sequencer.block = rsp.height.value(), + sequencer.tx_hash = ?rsp.hash, + rollup.height = rollup_height, + "withdraw batch successfully executed" + ); + self.state.set_last_rollup_height_submitted(rollup_height); + self.state.set_last_sequencer_height(rsp.height.value()); + self.state.set_last_sequencer_tx_hash(rsp.hash); + Ok(()) + } + tendermint::abci::Code::Err(check_tx_code) => { + // handle check_tx failure + error!( + abci.code = check_tx_code, + abci.log = rsp.check_tx.log, + rollup.height = rollup_height, + "transaction failed to be included in the mempool, aborting batch submission" + ); + return Err(eyre!("check_tx failure upon submitting transaction")); + } + } + } } /// Queries the sequencer for the latest nonce with an exponential backoff -#[instrument(name = "get latest nonce", skip_all, fields(%address))] +#[instrument(name = "get_latest_nonce", skip_all, fields(%address))] async fn get_latest_nonce( client: sequencer_client::HttpClient, address: Address, @@ -194,13 +213,13 @@ async fn get_latest_nonce( res } -/// Queries the sequencer for the latest nonce with an exponential backoff +/// Submits a `SignedTransaction` to the sequencer with an exponential backoff #[instrument( - name = "submit signed transaction", + name = "submit_tx", skip_all, fields( nonce = tx.unsigned_transaction().params.nonce, - transaction.hash = hex::encode(sha256(&tx.to_raw().encode_to_vec())), + transaction.hash = %telemetry::display::hex(&tx.sha256_of_proto_encoding()), ) )] async fn submit_tx( @@ -210,9 +229,6 @@ async fn submit_tx( ) -> eyre::Result { let nonce = tx.unsigned_transaction().params.nonce; metrics::gauge!(crate::metrics_init::CURRENT_NONCE).set(nonce); - - // TODO: change to info and log tx hash (to match info log in `SubmitFut`'s response handling - // logic) let start = std::time::Instant::now(); debug!("submitting signed transaction to sequencer"); let span = Span::current(); @@ -258,8 +274,3 @@ async fn submit_tx( res } - -fn sha256(data: &[u8]) -> [u8; 32] { - use sha2::Sha256; - Sha256::digest(data) -} From 2b66be11d0ca2c544e170e459251c33fbfc4c6f7 Mon Sep 17 00:00:00 2001 From: itamar Date: Thu, 23 May 2024 19:51:26 -0400 Subject: [PATCH 17/23] address pr comments --- .../local.env.example | 3 + crates/astria-bridge-withdrawer/src/config.rs | 2 + .../src/withdrawer/batch.rs | 67 ++++++++++++++++ .../src/withdrawer/ethereum/watcher.rs | 80 +++++++++---------- .../src/withdrawer/mod.rs | 22 +++-- .../src/withdrawer/submitter/builder.rs | 38 +++++++++ .../src/withdrawer/submitter/mod.rs | 68 +++++++++------- 7 files changed, 203 insertions(+), 77 deletions(-) create mode 100644 crates/astria-bridge-withdrawer/src/withdrawer/batch.rs diff --git a/crates/astria-bridge-withdrawer/local.env.example b/crates/astria-bridge-withdrawer/local.env.example index 1e4959323a..0662395c2e 100644 --- a/crates/astria-bridge-withdrawer/local.env.example +++ b/crates/astria-bridge-withdrawer/local.env.example @@ -31,6 +31,9 @@ ASTRIA_BRIDGE_WITHDRAWER_SEQUENCER_CHAIN_ID="astria-dev-1" # transactions. The file should contain a hex-encoded Ed25519 secret key. ASTRIA_BRIDGE_WITHDRAWER_SEQUENCER_KEY_PATH=/path/to/priv_sequencer_key.json +# The fee_asset_id to use for the bridge account's transactions. +ASTRIA_BRIDGE_WITHDRAWER_FEE_ASSET_ID="nria + ASTRIA_BRIDGE_WITHDRAWER_ETHEREUM_CONTRACT_ADDRESS="0x" ASTRIA_BRIDGE_WITHDRAWER_ETHEREUM_RPC_ENDPOINT="http://127.0.0.1:8545" diff --git a/crates/astria-bridge-withdrawer/src/config.rs b/crates/astria-bridge-withdrawer/src/config.rs index 4dad440695..cf471f9f97 100644 --- a/crates/astria-bridge-withdrawer/src/config.rs +++ b/crates/astria-bridge-withdrawer/src/config.rs @@ -15,6 +15,8 @@ pub struct Config { pub sequencer_chain_id: String, // The path to the private key used to sign transactions submitted to the sequencer. pub sequencer_key_path: String, + // The fee_asset_id to use for the bridge account's transactions. + pub fee_asset_id_str: String, // The address of the AstriaWithdrawer contract on the evm rollup. pub ethereum_contract_address: String, // The rpc endpoint of the evm rollup. diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/batch.rs b/crates/astria-bridge-withdrawer/src/withdrawer/batch.rs new file mode 100644 index 0000000000..5b434e02fe --- /dev/null +++ b/crates/astria-bridge-withdrawer/src/withdrawer/batch.rs @@ -0,0 +1,67 @@ +use astria_core::{ + primitive::v1::asset, + protocol::transaction::v1alpha1::{ + action::BridgeUnlockAction, + Action, + }, +}; +use astria_eyre::eyre; +use ethers::types::{ + TxHash, + U64, +}; + +use super::ethereum::astria_withdrawer::WithdrawalFilter; + +const FEE_ASSET_ID: &str = "fee"; + +#[derive(Debug, PartialEq, Eq)] +pub(crate) struct EventWithMetadata { + pub(crate) event: WithdrawalFilter, + /// The block in which the log was emitted + pub(crate) block_number: U64, + /// The transaction hash in which the log was emitted + pub(crate) transaction_hash: TxHash, +} + +pub(crate) fn event_to_action( + event_with_metadata: EventWithMetadata, + fee_asset_id: asset::Id, +) -> eyre::Result { + let action = BridgeUnlockAction { + to: event_with_metadata.event.sender.to_fixed_bytes().into(), + amount: event_with_metadata.event.amount.as_u128(), + memo: event_with_metadata.event.memo.to_vec(), // TODO: store block number and tx hash here + fee_asset_id, + }; + Ok(Action::BridgeUnlock(action)) +} + +pub(crate) struct Batch { + /// The withdrawal payloads + pub(crate) actions: Vec, + /// The corresponding rollup block height + pub(crate) rollup_height: u64, +} + +#[cfg(test)] +mod tests { + #[test] + fn event_to_bridge_unlock() { + // let event_with_meta = EventWithMetadata { + // event: WithdrawalFilter { + // sender: wallet.address(), + // amount: value, + // memo: b"nootwashere".into(), + // }, + // block_number: receipt.block_number.unwrap(), + // transaction_hash: receipt.transaction_hash, + //}; + // let action = event_to_action(event_with_meta); + // + // assert!(matches!( + // action, + // Action::BridgeUnlock(BridgeLockAction { .. }) + //)) + } +} diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs b/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs index ffc0149d96..5abdf4f613 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs +++ b/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs @@ -1,11 +1,8 @@ use std::sync::Arc; use astria_core::{ - primitive::v1::asset::Id, - protocol::transaction::v1alpha1::{ - action::BridgeUnlockAction, - Action, - }, + primitive::v1::asset, + protocol::transaction::v1alpha1::Action, }; use astria_eyre::{ eyre::{ @@ -39,9 +36,14 @@ use super::astria_withdrawer::{ astria_withdrawer::WithdrawalFilter, AstriaWithdrawer, }; -use crate::withdrawer::state::State; - -const FEE_ASSET_ID: &str = "fee"; +use crate::withdrawer::{ + batch::{ + event_to_action, + Batch, + EventWithMetadata, + }, + state::State, +}; /// Watches for withdrawal events emitted by the `AstriaWithdrawer` contract. pub(crate) struct Watcher { @@ -59,6 +61,7 @@ impl Watcher { batch_tx: mpsc::Sender<(Vec, u64)>, shutdown_token: &CancellationToken, state: Arc, + fee_asset_id: asset::Id, ) -> Result { let provider = Arc::new( Provider::::connect(ethereum_rpc_endpoint) @@ -70,7 +73,9 @@ impl Watcher { let contract = AstriaWithdrawer::new(contract_address, provider); let (event_tx, event_rx) = mpsc::channel(100); - let batcher = Batcher::new(event_rx, batch_tx, shutdown_token); + + // TODO: verify fee_asset_id against sequencer + let batcher = Batcher::new(event_rx, batch_tx, shutdown_token, fee_asset_id); Ok(Self { contract, event_tx, @@ -126,39 +131,35 @@ impl Watcher { } } -#[derive(Debug, PartialEq, Eq)] -pub(crate) struct EventWithMetadata { - event: WithdrawalFilter, - /// The block in which the log was emitted - block_number: U64, - /// The transaction hash in which the log was emitted - transaction_hash: TxHash, -} - struct Batcher { event_rx: mpsc::Receiver<(WithdrawalFilter, LogMeta)>, - batch_tx: mpsc::Sender<(Vec, u64)>, + batch_tx: mpsc::Sender, shutdown_token: CancellationToken, + fee_asset_id: asset::Id, } impl Batcher { pub(crate) fn new( event_rx: mpsc::Receiver<(WithdrawalFilter, LogMeta)>, - batch_tx: mpsc::Sender<(Vec, u64)>, + batch_tx: mpsc::Sender, shutdown_token: &CancellationToken, + fee_asset_id: asset::Id, ) -> Self { Self { event_rx, batch_tx, shutdown_token: shutdown_token.clone(), + fee_asset_id, } } } impl Batcher { pub(crate) async fn run(mut self) -> Result<()> { - let mut actions = Vec::new(); - let mut last_block_number: U64 = 0.into(); + let mut curr_batch = Batch { + actions: Vec::new(), + rollup_height: 0, + }; loop { select! { @@ -173,22 +174,24 @@ impl Batcher { block_number: meta.block_number, transaction_hash: meta.transaction_hash, }; - let action = event_to_action(event_with_metadata)?; + let action = event_to_action(event_with_metadata, self.fee_asset_id)?; - if meta.block_number == last_block_number { + if meta.block_number.into() == curr_batch.rollup_height { // block number was the same; add event to current batch - actions.push(action); + curr_batch.actions.push(action); } else { // block number increased; send current batch and start a new one - if !actions.is_empty() { + if !curr_batch.actions.is_empty() { self.batch_tx - .send((actions, last_block_number.as_u64())) + .send(curr_batch) .await .wrap_err("failed to send batched events; receiver dropped?")?; } - actions = vec![action]; - last_block_number = meta.block_number; + curr_batch = Batch { + actions: vec![action], + rollup_height = meta.block_number.into() + }; } } } @@ -210,16 +213,6 @@ fn address_from_string(s: &str) -> Result { Ok(address.into()) } -fn event_to_action(event_with_metadata: EventWithMetadata) -> eyre::Result { - let action = BridgeUnlockAction { - to: event_with_metadata.event.sender.to_fixed_bytes().into(), - amount: event_with_metadata.event.amount.as_u128(), - memo: event_with_metadata.event.memo.to_vec(), // TODO: store block number and tx hash here - fee_asset_id: Id::from_denom(FEE_ASSET_ID), - }; - Ok(Action::BridgeUnlock(action)) -} - #[cfg(test)] mod tests { use ethers::{ @@ -234,7 +227,10 @@ mod tests { }; use super::*; - use crate::withdrawer::ethereum::test_utils::deploy_astria_withdrawer; + use crate::withdrawer::{ + batch::EventWithMetadata, + ethereum::test_utils::deploy_astria_withdrawer, + }; #[test] fn address_from_string_prefix() { @@ -294,7 +290,8 @@ mod tests { block_number: receipt.block_number.unwrap(), transaction_hash: receipt.transaction_hash, }; - let expected_action = event_to_action(expected_event).unwrap(); + let expected_action = + event_to_action(expected_event, asset::Id::from_denom("nria")).unwrap(); let Action::BridgeUnlock(expected_action) = expected_action else { panic!( "expected action to be BridgeUnlock, got {:?}", @@ -309,6 +306,7 @@ mod tests { event_tx, &CancellationToken::new(), Arc::new(State::new()), + asset::Id::from_denom("nria"), ) .await .unwrap(); diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/mod.rs b/crates/astria-bridge-withdrawer/src/withdrawer/mod.rs index 2def72b22f..ec0b3d5350 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/mod.rs +++ b/crates/astria-bridge-withdrawer/src/withdrawer/mod.rs @@ -34,6 +34,7 @@ use crate::{ config::Config, }; +mod batch; mod ethereum; mod state; mod submitter; @@ -56,7 +57,14 @@ impl WithdrawerService { pub async fn new(cfg: Config) -> eyre::Result<(Self, ShutdownHandle)> { let shutdown_handle = ShutdownHandle::new(); let Config { - api_addr, .. + api_addr, + cometbft_endpoint, + sequencer_chain_id, + sequencer_key_path, + fee_asset_id_str, + ethereum_contract_address, + ethereum_rpc_endpoint, + .. } = cfg; let state = Arc::new(State::new()); @@ -64,21 +72,21 @@ impl WithdrawerService { // make submitter object let (submitter, submitter_handle) = submitter::Builder { shutdown_token: shutdown_handle.token(), - cometbft_endpoint: cfg.cometbft_endpoint, - sequencer_chain_id: cfg.sequencer_chain_id, - sequencer_key_path: cfg.sequencer_key_path, + cometbft_endpoint, + sequencer_chain_id, + sequencer_key_path, state: state.clone(), } .build() .wrap_err("failed to initialize submitter")?; - // TODO: use event_rx in the sequencer submitter let ethereum_watcher = Watcher::new( - &cfg.ethereum_contract_address, - &cfg.ethereum_rpc_endpoint, + ðereum_contract_address, + ðereum_rpc_endpoint, submitter_handle.batches_tx, &shutdown_handle.token(), state.clone(), + asset::Id::from_denom(fee_asset_id_str), ) .await .wrap_err("failed to initialize ethereum watcher")?; diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/submitter/builder.rs b/crates/astria-bridge-withdrawer/src/withdrawer/submitter/builder.rs index 51bded9efb..b21cfeb5d2 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/submitter/builder.rs +++ b/crates/astria-bridge-withdrawer/src/withdrawer/submitter/builder.rs @@ -2,6 +2,7 @@ use std::sync::Arc; use astria_eyre::eyre::{ self, + ensure, Context as _, }; use tokio_util::sync::CancellationToken; @@ -35,6 +36,12 @@ impl Builder { let sequencer_cometbft_client = sequencer_client::HttpClient::new(&*cometbft_endpoint) .context("failed constructing cometbft http client")?; + let actual_chain_id = get_sequencer_chain_id(sequencer_cometbft_client).await?; + ensure!( + sequencer_chain_id == actual_chain_id, + "sequencer_chain_id provided in config does not match chain_id returned from sequencer" + ); + Ok(( super::Submitter { shutdown_token, @@ -50,3 +57,34 @@ impl Builder { )) } } + +async fn get_sequencer_chain_id( + client: sequencer_client::HttpClient, +) -> eyre::Result { + use sequencer_client::Client as _; + + let retry_config = tryhard::RetryFutureConfig::new(u32::MAX) + .exponential_backoff(Duration::from_millis(100)) + .max_delay(Duration::from_secs(20)) + .on_retry( + |attempt: u32, next_delay: Option, error: &tendermint_rpc::Error| { + let wait_duration = next_delay + .map(humantime::format_duration) + .map(tracing::field::display); + warn!( + attempt, + wait_duration, + error = error as &dyn std::error::Error, + "attempt to fetch sequencer genesis info; retrying after backoff", + ); + futures::future::ready(()) + }, + ); + + let genesis: tendermint::Genesis = tryhard::retry_fn(|| client.genesis()) + .with_config(retry_config) + .await + .wrap_err("failed to get genesis info from Sequencer after a lot of attempts")?; + + Ok(genesis.chain_id) +} diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/submitter/mod.rs b/crates/astria-bridge-withdrawer/src/withdrawer/submitter/mod.rs index db8b856e35..eb705279c0 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/submitter/mod.rs +++ b/crates/astria-bridge-withdrawer/src/withdrawer/submitter/mod.rs @@ -39,19 +39,22 @@ use tracing::{ Span, }; -use super::state; +use super::{ + batch::Batch, + state, +}; mod builder; mod signer; pub(super) struct Handle { - pub(super) batches_tx: mpsc::Sender<(Vec, u64)>, + pub(super) batches_tx: mpsc::Sender, } pub(super) struct Submitter { shutdown_token: CancellationToken, state: Arc, - batches_rx: mpsc::Receiver<(Vec, u64)>, + batches_rx: mpsc::Receiver, sequencer_cometbft_client: sequencer_client::HttpClient, signer: SequencerSigner, sequencer_chain_id: String, @@ -71,7 +74,7 @@ impl Submitter { } batch = self.batches_rx.recv() => { - let Some((actions, rollup_height)) = batch else { + let Some(Batch { actions, rollup_height }) = batch else { info!("received None from batch channel, shutting down"); break Err(eyre!("batch channel closed")); }; @@ -132,31 +135,38 @@ impl Submitter { ) .await .context("failed to submit transaction to to cometbft")?; - - match rsp.check_tx.code { - tendermint::abci::Code::Ok => { - // update state after successful submission - info!( - sequencer.block = rsp.height.value(), - sequencer.tx_hash = ?rsp.hash, - rollup.height = rollup_height, - "withdraw batch successfully executed" - ); - self.state.set_last_rollup_height_submitted(rollup_height); - self.state.set_last_sequencer_height(rsp.height.value()); - self.state.set_last_sequencer_tx_hash(rsp.hash); - Ok(()) - } - tendermint::abci::Code::Err(check_tx_code) => { - // handle check_tx failure - error!( - abci.code = check_tx_code, - abci.log = rsp.check_tx.log, - rollup.height = rollup_height, - "transaction failed to be included in the mempool, aborting batch submission" - ); - return Err(eyre!("check_tx failure upon submitting transaction")); - } + if let tendermint::abci::Code::Err(check_tx_code) = rsp.check_tx.code { + error!( + abci.code = check_tx_code, + abci.log = rsp.check_tx.log, + rollup.height = rollup_height, + "transaction failed to be included in the mempool, aborting." + ); + Err(eyre!( + "check_tx failure upon submitting transaction to sequencer" + )) + } else if let tendermint::abci::Code::Err(deliver_tx_code) = rsp.tx_result.code { + error!( + abci.code = deliver_tx_code, + abci.log = rsp.tx_result.log, + rollup.height = rollup_height, + "transaction failed to be executed in a block, aborting." + ); + Err(eyre!( + "deliver_tx failure upon submitting transaction to sequencer" + )) + } else { + // update state after successful submission + info!( + sequencer.block = rsp.height.value(), + sequencer.tx_hash = ?rsp.hash, + rollup.height = rollup_height, + "withdraw batch successfully executed." + ); + self.state.set_last_rollup_height_submitted(rollup_height); + self.state.set_last_sequencer_height(rsp.height.value()); + self.state.set_last_sequencer_tx_hash(rsp.hash); + Ok(()) } } } From 6afbce92db27393d50b287df56a4ca12a69a7d14 Mon Sep 17 00:00:00 2001 From: elizabeth Date: Thu, 23 May 2024 20:41:05 -0400 Subject: [PATCH 18/23] event conversion to action --- Cargo.lock | 1 + crates/astria-bridge-withdrawer/Cargo.toml | 1 + .../src/withdrawer/batch.rs | 69 ++++++++++++++++--- .../src/withdrawer/ethereum/watcher.rs | 35 +++++----- .../protocol/transaction/v1alpha1/action.rs | 18 ++--- 5 files changed, 85 insertions(+), 39 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d37fa2900e..c7b5f633b5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -512,6 +512,7 @@ dependencies = [ "http", "humantime", "hyper", + "ibc-types", "metrics", "pin-project-lite", "prost", diff --git a/crates/astria-bridge-withdrawer/Cargo.toml b/crates/astria-bridge-withdrawer/Cargo.toml index bfde7151fa..1612bbc2e1 100644 --- a/crates/astria-bridge-withdrawer/Cargo.toml +++ b/crates/astria-bridge-withdrawer/Cargo.toml @@ -21,6 +21,7 @@ hex = { workspace = true } ethers = { workspace = true, features = ["ethers-solc", "ws"] } hyper = { workspace = true } humantime = { workspace = true } +ibc-types = { workspace = true } metrics = { workspace = true } pin-project-lite = { workspace = true } prost = { workspace = true } diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/batch.rs b/crates/astria-bridge-withdrawer/src/withdrawer/batch.rs index 05e659baac..68c3a4bb2e 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/batch.rs +++ b/crates/astria-bridge-withdrawer/src/withdrawer/batch.rs @@ -1,5 +1,9 @@ use astria_core::{ - primitive::v1::asset, + primitive::v1::{ + asset, + asset::Denom, + Address, + }, protocol::transaction::v1alpha1::{ action::{ BridgeUnlockAction, @@ -8,9 +12,10 @@ use astria_core::{ Action, }, }; -use astria_eyre::{ - eyre, - eyre::WrapErr as _, +use astria_eyre::eyre::{ + self, + OptionExt, + WrapErr as _, }; use ethers::types::{ TxHash, @@ -20,14 +25,13 @@ use serde::{ Deserialize, Serialize, }; +use tendermint::block::Height; use super::ethereum::astria_withdrawer::{ Ics20WithdrawalFilter, SequencerWithdrawalFilter, }; -// const FEE_ASSET_ID: &str = "fee"; - #[derive(Debug, PartialEq, Eq)] pub(crate) enum WithdrawalEvent { Sequencer(SequencerWithdrawalFilter), @@ -36,11 +40,11 @@ pub(crate) enum WithdrawalEvent { #[derive(Debug, PartialEq, Eq)] pub(crate) struct EventWithMetadata { - event: WithdrawalEvent, + pub(crate) event: WithdrawalEvent, /// The block in which the log was emitted - block_number: U64, + pub(crate) block_number: U64, /// The transaction hash in which the log was emitted - transaction_hash: TxHash, + pub(crate) transaction_hash: TxHash, } pub(crate) fn event_to_action( @@ -85,22 +89,65 @@ fn event_to_bridge_unlock( let action = BridgeUnlockAction { to: event.sender.to_fixed_bytes().into(), amount: event.amount.as_u128(), - memo: serde_json::to_vec(&memo)?, + memo: serde_json::to_vec(&memo).wrap_err("failed to serialize memo to json")?, fee_asset_id, }; Ok(Action::BridgeUnlock(action)) } +#[derive(Debug, Serialize, Deserialize)] +struct Ics20WithdrawalMemo { + memo: String, + block_number: U64, + transaction_hash: TxHash, +} + fn event_to_ics20_withdrawal( event: Ics20WithdrawalFilter, block_number: U64, transaction_hash: TxHash, fee_asset_id: asset::Id, ) -> eyre::Result { + use ibc_types::core::client::Height as IbcHeight; + + let sender: [u8; 20] = event + .sender + .as_bytes() + .try_into() + .expect("U160 must be 20 bytes"); + let rollup_asset_denom = "transfer/channel-0/utia".to_string(); // TODO: this should be fetched from the sequencer + let denom = Denom::from(rollup_asset_denom.clone()); + let (_, channel) = denom + .prefix() + .rsplit_once('/') + .ok_or_eyre("denom must have a channel to be withdrawn via IBC")?; + + let memo = Ics20WithdrawalMemo { + memo: String::from_utf8(event.memo.to_vec()) + .wrap_err("failed to convert event memo to utf8")?, + block_number, + transaction_hash, + }; + let action = Ics20Withdrawal { + denom: Denom::from(rollup_asset_denom), + destination_chain_address: event.destination_chain_address, + // note: this is actually a rollup address; we expect failed ics20 withdrawals to be + // returned to the rollup. + // this is only ok for now because addresses on the sequencer and the rollup are both 20 + // bytes, but this won't work otherwise. + return_address: Address::from(sender), amount: event.amount.as_u128(), - memo: event.memo.to_vec(), // TODO: + memo: serde_json::to_string(&memo).wrap_err("failed to serialize memo to json")?, fee_asset_id, + // note: this refers to the timeout on the destination chain, which we are unaware of. + // thus, we set it to the maximum possible value. + timeout_height: IbcHeight::new(u64::MAX, u64::MAX) + .wrap_err("failed to generate timeout height")?, + timeout_time: u64::MAX, + source_channel: channel + .parse() + .wrap_err("failed to parse channel from denom")?, }; Ok(Action::Ics20Withdrawal(action)) } diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs b/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs index e63a1a88c8..2e42469f9d 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs +++ b/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs @@ -32,24 +32,23 @@ use tokio::{ use tokio_util::sync::CancellationToken; use tracing::info; -use crate::{ - ethereum::{ - astria_withdrawer::{ - astria_withdrawer::{ - Ics20WithdrawalFilter, - SequencerWithdrawalFilter, - }, - AstriaWithdrawer, - }, - state::{ - State, - StateSnapshot, - }, - }, - withdrawer::batch::{ +use crate::withdrawer::{ + batch::{ event_to_action, Batch, EventWithMetadata, + WithdrawalEvent, + }, + ethereum::astria_withdrawer::{ + astria_withdrawer::{ + Ics20WithdrawalFilter, + SequencerWithdrawalFilter, + }, + AstriaWithdrawer, + }, + state::{ + State, + StateSnapshot, }, }; @@ -106,8 +105,6 @@ impl Watcher { tokio::task::spawn(batcher.run()); - state.set_ready(); - // start from block 1 right now // TODO: determine the last block we've seen based on the sequencer data let sequencer_withdrawal_event_handler = tokio::task::spawn( @@ -233,7 +230,7 @@ impl Batcher { }; let action = event_to_action(event_with_metadata, self.fee_asset_id)?; - if meta.block_number.into() == curr_batch.rollup_height { + if meta.block_number.as_u64() == curr_batch.rollup_height { // block number was the same; add event to current batch curr_batch.actions.push(action); } else { @@ -247,7 +244,7 @@ impl Batcher { curr_batch = Batch { actions: vec![action], - rollup_height = meta.block_number.into() + rollup_height: meta.block_number.as_u64(), }; } } diff --git a/crates/astria-core/src/protocol/transaction/v1alpha1/action.rs b/crates/astria-core/src/protocol/transaction/v1alpha1/action.rs index 2d8da8e24e..60cbadbf99 100644 --- a/crates/astria-core/src/protocol/transaction/v1alpha1/action.rs +++ b/crates/astria-core/src/protocol/transaction/v1alpha1/action.rs @@ -697,23 +697,23 @@ enum MintActionErrorKind { #[derive(Debug, Clone)] pub struct Ics20Withdrawal { // a transparent value consisting of an amount and a denom. - amount: u128, - denom: Denom, + pub amount: u128, + pub denom: Denom, // the address on the destination chain to send the transfer to. - destination_chain_address: String, + pub destination_chain_address: String, // an Astria address to use to return funds from this withdrawal // in the case it fails. - return_address: Address, + pub return_address: Address, // the height (on Astria) at which this transfer expires. - timeout_height: IbcHeight, + pub timeout_height: IbcHeight, // the unix timestamp (in nanoseconds) at which this transfer expires. - timeout_time: u64, + pub timeout_time: u64, // the source channel used for the withdrawal. - source_channel: ChannelId, + pub source_channel: ChannelId, // the asset to use for fee payment. - fee_asset_id: asset::Id, + pub fee_asset_id: asset::Id, // a memo to include with the transfer - memo: String, + pub memo: String, } impl Ics20Withdrawal { From 5c2afdd6327fdc1499a9c82aef6c9d0dec6c0f54 Mon Sep 17 00:00:00 2001 From: elizabeth Date: Thu, 23 May 2024 20:45:48 -0400 Subject: [PATCH 19/23] fix tests and lint --- .../src/withdrawer/batch.rs | 1 - .../src/withdrawer/ethereum/watcher.rs | 48 +++++++++---------- crates/astria-core/src/primitive/v1/asset.rs | 2 +- .../protocol/transaction/v1alpha1/action.rs | 2 +- 4 files changed, 25 insertions(+), 28 deletions(-) diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/batch.rs b/crates/astria-bridge-withdrawer/src/withdrawer/batch.rs index 68c3a4bb2e..694f104d19 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/batch.rs +++ b/crates/astria-bridge-withdrawer/src/withdrawer/batch.rs @@ -25,7 +25,6 @@ use serde::{ Deserialize, Serialize, }; -use tendermint::block::Height; use super::ethereum::astria_withdrawer::{ Ics20WithdrawalFilter, diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs b/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs index 2e42469f9d..e01ac5818b 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs +++ b/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs @@ -1,12 +1,8 @@ use std::sync::Arc; -use astria_core::{ - primitive::v1::asset, - protocol::transaction::v1alpha1::Action, -}; +use astria_core::primitive::v1::asset; use astria_eyre::{ eyre::{ - self, eyre, WrapErr as _, }, @@ -19,10 +15,6 @@ use ethers::{ StreamExt as _, Ws, }, - types::{ - TxHash, - U64, - }, utils::hex, }; use tokio::{ @@ -39,17 +31,8 @@ use crate::withdrawer::{ EventWithMetadata, WithdrawalEvent, }, - ethereum::astria_withdrawer::{ - astria_withdrawer::{ - Ics20WithdrawalFilter, - SequencerWithdrawalFilter, - }, - AstriaWithdrawer, - }, - state::{ - State, - StateSnapshot, - }, + ethereum::astria_withdrawer::AstriaWithdrawer, + state::State, }; /// Watches for withdrawal events emitted by the `AstriaWithdrawer` contract. @@ -116,6 +99,8 @@ impl Watcher { 1, )); + state.set_watcher_ready(); + tokio::select! { res = sequencer_withdrawal_event_handler => { info!("sequencer withdrawal event handler exited"); @@ -269,6 +254,7 @@ fn address_from_string(s: &str) -> Result { #[cfg(test)] mod tests { + use astria_core::protocol::transaction::v1alpha1::Action; use ethers::{ prelude::SignerMiddleware, providers::Middleware, @@ -283,7 +269,13 @@ mod tests { use super::*; use crate::withdrawer::{ batch::EventWithMetadata, - ethereum::test_utils::deploy_astria_withdrawer, + ethereum::{ + astria_withdrawer::{ + Ics20WithdrawalFilter, + SequencerWithdrawalFilter, + }, + test_utils::deploy_astria_withdrawer, + }, }; #[test] @@ -372,9 +364,15 @@ mod tests { // make another tx to trigger anvil to make another block send_sequencer_withdraw_transaction(&contract, value, recipient).await; - let events = event_rx.recv().await.unwrap(); - assert_eq!(events.len(), 1); - assert_eq!(events[0], expected_event); + let batch = event_rx.recv().await.unwrap(); + assert_eq!(batch.actions.len(), 1); + let Action::BridgeUnlock(action) = &batch.actions[0] else { + panic!( + "expected action to be BridgeUnlock, got {:?}", + batch.actions[0] + ); + }; + assert_eq!(action, &expected_action); } async fn send_ics20_withdraw_transaction( @@ -432,7 +430,7 @@ mod tests { &anvil.ws_endpoint(), event_tx, &CancellationToken::new(), - State::new(), + Arc::new(State::new()), asset::Id::from_denom("nria"), ) .await diff --git a/crates/astria-core/src/primitive/v1/asset.rs b/crates/astria-core/src/primitive/v1/asset.rs index 2525971f2c..3415d8ec84 100644 --- a/crates/astria-core/src/primitive/v1/asset.rs +++ b/crates/astria-core/src/primitive/v1/asset.rs @@ -23,7 +23,7 @@ pub fn default_native_asset_id() -> Id { /// Note that the full denomination trace of the token is `prefix/base_denom`, /// in the case that a prefix is present. /// This is hashed to create the ID. -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone, PartialEq, Eq)] pub struct Denom { id: Id, diff --git a/crates/astria-core/src/protocol/transaction/v1alpha1/action.rs b/crates/astria-core/src/protocol/transaction/v1alpha1/action.rs index 60cbadbf99..d30e416ab3 100644 --- a/crates/astria-core/src/protocol/transaction/v1alpha1/action.rs +++ b/crates/astria-core/src/protocol/transaction/v1alpha1/action.rs @@ -694,7 +694,7 @@ enum MintActionErrorKind { /// /// It also contains a `return_address` field which may or may not be the same as the signer /// of the packet. The funds will be returned to the `return_address` in the case of a timeout. -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq, Eq)] pub struct Ics20Withdrawal { // a transparent value consisting of an amount and a denom. pub amount: u128, From d5bf4212b700dd645e646efe6c7a3d027a8c26a2 Mon Sep 17 00:00:00 2001 From: elizabeth Date: Thu, 23 May 2024 20:57:35 -0400 Subject: [PATCH 20/23] add rollup_asset_denom to config --- .../local.env.example | 5 +++ crates/astria-bridge-withdrawer/src/config.rs | 2 + .../src/withdrawer/batch.rs | 4 +- .../src/withdrawer/ethereum/watcher.rs | 44 +++++++++++++++---- .../src/withdrawer/mod.rs | 1 + 5 files changed, 46 insertions(+), 10 deletions(-) diff --git a/crates/astria-bridge-withdrawer/local.env.example b/crates/astria-bridge-withdrawer/local.env.example index 1d8c7fe384..4e56523a34 100644 --- a/crates/astria-bridge-withdrawer/local.env.example +++ b/crates/astria-bridge-withdrawer/local.env.example @@ -34,8 +34,13 @@ ASTRIA_BRIDGE_WITHDRAWER_SEQUENCER_KEY_PATH=/path/to/priv_sequencer_key.json # The fee asset denomination to use for the bridge account's transactions. ASTRIA_BRIDGE_WITHDRAWER_FEE_ASSET_DENOMINATION="nria" +# The asset denomination being withdrawn from the rollup. +ASTRIA_BRIDGE_ROLLUP_ASSET_DENOMINATION="nria", + +# The address of the AstriaWithdrawer contract on the evm rollup. ASTRIA_BRIDGE_WITHDRAWER_ETHEREUM_CONTRACT_ADDRESS="0x" +# The rpc endpoint of the evm rollup. ASTRIA_BRIDGE_WITHDRAWER_ETHEREUM_RPC_ENDPOINT="http://127.0.0.1:8545" # The socket address at which the bridge service will server healthz, readyz, and status calls. diff --git a/crates/astria-bridge-withdrawer/src/config.rs b/crates/astria-bridge-withdrawer/src/config.rs index 2ea62f6472..dd5462edea 100644 --- a/crates/astria-bridge-withdrawer/src/config.rs +++ b/crates/astria-bridge-withdrawer/src/config.rs @@ -17,6 +17,8 @@ pub struct Config { pub sequencer_key_path: String, // The fee asset denomination to use for the bridge account's transactions. pub fee_asset_denomination: String, + // The asset denomination being withdrawn from the rollup. + pub rollup_asset_denomination: String, // The address of the AstriaWithdrawer contract on the evm rollup. pub ethereum_contract_address: String, // The rpc endpoint of the evm rollup. diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/batch.rs b/crates/astria-bridge-withdrawer/src/withdrawer/batch.rs index 694f104d19..d58751fbf7 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/batch.rs +++ b/crates/astria-bridge-withdrawer/src/withdrawer/batch.rs @@ -49,6 +49,7 @@ pub(crate) struct EventWithMetadata { pub(crate) fn event_to_action( event_with_metadata: EventWithMetadata, fee_asset_id: asset::Id, + rollup_asset_denom: Denom, ) -> eyre::Result { let action = match event_with_metadata.event { WithdrawalEvent::Sequencer(event) => event_to_bridge_unlock( @@ -63,6 +64,7 @@ pub(crate) fn event_to_action( event_with_metadata.block_number, event_with_metadata.transaction_hash, fee_asset_id, + rollup_asset_denom, ) .wrap_err("failed to convert ics20 withdrawal event to action")?, }; @@ -106,6 +108,7 @@ fn event_to_ics20_withdrawal( block_number: U64, transaction_hash: TxHash, fee_asset_id: asset::Id, + rollup_asset_denom: Denom, ) -> eyre::Result { use ibc_types::core::client::Height as IbcHeight; @@ -114,7 +117,6 @@ fn event_to_ics20_withdrawal( .as_bytes() .try_into() .expect("U160 must be 20 bytes"); - let rollup_asset_denom = "transfer/channel-0/utia".to_string(); // TODO: this should be fetched from the sequencer let denom = Denom::from(rollup_asset_denom.clone()); let (_, channel) = denom .prefix() diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs b/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs index e01ac5818b..7057c2aa16 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs +++ b/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs @@ -1,6 +1,9 @@ use std::sync::Arc; -use astria_core::primitive::v1::asset; +use astria_core::primitive::v1::{ + asset, + asset::Denom, +}; use astria_eyre::{ eyre::{ eyre, @@ -22,7 +25,10 @@ use tokio::{ sync::mpsc, }; use tokio_util::sync::CancellationToken; -use tracing::info; +use tracing::{ + info, + warn, +}; use crate::withdrawer::{ batch::{ @@ -52,6 +58,7 @@ impl Watcher { shutdown_token: &CancellationToken, state: Arc, fee_asset_id: asset::Id, + rollup_asset_denom: Denom, ) -> Result { let provider = Arc::new( Provider::::connect(ethereum_rpc_endpoint) @@ -64,8 +71,21 @@ impl Watcher { let (event_tx, event_rx) = mpsc::channel(100); + if rollup_asset_denom.prefix().is_empty() { + warn!( + "rollup asset denomination is not prefixed; Ics20Withdrawal actions will not be \ + submitted" + ); + } + // TODO: verify fee_asset_id against sequencer - let batcher = Batcher::new(event_rx, batch_tx, shutdown_token, fee_asset_id); + let batcher = Batcher::new( + event_rx, + batch_tx, + shutdown_token, + fee_asset_id, + rollup_asset_denom, + ); Ok(Self { contract, event_tx, @@ -175,6 +195,7 @@ struct Batcher { batch_tx: mpsc::Sender, shutdown_token: CancellationToken, fee_asset_id: asset::Id, + rollup_asset_denom: Denom, } impl Batcher { @@ -183,12 +204,14 @@ impl Batcher { batch_tx: mpsc::Sender, shutdown_token: &CancellationToken, fee_asset_id: asset::Id, + rollup_asset_denom: Denom, ) -> Self { Self { event_rx, batch_tx, shutdown_token: shutdown_token.clone(), fee_asset_id, + rollup_asset_denom, } } } @@ -213,7 +236,7 @@ impl Batcher { block_number: meta.block_number, transaction_hash: meta.transaction_hash, }; - let action = event_to_action(event_with_metadata, self.fee_asset_id)?; + let action = event_to_action(event_with_metadata, self.fee_asset_id, self.rollup_asset_denom.clone())?; if meta.block_number.as_u64() == curr_batch.rollup_height { // block number was the same; add event to current batch @@ -338,8 +361,8 @@ mod tests { block_number: receipt.block_number.unwrap(), transaction_hash: receipt.transaction_hash, }; - let expected_action = - event_to_action(expected_event, asset::Id::from_denom("nria")).unwrap(); + let denom: Denom = Denom::from_base_denom("nria"); + let expected_action = event_to_action(expected_event, denom.id(), denom.clone()).unwrap(); let Action::BridgeUnlock(expected_action) = expected_action else { panic!( "expected action to be BridgeUnlock, got {:?}", @@ -354,7 +377,8 @@ mod tests { event_tx, &CancellationToken::new(), Arc::new(State::new()), - asset::Id::from_denom("nria"), + denom.id(), + denom, ) .await .unwrap(); @@ -418,8 +442,9 @@ mod tests { block_number: receipt.block_number.unwrap(), transaction_hash: receipt.transaction_hash, }; + let denom = Denom::from_base_denom("transfer/channel-0/utia"); let Action::Ics20Withdrawal(expected_action) = - event_to_action(expected_event, asset::Id::from_denom("nria")).unwrap() + event_to_action(expected_event, denom.id(), denom.clone()).unwrap() else { panic!("expected action to be Ics20Withdrawal",); }; @@ -431,7 +456,8 @@ mod tests { event_tx, &CancellationToken::new(), Arc::new(State::new()), - asset::Id::from_denom("nria"), + denom.id(), + denom, ) .await .unwrap(); diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/mod.rs b/crates/astria-bridge-withdrawer/src/withdrawer/mod.rs index 43498f78da..ac9ef623b5 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/mod.rs +++ b/crates/astria-bridge-withdrawer/src/withdrawer/mod.rs @@ -89,6 +89,7 @@ impl WithdrawerService { &shutdown_handle.token(), state.clone(), asset::Id::from_denom(&fee_asset_denomination), + asset::Denom::from(cfg.rollup_asset_denomination), ) .await .wrap_err("failed to initialize ethereum watcher")?; From a47aa86ba436b1717d51e8e4e5fa779bf0d4962f Mon Sep 17 00:00:00 2001 From: elizabeth Date: Thu, 23 May 2024 21:02:13 -0400 Subject: [PATCH 21/23] fix unit tests --- crates/astria-bridge-withdrawer/local.env.example | 2 +- crates/astria-bridge-withdrawer/src/withdrawer/batch.rs | 3 ++- .../src/withdrawer/ethereum/watcher.rs | 4 ++-- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/crates/astria-bridge-withdrawer/local.env.example b/crates/astria-bridge-withdrawer/local.env.example index 4e56523a34..becc6aaf49 100644 --- a/crates/astria-bridge-withdrawer/local.env.example +++ b/crates/astria-bridge-withdrawer/local.env.example @@ -35,7 +35,7 @@ ASTRIA_BRIDGE_WITHDRAWER_SEQUENCER_KEY_PATH=/path/to/priv_sequencer_key.json ASTRIA_BRIDGE_WITHDRAWER_FEE_ASSET_DENOMINATION="nria" # The asset denomination being withdrawn from the rollup. -ASTRIA_BRIDGE_ROLLUP_ASSET_DENOMINATION="nria", +ASTRIA_BRIDGE_WITHDRAWER_ROLLUP_ASSET_DENOMINATION="nria", # The address of the AstriaWithdrawer contract on the evm rollup. ASTRIA_BRIDGE_WITHDRAWER_ETHEREUM_CONTRACT_ADDRESS="0x" diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/batch.rs b/crates/astria-bridge-withdrawer/src/withdrawer/batch.rs index d58751fbf7..c23b2e5804 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/batch.rs +++ b/crates/astria-bridge-withdrawer/src/withdrawer/batch.rs @@ -117,7 +117,8 @@ fn event_to_ics20_withdrawal( .as_bytes() .try_into() .expect("U160 must be 20 bytes"); - let denom = Denom::from(rollup_asset_denom.clone()); + let denom = rollup_asset_denom.clone(); + let (_, channel) = denom .prefix() .rsplit_once('/') diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs b/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs index 7057c2aa16..c3642c2406 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs +++ b/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs @@ -442,11 +442,11 @@ mod tests { block_number: receipt.block_number.unwrap(), transaction_hash: receipt.transaction_hash, }; - let denom = Denom::from_base_denom("transfer/channel-0/utia"); + let denom = Denom::from("transfer/channel-0/utia".to_string()); let Action::Ics20Withdrawal(expected_action) = event_to_action(expected_event, denom.id(), denom.clone()).unwrap() else { - panic!("expected action to be Ics20Withdrawal",); + panic!("expected action to be Ics20Withdrawal"); }; let (event_tx, mut event_rx) = mpsc::channel(100); From fc1307bfe0368353d6f2636e61a8538dc5ecc2bb Mon Sep 17 00:00:00 2001 From: elizabeth Date: Thu, 23 May 2024 21:24:55 -0400 Subject: [PATCH 22/23] more unit tests --- .../src/withdrawer/batch.rs | 106 +++++++++++++++--- .../src/withdrawer/ethereum/watcher.rs | 8 +- 2 files changed, 94 insertions(+), 20 deletions(-) diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/batch.rs b/crates/astria-bridge-withdrawer/src/withdrawer/batch.rs index c23b2e5804..28cb26ced9 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/batch.rs +++ b/crates/astria-bridge-withdrawer/src/withdrawer/batch.rs @@ -1,3 +1,5 @@ +use std::time::Duration; + use astria_core::{ primitive::v1::{ asset, @@ -21,6 +23,7 @@ use ethers::types::{ TxHash, U64, }; +use ibc_types::core::client::Height as IbcHeight; use serde::{ Deserialize, Serialize, @@ -110,7 +113,8 @@ fn event_to_ics20_withdrawal( fee_asset_id: asset::Id, rollup_asset_denom: Denom, ) -> eyre::Result { - use ibc_types::core::client::Height as IbcHeight; + // TODO: make this configurable + const ICS20_WITHDRAWAL_TIMEOUT: Duration = Duration::from_secs(300); let sender: [u8; 20] = event .sender @@ -146,7 +150,8 @@ fn event_to_ics20_withdrawal( // thus, we set it to the maximum possible value. timeout_height: IbcHeight::new(u64::MAX, u64::MAX) .wrap_err("failed to generate timeout height")?, - timeout_time: u64::MAX, + timeout_time: calculate_packet_timeout_time(ICS20_WITHDRAWAL_TIMEOUT) + .wrap_err("failed to calculate packet timeout time")?, source_channel: channel .parse() .wrap_err("failed to parse channel from denom")?, @@ -154,6 +159,15 @@ fn event_to_ics20_withdrawal( Ok(Action::Ics20Withdrawal(action)) } +fn calculate_packet_timeout_time(timeout_delta: Duration) -> eyre::Result { + tendermint::Time::now() + .checked_add(timeout_delta) + .ok_or_eyre("time must not overflow from now plus 10 minutes")? + .unix_timestamp_nanos() + .try_into() + .wrap_err("failed to convert packet timeout i128 to u64") +} + pub(crate) struct Batch { /// The withdrawal payloads pub(crate) actions: Vec, @@ -163,22 +177,80 @@ pub(crate) struct Batch { #[cfg(test)] mod tests { + use super::*; + use crate::withdrawer::ethereum::astria_withdrawer::SequencerWithdrawalFilter; + #[test] fn event_to_bridge_unlock() { - // let event_with_meta = EventWithMetadata { - // event: WithdrawalFilter { - // sender: wallet.address(), - // amount: value, - // memo: b"nootwashere".into(), - // }, - // block_number: receipt.block_number.unwrap(), - // transaction_hash: receipt.transaction_hash, - //}; - // let action = event_to_action(event_with_meta); - // - // assert!(matches!( - // action, - // Action::BridgeUnlock(BridgeLockAction { .. }) - //)) + let denom = Denom::from("nria".to_string()); + let event_with_meta = EventWithMetadata { + event: WithdrawalEvent::Sequencer(SequencerWithdrawalFilter { + sender: [0u8; 20].into(), + amount: 99.into(), + destination_chain_address: [1u8; 20].into(), + }), + block_number: 1.into(), + transaction_hash: [2u8; 32].into(), + }; + let action = event_to_action(event_with_meta, denom.id(), denom.clone()).unwrap(); + let Action::BridgeUnlock(action) = action else { + panic!("expected BridgeUnlock action, got {:?}", action); + }; + + let expected_action = BridgeUnlockAction { + to: [0u8; 20].into(), + amount: 99, + memo: serde_json::to_vec(&BridgeUnlockMemo { + block_number: 1.into(), + transaction_hash: [2u8; 32].into(), + }) + .unwrap(), + fee_asset_id: denom.id(), + }; + + assert_eq!(action, expected_action); + } + + #[test] + fn event_to_ics20_withdrawal() { + let denom = Denom::from("transfer/channel-0/utia".to_string()); + let destination_chain_address = "address".to_string(); + let event_with_meta = EventWithMetadata { + event: WithdrawalEvent::Ics20(Ics20WithdrawalFilter { + sender: [0u8; 20].into(), + amount: 99.into(), + destination_chain_address: destination_chain_address.clone(), + memo: b"hello".into(), + }), + block_number: 1.into(), + transaction_hash: [2u8; 32].into(), + }; + + let action = event_to_action(event_with_meta, denom.id(), denom.clone()).unwrap(); + let Action::Ics20Withdrawal(mut action) = action else { + panic!("expected Ics20Withdrawal action, got {:?}", action); + }; + + // TODO: instead of zeroing this, we should pass in the latest block time to the function + // and generate the timeout time from that. + action.timeout_time = 0; // zero this for testing + + let expected_action = Ics20Withdrawal { + denom: denom.clone(), + destination_chain_address, + return_address: [0u8; 20].into(), + amount: 99, + memo: serde_json::to_string(&Ics20WithdrawalMemo { + memo: "hello".to_string(), + block_number: 1.into(), + transaction_hash: [2u8; 32].into(), + }) + .unwrap(), + fee_asset_id: denom.id(), + timeout_height: IbcHeight::new(u64::MAX, u64::MAX).unwrap(), + timeout_time: 0, // zero this for testing + source_channel: "channel-0".parse().unwrap(), + }; + assert_eq!(action, expected_action); } } diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs b/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs index c3642c2406..af8cd3995b 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs +++ b/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs @@ -443,11 +443,12 @@ mod tests { transaction_hash: receipt.transaction_hash, }; let denom = Denom::from("transfer/channel-0/utia".to_string()); - let Action::Ics20Withdrawal(expected_action) = + let Action::Ics20Withdrawal(mut expected_action) = event_to_action(expected_event, denom.id(), denom.clone()).unwrap() else { panic!("expected action to be Ics20Withdrawal"); }; + expected_action.timeout_time = 0; // zero this for testing let (event_tx, mut event_rx) = mpsc::channel(100); let watcher = Watcher::new( @@ -467,14 +468,15 @@ mod tests { // make another tx to trigger anvil to make another block send_ics20_withdraw_transaction(&contract, value, recipient).await; - let batch = event_rx.recv().await.unwrap(); + let mut batch = event_rx.recv().await.unwrap(); assert_eq!(batch.actions.len(), 1); - let Action::Ics20Withdrawal(action) = &batch.actions[0] else { + let Action::Ics20Withdrawal(ref mut action) = batch.actions[0] else { panic!( "expected action to be Ics20Withdrawal, got {:?}", batch.actions[0] ); }; + action.timeout_time = 0; // zero this for testing assert_eq!(action, &expected_action); } } From b523f4a707615badc497b2680d9ec31c101f346e Mon Sep 17 00:00:00 2001 From: elizabeth Date: Thu, 23 May 2024 21:26:45 -0400 Subject: [PATCH 23/23] fmt --- crates/astria-bridge-withdrawer/src/withdrawer/batch.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/batch.rs b/crates/astria-bridge-withdrawer/src/withdrawer/batch.rs index 28cb26ced9..38009ff9e4 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/batch.rs +++ b/crates/astria-bridge-withdrawer/src/withdrawer/batch.rs @@ -230,7 +230,7 @@ mod tests { let Action::Ics20Withdrawal(mut action) = action else { panic!("expected Ics20Withdrawal action, got {:?}", action); }; - + // TODO: instead of zeroing this, we should pass in the latest block time to the function // and generate the timeout time from that. action.timeout_time = 0; // zero this for testing