From 8dc794a11ab3f066b9a2d09bea4212f1a39d6144 Mon Sep 17 00:00:00 2001 From: decanus Date: Thu, 26 Apr 2018 19:14:21 +0200 Subject: [PATCH 1/4] started --- contracts/Exchange.sol | 11 ++++------- contracts/ExchangeInterface.sol | 1 + 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/contracts/Exchange.sol b/contracts/Exchange.sol index ee833c5..acf1d25 100644 --- a/contracts/Exchange.sol +++ b/contracts/Exchange.sol @@ -1,13 +1,13 @@ pragma solidity ^0.4.21; +pragma experimental ABIEncoderV2; -import "./ExchangeInterface.sol"; import "./Libraries/SafeMath.sol"; import "./Libraries/SignatureValidator.sol"; import "./Libraries/OrderLibrary.sol"; import "./Ownership/Ownable.sol"; import "./Tokens/ERC20.sol"; -contract Exchange is Ownable, ExchangeInterface { +contract Exchange is Ownable { using SafeMath for *; using OrderLibrary for OrderLibrary.Order; @@ -73,11 +73,8 @@ contract Exchange is Ownable, ExchangeInterface { } /// @dev Cancels an order. - /// @param addresses Array of trade's maker, makerToken and takerToken. - /// @param values Array of trade's makerTokenAmount, takerTokenAmount, expires and nonce. - function cancel(address[3] addresses, uint[4] values) external { - OrderLibrary.Order memory order = OrderLibrary.createOrder(addresses, values); - + /// @param order Order struct for the cancelling order. + function cancel(OrderLibrary.Order order) external { require(msg.sender == order.maker); require(order.makerTokenAmount > 0 && order.takerTokenAmount > 0); diff --git a/contracts/ExchangeInterface.sol b/contracts/ExchangeInterface.sol index 703d1ca..a7989f1 100644 --- a/contracts/ExchangeInterface.sol +++ b/contracts/ExchangeInterface.sol @@ -2,6 +2,7 @@ pragma solidity ^0.4.21; import "./Vault/VaultInterface.sol"; +// @todo update once we can have structs interface ExchangeInterface { event Cancelled(bytes32 indexed hash); From 937d79d331c6a0770ddf7a6a7261981ecebff46c Mon Sep 17 00:00:00 2001 From: decanus Date: Fri, 27 Apr 2018 00:10:56 +0200 Subject: [PATCH 2/4] Started moving all external functions to use struct --- contracts/Exchange.sol | 120 +++++++++++++++++------------------------ 1 file changed, 50 insertions(+), 70 deletions(-) diff --git a/contracts/Exchange.sol b/contracts/Exchange.sol index 87bfd3d..ecf0c13 100644 --- a/contracts/Exchange.sol +++ b/contracts/Exchange.sol @@ -64,12 +64,51 @@ contract Exchange is Ownable { } /// @dev Takes an order. - /// @param addresses Array of trade's maker, makerToken and takerToken. - /// @param values Array of trade's makerTokenAmount, takerTokenAmount, expires and nonce. + /// @param order Order to take. /// @param signature Signed order along with signature mode. /// @param maxFillAmount Maximum amount of the order to be filled. - function trade(address[3] addresses, uint[4] values, bytes signature, uint maxFillAmount) external { - trade(OrderLibrary.createOrder(addresses, values), msg.sender, signature, maxFillAmount); + function trade(OrderLibrary.Order order, bytes signature, uint maxFillAmount) external { + require(msg.sender != order.maker); + bytes32 hash = order.hash(); + + require(order.makerToken != order.takerToken); + require(canTrade(order, signature, hash)); + + uint fillAmount = SafeMath.min256(maxFillAmount, availableAmount(order, hash)); + + require(roundingPercent(fillAmount, order.takerTokenAmount, order.makerTokenAmount) <= MAX_ROUNDING_PERCENTAGE); + require(vault.balanceOf(order.takerToken, msg.sender) >= fillAmount); + + uint makeAmount = order.makerTokenAmount.mul(fillAmount).div(order.takerTokenAmount); + uint tradeTakerFee = makeAmount.mul(takerFee).div(1 ether); + + if (tradeTakerFee > 0) { + vault.transfer(order.makerToken, order.maker, feeAccount, tradeTakerFee); + } + + vault.transfer(order.takerToken, msg.sender, order.maker, fillAmount); + vault.transfer(order.makerToken, order.maker, msg.sender, makeAmount.sub(tradeTakerFee)); + + fills[hash] = fills[hash].add(fillAmount); + assert(fills[hash] <= order.takerTokenAmount); + + if (subscribed[order.maker]) { + order.maker.call.gas(MAX_HOOK_GAS)( + HookSubscriber(order.maker).tradeExecuted.selector, + order.takerToken, + fillAmount + ); + } + + emit Traded( + hash, + order.makerToken, + makeAmount, + order.takerToken, + fillAmount, + order.maker, + msg.sender + ); } /// @dev Cancels an order. @@ -87,13 +126,9 @@ contract Exchange is Ownable { } /// @dev Creates an order which is then indexed in the orderbook. - /// @param addresses Array of trade's makerToken and takerToken. - /// @param values Array of trade's makerTokenAmount, takerTokenAmount, expires and nonce. - function order(address[2] addresses, uint[4] values) external { - OrderLibrary.Order memory order = OrderLibrary.createOrder( - [msg.sender, addresses[0], addresses[1]], - values - ); + /// @param order Order to create. + function order(OrderLibrary.Order order) external { + order.maker = msg.sender; require(vault.isApproved(order.maker, this)); require(vault.balanceOf(order.makerToken, order.maker) >= order.makerTokenAmount); @@ -118,19 +153,11 @@ contract Exchange is Ownable { } /// @dev Checks if a order can be traded. - /// @param addresses Array of trade's maker, makerToken and takerToken. - /// @param values Array of trade's makerTokenAmount, takerTokenAmount, expires and nonce. + /// @param order Order to check. /// @param signature Signed order along with signature mode. /// @return Boolean if order can be traded - function canTrade(address[3] addresses, uint[4] values, bytes signature) - external - view - returns (bool) - { - OrderLibrary.Order memory order = OrderLibrary.createOrder(addresses, values); - + function canTrade(OrderLibrary.Order order, bytes signature) external view returns (bool) { bytes32 hash = order.hash(); - return canTrade(order, signature, hash); } @@ -142,11 +169,9 @@ contract Exchange is Ownable { } /// @dev Checks how much of an order can be filled. - /// @param addresses Array of trade's maker, makerToken and takerToken. - /// @param values Array of trade's makerTokenAmount, takerTokenAmount, expires and nonce. + /// @param order Order to check. /// @return Amount of the order which can be filled. - function availableAmount(address[3] addresses, uint[4] values) external view returns (uint) { - OrderLibrary.Order memory order = OrderLibrary.createOrder(addresses, values); + function availableAmount(OrderLibrary.Order order) external view returns (uint) { return availableAmount(order, order.hash()); } @@ -183,51 +208,6 @@ contract Exchange is Ownable { return orders[user][hash]; } - /// @dev Executes the actual trade by transferring balances. - /// @param order Order to be traded. - /// @param taker Address of the taker. - /// @param signature Signed order along with signature mode. - /// @param maxFillAmount Maximum amount of the order to be filled. - function trade(OrderLibrary.Order memory order, address taker, bytes signature, uint maxFillAmount) internal { - require(taker != order.maker); - bytes32 hash = order.hash(); - - require(order.makerToken != order.takerToken); - require(canTrade(order, signature, hash)); - - uint fillAmount = SafeMath.min256(maxFillAmount, availableAmount(order, hash)); - - require(roundingPercent(fillAmount, order.takerTokenAmount, order.makerTokenAmount) <= MAX_ROUNDING_PERCENTAGE); - require(vault.balanceOf(order.takerToken, taker) >= fillAmount); - - uint makeAmount = order.makerTokenAmount.mul(fillAmount).div(order.takerTokenAmount); - uint tradeTakerFee = makeAmount.mul(takerFee).div(1 ether); - - if (tradeTakerFee > 0) { - vault.transfer(order.makerToken, order.maker, feeAccount, tradeTakerFee); - } - - vault.transfer(order.takerToken, taker, order.maker, fillAmount); - vault.transfer(order.makerToken, order.maker, taker, makeAmount.sub(tradeTakerFee)); - - fills[hash] = fills[hash].add(fillAmount); - assert(fills[hash] <= order.takerTokenAmount); - - if (subscribed[order.maker]) { - order.maker.call.gas(MAX_HOOK_GAS)(HookSubscriber(order.maker).tradeExecuted.selector, order.takerToken, fillAmount); - } - - emit Traded( - hash, - order.makerToken, - makeAmount, - order.takerToken, - fillAmount, - order.maker, - taker - ); - } - /// @dev Indicates whether or not an certain amount of an order can be traded. /// @param order Order to be traded. /// @param signature Signed order along with signature mode. From 44a9312df022b98e0cdfac346af926a002b4127b Mon Sep 17 00:00:00 2001 From: decanus Date: Wed, 9 May 2018 11:02:20 +0200 Subject: [PATCH 3/4] readded --- contracts/Exchange.sol | 52 +++++++++++++++++++++++------------------- 1 file changed, 29 insertions(+), 23 deletions(-) diff --git a/contracts/Exchange.sol b/contracts/Exchange.sol index 993a4d4..4a01e51 100644 --- a/contracts/Exchange.sol +++ b/contracts/Exchange.sol @@ -5,26 +5,32 @@ import "./Libraries/SafeMath.sol"; import "./Libraries/SignatureValidator.sol"; import "./ExchangeInterface.sol"; import "./Libraries/OrderLibrary.sol"; -import "./Libraries/ExchangeLibrary.sol"; import "./Ownership/Ownable.sol"; import "./Tokens/ERC20.sol"; contract Exchange is Ownable { using OrderLibrary for OrderLibrary.Order; - using ExchangeLibrary for ExchangeLibrary.Exchange; - address constant public ETH = 0x0; + VaultInterface vault; - uint256 constant public MAX_FEE = 5000000000000000; // 0.5% ((0.5 / 100) * 10**18) + uint takerFee; + address feeAccount; - ExchangeLibrary.Exchange public exchange; + mapping (address => mapping (bytes32 => bool)) orders; + mapping (bytes32 => uint) fills; + mapping (bytes32 => bool) cancelled; + mapping (address => bool) subscribed; + address constant public ETH = 0x0; + + uint256 constant public MAX_FEE = 5000000000000000; // 0.5% ((0.5 / 100) * 10**18) + function Exchange(uint _takerFee, address _feeAccount, VaultInterface _vault) public { require(address(_vault) != 0x0); setFees(_takerFee); setFeeAccount(_feeAccount); - exchange.vault = _vault; + vault = _vault; } /// @dev Withdraws tokens accidentally sent to this contract. @@ -41,15 +47,15 @@ contract Exchange is Ownable { /// @dev Subscribes user to trade hooks. function subscribe() external { - require(!exchange.subscribed[msg.sender]); - exchange.subscribed[msg.sender] = true; + require(!subscribed[msg.sender]); + subscribed[msg.sender] = true; emit Subscribed(msg.sender); } /// @dev Unsubscribes user from trade hooks. function unsubscribe() external { - require(exchange.subscribed[msg.sender]); - exchange.subscribed[msg.sender] = false; + require(subscribed[msg.sender]); + subscribed[msg.sender] = false; emit Unsubscribed(msg.sender); } @@ -108,10 +114,10 @@ contract Exchange is Ownable { require(order.makerTokenAmount > 0 && order.takerTokenAmount > 0); bytes32 hash = order.hash(); - require(exchange.fills[hash] < order.takerTokenAmount); - require(!exchange.cancelled[hash]); + require(fills[hash] < order.takerTokenAmount); + require(!cancelled[hash]); - exchange.cancelled[hash] = true; + cancelled[hash] = true; emit Cancelled(hash); } @@ -120,16 +126,16 @@ contract Exchange is Ownable { function order(OrderLibrary.Order order) external { order.maker = msg.sender; - require(exchange.vault.isApproved(order.maker, this)); - require(exchange.vault.balanceOf(order.makerToken, order.maker) >= order.makerTokenAmount); + require(vault.isApproved(order.maker, this)); + require(vault.balanceOf(order.makerToken, order.maker) >= order.makerTokenAmount); require(order.makerToken != order.takerToken); require(order.makerTokenAmount > 0); require(order.takerTokenAmount > 0); bytes32 hash = order.hash(); - require(!exchange.orders[msg.sender][hash]); - exchange.orders[msg.sender][hash] = true; + require(!orders[msg.sender][hash]); + orders[msg.sender][hash] = true; emit Ordered( order.maker, @@ -155,7 +161,7 @@ contract Exchange is Ownable { /// @param subscriber Address of the subscriber. /// @return Boolean if user is subscribed. function isSubscribed(address subscriber) external view returns (bool) { - return exchange.subscribed[subscriber]; + return subscribed[subscriber]; } /// @dev Checks how much of an order can be filled. @@ -169,25 +175,25 @@ contract Exchange is Ownable { /// @param hash Hash of the order. /// @return Amount which was filled. function filled(bytes32 hash) external view returns (uint) { - return exchange.fills[hash]; + return fills[hash]; } /// @dev Sets the taker fee. /// @param _takerFee New taker fee. function setFees(uint _takerFee) public onlyOwner { require(_takerFee <= MAX_FEE); - exchange.takerFee = _takerFee; + takerFee = _takerFee; } /// @dev Sets the account where fees will be transferred to. /// @param _feeAccount Address for the account. function setFeeAccount(address _feeAccount) public onlyOwner { require(_feeAccount != 0x0); - exchange.feeAccount = _feeAccount; + feeAccount = _feeAccount; } function vault() public view returns (VaultInterface) { - return exchange.vault; + return vault; } /// @dev Checks if an order was created on chain. @@ -257,7 +263,7 @@ contract Exchange is Ownable { /// @param target Value to multiply with. /// @return Percentage rounded. function roundingPercent(uint numerator, uint denominator, uint target) internal pure returns (uint) { - // Inspired by https://github.com/0xProject/contracts/blob/1.0.0/contracts/Exchange.sol#L472-L490 + // Inspired by https://github.com/0xProject/contracts/blob/1.0.0/contracts/sol#L472-L490 uint remainder = mulmod(target, numerator, denominator); if (remainder == 0) { return 0; From 3b28375a304df40c1c4b5a510a2d221792df9ec4 Mon Sep 17 00:00:00 2001 From: decanus Date: Wed, 9 May 2018 11:12:07 +0200 Subject: [PATCH 4/4] updates --- contracts/Exchange.sol | 25 +++++++++++++++---------- contracts/ExchangeInterface.sol | 16 ++++++++-------- test/mocks/HookSubscriberMock.sol | 7 ++++--- 3 files changed, 27 insertions(+), 21 deletions(-) diff --git a/contracts/Exchange.sol b/contracts/Exchange.sol index 4a01e51..8f3b01b 100644 --- a/contracts/Exchange.sol +++ b/contracts/Exchange.sol @@ -2,30 +2,35 @@ pragma solidity ^0.4.21; pragma experimental ABIEncoderV2; import "./Libraries/SafeMath.sol"; -import "./Libraries/SignatureValidator.sol"; import "./ExchangeInterface.sol"; +import "./HookSubscriber.sol"; import "./Libraries/OrderLibrary.sol"; import "./Ownership/Ownable.sol"; import "./Tokens/ERC20.sol"; -contract Exchange is Ownable { +import "@DexyProject/signature-validator/contracts/SignatureValidator.sol"; +contract Exchange is ExchangeInterface, Ownable { + + using SafeMath for *; using OrderLibrary for OrderLibrary.Order; - VaultInterface vault; + VaultInterface public vault; - uint takerFee; - address feeAccount; + uint public takerFee; + address public feeAccount; - mapping (address => mapping (bytes32 => bool)) orders; - mapping (bytes32 => uint) fills; - mapping (bytes32 => bool) cancelled; - mapping (address => bool) subscribed; + mapping (address => mapping (bytes32 => bool)) public orders; + mapping (bytes32 => uint) public fills; + mapping (bytes32 => bool) public cancelled; + mapping (address => bool) public subscribed; address constant public ETH = 0x0; uint256 constant public MAX_FEE = 5000000000000000; // 0.5% ((0.5 / 100) * 10**18) - + uint256 constant public MAX_ROUNDING_PERCENTAGE = 1000; // 0.1% + uint256 constant public MAX_HOOK_GAS = 40000; // enough for a storage write and some accounting logic + function Exchange(uint _takerFee, address _feeAccount, VaultInterface _vault) public { require(address(_vault) != 0x0); setFees(_takerFee); diff --git a/contracts/ExchangeInterface.sol b/contracts/ExchangeInterface.sol index 9c71b1e..dd99535 100644 --- a/contracts/ExchangeInterface.sol +++ b/contracts/ExchangeInterface.sol @@ -33,14 +33,14 @@ interface ExchangeInterface { function subscribe() external; function unsubscribe() external; - function trade(address[3] addresses, uint[4] values, bytes signature, uint maxFillAmount) external; - function cancel(address[3] addresses, uint[4] values) external; - function order(address[2] addresses, uint[4] values) external; - - function canTrade(address[3] addresses, uint[4] values, bytes signature) - external - view - returns (bool); +// function trade(address[3] addresses, uint[4] values, bytes signature, uint maxFillAmount) external; +// function cancel(address[3] addresses, uint[4] values) external; +// function order(address[2] addresses, uint[4] values) external; + +// function canTrade(address[3] addresses, uint[4] values, bytes signature) +// external +// view +// returns (bool); function isSubscribed(address subscriber) external view returns (bool); function availableAmount(address[3] addresses, uint[4] values) external view returns (uint); diff --git a/test/mocks/HookSubscriberMock.sol b/test/mocks/HookSubscriberMock.sol index 294e2e0..bafb8d5 100644 --- a/test/mocks/HookSubscriberMock.sol +++ b/test/mocks/HookSubscriberMock.sol @@ -1,6 +1,7 @@ pragma solidity ^0.4.18; -import "./../../contracts/ExchangeInterface.sol"; +import "./../../contracts/Exchange.sol"; +import "./../../contracts/Libraries/OrderLibrary.sol"; contract HookSubscriberMock { @@ -10,10 +11,10 @@ contract HookSubscriberMock { tokens[token] += amount; } - function createOrder(address[2] addresses, uint[4] values, ExchangeInterface exchange) external { + function createOrder(address[2] addresses, uint[4] values, Exchange exchange) external { exchange.subscribe(); exchange.vault().approve(exchange); exchange.vault().deposit(addresses[0], values[0]); - exchange.order(addresses, values); + exchange.order(OrderLibrary.createOrder([msg.sender, addresses[0], addresses[1]], values)); } }