Merge pull request #125 from stejbac/refactor-protocol-trade-model

Refactor trade model & pass contract-forming txids to RPC client

Author: ChrisSon15

protocol/src/protocol_musig_adaptor.rs

@@ -5,7 +5,7 @@ use bdk_electrum::BdkElectrumClient;
 use bdk_electrum::electrum_client::Client;
 use bdk_wallet::bitcoin::bip32::Xpriv;
 use bdk_wallet::bitcoin::{
-    Address, Amount, FeeRate, Network, OutPoint, Psbt, ScriptBuf, Transaction, TxOut, Txid,
+    Address, Amount, FeeRate, Network, OutPoint, Psbt, Script, ScriptBuf, Transaction, TxOut, Txid,
     relative,
 };
 use bdk_wallet::template::{Bip86, DescriptorTemplate as _};
@@ -333,7 +333,7 @@ impl BMPProtocol {
             ProtocolRole::Buyer => self.p_tik.peers_key_share(),
         }?.pub_key();
         // given the DepositTx, we can create SwapTx for Alice.
-        self.swap_tx.build(&self.q_tik, adaptor_point, &self.deposit_tx, bob.swap_script.as_ref())?;
+        self.swap_tx.build(&self.q_tik, adaptor_point, &self.deposit_tx, bob.swap_script.as_deref())?;
         // let start the signing process for SwapTx already.
         let swap_pub_nonce = self.swap_tx.get_pub_nonce(); // could be one round earlier, if we solve secure nonce generation
 
@@ -377,8 +377,7 @@ impl BMPProtocol {
         assert_eq!(bob.p_agg, *self.p_tik.aggregated_key()?.pub_key(), "Bob is sending the wrong P' for his aggregated key.");
         assert_eq!(bob.q_agg, *self.q_tik.aggregated_key()?.pub_key(), "Bob is sending the wrong Q' for his aggregated key.");
 
-        // let txid = self.deposit_tx.transfer_sig_and_broadcast(&mut self.ctx, bob.deposit_tx_merged)?;
-        let txid = self.deposit_tx.tx()?.compute_txid();
+        let txid = self.deposit_tx.txid()?;
         // here we are building the partial signature of the SwapTx, note that there is only one SwapTx (for Alice)
         let swap_part_sig = self.swap_tx.build_partial_sig(bob.swap_pub_nonce)?;
 
@@ -464,8 +463,7 @@ impl RedirectTx {
         let fee_rate = FeeRate::from_sat_per_vb_unchecked(10); // TODO: feerates shall come from pricenodes
 
         let escrow_amount = warn_tx.builder.escrow()?.prevout.value;
-        let available_amount_msat = RedirectTxBuilder::available_amount_msat(escrow_amount, fee_rate)
-            .ok_or(anyhow::anyhow!("Overflow computing available amount for receivers"))?;
+        let available_amount_msat = RedirectTxBuilder::available_amount_msat(escrow_amount, fee_rate)?;
 
         let receivers = Receiver::compute_receivers_from_shares(receiver_shares, available_amount_msat, fee_rate)
             .ok_or(anyhow::anyhow!("Could not compute receiver list"))?;
@@ -582,15 +580,9 @@ impl WarningTx {
 
     pub fn signed_tx(&self) -> anyhow::Result<&Transaction> { Ok(self.builder.signed_tx()?) }
 
-    pub fn funds_as_outpoint(&self) -> OutPoint {
-        OutPoint::new(self.unsigned_tx().unwrap().compute_txid(), 0)
-    }
+    pub fn funds_as_outpoint(&self) -> OutPoint { self.builder.escrow().unwrap().outpoint }
 
-    pub fn funds_as_output(&self) -> TxOut {
-        let w = self.unsigned_tx().unwrap();
-        let wout: &Vec<TxOut> = w.output.as_ref();
-        wout[0].clone()
-    }
+    pub fn funds_as_output(&self) -> TxOut { self.builder.escrow().unwrap().prevout }
 
     pub fn new(role: ProtocolRole) -> Self {
         Self {
@@ -614,17 +606,17 @@ impl WarningTx {
             ProtocolRole::Buyer => p_tik
         }.with_taproot_tweak(None)?;
 
-        let tx = self.builder
+        let txid = self.builder
             .set_buyer_input(deposit_tx.builder.buyer_payout()?.clone())
             .set_seller_input(deposit_tx.builder.seller_payout()?.clone())
             .set_escrow_address(key_spend.p2tr_address(Network::Regtest))
             .set_anchor_address(Address::from_script(self.anchor_spend.as_ref().unwrap(), Network::Regtest)?) // TODO: Improve.
             .set_lock_time(relative::LockTime::ZERO)
             .set_fee_rate(FeeRate::from_sat_per_vb_unchecked(10)) // TODO: feerates shall come from pricenodes
             .compute_unsigned_tx()?
-            .unsigned_tx()?;
+            .txid()?;
 
-        dbg!(ctx.role, self.role, tx.compute_txid());
+        dbg!(ctx.role, self.role, txid);
         Ok(())
     }
 
@@ -700,15 +692,15 @@ impl SwapTx {
     }
 
     // round 1
-    pub fn build(&mut self, q_tik: &KeyCtx, p_a: Point, deposit_tx: &DepositTx, swap_spend_opt: Option<&ScriptBuf>) -> anyhow::Result<()> {
+    pub fn build(&mut self, q_tik: &KeyCtx, p_a: Point, deposit_tx: &DepositTx, swap_spend_opt: Option<&Script>) -> anyhow::Result<()> {
         self.fund_sig.set_tweaked_key_ctx(q_tik.with_taproot_tweak(None)?);
         // SwapTx is asymmetric, both parties need to agree on P_a being the public adaptor
         // P_a is the Public key which Alice (the seller) contributes to 2of2 Multisig to lock the deposit and trade amount in the DepositTx
         // if secret key of P_a is revealed to Bob, then we has both partial keys to it and is able to spend it.
         self.fund_sig.set_adaptor_point(p_a)?;
         self.fund_sig.init_my_nonce_share()?;
         let Some(use_spend) = (match self.role {
-            ProtocolRole::Seller => self.swap_spend.as_ref(),
+            ProtocolRole::Seller => self.swap_spend.as_deref(),
             ProtocolRole::Buyer => swap_spend_opt,
         }) else { panic!("No spend-condition from role {:?}", self.role) };
 
@@ -781,7 +773,7 @@ impl DepositTx {
 
     fn merged_psbt(&self) -> anyhow::Result<&Psbt> { Ok(self.builder.psbt()?) }
 
-    fn tx(&self) -> anyhow::Result<&Transaction> { Ok(&self.merged_psbt()?.unsigned_tx) }
+    fn txid(&self) -> anyhow::Result<Txid> { Ok(*self.builder.txid()?) }
 
     pub fn generate_part_tx(&mut self, ctx: &mut BMPContext) -> anyhow::Result<Psbt> {
         self.builder
@@ -837,13 +829,13 @@ impl DepositTx {
         Ok(deposit_txid)
     }
 
-    fn _get_outpoint_for(self, script: &ScriptBuf) -> anyhow::Result<OutPoint> {
-        let tx = self.tx()?;
+    fn _get_outpoint_for(self, script: &Script) -> anyhow::Result<OutPoint> {
+        let tx = &self.merged_psbt()?.unsigned_tx;
 
         for (index, output) in tx.output.iter().enumerate() {
             if output.script_pubkey == *script {
                 return Ok(OutPoint {
-                    txid: tx.compute_txid(),
+                    txid: self.txid()?,
                     vout: u32::try_from(index)?,
                 });
             }

protocol/src/psbt.rs

@@ -80,8 +80,7 @@ impl<Cs: Iterator<Item=TxOutput>, As: Iterator<Item=Address>> TradeWallet for Mo
         output.extend(trade_fee_receivers.iter().map(TxOut::from));
         let mut change_output = TxOut { value: Amount::ZERO, script_pubkey: self.new_address()?.script_pubkey() };
 
-        let mut cost_msat = Receiver::total_output_cost_msat(trade_fee_receivers, fee_rate, 2)
-            .ok_or(TransactionErrorKind::Overflow)?
+        let mut cost_msat = Receiver::total_output_cost_msat(trade_fee_receivers, fee_rate, 2)?
             .checked_add(deposit_amount_msat)
             .ok_or(TransactionErrorKind::Overflow)?
             .checked_add(fee_cost_msat(HALF_DEPOSIT_TX_BASE_WEIGHT)?)

protocol/src/receiver.rs

@@ -4,7 +4,7 @@ use bdk_wallet::bitcoin::address::{NetworkChecked, NetworkUnchecked, NetworkVali
 use bdk_wallet::bitcoin::amount::CheckedSum as _;
 use bdk_wallet::bitcoin::{Address, Amount, FeeRate, Network, TxOut, Weight};
 
-use crate::transaction::Result;
+use crate::transaction::{Result, TransactionErrorKind};
 
 // Receivers paid less than this absolute satoshi amount are excluded:
 const MIN_OUTPUT_AMOUNT: Amount = Amount::from_sat(1000);
@@ -32,21 +32,23 @@ impl Receiver {
         amount_msat.checked_add(fee_msat)
     }
 
-    pub fn total_output_cost_msat<'a, I>(receivers: I, fee_rate: FeeRate, extra_output_num: u16) -> Option<u64>
+    pub fn total_output_cost_msat<'a, I>(receivers: I, fee_rate: FeeRate, extra_output_num: u16) -> Result<u64>
         where I: IntoIterator<Item=&'a Self>
     {
-        let mut cost = 0u64;
-        let mut num = extra_output_num;
-        for receiver in receivers {
-            cost = cost.checked_add(receiver.output_cost_msat(fee_rate)?)?;
-            // Fail if more than 65535 outputs, which will never happen for a standard tx:
-            num = num.checked_add(1)?;
-        }
-        if num > 252 {
-            // For more than 252 outputs, we get a 3-byte length encoding instead of 1, adding 8 wu.
-            cost = cost.checked_add(fee_rate.to_sat_per_kwu().checked_mul(8)?)?;
-        }
-        Some(cost)
+        (|| {
+            let mut cost = 0u64;
+            let mut num = extra_output_num;
+            for receiver in receivers {
+                cost = cost.checked_add(receiver.output_cost_msat(fee_rate)?)?;
+                // Fail if more than 65535 outputs, which will never happen for a standard tx:
+                num = num.checked_add(1)?;
+            }
+            if num > 252 {
+                // For >252 outputs, we get a 3-byte length encoding instead of 1, adding 8 wu.
+                cost = cost.checked_add(fee_rate.to_sat_per_kwu().checked_mul(8)?)?;
+            }
+            Some(cost)
+        })().ok_or(TransactionErrorKind::Overflow)
     }
 
     // TODO: Consider returning a `Result<T>` instead of an `Option<T>` to distinguish overflows

protocol/src/transaction.rs

@@ -10,7 +10,7 @@ use bdk_wallet::bitcoin::taproot::{Signature, TAPROOT_ANNEX_PREFIX, TaprootBuild
 use bdk_wallet::bitcoin::transaction::Version;
 use bdk_wallet::bitcoin::{
     Address, Amount, FeeRate, Network, OutPoint, Psbt, ScriptBuf, Sequence, TapNodeHash,
-    TapSighash, TapSighashType, Transaction, TxIn, TxOut, Weight, Witness, XOnlyPublicKey,
+    TapSighash, TapSighashType, Transaction, TxIn, TxOut, Txid, Weight, Witness, XOnlyPublicKey,
     absolute, relative, script, secp256k1,
 };
 use paste::paste;
@@ -93,9 +93,12 @@ impl NetworkParams for Network {
 
 macro_rules! make_getter {
     ($field_name:ident: $field_type:ident) => {
+        make_getter!($field_name: $field_type: $field_type);
+    };
+    ($field_name:ident: $field_type:ident: $base_field_type:ident) => {
         paste! {
             pub fn $field_name(&self) -> Result<&$field_type> {
-                self.$field_name.as_ref().ok_or(TransactionErrorKind::[<Missing $field_type>])
+                self.$field_name.as_ref().ok_or(TransactionErrorKind::[<Missing $base_field_type>])
             }
         }
     };
@@ -253,9 +256,9 @@ impl DepositTxBuilder {
     make_getter_setter!(fee_rate: FeeRate);
     make_getter_setter!(buyers_half_psbt: Psbt);
     make_getter_setter!(sellers_half_psbt: Psbt);
-    make_getter!(psbt: Psbt);
-    make_getter!(buyer_payout: TxOutput);
-    make_getter!(seller_payout: TxOutput);
+    make_getter!(psbt: Psbt: Transaction);
+    make_getter!(buyer_payout: TxOutput: Transaction);
+    make_getter!(seller_payout: TxOutput: Transaction);
 
     fn sellers_trade_deposit(&self) -> Result<Amount> {
         self.trade_amount()?.checked_add(*self.sellers_security_deposit()?)
@@ -316,6 +319,8 @@ impl DepositTxBuilder {
         Ok(self)
     }
 
+    pub fn txid(&self) -> Result<&Txid> { Ok(&self.buyer_payout()?.outpoint.txid) }
+
     pub fn sign_buyer_inputs(&mut self, trade_wallet: &(impl TradeWallet + ?Sized)) -> Result<&mut Self> {
         self.sign_matching_inputs(trade_wallet, &prevout_set(self.buyers_half_psbt()?))
     }
@@ -329,14 +334,14 @@ impl DepositTxBuilder {
         trade_wallet: &(impl TradeWallet + ?Sized),
         prevouts: &BTreeSet<OutPoint>,
     ) -> Result<&mut Self> {
-        let psbt = self.psbt.as_mut().ok_or(TransactionErrorKind::MissingPsbt)?;
+        let psbt = self.psbt.as_mut().ok_or(TransactionErrorKind::MissingTransaction)?;
         trade_wallet.sign_selected_inputs(psbt, &|o| prevouts.contains(o))?;
         Ok(self)
     }
 
     pub fn combine_psbts(&mut self, other: Psbt) -> Result<&mut Self> {
         // TODO: We may need to do some validation of the provided PSBT.
-        self.psbt.as_mut().ok_or(TransactionErrorKind::MissingPsbt)?.combine(other)?;
+        self.psbt.as_mut().ok_or(TransactionErrorKind::MissingTransaction)?.combine(other)?;
         Ok(self)
     }
 
@@ -357,6 +362,7 @@ pub struct WarningTxBuilder {
     // Derived fields:
     unsigned_tx: Option<Transaction>,
     signed_tx: Option<Transaction>,
+    txid: Option<Txid>,
 }
 
 impl WarningTxBuilder {
@@ -370,19 +376,20 @@ impl WarningTxBuilder {
     make_getter_setter!(seller_input_signature: Signature);
     make_getter!(unsigned_tx: Transaction);
     make_getter!(signed_tx: Transaction);
+    make_getter!(txid: Txid: Transaction);
 
-    pub fn escrow_amount(input_amounts: impl IntoIterator<Item=Amount>, fee_rate: FeeRate) -> Option<Amount> {
-        input_amounts.into_iter().checked_sum()?
+    pub fn escrow_amount(input_amounts: impl IntoIterator<Item=Amount>, fee_rate: FeeRate) -> Result<Amount> {
+        (|| input_amounts.into_iter().checked_sum()?
             .checked_sub(ANCHOR_AMOUNT)?
             .checked_sub(fee_rate.checked_mul_by_weight(SIGNED_WARNING_TX_WEIGHT)?)
+        )().ok_or(TransactionErrorKind::Overflow)
     }
 
     pub fn compute_unsigned_tx(&mut self) -> Result<&mut Self> {
         let escrow_output = TxOut {
             value: Self::escrow_amount(
                 [self.buyer_input()?.prevout.value, self.seller_input()?.prevout.value],
-                *self.fee_rate()?,
-            ).ok_or(TransactionErrorKind::Overflow)?,
+                *self.fee_rate()?)?,
             script_pubkey: self.escrow_address()?.script_pubkey(),
         };
         let anchor_output = TxOut {
@@ -395,14 +402,13 @@ impl WarningTxBuilder {
             input: self.tx_ins(*self.lock_time()?)?.to_vec(),
             output: vec![escrow_output, anchor_output],
         };
-        self.unsigned_tx.get_or_insert(tx);
+        self.txid = Some(self.unsigned_tx.get_or_insert(tx).compute_txid());
         Ok(self)
     }
 
     pub fn escrow(&self) -> Result<TxOutput> {
-        let txid = self.unsigned_tx()?.compute_txid();
         let output = self.unsigned_tx()?.output[0].clone();
-        Ok(TxOutput::new(OutPoint::new(txid, 0), output))
+        Ok(TxOutput::new(OutPoint::new(*self.txid()?, 0), output))
     }
 
     pub fn buyer_input_sighash(&self) -> Result<TapSighash> {
@@ -437,6 +443,7 @@ pub struct RedirectTxBuilder {
     // Derived fields:
     unsigned_tx: Option<Transaction>,
     signed_tx: Option<Transaction>,
+    txid: Option<Txid>,
 }
 
 impl RedirectTxBuilder {
@@ -447,14 +454,17 @@ impl RedirectTxBuilder {
     make_getter_setter!(input_signature: Signature);
     make_getter!(unsigned_tx: Transaction);
     make_getter!(signed_tx: Transaction);
+    make_getter!(txid: Txid: Transaction);
 
-    pub fn available_amount_msat(escrow_amount: Amount, fee_rate: FeeRate) -> Option<u64> {
-        let redirection_tx_base_fee = fee_rate.to_sat_per_kwu()
-            .checked_mul(SIGNED_REDIRECT_TX_BASE_WEIGHT.to_wu())?;
-        escrow_amount
-            .checked_sub(ANCHOR_AMOUNT)?
-            .to_sat().checked_mul(1000)?
-            .checked_sub(redirection_tx_base_fee)
+    pub fn available_amount_msat(escrow_amount: Amount, fee_rate: FeeRate) -> Result<u64> {
+        (|| {
+            let redirection_tx_base_fee = fee_rate.to_sat_per_kwu()
+                .checked_mul(SIGNED_REDIRECT_TX_BASE_WEIGHT.to_wu())?;
+            escrow_amount
+                .checked_sub(ANCHOR_AMOUNT)?
+                .to_sat().checked_mul(1000)?
+                .checked_sub(redirection_tx_base_fee)
+        })().ok_or(TransactionErrorKind::Overflow)
     }
 
     pub fn compute_unsigned_tx(&mut self) -> Result<&mut Self> {
@@ -470,7 +480,7 @@ impl RedirectTxBuilder {
             input: self.tx_ins(*self.lock_time()?)?.to_vec(),
             output,
         };
-        self.unsigned_tx.get_or_insert(tx);
+        self.txid = Some(self.unsigned_tx.get_or_insert(tx).compute_txid());
         Ok(self)
     }
 

rpc/build.rs

@@ -28,6 +28,10 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
             "ReceiverAddressAndAmount", "PartialSignaturesRequest", "DepositTxSignatureRequest",
             "PublishDepositTxRequest", "SubscribeTxConfirmationStatusRequest"
         ])
+        .serde_serialized_type("ContractualTxIds", &[
+            rev_hex("depositTxId"), rev_hex("buyersWarningTxId"), rev_hex("sellersWarningTxId"),
+            rev_hex("buyersRedirectTxId"), rev_hex("sellersRedirectTxId")
+        ])
         .serde_serialized_type("PubKeySharesRequest", &[
             enum_field("myRole", "Role")
         ])

rpc/src/main/java/bisq/TradeProtocolClient.java

@@ -128,7 +128,8 @@ private void setupTakerIsBuyerTrade(String buyerTradeId, String sellerTradeId) {
 
         var sellerDepositPsbt = stub.signDepositTx(DepositTxSignatureRequest.newBuilder()
                 .setTradeId(sellerTradeId)
-                .setPeersPartialSignatures(buyerPartialSignatureMessage)
+                // Don't include contract-forming (Deposit/Warning/Redirect) txids, as they are ignored:
+                .setPeersPartialSignatures(buyerPartialSignatureMessage.toBuilder().clearContractualTxIds())
                 .build());
         System.out.println("Got reply: " + sellerDepositPsbt);
 
@@ -141,7 +142,8 @@ private void setupTakerIsBuyerTrade(String buyerTradeId, String sellerTradeId) {
 
         var buyerDepositPsbt = stub.signDepositTx(DepositTxSignatureRequest.newBuilder()
                 .setTradeId(buyerTradeId)
-                .setPeersPartialSignatures(sellerPartialSignatureMessage)
+                // Don't include contract-forming (Deposit/Warning/Redirect) txids, as they are ignored:
+                .setPeersPartialSignatures(sellerPartialSignatureMessage.toBuilder().clearContractualTxIds())
                 .build());
         System.out.println("Got reply: " + buyerDepositPsbt);
 
@@ -219,7 +221,8 @@ private void setupTakerIsSellerTrade(String buyerTradeId, String sellerTradeId)
 
         var buyerDepositPsbt = stub.signDepositTx(DepositTxSignatureRequest.newBuilder()
                 .setTradeId(buyerTradeId)
-                .setPeersPartialSignatures(sellerPartialSignatureMessage)
+                // Don't include contract-forming (Deposit/Warning/Redirect) txids, as they are ignored:
+                .setPeersPartialSignatures(sellerPartialSignatureMessage.toBuilder().clearContractualTxIds())
                 .build());
         System.out.println("Got reply: " + buyerDepositPsbt);
 
@@ -232,7 +235,8 @@ private void setupTakerIsSellerTrade(String buyerTradeId, String sellerTradeId)
 
         var sellerDepositPsbt = stub.signDepositTx(DepositTxSignatureRequest.newBuilder()
                 .setTradeId(sellerTradeId)
-                .setPeersPartialSignatures(buyerPartialSignatureMessage)
+                // Don't include contract-forming (Deposit/Warning/Redirect) txids, as they are ignored:
+                .setPeersPartialSignatures(buyerPartialSignatureMessage.toBuilder().clearContractualTxIds())
                 .build());
         System.out.println("Got reply: " + sellerDepositPsbt);