refactor(data): refactor parameter validation using @zk-kit/utils #938

FaezehShakouri · 2025-01-13T20:43:56Z

Description

This pull request replaces the custom checkParameter function with the errorHandlers utility from the @zk-kit/utils package for parameter validation across multiple files. This change improves code consistency and leverages existing utility functions for better error handling.

Current Behavior: The code uses a custom function checkParameter to validate parameters.
New Behavior: The code now uses errorHandlers.requireString and errorHandlers.requireObject for parameter validation, which provides a more standardized approach to error handling.

Note: Currently, there is no requireBoolean function available in @zk-kit/utils, so we are using requireDefined for parameter validation in the meantime.

Related Issue(s)

Closes #773

Other information

No visual changes are introduced by this PR, as it primarily focuses on refactoring the parameter validation logic.

Checklist

My code follows the style guidelines of this project
I have performed a self-review of my code
I have commented my code, particularly in hard-to-understand areas
My changes generate no new warnings
I have run yarn format and yarn lint without getting any errors
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes

Co-authored-by: Vivian Plasencia <[email protected]>

…/convert-hex-pk Convert hexadecimal private key before signing messages

Turn on solidity optimizer re semaphore-protocol#740

perf(contracts): turn on solidity optimizer

fix: incorrect alias check brought from snarkjs. (possible security vulnerability)

…s/update-code Update Solidity code to create groups in documentation website

…/baby-jubjub Add check to make sure Baby Jubjub secret scalar is < l

Don't version control `asdf` and `direnv` configuration files

…/install-circom-deps ci: fix circom deps installation steps

…/docs docs: fix `typedoc` compilation warnings

…ignore chore: update `.gitignore`

…emaphore-protocol#747) * chore(proof): bump `@zk-kit/utils` dep * refactor(proof): use `maybeGetSemaphoreSnarkArtifacts` from `@zk-kit/utils` Delete logic related to fetching snark artifacts (wasm and zkey files) that was moved to `@zk-kit/utils` * revert(proof): add back `requireObject(snarkArtifacts)` check * chore(proof): update rollup.browser.config.ts * docs(proof): update README * chore(proof): remove unused import in rollup.browser.config.ts * Update packages/proof/package.json Co-authored-by: Cedoor <[email protected]> * docs(proof): add links to other repos in proof README * chore: bump `yarn.lock` * docs(proof): add punctuation --------- Co-authored-by: Cedoor <[email protected]>

chore: remove heyauthn package The heyauthn package was moved to the semaphore-protocol extensions repo. re semaphore-protocol#752

re semaphore-protocol#713, semaphore-protocol#675

* chore(docs): format `mdx` files with `remark` `prettier` doesn't not have proper support for MDX v3. Docusaurus recommends using `remark` instead. https://docusaurus.io/docs/markdown-features/react re semaphore-protocol#503 * chore(docs): add remark lint plugins * chore: bump yarn.lock * chore: add `quiet` option to remark

Signed-off-by: hanghuge <[email protected]>

docs(website): update roadmap on the website

…tocol#945) * docs: make year update dynamically on website and docs * docs(docs): make year update dynamically

* broken redirect CONTRIBUTING.md * typo README.md

fix: circuit logic url

) * chore(subgraph): update matchstick-as dependency * chore(subgraph): update dependency versions * ci: update pull request workflow to install libssl1.1 before testing the subgraph * ci: update pull requests workflow * ci: update ubuntu version * ci: update ubuntu version in tests * ci: update ubuntu version

* Add new article * Update apps/website/src/data/articles.json remove referral link Co-authored-by: Vivian Plasencia <[email protected]> --------- Co-authored-by: Vivian Plasencia <[email protected]>

vplasencia

Hey @FaezehShakouri! Great work! Thank you very much! I just left some comments.

vplasencia · 2025-01-29T23:02:30Z

packages/data/src/ethers.ts

@@ -17,8 +17,8 @@ import {
    Provider
 } from "ethers/providers"
 import { SemaphoreABI } from "@semaphore-protocol/utils/constants"
+import { errorHandlers } from "@zk-kit/utils"


Suggested change

import { errorHandlers } from "@zk-kit/utils"

import { requireString } from "@zk-kit/utils/error-handlers"

The module can be imported directly. This way it's no longer necessary to use errorHandlers.requireString and requireString can be used directly.

vplasencia · 2025-01-29T23:06:56Z

packages/data/src/subgraph.ts


        const { members = false, validatedProofs = false } = options

-        checkParameter(members, "members", "boolean")
-        checkParameter(validatedProofs, "validatedProofs", "boolean")
+        // NOTE: There is no function to check if a parameter is a boolean, so currently we are just checking if it is defined or not


I just created a PR to support requireBoolean , see privacy-scaling-explorations/zk-kit#373.

When the new version of the package is ready, this section can be updated.

Hey! There is a new version of the @zk-kit/utils package that supports requireBoolean . Feel free to update the version in the project and use the new function.

The packages using the @zk-kit/utils library are: group, identity, proof.

The @zk-kit/utils library should also be added as a dependency to the data package.

vplasencia · 2025-01-29T23:13:36Z

Hey @FaezehShakouri! Next time, please make sure to ask for the issue first, get it assigned, and then start working on it. Thank you for this PR.

Could you write something in the issue so that we can assign it to you?

vplasencia · 2025-01-31T12:20:40Z

packages/data/src/subgraph.ts


        const { members = false, validatedProofs = false } = options

-        checkParameter(members, "members", "boolean")
-        checkParameter(validatedProofs, "validatedProofs", "boolean")
+        // NOTE: There is no function to check if a parameter is a boolean, so currently we are just checking if it is defined or not


Hey! There is a new version of the @zk-kit/utils package that supports requireBoolean . Feel free to update the version in the project and use the new function.

The packages using the @zk-kit/utils library are: group, identity, proof.

The @zk-kit/utils library should also be added as a dependency to the data package.

vplasencia · 2025-01-31T12:28:50Z

packages/data/src/subgraph.ts

@@ -1,6 +1,6 @@
 import { defaultNetwork, SupportedNetwork } from "@semaphore-protocol/utils/networks"
 import { AxiosRequestConfig } from "axios"
-import checkParameter from "./checkParameter"
+import { errorHandlers } from "@zk-kit/utils"


Suggested change

import { errorHandlers } from "@zk-kit/utils"

import { requireString, requireObject, requireBoolean } from "@zk-kit/utils/error-handlers"

Similar to the ethers.ts file, you can use the error-handlers module directly.

…ct, and requireBoolean

openzeppelin-code · 2025-02-01T10:43:30Z

refactor(data): refactor parameter validation using @zk-kit/utils

Generated at commit: 2ba7e82628a6894ea2cbd928fab98bbf90674dd4

🚨 Report Summary

	Severity Level	Results
Contracts	Critical High Medium Low Note Total	0 0 0 4 15 19
Dependencies	Critical High Medium Low Note Total	0 0 0 0 0 0

For more details view the full report in OpenZeppelin Code Inspector

FaezehShakouri · 2025-02-01T10:48:52Z

I'm closing this PR because I accidentally rewrote the commit messages. I will open another PR. Sorry for the inconvenience.

cedoor and others added 30 commits March 28, 2024 17:57

Update packages/identity/tests/index.test.ts

2d63616

Co-authored-by: Vivian Plasencia <[email protected]>

Merge pull request semaphore-protocol#734 from semaphore-protocol/fix…

c8aff52

…/convert-hex-pk Convert hexadecimal private key before signing messages

chore: v4.0.0-beta.7

b97c91e

fix(circuits): check whether the secret scalar is within the r field

7f8d303

chore: update lockfile

4b52c56

fix: incorrect alias check brought from snarkjs.

3bef8cb

perf(contracts): turn on solidity optimizer

58a7377

Turn on solidity optimizer re semaphore-protocol#740

Merge pull request semaphore-protocol#741 from 0xbok/sol-op

df35284

perf(contracts): turn on solidity optimizer

Merge pull request semaphore-protocol#737 from Stumble/yumin/fix-alias

49a7f29

fix: incorrect alias check brought from snarkjs. (possible security vulnerability)

refactor(circuits): remove greater-than check and update less-than bits

51c7a31

test(circuits): add new test for l = r - 1

b1ad26a

docs(docs): update solidity code to create groups

946b74d

Merge pull request semaphore-protocol#742 from semaphore-protocol/doc…

c6b60b4

…s/update-code Update Solidity code to create groups in documentation website

chore: update lockfile

c7122d8

Merge pull request semaphore-protocol#743 from semaphore-protocol/fix…

fa26335

…/baby-jubjub Add check to make sure Baby Jubjub secret scalar is < l

chore: update .gitignore

97c86cc

Don't version control `asdf` and `direnv` configuration files

docs: fix typedoc entryPoints

90b9e84

docs: fix warnings about function signatures

c33e3ca

ci: debug pull-requests workflow

b1588fe

ci: remove microsoft.prod sources file

2704563

chore: format

8b098ee

ci: add comment

ef7ff4e

Merge pull request semaphore-protocol#751 from semaphore-protocol/fix…

203266b

…/install-circom-deps ci: fix circom deps installation steps

Merge pull request semaphore-protocol#750 from semaphore-protocol/fix…

1856414

…/docs docs: fix `typedoc` compilation warnings

Merge pull request semaphore-protocol#748 from semaphore-protocol/git…

5830c99

…ignore chore: update `.gitignore`

Move heyauthn package (semaphore-protocol#753)

2e1fe06

chore: remove heyauthn package The heyauthn package was moved to the semaphore-protocol extensions repo. re semaphore-protocol#752

fix: update snarkjs dependency version (semaphore-protocol#755)

4a30534

re semaphore-protocol#713, semaphore-protocol#675

chore: v4.0.0-beta.8

3c5c4a5

hanghuge and others added 7 commits January 20, 2025 12:43

chore: fix 404 status URL (semaphore-protocol#917)

01aff91

Signed-off-by: hanghuge <[email protected]>

docs(website): update roadmap (semaphore-protocol#943)

38dcd98

docs(website): update roadmap on the website

docs: make year update dynamically on website and docs (semaphore-pro…

b2e5e65

…tocol#945) * docs: make year update dynamically on website and docs * docs(docs): make year update dynamically

docs: fix broken link and typo correction (semaphore-protocol#923)

f64835d

* broken redirect CONTRIBUTING.md * typo README.md

docs: fixed circuit logic url (semaphore-protocol#931)

ea57b1d

fix: circuit logic url

Add new article (semaphore-protocol#918)

ea0fa1f

* Add new article * Update apps/website/src/data/articles.json remove referral link Co-authored-by: Vivian Plasencia <[email protected]> --------- Co-authored-by: Vivian Plasencia <[email protected]>

vplasencia requested changes Jan 29, 2025

View reviewed changes

vercel bot deployed to Preview – semaphore-website January 30, 2025 17:18 View deployment

vercel bot deployed to Preview – semaphore-docs January 30, 2025 17:20 View deployment

vplasencia requested changes Jan 31, 2025

View reviewed changes

vercel bot deployed to Preview – semaphore-docs January 31, 2025 16:06 View deployment

vercel bot deployed to Preview – semaphore-website January 31, 2025 16:08 View deployment

FaezehShakouri added 4 commits January 31, 2025 17:18

refactor(data): refactor parameter validation using @zk-kit/utils

a102a4f

refactor(data): only import requireString

222afbc

chore(data): add @zk-kit/utils dependency

4f9f9d9

refactor(data): replace errorHandlers with requireString, requireObje…

e58660e

…ct, and requireBoolean

FaezehShakouri force-pushed the refactor/validation branch from 0b0b4fb to 0e5cf7c Compare January 31, 2025 16:23

vercel bot had a problem deploying to Preview – semaphore-website January 31, 2025 16:23 Failure

vercel bot had a problem deploying to Preview – semaphore-docs January 31, 2025 16:24 Failure

chore(deps): remove unused JSONStream package version

2ba7e82

vercel bot deployed to Preview – semaphore-website January 31, 2025 16:42 View deployment

vercel bot deployed to Preview – semaphore-docs January 31, 2025 16:44 View deployment

FaezehShakouri force-pushed the refactor/validation branch from 8fb970c to 2ba7e82 Compare February 1, 2025 10:41

vercel bot deployed to Preview – semaphore-docs February 1, 2025 10:42 View deployment

vercel bot deployed to Preview – semaphore-website February 1, 2025 10:44 View deployment

FaezehShakouri closed this Feb 1, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(data): refactor parameter validation using @zk-kit/utils #938

refactor(data): refactor parameter validation using @zk-kit/utils #938

FaezehShakouri commented Jan 13, 2025 •

edited

Loading

vplasencia left a comment

vplasencia Jan 29, 2025

vplasencia Jan 29, 2025

vplasencia Jan 31, 2025

vplasencia commented Jan 29, 2025 •

edited

Loading

vplasencia Jan 31, 2025

vplasencia Jan 31, 2025

openzeppelin-code bot commented Feb 1, 2025

FaezehShakouri commented Feb 1, 2025

	import { errorHandlers } from "@zk-kit/utils"
	import { requireString } from "@zk-kit/utils/error-handlers"

	import { errorHandlers } from "@zk-kit/utils"
	import { requireString, requireObject, requireBoolean } from "@zk-kit/utils/error-handlers"

refactor(data): refactor parameter validation using @zk-kit/utils #938

refactor(data): refactor parameter validation using @zk-kit/utils #938

Conversation

FaezehShakouri commented Jan 13, 2025 • edited Loading

Description

Related Issue(s)

Other information

Checklist

vplasencia left a comment

Choose a reason for hiding this comment

vplasencia Jan 29, 2025

Choose a reason for hiding this comment

vplasencia Jan 29, 2025

Choose a reason for hiding this comment

vplasencia Jan 31, 2025

Choose a reason for hiding this comment

vplasencia commented Jan 29, 2025 • edited Loading

vplasencia Jan 31, 2025

Choose a reason for hiding this comment

vplasencia Jan 31, 2025

Choose a reason for hiding this comment

openzeppelin-code bot commented Feb 1, 2025

refactor(data): refactor parameter validation using @zk-kit/utils

🚨 Report Summary

FaezehShakouri commented Feb 1, 2025

FaezehShakouri commented Jan 13, 2025 •

edited

Loading

vplasencia commented Jan 29, 2025 •

edited

Loading