diff --git a/.gitmodules b/.gitmodules index 4c0d6cdd..ddf6254e 100644 --- a/.gitmodules +++ b/.gitmodules @@ -7,6 +7,9 @@ [submodule "contracts/lib/world-id-contracts"] path = contracts/lib/world-id-contracts url = https://github.com/worldcoin/world-id-contracts +[submodule "contracts/lib/safe-contracts"] + path = contracts/lib/safe-contracts + url = https://github.com/safe-global/safe-contracts [submodule "contracts/lib/openzeppelin-contracts"] path = contracts/lib/openzeppelin-contracts url = https://github.com/OpenZeppelin/openzeppelin-contracts @@ -16,3 +19,6 @@ [submodule "contracts/lib/openzeppelin-contracts-upgradeable"] path = contracts/lib/openzeppelin-contracts-upgradeable url = https://github.com/OpenZeppelin/openzeppelin-contracts-upgradeable +[submodule "contracts/lib/safe-modules"] + path = contracts/lib/safe-modules + url = https://github.com/worldcoin/safe-modules diff --git a/contracts/foundry.toml b/contracts/foundry.toml index e0482b04..401fd978 100644 --- a/contracts/foundry.toml +++ b/contracts/foundry.toml @@ -3,13 +3,9 @@ evm_version = 'cancun' src = "src" out = "out" libs = ["lib"] -via_ir = true optimizer = true optimizer_runs = 200 [fuzz] runs = 5000 -max_test_rejects = 150000 - - - +max_test_rejects = 150000 \ No newline at end of file diff --git a/contracts/lib/safe-contracts b/contracts/lib/safe-contracts new file mode 160000 index 00000000..192c7dc6 --- /dev/null +++ b/contracts/lib/safe-contracts @@ -0,0 +1 @@ +Subproject commit 192c7dc67290940fcbc75165522bb86a37187069 diff --git a/contracts/lib/safe-modules b/contracts/lib/safe-modules new file mode 160000 index 00000000..9abf69ea --- /dev/null +++ b/contracts/lib/safe-modules @@ -0,0 +1 @@ +Subproject commit 9abf69ea1df673c1010aeb9bbbc6aa14124ba425 diff --git a/contracts/remappings.txt b/contracts/remappings.txt index 37bc3748..cb51404c 100644 --- a/contracts/remappings.txt +++ b/contracts/remappings.txt @@ -5,5 +5,7 @@ @BokkyPooBahsDateTimeLibrary/=lib/BokkyPooBahsDateTimeLibrary/contracts/ @helpers/=src/helpers/ openzeppelin-contracts/=lib/world-id-contracts/lib/openzeppelin-contracts/contracts/ +@4337=lib/safe-modules/modules/4337/contracts/ +@safe-global/safe-contracts/contracts/=lib/safe-contracts/contracts/ @forge-std/=lib/forge-std/src/ forge-std/=lib/forge-std/src/ \ No newline at end of file diff --git a/contracts/src/PBH4337Module.sol b/contracts/src/PBH4337Module.sol new file mode 100644 index 00000000..898ea2ec --- /dev/null +++ b/contracts/src/PBH4337Module.sol @@ -0,0 +1,94 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.28; + +import {Safe4337Module} from "@4337/Safe4337Module.sol"; +import {PackedUserOperation} from "@account-abstraction/contracts/interfaces/PackedUserOperation.sol"; +import {ValidationData} from "@account-abstraction/contracts/core/Helpers.sol"; +import {_packValidationData} from "@account-abstraction/contracts/core/Helpers.sol"; +import {ISafe} from "@4337/interfaces/Safe.sol"; + +contract PBHSafe4337Module is Safe4337Module { + uint256 constant ECDSA_SIGNATURE_LENGTH = 65; + uint256 constant TIMESTAMP_BYTES = 12; // 6 bytes each for validAfter and validUntil + + address public immutable PBH_SIGNATURE_AGGREGATOR; + uint192 public immutable PBH_NONCE_KEY; + + error InvalidProofSize(); + + constructor(address entryPoint, address _pbhSignatureAggregator, uint192 _pbhNonceKey) Safe4337Module(entryPoint) { + PBH_SIGNATURE_AGGREGATOR = _pbhSignatureAggregator; + PBH_NONCE_KEY = _pbhNonceKey; + } + + // TODO: Fork the Safe4337Module dependency and add 'override' to _validateSignatures. It is manually updated currently and CI will fail + /** + * @dev Validates that the user operation is correctly signed and returns an ERC-4337 packed validation data + * of `validAfter || validUntil || authorizer`: + * - `authorizer`: 20-byte address, 0 for valid signature or 1 to mark signature failure (this module does not make use of signature aggregators). + * - `validUntil`: 6-byte timestamp value, or zero for "infinite". The user operation is valid only up to this time. + * - `validAfter`: 6-byte timestamp. The user operation is valid only after this time. + * @param userOp User operation struct. + * @return validationData An integer indicating the result of the validation. + */ + function _validateSignatures(PackedUserOperation calldata userOp) + internal + view + override + returns (uint256 validationData) + { + // Check if the userOp has the specified PBH key + // https://github.com/eth-infinitism/account-abstraction/blob/develop/contracts/core/NonceManager.sol#L38 + uint192 key = uint192(userOp.nonce >> 64); + + // This does NOT validate the proof + // It removes the first 12 bytes from the signature as it represents the validAfter and validUntil values + // operationData is not determined by the signature + (bytes memory operationData, uint48 validAfter, uint48 validUntil, bytes calldata signatures) = + _getSafeOp(userOp); + + // If it is a PBH transaction, we need to handle two cases with the signature: + // 1. The bundler simulates the call with the proof appended + // 2. UserOp execution without proof appended + bool isPBH = (key == PBH_NONCE_KEY); + + // Base signature length calculation: + // TIMESTAMP_BYTES (12) + (threshold * ECDSA_SIGNATURE_LENGTH) + uint256 expectedLength = + TIMESTAMP_BYTES + (ISafe(payable(userOp.sender)).getThreshold() * ECDSA_SIGNATURE_LENGTH); + + // If the signature length is greater than the expected length, then we know that the bundler appended the proof + // We need to remove the proof from the signature before validation + if (isPBH && userOp.signature.length > expectedLength) { + if (userOp.signature.length - expectedLength != 352) { + revert InvalidProofSize(); + } + // Remove the proof from the signature + signatures = userOp.signature[0:expectedLength]; + } + + // The `checkSignatures` function in the Safe contract does not force a fixed size on signature length. + // A malicious bundler can pad the Safe operation `signatures` with additional bytes, causing the account to pay + // more gas than needed for user operation validation (capped by `verificationGasLimit`). + // `_checkSignaturesLength` ensures that there are no additional bytes in the `signature` than are required. + bool validSignature = _checkSignaturesLength(signatures, ISafe(payable(userOp.sender)).getThreshold()); + + try ISafe(payable(userOp.sender)).checkSignatures(keccak256(operationData), operationData, signatures) {} + catch { + validSignature = false; + } + + address authorizer; + + // If the signature is valid and the userOp is a PBH userOp, return the PBH signature aggregator as the authorizer + // Else return 0 for valid signature and 1 for invalid signature + if (isPBH && validSignature) { + authorizer = PBH_SIGNATURE_AGGREGATOR; + } else { + authorizer = validSignature ? address(0) : address(1); + } + + // The timestamps are validated by the entry point, therefore we will not check them again. + validationData = _packValidationData(ValidationData(authorizer, validUntil, validAfter)); + } +} diff --git a/contracts/src/PBHEntryPoint.sol b/contracts/src/PBHEntryPoint.sol index 5e4e4383..3c07f6de 100644 --- a/contracts/src/PBHEntryPoint.sol +++ b/contracts/src/PBHEntryPoint.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.20; +pragma solidity ^0.8.28; import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol"; diff --git a/contracts/src/PBHEntryPointImplV1.sol b/contracts/src/PBHEntryPointImplV1.sol index e471b195..7ee4cbc7 100644 --- a/contracts/src/PBHEntryPointImplV1.sol +++ b/contracts/src/PBHEntryPointImplV1.sol @@ -1,16 +1,19 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.20; +pragma solidity ^0.8.28; -import {PBHVerifier} from "./PBHVerifier.sol"; import {IWorldIDGroups} from "@world-id-contracts/interfaces/IWorldIDGroups.sol"; import {IEntryPoint} from "@account-abstraction/contracts/interfaces/IEntryPoint.sol"; import {IPBHEntryPoint} from "./interfaces/IPBHEntryPoint.sol"; +import {ByteHasher} from "./helpers/ByteHasher.sol"; +import {PBHExternalNullifier} from "./helpers/PBHExternalNullifier.sol"; +import {WorldIDImpl} from "@world-id-contracts/abstract/WorldIDImpl.sol"; +import "@BokkyPooBahsDateTimeLibrary/BokkyPooBahsDateTimeLibrary.sol"; /// @title PBH Entry Point Implementation V1 /// @dev This contract is an implementation of the PBH Entry Point. /// It is used to verify the signature of a Priority User Operation, and Relaying Priority Bundles to the EIP-4337 Entry Point. /// @author Worldcoin -contract PBHEntryPointImplV1 is IPBHEntryPoint, PBHVerifier { +contract PBHEntryPointImplV1 is IPBHEntryPoint, WorldIDImpl { /////////////////////////////////////////////////////////////////////////////// /// A NOTE ON IMPLEMENTATION CONTRACTS /// /////////////////////////////////////////////////////////////////////////////// @@ -48,6 +51,15 @@ contract PBHEntryPointImplV1 is IPBHEntryPoint, PBHVerifier { // these variables takes place. If reordering happens, a storage clash will occur (effectively a // memory safety error). + using ByteHasher for bytes; + + /////////////////////////////////////////////////////////////////////////////// + /// ERRORS /// + ////////////////////////////////////////////////////////////////////////////// + + /// @notice Thrown when attempting to reuse a nullifier + error InvalidNullifier(); + /////////////////////////////////////////////////////////////////////////////// /// Events /// ////////////////////////////////////////////////////////////////////////////// @@ -56,6 +68,23 @@ contract PBHEntryPointImplV1 is IPBHEntryPoint, PBHVerifier { IWorldIDGroups indexed worldId, IEntryPoint indexed entryPoint, uint8 indexed numPbhPerMonth ); + /// @notice Emitted once for each successful PBH verification. + /// + /// @param sender The sender of this particular transaction or UserOp. + /// @param nonce Transaction/UserOp nonce. + /// @param payload The zero-knowledge proof that demonstrates the claimer is registered with World ID. + event PBH(address indexed sender, uint256 indexed nonce, PBHPayload payload); + + /// @notice Emitted when the World ID address is set. + /// + /// @param worldId The World ID instance that will be used for verifying proofs. + event WorldIdSet(address indexed worldId); + + /// @notice Emitted when the number of PBH transactions allowed per month is set. + /// + /// @param numPbhPerMonth The number of allowed PBH transactions per month. + event NumPbhPerMonthSet(uint8 indexed numPbhPerMonth); + /////////////////////////////////////////////////////////////////////////////// /// Vars /// ////////////////////////////////////////////////////////////////////////////// @@ -65,6 +94,22 @@ contract PBHEntryPointImplV1 is IPBHEntryPoint, PBHVerifier { /// The PBHVerifier is always the proxy to the EntryPoint for PBH Bundles. bytes32 internal _hashedOps; + /// @dev The World ID group ID (always 1) + uint256 internal constant _GROUP_ID = 1; + + /// @dev The World ID instance that will be used for verifying proofs + IWorldIDGroups public worldId; + + /// @dev The EntryPoint where Aggregated PBH Bundles will be proxied to. + IEntryPoint public entryPoint; + + /// @notice The number of PBH transactions that may be used by a single + /// World ID in a given month. + uint8 public numPbhPerMonth; + + /// @dev Whether a nullifier hash has been used already. Used to guarantee an action is only performed once by a single person + mapping(uint256 => bool) public nullifierHashes; + /////////////////////////////////////////////////////////////////////////////// /// INITIALIZATION /// /////////////////////////////////////////////////////////////////////////////// @@ -158,4 +203,60 @@ contract PBHEntryPointImplV1 is IPBHEntryPoint, PBHVerifier { if iszero(eq(tload(hashedOps), hashedOps)) { revert(0, 0) } } } + + /// @param sender The sender of this particular transaction or UserOp. + /// @param nonce Transaction/UserOp nonce. + /// @param callData Transaction/UserOp call data. + /// @param pbhPayload The PBH payload containing the proof data. + function verifyPbh(address sender, uint256 nonce, bytes calldata callData, PBHPayload memory pbhPayload) + public + virtual + onlyInitialized + onlyProxy + { + // First, we make sure this nullifier has not been used before. + if (nullifierHashes[pbhPayload.nullifierHash]) { + revert InvalidNullifier(); + } + + // Verify the external nullifier + PBHExternalNullifier.verify(pbhPayload.pbhExternalNullifier, numPbhPerMonth); + + // If worldId address is set, proceed with on chain verification, + // otherwise assume verification has been done off chain by the builder. + if (address(worldId) != address(0)) { + // We now generate the signal hash from the sender, nonce, and calldata + uint256 signalHash = abi.encodePacked(sender, nonce, callData).hashToField(); + + // We now verify the provided proof is valid and the user is verified by World ID + worldId.verifyProof( + pbhPayload.root, + _GROUP_ID, + signalHash, + pbhPayload.nullifierHash, + pbhPayload.pbhExternalNullifier, + pbhPayload.proof + ); + } + + // We now record the user has done this, so they can't do it again (proof of uniqueness) + nullifierHashes[pbhPayload.nullifierHash] = true; + + emit PBH(sender, nonce, pbhPayload); + } + + /// @notice Sets the number of PBH transactions allowed per month. + /// @param _numPbhPerMonth The number of allowed PBH transactions per month. + function setNumPbhPerMonth(uint8 _numPbhPerMonth) external virtual onlyOwner onlyProxy onlyInitialized { + numPbhPerMonth = _numPbhPerMonth; + emit NumPbhPerMonthSet(_numPbhPerMonth); + } + + /// @dev If the World ID address is set to 0, then it is assumed that verification will take place off chain. + /// @notice Sets the World ID instance that will be used for verifying proofs. + /// @param _worldId The World ID instance that will be used for verifying proofs. + function setWorldId(address _worldId) external virtual onlyOwner onlyProxy onlyInitialized { + worldId = IWorldIDGroups(_worldId); + emit WorldIdSet(_worldId); + } } diff --git a/contracts/src/PBHSignatureAggregator.sol b/contracts/src/PBHSignatureAggregator.sol index fadb60bc..e4d2255d 100644 --- a/contracts/src/PBHSignatureAggregator.sol +++ b/contracts/src/PBHSignatureAggregator.sol @@ -1,9 +1,8 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.20; +pragma solidity ^0.8.28; import "@account-abstraction/contracts/interfaces/PackedUserOperation.sol"; import {IPBHEntryPoint} from "./interfaces/IPBHEntryPoint.sol"; -import {IPBHVerifier} from "./interfaces/IPBHVerifier.sol"; import {IAggregator} from "@account-abstraction/contracts/interfaces/IAggregator.sol"; /// @title PBH Signature Aggregator @@ -64,12 +63,12 @@ contract PBHSignatureAggregator is IAggregator { pure returns (bytes memory aggregatedSignature) { - IPBHVerifier.PBHPayload[] memory pbhPayloads = new IPBHVerifier.PBHPayload[](userOps.length); + IPBHEntryPoint.PBHPayload[] memory pbhPayloads = new IPBHEntryPoint.PBHPayload[](userOps.length); for (uint256 i = 0; i < userOps.length; ++i) { // Bytes (0:65) - UserOp Signature // Bytes (65:65 + 352) - Packed Proof Data bytes memory proofData = userOps[i].signature[65:]; - pbhPayloads[i] = abi.decode(proofData, (IPBHVerifier.PBHPayload)); + pbhPayloads[i] = abi.decode(proofData, (IPBHEntryPoint.PBHPayload)); } aggregatedSignature = abi.encode(pbhPayloads); } diff --git a/contracts/src/PBHVerifier.sol b/contracts/src/PBHVerifier.sol deleted file mode 100644 index eebe0eb0..00000000 --- a/contracts/src/PBHVerifier.sol +++ /dev/null @@ -1,165 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity ^0.8.20; - -import {ByteHasher} from "./helpers/ByteHasher.sol"; -import {PBHExternalNullifier} from "./helpers/PBHExternalNullifier.sol"; -import {IWorldIDGroups} from "@world-id-contracts/interfaces/IWorldIDGroups.sol"; -import {WorldIDImpl} from "@world-id-contracts/abstract/WorldIDImpl.sol"; -import {IEntryPoint} from "@account-abstraction/contracts/interfaces/IEntryPoint.sol"; -import {IPBHVerifier} from "./interfaces/IPBHVerifier.sol"; -import "@BokkyPooBahsDateTimeLibrary/BokkyPooBahsDateTimeLibrary.sol"; - -/// @title PBH Verifier Implementation Version 1 -/// @author Worldcoin -/// @notice An implementation of a priority blockspace for humans (PBH) transaction verifier. -/// @dev This is the implementation delegated to by a proxy. -contract PBHVerifier is IPBHVerifier, WorldIDImpl { - /////////////////////////////////////////////////////////////////////////////// - /// A NOTE ON IMPLEMENTATION CONTRACTS /// - /////////////////////////////////////////////////////////////////////////////// - - // This contract is designed explicitly to operate from behind a proxy contract. As a result, - // there are a few important implementation considerations: - // - // - All updates made after deploying a given version of the implementation should inherit from - // the latest version of the implementation. This prevents storage clashes. - // - All functions that are less access-restricted than `private` should be marked `virtual` in - // order to enable the fixing of bugs in the existing interface. - // - Any function that reads from or modifies state (i.e. is not marked `pure`) must be - // annotated with the `onlyProxy` and `onlyInitialized` modifiers. This ensures that it can - // only be called when it has access to the data in the proxy, otherwise results are likely to - // be nonsensical. - // - This contract deals with important data for the PBH system. Ensure that all newly-added - // functionality is carefully access controlled using `onlyOwner`, or a more granular access - // mechanism. - // - Do not assign any contract-level variables at the definition site unless they are - // `constant`. - // - // Additionally, the following notes apply: - // - // - Initialisation and ownership management are not protected behind `onlyProxy` intentionally. - // This ensures that the contract can safely be disposed of after it is no longer used. - // - Carefully consider what data recovery options are presented as new functionality is added. - // Care must be taken to ensure that a migration plan can exist for cases where upgrades - // cannot recover from an issue or vulnerability. - - /////////////////////////////////////////////////////////////////////////////// - /// !!!!! DATA: DO NOT REORDER !!!!! /// - /////////////////////////////////////////////////////////////////////////////// - - // To ensure compatibility between upgrades, it is exceedingly important that no reordering of - // these variables takes place. If reordering happens, a storage clash will occur (effectively a - // memory safety error). - - using ByteHasher for bytes; - - /////////////////////////////////////////////////////////////////////////////// - /// ERRORS /// - ////////////////////////////////////////////////////////////////////////////// - - /// @notice Thrown when attempting to reuse a nullifier - error InvalidNullifier(); - - /////////////////////////////////////////////////////////////////////////////// - /// Events /// - ////////////////////////////////////////////////////////////////////////////// - - /// @notice Emitted once for each successful PBH verification. - /// - /// @param sender The sender of this particular transaction or UserOp. - /// @param nonce Transaction/UserOp nonce. - /// @param payload The zero-knowledge proof that demonstrates the claimer is registered with World ID. - event PBH(address indexed sender, uint256 indexed nonce, PBHPayload payload); - - /// @notice Emitted when the World ID address is set. - /// - /// @param worldId The World ID instance that will be used for verifying proofs. - event WorldIdSet(address indexed worldId); - - /// @notice Emitted when the number of PBH transactions allowed per month is set. - /// - /// @param numPbhPerMonth The number of allowed PBH transactions per month. - event NumPbhPerMonthSet(uint8 indexed numPbhPerMonth); - - /////////////////////////////////////////////////////////////////////////////// - /// Vars /// - ////////////////////////////////////////////////////////////////////////////// - - /// @dev The World ID group ID (always 1) - uint256 internal constant _GROUP_ID = 1; - - /// @dev The World ID instance that will be used for verifying proofs - IWorldIDGroups public worldId; - - /// @dev The EntryPoint where Aggregated PBH Bundles will be proxied to. - IEntryPoint public entryPoint; - - /// @notice The number of PBH transactions that may be used by a single - /// World ID in a given month. - uint8 public numPbhPerMonth; - - /////////////////////////////////////////////////////////////////////////////// - /// Mappings /// - ////////////////////////////////////////////////////////////////////////////// - - /// @dev Whether a nullifier hash has been used already. Used to guarantee an action is only performed once by a single person - mapping(uint256 => bool) public nullifierHashes; - - /////////////////////////////////////////////////////////////////////////////// - /// Functions /// - ////////////////////////////////////////////////////////////////////////////// - - /// @param sender The sender of this particular transaction or UserOp. - /// @param nonce Transaction/UserOp nonce. - /// @param callData Transaction/UserOp call data. - /// @param pbhPayload The PBH payload containing the proof data. - function verifyPbh(address sender, uint256 nonce, bytes calldata callData, PBHPayload memory pbhPayload) - public - virtual - onlyInitialized - onlyProxy - { - // First, we make sure this nullifier has not been used before. - if (nullifierHashes[pbhPayload.nullifierHash]) revert InvalidNullifier(); - - // Verify the external nullifier - PBHExternalNullifier.verify(pbhPayload.pbhExternalNullifier, numPbhPerMonth); - - // If worldId address is set, proceed with on chain verification, - // otherwise assume verification has been done off chain by the builder. - if (address(worldId) != address(0)) { - // We now generate the signal hash from the sender, nonce, and calldata - uint256 signalHash = abi.encodePacked(sender, nonce, callData).hashToField(); - - // We now verify the provided proof is valid and the user is verified by World ID - worldId.verifyProof( - pbhPayload.root, - _GROUP_ID, - signalHash, - pbhPayload.nullifierHash, - pbhPayload.pbhExternalNullifier, - pbhPayload.proof - ); - } - - // We now record the user has done this, so they can't do it again (proof of uniqueness) - nullifierHashes[pbhPayload.nullifierHash] = true; - - emit PBH(sender, nonce, pbhPayload); - } - - /// @notice Sets the number of PBH transactions allowed per month. - /// @param _numPbhPerMonth The number of allowed PBH transactions per month. - function setNumPbhPerMonth(uint8 _numPbhPerMonth) external virtual onlyOwner onlyProxy onlyInitialized { - numPbhPerMonth = _numPbhPerMonth; - emit NumPbhPerMonthSet(_numPbhPerMonth); - } - - /// @dev If the World ID address is set to 0, then it is assumed that verification will take place off chain. - /// @notice Sets the World ID instance that will be used for verifying proofs. - /// @param _worldId The World ID instance that will be used for verifying proofs. - function setWorldId(address _worldId) external virtual onlyOwner onlyProxy onlyInitialized { - worldId = IWorldIDGroups(_worldId); - emit WorldIdSet(_worldId); - } -} diff --git a/contracts/src/helpers/ByteHasher.sol b/contracts/src/helpers/ByteHasher.sol index c655a1d9..960d282d 100644 --- a/contracts/src/helpers/ByteHasher.sol +++ b/contracts/src/helpers/ByteHasher.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.20; +pragma solidity ^0.8.28; library ByteHasher { /// @dev Creates a keccak256 hash of a bytestring. diff --git a/contracts/src/helpers/PBHExternalNullifier.sol b/contracts/src/helpers/PBHExternalNullifier.sol index 45999099..8eeb86df 100644 --- a/contracts/src/helpers/PBHExternalNullifier.sol +++ b/contracts/src/helpers/PBHExternalNullifier.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.20; +pragma solidity ^0.8.28; import "@BokkyPooBahsDateTimeLibrary/BokkyPooBahsDateTimeLibrary.sol"; diff --git a/contracts/src/interfaces/IPBHEntryPoint.sol b/contracts/src/interfaces/IPBHEntryPoint.sol index ca8a05e5..f5fb8890 100644 --- a/contracts/src/interfaces/IPBHEntryPoint.sol +++ b/contracts/src/interfaces/IPBHEntryPoint.sol @@ -1,15 +1,36 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.20; +pragma solidity ^0.8.28; import {IWorldIDGroups} from "@world-id-contracts/interfaces/IWorldIDGroups.sol"; import {IEntryPoint} from "@account-abstraction/contracts/interfaces/IEntryPoint.sol"; -import {IPBHVerifier} from "./IPBHVerifier.sol"; -interface IPBHEntryPoint is IPBHVerifier { +interface IPBHEntryPoint { + /// @notice The Packed World ID Proof data. + /// @param root The root of the Merkle tree. + /// @param pbhExternalNullifier The external nullifier for the PBH User Operation. + /// @param nullifierHash The nullifier hash for the PBH User Operation. + /// @param proof The Semaphore proof. + struct PBHPayload { + uint256 root; + uint256 pbhExternalNullifier; + uint256 nullifierHash; + uint256[8] proof; + } + function handleAggregatedOps( IEntryPoint.UserOpsPerAggregator[] calldata opsPerAggregator, address payable beneficiary ) external; + function initialize(IWorldIDGroups worldId, IEntryPoint entryPoint, uint8 _numPbhPerMonth) external; + function validateSignaturesCallback(bytes32 hashedOps) external view; + + function nullifierHashes(uint256) external view returns (bool); + + function verifyPbh(address sender, uint256 nonce, bytes memory callData, PBHPayload memory pbhPayload) external; + + function setNumPbhPerMonth(uint8 _numPbhPerMonth) external; + + function setWorldId(address _worldId) external; } diff --git a/contracts/src/interfaces/IPBHVerifier.sol b/contracts/src/interfaces/IPBHVerifier.sol deleted file mode 100644 index 11b8551f..00000000 --- a/contracts/src/interfaces/IPBHVerifier.sol +++ /dev/null @@ -1,24 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity ^0.8.20; - -import {IWorldIDGroups} from "@world-id-contracts/interfaces/IWorldIDGroups.sol"; -import {IEntryPoint} from "@account-abstraction/contracts/interfaces/IEntryPoint.sol"; - -interface IPBHVerifier { - /// @notice The Packed World ID Proof data. - /// @param root The root of the Merkle tree. - /// @param pbhExternalNullifier The external nullifier for the PBH User Operation. - /// @param nullifierHash The nullifier hash for the PBH User Operation. - /// @param proof The Semaphore proof. - struct PBHPayload { - uint256 root; - uint256 pbhExternalNullifier; - uint256 nullifierHash; - uint256[8] proof; - } - - function nullifierHashes(uint256) external view returns (bool); - function verifyPbh(address sender, uint256 nonce, bytes memory callData, PBHPayload memory pbhPayload) external; - function setNumPbhPerMonth(uint8 _numPbhPerMonth) external; - function setWorldId(address _worldId) external; -} diff --git a/contracts/test/Mock4337Module.sol b/contracts/test/Mock4337Module.sol new file mode 100644 index 00000000..28b28d43 --- /dev/null +++ b/contracts/test/Mock4337Module.sol @@ -0,0 +1,24 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.20; + +import {PBHSafe4337Module} from "../src/PBH4337Module.sol"; +import {Safe4337Module} from "@4337/Safe4337Module.sol"; +import {PackedUserOperation} from "@account-abstraction/contracts/interfaces/PackedUserOperation.sol"; +import {ValidationData} from "@account-abstraction/contracts/core/Helpers.sol"; +import {_packValidationData} from "@account-abstraction/contracts/core/Helpers.sol"; +import {ISafe} from "@4337/interfaces/Safe.sol"; +import "forge-std/console.sol"; + +contract Mock4337Module is PBHSafe4337Module { + constructor(address entryPoint, address _pbhSignatureAggregator, uint192 _pbhNonceKey) + PBHSafe4337Module(entryPoint, _pbhSignatureAggregator, _pbhNonceKey) + {} + + function validateSignaturesExternal(PackedUserOperation calldata userOp) + external + view + returns (uint256 validationData) + { + return _validateSignatures(userOp); + } +} diff --git a/contracts/test/PBH4337Module.t.sol b/contracts/test/PBH4337Module.t.sol new file mode 100644 index 00000000..31a56ef9 --- /dev/null +++ b/contracts/test/PBH4337Module.t.sol @@ -0,0 +1,180 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.20; + +import {Vm} from "forge-std/Vm.sol"; +import {Test} from "forge-std/Test.sol"; +import "forge-std/console.sol"; +import {PackedUserOperation} from "@account-abstraction/contracts/interfaces/PackedUserOperation.sol"; + +import {Safe} from "@safe-global/safe-contracts/contracts/Safe.sol"; +import {SafeProxyFactory} from "@safe-global/safe-contracts/contracts/proxies/SafeProxyFactory.sol"; +import {SafeProxy} from "@safe-global/safe-contracts/contracts/proxies/SafeProxy.sol"; +import {Enum} from "@safe-global/safe-contracts/contracts/common/Enum.sol"; +import {SafeModuleSetup} from "@4337/SafeModuleSetup.sol"; +import {PBHSafe4337Module} from "../src/PBH4337Module.sol"; +import {Mock4337Module} from "./Mock4337Module.sol"; +import {Safe4337Module} from "@4337/Safe4337Module.sol"; + +contract PBHSafe4337ModuleTest is Test { + Mock4337Module public module; + Safe public singleton; + Safe public safe; + SafeProxyFactory public factory; + SafeModuleSetup public moduleSetup; + + address public owner; + uint256 public ownerKey; + + address public constant PBH_SIGNATURE_AGGREGATOR = address(0x123); + uint192 public constant PBH_NONCE_KEY = 1123123123; + + function setUp() public { + // Create single EOA owner + ownerKey = 0x1; + owner = vm.addr(ownerKey); + + module = new Mock4337Module(owner, PBH_SIGNATURE_AGGREGATOR, PBH_NONCE_KEY); + + // Deploy SafeModuleSetup + moduleSetup = new SafeModuleSetup(); + + // Deploy Safe singleton and factory + singleton = new Safe(); + factory = new SafeProxyFactory(); + + // Prepare module initialization + address[] memory modules = new address[](1); + modules[0] = address(module); + + // Encode the moduleSetup.enableModules call + bytes memory moduleSetupCall = abi.encodeCall(SafeModuleSetup.enableModules, (modules)); + + // Create owners array with single owner + address[] memory owners = new address[](1); + owners[0] = owner; + + // Encode initialization data for proxy + bytes memory initData = abi.encodeCall( + Safe.setup, + ( + owners, + 1, // threshold + address(moduleSetup), // to + moduleSetupCall, // data + address(0), // fallbackHandler + address(0), // paymentToken + 0, // payment + payable(address(0)) // paymentReceiver + ) + ); + + // Deploy and initialize Safe proxy + SafeProxy proxy = factory.createProxyWithNonce( + address(singleton), + initData, + 0 // salt nonce + ); + + // Cast proxy to Safe for easier interaction + safe = Safe(payable(address(proxy))); + } + + function testValidSignature() public { + bytes memory signatureBefore = abi.encodePacked(uint48(0), uint48(0)); + + PackedUserOperation memory userOp = PackedUserOperation({ + sender: address(safe), + nonce: uint256(PBH_NONCE_KEY) << 64, // Keep the nonce key format + initCode: "", // Empty for already deployed safe + callData: "", + accountGasLimits: bytes32( + abi.encode( // Pack verification and call gas limits + uint128(100000), // verification gas limit + uint128(300000) // call gas limit + ) + ), + preVerificationGas: 21000, // Base cost + gasFees: bytes32( + abi.encode( // Pack max priority fee and max fee + uint128(1 gwei), // maxPriorityFeePerGas + uint128(100 gwei) // maxFeePerGas + ) + ), + paymasterAndData: "", // No paymaster + signature: signatureBefore + }); + + bytes32 operationHash = module.getOperationHash(userOp); + + (uint8 v, bytes32 r, bytes32 s) = vm.sign(ownerKey, operationHash); + + bytes memory signature = abi.encodePacked( + uint48(0), + uint48(0), + r, + s, + v // The raw signature components + ); + userOp.signature = signature; + + uint256 validationData = module.validateSignaturesExternal(userOp); + + // // Extract validation components + address authorizer = address(uint160(validationData)); + uint48 validUntil = uint48(validationData >> 160); + uint48 validAfter = uint48(validationData >> 208); + + // Verify signature was valid (authorizer should be address(0) for valid non-PBH signature) + assertEq(authorizer, PBH_SIGNATURE_AGGREGATOR, "PBH Aggregator address not returned"); + assertEq(validUntil, 0, "ValidUntil should be 0"); + assertEq(validAfter, 0, "ValidAfter should be 0"); + } + + function testInvalidSignature() public { + bytes memory signatureBefore = abi.encodePacked(uint48(0), uint48(0)); + + PackedUserOperation memory userOp = PackedUserOperation({ + sender: address(safe), + nonce: uint256(PBH_NONCE_KEY) << 64, // Keep the nonce key format + initCode: "", // Empty for already deployed safe + callData: "", + accountGasLimits: bytes32( + abi.encode( // Pack verification and call gas limits + uint128(100000), // verification gas limit + uint128(300000) // call gas limit + ) + ), + preVerificationGas: 21000, // Base cost + gasFees: bytes32( + abi.encode( // Pack max priority fee and max fee + uint128(1 gwei), // maxPriorityFeePerGas + uint128(100 gwei) // maxFeePerGas + ) + ), + paymasterAndData: "", // No paymaster + signature: signatureBefore + }); + + bytes32 operationHash = module.getOperationHash(userOp); + + // Create an invalid signature + (uint8 v, bytes32 r, bytes32 s) = vm.sign(ownerKey, keccak256("invalid")); + + bytes memory signature = abi.encodePacked( + uint48(0), + uint48(0), + r, + s, + v // The raw signature components + ); + userOp.signature = signature; + + uint256 validationData = module.validateSignaturesExternal(userOp); + + // // Extract validation components + address authorizer = address(uint160(validationData)); + + // Verify signature was invalid (authorizer should be 1 for invalid signature) + assertEq(authorizer, address(1), "PBH Aggregator address not returned"); + } +} diff --git a/contracts/test/PBHEntryPointOwnershipManagement.t.sol b/contracts/test/PBHEntryPointOwnershipManagement.t.sol index 10301ad5..183225c6 100644 --- a/contracts/test/PBHEntryPointOwnershipManagement.t.sol +++ b/contracts/test/PBHEntryPointOwnershipManagement.t.sol @@ -10,8 +10,6 @@ import {OwnableUpgradeable} from "@openzeppelin/contracts-upgradeable/access/Own import {UUPSUpgradeable} from "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol"; import {WorldIDImpl} from "@world-id-contracts/abstract/WorldIDImpl.sol"; -import {PBHVerifier} from "../src/PBHVerifier.sol"; - /// @title World ID PBHEntryPoint Routing Tests /// @notice Contains tests for the WorldID pbhVerifier /// @author Worldcoin diff --git a/contracts/test/PBHSignatureAggregator.t.sol b/contracts/test/PBHSignatureAggregator.t.sol index d155caee..03099185 100644 --- a/contracts/test/PBHSignatureAggregator.t.sol +++ b/contracts/test/PBHSignatureAggregator.t.sol @@ -2,10 +2,10 @@ pragma solidity ^0.8.21; import {Setup} from "./Setup.sol"; -import {IPBHVerifier} from "../src/interfaces/IPBHVerifier.sol"; import {console} from "@forge-std/console.sol"; import {TestUtils} from "./TestUtils.sol"; import {IEntryPoint} from "@account-abstraction/contracts/interfaces/IEntryPoint.sol"; +import {IPBHEntryPoint} from "../src/interfaces/IPBHEntryPoint.sol"; import "@account-abstraction/contracts/interfaces/PackedUserOperation.sol"; import "@BokkyPooBahsDateTimeLibrary/BokkyPooBahsDateTimeLibrary.sol"; import "../src/helpers/PBHExternalNullifier.sol"; @@ -22,14 +22,14 @@ contract PBHSignatureAggregatorTest is TestUtils, Setup { uint256 encoded = PBHExternalNullifier.encode(PBHExternalNullifier.V1, 0, month, year); worldIDGroups.setVerifyProofSuccess(true); - IPBHVerifier.PBHPayload memory proof0 = IPBHVerifier.PBHPayload({ + IPBHEntryPoint.PBHPayload memory proof0 = IPBHEntryPoint.PBHPayload({ root: 1, pbhExternalNullifier: encoded, nullifierHash: 0, proof: [uint256(0), 0, 0, 0, 0, 0, 0, 0] }); - IPBHVerifier.PBHPayload memory proof1 = IPBHVerifier.PBHPayload({ + IPBHEntryPoint.PBHPayload memory proof1 = IPBHEntryPoint.PBHPayload({ root: 2, pbhExternalNullifier: encoded, nullifierHash: 1, @@ -54,7 +54,7 @@ contract PBHSignatureAggregatorTest is TestUtils, Setup { } function testAggregateSignatures(uint256 root, uint256 pbhExternalNullifier, uint256 nullifierHash) public { - IPBHVerifier.PBHPayload memory proof = IPBHVerifier.PBHPayload({ + IPBHEntryPoint.PBHPayload memory proof = IPBHEntryPoint.PBHPayload({ root: root, pbhExternalNullifier: pbhExternalNullifier, nullifierHash: nullifierHash, @@ -66,7 +66,8 @@ contract PBHSignatureAggregatorTest is TestUtils, Setup { proofs[1] = abi.encode(proof); PackedUserOperation[] memory uoTestFixture = createUOTestData(address(safe), proofs); bytes memory aggregatedSignature = pbhAggregator.aggregateSignatures(uoTestFixture); - IPBHVerifier.PBHPayload[] memory decodedProofs = abi.decode(aggregatedSignature, (IPBHVerifier.PBHPayload[])); + IPBHEntryPoint.PBHPayload[] memory decodedProofs = + abi.decode(aggregatedSignature, (IPBHEntryPoint.PBHPayload[])); assertEq(decodedProofs.length, 2, "Decoded proof length should be 1"); assertEq(decodedProofs[0].root, proof.root, "Root should match"); assertEq( diff --git a/contracts/test/PBHVerifier.t.sol b/contracts/test/PBHVerifier.t.sol index 8a2901bb..e8388f05 100644 --- a/contracts/test/PBHVerifier.t.sol +++ b/contracts/test/PBHVerifier.t.sol @@ -6,10 +6,11 @@ import {MockWorldIDGroups} from "./mocks/MockWorldIDGroups.sol"; import {CheckInitialized} from "@world-id-contracts/utils/CheckInitialized.sol"; import {WorldIDImpl} from "@world-id-contracts/abstract/WorldIDImpl.sol"; import {ByteHasher} from "@helpers/ByteHasher.sol"; +import {IPBHEntryPoint} from "../src/interfaces/IPBHEntryPoint.sol"; +import {PBHEntryPointImplV1} from "../src/PBHEntryPointImplV1.sol"; + import "@BokkyPooBahsDateTimeLibrary/BokkyPooBahsDateTimeLibrary.sol"; import "@helpers/PBHExternalNullifier.sol"; -import {PBHVerifier} from "../src/PBHVerifier.sol"; -import {IPBHVerifier} from "../src/interfaces/IPBHVerifier.sol"; import {Setup} from "./Setup.sol"; /// @title PBHVerifer Verify Tests @@ -18,12 +19,12 @@ import {Setup} from "./Setup.sol"; contract PBHVerifierTest is Setup { using ByteHasher for bytes; - event PBH(address indexed sender, uint256 indexed nonce, IPBHVerifier.PBHPayload payload); + event PBH(address indexed sender, uint256 indexed nonce, IPBHEntryPoint.PBHPayload payload); event NumPbhPerMonthSet(uint8 indexed numPbhPerMonth); event WorldIdSet(address indexed worldId); /// @notice Test payload for the PBHVerifier - IPBHVerifier.PBHPayload public testPayload = IPBHVerifier.PBHPayload({ + IPBHEntryPoint.PBHPayload public testPayload = IPBHEntryPoint.PBHPayload({ root: 1, pbhExternalNullifier: getValidPBHExternalNullifier(), nullifierHash: 1, @@ -58,7 +59,7 @@ contract PBHVerifierTest is Setup { assertTrue(used, "Nullifier hash should be marked as used"); // Now try to use the same nullifier hash again - vm.expectRevert(PBHVerifier.InvalidNullifier.selector); + vm.expectRevert(PBHEntryPointImplV1.InvalidNullifier.selector); pbhEntryPoint.verifyPbh(sender, nonce, testCallData, testPayload); }