From 407f63ef2055e8cd01af010a2e6d3a6c548c9793 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Mon, 20 Aug 2018 13:13:38 -0700 Subject: Removed calculateFillResults from matchOrders workflow. Eliminates compounded rounding errors. --- .../2.0.0/protocol/Exchange/MixinMatchOrders.sol | 67 ++++++++++------------ 1 file changed, 29 insertions(+), 38 deletions(-) (limited to 'packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol') diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol b/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol index 25220a673..2012aa6db 100644 --- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol +++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol @@ -109,7 +109,7 @@ contract MixinMatchOrders is rightOrderInfo.orderTakerAssetFilledAmount, matchedFillResults.right ); - + // Settle matched orders. Succeeds or throws. settleMatchedOrders( leftOrder, @@ -169,52 +169,43 @@ contract MixinMatchOrders is // The amount saved by the left maker goes to the taker. // Either the left or right order will be fully filled; possibly both. // The left order is fully filled iff the right order can sell more than left can buy. - // That is: the amount required to fill the left order is less than or equal to - // the amount we can spend from the right order: - // <= * - // <= * / - // * <= * + + // Derive maker asset amounts for left & right orders, given store taker assert amounts uint256 leftTakerAssetAmountRemaining = safeSub(leftOrder.takerAssetAmount, leftOrderTakerAssetFilledAmount); + uint256 leftMakerAssetAmountRemaining = getPartialAmountFloor( + leftOrder.makerAssetAmount, + leftOrder.takerAssetAmount, + leftTakerAssetAmountRemaining + ); uint256 rightTakerAssetAmountRemaining = safeSub(rightOrder.takerAssetAmount, rightOrderTakerAssetFilledAmount); - uint256 leftTakerAssetFilledAmount; - uint256 rightTakerAssetFilledAmount; - if ( - safeMul(leftTakerAssetAmountRemaining, rightOrder.takerAssetAmount) <= - safeMul(rightTakerAssetAmountRemaining, rightOrder.makerAssetAmount) - ) { - // Left order will be fully filled: maximally fill left - leftTakerAssetFilledAmount = leftTakerAssetAmountRemaining; + uint256 rightMakerAssetAmountRemaining = getPartialAmountFloor( + rightOrder.makerAssetAmount, + rightOrder.takerAssetAmount, + rightTakerAssetAmountRemaining + ); - // The right order receives an amount proportional to how much was spent. - rightTakerAssetFilledAmount = getPartialAmountFloor( - rightOrder.takerAssetAmount, - rightOrder.makerAssetAmount, - leftTakerAssetFilledAmount + if (leftTakerAssetAmountRemaining >= rightMakerAssetAmountRemaining) { + // Case 1: Right order is fully filled: maximally fill right + matchedFillResults.right.makerAssetFilledAmount = rightMakerAssetAmountRemaining; + matchedFillResults.right.takerAssetFilledAmount = rightTakerAssetAmountRemaining; + matchedFillResults.left.makerAssetFilledAmount = getPartialAmountFloor( + leftOrder.makerAssetAmount, + leftOrder.takerAssetAmount, + matchedFillResults.right.makerAssetFilledAmount ); + matchedFillResults.left.takerAssetFilledAmount = matchedFillResults.right.makerAssetFilledAmount; } else { - // Right order will be fully filled: maximally fill right - rightTakerAssetFilledAmount = rightTakerAssetAmountRemaining; - - // The left order receives an amount proportional to how much was spent. - leftTakerAssetFilledAmount = getPartialAmountFloor( - rightOrder.makerAssetAmount, + // Case 2: Left order is fully filled: maximall fill left + matchedFillResults.left.makerAssetFilledAmount = leftMakerAssetAmountRemaining; + matchedFillResults.left.takerAssetFilledAmount = leftTakerAssetAmountRemaining; + matchedFillResults.right.makerAssetFilledAmount = matchedFillResults.left.takerAssetFilledAmount; + matchedFillResults.right.takerAssetFilledAmount = getPartialAmountCeil( rightOrder.takerAssetAmount, - rightTakerAssetFilledAmount + rightOrder.makerAssetAmount, + matchedFillResults.left.takerAssetFilledAmount ); } - // Calculate fill results for left order - matchedFillResults.left = calculateFillResults( - leftOrder, - leftTakerAssetFilledAmount - ); - - // Calculate fill results for right order - matchedFillResults.right = calculateFillResults( - rightOrder, - rightTakerAssetFilledAmount - ); - // Calculate amount given to taker matchedFillResults.leftMakerAssetSpreadAmount = safeSub( matchedFillResults.left.makerAssetFilledAmount, -- cgit v1.2.3 From 057891b342cf639b493adcf0a136f7ee421aaaf9 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Mon, 20 Aug 2018 13:22:21 -0700 Subject: Added fees to matchOrders (previously in calculateFillResults --- .../2.0.0/protocol/Exchange/MixinMatchOrders.sol | 24 ++++++++++++++++++++++ 1 file changed, 24 insertions(+) (limited to 'packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol') diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol b/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol index 2012aa6db..58f131819 100644 --- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol +++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol @@ -212,6 +212,30 @@ contract MixinMatchOrders is matchedFillResults.right.takerAssetFilledAmount ); + // Compute fees for left order + matchedFillResults.left.makerFeePaid = getPartialAmount( + matchedFillResults.left.takerAssetFilledAmount, + leftOrder.takerAssetAmount, + leftOrder.makerFee + ); + matchedFillResults.left.takerFeePaid = getPartialAmount( + matchedFillResults.left.takerAssetFilledAmount, + leftOrder.takerAssetAmount, + leftOrder.takerFee + ); + + // Compute fees for right order + matchedFillResults.right.makerFeePaid = getPartialAmount( + matchedFillResults.right.takerAssetFilledAmount, + rightOrder.takerAssetAmount, + rightOrder.makerFee + ); + matchedFillResults.right.takerFeePaid = getPartialAmount( + matchedFillResults.right.takerAssetFilledAmount, + rightOrder.takerAssetAmount, + rightOrder.takerFee + ); + // Return fill results return matchedFillResults; } -- cgit v1.2.3 From a32b201afe6c90a6bd06b88a5d512408cd3cb032 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Mon, 20 Aug 2018 17:00:08 -0700 Subject: Rounding for fees in match orders addressed, plus example --- .../contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol') diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol b/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol index 58f131819..b3f376eb8 100644 --- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol +++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol @@ -214,8 +214,8 @@ contract MixinMatchOrders is // Compute fees for left order matchedFillResults.left.makerFeePaid = getPartialAmount( - matchedFillResults.left.takerAssetFilledAmount, - leftOrder.takerAssetAmount, + matchedFillResults.left.makerAssetFilledAmount, + leftOrder.makerAssetAmount, leftOrder.makerFee ); matchedFillResults.left.takerFeePaid = getPartialAmount( @@ -226,8 +226,8 @@ contract MixinMatchOrders is // Compute fees for right order matchedFillResults.right.makerFeePaid = getPartialAmount( - matchedFillResults.right.takerAssetFilledAmount, - rightOrder.takerAssetAmount, + matchedFillResults.right.makerAssetFilledAmount, + rightOrder.makerAssetAmount, rightOrder.makerFee ); matchedFillResults.right.takerFeePaid = getPartialAmount( -- cgit v1.2.3 From 81dc893d1d07e5a8485fb40aa247428cda79d788 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Thu, 23 Aug 2018 16:07:39 -0700 Subject: Removed a redundant comment from matchOrders --- packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) (limited to 'packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol') diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol b/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol index b3f376eb8..d932c743f 100644 --- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol +++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol @@ -165,11 +165,6 @@ contract MixinMatchOrders is pure returns (LibFillResults.MatchedFillResults memory matchedFillResults) { - // We settle orders at the exchange rate of the right order. - // The amount saved by the left maker goes to the taker. - // Either the left or right order will be fully filled; possibly both. - // The left order is fully filled iff the right order can sell more than left can buy. - // Derive maker asset amounts for left & right orders, given store taker assert amounts uint256 leftTakerAssetAmountRemaining = safeSub(leftOrder.takerAssetAmount, leftOrderTakerAssetFilledAmount); uint256 leftMakerAssetAmountRemaining = getPartialAmountFloor( @@ -184,6 +179,7 @@ contract MixinMatchOrders is rightTakerAssetAmountRemaining ); + // Calculate fill results for maker and taker assets if (leftTakerAssetAmountRemaining >= rightMakerAssetAmountRemaining) { // Case 1: Right order is fully filled: maximally fill right matchedFillResults.right.makerAssetFilledAmount = rightMakerAssetAmountRemaining; -- cgit v1.2.3 From a93f95c55ea43bc89d0633190590865d9723b2f9 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Thu, 23 Aug 2018 16:16:01 -0700 Subject: Wording in MixinMatchOrders --- packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol') diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol b/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol index d932c743f..a0696c616 100644 --- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol +++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol @@ -181,7 +181,7 @@ contract MixinMatchOrders is // Calculate fill results for maker and taker assets if (leftTakerAssetAmountRemaining >= rightMakerAssetAmountRemaining) { - // Case 1: Right order is fully filled: maximally fill right + // Case 1: Right order is fully filled matchedFillResults.right.makerAssetFilledAmount = rightMakerAssetAmountRemaining; matchedFillResults.right.takerAssetFilledAmount = rightTakerAssetAmountRemaining; matchedFillResults.left.makerAssetFilledAmount = getPartialAmountFloor( @@ -191,7 +191,7 @@ contract MixinMatchOrders is ); matchedFillResults.left.takerAssetFilledAmount = matchedFillResults.right.makerAssetFilledAmount; } else { - // Case 2: Left order is fully filled: maximall fill left + // Case 2: Left order is fully filled matchedFillResults.left.makerAssetFilledAmount = leftMakerAssetAmountRemaining; matchedFillResults.left.takerAssetFilledAmount = leftTakerAssetAmountRemaining; matchedFillResults.right.makerAssetFilledAmount = matchedFillResults.left.takerAssetFilledAmount; -- cgit v1.2.3 From 0a6f107243eb7df309766cc83942e667ce28e858 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Thu, 23 Aug 2018 16:28:03 -0700 Subject: Added temporary @todo to MixinMatchOrders --- .../contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol') diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol b/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol index a0696c616..8f289ee85 100644 --- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol +++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol @@ -209,24 +209,24 @@ contract MixinMatchOrders is ); // Compute fees for left order - matchedFillResults.left.makerFeePaid = getPartialAmount( + matchedFillResults.left.makerFeePaid = getPartialAmountFloor( matchedFillResults.left.makerAssetFilledAmount, leftOrder.makerAssetAmount, leftOrder.makerFee ); - matchedFillResults.left.takerFeePaid = getPartialAmount( + matchedFillResults.left.takerFeePaid = getPartialAmountFloor( matchedFillResults.left.takerAssetFilledAmount, leftOrder.takerAssetAmount, leftOrder.takerFee ); // Compute fees for right order - matchedFillResults.right.makerFeePaid = getPartialAmount( + matchedFillResults.right.makerFeePaid = getPartialAmountFloor( matchedFillResults.right.makerAssetFilledAmount, rightOrder.makerAssetAmount, rightOrder.makerFee ); - matchedFillResults.right.takerFeePaid = getPartialAmount( + matchedFillResults.right.takerFeePaid = getPartialAmountFloor( matchedFillResults.right.takerAssetFilledAmount, rightOrder.takerAssetAmount, rightOrder.takerFee -- cgit v1.2.3 From ec2e726be0a653465d3c83e640aa4c1556a0f10f Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Fri, 24 Aug 2018 18:09:32 -0700 Subject: Rephrased some of the math in MixinMatchOrders to improve readability --- packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol') diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol b/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol index 8f289ee85..226a1bfb6 100644 --- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol +++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol @@ -184,12 +184,12 @@ contract MixinMatchOrders is // Case 1: Right order is fully filled matchedFillResults.right.makerAssetFilledAmount = rightMakerAssetAmountRemaining; matchedFillResults.right.takerAssetFilledAmount = rightTakerAssetAmountRemaining; + matchedFillResults.left.takerAssetFilledAmount = matchedFillResults.right.makerAssetFilledAmount; matchedFillResults.left.makerAssetFilledAmount = getPartialAmountFloor( leftOrder.makerAssetAmount, leftOrder.takerAssetAmount, - matchedFillResults.right.makerAssetFilledAmount + matchedFillResults.left.takerAssetFilledAmount ); - matchedFillResults.left.takerAssetFilledAmount = matchedFillResults.right.makerAssetFilledAmount; } else { // Case 2: Left order is fully filled matchedFillResults.left.makerAssetFilledAmount = leftMakerAssetAmountRemaining; @@ -198,7 +198,7 @@ contract MixinMatchOrders is matchedFillResults.right.takerAssetFilledAmount = getPartialAmountCeil( rightOrder.takerAssetAmount, rightOrder.makerAssetAmount, - matchedFillResults.left.takerAssetFilledAmount + matchedFillResults.right.makerAssetFilledAmount ); } -- cgit v1.2.3 From 878db3b84993d7c9724c0ed8591493a4e2b6cc94 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Fri, 24 Aug 2018 18:48:29 -0700 Subject: Added comments to order matching --- .../src/2.0.0/protocol/Exchange/MixinMatchOrders.sol | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) (limited to 'packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol') diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol b/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol index 226a1bfb6..4b8360c23 100644 --- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol +++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol @@ -179,12 +179,22 @@ contract MixinMatchOrders is rightTakerAssetAmountRemaining ); - // Calculate fill results for maker and taker assets + // Calculate fill results for maker and taker assets: at least one order will be fully filled. + // The maximum amount the left maker can buy is `leftTakerAssetAmountRemaining` + // The maximum amount the right maker can sell is `rightMakerAssetAmountRemaining` + // We have two distinct cases for calculating the fill results: + // Case 1. + // If the left maker can buy more than the right maker can sell, then only the right order is fully filled. + // If the left maker can buy exactly what the right maker can sell, then both orders are fully filled. + // Case 2. + // If the left maker cannot buy more than the right maker can sell, then only the left order is fully filled. if (leftTakerAssetAmountRemaining >= rightMakerAssetAmountRemaining) { // Case 1: Right order is fully filled matchedFillResults.right.makerAssetFilledAmount = rightMakerAssetAmountRemaining; matchedFillResults.right.takerAssetFilledAmount = rightTakerAssetAmountRemaining; matchedFillResults.left.takerAssetFilledAmount = matchedFillResults.right.makerAssetFilledAmount; + // Round down to ensure the maker's exchange rate does not exceed the price specified by the order. + // We favor the maker when the exchange rate must be rounded. matchedFillResults.left.makerAssetFilledAmount = getPartialAmountFloor( leftOrder.makerAssetAmount, leftOrder.takerAssetAmount, @@ -195,6 +205,8 @@ contract MixinMatchOrders is matchedFillResults.left.makerAssetFilledAmount = leftMakerAssetAmountRemaining; matchedFillResults.left.takerAssetFilledAmount = leftTakerAssetAmountRemaining; matchedFillResults.right.makerAssetFilledAmount = matchedFillResults.left.takerAssetFilledAmount; + // Round up to ensure the maker's exchange rate does not exceed the price specified by the order. + // We favor the maker when the exchange rate must be rounded. matchedFillResults.right.takerAssetFilledAmount = getPartialAmountCeil( rightOrder.takerAssetAmount, rightOrder.makerAssetAmount, -- cgit v1.2.3