From 36b065981a10c30f37548f5df792885d5397730f Mon Sep 17 00:00:00 2001
From: Andrew Poelstra <apoelstra@wpsoftware.net>
Date: Sat, 24 Aug 2024 16:50:18 +0000
Subject: [PATCH] translator: make target pk and error associated types

This is an annoying breaking change for users of the Translator trait
but I think it greatly improves the ergonomics of using the trait.
Rather than having it be parameterized over 3 types, it is now
parameterized over just one (the "source pk type").

This matches how this trait is used in practice -- you typically have a
miniscript/policy/whatever with a keytype Pk, and you want to use a
translator from Pk to "whatever the translator maps to" with "whatever
error the translator yields". So the only type parameter you really need
to type is Pk; the others are irrelevant, and making the user name and
type them is annoying.

Since this eliminates the need to explicitly write out the error types
except when actually implementing the trait, this also changes a ton of
error types from () to Infallible, which is more efficient and correct.
---
 bitcoind-tests/tests/setup/test_util.rs | 30 ++++++++------
 examples/big.rs                         |  9 +++--
 examples/taproot.rs                     |  9 +++--
 src/descriptor/bare.rs                  | 10 ++---
 src/descriptor/mod.rs                   | 53 ++++++++++++++++---------
 src/descriptor/segwitv0.rs              | 10 ++---
 src/descriptor/sh.rs                    |  5 +--
 src/descriptor/sortedmulti.rs           |  7 ++--
 src/descriptor/tr.rs                    | 17 ++++----
 src/interpreter/inner.rs                | 19 +++++----
 src/lib.rs                              | 51 +++++++++++++++---------
 src/miniscript/mod.rs                   | 15 +++----
 src/policy/concrete.rs                  | 28 ++++++++-----
 src/policy/semantic.rs                  | 12 +++---
 src/psbt/mod.rs                         |  7 ++--
 src/pub_macros.rs                       |  7 +++-
 src/test_utils.rs                       | 31 +++++++++------
 17 files changed, 192 insertions(+), 128 deletions(-)

diff --git a/bitcoind-tests/tests/setup/test_util.rs b/bitcoind-tests/tests/setup/test_util.rs
index b16e8ed47..64ffa6f5a 100644
--- a/bitcoind-tests/tests/setup/test_util.rs
+++ b/bitcoind-tests/tests/setup/test_util.rs
@@ -155,8 +155,11 @@ pub fn parse_insane_ms<Ctx: ScriptContext>(
 #[derive(Debug, Clone)]
 struct StrDescPubKeyTranslator<'a>(usize, &'a PubData);
 
-impl<'a> Translator<String, DescriptorPublicKey, ()> for StrDescPubKeyTranslator<'a> {
-    fn pk(&mut self, pk_str: &String) -> Result<DescriptorPublicKey, ()> {
+impl<'a> Translator<String> for StrDescPubKeyTranslator<'a> {
+    type TargetPk = DescriptorPublicKey;
+    type Error = core::convert::Infallible;
+
+    fn pk(&mut self, pk_str: &String) -> Result<Self::TargetPk, Self::Error> {
         let avail = !pk_str.ends_with('!');
         if avail {
             self.0 += 1;
@@ -181,22 +184,22 @@ impl<'a> Translator<String, DescriptorPublicKey, ()> for StrDescPubKeyTranslator
         }
     }
 
-    fn sha256(&mut self, sha256: &String) -> Result<sha256::Hash, ()> {
+    fn sha256(&mut self, sha256: &String) -> Result<sha256::Hash, Self::Error> {
         let sha = sha256::Hash::from_str(sha256).unwrap();
         Ok(sha)
     }
 
-    fn hash256(&mut self, hash256: &String) -> Result<hash256::Hash, ()> {
+    fn hash256(&mut self, hash256: &String) -> Result<hash256::Hash, Self::Error> {
         let hash256 = hash256::Hash::from_str(hash256).unwrap();
         Ok(hash256)
     }
 
-    fn ripemd160(&mut self, ripemd160: &String) -> Result<ripemd160::Hash, ()> {
+    fn ripemd160(&mut self, ripemd160: &String) -> Result<ripemd160::Hash, Self::Error> {
         let ripemd160 = ripemd160::Hash::from_str(ripemd160).unwrap();
         Ok(ripemd160)
     }
 
-    fn hash160(&mut self, hash160: &String) -> Result<hash160::Hash, ()> {
+    fn hash160(&mut self, hash160: &String) -> Result<hash160::Hash, Self::Error> {
         let hash160 = hash160::Hash::from_str(hash160).unwrap();
         Ok(hash160)
     }
@@ -208,8 +211,11 @@ impl<'a> Translator<String, DescriptorPublicKey, ()> for StrDescPubKeyTranslator
 #[derive(Debug, Clone)]
 struct StrTranslatorLoose<'a>(usize, &'a PubData);
 
-impl<'a> Translator<String, DescriptorPublicKey, ()> for StrTranslatorLoose<'a> {
-    fn pk(&mut self, pk_str: &String) -> Result<DescriptorPublicKey, ()> {
+impl<'a> Translator<String> for StrTranslatorLoose<'a> {
+    type TargetPk = DescriptorPublicKey;
+    type Error = core::convert::Infallible;
+
+    fn pk(&mut self, pk_str: &String) -> Result<Self::TargetPk, Self::Error> {
         let avail = !pk_str.ends_with('!');
         if avail {
             self.0 += 1;
@@ -238,22 +244,22 @@ impl<'a> Translator<String, DescriptorPublicKey, ()> for StrTranslatorLoose<'a>
         }
     }
 
-    fn sha256(&mut self, sha256: &String) -> Result<sha256::Hash, ()> {
+    fn sha256(&mut self, sha256: &String) -> Result<sha256::Hash, Self::Error> {
         let sha = sha256::Hash::from_str(sha256).unwrap();
         Ok(sha)
     }
 
-    fn hash256(&mut self, hash256: &String) -> Result<hash256::Hash, ()> {
+    fn hash256(&mut self, hash256: &String) -> Result<hash256::Hash, Self::Error> {
         let hash256 = hash256::Hash::from_str(hash256).unwrap();
         Ok(hash256)
     }
 
-    fn ripemd160(&mut self, ripemd160: &String) -> Result<ripemd160::Hash, ()> {
+    fn ripemd160(&mut self, ripemd160: &String) -> Result<ripemd160::Hash, Self::Error> {
         let ripemd160 = ripemd160::Hash::from_str(ripemd160).unwrap();
         Ok(ripemd160)
     }
 
-    fn hash160(&mut self, hash160: &String) -> Result<hash160::Hash, ()> {
+    fn hash160(&mut self, hash160: &String) -> Result<hash160::Hash, Self::Error> {
         let hash160 = hash160::Hash::from_str(hash160).unwrap();
         Ok(hash160)
     }
diff --git a/examples/big.rs b/examples/big.rs
index 2a80f4de6..230d656b6 100644
--- a/examples/big.rs
+++ b/examples/big.rs
@@ -82,12 +82,15 @@ struct StrPkTranslator {
     pk_map: HashMap<String, XOnlyPublicKey>,
 }
 
-impl Translator<String, XOnlyPublicKey, ()> for StrPkTranslator {
-    fn pk(&mut self, pk: &String) -> Result<XOnlyPublicKey, ()> {
+impl Translator<String> for StrPkTranslator {
+    type TargetPk = XOnlyPublicKey;
+    type Error = ();
+
+    fn pk(&mut self, pk: &String) -> Result<XOnlyPublicKey, Self::Error> {
         self.pk_map.get(pk).copied().ok_or(())
     }
 
     // We don't need to implement these methods as we are not using them in the policy.
     // Fail if we encounter any hash fragments. See also translate_hash_clone! macro.
-    translate_hash_fail!(String, XOnlyPublicKey, ());
+    translate_hash_fail!(String, XOnlyPublicKey, Self::Error);
 }
diff --git a/examples/taproot.rs b/examples/taproot.rs
index 3ca109552..1e04f7b60 100644
--- a/examples/taproot.rs
+++ b/examples/taproot.rs
@@ -17,14 +17,17 @@ struct StrPkTranslator {
     pk_map: HashMap<String, XOnlyPublicKey>,
 }
 
-impl Translator<String, XOnlyPublicKey, ()> for StrPkTranslator {
-    fn pk(&mut self, pk: &String) -> Result<XOnlyPublicKey, ()> {
+impl Translator<String> for StrPkTranslator {
+    type TargetPk = XOnlyPublicKey;
+    type Error = ();
+
+    fn pk(&mut self, pk: &String) -> Result<XOnlyPublicKey, Self::Error> {
         self.pk_map.get(pk).copied().ok_or(())
     }
 
     // We don't need to implement these methods as we are not using them in the policy.
     // Fail if we encounter any hash fragments. See also translate_hash_clone! macro.
-    translate_hash_fail!(String, XOnlyPublicKey, ());
+    translate_hash_fail!(String, XOnlyPublicKey, Self::Error);
 }
 
 fn main() {
diff --git a/src/descriptor/bare.rs b/src/descriptor/bare.rs
index f720ac29f..176137ad8 100644
--- a/src/descriptor/bare.rs
+++ b/src/descriptor/bare.rs
@@ -94,10 +94,9 @@ impl<Pk: MiniscriptKey> Bare<Pk> {
     }
 
     /// Converts the keys in the script from one type to another.
-    pub fn translate_pk<Q, T, E>(&self, t: &mut T) -> Result<Bare<Q>, TranslateErr<E>>
+    pub fn translate_pk<T>(&self, t: &mut T) -> Result<Bare<T::TargetPk>, TranslateErr<T::Error>>
     where
-        T: Translator<Pk, Q, E>,
-        Q: MiniscriptKey,
+        T: Translator<Pk>,
     {
         Bare::new(self.ms.translate_pk(t)?).map_err(TranslateErr::OuterError)
     }
@@ -256,10 +255,9 @@ impl<Pk: MiniscriptKey> Pkh<Pk> {
     pub fn max_satisfaction_weight(&self) -> usize { 4 * (1 + 73 + BareCtx::pk_len(&self.pk)) }
 
     /// Converts the keys in a script from one type to another.
-    pub fn translate_pk<Q, T, E>(&self, t: &mut T) -> Result<Pkh<Q>, TranslateErr<E>>
+    pub fn translate_pk<T>(&self, t: &mut T) -> Result<Pkh<T::TargetPk>, TranslateErr<T::Error>>
     where
-        T: Translator<Pk, Q, E>,
-        Q: MiniscriptKey,
+        T: Translator<Pk>,
     {
         let res = Pkh::new(t.pk(&self.pk)?);
         match res {
diff --git a/src/descriptor/mod.rs b/src/descriptor/mod.rs
index 1c6393a85..1a2a7da16 100644
--- a/src/descriptor/mod.rs
+++ b/src/descriptor/mod.rs
@@ -361,10 +361,12 @@ impl<Pk: MiniscriptKey> Descriptor<Pk> {
     }
 
     /// Converts a descriptor using one kind of keys to another kind of key.
-    pub fn translate_pk<T, Q, E>(&self, t: &mut T) -> Result<Descriptor<Q>, TranslateErr<E>>
+    pub fn translate_pk<T>(
+        &self,
+        t: &mut T,
+    ) -> Result<Descriptor<T::TargetPk>, TranslateErr<T::Error>>
     where
-        T: Translator<Pk, Q, E>,
-        Q: MiniscriptKey,
+        T: Translator<Pk>,
     {
         let desc = match *self {
             Descriptor::Bare(ref bare) => Descriptor::Bare(bare.translate_pk(t)?),
@@ -600,7 +602,10 @@ impl Descriptor<DescriptorPublicKey> {
     ) -> Result<Descriptor<DefiniteDescriptorKey>, ConversionError> {
         struct Derivator(u32);
 
-        impl Translator<DescriptorPublicKey, DefiniteDescriptorKey, ConversionError> for Derivator {
+        impl Translator<DescriptorPublicKey> for Derivator {
+            type TargetPk = DefiniteDescriptorKey;
+            type Error = ConversionError;
+
             fn pk(
                 &mut self,
                 pk: &DescriptorPublicKey,
@@ -691,9 +696,10 @@ impl Descriptor<DescriptorPublicKey> {
 
         struct KeyMapWrapper<'a, C: secp256k1::Signing>(KeyMap, &'a secp256k1::Secp256k1<C>);
 
-        impl<'a, C: secp256k1::Signing> Translator<String, DescriptorPublicKey, Error>
-            for KeyMapWrapper<'a, C>
-        {
+        impl<'a, C: secp256k1::Signing> Translator<String> for KeyMapWrapper<'a, C> {
+            type TargetPk = DescriptorPublicKey;
+            type Error = Error;
+
             fn pk(&mut self, pk: &String) -> Result<DescriptorPublicKey, Error> {
                 parse_key(pk, &mut self.0, self.1)
             }
@@ -738,29 +744,35 @@ impl Descriptor<DescriptorPublicKey> {
     pub fn to_string_with_secret(&self, key_map: &KeyMap) -> String {
         struct KeyMapLookUp<'a>(&'a KeyMap);
 
-        impl<'a> Translator<DescriptorPublicKey, String, ()> for KeyMapLookUp<'a> {
-            fn pk(&mut self, pk: &DescriptorPublicKey) -> Result<String, ()> {
+        impl<'a> Translator<DescriptorPublicKey> for KeyMapLookUp<'a> {
+            type TargetPk = String;
+            type Error = core::convert::Infallible;
+
+            fn pk(&mut self, pk: &DescriptorPublicKey) -> Result<String, Self::Error> {
                 key_to_string(pk, self.0)
             }
 
-            fn sha256(&mut self, sha256: &sha256::Hash) -> Result<String, ()> {
+            fn sha256(&mut self, sha256: &sha256::Hash) -> Result<String, Self::Error> {
                 Ok(sha256.to_string())
             }
 
-            fn hash256(&mut self, hash256: &hash256::Hash) -> Result<String, ()> {
+            fn hash256(&mut self, hash256: &hash256::Hash) -> Result<String, Self::Error> {
                 Ok(hash256.to_string())
             }
 
-            fn ripemd160(&mut self, ripemd160: &ripemd160::Hash) -> Result<String, ()> {
+            fn ripemd160(&mut self, ripemd160: &ripemd160::Hash) -> Result<String, Self::Error> {
                 Ok(ripemd160.to_string())
             }
 
-            fn hash160(&mut self, hash160: &hash160::Hash) -> Result<String, ()> {
+            fn hash160(&mut self, hash160: &hash160::Hash) -> Result<String, Self::Error> {
                 Ok(hash160.to_string())
             }
         }
 
-        fn key_to_string(pk: &DescriptorPublicKey, key_map: &KeyMap) -> Result<String, ()> {
+        fn key_to_string(
+            pk: &DescriptorPublicKey,
+            key_map: &KeyMap,
+        ) -> Result<String, core::convert::Infallible> {
             Ok(match key_map.get(pk) {
                 Some(secret) => secret.to_string(),
                 None => pk.to_string(),
@@ -835,7 +847,10 @@ impl Descriptor<DescriptorPublicKey> {
 
         // Now, transform the multipath key of each descriptor into a single-key using each index.
         struct IndexChoser(usize);
-        impl Translator<DescriptorPublicKey, DescriptorPublicKey, Error> for IndexChoser {
+        impl Translator<DescriptorPublicKey> for IndexChoser {
+            type TargetPk = DescriptorPublicKey;
+            type Error = Error;
+
             fn pk(&mut self, pk: &DescriptorPublicKey) -> Result<DescriptorPublicKey, Error> {
                 match pk {
                     DescriptorPublicKey::Single(..) | DescriptorPublicKey::XPub(..) => {
@@ -892,10 +907,10 @@ impl Descriptor<DefiniteDescriptorKey> {
     ) -> Result<Descriptor<bitcoin::PublicKey>, ConversionError> {
         struct Derivator<'a, C: secp256k1::Verification>(&'a secp256k1::Secp256k1<C>);
 
-        impl<'a, C: secp256k1::Verification>
-            Translator<DefiniteDescriptorKey, bitcoin::PublicKey, ConversionError>
-            for Derivator<'a, C>
-        {
+        impl<'a, C: secp256k1::Verification> Translator<DefiniteDescriptorKey> for Derivator<'a, C> {
+            type TargetPk = bitcoin::PublicKey;
+            type Error = ConversionError;
+
             fn pk(
                 &mut self,
                 pk: &DefiniteDescriptorKey,
diff --git a/src/descriptor/segwitv0.rs b/src/descriptor/segwitv0.rs
index 2a30f1db1..2f2532b85 100644
--- a/src/descriptor/segwitv0.rs
+++ b/src/descriptor/segwitv0.rs
@@ -130,10 +130,9 @@ impl<Pk: MiniscriptKey> Wsh<Pk> {
     }
 
     /// Converts the keys in a script from one type to another.
-    pub fn translate_pk<Q, T, E>(&self, t: &mut T) -> Result<Wsh<Q>, TranslateErr<E>>
+    pub fn translate_pk<T>(&self, t: &mut T) -> Result<Wsh<T::TargetPk>, TranslateErr<T::Error>>
     where
-        T: Translator<Pk, Q, E>,
-        Q: MiniscriptKey,
+        T: Translator<Pk>,
     {
         let inner = match self.inner {
             WshInner::SortedMulti(ref smv) => WshInner::SortedMulti(smv.translate_pk(t)?),
@@ -366,10 +365,9 @@ impl<Pk: MiniscriptKey> Wpkh<Pk> {
     pub fn max_satisfaction_weight(&self) -> usize { 4 + 1 + 73 + Segwitv0::pk_len(&self.pk) }
 
     /// Converts the keys in a script from one type to another.
-    pub fn translate_pk<Q, T, E>(&self, t: &mut T) -> Result<Wpkh<Q>, TranslateErr<E>>
+    pub fn translate_pk<T>(&self, t: &mut T) -> Result<Wpkh<T::TargetPk>, TranslateErr<T::Error>>
     where
-        T: Translator<Pk, Q, E>,
-        Q: MiniscriptKey,
+        T: Translator<Pk>,
     {
         let res = Wpkh::new(t.pk(&self.pk)?);
         match res {
diff --git a/src/descriptor/sh.rs b/src/descriptor/sh.rs
index 99923ca77..cf05c1b71 100644
--- a/src/descriptor/sh.rs
+++ b/src/descriptor/sh.rs
@@ -261,10 +261,9 @@ impl<Pk: MiniscriptKey> Sh<Pk> {
     }
 
     /// Converts the keys in a script from one type to another.
-    pub fn translate_pk<Q, T, E>(&self, t: &mut T) -> Result<Sh<Q>, TranslateErr<E>>
+    pub fn translate_pk<T>(&self, t: &mut T) -> Result<Sh<T::TargetPk>, TranslateErr<T::Error>>
     where
-        T: Translator<Pk, Q, E>,
-        Q: MiniscriptKey,
+        T: Translator<Pk>,
     {
         let inner = match self.inner {
             ShInner::Wsh(ref wsh) => ShInner::Wsh(wsh.translate_pk(t)?),
diff --git a/src/descriptor/sortedmulti.rs b/src/descriptor/sortedmulti.rs
index 5b7079292..b8a378a26 100644
--- a/src/descriptor/sortedmulti.rs
+++ b/src/descriptor/sortedmulti.rs
@@ -77,13 +77,12 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> SortedMultiVec<Pk, Ctx> {
     /// This will panic if fpk returns an uncompressed key when
     /// converting to a Segwit descriptor. To prevent this panic, ensure
     /// fpk returns an error in this case instead.
-    pub fn translate_pk<T, Q, FuncError>(
+    pub fn translate_pk<T>(
         &self,
         t: &mut T,
-    ) -> Result<SortedMultiVec<Q, Ctx>, TranslateErr<FuncError>>
+    ) -> Result<SortedMultiVec<T::TargetPk, Ctx>, TranslateErr<T::Error>>
     where
-        T: Translator<Pk, Q, FuncError>,
-        Q: MiniscriptKey,
+        T: Translator<Pk>,
     {
         let ret = SortedMultiVec {
             inner: self.inner.translate_ref(|pk| t.pk(pk))?,
diff --git a/src/descriptor/tr.rs b/src/descriptor/tr.rs
index 1716682d9..2f505780c 100644
--- a/src/descriptor/tr.rs
+++ b/src/descriptor/tr.rs
@@ -132,10 +132,9 @@ impl<Pk: MiniscriptKey> TapTree<Pk> {
     pub fn iter(&self) -> TapTreeIter<Pk> { TapTreeIter { stack: vec![(0, self)] } }
 
     // Helper function to translate keys
-    fn translate_helper<T, Q, E>(&self, t: &mut T) -> Result<TapTree<Q>, TranslateErr<E>>
+    fn translate_helper<T>(&self, t: &mut T) -> Result<TapTree<T::TargetPk>, TranslateErr<T::Error>>
     where
-        T: Translator<Pk, Q, E>,
-        Q: MiniscriptKey,
+        T: Translator<Pk>,
     {
         let frag = match *self {
             TapTree::Tree { ref left, ref right, ref height } => TapTree::Tree {
@@ -353,17 +352,19 @@ impl<Pk: MiniscriptKey> Tr<Pk> {
     }
 
     /// Converts keys from one type of public key to another.
-    pub fn translate_pk<Q, T, E>(&self, translate: &mut T) -> Result<Tr<Q>, TranslateErr<E>>
+    pub fn translate_pk<T>(
+        &self,
+        translate: &mut T,
+    ) -> Result<Tr<T::TargetPk>, TranslateErr<T::Error>>
     where
-        T: Translator<Pk, Q, E>,
-        Q: MiniscriptKey,
+        T: Translator<Pk>,
     {
         let tree = match &self.tree {
             Some(tree) => Some(tree.translate_helper(translate)?),
             None => None,
         };
-        let translate_desc = Tr::new(translate.pk(&self.internal_key)?, tree)
-            .map_err(|e| TranslateErr::OuterError(e))?;
+        let translate_desc =
+            Tr::new(translate.pk(&self.internal_key)?, tree).map_err(TranslateErr::OuterError)?;
         Ok(translate_desc)
     }
 }
diff --git a/src/interpreter/inner.rs b/src/interpreter/inner.rs
index e607266e7..49134c6de 100644
--- a/src/interpreter/inner.rs
+++ b/src/interpreter/inner.rs
@@ -356,12 +356,15 @@ impl<Ctx: ScriptContext> ToNoChecks for Miniscript<bitcoin::PublicKey, Ctx> {
     fn to_no_checks_ms(&self) -> Miniscript<BitcoinKey, NoChecks> {
         struct TranslateFullPk;
 
-        impl Translator<bitcoin::PublicKey, BitcoinKey, ()> for TranslateFullPk {
-            fn pk(&mut self, pk: &bitcoin::PublicKey) -> Result<BitcoinKey, ()> {
+        impl Translator<bitcoin::PublicKey> for TranslateFullPk {
+            type TargetPk = BitcoinKey;
+            type Error = core::convert::Infallible;
+
+            fn pk(&mut self, pk: &bitcoin::PublicKey) -> Result<BitcoinKey, Self::Error> {
                 Ok(BitcoinKey::Fullkey(*pk))
             }
 
-            translate_hash_clone!(bitcoin::PublicKey, BitcoinKey, ());
+            translate_hash_clone!(bitcoin::PublicKey, BitcoinKey, Self::Error);
         }
 
         self.translate_pk_ctx(&mut TranslateFullPk)
@@ -371,15 +374,17 @@ impl<Ctx: ScriptContext> ToNoChecks for Miniscript<bitcoin::PublicKey, Ctx> {
 
 impl<Ctx: ScriptContext> ToNoChecks for Miniscript<bitcoin::key::XOnlyPublicKey, Ctx> {
     fn to_no_checks_ms(&self) -> Miniscript<BitcoinKey, NoChecks> {
-        // specify the () error type as this cannot error
         struct TranslateXOnlyPk;
 
-        impl Translator<bitcoin::key::XOnlyPublicKey, BitcoinKey, ()> for TranslateXOnlyPk {
-            fn pk(&mut self, pk: &bitcoin::key::XOnlyPublicKey) -> Result<BitcoinKey, ()> {
+        impl Translator<bitcoin::key::XOnlyPublicKey> for TranslateXOnlyPk {
+            type TargetPk = BitcoinKey;
+            type Error = core::convert::Infallible;
+
+            fn pk(&mut self, pk: &bitcoin::key::XOnlyPublicKey) -> Result<BitcoinKey, Self::Error> {
                 Ok(BitcoinKey::XOnlyPublicKey(*pk))
             }
 
-            translate_hash_clone!(bitcoin::key::XOnlyPublicKey, BitcoinKey, ());
+            translate_hash_clone!(bitcoin::key::XOnlyPublicKey, BitcoinKey, Self::Error);
         }
         self.translate_pk_ctx(&mut TranslateXOnlyPk)
             .expect("Translation should succeed")
diff --git a/src/lib.rs b/src/lib.rs
index 7f7040f29..4cf566392 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -296,25 +296,38 @@ impl ToPublicKey for bitcoin::secp256k1::XOnlyPublicKey {
 
 /// Describes an object that can translate various keys and hashes from one key to the type
 /// associated with the other key. Used by the [`TranslatePk`] trait to do the actual translations.
-pub trait Translator<P, Q, E>
-where
-    P: MiniscriptKey,
-    Q: MiniscriptKey,
-{
-    /// Translates public keys P -> Q.
-    fn pk(&mut self, pk: &P) -> Result<Q, E>;
-
-    /// Provides the translation from P::Sha256 -> Q::Sha256
-    fn sha256(&mut self, sha256: &P::Sha256) -> Result<Q::Sha256, E>;
-
-    /// Provides the translation from P::Hash256 -> Q::Hash256
-    fn hash256(&mut self, hash256: &P::Hash256) -> Result<Q::Hash256, E>;
-
-    /// Translates ripemd160 hashes from P::Ripemd160 -> Q::Ripemd160
-    fn ripemd160(&mut self, ripemd160: &P::Ripemd160) -> Result<Q::Ripemd160, E>;
-
-    /// Translates hash160 hashes from P::Hash160 -> Q::Hash160
-    fn hash160(&mut self, hash160: &P::Hash160) -> Result<Q::Hash160, E>;
+pub trait Translator<P: MiniscriptKey> {
+    /// The public key (and associated hash types that this translator converts to.
+    type TargetPk: MiniscriptKey;
+    /// An error that may occur during transalation.
+    type Error;
+
+    /// Translates keys.
+    fn pk(&mut self, pk: &P) -> Result<Self::TargetPk, Self::Error>;
+
+    /// Translates SHA256 hashes.
+    fn sha256(
+        &mut self,
+        sha256: &P::Sha256,
+    ) -> Result<<Self::TargetPk as MiniscriptKey>::Sha256, Self::Error>;
+
+    /// Translates HASH256 hashes.
+    fn hash256(
+        &mut self,
+        hash256: &P::Hash256,
+    ) -> Result<<Self::TargetPk as MiniscriptKey>::Hash256, Self::Error>;
+
+    /// Translates RIPEMD160 hashes.
+    fn ripemd160(
+        &mut self,
+        ripemd160: &P::Ripemd160,
+    ) -> Result<<Self::TargetPk as MiniscriptKey>::Ripemd160, Self::Error>;
+
+    /// Translates HASH160 hashes.
+    fn hash160(
+        &mut self,
+        hash160: &P::Hash160,
+    ) -> Result<<Self::TargetPk as MiniscriptKey>::Hash160, Self::Error>;
 }
 
 /// An enum for representing translation errors
diff --git a/src/miniscript/mod.rs b/src/miniscript/mod.rs
index 0091691e5..1a68606ba 100644
--- a/src/miniscript/mod.rs
+++ b/src/miniscript/mod.rs
@@ -516,22 +516,23 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> ForEachKey<Pk> for Miniscript<Pk, Ct
 impl<Pk: MiniscriptKey, Ctx: ScriptContext> Miniscript<Pk, Ctx> {
     /// Translates a struct from one generic to another where the translation
     /// for Pk is provided by [`Translator`]
-    pub fn translate_pk<Q, T, E>(&self, t: &mut T) -> Result<Miniscript<Q, Ctx>, TranslateErr<E>>
+    pub fn translate_pk<T>(
+        &self,
+        t: &mut T,
+    ) -> Result<Miniscript<T::TargetPk, Ctx>, TranslateErr<T::Error>>
     where
-        T: Translator<Pk, Q, E>,
-        Q: MiniscriptKey,
+        T: Translator<Pk>,
     {
         self.translate_pk_ctx(t)
     }
 
-    pub(super) fn translate_pk_ctx<Q, CtxQ, T, FuncError>(
+    pub(super) fn translate_pk_ctx<CtxQ, T>(
         &self,
         t: &mut T,
-    ) -> Result<Miniscript<Q, CtxQ>, TranslateErr<FuncError>>
+    ) -> Result<Miniscript<T::TargetPk, CtxQ>, TranslateErr<T::Error>>
     where
-        Q: MiniscriptKey,
         CtxQ: ScriptContext,
-        T: Translator<Pk, Q, FuncError>,
+        T: Translator<Pk>,
     {
         let mut translated = vec![];
         for data in self.rtl_post_order_iter() {
diff --git a/src/policy/concrete.rs b/src/policy/concrete.rs
index 6382d837b..8e31d5474 100644
--- a/src/policy/concrete.rs
+++ b/src/policy/concrete.rs
@@ -507,10 +507,9 @@ impl<Pk: MiniscriptKey> Policy<Pk> {
     /// Converts a policy using one kind of public key to another type of public key.
     ///
     /// For example usage please see [`crate::policy::semantic::Policy::translate_pk`].
-    pub fn translate_pk<Q, E, T>(&self, t: &mut T) -> Result<Policy<Q>, E>
+    pub fn translate_pk<T>(&self, t: &mut T) -> Result<Policy<T::TargetPk>, T::Error>
     where
-        T: Translator<Pk, Q, E>,
-        Q: MiniscriptKey,
+        T: Translator<Pk>,
     {
         use Policy::*;
 
@@ -1140,15 +1139,26 @@ mod tests {
     #[test]
     fn tranaslate_pk() {
         pub struct TestTranslator;
-        impl Translator<String, String, ()> for TestTranslator {
-            fn pk(&mut self, pk: &String) -> Result<String, ()> {
+        impl Translator<String> for TestTranslator {
+            type TargetPk = String;
+            type Error = core::convert::Infallible;
+
+            fn pk(&mut self, pk: &String) -> Result<String, Self::Error> {
                 let new = format!("NEW-{}", pk);
                 Ok(new.to_string())
             }
-            fn sha256(&mut self, hash: &String) -> Result<String, ()> { Ok(hash.to_string()) }
-            fn hash256(&mut self, hash: &String) -> Result<String, ()> { Ok(hash.to_string()) }
-            fn ripemd160(&mut self, hash: &String) -> Result<String, ()> { Ok(hash.to_string()) }
-            fn hash160(&mut self, hash: &String) -> Result<String, ()> { Ok(hash.to_string()) }
+            fn sha256(&mut self, hash: &String) -> Result<String, Self::Error> {
+                Ok(hash.to_string())
+            }
+            fn hash256(&mut self, hash: &String) -> Result<String, Self::Error> {
+                Ok(hash.to_string())
+            }
+            fn ripemd160(&mut self, hash: &String) -> Result<String, Self::Error> {
+                Ok(hash.to_string())
+            }
+            fn hash160(&mut self, hash: &String) -> Result<String, Self::Error> {
+                Ok(hash.to_string())
+            }
         }
         let policy = Policy::<String>::from_str("or(and(pk(A),pk(B)),pk(C))").unwrap();
         let mut t = TestTranslator;
diff --git a/src/policy/semantic.rs b/src/policy/semantic.rs
index 7d2959764..62487a54a 100644
--- a/src/policy/semantic.rs
+++ b/src/policy/semantic.rs
@@ -120,8 +120,11 @@ impl<Pk: MiniscriptKey> Policy<Pk> {
     ///
     /// // If we also wanted to provide mapping of other associated types (sha256, older etc),
     /// // we would use the general [`Translator`] trait.
-    /// impl Translator<String, bitcoin::PublicKey, ()> for StrPkTranslator {
-    ///     fn pk(&mut self, pk: &String) -> Result<bitcoin::PublicKey, ()> {
+    /// impl Translator<String> for StrPkTranslator {
+    ///     type TargetPk = bitcoin::PublicKey;
+    ///     type Error = ();
+    ///
+    ///     fn pk(&mut self, pk: &String) -> Result<bitcoin::PublicKey, Self::Error> {
     ///         self.pk_map.get(pk).copied().ok_or(()) // Dummy Err
     ///     }
     ///
@@ -140,10 +143,9 @@ impl<Pk: MiniscriptKey> Policy<Pk> {
     /// let expected_policy = Policy::from_str(&format!("and(pk({}),pk({}))", alice_pk, bob_pk)).unwrap();
     /// assert_eq!(real_policy, expected_policy);
     /// ```
-    pub fn translate_pk<Q, E, T>(&self, t: &mut T) -> Result<Policy<Q>, E>
+    pub fn translate_pk<T>(&self, t: &mut T) -> Result<Policy<T::TargetPk>, T::Error>
     where
-        T: Translator<Pk, Q, E>,
-        Q: MiniscriptKey,
+        T: Translator<Pk>,
     {
         use Policy::*;
 
diff --git a/src/psbt/mod.rs b/src/psbt/mod.rs
index bb06bb3f2..96615df4c 100644
--- a/src/psbt/mod.rs
+++ b/src/psbt/mod.rs
@@ -976,9 +976,10 @@ struct KeySourceLookUp(
     pub secp256k1::Secp256k1<VerifyOnly>,
 );
 
-impl Translator<DefiniteDescriptorKey, bitcoin::PublicKey, descriptor::ConversionError>
-    for KeySourceLookUp
-{
+impl Translator<DefiniteDescriptorKey> for KeySourceLookUp {
+    type TargetPk = bitcoin::PublicKey;
+    type Error = descriptor::ConversionError;
+
     fn pk(
         &mut self,
         xpk: &DefiniteDescriptorKey,
diff --git a/src/pub_macros.rs b/src/pub_macros.rs
index bdb0d59b4..538d37f3c 100644
--- a/src/pub_macros.rs
+++ b/src/pub_macros.rs
@@ -27,9 +27,12 @@
 ///
 /// // If we also wanted to provide mapping of other associated types(sha256, older etc),
 /// // we would use the general Translator Trait.
-/// impl Translator<String, bitcoin::PublicKey, ()> for StrPkTranslator {
+/// impl Translator<String> for StrPkTranslator {
+///     type TargetPk = bitcoin::PublicKey;
+///     type Error = ();
+///
 ///     // Provides the translation public keys P -> Q
-///     fn pk(&mut self, pk: &String) -> Result<bitcoin::PublicKey, ()> {
+///     fn pk(&mut self, pk: &String) -> Result<bitcoin::PublicKey, Self::Error> {
 ///         self.pk_map.get(pk).copied().ok_or(()) // Dummy Err
 ///     }
 ///
diff --git a/src/test_utils.rs b/src/test_utils.rs
index 170e3c7f6..9d52b7c25 100644
--- a/src/test_utils.rs
+++ b/src/test_utils.rs
@@ -2,6 +2,7 @@
 
 //! Generally useful utilities for test scripts
 
+use core::convert::Infallible;
 use std::collections::HashMap;
 use std::str::FromStr;
 
@@ -25,8 +26,11 @@ pub struct StrKeyTranslator {
     pub hash160_map: HashMap<String, hash160::Hash>,
 }
 
-impl Translator<String, bitcoin::PublicKey, ()> for StrKeyTranslator {
-    fn pk(&mut self, pk: &String) -> Result<bitcoin::PublicKey, ()> {
+impl Translator<String> for StrKeyTranslator {
+    type TargetPk = bitcoin::PublicKey;
+    type Error = Infallible;
+
+    fn pk(&mut self, pk: &String) -> Result<bitcoin::PublicKey, Infallible> {
         let key = self.pk_map.get(pk).copied().unwrap_or_else(|| {
             bitcoin::PublicKey::from_str(
                 "02c2122e30e73f7fe37986e3f81ded00158e94b7ad472369b83bbdd28a9a198a39",
@@ -36,7 +40,7 @@ impl Translator<String, bitcoin::PublicKey, ()> for StrKeyTranslator {
         Ok(key)
     }
 
-    fn sha256(&mut self, _sha256: &String) -> Result<sha256::Hash, ()> {
+    fn sha256(&mut self, _sha256: &String) -> Result<sha256::Hash, Infallible> {
         let hash = sha256::Hash::from_str(
             "4ae81572f06e1b88fd5ced7a1a000945432e83e1551e6f721ee9c00b8cc33260",
         )
@@ -44,7 +48,7 @@ impl Translator<String, bitcoin::PublicKey, ()> for StrKeyTranslator {
         Ok(hash)
     }
 
-    fn hash256(&mut self, _hash256: &String) -> Result<hash256::Hash, ()> {
+    fn hash256(&mut self, _hash256: &String) -> Result<hash256::Hash, Infallible> {
         // hard coded value
         let hash = hash256::Hash::from_str(
             "4ae81572f06e1b88fd5ced7a1a000945432e83e1551e6f721ee9c00b8cc33260",
@@ -53,12 +57,12 @@ impl Translator<String, bitcoin::PublicKey, ()> for StrKeyTranslator {
         Ok(hash)
     }
 
-    fn ripemd160(&mut self, _ripemd160: &String) -> Result<ripemd160::Hash, ()> {
+    fn ripemd160(&mut self, _ripemd160: &String) -> Result<ripemd160::Hash, Infallible> {
         let hash = ripemd160::Hash::from_str("4ae81572f06e1b88fd5ced7a1a00094543a0069").unwrap();
         Ok(hash)
     }
 
-    fn hash160(&mut self, _hash160: &String) -> Result<hash160::Hash, ()> {
+    fn hash160(&mut self, _hash160: &String) -> Result<hash160::Hash, Infallible> {
         let hash = hash160::Hash::from_str("4ae81572f06e1b88fd5ced7a1a00094543a0069").unwrap();
         Ok(hash)
     }
@@ -74,8 +78,11 @@ pub struct StrXOnlyKeyTranslator {
     pub hash160_map: HashMap<String, hash160::Hash>,
 }
 
-impl Translator<String, XOnlyPublicKey, ()> for StrXOnlyKeyTranslator {
-    fn pk(&mut self, pk: &String) -> Result<XOnlyPublicKey, ()> {
+impl Translator<String> for StrXOnlyKeyTranslator {
+    type TargetPk = XOnlyPublicKey;
+    type Error = Infallible;
+
+    fn pk(&mut self, pk: &String) -> Result<XOnlyPublicKey, Infallible> {
         let key = self.pk_map.get(pk).copied().unwrap_or_else(|| {
             XOnlyPublicKey::from_str(
                 "c2122e30e73f7fe37986e3f81ded00158e94b7ad472369b83bbdd28a9a198a39",
@@ -85,7 +92,7 @@ impl Translator<String, XOnlyPublicKey, ()> for StrXOnlyKeyTranslator {
         Ok(key)
     }
 
-    fn sha256(&mut self, _sha256: &String) -> Result<sha256::Hash, ()> {
+    fn sha256(&mut self, _sha256: &String) -> Result<sha256::Hash, Infallible> {
         let hash = sha256::Hash::from_str(
             "4ae81572f06e1b88fd5ced7a1a000945432e83e1551e6f721ee9c00b8cc33260",
         )
@@ -93,7 +100,7 @@ impl Translator<String, XOnlyPublicKey, ()> for StrXOnlyKeyTranslator {
         Ok(hash)
     }
 
-    fn hash256(&mut self, _hash256: &String) -> Result<hash256::Hash, ()> {
+    fn hash256(&mut self, _hash256: &String) -> Result<hash256::Hash, Infallible> {
         let hash = hash256::Hash::from_str(
             "4ae81572f06e1b88fd5ced7a1a000945432e83e1551e6f721ee9c00b8cc33260",
         )
@@ -101,12 +108,12 @@ impl Translator<String, XOnlyPublicKey, ()> for StrXOnlyKeyTranslator {
         Ok(hash)
     }
 
-    fn ripemd160(&mut self, _ripemd160: &String) -> Result<ripemd160::Hash, ()> {
+    fn ripemd160(&mut self, _ripemd160: &String) -> Result<ripemd160::Hash, Infallible> {
         let hash = ripemd160::Hash::from_str("4ae81572f06e1b88fd5ced7a1a00094543a0069").unwrap();
         Ok(hash)
     }
 
-    fn hash160(&mut self, _hash160: &String) -> Result<hash160::Hash, ()> {
+    fn hash160(&mut self, _hash160: &String) -> Result<hash160::Hash, Infallible> {
         let hash = hash160::Hash::from_str("4ae81572f06e1b88fd5ced7a1a00094543a0069").unwrap();
         Ok(hash)
     }