In @0x/order-utils: Add signerAddress and signature to ExchangeRevertErrors.SignatureError.

In `@0x/contracts-exchange`: Add `signerAddress` and `signature` to `SignatureError` reverts.
This commit is contained in:
Lawrence Forman
2019-04-11 18:13:55 -04:00
committed by Amir Bandeali
parent ac18359410
commit a0223835b8
10 changed files with 142 additions and 106 deletions

View File

@@ -352,14 +352,16 @@ contract MixinExchangeCore is
// Validate Maker signature (check only if first time seen)
if (orderInfo.orderTakerAssetFilledAmount == 0) {
address makerAddress = order.makerAddress;
if (!isValidSignature(
orderInfo.orderHash,
order.makerAddress,
signature
)) {
makerAddress,
signature)) {
rrevert(SignatureError(
SignatureErrorCodes.BAD_SIGNATURE,
orderInfo.orderHash
orderInfo.orderHash,
makerAddress,
signature
));
}
}

View File

@@ -30,7 +30,9 @@ contract MixinExchangeRichErrors is
// solhint-disable func-name-mixedcase
function SignatureError(
SignatureErrorCodes error,
bytes32 orderHash
bytes32 orderHash,
address signer,
bytes memory signature
)
internal
pure
@@ -39,7 +41,9 @@ contract MixinExchangeRichErrors is
return abi.encodeWithSelector(
SIGNATURE_ERROR_SELECTOR,
error,
orderHash
orderHash,
signer,
signature
);
}

View File

@@ -57,11 +57,12 @@ contract MixinSignatureValidator is
if (!isValidSignature(
hash,
signerAddress,
signature
)) {
signature)) {
rrevert(SignatureError(
SignatureErrorCodes.BAD_SIGNATURE,
hash
hash,
signerAddress,
signature
));
}
}
@@ -102,15 +103,25 @@ contract MixinSignatureValidator is
returns (bool isValid)
{
if (signature.length == 0) {
rrevert(SignatureError(SignatureErrorCodes.INVALID_LENGTH, hash));
rrevert(SignatureError(
SignatureErrorCodes.INVALID_LENGTH,
hash,
signerAddress,
signature
));
}
// Pop last byte off of signature byte array.
uint8 signatureTypeRaw = uint8(signature.popLastByte());
uint8 signatureTypeRaw = uint8(signature[signature.length-1]);
// Ensure signature is supported
if (signatureTypeRaw >= uint8(SignatureType.NSignatureTypes)) {
rrevert(SignatureError(SignatureErrorCodes.UNSUPPORTED, hash));
rrevert(SignatureError(
SignatureErrorCodes.UNSUPPORTED,
hash,
signerAddress,
signature
));
}
SignatureType signatureType = SignatureType(signatureTypeRaw);
@@ -129,7 +140,9 @@ contract MixinSignatureValidator is
if (signatureType == SignatureType.Illegal) {
rrevert(SignatureError(
SignatureErrorCodes.ILLEGAL,
hash
hash,
signerAddress,
signature
));
// Always invalid signature.
@@ -137,10 +150,12 @@ contract MixinSignatureValidator is
// offered explicitly. It can be implicitly created by providing
// a correctly formatted but incorrect signature.
} else if (signatureType == SignatureType.Invalid) {
if (signature.length != 0) {
if (signature.length != 1) {
rrevert(SignatureError(
SignatureErrorCodes.INVALID_LENGTH,
hash
hash,
signerAddress,
signature
));
}
isValid = false;
@@ -148,10 +163,12 @@ contract MixinSignatureValidator is
// Signature using EIP712
} else if (signatureType == SignatureType.EIP712) {
if (signature.length != 65) {
if (signature.length != 66) {
rrevert(SignatureError(
SignatureErrorCodes.INVALID_LENGTH,
hash
hash,
signerAddress,
signature
));
}
v = uint8(signature[0]);
@@ -168,10 +185,12 @@ contract MixinSignatureValidator is
// Signed using web3.eth_sign
} else if (signatureType == SignatureType.EthSign) {
if (signature.length != 65) {
if (signature.length != 66) {
rrevert(SignatureError(
SignatureErrorCodes.INVALID_LENGTH,
hash
hash,
signerAddress,
signature
));
}
v = uint8(signature[0]);
@@ -207,15 +226,7 @@ contract MixinSignatureValidator is
// | 0x00 + x | 20 | Address of validator contract |
// | 0x14 + x | 1 | Signature type is always "\x06" |
} else if (signatureType == SignatureType.Validator) {
// Pop last 20 bytes off of signature byte array.
address validatorAddress = signature.popLast20Bytes();
// Ensure signer has approved validator.
if (!allowedValidators[signerAddress][validatorAddress]) {
return false;
}
isValid = isValidValidatorSignature(
validatorAddress,
hash,
signerAddress,
signature
@@ -233,7 +244,12 @@ contract MixinSignatureValidator is
// that we currently support. In this case returning false
// may lead the caller to incorrectly believe that the
// signature was invalid.)
rrevert(SignatureError(SignatureErrorCodes.UNSUPPORTED, hash));
rrevert(SignatureError(
SignatureErrorCodes.UNSUPPORTED,
hash,
signerAddress,
signature
));
}
/// @dev Verifies signature using logic defined by Wallet contract.
@@ -247,78 +263,90 @@ contract MixinSignatureValidator is
address walletAddress,
bytes memory signature
)
internal
private
view
returns (bool isValid)
{
uint256 signatureLength = signature.length;
// Shave the signature type off the signature.
assembly {
mstore(signature, sub(signatureLength, 1))
}
// Encode the call data.
bytes memory callData = abi.encodeWithSelector(
IWallet(walletAddress).isValidSignature.selector,
hash,
signature
);
bool didSucceed;
assembly {
let cdStart := add(callData, 32)
didSucceed := staticcall(
gas, // forward all gas
walletAddress, // address of Wallet contract
cdStart, // pointer to start of input
mload(callData), // length of input
cdStart, // write output over input
32 // output size is 32 bytes
)
// 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) != 0;
if eq(didSucceed, 1) {
// Signature is valid if call did not revert and returned true
isValid := mload(cdStart)
}
// Static call to verifier failed.
// Restore the full signature.
assembly {
mstore(signature, signatureLength)
}
if (didSucceed)
return isValid;
rrevert(SignatureError(SignatureErrorCodes.WALLET_ERROR, hash));
rrevert(SignatureError(
SignatureErrorCodes.WALLET_ERROR,
hash,
walletAddress,
signature
));
}
/// @dev Verifies signature using logic defined by Validator contract.
/// @param validatorAddress Address of validator contract.
/// @param hash Any 32 byte hash.
/// @param signerAddress Address that should have signed the given hash.
/// @param signature Proof that the hash has been signed by signer.
/// @return True if the address recovered from the provided signature matches the input signer address.
function isValidValidatorSignature(
address validatorAddress,
bytes32 hash,
address signerAddress,
bytes memory signature
)
internal
private
view
returns (bool isValid)
{
uint256 signatureLength = signature.length;
// Read the validator address from the signature.
address validatorAddress = signature.readAddress(signatureLength - 21);
// Ensure signer has approved validator.
if (!allowedValidators[signerAddress][validatorAddress]) {
return false;
}
// Shave the validator address and signature type from the signature.
assembly {
mstore(signature, sub(signatureLength, 21))
}
// Encode the call data.
bytes memory callData = abi.encodeWithSelector(
IValidator(signerAddress).isValidSignature.selector,
IValidator(validatorAddress).isValidSignature.selector,
hash,
signerAddress,
signature
);
bool didSucceed;
assembly {
let cdStart := add(callData, 32)
didSucceed := staticcall(
gas, // forward all gas
validatorAddress, // address of Validator contract
cdStart, // pointer to start of input
mload(callData), // length of input
cdStart, // write output over input
32 // output size is 32 bytes
)
// Static call the verification function.
(bool didSucceed, bytes memory returnData) = validatorAddress.staticcall(callData);
// Return data should be a single bool.
if (didSucceed && returnData.length == 32)
return returnData.readUint256(0) != 0;
if eq(didSucceed, 1) {
// Signature is valid if call did not revert and returned true
isValid := mload(cdStart)
}
// Static call to verifier failed.
// Restore the full signature.
assembly {
mstore(signature, signatureLength)
}
if (didSucceed)
return isValid;
rrevert(SignatureError(SignatureErrorCodes.VALIDATOR_ERROR, hash));
rrevert(SignatureError(
SignatureErrorCodes.VALIDATOR_ERROR,
hash,
signerAddress,
signature
));
}
}

View File

@@ -54,11 +54,13 @@ contract MExchangeRichErrors is
// solhint-disable func-name-mixedcase
bytes4 internal constant SIGNATURE_ERROR_SELECTOR =
bytes4(keccak256("SignatureError(uint8,bytes32)"));
bytes4(keccak256("SignatureError(uint8,bytes32,address,bytes)"));
function SignatureError(
SignatureErrorCodes error,
bytes32 orderHash
bytes32 orderHash,
address signer,
bytes memory signature
)
internal
pure

View File

@@ -42,35 +42,4 @@ contract MSignatureValidator is
PreSigned, // 0x06
NSignatureTypes // 0x07, number of signature types. Always leave at end.
}
/// @dev Verifies signature using logic defined by Wallet contract.
/// @param hash Any 32 byte hash.
/// @param walletAddress Address that should have signed the given hash
/// and defines its own signature verification method.
/// @param signature Proof that the hash has been signed by signer.
/// @return True if the address recovered from the provided signature matches the input signer address.
function isValidWalletSignature(
bytes32 hash,
address walletAddress,
bytes memory signature
)
internal
view
returns (bool isValid);
/// @dev Verifies signature using logic defined by Validator contract.
/// @param validatorAddress Address of validator contract.
/// @param hash Any 32 byte hash.
/// @param signerAddress Address that should have signed the given hash.
/// @param signature Proof that the hash has been signed by signer.
/// @return True if the address recovered from the provided signature matches the input signer address.
function isValidValidatorSignature(
address validatorAddress,
bytes32 hash,
address signerAddress,
bytes memory signature
)
internal
view
returns (bool isValid);
}

View File

@@ -265,6 +265,8 @@ describe('Exchange core', () => {
const expectedError = new ExchangeRevertErrors.SignatureError(
ExchangeRevertErrors.SignatureErrorCode.BadSignature,
orderHashHex,
signedOrder.makerAddress,
invalidSigHex,
);
const tx = exchangeWrapper.fillOrderAsync(signedOrder, takerAddress);
return expect(tx).to.revertWith(expectedError);
@@ -299,6 +301,8 @@ describe('Exchange core', () => {
const expectedError = new ExchangeRevertErrors.SignatureError(
ExchangeRevertErrors.SignatureErrorCode.WalletError,
orderHashHex,
signedOrder.makerAddress,
signedOrder.signature,
);
const tx = exchangeWrapper.fillOrderAsync(signedOrder, takerAddress);
return expect(tx).to.revertWith(expectedError);
@@ -316,6 +320,8 @@ describe('Exchange core', () => {
const expectedError = new ExchangeRevertErrors.SignatureError(
ExchangeRevertErrors.SignatureErrorCode.ValidatorError,
orderHashHex,
signedOrder.makerAddress,
signedOrder.signature,
);
const tx = exchangeWrapper.fillOrderAsync(signedOrder, takerAddress);
return expect(tx).to.revertWith(expectedError);

View File

@@ -1191,6 +1191,8 @@ describe('matchOrders', () => {
const expectedError = new ExchangeRevertErrors.SignatureError(
ExchangeRevertErrors.SignatureErrorCode.BadSignature,
orderHashHex,
signedOrderRight.makerAddress,
signedOrderRight.signature,
);
// Match orders
const tx = exchangeWrapper.matchOrdersAsync(signedOrderLeft, signedOrderRight, takerAddress);
@@ -1216,6 +1218,8 @@ describe('matchOrders', () => {
const expectedError = new ExchangeRevertErrors.SignatureError(
ExchangeRevertErrors.SignatureErrorCode.BadSignature,
orderHashHex,
signedOrderRight.makerAddress,
signedOrderRight.signature,
);
// Match orders
const tx = exchangeWrapper.matchOrdersAsync(signedOrderLeft, signedOrderRight, takerAddress);

View File

@@ -132,6 +132,8 @@ describe('MixinSignatureValidator', () => {
const expectedError = new ExchangeRevertErrors.SignatureError(
ExchangeRevertErrors.SignatureErrorCode.InvalidLength,
orderHashHex,
signedOrder.makerAddress,
emptySignature,
);
const tx = signatureValidator.publicIsValidSignature.callAsync(
orderHashHex,
@@ -148,6 +150,8 @@ describe('MixinSignatureValidator', () => {
const expectedError = new ExchangeRevertErrors.SignatureError(
ExchangeRevertErrors.SignatureErrorCode.Unsupported,
orderHashHex,
signedOrder.makerAddress,
unsupportedSignatureHex,
);
const tx = signatureValidator.publicIsValidSignature.callAsync(
orderHashHex,
@@ -158,16 +162,18 @@ describe('MixinSignatureValidator', () => {
});
it('should revert when SignatureType=Illegal', async () => {
const unsupportedSignatureHex = `0x${Buffer.from([SignatureType.Illegal]).toString('hex')}`;
const illegalSignatureHex = `0x${Buffer.from([SignatureType.Illegal]).toString('hex')}`;
const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder);
const expectedError = new ExchangeRevertErrors.SignatureError(
ExchangeRevertErrors.SignatureErrorCode.Illegal,
orderHashHex,
signedOrder.makerAddress,
illegalSignatureHex,
);
const tx = signatureValidator.publicIsValidSignature.callAsync(
orderHashHex,
signedOrder.makerAddress,
unsupportedSignatureHex,
illegalSignatureHex,
);
return expect(tx).to.revertWith(expectedError);
});
@@ -192,6 +198,8 @@ describe('MixinSignatureValidator', () => {
const expectedError = new ExchangeRevertErrors.SignatureError(
ExchangeRevertErrors.SignatureErrorCode.InvalidLength,
orderHashHex,
signedOrder.makerAddress,
signatureHex,
);
const tx = signatureValidator.publicIsValidSignature.callAsync(
orderHashHex,
@@ -354,6 +362,8 @@ describe('MixinSignatureValidator', () => {
const expectedError = new ExchangeRevertErrors.SignatureError(
ExchangeRevertErrors.SignatureErrorCode.WalletError,
orderHashHex,
maliciousWallet.address,
signatureHex,
);
const tx = signatureValidator.publicIsValidSignature.callAsync(
orderHashHex,
@@ -402,6 +412,8 @@ describe('MixinSignatureValidator', () => {
const expectedError = new ExchangeRevertErrors.SignatureError(
ExchangeRevertErrors.SignatureErrorCode.ValidatorError,
orderHashHex,
signedOrder.makerAddress,
signatureHex,
);
const tx = signatureValidator.publicIsValidSignature.callAsync(orderHashHex, signerAddress, signatureHex);
return expect(tx).to.revertWith(expectedError);

View File

@@ -835,6 +835,8 @@ describe('Exchange wrappers', () => {
const expectedError = new ExchangeRevertErrors.SignatureError(
ExchangeRevertErrors.SignatureErrorCode.BadSignature,
orderHashHex,
signedOrders[1].makerAddress,
signedOrders[1].signature,
);
const tx = exchangeWrapper.marketSellOrdersAsync(signedOrders, takerAddress, {
takerAssetFillAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(1000), 18),
@@ -1098,6 +1100,8 @@ describe('Exchange wrappers', () => {
const expectedError = new ExchangeRevertErrors.SignatureError(
ExchangeRevertErrors.SignatureErrorCode.BadSignature,
orderHashHex,
signedOrders[1].makerAddress,
signedOrders[1].signature,
);
const tx = exchangeWrapper.marketBuyOrdersAsync(signedOrders, takerAddress, {
makerAssetFillAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(1000), 18),

View File

@@ -32,8 +32,13 @@ export enum TransactionErrorCode {
}
export class SignatureError extends RevertError {
constructor(error?: SignatureErrorCode, orderHash?: string) {
super('SignatureError(uint8 error,bytes32 orderHash)', { error, orderHash });
constructor(error?: SignatureErrorCode, orderHash?: string, signer?: string, signature?: string) {
super('SignatureError(uint8 error, bytes32 orderHash, address signer, bytes signature)', {
error,
orderHash,
signer,
signature,
});
}
}