Merging in update_spend_tx.

doc/API.md

@@ -349,7 +349,7 @@ feerate.
 ### `updatespendtx`
 
 The `updatespendtx` RPC Command stores or update the stored Spend transaction with the
-given one.
+given one. The signatures from both the old & the new transactions will be merged.
 
 #### Request
 

src/commands/mod.rs

@@ -78,6 +78,7 @@ pub enum CommandError {
     /// (Got, Expected)
     SpendNotEnoughSig(usize, usize),
     SpendInvalidSig(Vec<u8>),
+    SpendSignaturesUpdate(revault_tx::error::InputSatisfactionError),
     MissingCpfpKey,
     ManagerOnly,
     StakeholderOnly,
@@ -148,6 +149,9 @@ impl fmt::Display for CommandError {
             }
             Self::Race => write!(f, "Internal error due to a race. Please try again."),
             Self::UnknownCancel(txid) => write!(f, "Unknown Cancel transaction: '{}'", txid),
+            Self::SpendSignaturesUpdate(error) => {
+                write!(f, "Error in updating the signature '{}'", error)
+            }
         }
     }
 }
@@ -190,6 +194,7 @@ impl CommandError {
             CommandError::Bitcoind(_) => ErrorCode::BITCOIND_ERROR,
             CommandError::Tx(_) => ErrorCode::INTERNAL_ERROR,
             CommandError::SpendFeerateTooLow(_, _) => ErrorCode::INVALID_PARAMS,
+            CommandError::SpendSignaturesUpdate(_) => ErrorCode::INVALID_PARAMS,
             // TODO: some of these probably need specific error codes
             CommandError::SpendTooLarge
             | CommandError::SpendUnknownUnVault(_)
@@ -1110,15 +1115,32 @@ impl DaemonControl {
 
             db_unvaults.push(db_unvault);
         }
+        let mut old_tx =
+            db_spend_transaction(&db_path, &spend_txid).expect("Database must be available");
+
+        if let Some(unwrapped_tx) = old_tx.as_mut() {
+            // Merging the signatures of `old_tx` into `spend_tx` and saving it.
+
+            let psbt_ins = spend_tx.into_psbt().inputs;
+
+            for (i, input) in psbt_ins.into_iter().enumerate() {
+                for (pubkey, raw_sig) in input.partial_sigs {
+                    let sig = secp256k1::Signature::from_der(&raw_sig[..raw_sig.len() - 1])
+                        .map_err(|_| CommandError::SpendInvalidSig(raw_sig.clone()))?;
+
+                    // i remains in bound since the transaction is taken based on txid.
+                    unwrapped_tx
+                        .psbt
+                        .add_signature(i, pubkey.key, sig, &revaultd.secp_ctx)
+                        .map_err(|e| CommandError::SpendSignaturesUpdate(e))?;
+                }
+            }
 
-        // The user has the ability to set priority to the transaction in
-        // setspendtx, here we always set it to false.
-        if db_spend_transaction(&db_path, &spend_txid)
-            .expect("Database must be available")
-            .is_some()
-        {
             log::debug!("Updating Spend transaction '{}'", spend_txid);
-            db_update_spend(&db_path, &spend_tx, false).expect("Database must be available");
+            // The user has the ability to set priority to the transaction in
+            // setspendtx, here we always set it to false.
+            db_update_spend(&db_path, &unwrapped_tx.psbt, false)
+                .expect("Database must be available");
         } else {
             log::debug!("Storing new Spend transaction '{}'", spend_txid);
             db_insert_spend(&db_path, &db_unvaults, &spend_tx).expect("Database must be available");

tests/test_spend.py

@@ -17,6 +17,61 @@
 )
 
 
+@pytest.mark.skipif(not POSTGRES_IS_SETUP, reason="Needs Postgres for servers db")
+def test_update_spend_tx(revault_network, bitcoind):
+    CSV = 12
+    revault_network.deploy(
+        n_stakeholders=2, n_managers=3, managers_threshold=3, csv=CSV
+    )
+    managers = [revault_network.man(i) for i in range(3)]
+    amount = 0.24
+    vault = revault_network.fund(amount)
+    deposit = f"{vault['txid']}:{vault['vout']}"
+    derivation_indices = [vault["derivation_index"]]
+
+    addr = bitcoind.rpc.getnewaddress()
+    spent_vaults = [deposit]
+    feerate = 2
+    fees = revault_network.compute_spendtx_fees(feerate, len(spent_vaults), 1)
+    destination = {addr: vault["amount"] - fees}
+
+    revault_network.secure_vault(vault)
+    revault_network.activate_vault(vault)
+
+    spend_tx = managers[0].rpc.getspendtx(spent_vaults, destination, feerate)[
+        "spend_tx"
+    ]["psbt"]
+    spend_tx_a = spend_tx
+    spend_tx_b = spend_tx
+
+    # Transaction with only the first signature
+    spend_tx_a = managers[0].man_keychain.sign_spend_psbt(
+        spend_tx_a, derivation_indices
+    )
+
+    # Transaction with other 2 signatures.
+    for man in managers[1:]:
+        spend_tx_b = man.man_keychain.sign_spend_psbt(spend_tx_b, derivation_indices)
+
+    # Storing the transaction with first signature.
+    managers[0].rpc.updatespendtx(spend_tx_a)
+    managers[0].wait_for_log("Storing new Spend transaction")
+
+    # Updating the transaction with the last 2 signatures.
+    managers[0].rpc.updatespendtx(spend_tx_b)
+    managers[0].wait_for_log("Updating Spend transaction")
+
+    spend_txs = managers[0].rpc.listspendtxs()["spend_txs"]
+    assert len(spend_txs) == 1
+
+    serialized_psbt = spend_txs[0]["psbt"]
+    psbt = serializations.PSBT()
+    psbt.deserialize(serialized_psbt)
+
+    # Must have 3 signatures if they are merged.
+    assert len(psbt.inputs[0].partial_sigs) == 3
+
+
 @pytest.mark.skipif(not POSTGRES_IS_SETUP, reason="Needs Postgres for servers db")
 def test_spendtx_management(revault_network, bitcoind):
     CSV = 12