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.

Sign up for GitHub

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

fix(rln-relay): make nullifier log abide by epoch ordering #2508

Merged
merged 3 commits into from
Mar 6, 2024
Merged

fix(rln-relay): make nullifier log abide by epoch ordering #2508

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9b04d9e
fix(rln-relay): nullifier log abide by epoch ordering
rymnc Mar 5, 2024
6d20aef
fix: cleaner hasKey method, test
rymnc Mar 6, 2024
c8bc4a6
chore: idiomatic usage of results, error handling
rymnc Mar 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 63 additions & 19 deletions tests/waku_rln_relay/test_waku_rln_relay.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -632,30 +632,25 @@ suite "Waku rln relay":

# check whether hasDuplicate correctly finds records with the same nullifiers but different secret shares
# no duplicate for proof1 should be found, since the log is empty
alrevuelta marked this conversation as resolved.
Show resolved Hide resolved
let result1 = wakurlnrelay.hasDuplicate(proof1.extractMetadata().tryGet())
require:
result1.isOk()
# no duplicate is found
result1.value == false
let result1 = wakurlnrelay.hasDuplicate(epoch, proof1.extractMetadata().tryGet())
assert result1.isOk(), $result1.error
assert result1.value == false, "no duplicate should be found"
# add it to the log
discard wakurlnrelay.updateLog(proof1.extractMetadata().tryGet())
discard wakurlnrelay.updateLog(epoch, proof1.extractMetadata().tryGet())

# # no duplicate for proof2 should be found, its nullifier differs from proof1
let result2 = wakurlnrelay.hasDuplicate(proof2.extractMetadata().tryGet())
require:
result2.isOk()
# no duplicate is found
result2.value == false
# no duplicate for proof2 should be found, its nullifier differs from proof1
let result2 = wakurlnrelay.hasDuplicate(epoch, proof2.extractMetadata().tryGet())
assert result2.isOk(), $result2.error
# no duplicate is found
assert result2.value == false, "no duplicate should be found"
# add it to the log
discard wakurlnrelay.updateLog(proof2.extractMetadata().tryGet())
discard wakurlnrelay.updateLog(epoch, proof2.extractMetadata().tryGet())

# proof3 has the same nullifier as proof1 but different secret shares, it should be detected as duplicate
let result3 = wakurlnrelay.hasDuplicate(proof3.extractMetadata().tryGet())
require:
result3.isOk()
check:
# it is a duplicate
result3.value == true
let result3 = wakurlnrelay.hasDuplicate(epoch, proof3.extractMetadata().tryGet())
assert result3.isOk(), $result3.error
# it is a duplicate
assert result3.value, "duplicate should be found"

asyncTest "validateMessageAndUpdateLog test":
let index = MembershipIndex(5)
Expand Down Expand Up @@ -710,6 +705,55 @@ suite "Waku rln relay":
msgValidate3 == MessageValidationResult.Valid
msgValidate4 == MessageValidationResult.Invalid

asyncTest "validateMessageAndUpdateLog: multiple senders with same external nullifier":
let index1 = MembershipIndex(5)
let index2 = MembershipIndex(6)

let rlnConf1 = WakuRlnConfig(rlnRelayDynamic: false,
rlnRelayCredIndex: some(index1),
rlnEpochSizeSec: 1,
rlnRelayTreePath: genTempPath("rln_tree", "waku_rln_relay_3"))
let wakuRlnRelay1Res = await WakuRlnRelay.new(rlnConf1)
if wakuRlnRelay1Res.isErr(): assert false, "failed to create waku rln relay: " & $wakuRlnRelay1Res.error
let wakuRlnRelay1 = wakuRlnRelay1Res.get()
rymnc marked this conversation as resolved.
Show resolved Hide resolved

let rlnConf2 = WakuRlnConfig(rlnRelayDynamic: false,
rlnRelayCredIndex: some(index2),
rlnEpochSizeSec: 1,
rlnRelayTreePath: genTempPath("rln_tree", "waku_rln_relay_4"))
let wakuRlnRelay2Res = await WakuRlnRelay.new(rlnConf2)
if wakuRlnRelay2Res.isErr():
assert false, "failed to create waku rln relay: " & $wakuRlnRelay2Res.error
let wakuRlnRelay2 = wakuRlnRelay2Res.get()
# get the current epoch time
let time = epochTime()

# create messages from different peers and append rln proofs to them
var
wm1 = WakuMessage(payload: "Valid message from sender 1".toBytes())
# another message in the same epoch as wm1, it will break the messaging rate limit
wm2 = WakuMessage(payload: "Valid message from sender 2".toBytes())


let
proofAdded1 = wakuRlnRelay1.appendRLNProof(wm1, time)
proofAdded2 = wakuRlnRelay2.appendRLNProof(wm2, time)

# ensure proofs are added
assert proofAdded1.isOk(), "failed to append rln proof: " & $proofAdded1.error
assert proofAdded2.isOk(), "failed to append rln proof: " & $proofAdded2.error

# validate messages
# validateMessage proc checks the validity of the message fields and adds it to the log (if valid)
let
msgValidate1 = wakuRlnRelay1.validateMessageAndUpdateLog(wm1, some(time))
# since this message is from a different sender, it should be validated successfully
msgValidate2 = wakuRlnRelay1.validateMessageAndUpdateLog(wm2, some(time))

check:
msgValidate1 == MessageValidationResult.Valid
msgValidate2 == MessageValidationResult.Valid

test "toIDCommitment and toUInt256":
# create an instance of rln
let rlnInstance = createRLNInstanceWrapper()
Expand Down
41 changes: 16 additions & 25 deletions waku/waku_rln_relay/rln_relay.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ proc createMembershipList*(rln: ptr RLN, n: int): RlnRelayResult[(

type WakuRLNRelay* = ref object of RootObj
# the log of nullifiers and Shamir shares of the past messages grouped per epoch
nullifierLog*: OrderedTable[Epoch, seq[ProofMetadata]]
nullifierLog*: OrderedTable[Epoch, Table[Nullifier, ProofMetadata]]
lastEpoch*: Epoch # the epoch of the last published rln message
rlnEpochSizeSec*: uint64
rlnMaxEpochGap*: uint64
Expand All @@ -103,58 +103,49 @@ proc stop*(rlnPeer: WakuRLNRelay) {.async: (raises: [Exception]).} =
await rlnPeer.groupManager.stop()

proc hasDuplicate*(rlnPeer: WakuRLNRelay,
epoch: Epoch,
proofMetadata: ProofMetadata): RlnRelayResult[bool] =
## returns true if there is another message in the `nullifierLog` of the `rlnPeer` with the same
## epoch and nullifier as `proofMetadata`'s epoch and nullifier
## otherwise, returns false
## Returns an error if it cannot check for duplicates

let externalNullifier = proofMetadata.externalNullifier
# check if the epoch exists
if not rlnPeer.nullifierLog.hasKey(externalNullifier):
let nullifier = proofMetadata.nullifier
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe all this can be replace by something like (disregard types). getOrDefault might be useful.

return defaultTable.getOrDefault("epoch1", initTable[string, int]()).hasKey("key1")

A quick example would be something like this.

import tables

var defaultTable: Table[string, Table[string, int]] = {"epoch1":{"key1": 3, "key2": 3}.toTable,
                                                       "epoch2": {"key3": 3, "key4": 3}.toTable}.toTable

echo defaultTable.getOrDefault("epoch1", initTable[string, int]()).hasKey("key1")
# true
  
echo defaultTable.getOrDefault("epoch1", initTable[string, int]()).hasKey("key200")
# false

echo defaultTable.getOrDefault("epoch5", initTable[string, int]()).hasKey("key1")
# false

just an idea though, at your discretion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

addressed in 6d20aef

Copy link
Contributor

Choose a reason for hiding this comment

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

not sure thats addressed there. in any case not blocking.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we don't actually need to get the value from the nullifierLog table, just check if it exists.

if not rlnPeer.nullifierLog.hasKey(epoch):
return ok(false)
try:
if rlnPeer.nullifierLog[externalNullifier].contains(proofMetadata):
if rlnPeer.nullifierLog[epoch].hasKey(nullifier):
# there is an identical record, mark it as spam
return ok(true)

# check for a message with the same nullifier but different secret shares
let matched = rlnPeer.nullifierLog[externalNullifier].filterIt((
it.nullifier == proofMetadata.nullifier) and ((it.shareX != proofMetadata.shareX) or
(it.shareY != proofMetadata.shareY)))

if matched.len != 0:
# there is a duplicate
return ok(true)

# there is no duplicate
return ok(false)

except KeyError as e:
except KeyError:
return err("the epoch was not found")
rymnc marked this conversation as resolved.
Show resolved Hide resolved

proc updateLog*(rlnPeer: WakuRLNRelay,
epoch: Epoch,
proofMetadata: ProofMetadata): RlnRelayResult[void] =
## saves supplied proofMetadata `proofMetadata`
## in the `nullifierLog` of the `rlnPeer`
## Returns an error if it cannot update the log

let externalNullifier = proofMetadata.externalNullifier
# check if the externalNullifier exists
if not rlnPeer.nullifierLog.hasKey(externalNullifier):
rlnPeer.nullifierLog[externalNullifier] = @[proofMetadata]
# check if the epoch exists
if not rlnPeer.nullifierLog.hasKeyOrPut(epoch, { proofMetadata.nullifier: proofMetadata }.toTable()):
return ok()

try:
# check if an identical record exists
if rlnPeer.nullifierLog[externalNullifier].contains(proofMetadata):
if rlnPeer.nullifierLog[epoch].hasKeyOrPut(proofMetadata.nullifier, proofMetadata):
# the above condition could be `discarded` but it is kept for clarity, that slashing will
# be implemented here
# TODO: slashing logic
return ok()
# add proofMetadata to the log
rlnPeer.nullifierLog[externalNullifier].add(proofMetadata)
return ok()
except KeyError as e:
return err("the external nullifier was not found") # should never happen
except KeyError:
return err("the epoch was not found") # should never happen
rymnc marked this conversation as resolved.
Show resolved Hide resolved

proc getCurrentEpoch*(rlnPeer: WakuRLNRelay): Epoch =
## gets the current rln Epoch time
Expand Down Expand Up @@ -249,7 +240,7 @@ proc validateMessage*(rlnPeer: WakuRLNRelay,
if proofMetadataRes.isErr():
waku_rln_errors_total.inc(labelValues=["proof_metadata_extraction"])
return MessageValidationResult.Invalid
let hasDup = rlnPeer.hasDuplicate(proofMetadataRes.get())
let hasDup = rlnPeer.hasDuplicate(msgEpoch, proofMetadataRes.get())
if hasDup.isErr():
waku_rln_errors_total.inc(labelValues=["duplicate_check"])
elif hasDup.value == true:
Expand Down Expand Up @@ -282,7 +273,7 @@ proc validateMessageAndUpdateLog*(
return MessageValidationResult.Invalid

# insert the message to the log (never errors)
discard rlnPeer.updateLog(proofMetadataRes.get())
discard rlnPeer.updateLog(msgProof.epoch, proofMetadataRes.get())

return result

Expand Down
Loading