chore(common): checkSignatures, ZamaConfig.sol, mock publicDecrypt an… #1316

0xalexbel · 2025-11-12T15:12:17Z

FHE.sol : remove verifySignatures
FHE.sol : add checkSignatures
New ZamaConfig.sol
Add local coprocessor config
Add mock publicDecrypt function
Add publicDecrypt tests
Add verifySignatures tests
Refactor the HH mock typescript code: fhevmjsMock.ts, instance.ts etc.
host-contracts & library-solidity must have the same mock code.

isaacdecoded · 2025-11-12T15:34:07Z

host-contracts/.env.example

@@ -1,3 +1,4 @@
+ENV_HOST_ADDRESSES_PATH="addresses/.env.host"


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.

I agree with Isaac, don't put this in the .env, this was not needed until now, I don't see the point.

jatZama · 2025-11-12T15:36:55Z

host-contracts/eslint.config.mjs

@@ -0,0 +1,133 @@
+import { FlatCompat } from "@eslint/eslintrc";


why are all those eslint files needed now? linter was working before without those.

isaacdecoded · 2025-11-12T15:43:12Z

host-contracts/package.json

  "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",


nit: having this "addresses" script seems a bit redundant already having "deploy:emptyProxies"

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.

zmalatrax · 2025-11-12T15:44:22Z

host-contracts/lib/FHE.sol

+    /// @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.


Suggested change

/// to verify the signatures and prevent fake decryption results for being submitted.

/// to verify the signatures and prevent fake decryption results from being submitted.

isaacdecoded · 2025-11-12T15:45:22Z

host-contracts/package.json

+    "addresses": "npm run deploy:emptyProxies",
+    "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",


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.

isaacdecoded · 2025-11-12T15:57:48Z

host-contracts/test/encryptedERC20/EncryptedERC20.ts

      expect.fail('Expected an error to be thrown - Bob should not be able to reencrypt Alice balance');
    } catch (error) {
-      expect(error.message).to.equal('User is not authorized to reencrypt this handle!');
+      expect((error as any).message).to.equal('User is not authorized to reencrypt this handle!');


Maybe better to use error as Error to avoid the usage of any.

jatZama · 2025-11-12T16:06:50Z

host-contracts/test/signers.ts

-  dave: HardhatEthersSigner;
-  eve: HardhatEthersSigner;
-  fred: HardhatEthersSigner;
+  alice: HardhatEthersSigner | HDNodeWallet;


Why do you need HDNodeWallet here?

isaacdecoded · 2025-11-12T16:24:21Z

host-contracts/test/instance.ts

-    return getDecryptor().decryptEbytes256(await getCiphertext(handle, ethers));
-  }
-};
+export { getTxHCUFromTxReceipt, awaitCoprocessor };


I'm wondering what the intention of re-exporting these two functions here is, if they're already exported by fhevmjsMocked?

isaacdecoded · 2025-11-12T16:32:16Z

host-contracts/test/tasks/pausing.ts

 describe('Pausing and Unpausing Tasks', function () {
  let pauserSet;
-  let acl;
+  let acl: ACL;


Maybe doesn't make much sense to define its type if you then need to do the type assertion as ACL in line 19. In such a case, should the pauserSet also have its type defined to be consistent?

jatZama · 2025-11-12T16:39:58Z

host-contracts/test/fhevmjsMocked.ts

-  }
-  return coprocessorSigners;
+function loadEnvFhevmMockConfig(): EnvFhevmMockConfig {
+  const hostEnvPath = getRequiredEnvVar('ENV_HOST_ADDRESSES_PATH');


Do you really need this to be inside a .env?

Maybe for getting identical code for fhevmjsMocked between host and lib?

don't have a lot of context here, but env var should be kept only for infra / production tools as much as possible, if we can avoid them for test-only purposes, then it's much better

jatZama · 2025-11-12T16:53:48Z

library-solidity/.env.example

 INPUT_VERIFICATION_ADDRESS="0x812b06e1CDCE800494b79fFE4f925A504a9A9810"
 NUM_KMS_NODES="1"
-KMS_THRESHOLD="1"
+# In practice, the `PUBLIC_DECRYPTION_THRESHOLD` is currently set to `floor(n/2) + 1` with `n`` the number of KMS nodes


I am not sure about this comment, can you remove it? This changed several times very recently.

Ah but this was already inside the host-contracts' .env.
Let's keep it then, even if i am not sure.

better to remove and replace by

Should be the same as `PUBLIC_DECRYPTION_THRESHOLD` set in the Gateway

same for host

jatZama · 2025-11-12T17:04:17Z

library-solidity/config/ZamaConfig.sol

-    function getEthereumProtocolId() internal pure returns (uint256) {
-        /// @note Zama Ethereum protocol id is '1'
-        return 1;
+    /// @dev chainid == 11155111


wrong chain id here

jatZama · 2025-11-12T17:04:49Z

library-solidity/config/ZamaConfig.sol

+    /// @dev chainid == 11155111
+    function _getLocalProtocolId() private pure returns (uint256) {
+        /// @note Development protocol id is '100000 + Zama Ethereum protocol id'
+        return 100001;


maybe for local net, send back type(uint256).max , just in case.
10001 seems too small

…d cleaning

…ifier tests

0xalexbel requested review from a team as code owners November 12, 2025 15:12

cla-bot bot added the cla-signed label Nov 12, 2025

isaacdecoded reviewed Nov 12, 2025

View reviewed changes

jatZama reviewed Nov 12, 2025

View reviewed changes

isaacdecoded reviewed Nov 12, 2025

View reviewed changes

zmalatrax reviewed Nov 12, 2025

View reviewed changes

isaacdecoded reviewed Nov 12, 2025

View reviewed changes

jatZama reviewed Nov 12, 2025

View reviewed changes

isaacdecoded reviewed Nov 12, 2025

View reviewed changes

jatZama reviewed Nov 12, 2025

View reviewed changes

0xalexbel added 4 commits November 14, 2025 09:55

chore(common): checkSignatures, ZamaConfig.sol, mock publicDecrypt an…

4752748

…d cleaning

fix(host-contracts): add after block code in inputVerifier and kmsVer…

4aa7b77

…ifier tests

chore(library-solidity): fix typo in comment

26082e3

fix(library-solidity): new local protocolid

fd33308

0xalexbel force-pushed the alexB/check-signatures branch from c1d52a0 to fd33308 Compare November 14, 2025 08:56

		@@ -1,3 +1,4 @@
		ENV_HOST_ADDRESSES_PATH="addresses/.env.host"

		@@ -0,0 +1,133 @@
		import { FlatCompat } from "@eslint/eslintrc";

	/// to verify the signatures and prevent fake decryption results for being submitted.
	/// to verify the signatures and prevent fake decryption results from being submitted.

chore(common): checkSignatures, ZamaConfig.sol, mock publicDecrypt an… #1316

Are you sure you want to change the base?

chore(common): checkSignatures, ZamaConfig.sol, mock publicDecrypt an… #1316

Uh oh!

Conversation

0xalexbel commented Nov 12, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

melanciani Nov 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

melanciani Nov 12, 2025 •

edited

Loading