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: fill msg timestamp while appending RLN proof it is not set #940

Merged
merged 4 commits into from
Dec 6, 2023

Conversation

chaitanyaprem
Copy link
Collaborator

Description

If message timestamp is not set, then as part of appending RLN proof to the message, the timestamp is also set based on which the RLN proof is calculated.

Fixes #926 and not just REST API but for all API's.

cc @waku-org/nwaku-developers This would have to be addressed in nwaku REST API as well.

@chaitanyaprem chaitanyaprem changed the title feat: fill timestamp while appending RLN proof to msg if it is not set feat: fill msg timestamp while appending RLN proof it is not set Nov 30, 2023
@status-im-auto
Copy link

status-im-auto commented Nov 30, 2023

Jenkins Builds

Click to see older builds (2)
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 2df3282 #1 2023-11-30 10:07:08 ~1 min nix-flake 📄log
✔️ 40c0bb2 #2 2023-12-01 01:26:14 ~1 min nix-flake 📄log
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ da636bd #3 2023-12-05 14:20:56 ~1 min nix-flake 📄log
✔️ c4db7df #4 2023-12-06 01:50:34 ~2 min nix-flake 📄log

@richard-ramos
Copy link
Member

The code looks okay, but I'm wondering why is it necessary to set the timestamp in the message?
AFAIK it's not used in the RLN validator, but instead the validation is done against the epoch attribute part of the RLN Proof

@chaitanyaprem
Copy link
Collaborator Author

The code looks okay, but I'm wondering why is it necessary to set the timestamp in the message? AFAIK it's not used in the RLN validator, but instead the validation is done against the epoch attribute part of the RLN Proof

True, but for Store queries to be consistent across nodes having a senderTimestamp helps with it.
Also this is already being tracked as an issue in nwaku at waku-org/nwaku#2193

More context regarding this can be found at https://discord.com/channels/1110799176264056863/1111540684575477821/1177574753100378123 as further improvements in store protocol also expects timestamp to be part of message. Hence it would help making it mandatory.

@chaitanyaprem chaitanyaprem merged commit 021265e into master Dec 6, 2023
12 checks passed
@chaitanyaprem chaitanyaprem deleted the feat/relay-msg-timestamp branch December 6, 2023 01:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

feat: use current timestamp when not provided in relay rest api
3 participants