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

Refactor CelestiaAddress to fix From<[u8; 32]> #469

Closed
citizen-stig opened this issue Jul 6, 2023 · 0 comments · Fixed by #551
Closed

Refactor CelestiaAddress to fix From<[u8; 32]> #469

citizen-stig opened this issue Jul 6, 2023 · 0 comments · Fixed by #551
Assignees
Labels
Small Use this label for quick cleanup and maintenance tasks

Comments

@citizen-stig
Copy link
Member

citizen-stig commented Jul 6, 2023

Problem

Current implmentation CelestiaAddress:

pub struct CelestiaAddress(pub Vec<u8>);

We store raw ASCII string of celestia address "as is", with HRP and without any decoding:

pub const SEQUENCER_DA_ADDRESS: [u8; 47] = *b"celestia1w7wcupk5gswj25c0khnkey5fwmlndx6t5aarmk";

Length of this string is 47 bytes, and even without HRP("celesita") it is 39 bytes.

Which means that impl From<[u8; 32]> for CelestiaAddress { required by AddressTrait cannot be done with current representation.

Suggested solution 1

Change internal represenation of celestia address to store only decoded Vec<u5> as Vec<u8>. Here's example how it might look like:

TBD:

const HRP: &str = "celestia";
#[derive(Debug, PartialEq, Clone, Eq, Serialize, Deserialize, BorshDeserialize, BorshSerialize)]
struct CelestiaAddressNew([u8; 32]);

impl AsRef<[u8]> for CelestiaAddressNew {
    fn as_ref(&self) -> &[u8] {
        self.0.as_ref()
    }
}

impl<'a> TryFrom<&'a [u8]> for CelestiaAddressNew {
    type Error = anyhow::Error;

    fn try_from(value: &'a [u8]) -> Result<Self, Self::Error> {
        match value.len() {
            // Different cases of ASCII
            39 | 47 => {
                todo!()
                // let raw = if value.len() == 47 {
                //    value[HRP.len()..]
                // } else {
                //     value
                // };
                // let mut result: [u8; 32] = [0; 32];
                // for (i, &item) in value.iter().enumerate() {
                //     let v = bech32::u5::try_from_u8(item)?;
                //     result[i] = v.to_u8();
                // }
                // Ok(Self(result))
            }
            // Raw data
            32 => {
                for &item in value {
                    bech32::u5::try_from_u8(item)?;
                }
                // Should not panic, because len is always 32
                let value: [u8; 32] = value.try_into().unwrap();
                Ok(Self(value))
            }
            l => anyhow::bail!("Unexpected length of slice: {}, expect 32, 47 or 39", l),
        }
    }
}

impl Display for CelestiaAddressNew {
    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
        // Should not panic, as we sanitize input before
        let data: Vec<bech32::u5> = self
            .0
            .iter()
            .map(|&i| bech32::u5::try_from_u8(i).unwrap())
            .collect();
        bech32::encode_to_fmt(f, HRP, &data, Variant::Bech32)
            .map_err(|e| std::fmt::Error::custom(e.to_string()))?
    }
}

impl From<[u8; 32]> for CelestiaAddressNew {
    fn from(value: [u8; 32]) -> Self {
        // TODO: Add checks and panic?
        Self(value)
    }
}

impl FromStr for CelestiaAddressNew {
    type Err = anyhow::Error;

    fn from_str(s: &str) -> Result<Self, Self::Err> {
        let (hrp, raw_address_u5, variant) = bech32::decode(s)?;
        // TODO: Check hrp against hrp
        // TODO: Probably store variant in struct
        let mut value: [u8; 32] = [0; 32];
        println!("FromStr LEN: {}", raw_address_u5.len());
        for (i, raw) in raw_address_u5.iter().enumerate() {
            value[i] = raw.to_u8();
        }
        Ok(Self(value))
    }
}

impl AddressTrait for CelestiaAddressNew {}

#[test]
fn play_compact_address() {
    let raw_address_str = "celestia1w7wcupk5gswj25c0khnkey5fwmlndx6t5aarmk";
    let (hrp, raw_address_u5, variant) = bech32::decode(raw_address_str).unwrap();
    // 32
    println!("L1: {} {:?}", raw_address_u5.len(), raw_address_u5);
    let raw_address_u8 = Vec::<u8>::from_base32(&raw_address_u5).unwrap();
    // 20
    println!("L2: {} {:?}", raw_address_u8.len(), raw_address_u8);
    // 32
    let my_u8: Vec<u8> = raw_address_u5.iter().map(|&u| u.to_u8()).collect();
    println!("L3: {} {:?}", my_u8.len(), my_u8);
    let raw_u8: [u8; 32] = my_u8.try_into().unwrap();

    let address1 = CelestiaAddressNew::from_str(raw_address_str).unwrap();
    let raw_bytes = raw_address_str.as_bytes().to_vec();
    // let address2 = CelestiaAddressNew::try_from(&raw_bytes[..]).unwrap();
    // let address3 = CelestiaAddressNew::try_from(&raw_bytes[HRP.len()..]).unwrap();
    let address4 = CelestiaAddressNew::from(raw_u8);

    // assert_eq!(address1, address2);
    // assert_eq!(address2, address3);
    assert_eq!(address1, address4);

    let output_1 = format!("{}", address1);
    // let output_2 = format!("{}", address2);
    // let output_3 = format!("{}", address3);
    let output_4 = format!("{}", address4);

    assert_eq!(raw_address_str, output_1);
    // assert_eq!(raw_address_str, output_2);
    // assert_eq!(raw_address_str, output_3);
    assert_eq!(raw_address_str, output_4);
}

Suggested solution 2

Change AddressTrait to replace + for<'a> TryFrom<&'a [u8], Error = anyhow::Error> with FromStr

Current usage of this trait essentially using it from string. This way we empathis that AddressTrait can be only 32bytes in "raw" format, or it should be possible to convert it from string.

Another improvement would be to replace From<[u8; 32]> to TryFrom<[u8; 32]> so incorrect conversion would not cause panic.

Second suggestion does not contradict first and they both can be implemented

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Small Use this label for quick cleanup and maintenance tasks
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants