Skip to content
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 #22

Merged
merged 7 commits into from
Oct 22, 2024
Merged

feat: waku sync #22

merged 7 commits into from
Oct 22, 2024

Conversation

SionoiS
Copy link
Contributor

@SionoiS SionoiS commented Jun 12, 2024

First draft of what a Waku Sync specification would look like.

I'm not sure about the direction, do we want more details or maybe write more about what we want in the future?

I feel like it should be really short since it only wraps Negentropy.

WDYT?

Copy link
Contributor

@jm-clius jm-clius left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a good start for a basic spec. I would indeed remain light on details. A separate spec should explain the integrated approach of a synchronised Store (Store-Sync) using resume and Waku-Sync with reference to this spec.
I would avoid tying any directives in this doc directly to any other Waku Protocol, although referencing Relay and Store as the most obvious environment in which Sync can be setup makes sense to me.

Comment on lines 32 to 33
Nodes enabling Waku Sync SHOULD have the relay and store protocols enabled and
keep messages for at least the last hour. TODO do we need to say this, sounds like an impl. detail
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reference to an hour is indeed an implementation detail. We want to write "friendly" specs, in that we shouldn't avoid specific examples that may help understanding. I just wouldn't phrase it as part of the protocol.
The requirements are:

  • nodes that send and receive Waku messages (most likely via Waku Relay, but they could also be using Waku Filter)
  • subset of these nodes that want to cache and synchronise the Waku messages (most likely because they run Waku Store service)

In other words, I wouldn't make the protocol dependent on Relay and Store, but would mention that this is the most obvious example use case. Then I'd say:

  • Sync nodes maintain a representative set of message hashes corresponding to locally cached messages
  • This representative set should cover the time period over which the nodes require to sync. 1 hour is a sensible default here.
  • The sync protocol operates on message hashes and assume that after reconciliation of message hash sets, the full contents of messages can be retrieved in a secondary step (e.g. via Store protocol) and added to the local now-synced cache

In other words:

  • describe the function not the protocol
  • frame the most likely specific protocols performing functions above as examples
  • where implementation is involved, name (but do not mandate) sensible defaults to be as useful as possible

Comment on lines 72 to 73
### Storage Pruning
TODO? do we need to talk about that, seams like an implementation detail
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps just note under Design Requirements that the size of the local set of message hashes should be managed to correspond to the time period over which you want to sync. No need for negentropy implementation specifics.

@SionoiS
Copy link
Contributor Author

SionoiS commented Oct 21, 2024

Some of this spec will have to change for 2.0 but lets merge.

@SionoiS SionoiS requested a review from jm-clius October 21, 2024 15:31
Copy link
Contributor

@jm-clius jm-clius left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving as a raw spec. I think this will change substantially with the intro of Sync 2.0.

standards/core/sync.md Outdated Show resolved Hide resolved
standards/core/sync.md Outdated Show resolved Hide resolved
standards/core/sync.md Show resolved Hide resolved
standards/core/sync.md Outdated Show resolved Hide resolved
@SionoiS SionoiS marked this pull request as ready for review October 22, 2024 14:23
@SionoiS SionoiS merged commit 940ddf4 into master Oct 22, 2024
0 of 2 checks passed
@SionoiS SionoiS deleted the feat--waku-sync branch October 22, 2024 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants