lite: impl Requester, Store, and a basic syncing daemon #116

ebuchman · 2019-12-29T20:43:43Z

Started in a new crate as per #110 #110 (comment)

Includes a basic test for the Requester against a running tendermint node.

Not really sure how to use async stuff yet so I just copied the block_on thing from the integration tests for now ...

Also includes a basic daemon that uses bisection to sync against a local node from hard coded values - this is working against a my local node.

Referenced an issue explaining the need for the change
Updated all relevant documentation in docs
Updated all code comments where relevant
Wrote tests
Updated CHANGES.md

xla · 2019-12-30T13:00:09Z

@ebuchman As the Requesters async parts will be primarily around I/O async-std might be a better fit here.

This could be seen as a benefit for the codebase at large actually.

tarcieri · 2019-12-30T14:53:23Z

At least for now, async-std is fairly new and immature/rough-around-the-edges, lacking an ecosystem of higher level libraries (e.g. tower, hyper), and at least anecdotally I’ve heard mixed reports about how well using async-std with a tokio reactor actually works in practice.

FWIW, I was interested in async-std for awhile (see e.g. #2 (comment) ) but much of what I liked about it ergonomically also landed in tokio 0.2.

ebuchman · 2019-12-31T17:38:55Z

Merged in #114 and #117.

Note that since #114 was squash merged to master, the original commits from that PR are showing up in this PR's commit history, even though the changes are already on master.

it's not actually a hash! it's whatever is returned by the application, which can be arbitrary bytes.

ebuchman · 2020-01-08T23:46:05Z

Merged in master and fixed conflicts. Also did some minor cleanup.

We can probably merge this more or less as is for now but we'll want to follow up with:

move subjective_init into the lite module, make it generic over the traits, and add the extra validation as noted
fix package structure (some impls for lite are in the new tendermint-lite crate while others are just in tendermint. Maybe they should all be in a module together in tendermint but lite itself should be factored out into its own crate and then we just need a crate for a tendermint-lite-node binary ?
refactor main.rs to be a CLI and take config & args

liamsi · 2020-01-09T00:44:32Z

tendermint-lite/Cargo.toml

+
+[dependencies]
+tendermint = { path = "../tendermint" }
+tokio = "0.2"


Cool that there is (again) a light client crate now. Is the plan to move everything from tendermint/src/lite into this crate too?

I'm starting to think the tendermint/src/lite should be its own crate and the non main stuff here should be in the tendermint crate

OK, I see. For binaries it is good practice to add the Cargo.lock to the repo, too.

liamsi · 2020-01-09T00:46:08Z

tendermint-lite/src/requester.rs

+        assert_eq!(r1.hash(), r2.header().validators_hash());
+    }
+}
+*/


What's up with this? Why is it commented out?

It's an integration test requiring a full node and i wasn't sure how to indicate that ...

I think adding a #[ignore] to the test and adding one sentence about why this is disabled and what it needs to be able to run would be cool.

An example of this is:

https://github.com/interchainio/tendermint-rs/blob/1cc2071cead14533e71a69e8db4051eb0ea09c60/tendermint/tests/integration.rs#L3-L32

tendermint-lite/src/requester.rs

liamsi · 2020-01-09T10:49:00Z

tendermint-lite/Cargo.toml

+# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
+
+[dependencies]
+tendermint = { path = "../tendermint" }


Shouldn't this be a proper dependency instead of a local path?

You can do both...

Suggested change

tendermint = { path = "../tendermint" }

tendermint = { version = "0.11", path = "../tendermint" }

Co-Authored-By: Ismail Khoffi <Ismail.Khoffi@gmail.com>

liamsi

Awesome work! We should merge this as is and address the following in follow-up work; e.g., in the PR that deals with making the consts config params (using abssicca):
- add a comment about why the integration test is commented out (and what is needed to run it)
- update Cargo.toml of the binary crate to point to specific tm version
- add Cargo.lock

ebuchman added 12 commits December 29, 2019 14:10

new verify_and_update_single func

40f374b

verify_and_update funcs

5930e7c

fmt

d8e4120

clippy

ae32577

simplest test for verify_single

a8867d2

test_verify_single_1_val_skip

30796ab

convenience funcs and more tests

30c3850

cleanup and more tests

bd72fc0

fmt

d988689

comments

8f75582

clippy

f8f9e88

impl Requester

b1032cc

ebuchman and others added 16 commits December 30, 2019 10:24

rename

bc12907

cargo

32b6862

impl state

c6783dd

impl store

41518d6

TrustedState impls Clone

4c5d66d

first take at hooking up a node

182c784

fmt

c656854

Please check and clippy

36d4291

Switch to qualified reference for algo

712e673

Shorten height field assignment

7477c9d

consolidate validate_ funcs

93209f4

fixes to bisection func from zarko review

60c8b16

check next validators are correct for sequential

39be631

fix tests

97c9e06

lite json tests use verify_single

525c89f

remove old functions

bb78cf5

ebuchman added 3 commits December 31, 2019 12:43

compile fix

0738b24

fix header.app_hash encoding

a8d6946

it's not actually a hash! it's whatever is returned by the application, which can be arbitrary bytes.

only try bisection on higher heights

93edf08

ebuchman changed the title ~~impl Requester~~ impl Requester, Store, and a basic syncing daemon Dec 31, 2019

ebuchman changed the title ~~impl Requester, Store, and a basic syncing daemon~~ lite: impl Requester, Store, and a basic syncing daemon Dec 31, 2019

ebuchman changed the base branch from bucky/lite-new-unit-tests to master December 31, 2019 22:24

ebuchman mentioned this pull request Dec 31, 2019

Light Client Daemon #112

Closed

ebuchman added 8 commits January 8, 2020 17:48

Merge branch 'master' into bucky/lite-reqester-store-impl

21ec189

fix parse_hex

4a9aaaa

comments about time

cc4d565

remove printlns

4ed5d8c

use latest_commit

f99ea6e

Requester -> RPCRequester

c46fade

fix

542e022

comment out integration test

6a068d4

ebuchman marked this pull request as ready for review January 8, 2020 23:46

fmt

53c1b30

liamsi reviewed Jan 9, 2020

View reviewed changes

tendermint-lite/src/requester.rs Show resolved Hide resolved

liamsi reviewed Jan 9, 2020

View reviewed changes

tendermint-lite/src/requester.rs Outdated Show resolved Hide resolved

liamsi reviewed Jan 9, 2020

View reviewed changes

tendermint-lite/src/requester.rs Outdated Show resolved Hide resolved

liamsi reviewed Jan 9, 2020

View reviewed changes

Apply suggestions from code review

e733095

Co-Authored-By: Ismail Khoffi <Ismail.Khoffi@gmail.com>

liamsi approved these changes Jan 9, 2020

View reviewed changes

liamsi merged commit a6dcab4 into master Jan 9, 2020

liamsi deleted the bucky/lite-reqester-store-impl branch January 9, 2020 17:36

liamsi mentioned this pull request Mar 6, 2020

Abscissify light node #125

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

lite: impl Requester, Store, and a basic syncing daemon #116

lite: impl Requester, Store, and a basic syncing daemon #116

ebuchman commented Dec 29, 2019 •

edited

Loading

xla commented Dec 30, 2019 •

edited

Loading

tarcieri commented Dec 30, 2019

ebuchman commented Dec 31, 2019 •

edited

Loading

ebuchman commented Jan 8, 2020

liamsi Jan 9, 2020

ebuchman Jan 9, 2020

liamsi Jan 9, 2020

liamsi Jan 9, 2020

ebuchman Jan 9, 2020

liamsi Jan 9, 2020

liamsi Jan 9, 2020

tarcieri Jan 9, 2020

liamsi left a comment •

edited

Loading

	tendermint = { path = "../tendermint" }
	tendermint = { version = "0.11", path = "../tendermint" }

lite: impl Requester, Store, and a basic syncing daemon #116

lite: impl Requester, Store, and a basic syncing daemon #116

Conversation

ebuchman commented Dec 29, 2019 • edited Loading

xla commented Dec 30, 2019 • edited Loading

tarcieri commented Dec 30, 2019

ebuchman commented Dec 31, 2019 • edited Loading

ebuchman commented Jan 8, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liamsi left a comment • edited Loading

Choose a reason for hiding this comment

ebuchman commented Dec 29, 2019 •

edited

Loading

xla commented Dec 30, 2019 •

edited

Loading

ebuchman commented Dec 31, 2019 •

edited

Loading

liamsi left a comment •

edited

Loading