Eliminate build_bitcoin_punish_transaction

We reduce indirection by constructing TxPunish directly based off
`State3` and make the type itself more powerful by moving the logic
of completing it with a signature onto it.

swap/src/bitcoin.rs

@@ -110,6 +110,15 @@ impl From<PublicKey> for Point {
     }
 }
 
+impl From<PublicKey> for ::bitcoin::PublicKey {
+    fn from(from: PublicKey) -> Self {
+        ::bitcoin::PublicKey {
+            compressed: true,
+            key: from.0.into(),
+        }
+    }
+}
+
 impl From<Point> for PublicKey {
     fn from(p: Point) -> Self {
         Self(p)

swap/src/bitcoin/punish.rs

@@ -1,12 +1,11 @@
-use crate::bitcoin::{Address, PublicKey, PunishTimelock, Transaction, TxCancel};
+use crate::bitcoin::{self, Address, PunishTimelock, Transaction, TxCancel};
 use ::bitcoin::util::bip143::SigHashCache;
 use ::bitcoin::{SigHash, SigHashType};
-use anyhow::Result;
-use ecdsa_fun::Signature;
+use anyhow::{Context, Result};
 use miniscript::{Descriptor, DescriptorTrait};
 use std::collections::HashMap;
 
-#[derive(Debug)]
+#[derive(Clone, Debug)]
 pub struct TxPunish {
     inner: Transaction,
     digest: SigHash,
@@ -39,22 +38,20 @@ impl TxPunish {
         self.digest
     }
 
-    pub fn add_signatures(
+    pub fn complete(
         self,
-        (A, sig_a): (PublicKey, Signature),
-        (B, sig_b): (PublicKey, Signature),
+        tx_punish_sig_bob: bitcoin::Signature,
+        a: bitcoin::SecretKey,
+        B: bitcoin::PublicKey,
     ) -> Result<Transaction> {
+        let sig_a = a.sign(self.digest());
+        let sig_b = tx_punish_sig_bob;
+
         let satisfier = {
             let mut satisfier = HashMap::with_capacity(2);
 
-            let A = ::bitcoin::PublicKey {
-                compressed: true,
-                key: A.0.into(),
-            };
-            let B = ::bitcoin::PublicKey {
-                compressed: true,
-                key: B.0.into(),
-            };
+            let A = a.public().into();
+            let B = B.into();
 
             // The order in which these are inserted doesn't matter
             satisfier.insert(A, (sig_a.into(), ::bitcoin::SigHashType::All));
@@ -65,7 +62,8 @@ impl TxPunish {
 
         let mut tx_punish = self.inner;
         self.cancel_output_descriptor
-            .satisfy(&mut tx_punish.input[0], satisfier)?;
+            .satisfy(&mut tx_punish.input[0], satisfier)
+            .context("Failed to satisfy inputs with given signatures")?;
 
         Ok(tx_punish)
     }

swap/src/protocol/alice/state.rs

@@ -1,6 +1,6 @@
 use crate::bitcoin::{
     current_epoch, wait_for_cancel_timelock_to_expire, CancelTimelock, ExpiredTimelocks,
-    PunishTimelock, TxCancel, TxRefund,
+    PunishTimelock, TxCancel, TxPunish, TxRefund,
 };
 use crate::execution_params::ExecutionParams;
 use crate::protocol::alice::{Message1, Message3};
@@ -343,4 +343,11 @@ impl State3 {
         )
         .await
     }
+
+    pub fn tx_punish(&self) -> TxPunish {
+        let tx_cancel =
+            bitcoin::TxCancel::new(&self.tx_lock, self.cancel_timelock, self.a.public(), self.B);
+
+        bitcoin::TxPunish::new(&tx_cancel, &self.punish_address, self.punish_timelock)
+    }
 }

swap/src/protocol/alice/steps.rs

@@ -143,25 +143,3 @@ pub fn extract_monero_private_key(
 
     Ok(spend_key)
 }
-
-pub fn build_bitcoin_punish_transaction(
-    tx_lock: &TxLock,
-    cancel_timelock: CancelTimelock,
-    punish_address: &bitcoin::Address,
-    punish_timelock: PunishTimelock,
-    tx_punish_sig_bob: bitcoin::Signature,
-    a: bitcoin::SecretKey,
-    B: bitcoin::PublicKey,
-) -> Result<bitcoin::Transaction> {
-    let tx_cancel = bitcoin::TxCancel::new(&tx_lock, cancel_timelock, a.public(), B);
-    let tx_punish = bitcoin::TxPunish::new(&tx_cancel, &punish_address, punish_timelock);
-
-    let sig_a = a.sign(tx_punish.digest());
-    let sig_b = tx_punish_sig_bob;
-
-    let signed_tx_punish = tx_punish
-        .add_signatures((a.public(), sig_a), (B, sig_b))
-        .expect("sig_{a,b} to be valid signatures for tx_cancel");
-
-    Ok(signed_tx_punish)
-}

swap/src/protocol/alice/swap.rs

@@ -7,8 +7,8 @@ use crate::monero_ext::ScalarExt;
 use crate::protocol::alice;
 use crate::protocol::alice::event_loop::EventLoopHandle;
 use crate::protocol::alice::steps::{
-    build_bitcoin_punish_transaction, extract_monero_private_key, lock_xmr,
-    publish_cancel_transaction, wait_for_bitcoin_encrypted_signature, wait_for_bitcoin_refund,
+    extract_monero_private_key, lock_xmr, publish_cancel_transaction,
+    wait_for_bitcoin_encrypted_signature, wait_for_bitcoin_refund,
 };
 use crate::protocol::alice::AliceState;
 use crate::{bitcoin, database, monero};
@@ -398,11 +398,7 @@ async fn run_until_internal(
                 state3,
                 monero_wallet_restore_blockheight,
             } => {
-                let signed_tx_punish = build_bitcoin_punish_transaction(
-                    &state3.tx_lock,
-                    state3.cancel_timelock,
-                    &state3.punish_address,
-                    state3.punish_timelock,
+                let signed_tx_punish = state3.tx_punish().complete(
                     state3.tx_punish_sig_bob.clone(),
                     state3.a.clone(),
                     state3.B,