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

Light client: further trait impls, improvements, and tests #63

Closed
wants to merge 56 commits into from

Conversation

liamsi
Copy link
Member

@liamsi liamsi commented Nov 11, 2019

#59 should be merged first (hence a draft)
ref #36

Update: Since this PR was opened (mostly to show the impact of #59), it grew to contain several further PRs added in by @yihuang (big thanks). These added 2 missing impls, resolved a few encoding edge cases and simple (json-based) tests.

Particularly:

Additionally this PR introduces a TrustLevel trait via bd26e74 and adds a verify method that is closer to the (current) spec / go-code in 017c1ba (with a few tests from JSON).

liamsi and others added 30 commits September 18, 2019 17:48
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).
respective amino types, and, directly encode some
 2DC46AD76277039F1B65FE3C7F2064788B1C12FE701CFE7EC93F751586A48781)

will add a test after #42 gets merged
 - Consistency: Rename encode_bytes to bytes_enc
 - remove redundant return statement
 - work in progress as we might want to let an inner function for the
   recursion
 - also we might simply want to use From to consume (or create a Vec)
   instead of changing the method signature (as suggested here:
   #57 (comment))
 direct vector initialization instead of going through &[&[u8]]
liamsi and others added 4 commits November 19, 2019 11:01
Signing bytes need to encoded with encode_length_delimited
and correctly compute header hash when block id is empty.

Represent empty Hash with Option
@liamsi liamsi marked this pull request as ready for review November 27, 2019 21:50
@liamsi liamsi changed the title trait impl Improvements for #36 after #59 Light client: trait impl improvements Nov 27, 2019
@liamsi liamsi changed the title Light client: trait impl improvements Light client: further trait impls & improvements Nov 27, 2019
liamsi and others added 3 commits November 27, 2019 23:37
@liamsi liamsi changed the title Light client: further trait impls & improvements Light client: further trait impls, improvements, and tests Nov 28, 2019
yihuang and others added 8 commits November 29, 2019 00:04
accept iff +1/3 of the voting power signed
…code (lite2) and

tests:

 - rename former verify method to verify_header_and_commit (might be obsolete)
 - make the expired method public to be able to use and test it from the outside
 - add some helpers to be able to deal with deserialization hickups
 - add a bunch of tests using @Shivani912's generator code (with minor modifications)
@liamsi liamsi force-pushed the lite_impl_simple_merkle_merged branch from d2b3bc1 to a53098c Compare December 10, 2019 01:33
@liamsi
Copy link
Member Author

liamsi commented Dec 10, 2019

With the last commit the build started failing with:

 Checking ed25519-dalek v1.0.0-pre.3
error[E0432]: unresolved import `rand`
  --> /usr/local/cargo/registry/src/github.com-1ecc6299db9ec823/ed25519-dalek-1.0.0-pre.3/src/ed25519.rs:14:5
   |
14 | use rand::{CryptoRng, RngCore};
   |     ^^^^ use of undeclared type or module `rand`

Probably related circle-ci catching up with the latest ed25519-dalek release? cc @tarcieri

@liamsi liamsi changed the base branch from lite_impl to master December 10, 2019 10:30
@liamsi
Copy link
Member Author

liamsi commented Dec 10, 2019

This needs rebasing onto master...

…imple_merkle_merged

# Conflicts:
#	tendermint/src/amino_types/block_id.rs
#	tendermint/src/amino_types/vote.rs
#	tendermint/src/block/commit.rs
#	tendermint/src/block/header.rs
#	tendermint/src/genesis.rs
#	tendermint/src/merkle.rs
#	tendermint/src/serializers.rs
#	tendermint/src/validator.rs
#	tendermint/src/vote.rs
@liamsi
Copy link
Member Author

liamsi commented Dec 10, 2019

replaced by #84

@liamsi liamsi closed this Dec 10, 2019
@liamsi liamsi deleted the lite_impl_simple_merkle_merged branch January 19, 2020 17:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants