Skip to content

Commit

Permalink
chore: review fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
howydev committed Oct 15, 2024
1 parent 19fb1cf commit 04e0743
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 40 deletions.
5 changes: 3 additions & 2 deletions src/account/SemiModularAccountBase.sol
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,10 @@ import {ModularAccountBase} from "./ModularAccountBase.sol";

import {ERC7739ReplaySafeWrapper} from "../helpers/ERC7739ReplaySafeWrapper.sol";

abstract contract SemiModularAccountBase is ModularAccountBase, ERC7739ReplaySafeWrapper {
abstract contract SemiModularAccountBase is ModularAccountBase {
using MessageHashUtils for bytes32;
using ModuleEntityLib for ModuleEntity;
using ERC7739ReplaySafeWrapper for address;

struct SemiModularAccountStorage {
address fallbackSigner;
Expand Down Expand Up @@ -123,7 +124,7 @@ abstract contract SemiModularAccountBase is ModularAccountBase, ERC7739ReplaySaf
address fallbackSigner = _getFallbackSigner();

(bytes32 digest, bytes calldata innerSignature) =
_validateERC7739SigFormatForAccount(address(this), hash, signature);
address(this).validateERC7739SigFormatForAccount(hash, signature);
if (_checkSignature(fallbackSigner, digest, innerSignature)) {
return _1271_MAGIC_VALUE;
}
Expand Down
55 changes: 25 additions & 30 deletions src/helpers/ERC7739ReplaySafeWrapper.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ pragma solidity ^0.8.26;

// A contract mixin ERC7739-compliant nested EIP-712 wrappers over EIP-1271 digests.
// This allows for efficient, readable ERC-1271 signature schemes for smart contract accounts.
abstract contract ERC7739ReplaySafeWrapper {
library ERC7739ReplaySafeWrapper {
type TypeStruct is bytes32;

/// @dev `keccak256("PersonalSign(bytes prefixed)")`.
bytes32 internal constant _PERSONAL_SIGN_TYPEHASH =
0x983e65e5148e570cd828ead231ee759a8d7958721a768f93bc4483ba005c32de;
Expand All @@ -26,14 +28,14 @@ abstract contract ERC7739ReplaySafeWrapper {
0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470;

// Used in the account
function _validateERC7739SigFormatForAccount(address account, bytes32 hash, bytes calldata signature)
function validateERC7739SigFormatForAccount(address account, bytes32 hash, bytes calldata signature)
internal
view
returns (bytes32, bytes calldata)
{
bytes32 t;
(t, hash, signature) = _validateERC7739SigFormat(_typedDataSignFieldsForAccount(account), hash, signature);
if (t == bytes32(0)) hash = _hashTypedDataAccount(account, hash); // `PersonalSign` workflow.
TypeStruct t;
(t, hash, signature) = _validateERC7739SigFormat(typedDataSignFieldsForAccount(account), hash, signature);
if (isZero(t)) hash = hashTypedDataForAccount(account, hash); // `PersonalSign` workflow.
return (hash, signature);
}

Expand All @@ -42,37 +44,32 @@ abstract contract ERC7739ReplaySafeWrapper {
/// github/erc7579/erc7739Validator/blob/f8cbd4b58a7226cce18e9b8bc380da51174daf53/src/ERC7739Validator.sol#L283
/// which is based on
/// github/Vectorized/solady/blob/351548a824d57c1c0fec688fdfe3a44a8e17efc3/src/accounts/ERC1271.sol#L253
function _typedDataSignFieldsForAccount(address account) internal view virtual returns (bytes32 m) {
function typedDataSignFieldsForAccount(address account) internal view returns (TypeStruct m) {
bytes1 fields = bytes1(hex"0C"); // 001100
// !string memory name;
// !string memory version;
// uint256 chainId = chainId;
// address verifyingContract = address(this);
// bytes32 salt = bytes32(uint160(account));
// !bytes32 salt;
// !uint256[] memory extensions;
assembly ("memory-safe") {
m := mload(0x40) // Grab the free memory pointer.
mstore(0x40, add(m, 0x120)) // Allocate the memory.
// Skip 2 words for the `typedDataSignTypehash` and `contents` struct hash.
mstore(add(m, 0x40), shl(248, fields)) // TODO: maybe parse fields and only use the necessary ones
mstore(add(m, 0x40), shl(248, fields))
mstore(add(m, 0x60), _HASH_EMPTY_BYTES)
mstore(add(m, 0x80), _HASH_EMPTY_BYTES)
mstore(add(m, 0xa0), chainid())
mstore(add(m, 0xc0), account)
// mstore(add(m, 0xe0), address(0))
// mstore(add(m, 0xe0), 0)
mstore(add(m, 0x100), _HASH_EMPTY_BYTES)
}
}

/// @notice Helper function to build the full personal sign hash
/// @dev From
/// github/erc7579/erc7739Validator/blob/f8cbd4b58a7226cce18e9b8bc380da51174daf53/src/ERC7739Validator.sol#L312
function _hashTypedDataAccount(address account, bytes32 structHash)
internal
view
virtual
returns (bytes32 digest)
{
function hashTypedDataForAccount(address account, bytes32 structHash) internal view returns (bytes32 digest) {
// !string memory name;
// !string memory version;
// uint256 chainId = chainId;
Expand Down Expand Up @@ -102,16 +99,16 @@ abstract contract ERC7739ReplaySafeWrapper {

// Used in the module
// Validates the ERC7739 signature format, for use in a module
function _validateERC7739SigFormatForModule(
function validateERC7739SigFormatForModule(
address account,
address module,
bytes32 hash,
bytes calldata signature
) internal view returns (bytes32, bytes calldata) {
bytes32 t;
TypeStruct t;
(t, hash, signature) =
_validateERC7739SigFormat(_typedDataSignFieldsForModule(account, module), hash, signature);
if (t == bytes32(0)) hash = _hashTypedDataModule(account, module, hash); // `PersonalSign` workflow.
_validateERC7739SigFormat(typedDataSignFieldsForModule(account, module), hash, signature);
if (isZero(t)) hash = hashTypedDataForModule(account, module, hash); // `PersonalSign` workflow.
return (hash, signature);
}

Expand All @@ -120,12 +117,7 @@ abstract contract ERC7739ReplaySafeWrapper {
/// github/erc7579/erc7739Validator/blob/f8cbd4b58a7226cce18e9b8bc380da51174daf53/src/ERC7739Validator.sol#L283
/// which is based on
/// github/Vectorized/solady/blob/351548a824d57c1c0fec688fdfe3a44a8e17efc3/src/accounts/ERC1271.sol#L253
function _typedDataSignFieldsForModule(address account, address module)
internal
view
virtual
returns (bytes32 m)
{
function typedDataSignFieldsForModule(address account, address module) internal view returns (TypeStruct m) {
bytes1 fields = bytes1(hex"0E"); // 001110
// !string memory name;
// !string memory version;
Expand All @@ -151,17 +143,16 @@ abstract contract ERC7739ReplaySafeWrapper {
/// @notice Helper function to build the full personal sign hash
/// @dev From
/// github/erc7579/erc7739Validator/blob/f8cbd4b58a7226cce18e9b8bc380da51174daf53/src/ERC7739Validator.sol#L312
function _hashTypedDataModule(address account, address module, bytes32 structHash)
function hashTypedDataForModule(address account, address module, bytes32 structHash)
internal
view
virtual
returns (bytes32 digest)
{
// !string memory name;
// !string memory version;
// uint256 chainId = chainId;
// address verifyingContract = address(this);
// bytes32 salt = address(account);
// bytes32 salt = account;
// !uint256[] memory extensions;
/// @solidity memory-safe-assembly
assembly {
Expand All @@ -185,6 +176,10 @@ abstract contract ERC7739ReplaySafeWrapper {
}
}

function isZero(TypeStruct t) internal pure returns (bool) {
return TypeStruct.unwrap(t) == bytes32(0);
}

/// @notice Helper function to validate ERC7739 compatible nested EIP712 structs to guard against signature
/// replay
/// @dev Parses out the inner signature from the full signature and returns it
Expand All @@ -198,10 +193,10 @@ abstract contract ERC7739ReplaySafeWrapper {
/// @param hash The hash to wrap.
/// @return bytes32 Replay-safe hash, computed by wrapping the input hash in an EIP-712 struct.
/// @return bytes Inner signature to use for verification.
function _validateERC7739SigFormat(bytes32 t, bytes32 hash, bytes calldata signature)
function _validateERC7739SigFormat(TypeStruct t, bytes32 hash, bytes calldata signature)
private
pure
returns (bytes32, bytes32, bytes calldata)
returns (TypeStruct, bytes32, bytes calldata)
{
/// @solidity memory-safe-assembly
assembly {
Expand Down
5 changes: 3 additions & 2 deletions src/modules/validation/ECDSAValidationModule.sol
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,9 @@ import {BaseModule} from "../BaseModule.sol";
/// - This validation supports ERC-1271. The signature is valid if it is signed by the owner's private key.
/// - This validation supports composition that other validation can relay on entities in this validation
/// to validate partially or fully.
contract ECDSAValidationModule is IValidationModule, ERC7739ReplaySafeWrapper, BaseModule {
contract ECDSAValidationModule is IValidationModule, BaseModule {
using MessageHashUtils for bytes32;
using ERC7739ReplaySafeWrapper for address;

uint256 internal constant _SIG_VALIDATION_PASSED = 0;
uint256 internal constant _SIG_VALIDATION_FAILED = 1;
Expand Down Expand Up @@ -111,7 +112,7 @@ contract ECDSAValidationModule is IValidationModule, ERC7739ReplaySafeWrapper, B
override
returns (bytes4)
{
(digest, signature) = _validateERC7739SigFormatForModule(account, address(this), digest, signature);
(digest, signature) = account.validateERC7739SigFormatForModule(address(this), digest, signature);
if (_checkSig(signers[entityId][account], digest, signature)) {
return _1271_MAGIC_VALUE;
}
Expand Down
5 changes: 3 additions & 2 deletions src/modules/validation/WebauthnValidationModule.sol
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,10 @@ import {BaseModule} from "../BaseModule.sol";
/// - This validation supports ERC-1271. The signature is valid if it is signed by the owner's private key.
/// - This validation supports composition that other validation can relay on entities in this validation
/// to validate partially or fully.
contract WebauthnValidationModule is IValidationModule, ERC7739ReplaySafeWrapper, BaseModule {
contract WebauthnValidationModule is IValidationModule, BaseModule {
using MessageHashUtils for bytes32;
using WebAuthn for WebAuthn.WebAuthnAuth;
using ERC7739ReplaySafeWrapper for address;

struct PubKey {
uint256 x;
Expand Down Expand Up @@ -107,7 +108,7 @@ contract WebauthnValidationModule is IValidationModule, ERC7739ReplaySafeWrapper
override
returns (bytes4)
{
(digest, signature) = _validateERC7739SigFormatForModule(account, address(this), digest, signature);
(digest, signature) = account.validateERC7739SigFormatForModule(address(this), digest, signature);
if (_validateSignature(entityId, account, digest, signature)) {
return _1271_MAGIC_VALUE;
}
Expand Down
11 changes: 7 additions & 4 deletions test/utils/ModuleSignatureUtils.sol
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,9 @@ import {ERC7739ReplaySafeWrapper} from "../../src/helpers/ERC7739ReplaySafeWrapp

/// @dev Utilities for encoding signatures for modular account validation. Used for encoding user op, runtime, and
/// 1271 signatures.
contract ModuleSignatureUtils is ERC7739ReplaySafeWrapper {
contract ModuleSignatureUtils {
using ModuleEntityLib for ModuleEntity;
using ERC7739ReplaySafeWrapper for address;

struct PreValidationHookData {
uint8 index;
Expand Down Expand Up @@ -56,15 +57,15 @@ contract ModuleSignatureUtils is ERC7739ReplaySafeWrapper {
bytes32 mockAppDigest,
bytes calldata sig
) external view returns (bytes32 digest) {
(digest,) = _validateERC7739SigFormatForModule(account, module, mockAppDigest, sig);
(digest,) = account.validateERC7739SigFormatForModule(module, mockAppDigest, sig);
}

function generate1271DigestForAccount(address account, bytes32 mockAppDigest, bytes calldata sig)
external
view
returns (bytes32 digest)
{
(digest,) = _validateERC7739SigFormatForAccount(account, mockAppDigest, sig);
(digest,) = account.validateERC7739SigFormatForAccount(mockAppDigest, sig);
}

function _encode1271Signature(ModuleEntity validationFunction, bytes memory validationData, bytes32 structHash)
Expand Down Expand Up @@ -317,6 +318,8 @@ contract ModuleSignatureUtils is ERC7739ReplaySafeWrapper {
// EIP-712 helpers

function _computeDomainSeparator(address account) internal view returns (bytes32) {
return keccak256(abi.encode(_DOMAIN_SEPARATOR_TYPEHASH_ACCOUNT, block.chainid, account));
return keccak256(
abi.encode(ERC7739ReplaySafeWrapper._DOMAIN_SEPARATOR_TYPEHASH_ACCOUNT, block.chainid, account)
);
}
}

0 comments on commit 04e0743

Please sign in to comment.