Add light client spec by milosevic · Pull Request #1336 · tendermint/tendermint

milosevic · 2018-03-19T15:49:27Z

No description provided.

codecov-io · 2018-03-19T15:57:33Z

Codecov Report

Merging #1336 into develop will decrease coverage by 0.04%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop    #1336      +/-   ##
===========================================
- Coverage    61.48%   61.43%   -0.05%     
===========================================
  Files          114      114              
  Lines        10988    10988              
===========================================
- Hits          6756     6751       -5     
- Misses        3585     3587       +2     
- Partials       647      650       +3

Impacted Files	Coverage Δ
rpc/client/httpclient.go	`67.53% <0%> (-1.05%)`	⬇️
consensus/reactor.go	`72.15% <0%> (-0.42%)`	⬇️
consensus/state.go	`76.75% <0%> (-0.41%)`	⬇️
p2p/conn/connection.go	`82.33% <0%> (-0.29%)`	⬇️
blockchain/pool.go	`69.44% <0%> (+1.38%)`	⬆️

melekes · 2018-03-20T09:09:58Z

docs/specification/new-spec/light-client.md

+    if isValid(signedHeader) then
+      setValidatorSet(signedHeader)
+    else return error
+  return


missed closing "```"

xla · 2018-03-20T09:16:08Z

docs/specification/new-spec/light-client.md

+```
+
+We assume that Tendermint keeps track of the validator set changes and that each time a validator set is changed it is 
+being assigned the next sequence number. We can this number the validator set sequence number. Tendermint also remembers 


We can this number the validator set sequence number. is very funky, probably needs to be grammatically corrected.

It's a typo. It should be " We call this number the validator set sequence number".

"We can call this number ..."

xla · 2018-03-20T09:18:16Z

docs/specification/new-spec/light-client.md

+validator set (modified based on transactions from block H) will be in block `H+1` (and signed by the current validator 
+set), and then starting from the block `H+2`, it will be signed by the next validator set.    
+
+Note that the real Tendermint RPC api is slightly different (for example, response messages contains more data and function 


API should be in caps as it is an acronym.

contains -> contain.

xla · 2018-03-20T09:22:56Z

docs/specification/new-spec/light-client.md

+valSetNumber int64     // sequence number of the current validator set 
+valSetHash []byte      // hash of the current validator set
+valSetTime int64       // time when the current validator set is initialised  
+```


Improved readbility:

valSet []Validator // current validator set (last known and verified validator set) valSetNumber int64 // sequence number of the current validator set valSetHash []byte // hash of the current validator set valSetTime int64 // time when the current validator set is initialised

xla · 2018-03-20T09:26:13Z

docs/specification/new-spec/light-client.md

+    Validators  []Validator 
+    ValSetTime int64               // time the current validator set is initialised, i.e, time of the last validator change before header BlockHeight 
+}
+```


Improve formatting:

type SignedHeader struct { Header Header Commit Commit ValSetNumber int64 } type ResultValidators struct { BlockHeight int64 Validators []Validator // time the current validator set is initialised, i.e, time of the last validator change before header BlockHeight ValSetTime int64 }

xla · 2018-03-20T09:28:48Z

docs/specification/new-spec/light-client.md

+    if isValid(signedHeader) then
+      setValidatorSet(signedHeader)
+    else return error
+  return


xla · 2018-03-20T09:30:28Z

docs/specification/new-spec/light-client.md

+and 2) update the validator set (when the given header is valid and it is more recent than the seen headers). 
+
+```golang
+Certify(signedHeader SignedHeader):


I know this is just a description of the current working of the light client, but Certify seems to conflate too many things, validation and mutation namely, while the name does not communicate that.

This can be seen more as a description of core mechanisms of the light client. Current implementation is quite different. I agree that this is not the best name and that updating validator set is a side effect. My idea was just to have Certify as an entry point where we show how everything works together. The same underlying mechanisms can be used in the context of IBC, pooled security architecture and the light client. Do you have a suggestion how we organise the content differently?

Something that would imply mutation could be a good start e.g.: Update

xla · 2018-03-20T13:54:06Z

docs/specification/new-spec/light-client.md

+    return true
+  else
+    updateValidatorSet(signedHeader.ValSetNumber)
+    return Certify(signedHeader)


This is a recursive call, correct? What happened if the subsequent call to Certify fails even so updateValidatorSet has been called already?

I think that it's not a problem in updating validator set and failing to check next header. With a correct FullNode this should never happen. In general, any call to the FullNode might fail or some of subsequent check could fail. At that point Certify function should fail and light client should decide how to deal with it, for example to connect to other FullNode. I added a note at the end.

Gotcha, thanks for the clarification. Not sure if it was made explicit somehwere what the guarantees are for the light client.

We should make guarantees explicit, this is a first step to agree on the basic mechanisms.

odeke-em · 2018-03-23T23:50:46Z

docs/specification/new-spec/light-client.md

+blockchain header, and further the corresponding Merkle proofs. 
+
+For the purpose of this light client specification, we assume that the Tendermint Full Node exposes the following functions over
+Tendermint RPC:


What's Tendermint RPC?

http://tendermint.readthedocs.io/projects/tools/en/master/specification/rpc.html

ethanfrey · 2018-03-26T16:31:46Z

docs/specification/new-spec/light-client.md

+type SignedHeader struct {
+    Header        Header 
+    Commit        Commit
+    ValSetNumber  int64 


Why is this number needed?
You return it on the signed header, but you don't return it on ResultValidator.

Strictly speaking, it is not needed, but it emphasises that validator set changes are first class citizens. It also simplifies supporting applications that require to strictly track validator set changes. Light client logic also seems simpler. The cost of supporting it does not seem high. We could also add this number in ResultValidator for symmetry.

ebuchman · 2018-04-03T20:02:25Z

I think we need to fix the condition re the Unbonding period (if its longer than the unbonding period, we have to revert to social consensus, we cant trust Tendermint nodes!).

Otherwise I think it looks good.

Notation could be a bit better re mutable state but good enough to merge for now.

milosevic requested review from ebuchman and melekes as code owners March 19, 2018 15:49

xla assigned milosevic Mar 19, 2018

martindale requested review from mappum and mossid March 19, 2018 18:42

melekes reviewed Mar 20, 2018

View reviewed changes

melekes approved these changes Mar 20, 2018

View reviewed changes

xla suggested changes Mar 20, 2018

View reviewed changes

milosevic force-pushed the zarko/1308-add-light-client-spec branch from 9a20f10 to 1660411 Compare March 20, 2018 13:50

xla reviewed Mar 20, 2018

View reviewed changes

Add light client spec

416f03c

milosevic force-pushed the zarko/1308-add-light-client-spec branch from 1660411 to 416f03c Compare March 21, 2018 09:00

xla approved these changes Mar 21, 2018

View reviewed changes

odeke-em reviewed Mar 23, 2018

View reviewed changes

ethanfrey reviewed Mar 26, 2018

View reviewed changes

ebuchman merged commit 991017f into develop Apr 3, 2018

ebuchman deleted the zarko/1308-add-light-client-spec branch April 3, 2018 20:02

ebuchman mentioned this pull request Apr 3, 2018

Fixes to and for Light Client Spec #1413

Closed

4 tasks

firelizzard18 pushed a commit to AccumulateNetwork/tendermint that referenced this pull request Nov 24, 2023

Added .md extension to vote extensions changelog (tendermint#1336)

8d8dbc2

Conversation

milosevic commented Mar 19, 2018

Uh oh!

codecov-io commented Mar 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

milosevic Mar 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

milosevic Mar 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ebuchman commented Apr 3, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

codecov-io commented Mar 19, 2018 •

edited

Loading

milosevic Mar 21, 2018 •

edited

Loading

milosevic Mar 27, 2018 •

edited

Loading