Skip to content
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.

Sign up for GitHub

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

feat: add e3Id and seed to validate() calls to IE3Program and IComputeProvider #43

Merged
merged 1 commit into from
Sep 5, 2024
Merged

feat: add e3Id and seed to validate() calls to IE3Program and IComputeProvider #43

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ sequenceDiagram
Users->>Enclave: request(parameters)
Enclave->>E3Program: validate(e3ProgramParams)
E3Program-->>Enclave: inputValidator
Enclave->>ComputeProvider: validate(emParams)
Enclave->>ComputeProvider: validate(computeProviderParams)
ComputeProvider-->>Enclave: decryptionVerifier
Enclave->>CiphernodeRegistry: requestCommittee(e3Id, filter, threshold)
CiphernodeRegistry-->>Enclave: success
Expand Down
15 changes: 11 additions & 4 deletions packages/evm/contracts/Enclave.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ contract Enclave is IEnclave, OwnableUpgradeable {
IE3Program e3Program,
bytes memory e3ProgramParams,
IComputeProvider computeProvider,
bytes memory emParams
bytes memory computeProviderParams
) external payable returns (uint256 e3Id, E3 memory e3) {
// TODO: allow for other payment methods or only native tokens?
// TODO: should payment checks be somewhere else? Perhaps in the E3 Program or ciphernode registry?
Expand Down Expand Up @@ -156,22 +156,29 @@ contract Enclave is IEnclave, OwnableUpgradeable {
// TODO: should IDs be incremental or produced deterministically?
e3Id = nexte3Id;
nexte3Id++;
uint256 seed = uint256(keccak256(abi.encode(block.prevrandao, e3Id)));

IInputValidator inputValidator = e3Program.validate(e3ProgramParams);
IInputValidator inputValidator = e3Program.validate(
e3Id,
seed,
e3ProgramParams
);
require(address(inputValidator) != address(0), InvalidComputation());

// TODO: validate that the requested computation can be performed by the given compute provider.
// Perhaps the compute provider should be returned by the E3 Program?
IDecryptionVerifier decryptionVerifier = computeProvider.validate(
emParams
e3Id,
seed,
computeProviderParams
);
require(
address(decryptionVerifier) != address(0),
InvalidComputeProviderSetup()
);

e3 = E3({
seed: keccak256(abi.encode(block.prevrandao, e3Id)),
seed: seed,
threshold: threshold,
startWindow: startWindow,
duration: duration,
Expand Down
2 changes: 2 additions & 0 deletions packages/evm/contracts/interfaces/IComputeProvider.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ interface IComputeProvider {
/// @notice This function should be called by the Enclave contract to validate the compute provider parameters.
/// @param params ABI encoded compute provider parameters.
function validate(
uint256 e3Id,
uint256 seed,
bytes calldata params
) external returns (IDecryptionVerifier decryptionVerifier);
}
2 changes: 1 addition & 1 deletion packages/evm/contracts/interfaces/IE3.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import { IDecryptionVerifier } from "./IDecryptionVerifier.sol";
/// @param ciphertextOutput Encrypted output data.
/// @param plaintextOutput Decrypted output data.
struct E3 {
bytes32 seed;
uint256 seed;
uint32[2] threshold;
uint256[2] startWindow;
uint256 duration;
Expand Down
4 changes: 4 additions & 0 deletions packages/evm/contracts/interfaces/IE3Program.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,13 @@ import { IInputValidator } from "./IInputValidator.sol";

interface IE3Program {
/// @notice This function should be called by the Enclave contract to validate the computation parameters.
/// @param e3Id ID of the E3.
/// @param seed Seed for the computation.
/// @param params ABI encoded computation parameters.
/// @return inputValidator The input validator to be used for the computation.
function validate(
uint256 e3Id,
uint256 seed,
bytes calldata params
) external returns (IInputValidator inputValidator);

Expand Down
4 changes: 2 additions & 2 deletions packages/evm/contracts/interfaces/IEnclave.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ interface IEnclave {
/// @param e3Program Address of the E3 Program.
/// @param e3ProgramParams ABI encoded computation parameters.
/// @param computeProvider Address of the compute provider.
/// @param emParams ABI encoded compute provider parameters.
/// @param computeProviderParams ABI encoded compute provider parameters.
/// @return e3Id ID of the E3.
/// @return e3 The E3 struct.
function request(
Expand All @@ -109,7 +109,7 @@ interface IEnclave {
IE3Program e3Program,
bytes memory e3ProgramParams,
IComputeProvider computeProvider,
bytes memory emParams
bytes memory computeProviderParams
) external payable returns (uint256 e3Id, E3 memory e3);

/// @notice This function should be called to activate an Encrypted Execution Environment (E3) once it has been
Expand Down
2 changes: 2 additions & 0 deletions packages/evm/contracts/test/MockComputeProvider.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ contract MockComputeProvider is IComputeProvider {
error invalidParams();

function validate(
uint256,
uint256,
Comment on lines +13 to +14
Copy link
Contributor

Choose a reason for hiding this comment

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

Review the function body for potential improvements.

The function body uses both inline assembly and abi.decode to manipulate and decode the params. This redundancy could lead to confusion and potential errors. It's recommended to stick to one method for clarity and safety, preferably abi.decode which is safer and more readable than inline assembly.

Consider simplifying the function body by removing the inline assembly:

 function validate(
     uint256 e3Id,
     uint256 seed,
     bytes memory params
 ) external pure returns (IDecryptionVerifier decryptionVerifier) {
     require(params.length == 32, invalidParams());
-    // solhint-disable no-inline-assembly
-    assembly {
-        decryptionVerifier := mload(add(params, 32))
-    }
     (decryptionVerifier) = abi.decode(params, (IDecryptionVerifier));
 }

Committable suggestion was skipped due to low confidence.

Review the addition of new parameters to the validate function.

The addition of two uint256 parameters (e3Id and seed) to the validate function is aligned with the PR objectives and the linked issue #33. This change is intended to pass necessary context to the contracts, enhancing their functionality and interoperability.

However, the parameters are unnamed in the function signature, which might reduce code readability and maintainability. It's recommended to name these parameters to clarify their purpose.

Consider naming the parameters for better clarity and maintainability:

-function validate(
-    uint256,
-    uint256,
+function validate(
+    uint256 e3Id,
+    uint256 seed,
     bytes memory params
 ) external pure returns (IDecryptionVerifier decryptionVerifier) {
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
uint256,
uint256,
uint256 e3Id,
uint256 seed,

bytes memory params
) external pure returns (IDecryptionVerifier decryptionVerifier) {
require(params.length == 32, invalidParams());
Expand Down
2 changes: 2 additions & 0 deletions packages/evm/contracts/test/MockE3Program.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ contract MockE3Program is IE3Program {
error invalidParams(bytes params);

function validate(
uint256,
uint256,
Comment on lines +10 to +11
Copy link
Contributor

Choose a reason for hiding this comment

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

Review the addition of new parameters in the validate function.

The addition of two uint256 parameters to the validate function is intended to pass e3Id and seed as per the PR objectives. This change aligns with the requirements outlined in issue #33, ensuring that component contracts can utilize the e3Id effectively.

However, the function parameters are unnamed, which could lead to confusion and errors in future maintenance or development. Naming these parameters would improve readability and maintainability of the code.

Consider naming the parameters to enhance clarity:

- function validate(uint256, uint256, bytes memory params)
+ function validate(uint256 e3Id, uint256 seed, bytes memory params)
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
uint256,
uint256,
uint256 e3Id,
uint256 seed,

bytes memory params
) external pure returns (IInputValidator inputValidator) {
require(params.length == 32, "invalid params");
Expand Down
Loading