From 35b511a4c65700bc6934429096a63ed822067baa Mon Sep 17 00:00:00 2001 From: Ismail Khoffi Date: Tue, 29 Oct 2019 20:06:25 +0100 Subject: [PATCH 1/5] Use concrete basic types for time, hash, bytes, validator id --- lite/src/types.rs | 21 ++++++++++++--------- lite/src/verifier.rs | 3 ++- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/lite/src/types.rs b/lite/src/types.rs index f1e5ab43f..f1af7ca59 100644 --- a/lite/src/types.rs +++ b/lite/src/types.rs @@ -1,3 +1,4 @@ +use std::time::{Instant}; /// TrustedState stores the latest state trusted by a lite client, /// including the last header and the validator set to use to verify /// the next header. @@ -10,12 +11,14 @@ where pub validators: V, // height H } -/// Need to do something better here :) pub type Height = u64; -pub type Hash = u64; // TODO -pub type Time = u64; // TODO -pub type Bytes = u64; // TODO -pub type ValID = u64; // TODO +/// Size of the underlying Hash in bytes +pub const HASH_LENGTH: usize = 32; +#[derive(Eq, PartialEq)] +pub struct Hash([u8; HASH_LENGTH]); +/// Size of a validator ID in bytes +pub const VAL_ID_LENGTH: usize = 20; +pub struct ValID([u8; VAL_ID_LENGTH]); /// Header contains meta data about the block - /// the height, the time, the hash of the validator set @@ -23,7 +26,7 @@ pub type ValID = u64; // TODO /// set that should sign the next header. pub trait Header { fn height(&self) -> Height; - fn bft_time(&self) -> Time; + fn bft_time(&self) -> Instant; fn validators_hash(&self) -> Hash; fn next_validators_hash(&self) -> Hash; @@ -60,7 +63,7 @@ pub trait ValidatorSetLookup: ValidatorSet { /// to its public key material to verify signatures. pub trait Validator { fn power(&self) -> u64; - fn verify_signature(&self, sign_bytes: Bytes, signature: Bytes) -> bool; + fn verify_signature(&self, sign_bytes: &[u8], signature: &[u8]) -> bool; } /// Commit is proof a Header is valid. @@ -93,8 +96,8 @@ pub trait Commit { /// is only necessary to avoid slashing in the multi chain context. pub trait Vote { fn validator_id(&self) -> ValID; - fn sign_bytes(&self) -> Bytes; - fn signature(&self) -> Bytes; + fn sign_bytes(&self) -> &[u8]; + fn signature(&self) -> &[u8]; } pub enum Error { diff --git a/lite/src/verifier.rs b/lite/src/verifier.rs index 91d0ebe6a..45104b9d2 100644 --- a/lite/src/verifier.rs +++ b/lite/src/verifier.rs @@ -1,11 +1,12 @@ use crate::types::*; +use std::time::{Duration, Instant}; /// Returns an error if the header has expired according to the given /// trusting_period and current time. If so, the verifier must be reset subjectively. /// NOTE: this doesn't belong here. It should be called by something that handles whether to trust /// a verifieds commit. Verified here is really just about the header/commit/validators. Time is an /// external concern :) -fn expired(last_header: &H, trusting_period: Time, now: Time) -> Result<(), Error> +fn expired(last_header: &H, trusting_period: Duration, now: Instant) -> Result<(), Error> where H: Header, { From f07f97d84deaf7b4d257c962cb7369577ce7ef63 Mon Sep 17 00:00:00 2001 From: Ismail Khoffi Date: Tue, 29 Oct 2019 20:13:29 +0100 Subject: [PATCH 2/5] cargo fmt --- lite/src/types.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lite/src/types.rs b/lite/src/types.rs index f1af7ca59..d911f4c91 100644 --- a/lite/src/types.rs +++ b/lite/src/types.rs @@ -1,4 +1,4 @@ -use std::time::{Instant}; +use std::time::Instant; /// TrustedState stores the latest state trusted by a lite client, /// including the last header and the validator set to use to verify /// the next header. From b5d0a372a78620f450404b089989ac2126599296 Mon Sep 17 00:00:00 2001 From: Ismail Khoffi Date: Tue, 29 Oct 2019 20:29:14 +0100 Subject: [PATCH 3/5] make the lite client a module instead of a separate crate: 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 https://github.com/interchainio/tendermint-rs/commit/d3ce237b13e6289b44a92f807ebec0fa8a5b7d00 --- Cargo.toml | 1 - lite/Cargo.toml | 7 ------ lite/src/lib.rs | 2 -- tendermint/Cargo.toml | 2 +- tendermint/src/block/header.rs | 25 +++++++++++++++++-- tendermint/src/lib.rs | 2 ++ tendermint/src/lite.rs | 4 +++ {lite/src => tendermint/src/lite}/types.rs | 22 +++++++--------- {lite/src => tendermint/src/lite}/verifier.rs | 22 ++++++++++------ 9 files changed, 53 insertions(+), 34 deletions(-) delete mode 100644 lite/Cargo.toml delete mode 100644 lite/src/lib.rs create mode 100644 tendermint/src/lite.rs rename {lite/src => tendermint/src/lite}/types.rs (89%) rename {lite/src => tendermint/src/lite}/verifier.rs (88%) diff --git a/Cargo.toml b/Cargo.toml index 77f2fb737..40857dd30 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -2,5 +2,4 @@ members = [ "tendermint", - "lite", ] diff --git a/lite/Cargo.toml b/lite/Cargo.toml deleted file mode 100644 index 8a66b287b..000000000 --- a/lite/Cargo.toml +++ /dev/null @@ -1,7 +0,0 @@ -[package] -name = "lite" -version = "0.1.0" -authors = ["Ethan Buchman "] -edition = "2018" - -[dependencies] diff --git a/lite/src/lib.rs b/lite/src/lib.rs deleted file mode 100644 index 8398f513e..000000000 --- a/lite/src/lib.rs +++ /dev/null @@ -1,2 +0,0 @@ -mod types; -mod verifier; diff --git a/tendermint/Cargo.toml b/tendermint/Cargo.toml index a7921321d..c7930d0da 100644 --- a/tendermint/Cargo.toml +++ b/tendermint/Cargo.toml @@ -25,7 +25,7 @@ authors = [ ] [badges] -circle-ci = { repository = "tendermint/kms" } +circle-ci = { repository = "interchainio/tendermint-rs" } [dependencies] byteorder = { version = "1.2" } diff --git a/tendermint/src/block/header.rs b/tendermint/src/block/header.rs index c609bd05a..fa7372b01 100644 --- a/tendermint/src/block/header.rs +++ b/tendermint/src/block/header.rs @@ -1,6 +1,5 @@ //! Block headers - -use crate::{account, block, chain, Hash, Time}; +use crate::{account, block, chain, lite, Hash, Time}; use { crate::serializers, serde::{Deserialize, Serialize}, @@ -70,6 +69,28 @@ pub struct Header { pub proposer_address: account::Id, } +impl lite::Header for Header { + fn height(&self) -> block::Height { + self.height + } + + fn bft_time(&self) -> Time { + self.time + } + + fn validators_hash(&self) -> Hash { + self.validators_hash + } + + fn next_validators_hash(&self) -> Hash { + unimplemented!() + } + + fn hash(&self) -> Hash { + unimplemented!() + } +} + /// `Version` contains the protocol version for the blockchain and the /// application. /// diff --git a/tendermint/src/lib.rs b/tendermint/src/lib.rs index 08c765150..227a9893a 100644 --- a/tendermint/src/lib.rs +++ b/tendermint/src/lib.rs @@ -36,6 +36,8 @@ pub mod consensus; pub mod evidence; pub mod genesis; pub mod hash; +#[allow(dead_code, missing_docs)] +pub mod lite; pub mod merkle; mod moniker; pub mod net; diff --git a/tendermint/src/lite.rs b/tendermint/src/lite.rs new file mode 100644 index 000000000..283bd2e99 --- /dev/null +++ b/tendermint/src/lite.rs @@ -0,0 +1,4 @@ +pub use self::types::*; +pub mod types; +pub mod verifier; +pub use self::verifier::*; diff --git a/lite/src/types.rs b/tendermint/src/lite/types.rs similarity index 89% rename from lite/src/types.rs rename to tendermint/src/lite/types.rs index d911f4c91..ab619c9fd 100644 --- a/lite/src/types.rs +++ b/tendermint/src/lite/types.rs @@ -1,4 +1,9 @@ -use std::time::Instant; +use crate::account::Id; +use crate::block::Height; +use crate::Hash; +#[allow(clippy::all)] +use crate::Time; + /// TrustedState stores the latest state trusted by a lite client, /// including the last header and the validator set to use to verify /// the next header. @@ -11,22 +16,13 @@ where pub validators: V, // height H } -pub type Height = u64; -/// Size of the underlying Hash in bytes -pub const HASH_LENGTH: usize = 32; -#[derive(Eq, PartialEq)] -pub struct Hash([u8; HASH_LENGTH]); -/// Size of a validator ID in bytes -pub const VAL_ID_LENGTH: usize = 20; -pub struct ValID([u8; VAL_ID_LENGTH]); - /// Header contains meta data about the block - /// the height, the time, the hash of the validator set /// that should sign this header, and the hash of the validator /// set that should sign the next header. pub trait Header { fn height(&self) -> Height; - fn bft_time(&self) -> Instant; + fn bft_time(&self) -> Time; fn validators_hash(&self) -> Hash; fn next_validators_hash(&self) -> Hash; @@ -55,7 +51,7 @@ pub trait ValidatorSet { /// ValidatorSetLookup allows validator to be fetched via their ID /// (ie. their address). pub trait ValidatorSetLookup: ValidatorSet { - fn validator(&self, val_id: ValID) -> Option; + fn validator(&self, val_id: Id) -> Option; } /// Validator has a voting power and can verify @@ -95,7 +91,7 @@ pub trait Commit { /// within an enum at the VoteSet level indicating which block it is for, and the chain id /// is only necessary to avoid slashing in the multi chain context. pub trait Vote { - fn validator_id(&self) -> ValID; + fn validator_id(&self) -> Id; fn sign_bytes(&self) -> &[u8]; fn signature(&self) -> &[u8]; } diff --git a/lite/src/verifier.rs b/tendermint/src/lite/verifier.rs similarity index 88% rename from lite/src/verifier.rs rename to tendermint/src/lite/verifier.rs index 45104b9d2..856c4341e 100644 --- a/lite/src/verifier.rs +++ b/tendermint/src/lite/verifier.rs @@ -1,18 +1,24 @@ -use crate::types::*; -use std::time::{Duration, Instant}; +#[allow(clippy::all)] +use crate::lite::{Commit, Error, Header, Validator, ValidatorSet, ValidatorSetLookup, Vote}; +use crate::Time; +use std::time::Duration; /// Returns an error if the header has expired according to the given /// trusting_period and current time. If so, the verifier must be reset subjectively. /// NOTE: this doesn't belong here. It should be called by something that handles whether to trust -/// a verifieds commit. Verified here is really just about the header/commit/validators. Time is an +/// a verified commit. Verified here is really just about the header/commit/validators. Time is an /// external concern :) -fn expired(last_header: &H, trusting_period: Duration, now: Instant) -> Result<(), Error> +fn expired(_last_header: &H, _trusting_period: Duration, _now: Time) -> Result<(), Error> where H: Header, { - if last_header.bft_time() + trusting_period < now { - return Err(Error::Expired); - } + // if let Ok(passed) = now.duration_since(last_header.bft_time()) { + // if passed > trusting_period { + // return Err(Error::Expired); + // } + // } + // TODO move this out of the verifier + // - uncomment above logic but also deal with overflows etc (proper err handling) Ok(()) } @@ -107,7 +113,7 @@ where return Err(Error::InvalidCommitLength); } - // The vals and commit have a 1-to-1 correspondance. + // The vals and commit have a 1-to-1 correspondence. // This means we don't need the validator IDs or to do any lookup, // we can just zip the iterators. for (val, vote_opt) in vals_iter.zip(commit_iter) { From 8dcc1e4694b74781974eac54f7012cd7637d7ac1 Mon Sep 17 00:00:00 2001 From: Ismail Khoffi Date: Tue, 29 Oct 2019 20:40:57 +0100 Subject: [PATCH 4/5] clippy: ineffective operation `total_power * 1` --- tendermint/src/lite/verifier.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tendermint/src/lite/verifier.rs b/tendermint/src/lite/verifier.rs index 856c4341e..ccc9242c5 100644 --- a/tendermint/src/lite/verifier.rs +++ b/tendermint/src/lite/verifier.rs @@ -183,7 +183,7 @@ where // check the signers account for +1/3 of the voting power // TODO: incorporate "trust_level" in here to possibly increase // beyond 1/3. - if signed_power * 3 <= total_power * 1 { + if signed_power * 3 <= total_power { return Err(Error::InsufficientVotingPower); } From b23d745ba6f85c85ed9ee6f43bc0fcd7653fea40 Mon Sep 17 00:00:00 2001 From: Ismail Khoffi Date: Tue, 29 Oct 2019 20:53:41 +0100 Subject: [PATCH 5/5] uncomment `expired` logic --- tendermint/src/lite/verifier.rs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/tendermint/src/lite/verifier.rs b/tendermint/src/lite/verifier.rs index ccc9242c5..f178a4101 100644 --- a/tendermint/src/lite/verifier.rs +++ b/tendermint/src/lite/verifier.rs @@ -8,17 +8,16 @@ use std::time::Duration; /// NOTE: this doesn't belong here. It should be called by something that handles whether to trust /// a verified commit. Verified here is really just about the header/commit/validators. Time is an /// external concern :) -fn expired(_last_header: &H, _trusting_period: Duration, _now: Time) -> Result<(), Error> +fn expired(last_header: &H, trusting_period: Duration, now: Time) -> Result<(), Error> where H: Header, { - // if let Ok(passed) = now.duration_since(last_header.bft_time()) { - // if passed > trusting_period { - // return Err(Error::Expired); - // } - // } - // TODO move this out of the verifier - // - uncomment above logic but also deal with overflows etc (proper err handling) + if let Ok(passed) = now.duration_since(last_header.bft_time()) { + if passed > trusting_period { + return Err(Error::Expired); + } + } + // TODO move this out of the verifier and deal with overflows etc (proper err handling) Ok(()) }