Sign and verify P2SH-P2WPKH (#32)

Author: raphjaph

README.md

@@ -6,14 +6,14 @@ generic message signing and verification.
 
 ## Types of Signatures 
 
-At the moment this crate supports ONLY `P2TR` and `P2WPKH` addresses. We're
-looking to stabilize the interface before implementing different address types.
-Feedback through issues or PRs is welcome and encouraged. 
+At the moment this crate supports `P2TR`, `P2WPKH` and `P2SH-P2WPKH` single-sig
+addresses. Feedback through issues or PRs on the interface design and security
+is welcome and encouraged. 
 
-- [ ] legacy
 - [x] simple
 - [x] full 
 - [ ] full (proof-of-funds)
+- [ ] legacy (BIP-137)
 
 The goal is to provide a full signing and verifying library similar to
 [this](https://github.com/ACken2/bip322-js/tree/main) Javascript library.

src/error.rs

@@ -10,7 +10,7 @@ pub enum Error {
   },
   #[snafu(display("Failed to parse private key"))]
   PrivateKeyParse { source: bitcoin::key::Error },
-  #[snafu(display("Unsuported address `{address}`, only P2TR or P2WPKH allowed"))]
+  #[snafu(display("Unsuported address `{address}`, only P2TR, P2WPKH and P2SH-P2WPKH allowed"))]
   UnsupportedAddress { address: String },
   #[snafu(display("Decode error for signature `{signature}`"))]
   SignatureDecode {

src/lib.rs

@@ -42,6 +42,11 @@ mod tests {
   const WIF_PRIVATE_KEY: &str = "L3VFeEujGtevx9w18HD1fhRbCH67Az2dpCymeRE1SoPK6XQtaN2k";
   const SEGWIT_ADDRESS: &str = "bc1q9vza2e8x573nczrlzms0wvx3gsqjx7vavgkx0l";
   const TAPROOT_ADDRESS: &str = "bc1ppv609nr0vr25u07u95waq5lucwfm6tde4nydujnu8npg4q75mr5sxq8lt3";
+  const LEGACY_ADDRESS: &str = "14vV3aCHBeStb5bkenkNHbe2YAFinYdXgc";
+
+  const NESTED_SEGWIT_WIF_PRIVATE_KEY: &str =
+    "KwTbAxmBXjoZM3bzbXixEr9nxLhyYSM4vp2swet58i19bw9sqk5z";
+  const NESTED_SEGWIT_ADDRESS: &str = "3HSVzEhCFuH9Z3wvoWTexy7BMVVp3PjS6f";
 
   #[test]
   fn message_hashes_are_correct() {
@@ -161,10 +166,10 @@ mod tests {
   #[test]
   fn invalid_address() {
     assert_eq!(verify::verify_simple_encoded(
-      "3B5fQsEXEaV8v6U3ejYc8XaKXAkyQj2MjV",
+      LEGACY_ADDRESS,
       "",
       "AkcwRAIgM2gBAQqvZX15ZiysmKmQpDrG83avLIT492QBzLnQIxYCIBaTpOaD20qRlEylyxFSeEA2ba9YOixpX8z46TSDtS40ASECx/EgAxlkQpQ9hYjgGu6EBCPMVPwVIVJqO4XCsMvViHI=").unwrap_err().to_string(),
-      "Unsuported address `3B5fQsEXEaV8v6U3ejYc8XaKXAkyQj2MjV`, only P2TR or P2WPKH allowed"
+      format!("Unsuported address `{LEGACY_ADDRESS}`, only P2TR, P2WPKH and P2SH-P2WPKH allowed")
     )
   }
 
@@ -274,4 +279,59 @@ mod tests {
     )
     .is_ok());
   }
+
+  #[test]
+  fn simple_verify_and_falsify_p2sh_p2wpkh() {
+    assert!(verify::verify_simple_encoded(
+        NESTED_SEGWIT_ADDRESS,
+        "Hello World",
+        "AkgwRQIhAMd2wZSY3x0V9Kr/NClochoTXcgDaGl3OObOR17yx3QQAiBVWxqNSS+CKen7bmJTG6YfJjsggQ4Fa2RHKgBKrdQQ+gEhAxa5UDdQCHSQHfKQv14ybcYm1C9y6b12xAuukWzSnS+w"
+      ).is_ok()
+    );
+
+    assert!(verify::verify_simple_encoded(
+        NESTED_SEGWIT_ADDRESS,
+        "Hello World - this should fail",
+        "AkgwRQIhAMd2wZSY3x0V9Kr/NClochoTXcgDaGl3OObOR17yx3QQAiBVWxqNSS+CKen7bmJTG6YfJjsggQ4Fa2RHKgBKrdQQ+gEhAxa5UDdQCHSQHfKQv14ybcYm1C9y6b12xAuukWzSnS+w"
+      ).is_err()
+    );
+  }
+
+  #[test]
+  fn simple_sign_p2sh_p2wpkh() {
+    assert_eq!(
+      sign::sign_simple_encoded(NESTED_SEGWIT_ADDRESS, "Hello World", NESTED_SEGWIT_WIF_PRIVATE_KEY).unwrap(),
+      "AkgwRQIhAMd2wZSY3x0V9Kr/NClochoTXcgDaGl3OObOR17yx3QQAiBVWxqNSS+CKen7bmJTG6YfJjsggQ4Fa2RHKgBKrdQQ+gEhAxa5UDdQCHSQHfKQv14ybcYm1C9y6b12xAuukWzSnS+w"
+    );
+  }
+
+  #[test]
+  fn roundtrip_p2sh_p2wpkh_simple() {
+    assert!(verify::verify_simple_encoded(
+      NESTED_SEGWIT_ADDRESS,
+      "Hello World",
+      &sign::sign_simple_encoded(
+        NESTED_SEGWIT_ADDRESS,
+        "Hello World",
+        NESTED_SEGWIT_WIF_PRIVATE_KEY
+      )
+      .unwrap()
+    )
+    .is_ok());
+  }
+
+  #[test]
+  fn roundtrip_p2sh_p2wpkh_full() {
+    assert!(verify::verify_full_encoded(
+      NESTED_SEGWIT_ADDRESS,
+      "Hello World",
+      &sign::sign_full_encoded(
+        NESTED_SEGWIT_ADDRESS,
+        "Hello World",
+        NESTED_SEGWIT_WIF_PRIVATE_KEY
+      )
+      .unwrap()
+    )
+    .is_ok());
+  }
 }

src/sign.rs

@@ -55,8 +55,8 @@ pub fn sign_full(
   let to_spend = create_to_spend(address, message)?;
   let mut to_sign = create_to_sign(&to_spend, None)?;
 
-  let witness =
-    if let bitcoin::address::Payload::WitnessProgram(witness_program) = address.payload() {
+  let witness = match address.payload() {
+    Payload::WitnessProgram(witness_program) => {
       let version = witness_program.version().to_num();
       let program_len = witness_program.program().len();
 
@@ -65,7 +65,7 @@ pub fn sign_full(
           if program_len != 20 {
             return Err(Error::NotKeyPathSpend);
           }
-          create_message_signature_p2wpkh(&to_spend, &to_sign, private_key)
+          create_message_signature_p2wpkh(&to_spend, &to_sign, private_key, false)
         }
         1 => {
           if program_len != 32 {
@@ -79,11 +79,16 @@ pub fn sign_full(
           })
         }
       }
-    } else {
+    }
+    Payload::ScriptHash(_) => {
+      create_message_signature_p2wpkh(&to_spend, &to_sign, private_key, true)
+    }
+    _ => {
       return Err(Error::UnsupportedAddress {
         address: address.to_string(),
       });
-    };
+    }
+  };
 
   to_sign.inputs[0].final_script_witness = Some(witness);
 
@@ -94,15 +99,22 @@ fn create_message_signature_p2wpkh(
   to_spend_tx: &Transaction,
   to_sign: &Psbt,
   private_key: PrivateKey,
+  is_p2sh: bool,
 ) -> Witness {
   let secp = Secp256k1::new();
   let sighash_type = EcdsaSighashType::All;
   let mut sighash_cache = SighashCache::new(to_sign.unsigned_tx.clone());
 
+  let pub_key = private_key.public_key(&secp);
+
   let sighash = sighash_cache
     .p2wpkh_signature_hash(
       0,
-      &to_spend_tx.output[0].script_pubkey,
+      &if is_p2sh {
+        ScriptBuf::new_p2wpkh(&pub_key.wpubkey_hash().unwrap())
+      } else {
+        to_spend_tx.output[0].script_pubkey.clone()
+      },
       to_spend_tx.output[0].value,
       sighash_type,
     )
@@ -126,7 +138,7 @@ fn create_message_signature_p2wpkh(
     .to_vec(),
   );
 
-  witness.push(private_key.public_key(&secp).to_bytes());
+  witness.push(pub_key.to_bytes());
 
   witness.to_owned()
 }

src/verify.rs

@@ -53,15 +53,27 @@ pub fn verify_simple(address: &Address, message: &[u8], signature: Witness) -> R
 /// Verifies the BIP-322 full from proper Rust types.
 pub fn verify_full(address: &Address, message: &[u8], to_sign: Transaction) -> Result<()> {
   match address.payload() {
-    Payload::WitnessProgram(wp) if wp.version().to_num() == 0 && wp.program().len() == 20 => {
+    Payload::WitnessProgram(witness)
+      if witness.version().to_num() == 1 && witness.program().len() == 32 =>
+    {
+      let pub_key = XOnlyPublicKey::from_slice(witness.program().as_bytes())
+        .map_err(|_| Error::InvalidPublicKey)?;
+
+      verify_full_p2tr(address, message, to_sign, pub_key)
+    }
+    Payload::WitnessProgram(witness)
+      if witness.version().to_num() == 0 && witness.program().len() == 20 =>
+    {
       let pub_key =
         PublicKey::from_slice(&to_sign.input[0].witness[1]).map_err(|_| Error::InvalidPublicKey)?;
-      verify_full_p2wpkh(address, message, to_sign, pub_key)
+
+      verify_full_p2wpkh(address, message, to_sign, pub_key, false)
     }
-    Payload::WitnessProgram(wp) if wp.version().to_num() == 1 && wp.program().len() == 32 => {
+    Payload::ScriptHash(_) => {
       let pub_key =
-        XOnlyPublicKey::from_slice(wp.program().as_bytes()).map_err(|_| Error::InvalidPublicKey)?;
-      verify_full_p2tr(address, message, to_sign, pub_key)
+        PublicKey::from_slice(&to_sign.input[0].witness[1]).map_err(|_| Error::InvalidPublicKey)?;
+
+      verify_full_p2wpkh(address, message, to_sign, pub_key, true)
     }
     _ => Err(Error::UnsupportedAddress {
       address: address.to_string(),
@@ -74,6 +86,7 @@ fn verify_full_p2wpkh(
   message: &[u8],
   to_sign: Transaction,
   pub_key: PublicKey,
+  is_p2sh: bool,
 ) -> Result<()> {
   let to_spend = create_to_spend(address, message)?;
   let to_sign = create_to_sign(&to_spend, Some(to_sign.input[0].witness.clone()))?;
@@ -131,7 +144,11 @@ fn verify_full_p2wpkh(
   let sighash = sighash_cache
     .p2wpkh_signature_hash(
       0,
-      &to_spend.output[0].script_pubkey,
+      &if is_p2sh {
+        ScriptBuf::new_p2wpkh(&pub_key.wpubkey_hash().unwrap())
+      } else {
+        to_spend.output[0].script_pubkey.clone()
+      },
       to_spend.output[0].value,
       sighash_type,
     )