-
Notifications
You must be signed in to change notification settings - Fork 2.7k
chore(common): checkSignatures, ZamaConfig.sol, mock publicDecrypt an… #1316
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
base: main
Are you sure you want to change the base?
Changes from all commits
4752748
4aa7b77
26082e3
fd33308
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| { | ||
| "root": true, | ||
| "extends": ["eslint:recommended", "prettier"], | ||
| "env": { | ||
| "es2022": true, | ||
| "browser": true, | ||
| "node": true, | ||
| "mocha": true | ||
| }, | ||
| "globals": { | ||
| "artifacts": "readonly", | ||
| "contract": "readonly", | ||
| "web3": "readonly", | ||
| "extendEnvironment": "readonly", | ||
| "expect": "readonly" | ||
| }, | ||
| "overrides": [ | ||
| { | ||
| "files": ["*.ts"], | ||
| "excludedFiles": ["./dist/**/*.js"], | ||
| "extends": ["eslint:recommended", "plugin:@typescript-eslint/recommended", "prettier"], | ||
| "parser": "@typescript-eslint/parser", | ||
| "parserOptions": { | ||
| "ecmaVersion": "latest", | ||
| "sourceType": "module" | ||
| }, | ||
| "plugins": ["@typescript-eslint"] | ||
| } | ||
| ] | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,133 @@ | ||
| import { FlatCompat } from "@eslint/eslintrc"; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why are all those eslint files needed now? linter was working before without those. |
||
| import eslint from "@eslint/js"; | ||
| import typescriptEslint from "@typescript-eslint/eslint-plugin"; | ||
| import tsParser from "@typescript-eslint/parser"; | ||
| import importPlugin from "eslint-plugin-import"; | ||
| import globals from "globals"; | ||
| import path from "node:path"; | ||
| import { fileURLToPath } from "node:url"; | ||
|
|
||
| const __filename = fileURLToPath(import.meta.url); | ||
| const __dirname = path.dirname(__filename); | ||
| const compat = new FlatCompat({ | ||
| baseDirectory: __dirname, | ||
| recommendedConfig: eslint.configs.recommended, | ||
| allConfig: eslint.configs.all, | ||
| }); | ||
|
|
||
| const config = [ | ||
| // 0 | ||
| { | ||
| ignores: ["typechain-types/*", "tmp/*", "coverage/*"], | ||
| }, | ||
| // 1 | ||
| ...compat.extends("eslint:recommended", "prettier"), | ||
| // 2 | ||
| { | ||
| languageOptions: { | ||
| globals: { | ||
| ...globals.browser, | ||
| ...globals.node, | ||
| ...globals.mocha, | ||
| artifacts: "readonly", | ||
| contract: "readonly", | ||
| web3: "readonly", | ||
| extendEnvironment: "readonly", | ||
| expect: "readonly", | ||
| }, | ||
| }, | ||
| }, | ||
| // 3 | ||
| ...compat | ||
| .extends( | ||
| "eslint:recommended", | ||
| "plugin:@typescript-eslint/strict", | ||
| "plugin:@typescript-eslint/strict-type-checked", | ||
| "prettier", | ||
| ) | ||
| .map((config) => ({ | ||
| ...config, | ||
| files: ["**/*.ts"], | ||
| rules: { | ||
| "@typescript-eslint/no-floating-promises": "error", | ||
| }, | ||
| })), | ||
| // 4 | ||
| { | ||
| files: ["**/*.ts"], | ||
| plugins: { | ||
| "@typescript-eslint": typescriptEslint, | ||
| }, | ||
| languageOptions: { | ||
| parser: tsParser, | ||
| parserOptions: { | ||
| project: "./tsconfig.json", | ||
| tsconfigRootDir: __dirname, | ||
| }, | ||
| ecmaVersion: "latest", | ||
| sourceType: "module", | ||
| }, | ||
| rules: { | ||
| "no-unused-vars": "off", | ||
| "@typescript-eslint/no-unused-vars": [ | ||
| "error", | ||
| { | ||
| vars: "all", | ||
| args: "after-used", | ||
| ignoreRestSiblings: true, | ||
| varsIgnorePattern: "^_", | ||
| argsIgnorePattern: "^_", | ||
| }, | ||
| ], | ||
| }, | ||
| }, | ||
| // 5 - Restrict "Buffer" usage | ||
| { | ||
| files: ["./src/internal/libs/**/*.{js,ts,tsx}"], | ||
| rules: { | ||
| "no-restricted-globals": [ | ||
| "error", | ||
| { | ||
| name: "Buffer", | ||
| message: "Avoid using Buffer in internal/libs. Use a custom abstraction if needed.", | ||
| }, | ||
| ], | ||
| }, | ||
| }, | ||
|
|
||
| // 6 - Enforce imports must be declared in dependencies | ||
| { | ||
| files: ["**/*.ts"], | ||
| plugins: { | ||
| import: importPlugin, | ||
| }, | ||
| rules: { | ||
| "import/no-extraneous-dependencies": [ | ||
| "error", | ||
| { | ||
| devDependencies: false, | ||
| optionalDependencies: false, | ||
| peerDependencies: true, | ||
| bundledDependencies: false, | ||
| }, | ||
| ], | ||
| }, | ||
| }, | ||
|
|
||
| // 7 - Relax rule 6 for tests | ||
| { | ||
| files: ["hardhat.config.ts", "**/test/**/*.{ts,js}", "**/*.test.{ts,js}"], | ||
| rules: { | ||
| "import/no-extraneous-dependencies": [ | ||
| "error", | ||
| { | ||
| devDependencies: true, | ||
| optionalDependencies: false, | ||
| peerDependencies: true, | ||
| }, | ||
| ], | ||
| }, | ||
| }, | ||
| ]; | ||
|
|
||
| export default config; | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -27,6 +27,9 @@ library FHE { | |||||
| /// @notice Returned if the returned KMS signatures are not valid. | ||||||
| error InvalidKMSSignatures(); | ||||||
|
|
||||||
| /// @notice This event is emitted when public decryption has been successfully verified. | ||||||
| event PublicDecryptionVerified(bytes32[] handlesList, bytes abiEncodedCleartexts); | ||||||
|
|
||||||
| /** | ||||||
| * @notice Sets the coprocessor addresses. | ||||||
| * @param coprocessorConfig Coprocessor config struct that contains contract addresses. | ||||||
|
|
@@ -9072,24 +9075,57 @@ library FHE { | |||||
| expirationDate = Impl.getUserDecryptionDelegationExpirationDate(delegator, delegate, contractAddress); | ||||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * @dev Internal low-level function used to verify the KMS signatures. | ||||||
| * @notice Warning: MUST be called directly in the callback function called by the relayer. | ||||||
| * @notice Warning: this function never reverts, its boolean return value must be checked. | ||||||
| * @dev clearTexts is the abi-encoding of the list of all decrypted values assiociated to handlesList, in same order. | ||||||
| * @dev Only static native solidity types for clear values are supported, so clearTexts is the concatenation of all clear values appended to 32 bytes. | ||||||
| * @dev decryptionProof contains KMS signatures corresponding to clearTexts and associated handlesList, and needed metadata for KMS context. | ||||||
| **/ | ||||||
| function verifySignatures( | ||||||
| /// @notice Reverts if the KMS signatures verification against the provided handles and public decryption data | ||||||
| /// fails. | ||||||
| /// @dev The function MUST be called inside a public decryption callback function of a dApp contract | ||||||
| /// to verify the signatures and prevent fake decryption results for being submitted. | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| /// @param handlesList The list of handles as an array of bytes32 to check | ||||||
| /// @param abiEncodedCleartexts The ABI-encoded list of decrypted values associated with each handle in the `handlesList`. | ||||||
| /// The ABI-encoded list order must match the `handlesList` order. | ||||||
| /// @param decryptionProof The KMS public decryption proof. It includes the KMS signatures, associated metadata, | ||||||
| /// and the context needed for verification. | ||||||
| /// @dev Reverts if any of the following conditions are met: | ||||||
| /// - The `decryptionProof` is empty or has an invalid length. | ||||||
| /// - The number of valid signatures is zero or less than the configured KMS signers threshold. | ||||||
| /// - Any signature is produced by an address that is not a registered KMS signer. | ||||||
| /// - The signatures verification returns false. | ||||||
| function checkSignatures( | ||||||
| bytes32[] memory handlesList, | ||||||
| bytes memory abiEncodedCleartexts, | ||||||
| bytes memory decryptionProof | ||||||
| ) internal { | ||||||
| bool isVerified = _verifySignatures(handlesList, abiEncodedCleartexts, decryptionProof); | ||||||
| if (!isVerified) { | ||||||
| revert InvalidKMSSignatures(); | ||||||
| } | ||||||
| emit PublicDecryptionVerified(handlesList, abiEncodedCleartexts); | ||||||
| } | ||||||
|
|
||||||
| /// @notice Verifies KMS signatures against the provided handles and public decryption data. | ||||||
| /// @param handlesList The list of handles as an array of bytes32 to verify | ||||||
| /// @param abiEncodedCleartexts The ABI-encoded list of decrypted values associated with each handle in the `handlesList`. | ||||||
| /// The list order must match the list of handles in `handlesList` | ||||||
| /// @param decryptionProof The KMS public decryption proof computed by the KMS Signers associated to `handlesList` and | ||||||
| /// `abiEncodedCleartexts` | ||||||
| /// @return true if the signatures verification succeeds, false otherwise | ||||||
| /// @dev Private low-level function used to verify the KMS signatures. | ||||||
| /// Warning: this function never reverts, its boolean return value must be checked. | ||||||
| /// The decryptionProof is the numSigners + kmsSignatures + extraData (1 + 65*numSigners + extraData bytes) | ||||||
| /// Only static native solidity types for clear values are supported, so `abiEncodedCleartexts` is the concatenation of all clear values appended to 32 bytes. | ||||||
| /// @dev Reverts if any of the following conditions are met by the underlying KMS verifier: | ||||||
| /// - The `decryptionProof` is empty or has an invalid length. | ||||||
| /// - The number of valid signatures is zero or less than the configured KMS signers threshold. | ||||||
| /// - Any signature is produced by an address that is not a registered KMS signer. | ||||||
| function _verifySignatures( | ||||||
| bytes32[] memory handlesList, | ||||||
| bytes memory cleartexts, | ||||||
| bytes memory abiEncodedCleartexts, | ||||||
| bytes memory decryptionProof | ||||||
| ) internal returns (bool) { | ||||||
| ) private returns (bool) { | ||||||
| CoprocessorConfig storage $ = Impl.getCoprocessorConfig(); | ||||||
| return | ||||||
| IKMSVerifier($.KMSVerifierAddress).verifyDecryptionEIP712KMSSignatures( | ||||||
| handlesList, | ||||||
| cleartexts, | ||||||
| abiEncodedCleartexts, | ||||||
| decryptionProof | ||||||
| ); | ||||||
| } | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,16 +8,21 @@ | |
| }, | ||
| "main": "contracts/ACL.sol", | ||
| "scripts": { | ||
| "compile": "HARDHAT_NETWORK=hardhat npm run deploy:emptyProxies && cross-env TS_NODE_TRANSPILE_ONLY=true hardhat compile", | ||
| "deploy:emptyProxies": "hardhat task:deployEmptyUUPSProxies && hardhat compile:specific --contract 'contracts/immutable' && hardhat task:deployPauserSet", | ||
| "addresses": "npm run deploy:emptyProxies", | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: having this "addresses" script seems a bit redundant already having "deploy:emptyProxies"
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not sure what's the point indeed of changing those. Also this might break the e2e deployment script made by @enitrat but I am not sure. |
||
| "tsc": "tsc --project tsconfig.json --noEmit", | ||
| "clean": "rm -rf addresses dist build artifacts cache typechain-types", | ||
| "typechain": "cross-env TS_NODE_TRANSPILE_ONLY=true hardhat typechain", | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: note that the typechain is also generated when running "hardhat compile", so maybe not needed to have a separate script for generating the typechain. |
||
| "compile:examples": "cross-env TS_NODE_TRANSPILE_ONLY=true hardhat compile:specific --contract 'examples'", | ||
| "compile": "cross-env HARDHAT_NETWORK=hardhat npm run deploy:emptyProxies && cross-env TS_NODE_TRANSPILE_ONLY=true hardhat compile", | ||
| "deploy:emptyProxies": "hardhat task:deployEmptyUUPSProxies && hardhat compile:specific --contract 'contracts/immutable' && hardhat task:deployPauserSet", | ||
| "lint": "npm run lint:sol && npm run lint:ts && npm run prettier:check", | ||
| "lint:ts": "eslint --ignore-path ./.eslintignore --ext .js,.ts .", | ||
| "prettier:check": "prettier --check \"**/*.{js,json,md,sol,ts,yml}\"", | ||
| "prettier": "prettier --write \"**/*.{js,json,md,sol,ts,yml}\"", | ||
| "test": "hardhat test", | ||
| "coverage": "SOLIDITY_COVERAGE=true hardhat coverage", | ||
| "codegen": "../library-solidity/codegen/codegen.mjs lib --config ./codegen.config.json --verbose", | ||
| "codegen:overloads": "../library-solidity/codegen/codegen.mjs overloads --config ./codegen.config.json --verbose --force", | ||
| "test": "hardhat test", | ||
| "test:gas": "hardhat test --grep Gas", | ||
| "test:forge": "forge test", | ||
| "forge:soldeer": "forge soldeer install" | ||
|
|
@@ -47,8 +52,7 @@ | |
| "prettier-plugin-solidity": "^1.1.3", | ||
| "sha3": "^2.1.4", | ||
| "solidity-coverage": "^0.8.14", | ||
| "sqlite3": "^5.1.7", | ||
| "web3-validator": "^2.0.6" | ||
| "sqlite3": "^5.1.7" | ||
| }, | ||
| "optionalDependencies": { | ||
| "solidity-comments-darwin-arm64": "0.1.1", | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is an environment variable used only for tests, maybe you could define it in the `hardhat.config.js file, similar to what we do at the Gateway.
Same here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with Isaac, don't put this in the .env, this was not needed until now, I don't see the point.