listener: expect and treat all the Cancel transactions

darosior · darosior · commit 1a1a31baba62 · 2022-05-06T19:22:41.000+02:00
diff --git a/contrib/tools/txbuilder/src/main.rs b/contrib/tools/txbuilder/src/main.rs
@@ -1,5 +1,6 @@
 use std::{collections::BTreeMap, env, process, str::FromStr};
 
+use revault_net::message::watchtower::CancelFeerate;
 use revault_tx::{
     bitcoin::{
         consensus::encode::serialize_hex, secp256k1, util::bip32, Address, Amount, OutPoint,
@@ -190,7 +191,6 @@ fn main() {
             eprintln!("Failed to derive transaction chain: '{}'", e);
             process::exit(1);
         });
-    let mut cancel_tx = cancel_batch.into_feerate_20();
     let der_unvault_desc = unvault_desc.derive(derivation_index.into(), &secp);
     let unvault_txin = unvault_tx.spend_unvault_txin(&der_unvault_desc);
     let spend_txo = revault_tx::txouts::SpendTxOut::new(TxOut {
@@ -222,7 +222,15 @@ fn main() {
         })
         .collect();
     let unvault_sigs = sign(&mut unvault_tx, &stk_privkeys, &secp);
-    let cancel_sigs = sign(&mut cancel_tx, &stk_privkeys, &secp);
+    let mut cancel_sigs = BTreeMap::new();
+    let mut cancel_txs = BTreeMap::new();
+    for (feerate, mut cancel_tx) in cancel_batch.feerates_map() {
+        cancel_sigs.insert(
+            CancelFeerate(feerate),
+            sign(&mut cancel_tx, &stk_privkeys, &secp),
+        );
+        cancel_txs.insert(CancelFeerate(feerate), serialize_hex(&cancel_tx.into_tx()));
+    }
     let emer_sigs = sign(&mut emer_tx, &stk_privkeys, &secp);
     let unemer_sigs = sign(&mut unemer_tx, &stk_privkeys, &secp);
     let spend_privkeys: Vec<secp256k1::SecretKey> = man_xprivs
@@ -246,7 +254,7 @@ fn main() {
                 "sigs": unvault_sigs,
             }),
             "cancel": serde_json::json!({
-                "tx": serialize_hex(&cancel_tx.into_tx()),
+                "tx": cancel_txs,
                 "sigs": cancel_sigs,
             }),
             "emer": serde_json::json!({
diff --git a/src/listener.rs b/src/listener.rs
@@ -6,7 +6,7 @@ use crate::{
 
 use revault_net::{
     message::{
-        watchtower::{Signatures, Sigs, SigsResult},
+        watchtower::{CancelFeerate, Signatures, Sigs, SigsResult},
         RequestParams, ResponseResult,
     },
     noise::SecretKey as NoisePrivkey,
@@ -16,7 +16,7 @@ use revault_tx::{
     transactions::{transaction_chain, RevaultPresignedTransaction, RevaultTransaction},
 };
 
-use std::{collections::BTreeMap, io, net::TcpListener, path, sync};
+use std::{io, net::TcpListener, path, sync};
 
 #[derive(Debug)]
 pub enum ListenerError {
@@ -92,7 +92,6 @@ fn process_sigs_message<C: secp256k1::Verification>(
         scripts_config.emergency_address.clone(),
         secp,
     )?;
-    let mut cancel_tx = cancel_batch.into_feerate_20();
 
     // If we already have it, just acknowledge it.
     if db_vault(db_path, &msg.deposit_outpoint)?.is_some() {
@@ -103,7 +102,8 @@ fn process_sigs_message<C: secp256k1::Verification>(
         return Ok(SigsResult { ack: true });
     }
 
-    // Otherwise, check the sigs they gave us are valid
+    // Otherwise, check the sigs they gave us are valid and they provided all the expected feerates
+    // for the Cancel transaction.
     let Sigs {
         signatures:
             Signatures {
@@ -113,19 +113,27 @@ fn process_sigs_message<C: secp256k1::Verification>(
             },
         ..
     } = msg;
-    let cancel_sigs = cancel
-        .values()
-        .cloned()
-        .next()
-        .unwrap_or_else(|| BTreeMap::new()); // FIXME
+
     for (key, sig) in emergency.iter() {
         emer_tx.add_sig(*key, *sig, secp)?;
     }
     emer_tx.finalize(secp)?;
-    for (key, sig) in cancel_sigs.iter() {
-        cancel_tx.add_sig(*key, *sig, secp)?;
+
+    for (feerate, mut cancel_tx) in cancel_batch.feerates_map() {
+        if let Some(sigs) = cancel.get(&CancelFeerate(feerate)) {
+            for (key, sig) in sigs {
+                cancel_tx.add_sig(*key, *sig, secp)?;
+            }
+            cancel_tx.finalize(secp)?;
+        } else {
+            eprintln!(
+                "'sig' message is missing Cancel signature at {} sat/vb",
+                feerate
+            );
+            return Ok(SigsResult { ack: false });
+        }
     }
-    cancel_tx.finalize(secp)?;
+
     for (key, sig) in unvault_emergency.iter() {
         unemer_tx.add_sig(*key, *sig, secp)?;
     }
@@ -462,7 +470,7 @@ mod tests {
             &secp_ctx,
         )
         .unwrap();
-        let cancel_tx = cancel_batch.into_feerate_20();
+        let cancel_feerates_map = cancel_batch.feerates_map();
 
         // This starts a dummy server to answer our gettxout requests
         let bitcoind = sync::Arc::from(dummy_bitcoind(deposit_value));
@@ -534,7 +542,7 @@ mod tests {
                 .contains("Invalid signature"),
         );
 
-        // Enough invalid emergency sigs
+        // Enough valid emergency sigs, but not for the right key
         let emergency: BTreeMap<secp256k1::PublicKey, secp256k1::Signature> = stakeholders_priv
             .iter()
             .map(|xpriv| {
@@ -581,24 +589,70 @@ mod tests {
                 })
                 .collect();
 
-        // Now do the same dance with Cancel signatures.
+        // Now do a similar dance with Cancel signatures.
+
+        // Cancel signatures are missing one required feerate. It will NACK it.
+        let mut cancel = BTreeMap::new();
+        for (feerate, cancel_tx) in cancel_feerates_map.iter() {
+            if feerate == &Amount::from_sat(500) {
+                continue;
+            }
+
+            let sighash = cancel_tx.sig_hash().unwrap();
+            let sighash = secp256k1::Message::from_slice(&sighash).unwrap();
+            let cancel_sigs: BTreeMap<secp256k1::PublicKey, secp256k1::Signature> =
+                stakeholders_priv
+                    .iter()
+                    .map(|xpriv| {
+                        let privkey = xpriv.derive_priv(&secp_ctx, &[derivation_index]).unwrap();
+                        (
+                            privkey.private_key.public_key(&secp_ctx).key,
+                            secp_ctx.sign(&sighash, &privkey.private_key.key),
+                        )
+                    })
+                    .collect();
+            cancel.insert(CancelFeerate(*feerate), cancel_sigs);
+        }
+        let unvault_emergency: BTreeMap<secp256k1::PublicKey, secp256k1::Signature> =
+            BTreeMap::new();
+        let signatures = Signatures {
+            emergency: emergency_valid.clone(),
+            cancel,
+            unvault_emergency,
+        };
+        let msg = Sigs {
+            signatures,
+            deposit_outpoint,
+            derivation_index,
+        };
+        assert!(
+            !process_sigs_message(&db_path, &scripts_config, &bitcoind, msg, &secp_ctx)
+                .unwrap()
+                .ack
+        );
 
         // Not enough Cancel signatures
-        let sighash = cancel_tx.sig_hash().unwrap();
-        let sighash = secp256k1::Message::from_slice(&sighash).unwrap();
-        let cancel_sigs: BTreeMap<secp256k1::PublicKey, secp256k1::Signature> = stakeholders_priv
-            [..2]
-            .iter()
-            .map(|xpriv| {
-                let privkey = xpriv.derive_priv(&secp_ctx, &[derivation_index]).unwrap();
-                (
-                    privkey.private_key.public_key(&secp_ctx).key,
-                    secp_ctx.sign(&sighash, &privkey.private_key.key),
-                )
-            })
-            .collect();
-        let cancel =
-            IntoIterator::into_iter([(CancelFeerate(Amount::from_sat(20)), cancel_sigs)]).collect();
+        let mut cancel = BTreeMap::new();
+        for (feerate, cancel_tx) in cancel_feerates_map.iter() {
+            let sighash = cancel_tx.sig_hash().unwrap();
+            let sighash = secp256k1::Message::from_slice(&sighash).unwrap();
+            let privs = if feerate == &Amount::from_sat(20) {
+                &stakeholders_priv[..2]
+            } else {
+                &stakeholders_priv
+            };
+            let cancel_sigs: BTreeMap<secp256k1::PublicKey, secp256k1::Signature> = privs
+                .iter()
+                .map(|xpriv| {
+                    let privkey = xpriv.derive_priv(&secp_ctx, &[derivation_index]).unwrap();
+                    (
+                        privkey.private_key.public_key(&secp_ctx).key,
+                        secp_ctx.sign(&sighash, &privkey.private_key.key),
+                    )
+                })
+                .collect();
+            cancel.insert(CancelFeerate(*feerate), cancel_sigs);
+        }
         let unvault_emergency: BTreeMap<secp256k1::PublicKey, secp256k1::Signature> =
             BTreeMap::new();
         let signatures = Signatures {
@@ -618,21 +672,24 @@ mod tests {
                 .contains("Revault transaction finalisation error"),
         );
 
-        // Enough Cancel sigs, but invalid signature
-        let bad_sighash = [0; 32];
-        let bad_sighash = secp256k1::Message::from_slice(&bad_sighash).unwrap();
-        let cancel_sigs: BTreeMap<secp256k1::PublicKey, secp256k1::Signature> = stakeholders_priv
-            .iter()
-            .map(|xpriv| {
-                let privkey = xpriv.derive_priv(&secp_ctx, &[derivation_index]).unwrap();
-                (
-                    privkey.private_key.public_key(&secp_ctx).key,
-                    secp_ctx.sign(&bad_sighash, &privkey.private_key.key),
-                )
-            })
-            .collect();
-        let cancel =
-            IntoIterator::into_iter([(CancelFeerate(Amount::from_sat(20)), cancel_sigs)]).collect();
+        // Enough Cancel sigs, but invalid signatures
+        let mut cancel = BTreeMap::new();
+        let sighash = [0; 32];
+        let sighash = secp256k1::Message::from_slice(&sighash).unwrap();
+        for (feerate, _) in cancel_feerates_map.iter() {
+            let cancel_sigs: BTreeMap<secp256k1::PublicKey, secp256k1::Signature> =
+                stakeholders_priv
+                    .iter()
+                    .map(|xpriv| {
+                        let privkey = xpriv.derive_priv(&secp_ctx, &[derivation_index]).unwrap();
+                        (
+                            privkey.private_key.public_key(&secp_ctx).key,
+                            secp_ctx.sign(&sighash, &privkey.private_key.key),
+                        )
+                    })
+                    .collect();
+            cancel.insert(CancelFeerate(*feerate), cancel_sigs);
+        }
         let unvault_emergency: BTreeMap<secp256k1::PublicKey, secp256k1::Signature> =
             BTreeMap::new();
         let signatures = Signatures {
@@ -652,22 +709,26 @@ mod tests {
                 .contains("Invalid signature")
         );
 
-        // Enough invalid Cancel sigs
-        let cancel_sigs: BTreeMap<secp256k1::PublicKey, secp256k1::Signature> = stakeholders_priv
-            .iter()
-            .map(|xpriv| {
-                // Note the derivation index increment
-                let privkey = xpriv
-                    .derive_priv(&secp_ctx, &[derivation_index.increment().unwrap()])
-                    .unwrap();
-                (
-                    privkey.private_key.public_key(&secp_ctx).key,
-                    secp_ctx.sign(&sighash, &privkey.private_key.key),
-                )
-            })
-            .collect();
-        let cancel =
-            IntoIterator::into_iter([(CancelFeerate(Amount::from_sat(20)), cancel_sigs)]).collect();
+        // Enough Cancel sigs, but invalid pubkeys
+        let mut cancel = BTreeMap::new();
+        for (feerate, cancel_tx) in cancel_feerates_map.iter() {
+            let sighash = cancel_tx.sig_hash().unwrap();
+            let sighash = secp256k1::Message::from_slice(&sighash).unwrap();
+            let cancel_sigs: BTreeMap<secp256k1::PublicKey, secp256k1::Signature> =
+                stakeholders_priv
+                    .iter()
+                    .map(|xpriv| {
+                        let privkey = xpriv
+                            .derive_priv(&secp_ctx, &[derivation_index.increment().unwrap()])
+                            .unwrap();
+                        (
+                            privkey.private_key.public_key(&secp_ctx).key,
+                            secp_ctx.sign(&sighash, &privkey.private_key.key),
+                        )
+                    })
+                    .collect();
+            cancel.insert(CancelFeerate(*feerate), cancel_sigs);
+        }
         let unvault_emergency: BTreeMap<secp256k1::PublicKey, secp256k1::Signature> =
             BTreeMap::new();
         let signatures = Signatures {
@@ -688,20 +749,23 @@ mod tests {
         );
 
         // Valid Cancel signatures.
-        let cancel_valid_sigs: BTreeMap<secp256k1::PublicKey, secp256k1::Signature> =
-            stakeholders_priv
-                .iter()
-                .map(|xpriv| {
-                    let privkey = xpriv.derive_priv(&secp_ctx, &[derivation_index]).unwrap();
-                    (
-                        privkey.private_key.public_key(&secp_ctx).key,
-                        secp_ctx.sign(&sighash, &privkey.private_key.key),
-                    )
-                })
-                .collect();
-        let cancel_valid: BTreeMap<_, _> =
-            IntoIterator::into_iter([(CancelFeerate(Amount::from_sat(20)), cancel_valid_sigs)])
-                .collect();
+        let mut cancel_valid = BTreeMap::new();
+        for (feerate, cancel_tx) in cancel_feerates_map.iter() {
+            let sighash = cancel_tx.sig_hash().unwrap();
+            let sighash = secp256k1::Message::from_slice(&sighash).unwrap();
+            let cancel_sigs: BTreeMap<secp256k1::PublicKey, secp256k1::Signature> =
+                stakeholders_priv
+                    .iter()
+                    .map(|xpriv| {
+                        let privkey = xpriv.derive_priv(&secp_ctx, &[derivation_index]).unwrap();
+                        (
+                            privkey.private_key.public_key(&secp_ctx).key,
+                            secp_ctx.sign(&sighash, &privkey.private_key.key),
+                        )
+                    })
+                    .collect();
+            cancel_valid.insert(CancelFeerate(*feerate), cancel_sigs);
+        }
 
         // Now do the same dance with Unvault Emergency signatures.
 
diff --git a/tests/test_chain.py b/tests/test_chain.py
@@ -27,11 +27,11 @@ def test_simple_unvault_broadcast(miradord, bitcoind):
     bitcoind.rpc.sendrawtransaction(txs["unvault"]["tx"])
     bitcoind.generate_block(1, unvault_txid)
 
-    cancel_txid = bitcoind.rpc.decoderawtransaction(txs["cancel"]["tx"])["txid"]
+    cancel_txid = bitcoind.rpc.decoderawtransaction(txs["cancel"]["tx"]["20"])["txid"]
     miradord.wait_for_logs(
         [
             f"Got a confirmed Unvault UTXO for vault at '{deposit_outpoint}'",
-            f"Broadcasted Cancel transaction '{txs['cancel']['tx']}'",
+            f"Broadcasted Cancel transaction '{txs['cancel']['tx']['20']}'",
             f"Unvault transaction '{unvault_txid}' for vault at '{deposit_outpoint}' is still unspent",
         ]
     )
@@ -65,11 +65,11 @@ def test_spent_cancel_detection(miradord, bitcoind):
     unvault_txid = bitcoind.rpc.decoderawtransaction(txs["unvault"]["tx"])["txid"]
     bitcoind.rpc.sendrawtransaction(txs["unvault"]["tx"])
     bitcoind.generate_block(1, unvault_txid)
-    cancel_tx = bitcoind.rpc.decoderawtransaction(txs["cancel"]["tx"])
+    cancel_tx = bitcoind.rpc.decoderawtransaction(txs["cancel"]["tx"]["20"])
     miradord.wait_for_logs(
         [
             f"Got a confirmed Unvault UTXO for vault at '{deposit_outpoint}'",
-            f"Broadcasted Cancel transaction '{txs['cancel']['tx']}'",
+            f"Broadcasted Cancel transaction '{txs['cancel']['tx']['20']}'",
             f"Unvault transaction '{unvault_txid}' for vault at '{deposit_outpoint}' is still unspent",
         ]
     )
diff --git a/tests/test_framework/miradord.py b/tests/test_framework/miradord.py
@@ -215,9 +215,7 @@ def send_sigs(
         params = {
             "signatures": {
                 "emergency": emer_sigs,
-                "cancel": {
-                    "20": cancel_sigs,
-                },
+                "cancel": cancel_sigs,
                 "unvault_emergency": unemer_sigs,
             },
             "deposit_outpoint": deposit_outpoint,
diff --git a/tests/test_plugins.py b/tests/test_plugins.py

Original file line number	Diff line number	Diff line change
`@@ -27,11 +27,11 @@ def test_simple_unvault_broadcast(miradord, bitcoind):`
`27`	`27`	`bitcoind.rpc.sendrawtransaction(txs["unvault"]["tx"])`
`28`	`28`	`bitcoind.generate_block(1, unvault_txid)`
`29`	`29`
`30`		`- cancel_txid = bitcoind.rpc.decoderawtransaction(txs["cancel"]["tx"])["txid"]`
	`30`	`+ cancel_txid = bitcoind.rpc.decoderawtransaction(txs["cancel"]["tx"]["20"])["txid"]`
`31`	`31`	`miradord.wait_for_logs(`
`32`	`32`	`[`
`33`	`33`	`f"Got a confirmed Unvault UTXO for vault at '{deposit_outpoint}'",`
`34`		`- f"Broadcasted Cancel transaction '{txs['cancel']['tx']}'",`
	`34`	`+ f"Broadcasted Cancel transaction '{txs['cancel']['tx']['20']}'",`
`35`	`35`	`f"Unvault transaction '{unvault_txid}' for vault at '{deposit_outpoint}' is still unspent",`
`36`	`36`	`]`
`37`	`37`	`)`
`@@ -65,11 +65,11 @@ def test_spent_cancel_detection(miradord, bitcoind):`
`65`	`65`	`unvault_txid = bitcoind.rpc.decoderawtransaction(txs["unvault"]["tx"])["txid"]`
`66`	`66`	`bitcoind.rpc.sendrawtransaction(txs["unvault"]["tx"])`
`67`	`67`	`bitcoind.generate_block(1, unvault_txid)`
`68`		`- cancel_tx = bitcoind.rpc.decoderawtransaction(txs["cancel"]["tx"])`
	`68`	`+ cancel_tx = bitcoind.rpc.decoderawtransaction(txs["cancel"]["tx"]["20"])`
`69`	`69`	`miradord.wait_for_logs(`
`70`	`70`	`[`
`71`	`71`	`f"Got a confirmed Unvault UTXO for vault at '{deposit_outpoint}'",`
`72`		`- f"Broadcasted Cancel transaction '{txs['cancel']['tx']}'",`
	`72`	`+ f"Broadcasted Cancel transaction '{txs['cancel']['tx']['20']}'",`
`73`	`73`	`f"Unvault transaction '{unvault_txid}' for vault at '{deposit_outpoint}' is still unspent",`
`74`	`74`	`]`
`75`	`75`	`)`