From 04b26d19a665e64b56a5ced76efbf031fa2f7ca4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mehmet=20Efe=20Ak=C3=A7a?= Date: Thu, 30 Jan 2025 12:38:56 +0300 Subject: [PATCH 1/8] chore: add unwrap_used lint and fix 20 unwraps --- core/Cargo.toml | 7 +++++-- core/build.rs | 15 ++++++++++++--- core/src/errors.rs | 5 ++++- core/src/musig2.rs | 2 +- core/src/rpc/aggregator.rs | 3 ++- core/src/rpc/operator.rs | 4 ++-- core/src/rpc/verifier.rs | 26 +++++++++++++++++--------- core/src/rpc/watchtower.rs | 18 ++++++++++-------- core/src/rpc/wrapper.rs | 36 ++++++++++++++++++++++++++---------- core/src/servers.rs | 8 ++++---- core/src/utils.rs | 6 +++--- core/src/verifier.rs | 22 +++++++++++++--------- 12 files changed, 99 insertions(+), 53 deletions(-) diff --git a/core/Cargo.toml b/core/Cargo.toml index cb41481a..426cb72a 100644 --- a/core/Cargo.toml +++ b/core/Cargo.toml @@ -28,8 +28,8 @@ clap = { workspace = true, features = ["derive"] } toml = { workspace = true } sqlx = { workspace = true, features = ["runtime-tokio", "postgres", "macros"] } header-chain = { workspace = true } -borsh = { workspace = true} -tonic = { workspace = true} +borsh = { workspace = true } +tonic = { workspace = true } prost = { workspace = true } tokio-stream = { workspace = true } async-stream = { workspace = true } @@ -48,3 +48,6 @@ testing = [] [[bin]] name = "server" path = "src/bin/server.rs" + +[lints.clippy] +unwrap_used = { level = "deny", allow_in_tests = true } diff --git a/core/build.rs b/core/build.rs index ddf2eafe..095567d8 100644 --- a/core/build.rs +++ b/core/build.rs @@ -32,7 +32,13 @@ fn compile_protobuf() { let proto_files: Vec = protos .iter() - .map(|proto| proto_root.join(proto).to_str().unwrap().to_owned()) + .map(|proto| { + proto_root + .join(proto) + .to_str() + .expect("proto_root is not a valid path") + .to_owned() + }) .collect(); // Tell Cargo that if a proto file changes, rerun this build script. @@ -45,8 +51,11 @@ fn compile_protobuf() { .build_server(true) .build_client(true) .out_dir("./src/rpc") - .compile_protos(&proto_files, &[proto_root.to_str().unwrap()]) - .unwrap(); + .compile_protos( + &proto_files, + &[proto_root.to_str().expect("proto_root is not a valid path")], + ) + .expect("Failed to compile protos"); } fn main() { diff --git a/core/src/errors.rs b/core/src/errors.rs index 14285a8b..697f1fde 100644 --- a/core/src/errors.rs +++ b/core/src/errors.rs @@ -100,7 +100,7 @@ pub enum BridgeError { #[error("RPC function field {0} is required!")] RPCRequiredParam(&'static str), #[error("RPC function parameter {0} is malformed: {1}")] - RPCParamMalformed(&'static str, String), + RPCParamMalformed(String, String), #[error("RPC stream ended unexpectedly: {0}")] RPCStreamEndedUnexpectedly(String), #[error("Invalid response from an RPC endpoint: {0}")] @@ -127,6 +127,9 @@ pub enum BridgeError { /// There was an error while creating a server. #[error("RPC server can't be created: {0}")] ServerError(std::io::Error), + /// Invalid binding address given in config file + #[error("Invalid server address: {0}")] + InvalidServerAddress(#[from] core::net::AddrParseError), /// When the operators funding utxo is not found #[error("OperatorFundingUtxoNotFound: Funding utxo not found, pls send some amount here: {0}, then call the set_operator_funding_utxo RPC")] OperatorFundingUtxoNotFound(bitcoin::Address), diff --git a/core/src/musig2.rs b/core/src/musig2.rs index 33c4b3a9..073b1373 100644 --- a/core/src/musig2.rs +++ b/core/src/musig2.rs @@ -134,7 +134,7 @@ impl AggregateFromPublicKeys for XOnlyPublicKey { pks: Vec, tweak: Option, ) -> Result { - let musig_key_agg_cache = create_key_agg_cache(pks, tweak).unwrap(); + let musig_key_agg_cache = create_key_agg_cache(pks, tweak)?; Ok(XOnlyPublicKey::from_slice( &musig_key_agg_cache.agg_pk().serialize(), diff --git a/core/src/rpc/aggregator.rs b/core/src/rpc/aggregator.rs index 69e0100b..8cb7fefd 100644 --- a/core/src/rpc/aggregator.rs +++ b/core/src/rpc/aggregator.rs @@ -348,7 +348,8 @@ impl Aggregator { .collect::, _>>() .map_err(|e| { BridgeError::RPCParamMalformed( - "Partial sigs for movetx could not be parsed into MusigPartialSignature", + "Partial sigs for movetx could not be parsed into MusigPartialSignature" + .to_string(), e.to_string(), ) })?; diff --git a/core/src/rpc/operator.rs b/core/src/rpc/operator.rs index 5d812427..84fbb364 100644 --- a/core/src/rpc/operator.rs +++ b/core/src/rpc/operator.rs @@ -1,7 +1,7 @@ use super::clementine::{ self, clementine_operator_server::ClementineOperator, operator_params, ChallengeAckDigest, DepositSignSession, Empty, NewWithdrawalSigParams, NewWithdrawalSigResponse, OperatorBurnSig, - OperatorParams, WinternitzPubkey, WithdrawalFinalizedParams, + OperatorParams, WithdrawalFinalizedParams, }; use crate::builder::sighash::create_operator_sighash_stream; use crate::rpc::parsers; @@ -46,7 +46,7 @@ impl ClementineOperator for Operator { let winternitz_pubkeys = operator.get_winternitz_public_keys().unwrap(); // TODO: Handle unwrap. let winternitz_pubkeys = winternitz_pubkeys .into_iter() - .map(WinternitzPubkey::from_bitvm) + .map(From::from) .collect::>(); for wpk in winternitz_pubkeys { tx.send(Ok(OperatorParams { diff --git a/core/src/rpc/verifier.rs b/core/src/rpc/verifier.rs index 8b45c5dc..9cb8672b 100644 --- a/core/src/rpc/verifier.rs +++ b/core/src/rpc/verifier.rs @@ -121,7 +121,7 @@ impl ClementineVerifier for Verifier { .map(|pk| PublicKey::from_slice(pk).unwrap()) .collect(); - let nofn = NofN::new(self.signer.public_key, verifiers_public_keys.clone()); + let nofn = NofN::new(self.signer.public_key, verifiers_public_keys.clone())?; // Save verifiers public keys to db self.db @@ -193,7 +193,7 @@ impl ClementineVerifier for Verifier { ))?; if let operator_params::Response::WinternitzPubkeys(wpk) = operator_params { - operator_winternitz_public_keys.push(wpk.to_bitvm()); + operator_winternitz_public_keys.push(wpk.try_into()?); } else { return Err(Status::invalid_argument("Expected WinternitzPubkeys")); } @@ -401,7 +401,7 @@ impl ClementineVerifier for Verifier { .ok_or(Status::invalid_argument("No message is received"))?; if let watchtower_params::Response::WinternitzPubkeys(wpk) = wpks { - watchtower_winternitz_public_keys.push(wpk.to_bitvm()); + watchtower_winternitz_public_keys.push(wpk.try_into()?); } else { return Err(Status::invalid_argument("Expected WinternitzPubkeys")); } @@ -438,7 +438,10 @@ impl ClementineVerifier for Verifier { xonly_pk ); let xonly_pk = XOnlyPublicKey::from_slice(&xonly_pk).map_err(|_| { - BridgeError::RPCParamMalformed("watchtower.xonly_pk", "Invalid xonly key".to_string()) + BridgeError::RPCParamMalformed( + "watchtower.xonly_pk".to_string(), + "Invalid xonly key".to_string(), + ) })?; tracing::info!("Verifier receives watchtower index: {:?}", watchtower_id); tracing::info!( @@ -833,15 +836,20 @@ impl ClementineVerifier for Verifier { self.config.bridge_amount_sats, self.config.network, )); - while let Some(result) = in_stream.message().await? { + while let Some(in_msg) = in_stream.message().await? { let sighash = sighash_stream.next().await.unwrap()?; - let operator_sig = result + let operator_sig = in_msg .params .ok_or(Status::internal("No operator sig received"))?; + let final_sig = match operator_sig { - Params::SchnorrSig(final_sig) => { - schnorr::Signature::from_slice(&final_sig).unwrap() - } + Params::SchnorrSig(final_sig) => schnorr::Signature::from_slice(&final_sig) + .map_err(|_| { + BridgeError::RPCParamMalformed( + "Operator sig".to_string(), + "Invalid signature length".to_string(), + ) + })?, _ => { return Err(Status::internal(format!( "Expected Operator Sig, got: {:?}", diff --git a/core/src/rpc/watchtower.rs b/core/src/rpc/watchtower.rs index ea1e0130..274e17bf 100644 --- a/core/src/rpc/watchtower.rs +++ b/core/src/rpc/watchtower.rs @@ -3,7 +3,7 @@ use super::clementine::{ WinternitzPubkey, }; use crate::watchtower::Watchtower; -use tokio::sync::mpsc; +use tokio::sync::mpsc::{self, error::SendError}; use tokio_stream::wrappers::ReceiverStream; use tonic::{async_trait, Request, Response, Status}; @@ -21,7 +21,7 @@ impl ClementineWatchtower for Watchtower { .get_watchtower_winternitz_public_keys() .await? .into_iter() - .map(WinternitzPubkey::from_bitvm) + .map(From::from) .collect::>(); let watchtower = self.clone(); @@ -32,16 +32,15 @@ impl ClementineWatchtower for Watchtower { watchtower.config.index, )), })) - .await - .unwrap(); + .await?; for wpk in winternitz_pubkeys { tx.send(Ok(WatchtowerParams { response: Some(watchtower_params::Response::WinternitzPubkeys(wpk)), })) - .await - .unwrap(); + .await?; } + tracing::info!( "Watchtower gives watchtower xonly public key: {:?}", watchtower.actor.xonly_public_key @@ -51,15 +50,18 @@ impl ClementineWatchtower for Watchtower { watchtower.config.index ); let xonly_pk = watchtower.actor.xonly_public_key.serialize().to_vec(); + tracing::info!( "Watchtower gives watchtower xonly public key bytes: {:?}", xonly_pk ); + tx.send(Ok(WatchtowerParams { response: Some(watchtower_params::Response::XonlyPk(xonly_pk)), })) - .await - .unwrap(); + .await?; + + Ok::<(), SendError<_>>(()) }); let out_stream: Self::GetParamsStream = ReceiverStream::new(rx); diff --git a/core/src/rpc/wrapper.rs b/core/src/rpc/wrapper.rs index 518e223b..17efcc63 100644 --- a/core/src/rpc/wrapper.rs +++ b/core/src/rpc/wrapper.rs @@ -29,20 +29,34 @@ impl From for Outpoint { } } -impl WinternitzPubkey { - pub fn to_bitvm(self) -> winternitz::PublicKey { - let inner = self.digit_pubkey; +impl TryFrom for winternitz::PublicKey { + type Error = BridgeError; + + fn try_from(value: WinternitzPubkey) -> Result { + let inner = value.digit_pubkey; inner .into_iter() - .map(|inner_vec| inner_vec.try_into().unwrap()) - .collect::>() + .enumerate() + .map(|(i, inner_vec)| { + inner_vec.try_into().map_err(|e: Vec| { + BridgeError::RPCParamMalformed( + format!("digit_pubkey.[{}]", i), + format!("Incorrect length {:?}, expected 20", e.len()), + ) + }) + }) + .collect::, BridgeError>>() } +} - pub fn from_bitvm(pk: winternitz::PublicKey) -> Self { - let digit_pubkey = pk.into_iter().map(|inner| inner.to_vec()).collect(); +impl From for WinternitzPubkey { + fn from(value: winternitz::PublicKey) -> Self { + { + let digit_pubkey = value.into_iter().map(|inner| inner.to_vec()).collect(); - WinternitzPubkey { digit_pubkey } + WinternitzPubkey { digit_pubkey } + } } } @@ -50,6 +64,7 @@ impl WinternitzPubkey { mod tests { use crate::rpc::clementine::{Outpoint, WinternitzPubkey}; use bitcoin::{hashes::Hash, OutPoint, Txid}; + use bitvm::signatures::winternitz; #[test] fn from_bitcoin_outpoint_to_proto_outpoint() { @@ -93,8 +108,9 @@ mod tests { fn from_proto_winternitz_public_key_to_bitvm() { let og_wpk = vec![[0x45u8; 20]]; - let rpc_wpk = WinternitzPubkey::from_bitvm(og_wpk.clone()); - let rpc_converted_wpk = rpc_wpk.to_bitvm(); + let rpc_wpk: WinternitzPubkey = og_wpk.clone().into(); + let rpc_converted_wpk: winternitz::PublicKey = + rpc_wpk.try_into().expect("encoded wpk has to be valid"); assert_eq!(og_wpk, rpc_converted_wpk); } } diff --git a/core/src/servers.rs b/core/src/servers.rs index 41ed1aef..cd80f6d5 100644 --- a/core/src/servers.rs +++ b/core/src/servers.rs @@ -27,7 +27,7 @@ pub async fn create_verifier_grpc_server( config: BridgeConfig, rpc: ExtendedRpc, ) -> Result<(std::net::SocketAddr,), BridgeError> { - let addr = format!("{}:{}", config.host, config.port).parse().unwrap(); + let addr = format!("{}:{}", config.host, config.port).parse()?; tracing::info!("Starting verifier gRPC server with address: {}", addr); let verifier = Verifier::new(rpc, config).await?; let svc = ClementineVerifierServer::new(verifier); @@ -66,7 +66,7 @@ pub async fn create_operator_grpc_server( config.host, config.port ); - let addr = format!("{}:{}", config.host, config.port).parse().unwrap(); + let addr = format!("{}:{}", config.host, config.port).parse()?; tracing::info!("Starting operator gRPC server with address: {}", addr); let operator = Operator::new(config, rpc).await?; tracing::info!("Operator gRPC server created"); @@ -96,7 +96,7 @@ pub async fn create_operator_grpc_server( pub async fn create_aggregator_grpc_server( config: BridgeConfig, ) -> Result<(std::net::SocketAddr,), BridgeError> { - let addr = format!("{}:{}", config.host, config.port).parse().unwrap(); + let addr = format!("{}:{}", config.host, config.port).parse()?; let aggregator = Aggregator::new(config).await?; let svc = ClementineAggregatorServer::new(aggregator); @@ -125,7 +125,7 @@ pub async fn create_aggregator_grpc_server( pub async fn create_watchtower_grpc_server( config: BridgeConfig, ) -> Result<(std::net::SocketAddr,), BridgeError> { - let addr = format!("{}:{}", config.host, config.port).parse().unwrap(); + let addr = format!("{}:{}", config.host, config.port).parse()?; let watchtower = Watchtower::new(config).await?; let svc = ClementineWatchtowerServer::new(watchtower); diff --git a/core/src/utils.rs b/core/src/utils.rs index 6906c0a5..4ced5b5f 100644 --- a/core/src/utils.rs +++ b/core/src/utils.rs @@ -25,7 +25,7 @@ lazy_static::lazy_static! { /// /// See https://github.com/bitcoin/bips/blob/master/bip-0341.mediawiki#constructing-and-spending-taproot-outputs pub static ref UNSPENDABLE_PUBKEY: bitcoin::secp256k1::PublicKey = - "93c7378d96518a75448821c4f7c8f4bae7ce60f804d03d1f0628dd5dd0f5de51".parse().unwrap(); + "93c7378d96518a75448821c4f7c8f4bae7ce60f804d03d1f0628dd5dd0f5de51".parse().expect("this key is valid"); } lazy_static::lazy_static! { @@ -33,7 +33,7 @@ lazy_static::lazy_static! { /// /// See https://github.com/bitcoin/bips/blob/master/bip-0341.mediawiki#constructing-and-spending-taproot-outputs pub static ref UNSPENDABLE_XONLY_PUBKEY: bitcoin::secp256k1::XOnlyPublicKey = - XOnlyPublicKey::from_str("93c7378d96518a75448821c4f7c8f4bae7ce60f804d03d1f0628dd5dd0f5de51").unwrap(); + XOnlyPublicKey::from_str("93c7378d96518a75448821c4f7c8f4bae7ce60f804d03d1f0628dd5dd0f5de51").expect("this key is valid"); } lazy_static::lazy_static! { @@ -91,7 +91,7 @@ pub fn get_configuration_for_binaries() -> (BridgeConfig, Args) { let level_filter = match args.verbose { 0 => None, other => Some(LevelFilter::from_level( - Level::from_str(&other.to_string()).unwrap(), + Level::from_str(&other.to_string()).unwrap_or(Level::INFO), )), }; diff --git a/core/src/verifier.rs b/core/src/verifier.rs index 7b67b52d..0e3ddf9e 100644 --- a/core/src/verifier.rs +++ b/core/src/verifier.rs @@ -32,15 +32,20 @@ pub struct NofN { } impl NofN { - pub fn new(self_pk: secp256k1::PublicKey, public_keys: Vec) -> Self { - let idx = public_keys.iter().position(|pk| pk == &self_pk).unwrap(); - let agg_xonly_pk = - secp256k1::XOnlyPublicKey::from_musig2_pks(public_keys.clone(), None).unwrap(); - NofN { + pub fn new( + self_pk: secp256k1::PublicKey, + public_keys: Vec, + ) -> Result { + let idx = public_keys + .iter() + .position(|pk| pk == &self_pk) + .ok_or(BridgeError::PublicKeyNotFound)?; + let agg_xonly_pk = secp256k1::XOnlyPublicKey::from_musig2_pks(public_keys.clone(), None)?; + Ok(NofN { public_keys, agg_xonly_pk, idx, - } + }) } } @@ -77,8 +82,7 @@ impl Verifier { let db = Database::new(&config).await?; let nofn_xonly_pk = - secp256k1::XOnlyPublicKey::from_musig2_pks(config.verifiers_public_keys.clone(), None) - .unwrap(); + secp256k1::XOnlyPublicKey::from_musig2_pks(config.verifiers_public_keys.clone(), None)?; let operator_xonly_pks = config.operators_xonly_pks.clone(); @@ -91,7 +95,7 @@ impl Verifier { let nofn = if !verifiers_pks.is_empty() { tracing::debug!("Verifiers public keys found: {:?}", verifiers_pks); - let nofn = NofN::new(signer.public_key, verifiers_pks); + let nofn = NofN::new(signer.public_key, verifiers_pks)?; Some(nofn) } else { None From 8abe0fa424dc9774d18910af1277e63f18f93d7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mehmet=20Efe=20Ak=C3=A7a?= Date: Thu, 30 Jan 2025 14:37:00 +0300 Subject: [PATCH 2/8] WIP --- clippy.toml | 2 + core/Cargo.toml | 2 +- core/clippy.toml | 2 + core/src/aggregator.rs | 4 +- core/src/builder/address.rs | 28 ++- core/src/builder/script.rs | 4 +- core/src/builder/sighash.rs | 4 +- core/src/builder/transaction/mod.rs | 9 +- .../builder/transaction/operator_assert.rs | 6 +- core/src/config.rs | 60 ++--- core/src/database/common.rs | 9 +- core/src/database/wrapper.rs | 19 +- core/src/extended_rpc.rs | 2 +- core/src/header_chain_prover/blockgazer.rs | 7 +- core/src/header_chain_prover/prover.rs | 22 +- core/src/musig2.rs | 15 +- core/src/rpc/aggregator.rs | 2 +- core/src/rpc/verifier.rs | 213 +++++++++++++----- core/src/verifier.rs | 2 +- 19 files changed, 262 insertions(+), 150 deletions(-) create mode 100644 clippy.toml create mode 100644 core/clippy.toml diff --git a/clippy.toml b/clippy.toml new file mode 100644 index 00000000..0358cdb5 --- /dev/null +++ b/clippy.toml @@ -0,0 +1,2 @@ +allow-unwrap-in-tests = true +allow-expect-in-tests = true diff --git a/core/Cargo.toml b/core/Cargo.toml index 426cb72a..a3d64412 100644 --- a/core/Cargo.toml +++ b/core/Cargo.toml @@ -50,4 +50,4 @@ name = "server" path = "src/bin/server.rs" [lints.clippy] -unwrap_used = { level = "deny", allow_in_tests = true } +unwrap_used = { level = "deny", allow-unwrap-in-tests = true } diff --git a/core/clippy.toml b/core/clippy.toml new file mode 100644 index 00000000..0358cdb5 --- /dev/null +++ b/core/clippy.toml @@ -0,0 +1,2 @@ +allow-unwrap-in-tests = true +allow-expect-in-tests = true diff --git a/core/src/aggregator.rs b/core/src/aggregator.rs index 3750e8e5..456f2fd0 100644 --- a/core/src/aggregator.rs +++ b/core/src/aggregator.rs @@ -228,7 +228,7 @@ impl Aggregator { self.config.user_takes_after, self.config.bridge_amount_sats, self.config.network, - ); + )?; // println!("MOVE_TX: {:?}", tx); // println!("MOVE_TXID: {:?}", tx.tx.compute_txid()); let message = Message::from_digest( @@ -354,7 +354,7 @@ impl Aggregator { self.config.user_takes_after, self.config.bridge_amount_sats, self.config.network, - ); + )?; let move_tx_witness_elements = vec![move_tx_sig.serialize().to_vec()]; set_p2tr_script_spend_witness(&mut move_tx_handler, &move_tx_witness_elements, 0, 0)?; diff --git a/core/src/builder/address.rs b/core/src/builder/address.rs index 8caa1fc3..3197433e 100644 --- a/core/src/builder/address.rs +++ b/core/src/builder/address.rs @@ -4,6 +4,7 @@ //! addresses. use crate::builder; +use crate::errors::BridgeError; use crate::utils::SECP; use crate::{utils, EVMAddress}; use bitcoin::address::NetworkUnchecked; @@ -23,7 +24,11 @@ pub fn taproot_builder_with_scripts(scripts: &[ScriptBuf]) -> TaprootBuilder { // Special return cases for n = 0 or n = 1 match num_scripts { 0 => return builder, - 1 => return builder.add_leaf(0, scripts[0].clone()).unwrap(), + 1 => { + return builder + .add_leaf(0, scripts[0].clone()) + .expect("one root leaf added on empty builder") + } _ => {} } @@ -39,7 +44,7 @@ pub fn taproot_builder_with_scripts(scripts: &[ScriptBuf]) -> TaprootBuilder { deepest_layer_depth - is_node_in_last_minus_one_depth, scripts[i].clone(), ) - .unwrap() + .expect("algorithm tested to be correct") }) } @@ -71,10 +76,12 @@ pub fn create_taproot_address( let taproot_builder = taproot_builder_with_scripts(scripts); // Finalize the tree let tree_info = match internal_key { - Some(xonly_pk) => taproot_builder.finalize(&SECP, xonly_pk).unwrap(), + Some(xonly_pk) => taproot_builder + .finalize(&SECP, xonly_pk) + .expect("builder return is finalizable"), None => taproot_builder .finalize(&SECP, *utils::UNSPENDABLE_XONLY_PUBKEY) - .unwrap(), + .expect("builder return is finalizable"), }; // Create the address let taproot_address = match internal_key { @@ -119,7 +126,7 @@ pub fn generate_deposit_address( amount: Amount, network: bitcoin::Network, user_takes_after: u16, -) -> (Address, TaprootSpendInfo) { +) -> Result<(Address, TaprootSpendInfo), BridgeError> { let deposit_script = builder::script::create_deposit_script(nofn_xonly_pk, user_evm_address, amount); @@ -128,14 +135,18 @@ pub fn generate_deposit_address( .assume_checked() .script_pubkey(); let recovery_extracted_xonly_pk = - XOnlyPublicKey::from_slice(&recovery_script_pubkey.as_bytes()[2..34]).unwrap(); + XOnlyPublicKey::from_slice(&recovery_script_pubkey.as_bytes()[2..34])?; let script_timelock = builder::script::generate_checksig_relative_timelock_script( recovery_extracted_xonly_pk, user_takes_after, ); - create_taproot_address(&[deposit_script, script_timelock], None, network) + Ok(create_taproot_address( + &[deposit_script, script_timelock], + None, + network, + )) } /// Shorthand function for creating a checksig taproot address: A single checksig script with the given xonly PK and no internal key. @@ -287,7 +298,8 @@ mod tests { Amount::from_sat(100_000_000), bitcoin::Network::Regtest, 200, - ); + ) + .unwrap(); // Comparing it to the taproot address generated in bridge backend. assert_eq!( diff --git a/core/src/builder/script.rs b/core/src/builder/script.rs index a63c8545..b4ee8ebc 100644 --- a/core/src/builder/script.rs +++ b/core/src/builder/script.rs @@ -30,7 +30,7 @@ pub fn anyone_can_spend_txout() -> TxOut { pub fn anchor_output() -> TxOut { TxOut { value: ANCHOR_AMOUNT, - script_pubkey: ScriptBuf::from_hex("51024e73").unwrap(), + script_pubkey: ScriptBuf::from_hex("51024e73").expect("anchor script is valid"), } } @@ -53,7 +53,7 @@ pub fn create_deposit_script( evm_address: EVMAddress, amount: Amount, ) -> ScriptBuf { - let citrea: [u8; 6] = "citrea".as_bytes().try_into().unwrap(); + let citrea = b"citrea"; Builder::new() .push_x_only_key(&nofn_xonly_pk) diff --git a/core/src/builder/sighash.rs b/core/src/builder/sighash.rs index ed79975b..8f4a854d 100644 --- a/core/src/builder/sighash.rs +++ b/core/src/builder/sighash.rs @@ -66,7 +66,7 @@ pub fn create_nofn_sighash_stream( _user_takes_after, bridge_amount_sats, network, - ); + )?; // Get operator details (for each operator, (X-Only Public Key, Address, Collateral Funding Txid)) let operators: Vec<(XOnlyPublicKey, bitcoin::Address, Txid)> = db.get_operators(None).await?; @@ -349,7 +349,7 @@ pub fn create_operator_sighash_stream( _user_takes_after, bridge_amount_sats, network, - ); + )?; let mut input_txid = collateral_funding_txid; let mut input_amount = collateral_funding_amount; diff --git a/core/src/builder/transaction/mod.rs b/core/src/builder/transaction/mod.rs index 2486afd1..767004e0 100644 --- a/core/src/builder/transaction/mod.rs +++ b/core/src/builder/transaction/mod.rs @@ -4,6 +4,7 @@ //! transactions. use crate::builder; +use crate::errors::BridgeError; use crate::EVMAddress; use bitcoin::address::NetworkUnchecked; use bitcoin::Sequence; @@ -119,7 +120,7 @@ pub fn create_move_to_vault_txhandler( user_takes_after: u16, bridge_amount_sats: Amount, network: bitcoin::Network, -) -> TxHandler { +) -> Result { let (musig2_address, musig2_spendinfo) = builder::address::create_checksig_address(nofn_xonly_pk, network); @@ -140,7 +141,7 @@ pub fn create_move_to_vault_txhandler( bridge_amount_sats, network, user_takes_after, - ); + )?; let prevouts = vec![TxOut { script_pubkey: deposit_address.script_pubkey(), @@ -153,7 +154,7 @@ pub fn create_move_to_vault_txhandler( bridge_amount_sats, )]; - TxHandler { + Ok(TxHandler { txid: move_tx.compute_txid(), tx: move_tx, prevouts, @@ -161,7 +162,7 @@ pub fn create_move_to_vault_txhandler( prev_taproot_spend_infos: vec![Some(deposit_taproot_spend_info)], out_scripts: vec![vec![], vec![]], out_taproot_spend_infos: vec![Some(musig2_spendinfo), None], - } + }) } #[cfg(test)] diff --git a/core/src/builder/transaction/operator_assert.rs b/core/src/builder/transaction/operator_assert.rs index 15c97675..e86b0179 100644 --- a/core/src/builder/transaction/operator_assert.rs +++ b/core/src/builder/transaction/operator_assert.rs @@ -145,10 +145,10 @@ pub fn create_assert_end_txhandler( }); let disprove_taproot_spend_info = TaprootBuilder::new() - .add_hidden_node(0, TapNodeHash::from_slice(root_hash).unwrap()) - .unwrap() + .add_hidden_node(0, TapNodeHash::from_byte_array(*root_hash)) + .expect("empty taptree will accept a node at depth 0") .finalize(&SECP, nofn_xonly_pk) // TODO: we should convert this to script spend but we only have partial access to the taptree - .unwrap(); + .expect("finalize always succeeds for taptree with single node at depth 0"); let disprove_address = Address::p2tr( &SECP, diff --git a/core/src/config.rs b/core/src/config.rs index 3f92ca90..0d057440 100644 --- a/core/src/config.rs +++ b/core/src/config.rs @@ -143,38 +143,38 @@ impl Default for BridgeConfig { secret_key: SecretKey::from_str( "3333333333333333333333333333333333333333333333333333333333333333", ) - .unwrap(), + .expect("known valid input"), num_verifiers: 7, verifiers_public_keys: vec![ PublicKey::from_str( "034f355bdcb7cc0af728ef3cceb9615d90684bb5b2ca5f859ab0f0b704075871aa", ) - .unwrap(), + .expect("known valid input"), PublicKey::from_str( "02466d7fcae563e5cb09a0d1870bb580344804617879a14949cf22285f1bae3f27", ) - .unwrap(), + .expect("known valid input"), PublicKey::from_str( "023c72addb4fdf09af94f0c94d7fe92a386a7e70cf8a1d85916386bb2535c7b1b1", ) - .unwrap(), + .expect("known valid input"), PublicKey::from_str( "032c0b7cf95324a07d05398b240174dc0c2be444d96b159aa6c7f7b1e668680991", ) - .unwrap(), + .expect("known valid input"), PublicKey::from_str( "029ac20335eb38768d2052be1dbbc3c8f6178407458e51e6b4ad22f1d91758895b", ) - .unwrap(), + .expect("known valid input"), PublicKey::from_str( "035ab4689e400a4a160cf01cd44730845a54768df8547dcdf073d964f109f18c30", ) - .unwrap(), + .expect("known valid input"), PublicKey::from_str( "037962d45b38e8bcf82fa8efa8432a01f20c9a53e24c7d3f11df197cb8e70926da", ) - .unwrap(), + .expect("known valid input"), ], num_operators: 3, @@ -185,15 +185,15 @@ impl Default for BridgeConfig { XOnlyPublicKey::from_str( "4f355bdcb7cc0af728ef3cceb9615d90684bb5b2ca5f859ab0f0b704075871aa", ) - .unwrap(), + .expect("known valid input"), XOnlyPublicKey::from_str( "466d7fcae563e5cb09a0d1870bb580344804617879a14949cf22285f1bae3f27", ) - .unwrap(), + .expect("known valid input"), XOnlyPublicKey::from_str( "3c72addb4fdf09af94f0c94d7fe92a386a7e70cf8a1d85916386bb2535c7b1b1", ) - .unwrap(), + .expect("known valid input"), ], operator_takes_after: 5, @@ -202,15 +202,15 @@ impl Default for BridgeConfig { Address::from_str( "bcrt1pvaua4gvvglk27al5trh337xz8l8zzhgzageky0xt0dgv64xee8tqwwvzmf", ) - .unwrap(), + .expect("known valid input"), Address::from_str( "bcrt1pvaua4gvvglk27al5trh337xz8l8zzhgzageky0xt0dgv64xee8tqwwvzmf", ) - .unwrap(), + .expect("known valid input"), Address::from_str( "bcrt1pvaua4gvvglk27al5trh337xz8l8zzhgzageky0xt0dgv64xee8tqwwvzmf", ) - .unwrap(), + .expect("known valid input"), ], operator_withdrawal_fee_sats: Some(Amount::from_sat(100000)), @@ -237,77 +237,77 @@ impl Default for BridgeConfig { bridge_contract_address: "3100000000000000000000000000000000000002".to_string(), header_chain_proof_path: Some( - PathBuf::from_str("../core/tests/data/first_1.bin").unwrap(), + PathBuf::from_str("../core/tests/data/first_1.bin").expect("known valid input"), ), all_verifiers_secret_keys: Some(vec![ SecretKey::from_str( "1111111111111111111111111111111111111111111111111111111111111111", ) - .unwrap(), + .expect("known valid input"), SecretKey::from_str( "2222222222222222222222222222222222222222222222222222222222222222", ) - .unwrap(), + .expect("known valid input"), SecretKey::from_str( "3333333333333333333333333333333333333333333333333333333333333333", ) - .unwrap(), + .expect("known valid input"), SecretKey::from_str( "4444444444444444444444444444444444444444444444444444444444444444", ) - .unwrap(), + .expect("known valid input"), SecretKey::from_str( "5555555555555555555555555555555555555555555555555555555555555555", ) - .unwrap(), + .expect("known valid input"), SecretKey::from_str( "6666666666666666666666666666666666666666666666666666666666666666", ) - .unwrap(), + .expect("known valid input"), SecretKey::from_str( "7777777777777777777777777777777777777777777777777777777777777777", ) - .unwrap(), + .expect("known valid input"), ]), all_operators_secret_keys: Some(vec![ SecretKey::from_str( "1111111111111111111111111111111111111111111111111111111111111111", ) - .unwrap(), + .expect("known valid input"), SecretKey::from_str( "2222222222222222222222222222222222222222222222222222222222222222", ) - .unwrap(), + .expect("known valid input"), SecretKey::from_str( "3333333333333333333333333333333333333333333333333333333333333333", ) - .unwrap(), + .expect("known valid input"), ]), all_watchtowers_secret_keys: Some(vec![ SecretKey::from_str( "1111111111111111111111111111111111111111111111111111111111111111", ) - .unwrap(), + .expect("known valid input"), SecretKey::from_str( "2222222222222222222222222222222222222222222222222222222222222222", ) - .unwrap(), + .expect("known valid input"), SecretKey::from_str( "3333333333333333333333333333333333333333333333333333333333333333", ) - .unwrap(), + .expect("known valid input"), SecretKey::from_str( "4444444444444444444444444444444444444444444444444444444444444444", ) - .unwrap(), + .expect("known valid input"), ]), winternitz_secret_key: Some( SecretKey::from_str( "2222222222222222222222222222222222222222222222222222222222222222", ) - .unwrap(), + .expect("known valid input"), ), verifier_endpoints: None, diff --git a/core/src/database/common.rs b/core/src/database/common.rs index 7aeade0f..e31bb91d 100644 --- a/core/src/database/common.rs +++ b/core/src/database/common.rs @@ -303,8 +303,9 @@ impl Database { match result { Ok((txid, raw_signed_tx, cur_unused_kickoff_index)) => { // Deserialize the transaction - let tx: bitcoin::Transaction = - bitcoin::consensus::deserialize(&hex::decode(raw_signed_tx).unwrap())?; + let tx: bitcoin::Transaction = bitcoin::consensus::deserialize( + &hex::decode(raw_signed_tx).map_err(|e| BridgeError::Error(e.to_string()))?, + )?; // Create the outpoint and txout let outpoint = OutPoint { @@ -1068,8 +1069,8 @@ impl Database { operator_id: u32, ) -> Result, BridgeError> { let query = sqlx::query_as::<_, (Vec>,)>( - "SELECT challenge_addresses - FROM watchtower_challenge_addresses + "SELECT challenge_addresses + FROM watchtower_challenge_addresses WHERE watchtower_id = $1 AND operator_id = $2;", ) .bind(watchtower_id as i64) diff --git a/core/src/database/wrapper.rs b/core/src/database/wrapper.rs index 99547c7f..895ed3ab 100644 --- a/core/src/database/wrapper.rs +++ b/core/src/database/wrapper.rs @@ -204,7 +204,7 @@ impl_bytea_wrapper_custom!( MessageDB, Message, |msg: &Message| *msg, // Message is Copy, which requires this hack - |x: &[u8]| -> Result { Ok(Message::from_digest(x.try_into().unwrap())) } + |x: &[u8]| -> Result { Ok(Message::from_digest(x.try_into()?)) } ); impl_bytea_wrapper_custom!( @@ -217,13 +217,15 @@ impl_bytea_wrapper_custom!( .map(|signature| signature.serialize().to_vec()) .collect::>(), ) - .unwrap() + .expect("signatures array too big (len() > 2^32) or ran out of memory") }, |x: &[u8]| -> Result, BoxDynError> { - Ok(borsh::from_slice::>>(x)? + borsh::from_slice::>>(x)? .iter() - .map(|signature| schnorr::Signature::from_slice(signature).unwrap()) - .collect()) + .map(|signature| -> Result { + schnorr::Signature::from_slice(signature).map_err(Into::into) + }) + .collect::, BoxDynError>>() } ); @@ -232,13 +234,14 @@ impl_text_wrapper_custom!( block::Header, |header: &block::Header| { let mut bytes = Vec::new(); - header.consensus_encode(&mut bytes).unwrap(); + header + .consensus_encode(&mut bytes) + .expect("exceeded max Vec size or ran out of memory"); bytes.to_hex_string(bitcoin::hex::Case::Lower) }, |s: &str| -> Result { let bytes = hex::decode(s)?; - block::Header::consensus_decode(&mut bytes.as_slice()) - .map_err(|e| Box::new(e) as sqlx::error::BoxDynError) + block::Header::consensus_decode(&mut bytes.as_slice()).map_err(Into::into) } ); diff --git a/core/src/extended_rpc.rs b/core/src/extended_rpc.rs index 4a74e146..d6c7c141 100644 --- a/core/src/extended_rpc.rs +++ b/core/src/extended_rpc.rs @@ -151,7 +151,7 @@ impl ExtendedRpc { amount_sats, network, user_takes_after, - ); + )?; if !self .check_utxo_address_and_amount( diff --git a/core/src/header_chain_prover/blockgazer.rs b/core/src/header_chain_prover/blockgazer.rs index 5823ced9..7494f518 100644 --- a/core/src/header_chain_prover/blockgazer.rs +++ b/core/src/header_chain_prover/blockgazer.rs @@ -187,10 +187,9 @@ impl HeaderChainProver { match status { BlockFetchStatus::UpToDate => (), BlockFetchStatus::FallenBehind(block_height, block_hashes) => { - prover - .sync_blockchain(block_height, block_hashes) - .await - .unwrap(); + if let Err(e) = prover.sync_blockchain(block_height, block_hashes).await { + tracing::error!("Failed to sync blockchain: {e}"); + } } } }; diff --git a/core/src/header_chain_prover/prover.rs b/core/src/header_chain_prover/prover.rs index 3cfdf482..7e55bb32 100644 --- a/core/src/header_chain_prover/prover.rs +++ b/core/src/header_chain_prover/prover.rs @@ -19,25 +19,25 @@ const SIGNET_ELF: &[u8; 199828] = include_bytes!("../../../scripts/signet-header const REGTEST_ELF: &[u8; 194128] = include_bytes!("../../../scripts/regtest-header-chain-guest"); lazy_static! { static ref MAINNET_IMAGE_ID: [u32; 8] = compute_image_id(MAINNET_ELF) - .unwrap() + .expect("hardcoded ELF is valid") .as_words() .try_into() - .unwrap(); + .expect("hardcoded ELF is valid"); static ref TESTNET4_IMAGE_ID: [u32; 8] = compute_image_id(TESTNET4_ELF) - .unwrap() + .expect("hardcoded ELF is valid") .as_words() .try_into() - .unwrap(); + .expect("hardcoded ELF is valid"); static ref SIGNET_IMAGE_ID: [u32; 8] = compute_image_id(SIGNET_ELF) - .unwrap() + .expect("hardcoded ELF is valid") .as_words() .try_into() - .unwrap(); + .expect("hardcoded ELF is valid"); static ref REGTEST_IMAGE_ID: [u32; 8] = compute_image_id(REGTEST_ELF) - .unwrap() + .expect("hardcoded ELF is valid") .as_words() .try_into() - .unwrap(); + .expect("hardcoded ELF is valid"); } impl HeaderChainProver { @@ -147,11 +147,13 @@ impl HeaderChainProver { match receipt { Ok(receipt) => { - prover + if let Err(e) = prover .db .save_block_proof(None, current_block_hash, receipt) .await - .unwrap(); + { + tracing::error!("Can't save proof for header {:?}: {}", header, e); + } } Err(e) => { tracing::error!("Can't prove for header {:?}: {}", header, e) diff --git a/core/src/musig2.rs b/core/src/musig2.rs index 073b1373..f0ee6698 100644 --- a/core/src/musig2.rs +++ b/core/src/musig2.rs @@ -23,29 +23,30 @@ use sha2::{Digest, Sha256}; pub type MuSigNoncePair = (MusigSecNonce, MusigPubNonce); pub fn from_secp_xonly(xpk: secp256k1::XOnlyPublicKey) -> XOnlyPublicKey { - XOnlyPublicKey::from_slice(&xpk.serialize()).unwrap() + XOnlyPublicKey::from_slice(&xpk.serialize()).expect("serialized pubkey is valid") } pub fn to_secp_pk(pk: PublicKey) -> secp256k1::PublicKey { - secp256k1::PublicKey::from_slice(&pk.serialize()).unwrap() + secp256k1::PublicKey::from_slice(&pk.serialize()).expect("serialized pubkey is valid") } pub fn from_secp_pk(pk: secp256k1::PublicKey) -> PublicKey { - PublicKey::from_slice(&pk.serialize()).unwrap() + PublicKey::from_slice(&pk.serialize()).expect("serialized pubkey is valid") } pub fn to_secp_sk(sk: SecretKey) -> secp256k1::SecretKey { - secp256k1::SecretKey::from_slice(&sk.secret_bytes()).unwrap() + secp256k1::SecretKey::from_slice(&sk.secret_bytes()).expect("serialized secret key is valid") } pub fn to_secp_kp(kp: &Keypair) -> secp256k1::Keypair { - secp256k1::Keypair::from_seckey_slice(SECP256K1, &kp.secret_bytes()).unwrap() + secp256k1::Keypair::from_seckey_slice(SECP256K1, &kp.secret_bytes()) + .expect("serialized secret key is valid") } pub fn from_secp_kp(kp: &secp256k1::Keypair) -> Keypair { - Keypair::from_seckey_slice(&SECP, &kp.secret_bytes()).unwrap() + Keypair::from_seckey_slice(&SECP, &kp.secret_bytes()).expect("serialized secret key is valid") } pub fn from_secp_sig(sig: secp256k1::schnorr::Signature) -> schnorr::Signature { - schnorr::Signature::from_slice(&sig.to_byte_array()).unwrap() + schnorr::Signature::from_slice(&sig.to_byte_array()).expect("serialized signature is valid") } pub fn to_secp_msg(msg: &Message) -> secp256k1::Message { diff --git a/core/src/rpc/aggregator.rs b/core/src/rpc/aggregator.rs index 8cb7fefd..cf27942a 100644 --- a/core/src/rpc/aggregator.rs +++ b/core/src/rpc/aggregator.rs @@ -363,7 +363,7 @@ impl Aggregator { user_takes_after, self.config.bridge_amount_sats, self.config.network, - ); + )?; let sighash = move_txhandler.calculate_script_spend_sighash(0, 0, None)?; // aggregate partial signatures diff --git a/core/src/rpc/verifier.rs b/core/src/rpc/verifier.rs index 9cb8672b..3e4c56c2 100644 --- a/core/src/rpc/verifier.rs +++ b/core/src/rpc/verifier.rs @@ -33,12 +33,43 @@ use bitvm::signatures::{ use crate::utils::SECP; use futures::StreamExt; use secp256k1::musig::{MusigAggNonce, MusigPubNonce, MusigSecNonce}; -use std::collections::BTreeMap; +use std::{collections::BTreeMap, fmt::Display}; use std::{pin::pin, str::FromStr}; -use tokio::sync::mpsc; +use tokio::sync::mpsc::{self, error::SendError}; use tokio_stream::wrappers::ReceiverStream; use tonic::{async_trait, Request, Response, Status, Streaming}; +fn expected_msg_got_error(msg: Status) -> Status { + Status::invalid_argument(format!("Expected message, got error: {msg}")) +} + +fn expected_msg_got_none<'a>(msg: &'a str) -> impl (Fn() -> Status) + 'a { + move || Status::invalid_argument(format!("Expected {msg} but received None")) +} + +fn stream_ended_prematurely(name: &str) -> Status { + Status::internal(format!("{name} stream ended prematurely")) +} + +fn input_ended_prematurely() -> Status { + Status::invalid_argument("Input stream ended prematurely") +} + +fn sighash_stream_ended_prematurely() -> Status { + Status::internal("Sighash stream ended prematurely") +} + +fn sighash_stream_failed(msg: Status) -> Status { + Status::internal(format!("Sighash stream failed: {msg}")) +} + +fn invalid_argument<'a, T: std::error::Error + Send + Sync + 'static + Display>( + field: &'a str, + msg: &'a str, +) -> impl 'a + Fn(T) -> Status { + move |e| Status::invalid_argument(format!("Failed to parse {field}: {msg}\n{e}")) +} + fn get_deposit_params( deposit_sign_session: clementine::DepositSignSession, verifier_idx: usize, @@ -59,7 +90,12 @@ fn get_deposit_params( .deposit_outpoint .ok_or(Status::invalid_argument("No deposit outpoint received"))? .try_into()?; - let evm_address: EVMAddress = deposit_params.evm_address.try_into().unwrap(); + let evm_address: EVMAddress = deposit_params.evm_address.try_into().map_err(|e| { + Status::invalid_argument(format!( + "Failed to convert evm_address to EVMAddress: {}", + e + )) + })?; let recovery_taproot_address = deposit_params .recovery_taproot_address .parse::>() @@ -114,12 +150,19 @@ impl ClementineVerifier for Verifier { } // Extract the public keys from the request - let verifiers_public_keys: Vec = req + let verifiers_public_keys = req .into_inner() .verifier_public_keys .iter() - .map(|pk| PublicKey::from_slice(pk).unwrap()) - .collect(); + .map(|pk| { + PublicKey::from_slice(pk).map_err(|e| { + BridgeError::RPCParamMalformed( + "verifier_public_keys".to_string(), + e.to_string(), + ) + }) + }) + .collect::, BridgeError>>()?; let nofn = NofN::new(self.signer.public_key, verifiers_public_keys.clone())?; @@ -144,34 +187,42 @@ impl ClementineVerifier for Verifier { let operator_params = in_stream .message() - .await? - .ok_or(Status::invalid_argument("No message is received"))? + .await + .map_err(expected_msg_got_error)? + .ok_or_else(input_ended_prematurely)? .response - .ok_or(Status::invalid_argument("No message is received"))?; + .ok_or_else(expected_msg_got_none("Response"))?; - let operator_config = + let operator_details = if let operator_params::Response::OperatorDetails(operator_config) = operator_params { operator_config } else { - return Err(Status::invalid_argument("Expected OperatorDetails")); + return Err(expected_msg_got_none("OperatorDetails")()); }; - let operator_xonly_pk = XOnlyPublicKey::from_str(&operator_config.xonly_pk) - .map_err(|_| BridgeError::Error("Invalid xonly public key".to_string()))?; + let operator_xonly_pk = + XOnlyPublicKey::from_str(&operator_details.xonly_pk).map_err(|_| { + Status::invalid_argument("Invalid operator xonly public key".to_string()) + })?; // Save the operator details to the db self.db .set_operator( None, - operator_config.operator_idx as i32, + operator_details.operator_idx as i32, operator_xonly_pk, - operator_config.wallet_reimburse_address, + operator_details.wallet_reimburse_address, Txid::from_byte_array( - operator_config + operator_details .collateral_funding_txid .clone() .try_into() - .unwrap(), + .map_err(|e| { + Status::invalid_argument(format!( + "Failed to convert collateral funding txid to Txid: {:?}", + e + )) + })?, ), ) .await?; @@ -195,7 +246,7 @@ impl ClementineVerifier for Verifier { if let operator_params::Response::WinternitzPubkeys(wpk) = operator_params { operator_winternitz_public_keys.push(wpk.try_into()?); } else { - return Err(Status::invalid_argument("Expected WinternitzPubkeys")); + return Err(expected_msg_got_none("WinternitzPubkeys")()); } } let operator_winternitz_public_keys = operator_winternitz_public_keys @@ -206,7 +257,7 @@ impl ClementineVerifier for Verifier { self.db .save_operator_winternitz_public_keys( None, - operator_config.operator_idx, + operator_details.operator_idx, operator_winternitz_public_keys.clone(), ) .await?; @@ -251,7 +302,7 @@ impl ClementineVerifier for Verifier { self.db .save_public_hashes( None, - operator_config.operator_idx as i32, + operator_details.operator_idx as i32, i as i32, j as i32, &operators_challenge_ack_public_hashes[self.config.num_watchtowers @@ -344,14 +395,17 @@ impl ClementineVerifier for Verifier { }; let taproot_builder = taproot_builder_with_scripts(&scripts); - let root_hash = taproot_builder.try_into_taptree().unwrap().root_hash(); + let root_hash = taproot_builder + .try_into_taptree() + .expect("taproot builder always builds a full taptree") + .root_hash(); let root_hash_bytes = root_hash.to_raw_hash().to_byte_array(); // Save the public input wots to db along with the root hash self.db .save_bitvm_setup( None, - operator_config.operator_idx as i32, + operator_details.operator_idx as i32, sequential_collateral_tx_idx as i32, kickoff_idx as i32, assert_tx_addrs, @@ -514,11 +568,12 @@ impl ClementineVerifier for Verifier { let (sec_nonce, pub_nonce) = musig2::nonce_pair( &self.signer.keypair, &mut bitcoin::secp256k1::rand::thread_rng(), - ) - .unwrap(); - (sec_nonce, pub_nonce) + )?; + Ok((sec_nonce, pub_nonce)) }) - .unzip(); + .collect::, BridgeError>>()? + .into_iter() + .unzip(); // TODO: fix extra copies let session = NonceSession { nonces: sec_nonces }; @@ -545,7 +600,7 @@ impl ClementineVerifier for Verifier { nonce_gen_first_response, )), }; - tx.send(Ok(response)).await.unwrap(); + tx.send(Ok(response)).await?; // Then send the public nonces for pub_nonce in &pub_nonces[..] { @@ -554,8 +609,10 @@ impl ClementineVerifier for Verifier { pub_nonce.serialize().to_vec(), )), }; - tx.send(Ok(response)).await.unwrap(); + tx.send(Ok(response)).await?; } + + Ok::<(), SendError<_>>(()) }); Ok(Response::new(ReceiverStream::new(rx))) } @@ -568,23 +625,22 @@ impl ClementineVerifier for Verifier { let (tx, rx) = mpsc::channel(1280); + let error_tx = tx.clone(); + tracing::info!("Received deposit sign request"); let verifier = self.clone(); - tokio::spawn(async move { + let handle = tokio::spawn(async move { let first_message = in_stream .message() - .await - .unwrap() - .ok_or(Status::internal("No first message received")) - .unwrap(); + .await? + .ok_or(Status::internal("No first message received"))?; // Parse the first message let params = first_message .params - .ok_or(Status::internal("No deposit outpoint received")) - .unwrap(); + .ok_or(Status::internal("No deposit outpoint received"))?; let ( deposit_outpoint, @@ -595,16 +651,14 @@ impl ClementineVerifier for Verifier { ) = match params { clementine::verifier_deposit_sign_params::Params::DepositSignFirstParam( deposit_sign_session, - ) => get_deposit_params(deposit_sign_session, verifier.idx).unwrap(), - _ => panic!("Expected DepositOutpoint"), + ) => get_deposit_params(deposit_sign_session, verifier.idx)?, + _ => return Err(Status::invalid_argument("Expected DepositOutpoint")), }; let mut session_map = verifier.nonces.lock().await; - let session = session_map - .sessions - .get_mut(&session_id) - .ok_or_else(|| Status::internal(format!("Could not find session id {session_id}"))) - .unwrap(); + let session = session_map.sessions.get_mut(&session_id).ok_or_else(|| { + Status::internal(format!("Could not find session id {session_id}")) + })?; session.nonces.reverse(); let mut nonce_idx: usize = 0; @@ -630,19 +684,23 @@ impl ClementineVerifier for Verifier { "Expected nonce count to be num_required_sigs + 1 (movetx)" ); - while let Some(result) = in_stream.message().await.unwrap() { + while let Some(result) = in_stream.message().await? { let agg_nonce = match result .params - .ok_or(Status::internal("No agg nonce received")) - .unwrap() + .ok_or(Status::internal("No agg nonce received"))? { clementine::verifier_deposit_sign_params::Params::AggNonce(agg_nonce) => { - MusigAggNonce::from_slice(agg_nonce.as_slice()).unwrap() + MusigAggNonce::from_slice(agg_nonce.as_slice()).map_err(|e| { + BridgeError::RPCParamMalformed("AggNonce".to_string(), e.to_string()) + })? } - _ => panic!("Expected AggNonce"), + _ => return Err(Status::invalid_argument("Expected AggNonce")), }; - let sighash = sighash_stream.next().await.unwrap().unwrap(); + let sighash = sighash_stream + .next() + .await + .ok_or(Status::internal("No sighash received"))??; tracing::debug!("Verifier {} found sighash: {:?}", verifier.idx, sighash); let nonce = session.nonces.pop().expect("No nonce available"); @@ -653,14 +711,17 @@ impl ClementineVerifier for Verifier { agg_nonce, verifier.signer.keypair, Message::from_digest(*sighash.as_byte_array()), - ) - .unwrap(); + )?; tx.send(Ok(PartialSig { partial_sig: partial_sig.serialize().to_vec(), })) .await - .unwrap(); + .map_err(|e| { + Status::aborted(format!( + "Error sending partial sig, stream ended prematurely: {e}" + )) + })?; nonce_idx += 1; if nonce_idx == num_required_sigs { @@ -669,9 +730,24 @@ impl ClementineVerifier for Verifier { } // Drop all the nonces except the last one, to avoid reusing the nonces. - let last_nonce = session.nonces.pop().unwrap(); + let last_nonce = session + .nonces + .pop() + .ok_or(Status::internal("No last nonce available"))?; session.nonces.clear(); session.nonces.push(last_nonce); + + Ok::<(), Status>(()) + }); + + // Background task to handle the error case where the background task fails, notifies caller + tokio::spawn(async move { + if let Ok(Err(bg_err)) = handle.await { + let ret_res = error_tx.send(Err(bg_err)).await; + if let Err(SendError(Err(e))) = ret_res { + tracing::error!("deposit_sign background task failed and the return stream ended prematurely:\n\n Background task error: {e}"); + } + } }); let out_stream: Self::DepositSignStream = ReceiverStream::new(rx); @@ -727,15 +803,21 @@ impl ClementineVerifier for Verifier { let mut nonce_idx: usize = 0; - while let Some(result) = in_stream.message().await.unwrap() { - let sighash = sighash_stream.next().await.unwrap().unwrap(); + while let Some(result) = in_stream.message().await.map_err(expected_msg_got_error)? { + let sighash = sighash_stream + .next() + .await + .ok_or_else(sighash_stream_ended_prematurely)? + .map_err(Into::into) + .map_err(sighash_stream_failed)?; + let final_sig = result .params - .ok_or(Status::internal("No final sig received"))?; + .ok_or_else(expected_msg_got_none("FinalSig"))?; + let final_sig = match final_sig { - Params::SchnorrSig(final_sig) => { - schnorr::Signature::from_slice(&final_sig).unwrap() - } + Params::SchnorrSig(final_sig) => schnorr::Signature::from_slice(&final_sig) + .map_err(invalid_argument("FinalSig", "Invalid signature length"))?, _ => return Err(Status::internal("Expected FinalSig")), }; @@ -775,14 +857,21 @@ impl ClementineVerifier for Verifier { user_takes_after, self.config.bridge_amount_sats, self.config.network, - ); + )?; let move_tx_sighash = move_txhandler.calculate_script_spend_sighash(0, 0, None)?; - let agg_nonce = match in_stream.message().await.unwrap().unwrap().params.unwrap() { + let agg_nonce = match in_stream + .message() + .await + .map_err(expected_msg_got_error)? + .ok_or_else(expected_msg_got_none("Params.MusigAggNonce"))? + .params + .ok_or_else(expected_msg_got_none("Params.MusigAggNonce"))? + { Params::MoveTxAggNonce(aggnonce) => MusigAggNonce::from_slice(&aggnonce) - .map_err(|e| Status::internal(format!("Invalid aggregate nonce: {}", e)))?, - _ => Err(Status::internal("Expected MoveTxAggNonce"))?, + .map_err(invalid_argument("MusigAggNonce", "failed to parse"))?, + _ => Err(expected_msg_got_none("MusigAggNonce")())?, }; let movetx_secnonce = { diff --git a/core/src/verifier.rs b/core/src/verifier.rs index 0e3ddf9e..401a31af 100644 --- a/core/src/verifier.rs +++ b/core/src/verifier.rs @@ -349,7 +349,7 @@ impl Verifier { self.config.user_takes_after, self.config.bridge_amount_sats, self.config.network, - ); + )?; let bridge_fund_outpoint = OutPoint { txid: move_tx_handler.tx.compute_txid(), From 9999f0495488bc1b7dea89d4c15b6c6492978583 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mehmet=20Efe=20Ak=C3=A7a?= Date: Thu, 30 Jan 2025 17:14:25 +0300 Subject: [PATCH 3/8] Remove all unwraps --- core/src/errors.rs | 3 +++ core/src/operator.rs | 32 +++++++++++++++++------- core/src/rpc/aggregator.rs | 39 ++++++++++++++++++++--------- core/src/rpc/error.rs | 34 +++++++++++++++++++++++++ core/src/rpc/mod.rs | 1 + core/src/rpc/operator.rs | 17 +++++++------ core/src/rpc/parsers/mod.rs | 4 ++- core/src/rpc/verifier.rs | 50 +++++++++++-------------------------- 8 files changed, 114 insertions(+), 66 deletions(-) create mode 100644 core/src/rpc/error.rs diff --git a/core/src/errors.rs b/core/src/errors.rs index 697f1fde..ccbf85ed 100644 --- a/core/src/errors.rs +++ b/core/src/errors.rs @@ -230,6 +230,9 @@ pub enum BridgeError { #[error("WatchtowerChallengeAddressesNotFound for watchtower {0}, operator {1}")] WatchtowerChallengeAddressesNotFound(u32, u32), + + #[error("Invalid response from Citrea: {0}")] + InvalidCitreaResponse(String), } impl From for ErrorObject<'static> { diff --git a/core/src/operator.rs b/core/src/operator.rs index ad4cd256..5f85f3d1 100644 --- a/core/src/operator.rs +++ b/core/src/operator.rs @@ -91,11 +91,7 @@ impl Operator { tx.commit().await?; let citrea_client = if !config.citrea_rpc_url.is_empty() { - Some( - HttpClientBuilder::default() - .build(config.citrea_rpc_url.clone()) - .unwrap(), - ) + Some(HttpClientBuilder::default().build(config.citrea_rpc_url.clone())?) } else { None }; @@ -518,8 +514,17 @@ impl Operator { ]; let response: String = citrea_client.request("eth_call", params).await?; - let operator_idx_as_vec = hex::decode(&response[58..66]).unwrap(); - let operator_idx = u32::from_be_bytes(operator_idx_as_vec.try_into().unwrap()); + let operator_idx_as_vec = hex::decode(&response[58..66]).map_err(|_| { + BridgeError::InvalidCitreaResponse(format!( + "Failed to decode operator_idx hex from response: OperatorIdx = {}", + &response[58..66] + )) + })?; + let operator_idx = u32::from_be_bytes( + operator_idx_as_vec + .try_into() + .expect("length statically known"), + ); if operator_idx - 1 != self.idx as u32 { return Err(BridgeError::InvalidOperatorIndex( @@ -548,8 +553,17 @@ impl Operator { let response: String = citrea_client.request("eth_call", params).await?; let deposit_idx_response = &response[58..66]; - let deposit_idx_as_vec = hex::decode(deposit_idx_response).unwrap(); - let deposit_idx = u32::from_be_bytes(deposit_idx_as_vec.try_into().unwrap()); + let deposit_idx_as_vec = hex::decode(deposit_idx_response).map_err(|_| { + BridgeError::InvalidCitreaResponse(format!( + "Invalid deposit idx response from Citrea, deposit idx = {}", + &response[58..66] + )) + })?; + let deposit_idx = u32::from_be_bytes( + deposit_idx_as_vec + .try_into() + .expect("length statically known"), + ); if deposit_idx - 1 != withdrawal_idx { return Err(BridgeError::InvalidDepositOutpointGiven( diff --git a/core/src/rpc/aggregator.rs b/core/src/rpc/aggregator.rs index cf27942a..fac2796d 100644 --- a/core/src/rpc/aggregator.rs +++ b/core/src/rpc/aggregator.rs @@ -6,6 +6,7 @@ use crate::builder::transaction::create_move_to_vault_txhandler; use crate::config::BridgeConfig; use crate::rpc::clementine::clementine_operator_client::ClementineOperatorClient; use crate::rpc::clementine::clementine_verifier_client::ClementineVerifierClient; +use crate::rpc::error::output_stream_ended_prematurely; use crate::rpc::parsers; use crate::{ aggregator::Aggregator, @@ -182,9 +183,9 @@ async fn signature_distributor( }; for tx in &deposit_finalize_sender { - tx.send(final_params.clone()).await.map_err(|e| { - BridgeError::RPCStreamEndedUnexpectedly(format!("Can't send final params: {}", e)) - })?; + tx.send(final_params.clone()) + .await + .map_err(|_| output_stream_ended_prematurely())?; } } @@ -196,7 +197,7 @@ async fn signature_distributor( )), }) .await - .unwrap(); + .map_err(|_| output_stream_ended_prematurely())? } Ok(()) @@ -447,7 +448,9 @@ impl ClementineAggregator for Aggregator { client.set_operator(tokio_stream::wrappers::ReceiverStream::new(rx)); for param in params { - tx.send(param).await.unwrap(); + tx.send(param) + .await + .map_err(|_| output_stream_ended_prematurely())?; } future.await?; // TODO: This is dangerous: If channel size becomes not sufficient, this will block forever. @@ -488,7 +491,9 @@ impl ClementineAggregator for Aggregator { let future = client.set_watchtower(tokio_stream::wrappers::ReceiverStream::new(rx)); for param in params { - tx.send(param).await.unwrap(); + tx.send(param) + .await + .map_err(|_| output_stream_ended_prematurely())?; } future.await?; // TODO: This is dangerous: If channel size becomes not sufficient, this will block forever. @@ -664,7 +669,9 @@ impl ClementineAggregator for Aggregator { )); // Join the nonce aggregation handle to get the movetx agg nonce. - let movetx_agg_nonce = nonce_agg_handle.await.unwrap()?; + let movetx_agg_nonce = nonce_agg_handle + .await + .expect("cancelled task or panic in task")?; // Start the deposit finalization pipe. let sig_dist_handle = tokio::spawn(signature_distributor( @@ -685,10 +692,18 @@ impl ClementineAggregator for Aggregator { "Waiting for pipeline tasks to complete (nonce agg, sig agg, sig dist, operator sigs)" ); // Wait for all pipeline tasks to complete - nonce_dist_handle.await.unwrap()?; - sig_agg_handle.await.unwrap()?; - sig_dist_handle.await.unwrap()?; - let operator_sigs = operator_sigs_fut.await.unwrap()?; + nonce_dist_handle + .await + .expect("cancelled task or panic in task")?; + sig_agg_handle + .await + .expect("cancelled task or panic in task")?; + sig_dist_handle + .await + .expect("cancelled task or panic in task")?; + let operator_sigs = operator_sigs_fut + .await + .expect("cancelled task or panic in task")?; // send operators sigs to verifiers after all verifiers have signed let send_operator_sigs: Vec<_> = deposit_finalize_sender @@ -723,7 +738,7 @@ impl ClementineAggregator for Aggregator { let move_tx_partial_sigs = try_join_all( deposit_finalize_futures .iter_mut() - .map(|f| async { Ok::<_, Status>(f.await.unwrap().into_inner().partial_sig) }), + .map(|fut| async { Ok::<_, Status>(fut.await?.into_inner().partial_sig) }), ) .await .map_err(|e| Status::internal(format!("Failed to finalize deposit: {:?}", e)))?; diff --git a/core/src/rpc/error.rs b/core/src/rpc/error.rs new file mode 100644 index 00000000..5b865f2f --- /dev/null +++ b/core/src/rpc/error.rs @@ -0,0 +1,34 @@ +use std::fmt::Display; + +use tonic::Status; + +pub(crate) fn expected_msg_got_error(msg: Status) -> Status { + Status::invalid_argument(format!("Expected message, got error: {msg}")) +} + +pub(crate) fn expected_msg_got_none(msg: &str) -> impl (Fn() -> Status) + '_ { + move || Status::invalid_argument(format!("Expected {msg} but received None")) +} + +pub(crate) fn input_ended_prematurely() -> Status { + Status::invalid_argument("Input stream ended prematurely") +} + +pub(crate) fn sighash_stream_ended_prematurely() -> Status { + Status::internal("Sighash stream ended prematurely") +} + +pub(crate) fn output_stream_ended_prematurely() -> Status { + Status::internal("Output stream ended prematurely".to_string()) +} + +pub(crate) fn sighash_stream_failed(msg: Status) -> Status { + Status::internal(format!("Sighash stream failed: {msg}")) +} + +pub(crate) fn invalid_argument<'a, T: std::error::Error + Send + Sync + 'static + Display>( + field: &'a str, + msg: &'a str, +) -> impl 'a + Fn(T) -> Status { + move |e| Status::invalid_argument(format!("Failed to parse {field}: {msg}\n{e}")) +} diff --git a/core/src/rpc/mod.rs b/core/src/rpc/mod.rs index 82a88573..bb8f93c9 100644 --- a/core/src/rpc/mod.rs +++ b/core/src/rpc/mod.rs @@ -7,6 +7,7 @@ use tonic::transport::Uri; pub mod clementine; pub mod aggregator; +mod error; pub mod operator; mod parsers; pub mod verifier; diff --git a/core/src/rpc/operator.rs b/core/src/rpc/operator.rs index 84fbb364..d31f4eca 100644 --- a/core/src/rpc/operator.rs +++ b/core/src/rpc/operator.rs @@ -3,6 +3,7 @@ use super::clementine::{ DepositSignSession, Empty, NewWithdrawalSigParams, NewWithdrawalSigResponse, OperatorBurnSig, OperatorParams, WithdrawalFinalizedParams, }; +use super::error::*; use crate::builder::sighash::create_operator_sighash_stream; use crate::rpc::parsers; use crate::{errors::BridgeError, operator::Operator}; @@ -41,9 +42,9 @@ impl ClementineOperator for Operator { response: Some(operator_params::Response::OperatorDetails(operator_config)), })) .await - .unwrap(); + .map_err(|_| output_stream_ended_prematurely())?; - let winternitz_pubkeys = operator.get_winternitz_public_keys().unwrap(); // TODO: Handle unwrap. + let winternitz_pubkeys = operator.get_winternitz_public_keys()?; let winternitz_pubkeys = winternitz_pubkeys .into_iter() .map(From::from) @@ -53,12 +54,10 @@ impl ClementineOperator for Operator { response: Some(operator_params::Response::WinternitzPubkeys(wpk)), })) .await - .unwrap(); + .map_err(|_| output_stream_ended_prematurely())?; } - let public_hashes = operator - .generate_challenge_ack_preimages_and_hashes() - .unwrap(); // TODO: Handle unwrap. + let public_hashes = operator.generate_challenge_ack_preimages_and_hashes()?; let public_hashes = public_hashes .into_iter() .map(|hash| ChallengeAckDigest { @@ -71,8 +70,10 @@ impl ClementineOperator for Operator { response: Some(operator_params::Response::ChallengeAckDigests(hash)), })) .await - .unwrap(); + .map_err(|_| output_stream_ended_prematurely())?; } + + Ok::<(), Status>(()) }); let out_stream: Self::GetParamsStream = ReceiverStream::new(rx); @@ -89,7 +90,7 @@ impl ClementineOperator for Operator { let (deposit_outpoint, evm_address, recovery_taproot_address, user_takes_after) = match deposit_sign_session.deposit_params { Some(deposit_params) => parsers::parse_deposit_params(deposit_params)?, - _ => panic!("Expected Deposit Params"), + _ => return Err(expected_msg_got_none("Deposit Params")()), }; let (tx, rx) = mpsc::channel(1280); let operator = self.clone(); diff --git a/core/src/rpc/parsers/mod.rs b/core/src/rpc/parsers/mod.rs index 10b5bea6..080bdad0 100644 --- a/core/src/rpc/parsers/mod.rs +++ b/core/src/rpc/parsers/mod.rs @@ -18,7 +18,9 @@ pub fn parse_deposit_params( .deposit_outpoint .ok_or(Status::invalid_argument("No deposit outpoint received"))? .try_into()?; - let evm_address: EVMAddress = deposit_params.evm_address.try_into().unwrap(); + let evm_address: EVMAddress = deposit_params.evm_address.try_into().map_err(|_| { + Status::invalid_argument("Could not parse deposit outpoint EVM address") + })?; let recovery_taproot_address = deposit_params .recovery_taproot_address .parse::>() diff --git a/core/src/rpc/verifier.rs b/core/src/rpc/verifier.rs index 3e4c56c2..a2a9bb43 100644 --- a/core/src/rpc/verifier.rs +++ b/core/src/rpc/verifier.rs @@ -30,46 +30,16 @@ use bitvm::signatures::{ winternitz, }; +use super::error::*; use crate::utils::SECP; use futures::StreamExt; use secp256k1::musig::{MusigAggNonce, MusigPubNonce, MusigSecNonce}; -use std::{collections::BTreeMap, fmt::Display}; +use std::collections::BTreeMap; use std::{pin::pin, str::FromStr}; use tokio::sync::mpsc::{self, error::SendError}; use tokio_stream::wrappers::ReceiverStream; use tonic::{async_trait, Request, Response, Status, Streaming}; -fn expected_msg_got_error(msg: Status) -> Status { - Status::invalid_argument(format!("Expected message, got error: {msg}")) -} - -fn expected_msg_got_none<'a>(msg: &'a str) -> impl (Fn() -> Status) + 'a { - move || Status::invalid_argument(format!("Expected {msg} but received None")) -} - -fn stream_ended_prematurely(name: &str) -> Status { - Status::internal(format!("{name} stream ended prematurely")) -} - -fn input_ended_prematurely() -> Status { - Status::invalid_argument("Input stream ended prematurely") -} - -fn sighash_stream_ended_prematurely() -> Status { - Status::internal("Sighash stream ended prematurely") -} - -fn sighash_stream_failed(msg: Status) -> Status { - Status::internal(format!("Sighash stream failed: {msg}")) -} - -fn invalid_argument<'a, T: std::error::Error + Send + Sync + 'static + Display>( - field: &'a str, - msg: &'a str, -) -> impl 'a + Fn(T) -> Status { - move |e| Status::invalid_argument(format!("Failed to parse {field}: {msg}\n{e}")) -} - fn get_deposit_params( deposit_sign_session: clementine::DepositSignSession, verifier_idx: usize, @@ -876,7 +846,12 @@ impl ClementineVerifier for Verifier { let movetx_secnonce = { let mut session_map = self.nonces.lock().await; - let session = session_map.sessions.get_mut(&session_id).unwrap(); + let session = session_map.sessions.get_mut(&session_id).ok_or_else(|| { + Status::internal(format!( + "could not find session with id {} in session cache", + session_id + )) + })?; session .nonces .pop() @@ -926,10 +901,13 @@ impl ClementineVerifier for Verifier { self.config.network, )); while let Some(in_msg) = in_stream.message().await? { - let sighash = sighash_stream.next().await.unwrap()?; + let sighash = sighash_stream + .next() + .await + .ok_or_else(sighash_stream_ended_prematurely)??; let operator_sig = in_msg .params - .ok_or(Status::internal("No operator sig received"))?; + .ok_or_else(expected_msg_got_none("Operator Signature"))?; let final_sig = match operator_sig { Params::SchnorrSig(final_sig) => schnorr::Signature::from_slice(&final_sig) @@ -951,7 +929,7 @@ impl ClementineVerifier for Verifier { .verify_schnorr(&final_sig, &Message::from(sighash), &tweaked_op_xonly_pk) .map_err(|x| { Status::internal(format!( - "Operator {} Signature {} Verification Failed: {}.", + "Operator {} Signature {}: verification failed: {}.", operator_idx, op_sig_count + 1, x From 9f484c61ae4dc292f3eafefc7da59f359ebc2419 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mehmet=20Efe=20Ak=C3=A7a?= Date: Thu, 30 Jan 2025 17:19:27 +0300 Subject: [PATCH 4/8] Enable test cfg option for test code --- core/tests/musig2.rs | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/core/tests/musig2.rs b/core/tests/musig2.rs index 45f29214..460925ed 100644 --- a/core/tests/musig2.rs +++ b/core/tests/musig2.rs @@ -5,6 +5,7 @@ use bitcoin::XOnlyPublicKey; use bitcoin::{hashes::Hash, script, Amount, ScriptBuf}; use bitcoincore_rpc::RpcApi; use clementine_core::builder::transaction::TxHandler; +use clementine_core::errors::BridgeError; use clementine_core::musig2::{ aggregate_nonces, aggregate_partial_signatures, AggregateFromPublicKeys, Musig2Mode, }; @@ -22,6 +23,7 @@ use std::{env, thread}; mod common; +#[cfg(test)] fn get_verifiers_keys(config: &BridgeConfig) -> (Vec, XOnlyPublicKey, Vec) { let verifiers_secret_keys = config.all_verifiers_secret_keys.clone().unwrap(); @@ -45,11 +47,14 @@ fn get_verifiers_keys(config: &BridgeConfig) -> (Vec, XOnlyPublicKey, V ) } -fn get_nonces(verifiers_secret_public_keys: Vec) -> (Vec, MusigAggNonce) { +#[cfg(test)] +fn get_nonces( + verifiers_secret_public_keys: Vec, +) -> Result<(Vec, MusigAggNonce), BridgeError> { let nonce_pairs: Vec = verifiers_secret_public_keys .iter() - .map(|kp| nonce_pair(kp, &mut secp256k1::rand::thread_rng()).unwrap()) - .collect(); + .map(|kp| nonce_pair(kp, &mut secp256k1::rand::thread_rng())) + .collect::, _>>()?; let agg_nonce = aggregate_nonces( nonce_pairs @@ -59,7 +64,7 @@ fn get_nonces(verifiers_secret_public_keys: Vec) -> (Vec = vec![dummy_script]; @@ -270,7 +275,7 @@ async fn script_spend() { let (verifiers_secret_public_keys, _untweaked_xonly_pubkey, verifier_public_keys) = get_verifiers_keys(&config); - let (nonce_pairs, agg_nonce) = get_nonces(verifiers_secret_public_keys.clone()); + let (nonce_pairs, agg_nonce) = get_nonces(verifiers_secret_public_keys.clone()).unwrap(); let agg_pk = XOnlyPublicKey::from_musig2_pks(verifier_public_keys.clone(), None).unwrap(); @@ -381,8 +386,8 @@ async fn key_and_script_spend() { let (verifiers_secret_public_keys, _untweaked_xonly_pubkey, verifier_public_keys) = get_verifiers_keys(&config); // Generate NofN nonces (need two for key and script spend) - let (nonce_pairs, agg_nonce) = get_nonces(verifiers_secret_public_keys.clone()); - let (nonce_pairs_2, agg_nonce_2) = get_nonces(verifiers_secret_public_keys.clone()); + let (nonce_pairs, agg_nonce) = get_nonces(verifiers_secret_public_keys.clone()).unwrap(); + let (nonce_pairs_2, agg_nonce_2) = get_nonces(verifiers_secret_public_keys.clone()).unwrap(); // Aggregate Pks let agg_pk = XOnlyPublicKey::from_musig2_pks(verifier_public_keys.clone(), None).unwrap(); From 16e37b9420facf41e0168ca8b9b7e6038019d29d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mehmet=20Efe=20Ak=C3=A7a?= Date: Thu, 30 Jan 2025 17:19:49 +0300 Subject: [PATCH 5/8] cargo fmt --- core/src/rpc/parsers/mod.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/core/src/rpc/parsers/mod.rs b/core/src/rpc/parsers/mod.rs index 080bdad0..3a227181 100644 --- a/core/src/rpc/parsers/mod.rs +++ b/core/src/rpc/parsers/mod.rs @@ -18,9 +18,10 @@ pub fn parse_deposit_params( .deposit_outpoint .ok_or(Status::invalid_argument("No deposit outpoint received"))? .try_into()?; - let evm_address: EVMAddress = deposit_params.evm_address.try_into().map_err(|_| { - Status::invalid_argument("Could not parse deposit outpoint EVM address") - })?; + let evm_address: EVMAddress = deposit_params + .evm_address + .try_into() + .map_err(|_| Status::invalid_argument("Could not parse deposit outpoint EVM address"))?; let recovery_taproot_address = deposit_params .recovery_taproot_address .parse::>() From 114f237308d29612347b53e20a0c39e1c6e61964 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mehmet=20Efe=20Ak=C3=A7a?= Date: Fri, 31 Jan 2025 10:47:56 +0300 Subject: [PATCH 6/8] remove panicking Clone impl for ExtendedRpc --- core/Cargo.toml | 2 +- core/src/bin/server.rs | 8 +-- core/src/builder/sighash.rs | 7 +-- core/src/database/operator.rs | 5 +- core/src/errors.rs | 3 ++ core/src/extended_rpc.rs | 39 +++++++------- core/src/header_chain_prover/blockgazer.rs | 63 ++++++++++++++-------- core/src/header_chain_prover/mod.rs | 23 +++++--- core/src/header_chain_prover/prover.rs | 27 ++++++---- core/src/operator.rs | 14 ++--- core/src/rpc/aggregator.rs | 9 ++-- core/src/servers.rs | 2 - core/src/test_utils.rs | 5 +- core/src/verifier.rs | 8 +-- core/src/watchtower.rs | 5 +- core/tests/common/mod.rs | 8 +-- core/tests/deposit.rs | 4 +- core/tests/musig2.rs | 20 ++++--- core/tests/rpc.rs | 8 +-- core/tests/taproot.rs | 5 +- 20 files changed, 156 insertions(+), 109 deletions(-) diff --git a/core/Cargo.toml b/core/Cargo.toml index a3d64412..9e4b1b46 100644 --- a/core/Cargo.toml +++ b/core/Cargo.toml @@ -50,4 +50,4 @@ name = "server" path = "src/bin/server.rs" [lints.clippy] -unwrap_used = { level = "deny", allow-unwrap-in-tests = true } +unwrap_used = { level = "deny" } diff --git a/core/src/bin/server.rs b/core/src/bin/server.rs index 73746730..c2c12f69 100644 --- a/core/src/bin/server.rs +++ b/core/src/bin/server.rs @@ -7,7 +7,7 @@ #[tokio::main] async fn main() { - panic!("grpc switch in progress. please inform us if you get this error.") + println!("grpc switch in progress. please inform us if you get this error.") // let (mut config, args) = get_configuration_for_binaries(); // if !args.verifier_server && !args.operator_server && !args.aggregator_server { @@ -15,7 +15,7 @@ async fn main() { // exit(1); // } - // let rpc = ExtendedRpc::new( + // let rpc = ExtendedRpc::connect( // config.bitcoin_rpc_url.clone(), // config.bitcoin_rpc_user.clone(), // config.bitcoin_rpc_password.clone(), @@ -28,7 +28,7 @@ async fn main() { // if args.verifier_server { // handles.push( - // create_verifier_server(config.clone(), rpc.clone()) + // create_verifier_server(config.clone(), rpc.clone_inner().await.unwrap()) // .await // .unwrap() // .1 @@ -41,7 +41,7 @@ async fn main() { // if args.operator_server { // handles.push( - // create_operator_server(config.clone(), rpc.clone()) + // create_operator_server(config.clone(), rpc.clone_inner().await.unwrap()) // .await // .unwrap() // .1 diff --git a/core/src/builder/sighash.rs b/core/src/builder/sighash.rs index f5d936d4..8a912327 100644 --- a/core/src/builder/sighash.rs +++ b/core/src/builder/sighash.rs @@ -71,7 +71,7 @@ pub fn create_nofn_sighash_stream( let operators: Vec<(XOnlyPublicKey, bitcoin::Address, Txid)> = db.get_operators(None).await?; if operators.len() < config.num_operators { - panic!("Not enough operators"); + Err(BridgeError::NotEnoughOperators)?; } for (operator_idx, (operator_xonly_pk, operator_reimburse_address, collateral_funding_txid)) in @@ -476,12 +476,13 @@ mod tests { async fn calculate_num_required_nofn_sigs() { let config = create_test_config_with_thread_name!(None); let db = Database::new(&config).await.unwrap(); - let rpc = ExtendedRpc::new( + let rpc = ExtendedRpc::connect( config.bitcoin_rpc_url.clone(), config.bitcoin_rpc_user.clone(), config.bitcoin_rpc_password.clone(), ) - .await; + .await + .unwrap(); let operator = Operator::new(config.clone(), rpc).await.unwrap(); let watchtower = Watchtower::new(config.clone()).await.unwrap(); diff --git a/core/src/database/operator.rs b/core/src/database/operator.rs index 78fd6d78..dc73f5db 100644 --- a/core/src/database/operator.rs +++ b/core/src/database/operator.rs @@ -235,6 +235,7 @@ impl Database { match result { Ok((txid, raw_signed_tx, cur_unused_kickoff_index)) => { // Deserialize the transaction + // let tx: bitcoin::Transaction = bitcoin::consensus::deserialize( &hex::decode(raw_signed_tx).map_err(|e| BridgeError::Error(e.to_string()))?, )?; @@ -1122,12 +1123,12 @@ mod tests { async fn set_get_operator_winternitz_public_keys() { let config = create_test_config_with_thread_name!(None); let database = Database::new(&config).await.unwrap(); - let rpc = ExtendedRpc::new( + let rpc = ExtendedRpc::connect( config.bitcoin_rpc_url.clone(), config.bitcoin_rpc_user.clone(), config.bitcoin_rpc_password.clone(), ) - .await; + .await.unwrap(); let operator = Operator::new(config, rpc).await.unwrap(); let operator_idx = 0x45; diff --git a/core/src/errors.rs b/core/src/errors.rs index cd12f31b..91c87a50 100644 --- a/core/src/errors.rs +++ b/core/src/errors.rs @@ -233,6 +233,9 @@ pub enum BridgeError { #[error("Invalid response from Citrea: {0}")] InvalidCitreaResponse(String), + + #[error("Not enough operators")] + NotEnoughOperators, } impl From for ErrorObject<'static> { diff --git a/core/src/extended_rpc.rs b/core/src/extended_rpc.rs index d6c7c141..e9c16bf1 100644 --- a/core/src/extended_rpc.rs +++ b/core/src/extended_rpc.rs @@ -2,6 +2,8 @@ //! //! This module provides helpful functions for Bitcoin RPC. +use std::sync::Arc; + use crate::builder; use crate::errors::BridgeError; use crate::EVMAddress; @@ -17,31 +19,29 @@ use bitcoincore_rpc::Auth; use bitcoincore_rpc::Client; use bitcoincore_rpc::RpcApi; -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct ExtendedRpc { url: String, auth: Auth, - pub client: Client, + pub client: Arc, } impl ExtendedRpc { /// Connects to Bitcoin RPC and returns a new `ExtendedRpc`. - /// - /// # Panics - /// - /// Panics if it cannot connect to Bitcoin RPC. - pub async fn new(url: String, user: String, password: String) -> Self { + pub async fn connect( + url: String, + user: String, + password: String, + ) -> Result { let auth = Auth::UserPass(user, password); - let rpc = Client::new(&url, auth.clone()) - .await - .unwrap_or_else(|e| panic!("Failed to connect to Bitcoin RPC: {}", e)); + let rpc = Client::new(&url, auth.clone()).await?; - Self { + Ok(Self { url, auth, - client: rpc, - } + client: Arc::new(rpc), + }) } #[tracing::instrument(skip(self), err(level = tracing::Level::ERROR), ret(level = tracing::Level::TRACE))] @@ -170,17 +170,14 @@ impl ExtendedRpc { Ok(()) } -} -impl Clone for ExtendedRpc { - fn clone(&self) -> Self { - let new_client = futures::executor::block_on(Client::new(&self.url, self.auth.clone())) - .unwrap_or_else(|e| panic!("Failed to clone Bitcoin RPC client: {}", e)); + pub async fn clone_inner(&self) -> Result { + let new_client = Client::new(&self.url, self.auth.clone()).await?; - Self { + Ok(Self { url: self.url.clone(), auth: self.auth.clone(), - client: new_client, - } + client: Arc::new(new_client), + }) } } diff --git a/core/src/header_chain_prover/blockgazer.rs b/core/src/header_chain_prover/blockgazer.rs index 601577c8..eab29f6d 100644 --- a/core/src/header_chain_prover/blockgazer.rs +++ b/core/src/header_chain_prover/blockgazer.rs @@ -257,13 +257,16 @@ mod tests { #[serial_test::parallel] async fn check_for_new_blocks_uptodate() { let config = create_test_config_with_thread_name!(None); - let rpc = ExtendedRpc::new( + let rpc = ExtendedRpc::connect( config.bitcoin_rpc_url.clone(), config.bitcoin_rpc_user.clone(), config.bitcoin_rpc_password.clone(), ) - .await; - let prover = HeaderChainProver::new(&config, rpc.clone()).await.unwrap(); + .await + .unwrap(); + let prover = HeaderChainProver::new(&config, rpc.clone_inner().await.unwrap()) + .await + .unwrap(); // Save current blockchain tip. let current_tip_height = rpc.client.get_block_count().await.unwrap(); @@ -291,13 +294,16 @@ mod tests { #[serial_test::serial] async fn check_for_new_blocks_fallen_behind_single() { let config = create_test_config_with_thread_name!(None); - let rpc = ExtendedRpc::new( + let rpc = ExtendedRpc::connect( config.bitcoin_rpc_url.clone(), config.bitcoin_rpc_user.clone(), config.bitcoin_rpc_password.clone(), ) - .await; - let prover = HeaderChainProver::new(&config, rpc.clone()).await.unwrap(); + .await + .unwrap(); + let prover = HeaderChainProver::new(&config, rpc.clone_inner().await.unwrap()) + .await + .unwrap(); // Mine initial block and save it to database. let block_hashes = mine_and_save_blocks(&prover, 1).await; @@ -332,13 +338,16 @@ mod tests { #[serial_test::serial] async fn check_for_new_blocks_fallen_behind_multiple() { let config = create_test_config_with_thread_name!(None); - let rpc = ExtendedRpc::new( + let rpc = ExtendedRpc::connect( config.bitcoin_rpc_url.clone(), config.bitcoin_rpc_user.clone(), config.bitcoin_rpc_password.clone(), ) - .await; - let prover = HeaderChainProver::new(&config, rpc.clone()).await.unwrap(); + .await + .unwrap(); + let prover = HeaderChainProver::new(&config, rpc.clone_inner().await.unwrap()) + .await + .unwrap(); // Mine initial block and save it to database. mine_and_save_blocks(&prover, 1).await; @@ -363,13 +372,16 @@ mod tests { #[serial_test::serial] async fn check_for_new_blocks_fork_and_mine_new() { let config = create_test_config_with_thread_name!(None); - let rpc = ExtendedRpc::new( + let rpc = ExtendedRpc::connect( config.bitcoin_rpc_url.clone(), config.bitcoin_rpc_user.clone(), config.bitcoin_rpc_password.clone(), ) - .await; - let prover = HeaderChainProver::new(&config, rpc.clone()).await.unwrap(); + .await + .unwrap(); + let prover = HeaderChainProver::new(&config, rpc.clone_inner().await.unwrap()) + .await + .unwrap(); // Save initial block. mine_and_save_blocks(&prover, 1).await; @@ -395,13 +407,16 @@ mod tests { #[serial_test::serial] async fn sync_blockchain_single_block() { let config = create_test_config_with_thread_name!(None); - let rpc = ExtendedRpc::new( + let rpc = ExtendedRpc::connect( config.bitcoin_rpc_url.clone(), config.bitcoin_rpc_user.clone(), config.bitcoin_rpc_password.clone(), ) - .await; - let prover = HeaderChainProver::new(&config, rpc.clone()).await.unwrap(); + .await + .unwrap(); + let prover = HeaderChainProver::new(&config, rpc.clone_inner().await.unwrap()) + .await + .unwrap(); // Save current blockchain tip. mine_and_save_blocks(&prover, 1).await; @@ -429,13 +444,16 @@ mod tests { #[serial_test::serial] async fn sync_blockchain_multiple_blocks() { let config = create_test_config_with_thread_name!(None); - let rpc = ExtendedRpc::new( + let rpc = ExtendedRpc::connect( config.bitcoin_rpc_url.clone(), config.bitcoin_rpc_user.clone(), config.bitcoin_rpc_password.clone(), ) - .await; - let prover = HeaderChainProver::new(&config, rpc.clone()).await.unwrap(); + .await + .unwrap(); + let prover = HeaderChainProver::new(&config, rpc.clone_inner().await.unwrap()) + .await + .unwrap(); // Save current blockchain tip. mine_and_save_blocks(&prover, 1).await; @@ -463,13 +481,16 @@ mod tests { #[serial_test::serial] async fn sync_blockchain_multiple_blocks_with_fork() { let config = create_test_config_with_thread_name!(None); - let rpc = ExtendedRpc::new( + let rpc = ExtendedRpc::connect( config.bitcoin_rpc_url.clone(), config.bitcoin_rpc_user.clone(), config.bitcoin_rpc_password.clone(), ) - .await; - let prover = HeaderChainProver::new(&config, rpc.clone()).await.unwrap(); + .await + .unwrap(); + let prover = HeaderChainProver::new(&config, rpc.clone_inner().await.unwrap()) + .await + .unwrap(); // Save current blockchain tip. mine_and_save_blocks(&prover, 1).await; diff --git a/core/src/header_chain_prover/mod.rs b/core/src/header_chain_prover/mod.rs index 41181c87..70d1a1c4 100644 --- a/core/src/header_chain_prover/mod.rs +++ b/core/src/header_chain_prover/mod.rs @@ -119,12 +119,13 @@ mod tests { #[tokio::test] async fn new() { let config = create_test_config_with_thread_name!(None); - let rpc = ExtendedRpc::new( + let rpc = ExtendedRpc::connect( config.bitcoin_rpc_url.clone(), config.bitcoin_rpc_user.clone(), config.bitcoin_rpc_password.clone(), ) - .await; + .await + .unwrap(); let _should_not_panic = HeaderChainProver::new(&config, rpc).await.unwrap(); } @@ -133,18 +134,21 @@ mod tests { #[serial_test::serial] async fn new_with_proof_assumption() { let config = create_test_config_with_thread_name!(None); - let rpc = ExtendedRpc::new( + let rpc = ExtendedRpc::connect( config.bitcoin_rpc_url.clone(), config.bitcoin_rpc_user.clone(), config.bitcoin_rpc_password.clone(), ) - .await; + .await + .unwrap(); // First block's assumption will be added to db: Make sure block exists // too. rpc.mine_blocks(1).await.unwrap(); - let prover = HeaderChainProver::new(&config, rpc.clone()).await.unwrap(); + let prover = HeaderChainProver::new(&config, rpc.clone_inner().await.unwrap()) + .await + .unwrap(); // Test assumption is for block 0. let hash = rpc.client.get_block_hash(0).await.unwrap(); @@ -160,13 +164,16 @@ mod tests { #[ignore = "This test is very host dependent and needs a human observer"] async fn start_header_chain_prover() { let config = create_test_config_with_thread_name!(None); - let rpc = ExtendedRpc::new( + let rpc = ExtendedRpc::connect( config.bitcoin_rpc_url.clone(), config.bitcoin_rpc_user.clone(), config.bitcoin_rpc_password.clone(), ) - .await; - let prover = HeaderChainProver::new(&config, rpc.clone()).await.unwrap(); + .await + .unwrap(); + let prover = HeaderChainProver::new(&config, rpc.clone_inner().await.unwrap()) + .await + .unwrap(); prover.run(); sleep(Duration::from_secs(1)).await; diff --git a/core/src/header_chain_prover/prover.rs b/core/src/header_chain_prover/prover.rs index e0dc68b5..ade17de8 100644 --- a/core/src/header_chain_prover/prover.rs +++ b/core/src/header_chain_prover/prover.rs @@ -208,13 +208,16 @@ mod tests { #[serial_test::parallel] async fn prove_block_headers_genesis() { let config = create_test_config_with_thread_name!(None); - let rpc = ExtendedRpc::new( + let rpc = ExtendedRpc::connect( config.bitcoin_rpc_url.clone(), config.bitcoin_rpc_user.clone(), config.bitcoin_rpc_password.clone(), ) - .await; - let prover = HeaderChainProver::new(&config, rpc.clone()).await.unwrap(); + .await + .unwrap(); + let prover = HeaderChainProver::new(&config, rpc.clone_inner().await.unwrap()) + .await + .unwrap(); let receipt = prover.prove_block_headers(None, vec![]).unwrap(); @@ -232,13 +235,16 @@ mod tests { #[serial_test::serial] async fn prove_block_headers_second() { let config = create_test_config_with_thread_name!(None); - let rpc = ExtendedRpc::new( + let rpc = ExtendedRpc::connect( config.bitcoin_rpc_url.clone(), config.bitcoin_rpc_user.clone(), config.bitcoin_rpc_password.clone(), ) - .await; - let prover = HeaderChainProver::new(&config, rpc.clone()).await.unwrap(); + .await + .unwrap(); + let prover = HeaderChainProver::new(&config, rpc.clone_inner().await.unwrap()) + .await + .unwrap(); // Prove genesis block and get it's receipt. let receipt = prover.prove_block_headers(None, vec![]).unwrap(); @@ -258,13 +264,16 @@ mod tests { #[serial_test::serial] async fn save_and_get_proof() { let config = create_test_config_with_thread_name!(None); - let rpc = ExtendedRpc::new( + let rpc = ExtendedRpc::connect( config.bitcoin_rpc_url.clone(), config.bitcoin_rpc_user.clone(), config.bitcoin_rpc_password.clone(), ) - .await; - let prover = HeaderChainProver::new(&config, rpc.clone()).await.unwrap(); + .await + .unwrap(); + let prover = HeaderChainProver::new(&config, rpc.clone_inner().await.unwrap()) + .await + .unwrap(); let block_headers = mine_and_get_first_n_block_headers(rpc, 3).await; // Prove genesis block. diff --git a/core/src/operator.rs b/core/src/operator.rs index 5f85f3d1..edc8a687 100644 --- a/core/src/operator.rs +++ b/core/src/operator.rs @@ -788,7 +788,7 @@ mod tests { // #[tokio::test] // async fn set_funding_utxo() { // let config = create_test_config_with_thread_name!(None); - // let rpc = ExtendedRpc::new( + // let rpc = ExtendedRpc::connect( // config.bitcoin_rpc_url.clone(), // config.bitcoin_rpc_user.clone(), // config.bitcoin_rpc_password.clone(), @@ -821,7 +821,7 @@ mod tests { // #[tokio::test] // async fn is_profitable() { // let mut config = create_test_config_with_thread_name!(None); - // let rpc = ExtendedRpc::new( + // let rpc = ExtendedRpc::connect( // config.bitcoin_rpc_url.clone(), // config.bitcoin_rpc_user.clone(), // config.bitcoin_rpc_password.clone(), @@ -858,12 +858,13 @@ mod tests { #[ignore = "Design changes in progress"] async fn get_winternitz_public_keys() { let config = create_test_config_with_thread_name!(None); - let rpc = ExtendedRpc::new( + let rpc = ExtendedRpc::connect( config.bitcoin_rpc_url.clone(), config.bitcoin_rpc_user.clone(), config.bitcoin_rpc_password.clone(), ) - .await; + .await + .unwrap(); let operator = Operator::new(config.clone(), rpc).await.unwrap(); @@ -877,12 +878,13 @@ mod tests { #[tokio::test] async fn test_generate_preimages_and_hashes() { let config = create_test_config_with_thread_name!(None); - let rpc = ExtendedRpc::new( + let rpc = ExtendedRpc::connect( config.bitcoin_rpc_url.clone(), config.bitcoin_rpc_user.clone(), config.bitcoin_rpc_password.clone(), ) - .await; + .await + .unwrap(); let operator = Operator::new(config.clone(), rpc).await.unwrap(); diff --git a/core/src/rpc/aggregator.rs b/core/src/rpc/aggregator.rs index fac2796d..094b612d 100644 --- a/core/src/rpc/aggregator.rs +++ b/core/src/rpc/aggregator.rs @@ -818,12 +818,13 @@ mod tests { .get_watchtower_winternitz_public_keys() .await .unwrap(); - let rpc = ExtendedRpc::new( + let rpc = ExtendedRpc::connect( config.bitcoin_rpc_url.clone(), config.bitcoin_rpc_user.clone(), config.bitcoin_rpc_password.clone(), ) - .await; + .await + .unwrap(); config.db_name += "0"; // This modification is done by the create_actors_grpc function. let verifier = Verifier::new(rpc, config.clone()).await.unwrap(); let verifier_wpks = verifier @@ -877,12 +878,12 @@ mod tests { .get_watchtower_challenge_addresses() .await .unwrap(); - let rpc = ExtendedRpc::new( + let rpc = ExtendedRpc::connect( config.bitcoin_rpc_url.clone(), config.bitcoin_rpc_user.clone(), config.bitcoin_rpc_password.clone(), ) - .await; + .await.unwrap(); config.db_name += "0"; // This modification is done by the create_actors_grpc function. let verifier = Verifier::new(rpc, config.clone()).await.unwrap(); tracing::info!("verifier config: {:#?}", verifier.config); diff --git a/core/src/servers.rs b/core/src/servers.rs index cd80f6d5..24dda611 100644 --- a/core/src/servers.rs +++ b/core/src/servers.rs @@ -112,7 +112,6 @@ pub async fn create_aggregator_grpc_server( tokio::spawn(async move { if let Err(e) = handle.await { tracing::error!("gRPC server error: {:?}", e); - panic!("gRPC server error: {:?}", e); } }); @@ -141,7 +140,6 @@ pub async fn create_watchtower_grpc_server( tokio::spawn(async move { if let Err(e) = handle.await { tracing::error!("gRPC server error: {:?}", e); - panic!("gRPC server error: {:?}", e); } }); diff --git a/core/src/test_utils.rs b/core/src/test_utils.rs index dcca2af3..78035979 100644 --- a/core/src/test_utils.rs +++ b/core/src/test_utils.rs @@ -169,12 +169,13 @@ macro_rules! initialize_database { macro_rules! create_actors { ($config:expr) => {{ let start_port = $config.port; - let rpc = ExtendedRpc::new( + let rpc = ExtendedRpc::connect( $config.bitcoin_rpc_url.clone(), $config.bitcoin_rpc_user.clone(), $config.bitcoin_rpc_password.clone(), ) - .await; + .await + .unwrap(); let all_verifiers_secret_keys = $config .all_verifiers_secret_keys diff --git a/core/src/verifier.rs b/core/src/verifier.rs index 9ce3a420..eded4169 100644 --- a/core/src/verifier.rs +++ b/core/src/verifier.rs @@ -811,7 +811,7 @@ impl Verifier { // #[tokio::test] // async fn verifier_new_public_key_check() { // let mut config = create_test_config_with_thread_name!(None); -// let rpc = ExtendedRpc::new( +// let rpc = ExtendedRpc::connect( // config.bitcoin_rpc_url.clone(), // config.bitcoin_rpc_user.clone(), // config.bitcoin_rpc_password.clone(), @@ -819,7 +819,7 @@ impl Verifier { // .await; // // Test config file has correct keys. -// Verifier::new(rpc.clone(), config.clone()).await.unwrap(); +// Verifier::new(rpc.clone_inner().await.unwrap(), config.clone()).await.unwrap(); // // Clearing them should result in error. // config.verifiers_public_keys.clear(); @@ -830,13 +830,13 @@ impl Verifier { // #[serial_test::serial] // async fn new_deposit_nonce_checks() { // let config = create_test_config_with_thread_name!(None); -// let rpc = ExtendedRpc::new( +// let rpc = ExtendedRpc::connect( // config.bitcoin_rpc_url.clone(), // config.bitcoin_rpc_user.clone(), // config.bitcoin_rpc_password.clone(), // ) // .await; -// let verifier = Verifier::new(rpc.clone(), config.clone()).await.unwrap(); +// let verifier = Verifier::new(rpc.clone_inner().await.unwrap(), config.clone()).await.unwrap(); // let evm_address = EVMAddress([1u8; 20]); // let deposit_address = get_deposit_address(config, evm_address).unwrap(); This line needs to be converted into get_deposit_address! diff --git a/core/src/watchtower.rs b/core/src/watchtower.rs index 2c8df512..3061ea23 100644 --- a/core/src/watchtower.rs +++ b/core/src/watchtower.rs @@ -19,12 +19,13 @@ pub struct Watchtower { impl Watchtower { pub async fn new(config: BridgeConfig) -> Result { - let _erpc = ExtendedRpc::new( + let _erpc = ExtendedRpc::connect( config.bitcoin_rpc_url.clone(), config.bitcoin_rpc_user.clone(), config.bitcoin_rpc_password.clone(), ) - .await; + .await?; + let _db = Database::new(&config).await?; let actor = Actor::new( config.secret_key, diff --git a/core/tests/common/mod.rs b/core/tests/common/mod.rs index 2feefa47..49bad6a9 100644 --- a/core/tests/common/mod.rs +++ b/core/tests/common/mod.rs @@ -26,7 +26,7 @@ mod test_utils; // pub async fn run_multiple_deposits(test_config_name: &str) { // let config = create_test_config_with_thread_name!(test_config_name, None); -// let rpc = ExtendedRpc::new( +// let rpc = ExtendedRpc::connect( // config.bitcoin_rpc_url.clone(), // config.bitcoin_rpc_user.clone(), // config.bitcoin_rpc_password.clone(), @@ -45,7 +45,7 @@ mod test_utils; // .address // .as_unchecked() // .clone(); -// let user = User::new(rpc.clone(), secret_key, config.clone()); +// let user = User::new(rpc.clone_inner().await.unwrap(), secret_key, config.clone()); // let evm_address = EVMAddress([1u8; 20]); // let deposit_address = user.get_deposit_address(evm_address).unwrap(); This line needs to be converted into get_deposit_address! @@ -258,7 +258,7 @@ mod test_utils; // BridgeError, // > { // let config = create_test_config_with_thread_name!(test_config_name, None); -// let rpc = ExtendedRpc::new( +// let rpc = ExtendedRpc::connect( // config.bitcoin_rpc_url.clone(), // config.bitcoin_rpc_user.clone(), // config.bitcoin_rpc_password.clone(), @@ -271,7 +271,7 @@ mod test_utils; // .as_unchecked() // .clone(); -// let user = User::new(rpc.clone(), secret_key, config.clone()); +// let user = User::new(rpc.clone_inner().await.unwrap(), secret_key, config.clone()); // let evm_address = EVMAddress([1u8; 20]); // let deposit_address = user.get_deposit_address(evm_address).unwrap(); This line needs to be converted into get_deposit_address! diff --git a/core/tests/deposit.rs b/core/tests/deposit.rs index c62dbbba..b2e82cd1 100644 --- a/core/tests/deposit.rs +++ b/core/tests/deposit.rs @@ -17,7 +17,7 @@ // #[serial_test::serial] // async fn deposit_with_retry_checks() { // let config = create_test_config_with_thread_name!(None); -// let rpc = ExtendedRpc::new( +// let rpc = ExtendedRpc::connect( // config.bitcoin_rpc_url.clone(), // config.bitcoin_rpc_user.clone(), // config.bitcoin_rpc_password.clone(), @@ -29,7 +29,7 @@ // .address // .as_unchecked() // .clone(); -// let user = User::new(rpc.clone(), secret_key, config.clone()); +// let user = User::new(rpc.clone_inner().await.unwrap(), secret_key, config.clone()); // let evm_address: EVMAddress = EVMAddress([1u8; 20]); // let deposit_address = user.get_deposit_address(evm_address).unwrap(); This line needs to be converted into get_deposit_address! diff --git a/core/tests/musig2.rs b/core/tests/musig2.rs index 460925ed..e4abc792 100644 --- a/core/tests/musig2.rs +++ b/core/tests/musig2.rs @@ -71,12 +71,13 @@ fn get_nonces( #[serial_test::serial] async fn key_spend() { let config = create_test_config_with_thread_name!(None); - let rpc = ExtendedRpc::new( + let rpc = ExtendedRpc::connect( config.bitcoin_rpc_url.clone(), config.bitcoin_rpc_user.clone(), config.bitcoin_rpc_password.clone(), ) - .await; + .await + .unwrap(); let (verifiers_secret_public_keys, untweaked_xonly_pubkey, verifier_public_keys) = get_verifiers_keys(&config); @@ -166,12 +167,13 @@ async fn key_spend() { #[serial_test::serial] async fn key_spend_with_script() { let config = create_test_config_with_thread_name!(None); - let rpc = ExtendedRpc::new( + let rpc = ExtendedRpc::connect( config.bitcoin_rpc_url.clone(), config.bitcoin_rpc_user.clone(), config.bitcoin_rpc_password.clone(), ) - .await; + .await + .unwrap(); let (verifiers_secret_public_keys, untweaked_xonly_pubkey, verifier_public_keys) = get_verifiers_keys(&config); @@ -266,12 +268,13 @@ async fn key_spend_with_script() { #[serial_test::serial] async fn script_spend() { let config = create_test_config_with_thread_name!(None); - let rpc = ExtendedRpc::new( + let rpc = ExtendedRpc::connect( config.bitcoin_rpc_url.clone(), config.bitcoin_rpc_user.clone(), config.bitcoin_rpc_password.clone(), ) - .await; + .await + .unwrap(); let (verifiers_secret_public_keys, _untweaked_xonly_pubkey, verifier_public_keys) = get_verifiers_keys(&config); @@ -374,12 +377,13 @@ async fn key_and_script_spend() { // Arrange let config = create_test_config_with_thread_name!(None); - let rpc = ExtendedRpc::new( + let rpc = ExtendedRpc::connect( config.bitcoin_rpc_url.clone(), config.bitcoin_rpc_user.clone(), config.bitcoin_rpc_password.clone(), ) - .await; + .await + .unwrap(); // -- Musig2 Setup -- // Generate NofN keys diff --git a/core/tests/rpc.rs b/core/tests/rpc.rs index 63e507d9..32adfc7e 100644 --- a/core/tests/rpc.rs +++ b/core/tests/rpc.rs @@ -10,7 +10,7 @@ mod common; // async fn honest_operator_takes_refund() { // let (_verifiers, operators, config, deposit_outpoint) = // run_single_deposit("test_config.toml").await.unwrap(); -// let rpc = ExtendedRpc::new( +// let rpc = ExtendedRpc::connect( // config.bitcoin_rpc_url.clone(), // config.bitcoin_rpc_user.clone(), // config.bitcoin_rpc_password.clone(), @@ -18,7 +18,7 @@ mod common; // .await; // let user_sk = SecretKey::from_slice(&[13u8; 32]).unwrap(); -// let user = User::new(rpc.clone(), user_sk, config.clone()); +// let user = User::new(rpc.clone_inner().await.unwrap(), user_sk, config.clone()); // let withdrawal_address = Address::p2tr( // &SECP, @@ -86,7 +86,7 @@ mod common; // #[tokio::test] // async fn withdrawal_fee_too_low() { // let (_verifiers, operators, config, _) = run_single_deposit("test_config.toml").await.unwrap(); -// let rpc = ExtendedRpc::new( +// let rpc = ExtendedRpc::connect( // config.bitcoin_rpc_url.clone(), // config.bitcoin_rpc_user.clone(), // config.bitcoin_rpc_password.clone(), @@ -101,7 +101,7 @@ mod common; // config.network, // ); -// let user = User::new(rpc.clone(), user_sk, config.clone()); +// let user = User::new(rpc.clone_inner().await.unwrap(), user_sk, config.clone()); // // We are giving too much sats to the user so that operator won't pay it. // let (empty_utxo, withdrawal_tx_out, user_sig) = user diff --git a/core/tests/taproot.rs b/core/tests/taproot.rs index f45e052f..c08a4753 100644 --- a/core/tests/taproot.rs +++ b/core/tests/taproot.rs @@ -18,12 +18,13 @@ mod common; #[serial_test::serial] async fn create_address_and_transaction_then_sign_transaction() { let config = create_test_config_with_thread_name!(None); - let rpc = ExtendedRpc::new( + let rpc = ExtendedRpc::connect( config.bitcoin_rpc_url, config.bitcoin_rpc_user, config.bitcoin_rpc_password, ) - .await; + .await + .unwrap(); let (xonly_pk, _) = config.secret_key.public_key(&SECP).x_only_public_key(); let address = Address::p2tr(&SECP, xonly_pk, None, config.network); From 080a5553730ab340f95334a7371e76271f843230 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mehmet=20Efe=20Ak=C3=A7a?= Date: Fri, 31 Jan 2025 10:50:33 +0300 Subject: [PATCH 7/8] fmt fix --- core/src/database/operator.rs | 3 ++- core/src/rpc/aggregator.rs | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/core/src/database/operator.rs b/core/src/database/operator.rs index dc73f5db..8a25cbff 100644 --- a/core/src/database/operator.rs +++ b/core/src/database/operator.rs @@ -1128,7 +1128,8 @@ mod tests { config.bitcoin_rpc_user.clone(), config.bitcoin_rpc_password.clone(), ) - .await.unwrap(); + .await + .unwrap(); let operator = Operator::new(config, rpc).await.unwrap(); let operator_idx = 0x45; diff --git a/core/src/rpc/aggregator.rs b/core/src/rpc/aggregator.rs index 094b612d..5c0ed540 100644 --- a/core/src/rpc/aggregator.rs +++ b/core/src/rpc/aggregator.rs @@ -883,7 +883,8 @@ mod tests { config.bitcoin_rpc_user.clone(), config.bitcoin_rpc_password.clone(), ) - .await.unwrap(); + .await + .unwrap(); config.db_name += "0"; // This modification is done by the create_actors_grpc function. let verifier = Verifier::new(rpc, config.clone()).await.unwrap(); tracing::info!("verifier config: {:#?}", verifier.config); From d624edd2f43c3b5d726abdc5d52afeb7915bb2cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mehmet=20Efe=20Ak=C3=A7a?= Date: Fri, 31 Jan 2025 15:55:42 +0300 Subject: [PATCH 8/8] handle panics in pipelining tasks --- core/src/rpc/aggregator.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/core/src/rpc/aggregator.rs b/core/src/rpc/aggregator.rs index 5c0ed540..617cf4a3 100644 --- a/core/src/rpc/aggregator.rs +++ b/core/src/rpc/aggregator.rs @@ -671,7 +671,7 @@ impl ClementineAggregator for Aggregator { // Join the nonce aggregation handle to get the movetx agg nonce. let movetx_agg_nonce = nonce_agg_handle .await - .expect("cancelled task or panic in task")?; + .map_err(|_| Status::internal("panic when aggregating nonces"))??; // Start the deposit finalization pipe. let sig_dist_handle = tokio::spawn(signature_distributor( @@ -694,16 +694,16 @@ impl ClementineAggregator for Aggregator { // Wait for all pipeline tasks to complete nonce_dist_handle .await - .expect("cancelled task or panic in task")?; + .map_err(|_| Status::internal("panic when distributing nonces"))??; sig_agg_handle .await - .expect("cancelled task or panic in task")?; + .map_err(|_| Status::internal("panic when aggregating signatures"))??; sig_dist_handle .await - .expect("cancelled task or panic in task")?; + .map_err(|_| Status::internal("panic when aggregating nonces"))??; let operator_sigs = operator_sigs_fut .await - .expect("cancelled task or panic in task")?; + .map_err(|_| Status::internal("panic when collecting operator signatures"))??; // send operators sigs to verifiers after all verifiers have signed let send_operator_sigs: Vec<_> = deposit_finalize_sender