From 80e2c66ec76eaa6a89d53775ad66f92314820cff Mon Sep 17 00:00:00 2001 From: Lu Zhang <8418040+longbowlu@users.noreply.github.com> Date: Wed, 17 Apr 2024 15:19:04 -0700 Subject: [PATCH] [bridge/70] cache the result of get_mutable_bridge_object_arg (#17152) ## Description instead of querying the bridge arg on chain every time, we do this in the beginning of the program and cache it. If the fetch (with retry) fails, it panics. ## Test plan existing tests --- ## Release notes Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required. For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates. - [ ] Protocol: - [ ] Nodes (Validators and Full nodes): - [ ] Indexer: - [ ] JSON-RPC: - [ ] GraphQL: - [ ] CLI: - [ ] Rust SDK: --- crates/sui-bridge/src/action_executor.rs | 14 +++--- crates/sui-bridge/src/e2e_tests/basic.rs | 5 +- crates/sui-bridge/src/node.rs | 3 +- crates/sui-bridge/src/sui_client.rs | 47 +++++++++---------- .../sui-bridge/src/sui_transaction_builder.rs | 20 ++++++-- crates/sui-bridge/src/tools/cli.rs | 5 +- crates/sui/src/sui_commands.rs | 5 +- 7 files changed, 52 insertions(+), 47 deletions(-) diff --git a/crates/sui-bridge/src/action_executor.rs b/crates/sui-bridge/src/action_executor.rs index 1650e68aabb93e..f7853b47bbb88e 100644 --- a/crates/sui-bridge/src/action_executor.rs +++ b/crates/sui-bridge/src/action_executor.rs @@ -19,7 +19,6 @@ use sui_types::{ transaction::Transaction, }; -use crate::error::BridgeResult; use crate::{ client::bridge_authority_aggregator::BridgeAuthorityAggregator, error::BridgeError, @@ -94,9 +93,11 @@ where key: SuiKeyPair, sui_address: SuiAddress, gas_object_id: ObjectID, - ) -> BridgeResult { - let bridge_object_arg = sui_client.get_mutable_bridge_object_arg().await?; - Ok(Self { + ) -> Self { + let bridge_object_arg = sui_client + .get_mutable_bridge_object_arg_must_succeed() + .await; + Self { sui_client, bridge_auth_agg, store, @@ -104,7 +105,7 @@ where gas_object_id, sui_address, bridge_object_arg, - }) + } } fn run_inner( @@ -1088,8 +1089,7 @@ mod tests { sui_address, gas_object_ref.0, ) - .await - .unwrap(); + .await; let (executor_handle, signing_tx, execution_tx) = executor.run_inner(); handles.extend(executor_handle); diff --git a/crates/sui-bridge/src/e2e_tests/basic.rs b/crates/sui-bridge/src/e2e_tests/basic.rs index 34ad82fcc02547..b442bd56ce1af8 100644 --- a/crates/sui-bridge/src/e2e_tests/basic.rs +++ b/crates/sui-bridge/src/e2e_tests/basic.rs @@ -179,9 +179,8 @@ async fn test_bridge_from_eth_to_sui_to_eth() { // Now let the recipient send the coin back to ETH let eth_address_1 = EthAddress::random(); let bridge_obj_arg = sui_bridge_client - .get_mutable_bridge_object_arg() - .await - .unwrap(); + .get_mutable_bridge_object_arg_must_succeed() + .await; let nonce = 0; let sui_token_type_tags = sui_bridge_client.get_token_id_map().await.unwrap(); diff --git a/crates/sui-bridge/src/node.rs b/crates/sui-bridge/src/node.rs index f597389d3b4622..56fc770158e910 100644 --- a/crates/sui-bridge/src/node.rs +++ b/crates/sui-bridge/src/node.rs @@ -147,8 +147,7 @@ async fn start_client_components( client_config.sui_address, client_config.gas_object_ref.0, ) - .await - .expect("Failed to create bridge action executor"); + .await; let orchestrator = BridgeOrchestrator::new(sui_client, sui_events_rx, eth_events_rx, store.clone()); diff --git a/crates/sui-bridge/src/sui_client.rs b/crates/sui-bridge/src/sui_client.rs index 92fa7533e5328b..ac0ac03716c3d9 100644 --- a/crates/sui-bridge/src/sui_client.rs +++ b/crates/sui-bridge/src/sui_client.rs @@ -7,10 +7,10 @@ use anyhow::anyhow; use async_trait::async_trait; use axum::response::sse::Event; +use core::panic; use ethers::types::{Address, U256}; use fastcrypto::traits::KeyPair; use fastcrypto::traits::ToFromBytes; -use once_cell::sync::OnceCell; use serde::{Deserialize, Serialize}; use std::collections::HashMap; use std::str::from_utf8; @@ -62,6 +62,7 @@ use sui_types::{ }; use sui_types::{bridge, parse_sui_type_tag}; use tap::TapFallible; +use tokio::sync::OnceCell; use tracing::{error, warn}; use crate::crypto::BridgeAuthorityPublicKey; @@ -109,12 +110,22 @@ where Ok(()) } - // TODO(bridge): cache this - pub async fn get_mutable_bridge_object_arg(&self) -> BridgeResult { - self.inner - .get_mutable_bridge_object_arg() - .await - .map_err(|e| BridgeError::Generic(format!("Can't get mutable bridge object arg: {e}"))) + /// Get the mutable bridge object arg on chain. + // We retry a few times in case of errors. If it fails eventually, we panic. + // In generaly it's safe to call in the beginning of the program. + // After the first call, the result is cached since the value should never change. + pub async fn get_mutable_bridge_object_arg_must_succeed(&self) -> ObjectArg { + static ARG: OnceCell = OnceCell::const_new(); + *ARG.get_or_init(|| async move { + let Ok(Ok(bridge_object_arg)) = retry_with_max_elapsed_time!( + self.inner.get_mutable_bridge_object_arg(), + Duration::from_secs(30) + ) else { + panic!("Failed to get bridge object arg after retries"); + }; + bridge_object_arg + }) + .await } /// Query emitted Events that are defined in the given Move Module. @@ -276,14 +287,7 @@ where ) -> BridgeActionStatus { let now = std::time::Instant::now(); loop { - let Ok(Ok(bridge_object_arg)) = retry_with_max_elapsed_time!( - self.get_mutable_bridge_object_arg(), - Duration::from_secs(30) - ) else { - // TODO: add metrics and fire alert - error!("Failed to get bridge object arg"); - continue; - }; + let bridge_object_arg = self.get_mutable_bridge_object_arg_must_succeed().await; let Ok(Ok(status)) = retry_with_max_elapsed_time!( self.inner.get_token_transfer_action_onchain_status( bridge_object_arg, @@ -310,14 +314,7 @@ where ) -> Option>> { let now = std::time::Instant::now(); loop { - let Ok(Ok(bridge_object_arg)) = retry_with_max_elapsed_time!( - self.get_mutable_bridge_object_arg(), - Duration::from_secs(30) - ) else { - // TODO: add metrics and fire alert - error!("Failed to get bridge object arg"); - continue; - }; + let bridge_object_arg = self.get_mutable_bridge_object_arg_must_succeed().await; let Ok(Ok(sigs)) = retry_with_max_elapsed_time!( self.inner.get_token_transfer_action_onchain_signatures( bridge_object_arg, @@ -770,7 +767,9 @@ mod tests { let sender = context.active_address().unwrap(); let summary = sui_client.inner.get_bridge_summary().await.unwrap(); let usdc_amount = 5000000; - let bridge_object_arg = sui_client.get_mutable_bridge_object_arg().await.unwrap(); + let bridge_object_arg = sui_client + .get_mutable_bridge_object_arg_must_succeed() + .await; let id_token_map = sui_client.get_token_id_map().await.unwrap(); // 1. Create a Eth -> Sui Transfer (recipient is sender address), approve with validator secrets and assert its status to be Claimed diff --git a/crates/sui-bridge/src/sui_transaction_builder.rs b/crates/sui-bridge/src/sui_transaction_builder.rs index f428a4d55b9947..2b3b2d62379f6d 100644 --- a/crates/sui-bridge/src/sui_transaction_builder.rs +++ b/crates/sui-bridge/src/sui_transaction_builder.rs @@ -624,7 +624,9 @@ mod tests { let context = &mut test_cluster.wallet; let sender = context.active_address().unwrap(); let usdc_amount = 5000000; - let bridge_object_arg = sui_client.get_mutable_bridge_object_arg().await.unwrap(); + let bridge_object_arg = sui_client + .get_mutable_bridge_object_arg_must_succeed() + .await; let id_token_map = sui_client.get_token_id_map().await.unwrap(); // 1. Test Eth -> Sui Transfer approval @@ -692,7 +694,9 @@ mod tests { assert!(!summary.is_frozen); let context = &mut test_cluster.wallet; - let bridge_object_arg = sui_client.get_mutable_bridge_object_arg().await.unwrap(); + let bridge_object_arg = sui_client + .get_mutable_bridge_object_arg_must_succeed() + .await; let id_token_map = sui_client.get_token_id_map().await.unwrap(); // 1. Pause @@ -757,7 +761,9 @@ mod tests { } let context = &mut test_cluster.wallet; - let bridge_object_arg = sui_client.get_mutable_bridge_object_arg().await.unwrap(); + let bridge_object_arg = sui_client + .get_mutable_bridge_object_arg_must_succeed() + .await; let id_token_map = sui_client.get_token_id_map().await.unwrap(); // 1. blocklist The victim @@ -842,7 +848,9 @@ mod tests { .collect::>(); let context = &mut test_cluster.wallet; - let bridge_object_arg = sui_client.get_mutable_bridge_object_arg().await.unwrap(); + let bridge_object_arg = sui_client + .get_mutable_bridge_object_arg_must_succeed() + .await; let id_token_map = sui_client.get_token_id_map().await.unwrap(); // update limit @@ -898,7 +906,9 @@ mod tests { assert_ne!(notional_values[&TOKEN_ID_USDC], 69_000 * USD_MULTIPLIER); let context = &mut test_cluster.wallet; - let bridge_object_arg = sui_client.get_mutable_bridge_object_arg().await.unwrap(); + let bridge_object_arg = sui_client + .get_mutable_bridge_object_arg_must_succeed() + .await; let id_token_map = sui_client.get_token_id_map().await.unwrap(); // update price diff --git a/crates/sui-bridge/src/tools/cli.rs b/crates/sui-bridge/src/tools/cli.rs index 0d1ef3413beaf3..2421d90b49f61d 100644 --- a/crates/sui-bridge/src/tools/cli.rs +++ b/crates/sui-bridge/src/tools/cli.rs @@ -90,9 +90,8 @@ async fn main() -> anyhow::Result<()> { .await .expect("Failed to request committee signatures"); let bridge_arg = sui_client - .get_mutable_bridge_object_arg() - .await - .expect("Failed to get mutable bridge object arg"); + .get_mutable_bridge_object_arg_must_succeed() + .await; let id_token_map = sui_client.get_token_id_map().await.unwrap(); let tx = build_sui_transaction( sui_address, diff --git a/crates/sui/src/sui_commands.rs b/crates/sui/src/sui_commands.rs index 5c977244d1090b..ec2b6e26d54ccf 100644 --- a/crates/sui/src/sui_commands.rs +++ b/crates/sui/src/sui_commands.rs @@ -367,9 +367,8 @@ impl SuiCommand { println!("rpc_url: {}", rpc_url); let sui_bridge_client = SuiBridgeClient::new(rpc_url).await?; let bridge_arg = sui_bridge_client - .get_mutable_bridge_object_arg() - .await - .unwrap(); + .get_mutable_bridge_object_arg_must_succeed() + .await; assert_eq!( network_config.validator_configs().len(), bridge_committee_config