diff --git a/contracts/FuturesMarket.sol b/contracts/FuturesMarket.sol index 1e73e62614..d903a5cbe4 100644 --- a/contracts/FuturesMarket.sol +++ b/contracts/FuturesMarket.sol @@ -433,7 +433,7 @@ contract FuturesMarket is Owned, Proxyable, MixinFuturesMarketSettings, IFutures return false; } - function _notionalValue(Position storage position, uint price) internal view returns (int value) { + function _notionalValue(Position memory position, uint price) internal pure returns (int value) { return position.size.multiplyDecimalRound(int(price)); } @@ -445,7 +445,7 @@ contract FuturesMarket is Owned, Proxyable, MixinFuturesMarketSettings, IFutures return (_notionalValue(positions[account], price), isInvalid); } - function _profitLoss(Position storage position, uint price) internal view returns (int pnl) { + function _profitLoss(Position memory position, uint price) internal pure returns (int pnl) { int priceShift = int(price).sub(int(position.lastPrice)); return position.size.multiplyDecimalRound(priceShift); } @@ -460,7 +460,7 @@ contract FuturesMarket is Owned, Proxyable, MixinFuturesMarketSettings, IFutures } function _accruedFunding( - Position storage position, + Position memory position, uint endFundingIndex, uint price ) internal view returns (int funding) { @@ -484,15 +484,41 @@ contract FuturesMarket is Owned, Proxyable, MixinFuturesMarketSettings, IFutures * The initial margin of a position, plus any PnL and funding it has accrued. The resulting value may be negative. */ function _marginPlusProfitFunding( - Position storage position, + Position memory position, uint endFundingIndex, uint price ) internal view returns (int) { return int(position.margin).add(_profitLoss(position, price)).add(_accruedFunding(position, endFundingIndex, price)); } + /* + * The value in a position's margin after a deposit or withdrawal, accounting for funding and profit. + * If the resulting margin would be negative or below the liquidation threshold, an appropriate error is returned. + * If the result is not an error, callers of this function that use it to update a position's margin + * must ensure that this is accompanied by a corresponding debt correction update, as per `_applyDebtCorrection`. + */ + function _realisedMargin( + Position memory position, + uint currentFundingIndex, + uint price, + int marginDelta + ) internal view returns (uint margin, Status statusCode) { + int newMargin = _marginPlusProfitFunding(position, currentFundingIndex, price).add(marginDelta); + if (newMargin < 0) { + return (0, Status.InsufficientMargin); + } + + uint uMargin = uint(newMargin); + int positionSize = position.size; + if (positionSize != 0 && uMargin <= _liquidationFee()) { + return (uMargin, Status.CanLiquidate); + } + + return (uMargin, Status.Ok); + } + function _remainingMargin( - Position storage position, + Position memory position, uint endFundingIndex, uint price ) internal view returns (uint) { @@ -512,7 +538,7 @@ contract FuturesMarket is Owned, Proxyable, MixinFuturesMarketSettings, IFutures } function _liquidationPrice( - Position storage position, + Position memory position, bool includeFunding, uint fundingIndex, uint currentPrice @@ -574,7 +600,7 @@ contract FuturesMarket is Owned, Proxyable, MixinFuturesMarketSettings, IFutures } function _canLiquidate( - Position storage position, + Position memory position, uint liquidationFee, uint fundingIndex, uint price @@ -596,10 +622,10 @@ contract FuturesMarket is Owned, Proxyable, MixinFuturesMarketSettings, IFutures } function _currentLeverage( - Position storage position, + Position memory position, uint price, uint remainingMargin_ - ) internal view returns (int leverage) { + ) internal pure returns (int leverage) { // No position is open, or it is ready to be liquidated; leverage goes to nil if (remainingMargin_ == 0) { return 0; @@ -681,6 +707,109 @@ contract FuturesMarket is Owned, Proxyable, MixinFuturesMarketSettings, IFutures return (_orderFee(positionSize.add(sizeDelta), positionSize, price), isInvalid); } + function _postTradeDetails( + Position memory oldPos, + int sizeDelta, + uint price, + uint fundingIndex + ) + internal + view + returns ( + Position memory newPosition, + uint _fee, + Status tradeStatus + ) + { + // Reverts if the user is trying to submit a size-zero order. + if (sizeDelta == 0) { + return (oldPos, 0, Status.NilOrder); + } + + // The order is not submitted if the user's existing position needs to be liquidated. + if (_canLiquidate(oldPos, _liquidationFee(), fundingIndex, price)) { + return (oldPos, 0, Status.CanLiquidate); + } + + int newSize = oldPos.size.add(sizeDelta); + + // Deduct the fee. + // It is an error if the realised margin minus the fee is negative or subject to liquidation. + uint fee = _orderFee(newSize, oldPos.size, price); + (uint newMargin, Status status) = _realisedMargin(oldPos, fundingIndex, price, -int(fee)); + if (_isError(status)) { + return (oldPos, 0, status); + } + Position memory newPos = Position(oldPos.id, newMargin, newSize, price, fundingIndex); + + // Check that the user has sufficient margin given their order. + // We don't check the margin requirement if the position size is decreasing + bool positionDecreasing = _sameSide(oldPos.size, newPos.size) && _abs(newPos.size) < _abs(oldPos.size); + if (!positionDecreasing) { + // minMargin + fee <= margin is equivalent to minMargin <= margin - fee + // except that we get a nicer error message if fee > margin, rather than arithmetic overflow. + if (newPos.margin.add(fee) < _minInitialMargin()) { + return (oldPos, 0, Status.InsufficientMargin); + } + } + + // Check that the maximum leverage is not exceeded (ignoring the fee). + // We'll allow a little extra headroom for rounding errors. + int leverage = newSize.multiplyDecimalRound(int(price)).divideDecimalRound(int(newMargin.add(fee))); + if (_maxLeverage(baseAsset).add(uint(_UNIT) / 100) < _abs(leverage)) { + return (oldPos, 0, Status.MaxLeverageExceeded); + } + + // Check that the order isn't too large for the market. + // Allow a bit of extra value in case of rounding errors. + if ( + _orderSizeTooLarge( + uint(int(_maxMarketValue(baseAsset).add(100 * uint(_UNIT))).divideDecimalRound(int(price))), + oldPos.size, + newPos.size + ) + ) { + return (oldPos, 0, Status.MaxMarketSizeExceeded); + } + + return (newPos, fee, Status.Ok); + } + + /* + * Returns all new position details if a given order from `sender` was confirmed at the current price. + */ + function postTradeDetails(int sizeDelta, address sender) + external + view + returns ( + uint margin, + int size, + uint price, + uint liqPrice, + uint fee, + Status status + ) + { + bool invalid; + (price, invalid) = _assetPrice(_exchangeRates()); + if (invalid) { + return (0, 0, 0, 0, 0, Status.InvalidPrice); + } + + uint fundingIndex = fundingSequence.length; + (Position memory newPosition, uint fee_, Status status_) = + _postTradeDetails(positions[sender], sizeDelta, price, fundingIndex); + + return ( + newPosition.margin, + newPosition.size, + newPosition.lastPrice, + _liquidationPrice(newPosition, true, fundingIndex, newPosition.lastPrice), + fee_, + status_ + ); + } + /* ---------- Utilities ---------- */ /* @@ -780,32 +909,6 @@ contract FuturesMarket is Owned, Proxyable, MixinFuturesMarketSettings, IFutures _entryDebtCorrection = _entryDebtCorrection.add(newCorrection).sub(oldCorrection); } - /* - * The value in a position's margin after a deposit or withdrawal, accounting for funding and profit. - * If the resulting margin would be negative or below the liquidation threshold, an appropriate error is returned. - * If the result is not an error, callers of this function that use it to update a position's margin - * must ensure that this is accompanied by a corresponding debt correction update, as per `_applyDebtCorrection`. - */ - function _realisedMargin( - Position storage position, - uint currentFundingIndex, - uint price, - int marginDelta - ) internal view returns (uint margin, Status statusCode) { - int newMargin = _marginPlusProfitFunding(position, currentFundingIndex, price).add(marginDelta); - if (newMargin < 0) { - return (0, Status.InsufficientMargin); - } - - uint uMargin = uint(newMargin); - int positionSize = position.size; - if (positionSize != 0 && uMargin <= _liquidationFee()) { - return (uMargin, Status.CanLiquidate); - } - - return (uMargin, Status.Ok); - } - function _transferMargin( int marginDelta, uint price, @@ -831,9 +934,10 @@ contract FuturesMarket is Owned, Proxyable, MixinFuturesMarketSettings, IFutures } Position storage position = positions[sender]; + Position memory _position = position; // Determine new margin, ensuring that the result is positive. - (uint margin, Status status) = _realisedMargin(position, fundingIndex, price, marginDelta); + (uint margin, Status status) = _realisedMargin(_position, fundingIndex, price, marginDelta); _revertIfError(status); // Update the debt correction. @@ -908,80 +1012,44 @@ contract FuturesMarket is Owned, Proxyable, MixinFuturesMarketSettings, IFutures uint fundingIndex, address sender ) internal { - // Reverts if the user is trying to submit a size-zero order. - _revertIfError(sizeDelta == 0, Status.NilOrder); - - // The order is not submitted if the user's existing position needs to be liquidated. Position storage position = positions[sender]; - _revertIfError(_canLiquidate(position, _liquidationFee(), fundingIndex, price), Status.CanLiquidate); - - int oldSize = position.size; - int newSize = position.size.add(sizeDelta); - - // Deduct the fee. - // It is an error if the realised margin minus the fee is negative or subject to liquidation. - uint fee = _orderFee(newSize, oldSize, price); - (uint margin, Status marginStatus) = _realisedMargin(position, fundingIndex, price, -int(fee)); - _revertIfError(marginStatus); - - // Check that the user has sufficient margin given their order. - // We don't check the margin requirement if the position size is decreasing - bool positionDecreasing = _sameSide(oldSize, newSize) && _abs(newSize) < _abs(oldSize); - if (!positionDecreasing) { - // minMargin + fee <= margin is equivalent to minMargin <= margin - fee - // except that we get a nicer error message if fee > margin, rather than arithmetic overflow. - _revertIfError(margin.add(fee) < _minInitialMargin(), Status.InsufficientMargin); - } - - // Check that the maximum leverage is not exceeded (ignoring the fee). - // We'll allow a little extra headroom for rounding errors. - int desiredLeverage = newSize.multiplyDecimalRound(int(price)).divideDecimalRound(int(margin.add(fee))); - _revertIfError(_maxLeverage(baseAsset).add(uint(_UNIT) / 100) < _abs(desiredLeverage), Status.MaxLeverageExceeded); - - // Check that the order isn't too large for the market. - // Allow a bit of extra value in case of rounding errors. - _revertIfError( - _orderSizeTooLarge( - uint(int(_maxMarketValue(baseAsset).add(100 * uint(_UNIT))).divideDecimalRound(int(price))), - oldSize, - newSize - ), - Status.MaxMarketSizeExceeded - ); + Position memory oldPosition = position; - // Update the margin, and apply the resulting debt correction - _applyDebtCorrection( - Position(0, margin, newSize, price, fundingIndex), - Position(0, position.margin, oldSize, position.lastPrice, position.fundingIndex) - ); - position.margin = margin; + // Compute the new position after performing the trade + (Position memory newPosition, uint fee, Status status) = + _postTradeDetails(oldPosition, sizeDelta, price, fundingIndex); + _revertIfError(status); // Update the aggregated market size and skew with the new order size - marketSkew = marketSkew.add(newSize).sub(oldSize); - marketSize = marketSize.add(_abs(newSize)).sub(_abs(oldSize)); + marketSkew = marketSkew.add(newPosition.size).sub(oldPosition.size); + marketSize = marketSize.add(_abs(newPosition.size)).sub(_abs(oldPosition.size)); // Send the fee to the fee pool if (0 < fee) { _manager().payFee(fee); } - // Actually lodge the position and delete the order - // Updating the margin was already handled above - if (newSize == 0) { + // Update the margin, and apply the resulting debt correction + position.margin = newPosition.margin; + _applyDebtCorrection(newPosition, oldPosition); + + // Record the trade + if (newPosition.size == 0) { // If the position is being closed, we no longer need to track these details. delete position.size; delete position.lastPrice; delete position.fundingIndex; - emitPositionModified(position.id, sender, margin, 0, 0, 0, fee); + emitPositionModified(newPosition.id, sender, newPosition.margin, 0, 0, 0, fee); } else { - if (oldSize == 0) { + // New positions get new id's + if (oldPosition.size == 0) { position.id = _nextPositionId; _nextPositionId += 1; } - position.size = newSize; + position.size = newPosition.size; position.lastPrice = price; position.fundingIndex = fundingIndex; - emitPositionModified(position.id, sender, margin, newSize, price, fundingIndex, fee); + emitPositionModified(newPosition.id, sender, newPosition.margin, newPosition.size, price, fundingIndex, fee); } } diff --git a/contracts/interfaces/IFuturesMarket.sol b/contracts/interfaces/IFuturesMarket.sol index 98f5925ea3..64b062c5f4 100644 --- a/contracts/interfaces/IFuturesMarket.sol +++ b/contracts/interfaces/IFuturesMarket.sol @@ -106,6 +106,18 @@ interface IFuturesMarket { function orderFee(address account, int sizeDelta) external view returns (uint fee, bool invalid); + function postTradeDetails(int sizeDelta, address sender) + external + view + returns ( + uint margin, + int size, + uint price, + uint liqPrice, + uint fee, + Status status + ); + /* ---------- Market Operations ---------- */ function recomputeFunding() external returns (uint lastIndex); diff --git a/test/contracts/FuturesMarket.js b/test/contracts/FuturesMarket.js index 606de647d0..d490a5dd39 100644 --- a/test/contracts/FuturesMarket.js +++ b/test/contracts/FuturesMarket.js @@ -19,6 +19,20 @@ const { const MockExchanger = artifacts.require('MockExchanger'); +const Status = { + Ok: 0, + InvalidPrice: 1, + PriceOutOfBounds: 2, + CanLiquidate: 3, + CannotLiquidate: 4, + MaxMarketSizeExceeded: 5, + MaxLeverageExceeded: 6, + InsufficientMargin: 7, + NotPermitted: 8, + NilOrder: 9, + NoPositionOpen: 10, +}; + contract('FuturesMarket', accounts => { let proxyFuturesMarket, futuresMarketSettings, @@ -1040,9 +1054,23 @@ contract('FuturesMarket', accounts => { await fastForward(4 * 7 * 24 * 60 * 60); + const postDetails = await futuresMarket.postTradeDetails(size, trader); + assert.equal(postDetails.status, Status.InvalidPrice); + await assert.revert(futuresMarket.modifyPosition(size, { from: trader }), 'Invalid price'); }); + it('Empty orders fail', async () => { + const margin = toUnit('1000'); + await futuresMarket.transferMargin(margin, { from: trader }); + await assert.revert( + futuresMarket.modifyPosition(toBN('0'), { from: trader }), + 'Cannot submit empty order' + ); + const postDetails = await futuresMarket.postTradeDetails(toBN('0'), trader); + assert.equal(postDetails.status, Status.NilOrder); + }); + it('Cannot modify a position if the price has slipped too far', async () => { const startPrice = toUnit('200'); await setPrice(baseAsset, startPrice); @@ -1103,8 +1131,13 @@ contract('FuturesMarket', accounts => { await setPrice(baseAsset, toUnit('100')); // User realises the price has crashed and tries to outrun their liquidation, but it fails + + const sizeDelta = toUnit('-50'); + const postDetails = await futuresMarket.postTradeDetails(sizeDelta, trader); + assert.equal(postDetails.status, Status.CanLiquidate); + await assert.revert( - futuresMarket.modifyPosition(toUnit('-50'), { from: trader }), + futuresMarket.modifyPosition(sizeDelta, { from: trader }), 'Position can be liquidated' ); }); @@ -1156,11 +1189,15 @@ contract('FuturesMarket', accounts => { futuresMarket.modifyPosition(toUnit('101'), { from: trader }), 'Max leverage exceeded' ); + let postDetails = await futuresMarket.postTradeDetails(toUnit('101'), trader); + assert.equal(postDetails.status, Status.MaxLeverageExceeded); await assert.revert( futuresMarket.modifyPosition(toUnit('-101'), { from: trader2 }), 'Max leverage exceeded' ); + postDetails = await futuresMarket.postTradeDetails(toUnit('-101'), trader2); + assert.equal(postDetails.status, Status.MaxLeverageExceeded); // But we actually allow up to 10.01x leverage to account for rounding issues. await futuresMarket.modifyPosition(toUnit('100.09'), { from: trader }); @@ -1174,6 +1211,9 @@ contract('FuturesMarket', accounts => { futuresMarket.modifyPosition(toUnit('10'), { from: trader }), 'Insufficient margin' ); + + const postDetails = await futuresMarket.postTradeDetails(toUnit('10'), trader); + assert.equal(postDetails.status, Status.InsufficientMargin); }); describe('Max market size constraints', () => { @@ -1283,6 +1323,10 @@ contract('FuturesMarket', accounts => { it('Orders are blocked if they exceed max market size', async () => { await futuresMarket.transferMargin(maxMargin.add(toUnit('11')), { from: trader }); const tooBig = orderSize.div(toBN('10')).mul(toBN('11')); + + const postDetails = await futuresMarket.postTradeDetails(tooBig, trader); + assert.equal(postDetails.status, Status.MaxMarketSizeExceeded); + await assert.revert( futuresMarket.modifyPosition(tooBig, { from: trader, @@ -1309,8 +1353,12 @@ contract('FuturesMarket', accounts => { // The price moves, so the value of the already-confirmed order shunts out the pending one. await setPrice(baseAsset, toUnit('1.08')); + + const sizeDelta = orderSize.div(toBN(100)).mul(toBN(25)); + const postDetails = await futuresMarket.postTradeDetails(sizeDelta, trader); + assert.equal(postDetails.status, Status.MaxMarketSizeExceeded); await assert.revert( - futuresMarket.modifyPosition(orderSize.div(toBN(100)).mul(toBN(25)), { + futuresMarket.modifyPosition(sizeDelta, { from: trader, }), 'Max market size exceeded' @@ -1459,6 +1507,65 @@ contract('FuturesMarket', accounts => { assert.bnEqual(newPositionId, toBN('2')); }); }); + + describe('post-trade position details', async () => { + const getPositionDetails = async ({ account }) => { + const newPosition = await futuresMarket.positions(account); + const { price: liquidationPrice } = await futuresMarket.liquidationPrice(account, true); + return { + ...newPosition, + liquidationPrice, + }; + }; + const sizeDelta = toUnit('10'); + + it('can get position details for new position', async () => { + await futuresMarket.transferMargin(toUnit('1000'), { from: trader }); + await setPrice(await futuresMarket.baseAsset(), toUnit('240')); + + const expectedDetails = await futuresMarket.postTradeDetails(sizeDelta, trader); + + // Now execute the trade. + await futuresMarket.modifyPosition(sizeDelta, { + from: trader, + }); + + const details = await getPositionDetails({ account: trader }); + + assert.bnClose(expectedDetails.margin, details.margin, toUnit(0.01)); // one block of funding rate has accrued + assert.bnEqual(expectedDetails.size, details.size); + assert.bnEqual(expectedDetails.price, details.lastPrice); + assert.bnClose(expectedDetails.liqPrice, details.liquidationPrice, toUnit(0.01)); + assert.bnEqual(expectedDetails.fee, toUnit('7.2')); + assert.bnEqual(expectedDetails.status, Status.Ok); + }); + + it('uses the margin of an existing position', async () => { + await transferMarginAndModifyPosition({ + market: futuresMarket, + account: trader, + fillPrice: toUnit('240'), + marginDelta: toUnit('1000'), + sizeDelta, + }); + + const expectedDetails = await futuresMarket.postTradeDetails(sizeDelta, trader); + + // Now execute the trade. + await futuresMarket.modifyPosition(sizeDelta, { + from: trader, + }); + + const details = await getPositionDetails({ account: trader }); + + assert.bnClose(expectedDetails.margin, details.margin, toUnit(0.01)); // one block of funding rate has accrued + assert.bnEqual(expectedDetails.size, details.size); + assert.bnEqual(expectedDetails.price, details.lastPrice); + assert.bnClose(expectedDetails.positionLiquidationPrice, details.liqPrice, toUnit(0.01)); + assert.bnEqual(expectedDetails.fee, toUnit('7.2')); + assert.bnEqual(expectedDetails.status, Status.Ok); + }); + }); }); describe('Profit & Loss, margin, leverage', () => {