From 8149513ad2b7ac3966575555e069e80e09d52f54 Mon Sep 17 00:00:00 2001 From: itamar Date: Mon, 10 Jun 2024 20:34:14 -0400 Subject: [PATCH 01/32] add error messages --- .../src/withdrawer/ethereum/watcher.rs | 12 ++++++--- .../src/withdrawer/submitter/mod.rs | 27 ++++++++++++++----- .../src/withdrawer/submitter/signer.rs | 5 ++-- 3 files changed, 32 insertions(+), 12 deletions(-) diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs b/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs index 57fe4ee2da..6f5345a991 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs +++ b/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs @@ -112,7 +112,9 @@ pub(crate) struct Watcher { impl Watcher { pub(crate) async fn run(mut self) -> Result<()> { let (provider, contract, fee_asset_id, asset_withdrawal_divisor, next_rollup_block_height) = - self.startup().await?; + self.startup() + .await + .wrap_err("watcher failed to start up")?; let Self { contract_address: _contract_address, @@ -191,7 +193,11 @@ impl Watcher { let SequencerStartupInfo { fee_asset_id, next_batch_rollup_height, - } = self.submitter_handle.recv_startup_info().await?; + } = self + .submitter_handle + .recv_startup_info() + .await + .wrap_err("failed to get sequencer startup info")?; // connect to eth node let retry_config = tryhard::RetryFutureConfig::new(1024) @@ -373,7 +379,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, self.rollup_asset_denom.clone(), self.asset_withdrawal_divisor, self.bridge_address)?; + let action = event_to_action(event_with_metadata, self.fee_asset_id, self.rollup_asset_denom.clone(), self.asset_withdrawal_divisor, self.bridge_address).wrap_err("failed to convert event to action")?; if meta.block_number.as_u64() == curr_batch.rollup_height { // block number was the same; add event to current batch diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/submitter/mod.rs b/crates/astria-bridge-withdrawer/src/withdrawer/submitter/mod.rs index 55c2522e5e..ec6cd1adc5 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/submitter/mod.rs +++ b/crates/astria-bridge-withdrawer/src/withdrawer/submitter/mod.rs @@ -91,7 +91,10 @@ pub(super) struct Submitter { impl Submitter { pub(super) async fn run(mut self) -> eyre::Result<()> { // call startup - let startup = self.startup().await?; + let startup = self + .startup() + .await + .wrap_err("submitter failed to start up")?; self.startup_tx .send(startup) .map_err(|_startup| eyre!("failed to send startup info to watcher"))?; @@ -169,7 +172,8 @@ impl Submitter { async fn startup(&mut self) -> eyre::Result { let actual_chain_id = get_sequencer_chain_id(self.sequencer_cometbft_client.clone(), self.state.clone()) - .await?; + .await + .wrap_err("failed to get chain id from sequencer")?; ensure!( self.sequencer_chain_id == actual_chain_id.to_string(), "sequencer_chain_id provided in config does not match chain_id returned from sequencer" @@ -178,7 +182,8 @@ impl Submitter { // confirm that the fee asset ID is valid let allowed_fee_asset_ids_resp = get_allowed_fee_asset_ids(self.sequencer_cometbft_client.clone(), self.state.clone()) - .await?; + .await + .wrap_err("failed to get allowed fee asset ids from sequencer")?; ensure!( allowed_fee_asset_ids_resp .fee_asset_ids @@ -192,7 +197,8 @@ impl Submitter { self.state.clone(), self.signer.address, ) - .await?; + .await + .wrap_err("failed to get latest balance")?; let fee_asset_balance = fee_asset_balances .balances .into_iter() @@ -205,7 +211,10 @@ impl Submitter { ); // sync to latest on-chain state - let next_batch_rollup_height = self.get_next_rollup_height().await?; + let next_batch_rollup_height = self + .get_next_rollup_height() + .await + .wrap_err("failed to get next rollup block height")?; self.state.set_submitter_ready(); @@ -237,7 +246,10 @@ impl Submitter { /// 3. The last transaction by the bridge account did not contain a withdrawal action /// 4. The memo of the last transaction by the bridge account could not be parsed async fn get_next_rollup_height(&mut self) -> eyre::Result { - let signed_transaction = self.get_last_transaction().await?; + let signed_transaction = self + .get_last_transaction() + .await + .wrap_err("failed to get the bridge account's last sequencer transaction")?; let next_batch_rollup_height = if let Some(signed_transaction) = signed_transaction { rollup_height_from_signed_transaction(&signed_transaction).wrap_err( "failed to extract rollup height from last transaction by the bridge account", @@ -314,7 +326,8 @@ async fn process_batch( sequencer_key.address, state.clone(), ) - .await?; + .await + .wrap_err("failed to get nonce from sequencer")?; debug!(nonce, "fetched latest nonce"); let unsigned = UnsignedTransaction { diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/submitter/signer.rs b/crates/astria-bridge-withdrawer/src/withdrawer/submitter/signer.rs index a75e6a5137..746a2f35dd 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/submitter/signer.rs +++ b/crates/astria-bridge-withdrawer/src/withdrawer/submitter/signer.rs @@ -26,8 +26,9 @@ impl SequencerKey { /// /// The file should contain a hex-encoded ed25519 secret key. pub(super) fn try_from_path>(path: P) -> eyre::Result { - let hex = fs::read_to_string(path)?; - let bytes: [u8; 32] = hex::decode(hex.trim())? + let hex = fs::read_to_string(path).wrap_err("failed to read sequencer key from path")?; + let bytes: [u8; 32] = hex::decode(hex.trim()) + .wrap_err("failed to decode hex")? .try_into() .map_err(|_| eyre!("invalid private key length; must be 32 bytes"))?; let signing_key = SigningKey::from(bytes); From 5948dfe237d4e30222790bfdba7766e557a5c6a6 Mon Sep 17 00:00:00 2001 From: itamar Date: Tue, 11 Jun 2024 21:08:32 -0400 Subject: [PATCH 02/32] fix service name --- crates/astria-bridge-withdrawer/src/lib.rs | 2 +- crates/astria-bridge-withdrawer/src/main.rs | 5 +++-- crates/astria-bridge-withdrawer/src/withdrawer/mod.rs | 4 ++-- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/crates/astria-bridge-withdrawer/src/lib.rs b/crates/astria-bridge-withdrawer/src/lib.rs index 15e25f0c2e..ed3283b6aa 100644 --- a/crates/astria-bridge-withdrawer/src/lib.rs +++ b/crates/astria-bridge-withdrawer/src/lib.rs @@ -8,4 +8,4 @@ pub use build_info::BUILD_INFO; pub use config::Config; #[cfg(test)] pub(crate) use withdrawer::astria_address; -pub use withdrawer::Service; +pub use withdrawer::BridgeWithdrawer; diff --git a/crates/astria-bridge-withdrawer/src/main.rs b/crates/astria-bridge-withdrawer/src/main.rs index 53ef465bd9..e04ce83fff 100644 --- a/crates/astria-bridge-withdrawer/src/main.rs +++ b/crates/astria-bridge-withdrawer/src/main.rs @@ -2,8 +2,8 @@ use std::process::ExitCode; use astria_bridge_withdrawer::{ metrics_init, + BridgeWithdrawer, Config, - Service, BUILD_INFO, }; use astria_eyre::eyre::WrapErr as _; @@ -54,7 +54,8 @@ async fn main() -> ExitCode { let mut sigterm = signal(SignalKind::terminate()) .expect("setting a SIGTERM listener should always work on Unix"); - let (withdrawer, shutdown_handle) = Service::new(cfg).expect("could not initialize withdrawer"); + let (withdrawer, shutdown_handle) = + BridgeWithdrawer::new(cfg).expect("could not initialize withdrawer"); let withdrawer_handle = tokio::spawn(withdrawer.run()); let shutdown_token = shutdown_handle.token(); diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/mod.rs b/crates/astria-bridge-withdrawer/src/withdrawer/mod.rs index 9b9c2f6ce1..74d364a2df 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/mod.rs +++ b/crates/astria-bridge-withdrawer/src/withdrawer/mod.rs @@ -46,7 +46,7 @@ mod ethereum; mod state; mod submitter; -pub struct Service { +pub struct BridgeWithdrawer { // Token to signal all subtasks to shut down gracefully. shutdown_token: CancellationToken, api_server: api::ApiServer, @@ -55,7 +55,7 @@ pub struct Service { state: Arc, } -impl Service { +impl BridgeWithdrawer { /// Instantiates a new `Service`. /// /// # Errors From f6d2baec0e474eb82136fc22092281722f3e08df Mon Sep 17 00:00:00 2001 From: itamar Date: Thu, 13 Jun 2024 12:32:40 -0400 Subject: [PATCH 03/32] fix name --- .../src/withdrawer/ethereum/watcher.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs b/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs index 6f5345a991..a958ee0e87 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs +++ b/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs @@ -39,7 +39,7 @@ use tracing::{ warn, }; -use crate::withdrawer::{ +use crate::bridge_withdrawer::{ batch::Batch, ethereum::{ astria_withdrawer_interface::IAstriaWithdrawer, @@ -446,7 +446,7 @@ mod tests { use tokio::sync::oneshot; use super::*; - use crate::withdrawer::ethereum::{ + use crate::bridge_withdrawer::ethereum::{ astria_bridgeable_erc20::AstriaBridgeableERC20, astria_withdrawer::AstriaWithdrawer, astria_withdrawer_interface::{ From 86a1a035869020c93e5e1dba9b5559436f2bbebc Mon Sep 17 00:00:00 2001 From: itamar Date: Thu, 13 Jun 2024 13:05:40 -0400 Subject: [PATCH 04/32] fix renaming --- crates/astria-bridge-withdrawer/build.rs | 6 +++--- crates/astria-bridge-withdrawer/src/api.rs | 2 +- .../src/{withdrawer => bridge_withdrawer}/batch.rs | 0 .../{withdrawer => bridge_withdrawer}/ethereum/convert.rs | 4 ++-- .../ethereum/generated/astria_bridgeable_erc20.rs | 0 .../ethereum/generated/astria_withdrawer.rs | 0 .../ethereum/generated/astria_withdrawer_interface.rs | 0 .../ethereum/generated/mod.rs | 0 .../src/{withdrawer => bridge_withdrawer}/ethereum/mod.rs | 0 .../ethereum/test_utils.rs | 2 +- .../{withdrawer => bridge_withdrawer}/ethereum/watcher.rs | 0 .../src/{withdrawer => bridge_withdrawer}/mod.rs | 0 .../src/{withdrawer => bridge_withdrawer}/state.rs | 0 .../submitter/builder.rs | 2 +- .../{withdrawer => bridge_withdrawer}/submitter/mod.rs | 2 +- .../{withdrawer => bridge_withdrawer}/submitter/signer.rs | 0 .../{withdrawer => bridge_withdrawer}/submitter/tests.rs | 2 +- crates/astria-bridge-withdrawer/src/lib.rs | 8 ++++---- 18 files changed, 14 insertions(+), 14 deletions(-) rename crates/astria-bridge-withdrawer/src/{withdrawer => bridge_withdrawer}/batch.rs (100%) rename crates/astria-bridge-withdrawer/src/{withdrawer => bridge_withdrawer}/ethereum/convert.rs (98%) rename crates/astria-bridge-withdrawer/src/{withdrawer => bridge_withdrawer}/ethereum/generated/astria_bridgeable_erc20.rs (100%) rename crates/astria-bridge-withdrawer/src/{withdrawer => bridge_withdrawer}/ethereum/generated/astria_withdrawer.rs (100%) rename crates/astria-bridge-withdrawer/src/{withdrawer => bridge_withdrawer}/ethereum/generated/astria_withdrawer_interface.rs (100%) rename crates/astria-bridge-withdrawer/src/{withdrawer => bridge_withdrawer}/ethereum/generated/mod.rs (100%) rename crates/astria-bridge-withdrawer/src/{withdrawer => bridge_withdrawer}/ethereum/mod.rs (100%) rename crates/astria-bridge-withdrawer/src/{withdrawer => bridge_withdrawer}/ethereum/test_utils.rs (99%) rename crates/astria-bridge-withdrawer/src/{withdrawer => bridge_withdrawer}/ethereum/watcher.rs (100%) rename crates/astria-bridge-withdrawer/src/{withdrawer => bridge_withdrawer}/mod.rs (100%) rename crates/astria-bridge-withdrawer/src/{withdrawer => bridge_withdrawer}/state.rs (100%) rename crates/astria-bridge-withdrawer/src/{withdrawer => bridge_withdrawer}/submitter/builder.rs (98%) rename crates/astria-bridge-withdrawer/src/{withdrawer => bridge_withdrawer}/submitter/mod.rs (99%) rename crates/astria-bridge-withdrawer/src/{withdrawer => bridge_withdrawer}/submitter/signer.rs (100%) rename crates/astria-bridge-withdrawer/src/{withdrawer => bridge_withdrawer}/submitter/tests.rs (99%) diff --git a/crates/astria-bridge-withdrawer/build.rs b/crates/astria-bridge-withdrawer/build.rs index 3540920d12..18a400b907 100644 --- a/crates/astria-bridge-withdrawer/build.rs +++ b/crates/astria-bridge-withdrawer/build.rs @@ -12,21 +12,21 @@ fn main() -> Result<(), Box> { "./astria-bridge-contracts/out/IAstriaWithdrawer.sol/IAstriaWithdrawer.json", )? .generate()? - .write_to_file("./src/withdrawer/ethereum/generated/astria_withdrawer_interface.rs")?; + .write_to_file("./src/bridge_withdrawer/ethereum/generated/astria_withdrawer_interface.rs")?; Abigen::new( "AstriaWithdrawer", "./astria-bridge-contracts/out/AstriaWithdrawer.sol/AstriaWithdrawer.json", )? .generate()? - .write_to_file("./src/withdrawer/ethereum/generated/astria_withdrawer.rs")?; + .write_to_file("./src/bridge_withdrawer/ethereum/generated/astria_withdrawer.rs")?; Abigen::new( "AstriaBridgeableERC20", "./astria-bridge-contracts/out/AstriaBridgeableERC20.sol/AstriaBridgeableERC20.json", )? .generate()? - .write_to_file("./src/withdrawer/ethereum/generated/astria_bridgeable_erc20.rs")?; + .write_to_file("./src/bridge_withdrawer/ethereum/generated/astria_bridgeable_erc20.rs")?; Ok(()) } diff --git a/crates/astria-bridge-withdrawer/src/api.rs b/crates/astria-bridge-withdrawer/src/api.rs index 25e7e786f3..81eb2fa13e 100644 --- a/crates/astria-bridge-withdrawer/src/api.rs +++ b/crates/astria-bridge-withdrawer/src/api.rs @@ -21,7 +21,7 @@ use hyper::server::conn::AddrIncoming; use serde::Serialize; use tokio::sync::watch; -use crate::withdrawer::StateSnapshot; +use crate::bridge_withdrawer::StateSnapshot; pub(crate) type ApiServer = axum::Server>; diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/batch.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/batch.rs similarity index 100% rename from crates/astria-bridge-withdrawer/src/withdrawer/batch.rs rename to crates/astria-bridge-withdrawer/src/bridge_withdrawer/batch.rs diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/convert.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/convert.rs similarity index 98% rename from crates/astria-bridge-withdrawer/src/withdrawer/ethereum/convert.rs rename to crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/convert.rs index 86db4f8c45..2bc0d09b3e 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/convert.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/convert.rs @@ -31,7 +31,7 @@ use serde::{ Serialize, }; -use crate::withdrawer::ethereum::astria_withdrawer_interface::{ +use crate::bridge_withdrawer::ethereum::astria_withdrawer_interface::{ Ics20WithdrawalFilter, SequencerWithdrawalFilter, }; @@ -191,7 +191,7 @@ mod tests { use asset::default_native_asset; use super::*; - use crate::withdrawer::ethereum::astria_withdrawer_interface::SequencerWithdrawalFilter; + use crate::bridge_withdrawer::ethereum::astria_withdrawer_interface::SequencerWithdrawalFilter; #[test] fn event_to_bridge_unlock() { diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/generated/astria_bridgeable_erc20.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/generated/astria_bridgeable_erc20.rs similarity index 100% rename from crates/astria-bridge-withdrawer/src/withdrawer/ethereum/generated/astria_bridgeable_erc20.rs rename to crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/generated/astria_bridgeable_erc20.rs diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/generated/astria_withdrawer.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/generated/astria_withdrawer.rs similarity index 100% rename from crates/astria-bridge-withdrawer/src/withdrawer/ethereum/generated/astria_withdrawer.rs rename to crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/generated/astria_withdrawer.rs diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/generated/astria_withdrawer_interface.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/generated/astria_withdrawer_interface.rs similarity index 100% rename from crates/astria-bridge-withdrawer/src/withdrawer/ethereum/generated/astria_withdrawer_interface.rs rename to crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/generated/astria_withdrawer_interface.rs diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/generated/mod.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/generated/mod.rs similarity index 100% rename from crates/astria-bridge-withdrawer/src/withdrawer/ethereum/generated/mod.rs rename to crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/generated/mod.rs diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/mod.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/mod.rs similarity index 100% rename from crates/astria-bridge-withdrawer/src/withdrawer/ethereum/mod.rs rename to crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/mod.rs diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/test_utils.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/test_utils.rs similarity index 99% rename from crates/astria-bridge-withdrawer/src/withdrawer/ethereum/test_utils.rs rename to crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/test_utils.rs index b98bcfbb2a..1e1c3c1962 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/test_utils.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/test_utils.rs @@ -11,7 +11,7 @@ use ethers::{ utils::AnvilInstance, }; -use crate::withdrawer::ethereum::{ +use crate::bridge_withdrawer::ethereum::{ astria_bridgeable_erc20::{ ASTRIABRIDGEABLEERC20_ABI, ASTRIABRIDGEABLEERC20_BYTECODE, diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/watcher.rs similarity index 100% rename from crates/astria-bridge-withdrawer/src/withdrawer/ethereum/watcher.rs rename to crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/watcher.rs diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/mod.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/mod.rs similarity index 100% rename from crates/astria-bridge-withdrawer/src/withdrawer/mod.rs rename to crates/astria-bridge-withdrawer/src/bridge_withdrawer/mod.rs diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/state.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/state.rs similarity index 100% rename from crates/astria-bridge-withdrawer/src/withdrawer/state.rs rename to crates/astria-bridge-withdrawer/src/bridge_withdrawer/state.rs diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/submitter/builder.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/builder.rs similarity index 98% rename from crates/astria-bridge-withdrawer/src/withdrawer/submitter/builder.rs rename to crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/builder.rs index 47f85b69a1..0731f0fb36 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/submitter/builder.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/builder.rs @@ -13,7 +13,7 @@ use tokio_util::sync::CancellationToken; use tracing::info; use super::state::State; -use crate::withdrawer::{ +use crate::bridge_withdrawer::{ submitter::Batch, SequencerStartupInfo, }; diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/submitter/mod.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/mod.rs similarity index 99% rename from crates/astria-bridge-withdrawer/src/withdrawer/submitter/mod.rs rename to crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/mod.rs index ec6cd1adc5..ab5ecc3565 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/submitter/mod.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/mod.rs @@ -69,7 +69,7 @@ use super::{ state, SequencerStartupInfo, }; -use crate::withdrawer::ethereum::convert::BridgeUnlockMemo; +use crate::bridge_withdrawer::ethereum::convert::BridgeUnlockMemo; mod builder; mod signer; diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/submitter/signer.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/signer.rs similarity index 100% rename from crates/astria-bridge-withdrawer/src/withdrawer/submitter/signer.rs rename to crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/signer.rs diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/submitter/tests.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/tests.rs similarity index 99% rename from crates/astria-bridge-withdrawer/src/withdrawer/submitter/tests.rs rename to crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/tests.rs index 38ce986939..2839ecba70 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/submitter/tests.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/tests.rs @@ -81,7 +81,7 @@ use wiremock::{ }; use super::Submitter; -use crate::withdrawer::{ +use crate::bridge_withdrawer::{ batch::Batch, ethereum::convert::BridgeUnlockMemo, state, diff --git a/crates/astria-bridge-withdrawer/src/lib.rs b/crates/astria-bridge-withdrawer/src/lib.rs index ed3283b6aa..105bae1bfb 100644 --- a/crates/astria-bridge-withdrawer/src/lib.rs +++ b/crates/astria-bridge-withdrawer/src/lib.rs @@ -1,11 +1,11 @@ pub(crate) mod api; +pub mod bridge_withdrawer; mod build_info; pub(crate) mod config; pub mod metrics_init; -pub mod withdrawer; +#[cfg(test)] +pub(crate) use bridge_withdrawer::astria_address; +pub use bridge_withdrawer::BridgeWithdrawer; pub use build_info::BUILD_INFO; pub use config::Config; -#[cfg(test)] -pub(crate) use withdrawer::astria_address; -pub use withdrawer::BridgeWithdrawer; From ad934e7619757df3d0099d9ce1a3f0430b010ac9 Mon Sep 17 00:00:00 2001 From: itamar Date: Fri, 14 Jun 2024 20:35:12 -0400 Subject: [PATCH 05/32] startup refactor no tests --- .../src/bridge_withdrawer/ethereum/watcher.rs | 84 +-- .../src/bridge_withdrawer/mod.rs | 71 +- .../src/bridge_withdrawer/startup.rs | 610 ++++++++++++++++++ .../bridge_withdrawer/submitter/builder.rs | 41 +- .../src/bridge_withdrawer/submitter/mod.rs | 465 +------------ .../src/bridge_withdrawer/submitter/signer.rs | 8 +- 6 files changed, 753 insertions(+), 526 deletions(-) create mode 100644 crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/watcher.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/watcher.rs index a958ee0e87..1d805f9d83 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/watcher.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/watcher.rs @@ -49,19 +49,21 @@ use crate::bridge_withdrawer::{ WithdrawalEvent, }, }, + startup, + startup::WatcherInfo as StartupInfo, state::State, submitter, - SequencerStartupInfo, }; pub(crate) struct Builder { + pub(crate) shutdown_token: CancellationToken, + pub(crate) startup_handle: startup::WatcherHandle, pub(crate) ethereum_contract_address: String, pub(crate) ethereum_rpc_endpoint: String, - pub(crate) submitter_handle: submitter::Handle, - pub(crate) shutdown_token: CancellationToken, pub(crate) state: Arc, pub(crate) rollup_asset_denom: Denom, pub(crate) bridge_address: Address, + pub(crate) submitter_handle: submitter::Handle, } impl Builder { @@ -69,11 +71,12 @@ impl Builder { let Builder { ethereum_contract_address, ethereum_rpc_endpoint, - submitter_handle, + startup_handle, shutdown_token, state, rollup_asset_denom, bridge_address, + submitter_handle, } = self; let contract_address = address_from_string(ðereum_contract_address) @@ -89,24 +92,26 @@ impl Builder { Ok(Watcher { contract_address, ethereum_rpc_endpoint: ethereum_rpc_endpoint.to_string(), - submitter_handle, + startup_handle, rollup_asset_denom, bridge_address, state, shutdown_token: shutdown_token.clone(), + submitter_handle, }) } } /// Watches for withdrawal events emitted by the `AstriaWithdrawer` contract. pub(crate) struct Watcher { + shutdown_token: CancellationToken, + startup_handle: startup::WatcherHandle, + submitter_handle: submitter::Handle, contract_address: ethers::types::Address, ethereum_rpc_endpoint: String, - submitter_handle: submitter::Handle, rollup_asset_denom: Denom, bridge_address: Address, state: Arc, - shutdown_token: CancellationToken, } impl Watcher { @@ -117,13 +122,12 @@ impl Watcher { .wrap_err("watcher failed to start up")?; let Self { - contract_address: _contract_address, - ethereum_rpc_endpoint: _ethereum_rps_endpoint, - submitter_handle, rollup_asset_denom, bridge_address, state, shutdown_token, + submitter_handle, + .. } = self; let (event_tx, event_rx) = mpsc::channel(100); @@ -189,15 +193,19 @@ impl Watcher { u128, u64, )> { - // wait for submitter to be ready - let SequencerStartupInfo { + let StartupInfo { fee_asset_id, - next_batch_rollup_height, - } = self - .submitter_handle - .recv_startup_info() - .await - .wrap_err("failed to get sequencer startup info")?; + starting_rollup_height, + } = select! { + () = self.shutdown_token.cancelled() => { + info!("watcher received shutdown signal while waiting for startup"); + return Err(eyre!("watcher shutting down")); + } + + startup_info = self.startup_handle.recv() => { + startup_info.wrap_err("failed to receive startup info")? + } + }; // connect to eth node let retry_config = tryhard::RetryFutureConfig::new(1024) @@ -252,7 +260,7 @@ impl Watcher { contract, fee_asset_id, asset_withdrawal_divisor, - next_batch_rollup_height, + starting_rollup_height, )) } } @@ -564,22 +572,23 @@ mod tests { let (batch_tx, mut batch_rx) = mpsc::channel(100); let (startup_tx, startup_rx) = oneshot::channel(); - let submitter_handle = submitter::Handle::new(startup_rx, batch_tx); + let startup_handle = startup::WatcherHandle::new(startup_rx); startup_tx - .send(SequencerStartupInfo { + .send(StartupInfo { fee_asset_id: denom.id(), - next_batch_rollup_height: 0, + starting_rollup_height: 0, }) .unwrap(); let watcher = Builder { ethereum_contract_address: hex::encode(contract_address), ethereum_rpc_endpoint: anvil.ws_endpoint(), - submitter_handle, + startup_handle, shutdown_token: CancellationToken::new(), state: Arc::new(State::new()), rollup_asset_denom: denom, bridge_address, + submitter_handle: submitter::Handle::new(batch_tx), } .build() .unwrap(); @@ -656,22 +665,23 @@ mod tests { let (batch_tx, mut batch_rx) = mpsc::channel(100); let (startup_tx, startup_rx) = oneshot::channel(); - let submitter_handle = submitter::Handle::new(startup_rx, batch_tx); + let startup_handle = startup::WatcherHandle::new(startup_rx); startup_tx - .send(SequencerStartupInfo { + .send(StartupInfo { fee_asset_id: denom.id(), - next_batch_rollup_height: 0, + starting_rollup_height: 0, }) .unwrap(); let watcher = Builder { ethereum_contract_address: hex::encode(contract_address), ethereum_rpc_endpoint: anvil.ws_endpoint(), - submitter_handle, + startup_handle, shutdown_token: CancellationToken::new(), state: Arc::new(State::new()), rollup_asset_denom: denom, bridge_address, + submitter_handle: submitter::Handle::new(batch_tx), } .build() .unwrap(); @@ -778,22 +788,23 @@ mod tests { let (batch_tx, mut batch_rx) = mpsc::channel(100); let (startup_tx, startup_rx) = oneshot::channel(); - let submitter_handle = submitter::Handle::new(startup_rx, batch_tx); + let startup_handle = startup::WatcherHandle::new(startup_rx); startup_tx - .send(SequencerStartupInfo { + .send(StartupInfo { fee_asset_id: denom.id(), - next_batch_rollup_height: 0, + starting_rollup_height: 0, }) .unwrap(); let watcher = Builder { ethereum_contract_address: hex::encode(contract_address), ethereum_rpc_endpoint: anvil.ws_endpoint(), - submitter_handle, + startup_handle, shutdown_token: CancellationToken::new(), state: Arc::new(State::new()), rollup_asset_denom: denom, bridge_address, + submitter_handle: submitter::Handle::new(batch_tx), } .build() .unwrap(); @@ -880,22 +891,23 @@ mod tests { let (batch_tx, mut batch_rx) = mpsc::channel(100); let (startup_tx, startup_rx) = oneshot::channel(); - let submitter_handle = submitter::Handle::new(startup_rx, batch_tx); + let startup_handle = startup::WatcherHandle::new(startup_rx); startup_tx - .send(SequencerStartupInfo { - fee_asset_id: asset::Id::from_denom("transfer/channel-0/utia"), - next_batch_rollup_height: 0, + .send(StartupInfo { + fee_asset_id: denom.id(), + starting_rollup_height: 0, }) .unwrap(); let watcher = Builder { ethereum_contract_address: hex::encode(contract_address), ethereum_rpc_endpoint: anvil.ws_endpoint(), - submitter_handle, + startup_handle, shutdown_token: CancellationToken::new(), state: Arc::new(State::new()), rollup_asset_denom: denom, bridge_address, + submitter_handle: submitter::Handle::new(batch_tx), } .build() .unwrap(); diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/mod.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/mod.rs index 74d364a2df..16f99267de 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/mod.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/mod.rs @@ -43,6 +43,7 @@ use crate::{ mod batch; mod ethereum; +mod startup; mod state; mod submitter; @@ -52,6 +53,7 @@ pub struct BridgeWithdrawer { api_server: api::ApiServer, submitter: Submitter, ethereum_watcher: watcher::Watcher, + startup: startup::Startup, state: Arc, } @@ -78,15 +80,26 @@ impl BridgeWithdrawer { let state = Arc::new(State::new()); + // make startup object + let (startup, startup_submitter_handle, startup_watcher_handle) = startup::Builder { + shutdown_token: shutdown_handle.token(), + state: state.clone(), + sequencer_chain_id, + sequencer_cometbft_endpoint: sequencer_cometbft_endpoint.clone(), + sequencer_key_path: sequencer_key_path.clone(), + expected_fee_asset_id: asset::Id::from_denom(&fee_asset_denomination), + expected_min_fee_asset_balance: u128::from(min_expected_fee_asset_balance), + } + .build() + .wrap_err("failed to initialize startup")?; + // make submitter object let (submitter, submitter_handle) = submitter::Builder { shutdown_token: shutdown_handle.token(), + startup_handle: startup_submitter_handle, sequencer_cometbft_endpoint, - sequencer_chain_id, sequencer_key_path, state: state.clone(), - expected_fee_asset_id: asset::Id::from_denom(&fee_asset_denomination), - min_expected_fee_asset_balance: u128::from(min_expected_fee_asset_balance), } .build() .wrap_err("failed to initialize submitter")?; @@ -97,13 +110,14 @@ impl BridgeWithdrawer { let ethereum_watcher = watcher::Builder { ethereum_contract_address, ethereum_rpc_endpoint, - submitter_handle, + startup_handle: startup_watcher_handle, shutdown_token: shutdown_handle.token(), state: state.clone(), rollup_asset_denom: rollup_asset_denomination .parse::() .wrap_err("failed to parse ROLLUP_ASSET_DENOMINATION as Denom")?, bridge_address: sequencer_bridge_address, + submitter_handle, } .build() .wrap_err("failed to build ethereum watcher")?; @@ -120,6 +134,7 @@ impl BridgeWithdrawer { api_server, submitter, ethereum_watcher, + startup, state, }; @@ -132,6 +147,7 @@ impl BridgeWithdrawer { api_server, submitter, ethereum_watcher, + startup, state: _state, } = self; @@ -148,6 +164,9 @@ impl BridgeWithdrawer { }); info!("spawned API server"); + let mut startup_task = tokio::spawn(startup.run()); + info!("spawned startup task"); + let mut submitter_task = tokio::spawn(submitter.run()); info!("spawned submitter task"); let mut ethereum_watcher_task = tokio::spawn(ethereum_watcher.run()); @@ -160,16 +179,29 @@ impl BridgeWithdrawer { api_task: None, submitter_task: Some(submitter_task), ethereum_watcher_task: Some(ethereum_watcher_task), + startup_task: Some(startup_task), api_shutdown_signal, token: shutdown_token } } + o = &mut startup_task => { + report_exit("startup", o); + Shutdown { + api_task: Some(api_task), + submitter_task: Some(submitter_task), + ethereum_watcher_task: Some(ethereum_watcher_task), + startup_task: None, + api_shutdown_signal, + token: shutdown_token + } + } o = &mut submitter_task => { report_exit("submitter", o); Shutdown { api_task: Some(api_task), submitter_task: None, ethereum_watcher_task:Some(ethereum_watcher_task), + startup_task: Some(startup_task), api_shutdown_signal, token: shutdown_token } @@ -180,6 +212,7 @@ impl BridgeWithdrawer { api_task: Some(api_task), submitter_task: Some(submitter_task), ethereum_watcher_task: None, + startup_task: Some(startup_task), api_shutdown_signal, token: shutdown_token } @@ -190,12 +223,6 @@ impl BridgeWithdrawer { } } -#[derive(Debug)] -pub struct SequencerStartupInfo { - pub fee_asset_id: asset::Id, - pub next_batch_rollup_height: u64, -} - /// A handle for instructing the [`Service`] to shut down. /// /// It is returned along with its related `Service` from [`Service::new`]. The @@ -254,6 +281,7 @@ struct Shutdown { api_task: Option>>, submitter_task: Option>>, ethereum_watcher_task: Option>>, + startup_task: Option>>, api_shutdown_signal: oneshot::Sender<()>, token: CancellationToken, } @@ -261,19 +289,40 @@ struct Shutdown { impl Shutdown { const API_SHUTDOWN_TIMEOUT_SECONDS: u64 = 4; const ETHEREUM_WATCHER_SHUTDOWN_TIMEOUT_SECONDS: u64 = 5; - const SUBMITTER_SHUTDOWN_TIMEOUT_SECONDS: u64 = 20; + const STARTUP_SHUTDOWN_TIMEOUT_SECONDS: u64 = 1; + const SUBMITTER_SHUTDOWN_TIMEOUT_SECONDS: u64 = 19; async fn run(self) { let Self { api_task, submitter_task, ethereum_watcher_task, + startup_task, api_shutdown_signal, token, } = self; token.cancel(); + // Giving startup 1 second to shutdown because it should be very quick. + if let Some(mut startup_task) = startup_task { + info!("waiting for startup task to shut down"); + let limit = Duration::from_secs(Self::STARTUP_SHUTDOWN_TIMEOUT_SECONDS); + match timeout(limit, &mut startup_task).await.map(flatten_result) { + Ok(Ok(())) => info!("startup exited gracefully"), + Ok(Err(error)) => error!(%error, "startup exited with an error"), + Err(_) => { + error!( + timeout_secs = limit.as_secs(), + "startup did not shut down within timeout; killing it" + ); + startup_task.abort(); + } + } + } else { + info!("startup task was already dead"); + } + // 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"); diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs new file mode 100644 index 0000000000..846d0712cc --- /dev/null +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs @@ -0,0 +1,610 @@ +use std::{ + sync::Arc, + time::Duration, +}; + +use astria_core::{ + bridge::Ics20WithdrawalFromRollupMemo, + primitive::v1::asset, + protocol::{ + asset::v1alpha1::AllowedFeeAssetIdsResponse, + bridge::v1alpha1::BridgeAccountLastTxHashResponse, + transaction::v1alpha1::Action, + }, +}; +use astria_eyre::eyre::{ + self, + ensure, + eyre, + Context as _, + OptionExt as _, +}; +use prost::Message as _; +use sequencer_client::{ + tendermint_rpc, + Address, + BalanceResponse, + SequencerClientExt as _, + SignedTransaction, +}; +use tendermint_rpc::{ + endpoint::tx, + Client as _, +}; +use tokio::sync::oneshot; +use tokio_util::sync::CancellationToken; +use tracing::{ + error, + info, + instrument, + warn, +}; + +use super::state::State; +use crate::bridge_withdrawer::ethereum::convert::BridgeUnlockMemo; + +pub(super) struct Builder { + pub(super) shutdown_token: CancellationToken, + pub(super) state: Arc, + pub(super) sequencer_chain_id: String, + pub(super) sequencer_cometbft_endpoint: String, + pub(super) sequencer_key_path: String, + pub(super) expected_fee_asset_id: asset::Id, + // TODO: change the name of this config var + pub(super) expected_min_fee_asset_balance: u128, +} + +impl Builder { + pub(super) fn build(self) -> eyre::Result<(Startup, SubmitterHandle, WatcherHandle)> { + let Self { + shutdown_token, + state, + sequencer_chain_id, + sequencer_cometbft_endpoint, + sequencer_key_path, + expected_fee_asset_id, + expected_min_fee_asset_balance, + } = self; + + let signer = super::submitter::signer::SequencerKey::try_from_path(sequencer_key_path) + .wrap_err("failed to load sequencer private ky")?; + let address = signer.address; + info!(address = %telemetry::display::hex(&address), "loaded sequencer signer"); + + let sequencer_cometbft_client = + sequencer_client::HttpClient::new(&*sequencer_cometbft_endpoint) + .wrap_err("failed constructing cometbft http client")?; + + let (submitter_tx, submitter_rx) = oneshot::channel(); + let submitter_handle = SubmitterHandle::new(submitter_rx); + + let (watcher_tx, watcher_rx) = oneshot::channel(); + let watcher_handle = WatcherHandle::new(watcher_rx); + + let startup = Startup { + shutdown_token, + state, + submitter_tx, + watcher_tx, + sequencer_chain_id, + sequencer_cometbft_client, + address, + expected_fee_asset_id, + expected_min_fee_asset_balance, + }; + + Ok((startup, submitter_handle, watcher_handle)) + } +} + +pub(super) struct SubmitterInfo { + pub(super) sequencer_chain_id: String, +} + +pub(super) struct SubmitterHandle { + info_rx: Option>, +} + +impl SubmitterHandle { + pub(super) fn new(info_rx: oneshot::Receiver) -> Self { + Self { + info_rx: Some(info_rx), + } + } + + pub(super) async fn recv(&mut self) -> eyre::Result { + self.info_rx + .take() + .expect("startup info should only be taken once - this is a bug") + .await + .wrap_err("failed to get startup info from submitter. channel was dropped.") + } +} + +pub(super) struct WatcherInfo { + pub(super) fee_asset_id: asset::Id, + pub(super) starting_rollup_height: u64, +} + +pub(super) struct WatcherHandle { + info_rx: Option>, +} + +impl WatcherHandle { + pub(super) fn new(info_rx: oneshot::Receiver) -> Self { + Self { + info_rx: Some(info_rx), + } + } + + pub(super) async fn recv(&mut self) -> eyre::Result { + self.info_rx + .take() + .expect("startup info should only be taken once - this is a bug") + .await + .wrap_err("failed to get startup info from watcher. channel was dropped.") + } +} + +pub(super) struct Startup { + shutdown_token: CancellationToken, + state: Arc, + submitter_tx: oneshot::Sender, + watcher_tx: oneshot::Sender, + sequencer_chain_id: String, + sequencer_cometbft_client: sequencer_client::HttpClient, + address: Address, + expected_fee_asset_id: asset::Id, + expected_min_fee_asset_balance: u128, +} + +impl Startup { + pub(super) async fn run(mut self) -> eyre::Result<()> { + let shutdown_token = self.shutdown_token.clone(); + + let startup_task = tokio::spawn(async { + self.confirm_sequencer_config() + .await + .wrap_err("failed to confirm sequencer config")?; + let starting_rollup_height = self + .get_starting_rollup_height() + .await + .wrap_err("failed to get next rollup block height")?; + + // send the startup info to the submitter + let submitter_info = SubmitterInfo { + sequencer_chain_id: self.sequencer_chain_id.clone(), + }; + + let watcher_info = WatcherInfo { + fee_asset_id: self.expected_fee_asset_id, + starting_rollup_height, + }; + + self.submitter_tx + .send(submitter_info) + .map_err(|_submitter_info| eyre!("failed to send submitter info"))?; + self.watcher_tx + .send(watcher_info) + .map_err(|_watcher_info| eyre!("failed to send watcher info"))?; + + Ok(()) + }); + + tokio::select!( + _ = shutdown_token.cancelled() => { + Err(eyre!("startup was cancelled")) + } + res = startup_task => { + match res { + Ok(Ok(())) => Ok(()), + Ok(Err(err)) => { + error!(%err, "startup failed"); + Err(err)}, + Err(reason) => { + Err(reason.into()) + } + } + } + ) + } + + /// Confirms configuration values against the sequencer node. Values checked: + /// + /// - `self.sequencer_chain_id` matches the value returned from the sequencer node's genesis + /// - `self.fee_asset_id` is a valid fee asset on the sequencer node + /// - `self.sequencer_key.address` has a sufficient balance of `self.fee_asset_id` + /// + /// # Errors + /// + /// - `self.chain_id` does not match the value returned from the sequencer node + /// - `self.fee_asset_id` is not a valid fee asset on the sequencer node + /// - `self.sequencer_key.address` does not have a sufficient balance of `self.fee_asset_id`. + async fn confirm_sequencer_config(&mut self) -> eyre::Result<()> { + // confirm the sequencer chain id + let actual_chain_id = + get_sequencer_chain_id(self.sequencer_cometbft_client.clone(), self.state.clone()) + .await + .wrap_err("failed to get chain id from sequencer")?; + ensure!( + self.sequencer_chain_id == actual_chain_id.to_string(), + "sequencer_chain_id provided in config does not match chain_id returned from sequencer" + ); + + // confirm that the fee asset ID is valid + let allowed_fee_asset_ids_resp = + get_allowed_fee_asset_ids(self.sequencer_cometbft_client.clone(), self.state.clone()) + .await + .wrap_err("failed to get allowed fee asset ids from sequencer")?; + ensure!( + allowed_fee_asset_ids_resp + .fee_asset_ids + .contains(&self.expected_fee_asset_id), + "fee_asset_id provided in config is not a valid fee asset on the sequencer" + ); + + // confirm that the sequencer key has a sufficient balance of the fee asset + let fee_asset_balances = get_latest_balance( + self.sequencer_cometbft_client.clone(), + self.state.clone(), + self.address, + ) + .await + .wrap_err("failed to get latest balance")?; + let fee_asset_balance = fee_asset_balances + .balances + .into_iter() + .find(|balance| balance.denom.id() == self.expected_fee_asset_id) + .ok_or_eyre("withdrawer's account does not have the minimum balance of the fee asset")? + .balance; + ensure!( + fee_asset_balance >= self.expected_min_fee_asset_balance, + "sequencer key does not have a sufficient balance of the fee asset" + ); + + Ok(()) + } + + /// Gets the last transaction by the bridge account on the sequencer. This is used to + /// determine the starting rollup height for syncing to the latest on-chain state. + /// + /// # Returns + /// The last transaction by the bridge account on the sequencer, if it exists. + /// + /// # Errors + /// + /// 1. Failing to fetch the last transaction hash by the bridge account. + /// 2. Failing to convert the last transaction hash to a tendermint hash. + /// 3. Failing to fetch the last transaction by the bridge account. + /// 4. The last transaction by the bridge account failed to execute (this should not happen + /// in the sequencer logic). + /// 5. Failing to convert the transaction data from CometBFT to proto. + /// 6. Failing to convert the transaction data from proto to SignedTransaction. + async fn get_last_transaction(&self) -> eyre::Result> { + // get last transaction hash by the bridge account, if it exists + let last_transaction_hash_resp = get_bridge_account_last_transaction_hash( + self.sequencer_cometbft_client.clone(), + self.state.clone(), + self.address, + ) + .await + .wrap_err("failed to fetch last transaction hash by the bridge account")?; + + let Some(tx_hash) = last_transaction_hash_resp.tx_hash else { + return Ok(None); + }; + + let tx_hash = tendermint::Hash::try_from(tx_hash.to_vec()) + .wrap_err("failed to convert last transaction hash to tendermint hash")?; + + // get the corresponding transaction + let last_transaction = get_tx( + self.sequencer_cometbft_client.clone(), + self.state.clone(), + tx_hash, + ) + .await + .wrap_err("failed to fetch last transaction by the bridge account")?; + + // check that the transaction actually executed + ensure!( + last_transaction.tx_result.code == tendermint::abci::Code::Ok, + "last transaction by the bridge account failed to execute. this should not happen in \ + the sequencer logic." + ); + + let proto_tx = + astria_core::generated::protocol::transaction::v1alpha1::SignedTransaction::decode( + &*last_transaction.tx, + ) + .wrap_err("failed to convert transaction data from CometBFT to proto")?; + + let tx = SignedTransaction::try_from_raw(proto_tx) + .wrap_err("failed to convert transaction data from proto to SignedTransaction")?; + + info!( + last_bridge_account_tx.hash = %telemetry::display::hex(&tx_hash), + last_bridge_account_tx.height = i64::from(last_transaction.height), + "fetched last transaction by the bridge account" + ); + + Ok(Some(tx)) + } + + /// Gets the data necessary for syncing to the latest on-chain state from the sequencer. + /// Since we batch all events from a given rollup block into a single sequencer + /// transaction, we get the last tx finalized by the bridge account on the sequencer + /// and extract the rollup height from it. + /// + /// The rollup height is extracted from the block height value in the memo of one of the + /// actions in the batch. + /// + /// # Returns + /// The next batch rollup height to process. + /// + /// # Errors + /// + /// 1. Failing to get and deserialize a valid last transaction by the bridge account from the + /// sequencer. + /// 2. The last transaction by the bridge account failed to execute (this should not happen in + /// the sequencer logic) + /// 3. The last transaction by the bridge account did not contain a withdrawal action + /// 4. The memo of the last transaction by the bridge account could not be parsed + async fn get_starting_rollup_height(&mut self) -> eyre::Result { + let signed_transaction = self + .get_last_transaction() + .await + .wrap_err("failed to get the bridge account's last sequencer transaction")?; + let starting_rollup_height = if let Some(signed_transaction) = signed_transaction { + rollup_height_from_signed_transaction(&signed_transaction).wrap_err( + "failed to extract rollup height from last transaction by the bridge account", + )? + } else { + 1 + }; + Ok(starting_rollup_height) + } +} + +/// Extracts the rollup height from the last transaction by the bridge account on the sequencer. +/// Since all the withdrawals from a rollup block are batched into a single sequencer transaction, +/// he rollup height can be extracted from the memo of any withdrawal action in the batch. +/// +/// # Returns +/// +/// The rollup height of the last batch of withdrawals. +/// +/// # Errors +/// +/// 1. The last transaction by the bridge account did not contain a withdrawal action. +/// 2. The memo of the last transaction by the bridge account could not be parsed. +/// 3. The block number in the memo of the last transaction by the bridge account could not be +/// converted to a u64. +fn rollup_height_from_signed_transaction( + signed_transaction: &SignedTransaction, +) -> eyre::Result { + // find the last batch's rollup block height + let withdrawal_action = signed_transaction + .actions() + .iter() + .find(|action| matches!(action, Action::BridgeUnlock(_) | Action::Ics20Withdrawal(_))) + .ok_or_eyre("last transaction by the bridge account did not contain a withdrawal action")?; + + let last_batch_rollup_height = match withdrawal_action { + Action::BridgeUnlock(action) => { + let memo: BridgeUnlockMemo = serde_json::from_slice(&action.memo) + .wrap_err("failed to parse memo from last transaction by the bridge account")?; + Some(memo.block_number.as_u64()) + } + Action::Ics20Withdrawal(action) => { + let memo: Ics20WithdrawalFromRollupMemo = serde_json::from_str(&action.memo) + .wrap_err("failed to parse memo from last transaction by the bridge account")?; + Some(memo.block_number) + } + _ => None, + } + .expect("action is already checked to be either BridgeUnlock or Ics20Withdrawal"); + + info!( + last_batch.tx_hash = %telemetry::display::hex(&signed_transaction.sha256_of_proto_encoding()), + last_batch.rollup_height = last_batch_rollup_height, + "extracted rollup height from last batch of withdrawals", + ); + + Ok(last_batch_rollup_height) +} + +#[instrument(skip_all)] +async fn get_bridge_account_last_transaction_hash( + client: sequencer_client::HttpClient, + state: Arc, + address: Address, +) -> eyre::Result { + 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: &sequencer_client::extension_trait::Error| { + let state = Arc::clone(&state); + state.set_sequencer_connected(false); + + 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 last bridge account's transaction hash; retrying after \ + backoff", + ); + futures::future::ready(()) + }, + ); + + let res = tryhard::retry_fn(|| client.get_bridge_account_last_transaction_hash(address)) + .with_config(retry_config) + .await + .wrap_err( + "failed to fetch last bridge account's transaction hash from Sequencer after a lot of \ + attempts", + ); + + state.set_sequencer_connected(res.is_ok()); + + res +} + +#[instrument(skip_all)] +async fn get_tx( + client: sequencer_client::HttpClient, + state: Arc, + tx_hash: tendermint::Hash, +) -> eyre::Result { + 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 state = Arc::clone(&state); + state.set_sequencer_connected(false); + + 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 get transaction from Sequencer; retrying after backoff", + ); + futures::future::ready(()) + }, + ); + + let res = tryhard::retry_fn(|| client.tx(tx_hash, false)) + .with_config(retry_config) + .await + .wrap_err("failed to get transaction from Sequencer after a lot of attempts"); + + state.set_sequencer_connected(res.is_ok()); + + res +} + +#[instrument(skip_all)] +async fn get_sequencer_chain_id( + client: sequencer_client::HttpClient, + state: Arc, +) -> 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 state = Arc::clone(&state); + state.set_sequencer_connected(false); + + 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")?; + + state.set_sequencer_connected(true); + + Ok(genesis.chain_id) +} + +#[instrument(skip_all)] +async fn get_allowed_fee_asset_ids( + client: sequencer_client::HttpClient, + state: Arc, +) -> eyre::Result { + 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: &sequencer_client::extension_trait::Error| { + let state = Arc::clone(&state); + state.set_sequencer_connected(false); + + 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 allowed fee asset ids; retrying after backoff", + ); + futures::future::ready(()) + }, + ); + + let res = tryhard::retry_fn(|| client.get_allowed_fee_asset_ids()) + .with_config(retry_config) + .await + .wrap_err("failed to get allowed fee asset ids from Sequencer after a lot of attempts"); + + state.set_sequencer_connected(res.is_ok()); + + res +} + +#[instrument(skip_all)] +async fn get_latest_balance( + client: sequencer_client::HttpClient, + state: Arc, + address: Address, +) -> eyre::Result { + 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: &sequencer_client::extension_trait::Error| { + let state = Arc::clone(&state); + state.set_sequencer_connected(false); + + 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 get latest balance; retrying after backoff", + ); + futures::future::ready(()) + }, + ); + + let res = tryhard::retry_fn(|| client.get_latest_balance(address)) + .with_config(retry_config) + .await + .wrap_err("failed to get latest balance from Sequencer after a lot of attempts"); + + state.set_sequencer_connected(res.is_ok()); + + res +} diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/builder.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/builder.rs index 0731f0fb36..29e3212be9 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/builder.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/builder.rs @@ -1,65 +1,46 @@ use std::sync::Arc; -use astria_core::primitive::v1::asset; use astria_eyre::eyre::{ self, Context as _, }; -use tokio::sync::{ - mpsc, - oneshot, -}; +use tokio::sync::mpsc; use tokio_util::sync::CancellationToken; use tracing::info; use super::state::State; use crate::bridge_withdrawer::{ + startup, submitter::Batch, - SequencerStartupInfo, }; const BATCH_QUEUE_SIZE: usize = 256; pub(crate) struct Handle { - startup_info_rx: Option>, batches_tx: mpsc::Sender, } impl Handle { - pub(crate) fn new( - startup_info_rx: oneshot::Receiver, - batches_tx: mpsc::Sender, - ) -> Self { + pub(crate) fn new(batches_tx: mpsc::Sender) -> Self { Self { - startup_info_rx: Some(startup_info_rx), batches_tx, } } - pub(crate) async fn recv_startup_info(&mut self) -> eyre::Result { - self.startup_info_rx - .take() - .expect("startup info should only be taken once - this is a bug") - .await - .wrap_err("failed to get startup info from submitter. channel was dropped.") - } - pub(crate) async fn send_batch(&self, batch: Batch) -> eyre::Result<()> { self.batches_tx .send(batch) .await - .wrap_err("failed to send batch") + .wrap_err("failed submitter_handleto send batch") } } pub(crate) struct Builder { pub(crate) shutdown_token: CancellationToken, + pub(crate) startup_handle: startup::SubmitterHandle, pub(crate) sequencer_key_path: String, - pub(crate) sequencer_chain_id: String, pub(crate) sequencer_cometbft_endpoint: String, pub(crate) state: Arc, - pub(crate) expected_fee_asset_id: asset::Id, - pub(crate) min_expected_fee_asset_balance: u128, } impl Builder { @@ -67,12 +48,10 @@ impl Builder { pub(crate) fn build(self) -> eyre::Result<(super::Submitter, Handle)> { let Self { shutdown_token, + startup_handle, sequencer_key_path, - sequencer_chain_id, sequencer_cometbft_endpoint, state, - expected_fee_asset_id, - min_expected_fee_asset_balance, } = self; let signer = super::signer::SequencerKey::try_from_path(sequencer_key_path) @@ -84,20 +63,16 @@ impl Builder { .wrap_err("failed constructing cometbft http client")?; let (batches_tx, batches_rx) = tokio::sync::mpsc::channel(BATCH_QUEUE_SIZE); - let (startup_tx, startup_rx) = tokio::sync::oneshot::channel(); - let handle = Handle::new(startup_rx, batches_tx); + let handle = Handle::new(batches_tx); Ok(( super::Submitter { shutdown_token, + startup_handle, state, batches_rx, sequencer_cometbft_client, signer, - sequencer_chain_id, - startup_tx, - expected_fee_asset_id, - min_expected_fee_asset_balance, }, handle, )) diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/mod.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/mod.rs index ab5ecc3565..4f15d14456 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/mod.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/mod.rs @@ -3,53 +3,29 @@ use std::{ time::Duration, }; -use astria_core::{ - bridge::Ics20WithdrawalFromRollupMemo, - primitive::v1::asset, - protocol::{ - asset::v1alpha1::AllowedFeeAssetIdsResponse, - bridge::v1alpha1::BridgeAccountLastTxHashResponse, - transaction::v1alpha1::{ - Action, - TransactionParams, - UnsignedTransaction, - }, - }, +use astria_core::protocol::transaction::v1alpha1::{ + Action, + TransactionParams, + UnsignedTransaction, }; use astria_eyre::eyre::{ self, - ensure, eyre, Context, - OptionExt, }; pub(crate) use builder::Builder; pub(super) use builder::Handle; -use prost::Message as _; use sequencer_client::{ - tendermint_rpc::{ - self, - endpoint::broadcast::tx_commit, - }, + tendermint_rpc::endpoint::broadcast::tx_commit, Address, - BalanceResponse, SequencerClientExt, SignedTransaction, }; use signer::SequencerKey; use state::State; -use tendermint_rpc::{ - endpoint::tx, - Client, -}; use tokio::{ select, - sync::{ - mpsc, - oneshot::{ - self, - }, - }, + sync::mpsc, time::Instant, }; use tokio_util::sync::CancellationToken; @@ -66,38 +42,39 @@ use tracing::{ use super::{ batch::Batch, + startup, state, - SequencerStartupInfo, }; -use crate::bridge_withdrawer::ethereum::convert::BridgeUnlockMemo; +use crate::bridge_withdrawer::startup::SubmitterInfo as StartupInfo; mod builder; -mod signer; +pub(crate) mod signer; #[cfg(test)] mod tests; pub(super) struct Submitter { shutdown_token: CancellationToken, + startup_handle: startup::SubmitterHandle, state: Arc, batches_rx: mpsc::Receiver, sequencer_cometbft_client: sequencer_client::HttpClient, signer: SequencerKey, - sequencer_chain_id: String, - startup_tx: oneshot::Sender, - expected_fee_asset_id: asset::Id, - min_expected_fee_asset_balance: u128, } impl Submitter { pub(super) async fn run(mut self) -> eyre::Result<()> { - // call startup - let startup = self - .startup() - .await - .wrap_err("submitter failed to start up")?; - self.startup_tx - .send(startup) - .map_err(|_startup| eyre!("failed to send startup info to watcher"))?; + let sequencer_chain_id = select! { + () = self.shutdown_token.cancelled() => { + info!("submitter received shutdown signal while waiting for startup"); + return Ok(()); + } + + startup_info = self.startup_handle.recv() => { + let StartupInfo { sequencer_chain_id } = startup_info.wrap_err("submitter failed to get startup info")?; + self.state.set_submitter_ready(); + sequencer_chain_id + } + }; let reason = loop { select!( @@ -118,7 +95,7 @@ impl Submitter { self.sequencer_cometbft_client.clone(), &self.signer, self.state.clone(), - &self.sequencer_chain_id, + &sequencer_chain_id, actions, rollup_height, ).await { @@ -143,173 +120,6 @@ impl Submitter { Ok(()) } - - /// Confirms configuration values against the sequencer node and then syncs the next sequencer - /// nonce and rollup block according to the latest on-chain state. - /// - /// Configuration values checked: - /// - `self.chain_id` matches the value returned from the sequencer node's genesis - /// - `self.fee_asset_id` is a valid fee asset on the sequencer node - /// - `self.sequencer_key.address` has a sufficient balance of `self.fee_asset_id` - /// - /// Sync process: - /// - Fetch the last transaction hash by the bridge account from the sequencer - /// - Fetch the corresponding transaction - /// - Extract the last nonce used from the transaction - /// - Extract the rollup block height from the memo of one of the withdraw actions in the - /// transaction - /// - /// # Returns - /// A struct with the information collected and validated during startup: - /// - `fee_asset_id` - /// - `next_batch_rollup_height` - /// - /// # Errors - /// - /// - `self.chain_id` does not match the value returned from the sequencer node - /// - `self.fee_asset_id` is not a valid fee asset on the sequencer node - /// - `self.sequencer_key.address` does not have a sufficient balance of `self.fee_asset_id`. - async fn startup(&mut self) -> eyre::Result { - let actual_chain_id = - get_sequencer_chain_id(self.sequencer_cometbft_client.clone(), self.state.clone()) - .await - .wrap_err("failed to get chain id from sequencer")?; - ensure!( - self.sequencer_chain_id == actual_chain_id.to_string(), - "sequencer_chain_id provided in config does not match chain_id returned from sequencer" - ); - - // confirm that the fee asset ID is valid - let allowed_fee_asset_ids_resp = - get_allowed_fee_asset_ids(self.sequencer_cometbft_client.clone(), self.state.clone()) - .await - .wrap_err("failed to get allowed fee asset ids from sequencer")?; - ensure!( - allowed_fee_asset_ids_resp - .fee_asset_ids - .contains(&self.expected_fee_asset_id), - "fee_asset_id provided in config is not a valid fee asset on the sequencer" - ); - - // confirm that the sequencer key has a sufficient balance of the fee asset - let fee_asset_balances = get_latest_balance( - self.sequencer_cometbft_client.clone(), - self.state.clone(), - self.signer.address, - ) - .await - .wrap_err("failed to get latest balance")?; - let fee_asset_balance = fee_asset_balances - .balances - .into_iter() - .find(|balance| balance.denom.id() == self.expected_fee_asset_id) - .ok_or_eyre("withdrawer's account does not have the minimum balance of the fee asset")? - .balance; - ensure!( - fee_asset_balance >= self.min_expected_fee_asset_balance, - "sequencer key does not have a sufficient balance of the fee asset" - ); - - // sync to latest on-chain state - let next_batch_rollup_height = self - .get_next_rollup_height() - .await - .wrap_err("failed to get next rollup block height")?; - - self.state.set_submitter_ready(); - - // send startup info to watcher - let startup = SequencerStartupInfo { - fee_asset_id: self.expected_fee_asset_id, - next_batch_rollup_height, - }; - Ok(startup) - } - - /// Gets the data necessary for syncing to the latest on-chain state from the sequencer. Since - /// we batch all events from a given rollup block into a single sequencer transaction, we - /// get the last tx finalized by the bridge account on the sequencer and extract the rollup - /// height from it. - /// - /// The rollup height is extracted from the block height value in the memo of one of the actions - /// in the batch. - /// - /// # Returns - /// The next batch rollup height to process. - /// - /// # Errors - /// - /// 1. Failing to get and deserialize a valid last transaction by the bridge account from the - /// sequencer. - /// 2. The last transaction by the bridge account failed to execute (this should not happen in - /// the sequencer logic) - /// 3. The last transaction by the bridge account did not contain a withdrawal action - /// 4. The memo of the last transaction by the bridge account could not be parsed - async fn get_next_rollup_height(&mut self) -> eyre::Result { - let signed_transaction = self - .get_last_transaction() - .await - .wrap_err("failed to get the bridge account's last sequencer transaction")?; - let next_batch_rollup_height = if let Some(signed_transaction) = signed_transaction { - rollup_height_from_signed_transaction(&signed_transaction).wrap_err( - "failed to extract rollup height from last transaction by the bridge account", - )? - } else { - 1 - }; - Ok(next_batch_rollup_height) - } - - async fn get_last_transaction(&self) -> eyre::Result> { - // get last transaction hash by the bridge account, if it exists - let last_transaction_hash_resp = get_bridge_account_last_transaction_hash( - self.sequencer_cometbft_client.clone(), - self.state.clone(), - self.signer.address, - ) - .await - .wrap_err("failed to fetch last transaction hash by the bridge account")?; - - let Some(tx_hash) = last_transaction_hash_resp.tx_hash else { - return Ok(None); - }; - - let tx_hash = tendermint::Hash::try_from(tx_hash.to_vec()) - .wrap_err("failed to convert last transaction hash to tendermint hash")?; - - // get the corresponding transaction - let last_transaction = get_tx( - self.sequencer_cometbft_client.clone(), - self.state.clone(), - tx_hash, - ) - .await - .wrap_err("failed to fetch last transaction by the bridge account")?; - - // check that the transaction actually executed - ensure!( - last_transaction.tx_result.code == tendermint::abci::Code::Ok, - "last transaction by the bridge account failed to execute. this should not happen in \ - the sequencer logic." - ); - - let proto_tx = - astria_core::generated::protocol::transaction::v1alpha1::SignedTransaction::decode( - &*last_transaction.tx, - ) - .wrap_err("failed to convert transaction data from CometBFT to proto")?; - - let tx = SignedTransaction::try_from_raw(proto_tx) - .wrap_err("failed to convert transaction data from proto to SignedTransaction")?; - - info!( - last_bridge_account_tx.hash = %telemetry::display::hex(&tx_hash), - last_bridge_account_tx.height = i64::from(last_transaction.height), - "fetched last transaction by the bridge account" - ); - - Ok(Some(tx)) - } } async fn process_batch( @@ -496,232 +306,3 @@ async fn submit_tx( res } - -#[instrument(skip_all)] -async fn get_sequencer_chain_id( - client: sequencer_client::HttpClient, - state: Arc, -) -> 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 state = Arc::clone(&state); - state.set_sequencer_connected(false); - - 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")?; - - state.set_sequencer_connected(true); - - Ok(genesis.chain_id) -} - -#[instrument(skip_all)] -async fn get_allowed_fee_asset_ids( - client: sequencer_client::HttpClient, - state: Arc, -) -> eyre::Result { - 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: &sequencer_client::extension_trait::Error| { - let state = Arc::clone(&state); - state.set_sequencer_connected(false); - - 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 allowed fee asset ids; retrying after backoff", - ); - futures::future::ready(()) - }, - ); - - let res = tryhard::retry_fn(|| client.get_allowed_fee_asset_ids()) - .with_config(retry_config) - .await - .wrap_err("failed to get allowed fee asset ids from Sequencer after a lot of attempts"); - - state.set_sequencer_connected(res.is_ok()); - - res -} - -#[instrument(skip_all)] -async fn get_latest_balance( - client: sequencer_client::HttpClient, - state: Arc, - address: Address, -) -> eyre::Result { - 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: &sequencer_client::extension_trait::Error| { - let state = Arc::clone(&state); - state.set_sequencer_connected(false); - - 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 get latest balance; retrying after backoff", - ); - futures::future::ready(()) - }, - ); - - let res = tryhard::retry_fn(|| client.get_latest_balance(address)) - .with_config(retry_config) - .await - .wrap_err("failed to get latest balance from Sequencer after a lot of attempts"); - - state.set_sequencer_connected(res.is_ok()); - - res -} - -#[instrument(skip_all)] -async fn get_bridge_account_last_transaction_hash( - client: sequencer_client::HttpClient, - state: Arc, - address: Address, -) -> eyre::Result { - 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: &sequencer_client::extension_trait::Error| { - let state = Arc::clone(&state); - state.set_sequencer_connected(false); - - 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 last bridge account's transaction hash; retrying after \ - backoff", - ); - futures::future::ready(()) - }, - ); - - let res = tryhard::retry_fn(|| client.get_bridge_account_last_transaction_hash(address)) - .with_config(retry_config) - .await - .wrap_err( - "failed to fetch last bridge account's transaction hash from Sequencer after a lot of \ - attempts", - ); - - state.set_sequencer_connected(res.is_ok()); - - res -} - -#[instrument(skip_all)] -async fn get_tx( - client: sequencer_client::HttpClient, - state: Arc, - tx_hash: tendermint::Hash, -) -> eyre::Result { - 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 state = Arc::clone(&state); - state.set_sequencer_connected(false); - - 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 get transaction from Sequencer; retrying after backoff", - ); - futures::future::ready(()) - }, - ); - - let res = tryhard::retry_fn(|| client.tx(tx_hash, false)) - .with_config(retry_config) - .await - .wrap_err("failed to get transaction from Sequencer after a lot of attempts"); - - state.set_sequencer_connected(res.is_ok()); - - res -} - -fn rollup_height_from_signed_transaction( - signed_transaction: &SignedTransaction, -) -> eyre::Result { - // find the last batch's rollup block height - let withdrawal_action = signed_transaction - .actions() - .iter() - .find(|action| matches!(action, Action::BridgeUnlock(_) | Action::Ics20Withdrawal(_))) - .ok_or_eyre("last transaction by the bridge account did not contain a withdrawal action")?; - - let last_batch_rollup_height = match withdrawal_action { - Action::BridgeUnlock(action) => { - let memo: BridgeUnlockMemo = serde_json::from_slice(&action.memo) - .wrap_err("failed to parse memo from last transaction by the bridge account")?; - Some(memo.block_number.as_u64()) - } - Action::Ics20Withdrawal(action) => { - let memo: Ics20WithdrawalFromRollupMemo = serde_json::from_str(&action.memo) - .wrap_err("failed to parse memo from last transaction by the bridge account")?; - Some(memo.block_number) - } - _ => None, - } - .expect("action is already checked to be either BridgeUnlock or Ics20Withdrawal"); - - info!( - last_batch.tx_hash = %telemetry::display::hex(&signed_transaction.sha256_of_proto_encoding()), - last_batch.rollup_height = last_batch_rollup_height, - "extracted rollup height from last batch of withdrawals", - ); - - Ok(last_batch_rollup_height) -} diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/signer.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/signer.rs index 746a2f35dd..e44f12d7be 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/signer.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/signer.rs @@ -16,16 +16,16 @@ use astria_eyre::eyre::{ WrapErr as _, }; -pub(super) struct SequencerKey { - pub(super) address: Address, - pub(super) signing_key: SigningKey, +pub(crate) struct SequencerKey { + pub(crate) address: Address, + pub(crate) signing_key: SigningKey, } impl SequencerKey { /// Construct a `SequencerKey` from a file. /// /// The file should contain a hex-encoded ed25519 secret key. - pub(super) fn try_from_path>(path: P) -> eyre::Result { + pub(crate) fn try_from_path>(path: P) -> eyre::Result { let hex = fs::read_to_string(path).wrap_err("failed to read sequencer key from path")?; let bytes: [u8; 32] = hex::decode(hex.trim()) .wrap_err("failed to decode hex")? From 45552bc5958ad3a4de9f707c0320de40bf6afb28 Mon Sep 17 00:00:00 2001 From: itamar Date: Mon, 17 Jun 2024 18:50:29 -0400 Subject: [PATCH 06/32] cleanup tests --- .../src/bridge_withdrawer/startup.rs | 10 +- .../src/bridge_withdrawer/submitter/tests.rs | 230 ++++++++---------- 2 files changed, 112 insertions(+), 128 deletions(-) diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs index 846d0712cc..fcf9cf8ac5 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs @@ -97,10 +97,12 @@ impl Builder { } } +#[derive(Debug)] pub(super) struct SubmitterInfo { pub(super) sequencer_chain_id: String, } +#[derive(Debug)] pub(super) struct SubmitterHandle { info_rx: Option>, } @@ -121,11 +123,13 @@ impl SubmitterHandle { } } +#[derive(Debug)] pub(super) struct WatcherInfo { pub(super) fee_asset_id: asset::Id, pub(super) starting_rollup_height: u64, } +#[derive(Debug)] pub(super) struct WatcherHandle { info_rx: Option>, } @@ -192,7 +196,7 @@ impl Startup { }); tokio::select!( - _ = shutdown_token.cancelled() => { + () = shutdown_token.cancelled() => { Err(eyre!("startup was cancelled")) } res = startup_task => { @@ -278,8 +282,8 @@ impl Startup { /// 3. Failing to fetch the last transaction by the bridge account. /// 4. The last transaction by the bridge account failed to execute (this should not happen /// in the sequencer logic). - /// 5. Failing to convert the transaction data from CometBFT to proto. - /// 6. Failing to convert the transaction data from proto to SignedTransaction. + /// 5. Failing to convert the transaction data from bytes to proto. + /// 6. Failing to convert the transaction data from proto to `SignedTransaction`. async fn get_last_transaction(&self) -> eyre::Result> { // get last transaction hash by the bridge account, if it exists let last_transaction_hash_resp = get_bridge_account_last_transaction_hash( diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/tests.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/tests.rs index 2839ecba70..0eeb650f25 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/tests.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/tests.rs @@ -1,5 +1,4 @@ use std::{ - collections::HashMap, io::Write as _, sync::Arc, time::Duration, @@ -8,7 +7,6 @@ use std::{ use astria_core::{ bridge::Ics20WithdrawalFromRollupMemo, - crypto::SigningKey, generated::protocol::account::v1alpha1::NonceResponse, primitive::v1::{ asset::{ @@ -28,14 +26,11 @@ use astria_core::{ Ics20Withdrawal, }, Action, - TransactionParams, - UnsignedTransaction, }, }, }; use astria_eyre::eyre::{ self, - Context, }; use ibc_types::core::client::Height as IbcHeight; use once_cell::sync::Lazy; @@ -51,7 +46,6 @@ use serde_json::json; use tempfile::NamedTempFile; use tendermint::{ abci::{ - self, response::CheckTx, types::ExecTxResult, }, @@ -65,7 +59,10 @@ use tendermint_rpc::{ }, request, }; -use tokio::task::JoinHandle; +use tokio::{ + sync::oneshot, + task::JoinHandle, +}; use tokio_util::sync::CancellationToken; use tracing::debug; use wiremock::{ @@ -84,14 +81,15 @@ use super::Submitter; use crate::bridge_withdrawer::{ batch::Batch, ethereum::convert::BridgeUnlockMemo, + startup, state, submitter, }; const SEQUENCER_CHAIN_ID: &str = "test_sequencer-1000"; const DEFAULT_LAST_ROLLUP_HEIGHT: u64 = 1; -const DEFAULT_LAST_SEQUENCER_HEIGHT: u64 = 0; -const DEFAULT_SEQUENCER_NONCE: u32 = 0; +// const DEFAULT_LAST_SEQUENCER_HEIGHT: u64 = 0; +// const DEFAULT_SEQUENCER_NONCE: u32 = 0; const DEFAULT_IBC_DENOM: &str = "transfer/channel-0/utia"; static TELEMETRY: Lazy<()> = Lazy::new(|| { @@ -116,6 +114,7 @@ static TELEMETRY: Lazy<()> = Lazy::new(|| { struct TestSubmitter { submitter: Option, submitter_handle: submitter::Handle, + startup_tx: Option>, cometbft_mock: MockServer, submitter_task_handle: Option>>, } @@ -144,15 +143,15 @@ impl TestSubmitter { let state = Arc::new(state::State::new()); // not testing watcher here so just set it to ready state.set_watcher_ready(); + let (startup_tx, startup_rx) = oneshot::channel(); + let startup_handle = startup::SubmitterHandle::new(startup_rx); let (submitter, submitter_handle) = submitter::Builder { shutdown_token: shutdown_token.clone(), + startup_handle, sequencer_key_path, - sequencer_chain_id: SEQUENCER_CHAIN_ID.to_string(), sequencer_cometbft_endpoint, state, - expected_fee_asset_id: default_native_asset().id(), - min_expected_fee_asset_balance: 1_000_000, } .build() .unwrap(); @@ -160,28 +159,26 @@ impl TestSubmitter { Self { submitter: Some(submitter), submitter_task_handle: None, + startup_tx: Some(startup_tx), submitter_handle, cometbft_mock, } } - async fn startup_and_spawn_with_guards(&mut self, startup_guards: HashMap) { + async fn startup(&mut self) { let submitter = self.submitter.take().unwrap(); let mut state = submitter.state.subscribe(); self.submitter_task_handle = Some(tokio::spawn(submitter.run())); - // wait for all startup guards to be satisfied - for (name, guard) in startup_guards { - tokio::time::timeout(Duration::from_millis(100), guard.wait_until_satisfied()) - .await - .wrap_err(format!("{name} guard not satisfied in time.")) - .unwrap(); - } - - // consume the startup info in place of the watcher - self.submitter_handle.recv_startup_info().await.unwrap(); + self.startup_tx + .take() + .expect("should only send startup info once") + .send(startup::SubmitterInfo { + sequencer_chain_id: SEQUENCER_CHAIN_ID.to_string(), + }) + .unwrap(); // wait for the submitter to be ready state @@ -190,38 +187,26 @@ impl TestSubmitter { .unwrap(); } - async fn startup_and_spawn(&mut self) { - let startup_guards = register_startup_guards(&self.cometbft_mock).await; - let sync_guards = register_sync_guards(&self.cometbft_mock).await; - self.startup_and_spawn_with_guards( - startup_guards - .into_iter() - .chain(sync_guards.into_iter()) - .collect(), - ) - .await; - } - async fn spawn() -> Self { let mut submitter = Self::setup().await; - submitter.startup_and_spawn().await; + submitter.startup().await; submitter } } -async fn register_default_chain_id_guard(cometbft_mock: &MockServer) -> MockGuard { - register_genesis_chain_id_response(SEQUENCER_CHAIN_ID, cometbft_mock).await +async fn _register_default_chain_id_guard(cometbft_mock: &MockServer) -> MockGuard { + _register_genesis_chain_id_response(SEQUENCER_CHAIN_ID, cometbft_mock).await } -async fn register_default_fee_asset_ids_guard(cometbft_mock: &MockServer) -> MockGuard { +async fn _register_default_fee_asset_ids_guard(cometbft_mock: &MockServer) -> MockGuard { let fee_asset_ids = vec![default_native_asset().id()]; - register_allowed_fee_asset_ids_response(fee_asset_ids, cometbft_mock).await + _register_allowed_fee_asset_ids_response(fee_asset_ids, cometbft_mock).await } -async fn register_default_min_expected_fee_asset_balance_guard( +async fn _register_default_min_expected_fee_asset_balance_guard( cometbft_mock: &MockServer, ) -> MockGuard { - register_get_latest_balance( + _register_get_latest_balance( vec![AssetBalance { denom: default_native_asset(), balance: 1_000_000u128, @@ -231,43 +216,38 @@ async fn register_default_min_expected_fee_asset_balance_guard( .await } -async fn register_default_last_bridge_tx_hash_guard(cometbft_mock: &MockServer) -> MockGuard { - register_last_bridge_tx_hash_guard(cometbft_mock, make_last_bridge_tx_hash_response()).await -} - -async fn register_default_last_bridge_tx_guard(cometbft_mock: &MockServer) -> MockGuard { - register_tx_guard(cometbft_mock, make_tx_response()).await -} - -async fn register_startup_guards(cometbft_mock: &MockServer) -> HashMap { - HashMap::from([ - ( - "chain_id".to_string(), - register_default_chain_id_guard(cometbft_mock).await, - ), - ( - "fee_asset_ids".to_string(), - register_default_fee_asset_ids_guard(cometbft_mock).await, - ), - ( - "min_expected_fee_asset_balance".to_string(), - register_default_min_expected_fee_asset_balance_guard(cometbft_mock).await, - ), - ]) -} - -async fn register_sync_guards(cometbft_mock: &MockServer) -> HashMap { - HashMap::from([ - ( - "tx_hash".to_string(), - register_default_last_bridge_tx_hash_guard(cometbft_mock).await, - ), - ( - "last_bridge_tx".to_string(), - register_default_last_bridge_tx_guard(cometbft_mock).await, - ), - ]) -} +// async fn _register_default_last_bridge_tx_hash_guard(cometbft_mock: &MockServer) -> MockGuard { +// _register_last_bridge_tx_hash_guard(cometbft_mock, +// _make_last_bridge_tx_hash_response()).await } + +// async fn _register_default_last_bridge_tx_guard(cometbft_mock: &MockServer) -> MockGuard { +// _register_tx_guard(cometbft_mock, _make_tx_response()).await +// } + +// async fn _register_startup_guards(cometbft_mock: &MockServer) -> HashMap { +// HashMap::from([ +// ( +// "chain_id".to_string(), +// _register_default_chain_id_guard(cometbft_mock).await, +// ), +// ( +// "fee_asset_ids".to_string(), +// _register_default_fee_asset_ids_guard(cometbft_mock).await, +// ), +// ( +// "min_expected_fee_asset_balance".to_string(), +// _register_default_min_expected_fee_asset_balance_guard(cometbft_mock).await, +// ), +// ( +// "tx_hash".to_string(), +// _register_default_last_bridge_tx_hash_guard(cometbft_mock).await, +// ), +// ( +// "last_bridge_tx".to_string(), +// _register_default_last_bridge_tx_guard(cometbft_mock).await, +// ), +// ]) +// } fn make_ics20_withdrawal_action() -> Action { let denom = DEFAULT_IBC_DENOM.parse::().unwrap(); @@ -358,47 +338,47 @@ fn make_tx_commit_deliver_tx_failure_response() -> tx_commit::Response { } } -fn make_last_bridge_tx_hash_response() -> BridgeAccountLastTxHashResponse { - BridgeAccountLastTxHashResponse { - height: DEFAULT_LAST_ROLLUP_HEIGHT, - tx_hash: Some([0u8; 32]), - } -} - -fn make_signed_bridge_transaction() -> SignedTransaction { - let alice_secret_bytes: [u8; 32] = - hex::decode("2bd806c97f0e00af1a1fc3328fa763a9269723c8db8fac4f93af71db186d6e90") - .unwrap() - .try_into() - .unwrap(); - let alice_key = SigningKey::from(alice_secret_bytes); - - let actions = vec![make_bridge_unlock_action(), make_ics20_withdrawal_action()]; - UnsignedTransaction { - params: TransactionParams::builder() - .nonce(DEFAULT_SEQUENCER_NONCE) - .chain_id(SEQUENCER_CHAIN_ID) - .try_build() - .unwrap(), - actions, - } - .into_signed(&alice_key) -} - -fn make_tx_response() -> tx::Response { - let tx = make_signed_bridge_transaction(); - tx::Response { - hash: tx.sha256_of_proto_encoding().to_vec().try_into().unwrap(), - height: DEFAULT_LAST_SEQUENCER_HEIGHT.try_into().unwrap(), - index: 0, - tx_result: ExecTxResult { - code: abci::Code::Ok, - ..ExecTxResult::default() - }, - tx: tx.into_raw().encode_to_vec(), - proof: None, - } -} +// fn _make_last_bridge_tx_hash_response() -> BridgeAccountLastTxHashResponse { +// BridgeAccountLastTxHashResponse { +// height: DEFAULT_LAST_ROLLUP_HEIGHT, +// tx_hash: Some([0u8; 32]), +// } +// } + +// fn _make_signed_bridge_transaction() -> SignedTransaction { +// let alice_secret_bytes: [u8; 32] = +// hex::decode("2bd806c97f0e00af1a1fc3328fa763a9269723c8db8fac4f93af71db186d6e90") +// .unwrap() +// .try_into() +// .unwrap(); +// let alice_key = SigningKey::from(alice_secret_bytes); + +// let actions = vec![make_bridge_unlock_action(), make_ics20_withdrawal_action()]; +// UnsignedTransaction { +// params: TransactionParams::builder() +// .nonce(DEFAULT_SEQUENCER_NONCE) +// .chain_id(SEQUENCER_CHAIN_ID) +// .try_build() +// .unwrap(), +// actions, +// } +// .into_signed(&alice_key) +// } + +// fn _make_tx_response() -> tx::Response { +// let tx = _make_signed_bridge_transaction(); +// tx::Response { +// hash: tx.sha256_of_proto_encoding().to_vec().try_into().unwrap(), +// height: DEFAULT_LAST_SEQUENCER_HEIGHT.try_into().unwrap(), +// index: 0, +// tx_result: ExecTxResult { +// code: abci::Code::Ok, +// ..ExecTxResult::default() +// }, +// tx: tx.into_raw().encode_to_vec(), +// proof: None, +// } +// } /// Convert a `Request` object to a `SignedTransaction` fn signed_tx_from_request(request: &Request) -> SignedTransaction { @@ -417,7 +397,7 @@ fn signed_tx_from_request(request: &Request) -> SignedTransaction { signed_tx } -async fn register_genesis_chain_id_response(chain_id: &str, server: &MockServer) -> MockGuard { +async fn _register_genesis_chain_id_response(chain_id: &str, server: &MockServer) -> MockGuard { use tendermint::{ consensus::{ params::{ @@ -470,7 +450,7 @@ async fn register_genesis_chain_id_response(chain_id: &str, server: &MockServer) .await } -async fn register_allowed_fee_asset_ids_response( +async fn _register_allowed_fee_asset_ids_response( fee_asset_ids: Vec, cometbft_mock: &MockServer, ) -> MockGuard { @@ -498,7 +478,7 @@ async fn register_allowed_fee_asset_ids_response( .await } -async fn register_get_latest_balance( +async fn _register_get_latest_balance( balances: Vec, server: &MockServer, ) -> MockGuard { @@ -527,7 +507,7 @@ async fn register_get_latest_balance( .await } -async fn register_last_bridge_tx_hash_guard( +async fn _register_last_bridge_tx_hash_guard( server: &MockServer, response: BridgeAccountLastTxHashResponse, ) -> MockGuard { @@ -570,7 +550,7 @@ async fn register_get_nonce_response(server: &MockServer, response: NonceRespons .await } -async fn register_tx_guard(server: &MockServer, response: tx::Response) -> MockGuard { +async fn _register_tx_guard(server: &MockServer, response: tx::Response) -> MockGuard { let wrapper = response::Wrapper::new_with_id(tendermint_rpc::Id::Num(1), Some(response), None); Mock::given(body_partial_json(json!({"method": "tx"}))) .respond_with( From dc5109f418fb03f20173c8347bd0e3c250da43d1 Mon Sep 17 00:00:00 2001 From: itamar Date: Mon, 24 Jun 2024 18:29:30 -0400 Subject: [PATCH 07/32] address review comments --- .../src/bridge_withdrawer/ethereum/watcher.rs | 2 +- .../src/bridge_withdrawer/startup.rs | 6 +- .../src/bridge_withdrawer/submitter/tests.rs | 77 ------------------- 3 files changed, 4 insertions(+), 81 deletions(-) diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/watcher.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/watcher.rs index 7497991cfb..0380218b87 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/watcher.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/watcher.rs @@ -205,7 +205,7 @@ impl Watcher { } = select! { () = self.shutdown_token.cancelled() => { info!("watcher received shutdown signal while waiting for startup"); - return Err(eyre!("watcher shutting down")); + return Err(eyre!("watcher received shutdown signal while waiting for startup")); } startup_info = self.startup_handle.recv() => { diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs index baf82ad504..0ae2ccf1d7 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs @@ -259,11 +259,11 @@ impl Startup { .balances .into_iter() .find(|balance| balance.denom.id() == self.expected_fee_asset_id) - .ok_or_eyre("withdrawer's account does not have the minimum balance of the fee asset")? + .ok_or_eyre("withdrawer's account balance of the fee asset is zero")? .balance; ensure!( fee_asset_balance >= self.expected_min_fee_asset_balance, - "sequencer key does not have a sufficient balance of the fee asset" + "withdrawer account does not have a sufficient balance of the fee asset" ); Ok(()) @@ -362,7 +362,7 @@ impl Startup { let starting_rollup_height = if let Some(signed_transaction) = signed_transaction { rollup_height_from_signed_transaction(&signed_transaction).wrap_err( "failed to extract rollup height from last transaction by the bridge account", - )? + )? + 1 } else { 1 }; diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/tests.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/tests.rs index 627de225bc..fed608e619 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/tests.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/tests.rs @@ -91,8 +91,6 @@ use crate::{ const SEQUENCER_CHAIN_ID: &str = "test_sequencer-1000"; const DEFAULT_LAST_ROLLUP_HEIGHT: u64 = 1; -// const DEFAULT_LAST_SEQUENCER_HEIGHT: u64 = 0; -// const DEFAULT_SEQUENCER_NONCE: u32 = 0; const DEFAULT_IBC_DENOM: &str = "transfer/channel-0/utia"; static TELEMETRY: Lazy<()> = Lazy::new(|| { @@ -222,39 +220,6 @@ async fn _register_default_min_expected_fee_asset_balance_guard( .await } -// async fn _register_default_last_bridge_tx_hash_guard(cometbft_mock: &MockServer) -> MockGuard { -// _register_last_bridge_tx_hash_guard(cometbft_mock, -// _make_last_bridge_tx_hash_response()).await } - -// async fn _register_default_last_bridge_tx_guard(cometbft_mock: &MockServer) -> MockGuard { -// _register_tx_guard(cometbft_mock, _make_tx_response()).await -// } - -// async fn _register_startup_guards(cometbft_mock: &MockServer) -> HashMap { -// HashMap::from([ -// ( -// "chain_id".to_string(), -// _register_default_chain_id_guard(cometbft_mock).await, -// ), -// ( -// "fee_asset_ids".to_string(), -// _register_default_fee_asset_ids_guard(cometbft_mock).await, -// ), -// ( -// "min_expected_fee_asset_balance".to_string(), -// _register_default_min_expected_fee_asset_balance_guard(cometbft_mock).await, -// ), -// ( -// "tx_hash".to_string(), -// _register_default_last_bridge_tx_hash_guard(cometbft_mock).await, -// ), -// ( -// "last_bridge_tx".to_string(), -// _register_default_last_bridge_tx_guard(cometbft_mock).await, -// ), -// ]) -// } - fn make_ics20_withdrawal_action() -> Action { let denom = DEFAULT_IBC_DENOM.parse::().unwrap(); let destination_chain_address = "address".to_string(); @@ -344,48 +309,6 @@ fn make_tx_commit_deliver_tx_failure_response() -> tx_commit::Response { } } -// fn _make_last_bridge_tx_hash_response() -> BridgeAccountLastTxHashResponse { -// BridgeAccountLastTxHashResponse { -// height: DEFAULT_LAST_ROLLUP_HEIGHT, -// tx_hash: Some([0u8; 32]), -// } -// } - -// fn _make_signed_bridge_transaction() -> SignedTransaction { -// let alice_secret_bytes: [u8; 32] = -// hex::decode("2bd806c97f0e00af1a1fc3328fa763a9269723c8db8fac4f93af71db186d6e90") -// .unwrap() -// .try_into() -// .unwrap(); -// let alice_key = SigningKey::from(alice_secret_bytes); - -// let actions = vec![make_bridge_unlock_action(), make_ics20_withdrawal_action()]; -// UnsignedTransaction { -// params: TransactionParams::builder() -// .nonce(DEFAULT_SEQUENCER_NONCE) -// .chain_id(SEQUENCER_CHAIN_ID) -// .try_build() -// .unwrap(), -// actions, -// } -// .into_signed(&alice_key) -// } - -// fn _make_tx_response() -> tx::Response { -// let tx = _make_signed_bridge_transaction(); -// tx::Response { -// hash: tx.sha256_of_proto_encoding().to_vec().try_into().unwrap(), -// height: DEFAULT_LAST_SEQUENCER_HEIGHT.try_into().unwrap(), -// index: 0, -// tx_result: ExecTxResult { -// code: abci::Code::Ok, -// ..ExecTxResult::default() -// }, -// tx: tx.into_raw().encode_to_vec(), -// proof: None, -// } -// } - /// Convert a `Request` object to a `SignedTransaction` fn signed_tx_from_request(request: &Request) -> SignedTransaction { use astria_core::generated::protocol::transaction::v1alpha1::SignedTransaction as RawSignedTransaction; From 7daee501ba85fa315b4da0bbec9d70c6a324578d Mon Sep 17 00:00:00 2001 From: itamar Date: Mon, 24 Jun 2024 18:41:03 -0400 Subject: [PATCH 08/32] fix clippy --- .../src/bridge_withdrawer/startup.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs index 0ae2ccf1d7..c8408b3342 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs @@ -17,6 +17,7 @@ use astria_eyre::eyre::{ ensure, eyre, Context as _, + ContextCompat, OptionExt as _, }; use prost::Message as _; @@ -360,9 +361,12 @@ impl Startup { .await .wrap_err("failed to get the bridge account's last sequencer transaction")?; let starting_rollup_height = if let Some(signed_transaction) = signed_transaction { - rollup_height_from_signed_transaction(&signed_transaction).wrap_err( - "failed to extract rollup height from last transaction by the bridge account", - )? + 1 + rollup_height_from_signed_transaction(&signed_transaction) + .wrap_err( + "failed to extract rollup height from last transaction by the bridge account", + )? + .checked_add(1) + .wrap_err("failed to increment rollup height by 1")? } else { 1 }; From d73d66fb60f413b5077631cfb80fa2d3fe16571f Mon Sep 17 00:00:00 2001 From: itamar Date: Fri, 28 Jun 2024 16:36:08 -0400 Subject: [PATCH 09/32] simplify startup handle --- .../src/bridge_withdrawer/ethereum/watcher.rs | 75 +++++----- .../src/bridge_withdrawer/startup.rs | 133 +++++++----------- .../src/bridge_withdrawer/state.rs | 18 +++ .../bridge_withdrawer/submitter/builder.rs | 2 +- .../src/bridge_withdrawer/submitter/tests.rs | 11 +- 5 files changed, 110 insertions(+), 129 deletions(-) diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/watcher.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/watcher.rs index 0e1585d208..e28cbbe471 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/watcher.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/watcher.rs @@ -53,14 +53,13 @@ use crate::bridge_withdrawer::{ }, }, startup, - startup::WatcherInfo as StartupInfo, state::State, submitter, }; pub(crate) struct Builder { pub(crate) shutdown_token: CancellationToken, - pub(crate) startup_handle: startup::WatcherHandle, + pub(crate) startup_handle: startup::InfoHandle, pub(crate) ethereum_contract_address: String, pub(crate) ethereum_rpc_endpoint: String, pub(crate) state: Arc, @@ -75,8 +74,8 @@ impl Builder { let Builder { ethereum_contract_address, ethereum_rpc_endpoint, - startup_handle, shutdown_token, + startup_handle, state, rollup_asset_denom, bridge_address, @@ -100,11 +99,11 @@ impl Builder { Ok(Watcher { contract_address, ethereum_rpc_endpoint: ethereum_rpc_endpoint.to_string(), - startup_handle, rollup_asset_denom, bridge_address, state, shutdown_token: shutdown_token.clone(), + startup_handle, submitter_handle, sequencer_address_prefix, }) @@ -114,7 +113,7 @@ impl Builder { /// Watches for withdrawal events emitted by the `AstriaWithdrawer` contract. pub(crate) struct Watcher { shutdown_token: CancellationToken, - startup_handle: startup::WatcherHandle, + startup_handle: startup::InfoHandle, submitter_handle: submitter::Handle, contract_address: ethers::types::Address, ethereum_rpc_endpoint: String, @@ -205,16 +204,17 @@ impl Watcher { u128, u64, )> { - let StartupInfo { + let startup::Info { fee_asset_id, starting_rollup_height, + .. } = select! { () = self.shutdown_token.cancelled() => { info!("watcher received shutdown signal while waiting for startup"); return Err(eyre!("watcher received shutdown signal while waiting for startup")); } - startup_info = self.startup_handle.recv() => { + startup_info = self.startup_handle.get_info() => { startup_info.wrap_err("failed to receive startup info")? } }; @@ -468,7 +468,6 @@ mod tests { }, utils::hex, }; - use tokio::sync::oneshot; use super::*; use crate::bridge_withdrawer::ethereum::{ @@ -586,15 +585,14 @@ mod tests { panic!("expected action to be BridgeUnlock, got {expected_action:?}"); }; + let state = Arc::new(State::new()); + let startup_handle = startup::InfoHandle::new(state.subscribe()); + state.set_startup_info(startup::Info { + starting_rollup_height: 1, + fee_asset_id: denom.id(), + chain_id: "astria".to_string(), + }); let (batch_tx, mut batch_rx) = mpsc::channel(100); - let (startup_tx, startup_rx) = oneshot::channel(); - let startup_handle = startup::WatcherHandle::new(startup_rx); - startup_tx - .send(StartupInfo { - fee_asset_id: denom.id(), - starting_rollup_height: 0, - }) - .unwrap(); let watcher = Builder { ethereum_contract_address: hex::encode(contract_address), @@ -686,15 +684,14 @@ mod tests { }; expected_action.timeout_time = 0; // zero this for testing + let state = Arc::new(State::new()); + let startup_handle = startup::InfoHandle::new(state.subscribe()); + state.set_startup_info(startup::Info { + starting_rollup_height: 1, + fee_asset_id: denom.id(), + chain_id: "astria".to_string(), + }); let (batch_tx, mut batch_rx) = mpsc::channel(100); - let (startup_tx, startup_rx) = oneshot::channel(); - let startup_handle = startup::WatcherHandle::new(startup_rx); - startup_tx - .send(StartupInfo { - fee_asset_id: denom.id(), - starting_rollup_height: 0, - }) - .unwrap(); let watcher = Builder { ethereum_contract_address: hex::encode(contract_address), @@ -813,15 +810,14 @@ mod tests { panic!("expected action to be BridgeUnlock, got {expected_action:?}"); }; + let state = Arc::new(State::new()); + let startup_handle = startup::InfoHandle::new(state.subscribe()); + state.set_startup_info(startup::Info { + starting_rollup_height: 1, + fee_asset_id: denom.id(), + chain_id: "astria".to_string(), + }); let (batch_tx, mut batch_rx) = mpsc::channel(100); - let (startup_tx, startup_rx) = oneshot::channel(); - let startup_handle = startup::WatcherHandle::new(startup_rx); - startup_tx - .send(StartupInfo { - fee_asset_id: denom.id(), - starting_rollup_height: 0, - }) - .unwrap(); let watcher = Builder { ethereum_contract_address: hex::encode(contract_address), @@ -923,15 +919,14 @@ mod tests { }; expected_action.timeout_time = 0; // zero this for testing + let state = Arc::new(State::new()); + let startup_handle = startup::InfoHandle::new(state.subscribe()); + state.set_startup_info(startup::Info { + starting_rollup_height: 1, + fee_asset_id: denom.id(), + chain_id: "astria".to_string(), + }); let (batch_tx, mut batch_rx) = mpsc::channel(100); - let (startup_tx, startup_rx) = oneshot::channel(); - let startup_handle = startup::WatcherHandle::new(startup_rx); - startup_tx - .send(StartupInfo { - fee_asset_id: denom.id(), - starting_rollup_height: 0, - }) - .unwrap(); let watcher = Builder { ethereum_contract_address: hex::encode(contract_address), diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs index 7f72d82fbe..210497149e 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs @@ -32,7 +32,7 @@ use tendermint_rpc::{ endpoint::tx, Client as _, }; -use tokio::sync::oneshot; +use tokio::sync::watch; use tokio_util::sync::CancellationToken; use tracing::{ error, @@ -41,7 +41,10 @@ use tracing::{ warn, }; -use super::state::State; +use super::state::{ + self, + State, +}; use crate::bridge_withdrawer::ethereum::convert::BridgeUnlockMemo; pub(super) struct Builder { @@ -56,7 +59,7 @@ pub(super) struct Builder { } impl Builder { - pub(super) fn build(self) -> eyre::Result<(Startup, SubmitterHandle, WatcherHandle)> { + pub(super) fn build(self) -> eyre::Result { let Self { shutdown_token, state, @@ -71,86 +74,53 @@ impl Builder { sequencer_client::HttpClient::new(&*sequencer_cometbft_endpoint) .wrap_err("failed constructing cometbft http client")?; - let (submitter_tx, submitter_rx) = oneshot::channel(); - let submitter_handle = SubmitterHandle::new(submitter_rx); - - let (watcher_tx, watcher_rx) = oneshot::channel(); - let watcher_handle = WatcherHandle::new(watcher_rx); - - let startup = Startup { + Ok(Startup { shutdown_token, state, - submitter_tx, - watcher_tx, sequencer_chain_id, sequencer_cometbft_client, sequencer_bridge_address, expected_fee_asset_id, expected_min_fee_asset_balance, - }; - - Ok((startup, submitter_handle, watcher_handle)) + }) } } -#[derive(Debug)] -pub(super) struct SubmitterInfo { - pub(super) sequencer_chain_id: String, -} - -#[derive(Debug)] -pub(super) struct SubmitterHandle { - info_rx: Option>, -} - -impl SubmitterHandle { - pub(super) fn new(info_rx: oneshot::Receiver) -> Self { - Self { - info_rx: Some(info_rx), - } - } - - pub(super) async fn recv(&mut self) -> eyre::Result { - self.info_rx - .take() - .expect("startup info should only be taken once - this is a bug") - .await - .wrap_err("failed to get startup info from submitter. channel was dropped.") - } -} - -#[derive(Debug)] -pub(super) struct WatcherInfo { - pub(super) fee_asset_id: asset::Id, +#[derive(Debug, Clone, PartialEq, Eq, serde::Serialize)] +pub(super) struct Info { pub(super) starting_rollup_height: u64, + pub(super) fee_asset_id: asset::Id, + pub(super) chain_id: String, } -#[derive(Debug)] -pub(super) struct WatcherHandle { - info_rx: Option>, +pub(super) struct InfoHandle { + rx: watch::Receiver, } -impl WatcherHandle { - pub(super) fn new(info_rx: oneshot::Receiver) -> Self { +impl InfoHandle { + pub(super) fn new(rx: watch::Receiver) -> Self { Self { - info_rx: Some(info_rx), + rx, } } - pub(super) async fn recv(&mut self) -> eyre::Result { - self.info_rx - .take() - .expect("startup info should only be taken once - this is a bug") + pub(super) async fn get_info(&mut self) -> eyre::Result { + let state = self + .rx + .wait_for(|state| state.get_startup_info().is_some()) .await - .wrap_err("failed to get startup info from watcher. channel was dropped.") + .wrap_err("")?; + + Ok(state + .get_startup_info() + .expect("the previous line guarantes that the state is intialized") + .clone()) } } pub(super) struct Startup { shutdown_token: CancellationToken, state: Arc, - submitter_tx: oneshot::Sender, - watcher_tx: oneshot::Sender, sequencer_chain_id: String, sequencer_cometbft_client: sequencer_client::HttpClient, sequencer_bridge_address: Address, @@ -162,33 +132,28 @@ impl Startup { pub(super) async fn run(mut self) -> eyre::Result<()> { let shutdown_token = self.shutdown_token.clone(); - let startup_task = tokio::spawn(async { - self.confirm_sequencer_config() - .await - .wrap_err("failed to confirm sequencer config")?; - let starting_rollup_height = self - .get_starting_rollup_height() - .await - .wrap_err("failed to get next rollup block height")?; - - // send the startup info to the submitter - let submitter_info = SubmitterInfo { - sequencer_chain_id: self.sequencer_chain_id.clone(), - }; - - let watcher_info = WatcherInfo { - fee_asset_id: self.expected_fee_asset_id, - starting_rollup_height, - }; - - self.submitter_tx - .send(submitter_info) - .map_err(|_submitter_info| eyre!("failed to send submitter info"))?; - self.watcher_tx - .send(watcher_info) - .map_err(|_watcher_info| eyre!("failed to send watcher info"))?; - - Ok(()) + let startup_task = tokio::spawn({ + let state = self.state.clone(); + async move { + self.confirm_sequencer_config() + .await + .wrap_err("failed to confirm sequencer config")?; + let starting_rollup_height = self + .get_starting_rollup_height() + .await + .wrap_err("failed to get next rollup block height")?; + + // send the startup info to the submitter + let info = Info { + chain_id: self.sequencer_chain_id.clone(), + fee_asset_id: self.expected_fee_asset_id, + starting_rollup_height, + }; + + state.set_startup_info(info); + + Ok(()) + } }); tokio::select!( diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/state.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/state.rs index 647bb489b5..5f64ae69f9 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/state.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/state.rs @@ -1,5 +1,7 @@ use tokio::sync::watch; +use super::startup; + pub(crate) struct State { inner: tokio::sync::watch::Sender, } @@ -39,6 +41,7 @@ macro_rules! forward_setter { } forward_setter!( + [set_startup_info <- startup::Info], [set_sequencer_connected <- bool], [set_last_rollup_height_submitted <- u64], [set_last_sequencer_height <- u64], @@ -47,6 +50,8 @@ forward_setter!( #[derive(Clone, Debug, Default, PartialEq, Eq, serde::Serialize)] pub(crate) struct StateSnapshot { + startup_info: Option, + watcher_ready: bool, submitter_ready: bool, @@ -58,6 +63,19 @@ pub(crate) struct StateSnapshot { } impl StateSnapshot { + pub fn get_startup_info(&self) -> Option { + self.startup_info.clone() + } + + pub fn set_startup_info(&mut self, startup_info: startup::Info) -> bool { + if self.startup_info.is_none() { + self.startup_info = Some(startup_info); + true + } else { + false + } + } + pub(crate) fn set_watcher_ready(&mut self) { self.watcher_ready = true; } diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/builder.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/builder.rs index e44e1a56ee..8ce008cfe9 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/builder.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/builder.rs @@ -34,7 +34,7 @@ impl Handle { self.batches_tx .send(batch) .await - .wrap_err("failed submitter_handleto send batch") + .wrap_err("failed to send batch") } } diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/tests.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/tests.rs index dce2992172..8ba7f15a1a 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/tests.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/tests.rs @@ -141,7 +141,7 @@ impl TestSubmitter { // not testing watcher here so just set it to ready state.set_watcher_ready(); let (startup_tx, startup_rx) = oneshot::channel(); - let startup_handle = startup::SubmitterHandle::new(startup_rx); + let startup_handle = startup::InfoHandle::new(state.subscribe()); let metrics = Box::leak(Box::new(Metrics::new())); @@ -173,12 +173,15 @@ impl TestSubmitter { self.submitter_task_handle = Some(tokio::spawn(submitter.run())); + submitter.state.set_startup_info(startup::Info { + fee_asset_id: "fee-asset-id".to_string(), + starting_rollup_height: 1, + chain_id: SEQUENCER_CHAIN_ID.to_string(), + }); self.startup_tx .take() .expect("should only send startup info once") - .send(startup::SubmitterInfo { - sequencer_chain_id: SEQUENCER_CHAIN_ID.to_string(), - }) + .send(startup::SubmitterInfo {}) .unwrap(); // wait for the submitter to be ready From b4dc5311d7c0f467855be87ed7054efeefa587b9 Mon Sep 17 00:00:00 2001 From: itamar Date: Fri, 28 Jun 2024 16:58:11 -0400 Subject: [PATCH 10/32] fix tests --- .../src/bridge_withdrawer/mod.rs | 8 ++++--- .../src/bridge_withdrawer/startup.rs | 6 ++--- .../src/bridge_withdrawer/state.rs | 4 ++-- .../bridge_withdrawer/submitter/builder.rs | 2 +- .../src/bridge_withdrawer/submitter/mod.rs | 13 ++++------ .../src/bridge_withdrawer/submitter/tests.rs | 24 +++++-------------- 6 files changed, 22 insertions(+), 35 deletions(-) diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/mod.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/mod.rs index 50fda6e39d..e88e3896c3 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/mod.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/mod.rs @@ -91,7 +91,7 @@ impl BridgeWithdrawer { .wrap_err("failed to parse sequencer bridge address")?; // make startup object - let (startup, startup_submitter_handle, startup_watcher_handle) = startup::Builder { + let startup = startup::Builder { shutdown_token: shutdown_handle.token(), state: state.clone(), sequencer_chain_id, @@ -103,10 +103,12 @@ impl BridgeWithdrawer { .build() .wrap_err("failed to initialize startup")?; + let startup_handle = startup::InfoHandle::new(state.subscribe()); + // make submitter object let (submitter, submitter_handle) = submitter::Builder { shutdown_token: shutdown_handle.token(), - startup_handle: startup_submitter_handle, + startup_handle: startup_handle.clone(), sequencer_cometbft_endpoint, sequencer_key_path, sequencer_address_prefix: sequencer_address_prefix.clone(), @@ -119,7 +121,7 @@ impl BridgeWithdrawer { let ethereum_watcher = watcher::Builder { ethereum_contract_address, ethereum_rpc_endpoint, - startup_handle: startup_watcher_handle, + startup_handle, shutdown_token: shutdown_handle.token(), state: state.clone(), rollup_asset_denom: rollup_asset_denomination diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs index 210497149e..b1cbcb37f5 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs @@ -16,9 +16,8 @@ use astria_eyre::eyre::{ self, ensure, eyre, - Context as _, - ContextCompat, OptionExt as _, + WrapErr as _, }; use prost::Message as _; use sequencer_client::{ @@ -93,6 +92,7 @@ pub(super) struct Info { pub(super) chain_id: String, } +#[derive(Debug, Clone)] pub(super) struct InfoHandle { rx: watch::Receiver, } @@ -326,7 +326,7 @@ impl Startup { "failed to extract rollup height from last transaction by the bridge account", )? .checked_add(1) - .wrap_err("failed to increment rollup height by 1")? + .ok_or_eyre("failed to increment rollup height by 1")? } else { 1 }; diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/state.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/state.rs index 5f64ae69f9..bbdc4c561b 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/state.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/state.rs @@ -63,11 +63,11 @@ pub(crate) struct StateSnapshot { } impl StateSnapshot { - pub fn get_startup_info(&self) -> Option { + pub(super) fn get_startup_info(&self) -> Option { self.startup_info.clone() } - pub fn set_startup_info(&mut self, startup_info: startup::Info) -> bool { + pub(super) fn set_startup_info(&mut self, startup_info: startup::Info) -> bool { if self.startup_info.is_none() { self.startup_info = Some(startup_info); true diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/builder.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/builder.rs index 8ce008cfe9..8e61704e53 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/builder.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/builder.rs @@ -40,7 +40,7 @@ impl Handle { pub(crate) struct Builder { pub(crate) shutdown_token: CancellationToken, - pub(crate) startup_handle: startup::SubmitterHandle, + pub(crate) startup_handle: startup::InfoHandle, pub(crate) sequencer_key_path: String, pub(crate) sequencer_address_prefix: String, pub(crate) sequencer_cometbft_endpoint: String, diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/mod.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/mod.rs index 1349c8d45d..b869d7c7f3 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/mod.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/mod.rs @@ -45,10 +45,7 @@ use super::{ startup, state, }; -use crate::{ - bridge_withdrawer::startup::SubmitterInfo as StartupInfo, - metrics::Metrics, -}; +use crate::metrics::Metrics; mod builder; pub(crate) mod signer; @@ -57,7 +54,7 @@ mod tests; pub(super) struct Submitter { shutdown_token: CancellationToken, - startup_handle: startup::SubmitterHandle, + startup_handle: startup::InfoHandle, state: Arc, batches_rx: mpsc::Receiver, sequencer_cometbft_client: sequencer_client::HttpClient, @@ -73,10 +70,10 @@ impl Submitter { return Ok(()); } - startup_info = self.startup_handle.recv() => { - let StartupInfo { sequencer_chain_id } = startup_info.wrap_err("submitter failed to get startup info")?; + startup_info = self.startup_handle.get_info() => { + let startup::Info { chain_id, .. } = startup_info.wrap_err("submitter failed to get startup info")?; self.state.set_submitter_ready(); - sequencer_chain_id + chain_id } }; diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/tests.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/tests.rs index 8ba7f15a1a..b8dceac06b 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/tests.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/tests.rs @@ -55,10 +55,7 @@ use tendermint_rpc::{ }, request, }; -use tokio::{ - sync::oneshot, - task::JoinHandle, -}; +use tokio::task::JoinHandle; use tokio_util::sync::CancellationToken; use tracing::debug; use wiremock::{ @@ -111,7 +108,6 @@ static TELEMETRY: Lazy<()> = Lazy::new(|| { struct TestSubmitter { submitter: Option, submitter_handle: submitter::Handle, - startup_tx: Option>, cometbft_mock: MockServer, submitter_task_handle: Option>>, } @@ -136,12 +132,10 @@ impl TestSubmitter { let cometbft_mock = MockServer::start().await; let sequencer_cometbft_endpoint = format!("http://{}", cometbft_mock.address()); - // withdrawer state + // startup info let state = Arc::new(state::State::new()); - // not testing watcher here so just set it to ready - state.set_watcher_ready(); - let (startup_tx, startup_rx) = oneshot::channel(); let startup_handle = startup::InfoHandle::new(state.subscribe()); + state.set_watcher_ready(); let metrics = Box::leak(Box::new(Metrics::new())); @@ -160,7 +154,6 @@ impl TestSubmitter { Self { submitter: Some(submitter), submitter_task_handle: None, - startup_tx: Some(startup_tx), submitter_handle, cometbft_mock, } @@ -171,18 +164,13 @@ impl TestSubmitter { let mut state = submitter.state.subscribe(); - self.submitter_task_handle = Some(tokio::spawn(submitter.run())); - submitter.state.set_startup_info(startup::Info { - fee_asset_id: "fee-asset-id".to_string(), + fee_asset_id: asset::Id::from_str_unchecked("fee-asset-id"), starting_rollup_height: 1, chain_id: SEQUENCER_CHAIN_ID.to_string(), }); - self.startup_tx - .take() - .expect("should only send startup info once") - .send(startup::SubmitterInfo {}) - .unwrap(); + + self.submitter_task_handle = Some(tokio::spawn(submitter.run())); // wait for the submitter to be ready state From c65412c8be32678866f504bfb227cccd9e27f70a Mon Sep 17 00:00:00 2001 From: itamar Date: Fri, 28 Jun 2024 18:04:43 -0400 Subject: [PATCH 11/32] fix problems after merge --- .../src/bridge_withdrawer/ethereum/watcher.rs | 54 +++++++++++++++---- .../src/bridge_withdrawer/mod.rs | 7 +-- .../src/bridge_withdrawer/startup.rs | 20 +++---- .../src/bridge_withdrawer/submitter/tests.rs | 10 ++-- 4 files changed, 62 insertions(+), 29 deletions(-) diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/watcher.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/watcher.rs index e214c41e35..0fbffe1510 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/watcher.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/watcher.rs @@ -198,7 +198,7 @@ impl Watcher { u64, )> { let startup::Info { - fee_asset_id, + fee_asset, starting_rollup_height, .. } = select! { @@ -548,6 +548,10 @@ mod tests { }, utils::hex, }; + use tokio::sync::mpsc::{ + self, + error::TryRecvError, + }; use super::*; use crate::bridge_withdrawer::ethereum::{ @@ -644,6 +648,34 @@ mod tests { let value = 1_000_000_000.into(); let recipient = crate::astria_address([1u8; 20]); + let bridge_address = crate::astria_address([1u8; 20]); + let denom = "transfer/channel-0/utia".parse::().unwrap(); + + let state = Arc::new(State::new()); + let startup_handle = startup::InfoHandle::new(state.subscribe()); + state.set_startup_info(startup::Info { + starting_rollup_height: 1, + fee_asset: denom.clone(), + chain_id: "astria".to_string(), + }); + let (batch_tx, mut batch_rx) = mpsc::channel(100); + let submitter_handle = submitter::Handle::new(batch_tx); + + let watcher = Builder { + ethereum_contract_address: hex::encode(contract_address), + ethereum_rpc_endpoint: anvil.ws_endpoint(), + startup_handle, + submitter_handle, + shutdown_token: CancellationToken::new(), + state: Arc::new(State::new()), + rollup_asset_denom: denom.clone(), + bridge_address, + sequencer_address_prefix: crate::ASTRIA_ADDRESS_PREFIX.into(), + } + .build() + .unwrap(); + + tokio::task::spawn(watcher.run()); let receipt = send_sequencer_withdraw_transaction(&contract, value, recipient).await; let expected_event = EventWithMetadata { event: WithdrawalEvent::Sequencer(SequencerWithdrawalFilter { @@ -676,7 +708,7 @@ mod tests { ); }; assert_eq!(action, &expected_action); - assert_eq!(batch_rx.try_recv().unwrap_err(), Empty); + assert_eq!(batch_rx.try_recv().unwrap_err(), TryRecvError::Empty); } #[tokio::test] @@ -722,7 +754,7 @@ mod tests { let startup_handle = startup::InfoHandle::new(state.subscribe()); state.set_startup_info(startup::Info { starting_rollup_height: 1, - fee_asset_id: denom.id(), + fee_asset: denom.clone(), chain_id: "astria".to_string(), }); let (batch_tx, mut batch_rx) = mpsc::channel(100); @@ -806,6 +838,8 @@ mod tests { block_number: receipt.block_number.unwrap(), transaction_hash: receipt.transaction_hash, }; + let bridge_address = crate::astria_address([1u8; 20]); + let denom = "transfer/channel-0/utia".parse::().unwrap(); let Action::Ics20Withdrawal(mut expected_action) = event_to_action( expected_event, denom.clone(), @@ -823,7 +857,7 @@ mod tests { let startup_handle = startup::InfoHandle::new(state.subscribe()); state.set_startup_info(startup::Info { starting_rollup_height: 1, - fee_asset_id: denom.id(), + fee_asset: denom.clone(), chain_id: "astria".to_string(), }); let (batch_tx, mut batch_rx) = mpsc::channel(100); @@ -857,7 +891,7 @@ mod tests { }; action.timeout_time = 0; // zero this for testing assert_eq!(action, &expected_action); - assert_eq!(batch_rx.try_recv().unwrap_err(), Empty); + assert_eq!(batch_rx.try_recv().unwrap_err(), TryRecvError::Empty); } async fn mint_tokens( @@ -921,6 +955,7 @@ mod tests { let value = 1_000_000_000.into(); let recipient = crate::astria_address([1u8; 20]); + let bridge_address = crate::astria_address([1u8; 20]); let denom = default_native_asset(); let receipt = send_sequencer_withdraw_transaction_erc20(&contract, value, recipient).await; let expected_event = EventWithMetadata { @@ -949,7 +984,7 @@ mod tests { let startup_handle = startup::InfoHandle::new(state.subscribe()); state.set_startup_info(startup::Info { starting_rollup_height: 1, - fee_asset_id: denom.id(), + fee_asset: denom.clone(), chain_id: "astria".to_string(), }); let (batch_tx, mut batch_rx) = mpsc::channel(100); @@ -982,7 +1017,7 @@ mod tests { ); }; assert_eq!(action, &expected_action); - assert_eq!(batch_rx.try_recv().unwrap_err(), Empty); + assert_eq!(batch_rx.try_recv().unwrap_err(), TryRecvError::Empty); } async fn send_ics20_withdraw_transaction_astria_bridgeable_erc20( @@ -1024,6 +1059,7 @@ mod tests { let value = 1_000_000_000.into(); let recipient = "somebech32address".to_string(); + let bridge_address = crate::astria_address([1u8; 20]); let denom = "transfer/channel-0/utia".parse::().unwrap(); let receipt = send_ics20_withdraw_transaction_astria_bridgeable_erc20( &contract, @@ -1058,7 +1094,7 @@ mod tests { let startup_handle = startup::InfoHandle::new(state.subscribe()); state.set_startup_info(startup::Info { starting_rollup_height: 1, - fee_asset_id: denom.id(), + fee_asset: denom.clone(), chain_id: "astria".to_string(), }); let (batch_tx, mut batch_rx) = mpsc::channel(100); @@ -1092,6 +1128,6 @@ mod tests { }; action.timeout_time = 0; // zero this for testing assert_eq!(action, &expected_action); - assert_eq!(batch_rx.try_recv().unwrap_err(), Empty); + assert_eq!(batch_rx.try_recv().unwrap_err(), TryRecvError::Empty); } } diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/mod.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/mod.rs index e88e3896c3..d11668eea4 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/mod.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/mod.rs @@ -7,10 +7,7 @@ use std::{ time::Duration, }; -use astria_core::primitive::v1::asset::{ - self, - Denom, -}; +use astria_core::primitive::v1::asset::Denom; use astria_eyre::eyre::{ self, WrapErr as _, @@ -97,7 +94,7 @@ impl BridgeWithdrawer { sequencer_chain_id, sequencer_cometbft_endpoint: sequencer_cometbft_endpoint.clone(), sequencer_bridge_address, - expected_fee_asset_id: asset::Id::from_str_unchecked(&fee_asset_denomination), + expected_fee_asset: fee_asset_denomination, expected_min_fee_asset_balance: u128::from(min_expected_fee_asset_balance), } .build() diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs index c19967a034..44b109524f 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs @@ -52,7 +52,7 @@ pub(super) struct Builder { pub(super) sequencer_chain_id: String, pub(super) sequencer_cometbft_endpoint: String, pub(super) sequencer_bridge_address: Address, - pub(super) expected_fee_asset_id: asset::Id, + pub(super) expected_fee_asset: asset::Denom, // TODO: change the name of this config var pub(super) expected_min_fee_asset_balance: u128, } @@ -65,7 +65,7 @@ impl Builder { sequencer_chain_id, sequencer_cometbft_endpoint, sequencer_bridge_address, - expected_fee_asset_id, + expected_fee_asset, expected_min_fee_asset_balance, } = self; @@ -79,7 +79,7 @@ impl Builder { sequencer_chain_id, sequencer_cometbft_client, sequencer_bridge_address, - expected_fee_asset_id, + expected_fee_asset, expected_min_fee_asset_balance, }) } @@ -88,7 +88,7 @@ impl Builder { #[derive(Debug, Clone, PartialEq, Eq, serde::Serialize)] pub(super) struct Info { pub(super) starting_rollup_height: u64, - pub(super) fee_asset_id: asset::Id, + pub(super) fee_asset: asset::Denom, pub(super) chain_id: String, } @@ -124,7 +124,7 @@ pub(super) struct Startup { sequencer_chain_id: String, sequencer_cometbft_client: sequencer_client::HttpClient, sequencer_bridge_address: Address, - expected_fee_asset_id: asset::Id, + expected_fee_asset: asset::Denom, expected_min_fee_asset_balance: u128, } @@ -146,7 +146,7 @@ impl Startup { // send the startup info to the submitter let info = Info { chain_id: self.sequencer_chain_id.clone(), - fee_asset_id: self.expected_fee_asset_id, + fee_asset: self.expected_fee_asset, starting_rollup_height, }; @@ -203,8 +203,8 @@ impl Startup { .wrap_err("failed to get allowed fee asset ids from sequencer")?; ensure!( allowed_fee_asset_ids_resp - .fee_asset_ids - .contains(&self.expected_fee_asset_id), + .fee_assets + .contains(&self.expected_fee_asset), "fee_asset_id provided in config is not a valid fee asset on the sequencer" ); @@ -219,7 +219,7 @@ impl Startup { let fee_asset_balance = fee_asset_balances .balances .into_iter() - .find(|balance| balance.denom.id() == self.expected_fee_asset_id) + .find(|balance| balance.denom == self.expected_fee_asset) .ok_or_eyre("withdrawer's account balance of the fee asset is zero")? .balance; ensure!( @@ -528,7 +528,7 @@ async fn get_allowed_fee_asset_ids( }, ); - let res = tryhard::retry_fn(|| client.get_allowed_fee_asset_ids()) + let res = tryhard::retry_fn(|| client.get_allowed_fee_assets()) .with_config(retry_config) .await .wrap_err("failed to get allowed fee asset ids from Sequencer after a lot of attempts"); diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/tests.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/tests.rs index b0d4d55f52..4b43118e73 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/tests.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/tests.rs @@ -165,7 +165,7 @@ impl TestSubmitter { let mut state = submitter.state.subscribe(); submitter.state.set_startup_info(startup::Info { - fee_asset_id: asset::Id::from_str_unchecked("fee-asset-id"), + fee_asset: "fee-asset".parse::().unwrap(), starting_rollup_height: 1, chain_id: SEQUENCER_CHAIN_ID.to_string(), }); @@ -191,8 +191,8 @@ async fn _register_default_chain_id_guard(cometbft_mock: &MockServer) -> MockGua } async fn _register_default_fee_asset_ids_guard(cometbft_mock: &MockServer) -> MockGuard { - let fee_asset_ids = vec![default_native_asset().id()]; - _register_allowed_fee_asset_ids_response(fee_asset_ids, cometbft_mock).await + let fee_assets = vec![default_native_asset()]; + _register_allowed_fee_assets_response(fee_assets, cometbft_mock).await } async fn _register_default_min_expected_fee_asset_balance_guard( @@ -359,8 +359,8 @@ async fn _register_genesis_chain_id_response(chain_id: &str, server: &MockServer .await } -async fn _register_allowed_fee_asset_ids_response( - fee_asset_ids: Vec, +async fn _register_allowed_fee_assets_response( + fee_assets: Vec, cometbft_mock: &MockServer, ) -> MockGuard { let response = tendermint_rpc::endpoint::abci_query::Response { From ebae9aae4630b5811357d9323a430b7bdbf9ce8e Mon Sep 17 00:00:00 2001 From: itamar Date: Mon, 1 Jul 2024 15:50:49 +0200 Subject: [PATCH 12/32] clean up retries --- .../src/bridge_withdrawer/startup.rs | 199 ++++++++---------- .../src/bridge_withdrawer/state.rs | 9 + 2 files changed, 101 insertions(+), 107 deletions(-) diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs index 44b109524f..b0f1c16d34 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs @@ -39,6 +39,7 @@ use tracing::{ instrument, warn, }; +use tryhard::backoff_strategies::ExponentialBackoff; use super::state::{ self, @@ -388,32 +389,13 @@ async fn get_bridge_account_last_transaction_hash( state: Arc, address: Address, ) -> eyre::Result { - 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: &sequencer_client::extension_trait::Error| { - let state = Arc::clone(&state); - state.set_sequencer_connected(false); - - 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 last bridge account's transaction hash; retrying after \ - backoff", - ); - futures::future::ready(()) - }, - ); - let res = tryhard::retry_fn(|| client.get_bridge_account_last_transaction_hash(address)) - .with_config(retry_config) + .with_config(make_sequencer_retry_config( + state.clone(), + "attempt to fetch last bridge account's transaction hash from Sequencer; retrying \ + after backoff" + .to_string(), + )) .await .wrap_err( "failed to fetch last bridge account's transaction hash from Sequencer after a lot of \ @@ -431,29 +413,11 @@ async fn get_tx( state: Arc, tx_hash: tendermint::Hash, ) -> eyre::Result { - 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, error: &tendermint_rpc::Error| { - let state = Arc::clone(&state); - state.set_sequencer_connected(false); - - 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 get transaction from Sequencer; retrying after backoff", - ); - futures::future::ready(()) - }, - ); - let res = tryhard::retry_fn(|| client.tx(tx_hash, false)) - .with_config(retry_config) + .with_config(make_cometbft_retry_config( + state.clone(), + "attempt to get transaction from CometBFT; retrying after backoff".to_string(), + )) .await .wrap_err("failed to get transaction from Sequencer after a lot of attempts"); @@ -467,31 +431,11 @@ async fn get_sequencer_chain_id( client: sequencer_client::HttpClient, state: Arc, ) -> 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 state = Arc::clone(&state); - state.set_sequencer_connected(false); - - 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) + .with_config(make_cometbft_retry_config( + state.clone(), + "attempt to get genesis from CometBFT; retrying after backoff".to_string(), + )) .await .wrap_err("failed to get genesis info from Sequencer after a lot of attempts")?; @@ -505,15 +449,69 @@ async fn get_allowed_fee_asset_ids( client: sequencer_client::HttpClient, state: Arc, ) -> eyre::Result { - let retry_config = tryhard::RetryFutureConfig::new(u32::MAX) + let res = tryhard::retry_fn(|| client.get_allowed_fee_assets()) + .with_config(make_sequencer_retry_config( + state.clone(), + "attempt to get allowed fee assets from Sequencer; retrying after backoff".to_string(), + )) + .await + .wrap_err("failed to get allowed fee asset ids from Sequencer after a lot of attempts"); + + state.set_sequencer_connected(res.is_ok()); + + res +} + +#[instrument(skip_all)] +async fn get_latest_balance( + client: sequencer_client::HttpClient, + state: Arc, + address: Address, +) -> eyre::Result { + let res = tryhard::retry_fn(|| { + let client = client.clone(); + async move { + match client.get_latest_balance(address).await { + Ok(res) => Ok(Ok(res)), + Err(err) => { + if let Some(tendermint_err) = err.as_tendermint_rpc() { + Err(tendermint_err.inner().clone()) + } else { + Ok(Err(err)) + } + } + } + } + }) + .with_config(make_cometbft_retry_config( + state.clone(), + "attempt to get latest balance from CometBFT; retrying after backoff".to_string(), + )) + .await + .wrap_err("failed to get latest balance from Sequencer after a lot of attempts"); + + let res = res?.wrap_err("failed to deserialize the latest balance response from CometBFT"); + + // set cometbft as connected if received a response + state.set_sequencer_connected(res.is_ok()); + + res +} + +pub(self) fn make_cometbft_retry_config( + state: Arc, + retry_message: String, +) -> tryhard::RetryFutureConfig< + ExponentialBackoff, + impl Fn(u32, Option, &tendermint_rpc::Error) -> futures::future::Ready<()>, +> { + 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: &sequencer_client::extension_trait::Error| { + move |attempt: u32, next_delay: Option, error: &tendermint_rpc::Error| { let state = Arc::clone(&state); - state.set_sequencer_connected(false); + state.set_cometbft_connected(false); let wait_duration = next_delay .map(humantime::format_duration) @@ -522,35 +520,31 @@ async fn get_allowed_fee_asset_ids( attempt, wait_duration, error = error as &dyn std::error::Error, - "attempt to fetch sequencer allowed fee asset ids; retrying after backoff", + retry_message, ); futures::future::ready(()) }, - ); - - let res = tryhard::retry_fn(|| client.get_allowed_fee_assets()) - .with_config(retry_config) - .await - .wrap_err("failed to get allowed fee asset ids from Sequencer after a lot of attempts"); - - state.set_sequencer_connected(res.is_ok()); - - res + ) } -#[instrument(skip_all)] -async fn get_latest_balance( - client: sequencer_client::HttpClient, +pub(self) fn make_sequencer_retry_config( state: Arc, - address: Address, -) -> eyre::Result { - let retry_config = tryhard::RetryFutureConfig::new(u32::MAX) + retry_message: String, +) -> tryhard::RetryFutureConfig< + ExponentialBackoff, + impl Fn( + u32, + Option, + &sequencer_client::extension_trait::Error, + ) -> futures::future::Ready<()>, +> { + 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: &sequencer_client::extension_trait::Error| { + move |attempt: u32, + next_delay: Option, + error: &sequencer_client::extension_trait::Error| { let state = Arc::clone(&state); state.set_sequencer_connected(false); @@ -561,18 +555,9 @@ async fn get_latest_balance( attempt, wait_duration, error = error as &dyn std::error::Error, - "attempt to get latest balance; retrying after backoff", + retry_message, ); futures::future::ready(()) }, - ); - - let res = tryhard::retry_fn(|| client.get_latest_balance(address)) - .with_config(retry_config) - .await - .wrap_err("failed to get latest balance from Sequencer after a lot of attempts"); - - state.set_sequencer_connected(res.is_ok()); - - res + ) } diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/state.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/state.rs index bbdc4c561b..c34ffdf384 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/state.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/state.rs @@ -42,6 +42,7 @@ macro_rules! forward_setter { forward_setter!( [set_startup_info <- startup::Info], + [set_cometbft_connected <- bool], [set_sequencer_connected <- bool], [set_last_rollup_height_submitted <- u64], [set_last_sequencer_height <- u64], @@ -55,6 +56,7 @@ pub(crate) struct StateSnapshot { watcher_ready: bool, submitter_ready: bool, + cometbft_connected: bool, sequencer_connected: bool, last_rollup_height_submitted: Option, @@ -92,6 +94,13 @@ impl StateSnapshot { self.sequencer_connected } + /// Sets the CometBFT connection status to `connected`. + fn set_cometbft_connected(&mut self, connected: bool) -> bool { + let changed = self.cometbft_connected ^ connected; + self.cometbft_connected = connected; + changed + } + /// Sets the sequencer connection status to `connected`. fn set_sequencer_connected(&mut self, connected: bool) -> bool { let changed = self.sequencer_connected ^ connected; From c57b5b97fd93a3b9f29c0ec2fe87ab2d371efcb7 Mon Sep 17 00:00:00 2001 From: itamar Date: Mon, 1 Jul 2024 15:57:12 +0200 Subject: [PATCH 13/32] comment --- crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs index b0f1c16d34..8583508051 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs @@ -469,6 +469,7 @@ async fn get_latest_balance( address: Address, ) -> eyre::Result { let res = tryhard::retry_fn(|| { + // only retry on tendermint_rpc errors, not deserialization or native conversion let client = client.clone(); async move { match client.get_latest_balance(address).await { From a8b67a7bfb268dc749909ea01435f15c3956bf63 Mon Sep 17 00:00:00 2001 From: itamar Date: Mon, 1 Jul 2024 18:43:43 +0200 Subject: [PATCH 14/32] fix clippy --- .../astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs | 4 ++-- .../astria-bridge-withdrawer/src/bridge_withdrawer/state.rs | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs index 8583508051..27030c86ab 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs @@ -499,7 +499,7 @@ async fn get_latest_balance( res } -pub(self) fn make_cometbft_retry_config( +fn make_cometbft_retry_config( state: Arc, retry_message: String, ) -> tryhard::RetryFutureConfig< @@ -528,7 +528,7 @@ pub(self) fn make_cometbft_retry_config( ) } -pub(self) fn make_sequencer_retry_config( +fn make_sequencer_retry_config( state: Arc, retry_message: String, ) -> tryhard::RetryFutureConfig< diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/state.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/state.rs index c34ffdf384..95940db0ff 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/state.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/state.rs @@ -50,6 +50,7 @@ forward_setter!( ); #[derive(Clone, Debug, Default, PartialEq, Eq, serde::Serialize)] +#[allow(clippy::struct_excessive_bools)] pub(crate) struct StateSnapshot { startup_info: Option, @@ -94,7 +95,7 @@ impl StateSnapshot { self.sequencer_connected } - /// Sets the CometBFT connection status to `connected`. + /// Sets the `CometBFT` connection status to `connected`. fn set_cometbft_connected(&mut self, connected: bool) -> bool { let changed = self.cometbft_connected ^ connected; self.cometbft_connected = connected; From 277c1794bd384330c8a0f977f37753406983fee1 Mon Sep 17 00:00:00 2001 From: itamar Date: Mon, 1 Jul 2024 18:54:45 +0200 Subject: [PATCH 15/32] add comment for lint --- crates/astria-bridge-withdrawer/src/bridge_withdrawer/state.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/state.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/state.rs index 95940db0ff..2760c78142 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/state.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/state.rs @@ -50,6 +50,7 @@ forward_setter!( ); #[derive(Clone, Debug, Default, PartialEq, Eq, serde::Serialize)] +// This lint is allowed because there are 4 boolean fields that are necessary to track here. #[allow(clippy::struct_excessive_bools)] pub(crate) struct StateSnapshot { startup_info: Option, From c385713fc85de83882e92251d0be67735ae4b3a4 Mon Sep 17 00:00:00 2001 From: itamar Date: Tue, 2 Jul 2024 15:04:53 +0200 Subject: [PATCH 16/32] clean up comments --- .../src/bridge_withdrawer/ethereum/watcher.rs | 2 +- .../src/bridge_withdrawer/submitter/tests.rs | 174 +----------------- 2 files changed, 2 insertions(+), 174 deletions(-) diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/watcher.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/watcher.rs index 0fbffe1510..199f586c42 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/watcher.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/watcher.rs @@ -649,7 +649,7 @@ mod tests { let value = 1_000_000_000.into(); let recipient = crate::astria_address([1u8; 20]); let bridge_address = crate::astria_address([1u8; 20]); - let denom = "transfer/channel-0/utia".parse::().unwrap(); + let denom = "nria".parse::().unwrap(); let state = Arc::new(State::new()); let startup_handle = startup::InfoHandle::new(state.subscribe()); diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/tests.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/tests.rs index 4b43118e73..dc9b20e201 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/tests.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/tests.rs @@ -42,13 +42,9 @@ use tendermint::{ types::ExecTxResult, }, block::Height, - chain, }; use tendermint_rpc::{ - endpoint::{ - broadcast::tx_sync, - tx, - }, + endpoint::broadcast::tx_sync, request, }; use tokio::task::JoinHandle; @@ -186,28 +182,6 @@ impl TestSubmitter { } } -async fn _register_default_chain_id_guard(cometbft_mock: &MockServer) -> MockGuard { - _register_genesis_chain_id_response(SEQUENCER_CHAIN_ID, cometbft_mock).await -} - -async fn _register_default_fee_asset_ids_guard(cometbft_mock: &MockServer) -> MockGuard { - let fee_assets = vec![default_native_asset()]; - _register_allowed_fee_assets_response(fee_assets, cometbft_mock).await -} - -async fn _register_default_min_expected_fee_asset_balance_guard( - cometbft_mock: &MockServer, -) -> MockGuard { - _register_get_latest_balance( - vec![AssetBalance { - denom: default_native_asset(), - balance: 1_000_000u128, - }], - cometbft_mock, - ) - .await -} - fn make_ics20_withdrawal_action() -> Action { let denom = DEFAULT_IBC_DENOM.parse::().unwrap(); let destination_chain_address = "address".to_string(); @@ -306,139 +280,6 @@ fn signed_tx_from_request(request: &Request) -> SignedTransaction { signed_tx } -async fn _register_genesis_chain_id_response(chain_id: &str, server: &MockServer) -> MockGuard { - use tendermint::{ - consensus::{ - params::{ - AbciParams, - ValidatorParams, - }, - Params, - }, - genesis::Genesis, - time::Time, - }; - let response = tendermint_rpc::endpoint::genesis::Response:: { - genesis: Genesis { - genesis_time: Time::from_unix_timestamp(1, 1).unwrap(), - chain_id: chain::Id::try_from(chain_id).unwrap(), - initial_height: 1, - consensus_params: Params { - block: tendermint::block::Size { - max_bytes: 1024, - max_gas: 1024, - time_iota_ms: 1000, - }, - evidence: tendermint::evidence::Params { - max_age_num_blocks: 1000, - max_age_duration: tendermint::evidence::Duration(Duration::from_secs(3600)), - max_bytes: 1_048_576, - }, - validator: ValidatorParams { - pub_key_types: vec![tendermint::public_key::Algorithm::Ed25519], - }, - version: None, - abci: AbciParams::default(), - }, - validators: vec![], - app_hash: tendermint::hash::AppHash::default(), - app_state: serde_json::Value::Null, - }, - }; - - let wrapper = response::Wrapper::new_with_id(tendermint_rpc::Id::Num(1), Some(response), None); - Mock::given(body_partial_json(json!({"method": "genesis"}))) - .respond_with( - ResponseTemplate::new(200) - .set_body_json(&wrapper) - .append_header("Content-Type", "application/json"), - ) - .up_to_n_times(1) - .expect(1) - .mount_as_scoped(server) - .await -} - -async fn _register_allowed_fee_assets_response( - fee_assets: Vec, - cometbft_mock: &MockServer, -) -> MockGuard { - let response = tendermint_rpc::endpoint::abci_query::Response { - response: tendermint_rpc::endpoint::abci_query::AbciQuery { - value: astria_core::protocol::asset::v1alpha1::AllowedFeeAssetsResponse { - fee_assets, - height: 1, - } - .into_raw() - .encode_to_vec(), - ..Default::default() - }, - }; - let wrapper = response::Wrapper::new_with_id(tendermint_rpc::Id::Num(1), Some(response), None); - Mock::given(body_partial_json(json!({"method": "abci_query"}))) - .and(body_string_contains("asset/allowed_fee_assets")) - .respond_with( - ResponseTemplate::new(200) - .set_body_json(&wrapper) - .append_header("Content-Type", "application/json"), - ) - .expect(1) - .mount_as_scoped(cometbft_mock) - .await -} - -async fn _register_get_latest_balance( - balances: Vec, - server: &MockServer, -) -> MockGuard { - let response = tendermint_rpc::endpoint::abci_query::Response { - response: tendermint_rpc::endpoint::abci_query::AbciQuery { - value: astria_core::protocol::account::v1alpha1::BalanceResponse { - balances, - height: 1, - } - .into_raw() - .encode_to_vec(), - ..Default::default() - }, - }; - - let wrapper = response::Wrapper::new_with_id(tendermint_rpc::Id::Num(1), Some(response), None); - Mock::given(body_partial_json(json!({"method": "abci_query"}))) - .and(body_string_contains("accounts/balance")) - .respond_with( - ResponseTemplate::new(200) - .set_body_json(&wrapper) - .append_header("Content-Type", "application/json"), - ) - .expect(1) - .mount_as_scoped(server) - .await -} - -async fn _register_last_bridge_tx_hash_guard( - server: &MockServer, - response: BridgeAccountLastTxHashResponse, -) -> MockGuard { - let response = tendermint_rpc::endpoint::abci_query::Response { - response: tendermint_rpc::endpoint::abci_query::AbciQuery { - value: response.into_raw().encode_to_vec(), - ..Default::default() - }, - }; - let wrapper = response::Wrapper::new_with_id(tendermint_rpc::Id::Num(1), Some(response), None); - Mock::given(body_partial_json(json!({"method": "abci_query"}))) - .and(body_string_contains("bridge/account_last_tx_hash")) - .respond_with( - ResponseTemplate::new(200) - .set_body_json(&wrapper) - .append_header("Content-Type", "application/json"), - ) - .expect(1) - .mount_as_scoped(server) - .await -} - async fn register_get_nonce_response(server: &MockServer, response: NonceResponse) -> MockGuard { let response = tendermint_rpc::endpoint::abci_query::Response { response: tendermint_rpc::endpoint::abci_query::AbciQuery { @@ -459,19 +300,6 @@ async fn register_get_nonce_response(server: &MockServer, response: NonceRespons .await } -async fn _register_tx_guard(server: &MockServer, response: tx::Response) -> MockGuard { - let wrapper = response::Wrapper::new_with_id(tendermint_rpc::Id::Num(1), Some(response), None); - Mock::given(body_partial_json(json!({"method": "tx"}))) - .respond_with( - ResponseTemplate::new(200) - .set_body_json(&wrapper) - .append_header("Content-Type", "application/json"), - ) - .expect(1) - .mount_as_scoped(server) - .await -} - async fn register_broadcast_tx_commit_response( server: &MockServer, response: tx_commit::Response, From d0a8414eb31c506ae8ed59a93f2403b4009c2e6e Mon Sep 17 00:00:00 2001 From: itamar Date: Tue, 2 Jul 2024 15:30:01 +0200 Subject: [PATCH 17/32] fix tests --- .../src/bridge_withdrawer/ethereum/watcher.rs | 154 +++++++++--------- .../src/bridge_withdrawer/submitter/tests.rs | 14 +- 2 files changed, 80 insertions(+), 88 deletions(-) diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/watcher.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/watcher.rs index 199f586c42..a4511b1706 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/watcher.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/watcher.rs @@ -827,31 +827,9 @@ mod tests { let value = 1_000_000_000.into(); let recipient = "somebech32address".to_string(); - let receipt = send_ics20_withdraw_transaction(&contract, value, recipient.clone()).await; - let expected_event = EventWithMetadata { - event: WithdrawalEvent::Ics20(Ics20WithdrawalFilter { - sender: wallet.address(), - destination_chain_address: recipient.clone(), - amount: value, - memo: "nootwashere".to_string(), - }), - block_number: receipt.block_number.unwrap(), - transaction_hash: receipt.transaction_hash, - }; + let bridge_address = crate::astria_address([1u8; 20]); let denom = "transfer/channel-0/utia".parse::().unwrap(); - let Action::Ics20Withdrawal(mut expected_action) = event_to_action( - expected_event, - denom.clone(), - denom.clone(), - 1, - bridge_address, - crate::ASTRIA_ADDRESS_PREFIX, - ) - .unwrap() else { - panic!("expected action to be Ics20Withdrawal"); - }; - expected_action.timeout_time = 0; // zero this for testing let state = Arc::new(State::new()); let startup_handle = startup::InfoHandle::new(state.subscribe()); @@ -868,7 +846,7 @@ mod tests { startup_handle, shutdown_token: CancellationToken::new(), state: Arc::new(State::new()), - rollup_asset_denom: denom, + rollup_asset_denom: denom.clone(), bridge_address, submitter_handle: submitter::Handle::new(batch_tx), sequencer_address_prefix: crate::ASTRIA_ADDRESS_PREFIX.into(), @@ -878,8 +856,30 @@ mod tests { tokio::task::spawn(watcher.run()); - // make another tx to trigger anvil to make another block - send_ics20_withdraw_transaction(&contract, value, recipient).await; + let receipt = send_ics20_withdraw_transaction(&contract, value, recipient.clone()).await; + let expected_event = EventWithMetadata { + event: WithdrawalEvent::Ics20(Ics20WithdrawalFilter { + sender: wallet.address(), + destination_chain_address: recipient.clone(), + amount: value, + memo: "nootwashere".to_string(), + }), + block_number: receipt.block_number.unwrap(), + transaction_hash: receipt.transaction_hash, + }; + + let Action::Ics20Withdrawal(mut expected_action) = event_to_action( + expected_event, + denom.clone(), + denom.clone(), + 1, + bridge_address, + crate::ASTRIA_ADDRESS_PREFIX, + ) + .unwrap() else { + panic!("expected action to be Ics20Withdrawal"); + }; + expected_action.timeout_time = 0; // zero this for testing let mut batch = batch_rx.recv().await.unwrap(); assert_eq!(batch.actions.len(), 1); @@ -957,28 +957,6 @@ mod tests { let recipient = crate::astria_address([1u8; 20]); let bridge_address = crate::astria_address([1u8; 20]); let denom = default_native_asset(); - let receipt = send_sequencer_withdraw_transaction_erc20(&contract, value, recipient).await; - let expected_event = EventWithMetadata { - event: WithdrawalEvent::Sequencer(SequencerWithdrawalFilter { - sender: wallet.address(), - destination_chain_address: recipient.to_string(), - amount: value, - }), - block_number: receipt.block_number.unwrap(), - transaction_hash: receipt.transaction_hash, - }; - let expected_action = event_to_action( - expected_event, - denom.clone(), - denom.clone(), - 1, - bridge_address, - crate::ASTRIA_ADDRESS_PREFIX, - ) - .unwrap(); - let Action::BridgeUnlock(expected_action) = expected_action else { - panic!("expected action to be BridgeUnlock, got {expected_action:?}"); - }; let state = Arc::new(State::new()); let startup_handle = startup::InfoHandle::new(state.subscribe()); @@ -995,7 +973,7 @@ mod tests { startup_handle, shutdown_token: CancellationToken::new(), state: Arc::new(State::new()), - rollup_asset_denom: denom, + rollup_asset_denom: denom.clone(), bridge_address, submitter_handle: submitter::Handle::new(batch_tx), sequencer_address_prefix: crate::ASTRIA_ADDRESS_PREFIX.into(), @@ -1005,8 +983,28 @@ mod tests { tokio::task::spawn(watcher.run()); - // make another tx to trigger anvil to make another block - send_sequencer_withdraw_transaction_erc20(&contract, value, recipient).await; + let receipt = send_sequencer_withdraw_transaction_erc20(&contract, value, recipient).await; + let expected_event = EventWithMetadata { + event: WithdrawalEvent::Sequencer(SequencerWithdrawalFilter { + sender: wallet.address(), + destination_chain_address: recipient.to_string(), + amount: value, + }), + block_number: receipt.block_number.unwrap(), + transaction_hash: receipt.transaction_hash, + }; + let expected_action = event_to_action( + expected_event, + denom.clone(), + denom.clone(), + 1, + bridge_address, + crate::ASTRIA_ADDRESS_PREFIX, + ) + .unwrap(); + let Action::BridgeUnlock(expected_action) = expected_action else { + panic!("expected action to be BridgeUnlock, got {expected_action:?}"); + }; let batch = batch_rx.recv().await.unwrap(); assert_eq!(batch.actions.len(), 1); @@ -1061,6 +1059,32 @@ mod tests { let recipient = "somebech32address".to_string(); let bridge_address = crate::astria_address([1u8; 20]); let denom = "transfer/channel-0/utia".parse::().unwrap(); + + let state = Arc::new(State::new()); + let startup_handle = startup::InfoHandle::new(state.subscribe()); + state.set_startup_info(startup::Info { + starting_rollup_height: 1, + fee_asset: denom.clone(), + chain_id: "astria".to_string(), + }); + let (batch_tx, mut batch_rx) = mpsc::channel(100); + + let watcher = Builder { + ethereum_contract_address: hex::encode(contract_address), + ethereum_rpc_endpoint: anvil.ws_endpoint(), + startup_handle, + shutdown_token: CancellationToken::new(), + state: Arc::new(State::new()), + rollup_asset_denom: denom.clone(), + bridge_address, + submitter_handle: submitter::Handle::new(batch_tx), + sequencer_address_prefix: crate::ASTRIA_ADDRESS_PREFIX.into(), + } + .build() + .unwrap(); + + tokio::task::spawn(watcher.run()); + let receipt = send_ics20_withdraw_transaction_astria_bridgeable_erc20( &contract, value, @@ -1090,34 +1114,6 @@ mod tests { }; expected_action.timeout_time = 0; // zero this for testing - let state = Arc::new(State::new()); - let startup_handle = startup::InfoHandle::new(state.subscribe()); - state.set_startup_info(startup::Info { - starting_rollup_height: 1, - fee_asset: denom.clone(), - chain_id: "astria".to_string(), - }); - let (batch_tx, mut batch_rx) = mpsc::channel(100); - - let watcher = Builder { - ethereum_contract_address: hex::encode(contract_address), - ethereum_rpc_endpoint: anvil.ws_endpoint(), - startup_handle, - shutdown_token: CancellationToken::new(), - state: Arc::new(State::new()), - rollup_asset_denom: denom, - bridge_address, - submitter_handle: submitter::Handle::new(batch_tx), - sequencer_address_prefix: crate::ASTRIA_ADDRESS_PREFIX.into(), - } - .build() - .unwrap(); - - tokio::task::spawn(watcher.run()); - - // make another tx to trigger anvil to make another block - send_ics20_withdraw_transaction_astria_bridgeable_erc20(&contract, value, recipient).await; - let mut batch = batch_rx.recv().await.unwrap(); assert_eq!(batch.actions.len(), 1); let Action::Ics20Withdrawal(ref mut action) = batch.actions[0] else { diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/tests.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/tests.rs index dc9b20e201..acfe858a75 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/tests.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/tests.rs @@ -9,16 +9,12 @@ use astria_core::{ bridge::Ics20WithdrawalFromRollupMemo, generated::protocol::account::v1alpha1::NonceResponse, primitive::v1::asset, - protocol::{ - account::v1alpha1::AssetBalance, - bridge::v1alpha1::BridgeAccountLastTxHashResponse, - transaction::v1alpha1::{ - action::{ - BridgeUnlockAction, - Ics20Withdrawal, - }, - Action, + protocol::transaction::v1alpha1::{ + action::{ + BridgeUnlockAction, + Ics20Withdrawal, }, + Action, }, }; use astria_eyre::eyre::{ From 72c600c411e11a2e0cf2b29bee85ae12330471c7 Mon Sep 17 00:00:00 2001 From: itamar Date: Tue, 2 Jul 2024 15:51:44 +0200 Subject: [PATCH 18/32] clean up comment --- .../src/bridge_withdrawer/submitter/tests.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/tests.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/tests.rs index acfe858a75..8ffd34327b 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/tests.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/tests.rs @@ -124,7 +124,6 @@ impl TestSubmitter { let cometbft_mock = MockServer::start().await; let sequencer_cometbft_endpoint = format!("http://{}", cometbft_mock.address()); - // startup info let state = Arc::new(state::State::new()); let startup_handle = startup::InfoHandle::new(state.subscribe()); state.set_watcher_ready(); From 3ff81c1e07036562196ea9422895cccf370b0c23 Mon Sep 17 00:00:00 2001 From: itamar Date: Fri, 5 Jul 2024 18:15:44 +0200 Subject: [PATCH 19/32] remove balance check, clean up startup --- .../local.env.example | 3 - .../src/bridge_withdrawer/mod.rs | 2 - .../src/bridge_withdrawer/startup.rs | 76 +------------------ .../src/bridge_withdrawer/state.rs | 8 -- crates/astria-bridge-withdrawer/src/config.rs | 2 - 5 files changed, 1 insertion(+), 90 deletions(-) diff --git a/crates/astria-bridge-withdrawer/local.env.example b/crates/astria-bridge-withdrawer/local.env.example index b01f3ce07a..e9f0e77145 100644 --- a/crates/astria-bridge-withdrawer/local.env.example +++ b/crates/astria-bridge-withdrawer/local.env.example @@ -37,9 +37,6 @@ ASTRIA_BRIDGE_WITHDRAWER_SEQUENCER_ADDRESS_PREFIX=astria # The fee asset denomination to use for the bridge account's transactions. ASTRIA_BRIDGE_WITHDRAWER_FEE_ASSET_DENOMINATION="nria" -# The minimum expected balance of the fee asset in the bridge account. -ASTRIA_BRIDGE_WITHDRAWER_MIN_EXPECTED_FEE_ASSET_BALANCE=1000000 - # The asset denomination being withdrawn from the rollup. ASTRIA_BRIDGE_WITHDRAWER_ROLLUP_ASSET_DENOMINATION="nria" diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/mod.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/mod.rs index d11668eea4..bd3433eaaf 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/mod.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/mod.rs @@ -76,7 +76,6 @@ impl BridgeWithdrawer { ethereum_contract_address, ethereum_rpc_endpoint, rollup_asset_denomination, - min_expected_fee_asset_balance, sequencer_bridge_address, .. } = cfg; @@ -95,7 +94,6 @@ impl BridgeWithdrawer { sequencer_cometbft_endpoint: sequencer_cometbft_endpoint.clone(), sequencer_bridge_address, expected_fee_asset: fee_asset_denomination, - expected_min_fee_asset_balance: u128::from(min_expected_fee_asset_balance), } .build() .wrap_err("failed to initialize startup")?; diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs index 27030c86ab..553b1eab08 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs @@ -23,7 +23,6 @@ use prost::Message as _; use sequencer_client::{ tendermint_rpc, Address, - BalanceResponse, SequencerClientExt as _, SignedTransaction, }; @@ -54,8 +53,6 @@ pub(super) struct Builder { pub(super) sequencer_cometbft_endpoint: String, pub(super) sequencer_bridge_address: Address, pub(super) expected_fee_asset: asset::Denom, - // TODO: change the name of this config var - pub(super) expected_min_fee_asset_balance: u128, } impl Builder { @@ -67,7 +64,6 @@ impl Builder { sequencer_cometbft_endpoint, sequencer_bridge_address, expected_fee_asset, - expected_min_fee_asset_balance, } = self; let sequencer_cometbft_client = @@ -81,7 +77,6 @@ impl Builder { sequencer_cometbft_client, sequencer_bridge_address, expected_fee_asset, - expected_min_fee_asset_balance, }) } } @@ -110,7 +105,7 @@ impl InfoHandle { .rx .wait_for(|state| state.get_startup_info().is_some()) .await - .wrap_err("")?; + .wrap_err("failed to get startup info")?; Ok(state .get_startup_info() @@ -126,7 +121,6 @@ pub(super) struct Startup { sequencer_cometbft_client: sequencer_client::HttpClient, sequencer_bridge_address: Address, expected_fee_asset: asset::Denom, - expected_min_fee_asset_balance: u128, } impl Startup { @@ -209,25 +203,6 @@ impl Startup { "fee_asset_id provided in config is not a valid fee asset on the sequencer" ); - // confirm that the sequencer key has a sufficient balance of the fee asset - let fee_asset_balances = get_latest_balance( - self.sequencer_cometbft_client.clone(), - self.state.clone(), - self.sequencer_bridge_address, - ) - .await - .wrap_err("failed to get latest balance")?; - let fee_asset_balance = fee_asset_balances - .balances - .into_iter() - .find(|balance| balance.denom == self.expected_fee_asset) - .ok_or_eyre("withdrawer's account balance of the fee asset is zero")? - .balance; - ensure!( - fee_asset_balance >= self.expected_min_fee_asset_balance, - "withdrawer account does not have a sufficient balance of the fee asset" - ); - Ok(()) } @@ -391,7 +366,6 @@ async fn get_bridge_account_last_transaction_hash( ) -> eyre::Result { let res = tryhard::retry_fn(|| client.get_bridge_account_last_transaction_hash(address)) .with_config(make_sequencer_retry_config( - state.clone(), "attempt to fetch last bridge account's transaction hash from Sequencer; retrying \ after backoff" .to_string(), @@ -415,7 +389,6 @@ async fn get_tx( ) -> eyre::Result { let res = tryhard::retry_fn(|| client.tx(tx_hash, false)) .with_config(make_cometbft_retry_config( - state.clone(), "attempt to get transaction from CometBFT; retrying after backoff".to_string(), )) .await @@ -433,7 +406,6 @@ async fn get_sequencer_chain_id( ) -> eyre::Result { let genesis: tendermint::Genesis = tryhard::retry_fn(|| client.genesis()) .with_config(make_cometbft_retry_config( - state.clone(), "attempt to get genesis from CometBFT; retrying after backoff".to_string(), )) .await @@ -451,7 +423,6 @@ async fn get_allowed_fee_asset_ids( ) -> eyre::Result { let res = tryhard::retry_fn(|| client.get_allowed_fee_assets()) .with_config(make_sequencer_retry_config( - state.clone(), "attempt to get allowed fee assets from Sequencer; retrying after backoff".to_string(), )) .await @@ -462,45 +433,7 @@ async fn get_allowed_fee_asset_ids( res } -#[instrument(skip_all)] -async fn get_latest_balance( - client: sequencer_client::HttpClient, - state: Arc, - address: Address, -) -> eyre::Result { - let res = tryhard::retry_fn(|| { - // only retry on tendermint_rpc errors, not deserialization or native conversion - let client = client.clone(); - async move { - match client.get_latest_balance(address).await { - Ok(res) => Ok(Ok(res)), - Err(err) => { - if let Some(tendermint_err) = err.as_tendermint_rpc() { - Err(tendermint_err.inner().clone()) - } else { - Ok(Err(err)) - } - } - } - } - }) - .with_config(make_cometbft_retry_config( - state.clone(), - "attempt to get latest balance from CometBFT; retrying after backoff".to_string(), - )) - .await - .wrap_err("failed to get latest balance from Sequencer after a lot of attempts"); - - let res = res?.wrap_err("failed to deserialize the latest balance response from CometBFT"); - - // set cometbft as connected if received a response - state.set_sequencer_connected(res.is_ok()); - - res -} - fn make_cometbft_retry_config( - state: Arc, retry_message: String, ) -> tryhard::RetryFutureConfig< ExponentialBackoff, @@ -511,9 +444,6 @@ fn make_cometbft_retry_config( .max_delay(Duration::from_secs(20)) .on_retry( move |attempt: u32, next_delay: Option, error: &tendermint_rpc::Error| { - let state = Arc::clone(&state); - state.set_cometbft_connected(false); - let wait_duration = next_delay .map(humantime::format_duration) .map(tracing::field::display); @@ -529,7 +459,6 @@ fn make_cometbft_retry_config( } fn make_sequencer_retry_config( - state: Arc, retry_message: String, ) -> tryhard::RetryFutureConfig< ExponentialBackoff, @@ -546,9 +475,6 @@ fn make_sequencer_retry_config( move |attempt: u32, next_delay: Option, error: &sequencer_client::extension_trait::Error| { - let state = Arc::clone(&state); - state.set_sequencer_connected(false); - let wait_duration = next_delay .map(humantime::format_duration) .map(tracing::field::display); diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/state.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/state.rs index 2760c78142..7f39f02daf 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/state.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/state.rs @@ -42,7 +42,6 @@ macro_rules! forward_setter { forward_setter!( [set_startup_info <- startup::Info], - [set_cometbft_connected <- bool], [set_sequencer_connected <- bool], [set_last_rollup_height_submitted <- u64], [set_last_sequencer_height <- u64], @@ -96,13 +95,6 @@ impl StateSnapshot { self.sequencer_connected } - /// Sets the `CometBFT` connection status to `connected`. - fn set_cometbft_connected(&mut self, connected: bool) -> bool { - let changed = self.cometbft_connected ^ connected; - self.cometbft_connected = connected; - changed - } - /// Sets the sequencer connection status to `connected`. fn set_sequencer_connected(&mut self, connected: bool) -> bool { let changed = self.sequencer_connected ^ connected; diff --git a/crates/astria-bridge-withdrawer/src/config.rs b/crates/astria-bridge-withdrawer/src/config.rs index ad9ed3d1ad..7a2f33f754 100644 --- a/crates/astria-bridge-withdrawer/src/config.rs +++ b/crates/astria-bridge-withdrawer/src/config.rs @@ -18,8 +18,6 @@ pub struct Config { pub sequencer_key_path: String, // The fee asset denomination to use for the bridge account's transactions. pub fee_asset_denomination: asset::Denom, - // The minimum expected balance of the fee asset in the bridge account. - pub min_expected_fee_asset_balance: u64, // The asset denomination being withdrawn from the rollup. pub rollup_asset_denomination: String, // The bridge address corresponding to the bridged rollup asset on the sequencer. From 34e8491629697fa33391f590dc26b78dc4824d01 Mon Sep 17 00:00:00 2001 From: itamar Date: Sat, 6 Jul 2024 00:17:02 +0200 Subject: [PATCH 20/32] remove clippy allow --- crates/astria-bridge-withdrawer/src/bridge_withdrawer/state.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/state.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/state.rs index 7f39f02daf..4adba2faf4 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/state.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/state.rs @@ -49,8 +49,6 @@ forward_setter!( ); #[derive(Clone, Debug, Default, PartialEq, Eq, serde::Serialize)] -// This lint is allowed because there are 4 boolean fields that are necessary to track here. -#[allow(clippy::struct_excessive_bools)] pub(crate) struct StateSnapshot { startup_info: Option, From 092b3b56ccde65647e8284f97fb36aa0be9baea7 Mon Sep 17 00:00:00 2001 From: itamar Date: Mon, 8 Jul 2024 12:56:32 +0200 Subject: [PATCH 21/32] fix state --- crates/astria-bridge-withdrawer/src/bridge_withdrawer/state.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/state.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/state.rs index 4adba2faf4..bbdc4c561b 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/state.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/state.rs @@ -55,7 +55,6 @@ pub(crate) struct StateSnapshot { watcher_ready: bool, submitter_ready: bool, - cometbft_connected: bool, sequencer_connected: bool, last_rollup_height_submitted: Option, From 82f06a5014401526f1d07476502ebbb9ad10c7f9 Mon Sep 17 00:00:00 2001 From: itamar Date: Mon, 8 Jul 2024 14:30:13 +0200 Subject: [PATCH 22/32] address comments --- .../src/bridge_withdrawer/ethereum/watcher.rs | 1 - .../src/bridge_withdrawer/mod.rs | 8 -- .../src/bridge_withdrawer/startup.rs | 81 ++++++++++--------- 3 files changed, 41 insertions(+), 49 deletions(-) diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/watcher.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/watcher.rs index f1b321117b..3ba007caf1 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/watcher.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/watcher.rs @@ -201,7 +201,6 @@ impl Watcher { .. } = select! { () = self.shutdown_token.cancelled() => { - info!("watcher received shutdown signal while waiting for startup"); return Err(eyre!("watcher received shutdown signal while waiting for startup")); } diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/mod.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/mod.rs index bd3433eaaf..7a2a9123c2 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/mod.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/mod.rs @@ -326,8 +326,6 @@ impl Shutdown { startup_task.abort(); } } - } else { - info!("startup task was already dead"); } // Giving submitter 20 seconds to shutdown because Kubernetes issues a SIGKILL after 30. @@ -348,8 +346,6 @@ impl Shutdown { submitter_task.abort(); } } - } else { - info!("submitter task was already dead"); } // Giving ethereum watcher 5 seconds to shutdown because Kubernetes issues a SIGKILL after @@ -371,8 +367,6 @@ impl Shutdown { ethereum_watcher_task.abort(); } } - } else { - info!("watcher task was already dead"); } // Giving the API task 4 seconds. 5s for watcher + 20 for submitter + 4s = 29s (out of 30s @@ -392,8 +386,6 @@ impl Shutdown { api_task.abort(); } } - } else { - info!("API server was already dead"); } } } diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs index 553b1eab08..dd3cee1154 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs @@ -14,12 +14,16 @@ use astria_core::{ }; use astria_eyre::eyre::{ self, + bail, ensure, eyre, OptionExt as _, WrapErr as _, }; -use prost::Message as _; +use prost::{ + Message as _, + Name as _, +}; use sequencer_client::{ tendermint_rpc, Address, @@ -127,46 +131,40 @@ impl Startup { pub(super) async fn run(mut self) -> eyre::Result<()> { let shutdown_token = self.shutdown_token.clone(); - let startup_task = tokio::spawn({ - let state = self.state.clone(); - async move { - self.confirm_sequencer_config() - .await - .wrap_err("failed to confirm sequencer config")?; - let starting_rollup_height = self - .get_starting_rollup_height() - .await - .wrap_err("failed to get next rollup block height")?; - - // send the startup info to the submitter - let info = Info { - chain_id: self.sequencer_chain_id.clone(), - fee_asset: self.expected_fee_asset, - starting_rollup_height, - }; - - state.set_startup_info(info); - - Ok(()) - } - }); + let state = self.state.clone(); + let startup_task = async move { + self.confirm_sequencer_config() + .await + .wrap_err("failed to confirm sequencer config")?; + let starting_rollup_height = self + .get_starting_rollup_height() + .await + .wrap_err("failed to get next rollup block height")?; + + // send the startup info to the submitter + let info = Info { + chain_id: self.sequencer_chain_id.clone(), + fee_asset: self.expected_fee_asset, + starting_rollup_height, + }; + + state.set_startup_info(info); + + Ok(()) + }; tokio::select!( () = shutdown_token.cancelled() => { - Err(eyre!("startup was cancelled")) + bail!("startup was cancelled"); } res = startup_task => { - match res { - Ok(Ok(())) => Ok(()), - Ok(Err(err)) => { - error!(%err, "startup failed"); - Err(err)}, - Err(reason) => { - Err(reason.into()) - } + if let Err(err) = res { + error!(%err, "startup failed"); + return Err(err) } } - ) + ); + Ok(()) } /// Confirms configuration values against the sequencer node. Values checked: @@ -180,7 +178,7 @@ impl Startup { /// - `self.chain_id` does not match the value returned from the sequencer node /// - `self.fee_asset_id` is not a valid fee asset on the sequencer node /// - `self.sequencer_key.address` does not have a sufficient balance of `self.fee_asset_id`. - async fn confirm_sequencer_config(&mut self) -> eyre::Result<()> { + async fn confirm_sequencer_config(&self) -> eyre::Result<()> { // confirm the sequencer chain id let actual_chain_id = get_sequencer_chain_id(self.sequencer_cometbft_client.clone(), self.state.clone()) @@ -239,7 +237,7 @@ impl Startup { .wrap_err("failed to convert last transaction hash to tendermint hash")?; // get the corresponding transaction - let last_transaction = get_tx( + let last_transaction = get_sequencer_transaction_at_hash( self.sequencer_cometbft_client.clone(), self.state.clone(), tx_hash, @@ -258,14 +256,17 @@ impl Startup { astria_core::generated::protocol::transaction::v1alpha1::SignedTransaction::decode( &*last_transaction.tx, ) - .wrap_err("failed to convert transaction data from CometBFT to proto")?; + .wrap_err_with(|| format!( + "failed to decode data in Sequencer CometBFT transaction as `{}`", + astria_core::generated::protocol::transaction::v1alpha1::SignedTransaction::full_name(), + ))?; let tx = SignedTransaction::try_from_raw(proto_tx) - .wrap_err("failed to convert transaction data from proto to SignedTransaction")?; + .wrap_err_with(|| format!("failed to verify {}", astria_core::generated::protocol::transaction::v1alpha1::SignedTransaction::full_name()))?; info!( last_bridge_account_tx.hash = %telemetry::display::hex(&tx_hash), - last_bridge_account_tx.height = i64::from(last_transaction.height), + last_bridge_account_tx.height = %last_transaction.height, "fetched last transaction by the bridge account" ); @@ -382,7 +383,7 @@ async fn get_bridge_account_last_transaction_hash( } #[instrument(skip_all)] -async fn get_tx( +async fn get_sequencer_transaction_at_hash( client: sequencer_client::HttpClient, state: Arc, tx_hash: tendermint::Hash, From 5d93e95ee4342a13644fc053f26325c28262f028 Mon Sep 17 00:00:00 2001 From: itamar Date: Mon, 8 Jul 2024 14:32:16 +0200 Subject: [PATCH 23/32] remove dynamic strings --- .../src/bridge_withdrawer/startup.rs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs index dd3cee1154..089f7031fa 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs @@ -16,7 +16,6 @@ use astria_eyre::eyre::{ self, bail, ensure, - eyre, OptionExt as _, WrapErr as _, }; @@ -368,8 +367,7 @@ async fn get_bridge_account_last_transaction_hash( let res = tryhard::retry_fn(|| client.get_bridge_account_last_transaction_hash(address)) .with_config(make_sequencer_retry_config( "attempt to fetch last bridge account's transaction hash from Sequencer; retrying \ - after backoff" - .to_string(), + after backoff", )) .await .wrap_err( @@ -390,7 +388,7 @@ async fn get_sequencer_transaction_at_hash( ) -> eyre::Result { let res = tryhard::retry_fn(|| client.tx(tx_hash, false)) .with_config(make_cometbft_retry_config( - "attempt to get transaction from CometBFT; retrying after backoff".to_string(), + "attempt to get transaction from CometBFT; retrying after backoff", )) .await .wrap_err("failed to get transaction from Sequencer after a lot of attempts"); @@ -407,7 +405,7 @@ async fn get_sequencer_chain_id( ) -> eyre::Result { let genesis: tendermint::Genesis = tryhard::retry_fn(|| client.genesis()) .with_config(make_cometbft_retry_config( - "attempt to get genesis from CometBFT; retrying after backoff".to_string(), + "attempt to get genesis from CometBFT; retrying after backoff", )) .await .wrap_err("failed to get genesis info from Sequencer after a lot of attempts")?; @@ -424,7 +422,7 @@ async fn get_allowed_fee_asset_ids( ) -> eyre::Result { let res = tryhard::retry_fn(|| client.get_allowed_fee_assets()) .with_config(make_sequencer_retry_config( - "attempt to get allowed fee assets from Sequencer; retrying after backoff".to_string(), + "attempt to get allowed fee assets from Sequencer; retrying after backoff", )) .await .wrap_err("failed to get allowed fee asset ids from Sequencer after a lot of attempts"); @@ -435,7 +433,7 @@ async fn get_allowed_fee_asset_ids( } fn make_cometbft_retry_config( - retry_message: String, + retry_message: &'static str, ) -> tryhard::RetryFutureConfig< ExponentialBackoff, impl Fn(u32, Option, &tendermint_rpc::Error) -> futures::future::Ready<()>, @@ -460,7 +458,7 @@ fn make_cometbft_retry_config( } fn make_sequencer_retry_config( - retry_message: String, + retry_message: &'static str, ) -> tryhard::RetryFutureConfig< ExponentialBackoff, impl Fn( From cd5646a86cf1431d589e31e987ee22de806c535b Mon Sep 17 00:00:00 2001 From: itamar Date: Mon, 8 Jul 2024 14:35:16 +0200 Subject: [PATCH 24/32] minor cleanup? --- .../src/bridge_withdrawer/startup.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs index 089f7031fa..b352b10695 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs @@ -157,13 +157,9 @@ impl Startup { bail!("startup was cancelled"); } res = startup_task => { - if let Err(err) = res { - error!(%err, "startup failed"); - return Err(err) - } + res } - ); - Ok(()) + ) } /// Confirms configuration values against the sequencer node. Values checked: From 4ba76a169039fc6d4241a204fa38386dfb87d100 Mon Sep 17 00:00:00 2001 From: itamar Date: Mon, 8 Jul 2024 14:37:23 +0200 Subject: [PATCH 25/32] all hail the crab --- crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs index b352b10695..fb9af177cd 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs @@ -36,7 +36,6 @@ use tendermint_rpc::{ use tokio::sync::watch; use tokio_util::sync::CancellationToken; use tracing::{ - error, info, instrument, warn, From dc1a37c5ea8933ac973132b06eca7b29ab832def Mon Sep 17 00:00:00 2001 From: itamar Date: Mon, 8 Jul 2024 14:45:08 +0200 Subject: [PATCH 26/32] update charts to remove balance fix --- charts/evm-bridge-withdrawer/templates/configmaps.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/charts/evm-bridge-withdrawer/templates/configmaps.yaml b/charts/evm-bridge-withdrawer/templates/configmaps.yaml index c0b0e41901..d0e7c8c0e2 100644 --- a/charts/evm-bridge-withdrawer/templates/configmaps.yaml +++ b/charts/evm-bridge-withdrawer/templates/configmaps.yaml @@ -13,7 +13,6 @@ data: ASTRIA_BRIDGE_WITHDRAWER_SEQUENCER_BRIDGE_ADDRESS: "{{ .Values.config.sequencerBridgeAddress }}" ASTRIA_BRIDGE_WITHDRAWER_FEE_ASSET_DENOMINATION: "{{ .Values.config.feeAssetDenom }}" ASTRIA_BRIDGE_WITHDRAWER_ROLLUP_ASSET_DENOMINATION: "{{ .Values.config.rollupAssetDenom }}" - ASTRIA_BRIDGE_WITHDRAWER_MIN_EXPECTED_FEE_ASSET_BALANCE: "{{ .Values.config.minExpectedFeeAssetBalance }}" ASTRIA_BRIDGE_WITHDRAWER_ETHEREUM_CONTRACT_ADDRESS: "{{ .Values.config.evmContractAddress }}" ASTRIA_BRIDGE_WITHDRAWER_ETHEREUM_RPC_ENDPOINT: "{{ .Values.config.evmRpcEndpoint }}" ASTRIA_BRIDGE_WITHDRAWER_NO_METRICS: "{{ not .Values.metrics.enabled }}" @@ -30,6 +29,7 @@ data: OTEL_EXPORTER_OTLP_TRACE_HEADERS: "{{ .Values.otel.traceHeaders }}" OTEL_SERVICE_NAME: "{{ tpl .Values.otel.serviceName . }}" {{- if not .Values.global.dev }} + ASTRIA_BRIDGE_WITHDRAWER_MIN_EXPECTED_FEE_ASSET_BALANCE: "{{ .Values.config.minExpectedFeeAssetBalance }}" {{- else }} {{- end }} --- From b4ff7897f87399800ff14e5eee150bda03fb95f6 Mon Sep 17 00:00:00 2001 From: itamar Date: Mon, 8 Jul 2024 14:51:47 +0200 Subject: [PATCH 27/32] version bump --- charts/evm-bridge-withdrawer/Chart.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/charts/evm-bridge-withdrawer/Chart.yaml b/charts/evm-bridge-withdrawer/Chart.yaml index 8c87fbe42d..10054ccec2 100644 --- a/charts/evm-bridge-withdrawer/Chart.yaml +++ b/charts/evm-bridge-withdrawer/Chart.yaml @@ -15,7 +15,7 @@ type: application # This is the chart version. This version number should be incremented each time you make changes # to the chart and its templates, including the app version. # Versions are expected to follow Semantic Versioning (https://semver.org/) -version: 0.0.1 +version: 0.0.2 # This is the version number of the application being deployed. This version number should be # incremented each time you make changes to the application. Versions are not expected to From 197786147ad7b20199efc6ad76f11a15c289d761 Mon Sep 17 00:00:00 2001 From: itamar Date: Tue, 9 Jul 2024 11:27:12 +0200 Subject: [PATCH 28/32] fix memo import --- .../src/bridge_withdrawer/startup.rs | 12 +++++++----- .../src/bridge_withdrawer/submitter/tests.rs | 6 ++++-- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs index fb9af177cd..ef745c037d 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs @@ -4,7 +4,10 @@ use std::{ }; use astria_core::{ - bridge::Ics20WithdrawalFromRollupMemo, + bridge::{ + self, + Ics20WithdrawalFromRollupMemo, + }, primitive::v1::asset, protocol::{ asset::v1alpha1::AllowedFeeAssetsResponse, @@ -46,7 +49,6 @@ use super::state::{ self, State, }; -use crate::bridge_withdrawer::ethereum::convert::BridgeUnlockMemo; pub(super) struct Builder { pub(super) shutdown_token: CancellationToken, @@ -192,7 +194,7 @@ impl Startup { allowed_fee_asset_ids_resp .fee_assets .contains(&self.expected_fee_asset), - "fee_asset_id provided in config is not a valid fee asset on the sequencer" + "fee_asset provided in config is not a valid fee asset on the sequencer" ); Ok(()) @@ -331,9 +333,9 @@ fn rollup_height_from_signed_transaction( let last_batch_rollup_height = match withdrawal_action { Action::BridgeUnlock(action) => { - let memo: BridgeUnlockMemo = serde_json::from_slice(&action.memo) + let memo: bridge::UnlockMemo = serde_json::from_slice(&action.memo) .wrap_err("failed to parse memo from last transaction by the bridge account")?; - Some(memo.block_number.as_u64()) + Some(memo.block_number) } Action::Ics20Withdrawal(action) => { let memo: Ics20WithdrawalFromRollupMemo = serde_json::from_str(&action.memo) diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/tests.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/tests.rs index 290309d103..edc2615b6c 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/tests.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/tests.rs @@ -6,7 +6,10 @@ use std::{ }; use astria_core::{ - bridge::Ics20WithdrawalFromRollupMemo, + bridge::{ + self, + Ics20WithdrawalFromRollupMemo, + }, generated::protocol::account::v1alpha1::NonceResponse, primitive::v1::asset, protocol::transaction::v1alpha1::{ @@ -62,7 +65,6 @@ use super::Submitter; use crate::{ bridge_withdrawer::{ batch::Batch, - ethereum::convert::BridgeUnlockMemo, startup, state, submitter, From 18a8510d21cb6c88bba92bcb456ab543c2fd7875 Mon Sep 17 00:00:00 2001 From: itamar Date: Tue, 9 Jul 2024 12:57:56 +0200 Subject: [PATCH 29/32] fix startup --- .../src/bridge_withdrawer/ethereum/watcher.rs | 2 +- .../src/bridge_withdrawer/mod.rs | 96 ++++++++++--------- .../src/bridge_withdrawer/startup.rs | 4 +- 3 files changed, 57 insertions(+), 45 deletions(-) diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/watcher.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/watcher.rs index 3ba007caf1..6b29c245fa 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/watcher.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/watcher.rs @@ -412,7 +412,7 @@ async fn get_and_send_events_at_block( } if batch.actions.is_empty() { - debug!("no actions to send at block {block_number}"); + trace!("no actions to send at block {block_number}"); } else { let actions_len = batch.actions.len(); submitter_handle diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/mod.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/mod.rs index 7a2a9123c2..915a225780 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/mod.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/mod.rs @@ -171,7 +171,7 @@ impl BridgeWithdrawer { }); info!("spawned API server"); - let mut startup_task = tokio::spawn(startup.run()); + let mut startup_task = Some(tokio::spawn(startup.run())); info!("spawned startup task"); let mut submitter_task = tokio::spawn(submitter.run()); @@ -179,53 +179,63 @@ impl BridgeWithdrawer { let mut ethereum_watcher_task = tokio::spawn(ethereum_watcher.run()); info!("spawned ethereum watcher task"); - let shutdown = select!( - o = &mut api_task => { - report_exit("api server", o); - Shutdown { - api_task: None, - submitter_task: Some(submitter_task), - ethereum_watcher_task: Some(ethereum_watcher_task), - startup_task: Some(startup_task), - api_shutdown_signal, - token: shutdown_token + let shutdown = loop { + select!( + o = async { startup_task.as_mut().unwrap().await }, if !startup_task.is_some() => { + match o { + Ok(_) => { + info!(task = "startup", "task has exited"); + startup_task = None; + }, + Err(error) => { + error!(task = "startup", %error, "task returned with error"); + break Shutdown { + api_task: Some(api_task), + submitter_task: Some(submitter_task), + ethereum_watcher_task: Some(ethereum_watcher_task), + startup_task: None, + api_shutdown_signal, + token: shutdown_token, + }; + } + } } - } - o = &mut startup_task => { - report_exit("startup", o); - Shutdown { - api_task: Some(api_task), - submitter_task: Some(submitter_task), - ethereum_watcher_task: Some(ethereum_watcher_task), - startup_task: None, - api_shutdown_signal, - token: shutdown_token + o = &mut api_task => { + report_exit("api server", o); + break Shutdown { + api_task: None, + submitter_task: Some(submitter_task), + ethereum_watcher_task: Some(ethereum_watcher_task), + startup_task: startup_task, + api_shutdown_signal, + token: shutdown_token + } } - } - o = &mut submitter_task => { - report_exit("submitter", o); - Shutdown { - api_task: Some(api_task), - submitter_task: None, - ethereum_watcher_task:Some(ethereum_watcher_task), - startup_task: Some(startup_task), - api_shutdown_signal, - token: shutdown_token + o = &mut submitter_task => { + report_exit("submitter", o); + break Shutdown { + api_task: Some(api_task), + submitter_task: None, + ethereum_watcher_task:Some(ethereum_watcher_task), + startup_task: startup_task, + api_shutdown_signal, + token: shutdown_token + } } - } - o = &mut ethereum_watcher_task => { - report_exit("ethereum watcher", o); - Shutdown { - api_task: Some(api_task), - submitter_task: Some(submitter_task), - ethereum_watcher_task: None, - startup_task: Some(startup_task), - api_shutdown_signal, - token: shutdown_token + o = &mut ethereum_watcher_task => { + report_exit("ethereum watcher", o); + break Shutdown { + api_task: Some(api_task), + submitter_task: Some(submitter_task), + ethereum_watcher_task: None, + startup_task: startup_task, + api_shutdown_signal, + token: shutdown_token + } } - } - ); + ) + }; shutdown.run().await; } } diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs index ef745c037d..d68e7392bc 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs @@ -190,10 +190,12 @@ impl Startup { get_allowed_fee_asset_ids(self.sequencer_cometbft_client.clone(), self.state.clone()) .await .wrap_err("failed to get allowed fee asset ids from sequencer")?; + let expected_fee_asset_ibc = self.expected_fee_asset.to_ibc_prefixed(); ensure!( allowed_fee_asset_ids_resp .fee_assets - .contains(&self.expected_fee_asset), + .iter() + .any(|asset| asset.to_ibc_prefixed() == expected_fee_asset_ibc), "fee_asset provided in config is not a valid fee asset on the sequencer" ); From 62e1fd977fca3da01ec3fe19d813f303453aab02 Mon Sep 17 00:00:00 2001 From: itamar Date: Tue, 9 Jul 2024 13:05:36 +0200 Subject: [PATCH 30/32] all hail the crab lord --- .../src/bridge_withdrawer/ethereum/watcher.rs | 1 + .../src/bridge_withdrawer/mod.rs | 13 +++++++------ 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/watcher.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/watcher.rs index 6b29c245fa..adb7de7457 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/watcher.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/watcher.rs @@ -50,6 +50,7 @@ use tokio_util::sync::CancellationToken; use tracing::{ debug, info, + trace, warn, }; diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/mod.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/mod.rs index 915a225780..25c8a86a94 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/mod.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/mod.rs @@ -148,6 +148,8 @@ impl BridgeWithdrawer { Ok((service, shutdown_handle)) } + // need this lint allow because startup_task is wrapped with an Option instead of being fused + #[allow(clippy::missing_panics_doc)] pub async fn run(self) { let Self { shutdown_token, @@ -181,7 +183,7 @@ impl BridgeWithdrawer { let shutdown = loop { select!( - o = async { startup_task.as_mut().unwrap().await }, if !startup_task.is_some() => { + o = async { startup_task.as_mut().unwrap().await }, if startup_task.is_none() => { match o { Ok(_) => { info!(task = "startup", "task has exited"); @@ -206,7 +208,7 @@ impl BridgeWithdrawer { api_task: None, submitter_task: Some(submitter_task), ethereum_watcher_task: Some(ethereum_watcher_task), - startup_task: startup_task, + startup_task, api_shutdown_signal, token: shutdown_token } @@ -217,7 +219,7 @@ impl BridgeWithdrawer { api_task: Some(api_task), submitter_task: None, ethereum_watcher_task:Some(ethereum_watcher_task), - startup_task: startup_task, + startup_task, api_shutdown_signal, token: shutdown_token } @@ -228,13 +230,12 @@ impl BridgeWithdrawer { api_task: Some(api_task), submitter_task: Some(submitter_task), ethereum_watcher_task: None, - startup_task: startup_task, + startup_task, api_shutdown_signal, token: shutdown_token } } - - ) + ); }; shutdown.run().await; } From b4a2ceebb92dbb0f24f0f0edec0b48fc55abfe2d Mon Sep 17 00:00:00 2001 From: itamar Date: Tue, 9 Jul 2024 13:26:18 +0200 Subject: [PATCH 31/32] fix comment --- crates/astria-bridge-withdrawer/src/bridge_withdrawer/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/mod.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/mod.rs index 25c8a86a94..d655f9c645 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/mod.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/mod.rs @@ -148,7 +148,7 @@ impl BridgeWithdrawer { Ok((service, shutdown_handle)) } - // need this lint allow because startup_task is wrapped with an Option instead of being fused + // Panic won't happen because `startup_task` is unwraped lazily after checking if it's `Some`. #[allow(clippy::missing_panics_doc)] pub async fn run(self) { let Self { From 6b23052567742450cb37f545d7db1ecb2610c07d Mon Sep 17 00:00:00 2001 From: itamar Date: Tue, 9 Jul 2024 13:51:03 +0200 Subject: [PATCH 32/32] update smoke test --- charts/deploy.just | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/charts/deploy.just b/charts/deploy.just index b502c56ce3..513fc92ae3 100644 --- a/charts/deploy.just +++ b/charts/deploy.just @@ -272,7 +272,7 @@ run-smoke-test: sleep 1 fi done - if [ $CHECKS -eq $MAX_CHECKS ]; then + if [ $CHECKS -gt $MAX_CHECKS ]; then echo "Bridge Out Sequencer failure" exit 1 fi @@ -293,8 +293,8 @@ run-smoke-test: sleep 1 fi done - if [ $CHECKS -eq $MAX_CHECKS ]; then - echo "Bridge Out Sequencer failure" + if [ $CHECKS -gt $MAX_CHECKS ]; then + echo "Finalization failure" exit 1 fi exit 0