[ADR - 42] State-sync by tac0turtle · Pull Request #3769 · tendermint/tendermint

tac0turtle · 2019-07-02T15:25:48Z

Initial State-sync discussions to be held in this PR. This is not the final stage of the PR.

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

docs/architecture/adr-042-state-sync.md

AdityaSripal · 2019-07-02T16:44:37Z

Would be useful to have a Security Models section that compares the Warp-Sync security model with a standard light-client security model. Discussion on how choosing one security model or the other also has trade-offs in performance/simplicity/etc. would be helpful

docs/architecture/adr-042-state-sync.md

codecov-io · 2019-07-04T16:35:28Z

Codecov Report

Merging #3769 into master will decrease coverage by 0.1%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #3769      +/-   ##
==========================================
- Coverage   64.98%   64.87%   -0.11%     
==========================================
  Files         217      217              
  Lines       18211    18189      -22     
==========================================
- Hits        11835    11801      -34     
- Misses       5458     5470      +12     
  Partials      918      918

Impacted Files	Coverage Δ
blockchain/v0/reactor.go	`73.11% <0%> (-6.14%)`	⬇️
consensus/reactor.go	`71.04% <0%> (-1.06%)`	⬇️
blockchain/v0/pool.go	`80.65% <0%> (-0.99%)`	⬇️
consensus/state.go	`79.95% <0%> (-0.12%)`	⬇️
p2p/pex/pex_reactor.go	`83.18% <0%> (+1.15%)`	⬆️
privval/signer_listener_endpoint.go	`89.13% <0%> (+2.17%)`	⬆️
privval/socket_listeners.go	`88.88% <0%> (+2.68%)`	⬆️
privval/signer_endpoint.go	`84% <0%> (+5.33%)`	⬆️

ebuchman

Wow this is so much clearer, thank you @brapse !

docs/architecture/adr-042-state-sync.md

cryptom-dev · 2019-07-05T03:12:43Z

docs/architecture/adr-042-state-sync.md

+[tendermint state sync proposal 2](https://docs.google.com/document/d/1npGTAa1qxe8EQZ1wG0a0Sip9t5oX2vYZNUDwr_LVRR4/edit) - ackratos proposal
+[proposal 2 implementation](https://github.com/tendermint/tendermint/pull/3243)  - ackratos implementation
+[WIP General/Lazy State-Sync pseudo-spec](https://github.com/tendermint/tendermint/issues/3639) - Jae Proposal
+[Warp Sync Implementation](https://github.com/tendermint/tendermint/pull/3594) - ackratos


we have some more stuff:

document for this enhancement: [R4R] BEP-18: for state sync enhancement bnb-chain/BEPs#18, included more chunking strategy we adopted

to try out state sync in binance chain prod and testnet: https://github.com/binance-chain/docs-site/blob/master/docs/fullnode.md#state-sync

docs/architecture/adr-042-state-sync.md

milosevic · 2019-07-05T11:46:52Z

docs/architecture/adr-042-state-sync.md

+[#3639](https://github.com/tendermint/tendermint/issues/3639) where chunking
+happens lazily and in a dynamic way: nodes request key ranges from their peers,
+and peers respond with some subset of the
+requested range and with notes on how to request the rest in parallel from other


This part of this design is super tricky to implement correctly in my view as we ask set of peers and each of them come back with notes how to download rest.

docs/architecture/adr-042-state-sync.md

brapse · 2019-07-05T14:29:54Z

Would be useful to have a Security Models section that compares the Warp-Sync security model with a standard light-client security model. Discussion on how choosing one security model or the other also has trade-offs in performance/simplicity/etc. would be helpful

https://github.com/tendermint/tendermint/blob/834ecd53da78433dd4e0a6a30944d7b964ae0444/docs/architecture/adr-042-state-sync.md#compairing-security-models

Is this what you had in mind @AdityaSripal ?

docs/architecture/adr-042-state-sync.md

AdityaSripal · 2019-07-05T17:29:02Z

docs/architecture/adr-042-state-sync.md

+WarpSync with per chunk light client validation might get the optimal
+performance and safety.
+
+## Decision: Eager StateSync With Per Chunk Light Client Validation


Would this require bringing manifest hash into consensus?

Will we only require validator nodes to compute and validate this hash?

No it would not, the application state would be validated against the merkle root produced from light client sync.

Co-Authored-By: Aditya <adityasripal@gmail.com>

docs/architecture/adr-042-state-sync.md

cwgoes · 2019-07-15T19:49:13Z

docs/architecture/adr-042-state-sync.md

+* Speed: Expected throughput of producing and consuming snapshots
+* Safety: Cost of pushing invalid snapshots to a node
+* Liveness: Cost of preventing a node from receiving/constructing a snapshot
+* Effort: How much effort does an implementation require


Also versatility - whether any additional requirements are imposed upon the ABCI application about e.g. storage layout.

Would you say that could somehow be captured by engineering effort or should it be a distinct dimension?

cwgoes · 2019-07-15T19:50:36Z

docs/architecture/adr-042-state-sync.md

+    * Complete snapshot 
+    * Ordered IAVL key ranges
+    * Compressed individually chunks which can be validated
+* How is data validated


Are these intended to be mutually exclusive? There's a continuum w.r.t. validation of subcomponents of state, e.g. after receiving a few chunks, validate one at random & ban the peer if invalid, etc.

Not at all. exactly as you state, the chunking scheme will have some impact on how soon data can be validated and peers banned.

docs/architecture/adr-042-state-sync.md

cwgoes · 2019-07-15T19:56:46Z

docs/architecture/adr-042-state-sync.md

+["Warp Sync"](https://wiki.parity.io/Warp-Sync-Snapshot-Format.html) to rapidly
+download both blocks and state snapshots from peers. Data is carved into ~4MB
+chunks and snappy compressed. Hashes of snappy compressed chunks are stored in a
+manifest file which co-ordinates the state-sync. Obtaining a correct manifest


That's lame. We should be able to verify random subsets of the state (random keys) pretty cheaply by keeping light client headers and requesting Merkle proofs occasionally, then ban any peer which sends invalid state, and recheck the whole state at the end.

Does Parity use this manifest file instead of a SPV Ethereum client?

Parity sync module takes a hybrid approach which will block header verification at the end but will sync intermediate state from snapshots first if WarpSync is enabled.

The general strategy is outlined here:
https://github.com/paritytech/parity-ethereum/blob/master/ethcore/sync/src/chain/mod.rs#L21-L34

Snapshots are vetted by rules and then taken from majority of peers here:
https://github.com/paritytech/parity-ethereum/blob/master/ethcore/sync/src/chain/mod.rs#L800-L828

docs/architecture/adr-042-state-sync.md

brapse

No new question in a little while, LGTM 👍

comments addressed.

docs/architecture/adr-042-state-sync.md

Co-Authored-By: Anton Kaliaev <anton.kalyaev@gmail.com>

--- #### PR checklist - [ ] Tests written/updated - [ ] Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog) - [ ] Updated relevant documentation (`docs/` or `spec/`) and code comments

ebuchman added 2 commits June 29, 2019 21:26

init adr 042: state sync

1cc88d1

link to blockchain reactor

a15205a

AdityaSripal previously requested changes Jul 2, 2019

View reviewed changes

tac0turtle mentioned this pull request Jul 3, 2019

Use linked-list to store versions cosmos/iavl#9

Closed

ebuchman reviewed Jul 3, 2019

View reviewed changes

docs/architecture/adr-042-state-sync.md Outdated Show resolved Hide resolved

brapse proposal

27f6dd3

formatting

051f461

ebuchman reviewed Jul 4, 2019

View reviewed changes

cryptom-dev reviewed Jul 5, 2019

View reviewed changes

milosevic reviewed Jul 5, 2019

View reviewed changes

docs/architecture/adr-042-state-sync.md Outdated Show resolved Hide resolved

milosevic reviewed Jul 5, 2019

View reviewed changes

docs/architecture/adr-042-state-sync.md Outdated Show resolved Hide resolved

milosevic reviewed Jul 5, 2019

View reviewed changes

docs/architecture/adr-042-state-sync.md Outdated Show resolved Hide resolved

milosevic reviewed Jul 5, 2019

View reviewed changes

docs/architecture/adr-042-state-sync.md Outdated Show resolved Hide resolved

milosevic reviewed Jul 5, 2019

View reviewed changes

docs/architecture/adr-042-state-sync.md Outdated Show resolved Hide resolved

response to feedback

834ecd5

AdityaSripal reviewed Jul 5, 2019

View reviewed changes

Update docs/architecture/adr-042-state-sync.md

9f68d4d

Co-Authored-By: Aditya <adityasripal@gmail.com>

tac0turtle mentioned this pull request Jul 6, 2019

[WIP] parity warp-like state sync #3594

Closed

4 tasks

tac0turtle changed the title ~~State-sync discussions~~ [ADR - 42] State-sync Jul 6, 2019

tac0turtle added the ready-for-review label Jul 6, 2019

Merge branch 'master' into bucky/state-sync

f8093ee

melekes reviewed Jul 10, 2019

View reviewed changes

docs/architecture/adr-042-state-sync.md Outdated Show resolved Hide resolved

docs/architecture/adr-042-state-sync.md Outdated Show resolved Hide resolved

cwgoes reviewed Jul 15, 2019

View reviewed changes

brapse added 3 commits July 22, 2019 20:29

Update security models and more

e0fcb28

clarify compression

5e8d0f3

typo

9ae1892

tac0turtle and others added 2 commits July 23, 2019 11:00

Merge branch 'master' into bucky/state-sync

742f0a0

Merge branch 'master' into bucky/state-sync

fe241cc

brapse marked this pull request as ready for review July 24, 2019 16:11

brapse requested a review from xla as a code owner July 24, 2019 16:11

Merge branch 'master' into bucky/state-sync

579cfea

jackzampolin approved these changes Aug 5, 2019

View reviewed changes

brapse approved these changes Aug 5, 2019

View reviewed changes

melekes approved these changes Aug 6, 2019

View reviewed changes

docs/architecture/adr-042-state-sync.md Outdated Show resolved Hide resolved

Update docs/architecture/adr-042-state-sync.md

05e4e0b

Co-Authored-By: Anton Kaliaev <anton.kalyaev@gmail.com>

tac0turtle merged commit d70135e into master Aug 6, 2019

tac0turtle deleted the bucky/state-sync branch August 6, 2019 08:25

Conversation

tac0turtle commented Jul 2, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AdityaSripal commented Jul 2, 2019

Uh oh!

Uh oh!

codecov-io commented Jul 4, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

ebuchman left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

brapse commented Jul 5, 2019

Uh oh!

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!

Uh oh!

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!

Uh oh!

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!

Uh oh!

brapse left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

11 participants

codecov-io commented Jul 4, 2019 •

edited

Loading