Skip to content

Commit

Permalink
refactor revert message
Browse files Browse the repository at this point in the history
  • Loading branch information
charlesjhongc committed Sep 13, 2023
1 parent 8bcb604 commit 472d85b
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 18 deletions.
22 changes: 7 additions & 15 deletions contracts/utils/SignatureValidator.sol
Original file line number Diff line number Diff line change
Expand Up @@ -34,27 +34,19 @@ function validateSignature(
bytes32 _hash,
bytes memory _sig
) view returns (bool isValid) {
require(_sig.length > 0, "SignatureValidator: length greater than 0 required");
require(_signerAddress != address(0), "SignatureValidator: invalid signer");
require(_sig.length > 0, "length greater than 0 required");
require(_signerAddress != address(0), "invalid signer");

// Pop last byte off of signature byte array.
uint8 signatureTypeRaw = uint8(LibBytes.popLastByte(_sig));

// Ensure signature is supported
require(signatureTypeRaw <= uint8(SignatureType.ZX1271), "SignatureValidator: unsupported signature");
require(signatureTypeRaw <= uint8(SignatureType.ZX1271), "unsupported signature type");

// Extract signature type
SignatureType signatureType = SignatureType(signatureTypeRaw);

if (signatureType == SignatureType.Illegal) {
// Always illegal signature.
// This is always an implicit option since a signer can create a
// signature array with invalid type or length. We may as well make
// it an explicit option. This aids testing and analysis. It is
// also the initialization value for the enum type.

revert("SignatureValidator#isValidSignature: illegal signature");
} else if (signatureType == SignatureType.EIP712) {
if (signatureType == SignatureType.EIP712) {
// To be backward compatible with previous signature format which has an extra 32 bytes padded in the end, here we just extract the (r,s,v) from the signature and ignore the rest.

bytes32 r = LibBytes.readBytes32(_sig, 0);
Expand All @@ -76,10 +68,10 @@ function validateSignature(
return ZX1271_MAGICVALUE == IERC1271Wallet(_signerAddress).isValidSignature(_hash, _sig);
}

// Anything else is illegal (We do not return false because
// Anything else is incorrect (We do not return false because
// the signature may actually be valid, just not in a format
// that we currently support. In this case returning false
// that we currently handled. In this case returning false
// may lead the caller to incorrectly believe that the
// signature was invalid.)
revert("SignatureValidator: unsupported signature");
revert("incorrect signature type");
}
20 changes: 17 additions & 3 deletions test/forkMainnet/SignatureValidator/General.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -8,21 +8,35 @@ import { validateSignature, SignatureType } from "contracts/utils/SignatureValid
contract TestGeneral is TestSignatureValidator {
function testEmptySignature() public {
bytes memory signature;
vm.expectRevert("SignatureValidator: length greater than 0 required");
vm.expectRevert("length greater than 0 required");
validateSignature(vm.addr(userPrivateKey), digest, signature);
}

function testZeroAddressSigner() public {
(uint8 v, bytes32 r, bytes32 s) = vm.sign(userPrivateKey, digest);
bytes memory signature = abi.encodePacked(r, s, v, uint8(SignatureType.EIP712));
vm.expectRevert("SignatureValidator: invalid signer");
vm.expectRevert("invalid signer");
validateSignature(address(0), digest, signature);
}

function testIllegalSignatureType() public {
(uint8 v, bytes32 r, bytes32 s) = vm.sign(userPrivateKey, digest);
bytes memory signature = abi.encodePacked(r, s, v, uint8(SignatureType.Illegal));
vm.expectRevert("incorrect signature type");
validateSignature(vm.addr(userPrivateKey), digest, signature);
}

function testInvalidSignatureType() public {
(uint8 v, bytes32 r, bytes32 s) = vm.sign(userPrivateKey, digest);
bytes memory signature = abi.encodePacked(r, s, v, uint8(SignatureType.Invalid));
vm.expectRevert("incorrect signature type");
validateSignature(vm.addr(userPrivateKey), digest, signature);
}

function testSignatureTypeOutOfEnumRange() public {
(uint8 v, bytes32 r, bytes32 s) = vm.sign(userPrivateKey, digest);
bytes memory signature = abi.encodePacked(r, s, v, uint8(8));
vm.expectRevert("SignatureValidator: unsupported signature");
vm.expectRevert("unsupported signature type");
validateSignature(vm.addr(userPrivateKey), digest, signature);
}
}

0 comments on commit 472d85b

Please sign in to comment.