Skip to content
This repository has been archived by the owner on Jul 9, 2021. It is now read-only.

Commit

Permalink
Merge pull request #2043 from jalextowle/feature/contracts/3.0/order-…
Browse files Browse the repository at this point in the history
…matching-unit-tests

MatchOrders Unit Tests
  • Loading branch information
jalextowle committed Aug 14, 2019
2 parents f66212c + 6b4e632 commit 434d027
Show file tree
Hide file tree
Showing 6 changed files with 2,152 additions and 125 deletions.
221 changes: 110 additions & 111 deletions contracts/exchange/contracts/src/MixinMatchOrders.sol
Original file line number Diff line number Diff line change
Expand Up @@ -473,6 +473,116 @@ contract MixinMatchOrders is
);
}

/// @dev Settles matched order by transferring appropriate funds between order makers, taker, and fee recipient.
/// @param leftOrderHash First matched order hash.
/// @param rightOrderHash Second matched order hash.
/// @param leftOrder First matched order.
/// @param rightOrder Second matched order.
/// @param takerAddress Address that matched the orders. The taker receives the spread between orders as profit.
/// @param matchedFillResults Struct holding amounts to transfer between makers, taker, and fee recipients.
function _settleMatchedOrders(
bytes32 leftOrderHash,
bytes32 rightOrderHash,
LibOrder.Order memory leftOrder,
LibOrder.Order memory rightOrder,
address takerAddress,
LibFillResults.MatchedFillResults memory matchedFillResults
)
internal
{
address leftFeeRecipientAddress = leftOrder.feeRecipientAddress;
address rightFeeRecipientAddress = rightOrder.feeRecipientAddress;

// Right maker asset -> left maker
_dispatchTransferFrom(
rightOrderHash,
rightOrder.makerAssetData,
rightOrder.makerAddress,
leftOrder.makerAddress,
matchedFillResults.left.takerAssetFilledAmount
);

// Left maker asset -> right maker
_dispatchTransferFrom(
leftOrderHash,
leftOrder.makerAssetData,
leftOrder.makerAddress,
rightOrder.makerAddress,
matchedFillResults.right.takerAssetFilledAmount
);

// Right maker fee -> right fee recipient
_dispatchTransferFrom(
rightOrderHash,
rightOrder.makerFeeAssetData,
rightOrder.makerAddress,
rightFeeRecipientAddress,
matchedFillResults.right.makerFeePaid
);

// Left maker fee -> left fee recipient
_dispatchTransferFrom(
leftOrderHash,
leftOrder.makerFeeAssetData,
leftOrder.makerAddress,
leftFeeRecipientAddress,
matchedFillResults.left.makerFeePaid
);

// Settle taker profits.
_dispatchTransferFrom(
leftOrderHash,
leftOrder.makerAssetData,
leftOrder.makerAddress,
takerAddress,
matchedFillResults.profitInLeftMakerAsset
);
_dispatchTransferFrom(
rightOrderHash,
rightOrder.makerAssetData,
rightOrder.makerAddress,
takerAddress,
matchedFillResults.profitInRightMakerAsset
);

// Settle taker fees.
if (
leftFeeRecipientAddress == rightFeeRecipientAddress &&
leftOrder.takerFeeAssetData.equals(rightOrder.takerFeeAssetData)
) {
// Fee recipients and taker fee assets are identical, so we can
// transfer them in one go.
_dispatchTransferFrom(
leftOrderHash,
leftOrder.takerFeeAssetData,
takerAddress,
leftFeeRecipientAddress,
_safeAdd(
matchedFillResults.left.takerFeePaid,
matchedFillResults.right.takerFeePaid
)
);
} else {
// Right taker fee -> right fee recipient
_dispatchTransferFrom(
rightOrderHash,
rightOrder.takerFeeAssetData,
takerAddress,
rightFeeRecipientAddress,
matchedFillResults.right.takerFeePaid
);

// Left taker fee -> left fee recipient
_dispatchTransferFrom(
leftOrderHash,
leftOrder.takerFeeAssetData,
takerAddress,
leftFeeRecipientAddress,
matchedFillResults.left.takerFeePaid
);
}
}

/// @dev Match complementary orders that have a profitable spread.
/// Each order is filled at their respective price point, and
/// the matcher receives a profit denominated in the left maker asset.
Expand Down Expand Up @@ -708,115 +818,4 @@ contract MixinMatchOrders is

return matchedFillResults;
}

/// @dev Settles matched order by transferring appropriate funds between order makers, taker, and fee recipient.
/// @param leftOrderHash First matched order hash.
/// @param rightOrderHash Second matched order hash.
/// @param leftOrder First matched order.
/// @param rightOrder Second matched order.
/// @param takerAddress Address that matched the orders. The taker receives the spread between orders as profit.
/// @param matchedFillResults Struct holding amounts to transfer between makers, taker, and fee recipients.
function _settleMatchedOrders(
bytes32 leftOrderHash,
bytes32 rightOrderHash,
LibOrder.Order memory leftOrder,
LibOrder.Order memory rightOrder,
address takerAddress,
LibFillResults.MatchedFillResults memory matchedFillResults
)
private
{
address leftFeeRecipientAddress = leftOrder.feeRecipientAddress;
address rightFeeRecipientAddress = rightOrder.feeRecipientAddress;

// Right maker asset -> left maker
_dispatchTransferFrom(
rightOrderHash,
rightOrder.makerAssetData,
rightOrder.makerAddress,
leftOrder.makerAddress,
matchedFillResults.left.takerAssetFilledAmount
);

// Left maker asset -> right maker
_dispatchTransferFrom(
leftOrderHash,
leftOrder.makerAssetData,
leftOrder.makerAddress,
rightOrder.makerAddress,
matchedFillResults.right.takerAssetFilledAmount
);

// Right maker fee -> right fee recipient
_dispatchTransferFrom(
rightOrderHash,
rightOrder.makerFeeAssetData,
rightOrder.makerAddress,
rightFeeRecipientAddress,
matchedFillResults.right.makerFeePaid
);

// Left maker fee -> left fee recipient
_dispatchTransferFrom(
leftOrderHash,
leftOrder.makerFeeAssetData,
leftOrder.makerAddress,
leftFeeRecipientAddress,
matchedFillResults.left.makerFeePaid
);

// Settle taker profits.
_dispatchTransferFrom(
leftOrderHash,
leftOrder.makerAssetData,
leftOrder.makerAddress,
takerAddress,
matchedFillResults.profitInLeftMakerAsset
);

_dispatchTransferFrom(
rightOrderHash,
rightOrder.makerAssetData,
rightOrder.makerAddress,
takerAddress,
matchedFillResults.profitInRightMakerAsset
);

// Settle taker fees.
if (
leftFeeRecipientAddress == rightFeeRecipientAddress &&
leftOrder.takerFeeAssetData.equals(rightOrder.takerFeeAssetData)
) {
// Fee recipients and taker fee assets are identical, so we can
// transfer them in one go.
_dispatchTransferFrom(
leftOrderHash,
leftOrder.takerFeeAssetData,
takerAddress,
leftFeeRecipientAddress,
_safeAdd(
matchedFillResults.left.takerFeePaid,
matchedFillResults.right.takerFeePaid
)
);
} else {
// Right taker fee -> right fee recipient
_dispatchTransferFrom(
rightOrderHash,
rightOrder.takerFeeAssetData,
takerAddress,
rightFeeRecipientAddress,
matchedFillResults.right.takerFeePaid
);

// Left taker fee -> left fee recipient
_dispatchTransferFrom(
leftOrderHash,
leftOrder.takerFeeAssetData,
takerAddress,
leftFeeRecipientAddress,
matchedFillResults.left.takerFeePaid
);
}
}
}
67 changes: 67 additions & 0 deletions contracts/exchange/contracts/test/TestExchangeInternals.sol
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,16 @@ contract TestExchangeInternals is
Exchange(chainId)
{}

function assertValidMatch(
LibOrder.Order memory leftOrder,
LibOrder.Order memory rightOrder
)
public
view
{
_assertValidMatch(leftOrder, rightOrder);
}

function calculateFillResults(
Order memory order,
uint256 takerAssetFilledAmount
Expand All @@ -50,6 +60,43 @@ contract TestExchangeInternals is
return _calculateFillResults(order, takerAssetFilledAmount);
}

function calculateCompleteFillBoth(
uint256 leftMakerAssetAmountRemaining,
uint256 leftTakerAssetAmountRemaining,
uint256 rightMakerAssetAmountRemaining,
uint256 rightTakerAssetAmountRemaining
)
public
pure
returns (MatchedFillResults memory fillResults)
{
_calculateCompleteFillBoth(
fillResults,
leftMakerAssetAmountRemaining,
leftTakerAssetAmountRemaining,
rightMakerAssetAmountRemaining,
rightTakerAssetAmountRemaining
);
return fillResults;
}

function calculateCompleteRightFill(
LibOrder.Order memory leftOrder,
uint256 rightMakerAssetAmountRemaining,
uint256 rightTakerAssetAmountRemaining
)
public
pure
returns (MatchedFillResults memory fillResults)
{
_calculateCompleteRightFill(
fillResults,
leftOrder,
rightMakerAssetAmountRemaining,
rightTakerAssetAmountRemaining
);
}

/// @dev Call `_updateFilledState()` but first set `filled[order]` to
/// `orderTakerAssetFilledAmount`.
function testUpdateFilledState(
Expand Down Expand Up @@ -82,6 +129,26 @@ contract TestExchangeInternals is
_settleOrder(orderHash, order, takerAddress, fillResults);
}

function settleMatchOrders(
bytes32 leftOrderHash,
bytes32 rightOrderHash,
LibOrder.Order memory leftOrder,
LibOrder.Order memory rightOrder,
address takerAddress,
LibFillResults.MatchedFillResults memory matchedFillResults
)
public
{
_settleMatchedOrders(
leftOrderHash,
rightOrderHash,
leftOrder,
rightOrder,
takerAddress,
matchedFillResults
);
}

/// @dev Overidden to only log arguments so we can test `_settleOrder()`.
function _dispatchTransferFrom(
bytes32 orderHash,
Expand Down
4 changes: 2 additions & 2 deletions contracts/exchange/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@
"lint-contracts": "solhint -c ../.solhint.json contracts/**/**/**/**/*.sol"
},
"config": {
"abis": "./generated-artifacts/@(Exchange|ExchangeWrapper|IAssetProxyDispatcher|IEIP1271Wallet|IExchange|IExchangeCore|IMatchOrders|ISignatureValidator|ITransactions|IWallet|IWrapperFunctions|IsolatedExchange|ReentrancyTester|TestAssetProxyDispatcher|TestExchangeInternals|TestLibExchangeRichErrorDecoder|TestSignatureValidator|TestValidatorWallet|TestWrapperFunctions|Whitelist).json",
"abis:comment": "This list is auto-generated by contracts-gen. Don't edit manually."
"abis:comment": "This list is auto-generated by contracts-gen. Don't edit manually.",
"abis": "./generated-artifacts/@(Exchange|ExchangeWrapper|IAssetProxyDispatcher|IEIP1271Wallet|IExchange|IExchangeCore|IMatchOrders|ISignatureValidator|ITransactions|IWallet|IWrapperFunctions|IsolatedExchange|ReentrancyTester|TestAssetProxyDispatcher|TestExchangeInternals|TestLibExchangeRichErrorDecoder|TestSignatureValidator|TestValidatorWallet|TestWrapperFunctions|Whitelist).json"
},
"repository": {
"type": "git",
Expand Down
2 changes: 1 addition & 1 deletion contracts/exchange/test/dispatcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ describe('AssetProxyDispatcher', () => {
return expect(tx).to.revertWith(expectedError);
});

it('should should revert with the correct error when assetData length < 4 bytes', async () => {
it('should revert with the correct error when assetData length < 4 bytes', async () => {
await assetProxyDispatcher.registerAssetProxy.awaitTransactionSuccessAsync(erc20Proxy.address, {
from: owner,
});
Expand Down
Loading

0 comments on commit 434d027

Please sign in to comment.