From b7852795c0409febdd7250f996a7b129091972d3 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Fri, 13 Dec 2024 16:06:57 -0500 Subject: [PATCH 1/3] Add an `account_id` to errors which may be reported This is going to be the `offender` account ID which gets submitted on-chain. --- crates/protocol/tests/helpers/mod.rs | 1 + .../src/helpers/signing.rs | 15 +++++- .../src/helpers/user.rs | 1 + .../src/signing_client/api.rs | 2 + .../src/signing_client/errors.rs | 20 ++++---- .../src/signing_client/protocol_transport.rs | 48 ++++++++++++++----- 6 files changed, 64 insertions(+), 23 deletions(-) diff --git a/crates/protocol/tests/helpers/mod.rs b/crates/protocol/tests/helpers/mod.rs index cae40306a..670775e37 100644 --- a/crates/protocol/tests/helpers/mod.rs +++ b/crates/protocol/tests/helpers/mod.rs @@ -255,6 +255,7 @@ async fn open_protocol_connections( // Check the response as to whether they accepted our SubscribeMessage let response_message = encrypted_connection.recv().await?; let subscribe_response: Result<(), String> = bincode::deserialize(&response_message)?; + if let Err(error_message) = subscribe_response { return Err(anyhow!(error_message)); } diff --git a/crates/threshold-signature-server/src/helpers/signing.rs b/crates/threshold-signature-server/src/helpers/signing.rs index 609dde2f2..b1e68c4ad 100644 --- a/crates/threshold-signature-server/src/helpers/signing.rs +++ b/crates/threshold-signature-server/src/helpers/signing.rs @@ -82,14 +82,25 @@ pub async fn do_signing( .map_err(|_| ProtocolErr::SessionError("Error getting lock".to_string()))? .insert(session_id.clone(), listener); - open_protocol_connections( + let result = open_protocol_connections( &sign_context.sign_init.validators_info, &session_id, signer, state, &x25519_secret_key, ) - .await?; + .await; + + match result { + Ok(_) => (), + Err(ProtocolErr::ConnectionError { source: _, account_id: _ }) => { + todo!() + }, + // Err(ProtocolErr::EncryptedConnection(_)) => todo!(), + // Err(ProtocolErr::BadSubscribeMessage(_)) => todo!(), + // Err(ProtocolErr::Subscribe(_)) => todo!(), + _ => todo!(), + } let channels = { let ready = timeout(Duration::from_secs(SETUP_TIMEOUT_SECONDS), rx_ready).await?; diff --git a/crates/threshold-signature-server/src/helpers/user.rs b/crates/threshold-signature-server/src/helpers/user.rs index 34aaeb41e..5b6ce48d0 100644 --- a/crates/threshold-signature-server/src/helpers/user.rs +++ b/crates/threshold-signature-server/src/helpers/user.rs @@ -82,6 +82,7 @@ pub async fn do_dkg( x25519_secret_key, ) .await?; + let channels = { let ready = timeout(Duration::from_secs(SETUP_TIMEOUT_SECONDS), rx_ready).await?; let broadcast_out = ready??; diff --git a/crates/threshold-signature-server/src/signing_client/api.rs b/crates/threshold-signature-server/src/signing_client/api.rs index eb55d12a3..d3ef8b2b2 100644 --- a/crates/threshold-signature-server/src/signing_client/api.rs +++ b/crates/threshold-signature-server/src/signing_client/api.rs @@ -139,6 +139,8 @@ async fn handle_socket_result(socket: WebSocket, app_state: AppState) { if let Err(err) = handle_socket(socket, app_state).await { tracing::warn!("Websocket connection closed unexpectedly {:?}", err); // TODO here we should inform the chain that signing failed + // + // TODO (Nando): Report error up here? }; } diff --git a/crates/threshold-signature-server/src/signing_client/errors.rs b/crates/threshold-signature-server/src/signing_client/errors.rs index 4079c0162..a5551b8b5 100644 --- a/crates/threshold-signature-server/src/signing_client/errors.rs +++ b/crates/threshold-signature-server/src/signing_client/errors.rs @@ -22,6 +22,7 @@ use axum::{ }; use entropy_kvdb::kv_manager::error::InnerKvError; use entropy_protocol::errors::ProtocolExecutionErr; +use subxt::utils::AccountId32; use thiserror::Error; use tokio::sync::oneshot::error::RecvError; @@ -46,8 +47,8 @@ pub enum ProtocolErr { Deserialization(String), #[error("Oneshot timeout error: {0}")] OneshotTimeout(#[from] RecvError), - #[error("Subscribe API error: {0}")] - Subscribe(#[from] SubscribeErr), + #[error("Subscribe API error: {source} by TSS Account `{account_id:?}`")] + Subscribe { source: SubscribeErr, account_id: AccountId32 }, #[error("reqwest error: {0}")] Reqwest(#[from] reqwest::Error), #[error("Utf8Error: {0:?}")] @@ -70,18 +71,21 @@ pub enum ProtocolErr { UserError(String), #[error("Validation Error: {0}")] ValidationErr(#[from] crate::validation::errors::ValidationErr), - #[error("Subscribe message rejected: {0}")] - BadSubscribeMessage(String), + #[error("Subscribe message rejected: {message} by TSS Account `{account_id:?}`")] + BadSubscribeMessage { message: String, account_id: AccountId32 }, #[error("From Hex Error: {0}")] FromHex(#[from] hex::FromHexError), #[error("Conversion Error: {0}")] Conversion(&'static str), - #[error("Could not open ws connection: {0}")] - ConnectionError(#[from] tokio_tungstenite::tungstenite::Error), + #[error("Could not open ws connection: {source} with the TSS Account `{account_id:?}`")] + ConnectionError { source: tokio_tungstenite::tungstenite::Error, account_id: AccountId32 }, #[error("Timed out waiting for remote party")] Timeout(#[from] tokio::time::error::Elapsed), - #[error("Encrypted connection error {0}")] - EncryptedConnection(String), + #[error("Encrypted connection error {source:?} with the TSS Account `{account_id:?}`")] + EncryptedConnection { + source: entropy_protocol::protocol_transport::errors::EncryptedConnectionErr, + account_id: AccountId32, + }, #[error("Program error: {0}")] ProgramError(#[from] crate::user::errors::ProgramError), #[error("Invalid length for converting address")] diff --git a/crates/threshold-signature-server/src/signing_client/protocol_transport.rs b/crates/threshold-signature-server/src/signing_client/protocol_transport.rs index 4912e2dce..98ec77fb4 100644 --- a/crates/threshold-signature-server/src/signing_client/protocol_transport.rs +++ b/crates/threshold-signature-server/src/signing_client/protocol_transport.rs @@ -62,7 +62,11 @@ pub async fn open_protocol_connections( .map(|validator_info| async move { // Open a ws connection let ws_endpoint = format!("ws://{}/ws", validator_info.ip_address); - let (ws_stream, _response) = connect_async(ws_endpoint).await?; + let (ws_stream, _response) = + connect_async(ws_endpoint).await.map_err(|e| ProtocolErr::ConnectionError { + source: e, + account_id: validator_info.tss_account.clone(), + })?; // Send a SubscribeMessage in the payload of the final handshake message let subscribe_message_vec = @@ -75,32 +79,50 @@ pub async fn open_protocol_connections( subscribe_message_vec, ) .await - .map_err(|e| ProtocolErr::EncryptedConnection(e.to_string()))?; + .map_err(|e| ProtocolErr::EncryptedConnection { + source: e, + account_id: validator_info.tss_account.clone(), + })?; // Check the response as to whether they accepted our SubscribeMessage - let response_message = encrypted_connection - .recv() - .await - .map_err(|e| ProtocolErr::EncryptedConnection(e.to_string()))?; + let response_message = encrypted_connection.recv().await.map_err(|e| { + ProtocolErr::EncryptedConnection { + source: e, + account_id: validator_info.tss_account.clone(), + } + })?; + let subscribe_response: Result<(), String> = bincode::deserialize(&response_message)?; if let Err(error_message) = subscribe_response { // In future versions, we can check here if the error is VersionTooNew(version) // and if possible the downgrade protocol messages used to be backward compatible - return Err(ProtocolErr::BadSubscribeMessage(error_message)); + return Err(ProtocolErr::BadSubscribeMessage { + message: error_message, + account_id: validator_info.tss_account.clone(), + }); } // Setup channels - let ws_channels = get_ws_channels(state, session_id, &validator_info.tss_account)?; + let ws_channels = get_ws_channels(state, session_id, &validator_info.tss_account) + .map_err(|e| ProtocolErr::Subscribe { + source: e, + account_id: validator_info.tss_account.clone(), + })?; let remote_party_id = PartyId::new(validator_info.tss_account.clone()); + let account_id = validator_info.tss_account.clone(); // Handle protocol messages tokio::spawn(async move { - if let Err(err) = - ws_to_channels(encrypted_connection, ws_channels, remote_party_id).await - { - tracing::warn!("{:?}", err); - }; + ws_to_channels(encrypted_connection, ws_channels, remote_party_id).await.map_err( + |err| { + tracing::warn!("{:?}", err); + Err::<(), ProtocolErr>(ProtocolErr::EncryptedConnection { + source: err.into(), + account_id, + }) + }, + ) }); Ok::<_, ProtocolErr>(()) From 3d88af1e311a5b90bd871388ff2e4e0846cecd77 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Fri, 13 Dec 2024 16:21:14 -0500 Subject: [PATCH 2/3] Add cursed match statement for protocol errors --- .../src/helpers/signing.rs | 25 ++++++++++++++----- 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/crates/threshold-signature-server/src/helpers/signing.rs b/crates/threshold-signature-server/src/helpers/signing.rs index b1e68c4ad..f8e17b5ba 100644 --- a/crates/threshold-signature-server/src/helpers/signing.rs +++ b/crates/threshold-signature-server/src/helpers/signing.rs @@ -93,13 +93,26 @@ pub async fn do_signing( match result { Ok(_) => (), - Err(ProtocolErr::ConnectionError { source: _, account_id: _ }) => { - todo!() + Err(e) + if matches!( + e, + ProtocolErr::ConnectionError { .. } + | ProtocolErr::EncryptedConnection { .. } + | ProtocolErr::BadSubscribeMessage { .. } + | ProtocolErr::Subscribe { .. } + ) => + { + let _account_id = match e { + ProtocolErr::ConnectionError { ref account_id, .. } => account_id, + ProtocolErr::EncryptedConnection { ref account_id, .. } => account_id, + ProtocolErr::BadSubscribeMessage { ref account_id, .. } => account_id, + ProtocolErr::Subscribe { ref account_id, .. } => account_id, + _ => unreachable!(), + }.clone(); + + return Err(e); }, - // Err(ProtocolErr::EncryptedConnection(_)) => todo!(), - // Err(ProtocolErr::BadSubscribeMessage(_)) => todo!(), - // Err(ProtocolErr::Subscribe(_)) => todo!(), - _ => todo!(), + Err(e) => return Err(e), } let channels = { From 69a97ccc1027e7e53e85fb240d678c56f1fd2c16 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Fri, 13 Dec 2024 16:45:38 -0500 Subject: [PATCH 3/3] Move cursed match up to the caller This way we have access to the `api` which we can use to submit an offline report on-chain. --- .../src/helpers/signing.rs | 28 +------------ .../src/user/api.rs | 40 ++++++++++++++++--- 2 files changed, 36 insertions(+), 32 deletions(-) diff --git a/crates/threshold-signature-server/src/helpers/signing.rs b/crates/threshold-signature-server/src/helpers/signing.rs index f8e17b5ba..609dde2f2 100644 --- a/crates/threshold-signature-server/src/helpers/signing.rs +++ b/crates/threshold-signature-server/src/helpers/signing.rs @@ -82,38 +82,14 @@ pub async fn do_signing( .map_err(|_| ProtocolErr::SessionError("Error getting lock".to_string()))? .insert(session_id.clone(), listener); - let result = open_protocol_connections( + open_protocol_connections( &sign_context.sign_init.validators_info, &session_id, signer, state, &x25519_secret_key, ) - .await; - - match result { - Ok(_) => (), - Err(e) - if matches!( - e, - ProtocolErr::ConnectionError { .. } - | ProtocolErr::EncryptedConnection { .. } - | ProtocolErr::BadSubscribeMessage { .. } - | ProtocolErr::Subscribe { .. } - ) => - { - let _account_id = match e { - ProtocolErr::ConnectionError { ref account_id, .. } => account_id, - ProtocolErr::EncryptedConnection { ref account_id, .. } => account_id, - ProtocolErr::BadSubscribeMessage { ref account_id, .. } => account_id, - ProtocolErr::Subscribe { ref account_id, .. } => account_id, - _ => unreachable!(), - }.clone(); - - return Err(e); - }, - Err(e) => return Err(e), - } + .await?; let channels = { let ready = timeout(Duration::from_secs(SETUP_TIMEOUT_SECONDS), rx_ready).await?; diff --git a/crates/threshold-signature-server/src/user/api.rs b/crates/threshold-signature-server/src/user/api.rs index c9397de91..98c0689a6 100644 --- a/crates/threshold-signature-server/src/user/api.rs +++ b/crates/threshold-signature-server/src/user/api.rs @@ -41,6 +41,7 @@ use x25519_dalek::StaticSecret; use super::UserErr; use crate::chain_api::entropy::runtime_types::pallet_registry::pallet::RegisteredInfo; +use crate::signing_client::ProtocolErr; use crate::{ chain_api::{entropy, get_api, get_rpc, EntropyConfig}, helpers::{ @@ -351,14 +352,41 @@ pub async fn sign_tx( request_limit, derivation_path, ) - .await - .map(|signature| { - ( + .await; + + let signing_protocol_output = match signing_protocol_output { + Ok(signature) => Ok(( BASE64_STANDARD.encode(signature.to_rsv_bytes()), signer.signer().sign(&signature.to_rsv_bytes()), - ) - }) - .map_err(|error| error.to_string()); + )), + Err(e) + if matches!( + e, + ProtocolErr::ConnectionError { .. } + | ProtocolErr::EncryptedConnection { .. } + | ProtocolErr::BadSubscribeMessage { .. } + | ProtocolErr::Subscribe { .. } + ) => + { + let account_id = match e { + ProtocolErr::ConnectionError { ref account_id, .. } => account_id, + ProtocolErr::EncryptedConnection { ref account_id, .. } => account_id, + ProtocolErr::BadSubscribeMessage { ref account_id, .. } => account_id, + ProtocolErr::Subscribe { ref account_id, .. } => account_id, + _ => unreachable!(), + } + .clone(); + + let report_unstable_peer_tx = + entropy::tx().staking_extension().report_unstable_peer(account_id); + submit_transaction(&api, &rpc, &signer, &report_unstable_peer_tx, None) + .await + .expect("TODO"); + + Err(e.to_string()) + }, + Err(e) => Err(e.to_string()), + }; // This response chunk is sent later with the result of the signing protocol if response_tx.try_send(serde_json::to_string(&signing_protocol_output)).is_err() {