crowetic/protocol
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

@0x/contracts-exchange: Update Wallet signature type behavior to match v2.1.

Browse Source 
`@0x/contracts-exchange`: Add EOA tests to `signature_validator`.
This commit is contained in:
Lawrence Forman
2019-08-07 15:48:30 -04:00
parent 6752fc9fe5
commit 3dd8dac146
5 changed files with 171 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
contracts/exchange/CHANGELOG.json
View File

@@ -141,6 +141,14 @@
{
"note": "Add `wrapper_unit_tests` tests and `TestWrapperFunctions` contract",
"pr": "TODO"
},
{
"note": "Disallow `signerAddress == 0` in signature validation functions.",
"pr": "TODO"
},
{
"note": "Update `Wallet` signature type behavior to be in line with v2.1.",
"pr": "TODO"
}
]
},

33
contracts/exchange/contracts/src/MixinSignatureValidator.sol
View File

@@ -43,6 +43,10 @@ contract MixinSignatureValidator is
{
using LibBytes for bytes;
// Magic bytes to be returned by `Wallet` signature type validators.
// bytes4(keccak256("isValidWalletSignature(bytes32,address,bytes)"))
bytes4 private constant LEGACY_WALLET_MAGIC_VALUE = 0xb0671381;
// Mapping of hash => signer => signed
mapping (bytes32 => mapping (address => bool)) public preSigned;
@@ -94,6 +98,8 @@ contract MixinSignatureValidator is
view
returns (bool isValid)
{
_assertValidSigner(signerAddress, hash, signature);
SignatureType signatureType = _readValidSignatureType(
hash,
signerAddress,
@@ -210,6 +216,8 @@ contract MixinSignatureValidator is
view
returns (bool isValid)
{
_assertValidSigner(signerAddress, orderHash, signature);
SignatureType signatureType = _readValidSignatureType(
orderHash,
signerAddress,
@@ -259,6 +267,8 @@ contract MixinSignatureValidator is
view
returns (bool isValid)
{
_assertValidSigner(signerAddress, transactionHash, signature);
SignatureType signatureType = _readValidSignatureType(
transactionHash,
signerAddress,
@@ -467,9 +477,9 @@ contract MixinSignatureValidator is
}
// Static call the verification function.
(bool didSucceed, bytes memory returnData) = walletAddress.staticcall(callData);
// Return data should be a single bool.
if (didSucceed && returnData.length == 32) {
return returnData.readUint256(0) == 1;
// Return data should be `LEGACY_WALLET_MAGIC_VALUE`.
if (didSucceed && returnData.length <= 32) {
return returnData.readBytes4(0) == LEGACY_WALLET_MAGIC_VALUE;
}
// Static call to verifier failed.
LibRichErrors._rrevert(LibExchangeRichErrors.SignatureWalletError(
@@ -599,4 +609,21 @@ contract MixinSignatureValidator is
));
}
function _assertValidSigner(
address signerAddress,
bytes32 hash,
bytes memory signature
)
private
pure
{
if (signerAddress == address(0)) {
LibRichErrors._rrevert(LibExchangeRichErrors.SignatureError(
IExchangeRichErrors.SignatureErrorCodes.INVALID_SIGNER,
hash,
signerAddress,
signature
));
}
}
}

3
contracts/exchange/contracts/src/interfaces/IExchangeRichErrors.sol
View File

@@ -27,7 +27,8 @@ contract IExchangeRichErrors {
INVALID_LENGTH,
UNSUPPORTED,
ILLEGAL,
INAPPROPRIATE_SIGNATURE_TYPE
INAPPROPRIATE_SIGNATURE_TYPE,
INVALID_SIGNER
}
enum TransactionErrorCodes {

12
contracts/exchange/contracts/test/TestValidatorWallet.sol
View File

@@ -45,6 +45,8 @@ contract TestValidatorWallet is
{
using LibBytes for bytes;
bytes4 private constant LEGACY_WALLET_MAGIC_VALUE = 0xb0671381;
/// @dev Revert reason for `Revert` `ValidatorAction`.
string constant public REVERT_REASON = "you shall not pass";
@@ -158,19 +160,19 @@ contract TestValidatorWallet is
/// @dev Validates a hash with the `Wallet` signature type.
/// @param hash Message hash that is signed.
/// @param signature Proof of signing.
/// @return isValid `true` if the signature check succeeds.
/// @return `LEGACY_WALLET_MAGIC_VALUE` if the signature check succeeds.
function isValidSignature(
bytes32 hash,
bytes memory signature
)
public
returns (bool isValid)
returns (bytes4 magicValue)
{
ValidatorAction action = _hashActions[hash];
if (action == ValidatorAction.Reject) {
isValid = false;
magicValue = 0x0;
} else if (action == ValidatorAction.Accept) {
isValid = true;
magicValue = LEGACY_WALLET_MAGIC_VALUE;
} else if (action == ValidatorAction.Revert) {
revert(REVERT_REASON);
} else if (action == ValidatorAction.UpdateState) {
@@ -179,7 +181,7 @@ contract TestValidatorWallet is
assert(action == ValidatorAction.MatchSignatureHash);
bytes32 expectedSignatureHash = _hashSignatureHashes[hash];
if (keccak256(signature) == expectedSignatureHash) {
isValid = true;
magicValue = LEGACY_WALLET_MAGIC_VALUE;
}
}
}

128
contracts/exchange/test/signature_validator.ts
View File

@@ -18,7 +18,7 @@ import {
transactionHashUtils,
} from '@0x/order-utils';
import { SignatureType, SignedOrder, SignedZeroExTransaction } from '@0x/types';
import { BigNumber, StringRevertError } from '@0x/utils';
import { BigNumber, LibBytesRevertErrors, StringRevertError } from '@0x/utils';
import { LogWithDecodedArgs } from 'ethereum-types';
import ethUtil = require('ethereumjs-util');
@@ -236,6 +236,18 @@ blockchainTests.resets('MixinSignatureValidator', env => {
return expect(tx).to.revertWith(expectedError);
});
it('should revert when signer is an EOA and SignatureType=Wallet', async () => {
const hashHex = getCurrentHashHex();
const signatureHex = hexConcat(SignatureType.Wallet);
const expectedError = new LibBytesRevertErrors.InvalidByteOperationError(
LibBytesRevertErrors.InvalidByteOperationErrorCodes.LengthGreaterThanOrEqualsFourRequired,
new BigNumber(0),
new BigNumber(4),
);
const tx = validateAsync(hashHex, signerAddress, signatureHex);
return expect(tx).to.revertWith(expectedError);
});
it('should revert when validator reverts and SignatureType=Wallet', async () => {
const hashHex = getCurrentHashHex(validatorWallet.address);
// Doesn't have to contain a real signature since our wallet contract
@@ -298,6 +310,18 @@ blockchainTests.resets('MixinSignatureValidator', env => {
return signatureValidator.isValidHashSignature.callAsync(_hashHex, _signerAddress, signatureHex);
};
it('should revert when signerAddress == 0', async () => {
const signatureHex = hexConcat(SignatureType.EIP712);
const expectedError = new ExchangeRevertErrors.SignatureError(
ExchangeRevertErrors.SignatureErrorCode.InvalidSigner,
hashHex,
constants.NULL_ADDRESS,
signatureHex,
);
const tx = validateAsync(hashHex, constants.NULL_ADDRESS, signatureHex);
return expect(tx).to.revertWith(expectedError);
});
it('should revert when SignatureType=Validator', async () => {
const signatureHex = hexConcat(SignatureType.Validator);
const expectedError = new ExchangeRevertErrors.SignatureError(
@@ -411,6 +435,23 @@ blockchainTests.resets('MixinSignatureValidator', env => {
return signatureValidator.isValidOrderSignature.callAsync(order, order.makerAddress, signatureHex);
};
it('should revert when signerAddress == 0', async () => {
const signatureHex = hexConcat(SignatureType.EIP712);
const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder);
const expectedError = new ExchangeRevertErrors.SignatureError(
ExchangeRevertErrors.SignatureErrorCode.InvalidSigner,
orderHashHex,
constants.NULL_ADDRESS,
signatureHex,
);
const tx = signatureValidator.isValidOrderSignature.callAsync(
signedOrder,
constants.NULL_ADDRESS,
signatureHex,
);
return expect(tx).to.revertWith(expectedError);
});
it('should return true when SignatureType=Validator, signature is valid and validator is approved', async () => {
// Doesn't have to contain a real signature since our wallet contract
// just does a hash comparison.
@@ -544,9 +585,7 @@ blockchainTests.resets('MixinSignatureValidator', env => {
it('should revert when validator reverts and SignatureType=EIP1271Wallet', async () => {
signedOrder.makerAddress = validatorWallet.address;
// Doesn't have to contain a real signature since our wallet contract
// just does a hash comparison.
const signatureHex = hexConcat(generateRandomSignature(), SignatureType.EIP1271Wallet);
const signatureHex = hexConcat(SignatureType.EIP1271Wallet);
const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder);
const expectedError = new ExchangeRevertErrors.SignatureWalletError(
orderHashHex,
@@ -558,6 +597,34 @@ blockchainTests.resets('MixinSignatureValidator', env => {
return expect(tx).to.revertWith(expectedError);
});
it('should revert when signer is an EOA and SignatureType=EIP1271Wallet', async () => {
const signatureHex = hexConcat(SignatureType.EIP1271Wallet);
const expectedError = new LibBytesRevertErrors.InvalidByteOperationError(
LibBytesRevertErrors.InvalidByteOperationErrorCodes.LengthGreaterThanOrEqualsFourRequired,
new BigNumber(0),
new BigNumber(4),
);
const tx = signatureValidator.isValidOrderSignature.callAsync(signedOrder, signerAddress, signatureHex);
return expect(tx).to.revertWith(expectedError);
});
it('should revert when signer is an EOA and SignatureType=Validator', async () => {
const signatureHex = hexConcat(notSignerAddress, SignatureType.Validator);
const expectedError = new LibBytesRevertErrors.InvalidByteOperationError(
LibBytesRevertErrors.InvalidByteOperationErrorCodes.LengthGreaterThanOrEqualsFourRequired,
new BigNumber(0),
new BigNumber(4),
);
// Register an EOA as a validator.
await signatureValidator.setSignatureValidatorApproval.awaitTransactionSuccessAsync(
notSignerAddress,
true,
{ from: signerAddress },
);
const tx = signatureValidator.isValidOrderSignature.callAsync(signedOrder, signerAddress, signatureHex);
return expect(tx).to.revertWith(expectedError);
});
// Run hash-only signature type tests as well.
const validateOrderHashAsync = async (
hashHex: string,
@@ -618,6 +685,23 @@ blockchainTests.resets('MixinSignatureValidator', env => {
);
};
it('should revert when signerAddress == 0', async () => {
const signatureHex = hexConcat(SignatureType.EIP712);
const transactionHashHex = transactionHashUtils.getTransactionHashHex(signedTransaction);
const expectedError = new ExchangeRevertErrors.SignatureError(
ExchangeRevertErrors.SignatureErrorCode.InvalidSigner,
transactionHashHex,
constants.NULL_ADDRESS,
signatureHex,
);
const tx = signatureValidator.isValidTransactionSignature.callAsync(
signedTransaction,
constants.NULL_ADDRESS,
signatureHex,
);
return expect(tx).to.revertWith(expectedError);
});
it('should return true when SignatureType=Validator, signature is valid and validator is approved', async () => {
// Doesn't have to contain a real signature since our wallet contract
// just does a hash comparison.
@@ -765,6 +849,42 @@ blockchainTests.resets('MixinSignatureValidator', env => {
return expect(tx).to.revertWith(expectedError);
});
it('should revert when signer is an EOA and SignatureType=EIP1271Wallet', async () => {
const signatureHex = hexConcat(SignatureType.EIP1271Wallet);
const expectedError = new LibBytesRevertErrors.InvalidByteOperationError(
LibBytesRevertErrors.InvalidByteOperationErrorCodes.LengthGreaterThanOrEqualsFourRequired,
new BigNumber(0),
new BigNumber(4),
);
const tx = signatureValidator.isValidTransactionSignature.callAsync(
signedTransaction,
signerAddress,
signatureHex,
);
return expect(tx).to.revertWith(expectedError);
});
it('should revert when signer is an EOA and SignatureType=Validator', async () => {
const signatureHex = hexConcat(notSignerAddress, SignatureType.Validator);
const expectedError = new LibBytesRevertErrors.InvalidByteOperationError(
LibBytesRevertErrors.InvalidByteOperationErrorCodes.LengthGreaterThanOrEqualsFourRequired,
new BigNumber(0),
new BigNumber(4),
);
// Register an EOA as a validator.
await signatureValidator.setSignatureValidatorApproval.awaitTransactionSuccessAsync(
notSignerAddress,
true,
{ from: signerAddress },
);
const tx = signatureValidator.isValidTransactionSignature.callAsync(
signedTransaction,
signerAddress,
signatureHex,
);
return expect(tx).to.revertWith(expectedError);
});
// Run hash-only signature type tests as well.
const validateOrderHashAsync = async (
hashHex: string,