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

13/WAKU2-STORE: Update #653

Merged
merged 6 commits into from
Jan 14, 2024
Merged

13/WAKU2-STORE: Update #653

merged 6 commits into from
Jan 14, 2024

Conversation

jimstir
Copy link
Contributor

@jimstir jimstir commented Dec 17, 2023

Updating links and format.

@jimstir jimstir marked this pull request as draft December 17, 2023 07:56
@jimstir jimstir requested a review from kaiserd December 18, 2023 02:31
@jimstir jimstir marked this pull request as ready for review December 18, 2023 03:32
@jimstir jimstir requested a review from rymnc January 10, 2024 20:56
Copy link
Contributor

@rymnc rymnc left a comment

Choose a reason for hiding this comment

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

LGTM in general, but will cc: @jm-clius / @Ivansete-status to approve based on store protocol specifics

Comment on lines +31 to +34
As such, they are required to be *highly available* and
specifically have a *high uptime* to consistently receive and store network messages.
The high uptime requirement makes sure that no message is missed out hence a complete and
intact view of the message history is delivered to the querying nodes.
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think high uptime is a requirement here, the waku network is meant to be slightly ephemeral in nature.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is correct, it is specified as a SHOULD in this specification.
"Nodes willing to provide the storage service using 13/WAKU2-STORE protocol,
SHOULD provide a complete and full view of message history."

Copy link
Contributor

@Ivansete-status Ivansete-status left a comment

Choose a reason for hiding this comment

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

LGTM!

Nodes running [13/WAKU2-STORE](/spec/13) SHOULD NOT store messages with the `ephemeral` flag set to `true`.

## Terminology
The term Personally identifiable information (PII) refers to any piece of data that can be used to uniquely identify a user.
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor nitpick

Suggested change
The term Personally identifiable information (PII) refers to any piece of data that can be used to uniquely identify a user.
The term Personally Identifiable Information (PII) refers to any piece of data that can be used to uniquely identify a user.

Comment on lines 118 to 126
- `pageSize`: A positive integer indicating the number of queried `WakuMessage`s in a `HistoryQuery` (or retrieved `WakuMessage`s in a `HistoryResponse`).
- `pageSize`: A positive integer indicating the number of queried `WakuMessage` in a `HistoryQuery` (or retrieved `WakuMessage` in a `HistoryResponse`).
Copy link
Contributor

Choose a reason for hiding this comment

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

I think is interesting to keep the notion of possible multiple messages being retrieved, which is the usual case.

This field maps to the `contentTopic` field of the [14/WAKU2-MESSAGE](/spec/14).

### HistoryQuery

RPC call to query historical messages.

- The `pubsubTopic` field MUST indicate the pubsub topic of the historical messages to be retrieved.
This field denotes the pubsub topic on which waku messages are published.
This field denotes the pubsub topic on which Waku messages are published.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
This field denotes the pubsub topic on which Waku messages are published.
This field denotes the pubsub topic on which `WakuMessage`s are published.

@jimstir jimstir merged commit 00f1081 into master Jan 14, 2024
@jimstir jimstir deleted the 13-WAKU2-STORE branch January 14, 2024 18:47
s-tikhomirov pushed a commit that referenced this pull request Jan 30, 2024
* Update README.md

* Update README.md

* Update README.md

* Update README.md

* Update README.md

* Update README.md
s-tikhomirov pushed a commit that referenced this pull request Jan 30, 2024
* Update README.md

* Update README.md

* Update README.md

* Update README.md

* Update README.md

* Update README.md
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.

None yet

3 participants