From 79eff010a16f0d733fa3a01b8d026f7e52871981 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Viktor=20Szathm=C3=A1ry?= Date: Wed, 14 Feb 2018 19:49:46 +0100 Subject: [PATCH 1/3] exchange: drop addedTime --- contracts/Exchange.sol | 16 +++++++--------- test/helpers/exchangeTestHelpers.js | 15 +++++---------- 2 files changed, 12 insertions(+), 19 deletions(-) diff --git a/contracts/Exchange.sol b/contracts/Exchange.sol index b1367daf..3497ec31 100644 --- a/contracts/Exchange.sol +++ b/contracts/Exchange.sol @@ -5,7 +5,6 @@ TODO: - deduct fee - consider take funcs (frequent rate changes with takeBuyToken? send more and send back remainder?) - - uint32 for addedTime? */ pragma solidity 0.4.19; @@ -21,7 +20,6 @@ contract Exchange { struct Order { uint index; address maker; - uint addedTime; // tokens per ether uint price; @@ -57,7 +55,7 @@ contract Exchange { require(price > 0); require(msg.value > 0); - orderId = buyTokenOrders.push(Order(activeBuyOrders.length, msg.sender, now, price, msg.value)) - 1; + orderId = buyTokenOrders.push(Order(activeBuyOrders.length, msg.sender, price, msg.value)) - 1; activeBuyOrders.push(orderId); NewOrder(orderId, msg.sender, price, 0, msg.value); @@ -121,20 +119,20 @@ contract Exchange { } // returns CHUNK_SIZE orders starting from offset - // orders are encoded as [id, maker, addedTime, price, amount] - function getActiveBuyOrders(uint offset) external view returns (uint[5][CHUNK_SIZE] response) { + // orders are encoded as [id, maker, price, amount] + function getActiveBuyOrders(uint offset) external view returns (uint[4][CHUNK_SIZE] response) { for (uint8 i = 0; i < CHUNK_SIZE && i + offset < activeBuyOrders.length; i++) { uint orderId = activeBuyOrders[offset + i]; Order storage order = buyTokenOrders[orderId]; - response[i] = [orderId, uint(order.maker), order.addedTime, order.price, order.amount]; + response[i] = [orderId, uint(order.maker), order.price, order.amount]; } } - function getActiveSellOrders(uint offset) external view returns (uint[5][CHUNK_SIZE] response) { + function getActiveSellOrders(uint offset) external view returns (uint[4][CHUNK_SIZE] response) { for (uint8 i = 0; i < CHUNK_SIZE && i + offset < activeSellOrders.length; i++) { uint orderId = activeSellOrders[offset + i]; Order storage order = sellTokenOrders[orderId]; - response[i] = [orderId, uint(order.maker), order.addedTime, order.price, order.amount]; + response[i] = [orderId, uint(order.maker), order.price, order.amount]; } } @@ -193,7 +191,7 @@ contract Exchange { require(price > 0); require(tokenAmount > 0); - orderId = sellTokenOrders.push(Order(activeSellOrders.length, maker, now, price, tokenAmount)) - 1; + orderId = sellTokenOrders.push(Order(activeSellOrders.length, maker, price, tokenAmount)) - 1; activeSellOrders.push(orderId); NewOrder(orderId, maker, price, tokenAmount, 0); diff --git a/test/helpers/exchangeTestHelpers.js b/test/helpers/exchangeTestHelpers.js index ae65f405..783afe1c 100644 --- a/test/helpers/exchangeTestHelpers.js +++ b/test/helpers/exchangeTestHelpers.js @@ -93,7 +93,6 @@ async function newOrder(testInstance, order) { assert.equal(state.sellCount, expSellCount, "sellCount should be set"); assert.equal(actualOrder.id, order.id, "orderId should be set in contract's order array"); assert.equal(actualOrder.maker, order.maker, "maker should be the userAccount in contract's order array"); - // TODO: assert order.addedTime assert.equal(actualOrder.price, order.price, "price should be set in contract's order array"); assert.equal( actualOrder.amount.toString(), @@ -327,21 +326,19 @@ function parseOrder(order) { return { index: order[0], maker: order[1], - addedTime: order[2].toNumber(), - price: order[3].toNumber(), - amount: order[4] + price: order[2].toNumber(), + amount: order[3] }; } function parseOrders(orderType, orders) { - return orders.filter(order => order[4].toNumber() != 0).map(function(order) { + return orders.filter(order => order[3].toNumber() != 0).map(function(order) { return { orderType: orderType, id: order[0].toNumber(), maker: "0x" + order[1].toString(16), - addedTime: order[2].toNumber(), - price: order[3].toNumber(), - amount: order[4] + price: order[2].toNumber(), + amount: order[3] }; }); } @@ -375,7 +372,6 @@ async function printOrderBook(_limit) { const order = await getSellTokenOrder(i); console.log( `SELL token: ACE/ETH: ${order.price / 10000} amount: ${order.amount.toString() / 10000} ACE` + - ` ${moment.unix(order.addedTime).format("HH:mm:ss")}` + ` orderIdx: ${i} orderId: ${order.id} acc: ${order.maker}` ); } @@ -384,7 +380,6 @@ async function printOrderBook(_limit) { const order = await getBuyTokenOrder(i); console.log( ` BUY token: ACE/EUR: ${order.price / 10000} amount: ${web3.fromWei(order.amount)} ETH` + - ` ${moment.unix(order.addedTime).format("HH:mm:ss")}` + ` orderIdx: ${i} orderId: ${order.id} acc: ${order.maker}` ); } From be9ac72108bbaf76483f2b67072034db91338b75 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Viktor=20Szathm=C3=A1ry?= Date: Wed, 14 Feb 2018 20:37:05 +0100 Subject: [PATCH 2/3] exchange: narrower types to reduce storage use --- contracts/Exchange.sol | 48 ++++++++++++++++++++++-------------------- 1 file changed, 25 insertions(+), 23 deletions(-) diff --git a/contracts/Exchange.sol b/contracts/Exchange.sol index 3497ec31..74069ba2 100644 --- a/contracts/Exchange.sol +++ b/contracts/Exchange.sol @@ -18,11 +18,11 @@ contract Exchange { uint public constant CHUNK_SIZE = 100; struct Order { - uint index; + uint64 index; address maker; // tokens per ether - uint price; + uint32 price; // buy order: amount in wei // sell order: token amount @@ -32,42 +32,43 @@ contract Exchange { Order[] public buyTokenOrders; Order[] public sellTokenOrders; - uint[] private activeBuyOrders; - uint[] private activeSellOrders; + uint64[] private activeBuyOrders; + uint64[] private activeSellOrders; /* used to stop executing matchMultiple when running out of gas. actual is much less, just leaving enough matchMultipleOrders() to finish TODO: fine tune & test it*/ uint32 private constant ORDER_MATCH_WORST_GAS = 100000; - event NewOrder(uint indexed orderId, address indexed maker, uint price, uint tokenAmount, + event NewOrder(uint64 indexed orderId, address indexed maker, uint32 price, uint tokenAmount, uint weiAmount); - event OrderFill(address indexed tokenBuyer, address indexed tokenSeller, uint buyTokenOrderId, - uint sellTokenOrderId, uint price, uint weiAmount, uint tokenAmount); + event OrderFill(address indexed tokenBuyer, address indexed tokenSeller, uint64 buyTokenOrderId, + uint64 sellTokenOrderId, uint32 price, uint weiAmount, uint tokenAmount); - event CancelledOrder(uint indexed orderId, address indexed maker, uint tokenAmount, uint weiAmount); + event CancelledOrder(uint64 indexed orderId, address indexed maker, uint tokenAmount, uint weiAmount); function Exchange(AugmintTokenInterface _augmintToken) public { augmintToken = _augmintToken; } - function placeBuyTokenOrder(uint price) external payable returns (uint orderId) { + function placeBuyTokenOrder(uint32 price) external payable returns (uint64 orderId) { require(price > 0); require(msg.value > 0); - orderId = buyTokenOrders.push(Order(activeBuyOrders.length, msg.sender, price, msg.value)) - 1; + uint64 index = uint64(activeBuyOrders.length); + orderId = uint64(buyTokenOrders.push(Order(index, msg.sender, price, msg.value)) - 1); activeBuyOrders.push(orderId); NewOrder(orderId, msg.sender, price, 0, msg.value); } /* this function requires previous approval to transfer tokens */ - function placeSellTokenOrder(uint price, uint tokenAmount) external returns (uint orderId) { + function placeSellTokenOrder(uint32 price, uint tokenAmount) external returns (uint orderId) { augmintToken.transferFrom(msg.sender, this, tokenAmount); return _placeSellTokenOrder(msg.sender, price, tokenAmount); } - function cancelBuyTokenOrder(uint buyTokenId) external { + function cancelBuyTokenOrder(uint64 buyTokenId) external { Order storage order = buyTokenOrders[buyTokenId]; require(order.maker == msg.sender); @@ -80,7 +81,7 @@ contract Exchange { CancelledOrder(buyTokenId, msg.sender, 0, amount); } - function cancelSellTokenOrder(uint sellTokenId) external { + function cancelSellTokenOrder(uint64 sellTokenId) external { Order storage order = sellTokenOrders[sellTokenId]; require(order.maker == msg.sender); @@ -97,7 +98,7 @@ contract Exchange { trade price meets in the middle reverts if any of the orders have been removed */ - function matchOrders(uint buyTokenId, uint sellTokenId) external { + function matchOrders(uint64 buyTokenId, uint64 sellTokenId) external { _fillOrder(buyTokenId, sellTokenId); } @@ -105,7 +106,7 @@ contract Exchange { Runs as long as gas is available for the call. Stops if any match is invalid (case when any of the orders removed after client generated the match list sent) */ - function matchMultipleOrders(uint[] buyTokenIds, uint[] sellTokenIds) external returns(uint matchCount) { + function matchMultipleOrders(uint64[] buyTokenIds, uint64[] sellTokenIds) external returns(uint matchCount) { uint len = buyTokenIds.length; require(len == sellTokenIds.length); for (uint i = 0; i < len && msg.gas > ORDER_MATCH_WORST_GAS; i++) { @@ -144,17 +145,17 @@ contract Exchange { */ function transferNotification(address maker, uint tokenAmount, uint price) public { require(msg.sender == address(augmintToken)); - _placeSellTokenOrder(maker, price, tokenAmount); + _placeSellTokenOrder(maker, uint32(price), tokenAmount); } - function _fillOrder(uint buyTokenId, uint sellTokenId) private { + function _fillOrder(uint64 buyTokenId, uint64 sellTokenId) private { Order storage buy = buyTokenOrders[buyTokenId]; Order storage sell = sellTokenOrders[sellTokenId]; require(buy.price >= sell.price); // meet in the middle - uint price = buy.price.add(sell.price).div(2); + uint price = uint(buy.price).add(sell.price).div(2); uint sellWei = sell.amount.mul(1 ether).div(price); @@ -182,16 +183,17 @@ contract Exchange { sell.maker.transfer(tradedWei); OrderFill(buy.maker, sell.maker, buyTokenId, - sellTokenId, price, tradedWei, tradedTokens); + sellTokenId, uint32(price), tradedWei, tradedTokens); } - function _placeSellTokenOrder(address maker, uint price, uint tokenAmount) - private returns (uint orderId) { + function _placeSellTokenOrder(address maker, uint32 price, uint tokenAmount) + private returns (uint64 orderId) { require(price > 0); require(tokenAmount > 0); - orderId = sellTokenOrders.push(Order(activeSellOrders.length, maker, price, tokenAmount)) - 1; + uint64 index = uint64(activeSellOrders.length); + orderId = uint64(sellTokenOrders.push(Order(index, maker, price, tokenAmount)) - 1); activeSellOrders.push(orderId); NewOrder(orderId, maker, price, tokenAmount, 0); @@ -205,7 +207,7 @@ contract Exchange { _removeOrder(activeSellOrders, order.index); } - function _removeOrder(uint[] storage orders, uint index) private { + function _removeOrder(uint64[] storage orders, uint64 index) private { if (index < orders.length - 1) { orders[index] = orders[orders.length - 1]; } From 45712f1d78f29be6ac15e43a0c337983d69d08ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Viktor=20Szathm=C3=A1ry?= Date: Wed, 14 Feb 2018 20:52:30 +0100 Subject: [PATCH 3/3] exchange: use same sequence for both buy/sell orders (so they can be ordered by time) --- contracts/Exchange.sol | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/contracts/Exchange.sol b/contracts/Exchange.sol index 74069ba2..c545ab00 100644 --- a/contracts/Exchange.sol +++ b/contracts/Exchange.sol @@ -29,8 +29,9 @@ contract Exchange { uint amount; } - Order[] public buyTokenOrders; - Order[] public sellTokenOrders; + uint64 public orderCount; + mapping(uint64 => Order) public buyTokenOrders; + mapping(uint64 => Order) public sellTokenOrders; uint64[] private activeBuyOrders; uint64[] private activeSellOrders; @@ -55,8 +56,8 @@ contract Exchange { require(price > 0); require(msg.value > 0); - uint64 index = uint64(activeBuyOrders.length); - orderId = uint64(buyTokenOrders.push(Order(index, msg.sender, price, msg.value)) - 1); + orderId = ++orderCount; + buyTokenOrders[orderId] = Order(uint64(activeBuyOrders.length), msg.sender, price, msg.value); activeBuyOrders.push(orderId); NewOrder(orderId, msg.sender, price, 0, msg.value); @@ -123,7 +124,7 @@ contract Exchange { // orders are encoded as [id, maker, price, amount] function getActiveBuyOrders(uint offset) external view returns (uint[4][CHUNK_SIZE] response) { for (uint8 i = 0; i < CHUNK_SIZE && i + offset < activeBuyOrders.length; i++) { - uint orderId = activeBuyOrders[offset + i]; + uint64 orderId = activeBuyOrders[offset + i]; Order storage order = buyTokenOrders[orderId]; response[i] = [orderId, uint(order.maker), order.price, order.amount]; } @@ -131,7 +132,7 @@ contract Exchange { function getActiveSellOrders(uint offset) external view returns (uint[4][CHUNK_SIZE] response) { for (uint8 i = 0; i < CHUNK_SIZE && i + offset < activeSellOrders.length; i++) { - uint orderId = activeSellOrders[offset + i]; + uint64 orderId = activeSellOrders[offset + i]; Order storage order = sellTokenOrders[orderId]; response[i] = [orderId, uint(order.maker), order.price, order.amount]; } @@ -192,8 +193,8 @@ contract Exchange { require(price > 0); require(tokenAmount > 0); - uint64 index = uint64(activeSellOrders.length); - orderId = uint64(sellTokenOrders.push(Order(index, maker, price, tokenAmount)) - 1); + orderId = ++orderCount; + sellTokenOrders[orderId] = Order(uint64(activeSellOrders.length), maker, price, tokenAmount); activeSellOrders.push(orderId); NewOrder(orderId, maker, price, tokenAmount, 0);