From 83eba38c54c85e8cdaabce471beb25d3e7a75e0d Mon Sep 17 00:00:00 2001 From: ryardley Date: Mon, 3 Feb 2025 17:52:27 +0800 Subject: [PATCH 1/2] Update Enclave and related interfaces to use IAdvancedPolicy instead of IBasePolicy --- packages/evm/contracts/Enclave.sol | 11 +++++++---- packages/evm/contracts/interfaces/IE3.sol | 6 ++++-- packages/evm/contracts/interfaces/IE3Program.sol | 8 ++++++-- packages/evm/contracts/test/MockE3Program.sol | 10 +++++----- 4 files changed, 22 insertions(+), 13 deletions(-) diff --git a/packages/evm/contracts/Enclave.sol b/packages/evm/contracts/Enclave.sol index 3da28764..cd7a5259 100644 --- a/packages/evm/contracts/Enclave.sol +++ b/packages/evm/contracts/Enclave.sol @@ -3,7 +3,10 @@ pragma solidity >=0.8.27; import { IEnclave, E3, IE3Program } from "./interfaces/IEnclave.sol"; import { ICiphernodeRegistry } from "./interfaces/ICiphernodeRegistry.sol"; -import { IBasePolicy } from "./excubiae/core/interfaces/IBasePolicy.sol"; +import { + IAdvancedPolicy +} from "./excubiae/core/interfaces/IAdvancedPolicy.sol"; +import { Check } from "./excubiae/core/interfaces/IAdvancedChecker.sol"; import { IDecryptionVerifier } from "./interfaces/IDecryptionVerifier.sol"; import { OwnableUpgradeable @@ -62,7 +65,7 @@ contract Enclave is IEnclave, OwnableUpgradeable { error InvalidEncryptionScheme(bytes32 encryptionSchemeId); error InputDeadlinePassed(uint256 e3Id, uint256 expiration); error InputDeadlineNotPassed(uint256 e3Id, uint256 expiration); - error InvalidComputationRequest(IBasePolicy inputValidator); + error InvalidComputationRequest(IAdvancedPolicy inputValidator); error InvalidCiphernodeRegistry(ICiphernodeRegistry ciphernodeRegistry); error InvalidDuration(uint256 duration); error InvalidOutput(bytes output); @@ -149,7 +152,7 @@ contract Enclave is IEnclave, OwnableUpgradeable { nexte3Id++; uint256 seed = uint256(keccak256(abi.encode(block.prevrandao, e3Id))); - (bytes32 encryptionSchemeId, IBasePolicy inputValidator) = e3Program + (bytes32 encryptionSchemeId, IAdvancedPolicy inputValidator) = e3Program .validate(e3Id, seed, e3ProgramParams, computeProviderParams); IDecryptionVerifier decryptionVerifier = decryptionVerifiers[ encryptionSchemeId @@ -231,7 +234,7 @@ contract Enclave is IEnclave, OwnableUpgradeable { bytes[] memory payload = new bytes[](1); payload[0] = data; - e3.inputValidator.enforce(msg.sender, payload); + e3.inputValidator.enforce(msg.sender, payload, Check.MAIN); uint256 inputHash = PoseidonT3.hash( [uint256(keccak256(data)), inputCounts[e3Id]] ); diff --git a/packages/evm/contracts/interfaces/IE3.sol b/packages/evm/contracts/interfaces/IE3.sol index 078f34f6..2a3eae09 100644 --- a/packages/evm/contracts/interfaces/IE3.sol +++ b/packages/evm/contracts/interfaces/IE3.sol @@ -1,7 +1,9 @@ // SPDX-License-Identifier: LGPL-3.0-only pragma solidity >=0.8.27; -import { IBasePolicy } from "../excubiae/core/interfaces/IBasePolicy.sol"; +import { + IAdvancedPolicy +} from "../excubiae/core/interfaces/IAdvancedPolicy.sol"; import { IE3Program } from "./IE3Program.sol"; import { IDecryptionVerifier } from "./IDecryptionVerifier.sol"; @@ -29,7 +31,7 @@ struct E3 { bytes32 encryptionSchemeId; IE3Program e3Program; bytes e3ProgramParams; - IBasePolicy inputValidator; + IAdvancedPolicy inputValidator; IDecryptionVerifier decryptionVerifier; bytes32 committeePublicKey; bytes32 ciphertextOutput; diff --git a/packages/evm/contracts/interfaces/IE3Program.sol b/packages/evm/contracts/interfaces/IE3Program.sol index 67bf7d42..695fc9d7 100644 --- a/packages/evm/contracts/interfaces/IE3Program.sol +++ b/packages/evm/contracts/interfaces/IE3Program.sol @@ -1,7 +1,9 @@ // SPDX-License-Identifier: LGPL-3.0-only pragma solidity >=0.8.27; -import { IBasePolicy } from "../excubiae/core/interfaces/IBasePolicy.sol"; +import { + IAdvancedPolicy +} from "../excubiae/core/interfaces/IAdvancedPolicy.sol"; interface IE3Program { /// @notice This function should be called by the Enclave contract to validate the computation parameters. @@ -16,7 +18,9 @@ interface IE3Program { uint256 seed, bytes calldata e3ProgramParams, bytes calldata computeProviderParams - ) external returns (bytes32 encryptionSchemeId, IBasePolicy inputValidator); + ) + external + returns (bytes32 encryptionSchemeId, IAdvancedPolicy inputValidator); /// @notice This function should be called by the Enclave contract to verify the decrypted output of an E3. /// @param e3Id ID of the E3. diff --git a/packages/evm/contracts/test/MockE3Program.sol b/packages/evm/contracts/test/MockE3Program.sol index 4cf4e831..46303a8e 100644 --- a/packages/evm/contracts/test/MockE3Program.sol +++ b/packages/evm/contracts/test/MockE3Program.sol @@ -1,20 +1,20 @@ // SPDX-License-Identifier: LGPL-3.0-only pragma solidity >=0.8.27; -import { IE3Program, IBasePolicy } from "../interfaces/IE3Program.sol"; +import { IE3Program, IAdvancedPolicy } from "../interfaces/IE3Program.sol"; contract MockE3Program is IE3Program { error invalidParams(bytes e3ProgramParams, bytes computeProviderParams); bytes32 public constant ENCRYPTION_SCHEME_ID = keccak256("fhe.rs:BFV"); - IBasePolicy private storageInputValidator; + IAdvancedPolicy private storageInputValidator; - constructor(IBasePolicy _inputValidator) { + constructor(IAdvancedPolicy _inputValidator) { storageInputValidator = _inputValidator; } - function setInputValidator(IBasePolicy _inputValidator) external { + function setInputValidator(IAdvancedPolicy _inputValidator) external { storageInputValidator = _inputValidator; } @@ -26,7 +26,7 @@ contract MockE3Program is IE3Program { ) external view - returns (bytes32 encryptionSchemeId, IBasePolicy inputValidator) + returns (bytes32 encryptionSchemeId, IAdvancedPolicy inputValidator) { require( computeProviderParams.length == 32, From e764557b93f3dd51f33c893a48bb968c0028a6d4 Mon Sep 17 00:00:00 2001 From: ryardley Date: Tue, 4 Feb 2025 11:16:58 +0800 Subject: [PATCH 2/2] Refactor InputValidatorPolicy to use AdvancedChecker and implement MainCheckAlreadyEnforced error handling --- packages/evm/contracts/Enclave.sol | 1 + .../excubiae/InputValidatorPolicy.sol | 14 +++++----- .../excubiae/core/AdvancedPolicy.sol | 27 ++++++++++++++----- .../test/MockInputValidatorChecker.sol | 11 ++++---- packages/evm/test/Enclave.spec.ts | 19 ++----------- 5 files changed, 38 insertions(+), 34 deletions(-) diff --git a/packages/evm/contracts/Enclave.sol b/packages/evm/contracts/Enclave.sol index cd7a5259..d7e3fdff 100644 --- a/packages/evm/contracts/Enclave.sol +++ b/packages/evm/contracts/Enclave.sol @@ -83,6 +83,7 @@ contract Enclave is IEnclave, OwnableUpgradeable { error TargetOnly(); error TargetAlreadySet(); error AlreadyEnforced(); + error MainCheckAlreadyEnforced(); //////////////////////////////////////////////////////////// // // diff --git a/packages/evm/contracts/excubiae/InputValidatorPolicy.sol b/packages/evm/contracts/excubiae/InputValidatorPolicy.sol index 22b39959..cc32494d 100644 --- a/packages/evm/contracts/excubiae/InputValidatorPolicy.sol +++ b/packages/evm/contracts/excubiae/InputValidatorPolicy.sol @@ -1,20 +1,22 @@ // SPDX-License-Identifier: LGPL-3.0-only pragma solidity >=0.8.27; -import { BasePolicy } from "./core/BasePolicy.sol"; -import { BaseChecker } from "./core/BaseChecker.sol"; +import { AdvancedPolicy } from "./core/AdvancedPolicy.sol"; +import { AdvancedChecker } from "./core/AdvancedChecker.sol"; /// @title BaseERC721Policy. /// @notice Policy enforcer for Enclave Input validation. /// @dev Extends BasePolicy with Enclave specific checks. -contract InputValidatorPolicy is BasePolicy { +contract InputValidatorPolicy is AdvancedPolicy { /// @notice Checker contract reference. - BaseChecker public immutable CHECKER; + AdvancedChecker public immutable CHECKER; /// @notice Initializes with checker contract. /// @param _checker Checker contract address. - constructor(BaseChecker _checker) BasePolicy(_checker) { - CHECKER = BaseChecker(_checker); + constructor( + AdvancedChecker _checker + ) AdvancedPolicy(_checker, true, true, true) { + CHECKER = AdvancedChecker(_checker); } /// @notice Returns policy identifier. diff --git a/packages/evm/contracts/excubiae/core/AdvancedPolicy.sol b/packages/evm/contracts/excubiae/core/AdvancedPolicy.sol index 49ef34ab..6f97a4c2 100644 --- a/packages/evm/contracts/excubiae/core/AdvancedPolicy.sol +++ b/packages/evm/contracts/excubiae/core/AdvancedPolicy.sol @@ -3,9 +3,10 @@ // Auto-generated from https://github.com/privacy-scaling-explorations/excubiae.git@96a3312455417dc1b2e0d87066661fdf8f490fac pragma solidity >=0.8.27; -import {Policy} from "./Policy.sol"; -import {IAdvancedPolicy, Check} from "./interfaces/IAdvancedPolicy.sol"; -import {AdvancedChecker, CheckStatus} from "./AdvancedChecker.sol"; +import { Policy } from "./Policy.sol"; +import { IAdvancedPolicy, Check } from "./interfaces/IAdvancedPolicy.sol"; +import { AdvancedChecker, CheckStatus } from "./AdvancedChecker.sol"; +import "hardhat/console.sol"; /// @title AdvancedPolicy. /// @notice Implements advanced policy checks with pre, main, and post validation stages. @@ -33,7 +34,12 @@ abstract contract AdvancedPolicy is IAdvancedPolicy, Policy { /// @param _skipPre Skip pre-condition validation. /// @param _skipPost Skip post-condition validation. /// @param _allowMultipleMain Allow multiple main validations. - constructor(AdvancedChecker _advancedChecker, bool _skipPre, bool _skipPost, bool _allowMultipleMain) { + constructor( + AdvancedChecker _advancedChecker, + bool _skipPre, + bool _skipPost, + bool _allowMultipleMain + ) { ADVANCED_CHECKER = _advancedChecker; SKIP_PRE = _skipPre; SKIP_POST = _skipPost; @@ -45,7 +51,11 @@ abstract contract AdvancedPolicy is IAdvancedPolicy, Policy { /// @param subject Address to validate. /// @param evidence Validation data. /// @param checkType Type of check (PRE, MAIN, POST). - function enforce(address subject, bytes[] calldata evidence, Check checkType) external override onlyTarget { + function enforce( + address subject, + bytes[] calldata evidence, + Check checkType + ) external override onlyTarget { _enforce(subject, evidence, checkType); } @@ -61,8 +71,13 @@ abstract contract AdvancedPolicy is IAdvancedPolicy, Policy { /// @custom:throws PreCheckNotEnforced If PRE check is required but not done. /// @custom:throws MainCheckNotEnforced If MAIN check is required but not done. /// @custom:throws MainCheckAlreadyEnforced If multiple MAIN checks not allowed. - function _enforce(address subject, bytes[] calldata evidence, Check checkType) internal { + function _enforce( + address subject, + bytes[] calldata evidence, + Check checkType + ) internal { if (!ADVANCED_CHECKER.check(subject, evidence, checkType)) { + console.log("_enforce()"); revert UnsuccessfulCheck(); } diff --git a/packages/evm/contracts/test/MockInputValidatorChecker.sol b/packages/evm/contracts/test/MockInputValidatorChecker.sol index f498acac..836360c9 100644 --- a/packages/evm/contracts/test/MockInputValidatorChecker.sol +++ b/packages/evm/contracts/test/MockInputValidatorChecker.sol @@ -1,24 +1,25 @@ // SPDX-License-Identifier: LGPL-3.0-only pragma solidity >=0.8.27; -import { BaseChecker } from "../excubiae/core/BaseChecker.sol"; +import { AdvancedChecker } from "../excubiae/core/AdvancedChecker.sol"; +import { Check } from "../excubiae/core/interfaces/IAdvancedChecker.sol"; /// @title MockInputValidatorChecker. /// @notice Enclave Input Validator /// @dev Extends BaseChecker for input verification. -contract MockInputValidatorChecker is BaseChecker { +contract MockInputValidatorChecker is AdvancedChecker { /// @param _verifiers Array of addresses for existing verification contracts. - constructor(address[] memory _verifiers) BaseChecker(_verifiers) {} + constructor(address[] memory _verifiers) AdvancedChecker(_verifiers) {} /// @notice Validates input /// @param subject Address to check. /// @param evidence mock proof /// @return True if proof is valid - function _check( + function _checkMain( address subject, bytes[] calldata evidence ) internal view override returns (bool) { - super._check(subject, evidence); + super._checkMain(subject, evidence); bool success; if (evidence[0].length == 3) { diff --git a/packages/evm/test/Enclave.spec.ts b/packages/evm/test/Enclave.spec.ts index fa7da90b..759c2e70 100644 --- a/packages/evm/test/Enclave.spec.ts +++ b/packages/evm/test/Enclave.spec.ts @@ -958,8 +958,7 @@ describe("Enclave", function () { ); }); - // XXX: Skipping for now as fixing this would mean implementing an AdvancedPolicy in excubiae - it.skip("adds inputHash to merkle tree", async function () { + it("adds inputHash to merkle tree", async function () { const { enclave, request } = await loadFixture(setup); const inputData = abiCoder.encode(["bytes"], ["0xaabbccddeeff"]); @@ -1332,20 +1331,6 @@ describe("Enclave", function () { }); describe("InputValidatorPolicy", function () { - it("should validate inputs using the input validator", async function () { - const [owner, notTheOwner] = await ethers.getSigners(); - const { inputValidatorPolicy } = await loadFixture( - deployInputValidatorContracts, - ); - await inputValidatorPolicy.connect(owner).setTarget(owner); - const shouldPass = "0x1234"; // length 2 = pass - const contract = inputValidatorPolicy.connect(owner); - await contract.connect(owner).enforce(notTheOwner, [shouldPass]); - await expect( - contract.connect(owner).enforce(notTheOwner, [shouldPass]), - ).to.be.revertedWithCustomError(inputValidatorPolicy, "AlreadyEnforced"); - }); - it("should fail with error if the checker fails", async function () { const [owner, notTheOwner] = await ethers.getSigners(); @@ -1356,7 +1341,7 @@ describe("InputValidatorPolicy", function () { const shouldFail = "0x123456"; // length 3 = fail const contract = inputValidatorPolicy.connect(owner); await expect( - contract.enforce(notTheOwner, [shouldFail]), + contract.enforce(notTheOwner, [shouldFail], 1), ).to.be.revertedWithCustomError(contract, "UnsuccessfulCheck"); }); });