From bb186c2450e1b33fc0783cd236076da46a87125f Mon Sep 17 00:00:00 2001 From: Samuel Onoja Date: Fri, 15 Nov 2024 14:31:58 +0100 Subject: [PATCH] fix: self review --- mm2src/coins/eth.rs | 2 +- .../src/connection_handler.rs | 2 +- mm2src/kdf_walletconnect/src/lib.rs | 37 +++++++------------ mm2src/kdf_walletconnect/src/session.rs | 17 ++------- .../src/session/rpc/event.rs | 23 +++++++----- mm2src/kdf_walletconnect/src/storage/mod.rs | 2 +- 6 files changed, 34 insertions(+), 49 deletions(-) diff --git a/mm2src/coins/eth.rs b/mm2src/coins/eth.rs index 0a5f327c82..247c90af91 100644 --- a/mm2src/coins/eth.rs +++ b/mm2src/coins/eth.rs @@ -2956,7 +2956,7 @@ async fn sign_raw_eth_tx(coin: &EthCoin, args: &SignEthTransactionParams) -> Raw }) }, EthPrivKeyPolicy::Trezor => MmError::err(RawTransactionError::InvalidParam( - "sign raw eth tx not implemented for Metamask".into(), + "sign raw eth tx not implemented for Trezor".into(), )), #[cfg(target_arch = "wasm32")] EthPrivKeyPolicy::Metamask(_) => MmError::err(RawTransactionError::InvalidParam( diff --git a/mm2src/kdf_walletconnect/src/connection_handler.rs b/mm2src/kdf_walletconnect/src/connection_handler.rs index 8c4c0feb61..28a52b0072 100644 --- a/mm2src/kdf_walletconnect/src/connection_handler.rs +++ b/mm2src/kdf_walletconnect/src/connection_handler.rs @@ -82,7 +82,7 @@ pub(crate) async fn initialize_connection(wc: Arc) { while let Err(err) = wc.connect_client().await { retry_count += 1; - info!( + error!( "Error during initial connection attempt {}: {:?}. Retrying in {retry_secs} seconds...", retry_count, err ); diff --git a/mm2src/kdf_walletconnect/src/lib.rs b/mm2src/kdf_walletconnect/src/lib.rs index c6c4d3320d..9b8ebcb4d7 100644 --- a/mm2src/kdf_walletconnect/src/lib.rs +++ b/mm2src/kdf_walletconnect/src/lib.rs @@ -43,8 +43,7 @@ use storage::SessionStorageDb; use storage::WalletConnectStorageOps; use wc_common::{decode_and_decrypt_type0, encrypt_and_encode, EnvelopeType, SymKey}; -const WAIT_DURATION: Duration = Duration::from_secs(60); -const PUBLISH_TIMEOUT_SECS: f64 = 5.0; +const PUBLISH_TIMEOUT_SECS: f64 = 6.; const MAX_RETRIES: usize = 5; #[async_trait::async_trait] @@ -70,36 +69,30 @@ pub trait WalletConnectOps { } pub struct WalletConnectCtx { - pub client: Client, - pub pairing: PairingClient, + pub(crate) client: Client, + pub(crate) pairing: PairingClient, pub session: SessionManager, - pub(crate) key_pair: SymKeyPair, - relay: Relay, metadata: Metadata, inbound_message_rx: Arc>>, connection_live_rx: Arc>>>, message_tx: UnboundedSender, message_rx: Arc>>, - _abort_handle: AbortOnDropHandle, } impl WalletConnectCtx { pub fn try_init(ctx: &MmArc) -> MmResult { - let (inbound_message_tx, inbound_message_rx) = unbounded(); - let (conn_live_sender, conn_live_receiver) = unbounded(); - let (message_tx, session_request_receiver) = unbounded(); - + let storage = SessionStorageDb::new(ctx)?; let pairing = PairingClient::new(); let relay = Relay { protocol: SUPPORTED_PROTOCOL.to_string(), data: None, }; - - let storage = SessionStorageDb::init(ctx)?; - + let (inbound_message_tx, inbound_message_rx) = unbounded(); + let (conn_live_sender, conn_live_receiver) = unbounded(); + let (message_tx, session_request_receiver) = unbounded(); let (client, _abort_handle) = Client::new_with_callback( Handler::new("Komodefi", inbound_message_tx, conn_live_sender), |r, h| spawn_abortable(client_event_loop(r, h)), @@ -112,12 +105,10 @@ impl WalletConnectCtx { metadata: generate_metadata(), key_pair: SymKeyPair::new(), session: SessionManager::new(storage), - inbound_message_rx: Arc::new(inbound_message_rx.into()), connection_live_rx: Arc::new(conn_live_receiver.into()), message_rx: Arc::new(session_request_receiver.into()), message_tx, - _abort_handle, }) } @@ -134,7 +125,7 @@ impl WalletConnectCtx { let key = SigningKey::generate(&mut rand::thread_rng()); AuthToken::new(AUTH_TOKEN_SUB) .aud(RELAY_ADDRESS) - .ttl(Duration::from_secs(8 * 60 * 60)) + .ttl(Duration::from_secs(5 * 60 * 60)) .as_jwt(&key) .map_to_mm(|err| WalletConnectError::InternalError(err.to_string()))? }; @@ -339,7 +330,7 @@ impl WalletConnectCtx { } MmError::err(WalletConnectError::InternalError( - "client connection timeout".to_string(), + "[{topic}] client connection timeout".to_string(), )) } @@ -501,10 +492,11 @@ impl WalletConnectCtx { params, }, }; - self.publish_request(&active_topic, RequestParams::SessionRequest(request)) - .await?; - // Wait for response - if let Ok(Some(resp)) = self.message_rx.lock().await.next().timeout(WAIT_DURATION).await { + let request = RequestParams::SessionRequest(request); + let ttl = request.irn_metadata().ttl; + self.publish_request(&active_topic, request).await?; + + if let Ok(Some(resp)) = self.message_rx.lock().await.next().timeout_secs(ttl as f64).await { let result = resp.mm_err(WalletConnectError::InternalError)?; if let ResponseParamsSuccess::Arbitrary(data) = result.data { let data = serde_json::from_value::(data)?; @@ -512,7 +504,6 @@ impl WalletConnectCtx { } } - // QUESTION: should we consider retrying request instead of returning error immediately. MmError::err(WalletConnectError::NoWalletFeedback) } diff --git a/mm2src/kdf_walletconnect/src/session.rs b/mm2src/kdf_walletconnect/src/session.rs index 0a6f9eb25a..967de77af8 100644 --- a/mm2src/kdf_walletconnect/src/session.rs +++ b/mm2src/kdf_walletconnect/src/session.rs @@ -10,6 +10,7 @@ use common::log::info; use dashmap::mapref::multiple::RefMulti; use dashmap::mapref::one::RefMut; use dashmap::DashMap; +use derive_more::Display; use key::SessionKey; use mm2_err_handle::prelude::{MmError, MmResult}; use relay_rpc::domain::Topic; @@ -26,15 +27,15 @@ use std::sync::Arc; use tokio::sync::Mutex; use wc_common::SymKey; -pub(crate) const FIVE_MINUTES: u64 = 300; -pub(crate) const THIRTY_DAYS: u64 = 60 * 60 * 30; +pub(crate) const FIVE_MINUTES: u64 = 5 * 60; +pub(crate) const THIRTY_DAYS: u64 = 30 * 24 * 60 * 60; pub(crate) type WcRequestResponseResult = MmResult<(Value, IrnMetadata), WalletConnectError>; /// In the WalletConnect protocol, a session involves two parties: a controller /// (typically a wallet) and a proposer (typically a dApp). This enum is used /// to distinguish between these two roles. -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +#[derive(Debug, Display, Clone, Serialize, Deserialize, PartialEq, Eq)] pub enum SessionType { /// Represents the controlling party in a session, typically a wallet. Controller, @@ -42,15 +43,6 @@ pub enum SessionType { Proposer, } -impl ToString for SessionType { - fn to_string(&self) -> String { - match self { - Self::Controller => "Controller".to_string(), - Self::Proposer => "Proposer".to_string(), - } - } -} - #[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Eq)] pub struct SessionRpcInfo { pub topic: Topic, @@ -146,7 +138,6 @@ impl Session { metadata: Metadata, session_type: SessionType, ) -> Self { - // handle proposer or controller let (proposer, controller) = match session_type { SessionType::Proposer => ( Proposer { diff --git a/mm2src/kdf_walletconnect/src/session/rpc/event.rs b/mm2src/kdf_walletconnect/src/session/rpc/event.rs index 78642f8327..a70ebf2ca3 100644 --- a/mm2src/kdf_walletconnect/src/session/rpc/event.rs +++ b/mm2src/kdf_walletconnect/src/session/rpc/event.rs @@ -32,17 +32,20 @@ pub async fn handle_session_event( ctx.validate_chain_id(&session, &chain_id).await?; - if let Some(active_chain_id) = session.get_active_chain_id().await { - if &chain_id == active_chain_id { - return Ok(()); - } + if session + .get_active_chain_id() + .await + .as_ref() + .map_or(false, |c| c == &chain_id) + { + return Ok(()); }; - // check if chain_id is supported. - let id_string = serde_json::from_value::(event.event.data)?; - let new_chain = chain_id.chain.derive_chain_id(id_string.to_string()); + // check if if new chain_id is supported. + let new_id = serde_json::from_value::(event.event.data)?; + let new_chain = chain_id.chain.derive_chain_id(new_id.to_string()); if let Err(err) = ctx.validate_chain_id(&session, &new_chain).await { - error!("{err:?}"); + error!("[{topic}] {err:?}"); let error_data = ErrorData { code: UNSUPPORTED_CHAINS, message: "Unsupported chain id".to_string(), @@ -57,7 +60,7 @@ pub async fn handle_session_event( .ok_or(MmError::new(WalletConnectError::SessionError( "No active WalletConnect session found".to_string(), )))? - .set_active_chain_id(chain_id.clone()) + .set_active_chain_id(chain_id) .await; } }; @@ -70,6 +73,6 @@ pub async fn handle_session_event( }, }; - info!("chainChanged event handled successfully"); + info!("[{topic}] chainChanged event handled successfully"); Ok(()) } diff --git a/mm2src/kdf_walletconnect/src/storage/mod.rs b/mm2src/kdf_walletconnect/src/storage/mod.rs index f94b5502cc..1c8c41fd78 100644 --- a/mm2src/kdf_walletconnect/src/storage/mod.rs +++ b/mm2src/kdf_walletconnect/src/storage/mod.rs @@ -37,7 +37,7 @@ impl Deref for SessionStorageDb { } impl SessionStorageDb { - pub(crate) fn init(ctx: &MmArc) -> MmResult { + pub(crate) fn new(ctx: &MmArc) -> MmResult { let db = DB::new(ctx).mm_err(|err| WalletConnectError::StorageError(err.to_string()))?; Ok(SessionStorageDb(db))