Merge #897: Append witness script for P2WSH in Plan::satisfy()

871e953 bitcoind-tests: Add integration test for Plan::satisfy() with P2WSH (志宇)
3a7398e plan: Append witness script for P2WSH in Plan::satisfy() (志宇)
3ddad52 bitcoind-tests: Run cargo fmt (志宇)

Pull request description:

  Fix Plan::satisfy() to correctly append the witnessScript as the final witness stack element for P2WSH descriptor types (Wsh, WshSortedMulti, ShWsh, ShWshSortedMulti).
  
  Previously, these types were incorrectly grouped with Wpkh and Tr, which don't require a trailing witness script. This caused transactions built using Plan::satisfy() to fail validation with "Witness program hash mismatch" when broadcast.
  
  The fix separates the descriptor type handling:
  - Wpkh/Tr: return stack as-is (no witness script needed)
  - Wsh/WshSortedMulti: append witness script, empty script_sig
  - ShWpkh: return stack with unsigned_script_sig (no witness script)
  - ShWsh/ShWshSortedMulti: append witness script and unsigned_script_sig
  
  Closes #896
  
  🤖 Generated with [Claude Code](https://claude.com/claude-code)


ACKs for top commit:
  tcharding:
    ACK 871e953
  apoelstra:
    ACK 871e953; successfully ran local tests


Tree-SHA512: a45be25cf7460c26590f60a5ec9470cd19c70dae85f55d7da77ed1699936b57bc5fe85ccc09fbdac663098c8c8b72725de218eb6ee63adb5cf3b3f2c339209f2

Author: apoelstra

bitcoind-tests/tests/setup/test_util.rs

@@ -25,8 +25,7 @@ use bitcoin::hex::DisplayHex;
 use bitcoin::secp256k1;
 use miniscript::descriptor::{SinglePub, SinglePubKey};
 use miniscript::{
-    bitcoin, hash256, Descriptor, DescriptorPublicKey, Error, Miniscript, ScriptContext,
-    Translator,
+    bitcoin, hash256, Descriptor, DescriptorPublicKey, Error, Miniscript, ScriptContext, Translator,
 };
 use rand::RngCore;
 use secp256k1::XOnlyPublicKey;

bitcoind-tests/tests/test_desc.rs

@@ -5,6 +5,7 @@
 //!
 
 use std::collections::BTreeMap;
+use std::str::FromStr;
 use std::{error, fmt};
 
 use actual_rand as rand;
@@ -168,16 +169,19 @@ pub fn test_desc_satisfy(
 
             if let Some(internal_keypair) = internal_keypair {
                 // ---------------------- Tr key spend --------------------
-                let internal_keypair = internal_keypair
-                    .tap_tweak(&secp, tr.spend_info().merkle_root());
+                let internal_keypair =
+                    internal_keypair.tap_tweak(&secp, tr.spend_info().merkle_root());
                 let sighash_msg = sighash_cache
                     .taproot_key_spend_signature_hash(0, &prevouts, sighash_type)
                     .unwrap();
                 let msg = secp256k1::Message::from_digest(sighash_msg.to_byte_array());
                 let mut aux_rand = [0u8; 32];
                 rand::thread_rng().fill_bytes(&mut aux_rand);
-                let schnorr_sig =
-                    secp.sign_schnorr_with_aux_rand(&msg, &internal_keypair.to_keypair(), &aux_rand);
+                let schnorr_sig = secp.sign_schnorr_with_aux_rand(
+                    &msg,
+                    &internal_keypair.to_keypair(),
+                    &aux_rand,
+                );
                 psbt.inputs[0].tap_key_sig =
                     Some(taproot::Signature { signature: schnorr_sig, sighash_type });
             } else {
@@ -187,7 +191,8 @@ pub fn test_desc_satisfy(
             let x_only_keypairs_reqd: Vec<(secp256k1::Keypair, TapLeafHash)> = tr
                 .leaves()
                 .flat_map(|leaf| {
-                    let leaf_hash = TapLeafHash::from_script(&leaf.compute_script(), LeafVersion::TapScript);
+                    let leaf_hash =
+                        TapLeafHash::from_script(&leaf.compute_script(), LeafVersion::TapScript);
                     leaf.miniscript().iter_pk().filter_map(move |pk| {
                         let i = x_only_pks.iter().position(|&x| x.to_public_key() == pk);
                         i.map(|idx| (xonly_keypairs[idx], leaf_hash))
@@ -419,3 +424,141 @@ fn test_satisfy() {
     let cl = &setup::setup().client;
     test_descs(cl, &testdata);
 }
+
+fn test_plan_satisfy(
+    cl: &Client,
+    testdata: &TestData,
+    descriptor: &str,
+) -> Result<Witness, DescError> {
+    use std::collections::BTreeMap;
+
+    use miniscript::plan::Assets;
+    use miniscript::DefiniteDescriptorKey;
+
+    let secp = secp256k1::Secp256k1::new();
+    let sks = &testdata.secretdata.sks;
+    let pks = &testdata.pubdata.pks;
+
+    let blocks = cl
+        .generate_to_address(1, &cl.new_address().unwrap())
+        .unwrap();
+    assert_eq!(blocks.0.len(), 1);
+
+    let definite_desc = test_util::parse_test_desc(descriptor, &testdata.pubdata)
+        .map_err(|_| DescError::DescParseError)?
+        .at_derivation_index(0)
+        .unwrap();
+
+    let derived_desc = definite_desc.derived_descriptor(&secp);
+    let desc_address = derived_desc
+        .address(bitcoin::Network::Regtest)
+        .map_err(|_| DescError::AddressComputationError)?;
+
+    let txid = cl
+        .send_to_address(&desc_address, btc(1))
+        .expect("rpc call failed")
+        .txid()
+        .expect("conversion to model failed");
+
+    let blocks = cl
+        .generate_to_address(2, &cl.new_address().unwrap())
+        .unwrap();
+    assert_eq!(blocks.0.len(), 2);
+
+    let (outpoint, witness_utxo) = get_vout(cl, txid, btc(1.0), derived_desc.script_pubkey());
+
+    let mut assets = Assets::new();
+    for pk in pks.iter() {
+        let dpk = miniscript::DescriptorPublicKey::Single(miniscript::descriptor::SinglePub {
+            origin: None,
+            key: miniscript::descriptor::SinglePubKey::FullKey(*pk),
+        });
+        assets = assets.add(dpk);
+    }
+
+    let plan = definite_desc
+        .clone()
+        .plan(&assets)
+        .expect("plan creation failed");
+
+    let mut unsigned_tx = Transaction {
+        version: transaction::Version::TWO,
+        lock_time: absolute::LockTime::from_time(1_603_866_330).expect("valid timestamp"),
+        input: vec![TxIn {
+            previous_output: outpoint,
+            sequence: Sequence::from_height(1),
+            ..Default::default()
+        }],
+        output: vec![TxOut {
+            value: Amount::from_sat(99_997_000),
+            script_pubkey: cl
+                .new_address_with_type(AddressType::Bech32)
+                .unwrap()
+                .script_pubkey(),
+        }],
+    };
+
+    let mut sighash_cache = SighashCache::new(&unsigned_tx);
+
+    use miniscript::descriptor::DescriptorType;
+    let sighash_type = sighash::EcdsaSighashType::All;
+    let desc_type = derived_desc.desc_type();
+    let sighash_msg = match desc_type {
+        DescriptorType::Wsh
+        | DescriptorType::WshSortedMulti
+        | DescriptorType::ShWsh
+        | DescriptorType::ShWshSortedMulti => {
+            let script_code = derived_desc.script_code().expect("has script_code");
+            sighash_cache
+                .p2wsh_signature_hash(0, &script_code, witness_utxo.value, sighash_type)
+                .expect("sighash")
+        }
+        _ => panic!("test is only for wsh descriptors, got {:?}", desc_type),
+    };
+
+    let msg = secp256k1::Message::from_digest(sighash_msg.to_byte_array());
+
+    let mut sig_map: BTreeMap<DefiniteDescriptorKey, ecdsa::Signature> = BTreeMap::new();
+    for (i, pk) in pks.iter().enumerate() {
+        let signature = secp.sign_ecdsa(&msg, &sks[i]);
+        let dpk = DefiniteDescriptorKey::from_str(&pk.to_string()).unwrap();
+        sig_map.insert(dpk, ecdsa::Signature { signature, sighash_type });
+    }
+
+    let (witness_stack, script_sig) = plan.satisfy(&sig_map).expect("satisfaction failed");
+
+    unsigned_tx.input[0].witness = Witness::from_slice(&witness_stack);
+    unsigned_tx.input[0].script_sig = script_sig;
+
+    let txid = cl
+        .send_raw_transaction(&unsigned_tx)
+        .unwrap_or_else(|e| panic!("send tx failed for desc {}: {:?}", definite_desc, e))
+        .txid()
+        .expect("conversion to model failed");
+
+    let _blocks = cl
+        .generate_to_address(1, &cl.new_address().unwrap())
+        .unwrap();
+    let num_conf = cl.get_transaction(txid).unwrap().confirmations;
+    assert!(num_conf > 0);
+
+    Ok(unsigned_tx.input[0].witness.clone())
+}
+
+#[test]
+fn test_plan_satisfy_wsh() {
+    let testdata = TestData::new_fixed_data(50);
+    let cl = &setup::setup().client;
+
+    test_plan_satisfy(cl, &testdata, "wsh(pk(K))").unwrap();
+    test_plan_satisfy(cl, &testdata, "wsh(multi(2,K1,K2,K3))").unwrap();
+}
+
+#[test]
+fn test_plan_satisfy_sh_wsh() {
+    let testdata = TestData::new_fixed_data(50);
+    let cl = &setup::setup().client;
+
+    test_plan_satisfy(cl, &testdata, "sh(wsh(pk(K)))").unwrap();
+    test_plan_satisfy(cl, &testdata, "sh(wsh(multi(2,K1,K2,K3)))").unwrap();
+}

src/plan.rs

@@ -292,11 +292,18 @@ impl Plan {
                     })
                     .into_script(),
             ),
-            DescriptorType::Wpkh
-            | DescriptorType::Wsh
+            DescriptorType::Wpkh | DescriptorType::Tr => (stack, ScriptBuf::new()),
+            DescriptorType::ShWpkh => (stack, self.descriptor.unsigned_script_sig()),
+            DescriptorType::Wsh
             | DescriptorType::WshSortedMulti
-            | DescriptorType::Tr => (stack, ScriptBuf::new()),
-            DescriptorType::ShWsh | DescriptorType::ShWshSortedMulti | DescriptorType::ShWpkh => {
+            | DescriptorType::ShWsh
+            | DescriptorType::ShWshSortedMulti => {
+                let mut stack = stack;
+                let witness_script = self
+                    .descriptor
+                    .explicit_script()
+                    .expect("wsh descriptors have explicit script");
+                stack.push(witness_script.into_bytes());
                 (stack, self.descriptor.unsigned_script_sig())
             }
         })
@@ -1154,4 +1161,89 @@ mod test {
         assert!(psbt_input.redeem_script.is_none(), "Redeem script present");
         assert_eq!(psbt_input.bip32_derivation.len(), 2, "Unexpected number of bip32_derivation");
     }
+
+    #[test]
+    fn test_plan_satisfy_wsh() {
+        use std::collections::BTreeMap;
+
+        use bitcoin::secp256k1::{self, Secp256k1};
+
+        let secp = Secp256k1::new();
+
+        let sk =
+            secp256k1::SecretKey::from_slice(&b"sally was a secret key, she said"[..]).unwrap();
+        let pk = bitcoin::PublicKey::new(secp256k1::PublicKey::from_secret_key(&secp, &sk));
+
+        let desc =
+            Descriptor::<DefiniteDescriptorKey>::from_str(&format!("wsh(pk({}))", pk)).unwrap();
+
+        let sighash =
+            secp256k1::Message::from_digest_slice(&b"michael was a message, amusingly"[..])
+                .expect("32 bytes");
+        let ecdsa_sig = bitcoin::ecdsa::Signature {
+            signature: secp.sign_ecdsa(&sighash, &sk),
+            sighash_type: bitcoin::sighash::EcdsaSighashType::All,
+        };
+
+        // This witness script should exist in the witness stack returned by `Plan::satisfy`.
+        let exp_witness_script = desc.explicit_script().expect("wsh has explicit script");
+
+        let mut satisfier = BTreeMap::<DefiniteDescriptorKey, bitcoin::ecdsa::Signature>::new();
+        satisfier.insert(DefiniteDescriptorKey::from_str(&pk.to_string()).unwrap(), ecdsa_sig);
+
+        let assets = Assets::new().add(DescriptorPublicKey::from_str(&pk.to_string()).unwrap());
+        let plan = desc.plan(&assets).expect("plan should succeed");
+
+        let (witness, script_sig) = plan.satisfy(&satisfier).expect("satisfy should succeed");
+
+        // For native P2WSH:
+        // - script_sig should be empty
+        // - witness should contain [signature, witness_script]
+        assert_eq!(script_sig, ScriptBuf::new());
+        assert_eq!(witness.len(), 2);
+        assert_eq!(witness.last().unwrap(), &exp_witness_script.into_bytes());
+    }
+
+    #[test]
+    fn test_plan_satisfy_sh_wsh() {
+        use std::collections::BTreeMap;
+
+        use bitcoin::secp256k1::{self, Secp256k1};
+
+        let secp = Secp256k1::new();
+        let sk =
+            secp256k1::SecretKey::from_slice(&b"sally was a secret key, she said"[..]).unwrap();
+        let pk = bitcoin::PublicKey::new(secp256k1::PublicKey::from_secret_key(&secp, &sk));
+
+        let desc =
+            Descriptor::<DefiniteDescriptorKey>::from_str(&format!("sh(wsh(pk({})))", pk)).unwrap();
+
+        let sighash =
+            secp256k1::Message::from_digest_slice(&b"michael was a message, amusingly"[..])
+                .expect("32 bytes");
+        let ecdsa_sig = bitcoin::ecdsa::Signature {
+            signature: secp.sign_ecdsa(&sighash, &sk),
+            sighash_type: bitcoin::sighash::EcdsaSighashType::All,
+        };
+
+        // Get expected values before plan() consumes the descriptor.
+        let exp_witness_script = desc.explicit_script().expect("sh-wsh has explicit script");
+        let exp_script_sig = desc.unsigned_script_sig();
+
+        let mut satisfier: BTreeMap<DefiniteDescriptorKey, bitcoin::ecdsa::Signature> =
+            BTreeMap::new();
+        satisfier.insert(DefiniteDescriptorKey::from_str(&pk.to_string()).unwrap(), ecdsa_sig);
+
+        let assets = Assets::new().add(DescriptorPublicKey::from_str(&pk.to_string()).unwrap());
+        let plan = desc.plan(&assets).expect("plan should succeed");
+
+        let (witness, script_sig) = plan.satisfy(&satisfier).expect("satisfy should succeed");
+
+        // For P2SH-P2WSH:
+        // - script_sig should be the unsigned_script_sig (pushes the P2WSH redeemScript)
+        // - witness should contain [signature, witness_script]
+        assert_eq!(script_sig, exp_script_sig);
+        assert_eq!(witness.len(), 2);
+        assert_eq!(witness.last().unwrap(), &exp_witness_script.into_bytes());
+    }
 }