Merge pull request #286 from chainwayxyz/ceyhun/nonce_pair_error_hand…

…ling

Musig2 error handling and general unwrap removal

core/src/actor.rs

@@ -199,7 +199,8 @@ impl Actor {
             ),
             bitcoin::sighash::TapSighashType::Default,
         )?;
-        Ok(self.sign_with_tweak(sig_hash, None).unwrap())
+
+        self.sign_with_tweak(sig_hash, None)
     }
 
     pub fn convert_tx_to_sighash_script_spend(

core/src/aggregator.rs

@@ -9,7 +9,7 @@ use crate::{
     traits::rpc::AggregatorServer,
     transaction_builder::TransactionBuilder,
     utils::{self, handle_taproot_witness_new},
-    ByteArray32, EVMAddress, UTXO,
+    ByteArray32, ByteArray66, EVMAddress, UTXO,
 };
 use async_trait::async_trait;
 use bitcoin::{address::NetworkUnchecked, Address, OutPoint};
@@ -67,9 +67,8 @@ impl Aggregator {
         );
         // tracing::debug!("SLASH_OR_TAKE_TX: {:?}", tx);
         tracing::debug!("SLASH_OR_TAKE_TX weight: {:?}", tx.tx.weight());
-        let message: [u8; 32] = Actor::convert_tx_to_sighash_script_spend(&mut tx, 0, 0)
-            .unwrap()
-            .to_byte_array();
+        let message: [u8; 32] =
+            Actor::convert_tx_to_sighash_script_spend(&mut tx, 0, 0)?.to_byte_array();
         // tracing::debug!("aggregate SLASH_OR_TAKE_TX message: {:?}", message);
         let final_sig: [u8; 64] = aggregate_partial_signatures(
             self.config.verifiers_public_keys.clone(),
@@ -139,7 +138,7 @@ impl Aggregator {
         };
         // tracing::debug!(
         //     "SERDE_UTXO: {:#?}",
-        //     serde_json::to_string(&slash_or_take_utxo).unwrap()
+        //     serde_json::to_string(&slash_or_take_utxo)?
         // );
 
         let mut tx_handler = TransactionBuilder::create_operator_takes_tx(
@@ -158,9 +157,8 @@ impl Aggregator {
         // );
         // tracing::debug!("OPERATOR_TAKES_TX_HEX: {:?}", tx_handler.tx.raw_hex());
         tracing::debug!("OPERATOR_TAKES_TX weight: {:?}", tx_handler.tx.weight());
-        let message: [u8; 32] = Actor::convert_tx_to_sighash_pubkey_spend(&mut tx_handler, 0)
-            .unwrap()
-            .to_byte_array();
+        let message: [u8; 32] =
+            Actor::convert_tx_to_sighash_pubkey_spend(&mut tx_handler, 0)?.to_byte_array();
         let final_sig: [u8; 64] = aggregate_partial_signatures(
             self.config.verifiers_public_keys.clone(),
             None,
@@ -192,9 +190,8 @@ impl Aggregator {
         );
         // println!("MOVE_TX: {:?}", tx);
         // println!("MOVE_TXID: {:?}", tx.tx.compute_txid());
-        let message: [u8; 32] = Actor::convert_tx_to_sighash_script_spend(&mut tx, 0, 0)
-            .unwrap()
-            .to_byte_array();
+        let message: [u8; 32] =
+            Actor::convert_tx_to_sighash_script_spend(&mut tx, 0, 0)?.to_byte_array();
         let final_sig: [u8; 64] = aggregate_partial_signatures(
             self.config.verifiers_public_keys.clone(),
             None,
@@ -213,15 +210,15 @@ impl Aggregator {
     ) -> Result<Vec<MuSigAggNonce>, BridgeError> {
         let mut agg_nonces = Vec::new();
         for i in 0..pub_nonces[0].len() {
-            let agg_nonce = aggregate_nonces(
-                pub_nonces
-                    .iter()
-                    .map(|v| v.get(i).cloned().unwrap())
-                    .collect::<Vec<_>>(),
-            );
+            let pub_nonces = pub_nonces
+                .iter()
+                .map(|v| v.get(i).cloned())
+                .collect::<Option<Vec<ByteArray66>>>()
+                .ok_or(BridgeError::NoncesNotFound)?;
 
-            agg_nonces.push(agg_nonce);
+            agg_nonces.push(aggregate_nonces(pub_nonces));
         }
+
         Ok(agg_nonces)
     }
 
@@ -239,18 +236,22 @@ impl Aggregator {
             agg_nonces,
             partial_sigs
         );
+
         let mut slash_or_take_sigs = Vec::new();
         for i in 0..partial_sigs[0].len() {
+            let partial_sigs = partial_sigs
+                .iter()
+                .map(|v| v.get(i).cloned())
+                .collect::<Option<Vec<ByteArray32>>>()
+                .ok_or(BridgeError::NoncesNotFound)?;
+
             let agg_sig = self.aggregate_slash_or_take_partial_sigs(
                 deposit_outpoint,
                 kickoff_utxos[i].clone(),
                 self.config.operators_xonly_pks[i],
                 i,
                 &agg_nonces[i].clone(),
-                partial_sigs
-                    .iter()
-                    .map(|v| v.get(i).cloned().unwrap())
-                    .collect::<Vec<_>>(),
+                partial_sigs,
             )?;
 
             slash_or_take_sigs.push(secp256k1::schnorr::Signature::from_slice(&agg_sig)?);

core/src/errors.rs

@@ -165,6 +165,9 @@ pub enum BridgeError {
 
     #[error("FromSliceError: {0}")]
     FromSliceError(#[from] bitcoin::hashes::FromSliceError),
+
+    #[error("Musig2 error: {0}")]
+    Musig2Error(#[from] musig2::secp256k1::Error),
 }
 
 impl From<BridgeError> for ErrorObject<'static> {

core/src/musig2.rs

@@ -99,7 +99,7 @@ pub fn aggregate_partial_signatures(
     partial_sigs: Vec<MuSigPartialSignature>,
     message: MuSigSigHash,
 ) -> Result<[u8; 64], BridgeError> {
-    let key_agg_ctx = create_key_agg_ctx(pks, tweak, tweak_flag).unwrap();
+    let key_agg_ctx = create_key_agg_ctx(pks, tweak, tweak_flag)?;
     let musig_partial_sigs: Vec<musig2::PartialSignature> = partial_sigs
         .iter()
         .map(|x| musig2::PartialSignature::from_slice(&x.0).unwrap())
@@ -125,12 +125,15 @@ pub fn nonce_pair(
     let spices = SecNonceSpices::new().with_seckey(
         musig2::secp256k1::SecretKey::from_slice(&keypair.secret_key().secret_bytes()).unwrap(),
     );
+
     let sec_nonce = SecNonce::build(rnd)
         .with_pubkey(musig_pubkey)
         .with_spices(spices)
         .build();
+
     let pub_nonce = ByteArray66(sec_nonce.public_nonce().into());
     let sec_nonce: [u8; 64] = sec_nonce.into();
+
     (ByteArray64(sec_nonce), pub_nonce)
 }
 
@@ -162,9 +165,7 @@ pub fn partial_sign(
 
 #[cfg(test)]
 mod tests {
-
-    use std::vec;
-
+    use super::{nonce_pair, MuSigNoncePair};
     use crate::{
         actor::Actor,
         errors::BridgeError,
@@ -177,8 +178,7 @@ mod tests {
         TxOut, Txid,
     };
     use secp256k1::{rand::Rng, Keypair, Message, XOnlyPublicKey};
-
-    use super::{nonce_pair, MuSigNoncePair};
+    use std::vec;
 
     // Generates a test setup with a given number of signers. Returns a vector of keypairs and a vector of nonce pairs.
     fn generate_test_setup(num_signers: usize) -> (Vec<Keypair>, Vec<MuSigNoncePair>) {

core/src/operator.rs

@@ -53,7 +53,10 @@ where
             .operators_xonly_pks
             .iter()
             .position(|xonly_pk| xonly_pk == &signer.xonly_public_key)
-            .unwrap();
+            .ok_or(BridgeError::ServerError(std::io::Error::other(format!(
+                "{} is not found in operator x-only public keys",
+                signer.xonly_public_key
+            ))))?;
 
         let mut tx = db.begin_transaction().await?;
         // check if funding utxo is already set

core/src/servers.rs

@@ -44,9 +44,8 @@ where
     let addr: std::net::SocketAddr = server.local_addr().map_err(BridgeError::ServerError)?;
     let handle = server.start(verifier.into_rpc());
 
-    let client = HttpClientBuilder::default()
-        .build(format!("http://{}:{}/", addr.ip(), addr.port()))
-        .unwrap();
+    let client =
+        HttpClientBuilder::default().build(format!("http://{}:{}/", addr.ip(), addr.port()))?;
 
     tracing::info!("Verifier server started with address: {}", addr);
 
@@ -74,9 +73,8 @@ where
     let addr: std::net::SocketAddr = server.local_addr().map_err(BridgeError::ServerError)?;
     let handle = server.start(operator.into_rpc());
 
-    let client = HttpClientBuilder::default()
-        .build(format!("http://{}:{}/", addr.ip(), addr.port()))
-        .unwrap();
+    let client =
+        HttpClientBuilder::default().build(format!("http://{}:{}/", addr.ip(), addr.port()))?;
 
     tracing::info!("Operator server started with address: {}", addr);
 
@@ -100,9 +98,8 @@ pub async fn create_aggregator_server(
     let addr: std::net::SocketAddr = server.local_addr().map_err(BridgeError::ServerError)?;
     let handle = server.start(aggregator.into_rpc());
 
-    let client = HttpClientBuilder::default()
-        .build(format!("http://{}:{}/", addr.ip(), addr.port()))
-        .unwrap();
+    let client =
+        HttpClientBuilder::default().build(format!("http://{}:{}/", addr.ip(), addr.port()))?;
 
     tracing::info!("Aggregator server started with address: {}", addr);
 
@@ -158,8 +155,7 @@ pub async fn create_verifiers_and_operators(
                     },
                     rpc,
                 )
-                .await
-                .unwrap();
+                .await?;
                 Ok::<
                     (
                         (HttpClient, ServerHandle, std::net::SocketAddr),

core/src/verifier.rs

@@ -9,7 +9,7 @@ use crate::musig2::{
 };
 use crate::traits::rpc::VerifierRpcServer;
 use crate::transaction_builder::{TransactionBuilder, TxHandler, KICKOFF_UTXO_AMOUNT_SATS};
-use crate::{utils, ByteArray32, EVMAddress, UTXO};
+use crate::{utils, ByteArray32, ByteArray64, ByteArray66, EVMAddress, UTXO};
 use bitcoin::address::NetworkUnchecked;
 use bitcoin::hashes::Hash;
 use bitcoin::Address;
@@ -99,7 +99,6 @@ where
             .get_pub_nonces(Some(&mut dbtx), deposit_outpoint)
             .await?;
         if let Some(pub_nonces) = pub_nonces_from_db {
-            tracing::debug!("AAAAAAAA");
             if !pub_nonces.is_empty() {
                 if pub_nonces.len() != num_required_nonces {
                     return Err(BridgeError::NoncesNotFound);
@@ -112,6 +111,9 @@ where
         let nonces = (0..num_required_nonces)
             .map(|_| musig2::nonce_pair(&self.signer.keypair, &mut rand::rngs::OsRng))
             .collect::<Vec<_>>();
+        let nonces: Vec<(ByteArray64, ByteArray66)> = nonces
+            .into_iter()
+            .collect::<Vec<(ByteArray64, ByteArray66)>>();
 
         self.db
             .save_deposit_info(
@@ -373,8 +375,7 @@ where
 
         self.db
             .save_slash_or_take_sigs(deposit_outpoint, slash_or_take_sigs)
-            .await
-            .unwrap();
+            .await?;
 
         // println!("Operator takes sighashes: {:?}", operator_takes_sighashes);
         let nonces = self
@@ -471,8 +472,7 @@ where
 
         self.db
             .save_operator_take_sigs(deposit_outpoint, kickoff_utxos)
-            .await
-            .unwrap();
+            .await?;
 
         // println!("MOVE_TX: {:?}", move_tx_handler);
         // println!("MOVE_TXID: {:?}", move_tx_handler.tx.compute_txid());