From 161dcce688386daeaf00a6c7bba1ee635bc05549 Mon Sep 17 00:00:00 2001 From: jatZama Date: Mon, 9 Dec 2024 17:52:21 +0100 Subject: [PATCH 1/2] fix: make GatewayContract compatible with current KMS in trustless case chore: prettier chore: typo in test --- contracts/gateway/GatewayContract.sol | 98 +++++++++++++++++++++++---- contracts/gatewayLib/lib/Gateway.sol | 63 +++++++++++++---- contracts/test/asyncDecrypt.ts | 10 +-- contracts/test/upgrades/upgrades.ts | 2 +- 4 files changed, 137 insertions(+), 36 deletions(-) diff --git a/contracts/gateway/GatewayContract.sol b/contracts/gateway/GatewayContract.sol index cefb722f..8ca390e6 100644 --- a/contracts/gateway/GatewayContract.sol +++ b/contracts/gateway/GatewayContract.sol @@ -24,7 +24,7 @@ contract GatewayContract is UUPSUpgradeable, Ownable2StepUpgradeable { /// @notice Version of the contract uint256 private constant MAJOR_VERSION = 0; uint256 private constant MINOR_VERSION = 1; - uint256 private constant PATCH_VERSION = 0; + uint256 private constant PATCH_VERSION = 1; IKMSVerifier private constant kmsVerifier = IKMSVerifier(kmsVerifierAdd); address private constant aclAddress = aclAdd; @@ -184,16 +184,8 @@ contract GatewayContract is UUPSUpgradeable, Ownable2StepUpgradeable { bytes[] memory signatures ) external payable virtual onlyRelayer { GatewayContractStorage storage $ = _getGatewayContractStorage(); - require( - kmsVerifier.verifyDecryptionEIP712KMSSignatures( - aclAddress, - $.decryptionRequests[requestID].cts, - decryptedCts, - signatures - ), - "KMS signature verification failed" - ); require(!$.isFulfilled[requestID], "Request is already fulfilled"); + $.isFulfilled[requestID] = true; DecryptionRequest memory decryptionReq = $.decryptionRequests[requestID]; require(block.timestamp <= decryptionReq.maxTimestamp, "Too late"); bytes memory callbackCalldata = abi.encodeWithSelector(decryptionReq.callbackSelector, requestID); @@ -201,18 +193,26 @@ contract GatewayContract is UUPSUpgradeable, Ownable2StepUpgradeable { callbackCalldata = abi.encodePacked(callbackCalldata, decryptedCts); // decryptedCts MUST be correctly abi-encoded by the relayer, according to the requested types of `ctsHandles` if (passSignatures) { bytes memory packedSignatures = abi.encode(signatures); - bytes memory packedSignaturesNoOffset = removeOffset(packedSignatures); // remove the offset (the first 32 bytes) before concatenating with the first part of calldata - callbackCalldata = abi.encodePacked(callbackCalldata, packedSignaturesNoOffset); + packedSignatures = removeOffset(packedSignatures); + callbackCalldata = replaceOffsets(callbackCalldata, decryptionReq.cts, packedSignatures); + } else { + require( + kmsVerifier.verifyDecryptionEIP712KMSSignatures( + aclAddress, + $.decryptionRequests[requestID].cts, + decryptedCts, + signatures + ), + "KMS signature verification failed" + ); } - (bool success, bytes memory result) = (decryptionReq.contractCaller).call{value: decryptionReq.msgValue}( callbackCalldata ); emit ResultCallback(requestID, success, result); - $.isFulfilled[requestID] = true; } - function removeOffset(bytes memory input) public pure virtual returns (bytes memory) { + function removeOffset(bytes memory input) internal pure virtual returns (bytes memory) { uint256 newLength = input.length - 32; bytes memory result = new bytes(newLength); for (uint256 i = 0; i < newLength; i++) { @@ -221,6 +221,74 @@ contract GatewayContract is UUPSUpgradeable, Ownable2StepUpgradeable { return result; } + function replaceOffsets( + bytes memory input, + uint256[] memory handlesList, + bytes memory packedSignatures + ) internal pure virtual returns (bytes memory) { + uint256 numArgs = handlesList.length; + uint256 signaturesOffset = 64; // requestID is always first argument of callback + own size of offset = 32+32 + for (uint256 i = 0; i < numArgs; i++) { + uint8 typeCt = uint8(handlesList[i] >> 8); + if (typeCt >= 9) { + input = addToBytes32Slice(input, 32 * (i + 1) + 4); // because we append the signatures, all bytes offsets are shifted by 0x20 + if (typeCt == 9) { + //ebytes64 + signaturesOffset += 128; + } else if (typeCt == 10) { + //ebytes128 + signaturesOffset += 192; + } else if (typeCt == 11) { + //ebytes256 + signaturesOffset += 320; + } + } else { + signaturesOffset += 32; + } + } + input = interleaveBytes(input, 4 + 32 * (numArgs + 1), signaturesOffset); // we add the offset of the signatures at correct location + input = abi.encodePacked(input, packedSignatures); + return input; + } + + function addToBytes32Slice(bytes memory data, uint256 offset) internal pure virtual returns (bytes memory) { + // @note: data is assumed to be more than 32+offset bytes long + assembly { + let ptr := add(add(data, 0x20), offset) + let val := mload(ptr) + val := add(val, 0x20) + mstore(ptr, val) + } + return data; + } + + function interleaveBytes( + bytes memory input, + uint256 position, + uint256 valueToPutINBetween + ) internal pure virtual returns (bytes memory) { + // @note: we assume position <= input.length + uint256 originalLength = input.length; + uint256 newLength = originalLength + 32; + bytes memory result = new bytes(newLength); + for (uint256 i = 0; i < position; i++) { + result[i] = input[i]; + } + + // Insert 32 bytes with the specified value + bytes32 valueToInsert = bytes32(valueToPutINBetween); + for (uint256 i = 0; i < 32; i++) { + result[position + i] = valueToInsert[i]; + } + + // Copy remainder of input (after insertion point) + for (uint256 i = position; i < originalLength; i++) { + result[i + 32] = input[i]; + } + + return result; + } + /// @notice Getter for the name and version of the contract /// @return string representing the name and the version of the contract function getVersion() external pure virtual returns (string memory) { diff --git a/contracts/gatewayLib/lib/Gateway.sol b/contracts/gatewayLib/lib/Gateway.sol index 932135c9..e02652f8 100644 --- a/contracts/gatewayLib/lib/Gateway.sol +++ b/contracts/gatewayLib/lib/Gateway.sol @@ -2,9 +2,8 @@ pragma solidity ^0.8.24; -import "../../addresses/GatewayContractAddress.sol"; import "../../lib/Impl.sol"; -import "../../addresses/ACLAddress.sol"; +import "../../addresses/GatewayContractAddress.sol"; interface IKMSVerifier { function verifyDecryptionEIP712KMSSignatures( @@ -25,24 +24,24 @@ interface IGatewayContract { ) external returns (uint256); } -library Gateway { - struct GatewayConfigStruct { - address GatewayContractAddress; - } +struct GatewayConfigStruct { + address GatewayContractAddress; +} +library Gateway { // keccak256(abi.encode(uint256(keccak256("fhevm.storage.GatewayConfig")) - 1)) & ~bytes32(uint256(0xff)) bytes32 private constant GatewayLocation = 0x93ab6e17f2c461cce6ea5d4ec117e51dda77a64affc2b2c05f8cd440def0e700; - function defaultGatewayAddress() internal pure returns (address) { - return GATEWAY_CONTRACT_PREDEPLOY_ADDRESS; - } - function getGetwayConfig() internal pure returns (GatewayConfigStruct storage $) { assembly { $.slot := GatewayLocation } } + function defaultGatewayAddress() internal pure returns (address) { + return GATEWAY_CONTRACT_PREDEPLOY_ADDRESS; + } + function setGateway(address gatewayAddress) internal { GatewayConfigStruct storage $ = getGetwayConfig(); $.GatewayContractAddress = gatewayAddress; @@ -130,10 +129,12 @@ library Gateway { assembly { calldatacopy(add(decryptedResult, 0x20), start, length) // Copy the relevant part of calldata to decryptedResult memory } + + decryptedResult = shiftOffsets(decryptedResult, handlesList); FHEVMConfig.FHEVMConfigStruct storage $ = Impl.getFHEVMConfig(); return IKMSVerifier($.KMSVerifierAddress).verifyDecryptionEIP712KMSSignatures( - aclAdd, + $.ACLAddress, handlesList, decryptedResult, signatures @@ -160,7 +161,45 @@ library Gateway { revert("Unsupported handle type"); } } - signedDataLength += 32; // for the signatures offset + signedDataLength += 32; // add offset of signatures return signedDataLength; } + + function shiftOffsets(bytes memory input, uint256[] memory handlesList) private pure returns (bytes memory) { + uint256 numArgs = handlesList.length; + for (uint256 i = 0; i < numArgs; i++) { + uint8 typeCt = uint8(handlesList[i] >> 8); + if (typeCt >= 9) { + input = subToBytes32Slice(input, 32 * i); // because we append the signatures, all bytes offsets are shifted by 0x20 + } + } + input = remove32Slice(input, 32 * numArgs); + return input; + } + + function subToBytes32Slice(bytes memory data, uint256 offset) private pure returns (bytes memory) { + // @note: data is assumed to be more than 32+offset bytes long + assembly { + let ptr := add(add(data, 0x20), offset) + let val := mload(ptr) + val := sub(val, 0x20) + mstore(ptr, val) + } + return data; + } + + function remove32Slice(bytes memory input, uint256 start) public pure returns (bytes memory) { + // @note we assume start+32 is less than input.length + bytes memory result = new bytes(input.length - 32); + + for (uint256 i = 0; i < start; i++) { + result[i] = input[i]; + } + + for (uint256 i = start + 32; i < input.length; i++) { + result[i - 32] = input[i]; + } + + return result; + } } diff --git a/contracts/test/asyncDecrypt.ts b/contracts/test/asyncDecrypt.ts index 98524b36..31000b1a 100644 --- a/contracts/test/asyncDecrypt.ts +++ b/contracts/test/asyncDecrypt.ts @@ -122,7 +122,6 @@ const fulfillAllPastRequestsIds = async (mocked: boolean) => { const handles = event.args[1]; const typesList = handles.map((handle) => parseInt(handle.toString(16).slice(-4, -2), 16)); const msgValue = event.args[4]; - const passSignaturesToCaller = event.args[6]; if (!results.includes(requestID)) { // if request is not already fulfilled if (mocked) { @@ -154,13 +153,8 @@ const fulfillAllPastRequestsIds = async (mocked: boolean) => { const abiCoder = new ethers.AbiCoder(); let encodedData; let calldata; - if (!passSignaturesToCaller) { - encodedData = abiCoder.encode(['uint256', ...types], [31, ...valuesFormatted4]); // 31 is just a dummy uint256 requestID to get correct abi encoding for the remaining arguments (i.e everything except the requestID) - calldata = '0x' + encodedData.slice(66); // we just pop the dummy requestID to get the correct value to pass for `decryptedCts` - } else { - encodedData = abiCoder.encode(['uint256', ...types, 'bytes[]'], [31, ...valuesFormatted4, []]); // adding also a dummy empty array of bytes for correct abi-encoding when used with signatures - calldata = '0x' + encodedData.slice(66).slice(0, -64); // we also pop the last 32 bytes (empty bytes[]) - } + encodedData = abiCoder.encode(['uint256', ...types], [31, ...valuesFormatted4]); // 31 is just a dummy uint256 requestID to get correct abi encoding for the remaining arguments (i.e everything except the requestID) + calldata = '0x' + encodedData.slice(66); // we just pop the dummy requestID to get the correct value to pass for `decryptedCts` const numSigners = +process.env.NUM_KMS_SIGNERS!; const decryptResultsEIP712signatures = await computeDecryptSignatures(handles, calldata, numSigners); diff --git a/contracts/test/upgrades/upgrades.ts b/contracts/test/upgrades/upgrades.ts index 0978cba8..1e6be770 100644 --- a/contracts/test/upgrades/upgrades.ts +++ b/contracts/test/upgrades/upgrades.ts @@ -84,7 +84,7 @@ describe('Upgrades', function () { kind: 'uups', }); await gateway.waitForDeployment(); - expect(await gateway.getVersion()).to.equal('GatewayContract v0.1.0'); + expect(await gateway.getVersion()).to.equal('GatewayContract v0.1.1'); const gateway2 = await upgrades.upgradeProxy(gateway, this.gatewayFactoryUpgraded); await gateway2.waitForDeployment(); expect(await gateway2.getVersion()).to.equal('GatewayContract v0.2.0'); From 20487601d606a76f59ebca8f2451cd4e839408e6 Mon Sep 17 00:00:00 2001 From: jatZama Date: Tue, 10 Dec 2024 00:12:42 +0100 Subject: [PATCH 2/2] chore: bump version for prerelease --- contracts/package-lock.json | 4 ++-- contracts/package.json | 2 +- fhevm-engine/Cargo.lock | 8 ++++---- fhevm-engine/coprocessor/Cargo.toml | 2 +- fhevm-engine/executor/Cargo.toml | 2 +- fhevm-engine/fhevm-engine-common/Cargo.toml | 2 +- fhevm-engine/scheduler/Cargo.toml | 2 +- 7 files changed, 11 insertions(+), 11 deletions(-) diff --git a/contracts/package-lock.json b/contracts/package-lock.json index 78f3fd70..48b56f70 100644 --- a/contracts/package-lock.json +++ b/contracts/package-lock.json @@ -1,12 +1,12 @@ { "name": "fhevm-core-contracts", - "version": "0.6.0", + "version": "0.6.1-0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "fhevm-core-contracts", - "version": "0.6.0", + "version": "0.6.1-0", "license": "BSD-3-Clause", "devDependencies": { "@nomicfoundation/hardhat-toolbox": "^5.0.0", diff --git a/contracts/package.json b/contracts/package.json index cd76d8cb..fb040c74 100644 --- a/contracts/package.json +++ b/contracts/package.json @@ -1,6 +1,6 @@ { "name": "fhevm-core-contracts", - "version": "0.6.0", + "version": "0.6.1-0", "description": "fhEVM contracts", "repository": { "type": "git", diff --git a/fhevm-engine/Cargo.lock b/fhevm-engine/Cargo.lock index 986ee968..a5c111bb 100644 --- a/fhevm-engine/Cargo.lock +++ b/fhevm-engine/Cargo.lock @@ -1758,7 +1758,7 @@ dependencies = [ [[package]] name = "coprocessor" -version = "0.6.0" +version = "0.6.1-alpha.0" dependencies = [ "actix-web", "alloy", @@ -2193,7 +2193,7 @@ checksum = "0206175f82b8d6bf6652ff7d71a1e27fd2e4efde587fd368662814d6ec1d9ce0" [[package]] name = "executor" -version = "0.6.0" +version = "0.6.1-alpha.0" dependencies = [ "anyhow", "bincode", @@ -2239,7 +2239,7 @@ dependencies = [ [[package]] name = "fhevm-engine-common" -version = "0.6.0" +version = "0.6.1-alpha.0" dependencies = [ "anyhow", "bigdecimal", @@ -4436,7 +4436,7 @@ dependencies = [ [[package]] name = "scheduler" -version = "0.6.0" +version = "0.6.1-alpha.0" dependencies = [ "anyhow", "daggy", diff --git a/fhevm-engine/coprocessor/Cargo.toml b/fhevm-engine/coprocessor/Cargo.toml index 6d92cb37..aa0059b0 100644 --- a/fhevm-engine/coprocessor/Cargo.toml +++ b/fhevm-engine/coprocessor/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "coprocessor" -version = "0.6.0" +version = "0.6.1-alpha.0" default-run = "coprocessor" authors.workspace = true edition.workspace = true diff --git a/fhevm-engine/executor/Cargo.toml b/fhevm-engine/executor/Cargo.toml index 395001f3..bd14f526 100644 --- a/fhevm-engine/executor/Cargo.toml +++ b/fhevm-engine/executor/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "executor" -version = "0.6.0" +version = "0.6.1-alpha.0" authors.workspace = true edition.workspace = true license.workspace = true diff --git a/fhevm-engine/fhevm-engine-common/Cargo.toml b/fhevm-engine/fhevm-engine-common/Cargo.toml index 525b330c..70d260e2 100644 --- a/fhevm-engine/fhevm-engine-common/Cargo.toml +++ b/fhevm-engine/fhevm-engine-common/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "fhevm-engine-common" -version = "0.6.0" +version = "0.6.1-alpha.0" authors.workspace = true edition.workspace = true license.workspace = true diff --git a/fhevm-engine/scheduler/Cargo.toml b/fhevm-engine/scheduler/Cargo.toml index e14e3d39..fea12eb7 100644 --- a/fhevm-engine/scheduler/Cargo.toml +++ b/fhevm-engine/scheduler/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "scheduler" -version = "0.6.0" +version = "0.6.1-alpha.0" edition = "2021" [dependencies]