Skip to content

Conversation

@ericglau
Copy link
Member

@ericglau ericglau commented Aug 28, 2025

Adds a Confidential tab that implements a confidential fungible token, using OpenZeppelin Confidential Contracts.

Includes:

  • @openzeppelin/wizard-confidential package, UI, AI assistant, MCP tools.

@ericglau
Copy link
Member Author

@SocketSecurity ignore npm/jszip@3.10.1
Appears to be false positive

Copy link
Contributor

@CoveMB CoveMB left a comment

Choose a reason for hiding this comment

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

Nice work!

I tried the security review from MCP unittest which resulted in the following (are any relevant to the case?)

Recommendations

New confidential tooling is structurally sound, but one concurrency bug can leak per-contract metadata, and newly introduced pre-release
FHE dependencies deserve extra due diligence.

Risk Level Low

UnvalidatedNetworkConfig
location: packages/core/confidential/src/confidentialFungible.ts:45
type: input validation / configuration hardening
description: addNetworkConfig silently does nothing if networkConfig falls outside the hard-coded list. Typed callers are protected,
but any untyped consumer (custom JS integration, AI tooling bugs) can emit a contract missing the required FHE network parent, leaving
deployments misconfigured and functions reverting at runtime.
recommendation: Throw an OptionsError when the value is not one of the supported configs so invalid input fails fast.

PreReleaseFHEDependencies
location: packages/core/confidential/package.json:27, packages/core/confidential/src/environments/hardhat/package.json:13
type: dependency risk
description: The new confidential package and Hardhat template depend on pre-release FHE artifacts (@fhevm/hardhat-plugin@0.0.1-6, @fhevm/
core-contracts@0.7.0-12, @zama-fhe/relayer-sdk@0.1.0-5). These builds have limited hardening history, so upstream changes or latent
vulnerabilities pose elevated supply-chain risk.
recommendation: Perform targeted security review of these transitive dependencies, pin to vetted versions, and monitor for advisories
before promoting the wizard to production use.

--

Suggested next steps:

  1. add runtime validation for networkConfig;
  2. schedule a dependency risk assessment (or version pinning) for the new FHE toolchain.

//--

Tests Recommendations

Risk Level High

printContractVersioned
location: packages/core/confidential/src/print-versioned.ts
change type: added
reason: New Remix printer rewrites @openzeppelin/* and @fhevm/* import paths using pinned versions; a regression would generate broken
Remix projects (external tool integration).
suggested tests:

  • Build a ConfidentialFungible contract with wrappable and votes enabled, run printContractVersioned, and assert the output replaces
    @openzeppelin/contracts, @openzeppelin/confidential-contracts, and @fhevm/solidity with the pinned versions from contract-version-pins.
  • Feed a contract that only imports third-party modules outside those prefixes and assert the printer leaves those paths untouched to
    prevent over-eager rewrites.

Risk Level Medium

injectHyperlinks
location: packages/ui/src/confidential/inject-hyperlinks.ts
change type: added
reason: Regex-based HTML injection now covers confidential and FHEVM imports; mistakes would surface as broken or unsafe links in the code
viewer (HTML sanitization risk).
suggested tests:

  • Unit test the helper with sample highlighted code containing each supported import (@openzeppelin/contracts, @openzeppelin/confidential-
    contracts, @fhevm/solidity) and assert the generated URLs include the correct pinned versions.
  • Add a negative case using a path with ../ segments to confirm the guard still prevents hyperlink injection for disallowed patterns.

evaluateSelection
location: packages/ui/src/main.ts:66
change type: modified
reason: Added the confidential branch that gates whether the new wizard loads or shows the incompatibility banner; misrouting would block
the feature.
suggested tests:

  • Unit test evaluateSelection('confidential', undefined) and evaluateSelection('confidential', incompatibleVersion) to verify it
    respectively returns the ConfidentialApp path or an incompatible response with the expected semver hint.
  • Integration-style test stubbing new ConfidentialApp to ensure the dispatcher is invoked when the selection reports compatible: true for
    the confidential language.

}

/**
* Check for potential premint overflow assuming the user's contract has decimals() = 6
Copy link
Contributor

Choose a reason for hiding this comment

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

would it be worth adding a comment in the contract that decimals must be 6 max? (I see ConfidentialFungibleTokenERC20Wrapper as maxDecimal of 6 but in case user override it)

Copy link
Member Author

@ericglau ericglau Oct 22, 2025

Choose a reason for hiding this comment

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

I think the user can possibly override _maxDecimals to be >6. Using a large decimals just reduces the logical max supply, and this is significant because the type euint64 has a smaller max value than typical uint256 of regular contracts.

OTOH, our checks for fields like premint might be a little restrictive if the user has less decimals(), since they could have higher supply. But I think it is fine since that's the default, and they could just manually edit the premint amount in the resulting contract if they know what they're doing.

"@upstash/redis": "https://esm.sh/@upstash/redis@1.35.6",
"openai": "https://esm.sh/openai@5.23.2"
"openai": "https://esm.sh/openai@5.23.2",
"jszip": "https://esm.sh/jszip@3.6.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

Is jszip needed on deno side?

Copy link
Member Author

Choose a reason for hiding this comment

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

If removed, yarn type:check:api fails:

error: Relative import path "jszip" not prefixed with / or ./ or ../ and not in import map from "file:///Users/eric/git/contracts-wizard/packages/core/solidity/dist/zip-hardhat.d.ts"
    at file:///Users/eric/git/contracts-wizard/packages/core/solidity/dist/zip-hardhat.d.ts:1:19

It might be needed because it's a transitive dependency of the line below: "@openzeppelin/wizard/dist/": "../core/solidity/dist/"

@ericglau
Copy link
Member Author

@SocketSecurity ignore-all
These are AI detected anomalies and look like false positives.

@ericglau
Copy link
Member Author

security review from MCP unittest

  • UnvalidatedNetworkConfig: fixed
  • PreReleaseFHEDependencies: this is the current state of the supported upstream dependencies. Will be improved when they are supported in newer versions of Confidential Contracts.
  • printContractVersioned: fixed
  • injectHyperlinks, evaluateSelection: unnecessary, since any issues should be quite apparent in the UI

@ericglau
Copy link
Member Author

ericglau commented Nov 28, 2025

Notes on recent updates:

  • Support Confidential Contracts v0.3.0: Renamed all API and UIs to ERC7984, changed tokenURI to contractURI
  • Confidential Contracts v0.3.0 has a peer dependency on @fhevm/solidity@0.9.1, but the NPM package for @fhevm/solidity@0.9.1 appears to correspond to GitHub tag v0.9.14. So we’ll use that NPM package, but will map import links in the UI to point users to v0.9.14.
  • SepoliaConfig and EthereumConfig have been combined into ZamaEthereumConfig. So the UI will only show 1 option under Network Configuration as “Zama Ethereum”, which is applicable to Ethereum mainnet and Sepolia.

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