diff options
| -rw-r--r-- | packages/contracts/src/2.0.0/protocol/Exchange/MixinSignatureValidator.sol | 32 | ||||
| -rw-r--r-- | packages/contracts/test/exchange/core.ts | 7 | ||||
| -rw-r--r-- | packages/contracts/test/exchange/signature_validator.ts | 25 | ||||
| -rw-r--r-- | packages/types/CHANGELOG.json | 9 | ||||
| -rw-r--r-- | packages/types/src/index.ts | 2 |
5 files changed, 53 insertions, 22 deletions
diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/MixinSignatureValidator.sol b/packages/contracts/src/2.0.0/protocol/Exchange/MixinSignatureValidator.sol index 6bfbb5107..017da742e 100644 --- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinSignatureValidator.sol +++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinSignatureValidator.sol @@ -293,8 +293,20 @@ contract MixinSignatureValidator is cdStart, // write input over output 32 // output size is 32 bytes ) - // Signature is valid if call did not revert and returned true - isValid := and(success, mload(cdStart)) + + switch success + case 0 { + // Revert with `Error("WALLET_ERROR")` + mstore(0, 0x08c379a000000000000000000000000000000000000000000000000000000000) + mstore(32, 0x0000002000000000000000000000000000000000000000000000000000000000) + mstore(64, 0x0000000c57414c4c45545f4552524f5200000000000000000000000000000000) + mstore(96, 0) + revert(0, 100) + } + case 1 { + // Signature is valid if call did not revert and returned true + isValid := mload(cdStart) + } } return isValid; } @@ -331,8 +343,20 @@ contract MixinSignatureValidator is cdStart, // write input over output 32 // output size is 32 bytes ) - // Signature is valid if call did not revert and returned true - isValid := and(success, mload(cdStart)) + + switch success + case 0 { + // Revert with `Error("VALIDATOR_ERROR")` + mstore(0, 0x08c379a000000000000000000000000000000000000000000000000000000000) + mstore(32, 0x0000002000000000000000000000000000000000000000000000000000000000) + mstore(64, 0x0000000f56414c494441544f525f4552524f5200000000000000000000000000) + mstore(96, 0) + revert(0, 100) + } + case 1 { + // Signature is valid if call did not revert and returned true + isValid := mload(cdStart) + } } return isValid; } diff --git a/packages/contracts/test/exchange/core.ts b/packages/contracts/test/exchange/core.ts index fdfb40155..f9d8b7783 100644 --- a/packages/contracts/test/exchange/core.ts +++ b/packages/contracts/test/exchange/core.ts @@ -195,7 +195,7 @@ describe('Exchange core', () => { signedOrder.signature = `0x0${SignatureType.Wallet}`; await expectTransactionFailedAsync( exchangeWrapper.fillOrderAsync(signedOrder, takerAddress), - RevertReason.InvalidOrderSignature, + RevertReason.WalletError, ); }); @@ -209,13 +209,10 @@ describe('Exchange core', () => { ), constants.AWAIT_TRANSACTION_MINED_MS, ); - signedOrder = await orderFactory.newSignedOrderAsync({ - makerAddress: maliciousValidator.address, - }); signedOrder.signature = `${maliciousValidator.address}0${SignatureType.Validator}`; await expectTransactionFailedAsync( exchangeWrapper.fillOrderAsync(signedOrder, takerAddress), - RevertReason.InvalidOrderSignature, + RevertReason.ValidatorError, ); }); }); diff --git a/packages/contracts/test/exchange/signature_validator.ts b/packages/contracts/test/exchange/signature_validator.ts index 947ebffcc..5b64d853c 100644 --- a/packages/contracts/test/exchange/signature_validator.ts +++ b/packages/contracts/test/exchange/signature_validator.ts @@ -352,7 +352,7 @@ describe('MixinSignatureValidator', () => { expect(isValidSignature).to.be.false(); }); - it('should return false when `isValidSignature` attempts to update state and not allow state to be updated when SignatureType=Wallet', async () => { + it('should revert when `isValidSignature` attempts to update state and SignatureType=Wallet', async () => { // Create EIP712 signature const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); const orderHashBuffer = ethUtil.toBuffer(orderHashHex); @@ -365,13 +365,14 @@ describe('MixinSignatureValidator', () => { ethUtil.toBuffer(`0x${SignatureType.Wallet}`), ]); const signatureHex = ethUtil.bufferToHex(signature); - // Validate signature - const isValid = await signatureValidator.publicIsValidSignature.callAsync( - orderHashHex, - maliciousWallet.address, - signatureHex, + await expectContractCallFailed( + signatureValidator.publicIsValidSignature.callAsync( + orderHashHex, + maliciousWallet.address, + signatureHex, + ), + RevertReason.WalletError, ); - expect(isValid).to.be.equal(false); }); it('should return true when SignatureType=Validator, signature is valid and validator is approved', async () => { @@ -404,18 +405,16 @@ describe('MixinSignatureValidator', () => { expect(isValidSignature).to.be.false(); }); - it('should return false when `isValidSignature` attempts to update state and not allow state to be updated when SignatureType=Validator', async () => { + it('should revert when `isValidSignature` attempts to update state and SignatureType=Validator', async () => { const validatorAddress = ethUtil.toBuffer(`${maliciousValidator.address}`); const signatureType = ethUtil.toBuffer(`0x${SignatureType.Validator}`); const signature = Buffer.concat([validatorAddress, signatureType]); const signatureHex = ethUtil.bufferToHex(signature); const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); - const isValid = await signatureValidator.publicIsValidSignature.callAsync( - orderHashHex, - maliciousValidator.address, - signatureHex, + await expectContractCallFailed( + signatureValidator.publicIsValidSignature.callAsync(orderHashHex, signerAddress, signatureHex), + RevertReason.ValidatorError, ); - expect(isValid).to.be.equal(false); }); it('should return false when SignatureType=Validator, signature is valid and validator is not approved', async () => { // Set approval of signature validator to false diff --git a/packages/types/CHANGELOG.json b/packages/types/CHANGELOG.json index fabc80ecf..980a12ad3 100644 --- a/packages/types/CHANGELOG.json +++ b/packages/types/CHANGELOG.json @@ -1,5 +1,14 @@ [ { + "version": "1.0.1-rc.6", + "changes": [ + { + "note": "Add WalletError and ValidatorError revert reasons", + "pr": 1012 + } + ] + }, + { "version": "1.0.1-rc.5", "changes": [ { diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index 298fa77d4..2831d816d 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -220,6 +220,8 @@ export enum RevertReason { Erc721InvalidSpender = 'ERC721_INVALID_SPENDER', Erc721ZeroOwner = 'ERC721_ZERO_OWNER', Erc721InvalidSelector = 'ERC721_INVALID_SELECTOR', + WalletError = 'WALLET_ERROR', + ValidatorError = 'VALIDATOR_ERROR', } export enum StatusCodes { |