Don't allow constructing unusable serai_client::bitcoin::Address es

processor/src/networks/bitcoin.rs

@@ -134,8 +134,7 @@ impl OutputTrait<Bitcoin> for Output {
 
   fn write<W: io::Write>(&self, writer: &mut W) -> io::Result<()> {
     self.kind.write(writer)?;
-    let presumed_origin: Option<Vec<u8>> =
+    let presumed_origin: Option<Vec<u8>> = self.presumed_origin.clone().map(Into::into);
-      self.presumed_origin.clone().map(|presumed_origin| presumed_origin.try_into().unwrap());
     writer.write_all(&presumed_origin.encode())?;
     self.output.write(writer)?;
     writer.write_all(&u16::try_from(self.data.len()).unwrap().to_le_bytes())?;
@@ -415,7 +414,7 @@ impl Bitcoin {
       .iter()
       .map(|payment| {
         (
-          payment.address.0.clone(),
+          payment.address.clone().into(),
           // If we're solely estimating the fee, don't specify the actual amount
           // This won't affect the fee calculation yet will ensure we don't hit a not enough funds
           // error
@@ -427,7 +426,7 @@ impl Bitcoin {
     match BSignableTransaction::new(
       inputs.iter().map(|input| input.output.clone()).collect(),
       &payments,
-      change.as_ref().map(|change| &change.0),
+      change.as_ref().map(AsRef::as_ref),
       None,
       fee.0,
     ) {
@@ -532,7 +531,8 @@ impl Network for Bitcoin {
   }
 
   fn external_address(key: ProjectivePoint) -> Address {
-    Address(BAddress::<NetworkChecked>::new(BNetwork::Bitcoin, address_payload(key).unwrap()))
+    Address::new(BAddress::<NetworkChecked>::new(BNetwork::Bitcoin, address_payload(key).unwrap()))
+      .unwrap()
   }
 
   fn branch_address(key: ProjectivePoint) -> Address {
@@ -603,17 +603,9 @@ impl Network for Bitcoin {
             }
             tx.unwrap().output.swap_remove(usize::try_from(spent_output.vout).unwrap())
           };
-          BAddress::from_script(&spent_output.script_pubkey, BNetwork::Bitcoin).ok().and_then(
+          BAddress::from_script(&spent_output.script_pubkey, BNetwork::Bitcoin)
-            |address| {
+            .ok()
-              let address = Address(address);
+            .and_then(Address::new)
-              let encoded: Result<Vec<u8>, _> = address.clone().try_into();
-              if encoded.is_ok() {
-                Some(address)
-              } else {
-                None
-              }
-            },
-          )
         };
 
         let output = Output { kind, presumed_origin, output, data };
@@ -802,7 +794,7 @@ impl Network for Bitcoin {
       }],
       output: vec![TxOut {
         value: tx.output[0].value - BAmount::from_sat(10000),
-        script_pubkey: address.0.script_pubkey(),
+        script_pubkey: address.as_ref().script_pubkey(),
       }],
     };
 

substrate/client/src/networks/bitcoin.rs

@@ -12,9 +12,8 @@ use bitcoin::{
 
 type BAddress = BAddressGeneric<NetworkChecked>;
 
-// TODO: Add a new so you can't create an address which can't be encoded
 #[derive(Clone, Eq, Debug)]
-pub struct Address(pub BAddress);
+pub struct Address(BAddress);
 
 impl PartialEq for Address {
   fn eq(&self, other: &Self) -> bool {
@@ -27,11 +26,12 @@ impl PartialEq for Address {
 impl FromStr for Address {
   type Err = Error;
   fn from_str(str: &str) -> Result<Address, Error> {
-    Ok(Address(
+    Address::new(
       BAddressGeneric::from_str(str)
         .map_err(|_| Error::UnrecognizedScript)?
         .require_network(Network::Bitcoin)?,
-    ))
+    )
+    .ok_or(Error::UnrecognizedScript)
   }
 }
 
@@ -77,38 +77,63 @@ impl TryFrom<Vec<u8>> for Address {
   }
 }
 
-#[allow(clippy::from_over_into)]
+fn try_to_vec(addr: &Address) -> Result<Vec<u8>, ()> {
-impl TryInto<Vec<u8>> for Address {
+  Ok(
-  type Error = ();
+    (match addr.0.payload() {
-  fn try_into(self) -> Result<Vec<u8>, ()> {
+      Payload::PubkeyHash(hash) => EncodedAddress::P2PKH(*hash.as_raw_hash().as_byte_array()),
-    Ok(
+      Payload::ScriptHash(hash) => EncodedAddress::P2SH(*hash.as_raw_hash().as_byte_array()),
-      (match self.0.payload() {
+      Payload::WitnessProgram(program) => match program.version() {
-        Payload::PubkeyHash(hash) => EncodedAddress::P2PKH(*hash.as_raw_hash().as_byte_array()),
+        WitnessVersion::V0 => {
-        Payload::ScriptHash(hash) => EncodedAddress::P2SH(*hash.as_raw_hash().as_byte_array()),
+          let program = program.program();
-        Payload::WitnessProgram(program) => match program.version() {
+          if program.len() == 20 {
-          WitnessVersion::V0 => {
+            let mut buf = [0; 20];
-            let program = program.program();
+            buf.copy_from_slice(program.as_ref());
-            if program.len() == 20 {
+            EncodedAddress::P2WPKH(buf)
-              let mut buf = [0; 20];
+          } else if program.len() == 32 {
-              buf.copy_from_slice(program.as_ref());
+            let mut buf = [0; 32];
-              EncodedAddress::P2WPKH(buf)
+            buf.copy_from_slice(program.as_ref());
-            } else if program.len() == 32 {
+            EncodedAddress::P2WSH(buf)
-              let mut buf = [0; 32];
+          } else {
-              buf.copy_from_slice(program.as_ref());
+            Err(())?
-              EncodedAddress::P2WSH(buf)
-            } else {
-              Err(())?
-            }
           }
-          WitnessVersion::V1 => {
+        }
-            let program_ref: &[u8] = program.program().as_ref();
+        WitnessVersion::V1 => {
-            EncodedAddress::P2TR(program_ref.try_into().map_err(|_| ())?)
+          let program_ref: &[u8] = program.program().as_ref();
-          }
+          EncodedAddress::P2TR(program_ref.try_into().map_err(|_| ())?)
-          _ => Err(())?,
+        }
-        },
         _ => Err(())?,
-      })
+      },
-      .encode(),
+      _ => Err(())?,
-    )
+    })
+    .encode(),
+  )
+}
+
+impl From<Address> for Vec<u8> {
+  fn from(addr: Address) -> Vec<u8> {
+    // Safe since only encodable addresses can be created
+    try_to_vec(&addr).unwrap()
+  }
+}
+
+impl From<Address> for BAddress {
+  fn from(addr: Address) -> BAddress {
+    addr.0
+  }
+}
+
+impl AsRef<BAddress> for Address {
+  fn as_ref(&self) -> &BAddress {
+    &self.0
+  }
+}
+
+impl Address {
+  pub fn new(address: BAddress) -> Option<Self> {
+    let res = Self(address);
+    if try_to_vec(&res).is_ok() {
+      return Some(res);
+    }
+    None
   }
 }

tests/full-stack/src/tests/mint_and_burn.rs

@@ -514,7 +514,7 @@ async fn mint_and_burn_test() {
         Coin::Bitcoin,
         1_000_000_00,
         ExternalAddress::new(
-          serai_client::networks::bitcoin::Address(bitcoin_addr.clone()).try_into().unwrap(),
+          serai_client::networks::bitcoin::Address::new(bitcoin_addr.clone()).unwrap().into(),
         )
         .unwrap(),
       )

tests/processor/src/networks.rs

@@ -389,9 +389,9 @@ impl Wallet {
       Wallet::Bitcoin { public_key, .. } => {
         use bitcoin_serai::bitcoin::{Network, Address};
         ExternalAddress::new(
-          networks::bitcoin::Address(Address::p2pkh(public_key, Network::Regtest))
+          networks::bitcoin::Address::new(Address::p2pkh(public_key, Network::Regtest))
-            .try_into()
+            .unwrap()
-            .unwrap(),
+            .into(),
         )
         .unwrap()
       }