Skip to content

Commit

Permalink
Convert string to bytes32 from subject
Browse files Browse the repository at this point in the history
  • Loading branch information
JohnGuilding committed Jun 21, 2024
1 parent 181c5be commit 10b1532
Show file tree
Hide file tree
Showing 15 changed files with 124 additions and 80 deletions.
5 changes: 3 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,10 @@
"test:optimized": "pnpm run build:optimized && FOUNDRY_PROFILE=test-optimized forge test"
},
"dependencies": {
"@rhinestone/modulekit": "v0.4.2",
"@openzeppelin/contracts-upgradeable": "v5.0.1",
"@zk-email/contracts": "v6.0.3"
"@rhinestone/modulekit": "v0.4.2",
"@zk-email/contracts": "v6.0.3",
"solidity-stringutils": "https://github.com/Arachnid/solidity-stringutils.git"
},
"files": [
"src",
Expand Down
85 changes: 47 additions & 38 deletions pnpm-lock.yaml

Large diffs are not rendered by default.

3 changes: 2 additions & 1 deletion remappings.txt
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,5 @@ solarray/=node_modules/solarray/src/
@prb/math/=node_modules/@prb/math/src/

ether-email-auth/=lib/ether-email-auth/
@zk-email/contracts/=node_modules/@zk-email/contracts/
@zk-email/contracts/=node_modules/@zk-email/contracts/
solidity-stringutils/=node_modules/solidity-stringutils/
12 changes: 4 additions & 8 deletions src/EmailRecoveryManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -349,9 +349,8 @@ contract EmailRecoveryManager is EmailAccountRecoveryNew, Initializable, IEmailR
revert InvalidTemplateIndex();
}

(address account, string memory calldataHashString) = IEmailRecoverySubjectHandler(
subjectHandler
).validateRecoverySubject(templateIdx, subjectParams, address(this));
(address account, bytes32 calldataHash) = IEmailRecoverySubjectHandler(subjectHandler)
.validateRecoverySubject(templateIdx, subjectParams, address(this));

if (IRecoveryModule(emailRecoveryModule).getAllowedValidators(account).length == 0) {
revert RecoveryModuleNotInstalled();
Expand All @@ -375,7 +374,7 @@ contract EmailRecoveryManager is EmailAccountRecoveryNew, Initializable, IEmailR

recoveryRequest.executeAfter = executeAfter;
recoveryRequest.executeBefore = executeBefore;
recoveryRequest.calldataHashString = calldataHashString;
recoveryRequest.calldataHash = calldataHash;

emit RecoveryProcessed(account, executeAfter, executeBefore);
}
Expand Down Expand Up @@ -422,11 +421,8 @@ contract EmailRecoveryManager is EmailAccountRecoveryNew, Initializable, IEmailR

delete recoveryRequests[account];

// TODO: Do ZkEmail contracts have a library for converting a string to bytes32?
bytes32 calldataHash = keccak256(recoveryCalldata);
string memory calldataHashString = uint256(calldataHash).toHexString(32);

if (!Strings.equal(calldataHashString, recoveryRequest.calldataHashString)) {
if (calldataHash != recoveryRequest.calldataHash) {
revert InvalidCalldataHash();
}

Expand Down
6 changes: 4 additions & 2 deletions src/handlers/EmailRecoverySubjectHandler.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ pragma solidity ^0.8.25;

import { IEmailRecoverySubjectHandler } from "../interfaces/IEmailRecoverySubjectHandler.sol";
import { EmailRecoveryManager } from "../EmailRecoveryManager.sol";
import { StringUtils } from "../libraries/StringUtils.sol";
import "forge-std/console2.sol";

/**
Expand Down Expand Up @@ -90,7 +91,7 @@ contract EmailRecoverySubjectHandler is IEmailRecoverySubjectHandler {
)
public
view
returns (address, string memory)
returns (address, bytes32)
{
if (subjectParams.length != 3) {
revert InvalidSubjectParams();
Expand All @@ -99,6 +100,7 @@ contract EmailRecoverySubjectHandler is IEmailRecoverySubjectHandler {
address accountInEmail = abi.decode(subjectParams[0], (address));
address recoveryModuleInEmail = abi.decode(subjectParams[1], (address));
string memory calldataHashInEmail = abi.decode(subjectParams[2], (string));
bytes32 calldataHash = StringUtils.hexToBytes32(calldataHashInEmail);

if (accountInEmail == address(0)) {
revert InvalidAccount();
Expand All @@ -114,6 +116,6 @@ contract EmailRecoverySubjectHandler is IEmailRecoverySubjectHandler {
revert InvalidRecoveryModule();
}

return (accountInEmail, calldataHashInEmail);
return (accountInEmail, calldataHash);
}
}
5 changes: 2 additions & 3 deletions src/handlers/SafeRecoverySubjectHandler.sol
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ contract SafeRecoverySubjectHandler is IEmailRecoverySubjectHandler {
)
public
view
returns (address, string memory)
returns (address, bytes32)
{
if (subjectParams.length != 4) {
revert InvalidSubjectParams();
Expand Down Expand Up @@ -136,9 +136,8 @@ contract SafeRecoverySubjectHandler is IEmailRecoverySubjectHandler {
functionSignature, previousOwnerInLinkedList, oldOwnerInEmail, newOwnerInEmail
);
bytes32 calldataHash = keccak256(recoveryCallData);
string memory calldataHashString = uint256(calldataHash).toHexString(32);

return (accountInEmail, calldataHashString);
return (accountInEmail, calldataHash);
}

function getPreviousOwnerInLinkedList(
Expand Down
2 changes: 1 addition & 1 deletion src/interfaces/IEmailRecoveryManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ interface IEmailRecoveryManager {
uint256 executeAfter; // the timestamp from which the recovery request can be executed
uint256 executeBefore; // the timestamp from which the recovery request becomes invalid
uint256 currentWeight; // total weight of all guardian approvals for the recovery request
string calldataHashString; // the keccak256 hash of the calldata used to execute the
bytes32 calldataHash; // the keccak256 hash of the calldata used to execute the
// recovery attempt
}

Expand Down
2 changes: 1 addition & 1 deletion src/interfaces/IEmailRecoverySubjectHandler.sol
Original file line number Diff line number Diff line change
Expand Up @@ -36,5 +36,5 @@ interface IEmailRecoverySubjectHandler {
)
external
view
returns (address, string memory);
returns (address, bytes32);
}
42 changes: 42 additions & 0 deletions src/libraries/StringUtils.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.13;

import "solidity-stringutils/src/strings.sol";

// Extracted from https://github.com/zkemail/email-wallet-sdk/blob/main/src/helpers/StringUtils.sol
library StringUtils {
using strings for *;

function hexToBytes32(string calldata hexStr) public pure returns (bytes32 result) {
require(hexStr.toSlice().startsWith("0x".toSlice()), "invalid hex prefix");
hexStr = hexStr[2:];
require(bytes(hexStr).length == 64, "invalid hex string length");
uint256[] memory ints = hex2Ints(hexStr);
uint256 sum = 0;
for (uint256 i = 0; i < 32; i++) {
sum = (256 * sum + ints[i]);
}
return bytes32(sum);
}

function hex2Ints(string memory hexStr) private pure returns (uint256[] memory) {
uint256[] memory result = new uint256[](bytes(hexStr).length / 2);
for (uint256 i = 0; i < result.length; i++) {
result[i] =
16 * hexChar2Int(bytes(hexStr)[2 * i]) + hexChar2Int(bytes(hexStr)[2 * i + 1]);
}
return result;
}

function hexChar2Int(bytes1 char) private pure returns (uint256) {
uint8 charInt = uint8(char);
if (charInt >= 48 && charInt <= 57) {
return charInt - 48;
} else if (charInt >= 97 && charInt <= 102) {
return charInt - 87;
} else {
require(false, "invalid hex char");
}
return 0;
}
}
15 changes: 6 additions & 9 deletions test/unit/EmailRecoveryManager/cancelRecovery.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,8 @@ import { EmailRecoveryModule } from "src/modules/EmailRecoveryModule.sol";
contract EmailRecoveryManager_cancelRecovery_Test is UnitBase {
using Strings for uint256;

string calldataHashString;

function setUp() public override {
super.setUp();
calldataHashString = uint256(calldataHash).toHexString(32);
}

function test_CancelRecovery_CannotCancelWrongRecoveryRequest() public {
Expand All @@ -29,7 +26,7 @@ contract EmailRecoveryManager_cancelRecovery_Test is UnitBase {
assertEq(recoveryRequest.executeAfter, 0);
assertEq(recoveryRequest.executeBefore, 0);
assertEq(recoveryRequest.currentWeight, 1);
assertEq(recoveryRequest.calldataHashString, "");
assertEq(recoveryRequest.calldataHash, "");

vm.startPrank(otherAddress);
emailRecoveryManager.cancelRecovery();
Expand All @@ -38,7 +35,7 @@ contract EmailRecoveryManager_cancelRecovery_Test is UnitBase {
assertEq(recoveryRequest.executeAfter, 0);
assertEq(recoveryRequest.executeBefore, 0);
assertEq(recoveryRequest.currentWeight, 1);
assertEq(recoveryRequest.calldataHashString, "");
assertEq(recoveryRequest.calldataHash, "");
}

function test_CancelRecovery_PartialRequest_Succeeds() public {
Expand All @@ -51,7 +48,7 @@ contract EmailRecoveryManager_cancelRecovery_Test is UnitBase {
assertEq(recoveryRequest.executeAfter, 0);
assertEq(recoveryRequest.executeBefore, 0);
assertEq(recoveryRequest.currentWeight, 1);
assertEq(recoveryRequest.calldataHashString, "");
assertEq(recoveryRequest.calldataHash, "");

vm.startPrank(accountAddress);
emailRecoveryManager.cancelRecovery();
Expand All @@ -60,7 +57,7 @@ contract EmailRecoveryManager_cancelRecovery_Test is UnitBase {
assertEq(recoveryRequest.executeAfter, 0);
assertEq(recoveryRequest.executeBefore, 0);
assertEq(recoveryRequest.currentWeight, 0);
assertEq(recoveryRequest.calldataHashString, "");
assertEq(recoveryRequest.calldataHash, "");
}

function test_CancelRecovery_FullRequest_Succeeds() public {
Expand All @@ -75,7 +72,7 @@ contract EmailRecoveryManager_cancelRecovery_Test is UnitBase {
assertEq(recoveryRequest.executeAfter, block.timestamp + delay);
assertEq(recoveryRequest.executeBefore, block.timestamp + expiry);
assertEq(recoveryRequest.currentWeight, 3);
assertEq(recoveryRequest.calldataHashString, calldataHashString);
assertEq(recoveryRequest.calldataHash, calldataHash);

vm.startPrank(accountAddress);
emailRecoveryManager.cancelRecovery();
Expand All @@ -84,6 +81,6 @@ contract EmailRecoveryManager_cancelRecovery_Test is UnitBase {
assertEq(recoveryRequest.executeAfter, 0);
assertEq(recoveryRequest.executeBefore, 0);
assertEq(recoveryRequest.currentWeight, 0);
assertEq(recoveryRequest.calldataHashString, "");
assertEq(recoveryRequest.calldataHash, "");
}
}
4 changes: 2 additions & 2 deletions test/unit/EmailRecoveryManager/completeRecovery.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ contract EmailRecoveryManager_completeRecovery_Test is UnitBase {
assertEq(recoveryRequest.executeAfter, 0);
assertEq(recoveryRequest.executeBefore, 0);
assertEq(recoveryRequest.currentWeight, 0);
assertEq(recoveryRequest.calldataHashString, "");
assertEq(recoveryRequest.calldataHash, "");
}

function test_CompleteRecovery_SucceedsAlmostExpiry() public {
Expand All @@ -127,6 +127,6 @@ contract EmailRecoveryManager_completeRecovery_Test is UnitBase {
assertEq(recoveryRequest.executeAfter, 0);
assertEq(recoveryRequest.executeBefore, 0);
assertEq(recoveryRequest.currentWeight, 0);
assertEq(recoveryRequest.calldataHashString, "");
assertEq(recoveryRequest.calldataHash, "");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ contract EmailRecoveryManager_deInitRecoveryFromModule_Test is UnitBase {
assertEq(recoveryRequest.executeAfter, 0);
assertEq(recoveryRequest.executeBefore, 0);
assertEq(recoveryRequest.currentWeight, 0);
assertEq(recoveryRequest.calldataHashString, "");
assertEq(recoveryRequest.calldataHash, "");

// assert that guardian storage has been cleared successfully for guardian 1
GuardianStorage memory guardianStorage1 =
Expand Down
4 changes: 2 additions & 2 deletions test/unit/EmailRecoveryManager/processRecovery.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ contract EmailRecoveryManager_processRecovery_Test is UnitBase {
assertEq(recoveryRequest.executeAfter, 0);
assertEq(recoveryRequest.executeBefore, 0);
assertEq(recoveryRequest.currentWeight, guardian1Weight);
assertEq(recoveryRequest.calldataHashString, "");
assertEq(recoveryRequest.calldataHash, "");
}

function test_ProcessRecovery_InitiatesRecovery() public {
Expand All @@ -118,6 +118,6 @@ contract EmailRecoveryManager_processRecovery_Test is UnitBase {
assertEq(recoveryRequest.executeAfter, block.timestamp + delay);
assertEq(recoveryRequest.executeBefore, block.timestamp + expiry);
assertEq(recoveryRequest.currentWeight, guardian1Weight + guardian2Weight);
assertEq(recoveryRequest.calldataHashString, calldataHashString);
assertEq(recoveryRequest.calldataHash, calldataHash);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,9 @@ contract EmailRecoverySubjectHandler_validateRecoverySubject_Test is UnitBase {
}

function test_ValidateRecoverySubject_Succeeds() public view {
(address account, string memory calldataHash) = emailRecoveryHandler.validateRecoverySubject(
templateIdx, subjectParams, emailRecoveryManagerAddress
);
assertEq(account, accountAddress);
assertEq(calldataHashString, calldataHash);
(address accountFromEmail, bytes32 calldataHashFromEmail) = emailRecoveryHandler
.validateRecoverySubject(templateIdx, subjectParams, emailRecoveryManagerAddress);
assertEq(accountFromEmail, accountAddress);
assertEq(calldataHashFromEmail, calldataHash);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,11 @@ import { SafeUnitBase } from "../../SafeUnitBase.t.sol";
contract SafeRecoverySubjectHandler_validateRecoverySubject_Test is SafeUnitBase {
using Strings for uint256;

string calldataHashString;
bytes[] subjectParams;

function setUp() public override {
super.setUp();

calldataHashString = uint256(calldataHash).toHexString(32);
subjectParams = new bytes[](4);
subjectParams[0] = abi.encode(accountAddress);
subjectParams[1] = abi.encode(owner);
Expand Down Expand Up @@ -85,9 +83,9 @@ contract SafeRecoverySubjectHandler_validateRecoverySubject_Test is SafeUnitBase
}

function test_ValidateRecoverySubject_Succeeds() public view {
(address account, string memory calldataHash) = safeRecoverySubjectHandler
(address accountFromEmail, bytes32 calldataHashFromEmail) = safeRecoverySubjectHandler
.validateRecoverySubject(templateIdx, subjectParams, emailRecoveryManagerAddress);
assertEq(account, accountAddress);
assertEq(calldataHash, calldataHashString);
assertEq(accountFromEmail, accountAddress);
assertEq(calldataHashFromEmail, calldataHash);
}
}

0 comments on commit 10b1532

Please sign in to comment.