Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Addresses #27

Merged
merged 3 commits into from
Feb 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .cargo/config.toml
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
[env]
ACCOUNT_ADDRESS_PREFIX = "cosmos"
BECH_32_MAIN_PREFIX = "cosmos"
2 changes: 1 addition & 1 deletion keyring/src/key_pair/secp256k1_key_pair.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ impl Secp256k1KeyPair {
&self,
password: impl AsRef<[u8]>,
) -> k256::elliptic_curve::zeroize::Zeroizing<String> {
// The pkcs8 crate doesn't directly support encrypting with the same scrypt params as openssl.
// TODO: The pkcs8 crate doesn't directly support encrypting with the same scrypt params as openssl.
// The following implementation is a workaround to achieve the same result.
// See https://github.com/RustCrypto/formats/issues/1205
// Once this is fixed, we can replace the following code with:
Expand Down
4 changes: 2 additions & 2 deletions proto-types/build.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use std::{env, error::Error};
fn main() -> Result<(), Box<dyn Error>> {
let account_address_prefix = env::var("ACCOUNT_ADDRESS_PREFIX").map_err(|_| "ACCOUNT_ADDRESS_PREFIX environment variable must be set. This is best done in a .cargo/config.toml file in the root of your project")?;
let account_address_prefix = env::var("BECH_32_MAIN_PREFIX").map_err(|_| "BECH_32_MAIN_PREFIX environment variable must be set. This is best done in a .cargo/config.toml file in the root of your project")?;
println!(
joneskm marked this conversation as resolved.
Show resolved Hide resolved
"cargo:rustc-env=ACCOUNT_ADDRESS_PREFIX={}",
"cargo:rustc-env=BECH_32_MAIN_PREFIX={}",
account_address_prefix
);

Expand Down
171 changes: 103 additions & 68 deletions proto-types/src/address.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,23 +8,32 @@ use serde::{Deserialize, Deserializer, Serialize};

use crate::error::AddressError;

#[derive(Debug, PartialEq, Clone)]
pub struct AccAddress(Vec<u8>);
const BECH_32_PREFIX_ACC_ADDR: &str = env!("BECH_32_MAIN_PREFIX");
const BECH_32_PREFIX_VAL_ADDR: &str = concat!(env!("BECH_32_MAIN_PREFIX"), "valoper");

const ACCOUNT_ADDRESS_PREFIX: &str = env!("ACCOUNT_ADDRESS_PREFIX");
const MAX_ADDR_LEN: u8 = 255;

impl AccAddress {
pub fn into_inner(self) -> Vec<u8> {
self.0
}
pub type AccAddress = BaseAddress<0>;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Newtype pattern instead of simple type alias?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What benefits do we get from using the Newtype pattern?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pub struct BaseAddress<const PREFIX: u8>(Vec<u8>);  

pub struct  AccAddress(  BaseAddress<0> );

impl BaseAddress
{
pub fn from_bech32(address: &str, prefix : u32 ) -> Result<Self, AddressError> {
        let (hrp, data, variant) = bech32::decode(address)?;

// This code could be removed
 //let prefix = if PREFIX == 0 {
//            BECH_32_PREFIX_ACC_ADDR
//        } else {
//            BECH_32_PREFIX_VAL_ADDR
//        };

     if hrp != prefix {
            return Err(AddressError::InvalidPrefix {
                expected: prefix.into(),
                found: hrp,
            });
        };
 // I omitted other code
}


impl AccAddress
{
 pub fn from_bech32(address: &str, ) -> Result<Self, AddressError> {
  Self ( BaseAddress::from_bech32( address, BECH_32_PREFIX_ACC_ADDR )
 }
}

Unsure about other parts of code, but this one method may be simpler.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We would need to do the same for the ValAddress and for the other public methods of AccAddress.

Once more complex const parameter types arrive in Rust, we'll be able to replace u8 with &'static str, see rust-lang/rust#95174. This will allow for much cleaner code, something like:

pub type AccAddress = BaseAddress<BECH_32_PREFIX_ACC_ADDR>;
pub type ValAddress = BaseAddress<BECH_32_PREFIX_VAL_ADDR>;

#[derive(Debug, PartialEq, Clone)]
pub struct BaseAddress<const PREFIX: &'static str>(Vec<u8>);

impl<const PREFIX: &'static str> BaseAddress<PREFIX> {
    pub fn from_bech32(address: &str) -> Result<Self, AddressError> {
        let (hrp, data, variant) = bech32::decode(address)?;

        // This could be removed
        // let prefix = if PREFIX == 0 {
        //     BECH_32_PREFIX_ACC_ADDR
        // } else {
        //     BECH_32_PREFIX_VAL_ADDR
        // };


        if hrp != PREFIX {
            return Err(AddressError::InvalidPrefix {
                expected: PREFIX.into(),
                found: hrp,
            });
        };

I guess there's two questions:

  1. Do we prefer the future const generic method using stror the newtype method?
  2. In the meantime which method should we use?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pub type AccAddress = BaseAddress<BECH_32_PREFIX_ACC_ADDR>;
pub type ValAddress = BaseAddress<BECH_32_PREFIX_VAL_ADDR>;

Looks much better. I didn't think about it.

  1. Depends on how quick this feature will arrive and MSV policy.
  2. Is it really important to use str over [u8]? If yes, then newtype

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO we should use const generics even though we are forced to use a u8

pub type ValAddress = BaseAddress<1>;

// TODO: when more complex const parameter types arrive, replace u8 with &'static str
// https://github.com/rust-lang/rust/issues/95174
#[derive(Debug, PartialEq, Clone)]
pub struct BaseAddress<const PREFIX: u8>(Vec<u8>);

impl<const PREFIX: u8> BaseAddress<PREFIX> {
pub fn from_bech32(address: &str) -> Result<Self, AddressError> {
let (hrp, data, variant) = bech32::decode(address)?;

if hrp != ACCOUNT_ADDRESS_PREFIX {
let prefix = if PREFIX == 0 {
BECH_32_PREFIX_ACC_ADDR
} else {
BECH_32_PREFIX_VAL_ADDR
};

if hrp != prefix {
return Err(AddressError::InvalidPrefix {
expected: ACCOUNT_ADDRESS_PREFIX.into(),
expected: prefix.into(),
found: hrp,
});
};
Expand All @@ -40,13 +49,7 @@ impl AccAddress {
// already returns a Result there's no harm in returning an error here.
let address = Vec::<u8>::from_base32(&data)?;

if address.len() > MAX_ADDR_LEN.into() {
return Err(AddressError::InvalidLength {
max: MAX_ADDR_LEN,
found: address.len(),
});
}

Self::verify_length(&address)?;
Ok(Self(address))
}

Expand All @@ -61,12 +64,25 @@ impl AccAddress {
hex::encode(&self.0)
}

fn verify_length(v: &[u8]) -> Result<(), AddressError> {
if v.len() > MAX_ADDR_LEN.into() {
Err(AddressError::InvalidLength {
max: MAX_ADDR_LEN,
found: v.len(),
})
} else if v.len() == 0 {
Err(AddressError::EmptyAddress)
} else {
Ok(())
}
}

pub fn as_upper_hex(&self) -> String {
data_encoding::HEXUPPER.encode(&self.0)
}
}

impl Serialize for AccAddress {
impl<const PREFIX: u8> Serialize for BaseAddress<PREFIX> {
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
where
S: serde::ser::Serializer,
Expand All @@ -75,19 +91,19 @@ impl Serialize for AccAddress {
}
}

impl<'de> Deserialize<'de> for AccAddress {
fn deserialize<D>(deserializer: D) -> Result<AccAddress, D::Error>
impl<'de, const PREFIX: u8> Deserialize<'de> for BaseAddress<PREFIX> {
fn deserialize<D>(deserializer: D) -> Result<BaseAddress<PREFIX>, D::Error>
where
D: Deserializer<'de>,
{
deserializer.deserialize_str(AccAddressVisitor)
deserializer.deserialize_str(BaseAddressVisitor)
}
}

struct AccAddressVisitor;
struct BaseAddressVisitor<const PREFIX: u8>;

impl<'de> serde::de::Visitor<'de> for AccAddressVisitor {
type Value = AccAddress;
impl<'de, const PREFIX: u8> serde::de::Visitor<'de> for BaseAddressVisitor<PREFIX> {
type Value = BaseAddress<PREFIX>;

fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
formatter.write_str("bech32 encoded address")
Expand All @@ -97,63 +113,56 @@ impl<'de> serde::de::Visitor<'de> for AccAddressVisitor {
where
E: serde::de::Error,
{
AccAddress::from_str(v).map_err(|e| E::custom(format!("invalid address '{}' - {}", v, e)))
BaseAddress::from_str(v).map_err(|e| E::custom(format!("invalid address '{}' - {}", v, e)))
}
}

impl FromStr for AccAddress {
type Err = AddressError;
impl<const PREFIX: u8> Display for BaseAddress<PREFIX> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let hrp = if PREFIX == 0 {
BECH_32_PREFIX_ACC_ADDR
} else {
BECH_32_PREFIX_VAL_ADDR
};
let addr = bech32::encode(hrp, self.0.to_base32(), Variant::Bech32)
.expect("method can only error if HRP is not valid, hard coded HRP is valid");
write!(f, "{}", addr)
}
}

fn from_str(s: &str) -> Result<Self, Self::Err> {
Self::from_bech32(s)
impl<const PREFIX: u8> From<BaseAddress<PREFIX>> for String {
fn from(v: BaseAddress<PREFIX>) -> String {
format!("{}", v)
}
}

impl TryFrom<Vec<u8>> for AccAddress {
type Error = AddressError;
impl<const PREFIX: u8> FromStr for BaseAddress<PREFIX> {
type Err = AddressError;

fn try_from(v: Vec<u8>) -> Result<AccAddress, AddressError> {
if v.len() > MAX_ADDR_LEN.into() {
return Err(AddressError::InvalidLength {
max: MAX_ADDR_LEN,
found: v.len(),
});
}
Ok(AccAddress(v))
fn from_str(s: &str) -> Result<Self, Self::Err> {
Self::from_bech32(s)
}
}

impl TryFrom<&[u8]> for AccAddress {
impl<const PREFIX: u8> TryFrom<Vec<u8>> for BaseAddress<PREFIX> {
type Error = AddressError;

fn try_from(v: &[u8]) -> Result<AccAddress, AddressError> {
if v.len() > MAX_ADDR_LEN.into() {
return Err(AddressError::InvalidLength {
max: MAX_ADDR_LEN,
found: v.len(),
});
}
Ok(AccAddress(v.to_vec()))
fn try_from(v: Vec<u8>) -> Result<BaseAddress<PREFIX>, AddressError> {
Self::verify_length(&v)?;
Ok(BaseAddress(v))
}
}

impl From<AccAddress> for String {
fn from(v: AccAddress) -> String {
bech32::encode(ACCOUNT_ADDRESS_PREFIX, v.0.to_base32(), Variant::Bech32)
.expect("method can only error if HRP is not valid, hard coded HRP is valid")
}
}
impl<const PREFIX: u8> TryFrom<&[u8]> for BaseAddress<PREFIX> {
type Error = AddressError;

impl Display for AccAddress {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let addr = bech32::encode(ACCOUNT_ADDRESS_PREFIX, self.0.to_base32(), Variant::Bech32)
.expect("method can only error if HRP is not valid, hard coded HRP is valid");
write!(f, "{}", addr)
fn try_from(v: &[u8]) -> Result<BaseAddress<PREFIX>, AddressError> {
v.to_vec().try_into()
}
}

impl From<AccAddress> for Vec<u8> {
fn from(v: AccAddress) -> Vec<u8> {
impl<const PREFIX: u8> From<BaseAddress<PREFIX>> for Vec<u8> {
fn from(v: BaseAddress<PREFIX>) -> Vec<u8> {
v.0
}
}
Expand All @@ -169,12 +178,12 @@ mod tests {
fn from_bech32_success() {
let input_address = vec![0x00, 0x01, 0x02];
let encoded = bech32::encode(
ACCOUNT_ADDRESS_PREFIX,
BECH_32_PREFIX_ACC_ADDR,
input_address.to_base32(),
Variant::Bech32,
)
.unwrap();
let expected_address = AccAddress(input_address);
let expected_address = BaseAddress::<0>(input_address);

let address = AccAddress::from_bech32(&encoded).unwrap();

Expand All @@ -185,7 +194,7 @@ mod tests {
fn from_bech32_failure_checksum() {
let input_address = vec![0x00, 0x01, 0x02];
let mut encoded = bech32::encode(
ACCOUNT_ADDRESS_PREFIX,
BECH_32_PREFIX_ACC_ADDR,
input_address.to_base32(),
Variant::Bech32,
)
Expand All @@ -199,8 +208,8 @@ mod tests {

#[test]
fn from_bech32_failure_wrong_prefix() {
let mut hrp = ACCOUNT_ADDRESS_PREFIX.to_string();
hrp.push_str("atom"); // adding to the ACCOUNT_ADDRESS_PREFIX ensures that hrp is different
let mut hrp = BECH_32_PREFIX_ACC_ADDR.to_string();
hrp.push_str("atom"); // adding to the BECH_32_PREFIX_ACC_ADDR ensures that hrp is different
let encoded =
bech32::encode(&hrp, vec![0x00, 0x01, 0x02].to_base32(), Variant::Bech32).unwrap();

Expand All @@ -209,7 +218,7 @@ mod tests {
assert_eq!(
err,
AddressError::InvalidPrefix {
expected: ACCOUNT_ADDRESS_PREFIX.into(),
expected: BECH_32_PREFIX_ACC_ADDR.into(),
found: hrp,
}
);
Expand All @@ -218,7 +227,7 @@ mod tests {
#[test]
fn from_bech32_failure_wrong_variant() {
let encoded = bech32::encode(
ACCOUNT_ADDRESS_PREFIX,
BECH_32_PREFIX_ACC_ADDR,
vec![0x00, 0x01, 0x02].to_base32(),
Variant::Bech32m,
)
Expand All @@ -238,7 +247,7 @@ mod tests {
#[test]
fn from_bech32_failure_too_long() {
let encoded = bech32::encode(
ACCOUNT_ADDRESS_PREFIX,
BECH_32_PREFIX_ACC_ADDR,
vec![0x00; 300].to_base32(),
Variant::Bech32,
)
Expand All @@ -255,6 +264,23 @@ mod tests {
);
}

#[test]
fn from_bech32_failure_empty_address() {
let encoded =
bech32::encode(BECH_32_PREFIX_ACC_ADDR, vec![].to_base32(), Variant::Bech32).unwrap();

let err = AccAddress::from_bech32(&encoded).unwrap_err();

assert_eq!(err, AddressError::EmptyAddress);
}

#[test]
fn from_slice_failure_empty_address() {
let address: Vec<u8> = vec![];
let err = AccAddress::try_from(address.as_slice()).unwrap_err();
assert_eq!(err, AddressError::EmptyAddress);
}

#[test]
fn to_string_success() {
let addr = "cosmos1syavy2npfyt9tcncdtsdzf7kny9lh777pahuux".to_string();
Expand All @@ -264,6 +290,15 @@ mod tests {
assert_eq!(addr, acc_addr.to_string());
}

#[test]
fn string_from_self_success() {
let addr = "cosmos1syavy2npfyt9tcncdtsdzf7kny9lh777pahuux".to_string();

let acc_addr = AccAddress::from_bech32(&addr).unwrap();

assert_eq!(addr, String::from(acc_addr));
}

#[test]
fn serialize_works() {
let addr = "cosmos1syavy2npfyt9tcncdtsdzf7kny9lh777pahuux".to_string();
Expand Down
3 changes: 3 additions & 0 deletions proto-types/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ pub enum AddressError {

#[error("invalid length, max length is: {max:?}, found {found:?})")]
InvalidLength { max: u8, found: usize },

#[error("bech32 decode error: address is empty")]
EmptyAddress,
}

#[derive(Error, Debug, PartialEq, Eq)]
Expand Down
2 changes: 1 addition & 1 deletion proto-types/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ mod decimal256;
mod denom;
mod error;

pub use address::AccAddress;
pub use address::{AccAddress, ValAddress};
pub use decimal256::Decimal256;
pub use denom::Denom;
pub use error::{AddressError, Error};