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

Use a single crate/implementation for handling hex-encoding/decoding #1554

Open
Fraser999 opened this issue Sep 24, 2024 · 1 comment
Open
Labels
ignore-stale Override for issues or PRs which should not be removed if stale. tech-debt

Comments

@Fraser999
Copy link
Contributor

Fraser999 commented Sep 24, 2024

We currently have a mixture of hand-rolled encoders and usage of the hex crate.

We should check the performance of hex-conservative and use it exclusively throughout our codebase if it's performant enough, or if not, we should just avoid using any third-party crates for this and extend our own hand-rolled implementation to cover all our needs.

Ensure that when using this to display a byte array we make use of appropriate non-allocating functionality provided by hex-conservative crate.

Relevant code paths in this repo:

  • astria-telemetry:
    pub fn hex<T: AsRef<[u8]>>(bytes: T) -> Hex<T> {
    Hex(bytes)
    }
    /// A newtype wrapper of a byte slice that implements [`std::fmt::Display`].
    ///
    /// To be used in tracing contexts. See the [`self::hex`] utility.
    #[derive(SerializeDisplay)]
    pub struct Hex<T>(T);
    impl<T> Display for Hex<T>
    where
    T: AsRef<[u8]>,
    {
    fn fmt(&self, f: &mut Formatter<'_>) -> Result {
    for byte in self.0.as_ref() {
    f.write_fmt(format_args!("{byte:02x}"))?;
    }
    Ok(())
    }
    }
  • astria-sequencer (will be removed with chore(sequencer)!: put blocks and deposits to non-verified storage (ENG-812) #1525):
    pub(crate) struct Hex<'a>(pub(crate) &'a [u8]);
    impl<'a> std::fmt::Display for Hex<'a> {
    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
    for byte in self.0 {
    f.write_fmt(format_args!("{byte:02x}"))?;
    }
    Ok(())
    }
    }

┆Issue Number: ENG-841

@joroshiba
Copy link
Member

This issue is stale because it has been open 45 days with no activity. Remove stale label or this issue
be closed in 7 days.

@Fraser999 Fraser999 added ignore-stale Override for issues or PRs which should not be removed if stale. and removed ignore-stale Override for issues or PRs which should not be removed if stale. stale labels Dec 16, 2024
@joroshiba joroshiba added tech-debt ignore-stale Override for issues or PRs which should not be removed if stale. labels Jan 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ignore-stale Override for issues or PRs which should not be removed if stale. tech-debt
Projects
None yet
Development

No branches or pull requests

2 participants