Skip to content

Commit

Permalink
feat(v2): Add enum support to MultiOwnerLightAccount 1271 validation (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
adam-alchemy authored Mar 16, 2024
1 parent 816168b commit d414af9
Show file tree
Hide file tree
Showing 2 changed files with 129 additions and 38 deletions.
39 changes: 22 additions & 17 deletions src/MultiOwnerLightAccount.sol
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
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

0 comments on commit d414af9

Please sign in to comment.