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

In @0x/contracts-exchange: Add SignatureWalletError and SignatureValidatorError rich reverts.

Browse Source 
In `@0x/contracts-exchange`: Change `AssetProxyTransferError` to accept a `revertData` bytes instead of a `revertReason` string.
In `@0x/contracts-exchange`: Aadd `contracts/test/TestRevertReceiver.sol` for testing that validator/wallet reverts are properly wrapped.
This commit is contained in:
Lawrence Forman
2019-04-12 17:05:35 -04:00
committed by Amir Bandeali
parent 2a6f02c764
commit a7fe47f295
14 changed files with 250 additions and 62 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
contracts/exchange/compiler.json
View File

@@ -41,6 +41,7 @@
"test/ReentrantERC20Token.sol",
"test/TestAssetProxyDispatcher.sol",
"test/TestExchangeInternals.sol",
"test/TestRevertReceiver.sol",
"test/TestSignatureValidator.sol",
"test/TestStaticCallReceiver.sol"
]

32
contracts/exchange/contracts/src/MixinAssetProxyDispatcher.sol
View File

@@ -112,8 +112,8 @@ contract MixinAssetProxyDispatcher is
// Whether the AssetProxy transfer succeeded.
bool didSucceed;
// On failure, the revert message returned by the asset proxy.
string memory revertMessage;
// On failure, the revert data thrown by the asset proxy.
bytes memory revertData;
// We construct calldata for the `assetProxy.transferFrom` ABI.
// The layout of this calldata is in the table below.
@@ -179,30 +179,14 @@ contract MixinAssetProxyDispatcher is
)
if iszero(didSucceed) { // Call reverted.
// The asset proxies always revert with a standard string error,
// which have the signature `Error(string)` and are ABI encoded
// the same way as calldata for a function call is.
// We will extract the string payload from this to upgrade it
// to a rich revert.
// Asset proxy revert errors have the following memory layout:
// | Area | Offset | Length | Contents |
// --------------------------------------------------------------------------------
// | Selector | 0 | 4 | bytes4 function selector (0x08c379a0) |
// | Params | | | |
// | | 4 | 32 | uint256 offset to data (always 32) |
// | Data | | | |
// | | 36 | 32 | uint256 length of data (n) |
// | | 68 | n | Left-aligned message bytes |
// Get the size of the string data/payload.
let revertDataSize := sub(returndatasize(), 36)
// Get the revert data.
let revertDataSize := returndatasize()
// We need to move the free memory pointer because we
// still have solidity logic that executes after this assembly.
mstore(64, add(cdStart, revertDataSize))
// Copy the revert string data.
returndatacopy(cdStart, 36, revertDataSize)
revertMessage := cdStart
// Copy the revert data.
returndatacopy(cdStart, 0, revertDataSize)
revertData := cdStart
}
}
@@ -210,7 +194,7 @@ contract MixinAssetProxyDispatcher is
rrevert(AssetProxyTransferError(
orderHash,
assetData,
revertMessage
revertData
));
}
}

42
contracts/exchange/contracts/src/MixinExchangeRichErrors.sol
View File

@@ -47,6 +47,44 @@ contract MixinExchangeRichErrors is
);
}
function SignatureValidatorError(
bytes32 orderHash,
address signer,
bytes memory signature,
bytes memory errorData
)
internal
pure
returns (bytes memory)
{
return abi.encodeWithSelector(
SIGNATURE_VALIDATOR_ERROR_SELECTOR,
orderHash,
signer,
signature,
errorData
);
}
function SignatureWalletError(
bytes32 orderHash,
address signer,
bytes memory signature,
bytes memory errorData
)
internal
pure
returns (bytes memory)
{
return abi.encodeWithSelector(
SIGNATURE_WALLET_ERROR_SELECTOR,
orderHash,
signer,
signature,
errorData
);
}
function OrderStatusError(
LibOrder.OrderStatus orderStatus,
bytes32 orderHash
@@ -172,7 +210,7 @@ contract MixinExchangeRichErrors is
function AssetProxyTransferError(
bytes32 orderHash,
bytes memory assetData,
string memory errorMessage
bytes memory errorData
)
internal
pure
@@ -182,7 +220,7 @@ contract MixinExchangeRichErrors is
ASSET_PROXY_TRANSFER_ERROR_SELECTOR,
orderHash,
assetData,
errorMessage
errorData
);
}

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

@@ -279,14 +279,15 @@ 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)
if (didSucceed && returnData.length == 32) {
return returnData.readUint256(0) != 0;
}
// Static call to verifier failed.
rrevert(SignatureError(
SignatureErrorCodes.WALLET_ERROR,
rrevert(SignatureWalletError(
hash,
walletAddress,
signature
signature,
returnData
));
}
@@ -336,14 +337,15 @@ contract MixinSignatureValidator is
// 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)
if (didSucceed && returnData.length == 32) {
return returnData.readUint256(0) != 0;
}
// Static call to verifier failed.
rrevert(SignatureError(
SignatureErrorCodes.VALIDATOR_ERROR,
rrevert(SignatureValidatorError(
hash,
signerAddress,
signature
signature,
returnData
));
}
}

34
contracts/exchange/contracts/src/mixins/MExchangeRichErrors.sol
View File

@@ -36,9 +36,7 @@ contract MExchangeRichErrors is
BAD_SIGNATURE,
INVALID_LENGTH,
UNSUPPORTED,
ILLEGAL,
WALLET_ERROR,
VALIDATOR_ERROR
ILLEGAL
}
enum AssetProxyDispatchErrorCodes {
@@ -65,6 +63,32 @@ contract MExchangeRichErrors is
pure
returns (bytes memory);
bytes4 internal constant SIGNATURE_VALIDATOR_ERROR_SELECTOR =
bytes4(keccak256("SignatureValidatorError(bytes32,address,bytes,bytes)"));
function SignatureValidatorError(
bytes32 orderHash,
address signer,
bytes memory signature,
bytes memory errorData
)
internal
pure
returns (bytes memory);
bytes4 internal constant SIGNATURE_WALLET_ERROR_SELECTOR =
bytes4(keccak256("SignatureWalletError(bytes32,address,bytes,bytes)"));
function SignatureWalletError(
bytes32 orderHash,
address signer,
bytes memory signature,
bytes memory errorData
)
internal
pure
returns (bytes memory);
bytes4 internal constant ORDER_STATUS_ERROR_SELECTOR =
bytes4(keccak256("OrderStatusError(uint8,bytes32)"));
@@ -155,12 +179,12 @@ contract MExchangeRichErrors is
returns (bytes memory);
bytes4 internal constant ASSET_PROXY_TRANSFER_ERROR_SELECTOR =
bytes4(keccak256("AssetProxyTransferError(bytes32,bytes,string)"));
bytes4(keccak256("AssetProxyTransferError(bytes32,bytes,bytes)"));
function AssetProxyTransferError(
bytes32 orderHash,
bytes memory assetData,
string memory errorMessage
bytes memory errorData
)
internal
pure

57
contracts/exchange/contracts/test/TestRevertReceiver.sol Normal file
View File

@@ -0,0 +1,57 @@
/*
Copyright 2018 ZeroEx Intl.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
pragma solidity ^0.5.5;
import "@0x/contracts-erc20/contracts/src/interfaces/IERC20Token.sol";
contract TestRevertReceiver {
string constant REVERT_REASON = "you shall not pass";
/// @dev Reverts with `REVERT_REASON`. Intended to be used with `Validator` signature type.
/// @param hash Message hash that is signed.
/// @param signerAddress Address that should have signed the given hash.
/// @param signature Proof of signing.
/// @return Validity of order signature.
function isValidSignature(
bytes32 hash,
address signerAddress,
bytes calldata signature
)
external
returns (bool isValid)
{
revert(REVERT_REASON);
}
/// @dev Reverts with `REVERT_REASON`. Intended to be used with `Wallet` signature type.
/// @param hash Message hash that is signed.
/// @param signature Proof of signing.
/// @return Validity of order signature.
function isValidSignature(
bytes32 hash,
bytes calldata signature
)
external
returns (bool isValid)
{
revert(REVERT_REASON);
}
}

2
contracts/exchange/package.json
View File

@@ -34,7 +34,7 @@
"lint-contracts": "solhint -c ../.solhint.json contracts/**/**/**/**/*.sol"
},
"config": {
"abis": "./generated-artifacts/@(Exchange|ExchangeWrapper|IAssetProxyDispatcher|IExchange|IExchangeCore|IMatchOrders|ISignatureValidator|ITransactions|IValidator|IWallet|IWrapperFunctions|ReentrantERC20Token|TestAssetProxyDispatcher|TestExchangeInternals|TestSignatureValidator|TestStaticCallReceiver|Validator|Wallet|Whitelist).json",
"abis": "./generated-artifacts/@(Exchange|ExchangeWrapper|IAssetProxyDispatcher|IExchange|IExchangeCore|IMatchOrders|ISignatureValidator|ITransactions|IValidator|IWallet|IWrapperFunctions|ReentrantERC20Token|TestAssetProxyDispatcher|TestExchangeInternals|TestRevertReceiver|TestSignatureValidator|TestStaticCallReceiver|Validator|Wallet|Whitelist).json",
"abis:comment": "This list is auto-generated by contracts-gen. Don't edit manually."
},
"repository": {

4
contracts/exchange/src/artifacts.ts
View File

@@ -19,6 +19,7 @@ import * as IWrapperFunctions from '../generated-artifacts/IWrapperFunctions.jso
import * as ReentrantERC20Token from '../generated-artifacts/ReentrantERC20Token.json';
import * as TestAssetProxyDispatcher from '../generated-artifacts/TestAssetProxyDispatcher.json';
import * as TestExchangeInternals from '../generated-artifacts/TestExchangeInternals.json';
import * as TestRevertReceiver from '../generated-artifacts/TestRevertReceiver.json';
import * as TestSignatureValidator from '../generated-artifacts/TestSignatureValidator.json';
import * as TestStaticCallReceiver from '../generated-artifacts/TestStaticCallReceiver.json';
import * as Validator from '../generated-artifacts/Validator.json';
@@ -39,9 +40,10 @@ export const artifacts = {
IValidator: IValidator as ContractArtifact,
IWallet: IWallet as ContractArtifact,
IWrapperFunctions: IWrapperFunctions as ContractArtifact,
ReentrantERC20Token: ReentrantERC20Token as ContractArtifact,
TestAssetProxyDispatcher: TestAssetProxyDispatcher as ContractArtifact,
TestExchangeInternals: TestExchangeInternals as ContractArtifact,
TestSignatureValidator: TestSignatureValidator as ContractArtifact,
TestStaticCallReceiver: TestStaticCallReceiver as ContractArtifact,
ReentrantERC20Token: ReentrantERC20Token as ContractArtifact,
TestRevertReceiver: TestRevertReceiver as ContractArtifact,
};

1
contracts/exchange/src/wrappers.ts
View File

@@ -17,6 +17,7 @@ export * from '../generated-wrappers/i_wrapper_functions';
export * from '../generated-wrappers/reentrant_erc20_token';
export * from '../generated-wrappers/test_asset_proxy_dispatcher';
export * from '../generated-wrappers/test_exchange_internals';
export * from '../generated-wrappers/test_revert_receiver';
export * from '../generated-wrappers/test_signature_validator';
export * from '../generated-wrappers/test_static_call_receiver';
export * from '../generated-wrappers/validator';

18
contracts/exchange/test/core.ts
View File

@@ -32,7 +32,7 @@ import {
import { BlockchainLifecycle } from '@0x/dev-utils';
import { assetDataUtils, ExchangeRevertErrors, orderHashUtils } from '@0x/order-utils';
import { RevertReason, SignatureType, SignedOrder } from '@0x/types';
import { BigNumber, providerUtils } from '@0x/utils';
import { BigNumber, providerUtils, StringRevertError } from '@0x/utils';
import { Web3Wrapper } from '@0x/web3-wrapper';
import * as chai from 'chai';
import { LogWithDecodedArgs } from 'ethereum-types';
@@ -298,11 +298,11 @@ describe('Exchange core', () => {
});
const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder);
signedOrder.signature = `0x0${SignatureType.Wallet}`;
const expectedError = new ExchangeRevertErrors.SignatureError(
ExchangeRevertErrors.SignatureErrorCode.WalletError,
const expectedError = new ExchangeRevertErrors.SignatureWalletError(
orderHashHex,
signedOrder.makerAddress,
signedOrder.signature,
constants.NULL_BYTES,
);
const tx = exchangeWrapper.fillOrderAsync(signedOrder, takerAddress);
return expect(tx).to.revertWith(expectedError);
@@ -317,11 +317,11 @@ describe('Exchange core', () => {
);
signedOrder.signature = `${maliciousValidator.address}0${SignatureType.Validator}`;
const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder);
const expectedError = new ExchangeRevertErrors.SignatureError(
ExchangeRevertErrors.SignatureErrorCode.ValidatorError,
const expectedError = new ExchangeRevertErrors.SignatureValidatorError(
orderHashHex,
signedOrder.makerAddress,
signedOrder.signature,
constants.NULL_BYTES,
);
const tx = exchangeWrapper.fillOrderAsync(signedOrder, takerAddress);
return expect(tx).to.revertWith(expectedError);
@@ -706,7 +706,7 @@ describe('Exchange core', () => {
const expectedError = new ExchangeRevertErrors.AssetProxyTransferError(
orderHashHex,
signedOrder.makerAssetData,
RevertReason.TransferFailed,
new StringRevertError(RevertReason.TransferFailed).encode(),
);
const tx = exchangeWrapper.fillOrderAsync(signedOrder, takerAddress, { takerAssetFillAmount });
return expect(tx).to.revertWith(expectedError);
@@ -733,7 +733,7 @@ describe('Exchange core', () => {
const expectedError = new ExchangeRevertErrors.AssetProxyTransferError(
orderHashHex,
signedOrder.takerAssetData,
RevertReason.TransferFailed,
new StringRevertError(RevertReason.TransferFailed).encode(),
);
const tx = exchangeWrapper.fillOrderAsync(signedOrder, takerAddress, { takerAssetFillAmount });
return expect(tx).to.revertWith(expectedError);
@@ -760,7 +760,7 @@ describe('Exchange core', () => {
const expectedError = new ExchangeRevertErrors.AssetProxyTransferError(
orderHashHex,
signedOrder.makerAssetData,
RevertReason.InvalidAmount,
new StringRevertError(RevertReason.InvalidAmount).encode(),
);
const tx = exchangeWrapper.fillOrderAsync(signedOrder, takerAddress, { takerAssetFillAmount });
return expect(tx).to.revertWith(expectedError);
@@ -787,7 +787,7 @@ describe('Exchange core', () => {
const expectedError = new ExchangeRevertErrors.AssetProxyTransferError(
orderHashHex,
signedOrder.takerAssetData,
RevertReason.InvalidAmount,
new StringRevertError(RevertReason.InvalidAmount).encode(),
);
const tx = exchangeWrapper.fillOrderAsync(signedOrder, takerAddress, { takerAssetFillAmount });
return expect(tx).to.revertWith(expectedError);

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

@@ -11,13 +11,14 @@ import {
import { BlockchainLifecycle } from '@0x/dev-utils';
import { assetDataUtils, ExchangeRevertErrors, orderHashUtils, signatureUtils } from '@0x/order-utils';
import { SignatureType, SignedOrder } from '@0x/types';
import { BigNumber, providerUtils } from '@0x/utils';
import { BigNumber, providerUtils, StringRevertError } from '@0x/utils';
import * as chai from 'chai';
import { LogWithDecodedArgs } from 'ethereum-types';
import ethUtil = require('ethereumjs-util');
import {
artifacts,
TestRevertReceiverContract,
TestSignatureValidatorContract,
TestSignatureValidatorSignatureValidatorApprovalEventArgs,
TestStaticCallReceiverContract,
@@ -39,6 +40,8 @@ describe('MixinSignatureValidator', () => {
let testValidator: ValidatorContract;
let maliciousWallet: TestStaticCallReceiverContract;
let maliciousValidator: TestStaticCallReceiverContract;
let revertingWallet: TestRevertReceiverContract;
let revertingValidator: TestRevertReceiverContract;
let signerAddress: string;
let signerPrivateKey: Buffer;
let notSignerAddress: string;
@@ -80,11 +83,20 @@ describe('MixinSignatureValidator', () => {
provider,
txDefaults,
);
revertingWallet = revertingValidator = await TestRevertReceiverContract.deployFrom0xArtifactAsync(
artifacts.TestRevertReceiver,
provider,
txDefaults,
);
signatureValidatorLogDecoder = new LogDecoder(web3Wrapper, artifacts);
await web3Wrapper.awaitTransactionSuccessAsync(
await signatureValidator.setSignatureValidatorApproval.sendTransactionAsync(testValidator.address, true, {
from: signerAddress,
}),
await signatureValidator.setSignatureValidatorApproval.sendTransactionAsync(
testValidator.address,
true,
{
from: signerAddress,
},
),
constants.AWAIT_TRANSACTION_MINED_MS,
);
await web3Wrapper.awaitTransactionSuccessAsync(
@@ -97,6 +109,16 @@ describe('MixinSignatureValidator', () => {
),
constants.AWAIT_TRANSACTION_MINED_MS,
);
await web3Wrapper.awaitTransactionSuccessAsync(
await signatureValidator.setSignatureValidatorApproval.sendTransactionAsync(
revertingValidator.address,
true,
{
from: signerAddress,
},
),
constants.AWAIT_TRANSACTION_MINED_MS,
);
const defaultOrderParams = {
...constants.STATIC_ORDER_PARAMS,
@@ -127,7 +149,7 @@ describe('MixinSignatureValidator', () => {
});
it('should revert when signature is empty', async () => {
const emptySignature = '0x';
const emptySignature = constants.NULL_BYTES;
const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder);
const expectedError = new ExchangeRevertErrors.SignatureError(
ExchangeRevertErrors.SignatureErrorCode.InvalidLength,
@@ -359,11 +381,11 @@ describe('MixinSignatureValidator', () => {
ethUtil.toBuffer(`0x${SignatureType.Wallet}`),
]);
const signatureHex = ethUtil.bufferToHex(signature);
const expectedError = new ExchangeRevertErrors.SignatureError(
ExchangeRevertErrors.SignatureErrorCode.WalletError,
const expectedError = new ExchangeRevertErrors.SignatureWalletError(
orderHashHex,
maliciousWallet.address,
signatureHex,
constants.NULL_BYTES,
);
const tx = signatureValidator.publicIsValidSignature.callAsync(
orderHashHex,
@@ -373,6 +395,33 @@ describe('MixinSignatureValidator', () => {
return expect(tx).to.revertWith(expectedError);
});
it('should revert when `isValidSignature` reverts and SignatureType=Wallet', async () => {
// Create EIP712 signature
const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder);
const orderHashBuffer = ethUtil.toBuffer(orderHashHex);
const ecSignature = ethUtil.ecsign(orderHashBuffer, signerPrivateKey);
// Create 0x signature from EIP712 signature
const signature = Buffer.concat([
ethUtil.toBuffer(ecSignature.v),
ecSignature.r,
ecSignature.s,
ethUtil.toBuffer(`0x${SignatureType.Wallet}`),
]);
const signatureHex = ethUtil.bufferToHex(signature);
const expectedError = new ExchangeRevertErrors.SignatureWalletError(
orderHashHex,
revertingWallet.address,
signatureHex,
new StringRevertError('you shall not pass').encode(),
);
const tx = signatureValidator.publicIsValidSignature.callAsync(
orderHashHex,
revertingWallet.address,
signatureHex,
);
return expect(tx).to.revertWith(expectedError);
});
it('should return true when SignatureType=Validator, signature is valid and validator is approved', async () => {
const validatorAddress = ethUtil.toBuffer(`${testValidator.address}`);
const signatureType = ethUtil.toBuffer(`0x${SignatureType.Validator}`);
@@ -409,15 +458,40 @@ describe('MixinSignatureValidator', () => {
const signature = Buffer.concat([validatorAddress, signatureType]);
const signatureHex = ethUtil.bufferToHex(signature);
const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder);
const expectedError = new ExchangeRevertErrors.SignatureError(
ExchangeRevertErrors.SignatureErrorCode.ValidatorError,
const expectedError = new ExchangeRevertErrors.SignatureValidatorError(
orderHashHex,
signedOrder.makerAddress,
signatureHex,
constants.NULL_BYTES,
);
const tx = signatureValidator.publicIsValidSignature.callAsync(
orderHashHex,
signerAddress,
signatureHex,
);
const tx = signatureValidator.publicIsValidSignature.callAsync(orderHashHex, signerAddress, signatureHex);
return expect(tx).to.revertWith(expectedError);
});
it('should revert when `isValidSignature` reverts and SignatureType=Validator', async () => {
const validatorAddress = ethUtil.toBuffer(`${revertingValidator.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 expectedError = new ExchangeRevertErrors.SignatureValidatorError(
orderHashHex,
signedOrder.makerAddress,
signatureHex,
new StringRevertError('you shall not pass').encode(),
);
const tx = signatureValidator.publicIsValidSignature.callAsync(
orderHashHex,
signerAddress,
signatureHex,
);
return expect(tx).to.revertWith(expectedError);
});
it('should return false when SignatureType=Validator, signature is valid and validator is not approved', async () => {
// Set approval of signature validator to false
await web3Wrapper.awaitTransactionSuccessAsync(

2
contracts/exchange/test/transactions.ts
View File

@@ -138,7 +138,7 @@ describe('Exchange transactions', () => {
makerTransactionFactory = new TransactionFactory(makerPrivateKey, exchange.address, chainId);
takerTransactionFactory = new TransactionFactory(takerPrivateKey, exchange.address, chainId);
});
describe.only('executeTransaction', () => {
describe('executeTransaction', () => {
describe('fillOrder', () => {
let takerAssetFillAmount: BigNumber;
beforeEach(async () => {

6
contracts/exchange/test/utils/fill_order_combinatorial_utils.ts
View File

@@ -954,7 +954,11 @@ function validationErrorToRevertError(order: Order, reason: RevertReason): Rever
case RevertReason.InvalidFillPrice:
return new ExchangeRevertErrors.FillError(ExchangeRevertErrors.FillErrorCode.InvalidFillPrice, orderHash);
case RevertReason.TransferFailed:
return new ExchangeRevertErrors.AssetProxyTransferError(orderHash, undefined, RevertReason.TransferFailed);
return new ExchangeRevertErrors.AssetProxyTransferError(
orderHash,
undefined,
new StringRevertError(RevertReason.TransferFailed).encode(),
);
default:
return new StringRevertError(reason);
}

1
contracts/exchange/tsconfig.json
View File

@@ -17,6 +17,7 @@
"generated-artifacts/ReentrantERC20Token.json",
"generated-artifacts/TestAssetProxyDispatcher.json",
"generated-artifacts/TestExchangeInternals.json",
"generated-artifacts/TestRevertReceiver.json",
"generated-artifacts/TestSignatureValidator.json",
"generated-artifacts/TestStaticCallReceiver.json",
"generated-artifacts/Validator.json",