From 9d831791f7a457b0028dae87425355e67091b5a0 Mon Sep 17 00:00:00 2001 From: Harnick Khera Date: Sun, 9 Dec 2018 22:48:52 +0000 Subject: [PATCH 1/4] WIP Small Gas Optomisations Change Log: Assemblified subring Check ~= 100 gas reduction Removed all add( x , 0) instances from assembly operations as they are redundant/wasting gas Replaced clear multiplication instances with answer ie instances with 32*5 with 160 ~= 250 gas reduction Refactored calculation of memory required by lists, using assembly and a new calculate function. this stops an extra for loop and ring index calculations ~= 300 gas reduction Assemblified the Validity Check ~= 700 gas reduction orderhelper - ValidateAllInfo is now all assembly ~= 750 gas reduction --- contracts/helper/MiningHelper.sol | 2 +- contracts/helper/OrderHelper.sol | 51 ++++++++------- contracts/helper/RingHelper.sol | 62 +++++++++++++----- contracts/impl/ExchangeDeserializer.sol | 24 +++---- contracts/impl/FeeHolder.sol | 2 +- contracts/impl/OrderBook.sol | 48 +++++++------- contracts/impl/RingSubmitter.sol | 85 +++++++++++++++---------- contracts/impl/TradeDelegate.sol | 2 +- contracts/impl/TradeHistory.sol | 4 +- 9 files changed, 168 insertions(+), 112 deletions(-) diff --git a/contracts/helper/MiningHelper.sol b/contracts/helper/MiningHelper.sol index 2205f2e..14159cb 100644 --- a/contracts/helper/MiningHelper.sol +++ b/contracts/helper/MiningHelper.sol @@ -69,7 +69,7 @@ library MiningHelper { // Store data back to front to allow overwriting data at the front because of padding mstore(add(data, 40), ringHashes) // ringHashes mstore(sub(add(data, 20), 12), mload(add(mining, 32))) // mining.miner - mstore(sub(data, 12), mload(add(mining, 0))) // mining.feeRecipient + mstore(sub(data, 12), mload(mining)) // mining.feeRecipient hash := keccak256(data, 72) // 20 + 20 + 32 } mining.hash = hash; diff --git a/contracts/helper/OrderHelper.sol b/contracts/helper/OrderHelper.sol index 55a5b0a..c9e8864 100644 --- a/contracts/helper/OrderHelper.sol +++ b/contracts/helper/OrderHelper.sol @@ -133,7 +133,7 @@ library OrderHelper { let transferDataSHash := keccak256(add(transferDataS, 32), mload(transferDataS)) let ptr := mload(64) - mstore(add(ptr, 0), _EIP712_ORDER_SCHEMA_HASH) // EIP712_ORDER_SCHEMA_HASH + mstore(ptr, _EIP712_ORDER_SCHEMA_HASH) // EIP712_ORDER_SCHEMA_HASH mstore(add(ptr, 32), mload(add(order, 128))) // order.amountS mstore(add(ptr, 64), mload(add(order, 160))) // order.amountB mstore(add(ptr, 96), mload(add(order, 640))) // order.feeAmount @@ -160,7 +160,7 @@ library OrderHelper { mstore(add(ptr, 768), transferDataSHash) // keccak256(order.transferDataS) let message := keccak256(ptr, 800) // 25 * 32 - mstore(add(ptr, 0), 0x1901) // EIP191_HEADER + mstore(ptr, 0x1901) // EIP191_HEADER mstore(add(ptr, 32), _EIP712_DOMAIN_HASH) // EIP712_DOMAIN_HASH mstore(add(ptr, 64), message) // message hash := keccak256(add(ptr, 30), 66) // 2 + 32 + 32 @@ -214,27 +214,34 @@ library OrderHelper { view { bool valid = true; - valid = valid && (order.version == 0); // unsupported order version - valid = valid && (order.owner != 0x0); // invalid order owner - valid = valid && (order.tokenS != 0x0); // invalid order tokenS - valid = valid && (order.tokenB != 0x0); // invalid order tokenB - valid = valid && (order.amountS != 0); // invalid order amountS - valid = valid && (order.amountB != 0); // invalid order amountB - valid = valid && (order.feeToken != 0x0); // invalid fee token - - valid = valid && (order.tokenSFeePercentage < ctx.feePercentageBase); // invalid tokenS percentage - valid = valid && (order.tokenBFeePercentage < ctx.feePercentageBase); // invalid tokenB percentage - valid = valid && (order.walletSplitPercentage <= 100); // invalid wallet split percentage - - // We only support ERC20 for now - valid = valid && (order.tokenTypeS == Data.TokenType.ERC20 && order.trancheS == 0x0); - valid = valid && (order.tokenTypeB == Data.TokenType.ERC20 && order.trancheB == 0x0); - valid = valid && (order.tokenTypeFee == Data.TokenType.ERC20); - valid = valid && (order.transferDataS.length == 0); - - valid = valid && (order.validSince <= now); // order is too early to match + uint feePercentageBase = ctx.feePercentageBase; + uint dataLength = order.transferDataS.length; + Data.TokenType ErcType = Data.TokenType.ERC20; + assembly{ - order.valid = order.valid && valid; + valid := and(valid, eq(mload(order), 0)) // unsupported order version + valid := and(valid, not(eq(mload(add(order, 32)), 0x0))) // invalid order owner + valid := and(valid, not(eq(mload(add(order, 64)), 0x0))) // invalid order tokenS + valid := and(valid, not(eq(mload(add(order, 96)), 0x0))) // invalid order tokenB + valid := and(valid, not(eq(mload(add(order, 128)), 0))) // invalid order amountS + valid := and(valid, not(eq(mload(add(order, 160)), 0x0))) // invalid order amountB + valid := and(valid, not(eq(mload(add(order, 608)), 0x0))) // invalid fee token + + + valid := and(valid, lt(mload(add(order, 704)), feePercentageBase)) // invalid tokenS percentage + valid := and(valid, lt(mload(add(order, 736)), feePercentageBase)) // invalid tokenB percentage + valid := and(valid, or(lt(mload(add(order, 800)), 100), eq(mload(add(order, 800)), 100))) // invalid wallet split percentage + + // We only support ERC20 for now + valid := and(valid, and(eq(mload(add(order, 1024)), ErcType), eq(mload(add(order, 1120)), 0x0))) // valid && (order.tokenTypeS == Data.TokenType.ERC20 && order.trancheS == 0x0) + valid := and(valid, and(eq(mload(add(order, 1056)), ErcType), eq(mload(add(order, 1152)), 0x0))) // valid && (order.tokenTypeB == Data.TokenType.ERC20 && order.trancheB == 0x0) + valid := and(valid, eq(mload(add(order, 1088)), ErcType)) //valid && (order.tokenTypeFee == Data.TokenType.ERC20); + valid := and(valid, eq(dataLength, 0)) // valid = valid && (order.transferDataS.length == 0); + + valid := and(valid, or(lt(mload(add(order, 192)), timestamp), eq(mload(add(order, 192)), timestamp))) // order is too early to match + + mstore(add(order, 992), and(valid, mload(add(order, 992)))) //order.valid = order.valid && valid; + } validateUnstableInfo(order, ctx); } diff --git a/contracts/helper/RingHelper.sol b/contracts/helper/RingHelper.sol index 1298276..285f00b 100644 --- a/contracts/helper/RingHelper.sol +++ b/contracts/helper/RingHelper.sol @@ -174,11 +174,25 @@ library RingHelper { internal pure { - ring.valid = ring.valid && (ring.size > 1 && ring.size <= 8); // invalid ring size - for (uint i = 0; i < ring.size; i++) { - uint prev = (i + ring.size - 1) % ring.size; - ring.valid = ring.valid && ring.participations[i].order.valid; - ring.valid = ring.valid && ring.participations[i].order.tokenS == ring.participations[prev].order.tokenB; + uint ringSize = ring.size; + bool valid = ring.valid; + assembly { + let prev := 0 + valid := and(valid, and(gt(ringSize,1), lt(ringSize, 8))) // ring.valid && (ring.size > 1 && ring.size <= 8) + + for { let i := 0 } lt(i, ringSize) { i := add(i, 1) } { // for (uint i = 0; i < ring.size; i++) + + prev := mod(sub(add(i, ringSize), 1), ringSize) //uint prev = (i + ring.size - 1) % ring.size; + let participations := mload(add(ring, 32)) + let participation := mload(add(participations, add(32, mul(i, 32)))) // participations[i] + let prevParticipation := mload(add(participations, add(32, mul(prev, 32)))) // participations[prev] + let order := mload(participation) + let prevOrder := mload(prevParticipation) + //ring.valid && ring.participations[i].order.valid && ring.participations[i].order.tokenS == ring.participations[prev].order.tokenB: + valid := and(and(valid, mload(add(order, 992))),eq(mload(add(order, 64)), mload(add(prevOrder, 96)))) + + } + mstore(add(ring, 128), valid) //update ring.valid } } @@ -188,11 +202,29 @@ library RingHelper { internal pure { - for (uint i = 0; i < ring.size - 1; i++) { - address tokenS = ring.participations[i].order.tokenS; - for (uint j = i + 1; j < ring.size; j++) { - ring.valid = ring.valid && (tokenS != ring.participations[j].order.tokenS); + uint ringSize = ring.size; + bool valid = ring.valid; + //address tokenS; + //address tokenJ; + assembly { + let participations := mload(add(ring, 32)) + + for { let i := 0 } lt(i, sub(ringSize,1)) { i := add(i, 1) } { // for (uint i = 0; i < ring.size - 1; i++) { + + let participation := mload(add(participations, add(32, mul(i, 32)))) // participations[i] + let order := mload(participation) + let tokenS := mload(add(order, 64)) //address tokenS = ring.participations[i].order.tokenS; + + for {let j := add(i,1) } lt(j, ringSize) { j := add(j, 1) } { // for (uint j = i + 1; j < ring.size; j++) { + + let participationj := mload(add(participations, add(32, mul(j, 32)))) // participations[j] + let orderj := mload(participationj) + let tokenJ := mload(add(orderj, 64)) //address tokenJ = ring.participations[j].order.tokenS; + valid := and(valid, not(eq(tokenS, tokenJ))) // ring.valid && (tokenS != ring.participations[j].order.tokenS); + } } + + mstore(add(ring, 128), valid) //update ring.valid } } @@ -240,7 +272,7 @@ library RingHelper { returns (uint fill) { uint ringSize = ring.size; - uint fillSize = 8 * 32; + uint fillSize = 256; //8 * 32 assembly { fill := destPtr let participations := mload(add(ring, 32)) // ring.participations @@ -263,7 +295,7 @@ library RingHelper { mload(add(participation, 224)) // participation.rebateFeeB ) - mstore(add(fill, 0), mload(add(order, 864))) // order.hash + mstore(fill, mload(add(order, 864))) // order.hash mstore(add(fill, 32), mload(add(order, 32))) // order.owner mstore(add(fill, 64), mload(add(order, 64))) // order.tokenS mstore(add(fill, 96), mload(add(participation, 256))) // participation.fillAmountS @@ -395,7 +427,7 @@ library RingHelper { // Try to find an existing fee payment of the same token to the same owner let addNew := 1 for { let p := data } lt(p, ptr) { p := add(p, 128) } { - let dataToken := mload(add(p, 0)) + let dataToken := mload(p) let dataFrom := mload(add(p, 32)) let dataTo := mload(add(p, 64)) // if(token == dataToken && from == dataFrom && to == dataTo) @@ -415,7 +447,7 @@ library RingHelper { } // Add a new transfer if eq(addNew, 1) { - mstore(add(ptr, 0), token) + mstore(ptr, token) mstore(add(ptr, 32), from) mstore(add(ptr, 64), to) mstore(add(ptr, 96), amount) @@ -666,7 +698,7 @@ library RingHelper { // Try to find an existing fee payment of the same token to the same owner let addNew := 1 for { let p := data } lt(p, ptr) { p := add(p, 96) } { - let dataToken := mload(add(p, 0)) + let dataToken := mload(p) let dataOwner := mload(add(p, 32)) // if(token == dataToken && owner == dataOwner) if and(eq(token, dataToken), eq(owner, dataOwner)) { @@ -685,7 +717,7 @@ library RingHelper { } // Add a new fee payment if eq(addNew, 1) { - mstore(add(ptr, 0), token) + mstore(ptr, token) mstore(add(ptr, 32), owner) mstore(add(ptr, 64), amount) ptr := add(ptr, 96) diff --git a/contracts/impl/ExchangeDeserializer.sol b/contracts/impl/ExchangeDeserializer.sol index ea5e966..fc7a2cd 100644 --- a/contracts/impl/ExchangeDeserializer.sol +++ b/contracts/impl/ExchangeDeserializer.sol @@ -88,9 +88,9 @@ library ExchangeDeserializer { mstore(add(data, 20), origin) // mining.feeRecipient - offset := mul(and(mload(add(tablesPtr, 0)), 0xFFFF), 4) + offset := mul(and(mload(tablesPtr), 0xFFFF), 4) mstore( - add(mining, 0), + mining, mload(add(add(data, 20), offset)) ) @@ -131,7 +131,7 @@ library ExchangeDeserializer { returns (Data.Order[] orders) { bytes memory emptyBytes = new bytes(0); - uint orderStructSize = 38 * 32; + uint orderStructSize = 1216; //38 * 32 // Memory for orders length + numOrders order pointers uint arrayDataSize = (1 + numOrders) * 32; Data.Spendable[] memory spendableList = new Data.Spendable[](numSpendables); @@ -140,7 +140,7 @@ library ExchangeDeserializer { assembly { // Allocate memory for all orders orders := mload(0x40) - mstore(add(orders, 0), numOrders) // orders.length + mstore(orders, numOrders) // orders.length // Reserve the memory for the orders array mstore(0x40, add(orders, add(arrayDataSize, mul(orderStructSize, numOrders)))) @@ -151,9 +151,9 @@ library ExchangeDeserializer { mstore(add(orders, mul(add(1, i), 32)), order) // order.version - offset := and(mload(add(tablesPtr, 0)), 0xFFFF) + offset := and(mload(tablesPtr), 0xFFFF) mstore( - add(order, 0), + order, offset ) @@ -412,13 +412,13 @@ library ExchangeDeserializer { returns (Data.Ring[] rings) { uint ringsArrayDataSize = (1 + numRings) * 32; - uint ringStructSize = 5 * 32; - uint participationStructSize = 10 * 32; + uint ringStructSize = 160; //5 * 32 + uint participationStructSize = 320; //10 * 32 assembly { // Allocate memory for all rings rings := mload(0x40) - mstore(add(rings, 0), numRings) // rings.length + mstore(rings, numRings) // rings.length // Reserve the memory for the rings array mstore(0x40, add(rings, add(ringsArrayDataSize, mul(ringStructSize, numRings)))) @@ -439,14 +439,14 @@ library ExchangeDeserializer { // Allocate memory for all participations let participations := mload(0x40) - mstore(add(participations, 0), ringSize) // participations.length + mstore(participations, ringSize) // participations.length // Memory for participations length + ringSize participation pointers let participationsData := add(participations, mul(add(1, ringSize), 32)) // Reserve the memory for the participations mstore(0x40, add(participationsData, mul(participationStructSize, ringSize))) // Initialize ring properties - mstore(add(ring, 0), ringSize) // ring.size + mstore(ring, ringSize) // ring.size mstore(add(ring, 32), participations) // ring.participations mstore(add(ring, 64), 0) // ring.hash mstore(add(ring, 96), 0) // ring.minerFeesToOrdersPercentage @@ -468,7 +468,7 @@ library ExchangeDeserializer { // participation.order mstore( - add(participation, 0), + participation, mload(add(orders, mul(add(orderIndex, 1), 32))) ) diff --git a/contracts/impl/FeeHolder.sol b/contracts/impl/FeeHolder.sol index edfd2a0..d51f5fa 100644 --- a/contracts/impl/FeeHolder.sol +++ b/contracts/impl/FeeHolder.sol @@ -60,7 +60,7 @@ contract FeeHolder is IFeeHolder, NoDefaultFunc { uint end = start + length * 32; for (uint p = start; p < end; p += 96) { assembly { - token := calldataload(add(p, 0)) + token := calldataload(p) owner := calldataload(add(p, 32)) value := calldataload(add(p, 64)) } diff --git a/contracts/impl/OrderBook.sol b/contracts/impl/OrderBook.sol index 97c77e5..a15d274 100644 --- a/contracts/impl/OrderBook.sol +++ b/contracts/impl/OrderBook.sol @@ -38,47 +38,47 @@ contract OrderBook is IOrderBook, NoDefaultFunc { external returns (bytes32) { - require(data.length >= 23 * 32, INVALID_SIZE); + require(data.length >= 736, INVALID_SIZE); //23 * 32 Data.Order memory order = Data.Order( 0, // version - address(data.bytesToUint(0 * 32)), // owner - address(data.bytesToUint(1 * 32)), // tokenS - address(data.bytesToUint(2 * 32)), // tokenB - data.bytesToUint(3 * 32), // amountS - data.bytesToUint(4 * 32), // amountB - data.bytesToUint(5 * 32), // validSince + address(data.bytesToUint(0)), //0 * 32 // owner + address(data.bytesToUint(32)), //1 * 32 // tokenS + address(data.bytesToUint(64)), //2 * 32 // tokenB + data.bytesToUint(96), //3 * 32 // amountS + data.bytesToUint(128), //4 * 32 // amountB + data.bytesToUint(160), //5 * 32 // validSince Data.Spendable(true, 0, 0), Data.Spendable(true, 0, 0), 0x0, - address(data.bytesToUint(6 * 32)), // broker + address(data.bytesToUint(192)), //6 * 32 // broker Data.Spendable(true, 0, 0), Data.Spendable(true, 0, 0), - address(data.bytesToUint(7 * 32)), // orderInterceptor - address(data.bytesToUint(8 * 32)), // wallet - uint(data.bytesToUint(9 * 32)), // validUtil + address(data.bytesToUint(224)), //7 * 32 // orderInterceptor + address(data.bytesToUint(256)), //8 * 32 // wallet + uint(data.bytesToUint(288)), //9 * 32 // validUtil new bytes(0), new bytes(0), - bool(data.bytesToUint(10 * 32) > 0), // allOrNone - address(data.bytesToUint(11 * 32)), // feeToken - data.bytesToUint(12 * 32), // feeAmount + bool(data.bytesToUint(320) > 0), //10 * 32 // allOrNone + address(data.bytesToUint(352)),//11 * 32 // feeToken + data.bytesToUint(384), //12 * 32 // feeAmount 0, - uint16(data.bytesToUint(13 * 32)), // tokenSFeePercentage - uint16(data.bytesToUint(14 * 32)), // tokenBFeePercentage - address(data.bytesToUint(15 * 32)), // tokenRecipient - uint16(data.bytesToUint(16 * 32)), // walletSplitPercentage + uint16(data.bytesToUint(416)), //13 * 32 // tokenSFeePercentage + uint16(data.bytesToUint(448)), //14 * 32 // tokenBFeePercentage + address(data.bytesToUint(480)), //15 * 32 // tokenRecipient + uint16(data.bytesToUint(512)), //16 * 32 // walletSplitPercentage false, bytes32(0x0), 0x0, 0, 0, true, - Data.TokenType(data.bytesToUint(17 * 32)), // tokenTypeS - Data.TokenType(data.bytesToUint(18 * 32)), // tokenTypeB - Data.TokenType(data.bytesToUint(19 * 32)), // tokenTypeFee - data.bytesToBytes32(20 * 32), // trancheS - data.bytesToBytes32(21 * 32), // trancheB - data.subBytes(22 * 32) // transferDataS + Data.TokenType(data.bytesToUint(544)), //17 * 32 // tokenTypeS + Data.TokenType(data.bytesToUint(576)), //18 * 32 // tokenTypeB + Data.TokenType(data.bytesToUint(608)), //19 * 32 // tokenTypeFee + data.bytesToBytes32(640), //20 * 32 // trancheS + data.bytesToBytes32(672),//21 * 32 // trancheB + data.subBytes(704) //22 * 32 // transferDataS ); require(data.length == 23 * 32 + order.transferDataS.length, INVALID_SIZE); diff --git a/contracts/impl/RingSubmitter.sol b/contracts/impl/RingSubmitter.sol index cee18b7..0cff0b4 100644 --- a/contracts/impl/RingSubmitter.sol +++ b/contracts/impl/RingSubmitter.sol @@ -271,7 +271,7 @@ contract RingSubmitter is IRingSubmitter, NoDefaultFunc { bytes32 ringHash = ring.hash; // keccak256("RingMined(uint256,bytes32,address,bytes)") bytes32 ringMinedSignature = 0xb2ef4bc5209dff0c46d5dfddb2b68a23bd4820e8f33107fde76ed15ba90695c9; - uint fillsSize = ring.size * 8 * 32; + uint fillsSize = ring.size * 256; //8*32 uint data; uint ptr; @@ -357,7 +357,7 @@ contract RingSubmitter is IRingSubmitter, NoDefaultFunc { // Try to find the spendable for the same (broker, owner, token) set let addNew := 1 for { let p := data } and(lt(p, ptr), eq(addNew, 1)) { p := add(p, 192) } { - let dataBroker := mload(add(p, 0)) + let dataBroker := mload(p) let dataOwner := mload(add(p, 32)) let dataToken := mload(add(p, 64)) // if(broker == dataBroker && owner == dataOwner && token == dataToken) @@ -367,7 +367,7 @@ contract RingSubmitter is IRingSubmitter, NoDefaultFunc { } } if eq(addNew, 1) { - mstore(add(ptr, 0), broker) + mstore(ptr, broker) mstore(add(ptr, 32), owner) mstore(add(ptr, 64), token) @@ -392,8 +392,7 @@ contract RingSubmitter is IRingSubmitter, NoDefaultFunc { pure { setupTokenBurnRateList(ctx, orders); - setupFeePaymentList(ctx, rings); - setupTokenTransferList(ctx, rings); + calculateAndSetupRingLists(ctx, rings); } function setupTokenBurnRateList( @@ -418,7 +417,8 @@ contract RingSubmitter is IRingSubmitter, NoDefaultFunc { ctx.tokenBurnRates = tokenBurnRates; } - function setupFeePaymentList( + + function calculateAndSetupRingLists( Data.Context ctx, Data.Ring[] rings ) @@ -426,21 +426,42 @@ contract RingSubmitter is IRingSubmitter, NoDefaultFunc { pure { uint totalMaxSizeFeePayments = 0; - for (uint i = 0; i < rings.length; i++) { - // Up to (ringSize + 3) * 3 payments per order (because of fee sharing by miner) - // (3 x 32 bytes for every fee payment) - uint ringSize = rings[i].size; - uint maxSize = (ringSize + 3) * 3 * ringSize * 3; - totalMaxSizeFeePayments += maxSize; + uint totalMaxSizeTransfers = 0; + assembly{ + let ringSize := mload(rings) + let ring := 0 + totalMaxSizeFeePayments := 0 + totalMaxSizeTransfers := 0 + + for { let i := 0 } lt(i, ringSize) { i := add(i, 1) } { + ring := add(rings, add(mul(add(ringSize,1),32), mul(160, i))) + totalMaxSizeFeePayments := add(totalMaxSizeFeePayments, mul(mul(9, mload(ring)), add(mload(ring), 3))) + totalMaxSizeTransfers := add(totalMaxSizeTransfers, mul(16, mload(ring))) + } } - // Store the data directly in the call data format as expected by batchAddFeeBalances: - // - 0x00: batchAddFeeBalances selector (4 bytes) - // - 0x04: parameter offset (batchAddFeeBalances has a single function parameter) (32 bytes) - // - 0x24: length of the array passed into the function (32 bytes) - // - 0x44: the array data (32 bytes x length) + + setupFeePaymentList(ctx, rings, totalMaxSizeFeePayments); + setupTokenTransferList(ctx, rings, totalMaxSizeTransfers); + + } + + function setupFeePaymentList( + Data.Context ctx, + Data.Ring[] rings, + uint totalMaxSizeFeePayments + ) + internal + pure + { bytes4 batchAddFeeBalancesSelector = ctx.feeHolder.batchAddFeeBalances.selector; uint ptr; - assembly { + assembly{ + // Store the data directly in the call data format as expected by batchAddFeeBalances: + // - 0x00: batchAddFeeBalances selector (4 bytes) + // - 0x04: parameter offset (batchAddFeeBalances has a single function parameter) (32 bytes) + // - 0x24: length of the array passed into the function (32 bytes) + // - 0x44: the array data (32 bytes x length) + let data := mload(0x40) mstore(data, batchAddFeeBalancesSelector) mstore(add(data, 4), 32) @@ -453,26 +474,22 @@ contract RingSubmitter is IRingSubmitter, NoDefaultFunc { function setupTokenTransferList( Data.Context ctx, - Data.Ring[] rings + Data.Ring[] rings, + uint totalMaxSizeTransfers ) internal pure { - uint totalMaxSizeTransfers = 0; - for (uint i = 0; i < rings.length; i++) { - // Up to 4 transfers per order - // (4 x 32 bytes for every transfer) - uint maxSize = 4 * rings[i].size * 4; - totalMaxSizeTransfers += maxSize; - } - // Store the data directly in the call data format as expected by batchTransfer: - // - 0x00: batchTransfer selector (4 bytes) - // - 0x04: parameter offset (batchTransfer has a single function parameter) (32 bytes) - // - 0x24: length of the array passed into the function (32 bytes) - // - 0x44: the array data (32 bytes x length) + //uint totalMaxSizeTransfers = 0; bytes4 batchTransferSelector = ctx.delegate.batchTransfer.selector; uint ptr; - assembly { + + assembly{ + // Store the data directly in the call data format as expected by batchTransfer: + // - 0x00: batchTransfer selector (4 bytes) + // - 0x04: parameter offset (batchTransfer has a single function parameter) (32 bytes) + // - 0x24: length of the array passed into the function (32 bytes) + // - 0x44: the array data (32 bytes x length) let data := mload(0x40) mstore(data, batchTransferSelector) mstore(add(data, 4), 32) @@ -512,7 +529,7 @@ contract RingSubmitter is IRingSubmitter, NoDefaultFunc { let filledAmountChanged := iszero(eq(filledAmount, initialFilledAmount)) // if (order.valid && filledAmountChanged) if and(gt(mload(add(order, 992)), 0), filledAmountChanged) { // order.valid - mstore(add(ptr, 0), mload(add(order, 864))) // order.hash + mstore(ptr, mload(add(order, 864))) // order.hash mstore(add(ptr, 32), filledAmount) ptr := add(ptr, 64) @@ -569,7 +586,7 @@ contract RingSubmitter is IRingSubmitter, NoDefaultFunc { let ptr := add(data, 68) for { let i := 0 } lt(i, mload(orders)) { i := add(i, 1) } { let order := mload(add(orders, mul(add(i, 1), 32))) // orders[i] - mstore(add(ptr, 0), mload(add(order, 320))) // order.broker + mstore(ptr, mload(add(order, 320))) // order.broker mstore(add(ptr, 32), mload(add(order, 32))) // order.owner mstore(add(ptr, 64), mload(add(order, 864))) // order.hash mstore(add(ptr, 96), mload(add(order, 192))) // order.validSince diff --git a/contracts/impl/TradeDelegate.sol b/contracts/impl/TradeDelegate.sol index d0e9c2a..9f3d968 100644 --- a/contracts/impl/TradeDelegate.sol +++ b/contracts/impl/TradeDelegate.sol @@ -49,7 +49,7 @@ contract TradeDelegate is ITradeDelegate, Authorizable, Killable, NoDefaultFunc address to; uint amount; assembly { - token := calldataload(add(p, 0)) + token := calldataload(p) from := calldataload(add(p, 32)) to := calldataload(add(p, 64)) amount := calldataload(add(p, 96)) diff --git a/contracts/impl/TradeHistory.sol b/contracts/impl/TradeHistory.sol index 71b7261..bcb9dbf 100644 --- a/contracts/impl/TradeHistory.sol +++ b/contracts/impl/TradeHistory.sol @@ -47,7 +47,7 @@ contract TradeHistory is ITradeHistory, Authorizable, Killable, NoDefaultFunc { bytes32 hash; uint filledAmount; assembly { - hash := calldataload(add(p, 0)) + hash := calldataload(p) filledAmount := calldataload(add(p, 32)) } filled[hash] = filledAmount; @@ -138,7 +138,7 @@ contract TradeHistory is ITradeHistory, Authorizable, Killable, NoDefaultFunc { uint validSince; bytes20 tradingPair; assembly { - broker := calldataload(add(p, 0)) + broker := calldataload(p) owner := calldataload(add(p, 32)) hash := calldataload(add(p, 64)) validSince := calldataload(add(p, 96)) From e0566659d8984ffb3cea9a5cc563478ad121e24d Mon Sep 17 00:00:00 2001 From: Harnick Khera Date: Fri, 14 Dec 2018 01:50:12 +0000 Subject: [PATCH 2/4] Refacored CheckOrdersValid and CheckForSubrings into a single function --- contracts/helper/RingHelper.sol | 49 ++++++++++---------------------- contracts/impl/RingSubmitter.sol | 5 ++-- 2 files changed, 17 insertions(+), 37 deletions(-) diff --git a/contracts/helper/RingHelper.sol b/contracts/helper/RingHelper.sol index 285f00b..f0a6395 100644 --- a/contracts/helper/RingHelper.sol +++ b/contracts/helper/RingHelper.sol @@ -168,8 +168,9 @@ library RingHelper { } } - function checkOrdersValid( - Data.Ring ring + function checkValidity( + Data.Ring ring, + bool checkSubring ) internal pure @@ -179,51 +180,31 @@ library RingHelper { assembly { let prev := 0 valid := and(valid, and(gt(ringSize,1), lt(ringSize, 8))) // ring.valid && (ring.size > 1 && ring.size <= 8) + let participations := mload(add(ring, 32)) for { let i := 0 } lt(i, ringSize) { i := add(i, 1) } { // for (uint i = 0; i < ring.size; i++) prev := mod(sub(add(i, ringSize), 1), ringSize) //uint prev = (i + ring.size - 1) % ring.size; - let participations := mload(add(ring, 32)) + let participation := mload(add(participations, add(32, mul(i, 32)))) // participations[i] let prevParticipation := mload(add(participations, add(32, mul(prev, 32)))) // participations[prev] let order := mload(participation) let prevOrder := mload(prevParticipation) + let tokenS := mload(add(order, 64)) //ring.valid && ring.participations[i].order.valid && ring.participations[i].order.tokenS == ring.participations[prev].order.tokenB: - valid := and(and(valid, mload(add(order, 992))),eq(mload(add(order, 64)), mload(add(prevOrder, 96)))) - - } - mstore(add(ring, 128), valid) //update ring.valid - } - } - - function checkForSubRings( - Data.Ring ring - ) - internal - pure - { - uint ringSize = ring.size; - bool valid = ring.valid; - //address tokenS; - //address tokenJ; - assembly { - let participations := mload(add(ring, 32)) + valid := and(and(valid, mload(add(order, 992))),eq(tokenS, mload(add(prevOrder, 96)))) - for { let i := 0 } lt(i, sub(ringSize,1)) { i := add(i, 1) } { // for (uint i = 0; i < ring.size - 1; i++) { + if and(checkSubring, lt(i, sub(ringSize, 1))) { + for {let j := add(i,1) } lt(j, ringSize) { j := add(j, 1) } { // for (uint j = i + 1; j < ring.size; j++) { - let participation := mload(add(participations, add(32, mul(i, 32)))) // participations[i] - let order := mload(participation) - let tokenS := mload(add(order, 64)) //address tokenS = ring.participations[i].order.tokenS; - - for {let j := add(i,1) } lt(j, ringSize) { j := add(j, 1) } { // for (uint j = i + 1; j < ring.size; j++) { - - let participationj := mload(add(participations, add(32, mul(j, 32)))) // participations[j] - let orderj := mload(participationj) - let tokenJ := mload(add(orderj, 64)) //address tokenJ = ring.participations[j].order.tokenS; - valid := and(valid, not(eq(tokenS, tokenJ))) // ring.valid && (tokenS != ring.participations[j].order.tokenS); + let participationj := mload(add(participations, add(32, mul(j, 32)))) // participations[j] + let orderj := mload(participationj) + let tokenJ := mload(add(orderj, 64)) //address tokenJ = ring.participations[j].order.tokenS; + valid := and(valid, not(eq(tokenS, tokenJ))) // ring.valid && (tokenS != ring.participations[j].order.tokenS); + } } - } + } mstore(add(ring, 128), valid) //update ring.valid } } diff --git a/contracts/impl/RingSubmitter.sol b/contracts/impl/RingSubmitter.sol index 0cff0b4..4702268 100644 --- a/contracts/impl/RingSubmitter.sol +++ b/contracts/impl/RingSubmitter.sol @@ -186,8 +186,7 @@ contract RingSubmitter is IRingSubmitter, NoDefaultFunc { for (i = 0; i < rings.length; i++) { Data.Ring memory ring = rings[i]; - ring.checkOrdersValid(); - ring.checkForSubRings(); + ring.checkValidity(true); ring.calculateFillAmountAndFee(ctx); if (ring.valid) { ring.adjustOrderStates(); @@ -249,7 +248,7 @@ contract RingSubmitter is IRingSubmitter, NoDefaultFunc { for (uint i = 0; i < rings.length; i++) { Data.Ring memory ring = rings[i]; if (ring.valid) { - ring.checkOrdersValid(); + ring.checkValidity(false); if (!ring.valid) { // If the ring was valid before the completely filled check we have to revert the filled amountS // of the orders in the ring. This is a bit awkward so maybe there's a better solution. From dafec53a8a5e90f0956d18db16ce855338fce055 Mon Sep 17 00:00:00 2001 From: Harnick Khera Date: Fri, 14 Dec 2018 15:12:35 +0000 Subject: [PATCH 3/4] Packed EmitRing to reduce empty bytes --- contracts/helper/MiningHelper.sol | 2 +- contracts/helper/OrderHelper.sol | 51 ++++++-------- contracts/helper/RingHelper.sol | 78 +++++++++------------ contracts/impl/ExchangeDeserializer.sol | 24 +++---- contracts/impl/FeeHolder.sol | 2 +- contracts/impl/OrderBook.sol | 48 ++++++------- contracts/impl/RingSubmitter.sol | 90 ++++++++++--------------- contracts/impl/TradeDelegate.sol | 2 +- contracts/impl/TradeHistory.sol | 4 +- test/testExchangeUtil.ts | 18 ++--- 10 files changed, 142 insertions(+), 177 deletions(-) diff --git a/contracts/helper/MiningHelper.sol b/contracts/helper/MiningHelper.sol index 14159cb..2205f2e 100644 --- a/contracts/helper/MiningHelper.sol +++ b/contracts/helper/MiningHelper.sol @@ -69,7 +69,7 @@ library MiningHelper { // Store data back to front to allow overwriting data at the front because of padding mstore(add(data, 40), ringHashes) // ringHashes mstore(sub(add(data, 20), 12), mload(add(mining, 32))) // mining.miner - mstore(sub(data, 12), mload(mining)) // mining.feeRecipient + mstore(sub(data, 12), mload(add(mining, 0))) // mining.feeRecipient hash := keccak256(data, 72) // 20 + 20 + 32 } mining.hash = hash; diff --git a/contracts/helper/OrderHelper.sol b/contracts/helper/OrderHelper.sol index c9e8864..55a5b0a 100644 --- a/contracts/helper/OrderHelper.sol +++ b/contracts/helper/OrderHelper.sol @@ -133,7 +133,7 @@ library OrderHelper { let transferDataSHash := keccak256(add(transferDataS, 32), mload(transferDataS)) let ptr := mload(64) - mstore(ptr, _EIP712_ORDER_SCHEMA_HASH) // EIP712_ORDER_SCHEMA_HASH + mstore(add(ptr, 0), _EIP712_ORDER_SCHEMA_HASH) // EIP712_ORDER_SCHEMA_HASH mstore(add(ptr, 32), mload(add(order, 128))) // order.amountS mstore(add(ptr, 64), mload(add(order, 160))) // order.amountB mstore(add(ptr, 96), mload(add(order, 640))) // order.feeAmount @@ -160,7 +160,7 @@ library OrderHelper { mstore(add(ptr, 768), transferDataSHash) // keccak256(order.transferDataS) let message := keccak256(ptr, 800) // 25 * 32 - mstore(ptr, 0x1901) // EIP191_HEADER + mstore(add(ptr, 0), 0x1901) // EIP191_HEADER mstore(add(ptr, 32), _EIP712_DOMAIN_HASH) // EIP712_DOMAIN_HASH mstore(add(ptr, 64), message) // message hash := keccak256(add(ptr, 30), 66) // 2 + 32 + 32 @@ -214,34 +214,27 @@ library OrderHelper { view { bool valid = true; - uint feePercentageBase = ctx.feePercentageBase; - uint dataLength = order.transferDataS.length; - Data.TokenType ErcType = Data.TokenType.ERC20; - assembly{ + valid = valid && (order.version == 0); // unsupported order version + valid = valid && (order.owner != 0x0); // invalid order owner + valid = valid && (order.tokenS != 0x0); // invalid order tokenS + valid = valid && (order.tokenB != 0x0); // invalid order tokenB + valid = valid && (order.amountS != 0); // invalid order amountS + valid = valid && (order.amountB != 0); // invalid order amountB + valid = valid && (order.feeToken != 0x0); // invalid fee token + + valid = valid && (order.tokenSFeePercentage < ctx.feePercentageBase); // invalid tokenS percentage + valid = valid && (order.tokenBFeePercentage < ctx.feePercentageBase); // invalid tokenB percentage + valid = valid && (order.walletSplitPercentage <= 100); // invalid wallet split percentage + + // We only support ERC20 for now + valid = valid && (order.tokenTypeS == Data.TokenType.ERC20 && order.trancheS == 0x0); + valid = valid && (order.tokenTypeB == Data.TokenType.ERC20 && order.trancheB == 0x0); + valid = valid && (order.tokenTypeFee == Data.TokenType.ERC20); + valid = valid && (order.transferDataS.length == 0); + + valid = valid && (order.validSince <= now); // order is too early to match - valid := and(valid, eq(mload(order), 0)) // unsupported order version - valid := and(valid, not(eq(mload(add(order, 32)), 0x0))) // invalid order owner - valid := and(valid, not(eq(mload(add(order, 64)), 0x0))) // invalid order tokenS - valid := and(valid, not(eq(mload(add(order, 96)), 0x0))) // invalid order tokenB - valid := and(valid, not(eq(mload(add(order, 128)), 0))) // invalid order amountS - valid := and(valid, not(eq(mload(add(order, 160)), 0x0))) // invalid order amountB - valid := and(valid, not(eq(mload(add(order, 608)), 0x0))) // invalid fee token - - - valid := and(valid, lt(mload(add(order, 704)), feePercentageBase)) // invalid tokenS percentage - valid := and(valid, lt(mload(add(order, 736)), feePercentageBase)) // invalid tokenB percentage - valid := and(valid, or(lt(mload(add(order, 800)), 100), eq(mload(add(order, 800)), 100))) // invalid wallet split percentage - - // We only support ERC20 for now - valid := and(valid, and(eq(mload(add(order, 1024)), ErcType), eq(mload(add(order, 1120)), 0x0))) // valid && (order.tokenTypeS == Data.TokenType.ERC20 && order.trancheS == 0x0) - valid := and(valid, and(eq(mload(add(order, 1056)), ErcType), eq(mload(add(order, 1152)), 0x0))) // valid && (order.tokenTypeB == Data.TokenType.ERC20 && order.trancheB == 0x0) - valid := and(valid, eq(mload(add(order, 1088)), ErcType)) //valid && (order.tokenTypeFee == Data.TokenType.ERC20); - valid := and(valid, eq(dataLength, 0)) // valid = valid && (order.transferDataS.length == 0); - - valid := and(valid, or(lt(mload(add(order, 192)), timestamp), eq(mload(add(order, 192)), timestamp))) // order is too early to match - - mstore(add(order, 992), and(valid, mload(add(order, 992)))) //order.valid = order.valid && valid; - } + order.valid = order.valid && valid; validateUnstableInfo(order, ctx); } diff --git a/contracts/helper/RingHelper.sol b/contracts/helper/RingHelper.sol index f0a6395..ffcc486 100644 --- a/contracts/helper/RingHelper.sol +++ b/contracts/helper/RingHelper.sol @@ -168,44 +168,31 @@ library RingHelper { } } - function checkValidity( - Data.Ring ring, - bool checkSubring + function checkOrdersValid( + Data.Ring ring ) internal pure { - uint ringSize = ring.size; - bool valid = ring.valid; - assembly { - let prev := 0 - valid := and(valid, and(gt(ringSize,1), lt(ringSize, 8))) // ring.valid && (ring.size > 1 && ring.size <= 8) - let participations := mload(add(ring, 32)) - - for { let i := 0 } lt(i, ringSize) { i := add(i, 1) } { // for (uint i = 0; i < ring.size; i++) - - prev := mod(sub(add(i, ringSize), 1), ringSize) //uint prev = (i + ring.size - 1) % ring.size; - - let participation := mload(add(participations, add(32, mul(i, 32)))) // participations[i] - let prevParticipation := mload(add(participations, add(32, mul(prev, 32)))) // participations[prev] - let order := mload(participation) - let prevOrder := mload(prevParticipation) - let tokenS := mload(add(order, 64)) - //ring.valid && ring.participations[i].order.valid && ring.participations[i].order.tokenS == ring.participations[prev].order.tokenB: - valid := and(and(valid, mload(add(order, 992))),eq(tokenS, mload(add(prevOrder, 96)))) - - if and(checkSubring, lt(i, sub(ringSize, 1))) { - for {let j := add(i,1) } lt(j, ringSize) { j := add(j, 1) } { // for (uint j = i + 1; j < ring.size; j++) { - - let participationj := mload(add(participations, add(32, mul(j, 32)))) // participations[j] - let orderj := mload(participationj) - let tokenJ := mload(add(orderj, 64)) //address tokenJ = ring.participations[j].order.tokenS; - valid := and(valid, not(eq(tokenS, tokenJ))) // ring.valid && (tokenS != ring.participations[j].order.tokenS); - } - } + ring.valid = ring.valid && (ring.size > 1 && ring.size <= 8); // invalid ring size + for (uint i = 0; i < ring.size; i++) { + uint prev = (i + ring.size - 1) % ring.size; + ring.valid = ring.valid && ring.participations[i].order.valid; + ring.valid = ring.valid && ring.participations[i].order.tokenS == ring.participations[prev].order.tokenB; + } + } + function checkForSubRings( + Data.Ring ring + ) + internal + pure + { + for (uint i = 0; i < ring.size - 1; i++) { + address tokenS = ring.participations[i].order.tokenS; + for (uint j = i + 1; j < ring.size; j++) { + ring.valid = ring.valid && (tokenS != ring.participations[j].order.tokenS); } - mstore(add(ring, 128), valid) //update ring.valid } } @@ -253,7 +240,7 @@ library RingHelper { returns (uint fill) { uint ringSize = ring.size; - uint fillSize = 256; //8 * 32 + uint fillSize = 232; assembly { fill := destPtr let participations := mload(add(ring, 32)) // ring.participations @@ -276,14 +263,15 @@ library RingHelper { mload(add(participation, 224)) // participation.rebateFeeB ) - mstore(fill, mload(add(order, 864))) // order.hash - mstore(add(fill, 32), mload(add(order, 32))) // order.owner - mstore(add(fill, 64), mload(add(order, 64))) // order.tokenS - mstore(add(fill, 96), mload(add(participation, 256))) // participation.fillAmountS - mstore(add(fill, 128), mload(add(participation, 32))) // participation.splitS - mstore(add(fill, 160), feeAmount) // feeAmount - mstore(add(fill, 192), feeAmountS) // feeAmountS - mstore(add(fill, 224), feeAmountB) // feeAmountB + + mstore(add(fill, 200), mload(add(order, 864))) // order.hash + mstore(add(fill, 168), mload(add(participation, 256))) // participation.fillAmountS + mstore(add(fill, 136), mload(add(order, 32))) // order.owner + mstore(add(fill, 116), mload(add(order, 64))) // order.tokenS + mstore(add(fill, 96), mload(add(participation, 32))) // participation.splitS + mstore(add(fill, 64), feeAmount) // feeAmount + mstore(add(fill, 32), feeAmountS) // feeAmountS + mstore(add(fill, 0), feeAmountB) // feeAmountB fill := add(fill, fillSize) } @@ -408,7 +396,7 @@ library RingHelper { // Try to find an existing fee payment of the same token to the same owner let addNew := 1 for { let p := data } lt(p, ptr) { p := add(p, 128) } { - let dataToken := mload(p) + let dataToken := mload(add(p, 0)) let dataFrom := mload(add(p, 32)) let dataTo := mload(add(p, 64)) // if(token == dataToken && from == dataFrom && to == dataTo) @@ -428,7 +416,7 @@ library RingHelper { } // Add a new transfer if eq(addNew, 1) { - mstore(ptr, token) + mstore(add(ptr, 0), token) mstore(add(ptr, 32), from) mstore(add(ptr, 64), to) mstore(add(ptr, 96), amount) @@ -679,7 +667,7 @@ library RingHelper { // Try to find an existing fee payment of the same token to the same owner let addNew := 1 for { let p := data } lt(p, ptr) { p := add(p, 96) } { - let dataToken := mload(p) + let dataToken := mload(add(p, 0)) let dataOwner := mload(add(p, 32)) // if(token == dataToken && owner == dataOwner) if and(eq(token, dataToken), eq(owner, dataOwner)) { @@ -698,7 +686,7 @@ library RingHelper { } // Add a new fee payment if eq(addNew, 1) { - mstore(ptr, token) + mstore(add(ptr, 0), token) mstore(add(ptr, 32), owner) mstore(add(ptr, 64), amount) ptr := add(ptr, 96) diff --git a/contracts/impl/ExchangeDeserializer.sol b/contracts/impl/ExchangeDeserializer.sol index fc7a2cd..ea5e966 100644 --- a/contracts/impl/ExchangeDeserializer.sol +++ b/contracts/impl/ExchangeDeserializer.sol @@ -88,9 +88,9 @@ library ExchangeDeserializer { mstore(add(data, 20), origin) // mining.feeRecipient - offset := mul(and(mload(tablesPtr), 0xFFFF), 4) + offset := mul(and(mload(add(tablesPtr, 0)), 0xFFFF), 4) mstore( - mining, + add(mining, 0), mload(add(add(data, 20), offset)) ) @@ -131,7 +131,7 @@ library ExchangeDeserializer { returns (Data.Order[] orders) { bytes memory emptyBytes = new bytes(0); - uint orderStructSize = 1216; //38 * 32 + uint orderStructSize = 38 * 32; // Memory for orders length + numOrders order pointers uint arrayDataSize = (1 + numOrders) * 32; Data.Spendable[] memory spendableList = new Data.Spendable[](numSpendables); @@ -140,7 +140,7 @@ library ExchangeDeserializer { assembly { // Allocate memory for all orders orders := mload(0x40) - mstore(orders, numOrders) // orders.length + mstore(add(orders, 0), numOrders) // orders.length // Reserve the memory for the orders array mstore(0x40, add(orders, add(arrayDataSize, mul(orderStructSize, numOrders)))) @@ -151,9 +151,9 @@ library ExchangeDeserializer { mstore(add(orders, mul(add(1, i), 32)), order) // order.version - offset := and(mload(tablesPtr), 0xFFFF) + offset := and(mload(add(tablesPtr, 0)), 0xFFFF) mstore( - order, + add(order, 0), offset ) @@ -412,13 +412,13 @@ library ExchangeDeserializer { returns (Data.Ring[] rings) { uint ringsArrayDataSize = (1 + numRings) * 32; - uint ringStructSize = 160; //5 * 32 - uint participationStructSize = 320; //10 * 32 + uint ringStructSize = 5 * 32; + uint participationStructSize = 10 * 32; assembly { // Allocate memory for all rings rings := mload(0x40) - mstore(rings, numRings) // rings.length + mstore(add(rings, 0), numRings) // rings.length // Reserve the memory for the rings array mstore(0x40, add(rings, add(ringsArrayDataSize, mul(ringStructSize, numRings)))) @@ -439,14 +439,14 @@ library ExchangeDeserializer { // Allocate memory for all participations let participations := mload(0x40) - mstore(participations, ringSize) // participations.length + mstore(add(participations, 0), ringSize) // participations.length // Memory for participations length + ringSize participation pointers let participationsData := add(participations, mul(add(1, ringSize), 32)) // Reserve the memory for the participations mstore(0x40, add(participationsData, mul(participationStructSize, ringSize))) // Initialize ring properties - mstore(ring, ringSize) // ring.size + mstore(add(ring, 0), ringSize) // ring.size mstore(add(ring, 32), participations) // ring.participations mstore(add(ring, 64), 0) // ring.hash mstore(add(ring, 96), 0) // ring.minerFeesToOrdersPercentage @@ -468,7 +468,7 @@ library ExchangeDeserializer { // participation.order mstore( - participation, + add(participation, 0), mload(add(orders, mul(add(orderIndex, 1), 32))) ) diff --git a/contracts/impl/FeeHolder.sol b/contracts/impl/FeeHolder.sol index d51f5fa..edfd2a0 100644 --- a/contracts/impl/FeeHolder.sol +++ b/contracts/impl/FeeHolder.sol @@ -60,7 +60,7 @@ contract FeeHolder is IFeeHolder, NoDefaultFunc { uint end = start + length * 32; for (uint p = start; p < end; p += 96) { assembly { - token := calldataload(p) + token := calldataload(add(p, 0)) owner := calldataload(add(p, 32)) value := calldataload(add(p, 64)) } diff --git a/contracts/impl/OrderBook.sol b/contracts/impl/OrderBook.sol index a15d274..97c77e5 100644 --- a/contracts/impl/OrderBook.sol +++ b/contracts/impl/OrderBook.sol @@ -38,47 +38,47 @@ contract OrderBook is IOrderBook, NoDefaultFunc { external returns (bytes32) { - require(data.length >= 736, INVALID_SIZE); //23 * 32 + require(data.length >= 23 * 32, INVALID_SIZE); Data.Order memory order = Data.Order( 0, // version - address(data.bytesToUint(0)), //0 * 32 // owner - address(data.bytesToUint(32)), //1 * 32 // tokenS - address(data.bytesToUint(64)), //2 * 32 // tokenB - data.bytesToUint(96), //3 * 32 // amountS - data.bytesToUint(128), //4 * 32 // amountB - data.bytesToUint(160), //5 * 32 // validSince + address(data.bytesToUint(0 * 32)), // owner + address(data.bytesToUint(1 * 32)), // tokenS + address(data.bytesToUint(2 * 32)), // tokenB + data.bytesToUint(3 * 32), // amountS + data.bytesToUint(4 * 32), // amountB + data.bytesToUint(5 * 32), // validSince Data.Spendable(true, 0, 0), Data.Spendable(true, 0, 0), 0x0, - address(data.bytesToUint(192)), //6 * 32 // broker + address(data.bytesToUint(6 * 32)), // broker Data.Spendable(true, 0, 0), Data.Spendable(true, 0, 0), - address(data.bytesToUint(224)), //7 * 32 // orderInterceptor - address(data.bytesToUint(256)), //8 * 32 // wallet - uint(data.bytesToUint(288)), //9 * 32 // validUtil + address(data.bytesToUint(7 * 32)), // orderInterceptor + address(data.bytesToUint(8 * 32)), // wallet + uint(data.bytesToUint(9 * 32)), // validUtil new bytes(0), new bytes(0), - bool(data.bytesToUint(320) > 0), //10 * 32 // allOrNone - address(data.bytesToUint(352)),//11 * 32 // feeToken - data.bytesToUint(384), //12 * 32 // feeAmount + bool(data.bytesToUint(10 * 32) > 0), // allOrNone + address(data.bytesToUint(11 * 32)), // feeToken + data.bytesToUint(12 * 32), // feeAmount 0, - uint16(data.bytesToUint(416)), //13 * 32 // tokenSFeePercentage - uint16(data.bytesToUint(448)), //14 * 32 // tokenBFeePercentage - address(data.bytesToUint(480)), //15 * 32 // tokenRecipient - uint16(data.bytesToUint(512)), //16 * 32 // walletSplitPercentage + uint16(data.bytesToUint(13 * 32)), // tokenSFeePercentage + uint16(data.bytesToUint(14 * 32)), // tokenBFeePercentage + address(data.bytesToUint(15 * 32)), // tokenRecipient + uint16(data.bytesToUint(16 * 32)), // walletSplitPercentage false, bytes32(0x0), 0x0, 0, 0, true, - Data.TokenType(data.bytesToUint(544)), //17 * 32 // tokenTypeS - Data.TokenType(data.bytesToUint(576)), //18 * 32 // tokenTypeB - Data.TokenType(data.bytesToUint(608)), //19 * 32 // tokenTypeFee - data.bytesToBytes32(640), //20 * 32 // trancheS - data.bytesToBytes32(672),//21 * 32 // trancheB - data.subBytes(704) //22 * 32 // transferDataS + Data.TokenType(data.bytesToUint(17 * 32)), // tokenTypeS + Data.TokenType(data.bytesToUint(18 * 32)), // tokenTypeB + Data.TokenType(data.bytesToUint(19 * 32)), // tokenTypeFee + data.bytesToBytes32(20 * 32), // trancheS + data.bytesToBytes32(21 * 32), // trancheB + data.subBytes(22 * 32) // transferDataS ); require(data.length == 23 * 32 + order.transferDataS.length, INVALID_SIZE); diff --git a/contracts/impl/RingSubmitter.sol b/contracts/impl/RingSubmitter.sol index 4702268..56a7fbf 100644 --- a/contracts/impl/RingSubmitter.sol +++ b/contracts/impl/RingSubmitter.sol @@ -186,7 +186,8 @@ contract RingSubmitter is IRingSubmitter, NoDefaultFunc { for (i = 0; i < rings.length; i++) { Data.Ring memory ring = rings[i]; - ring.checkValidity(true); + ring.checkOrdersValid(); + ring.checkForSubRings(); ring.calculateFillAmountAndFee(ctx); if (ring.valid) { ring.adjustOrderStates(); @@ -248,7 +249,7 @@ contract RingSubmitter is IRingSubmitter, NoDefaultFunc { for (uint i = 0; i < rings.length; i++) { Data.Ring memory ring = rings[i]; if (ring.valid) { - ring.checkValidity(false); + ring.checkOrdersValid(); if (!ring.valid) { // If the ring was valid before the completely filled check we have to revert the filled amountS // of the orders in the ring. This is a bit awkward so maybe there's a better solution. @@ -270,7 +271,7 @@ contract RingSubmitter is IRingSubmitter, NoDefaultFunc { bytes32 ringHash = ring.hash; // keccak256("RingMined(uint256,bytes32,address,bytes)") bytes32 ringMinedSignature = 0xb2ef4bc5209dff0c46d5dfddb2b68a23bd4820e8f33107fde76ed15ba90695c9; - uint fillsSize = ring.size * 256; //8*32 + uint fillsSize = ring.size * 232; uint data; uint ptr; @@ -356,7 +357,7 @@ contract RingSubmitter is IRingSubmitter, NoDefaultFunc { // Try to find the spendable for the same (broker, owner, token) set let addNew := 1 for { let p := data } and(lt(p, ptr), eq(addNew, 1)) { p := add(p, 192) } { - let dataBroker := mload(p) + let dataBroker := mload(add(p, 0)) let dataOwner := mload(add(p, 32)) let dataToken := mload(add(p, 64)) // if(broker == dataBroker && owner == dataOwner && token == dataToken) @@ -366,7 +367,7 @@ contract RingSubmitter is IRingSubmitter, NoDefaultFunc { } } if eq(addNew, 1) { - mstore(ptr, broker) + mstore(add(ptr, 0), broker) mstore(add(ptr, 32), owner) mstore(add(ptr, 64), token) @@ -391,7 +392,8 @@ contract RingSubmitter is IRingSubmitter, NoDefaultFunc { pure { setupTokenBurnRateList(ctx, orders); - calculateAndSetupRingLists(ctx, rings); + setupFeePaymentList(ctx, rings); + setupTokenTransferList(ctx, rings); } function setupTokenBurnRateList( @@ -416,8 +418,7 @@ contract RingSubmitter is IRingSubmitter, NoDefaultFunc { ctx.tokenBurnRates = tokenBurnRates; } - - function calculateAndSetupRingLists( + function setupFeePaymentList( Data.Context ctx, Data.Ring[] rings ) @@ -425,42 +426,21 @@ contract RingSubmitter is IRingSubmitter, NoDefaultFunc { pure { uint totalMaxSizeFeePayments = 0; - uint totalMaxSizeTransfers = 0; - assembly{ - let ringSize := mload(rings) - let ring := 0 - totalMaxSizeFeePayments := 0 - totalMaxSizeTransfers := 0 - - for { let i := 0 } lt(i, ringSize) { i := add(i, 1) } { - ring := add(rings, add(mul(add(ringSize,1),32), mul(160, i))) - totalMaxSizeFeePayments := add(totalMaxSizeFeePayments, mul(mul(9, mload(ring)), add(mload(ring), 3))) - totalMaxSizeTransfers := add(totalMaxSizeTransfers, mul(16, mload(ring))) - } + for (uint i = 0; i < rings.length; i++) { + // Up to (ringSize + 3) * 3 payments per order (because of fee sharing by miner) + // (3 x 32 bytes for every fee payment) + uint ringSize = rings[i].size; + uint maxSize = (ringSize + 3) * 3 * ringSize * 3; + totalMaxSizeFeePayments += maxSize; } - - setupFeePaymentList(ctx, rings, totalMaxSizeFeePayments); - setupTokenTransferList(ctx, rings, totalMaxSizeTransfers); - - } - - function setupFeePaymentList( - Data.Context ctx, - Data.Ring[] rings, - uint totalMaxSizeFeePayments - ) - internal - pure - { + // Store the data directly in the call data format as expected by batchAddFeeBalances: + // - 0x00: batchAddFeeBalances selector (4 bytes) + // - 0x04: parameter offset (batchAddFeeBalances has a single function parameter) (32 bytes) + // - 0x24: length of the array passed into the function (32 bytes) + // - 0x44: the array data (32 bytes x length) bytes4 batchAddFeeBalancesSelector = ctx.feeHolder.batchAddFeeBalances.selector; uint ptr; - assembly{ - // Store the data directly in the call data format as expected by batchAddFeeBalances: - // - 0x00: batchAddFeeBalances selector (4 bytes) - // - 0x04: parameter offset (batchAddFeeBalances has a single function parameter) (32 bytes) - // - 0x24: length of the array passed into the function (32 bytes) - // - 0x44: the array data (32 bytes x length) - + assembly { let data := mload(0x40) mstore(data, batchAddFeeBalancesSelector) mstore(add(data, 4), 32) @@ -473,22 +453,26 @@ contract RingSubmitter is IRingSubmitter, NoDefaultFunc { function setupTokenTransferList( Data.Context ctx, - Data.Ring[] rings, - uint totalMaxSizeTransfers + Data.Ring[] rings ) internal pure { - //uint totalMaxSizeTransfers = 0; + uint totalMaxSizeTransfers = 0; + for (uint i = 0; i < rings.length; i++) { + // Up to 4 transfers per order + // (4 x 32 bytes for every transfer) + uint maxSize = 4 * rings[i].size * 4; + totalMaxSizeTransfers += maxSize; + } + // Store the data directly in the call data format as expected by batchTransfer: + // - 0x00: batchTransfer selector (4 bytes) + // - 0x04: parameter offset (batchTransfer has a single function parameter) (32 bytes) + // - 0x24: length of the array passed into the function (32 bytes) + // - 0x44: the array data (32 bytes x length) bytes4 batchTransferSelector = ctx.delegate.batchTransfer.selector; uint ptr; - - assembly{ - // Store the data directly in the call data format as expected by batchTransfer: - // - 0x00: batchTransfer selector (4 bytes) - // - 0x04: parameter offset (batchTransfer has a single function parameter) (32 bytes) - // - 0x24: length of the array passed into the function (32 bytes) - // - 0x44: the array data (32 bytes x length) + assembly { let data := mload(0x40) mstore(data, batchTransferSelector) mstore(add(data, 4), 32) @@ -528,7 +512,7 @@ contract RingSubmitter is IRingSubmitter, NoDefaultFunc { let filledAmountChanged := iszero(eq(filledAmount, initialFilledAmount)) // if (order.valid && filledAmountChanged) if and(gt(mload(add(order, 992)), 0), filledAmountChanged) { // order.valid - mstore(ptr, mload(add(order, 864))) // order.hash + mstore(add(ptr, 0), mload(add(order, 864))) // order.hash mstore(add(ptr, 32), filledAmount) ptr := add(ptr, 64) @@ -585,7 +569,7 @@ contract RingSubmitter is IRingSubmitter, NoDefaultFunc { let ptr := add(data, 68) for { let i := 0 } lt(i, mload(orders)) { i := add(i, 1) } { let order := mload(add(orders, mul(add(i, 1), 32))) // orders[i] - mstore(ptr, mload(add(order, 320))) // order.broker + mstore(add(ptr, 0), mload(add(order, 320))) // order.broker mstore(add(ptr, 32), mload(add(order, 32))) // order.owner mstore(add(ptr, 64), mload(add(order, 864))) // order.hash mstore(add(ptr, 96), mload(add(order, 192))) // order.validSince diff --git a/contracts/impl/TradeDelegate.sol b/contracts/impl/TradeDelegate.sol index 9f3d968..d0e9c2a 100644 --- a/contracts/impl/TradeDelegate.sol +++ b/contracts/impl/TradeDelegate.sol @@ -49,7 +49,7 @@ contract TradeDelegate is ITradeDelegate, Authorizable, Killable, NoDefaultFunc address to; uint amount; assembly { - token := calldataload(p) + token := calldataload(add(p, 0)) from := calldataload(add(p, 32)) to := calldataload(add(p, 64)) amount := calldataload(add(p, 96)) diff --git a/contracts/impl/TradeHistory.sol b/contracts/impl/TradeHistory.sol index bcb9dbf..71b7261 100644 --- a/contracts/impl/TradeHistory.sol +++ b/contracts/impl/TradeHistory.sol @@ -47,7 +47,7 @@ contract TradeHistory is ITradeHistory, Authorizable, Killable, NoDefaultFunc { bytes32 hash; uint filledAmount; assembly { - hash := calldataload(p) + hash := calldataload(add(p, 0)) filledAmount := calldataload(add(p, 32)) } filled[hash] = filledAmount; @@ -138,7 +138,7 @@ contract TradeHistory is ITradeHistory, Authorizable, Killable, NoDefaultFunc { uint validSince; bytes20 tradingPair; assembly { - broker := calldataload(p) + broker := calldataload(add(p, 0)) owner := calldataload(add(p, 32)) hash := calldataload(add(p, 64)) validSince := calldataload(add(p, 96)) diff --git a/test/testExchangeUtil.ts b/test/testExchangeUtil.ts index ea634c8..75cdc62 100644 --- a/test/testExchangeUtil.ts +++ b/test/testExchangeUtil.ts @@ -63,19 +63,19 @@ export class ExchangeTestUtil { public async getRingMinedEvents(fromBlock: number) { const parseFillsData = (data: string) => { const b = new pjs.Bitstream(data); - const fillSize = 8 * 32; + const fillSize = 232; const numFills = b.length() / fillSize; const fills: pjs.Fill[] = []; for (let offset = 0; offset < b.length(); offset += fillSize) { const fill: pjs.Fill = { - orderHash: "0x" + b.extractBytes32(offset).toString("hex"), - owner: "0x" + b.extractBytes32(offset + 32).toString("hex").slice(24), - tokenS: "0x" + b.extractBytes32(offset + 64).toString("hex").slice(24), - amountS: b.extractUint(offset + 96), - split: b.extractUint(offset + 128), - feeAmount: b.extractUint(offset + 160), - feeAmountS: b.extractUint(offset + 192), - feeAmountB: b.extractUint(offset + 224), + orderHash: "0x" + b.extractBytes32(offset + 200).toString("hex"), + amountS: b.extractUint(offset + 168), + owner: "0x" + b.extractBytes32(offset + 136).toString("hex").slice(24), + tokenS: "0x" + b.extractBytes32(offset + 116).toString("hex").slice(24), + split: b.extractUint(offset + 96), + feeAmount: b.extractUint(offset + 64), + feeAmountS: b.extractUint(offset + 32), + feeAmountB: b.extractUint(offset + 0), }; fills.push(fill); } From 89ef424a7d11084ac27e89357d905f1989d1137c Mon Sep 17 00:00:00 2001 From: Harnick Khera Date: Sun, 16 Dec 2018 08:21:00 +0000 Subject: [PATCH 4/4] Reverted Data order, while retaining savings --- contracts/helper/RingHelper.sol | 7 +++---- test/testExchangeUtil.ts | 6 +++--- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/contracts/helper/RingHelper.sol b/contracts/helper/RingHelper.sol index ffcc486..92fe56a 100644 --- a/contracts/helper/RingHelper.sol +++ b/contracts/helper/RingHelper.sol @@ -263,11 +263,10 @@ library RingHelper { mload(add(participation, 224)) // participation.rebateFeeB ) - mstore(add(fill, 200), mload(add(order, 864))) // order.hash - mstore(add(fill, 168), mload(add(participation, 256))) // participation.fillAmountS - mstore(add(fill, 136), mload(add(order, 32))) // order.owner - mstore(add(fill, 116), mload(add(order, 64))) // order.tokenS + mstore(add(fill, 168), mload(add(order, 32))) // order.owner + mstore(add(fill, 148), mload(add(order, 64))) // order.tokenS + mstore(add(fill, 128), mload(add(participation, 256))) // participation.fillAmountS mstore(add(fill, 96), mload(add(participation, 32))) // participation.splitS mstore(add(fill, 64), feeAmount) // feeAmount mstore(add(fill, 32), feeAmountS) // feeAmountS diff --git a/test/testExchangeUtil.ts b/test/testExchangeUtil.ts index 75cdc62..5d2abd5 100644 --- a/test/testExchangeUtil.ts +++ b/test/testExchangeUtil.ts @@ -69,9 +69,9 @@ export class ExchangeTestUtil { for (let offset = 0; offset < b.length(); offset += fillSize) { const fill: pjs.Fill = { orderHash: "0x" + b.extractBytes32(offset + 200).toString("hex"), - amountS: b.extractUint(offset + 168), - owner: "0x" + b.extractBytes32(offset + 136).toString("hex").slice(24), - tokenS: "0x" + b.extractBytes32(offset + 116).toString("hex").slice(24), + owner: "0x" + b.extractBytes32(offset + 168).toString("hex").slice(24), + tokenS: "0x" + b.extractBytes32(offset + 148).toString("hex").slice(24), + amountS: b.extractUint(offset + 128), split: b.extractUint(offset + 96), feeAmount: b.extractUint(offset + 64), feeAmountS: b.extractUint(offset + 32),