miniscript: fix ExtData to correctly account for uncompressed keys

In our existing code we assume that all "ECDSA keys" are compressed,
i.e. 34 bytes long. This is not a valid assumption. We originally did
this because under some circumstances we legitimately did not know the
key and didn't want to penalize users by assuming uncompressed keys when
this was unlikely the case.

However, now the only way to have a pk or pkh where we don't know the
key is if the user is using a raw pkh. And if they do this in Taproot
or Segwit we still know that there are no uncompressed keys (though
with the current Ctx trait we can't distinguish Segwit from pre-Segwit
so the new logic assumes uncompressed keys even for Segwit.)

This logic will be cleaned up and improved with ValidationParams; but
currently it is wrong, and it is causing me trouble with later commits.
So fix the situation here as best we can.

The next commit will make a similar change to the multi() combinator
(multi_a is fine because the keys and sigs are fixed size); the commit
after that will introduce a unit test to SortedMulti which exercises
this logic.

Author: apoelstra

src/miniscript/mod.rs

@@ -183,19 +183,19 @@ mod private {
         /// The `pk_k` combinator.
         pub fn pk_k(pk: Pk) -> Self {
             Self {
+                ext: types::extra_props::ExtData::pk_k::<_, Ctx>(&pk),
                 node: Terminal::PkK(pk),
                 ty: types::Type::pk_k(),
-                ext: types::extra_props::ExtData::pk_k::<Ctx>(),
                 phantom: PhantomData,
             }
         }
 
         /// The `pk_h` combinator.
         pub fn pk_h(pk: Pk) -> Self {
             Self {
+                ext: types::extra_props::ExtData::pk_h::<_, Ctx>(Some(&pk)),
                 node: Terminal::PkH(pk),
                 ty: types::Type::pk_h(),
-                ext: types::extra_props::ExtData::pk_h::<Ctx>(),
                 phantom: PhantomData,
             }
         }
@@ -205,7 +205,7 @@ mod private {
             Self {
                 node: Terminal::RawPkH(hash),
                 ty: types::Type::pk_h(),
-                ext: types::extra_props::ExtData::pk_h::<Ctx>(),
+                ext: types::extra_props::ExtData::pk_h::<Pk, Ctx>(None),
                 phantom: PhantomData,
             }
         }

src/miniscript/types/extra_props.rs

@@ -7,7 +7,6 @@ use core::cmp;
 use core::iter::once;
 
 use super::ScriptContext;
-use crate::miniscript::context::SigType;
 use crate::prelude::*;
 use crate::{script_num_size, AbsLockTime, MiniscriptKey, RelLockTime, Terminal};
 
@@ -191,20 +190,22 @@ impl ExtData {
     }
 
     /// Extra properties for the `pk_k` fragment.
-    pub fn pk_k<Ctx: ScriptContext>() -> Self {
+    ///
+    /// The key must be provided to determine its size.
+    pub fn pk_k<Pk: MiniscriptKey, Ctx: ScriptContext>(pk: &Pk) -> Self {
+        let (key_bytes, max_sig_bytes) = match Ctx::sig_type() {
+            crate::SigType::Ecdsa if pk.is_uncompressed() => (65, 73),
+            crate::SigType::Ecdsa => (34, 73),
+            crate::SigType::Schnorr => (33, 66),
+        };
+
         ExtData {
-            pk_cost: match Ctx::sig_type() {
-                SigType::Ecdsa => 34,
-                SigType::Schnorr => 33,
-            },
+            pk_cost: key_bytes,
             has_free_verify: false,
             ops: OpLimits::new(0, Some(0), Some(0)),
             stack_elem_count_sat: Some(1),
             stack_elem_count_dissat: Some(1),
-            max_sat_size: match Ctx::sig_type() {
-                SigType::Ecdsa => Some((73, 73)),
-                SigType::Schnorr => Some((66, 66)),
-            },
+            max_sat_size: Some((max_sig_bytes, max_sig_bytes)),
             max_dissat_size: Some((1, 1)),
             timelock_info: TimelockInfo::default(),
             exec_stack_elem_count_sat: Some(1), // pushes the pk
@@ -214,21 +215,25 @@ impl ExtData {
     }
 
     /// Extra properties for the `pk_h` fragment.
-    pub fn pk_h<Ctx: ScriptContext>() -> Self {
+    ///
+    /// If the key is known, it should be provided to gain a size estimate from
+    /// it. If not, the worst-case for the context will be assumed.
+    pub fn pk_h<Pk: MiniscriptKey, Ctx: ScriptContext>(pk: Option<&Pk>) -> Self {
+        // With a raw pkh we don't know the preimage size so we have to assume the worst.
+        // FIXME with ValidationParams we will be able to determine if Ctx is Segwitv0 and exclude uncompressed keys.
+        let (key_bytes, max_sig_bytes) = match (Ctx::sig_type(), pk) {
+            (crate::SigType::Ecdsa, Some(pk)) if pk.is_uncompressed() => (65, 73),
+            (crate::SigType::Ecdsa, _) => (34, 73),
+            (crate::SigType::Schnorr, _) => (33, 66),
+        };
         ExtData {
             pk_cost: 24,
             has_free_verify: false,
             ops: OpLimits::new(3, Some(0), Some(0)),
             stack_elem_count_sat: Some(2),
             stack_elem_count_dissat: Some(2),
-            max_sat_size: match Ctx::sig_type() {
-                SigType::Ecdsa => Some((34 + 73, 34 + 73)),
-                SigType::Schnorr => Some((66 + 33, 33 + 66)),
-            },
-            max_dissat_size: match Ctx::sig_type() {
-                SigType::Ecdsa => Some((35, 35)),
-                SigType::Schnorr => Some((34, 34)),
-            },
+            max_sat_size: Some((key_bytes + max_sig_bytes, key_bytes + max_sig_bytes)),
+            max_dissat_size: Some((key_bytes + 1, key_bytes + 1)),
             timelock_info: TimelockInfo::default(),
             exec_stack_elem_count_sat: Some(2), // dup and hash push
             exec_stack_elem_count_dissat: Some(2),
@@ -932,8 +937,9 @@ impl ExtData {
         let ret = match *fragment {
             Terminal::True => Self::TRUE,
             Terminal::False => Self::FALSE,
-            Terminal::PkK(..) => Self::pk_k::<Ctx>(),
-            Terminal::PkH(..) | Terminal::RawPkH(..) => Self::pk_h::<Ctx>(),
+            Terminal::PkK(ref k) => Self::pk_k::<_, Ctx>(k),
+            Terminal::PkH(ref k) => Self::pk_h::<_, Ctx>(Some(k)),
+            Terminal::RawPkH(..) => Self::pk_h::<Pk, Ctx>(None),
             Terminal::Multi(ref thresh) => Self::multi(thresh.k(), thresh.n()),
             Terminal::MultiA(ref thresh) => Self::multi_a(thresh.k(), thresh.n()),
             Terminal::After(t) => Self::after(t),