feat: store v3 #2431

SionoiS · 2024-02-14T15:13:50Z

Description

The new Store protocol version 3 will work side by side with the old version for some time.

I copied all the code from v2 as a starting point. Store v2 is now known as legacy store.

Store v3 order results a bit differently than legacy. Sorted by time and then by hash (was digest previously).
Everything remain the same except now we have the ability to query by hashes too 🎉

Changes

Tracking #2425

github-actions · 2024-02-14T15:18:39Z

You can find the image built from this PR at

quay.io/wakuorg/nwaku-pr:2431-rln-v2-true

Built from ba28042

jm-clius

Direction lgtm! Added a couple of comments. Will also be making some minor changes to the draft RFC, but will keep you informed about those. :)

waku/waku_store_v3/client.nim

waku/node/waku_node.nim

waku/waku_store_v3/client.nim

waku/waku_store_v3/common.nim

waku/waku_store_v3/rpc_codec.nim

waku/waku_store_v3/protocol.nim

SionoiS · 2024-02-28T16:37:24Z

N.B. tests can't pass before archive is updated and store v3 is tied to it.

github-actions · 2024-03-13T15:28:24Z

This PR may contain changes to database schema of one of the drivers.

If you are introducing any changes to the schema, make sure the upgrade from the latest release to this change passes without any errors/issues.

Please make sure the label release-notes is added to make sure upgrade instructions properly highlight this change.

chaitanyaprem

Reviewed at high level only Store protocol changes.

Will do the rest again as i have to understand a lot of existing code to understand changes :)

waku/waku_store_legacy/client.nim

waku/waku_store/client.nim

chaitanyaprem · 2024-03-21T11:28:46Z

waku/waku_store/rpc_codec.nim

-proc encode*(index: PagingIndexRPC): ProtoBuffer =
-  ## Encode an Index object into a ProtoBuffer
-  ## returns the resultant ProtoBuffer
+proc encode*(req: StoreQueryRequest): ProtoBuffer =


General Question:

We are manually encoding protobufs, seems error-prone and high maintenance.
Can't we use something like https://github.com/PMunch/protobuf-nim to generate the nim code?

We should but if I'm not mistaken we use proto2 and not 3?

edit: nvm we do use v3

Maybe this can be taken up as an improvement as part of separate PR.

cc @Ivansete-status

Very interesting @chaitanyaprem ! I think that is something we could look at but I don't see super urgency for now as we don't change the protobuf implementation very often

wdyt @jm-clius ?

jm-clius

Thanks! The only blocking comment I have is that repeated fields are packed in proto3 (see #2509). Longer term, we should probably write our own library as wrapper for minprotobuf or use another.

apps/wakunode2/app.nim

waku/waku_store/rpc_codec.nim

Ivansete-status

Awesome! I think is getting a good shape!
I've added some comments that I hope you find useful.

Additionally:

Shall we keep both legacy and new store testing? I have the impression that we are reducing the legacy-store testing coverage.
Given that there are many changes, I think we need to validate that it is working properly from the Status point of view. @richard-ramos can help us on that matter :)

tests/node/test_wakunode_store.nim

tests/waku_store/test_resume.nim

waku/node/waku_node.nim

waku/waku_api/rest/store/handlers.nim

waku/waku_core/message/digest.nim

waku/waku_store/client.nim

waku/waku_store/common.nim

Ivansete-status · 2024-03-27T14:34:28Z

waku/waku_store/rpc_codec.nim

-proc encode*(index: PagingIndexRPC): ProtoBuffer =
-  ## Encode an Index object into a ProtoBuffer
-  ## returns the resultant ProtoBuffer
+proc encode*(req: StoreQueryRequest): ProtoBuffer =


Very interesting @chaitanyaprem ! I think that is something we could look at but I don't see super urgency for now as we don't change the protobuf implementation very often

wdyt @jm-clius ?

jm-clius

Approving, based on proto3 discussion with @SionoiS

github-actions · 2024-04-23T16:00:05Z

You can find the image built from this PR at

quay.io/wakuorg/nwaku-pr:2431-rln-v1

Built from 197d62d

github-actions · 2024-04-23T16:00:21Z

You can find the image built from this PR at

quay.io/wakuorg/nwaku-pr:2431-rln-v2

Built from 197d62d

Ivansete-status

LGTM! Thank you for such a great PR! 💯

Just added minor comment :)

Ivansete-status · 2024-04-24T14:35:14Z

waku/waku_store_legacy/client.nim

+proc sendHistoryQueryRPC(
+    w: WakuStoreClient, req: HistoryQuery, peer: RemotePeerInfo
+): Future[HistoryResult] {.async, gcsafe.} =
+  let connOpt = await w.peerManager.dialPeer(peer, WakuStoreCodec)


Maybe sth to check in the future but I can't see why dialing on every query. I think it would be better to separate the dialIn from query operations :)

What would you envision as an alternative? Dialing for each request makes this part of the code stateless which simplify things.

Do you mean separate for code organization purposes?

I made some changes in waku/waku_store/client.nim

SionoiS mentioned this pull request Feb 14, 2024

feat: reference implementation for Store v3 protocol #2425

Closed

2 tasks

SionoiS self-assigned this Feb 20, 2024

jm-clius reviewed Feb 20, 2024

View reviewed changes

SionoiS force-pushed the feat--store-v3 branch from 0d1e597 to 2a7dc0d Compare February 21, 2024 16:56

richard-ramos mentioned this pull request Feb 28, 2024

feat: storeV3 client waku-org/go-waku#1028

Merged

6 tasks

SionoiS force-pushed the feat--store-v3 branch 2 times, most recently from 15603b1 to 75177d9 Compare March 12, 2024 20:29

SionoiS force-pushed the feat--store-v3 branch from 6e43de6 to 4079160 Compare March 18, 2024 14:00

SionoiS marked this pull request as ready for review March 20, 2024 12:57

SionoiS requested review from jm-clius, chaitanyaprem and NagyZoltanPeter March 20, 2024 12:57

chaitanyaprem reviewed Mar 21, 2024

View reviewed changes

SionoiS requested a review from Ivansete-status March 25, 2024 12:47

SionoiS force-pushed the feat--store-v3 branch from 672f0b0 to 60480af Compare March 25, 2024 19:51

jm-clius requested changes Mar 26, 2024

View reviewed changes

apps/wakunode2/app.nim Outdated Show resolved Hide resolved

waku/waku_store/rpc_codec.nim Show resolved Hide resolved

Ivansete-status reviewed Mar 27, 2024

View reviewed changes

SionoiS mentioned this pull request Mar 27, 2024

Add Store v3 spec. waku-org/waku-rest-api#9

Merged

jm-clius approved these changes Apr 22, 2024

View reviewed changes

SionoiS force-pushed the feat--store-v3 branch from 7c93644 to 4edee18 Compare April 23, 2024 15:55

SionoiS force-pushed the feat--store-v3 branch from ed0023a to bb486ef Compare April 24, 2024 11:43

SionoiS requested review from Ivansete-status and chaitanyaprem April 24, 2024 12:02

Ivansete-status approved these changes Apr 24, 2024

View reviewed changes

feat: Store v3

c89b00a

Fix conflict & slight refactor

f9e154e

SionoiS force-pushed the feat--store-v3 branch from ef1aae5 to f9e154e Compare April 24, 2024 15:01

SionoiS merged commit 0b0fbfa into master Apr 25, 2024
13 of 15 checks passed

SionoiS deleted the feat--store-v3 branch April 25, 2024 13:09

NagyZoltanPeter mentioned this pull request Apr 25, 2024

fix: missing rate limit setting for legacy store protocol #2631

Merged

fbarbu15 mentioned this pull request Apr 26, 2024

bug: contentTopic naming not consistent in the store response where is's content_topic #2582

Closed

This was referenced Aug 30, 2024

[Epic: research] Store v3-beta - Message Hashes waku-org/research#103

Closed

[Epic: nwaku] Store v3-beta - Message Hashes #3070

Closed

chair28980 mentioned this pull request Sep 27, 2024

[Deliverable] Store v3-beta - Message Hashes waku-org/pm#131

Closed

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: store v3 #2431

feat: store v3 #2431

SionoiS commented Feb 14, 2024 •

edited

Loading

github-actions bot commented Feb 14, 2024 •

edited

Loading

jm-clius left a comment

SionoiS commented Feb 28, 2024

github-actions bot commented Mar 13, 2024

chaitanyaprem left a comment

chaitanyaprem Mar 21, 2024

SionoiS Mar 21, 2024 •

edited

Loading

chaitanyaprem Mar 25, 2024

Ivansete-status Mar 27, 2024

jm-clius left a comment

Ivansete-status left a comment

Ivansete-status Mar 27, 2024

jm-clius left a comment

github-actions bot commented Apr 23, 2024 •

edited

Loading

github-actions bot commented Apr 23, 2024 •

edited

Loading

Ivansete-status left a comment

Ivansete-status Apr 24, 2024

SionoiS Apr 24, 2024 •

edited

Loading

feat: store v3 #2431

feat: store v3 #2431

Conversation

SionoiS commented Feb 14, 2024 • edited Loading

Description

Changes

github-actions bot commented Feb 14, 2024 • edited Loading

jm-clius left a comment

Choose a reason for hiding this comment

SionoiS commented Feb 28, 2024

github-actions bot commented Mar 13, 2024

chaitanyaprem left a comment

Choose a reason for hiding this comment

chaitanyaprem Mar 21, 2024

Choose a reason for hiding this comment

SionoiS Mar 21, 2024 • edited Loading

Choose a reason for hiding this comment

chaitanyaprem Mar 25, 2024

Choose a reason for hiding this comment

Ivansete-status Mar 27, 2024

Choose a reason for hiding this comment

jm-clius left a comment

Choose a reason for hiding this comment

Ivansete-status left a comment

Choose a reason for hiding this comment

Ivansete-status Mar 27, 2024

Choose a reason for hiding this comment

jm-clius left a comment

Choose a reason for hiding this comment

github-actions bot commented Apr 23, 2024 • edited Loading

github-actions bot commented Apr 23, 2024 • edited Loading

Ivansete-status left a comment

Choose a reason for hiding this comment

Ivansete-status Apr 24, 2024

Choose a reason for hiding this comment

SionoiS Apr 24, 2024 • edited Loading

Choose a reason for hiding this comment

SionoiS commented Feb 14, 2024 •

edited

Loading

github-actions bot commented Feb 14, 2024 •

edited

Loading

SionoiS Mar 21, 2024 •

edited

Loading

github-actions bot commented Apr 23, 2024 •

edited

Loading

github-actions bot commented Apr 23, 2024 •

edited

Loading

SionoiS Apr 24, 2024 •

edited

Loading