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

add tasks for activating E3, removing ciphernodes and generating sibling data #146

Merged
merged 2 commits into from
Oct 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
48 changes: 48 additions & 0 deletions packages/evm/tasks/ciphernode.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { LeanIMT } from "@zk-kit/lean-imt";
import { task } from "hardhat/config";
import type { TaskArguments } from "hardhat/types";
import { poseidon2 } from "poseidon-lite";

task("ciphernode:add", "Register a ciphernode to the registry")
.addParam("ciphernodeAddress", "address of ciphernode to register")
Expand All @@ -18,3 +20,49 @@ task("ciphernode:add", "Register a ciphernode to the registry")

console.log(`Ciphernode ${taskArguments.ciphernodeAddress} registered`);
});

task("ciphernode:remove", "Remove a ciphernode from the registry")
.addParam("ciphernodeAddress", "address of ciphernode to remove")
.addParam("siblings", "comma separated siblings from tree proof")
.setAction(async function (taskArguments: TaskArguments, hre) {
const registry = await hre.deployments.get("CiphernodeRegistryOwnable");

const registryContract = await hre.ethers.getContractAt(
"CiphernodeRegistryOwnable",
registry.address,
);

const siblings = taskArguments.siblings
.split(",")
.map((s: string) => BigInt(s));
Comment on lines +35 to +37
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add validation for 'siblings' input

Similar to ciphernodeAddresses, the siblings input is parsed and converted to BigInt without validation. Malformed inputs could cause exceptions. It's recommended to validate or handle potential errors when parsing siblings.

Example of adding input validation:

    const siblings = taskArguments.siblings
      .split(",")
-     .map((s: string) => BigInt(s));
+     .map((s: string) => {
+       try {
+         return BigInt(s);
+       } catch (e) {
+         throw new Error(`Invalid sibling value: ${s}`);
+       }
+     });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const siblings = taskArguments.siblings
.split(",")
.map((s: string) => BigInt(s));
const siblings = taskArguments.siblings
.split(",")
.map((s: string) => {
try {
return BigInt(s);
} catch (e) {
throw new Error(`Invalid sibling value: ${s}`);
}
});


const tx = await registryContract.removeCiphernode(
taskArguments.ciphernodeAddress,
siblings,
);
await tx.wait();

console.log(`Ciphernode ${taskArguments.ciphernodeAddress} removed`);
});

task("ciphernode:siblings", "Get the sibling of a ciphernode in the registry")
.addParam("ciphernodeAddress", "address of ciphernode to get siblings for")
.addParam(
"ciphernodeAddresses",
"comma separated addresses of ciphernodes in the order they were added to the registry",
)
.setAction(async function (taskArguments: TaskArguments) {
const hash = (a: bigint, b: bigint) => poseidon2([a, b]);
const tree = new LeanIMT(hash);

const addresses = taskArguments.ciphernodeAddresses.split(",");

for (const address of addresses) {
tree.insert(BigInt(address));
}
Comment on lines +58 to +62
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Validate 'ciphernodeAddresses' input before processing

When processing user-provided inputs like ciphernodeAddresses, converting each address to BigInt without validation may result in runtime errors if the input is invalid. Consider adding input validation or error handling to ensure robustness.

Here's how you might add input validation:

    const addresses = taskArguments.ciphernodeAddresses.split(",");

    for (const address of addresses) {
+     let addressBigInt;
+     try {
+       addressBigInt = BigInt(address);
+     } catch (e) {
+       console.error(`Invalid address: ${address}`);
+       return;
+     }
      tree.insert(addressBigInt);
    }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const addresses = taskArguments.ciphernodeAddresses.split(",");
for (const address of addresses) {
tree.insert(BigInt(address));
}
const addresses = taskArguments.ciphernodeAddresses.split(",");
for (const address of addresses) {
let addressBigInt;
try {
addressBigInt = BigInt(address);
} catch (e) {
console.error(`Invalid address: ${address}`);
return;
}
tree.insert(addressBigInt);
}


const index = tree.indexOf(BigInt(taskArguments.ciphernodeAddress));
const { siblings } = tree.generateProof(index);

Comment on lines +64 to +66
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add error handling for missing ciphernode address in the tree

The code currently assumes that the ciphernodeAddress exists in the tree. If the address is not found, tree.indexOf may return -1, which could lead to errors when calling tree.generateProof(index). It's advisable to check if the index is valid before proceeding.

Apply this diff to handle cases where the ciphernode address is not found:

    const index = tree.indexOf(BigInt(taskArguments.ciphernodeAddress));
+   if (index === -1) {
+     console.error(`Ciphernode address ${taskArguments.ciphernodeAddress} not found in the tree.`);
+     return;
+   }
    const { siblings } = tree.generateProof(index);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const index = tree.indexOf(BigInt(taskArguments.ciphernodeAddress));
const { siblings } = tree.generateProof(index);
const index = tree.indexOf(BigInt(taskArguments.ciphernodeAddress));
if (index === -1) {
console.error(`Ciphernode address ${taskArguments.ciphernodeAddress} not found in the tree.`);
return;
}
const { siblings } = tree.generateProof(index);

console.log(`Siblings for ${taskArguments.ciphernodeAddress}: ${siblings}`);
});
18 changes: 18 additions & 0 deletions packages/evm/tasks/enclave.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,24 @@ task(
console.log(`Committee requested`);
});

task("enclave:enableE3", "Enable an E3 program")
.addParam("e3Address", "address of the E3 program")
.setAction(async function (taskArguments: TaskArguments, hre) {
const enclave = await hre.deployments.get("Enclave");

const enclaveContract = await hre.ethers.getContractAt(
"Enclave",
enclave.address,
);

const tx = await enclaveContract.enableE3Program(taskArguments.e3Address);

console.log("Enabling E3 program... ", tx.hash);
await tx.wait();

console.log(`E3 program enabled`);
});

task("committee:publish", "Publish the publickey of the committee")
.addOptionalParam(
"filter",
Expand Down
Loading