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 enum support to MultiOwnerLightAccount 1271 validation #42

Merged
merged 1 commit into from
Mar 16, 2024
Merged

feat(v2): Add enum support to MultiOwnerLightAccount 1271 validation #42

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
39 changes: 22 additions & 17 deletions src/MultiOwnerLightAccount.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,21 +31,10 @@ contract MultiOwnerLightAccount is BaseLightAccount, CustomSlotInitializable {
0xaa296a366a62f6551d3ddfceae892d1791068a359a0d3461aab99dfc6c5fd700;

// Signature types used for user operation validation and ERC-1271 signature validation.
// The enum value encodes the following bit layout:
// | is contract signature? | 0b_______1
// | has address provided? | 0b______1_
// | is transparent EIP-712? | 0b_____1__
// | empty | 0b00000___
enum SignatureTypes {
EOA, // 0
CONTRACT, // 1
empty_1, // skip 2 to align bitmap
CONTRACT_WITH_ADDR, // 3
TRANSPARENT_EOA, // 4
TRANSPARENT_CONTRACT, // 5
empty_2, // skip 6 to align bitmap
TRANSPARENT_CONTRACT_WITH_ADDR // 7

EOA,
CONTRACT,
CONTRACT_WITH_ADDR
}

struct LightAccountStorage {
Expand Down Expand Up @@ -245,9 +234,25 @@ contract MultiOwnerLightAccount is BaseLightAccount, CustomSlotInitializable {
override
returns (bool)
{
(address recovered, ECDSA.RecoverError error,) = derivedHash.tryRecover(trimmedSignature);
return (error == ECDSA.RecoverError.NoError && _getStorage().owners.contains(recovered.toSetValue()))
|| _isValidContractOwnerSignatureNowLoop(derivedHash, trimmedSignature);
if (trimmedSignature.length < 1) {
return false;
}
uint8 signatureType = uint8(trimmedSignature[0]);
if (signatureType == uint8(SignatureTypes.EOA)) {
// EOA signature;
bytes memory signature = trimmedSignature[1:];
return _isValidEOAOwnerSignature(derivedHash, signature);
} else if (signatureType == uint8(SignatureTypes.CONTRACT)) {
// Contract signature without address
bytes memory signature = trimmedSignature[1:];
return _isValidContractOwnerSignatureNowLoop(derivedHash, signature);
} else if (signatureType == uint8(SignatureTypes.CONTRACT_WITH_ADDR)) {
// Contract signature with address
address contractOwner = address(bytes20(trimmedSignature[1:21]));
bytes memory signature = trimmedSignature[21:];
return _isValidContractOwnerSignatureNowSingle(contractOwner, derivedHash, signature);
}
return false;
}

function _domainNameAndVersion()
Expand Down
128 changes: 107 additions & 21 deletions test/MultiOwnerLightAccount.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,10 +145,7 @@ contract MultiOwnerLightAccountTest is Test {
}

function testFuzz_rejectsUserOpsWithInvalidSignatureType(uint8 signatureType) public {
// Valid values are 0,1,3
if (signatureType != 2) {
signatureType = uint8(bound(signatureType, 4, type(uint8).max));
}
signatureType = uint8(bound(signatureType, 3, type(uint8).max));

PackedUserOperation memory op = _getUnsignedOp(
abi.encodeCall(BaseLightAccount.execute, (address(lightSwitch), 0, abi.encodeCall(LightSwitch.turnOn, ())))
Expand Down Expand Up @@ -410,74 +407,163 @@ contract MultiOwnerLightAccountTest 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
MultiOwnerLightAccount.SignatureTypes.EOA,
_sign(EOA_PRIVATE_KEY, _toERC1271Hash(child)),
_PARENT_TYPEHASH,
_domainSeparatorB(),
child
);
assertEq(
account.isValidSignature(_toChildHash(child), signature),
bytes4(keccak256("isValidSignature(bytes32,bytes)"))
);
}

function testIsValidSignatureForContractOwner() public {
function testIsValidSignatureForContractOwnerUnspecified() 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(
MultiOwnerLightAccount.SignatureTypes.CONTRACT,
contractOwner.sign(_toERC1271Hash(child)),
_PARENT_TYPEHASH,
_domainSeparatorB(),
child
);
assertEq(
account.isValidSignature(_toChildHash(child), signature),
bytes4(keccak256("isValidSignature(bytes32,bytes)"))
);
}

function testIsValidSignatureRejectsInvalid() public {
function testIsValidSignatureForContractOwnerSpecified() public {
_useContractOwner();
bytes32 child = keccak256(abi.encode(_CHILD_TYPEHASH, "hello world"));
bytes memory signature =
abi.encodePacked(_sign(123, _toERC1271Hash(child)), _PARENT_TYPEHASH, _domainSeparatorB(), child);
bytes memory signature = abi.encodePacked(
MultiOwnerLightAccount.SignatureTypes.CONTRACT_WITH_ADDR,
contractOwner,
contractOwner.sign(_toERC1271Hash(child)),
_PARENT_TYPEHASH,
_domainSeparatorB(),
child
);
assertEq(
account.isValidSignature(_toChildHash(child), signature),
bytes4(keccak256("isValidSignature(bytes32,bytes)"))
);
}

function testIsValidSignatureRejectsInvalidEOA() public {
bytes32 child = keccak256(abi.encode(_CHILD_TYPEHASH, "hello world"));
bytes memory signature = abi.encodePacked(
MultiOwnerLightAccount.SignatureTypes.EOA,
_sign(123, _toERC1271Hash(child)),
_PARENT_TYPEHASH,
_domainSeparatorB(),
child
);
assertEq(account.isValidSignature(_toChildHash(child), signature), bytes4(0xffffffff));

signature = abi.encodePacked(
_sign(EOA_PRIVATE_KEY, _toERC1271Hash(child)), _PARENT_TYPEHASH, _domainSeparatorA(), child
MultiOwnerLightAccount.SignatureTypes.EOA,
_sign(EOA_PRIVATE_KEY, _toERC1271Hash(child)),
_PARENT_TYPEHASH,
_domainSeparatorA(),
child
);
assertEq(account.isValidSignature(_toChildHash(child), signature), bytes4(0xffffffff));

assertEq(account.isValidSignature(_toChildHash(child), ""), bytes4(0xffffffff));
assertEq(
account.isValidSignature(_toChildHash(child), abi.encodePacked(MultiOwnerLightAccount.SignatureTypes.EOA)),
bytes4(0xffffffff)
);
}

function testIsValidSignatureRejectsInvalidContractOwner() public {
// Signature should fail, because the contract owner is not an owner
bytes32 child = keccak256(abi.encode(_CHILD_TYPEHASH, "hello world"));
bytes memory signature = abi.encodePacked(
MultiOwnerLightAccount.SignatureTypes.CONTRACT,
contractOwner.sign(_toERC1271Hash(child)),
_PARENT_TYPEHASH,
_domainSeparatorB(),
child
);
assertEq(account.isValidSignature(_toChildHash(child), signature), bytes4(0xffffffff));
}

function testFuzz_IsValidSignatureRejectsInvalidSignatureType(uint8 signatureType) public {
signatureType = uint8(bound(signatureType, 3, type(uint8).max));

bytes32 child = keccak256(abi.encode(_CHILD_TYPEHASH, "hello world"));
bytes memory signature = abi.encodePacked(
signatureType, _sign(EOA_PRIVATE_KEY, _toERC1271Hash(child)), _PARENT_TYPEHASH, _domainSeparatorB(), child
);
assertEq(account.isValidSignature(_toChildHash(child), signature), bytes4(0xffffffff));
}

function testIsValidSignaturePersonalSign() public {
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(
MultiOwnerLightAccount.SignatureTypes.EOA,
_sign(EOA_PRIVATE_KEY, _toERC1271HashPersonalSign(childHash)),
_PARENT_TYPEHASH
);
assertEq(account.isValidSignature(childHash, signature), bytes4(keccak256("isValidSignature(bytes32,bytes)")));
}

function testIsValidSignaturePersonalSignForContractOwnerUnspecified() public {
_useContractOwner();
string memory message = "hello world";
bytes32 childHash =
keccak256(abi.encodePacked("\x19Ethereum Signed Message:\n", bytes(message).length, message));
bytes memory signature = abi.encodePacked(
MultiOwnerLightAccount.SignatureTypes.CONTRACT,
contractOwner.sign(_toERC1271HashPersonalSign(childHash)),
_PARENT_TYPEHASH
);
assertEq(account.isValidSignature(childHash, signature), bytes4(keccak256("isValidSignature(bytes32,bytes)")));
}

function testIsValidSignaturePersonalSignForContractOwner() public {
function testIsValidSignaturePersonalSignForContractOwnerSpecified() public {
_useContractOwner();
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(
MultiOwnerLightAccount.SignatureTypes.CONTRACT_WITH_ADDR,
contractOwner,
contractOwner.sign(_toERC1271HashPersonalSign(childHash)),
_PARENT_TYPEHASH
);
assertEq(account.isValidSignature(childHash, signature), bytes4(keccak256("isValidSignature(bytes32,bytes)")));
}

function testIsValidSignaturePersonalSignRejectsInvalid() public {
string memory message = "hello world";
bytes32 childHash =
keccak256(abi.encodePacked("\x19Ethereum Signed Message:\n", bytes(message).length, message));
bytes memory signature = abi.encodePacked(_sign(123, _toERC1271HashPersonalSign(childHash)), _PARENT_TYPEHASH);
bytes memory signature = abi.encodePacked(
MultiOwnerLightAccount.SignatureTypes.EOA,
_sign(123, _toERC1271HashPersonalSign(childHash)),
_PARENT_TYPEHASH
);
assertEq(account.isValidSignature(childHash, signature), bytes4(0xffffffff));

signature = abi.encodePacked(
MultiOwnerLightAccount.SignatureTypes.EOA,
_sign(EOA_PRIVATE_KEY, _toERC1271HashPersonalSign(childHash)),
_PARENT_TYPEHASH,
_domainSeparatorB(),
childHash
);
assertEq(account.isValidSignature(childHash, signature), bytes4(0xffffffff));

assertEq(account.isValidSignature(childHash, ""), bytes4(0xffffffff));
assertEq(
account.isValidSignature(childHash, abi.encodePacked(MultiOwnerLightAccount.SignatureTypes.EOA)),
bytes4(0xffffffff)
);
}

function testOwnerCanUpgrade() public {
Expand Down Expand Up @@ -579,7 +665,7 @@ contract MultiOwnerLightAccountTest is Test {
bytes32(uint256(uint160(0x0000000071727De22E5E9d8BAf0edAc6f37da032)))
)
),
0x0a14a7d2e0fb6858a618de8e4a7cf52bd1cb8dad3f4adbf243078a4db5f13c92
0xead9408e740524880066371b7b1590829216c843719ab5a102236d1bb9d80ea1
);
}

Expand Down
Loading