Skip to content

feat(rln): generate e2e verifiable proofs using an encoder#2767

Draft
adklempner wants to merge 9 commits into
masterfrom
feat/rln-encoder
Draft

feat(rln): generate e2e verifiable proofs using an encoder#2767
adklempner wants to merge 9 commits into
masterfrom
feat/rln-encoder

Conversation

@adklempner

Copy link
Copy Markdown
Member

Problem / Description

Solution

Notes


Checklist

  • Code changes are covered by unit tests.
  • Code changes are covered by e2e tests, if applicable.
  • Dogfooding has been performed, if feasible.
  • A test version has been published, if required.
  • All CI checks pass successfully.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR implements end-to-end verifiable proof generation for RLN (Rate Limiting Nullifier) using a new encoder pattern. It adds the ability to generate and verify RLN proofs directly in the browser/node environment, enabling rate-limited message publishing in Waku.

Key Changes

  • Adds RLNEncoder class that wraps existing encoders to automatically attach RLN proofs to messages
  • Implements proof generation using the new @waku/zerokit-rln-wasm-utils library for cryptographic operations
  • Adds comprehensive utility functions for Merkle tree operations, byte conversions, and epoch handling

Reviewed changes

Copilot reviewed 19 out of 22 changed files in this pull request and generated 16 comments.

Show a summary per file
File Description
packages/rln/src/zerokit.ts Adds generateRLNProof and verifyRLNProof methods; refactors to use new wasm-utils library
packages/rln/src/codec.ts New file implementing RLNEncoder class that auto-generates proofs for messages
packages/rln/src/proof.ts New file defining Proof class that implements IRateLimitProof interface
packages/rln/src/utils/merkle.ts New utilities for Merkle tree operations (root reconstruction, rate commitment calculation)
packages/rln/src/utils/bytes.ts Adds bytes32FromBigInt method for BigInt to byte array conversions
packages/rln/src/utils/epoch.ts Refactors epoch handling; adds dateToEpochBytes and dateToNanosecondBytes functions
packages/rln/src/utils/hash.ts Migrates to use hash functions from @waku/zerokit-rln-wasm-utils
packages/rln/src/keystore/keystore.ts Adds bigIntReplacer function for proper JSON serialization of BigInt values
packages/rln/src/encoder.node.spec.ts New integration test that sends messages with RLN proofs to nwaku nodes
packages/rln/src/proof.spec.ts New unit tests for proof generation, validation, and parsing
packages/rln/src/zerokit.browser.spec.ts New browser test for seeded identity credential generation
packages/rln/src/test-utils/start-nwaku-fleet.ts New utility script to start a fleet of nwaku Docker nodes for testing
packages/rln/src/test-utils/run-integration-tests.js New test runner that orchestrates nwaku fleet and Karma tests
packages/rln/src/utils/test_keystore.ts Test data with keystore, merkle proof, and membership information
packages/rln/src/scripts/update_merkle_proof.ts Utility script to fetch and update merkle proof data from contract
packages/rln/karma.conf.cjs Updated to exclude node integration tests and serve new wasm-utils files
packages/rln/karma.node.conf.cjs New Karma config specifically for node integration tests
packages/rln/package.json Adds @waku/zerokit-rln-wasm-utils dependency; updates test scripts

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +119 to +131
// Handle graceful shutdown
process.on("SIGINT", () => {
console.log("\nReceived SIGINT, stopping fleet...");
void stopFleet().then(() => process.exit(0));
});

process.on("SIGTERM", () => {
console.log("\nReceived SIGTERM, stopping fleet...");
void stopFleet().then(() => process.exit(0));
});

// Keep process alive
await new Promise(() => {});

Copilot AI Dec 18, 2025

Copy link

Choose a reason for hiding this comment

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

This creates a Promise that never resolves or rejects, which will keep the process alive indefinitely. While this works, it could be clearer to use a simple setInterval or explicitly document the intentional infinite wait. Additionally, there's no error handling if startFleet fails - the signal handlers won't be registered.

Suggested change
// Handle graceful shutdown
process.on("SIGINT", () => {
console.log("\nReceived SIGINT, stopping fleet...");
void stopFleet().then(() => process.exit(0));
});
process.on("SIGTERM", () => {
console.log("\nReceived SIGTERM, stopping fleet...");
void stopFleet().then(() => process.exit(0));
});
// Keep process alive
await new Promise(() => {});
// Handle graceful shutdown and keep process alive until a signal is received
await new Promise<void>((resolve) => {
const handleShutdown = async (signal: string) => {
console.log(`\nReceived ${signal}, stopping fleet...`);
try {
await stopFleet();
} catch (err) {
console.log(`Error during shutdown: ${err}`);
} finally {
resolve();
}
};
process.on("SIGINT", () => {
void handleShutdown("SIGINT");
});
process.on("SIGTERM", () => {
void handleShutdown("SIGTERM");
});
});

Copilot uses AI. Check for mistakes.
Comment thread packages/rln/src/proof.ts
rlnIdentifier: Uint8Array
) {
if (proofBytes.length < nullifierOffset) {
throw new Error("invalid proof");

Copilot AI Dec 18, 2025

Copy link

Choose a reason for hiding this comment

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

The error message "invalid proof" is too generic. It should specify the actual issue, such as "proof bytes are too short: expected at least 288 bytes, got {proofBytes.length}" to help with debugging.

Suggested change
throw new Error("invalid proof");
throw new Error(
`invalid proof: expected at least ${nullifierOffset} bytes, got ${proofBytes.length}`
);

Copilot uses AI. Check for mistakes.
Comment on lines +117 to +118
const result = await waku.lightPush.send(rlnEncoder, message);
expect(result.successes.length).to.be.greaterThan(0);

Copilot AI Dec 18, 2025

Copy link

Choose a reason for hiding this comment

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

The test only verifies that the message was sent successfully but does not verify that the RLN proof was actually generated and attached to the message, or that it's valid. Consider adding assertions to check that message.rateLimitProof is populated and potentially verify the proof using verifyRLNProof.

Copilot uses AI. Check for mistakes.
Comment thread packages/rln/src/proof.ts
Comment on lines +23 to +31
export class Proof implements IRateLimitProof {
public readonly proof: Uint8Array;
public readonly merkleRoot: Uint8Array;
public readonly externalNullifier: Uint8Array;
public readonly shareX: Uint8Array;
public readonly shareY: Uint8Array;
public readonly nullifier: Uint8Array;
public readonly epoch: Uint8Array;
public readonly rlnIdentifier: Uint8Array;

Copilot AI Dec 18, 2025

Copy link

Choose a reason for hiding this comment

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

The Proof class includes an externalNullifier field (line 26) that is not part of the IRateLimitProof interface it implements. This creates an inconsistency where the Proof class has additional public fields beyond what the interface defines. Either the interface needs to be updated to include externalNullifier, or this field should be removed/made private if it's not intended to be part of the public API.

Copilot uses AI. Check for mistakes.
Comment thread packages/rln/src/codec.ts
pathElements,
identityPathIndex,
this.rateLimit,
0 // TODO: need to track messages sent per epoch

Copilot AI Dec 18, 2025

Copy link

Choose a reason for hiding this comment

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

The messageId is hardcoded to 0, which means all messages in an epoch will have the same messageId. This is a critical issue for RLN because it breaks the rate limiting mechanism - the system won't be able to track how many messages a user has sent in an epoch. This needs to be tracked per epoch, likely using a counter that resets when the epoch changes.

Copilot uses AI. Check for mistakes.
Comment thread packages/rln/src/proof.ts
Comment on lines +53 to +56
throw new Error("invalid epoch");
}
if (rlnIdentifier.length !== 32) {
throw new Error("invalid rlnIdentifier");

Copilot AI Dec 18, 2025

Copy link

Choose a reason for hiding this comment

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

The error messages "invalid epoch" and "invalid rlnIdentifier" are too generic. They should specify the actual issue, such as "epoch must be 32 bytes, got {epoch.length}" to help with debugging.

Suggested change
throw new Error("invalid epoch");
}
if (rlnIdentifier.length !== 32) {
throw new Error("invalid rlnIdentifier");
throw new Error(`invalid epoch: expected 32 bytes, got ${epoch.length}`);
}
if (rlnIdentifier.length !== 32) {
throw new Error(
`invalid rlnIdentifier: expected 32 bytes, got ${rlnIdentifier.length}`
);

Copilot uses AI. Check for mistakes.

// Run the tests
await runKarmaTests();
} catch (error) {

Copilot AI Dec 18, 2025

Copy link

Choose a reason for hiding this comment

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

The error is caught but not logged, which makes debugging failures difficult. The error should be logged with console.error before setting exitCode to 1, similar to how it's done in the outer catch block on line 147.

Suggested change
} catch (error) {
} catch (error) {
console.error("Error during integration tests:", error);

Copilot uses AI. Check for mistakes.
Comment thread packages/rln/package.json
"@waku/build-utils": "^1.0.0",
"@waku/interfaces": "0.0.34",
"@waku/message-encryption": "^0.0.37",
"@waku/sdk": "^0.0.36",

Copilot AI Dec 18, 2025

Copy link

Choose a reason for hiding this comment

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

The @waku/interfaces package was removed from devDependencies, but it is still imported in codec.ts and proof.ts. This will cause build errors. The package should either be kept in devDependencies or moved to dependencies, or the code should import from @waku/sdk or another package that exports these interfaces.

Suggested change
"@waku/sdk": "^0.0.36",
"@waku/sdk": "^0.0.36",
"@waku/interfaces": "^0.0.36",

Copilot uses AI. Check for mistakes.
Comment on lines +106 to +108
throw new Error(`Path elements must be ${MERKLE_TREE_DEPTH} bytes`);
if (identityPathIndex.length !== MERKLE_TREE_DEPTH)
throw new Error(`Identity path index must be ${MERKLE_TREE_DEPTH} bytes`);

Copilot AI Dec 18, 2025

Copy link

Choose a reason for hiding this comment

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

The error message says "Identity path index must be 20 bytes" but it should say "Identity path index must have 20 elements" or "Identity path index array length must be 20". The current message is misleading as each element is 1 byte, not the total being 20 bytes.

Suggested change
throw new Error(`Path elements must be ${MERKLE_TREE_DEPTH} bytes`);
if (identityPathIndex.length !== MERKLE_TREE_DEPTH)
throw new Error(`Identity path index must be ${MERKLE_TREE_DEPTH} bytes`);
throw new Error(
`Path elements array length must be ${MERKLE_TREE_DEPTH}, got ${pathElements.length}`
);
if (identityPathIndex.length !== MERKLE_TREE_DEPTH)
throw new Error(
`Identity path index array length must be ${MERKLE_TREE_DEPTH}, got ${identityPathIndex.length}`
);

Copilot uses AI. Check for mistakes.
if (signalLength.length !== 8)
throw new Error("signalLength must be 8 bytes");
if (proof.length !== 288) throw new Error("proof must be 288 bytes");
if (roots.length == 0) throw new Error("roots array is empty");

Copilot AI Dec 18, 2025

Copy link

Choose a reason for hiding this comment

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

Using loose equality (==) instead of strict equality (===) for checking array length. This could lead to unexpected type coercion. Use strict equality to avoid potential bugs.

Suggested change
if (roots.length == 0) throw new Error("roots array is empty");
if (roots.length === 0) throw new Error("roots array is empty");

Copilot uses AI. Check for mistakes.
@@ -0,0 +1,120 @@
import { multiaddr } from "@multiformats/multiaddr";

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit: encoder.node.spec.ts is a weird name, why .node.?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants