Malice 2 Situation.

Maker broadcasts contract txs prematurely.

README.md

@@ -97,7 +97,7 @@ If you're interested in contributing to the project, explore the [open issues](h
   - [x] Case2: Maker drops at `ContractSigsForRecvr` after broadcasting funding txs. Taker and other Makers recover. Maker gets banned.
   - [x] Case3L Maker Drops at `HashPreimage` message and doesn't respond back with privkeys. Taker and other Maker recovers. Maker gets banned.
 - [x] Malice 1: Taker broadcasts contract immaturely. Other Makers identify this, get their funds back via contract tx.
-- [ ] Malice 2: One of the Makers broadcast contract immaturely. The Taker identify this, bans the Maker's fidelity bond, other Makers get back funds via contract tx.
+- [x] Malice 2: One of the Makers broadcast contract immaturely. The Taker identify this, bans the Maker's fidelity bond, other Makers get back funds via contract tx.
 - [x] Fix all clippy warnings.
 - [ ] Achieve >80% test coverage, including bad and recovery paths in integration tests.
 - [ ] Switch to binary encoding for wallet data storage and network messages.

src/maker/api.rs

@@ -42,6 +42,7 @@ pub enum MakerBehavior {
     CloseAtContractSigsForRecvrAndSender,
     CloseAtContractSigsForRecvr,
     CloseAtHashPreimage,
+    BroadcastContractAfterSetup,
 }
 /// A structure denoting expectation of type of taker message.
 /// Used in the [ConnectionState] structure.

src/maker/handlers.rs

@@ -7,9 +7,13 @@ use bitcoin::{
 };
 use bitcoind::bitcoincore_rpc::RpcApi;
 
-use crate::protocol::{
-    messages::{MultisigPrivkey, PrivKeyHandover},
-    Hash160,
+use crate::{
+    maker::api::recover_from_swap,
+    protocol::{
+        messages::{MultisigPrivkey, PrivKeyHandover},
+        Hash160,
+    },
+    wallet::WalletSwapCoin,
 };
 
 use crate::{
@@ -133,7 +137,14 @@ pub async fn handle_message(
                     maker
                         .handle_contract_sigs_for_recvr_and_sender(connection_state, message, ip)
                         .await?;
-                    None
+                    if let MakerBehavior::BroadcastContractAfterSetup = maker.behavior {
+                        unexpected_recovery(maker.clone())?;
+                        return Err(MakerError::General(
+                            "Special Maker Behavior BroadcastContractAfterSetup",
+                        ));
+                    } else {
+                        None
+                    }
                 }
                 _ => {
                     return Err(MakerError::General(
@@ -615,3 +626,40 @@ impl Maker {
         Ok(())
     }
 }
+
+fn unexpected_recovery(maker: Arc<Maker>) -> Result<(), MakerError> {
+    let mut lock_on_state = maker.connection_state.lock()?;
+    for (_, (state, _)) in lock_on_state.iter_mut() {
+        let mut outgoings = Vec::new();
+        let mut incomings = Vec::new();
+        // Extract Incoming and Outgoing contracts, and timelock spends of the contract transactions.
+        // fully signed.
+        for (og_sc, ic_sc) in state
+            .outgoing_swapcoins
+            .iter()
+            .zip(state.incoming_swapcoins.iter())
+        {
+            let contract_timelock = og_sc.get_timelock();
+            let contract = og_sc.get_fully_signed_contract_tx()?;
+            let next_internal_address = &maker
+                .wallet
+                .read()
+                .unwrap()
+                .get_next_internal_addresses(1)
+                .unwrap()[0];
+            let time_lock_spend = og_sc.create_timelock_spend(next_internal_address);
+            outgoings.push((
+                (og_sc.get_multisig_redeemscript(), contract),
+                (contract_timelock, time_lock_spend),
+            ));
+            let incoming_contract = ic_sc.get_fully_signed_contract_tx().unwrap();
+            incomings.push((ic_sc.get_multisig_redeemscript(), incoming_contract));
+        }
+        // Spawn a separate thread to wait for contract maturity and broadcasting timelocked.
+        let maker_clone = maker.clone();
+        std::thread::spawn(move || {
+            recover_from_swap(maker_clone, outgoings, incomings);
+        });
+    }
+    Ok(())
+}

tests/malice2.rs

@@ -0,0 +1,155 @@
+#![cfg(feature = "integration-test")]
+use bitcoin::Amount;
+use coinswap::{
+    maker::{start_maker_server, MakerBehavior},
+    taker::{SwapParams, TakerBehavior},
+    test_framework::*,
+};
+use std::{collections::BTreeSet, thread, time::Duration};
+
+/// Malice 2: Maker Broadcasts contract transactions prematurely.
+///
+/// The Taker and other Makers identify the situation and gets their money back via contract txs. This is
+/// a potential DOS on other Makers. But the attacker Maker would loose money too in the process.
+///
+/// This case is hard to "blame". As the contract transactions is available to both the Makers, its not identifiable
+/// which Maker is the culrpit. This requires more protocol level considerations.
+#[tokio::test]
+async fn malice2_maker_broadcast_contract_prematurely() {
+    // ---- Setup ----
+
+    let makers_config_map = [
+        (6102, MakerBehavior::Normal),
+        (16102, MakerBehavior::BroadcastContractAfterSetup),
+    ];
+
+    // Initiate test framework, Makers.
+    // Taker has normal behavior.
+    let (test_framework, taker, makers) =
+        TestFramework::init(None, makers_config_map.into(), Some(TakerBehavior::Normal)).await;
+
+    // Fund the Taker and Makers with 3 utxos of 0.05 btc each.
+    for _ in 0..3 {
+        let taker_address = taker
+            .write()
+            .unwrap()
+            .get_wallet_mut()
+            .get_next_external_address()
+            .unwrap();
+        test_framework.send_to_address(&taker_address, Amount::from_btc(0.05).unwrap());
+        makers.iter().for_each(|maker| {
+            let maker_addrs = maker
+                .get_wallet()
+                .write()
+                .unwrap()
+                .get_next_external_address()
+                .unwrap();
+            test_framework.send_to_address(&maker_addrs, Amount::from_btc(0.05).unwrap());
+        })
+    }
+
+    // confirm balances
+    test_framework.generate_1_block();
+
+    let org_maker_balances = makers
+        .iter()
+        .map(|maker| {
+            maker
+                .get_wallet()
+                .read()
+                .unwrap()
+                .balance(false, false)
+                .unwrap()
+        })
+        .collect::<BTreeSet<_>>();
+
+    let org_take_balance = taker
+        .read()
+        .unwrap()
+        .get_wallet()
+        .balance(false, false)
+        .unwrap();
+
+    // ---- Start Servers and attempt Swap ----
+
+    // Start the Maker server threads
+    let maker_threads = makers
+        .iter()
+        .map(|maker| {
+            let maker_clone = maker.clone();
+            thread::spawn(move || {
+                start_maker_server(maker_clone).unwrap();
+            })
+        })
+        .collect::<Vec<_>>();
+
+    // Start swap
+    thread::sleep(Duration::from_secs(20)); // Take a delay because Makers take time to fully setup.
+    let swap_params = SwapParams {
+        send_amount: 500000,
+        maker_count: 2,
+        tx_count: 3,
+        required_confirms: 1,
+        fee_rate: 1000,
+    };
+
+    // Spawn a Taker coinswap thread.
+    let taker_clone = taker.clone();
+    let taker_thread = thread::spawn(move || {
+        taker_clone
+            .write()
+            .unwrap()
+            .send_coinswap(swap_params)
+            .unwrap();
+    });
+
+    // Wait for Taker swap thread to conclude.
+    taker_thread.join().unwrap();
+
+    // Wait for Maker threads to conclude.
+    makers.iter().for_each(|maker| maker.shutdown().unwrap());
+    maker_threads
+        .into_iter()
+        .for_each(|thread| thread.join().unwrap());
+
+    // ---- After Swap checks ----
+    let maker_balances = makers
+        .iter()
+        .map(|maker| {
+            maker
+                .get_wallet()
+                .read()
+                .unwrap()
+                .balance(false, false)
+                .unwrap()
+        })
+        .collect::<BTreeSet<_>>();
+
+    let taker_balance = taker
+        .read()
+        .unwrap()
+        .get_wallet()
+        .balance(false, false)
+        .unwrap();
+
+    assert_eq!(maker_balances.first().unwrap(), &Amount::from_sat(14995773));
+
+    // Everybody looses 4227 sats for contract transactions.
+    assert_eq!(
+        org_maker_balances
+            .first()
+            .unwrap()
+            .checked_sub(*maker_balances.first().unwrap())
+            .unwrap(),
+        Amount::from_sat(4227)
+    );
+    assert_eq!(
+        org_take_balance.checked_sub(taker_balance).unwrap(),
+        Amount::from_sat(4227)
+    );
+
+    // Stop test and clean everything.
+    // comment this line if you want the wallet directory and bitcoind to live. Can be useful for
+    // after test debugging.
+    test_framework.stop();
+}