docs(adr): BlockSync Overhaul ADR #011 - Part 1 #1037

Wondertan · 2022-08-23T17:50:56Z

codecov-commenter · 2022-09-02T11:41:34Z

Codecov Report

Merging #1037 (e6fda67) into main (190e4d0) will decrease coverage by 0.50%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #1037      +/-   ##
==========================================
- Coverage   57.39%   56.89%   -0.51%     
==========================================
  Files         133      136       +3     
  Lines        8307     9063     +756     
==========================================
+ Hits         4768     5156     +388     
- Misses       3067     3368     +301     
- Partials      472      539      +67

Impacted Files	Coverage Δ
cmd/flags_node.go	`66.66% <0.00%> (-10.26%)`	⬇️
params/default.go	`13.88% <0.00%> (-0.82%)`	⬇️
fraud/service.go	`76.00% <0.00%> (-0.09%)`	⬇️
node/node.go	`62.50% <0.00%> (ø)`
params/genesis.go	`33.33% <0.00%> (ø)`
params/bootstrap.go	`30.76% <0.00%> (ø)`
service/state/core_access.go	`11.59% <0.00%> (ø)`
ipld/get_namespaced_shares.go	`90.51% <0.00%> (ø)`
header/sync/sync_head.go	`58.77% <0.00%> (ø)`
fraud/sync.go	`59.43% <0.00%> (ø)`
... and 11 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

distractedm1nd

Absolutely amazing! Looking great. Excited for the diagrams as well

I mainly have questions, but also had some comments from what I learned from my experiments. And some opinions I want to debate on

Still some open questions on my mind but I will think about them and extend my review later

docs/adr/adr-11-blocksync-overhaul.md

distractedm1nd

Best ADR yet!

walldiss

Awesome work!

renaynay

This ADR is awesome and should be used as an example for writing ADRs org-wide :)

One nit/question: have you considered extracting EDSStore into a subpkg of share module? eds.Store ?

Wondertan · 2022-09-16T11:24:59Z

@renaynay

One nit/question: have you considered extracting EDSStore into a subpkg of share module? eds.Store ?

I thought that the share pkg would likely become too bloated and made a point about restructuring it in the EPIC. I like your idea to have a subpackage named as share/eds. Wil add it to the issue.

… sections

…ethod clarifications

liamsi

Awesome work!

liamsi · 2022-09-20T11:55:57Z

docs/adr/adr-11-blocksync-overhaul.md

+- LN - Light Node
+- FN - Full Node
+- BN - Bridge Node
+- [EDS(Extended Data Square)](https://github.com/celestiaorg/rsmt2d/blob/master/extendeddatasquare.go#L10) - plain Block


Use a permalink please.
https://github.com/celestiaorg/rsmt2d/blob/76b270f80f0b9ac966c6f6b043e31514574f90f3/extendeddatasquare.go#L10

liamsi · 2022-09-20T11:56:34Z

docs/adr/adr-11-blocksync-overhaul.md

+- [EDS(Extended Data Square)](https://github.com/celestiaorg/rsmt2d/blob/master/extendeddatasquare.go#L10) - plain Block
+data omitting headers and other block metadata.
+- [NMT](https://github.com/celestiaorg/nmt) - Namespaced Merkle Tree
+- [DataRoot](https://github.com/celestiaorg/celestia-node/blob/main/service/share/share.go#L35) - alias for


Same, use permalink.

liamsi · 2022-09-20T11:57:07Z

docs/adr/adr-11-blocksync-overhaul.md

+data omitting headers and other block metadata.
+- [NMT](https://github.com/celestiaorg/nmt) - Namespaced Merkle Tree
+- [DataRoot](https://github.com/celestiaorg/celestia-node/blob/main/service/share/share.go#L35) - alias for
+[DAHeader](https://github.com/celestiaorg/celestia-core/blob/v0.34.x-celestia/pkg/da/data_availability_header.go#L29)


Same as above.

liamsi · 2022-09-20T12:07:35Z

docs/adr/adr-11-blocksync-overhaul.md

+The DASing, on the other hand, shows acceptable metrics for the block sizes we are aiming for initially. In the case of
+the same block, a DAS operation takes 50ms * 8(technically 9) blocking requests, which is ~400ms in an ideal scenario
+(excluding disk IO).


This may sounds as if we won't be touching/improving DASing at all. Though my understandig is that why it is acceptable initially, we still will want to optimize DASing as well (I think that is sth @derrandz will look into among other things AFIU).
Nothing to change here but just double-checking that we are aligned.

liamsi · 2022-09-20T12:09:16Z

docs/adr/adr-11-blocksync-overhaul.md

+
+This ADR intends to outline design decisions for block data storage. In a nutshell, the decision is to use
+___[CAR format](https://ipld.io/specs/transport/car/carv2/)___ and ___[Dagstore](https://github.com/filecoin-project/dagstore)___
+for ___extended block storage___ and ___custom p2p Req/Resp protocol for block data syncing___(whole block and data by


nit: whitespaces between words and ( in general. No need to change anything but something to watch out in the future.

liamsi · 2022-09-20T12:21:02Z

docs/adr/adr-11-blocksync-overhaul.md

+is focused around storing entire EDSes as a whole rather than a set of individual chunks, s.t. storage subsystem
+can handle storing and streaming/serving blocks of 4MB and more.
+
+#### EDS Serde


What does serde stand for? Shouldn't the title here be:

EDS (De-)Serialization

liamsi · 2022-09-20T12:26:34Z

docs/adr/adr-11-blocksync-overhaul.md

+  - [Writes](https://github.com/ipld/go-car/blob/master/car.go#L118) the shares in row-by-row order
+- Iterates over in-memory Blockstore and [writes]((https://github.com/ipld/go-car/blob/master/car.go#L118)) NMT Merkle
+proofs stored in it
+


All above links should be perma-links. Otherwise they will be out of date shortly.

Wondertan added the docs:adr ADR label Aug 23, 2022

Wondertan self-assigned this Aug 23, 2022

Wondertan added the kind:misc Attached to miscellaneous PRs label Aug 23, 2022

Wondertan force-pushed the hlib/docs/adr-11 branch from bb99ca5 to 63ad7eb Compare August 23, 2022 17:55

distractedm1nd mentioned this pull request Sep 1, 2022

feat(blocksync): WIP experiments with DAGStore as a Blockstore #1051

Closed

9 tasks

Wondertan force-pushed the hlib/docs/adr-11 branch from 02e06d8 to f680f21 Compare September 2, 2022 09:11

Wondertan marked this pull request as ready for review September 2, 2022 14:06

Wondertan requested review from liamsi, renaynay, vgonkivs and distractedm1nd as code owners September 2, 2022 14:06

Wondertan force-pushed the hlib/docs/adr-11 branch from d22611a to 46c517f Compare September 2, 2022 14:06

distractedm1nd suggested changes Sep 5, 2022

View reviewed changes

Wondertan force-pushed the hlib/docs/adr-11 branch 6 times, most recently from 2d41034 to e6fda67 Compare September 14, 2022 16:51

distractedm1nd self-requested a review September 15, 2022 08:19

distractedm1nd approved these changes Sep 15, 2022

View reviewed changes

Wondertan mentioned this pull request Sep 15, 2022

[EPIC] BlockSync Overhaul - Storage #1099

Closed

45 tasks

walldiss self-requested a review September 15, 2022 16:19

walldiss approved these changes Sep 15, 2022

View reviewed changes

renaynay approved these changes Sep 16, 2022

View reviewed changes

Wondertan changed the title ~~docs(adr): BlockSync Overhaul ADR #011~~ docs(adr): BlockSync Overhaul ADR #011 - Part 1 Sep 20, 2022

Wondertan enabled auto-merge (rebase) September 20, 2022 08:16

Wondertan disabled auto-merge September 20, 2022 08:16

Wondertan added 16 commits September 20, 2022 11:22

docs(adr): initial unfinished draft for ADR #11

346e957

docs(adr): block/eds storage design

11cbaa2

docs(adr): serde for EDS

2a566ee

docs(adr): improve API comments

0cc97d2

docs(adr): minor improvements for Key Design Decion and Consideration…

cd2513e

… sections

docs(adr): elaborate on how CARv2 files are going to be stored via FS

593c9c4

docs(adr-11): apply grammar/stylistic suggestions from @distractedm1nd

46fbc2e

docs(adr-11): clarify CARv2 vs CARv1

125f50b

docs(adr-11): correct usage of mount.Registry; make all NOTEs italic

d9af37e

docs(adr-11): clarify what is DataRoot and it's usage; minor formatting

583b9ec

docs(adr-11): additional clarification notes

7b46364

docs(adr-11): Has method; empty block and EDS deduplication; GetCar m…

e9aa05a

…ethod clarifications

docs(adr-11): mention that the ADR is part 1 only

ce567e6

docs(adr-11): clarifications for Remove method; minor formatting issues

2c3eee9

docs(adr-11): describe WriteEDS; fix all TODOs and fix formatting

f3df39d

docs(adr-11): grammar fixes

e1249b9

Wondertan force-pushed the hlib/docs/adr-11 branch from e6fda67 to e1249b9 Compare September 20, 2022 09:22

Wondertan merged commit 016c3d8 into main Sep 20, 2022

Wondertan deleted the hlib/docs/adr-11 branch September 20, 2022 09:23

liamsi reviewed Sep 20, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

docs(adr): BlockSync Overhaul ADR #011 - Part 1 #1037

docs(adr): BlockSync Overhaul ADR #011 - Part 1 #1037

Wondertan commented Aug 23, 2022 •

edited

Loading

codecov-commenter commented Sep 2, 2022 •

edited

Loading

distractedm1nd left a comment •

edited

Loading

distractedm1nd left a comment

walldiss left a comment

renaynay left a comment

Wondertan commented Sep 16, 2022 •

edited

Loading

liamsi left a comment

liamsi Sep 20, 2022

liamsi Sep 20, 2022

liamsi Sep 20, 2022

liamsi Sep 20, 2022

liamsi Sep 20, 2022

liamsi Sep 20, 2022

liamsi Sep 20, 2022

docs(adr): BlockSync Overhaul ADR #011 - Part 1 #1037

docs(adr): BlockSync Overhaul ADR #011 - Part 1 #1037

Conversation

Wondertan commented Aug 23, 2022 • edited Loading

codecov-commenter commented Sep 2, 2022 • edited Loading

Codecov Report

distractedm1nd left a comment • edited Loading

Choose a reason for hiding this comment

distractedm1nd left a comment

Choose a reason for hiding this comment

walldiss left a comment

Choose a reason for hiding this comment

renaynay left a comment

Choose a reason for hiding this comment

Wondertan commented Sep 16, 2022 • edited Loading

liamsi left a comment

Choose a reason for hiding this comment

liamsi Sep 20, 2022

Choose a reason for hiding this comment

liamsi Sep 20, 2022

Choose a reason for hiding this comment

liamsi Sep 20, 2022

Choose a reason for hiding this comment

liamsi Sep 20, 2022

Choose a reason for hiding this comment

liamsi Sep 20, 2022

Choose a reason for hiding this comment

liamsi Sep 20, 2022

Choose a reason for hiding this comment

EDS (De-)Serialization

liamsi Sep 20, 2022

Choose a reason for hiding this comment

Wondertan commented Aug 23, 2022 •

edited

Loading

codecov-commenter commented Sep 2, 2022 •

edited

Loading

distractedm1nd left a comment •

edited

Loading

Wondertan commented Sep 16, 2022 •

edited

Loading