WAKU-RLN-CONTRACT: Update #62

jimstir · 2025-05-02T20:44:38Z

Made some changes to specification for clarity for some statements.

s-tikhomirov · 2025-05-14T09:03:32Z

standards/core/rln-contract.md

-The sender of a message MUST prove its validity according to RLN requirements.
-RLN Relay nodes MUST NOT relay invalid messages.
-For the full specification of RLN Relay, see See [17/WAKU2-RLN-RELAY](https://github.com/vacp2p/rfc-index/blob/main/waku/standards/core/17/rln-relay.md).
+The sender of a message must prove its validity according to RLN requirements.


Why not use MUST? (same question for the next sentence)

These statements are located in the Background section. In practice, use keywords like MUST in sections located after the RFC 2119 statement. And the RFC 2119 statement should be located before specifying components of the systems, not description/background info.

On line 56, I do mention that "contract SHOULD verify that the identity_commitment is valid". I think this should be changed to a MUST and elaborated. Do you think this is a good following statement: " If the identity_commitment is not checked or validated, the contract MAY be vulnerable to malicious or malformed inputs." ?

Do you think this is a good following statement: " If the identity_commitment is not checked or validated, the contract MAY be vulnerable to malicious or malformed inputs." ?

It sounds to me that MAY in its strict RFC meaning is not applicable here. AFAIU, RFC terms refer to the vendor's decision about their implementation (emphasis mine):

MAY This word, or the adjective "OPTIONAL", mean that an item is
truly optional. One vendor may choose to include the item because a
particular marketplace requires it or because the vendor feels that
it enhances the product while another vendor may omit the same item.

But in "contract MAY be vulnerable", MAY does not refer to anyone's decision.

Taking a step back: the reason why I feel this paragraph needs restructuring is that if the reference to RFC verbs (MUST, MAY etc) is moved down into the contract overview section, then we shouldn't be using these terms before they are defined. Additionally, I would try to avoid using the informal equivalents of the RFC verbs (e.g. "The sender of a message must prove its validity") as it raises the question in the reader's mind about whether "must" here has a strict definition, and if not, then what it means exactly.

I've pushed a commit with a minor rephrasing of this paragraph, let me know if this works.

@s-tikhomirov The rephrasing of the paragraph works. To the second point, my suggestion of the use of MAY does refer to a user's decision. Vulnerable was not the correct word to use. "If the identity_commitment is not checked or validated, the contract MAY be exploited using malicious or malformed inputs." The MAY refers to the use of malicious inputs by the msg.sender. What about this?

I'm not sure we need this sentence here.

My thinking is: validating identity_commitment is part of the RLN Relay specification, which we link to. This spec is about the RLN contract and membership management in particular. The goal of the background section, in my view, is to give a general overview of what the moving parts are, and link to the dedicated spec.

If we write in this spec:

If the identity_commitment is not checked or validated, the contract MAY be exploited using malicious or malformed inputs.

then I, as a reader, am confused: is this something I have to worry about? What should I do to avoid making my contract vulnerable? And what does vulnerable mean exactly? The full answers to these questions are (should be) in the RLN Relay spec anyway. Do you think we should bring them up here?

@s-tikhomirov If the reader asks themselves these questions, the reference to the RLN spec within this section will be useful. I suggested to add this to describe the use of this function, isValidIdCommitment(idCommitment), which is used in the contract.

It is useful to the reader to be informed why implementing this function is important( and/or recommended). I am assuming if an implementor decides to not implement this function( or implement incorrectly), a user could still register on the contract (as there is no other validation requirements for id_commit), but run into problems when interacting with Waku nodes.

standards/core/rln-contract.md

s-tikhomirov · 2025-05-14T09:18:01Z

standards/core/rln-contract.md

+Membership registration MAY be initiated by a different entity from the one that controls the RLN `identity_secret`,
+which is associated with the respective RLN `identity_commitment`.
+Therefore, the holder MAY be assigned to an blockchain address that does not control the `identity_secret`.
+The contract SHOULD verify that the `identity_commitment` is valid.


How is "valid" defined here?

Elaborated on this statement in earlier comment.

I'm not sure I understand: which comment are you referring to?

standards/core/rln-contract.md

s-tikhomirov · 2025-05-22T10:34:53Z

standards/core/rln-contract.md

-The sender of a message MUST prove its validity according to RLN requirements.
-RLN Relay nodes MUST NOT relay invalid messages.
-For the full specification of RLN Relay, see See [17/WAKU2-RLN-RELAY](https://github.com/vacp2p/rfc-index/blob/main/waku/standards/core/17/rln-relay.md).
+The sender of a message must prove its validity according to RLN requirements.


Do you think this is a good following statement: " If the identity_commitment is not checked or validated, the contract MAY be vulnerable to malicious or malformed inputs." ?

It sounds to me that MAY in its strict RFC meaning is not applicable here. AFAIU, RFC terms refer to the vendor's decision about their implementation (emphasis mine):

MAY This word, or the adjective "OPTIONAL", mean that an item is
truly optional. One vendor may choose to include the item because a
particular marketplace requires it or because the vendor feels that
it enhances the product while another vendor may omit the same item.

But in "contract MAY be vulnerable", MAY does not refer to anyone's decision.

Taking a step back: the reason why I feel this paragraph needs restructuring is that if the reference to RFC verbs (MUST, MAY etc) is moved down into the contract overview section, then we shouldn't be using these terms before they are defined. Additionally, I would try to avoid using the informal equivalents of the RFC verbs (e.g. "The sender of a message must prove its validity") as it raises the question in the reader's mind about whether "must" here has a strict definition, and if not, then what it means exactly.

s-tikhomirov · 2025-05-22T10:41:56Z

standards/core/rln-contract.md

-The sender of a message MUST prove its validity according to RLN requirements.
-RLN Relay nodes MUST NOT relay invalid messages.
-For the full specification of RLN Relay, see See [17/WAKU2-RLN-RELAY](https://github.com/vacp2p/rfc-index/blob/main/waku/standards/core/17/rln-relay.md).
+The sender of a message must prove its validity according to RLN requirements.


I've pushed a commit with a minor rephrasing of this paragraph, let me know if this works.

jm-clius

Thanks for this!

jimstir added 3 commits April 17, 2025 11:38

Update rln-contract.md

4c09596

Update rln-contract.md

f292c86

Update rln-contract.md

1c02083

jimstir marked this pull request as draft May 5, 2025 20:14

Update rln-contract.md

63c2fb7

jimstir requested a review from jm-clius May 6, 2025 09:43

jimstir marked this pull request as ready for review May 6, 2025 09:43

jm-clius requested a review from s-tikhomirov May 8, 2025 15:06

minor edits

8c2eb13

s-tikhomirov requested changes May 14, 2025

View reviewed changes

s-tikhomirov and others added 2 commits May 14, 2025 14:15

fix typo

b29961d

Update rln-contract.md

8056725

jimstir requested a review from s-tikhomirov May 21, 2025 21:10

minor edit, add reference

f5f1a82

s-tikhomirov approved these changes May 22, 2025

View reviewed changes

jm-clius approved these changes May 23, 2025

View reviewed changes

WAKU-RLN-CONTRACT: Update #62

Are you sure you want to change the base?

WAKU-RLN-CONTRACT: Update #62

Uh oh!

Conversation

jimstir commented May 2, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jimstir May 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jm-clius left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jimstir May 26, 2025 •

edited

Loading