-
Notifications
You must be signed in to change notification settings - Fork 2
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
feat: waku sync 2.0 #47
Conversation
I still don't know what to write for varints. I don't want to write a varint lib and spec and I also think the Nim std impl. might change. What do? @jm-clius edit: I decided to use https://github.com/status-im/nim-stew/blob/master/stew/leb128.nim |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this!
I left some comments below, many of them nitpicks.
I think the main comment is around providing some context and a functional overview before jumping into the detail (at least of the reconciliation
protocol). It can state the protocol flow and wire protocols in very simple terms:
- what agents are involved (two nodes that store Waku messages)
- what are the preconditions and goal (they want to initiate a bidirectional sync over the messages)
- what SHOULD and MUST they do, step by step, to achieve this:
- they identify the ranges that they want to sync over (and define what is meant by that)
- they create the fingerprint
- they encode in some way
etc.
I think this can be done in a very short paragraph, but will help first-time readers understand the big picture.
I agree, it's a bit hard to grasps the gist of how reconciliation work ATM. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this reads very well. Thanks! I have added some comments below, most of them nitpicks or phrasing suggestions which are up to you to address or not.
Co-authored-by: Hanno Cornelius <68783915+jm-clius@users.noreply.github.com>
Co-authored-by: Hanno Cornelius <68783915+jm-clius@users.noreply.github.com>
Co-authored-by: Hanno Cornelius <68783915+jm-clius@users.noreply.github.com>
Co-authored-by: Hanno Cornelius <68783915+jm-clius@users.noreply.github.com>
Co-authored-by: Hanno Cornelius <68783915+jm-clius@users.noreply.github.com>
Co-authored-by: Hanno Cornelius <68783915+jm-clius@users.noreply.github.com>
Co-authored-by: Hanno Cornelius <68783915+jm-clius@users.noreply.github.com>
Co-authored-by: Hanno Cornelius <68783915+jm-clius@users.noreply.github.com>
Co-authored-by: Hanno Cornelius <68783915+jm-clius@users.noreply.github.com>
Co-authored-by: Hanno Cornelius <68783915+jm-clius@users.noreply.github.com>
Co-authored-by: Hanno Cornelius <68783915+jm-clius@users.noreply.github.com>
Co-authored-by: Hanno Cornelius <68783915+jm-clius@users.noreply.github.com>
Co-authored-by: Hanno Cornelius <68783915+jm-clius@users.noreply.github.com>
Co-authored-by: Hanno Cornelius <68783915+jm-clius@users.noreply.github.com>
Co-authored-by: Hanno Cornelius <68783915+jm-clius@users.noreply.github.com>
Co-authored-by: Hanno Cornelius <68783915+jm-clius@users.noreply.github.com>
Co-authored-by: Hanno Cornelius <68783915+jm-clius@users.noreply.github.com>
WIP spec for Waku sync 2.0