aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAmir Bandeali <abandeali1@gmail.com>2018-05-10 05:41:44 +0800
committerAmir Bandeali <abandeali1@gmail.com>2018-05-31 08:11:30 +0800
commit4b71c65aea44bba34429e288c5411a090dd78071 (patch)
tree29fc4d79b6a74e7bfa0c23c5e5cada14de9f022f
parent34ab53173daad302e0d13cbf5369116b373d72d3 (diff)
downloaddexon-sol-tools-4b71c65aea44bba34429e288c5411a090dd78071.tar
dexon-sol-tools-4b71c65aea44bba34429e288c5411a090dd78071.tar.gz
dexon-sol-tools-4b71c65aea44bba34429e288c5411a090dd78071.tar.bz2
dexon-sol-tools-4b71c65aea44bba34429e288c5411a090dd78071.tar.lz
dexon-sol-tools-4b71c65aea44bba34429e288c5411a090dd78071.tar.xz
dexon-sol-tools-4b71c65aea44bba34429e288c5411a090dd78071.tar.zst
dexon-sol-tools-4b71c65aea44bba34429e288c5411a090dd78071.zip
Update Whitelist contract with comments, also require maker to be whitelisted
-rw-r--r--packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol5
-rw-r--r--packages/contracts/src/contracts/current/protocol/Exchange/libs/LibExchangeErrors.sol1
-rw-r--r--packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol47
-rw-r--r--packages/contracts/src/utils/artifacts.ts2
-rw-r--r--packages/contracts/test/exchange/transactions.ts33
5 files changed, 72 insertions, 16 deletions
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol
index f93a80705..d28df11d8 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol
@@ -47,7 +47,10 @@ contract MixinTransactions is
external
{
// Prevent reentrancy
- require(currentContextAddress == address(0));
+ require(
+ currentContextAddress == address(0),
+ REENTRANCY_NOT_ALLOWED
+ );
// Calculate transaction hash
bytes32 transactionHash = keccak256(
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibExchangeErrors.sol b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibExchangeErrors.sol
index 4712ee36c..7d67e5080 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibExchangeErrors.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibExchangeErrors.sol
@@ -32,6 +32,7 @@ contract LibExchangeErrors {
string constant INVALID_ORDER_MAKER_ASSET_AMOUNT = "Invalid order maker asset amount: expected a non-zero value.";
// Transaction revert reasons
+ string constant REENTRANCY_NOT_ALLOWED = "`executeTransaction` is not allowed to call itself recursively.";
string constant DUPLICATE_TRANSACTION_HASH = "Transaction has already been executed.";
string constant TRANSACTION_EXECUTION_FAILED = "Transaction execution failed.";
diff --git a/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol b/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol
index c53856602..4a2e9a931 100644
--- a/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol
+++ b/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol
@@ -23,22 +23,30 @@ import "../../protocol/Exchange/interfaces/IExchange.sol";
import "../../protocol/Exchange/libs/LibOrder.sol";
import "../../utils/Ownable/Ownable.sol";
-contract Whitelist is Ownable {
+contract Whitelist is
+ Ownable
+{
+ // Revert reasons
+ string constant ADDRESS_NOT_WHITELISTED = "Address not whitelisted.";
+ // Mapping of address => whitelist status.
mapping (address => bool) public isWhitelisted;
+
+ // Exchange contract.
IExchange EXCHANGE;
- bytes txOriginSignature = new bytes(1);
- bytes4 fillOrderFunctionSelector;
+ // TxOrigin signature type is the 5th value in enum SignatureType and has a length of 1.
+ bytes constant TX_ORIGIN_SIGNATURE = "\x04";
- function Whitelist(address _exchange)
+ constructor (address _exchange)
public
{
EXCHANGE = IExchange(_exchange);
- txOriginSignature[0] = 0x04;
- fillOrderFunctionSelector = EXCHANGE.fillOrder.selector;
}
+ /// @dev Adds or removes an address from the whitelist.
+ /// @param target Address to add or remove from whitelist.
+ /// @param isApproved Whitelist status to assign to address.
function updateWhitelistStatus(address target, bool isApproved)
external
onlyOwner
@@ -46,6 +54,14 @@ contract Whitelist is Ownable {
isWhitelisted[target] = isApproved;
}
+ /// @dev Fills an order using `msg.sender` as the taker.
+ /// The transaction will revert if both the maker and taker are not whitelisted.
+ /// Orders should specify this contract as the `senderAddress` in order to gaurantee
+ /// that both maker and taker have been whitelisted.
+ /// @param order Order struct containing order specifications.
+ /// @param takerAssetFillAmount Desired amount of takerAsset to sell.
+ /// @param salt Arbitrary value to gaurantee uniqueness of 0x transaction hash.
+ /// @param orderSignature Proof that order has been created by maker.
function fillOrderIfWhitelisted(
LibOrder.Order memory order,
uint256 takerAssetFillAmount,
@@ -58,23 +74,32 @@ contract Whitelist is Ownable {
// This contract must be the entry point for the transaction.
require(takerAddress == tx.origin);
- // Check if sender is on the whitelist.
- require(isWhitelisted[takerAddress]);
+ // Check if maker is on the whitelist
+ require(
+ isWhitelisted[order.makerAddress],
+ ADDRESS_NOT_WHITELISTED
+ );
+
+ // Check if taker is on the whitelist.
+ require(
+ isWhitelisted[takerAddress],
+ ADDRESS_NOT_WHITELISTED
+ );
// Encode arguments into byte array.
bytes memory data = abi.encodeWithSelector(
- fillOrderFunctionSelector,
+ EXCHANGE.fillOrder.selector,
order,
takerAssetFillAmount,
orderSignature
);
- // Call `fillOrder`via `executeTransaction`.
+ // Call `fillOrder` via `executeTransaction`.
EXCHANGE.executeTransaction(
salt,
takerAddress,
data,
- txOriginSignature
+ TX_ORIGIN_SIGNATURE
);
}
} \ No newline at end of file
diff --git a/packages/contracts/src/utils/artifacts.ts b/packages/contracts/src/utils/artifacts.ts
index fe74ea072..357c66a0a 100644
--- a/packages/contracts/src/utils/artifacts.ts
+++ b/packages/contracts/src/utils/artifacts.ts
@@ -15,6 +15,7 @@ import * as TestLibs from '../artifacts/TestLibs.json';
import * as TestSignatureValidator from '../artifacts/TestSignatureValidator.json';
import * as TokenRegistry from '../artifacts/TokenRegistry.json';
import * as EtherToken from '../artifacts/WETH9.json';
+import * as Whitelist from '../artifacts/Whitelist.json';
import * as ZRX from '../artifacts/ZRXToken.json';
export const artifacts = {
@@ -33,5 +34,6 @@ export const artifacts = {
TestLibs: (TestLibs as any) as ContractArtifact,
TestSignatureValidator: (TestSignatureValidator as any) as ContractArtifact,
TokenRegistry: (TokenRegistry as any) as ContractArtifact,
+ Whitelist: (Whitelist as any) as ContractArtifact,
ZRX: (ZRX as any) as ContractArtifact,
};
diff --git a/packages/contracts/test/exchange/transactions.ts b/packages/contracts/test/exchange/transactions.ts
index 7242b0063..fe6df2f75 100644
--- a/packages/contracts/test/exchange/transactions.ts
+++ b/packages/contracts/test/exchange/transactions.ts
@@ -212,8 +212,12 @@ describe('Exchange transactions', () => {
let whitelistOrderFactory: OrderFactory;
before(async () => {
- const whitelistInstance = await deployer.deployAsync(ContractName.Whitelist, [exchange.address]);
- whitelist = new WhitelistContract(whitelistInstance.abi, whitelistInstance.address, provider);
+ whitelist = await WhitelistContract.deployFrom0xArtifactAsync(
+ artifacts.Whitelist,
+ provider,
+ txDefaults,
+ exchange.address,
+ );
const defaultOrderParams = {
...constants.STATIC_ORDER_PARAMS,
senderAddress: whitelist.address,
@@ -231,7 +235,28 @@ describe('Exchange transactions', () => {
erc20Balances = await erc20Wrapper.getBalancesAsync();
});
+ it('should revert if maker has not been whitelisted', async () => {
+ const isApproved = true;
+ await whitelist.updateWhitelistStatus.sendTransactionAsync(takerAddress, isApproved, { from: owner });
+
+ const orderStruct = orderUtils.getOrderStruct(signedOrder);
+ const takerAssetFillAmount = signedOrder.takerAssetAmount;
+ const salt = ZeroEx.generatePseudoRandomSalt();
+ return expect(
+ whitelist.fillOrderIfWhitelisted.sendTransactionAsync(
+ orderStruct,
+ takerAssetFillAmount,
+ salt,
+ signedOrder.signature,
+ { from: takerAddress },
+ ),
+ ).to.be.rejectedWith(constants.REVERT);
+ });
+
it('should revert if taker has not been whitelisted', async () => {
+ const isApproved = true;
+ await whitelist.updateWhitelistStatus.sendTransactionAsync(makerAddress, isApproved, { from: owner });
+
const orderStruct = orderUtils.getOrderStruct(signedOrder);
const takerAssetFillAmount = signedOrder.takerAssetAmount;
const salt = ZeroEx.generatePseudoRandomSalt();
@@ -246,8 +271,9 @@ describe('Exchange transactions', () => {
).to.be.rejectedWith(constants.REVERT);
});
- it('should fill the order if taker has been whitelisted', async () => {
+ it('should fill the order if maker and taker have been whitelisted', async () => {
const isApproved = true;
+ await whitelist.updateWhitelistStatus.sendTransactionAsync(makerAddress, isApproved, { from: owner });
await whitelist.updateWhitelistStatus.sendTransactionAsync(takerAddress, isApproved, { from: owner });
const orderStruct = orderUtils.getOrderStruct(signedOrder);
@@ -260,7 +286,6 @@ describe('Exchange transactions', () => {
signedOrder.signature,
{ from: takerAddress },
);
-
const newBalances = await erc20Wrapper.getBalancesAsync();
const makerAssetFillAmount = signedOrder.makerAssetAmount;