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(rln-relay): process blocks atomically #1349

Merged
merged 41 commits into from
Nov 10, 2022
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
463e1a4
test(rln-relay): atomic block processing
rymnc Nov 7, 2022
9faa33f
fix(rln-relay): use correct starting index
rymnc Nov 7, 2022
6ab0031
fix(rln-relay): args
rymnc Nov 7, 2022
3e804a4
fix(rln-relay): append length
rymnc Nov 7, 2022
1468c36
fix(rln-relay): tests, remove insertMember
rymnc Nov 7, 2022
adb5c60
fix(rln-relay): camelCase, cleanup
rymnc Nov 7, 2022
0a60042
fix(rln-relay): actually process per block
rymnc Nov 7, 2022
83dae90
fix(rln-relay): clean up
rymnc Nov 7, 2022
dd8518e
chore(gitignore): Update .gitignore
rymnc Nov 7, 2022
6de3978
Merge branch 'master' into rln-add-new-zerokit-api
rymnc Nov 7, 2022
4551a7d
Update waku/v2/protocol/waku_rln_relay/waku_rln_relay_utils.nim
rymnc Nov 8, 2022
fb7853b
Update waku/v2/protocol/waku_rln_relay/waku_rln_relay_utils.nim
rymnc Nov 8, 2022
8b0ff46
fix(rln-relay): args
rymnc Nov 8, 2022
b77d7c6
fix(rln-relay): add prefix def
rymnc Nov 8, 2022
d40636d
fix(rln-relay): make test cleaner
rymnc Nov 8, 2022
83e8794
Merge branch 'master' into rln-add-new-zerokit-api
rymnc Nov 8, 2022
f23350e
Merge branch 'master' into rln-add-new-zerokit-api
rymnc Nov 8, 2022
84a72e3
chore(rln-relay): apply suggestions
rymnc Nov 9, 2022
8ca5216
chore(rln-relay): add member order check
rymnc Nov 9, 2022
c885640
test(rln-relay): batch insert in tests
rymnc Nov 9, 2022
032cc4c
fix(rln-relay): test batching
rymnc Nov 9, 2022
14f833b
fix(rln-relay): toSeq the HSlice
rymnc Nov 9, 2022
e239b61
fix(rln-relay): naming
rymnc Nov 9, 2022
37bd29f
fix(rln-relay): add insertMember back
rymnc Nov 9, 2022
ff02842
fix(rln-relay): serialize util, address review
rymnc Nov 9, 2022
8c08ed1
fix(rln-relay): add atomicity desc
rymnc Nov 9, 2022
0daf6c1
fix(rln-relay): inHex
rymnc Nov 9, 2022
0a5f79f
fix(rln-relay): explicit proc def
rymnc Nov 9, 2022
5a6ec33
fix(rln-relay): indexGap condition
rymnc Nov 9, 2022
ec52697
Merge branch 'master' into rln-add-new-zerokit-api
rymnc Nov 9, 2022
c40aee6
fix(rln-relay): func sig
rymnc Nov 9, 2022
33cba52
fix(rln-relay): onchain test
rymnc Nov 9, 2022
868be86
Merge branch 'master' into rln-add-new-zerokit-api
Nov 9, 2022
9835897
fix(rln-relay): use asyncSpawn vs asyncCheck
rymnc Nov 9, 2022
0ddcd11
fix(rln-relay): do not explicitly insert into the index
rymnc Nov 9, 2022
5919e05
fix(rln-relay): condition, semantics
rymnc Nov 9, 2022
103c80b
fix(rln-relay): index must be 1
rymnc Nov 9, 2022
c1d9c16
Merge branch 'master' into rln-add-new-zerokit-api
rymnc Nov 10, 2022
54dca37
chore(rln-relay): line br
rymnc Nov 10, 2022
71f9343
fix(rln-relay): missing return ok(true)
rymnc Nov 10, 2022
71c5bfd
Merge branch 'master' into rln-add-new-zerokit-api
rymnc Nov 10, 2022
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
8 changes: 8 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -43,3 +43,11 @@ testPath.txt

# Nimbus Build System
nimbus-build-system.paths

# sqlite db
*.db
*.db-shm
*.db-wal
*.sqlite3
*.sqlite3-shm
*.sqlite3-wal
42 changes: 21 additions & 21 deletions tests/v2/test_waku_rln_relay.nim
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ suite "Waku rln relay":
check:
deletionSuccess

test "insertMember rln utils":
test "insertMembers rln utils":
# create an RLN instance which also includes an empty Merkle tree
let rlnInstance = createRLNInstance()
require:
Expand All @@ -267,7 +267,7 @@ suite "Waku rln relay":
require:
keypairRes.isOk()
check:
rln.insertMember(keyPairRes.get().idCommitment)
rln.insertMembers(0, @[keyPairRes.get().idCommitment])
Copy link
Contributor

Choose a reason for hiding this comment

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

I would keep both insertMember (that calls set and sets next available leaf) and insertMembers, where you need to specify the index. The two API have different use cases and the first one is used where you don't care/know about next available index.

So I would structure this test by keeping previous checks and by having a batch insertion (that maybe replicates previous single inserts).

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 c40aee6


test "removeMember rln utils":
# create an RLN instance which also includes an empty Merkle tree
Expand Down Expand Up @@ -372,7 +372,7 @@ suite "Waku rln relay":
let keyPairRes = rln.membershipKeyGen()
require:
keyPairRes.isOk()
let memberInserted = rln.insertMember(keypairRes.get().idCommitment)
let memberInserted = rln.insertMembers(0, @[keypairRes.get().idCommitment])
require:
memberInserted

Expand Down Expand Up @@ -586,7 +586,7 @@ suite "Waku rln relay":

let
# peer's index in the Merkle Tree
index = 5
index = 5'u
# create a membership key pair
memKeysRes = membershipKeyGen(rln)

Expand All @@ -596,17 +596,17 @@ suite "Waku rln relay":
let memKeys = memKeysRes.get()

# Create a Merkle tree with random members
for i in 0..10:
for i in 0'u..10'u:
var memberAdded: bool = false
if (i == index):
# insert the current peer's pk
memberAdded = rln.insertMember(memKeys.idCommitment)
memberAdded = rln.insertMembers(i, @[memKeys.idCommitment])
else:
# create a new key pair
let memberKeysRes = rln.membershipKeyGen()
require:
memberKeysRes.isOk()
memberAdded = rln.insertMember(memberKeysRes.get().idCommitment)
memberAdded = rln.insertMembers(i, @[memberKeysRes.get().idCommitment])
# check the member is added
require:
memberAdded
Expand Down Expand Up @@ -645,7 +645,7 @@ suite "Waku rln relay":

let
# peer's index in the Merkle Tree
index = 5
index = 5'u
# create a membership key pair
memKeysRes = membershipKeyGen(rln)

Expand All @@ -655,17 +655,17 @@ suite "Waku rln relay":
let memKeys = memKeysRes.get()

# Create a Merkle tree with random members
for i in 0..10:
for i in 0'u..10'u:
var memberAdded: bool = false
if (i == index):
# insert the current peer's pk
memberAdded = rln.insertMember(memKeys.idCommitment)
memberAdded = rln.insertMembers(i, @[memKeys.idCommitment])
else:
# create a new key pair
let memberKeysRes = rln.membershipKeyGen()
require:
memberKeysRes.isOk()
memberAdded = rln.insertMember(memberKeysRes.get().idCommitment)
memberAdded = rln.insertMembers(i, @[memberKeysRes.get().idCommitment])
# check the member is added
require:
memberAdded
Expand Down Expand Up @@ -712,7 +712,7 @@ suite "Waku rln relay":

let
# peer's index in the Merkle Tree.
index = 5
index = 5'u
# create a membership key pair
memKeysRes = membershipKeyGen(rlnRelay.rlnInstance)

Expand All @@ -721,20 +721,20 @@ suite "Waku rln relay":

let memKeys = memKeysRes.get()

let membershipCount = AcceptableRootWindowSize + 5
let membershipCount: uint = AcceptableRootWindowSize + 5'u

# Create a Merkle tree with random members
for i in 0..membershipCount:
for i in 0'u..membershipCount:
Copy link
Contributor

Choose a reason for hiding this comment

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

This might be a good spot to test batch insertion: you generate a vector of leaves and replace the element at position index with the target commitment.

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 032cc4c

Copy link
Contributor

Choose a reason for hiding this comment

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

"members" is already in the form you need (i.e. at "index" you have the interesting commitment key): I meant why not just do a single batch addition from 0 of "members" rather than 3 operations as of now?

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 0ddcd11. I figured you wanted it to be inserted separately from your comment.

var memberIsAdded: RlnRelayResult[void]
if (i == index):
# insert the current peer's pk
memberIsAdded = rlnRelay.insertMember(memKeys.idCommitment)
memberIsAdded = rlnRelay.insertMembers(i, @[memKeys.idCommitment])
else:
# create a new key pair
let memberKeysRes = rlnRelay.rlnInstance.membershipKeyGen()
require:
memberKeysRes.isOk()
memberIsAdded = rlnRelay.insertMember(memberKeysRes.get().idCommitment)
memberIsAdded = rlnRelay.insertMembers(i, @[memberKeysRes.get().idCommitment])
# require that the member is added
require:
memberIsAdded.isOk()
Expand Down Expand Up @@ -804,7 +804,7 @@ suite "Waku rln relay":

let
# peer's index in the Merkle Tree.
index = 6
index = 6'u
# create a membership key pair
memKeysRes = membershipKeyGen(rlnRelay.rlnInstance)

Expand All @@ -813,20 +813,20 @@ suite "Waku rln relay":

let memKeys = memKeysRes.get()

let membershipCount = AcceptableRootWindowSize + 5
let membershipCount: uint = AcceptableRootWindowSize + 5'u

# Create a Merkle tree with random members
for i in 0..membershipCount:
for i in 0'u..membershipCount:
var memberIsAdded: RlnRelayResult[void]
if (i == index):
# insert the current peer's pk
memberIsAdded = rlnRelay.insertMember(memKeys.idCommitment)
memberIsAdded = rlnRelay.insertMembers(i, @[memKeys.idCommitment])
else:
# create a new key pair
let memberKeysRes = rlnRelay.rlnInstance.membershipKeyGen()
require:
memberKeysRes.isOk()
memberIsAdded = rlnRelay.insertMember(memberKeysRes.get().idCommitment)
memberIsAdded = rlnRelay.insertMembers(i, @[memberKeysRes.get().idCommitment])
# require that the member is added
require:
memberIsAdded.isOk()
Expand Down
33 changes: 16 additions & 17 deletions tests/v2/test_waku_rln_relay_onchain.nim
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
{.used.}

import
std/[options, osproc, streams, strutils],
std/[options, osproc, streams, strutils, sequtils],
testutils/unittests, chronos, chronicles, stint, web3, json,
stew/byteutils, stew/shims/net as stewNet,
libp2p/crypto/crypto,
Expand Down Expand Up @@ -278,14 +278,13 @@ procSuite "Waku-rln-relay":
let pk2 = keyPair2.idCommitment.toUInt256()
debug "membership commitment key", pk2 = pk2

var events = [newFuture[void](), newFuture[void]()]
proc handler(pubkey: Uint256, index: Uint256): RlnRelayResult[void] =
debug "handler is called", pubkey = pubkey, index = index
if pubkey == pk:
events[0].complete()
if pubkey == pk2:
events[1].complete()
let isSuccessful = rlnPeer.rlnInstance.insertMember(pubkey.toIDCommitment())
let event = newFuture[void]()
var handler: GroupUpdateHandler
handler = proc (blockNumber: BlockNumber,
members: seq[MembershipTuple]): RlnRelayResult[void] =
debug "handler is called", members = members
event.complete()
let isSuccessful = rlnPeer.rlnInstance.insertMembers(0, members.mapIt(it.idComm))
check:
isSuccessful
Copy link
Contributor

Choose a reason for hiding this comment

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

If possible, I'd move all the checks to the end of the test case, under the "THEN" section. Checks within a Future usually are problematic when debugging.

This can be taken as a separate piece. Not blocking, but important.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tracking #1358 thanks!

return ok()
Expand All @@ -305,8 +304,8 @@ procSuite "Waku-rln-relay":
let tx2 = await contractObj.register(pk2).send(value = MembershipFee)
debug "a member is registered", tx2 = tx2

# wait for all the events to be received by the rlnPeer
await all(events)
# wait for all the event to be processed
await event

# release resources -----------------------
await web3.close()
Expand Down Expand Up @@ -405,12 +404,12 @@ procSuite "Waku-rln-relay":

# Create a group of 10 members
var group = newSeq[IDCommitment]()
for i in 0..10:
for i in 0'u..10'u:
var memberAdded: bool = false
if (uint(i) == index):
if (i == index):
# insert the current peer's pk
group.add(keyPair.idCommitment)
memberAdded = rln.insertMember(keyPair.idCommitment)
memberAdded = rln.insertMembers(i, @[keyPair.idCommitment])
doAssert(memberAdded)
debug "member key", key = keyPair.idCommitment.inHex
else:
Expand All @@ -419,7 +418,7 @@ procSuite "Waku-rln-relay":
memberKeyPairRes.isOk()
let memberKeyPair = memberKeyPairRes.get()
group.add(memberKeyPair.idCommitment)
let memberAdded = rln.insertMember(memberKeyPair.idCommitment)
let memberAdded = rln.insertMembers(i, @[memberKeyPair.idCommitment])
require:
memberAdded
debug "member key", key = memberKeyPair.idCommitment.inHex
Expand Down Expand Up @@ -491,8 +490,8 @@ procSuite "Waku-rln-relay":

# add the rln keys to the Merkle tree
let
memberIsAdded1 = rln.insertMember(keyPair1.idCommitment)
memberIsAdded2 = rln.insertMember(keyPair2.idCommitment)
memberIsAdded1 = rln.insertMembers(0, @[keyPair1.idCommitment])
memberIsAdded2 = rln.insertMembers(1, @[keyPair2.idCommitment])

require:
memberIsAdded1
Expand Down
2 changes: 1 addition & 1 deletion vendor/zerokit
Submodule zerokit updated 3 files
+17 −1 CHANGELOG.md
+106 −5 rln/src/ffi.rs
+88 −7 rln/src/public.rs
10 changes: 9 additions & 1 deletion waku/v2/protocol/waku_rln_relay/rln.nim
Original file line number Diff line number Diff line change
Expand Up @@ -130,12 +130,20 @@ when defined(rlnzerokit):
## the input_buffer holds a serialized leaf of 32 bytes
## the return bool value indicates the success or failure of the operation

proc set_leaves*(ctx: ptr RLN, input_buffer: ptr Buffer): bool {.importc: "set_leaves".}
proc init_tree_with_leaves*(ctx: ptr RLN, input_buffer: ptr Buffer): bool {.importc: "init_tree_with_leaves".}
## sets multiple leaves in the tree stored by ctx to the value passed by input_buffer
## the input_buffer holds a serialized vector of leaves (32 bytes each)
## the input_buffer size is prefixed by a 32 bytes integer indicating the number of leaves
## leaves are set one after each other starting from index 0
## the return bool value indicates the success or failure of the operation

proc set_leaves_from*(ctx: ptr RLN, index: uint, input_buffer: ptr Buffer): bool {.importc: "set_leaves_from".}
## sets multiple leaves in the tree stored by ctx to the value passed by input_buffer
## the input_buffer holds a serialized vector of leaves (32 bytes each)
## the input_buffer size is prefixed by a 32 bytes integer indicating the number of leaves
## leaves are set one after each other starting from index `index`
## the return bool value indicates the success or failure of the operation

proc reset_tree*(ctx: ptr RLN, tree_height: uint): bool {.importc: "set_tree".}
## resets the tree stored by ctx to the the empty tree (all leaves set to 0) of height tree_height
## the return bool value indicates the success or failure of the operation
Expand Down
2 changes: 2 additions & 0 deletions waku/v2/protocol/waku_rln_relay/waku_rln_relay_types.nim
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ when defined(rln) or (not defined(rln) and not defined(rlnzerokit)):
lastEpoch*: Epoch # the epoch of the last published rln message
validMerkleRoots*: Deque[MerkleNode] # An array of valid merkle roots, which are updated in a FIFO fashion
lastSeenMembershipIndex*: MembershipIndex # the last seen membership index
lastProcessedBlock*: BlockNumber # the last processed block number

when defined(rlnzerokit):
type WakuRLNRelay* = ref object
Expand All @@ -137,6 +138,7 @@ when defined(rlnzerokit):
lastEpoch*: Epoch # the epoch of the last published rln message
validMerkleRoots*: Deque[MerkleNode] # An array of valid merkle roots, which are updated in a FIFO fashion
lastSeenMembershipIndex*: MembershipIndex # the last seen membership index
lastProcessedBlock*: BlockNumber # the last processed block number


type MessageValidationResult* {.pure.} = enum
Expand Down
Loading