Merge #210: Address err refactor

4269728 apply tomlfmt to Cargo.toml (Riccardo Casatta)
e9a90cc apply rustfmt to encode.rs (Riccardo Casatta)
6789492 apply rustfmt to pset/raw.rs (Riccardo Casatta)
8ae550b apply rustfmt to src/blind.rs (Riccardo Casatta)
084f7d4 Avoid instantiating base58 errors in Address::from_base58 (Riccardo Casatta)
5eefe85 apply rustfmt to address.rs (Riccardo Casatta)

Pull request description:

  This is propedeutic to upgrade to bitcoin 0.32 #209
  where the base58 error variant specific to address are removed

  Also it applies formatting around on which I think we agreed upon, but also on the toml file which I had doubt, let me know if I have to remove it

ACKs for top commit:
  apoelstra:
    ACK 4269728 successfully ran local tests

Tree-SHA512: 0cdc7077d4ec4550a4df6f5e6eb244c7a8c0793b507c9a4327b30bdaf9f597df0dee7c72e33414df0a87cb10f9a9be12588c9a564d71cb308564cf0226b7dec1

Author: apoelstra

Cargo.toml

@@ -10,32 +10,35 @@ documentation = "https://docs.rs/elements/"
 edition = "2018"
 
 [features]
-default = [ "json-contract" ]
+default = ["json-contract"]
 
-json-contract = [ "serde_json" ]
+json-contract = ["serde_json"]
 "serde" = [
     "bitcoin/serde",
     "bitcoin/serde",
     "secp256k1-zkp/serde",
-    "actual-serde"
+    "actual-serde",
 ]
 base64 = ["bitcoin/base64"]
 
 [dependencies]
 bitcoin = "0.31.0"
-secp256k1-zkp = { version = "0.10.0", features = [ "global-context", "hashes" ] }
+secp256k1-zkp = { version = "0.10.0", features = ["global-context", "hashes"] }
 
 # Used for ContractHash::from_json_contract.
 serde_json = { version = "1.0", optional = true }
 
-actual-serde = { package="serde", version = "1.0.103", features=["derive"], optional = true }
+actual-serde = { package = "serde", version = "1.0.103", features = [
+    "derive",
+], optional = true }
+
 
 [dev-dependencies]
 rand = "0.8"
 rand_chacha = "0.3"
 serde_test = "1.0.19"
 serde_json = "1.0"
-serde_cbor = "0.8"   # older than latest version to support 1.41.1
+serde_cbor = "0.8"    # older than latest version to support 1.41.1
 bincode = "1.3"
 
 [[example]]

src/address.rs

@@ -21,11 +21,11 @@ use std::fmt;
 use std::fmt::Write as _;
 use std::str::FromStr;
 
-use bitcoin::base58;
-use bitcoin::PublicKey;
-use crate::bech32::{Bech32, Bech32m, ByteIterExt, Fe32, Hrp, Fe32IterExt};
+use crate::bech32::{Bech32, Bech32m, ByteIterExt, Fe32, Fe32IterExt, Hrp};
 use crate::blech32::{Blech32, Blech32m};
 use crate::hashes::Hash;
+use bitcoin::base58;
+use bitcoin::PublicKey;
 use secp256k1_zkp;
 use secp256k1_zkp::Secp256k1;
 use secp256k1_zkp::Verification;
@@ -35,8 +35,8 @@ use serde;
 use crate::schnorr::{TapTweak, TweakedPublicKey, UntweakedPublicKey};
 use crate::taproot::TapNodeHash;
 
-use crate::{PubkeyHash, ScriptHash, WPubkeyHash, WScriptHash};
 use crate::{opcodes, script};
+use crate::{PubkeyHash, ScriptHash, WPubkeyHash, WScriptHash};
 
 /// Encoding error
 #[derive(Debug, PartialEq)]
@@ -62,6 +62,12 @@ pub enum AddressError {
 
     /// An invalid blinding pubkey was encountered.
     InvalidBlindingPubKey(secp256k1_zkp::UpstreamError),
+
+    /// The length (in bytes) of the object was not correct.
+    InvalidLength(usize),
+
+    /// Address version byte were not recognized.
+    InvalidAddressVersion(u8),
 }
 
 impl From<crate::bech32::primitives::decode::SegwitHrpstringError> for AddressError {
@@ -89,10 +95,18 @@ impl fmt::Display for AddressError {
                 write!(f, "invalid witness script version: {}", wver)
             }
             AddressError::InvalidWitnessProgramLength(ref len) => {
-                write!(f, "the witness program must be between 2 and 40 bytes in length, not {}", len)
+                write!(
+                    f,
+                    "the witness program must be between 2 and 40 bytes in length, not {}",
+                    len
+                )
             }
             AddressError::InvalidSegwitV0ProgramLength(ref len) => {
-                write!(f, "a v0 witness program must be length 20 or 32, not {}", len)
+                write!(
+                    f,
+                    "a v0 witness program must be length 20 or 32, not {}",
+                    len
+                )
             }
             AddressError::InvalidBlindingPubKey(ref e) => {
                 write!(f, "an invalid blinding pubkey was encountered: {}", e)
@@ -103,6 +117,12 @@ impl fmt::Display for AddressError {
             AddressError::InvalidSegwitV0Encoding => {
                 write!(f, "v0 witness program must use b(l)ech32 not b(l)ech32m")
             }
+            AddressError::InvalidLength(len) => {
+                write!(f, "Address data has invalid length {}", len)
+            }
+            AddressError::InvalidAddressVersion(v) => {
+                write!(f, "address version {} is invalid for this type", v)
+            }
         }
     }
 }
@@ -212,7 +232,8 @@ impl Address {
         params: &'static AddressParams,
     ) -> Address {
         let mut hash_engine = PubkeyHash::engine();
-        pk.write_into(&mut hash_engine).expect("engines don't error");
+        pk.write_into(&mut hash_engine)
+            .expect("engines don't error");
 
         Address {
             params,
@@ -244,7 +265,8 @@ impl Address {
         params: &'static AddressParams,
     ) -> Address {
         let mut hash_engine = WPubkeyHash::engine();
-        pk.write_into(&mut hash_engine).expect("engines don't error");
+        pk.write_into(&mut hash_engine)
+            .expect("engines don't error");
 
         Address {
             params,
@@ -264,7 +286,8 @@ impl Address {
         params: &'static AddressParams,
     ) -> Address {
         let mut hash_engine = ScriptHash::engine();
-        pk.write_into(&mut hash_engine).expect("engines don't error");
+        pk.write_into(&mut hash_engine)
+            .expect("engines don't error");
 
         let builder = script::Builder::new()
             .push_int(0)
@@ -401,7 +424,9 @@ impl Address {
             Payload::WitnessProgram {
                 version: witver,
                 program: ref witprog,
-            } => script::Builder::new().push_int(witver.to_u8() as i64).push_slice(witprog),
+            } => script::Builder::new()
+                .push_int(witver.to_u8() as i64)
+                .push_slice(witprog),
         }
         .into_script()
     }
@@ -450,10 +475,7 @@ impl Address {
 
         Ok(Address {
             params,
-            payload: Payload::WitnessProgram {
-                version,
-                program,
-            },
+            payload: Payload::WitnessProgram { version, program },
             blinding_pubkey,
         })
     }
@@ -468,13 +490,13 @@ impl Address {
         let (blinded, prefix) = match data[0] == params.blinded_prefix {
             true => {
                 if data.len() != 55 {
-                    return Err(base58::Error::InvalidLength(data.len()).into());
+                    return Err(AddressError::InvalidLength(data.len()));
                 }
                 (true, data[1])
             }
             false => {
                 if data.len() != 21 {
-                    return Err(base58::Error::InvalidLength(data.len()).into());
+                    return Err(AddressError::InvalidLength(data.len()));
                 }
                 (false, data[0])
             }
@@ -496,7 +518,7 @@ impl Address {
         } else if prefix == params.p2sh_prefix {
             Payload::ScriptHash(ScriptHash::from_slice(payload_data).unwrap())
         } else {
-            return Err(base58::Error::InvalidAddressVersion(prefix).into());
+            return Err(AddressError::InvalidAddressVersion(prefix));
         };
 
         Ok(Address {
@@ -522,7 +544,7 @@ impl Address {
 
         // Base58.
         if s.len() > 150 {
-            return Err(base58::Error::InvalidLength(s.len() * 11 / 15).into());
+            return Err(AddressError::InvalidLength(s.len() * 11 / 15));
         }
         let data = base58::decode_check(s)?;
         Address::from_base58(&data, params)
@@ -574,14 +596,23 @@ impl fmt::Display for Address {
                 // FIXME: surely we can fix this logic to not be so repetitive.
                 if self.is_blinded() {
                     if let Some(ref blinder) = self.blinding_pubkey {
-                        let byte_iter = IntoIterator::into_iter(blinder.serialize()).chain(witprog.iter().copied());
+                        let byte_iter = IntoIterator::into_iter(blinder.serialize())
+                            .chain(witprog.iter().copied());
                         let fe_iter = byte_iter.bytes_to_fes();
                         if witver.to_u8() == 0 {
-                            for c in fe_iter.with_checksum::<Blech32>(&hrp).with_witness_version(witver).chars() {
+                            for c in fe_iter
+                                .with_checksum::<Blech32>(&hrp)
+                                .with_witness_version(witver)
+                                .chars()
+                            {
                                 fmt.write_char(c)?;
                             }
                         } else {
-                            for c in fe_iter.with_checksum::<Blech32m>(&hrp).with_witness_version(witver).chars() {
+                            for c in fe_iter
+                                .with_checksum::<Blech32m>(&hrp)
+                                .with_witness_version(witver)
+                                .chars()
+                            {
                                 fmt.write_char(c)?;
                             }
                         }
@@ -592,11 +623,19 @@ impl fmt::Display for Address {
                 let byte_iter = witprog.iter().copied();
                 let fe_iter = byte_iter.bytes_to_fes();
                 if witver.to_u8() == 0 {
-                    for c in fe_iter.with_checksum::<Bech32>(&hrp).with_witness_version(witver).chars() {
+                    for c in fe_iter
+                        .with_checksum::<Bech32>(&hrp)
+                        .with_witness_version(witver)
+                        .chars()
+                    {
                         fmt.write_char(c)?;
                     }
                 } else {
-                    for c in fe_iter.with_checksum::<Bech32m>(&hrp).with_witness_version(witver).chars() {
+                    for c in fe_iter
+                        .with_checksum::<Bech32m>(&hrp)
+                        .with_witness_version(witver)
+                        .chars()
+                    {
                         fmt.write_char(c)?;
                     }
                 }
@@ -660,11 +699,11 @@ impl FromStr for Address {
 
         // Base58.
         if s.len() > 150 {
-            return Err(base58::Error::InvalidLength(s.len() * 11 / 15).into());
+            return Err(AddressError::InvalidLength(s.len() * 11 / 15));
         }
         let data = base58::decode_check(s)?;
         if data.is_empty() {
-            return Err(base58::Error::InvalidLength(data.len()).into());
+            return Err(AddressError::InvalidLength(data.len()));
         }
 
         let p = data[0];
@@ -734,9 +773,9 @@ impl serde::Serialize for Address {
 #[cfg(test)]
 mod test {
     use super::*;
+    use crate::Script;
     use bitcoin::key;
     use secp256k1_zkp::{PublicKey, Secp256k1};
-    use crate::Script;
     #[cfg(feature = "serde")]
     use serde_json;
 
@@ -755,7 +794,9 @@ mod test {
         );
         #[cfg(feature = "serde")]
         assert_eq!(
-            serde_json::from_value::<Address>(serde_json::to_value(addr).unwrap()).ok().as_ref(),
+            serde_json::from_value::<Address>(serde_json::to_value(addr).unwrap())
+                .ok()
+                .as_ref(),
             Some(addr)
         );
     }
@@ -831,7 +872,12 @@ mod test {
 
         for &(a, blinded, ref params) in &addresses {
             let result = a.parse();
-            assert!(result.is_ok(), "vector: {}, err: \"{}\"", a, result.unwrap_err());
+            assert!(
+                result.is_ok(),
+                "vector: {}, err: \"{}\"",
+                a,
+                result.unwrap_err()
+            );
             let addr: Address = result.unwrap();
             assert_eq!(a, &addr.to_string(), "vector: {}", a);
             assert_eq!(blinded, addr.is_blinded());
@@ -858,7 +904,8 @@ mod test {
             "blech32 error: invalid checksum", // is valid blech32m, but should be blech32
         );
 
-        let address: Result<Address, _> = "ert130xlxvlhemja6c4dqv22uapctqupfhlxm9h8z3k2e72q4k9hcz7vqqu2tys".parse();
+        let address: Result<Address, _> =
+            "ert130xlxvlhemja6c4dqv22uapctqupfhlxm9h8z3k2e72q4k9hcz7vqqu2tys".parse();
         assert_eq!(
             address.err().unwrap().to_string(),
             "bech32 error: invalid segwit witness version: 3", // FIXME https://github.com/rust-bitcoin/rust-bech32/issues/162 should be 17
@@ -879,14 +926,18 @@ mod test {
         );
     }
 
-
     #[test]
     fn test_fixed_addresses() {
-        let pk = bitcoin::PublicKey::from_str("0212bf0ea45b733dfde8ecb5e896306c4165c666c99fc5d1ab887f71393a975cea")
-            .unwrap();
+        let pk = bitcoin::PublicKey::from_str(
+            "0212bf0ea45b733dfde8ecb5e896306c4165c666c99fc5d1ab887f71393a975cea",
+        )
+        .unwrap();
         let script = Script::default();
         let secp = Secp256k1::verification_only();
-        let internal_key = UntweakedPublicKey::from_str("93c7378d96518a75448821c4f7c8f4bae7ce60f804d03d1f0628dd5dd0f5de51").unwrap();
+        let internal_key = UntweakedPublicKey::from_str(
+            "93c7378d96518a75448821c4f7c8f4bae7ce60f804d03d1f0628dd5dd0f5de51",
+        )
+        .unwrap();
         let tap_node_hash = TapNodeHash::all_zeros();
 
         let mut expected = IntoIterator::into_iter([
@@ -934,9 +985,12 @@ mod test {
             "tlq1pqgft7r4ytdenml0gaj67393sd3qkt3nxex0ut5dt3plhzwf6jaww5vx8c8vs0ywzejta7jjcc5f4asnacdtu0wlaas0upmsq90enaz2lekytucqf82vs",
         ]);
 
-        for params in [&AddressParams::ELEMENTS, &AddressParams::LIQUID, &AddressParams::LIQUID_TESTNET] {
+        for params in [
+            &AddressParams::ELEMENTS,
+            &AddressParams::LIQUID,
+            &AddressParams::LIQUID_TESTNET,
+        ] {
             for blinder in [None, Some(pk.inner)] {
-
                 let addr = Address::p2pkh(&pk, blinder, params);
                 assert_eq!(&addr.to_string(), expected.next().unwrap());
 
@@ -959,6 +1013,5 @@ mod test {
                 assert_eq!(&addr.to_string(), expected.next().unwrap());
             }
         }
-
     }
 }

src/blind.rs

@@ -26,11 +26,11 @@ use secp256k1_zkp::{Generator, RangeProof, Secp256k1, Signing, SurjectionProof};
 
 use crate::{AddressParams, Script, TxIn};
 
+use crate::hashes;
 use crate::{
     confidential::{Asset, AssetBlindingFactor, Nonce, Value, ValueBlindingFactor},
     Address, AssetId, Transaction, TxOut, TxOutWitness,
 };
-use crate::hashes;
 
 /// Transaction Output related errors
 #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
@@ -229,7 +229,11 @@ impl RangeProofMessage {
 }
 
 /// Information about Transaction Input Asset
-#[cfg_attr(feature = "serde", derive(Serialize, Deserialize), serde(crate = "actual_serde"))]
+#[cfg_attr(
+    feature = "serde",
+    derive(Serialize, Deserialize),
+    serde(crate = "actual_serde")
+)]
 #[derive(Debug, PartialEq, Eq, Clone, Copy, Hash)]
 pub struct TxOutSecrets {
     /// Asset