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(v2): add SignatureType handling to LightAccount #45

Merged
merged 1 commit into from
Mar 17, 2024
Merged

feat(v2): add SignatureType handling to LightAccount #45

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
55 changes: 44 additions & 11 deletions src/LightAccount.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,17 +124,37 @@ contract LightAccount is BaseLightAccount, CustomSlotInitializable {
override
returns (uint256 validationData)
{
address owner_ = owner();
bytes32 signedHash = userOpHash.toEthSignedMessageHash();
bytes memory signature = userOp.signature;
(address recovered, ECDSA.RecoverError error,) = signedHash.tryRecover(signature);
if (
(error == ECDSA.RecoverError.NoError && recovered == owner_)
|| SignatureChecker.isValidERC1271SignatureNow(owner_, userOpHash, signature)
) {
return 0;
uint8 signatureType = uint8(userOp.signature[0]);
if (signatureType == uint8(SignatureType.EOA)) {
// EOA signature
bytes32 signedHash = userOpHash.toEthSignedMessageHash();
bytes memory signature = userOp.signature[1:];
return _successToValidationData(_isValidEOAOwnerSignature(signedHash, signature));
} else if (signatureType == uint8(SignatureType.CONTRACT)) {
// Contract signature without address
bytes memory signature = userOp.signature[1:];
return _successToValidationData(_isValidContractOwnerSignatureNow(userOpHash, signature));
}
return SIG_VALIDATION_FAILED;

revert InvalidSignatureType();
}

/// @notice Check if the signature is a valid by the EOA owner for the given digest.
/// @dev Only supports 65-byte signatures, and uses the digest directly.
/// @param digest The digest to be checked.
/// @param signature The signature to be checked.
/// @return True if the signature is valid and by the owner, false otherwise.
function _isValidEOAOwnerSignature(bytes32 digest, bytes memory signature) internal view returns (bool) {
(address recovered, ECDSA.RecoverError error,) = digest.tryRecover(signature);
return error == ECDSA.RecoverError.NoError && recovered == owner();
}

/// @notice Check if the signature is a valid ERC-1271 signature by a contract owner for the given digest.
/// @param digest The digest to be checked.
/// @param signature The signature to be checked.
/// @return True if the signature is valid and by an owner, false otherwise.
function _isValidContractOwnerSignatureNow(bytes32 digest, bytes memory signature) internal view returns (bool) {
return SignatureChecker.isValidERC1271SignatureNow(owner(), digest, signature);
}

/// @dev The signature is valid if it is signed by the owner's private key (if the owner is an EOA) or if it is a
Expand All @@ -146,7 +166,20 @@ contract LightAccount is BaseLightAccount, CustomSlotInitializable {
override
returns (bool)
{
return SignatureChecker.isValidSignatureNow(owner(), derivedHash, trimmedSignature);
if (trimmedSignature.length < 1) {
return false;
}
uint8 signatureType = uint8(trimmedSignature[0]);
if (signatureType == uint8(SignatureType.EOA)) {
// EOA signature
bytes memory signature = trimmedSignature[1:];
return _isValidEOAOwnerSignature(derivedHash, signature);
} else if (signatureType == uint8(SignatureType.CONTRACT)) {
// Contract signature without address
bytes memory signature = trimmedSignature[1:];
return _isValidContractOwnerSignatureNow(derivedHash, signature);
}
return false;
}

function _domainNameAndVersion()
Expand Down
33 changes: 7 additions & 26 deletions src/MultiOwnerLightAccount.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import {MessageHashUtils} from "@openzeppelin/contracts/utils/cryptography/Messa
import {SignatureChecker} from "@openzeppelin/contracts/utils/cryptography/SignatureChecker.sol";
import {IEntryPoint} from "account-abstraction/interfaces/IEntryPoint.sol";
import {PackedUserOperation} from "account-abstraction/interfaces/PackedUserOperation.sol";
import {SIG_VALIDATION_FAILED, SIG_VALIDATION_SUCCESS} from "account-abstraction/core/Helpers.sol";
import {CastLib} from "modular-account/helpers/CastLib.sol";
import {SetValue} from "modular-account/libraries/Constants.sol";
import {LinkedListSet, LinkedListSetLib} from "modular-account/libraries/LinkedListSetLib.sol";
Expand All @@ -30,13 +29,6 @@ contract MultiOwnerLightAccount is BaseLightAccount, CustomSlotInitializable {
bytes32 internal constant _INITIALIZABLE_STORAGE_POSITION =
0xaa296a366a62f6551d3ddfceae892d1791068a359a0d3461aab99dfc6c5fd700;

// Signature types used for user operation validation and ERC-1271 signature validation.
enum SignatureTypes {
EOA,
CONTRACT,
CONTRACT_WITH_ADDR
}

struct LightAccountStorage {
LinkedListSet owners;
}
Expand All @@ -61,9 +53,6 @@ contract MultiOwnerLightAccount is BaseLightAccount, CustomSlotInitializable {
/// @dev The owner to be removed does not exist.
error OwnerDoesNotExist(address owner);

/// @dev The signature type provided is not valid.
error InvalidSignatureType();

constructor(IEntryPoint entryPoint_) CustomSlotInitializable(_INITIALIZABLE_STORAGE_POSITION) {
_ENTRY_POINT = entryPoint_;
_disableInitializers();
Expand Down Expand Up @@ -149,16 +138,16 @@ contract MultiOwnerLightAccount is BaseLightAccount, CustomSlotInitializable {
returns (uint256 validationData)
{
uint8 signatureType = uint8(userOp.signature[0]);
if (signatureType == uint8(SignatureTypes.EOA)) {
if (signatureType == uint8(SignatureType.EOA)) {
// EOA signature
bytes32 signedHash = userOpHash.toEthSignedMessageHash();
bytes memory signature = userOp.signature[1:];
return _successToValidationData(_isValidEOAOwnerSignature(signedHash, signature));
} else if (signatureType == uint8(SignatureTypes.CONTRACT)) {
} else if (signatureType == uint8(SignatureType.CONTRACT)) {
// Contract signature without address
bytes memory signature = userOp.signature[1:];
return _successToValidationData(_isValidContractOwnerSignatureNowLoop(userOpHash, signature));
} else if (signatureType == uint8(SignatureTypes.CONTRACT_WITH_ADDR)) {
} else if (signatureType == uint8(SignatureType.CONTRACT_WITH_ADDR)) {
// Contract signature with address
address contractOwner = address(bytes20(userOp.signature[1:21]));
bytes memory signature = userOp.signature[21:];
Expand Down Expand Up @@ -217,14 +206,6 @@ contract MultiOwnerLightAccount is BaseLightAccount, CustomSlotInitializable {
return false;
}

/// @dev Convert a boolean success value to a validation data value.
/// @param success The success value to be converted.
/// @return validationData The validation data value. 0 if success is true, 1 (SIG_VALIDATION_FAILED) if
/// success is false.
function _successToValidationData(bool success) internal pure returns (uint256 validationData) {
return success ? SIG_VALIDATION_SUCCESS : SIG_VALIDATION_FAILED;
}

/// @dev The signature is valid if it is signed by the owner's private key (if the owner is an EOA) or if it is a
/// valid ERC-1271 signature from the owner (if the owner is a contract).
function _isValidSignature(bytes32 derivedHash, bytes calldata trimmedSignature)
Expand All @@ -238,15 +219,15 @@ contract MultiOwnerLightAccount is BaseLightAccount, CustomSlotInitializable {
return false;
}
uint8 signatureType = uint8(trimmedSignature[0]);
if (signatureType == uint8(SignatureTypes.EOA)) {
// EOA signature;
if (signatureType == uint8(SignatureType.EOA)) {
// EOA signature
bytes memory signature = trimmedSignature[1:];
return _isValidEOAOwnerSignature(derivedHash, signature);
} else if (signatureType == uint8(SignatureTypes.CONTRACT)) {
} else if (signatureType == uint8(SignatureType.CONTRACT)) {
// Contract signature without address
bytes memory signature = trimmedSignature[1:];
return _isValidContractOwnerSignatureNowLoop(derivedHash, signature);
} else if (signatureType == uint8(SignatureTypes.CONTRACT_WITH_ADDR)) {
} else if (signatureType == uint8(SignatureType.CONTRACT_WITH_ADDR)) {
// Contract signature with address
address contractOwner = address(bytes20(trimmedSignature[1:21]));
bytes memory signature = trimmedSignature[21:];
Expand Down
19 changes: 19 additions & 0 deletions src/common/BaseLightAccount.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ pragma solidity ^0.8.23;

import {BaseAccount} from "account-abstraction/core/BaseAccount.sol";
import {SIG_VALIDATION_FAILED} from "account-abstraction/core/Helpers.sol";
import {SIG_VALIDATION_FAILED, SIG_VALIDATION_SUCCESS} from "account-abstraction/core/Helpers.sol";
import {IEntryPoint} from "account-abstraction/interfaces/IEntryPoint.sol";
import {PackedUserOperation} from "account-abstraction/interfaces/PackedUserOperation.sol";
import {TokenCallbackHandler} from "account-abstraction/samples/callback/TokenCallbackHandler.sol";
Expand All @@ -13,9 +14,19 @@ import {ERC1271} from "./ERC1271.sol";
abstract contract BaseLightAccount is BaseAccount, TokenCallbackHandler, UUPSUpgradeable, ERC1271 {
IEntryPoint internal immutable _ENTRY_POINT;

/// @notice Signature types used for user operation validation and ERC-1271 signature validation.
enum SignatureType {
EOA,
CONTRACT,
CONTRACT_WITH_ADDR
}

/// @dev The length of the array does not match the expected length.
error ArrayLengthMismatch();

/// @dev The signature type provided is not valid.
error InvalidSignatureType();

/// @dev The caller is not authorized.
error NotAuthorized(address caller);

Expand Down Expand Up @@ -105,6 +116,14 @@ abstract contract BaseLightAccount is BaseAccount, TokenCallbackHandler, UUPSUpg
}
}

/// @dev Convert a boolean success value to a validation data value.
/// @param success The success value to be converted.
/// @return validationData The validation data value. 0 if success is true, 1 (SIG_VALIDATION_FAILED) if
/// success is false.
function _successToValidationData(bool success) internal pure returns (uint256 validationData) {
return success ? SIG_VALIDATION_SUCCESS : SIG_VALIDATION_FAILED;
}

function _call(address target, uint256 value, bytes memory data) internal {
(bool success, bytes memory result) = target.call{value: value}(data);
if (!success) {
Expand Down
58 changes: 48 additions & 10 deletions test/LightAccount.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,8 @@ contract LightAccountTest is Test {
PackedUserOperation memory op = _getUnsignedOp(
abi.encodeCall(BaseLightAccount.execute, (address(lightSwitch), 0, abi.encodeCall(LightSwitch.turnOn, ())))
);
op.signature = contractOwner.sign(entryPoint.getUserOpHash(op));
op.signature =
abi.encodePacked(BaseLightAccount.SignatureType.CONTRACT, contractOwner.sign(entryPoint.getUserOpHash(op)));
PackedUserOperation[] memory ops = new PackedUserOperation[](1);
ops[0] = op;
entryPoint.handleOps(ops, BENEFICIARY);
Expand All @@ -92,6 +93,26 @@ contract LightAccountTest is Test {
entryPoint.handleOps(ops, BENEFICIARY);
}

function testFuzz_rejectsUserOpsWithInvalidSignatureType(uint8 signatureType) public {
signatureType = uint8(bound(signatureType, 2, type(uint8).max));

PackedUserOperation memory op = _getUnsignedOp(
abi.encodeCall(BaseLightAccount.execute, (address(lightSwitch), 0, abi.encodeCall(LightSwitch.turnOn, ())))
);
op.signature = abi.encodePacked(signatureType);
PackedUserOperation[] memory ops = new PackedUserOperation[](1);
ops[0] = op;
vm.expectRevert(
abi.encodeWithSelector(
IEntryPoint.FailedOpWithRevert.selector,
0,
"AA23 reverted",
abi.encodePacked(BaseLightAccount.InvalidSignatureType.selector)
)
);
entryPoint.handleOps(ops, BENEFICIARY);
}

function testExecuteCannotBeCalledByRandos() public {
vm.expectRevert(abi.encodeWithSelector(BaseLightAccount.NotAuthorized.selector, (address(this))));
account.execute(address(lightSwitch), 0, abi.encodeCall(LightSwitch.turnOn, ()));
Expand Down Expand Up @@ -283,7 +304,11 @@ contract LightAccountTest is Test {
function testIsValidSignatureForEoaOwner() public {
bytes32 child = keccak256(abi.encode(_CHILD_TYPEHASH, "hello world"));
bytes memory signature = abi.encodePacked(
_sign(EOA_PRIVATE_KEY, _toERC1271Hash(child)), _PARENT_TYPEHASH, _domainSeparatorB(), child
BaseLightAccount.SignatureType.EOA,
_sign(EOA_PRIVATE_KEY, _toERC1271Hash(child)),
_PARENT_TYPEHASH,
_domainSeparatorB(),
child
);
assertEq(
account.isValidSignature(_toChildHash(child), signature),
Expand All @@ -294,8 +319,13 @@ contract LightAccountTest is Test {
function testIsValidSignatureForContractOwner() public {
_useContractOwner();
bytes32 child = keccak256(abi.encode(_CHILD_TYPEHASH, "hello world"));
bytes memory signature =
abi.encodePacked(contractOwner.sign(_toERC1271Hash(child)), _PARENT_TYPEHASH, _domainSeparatorB(), child);
bytes memory signature = abi.encodePacked(
BaseLightAccount.SignatureType.CONTRACT,
contractOwner.sign(_toERC1271Hash(child)),
_PARENT_TYPEHASH,
_domainSeparatorB(),
child
);
assertEq(
account.isValidSignature(_toChildHash(child), signature),
bytes4(keccak256("isValidSignature(bytes32,bytes)"))
Expand All @@ -320,8 +350,11 @@ contract LightAccountTest is Test {
string memory message = "hello world";
bytes32 childHash =
keccak256(abi.encodePacked("\x19Ethereum Signed Message:\n", bytes(message).length, message));
bytes memory signature =
abi.encodePacked(_sign(EOA_PRIVATE_KEY, _toERC1271HashPersonalSign(childHash)), _PARENT_TYPEHASH);
bytes memory signature = abi.encodePacked(
BaseLightAccount.SignatureType.EOA,
_sign(EOA_PRIVATE_KEY, _toERC1271HashPersonalSign(childHash)),
_PARENT_TYPEHASH
);
assertEq(account.isValidSignature(childHash, signature), bytes4(keccak256("isValidSignature(bytes32,bytes)")));
}

Expand All @@ -330,8 +363,11 @@ contract LightAccountTest is Test {
string memory message = "hello world";
bytes32 childHash =
keccak256(abi.encodePacked("\x19Ethereum Signed Message:\n", bytes(message).length, message));
bytes memory signature =
abi.encodePacked(contractOwner.sign(_toERC1271HashPersonalSign(childHash)), _PARENT_TYPEHASH);
bytes memory signature = abi.encodePacked(
BaseLightAccount.SignatureType.CONTRACT,
contractOwner.sign(_toERC1271HashPersonalSign(childHash)),
_PARENT_TYPEHASH
);
assertEq(account.isValidSignature(childHash, signature), bytes4(keccak256("isValidSignature(bytes32,bytes)")));
}

Expand Down Expand Up @@ -451,7 +487,7 @@ contract LightAccountTest is Test {
bytes32(uint256(uint160(0x0000000071727De22E5E9d8BAf0edAc6f37da032)))
)
),
0x7ea0ecf5a976ba72e9bde105c40012e55ee74088186c29da658ec325a8f586c4
0x6aa8e89cd8a2df2df153f6959aace173a48e4ecedde04f7dc09add425d88a7e2
);
}

Expand Down Expand Up @@ -484,7 +520,9 @@ contract LightAccountTest is Test {
returns (PackedUserOperation memory)
{
PackedUserOperation memory op = _getUnsignedOp(callData);
op.signature = _sign(privateKey, entryPoint.getUserOpHash(op).toEthSignedMessageHash());
op.signature = abi.encodePacked(
BaseLightAccount.SignatureType.EOA, _sign(privateKey, entryPoint.getUserOpHash(op).toEthSignedMessageHash())
);
return op;
}

Expand Down
Loading
Loading