-
Notifications
You must be signed in to change notification settings - Fork 224
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 concrete basic types for time, hash, bytes, validator id #51
Conversation
bad27b0
to
35b511a
Compare
lite/src/types.rs
Outdated
/// Size of the underlying Hash in bytes | ||
pub const HASH_LENGTH: usize = 32; | ||
#[derive(Eq, PartialEq)] | ||
pub struct Hash([u8; HASH_LENGTH]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You might consider making this an enum and including the hash algorithm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just realized that how I've ended up in #36 was to not re-define the types here at all but rather re-use the types from the tendermint crate:
https://github.com/interchainio/tendermint-rs/pull/36/files#diff-b1b6c7ff77dbf13838f76b8403715185
That implies to make this (lite) a module instead of a separate crate (see commit message here: d3ce237)
|
This is just the simplest way to move forward implementing the traits of the lite package. There are alternatives: We do not want a create a circular dependency between lite and tendermint (which does not even compile). To avoid this we could: 1) make lite a module of tendermint, or, 2) replicate a lot of the types of the tendermint crate in the lite package (e.g. Time, Ids etc), or 3) have a dependency from tendermint to the lite package only (but then the trait impls do need to live in the lite crate instead with the types in the tendermint crate directly). copied changes over from d3ce237
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Woo thanks!
Fix the core types (bytes, hashes, etc.)
Hope we won't run into any trouble with
&[u8]
(at least in #36 I didn't).ref: #31