From 3867880a1a4c0a2a1c5e2ef11b8683aafb9ed23b Mon Sep 17 00:00:00 2001 From: artdgn Date: Wed, 2 Feb 2022 17:49:23 +1100 Subject: [PATCH 01/10] remove proxy from futures market --- contracts/FuturesMarket.sol | 7 +- contracts/FuturesMarketBase.sol | 111 ++++-------------- contracts/MixinFuturesNextPriceOrders.sol | 89 ++++++-------- .../test-helpers/TestableFuturesMarket.sol | 7 +- 4 files changed, 64 insertions(+), 150 deletions(-) diff --git a/contracts/FuturesMarket.sol b/contracts/FuturesMarket.sol index 443bad8354..988cff5460 100644 --- a/contracts/FuturesMarket.sol +++ b/contracts/FuturesMarket.sol @@ -61,10 +61,5 @@ import "./interfaces/IFuturesMarket.sol"; // https://docs.synthetix.io/contracts/source/contracts/FuturesMarket contract FuturesMarket is IFuturesMarket, FuturesMarketBase, MixinFuturesNextPriceOrders, MixinFuturesViews { - constructor( - address payable _proxy, - address _owner, - address _resolver, - bytes32 _baseAsset - ) public FuturesMarketBase(_proxy, _owner, _resolver, _baseAsset) {} + constructor(address _resolver, bytes32 _baseAsset) public FuturesMarketBase(_resolver, _baseAsset) {} } diff --git a/contracts/FuturesMarketBase.sol b/contracts/FuturesMarketBase.sol index 2c79405d23..49faa6cf13 100644 --- a/contracts/FuturesMarketBase.sol +++ b/contracts/FuturesMarketBase.sol @@ -1,8 +1,6 @@ pragma solidity ^0.5.16; // Inheritance -import "./Owned.sol"; -import "./Proxyable.sol"; import "./MixinFuturesMarketSettings.sol"; import "./interfaces/IFuturesMarketBaseTypes.sol"; @@ -80,7 +78,7 @@ interface IFuturesMarketManagerInternal { } // https://docs.synthetix.io/contracts/source/contracts/FuturesMarket -contract FuturesMarketBase is Owned, Proxyable, MixinFuturesMarketSettings, IFuturesMarketBaseTypes { +contract FuturesMarketBase is MixinFuturesMarketSettings, IFuturesMarketBaseTypes { /* ========== LIBRARIES ========== */ using SafeMath for uint; @@ -160,12 +158,7 @@ contract FuturesMarketBase is Owned, Proxyable, MixinFuturesMarketSettings, IFut /* ========== CONSTRUCTOR ========== */ - constructor( - address payable _proxy, - address _owner, - address _resolver, - bytes32 _baseAsset - ) public Owned(_owner) Proxyable(_proxy) MixinFuturesMarketSettings(_resolver) { + constructor(address _resolver, bytes32 _baseAsset) public MixinFuturesMarketSettings(_resolver) { baseAsset = _baseAsset; // Initialise the funding sequence with 0 initially accrued, so that the first usable funding index is 1. @@ -743,7 +736,7 @@ contract FuturesMarketBase is Owned, Proxyable, MixinFuturesMarketSettings, IFut int funding = _nextFundingEntry(sequenceLength, price); fundingSequence.push(int128(funding)); fundingLastRecomputed = uint32(block.timestamp); - emitFundingRecomputed(funding); + emit FundingRecomputed(funding); return sequenceLength; } @@ -837,9 +830,9 @@ contract FuturesMarketBase is Owned, Proxyable, MixinFuturesMarketSettings, IFut // Emit relevant events if (marginDelta != 0) { - emitMarginTransferred(sender, marginDelta); + emit MarginTransferred(sender, marginDelta); } - emitPositionModified(position.id, sender, position.margin, position.size, 0, price, fundingIndex, 0); + emit PositionModified(position.id, sender, position.margin, position.size, 0, price, fundingIndex, 0); } // udpates the stored position margin in place (on the stored position) @@ -888,18 +881,18 @@ contract FuturesMarketBase is Owned, Proxyable, MixinFuturesMarketSettings, IFut * Reverts on deposit if the caller lacks a sufficient sUSD balance. * Reverts on withdrawal if the amount to be withdrawn would expose an open position to liquidation. */ - function transferMargin(int marginDelta) external optionalProxy { + function transferMargin(int marginDelta) external { uint price = _assetPriceRequireChecks(); uint fundingIndex = _recomputeFunding(price); - _transferMargin(marginDelta, price, fundingIndex, messageSender); + _transferMargin(marginDelta, price, fundingIndex, msg.sender); } /* * Withdraws all accessible margin in a position. This will leave some remaining margin * in the account if the caller has a position open. Equivalent to `transferMargin(-accessibleMargin(sender))`. */ - function withdrawAllMargin() external optionalProxy { - address sender = messageSender; + function withdrawAllMargin() external { + address sender = msg.sender; uint price = _assetPriceRequireChecks(); uint fundingIndex = _recomputeFunding(price); int marginDelta = -int(_accessibleMargin(positions[sender], fundingIndex, price)); @@ -947,7 +940,7 @@ contract FuturesMarketBase is Owned, Proxyable, MixinFuturesMarketSettings, IFut position.lastFundingIndex = uint64(params.fundingIndex); } // emit the modification event - emitPositionModified( + emit PositionModified( id, sender, newPosition.margin, @@ -963,7 +956,7 @@ contract FuturesMarketBase is Owned, Proxyable, MixinFuturesMarketSettings, IFut * Adjust the sender's position size. * Reverts if the resulting position is too large, outside the max leverage, or is liquidating. */ - function modifyPosition(int sizeDelta) external optionalProxy { + function modifyPosition(int sizeDelta) external { uint price = _assetPriceRequireChecks(); uint fundingIndex = _recomputeFunding(price); TradeParams memory params = @@ -974,7 +967,7 @@ contract FuturesMarketBase is Owned, Proxyable, MixinFuturesMarketSettings, IFut takerFee: _takerFee(baseAsset), makerFee: _makerFee(baseAsset) }); - _modifyPosition(messageSender, params); + _modifyPosition(msg.sender, params); } function _revertIfPriceOutsideBounds( @@ -995,7 +988,7 @@ contract FuturesMarketBase is Owned, Proxyable, MixinFuturesMarketSettings, IFut int sizeDelta, uint minPrice, uint maxPrice - ) external optionalProxy { + ) external { uint price = _assetPriceRequireChecks(); _revertIfPriceOutsideBounds(price, minPrice, maxPrice); TradeParams memory params = @@ -1006,14 +999,14 @@ contract FuturesMarketBase is Owned, Proxyable, MixinFuturesMarketSettings, IFut takerFee: _takerFee(baseAsset), makerFee: _makerFee(baseAsset) }); - _modifyPosition(messageSender, params); + _modifyPosition(msg.sender, params); } /* * Submit an order to close a position. */ - function closePosition() external optionalProxy { - int size = positions[messageSender].size; + function closePosition() external { + int size = positions[msg.sender].size; _revertIfError(size == 0, Status.NoPositionOpen); uint price = _assetPriceRequireChecks(); TradeParams memory params = @@ -1024,14 +1017,14 @@ contract FuturesMarketBase is Owned, Proxyable, MixinFuturesMarketSettings, IFut takerFee: _takerFee(baseAsset), makerFee: _makerFee(baseAsset) }); - _modifyPosition(messageSender, params); + _modifyPosition(msg.sender, params); } /* * Submit an order to close a position; reverts if the asset price is outside the specified bounds. */ - function closePositionWithPriceBounds(uint minPrice, uint maxPrice) external optionalProxy { - int size = positions[messageSender].size; + function closePositionWithPriceBounds(uint minPrice, uint maxPrice) external { + int size = positions[msg.sender].size; _revertIfError(size == 0, Status.NoPositionOpen); uint price = _assetPriceRequireChecks(); _revertIfPriceOutsideBounds(price, minPrice, maxPrice); @@ -1043,7 +1036,7 @@ contract FuturesMarketBase is Owned, Proxyable, MixinFuturesMarketSettings, IFut takerFee: _takerFee(baseAsset), makerFee: _makerFee(baseAsset) }); - _modifyPosition(messageSender, params); + _modifyPosition(msg.sender, params); } function _liquidatePosition( @@ -1075,8 +1068,8 @@ contract FuturesMarketBase is Owned, Proxyable, MixinFuturesMarketSettings, IFut uint liqFee = _liquidationFee(positionSize, price); _manager().issueSUSD(liquidator, liqFee); - emitPositionModified(positionId, account, 0, 0, 0, price, fundingIndex, 0); - emitPositionLiquidated(positionId, account, liquidator, positionSize, price, liqFee); + emit PositionModified(positionId, account, 0, 0, 0, price, fundingIndex, 0); + emit PositionLiquidated(positionId, account, liquidator, positionSize, price, liqFee); // Send any positive margin buffer to the fee pool if (remMargin > liqFee) { @@ -1089,27 +1082,18 @@ contract FuturesMarketBase is Owned, Proxyable, MixinFuturesMarketSettings, IFut * `canLiquidate(account)` is true, and reverts otherwise. * Upon liquidation, the position will be closed, and the liquidation fee minted into the liquidator's account. */ - function liquidatePosition(address account) external optionalProxy { + function liquidatePosition(address account) external { uint price = _assetPriceRequireChecks(); uint fundingIndex = _recomputeFunding(price); _revertIfError(!_canLiquidate(positions[account], fundingIndex, price), Status.CannotLiquidate); - _liquidatePosition(account, messageSender, fundingIndex, price); + _liquidatePosition(account, msg.sender, fundingIndex, price); } /* ========== EVENTS ========== */ - function addressToBytes32(address input) internal pure returns (bytes32) { - return bytes32(uint256(uint160(input))); - } - event MarginTransferred(address indexed account, int marginDelta); - bytes32 internal constant SIG_MARGINTRANSFERRED = keccak256("MarginTransferred(address,int256)"); - - function emitMarginTransferred(address account, int marginDelta) internal { - proxy._emit(abi.encode(marginDelta), 2, SIG_MARGINTRANSFERRED, addressToBytes32(account), 0, 0); - } event PositionModified( uint indexed id, @@ -1121,28 +1105,6 @@ contract FuturesMarketBase is Owned, Proxyable, MixinFuturesMarketSettings, IFut uint fundingIndex, uint fee ); - bytes32 internal constant SIG_POSITIONMODIFIED = - keccak256("PositionModified(uint256,address,uint256,int256,int256,uint256,uint256,uint256)"); - - function emitPositionModified( - uint id, - address account, - uint margin, - int size, - int tradeSize, - uint lastPrice, - uint fundingIndex, - uint fee - ) internal { - proxy._emit( - abi.encode(margin, size, tradeSize, lastPrice, fundingIndex, fee), - 3, - SIG_POSITIONMODIFIED, - bytes32(id), - addressToBytes32(account), - 0 - ); - } event PositionLiquidated( uint indexed id, @@ -1152,31 +1114,6 @@ contract FuturesMarketBase is Owned, Proxyable, MixinFuturesMarketSettings, IFut uint price, uint fee ); - bytes32 internal constant SIG_POSITIONLIQUIDATED = - keccak256("PositionLiquidated(uint256,address,address,int256,uint256,uint256)"); - - function emitPositionLiquidated( - uint id, - address account, - address liquidator, - int size, - uint price, - uint fee - ) internal { - proxy._emit( - abi.encode(size, price, fee), - 4, - SIG_POSITIONLIQUIDATED, - bytes32(id), - addressToBytes32(account), - addressToBytes32(liquidator) - ); - } event FundingRecomputed(int funding); - bytes32 internal constant SIG_FUNDINGRECOMPUTED = keccak256("FundingRecomputed(int256)"); - - function emitFundingRecomputed(int funding) internal { - proxy._emit(abi.encode(funding), 1, SIG_FUNDINGRECOMPUTED, 0, 0, 0); - } } diff --git a/contracts/MixinFuturesNextPriceOrders.sol b/contracts/MixinFuturesNextPriceOrders.sol index d74dd9adce..dc426705d1 100644 --- a/contracts/MixinFuturesNextPriceOrders.sol +++ b/contracts/MixinFuturesNextPriceOrders.sol @@ -28,12 +28,12 @@ contract MixinFuturesNextPriceOrders is FuturesMarketBase { * incorrectly submitted orders (that cannot be filled). * @param sizeDelta size in baseAsset (notional terms) of the order, similar to `modifyPosition` interface */ - function submitNextPriceOrder(int sizeDelta) external optionalProxy { + function submitNextPriceOrder(int sizeDelta) external { // check that a previous order doesn't exist - require(nextPriceOrders[messageSender].sizeDelta == 0, "previous order exists"); + require(nextPriceOrders[msg.sender].sizeDelta == 0, "previous order exists"); // storage position as it's going to be modified to deduct commitFee and keeperFee - Position storage position = positions[messageSender]; + Position storage position = positions[msg.sender]; // to prevent submitting bad orders in good faith and being charged commitDeposit for them // simulate the order with current price and market and check that the order doesn't revert @@ -55,7 +55,7 @@ contract MixinFuturesNextPriceOrders is FuturesMarketBase { uint keeperDeposit = _minKeeperFee(); _updatePositionMargin(position, fundingIndex, price, -int(commitDeposit + keeperDeposit)); // emit event for modidying the position (subtracting the fees from margin) - emitPositionModified(position.id, messageSender, position.margin, position.size, 0, price, fundingIndex, 0); + emit PositionModified(position.id, msg.sender, position.margin, position.size, 0, price, fundingIndex, 0); // create order uint targetRoundId = _exchangeRates().getCurrentRoundId(baseAsset) + 1; // next round @@ -67,9 +67,15 @@ contract MixinFuturesNextPriceOrders is FuturesMarketBase { keeperDeposit: uint128(keeperDeposit) }); // emit event - emitNextPriceOrderSubmitted(messageSender, order); + emit NextPriceOrderSubmitted( + msg.sender, + order.sizeDelta, + order.targetRoundId, + order.commitDeposit, + order.keeperDeposit + ); // store order - nextPriceOrders[messageSender] = order; + nextPriceOrders[msg.sender] = order; } /** @@ -84,15 +90,15 @@ contract MixinFuturesNextPriceOrders is FuturesMarketBase { * or send to the msg.sender if it's not the account holder. * @param account the account for which the stored order should be cancelled */ - function cancelNextPriceOrder(address account) external optionalProxy { - // important!! order of the account, not the messageSender + function cancelNextPriceOrder(address account) external { + // important!! order of the account, not the msg.sender NextPriceOrder memory order = nextPriceOrders[account]; // check that a previous order exists require(order.sizeDelta != 0, "no previous order"); uint currentRoundId = _exchangeRates().getCurrentRoundId(baseAsset); - if (account == messageSender) { + if (account == msg.sender) { // this is account owner // refund keeper fee to margin Position storage position = positions[account]; @@ -101,7 +107,7 @@ contract MixinFuturesNextPriceOrders is FuturesMarketBase { _updatePositionMargin(position, fundingIndex, price, int(order.keeperDeposit)); // emit event for modidying the position (add the fee to margin) - emitPositionModified(position.id, account, position.margin, position.size, 0, price, fundingIndex, 0); + emit PositionModified(position.id, account, position.margin, position.size, 0, price, fundingIndex, 0); } else { // this is someone else (like a keeper) // cancellation by third party is only possible when execution cannot be attempted any longer @@ -109,17 +115,24 @@ contract MixinFuturesNextPriceOrders is FuturesMarketBase { require(_confirmationWindowOver(currentRoundId, order.targetRoundId), "cannot be cancelled by keeper yet"); // send keeper fee to keeper - _manager().issueSUSD(messageSender, order.keeperDeposit); + _manager().issueSUSD(msg.sender, order.keeperDeposit); } // pay the commitDeposit as fee to the FeePool _manager().payFee(order.commitDeposit); // remove stored order - // important!! position of the account, not the messageSender + // important!! position of the account, not the msg.sender delete nextPriceOrders[account]; // emit event - emitNextPriceOrderRemoved(account, currentRoundId, order); + emit NextPriceOrderRemoved( + account, + currentRoundId, + order.sizeDelta, + order.targetRoundId, + order.commitDeposit, + order.keeperDeposit + ); } /** @@ -135,7 +148,7 @@ contract MixinFuturesNextPriceOrders is FuturesMarketBase { * otherwise it sent to the msg.sender. * @param account address of the account for which to try to execute a next-price order */ - function executeNextPriceOrder(address account) external optionalProxy { + function executeNextPriceOrder(address account) external { // important!: order of the account, not the sender! NextPriceOrder memory order = nextPriceOrders[account]; // check that a previous order exists @@ -156,10 +169,10 @@ contract MixinFuturesNextPriceOrders is FuturesMarketBase { uint toRefund = order.commitDeposit; // refund the commitment deposit // refund keeperFee to margin if it's the account holder - if (messageSender == account) { + if (msg.sender == account) { toRefund += order.keeperDeposit; } else { - _manager().issueSUSD(messageSender, order.keeperDeposit); + _manager().issueSUSD(msg.sender, order.keeperDeposit); } Position storage position = positions[account]; @@ -169,7 +182,7 @@ contract MixinFuturesNextPriceOrders is FuturesMarketBase { // if the order later fails this is reverted of course _updatePositionMargin(position, fundingIndex, currentPrice, int(toRefund)); // emit event for modidying the position (refunding fee/s) - emitPositionModified(position.id, account, position.margin, position.size, 0, currentPrice, fundingIndex, 0); + emit PositionModified(position.id, account, position.margin, position.size, 0, currentPrice, fundingIndex, 0); // the correct price for the past round (uint pastPrice, ) = _exchangeRates().rateAndTimestampAtRound(baseAsset, order.targetRoundId); @@ -188,7 +201,14 @@ contract MixinFuturesNextPriceOrders is FuturesMarketBase { // remove stored order delete nextPriceOrders[account]; // emit event - emitNextPriceOrderRemoved(account, currentRoundId, order); + emit NextPriceOrderRemoved( + account, + currentRoundId, + order.sizeDelta, + order.targetRoundId, + order.commitDeposit, + order.keeperDeposit + ); } ///// Internal views @@ -219,7 +239,6 @@ contract MixinFuturesNextPriceOrders is FuturesMarketBase { } ///// Events - event NextPriceOrderSubmitted( address indexed account, int sizeDelta, @@ -228,20 +247,6 @@ contract MixinFuturesNextPriceOrders is FuturesMarketBase { uint keeperDeposit ); - bytes32 internal constant SIG_NEXTPRICEORDERSUBMITTED = - keccak256("NextPriceOrderSubmitted(address,int256,uint256,uint256,uint256)"); - - function emitNextPriceOrderSubmitted(address account, NextPriceOrder memory order) internal { - proxy._emit( - abi.encode(order.sizeDelta, order.targetRoundId, order.commitDeposit, order.keeperDeposit), - 2, - SIG_NEXTPRICEORDERSUBMITTED, - addressToBytes32(account), - 0, - 0 - ); - } - event NextPriceOrderRemoved( address indexed account, uint currentRoundId, @@ -250,22 +255,4 @@ contract MixinFuturesNextPriceOrders is FuturesMarketBase { uint commitDeposit, uint keeperDeposit ); - - bytes32 internal constant SIG_NEXTPRICEORDERREMOVED = - keccak256("NextPriceOrderRemoved(address,uint256,int256,uint256,uint256,uint256)"); - - function emitNextPriceOrderRemoved( - address account, - uint currentRoundId, - NextPriceOrder memory order - ) internal { - proxy._emit( - abi.encode(currentRoundId, order.sizeDelta, order.targetRoundId, order.commitDeposit, order.keeperDeposit), - 2, - SIG_NEXTPRICEORDERREMOVED, - addressToBytes32(account), - 0, - 0 - ); - } } diff --git a/contracts/test-helpers/TestableFuturesMarket.sol b/contracts/test-helpers/TestableFuturesMarket.sol index 28728866cc..3e5ca7f1fa 100644 --- a/contracts/test-helpers/TestableFuturesMarket.sol +++ b/contracts/test-helpers/TestableFuturesMarket.sol @@ -3,12 +3,7 @@ pragma solidity ^0.5.16; import "../FuturesMarket.sol"; contract TestableFuturesMarket is FuturesMarket { - constructor( - address payable _proxy, - address _owner, - address _resolver, - bytes32 _baseAsset - ) public FuturesMarket(_proxy, _owner, _resolver, _baseAsset) {} + constructor(address _resolver, bytes32 _baseAsset) public FuturesMarket(_resolver, _baseAsset) {} function entryDebtCorrection() external view returns (int) { return _entryDebtCorrection; From c70300e2ca9498f974c5cc328fc851ff8efad4d0 Mon Sep 17 00:00:00 2001 From: artdgn Date: Wed, 2 Feb 2022 18:13:09 +1100 Subject: [PATCH 02/10] fix tests for proxy removal --- publish/src/commands/deploy/deploy-futures.js | 25 +------------- test/contracts/FuturesMarket.js | 34 +++++++++---------- test/contracts/FuturesMarket.nextPrice.js | 18 +++++----- test/contracts/FuturesMarketData.js | 10 ------ test/contracts/setup.js | 19 ----------- 5 files changed, 25 insertions(+), 81 deletions(-) diff --git a/publish/src/commands/deploy/deploy-futures.js b/publish/src/commands/deploy/deploy-futures.js index 16426eee27..f755a8701f 100644 --- a/publish/src/commands/deploy/deploy-futures.js +++ b/publish/src/commands/deploy/deploy-futures.js @@ -73,40 +73,17 @@ module.exports = async ({ for (const asset of futuresMarkets.map(x => x.asset)) { const marketName = 'FuturesMarket' + asset.slice('1'); // remove s prefix - const proxyName = 'Proxy' + marketName; const baseAsset = toBytes32(asset); - const proxyFuturesMarket = await deployer.deployContract({ - name: proxyName, - source: 'Proxy', - args: [account], - }); - const futuresMarket = await deployer.deployContract({ name: marketName, source: 'FuturesMarket', - args: [ - addressOf(proxyFuturesMarket), - account, - addressOf(ReadProxyAddressResolver), - baseAsset, - ], + args: [addressOf(ReadProxyAddressResolver), baseAsset], }); if (futuresMarket) { deployedFuturesMarkets.push(addressOf(futuresMarket)); } - - if (proxyFuturesMarket && futuresMarket) { - await runStep({ - contract: proxyName, - target: proxyFuturesMarket, - read: 'target', - expected: input => input === addressOf(futuresMarket), - write: 'setTarget', - writeArg: addressOf(futuresMarket), - }); - } } // Now replace the relevant markets in the manager (if any) diff --git a/test/contracts/FuturesMarket.js b/test/contracts/FuturesMarket.js index 644265ed4b..8608fd9fbe 100644 --- a/test/contracts/FuturesMarket.js +++ b/test/contracts/FuturesMarket.js @@ -33,8 +33,7 @@ const Status = { }; contract('FuturesMarket', accounts => { - let proxyFuturesMarket, - futuresMarketSettings, + let futuresMarketSettings, futuresMarketManager, futuresMarket, exchangeRates, @@ -103,7 +102,6 @@ contract('FuturesMarket', accounts => { before(async () => { ({ - ProxyFuturesMarketBTC: proxyFuturesMarket, FuturesMarketSettings: futuresMarketSettings, FuturesMarketManager: futuresMarketManager, FuturesMarketBTC: futuresMarket, @@ -165,7 +163,7 @@ contract('FuturesMarket', accounts => { it('Only expected functions are mutative', () => { ensureOnlyExpectedMutativeFunctions({ abi: futuresMarket.abi, - ignoreParents: ['Owned', 'Proxyable', 'MixinFuturesMarketSettings'], + ignoreParents: ['MixinFuturesMarketSettings'], expected: [ 'transferMargin', 'withdrawAllMargin', @@ -357,7 +355,7 @@ contract('FuturesMarket', accounts => { decodedEventEqual({ event: 'PositionModified', - emittedFrom: proxyFuturesMarket.address, + emittedFrom: futuresMarket.address, args: [ toBN('1'), trader, @@ -699,14 +697,14 @@ contract('FuturesMarket', accounts => { decodedEventEqual({ event: 'MarginTransferred', - emittedFrom: proxyFuturesMarket.address, + emittedFrom: futuresMarket.address, args: [trader3, toUnit('1000')], log: decodedLogs[2], }); decodedEventEqual({ event: 'PositionModified', - emittedFrom: proxyFuturesMarket.address, + emittedFrom: futuresMarket.address, args: [ toBN('1'), trader3, @@ -745,14 +743,14 @@ contract('FuturesMarket', accounts => { decodedEventEqual({ event: 'MarginTransferred', - emittedFrom: proxyFuturesMarket.address, + emittedFrom: futuresMarket.address, args: [trader3, toUnit('-1000')], log: decodedLogs[2], }); decodedEventEqual({ event: 'PositionModified', - emittedFrom: proxyFuturesMarket.address, + emittedFrom: futuresMarket.address, args: [ toBN('1'), trader3, @@ -913,7 +911,7 @@ contract('FuturesMarket', accounts => { }); decodedEventEqual({ event: 'PositionModified', - emittedFrom: proxyFuturesMarket.address, + emittedFrom: futuresMarket.address, args: [toBN('1'), trader, margin.sub(fee), size, size, price, toBN(2), fee], log: decodedLogs[2], }); @@ -1414,7 +1412,7 @@ contract('FuturesMarket', accounts => { decodedEventEqual({ event: 'PositionModified', - emittedFrom: proxyFuturesMarket.address, + emittedFrom: futuresMarket.address, args: [ toBN('1'), trader, @@ -3255,7 +3253,7 @@ contract('FuturesMarket', accounts => { }); decodedEventEqual({ event: 'PositionModified', - emittedFrom: proxyFuturesMarket.address, + emittedFrom: futuresMarket.address, args: [ positionId, trader, @@ -3270,7 +3268,7 @@ contract('FuturesMarket', accounts => { }); decodedEventEqual({ event: 'PositionLiquidated', - emittedFrom: proxyFuturesMarket.address, + emittedFrom: futuresMarket.address, args: [positionId, trader, noBalance, positionSize, newPrice, liquidationFee], log: decodedLogs[3], bnCloseVariance: toUnit('0.001'), @@ -3356,7 +3354,7 @@ contract('FuturesMarket', accounts => { }); decodedEventEqual({ event: 'PositionModified', - emittedFrom: proxyFuturesMarket.address, + emittedFrom: futuresMarket.address, args: [ positionId, trader3, @@ -3371,7 +3369,7 @@ contract('FuturesMarket', accounts => { }); decodedEventEqual({ event: 'PositionLiquidated', - emittedFrom: proxyFuturesMarket.address, + emittedFrom: futuresMarket.address, args: [positionId, trader3, noBalance, positionSize, newPrice, liquidationFee], log: decodedLogs[3], bnCloseVariance: toUnit('0.001'), @@ -3445,7 +3443,7 @@ contract('FuturesMarket', accounts => { const decodedLogs = await getDecodedLogs({ hash: tx.tx, contracts: [sUSD, futuresMarket] }); decodedEventEqual({ event: 'PositionModified', - emittedFrom: proxyFuturesMarket.address, + emittedFrom: futuresMarket.address, args: [ positionId, trader, @@ -3460,7 +3458,7 @@ contract('FuturesMarket', accounts => { }); decodedEventEqual({ event: 'PositionLiquidated', - emittedFrom: proxyFuturesMarket.address, + emittedFrom: futuresMarket.address, args: [positionId, trader, noBalance, positionSize, newPrice, toUnit('100')], log: decodedLogs[3], bnCloseVariance: toUnit('0.001'), @@ -3817,7 +3815,7 @@ contract('FuturesMarket', accounts => { decodedEventEqual({ event: 'PositionModified', - emittedFrom: proxyFuturesMarket.address, + emittedFrom: futuresMarket.address, args: [ toBN('1'), trader, diff --git a/test/contracts/FuturesMarket.nextPrice.js b/test/contracts/FuturesMarket.nextPrice.js index 799def5b78..f420c74992 100644 --- a/test/contracts/FuturesMarket.nextPrice.js +++ b/test/contracts/FuturesMarket.nextPrice.js @@ -8,8 +8,7 @@ const { assert, addSnapshotBeforeRestoreAfterEach } = require('./common'); const { getDecodedLogs, decodedEventEqual, updateAggregatorRates } = require('./helpers'); contract('FuturesMarket MixinFuturesNextPriceOrders', accounts => { - let proxyFuturesMarket, - futuresMarketSettings, + let futuresMarketSettings, // futuresMarketManager, futuresMarket, exchangeRates, @@ -41,7 +40,6 @@ contract('FuturesMarket MixinFuturesNextPriceOrders', accounts => { before(async () => { ({ - ProxyFuturesMarketBTC: proxyFuturesMarket, FuturesMarketSettings: futuresMarketSettings, // FuturesMarketManager: futuresMarketManager, FuturesMarketBTC: futuresMarket, @@ -121,14 +119,14 @@ contract('FuturesMarket MixinFuturesNextPriceOrders', accounts => { // PositionModified decodedEventEqual({ event: 'PositionModified', - emittedFrom: proxyFuturesMarket.address, + emittedFrom: futuresMarket.address, args: [toBN('1'), trader, expectedMargin, 0, 0, price, toBN(2), 0], log: decodedLogs[1], }); // NextPriceOrderSubmitted decodedEventEqual({ event: 'NextPriceOrderSubmitted', - emittedFrom: proxyFuturesMarket.address, + emittedFrom: futuresMarket.address, args: [trader, size, roundId.add(toBN(1)), spotFee, keeperFee], log: decodedLogs[2], }); @@ -207,7 +205,7 @@ contract('FuturesMarket MixinFuturesNextPriceOrders', accounts => { // PositionModified decodedEventEqual({ event: 'PositionModified', - emittedFrom: proxyFuturesMarket.address, + emittedFrom: futuresMarket.address, args: [toBN('1'), trader, currentMargin.add(keeperFee), 0, 0, price, toBN(2), 0], log: decodedLogs[1], }); @@ -232,7 +230,7 @@ contract('FuturesMarket MixinFuturesNextPriceOrders', accounts => { // NextPriceOrderRemoved decodedEventEqual({ event: 'NextPriceOrderRemoved', - emittedFrom: proxyFuturesMarket.address, + emittedFrom: futuresMarket.address, args: [trader, roundId, size, roundId.add(toBN(1)), spotFee, keeperFee], log: decodedLogs.slice(-1)[0], }); @@ -473,7 +471,7 @@ contract('FuturesMarket MixinFuturesNextPriceOrders', accounts => { const currentPrice = (await futuresMarket.assetPrice()).price; decodedEventEqual({ event: 'PositionModified', - emittedFrom: proxyFuturesMarket.address, + emittedFrom: futuresMarket.address, args: [toBN('1'), trader, expectedMargin, 0, 0, currentPrice, toBN(2), 0], log: decodedLogs.slice(-4, -3)[0], }); @@ -490,7 +488,7 @@ contract('FuturesMarket MixinFuturesNextPriceOrders', accounts => { decodedEventEqual({ event: 'PositionModified', - emittedFrom: proxyFuturesMarket.address, + emittedFrom: futuresMarket.address, args: [toBN('1'), trader, expectedMargin, size, size, targetPrice, toBN(2), expectedFee], log: decodedLogs.slice(-2, -1)[0], }); @@ -498,7 +496,7 @@ contract('FuturesMarket MixinFuturesNextPriceOrders', accounts => { // NextPriceOrderRemoved decodedEventEqual({ event: 'NextPriceOrderRemoved', - emittedFrom: proxyFuturesMarket.address, + emittedFrom: futuresMarket.address, args: [trader, roundId, size, roundId.add(toBN(1)), commitFee, keeperFee], log: decodedLogs.slice(-1)[0], }); diff --git a/test/contracts/FuturesMarketData.js b/test/contracts/FuturesMarketData.js index 23481e5de9..c1a69ee251 100644 --- a/test/contracts/FuturesMarketData.js +++ b/test/contracts/FuturesMarketData.js @@ -70,13 +70,6 @@ contract('FuturesMarketData', accounts => { // Add a couple of additional markets. for (const symbol of ['sETH', 'sLINK']) { - const proxy = await setupContract({ - accounts, - contract: 'ProxyFuturesMarket' + symbol, - source: 'Proxy', - args: [accounts[1]], - cache: { FuturesMarketManager: futuresMarketManager }, - }); const assetKey = toBytes32(symbol); const market = await setupContract({ @@ -84,14 +77,11 @@ contract('FuturesMarketData', accounts => { contract: 'FuturesMarket' + symbol, source: 'FuturesMarket', args: [ - proxy.address, - accounts[1], addressResolver.address, assetKey, // base asset ], }); - await proxy.setTarget(market.address, { from: owner }); await addressResolver.rebuildCaches([market.address], { from: owner }); await futuresMarketManager.addMarkets([market.address], { from: owner }); diff --git a/test/contracts/setup.js b/test/contracts/setup.js index 4351472c31..926638b6ee 100644 --- a/test/contracts/setup.js +++ b/test/contracts/setup.js @@ -283,14 +283,10 @@ const setupContract = async ({ ], FuturesMarketSettings: [owner, tryGetAddressOf('AddressResolver')], FuturesMarketBTC: [ - tryGetAddressOf('ProxyFuturesMarketBTC'), - owner, tryGetAddressOf('AddressResolver'), toBytes32('sBTC'), // base asset ], FuturesMarketETH: [ - tryGetAddressOf('ProxyFuturesMarketETH'), - owner, tryGetAddressOf('AddressResolver'), toBytes32('sETH'), // base asset ], @@ -569,19 +565,11 @@ const setupContract = async ({ async FuturesMarketBTC() { await Promise.all([ cache['FuturesMarketManager'].addMarkets([instance.address], { from: owner }), - cache['ProxyFuturesMarketBTC'].setTarget(instance.address, { from: owner }), - instance.setProxy(cache['ProxyFuturesMarketBTC'].address, { - from: owner, - }), ]); }, async FuturesMarketETH() { await Promise.all([ cache['FuturesMarketManager'].addMarkets([instance.address], { from: owner }), - cache['ProxyFuturesMarketETH'].setTarget(instance.address, { from: owner }), - instance.setProxy(cache['ProxyFuturesMarketETH'].address, { - from: owner, - }), ]); }, async GenericMock() { @@ -986,15 +974,10 @@ const setupAllContracts = async ({ contract: 'FuturesMarketSettings', deps: ['AddressResolver', 'FlexibleStorage'], }, - { contract: 'Proxy', forContract: 'FuturesMarketBTC' }, { - // contract: 'TestableFuturesMarket', - // forContract: 'FuturesMarketBTC', - // resolverAlias: 'FuturesMarketBTC', contract: 'FuturesMarketBTC', source: 'TestableFuturesMarket', deps: [ - 'Proxy', 'AddressResolver', 'FuturesMarketManager', 'FuturesMarketSettings', @@ -1003,12 +986,10 @@ const setupAllContracts = async ({ 'ExchangeCircuitBreaker', ], }, - { contract: 'Proxy', forContract: 'FuturesMarketETH' }, { contract: 'FuturesMarketETH', source: 'TestableFuturesMarket', deps: [ - 'Proxy', 'AddressResolver', 'FuturesMarketManager', 'FlexibleStorage', From a3f0ce04e6e2be8a74c0703cc8f829607667f232 Mon Sep 17 00:00:00 2001 From: artdgn Date: Wed, 2 Feb 2022 18:25:09 +1100 Subject: [PATCH 03/10] remove proxy from market manager --- contracts/FuturesMarketManager.sol | 30 +++++-------------- publish/src/commands/deploy/deploy-futures.js | 21 +------------ test/contracts/FuturesMarketManager.js | 22 +++++++------- test/contracts/setup.js | 15 +--------- 4 files changed, 21 insertions(+), 67 deletions(-) diff --git a/contracts/FuturesMarketManager.sol b/contracts/FuturesMarketManager.sol index b42c3afa43..05af9f9017 100644 --- a/contracts/FuturesMarketManager.sol +++ b/contracts/FuturesMarketManager.sol @@ -18,7 +18,7 @@ import "./interfaces/IExchanger.sol"; import "./interfaces/IERC20.sol"; // https://docs.synthetix.io/contracts/source/contracts/FuturesMarketManager -contract FuturesMarketManager is Owned, MixinResolver, Proxyable, IFuturesMarketManager { +contract FuturesMarketManager is Owned, MixinResolver, IFuturesMarketManager { using SafeMath for uint; using AddressSetLib for AddressSetLib.AddressSet; @@ -36,11 +36,7 @@ contract FuturesMarketManager is Owned, MixinResolver, Proxyable, IFuturesMarket /* ========== CONSTRUCTOR ========== */ - constructor( - address payable _proxy, - address _owner, - address _resolver - ) public Owned(_owner) Proxyable(_proxy) MixinResolver(_resolver) {} + constructor(address _owner, address _resolver) public Owned(_owner) MixinResolver(_resolver) {} /* ========== VIEWS ========== */ @@ -120,7 +116,7 @@ contract FuturesMarketManager is Owned, MixinResolver, Proxyable, IFuturesMarket /* * Add a set of new markets. Reverts if some market's asset already has a market. */ - function addMarkets(address[] calldata marketsToAdd) external optionalProxy_onlyOwner { + function addMarkets(address[] calldata marketsToAdd) external onlyOwner { uint numOfMarkets = marketsToAdd.length; for (uint i; i < numOfMarkets; i++) { address market = marketsToAdd[i]; @@ -130,7 +126,7 @@ contract FuturesMarketManager is Owned, MixinResolver, Proxyable, IFuturesMarket require(marketForAsset[key] == address(0), "Market already exists for this asset"); marketForAsset[key] = market; _markets.add(market); - emitMarketAdded(market, key); + emit MarketAdded(market, key); } } @@ -144,21 +140,21 @@ contract FuturesMarketManager is Owned, MixinResolver, Proxyable, IFuturesMarket require(marketForAsset[key] != address(0), "Unknown market"); delete marketForAsset[key]; _markets.remove(market); - emitMarketRemoved(market, key); + emit MarketRemoved(market, key); } } /* * Remove a set of markets. Reverts if any market is not known to the manager. */ - function removeMarkets(address[] calldata marketsToRemove) external optionalProxy_onlyOwner { + function removeMarkets(address[] calldata marketsToRemove) external onlyOwner { return _removeMarkets(marketsToRemove); } /* * Remove the markets for a given set of assets. Reverts if any asset has no associated market. */ - function removeMarketsByAsset(bytes32[] calldata assetsToRemove) external optionalProxy_onlyOwner { + function removeMarketsByAsset(bytes32[] calldata assetsToRemove) external onlyOwner { _removeMarkets(_marketsForAssets(assetsToRemove)); } @@ -213,7 +209,7 @@ contract FuturesMarketManager is Owned, MixinResolver, Proxyable, IFuturesMarket /* ========== MODIFIERS ========== */ function _requireIsMarket() internal view { - require(_markets.contains(messageSender) || _markets.contains(msg.sender), "Permitted only for markets"); + require(_markets.contains(msg.sender), "Permitted only for markets"); } modifier onlyMarkets() { @@ -224,16 +220,6 @@ contract FuturesMarketManager is Owned, MixinResolver, Proxyable, IFuturesMarket /* ========== EVENTS ========== */ event MarketAdded(address market, bytes32 indexed asset); - bytes32 internal constant MARKETADDED_SIG = keccak256("MarketAdded(address,bytes32)"); - - function emitMarketAdded(address market, bytes32 asset) internal { - proxy._emit(abi.encode(market), 2, MARKETADDED_SIG, asset, 0, 0); - } event MarketRemoved(address market, bytes32 indexed asset); - bytes32 internal constant MARKETREMOVED_SIG = keccak256("MarketRemoved(address,bytes32)"); - - function emitMarketRemoved(address market, bytes32 asset) internal { - proxy._emit(abi.encode(market), 2, MARKETREMOVED_SIG, asset, 0, 0); - } } diff --git a/publish/src/commands/deploy/deploy-futures.js b/publish/src/commands/deploy/deploy-futures.js index f755a8701f..8dc4195288 100644 --- a/publish/src/commands/deploy/deploy-futures.js +++ b/publish/src/commands/deploy/deploy-futures.js @@ -26,18 +26,10 @@ module.exports = async ({ console.log(gray(`\n------ DEPLOY FUTURES MARKETS ------\n`)); - const proxyFuturesMarketManager = await deployer.deployContract({ - name: 'ProxyFuturesMarketManager', - source: 'Proxy', - args: [account], - }); - const futuresMarketManager = await deployer.deployContract({ name: 'FuturesMarketManager', source: useOvm ? 'FuturesMarketManager' : 'EmptyFuturesMarketManager', - args: useOvm - ? [addressOf(proxyFuturesMarketManager), account, addressOf(ReadProxyAddressResolver)] - : [], + args: useOvm ? [account, addressOf(ReadProxyAddressResolver)] : [], deps: ['ReadProxyAddressResolver'], }); @@ -45,17 +37,6 @@ module.exports = async ({ return; } - if (proxyFuturesMarketManager && futuresMarketManager) { - await runStep({ - contract: 'ProxyFuturesMarketManager', - target: proxyFuturesMarketManager, - read: 'target', - expected: input => input === addressOf(futuresMarketManager), - write: 'setTarget', - writeArg: addressOf(futuresMarketManager), - }); - } - // This belongs in dapp-utils, but since we are only deploying futures on L2, // I've colocated it here for now. await deployer.deployContract({ diff --git a/test/contracts/FuturesMarketManager.js b/test/contracts/FuturesMarketManager.js index 06deaa0606..da1b07e971 100644 --- a/test/contracts/FuturesMarketManager.js +++ b/test/contracts/FuturesMarketManager.js @@ -15,14 +15,13 @@ const ZERO_ADDRESS = constants.ZERO_ADDRESS; const MockExchanger = artifacts.require('MockExchanger'); contract('FuturesMarketManager', accounts => { - let proxyFuturesMarketManager, futuresMarketManager, sUSD, debtCache, synthetix, addressResolver; + let futuresMarketManager, sUSD, debtCache, synthetix, addressResolver; const owner = accounts[1]; const trader = accounts[2]; const initialMint = toUnit('100000'); before(async () => { ({ - ProxyFuturesMarketManager: proxyFuturesMarketManager, FuturesMarketManager: futuresMarketManager, SynthsUSD: sUSD, DebtCache: debtCache, @@ -33,7 +32,6 @@ contract('FuturesMarketManager', accounts => { synths: ['sUSD'], contracts: [ 'FuturesMarketManager', - // 'Proxy', 'AddressResolver', 'FeePool', 'ExchangeRates', @@ -60,7 +58,7 @@ contract('FuturesMarketManager', accounts => { it('only expected functions are mutable', () => { ensureOnlyExpectedMutativeFunctions({ abi: futuresMarketManager.abi, - ignoreParents: ['Owned', 'MixinResolver', 'Proxyable'], + ignoreParents: ['Owned', 'MixinResolver'], expected: [ 'addMarkets', 'removeMarkets', @@ -133,13 +131,13 @@ contract('FuturesMarketManager', accounts => { assert.equal(decodedLogs.length, 2); decodedEventEqual({ event: 'MarketAdded', - emittedFrom: proxyFuturesMarketManager.address, + emittedFrom: futuresMarketManager.address, args: [addresses[0], keys[0]], log: decodedLogs[0], }); decodedEventEqual({ event: 'MarketAdded', - emittedFrom: proxyFuturesMarketManager.address, + emittedFrom: futuresMarketManager.address, args: [addresses[1], keys[1]], log: decodedLogs[1], }); @@ -182,13 +180,13 @@ contract('FuturesMarketManager', accounts => { assert.equal(decodedLogs.length, 2); decodedEventEqual({ event: 'MarketRemoved', - emittedFrom: proxyFuturesMarketManager.address, + emittedFrom: futuresMarketManager.address, args: [addresses[0], currencyKeys[0]], log: decodedLogs[0], }); decodedEventEqual({ event: 'MarketRemoved', - emittedFrom: proxyFuturesMarketManager.address, + emittedFrom: futuresMarketManager.address, args: [addresses[1], currencyKeys[1]], log: decodedLogs[1], }); @@ -243,13 +241,15 @@ contract('FuturesMarketManager', accounts => { skipPostDeploy: true, }); + const revertReason = 'Only the contract owner may perform this action'; + await onlyGivenAddressCanInvoke({ fnc: futuresMarketManager.addMarkets, args: [[market.address]], accounts, address: owner, skipPassCheck: false, - reason: 'Owner only function', + reason: revertReason, }); await onlyGivenAddressCanInvoke({ @@ -258,7 +258,7 @@ contract('FuturesMarketManager', accounts => { accounts, address: owner, skipPassCheck: false, - reason: 'Owner only function', + reason: revertReason, }); await onlyGivenAddressCanInvoke({ @@ -267,7 +267,7 @@ contract('FuturesMarketManager', accounts => { accounts, address: owner, skipPassCheck: false, - reason: 'Owner only function', + reason: revertReason, }); }); }); diff --git a/test/contracts/setup.js b/test/contracts/setup.js index 926638b6ee..43b2c59592 100644 --- a/test/contracts/setup.js +++ b/test/contracts/setup.js @@ -276,11 +276,7 @@ const setupContract = async ({ ], WETH: [], SynthRedeemer: [tryGetAddressOf('AddressResolver')], - FuturesMarketManager: [ - tryGetAddressOf('ProxyFuturesMarketManager'), - owner, - tryGetAddressOf('AddressResolver'), - ], + FuturesMarketManager: [owner, tryGetAddressOf('AddressResolver')], FuturesMarketSettings: [owner, tryGetAddressOf('AddressResolver')], FuturesMarketBTC: [ tryGetAddressOf('AddressResolver'), @@ -554,14 +550,6 @@ const setupContract = async ({ { from: owner } ); }, - async FuturesMarketManager() { - await Promise.all([ - cache['ProxyFuturesMarketManager'].setTarget(instance.address, { from: owner }), - instance.setProxy(cache['ProxyFuturesMarketManager'].address, { - from: owner, - }), - ]); - }, async FuturesMarketBTC() { await Promise.all([ cache['FuturesMarketManager'].addMarkets([instance.address], { from: owner }), @@ -965,7 +953,6 @@ const setupAllContracts = async ({ contract: 'CollateralShort', deps: ['Collateral', 'CollateralManager', 'AddressResolver', 'CollateralUtil'], }, - { contract: 'Proxy', forContract: 'FuturesMarketManager' }, { contract: 'FuturesMarketManager', deps: ['AddressResolver', 'Exchanger'], From 4e852502eb815f34b377d5da914a592ef42c657a Mon Sep 17 00:00:00 2001 From: artdgn Date: Wed, 2 Feb 2022 19:20:59 +1100 Subject: [PATCH 04/10] remove maxFundingRateDelta param --- contracts/FuturesMarketBase.sol | 4 --- contracts/FuturesMarketData.sol | 9 ++---- contracts/FuturesMarketSettings.sol | 19 ++--------- contracts/MixinFuturesMarketSettings.sol | 9 +----- contracts/MixinFuturesViews.sol | 3 +- contracts/interfaces/IFuturesMarket.sol | 3 +- .../interfaces/IFuturesMarketSettings.sol | 6 +--- .../kovan-ovm-futures/futures-markets.json | 9 ++---- .../deployed/local-ovm/futures-markets.json | 9 ++---- .../src/commands/deploy/configure-futures.js | 2 -- test/contracts/FuturesMarket.js | 32 +++---------------- test/contracts/FuturesMarketData.js | 2 -- test/contracts/FuturesMarketSettings.js | 3 -- test/contracts/setup.js | 1 - 14 files changed, 19 insertions(+), 92 deletions(-) diff --git a/contracts/FuturesMarketBase.sol b/contracts/FuturesMarketBase.sol index 49faa6cf13..d4801ecb03 100644 --- a/contracts/FuturesMarketBase.sol +++ b/contracts/FuturesMarketBase.sol @@ -205,10 +205,6 @@ contract FuturesMarketBase is MixinFuturesMarketSettings, IFuturesMarketBaseType return ISystemStatus(requireAndGetAddress(CONTRACT_SYSTEMSTATUS)); } - function systemStatus() internal view returns (ISystemStatus) { - return ISystemStatus(requireAndGetAddress(CONTRACT_SYSTEMSTATUS)); - } - function _manager() internal view returns (IFuturesMarketManagerInternal) { return IFuturesMarketManagerInternal(requireAndGetAddress(CONTRACT_FUTURESMARKETMANAGER)); } diff --git a/contracts/FuturesMarketData.sol b/contracts/FuturesMarketData.sol index 268eebeab5..38f53bbf5a 100644 --- a/contracts/FuturesMarketData.sol +++ b/contracts/FuturesMarketData.sol @@ -57,7 +57,6 @@ contract FuturesMarketData { struct FundingParameters { uint maxFundingRate; uint skewScaleUSD; - uint maxFundingRateDelta; } struct FeeRates { @@ -145,8 +144,7 @@ contract FuturesMarketData { uint maxLeverage, uint maxMarketValueUSD, uint maxFundingRate, - uint skewScaleUSD, - uint maxFundingRateDelta + uint skewScaleUSD ) = _futuresMarketSettings().parameters(baseAsset); return IFuturesMarketSettings.Parameters( @@ -158,8 +156,7 @@ contract FuturesMarketData { maxLeverage, maxMarketValueUSD, maxFundingRate, - skewScaleUSD, - maxFundingRateDelta + skewScaleUSD ); } @@ -207,7 +204,7 @@ contract FuturesMarketData { pure returns (FundingParameters memory) { - return FundingParameters(params.maxFundingRate, params.skewScaleUSD, params.maxFundingRateDelta); + return FundingParameters(params.maxFundingRate, params.skewScaleUSD); } function _marketSizes(IFuturesMarket market) internal view returns (Sides memory) { diff --git a/contracts/FuturesMarketSettings.sol b/contracts/FuturesMarketSettings.sol index 3548455d93..8adabc6a30 100644 --- a/contracts/FuturesMarketSettings.sol +++ b/contracts/FuturesMarketSettings.sol @@ -99,13 +99,6 @@ contract FuturesMarketSettings is Owned, MixinFuturesMarketSettings, IFuturesMar return _skewScaleUSD(_baseAsset); } - /* - * The maximum speed that the funding rate can move per day. - */ - function maxFundingRateDelta(bytes32 _baseAsset) public view returns (uint) { - return _maxFundingRateDelta(_baseAsset); - } - function parameters(bytes32 _baseAsset) external view @@ -118,8 +111,7 @@ contract FuturesMarketSettings is Owned, MixinFuturesMarketSettings, IFuturesMar uint _maxLeverage, uint _maxMarketValueUSD, uint _maxFundingRate, - uint _skewScaleUSD, - uint _maxFundingRateDelta + uint _skewScaleUSD ) { return _parameters(_baseAsset); @@ -223,11 +215,6 @@ contract FuturesMarketSettings is Owned, MixinFuturesMarketSettings, IFuturesMar _setParameter(_baseAsset, PARAMETER_MIN_SKEW_SCALE, _skewScaleUSD); } - function setMaxFundingRateDelta(bytes32 _baseAsset, uint _maxFundingRateDelta) public onlyOwner { - _recomputeFunding(_baseAsset); - _setParameter(_baseAsset, PARAMETER_MAX_FUNDING_RATE_DELTA, _maxFundingRateDelta); - } - function setParameters( bytes32 _baseAsset, uint _takerFee, @@ -238,8 +225,7 @@ contract FuturesMarketSettings is Owned, MixinFuturesMarketSettings, IFuturesMar uint _maxLeverage, uint _maxMarketValueUSD, uint _maxFundingRate, - uint _skewScaleUSD, - uint _maxFundingRateDelta + uint _skewScaleUSD ) external onlyOwner { _recomputeFunding(_baseAsset); setTakerFee(_baseAsset, _takerFee); @@ -251,7 +237,6 @@ contract FuturesMarketSettings is Owned, MixinFuturesMarketSettings, IFuturesMar setMaxMarketValueUSD(_baseAsset, _maxMarketValueUSD); setMaxFundingRate(_baseAsset, _maxFundingRate); setSkewScaleUSD(_baseAsset, _skewScaleUSD); - setMaxFundingRateDelta(_baseAsset, _maxFundingRateDelta); } function setMinKeeperFee(uint _sUSD) external onlyOwner { diff --git a/contracts/MixinFuturesMarketSettings.sol b/contracts/MixinFuturesMarketSettings.sol index c1949c74aa..eb0e39133f 100644 --- a/contracts/MixinFuturesMarketSettings.sol +++ b/contracts/MixinFuturesMarketSettings.sol @@ -23,7 +23,6 @@ contract MixinFuturesMarketSettings is MixinResolver { bytes32 internal constant PARAMETER_MAX_MARKET_VALUE = "maxMarketValueUSD"; bytes32 internal constant PARAMETER_MAX_FUNDING_RATE = "maxFundingRate"; bytes32 internal constant PARAMETER_MIN_SKEW_SCALE = "skewScaleUSD"; - bytes32 internal constant PARAMETER_MAX_FUNDING_RATE_DELTA = "maxFundingRateDelta"; // Global settings // minimum liquidation fee payable to liquidator @@ -95,10 +94,6 @@ contract MixinFuturesMarketSettings is MixinResolver { return _parameter(_baseAsset, PARAMETER_MAX_FUNDING_RATE); } - function _maxFundingRateDelta(bytes32 _baseAsset) internal view returns (uint) { - return _parameter(_baseAsset, PARAMETER_MAX_FUNDING_RATE_DELTA); - } - function _parameters(bytes32 _baseAsset) internal view @@ -111,8 +106,7 @@ contract MixinFuturesMarketSettings is MixinResolver { uint maxLeverage, uint maxMarketValueUSD, uint maxFundingRate, - uint skewScaleUSD, - uint maxFundingRateDelta + uint skewScaleUSD ) { takerFee = _takerFee(_baseAsset); @@ -124,7 +118,6 @@ contract MixinFuturesMarketSettings is MixinResolver { maxMarketValueUSD = _maxMarketValueUSD(_baseAsset); maxFundingRate = _maxFundingRate(_baseAsset); skewScaleUSD = _skewScaleUSD(_baseAsset); - maxFundingRateDelta = _maxFundingRateDelta(_baseAsset); } function _minKeeperFee() internal view returns (uint) { diff --git a/contracts/MixinFuturesViews.sol b/contracts/MixinFuturesViews.sol index 6bfaae8bdb..33a2160568 100644 --- a/contracts/MixinFuturesViews.sol +++ b/contracts/MixinFuturesViews.sol @@ -78,8 +78,7 @@ contract MixinFuturesViews is FuturesMarketBase { uint maxLeverage, uint maxMarketValueUSD, uint maxFundingRate, - uint skewScaleUSD, - uint maxFundingRateDelta + uint skewScaleUSD ) { return _parameters(baseAsset); diff --git a/contracts/interfaces/IFuturesMarket.sol b/contracts/interfaces/IFuturesMarket.sol index b35aac2064..4834687a65 100644 --- a/contracts/interfaces/IFuturesMarket.sol +++ b/contracts/interfaces/IFuturesMarket.sol @@ -55,8 +55,7 @@ interface IFuturesMarket { uint maxLeverage, uint maxMarketValueUSD, uint maxFundingRate, - uint skewScaleUSD, - uint maxFundingRateDelta + uint skewScaleUSD ); function currentFundingRate() external view returns (int fundingRate); diff --git a/contracts/interfaces/IFuturesMarketSettings.sol b/contracts/interfaces/IFuturesMarketSettings.sol index 4b19ff7b69..e2d22d99bb 100644 --- a/contracts/interfaces/IFuturesMarketSettings.sol +++ b/contracts/interfaces/IFuturesMarketSettings.sol @@ -11,7 +11,6 @@ interface IFuturesMarketSettings { uint maxMarketValueUSD; uint maxFundingRate; uint skewScaleUSD; - uint maxFundingRateDelta; } function takerFee(bytes32 _baseAsset) external view returns (uint); @@ -32,8 +31,6 @@ interface IFuturesMarketSettings { function skewScaleUSD(bytes32 _baseAsset) external view returns (uint); - function maxFundingRateDelta(bytes32 _baseAsset) external view returns (uint); - function parameters(bytes32 _baseAsset) external view @@ -46,8 +43,7 @@ interface IFuturesMarketSettings { uint _maxLeverage, uint _maxMarketValueUSD, uint _maxFundingRate, - uint _skewScaleUSD, - uint _maxFundingRateDelta + uint _skewScaleUSD ); function minKeeperFee() external view returns (uint); diff --git a/publish/deployed/kovan-ovm-futures/futures-markets.json b/publish/deployed/kovan-ovm-futures/futures-markets.json index f83941c5e4..1d239c14a3 100644 --- a/publish/deployed/kovan-ovm-futures/futures-markets.json +++ b/publish/deployed/kovan-ovm-futures/futures-markets.json @@ -9,8 +9,7 @@ "maxLeverage": "10", "maxMarketValueUSD": "1000000000", "maxFundingRate": "0.1", - "skewScaleUSD": "50000000", - "maxFundingRateDelta": "0.0125" + "skewScaleUSD": "50000000" }, { "asset": "sETH", @@ -22,8 +21,7 @@ "maxLeverage": "10", "maxMarketValueUSD": "1000000000", "maxFundingRate": "0.1", - "skewScaleUSD": "50000000", - "maxFundingRateDelta": "0.0125" + "skewScaleUSD": "50000000" }, { "asset": "sLINK", @@ -35,7 +33,6 @@ "maxLeverage": "10", "maxMarketValueUSD": "1000000000", "maxFundingRate": "0.1", - "skewScaleUSD": "50000000", - "maxFundingRateDelta": "0.0125" + "skewScaleUSD": "50000000" } ] diff --git a/publish/deployed/local-ovm/futures-markets.json b/publish/deployed/local-ovm/futures-markets.json index f83941c5e4..1d239c14a3 100644 --- a/publish/deployed/local-ovm/futures-markets.json +++ b/publish/deployed/local-ovm/futures-markets.json @@ -9,8 +9,7 @@ "maxLeverage": "10", "maxMarketValueUSD": "1000000000", "maxFundingRate": "0.1", - "skewScaleUSD": "50000000", - "maxFundingRateDelta": "0.0125" + "skewScaleUSD": "50000000" }, { "asset": "sETH", @@ -22,8 +21,7 @@ "maxLeverage": "10", "maxMarketValueUSD": "1000000000", "maxFundingRate": "0.1", - "skewScaleUSD": "50000000", - "maxFundingRateDelta": "0.0125" + "skewScaleUSD": "50000000" }, { "asset": "sLINK", @@ -35,7 +33,6 @@ "maxLeverage": "10", "maxMarketValueUSD": "1000000000", "maxFundingRate": "0.1", - "skewScaleUSD": "50000000", - "maxFundingRateDelta": "0.0125" + "skewScaleUSD": "50000000" } ] diff --git a/publish/src/commands/deploy/configure-futures.js b/publish/src/commands/deploy/configure-futures.js index 1c183f6a7c..4032503089 100644 --- a/publish/src/commands/deploy/configure-futures.js +++ b/publish/src/commands/deploy/configure-futures.js @@ -103,7 +103,6 @@ module.exports = async ({ maxMarketValueUSD, maxFundingRate, skewScaleUSD, - maxFundingRateDelta, } = market; console.log(gray(`\n --- MARKET ${asset} ---\n`)); @@ -120,7 +119,6 @@ module.exports = async ({ maxMarketValueUSD: w3utils.toWei(maxMarketValueUSD), maxFundingRate: w3utils.toWei(maxFundingRate), skewScaleUSD: w3utils.toWei(skewScaleUSD), - maxFundingRateDelta: w3utils.toWei(maxFundingRateDelta), }; for (const setting in settings) { diff --git a/test/contracts/FuturesMarket.js b/test/contracts/FuturesMarket.js index 8608fd9fbe..55fa1b6e3d 100644 --- a/test/contracts/FuturesMarket.js +++ b/test/contracts/FuturesMarket.js @@ -63,7 +63,6 @@ contract('FuturesMarket', accounts => { const maxMarketValueUSD = toUnit('100000'); const maxFundingRate = toUnit('0.1'); const skewScaleUSD = toUnit('100000'); - const maxFundingRateDelta = toUnit('0.0125'); const initialPrice = toUnit('100'); const minKeeperFee = toUnit('20'); const minInitialMargin = toUnit('100'); @@ -191,7 +190,6 @@ contract('FuturesMarket', accounts => { assert.bnEqual(parameters.maxMarketValueUSD, maxMarketValueUSD); assert.bnEqual(parameters.maxFundingRate, maxFundingRate); assert.bnEqual(parameters.skewScaleUSD, skewScaleUSD); - assert.bnEqual(parameters.maxFundingRateDelta, maxFundingRateDelta); }); it('prices are properly fetched', async () => { @@ -2641,7 +2639,7 @@ contract('FuturesMarket', accounts => { assert.bnClose((await futuresMarket.unrecordedFunding())[0], toUnit('-6'), toUnit('0.01')); await futuresMarketSettings.setMaxFundingRate(baseAsset, toUnit('0.2'), { from: owner }); - let time = await currentTime(); + const time = await currentTime(); assert.bnEqual( await futuresMarket.fundingSequenceLength(), @@ -2671,22 +2669,6 @@ contract('FuturesMarket', accounts => { toUnit('-120'), toUnit('0.01') ); - - await futuresMarketSettings.setMaxFundingRateDelta(baseAsset, toUnit('0.05'), { - from: owner, - }); - time = await currentTime(); - - assert.bnEqual( - await futuresMarket.fundingSequenceLength(), - initialFundingIndex.add(toBN(8)) - ); - assert.bnEqual(await futuresMarket.fundingLastRecomputed(), time); - assert.bnClose( - await futuresMarket.fundingSequence(initialFundingIndex.add(toBN(7))), - toUnit('-126'), - toUnit('0.01') - ); }); }); @@ -3576,11 +3558,7 @@ contract('FuturesMarket', accounts => { 'Invalid price' ); await assert.revert( - futuresMarketSettings.setMaxFundingRateDelta(baseAsset, 0, { from: owner }), - 'Invalid price' - ); - await assert.revert( - futuresMarketSettings.setParameters(baseAsset, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, { + futuresMarketSettings.setParameters(baseAsset, 0, 0, 0, 0, 0, 0, 0, 0, 0, { from: owner, }), 'Invalid price' @@ -3769,8 +3747,7 @@ contract('FuturesMarket', accounts => { it('futuresMarketSettings parameter changes do not revert', async () => { await futuresMarketSettings.setMaxFundingRate(baseAsset, 0, { from: owner }); await futuresMarketSettings.setSkewScaleUSD(baseAsset, toUnit('100'), { from: owner }); - await futuresMarketSettings.setMaxFundingRateDelta(baseAsset, 0, { from: owner }); - await futuresMarketSettings.setParameters(baseAsset, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, { + await futuresMarketSettings.setParameters(baseAsset, 0, 0, 0, 0, 0, 0, 0, 0, 1, { from: owner, }); }); @@ -3852,8 +3829,7 @@ contract('FuturesMarket', accounts => { it('futuresMarketSettings parameter changes do not revert', async () => { await futuresMarketSettings.setMaxFundingRate(baseAsset, 0, { from: owner }); await futuresMarketSettings.setSkewScaleUSD(baseAsset, toUnit('100'), { from: owner }); - await futuresMarketSettings.setMaxFundingRateDelta(baseAsset, 0, { from: owner }); - await futuresMarketSettings.setParameters(baseAsset, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, { + await futuresMarketSettings.setParameters(baseAsset, 0, 0, 0, 0, 0, 0, 0, 0, 1, { from: owner, }); }); diff --git a/test/contracts/FuturesMarketData.js b/test/contracts/FuturesMarketData.js index c1a69ee251..8db715992d 100644 --- a/test/contracts/FuturesMarketData.js +++ b/test/contracts/FuturesMarketData.js @@ -100,7 +100,6 @@ contract('FuturesMarketData', accounts => { toWei('1000000'), // 1000000 max total margin toWei('0.2'), // 20% max funding rate toWei('100000'), // 100000 USD skewScaleUSD - toWei('0.025'), // 2.5% per hour max funding rate of change { from: owner } ); } @@ -178,7 +177,6 @@ contract('FuturesMarketData', accounts => { assert.bnEqual(details.fundingParameters.maxFundingRate, params.maxFundingRate); assert.bnEqual(details.fundingParameters.skewScaleUSD, params.skewScaleUSD); - assert.bnEqual(details.fundingParameters.maxFundingRateDelta, params.maxFundingRateDelta); assert.bnEqual(details.marketSizeDetails.marketSize, await futuresMarket.marketSize()); const marketSizes = await futuresMarket.marketSizes(); diff --git a/test/contracts/FuturesMarketSettings.js b/test/contracts/FuturesMarketSettings.js index 629f8ca3cb..2d0d9ddd7c 100644 --- a/test/contracts/FuturesMarketSettings.js +++ b/test/contracts/FuturesMarketSettings.js @@ -32,7 +32,6 @@ contract('FuturesMarketSettings', accounts => { const maxFundingRate = toUnit('0.1'); const skewScaleUSD = toUnit('10000'); - const maxFundingRateDelta = toUnit('0.0125'); before(async () => { ({ @@ -96,7 +95,6 @@ contract('FuturesMarketSettings', accounts => { 'setMaxMarketValueUSD', 'setMaxFundingRate', 'setSkewScaleUSD', - 'setMaxFundingRateDelta', 'setParameters', 'setMinKeeperFee', 'setLiquidationFeeRatio', @@ -120,7 +118,6 @@ contract('FuturesMarketSettings', accounts => { maxMarketValueUSD, maxFundingRate, skewScaleUSD, - maxFundingRateDelta, }).map(([key, val]) => { const capKey = key.charAt(0).toUpperCase() + key.slice(1); return [key, val, futuresMarketSettings[`set${capKey}`], futuresMarketSettings[`${key}`]]; diff --git a/test/contracts/setup.js b/test/contracts/setup.js index 43b2c59592..b521871687 100644 --- a/test/contracts/setup.js +++ b/test/contracts/setup.js @@ -1267,7 +1267,6 @@ const setupAllContracts = async ({ toWei('100000'), // 100000 max market debt toWei('0.1'), // 10% max funding rate toWei('100000'), // 100000 USD skewScaleUSD - toWei('0.0125'), // 1.25% per hour max funding rate of change { from: owner } ), ]); From 8df934aedb867766d8b3aa8fc24402b74f26e3ff Mon Sep 17 00:00:00 2001 From: artdgn Date: Thu, 3 Feb 2022 23:13:06 +1100 Subject: [PATCH 05/10] remove unused param and method --- contracts/FuturesMarketBase.sol | 52 ++++++------------- contracts/FuturesMarketData.sol | 2 +- contracts/MixinFuturesViews.sol | 11 ++-- contracts/interfaces/IFuturesMarket.sol | 2 +- test/contracts/FuturesMarket.js | 67 ++++++++++--------------- test/contracts/FuturesMarketData.js | 2 +- 6 files changed, 51 insertions(+), 85 deletions(-) diff --git a/contracts/FuturesMarketBase.sol b/contracts/FuturesMarketBase.sol index d4801ecb03..3f31b4177d 100644 --- a/contracts/FuturesMarketBase.sol +++ b/contracts/FuturesMarketBase.sol @@ -246,30 +246,24 @@ contract FuturesMarketBase is MixinFuturesMarketSettings, IFuturesMarketBaseType return _min(_max(-_UNIT, -_proportionalSkew(price)), _UNIT).multiplyDecimal(maxFundingRate); } - /* - * The current funding rate, rescaled to a percentage per second. - */ - function _currentFundingRatePerSecond(uint price) internal view returns (int) { - return _currentFundingRate(price) / 1 days; - } - function _unrecordedFunding(uint price) internal view returns (int funding) { int elapsed = int(block.timestamp.sub(fundingLastRecomputed)); - return _currentFundingRatePerSecond(price).multiplyDecimal(int(price)).mul(elapsed); + // The current funding rate, rescaled to a percentage per second. + int currentFundingRatePerSecond = _currentFundingRate(price) / 1 days; + return currentFundingRatePerSecond.multiplyDecimal(int(price)).mul(elapsed); } /* * The new entry in the funding sequence, appended when funding is recomputed. It is the sum of the * last entry and the unrecorded funding, so the sequence accumulates running total over the market's lifetime. */ - function _nextFundingEntry(uint sequenceLength, uint price) internal view returns (int funding) { - return int(fundingSequence[sequenceLength.sub(1)]).add(_unrecordedFunding(price)); + function _nextFundingEntry(uint price) internal view returns (int funding) { + return int(fundingSequence[fundingSequence.length.sub(1)]).add(_unrecordedFunding(price)); } function _netFundingPerUnit( uint startIndex, uint endIndex, - uint sequenceLength, uint price ) internal view returns (int) { int result; @@ -280,8 +274,8 @@ contract FuturesMarketBase is MixinFuturesMarketSettings, IFuturesMarketBaseType } // Determine whether we should include unrecorded funding. - if (endIndex == sequenceLength) { - result = _nextFundingEntry(sequenceLength, price); + if (endIndex == fundingSequence.length) { + result = _nextFundingEntry(price); } else { result = fundingSequence[endIndex]; } @@ -349,7 +343,7 @@ contract FuturesMarketBase is MixinFuturesMarketSettings, IFuturesMarketBaseType if (lastModifiedIndex == 0) { return 0; // The position does not exist -- no funding. } - int net = _netFundingPerUnit(lastModifiedIndex, endFundingIndex, fundingSequence.length, price); + int net = _netFundingPerUnit(lastModifiedIndex, endFundingIndex, price); return int(position.size).multiplyDecimal(net); } @@ -433,11 +427,7 @@ contract FuturesMarketBase is MixinFuturesMarketSettings, IFuturesMarketBaseType return remaining.sub(inaccessible); } - function _liquidationPrice( - Position memory position, - bool includeFunding, - uint currentPrice - ) internal view returns (uint) { + function _liquidationPrice(Position memory position, uint currentPrice) internal view returns (uint) { int positionSize = int(position.size); // short circuit @@ -445,17 +435,8 @@ contract FuturesMarketBase is MixinFuturesMarketSettings, IFuturesMarketBaseType return 0; } - // calculate funding if needed - int fundingPerUnit = 0; - if (includeFunding) { - // price = lastPrice + (liquidationMargin - margin) / positionSize - netAccrued - fundingPerUnit = _netFundingPerUnit( - position.lastFundingIndex, - fundingSequence.length, - fundingSequence.length, - currentPrice - ); - } + // price = lastPrice + (liquidationMargin - margin) / positionSize - netAccrued + int fundingPerUnit = _netFundingPerUnit(position.lastFundingIndex, fundingSequence.length, currentPrice); // minimum margin beyond which position can be liqudiated uint liqMargin = _liquidationMargin(positionSize, currentPrice); @@ -727,14 +708,14 @@ contract FuturesMarketBase is MixinFuturesMarketSettings, IFuturesMarketBaseType } function _recomputeFunding(uint price) internal returns (uint lastIndex) { - uint sequenceLength = fundingSequence.length; + uint sequenceLengthBefore = fundingSequence.length; - int funding = _nextFundingEntry(sequenceLength, price); + int funding = _nextFundingEntry(price); fundingSequence.push(int128(funding)); fundingLastRecomputed = uint32(block.timestamp); emit FundingRecomputed(funding); - return sequenceLength; + return sequenceLengthBefore; } /* @@ -780,7 +761,7 @@ contract FuturesMarketBase is MixinFuturesMarketSettings, IFuturesMarketBaseType return 0; } // see comment explaining this calculation in _positionDebtCorrection() - int priceWithFunding = int(price).add(_nextFundingEntry(fundingSequence.length, price)); + int priceWithFunding = int(price).add(_nextFundingEntry(price)); int totalDebt = int(marketSkew).multiplyDecimal(priceWithFunding).add(_entryDebtCorrection); return uint(_max(totalDebt, 0)); } @@ -954,12 +935,11 @@ contract FuturesMarketBase is MixinFuturesMarketSettings, IFuturesMarketBaseType */ function modifyPosition(int sizeDelta) external { uint price = _assetPriceRequireChecks(); - uint fundingIndex = _recomputeFunding(price); TradeParams memory params = TradeParams({ sizeDelta: sizeDelta, price: price, - fundingIndex: fundingIndex, + fundingIndex: _recomputeFunding(price), takerFee: _takerFee(baseAsset), makerFee: _makerFee(baseAsset) }); diff --git a/contracts/FuturesMarketData.sol b/contracts/FuturesMarketData.sol index 38f53bbf5a..c30b3e058b 100644 --- a/contracts/FuturesMarketData.sol +++ b/contracts/FuturesMarketData.sol @@ -287,7 +287,7 @@ contract FuturesMarketData { } function _liquidationPrice(IFuturesMarket market, address account) internal view returns (uint) { - (uint liquidationPrice, ) = market.liquidationPrice(account, true); + (uint liquidationPrice, ) = market.liquidationPrice(account); return liquidationPrice; } diff --git a/contracts/MixinFuturesViews.sol b/contracts/MixinFuturesViews.sol index 33a2160568..21c4d162f9 100644 --- a/contracts/MixinFuturesViews.sol +++ b/contracts/MixinFuturesViews.sol @@ -108,7 +108,7 @@ contract MixinFuturesViews is FuturesMarketBase { */ function netFundingPerUnit(uint startIndex, uint endIndex) external view returns (int funding, bool invalid) { (uint price, bool isInvalid) = _assetPrice(); - return (_netFundingPerUnit(startIndex, endIndex, fundingSequence.length, price), isInvalid); + return (_netFundingPerUnit(startIndex, endIndex, price), isInvalid); } /* @@ -176,12 +176,11 @@ contract MixinFuturesViews is FuturesMarketBase { * margin has run out. When they have just enough margin left to pay a liquidator, then they are liquidated. * If a position is long, then it is safe as long as the current price is above the liquidation price; if it is * short, then it is safe whenever the current price is below the liquidation price. - * A position's accurate liquidation price can move around slightly due to accrued funding - this contribution - * can be omitted by passing false to includeFunding. + * A position's accurate liquidation price can move around slightly due to accrued funding. */ - function liquidationPrice(address account, bool includeFunding) external view returns (uint price, bool invalid) { + function liquidationPrice(address account) external view returns (uint price, bool invalid) { (uint aPrice, bool isInvalid) = _assetPrice(); - uint liqPrice = _liquidationPrice(positions[account], includeFunding, aPrice); + uint liqPrice = _liquidationPrice(positions[account], aPrice); return (liqPrice, isInvalid); } @@ -275,7 +274,7 @@ contract MixinFuturesViews is FuturesMarketBase { }); (Position memory newPosition, uint fee_, Status status_) = _postTradeDetails(positions[sender], params); - liqPrice = _liquidationPrice(newPosition, true, newPosition.lastPrice); + liqPrice = _liquidationPrice(newPosition, newPosition.lastPrice); return (newPosition.margin, newPosition.size, newPosition.lastPrice, liqPrice, fee_, status_); } } diff --git a/contracts/interfaces/IFuturesMarket.sol b/contracts/interfaces/IFuturesMarket.sol index 4834687a65..9d0d2375f7 100644 --- a/contracts/interfaces/IFuturesMarket.sol +++ b/contracts/interfaces/IFuturesMarket.sol @@ -78,7 +78,7 @@ interface IFuturesMarket { function accessibleMargin(address account) external view returns (uint marginAccessible, bool invalid); - function liquidationPrice(address account, bool includeFunding) external view returns (uint price, bool invalid); + function liquidationPrice(address account) external view returns (uint price, bool invalid); function liquidationMargin(address account) external view returns (uint lMargin); diff --git a/test/contracts/FuturesMarket.js b/test/contracts/FuturesMarket.js index 55fa1b6e3d..8487e8a196 100644 --- a/test/contracts/FuturesMarket.js +++ b/test/contracts/FuturesMarket.js @@ -1597,7 +1597,7 @@ contract('FuturesMarket', accounts => { describe('post-trade position details', async () => { const getPositionDetails = async ({ account }) => { const newPosition = await futuresMarket.positions(account); - const { price: liquidationPrice } = await futuresMarket.liquidationPrice(account, true); + const { price: liquidationPrice } = await futuresMarket.liquidationPrice(account); return { ...newPosition, liquidationPrice, @@ -2813,34 +2813,28 @@ contract('FuturesMarket', accounts => { describe('Liquidations', () => { describe('Liquidation price', () => { - it('Liquidation price is accurate with no funding', async () => { + it('Liquidation price is accurate with funding', async () => { await setPrice(baseAsset, toUnit('100')); await futuresMarket.transferMargin(toUnit('1000'), { from: trader }); await futuresMarket.modifyPosition(toUnit('100'), { from: trader }); await futuresMarket.transferMargin(toUnit('1000'), { from: trader2 }); await futuresMarket.modifyPosition(toUnit('-100'), { from: trader2 }); - let liquidationPrice = await futuresMarket.liquidationPrice(trader, true); - let liquidationPriceNoFunding = await futuresMarket.liquidationPrice(trader, false); + let liquidationPrice = await futuresMarket.liquidationPrice(trader); // fee = 100 * 100 * 0.003 = 30 // liqMargin = max(20, 100*100*0.0035) + 100*100*0.0025 = 60 // liqPrice = 100 + (60 − (1000 - 30))÷100 = 90.9 - assert.bnEqual(liquidationPriceNoFunding.price, toUnit('90.9')); assert.bnClose(liquidationPrice.price, toUnit('90.9'), toUnit('0.001')); assert.isFalse(liquidationPrice.invalid); - assert.isFalse(liquidationPriceNoFunding.invalid); - liquidationPrice = await futuresMarket.liquidationPrice(trader2, true); - liquidationPriceNoFunding = await futuresMarket.liquidationPrice(trader2, false); + liquidationPrice = await futuresMarket.liquidationPrice(trader2); // fee = 100 * 100 * 0.001 = 10 // liqMargin = max(20, 100*100*0.0035) + 100*100*0.0025 = 60 // liqPrice = 100 + (60 − (1000 - 10))÷(-100) = 109.3 - assert.bnEqual(liquidationPrice.price, liquidationPriceNoFunding.price); assert.bnEqual(liquidationPrice.price, toUnit('109.3')); assert.isFalse(liquidationPrice.invalid); - assert.isFalse(liquidationPriceNoFunding.invalid); }); it('Liquidation price is accurate if the liquidation margin changes', async () => { @@ -2854,14 +2848,14 @@ contract('FuturesMarket', accounts => { // liqMargin = max(20, 250 * 20 *0.0035) + 250 * 20*0.0025 = 20 + 12.5 = 32.5 // liqPrice = 250 + (32.5 − (1000 - 15))÷(20) = 202.375 assert.bnClose( - (await futuresMarket.liquidationPrice(trader, true)).price, + (await futuresMarket.liquidationPrice(trader)).price, toUnit(202.375), toUnit('0.001') ); // fee = 250 * 20 * 0.001 = 5 // liqPrice = 250 + (32.5 − (1000 - 5))÷(-20) = 298.125 assert.bnClose( - (await futuresMarket.liquidationPrice(trader2, true)).price, + (await futuresMarket.liquidationPrice(trader2)).price, toUnit(298.125), toUnit('0.001') ); @@ -2871,13 +2865,13 @@ contract('FuturesMarket', accounts => { // liqMargin = max(100, 250 * 20 *0.0035) + 250 * 20*0.0025 = 100 + 12.5 = 112.5 // liqPrice = 250 + (112.5 − (1000 - 15))÷(20) = 206.375 assert.bnClose( - (await futuresMarket.liquidationPrice(trader, true)).price, + (await futuresMarket.liquidationPrice(trader)).price, toUnit(206.375), toUnit('0.001') ); // liqPrice = 250 + (112.5 − (1000 - 5))÷(-20) = 294.125 assert.bnClose( - (await futuresMarket.liquidationPrice(trader2, true)).price, + (await futuresMarket.liquidationPrice(trader2)).price, toUnit(294.125), toUnit('0.001') ); @@ -2886,13 +2880,13 @@ contract('FuturesMarket', accounts => { // liqMargin = max(100, 250 * 20 *0.03) + 250 * 20*0.0025 = 150 + 12.5 = 162.5 // liqPrice = 250 + (162.5 − (1000 - 15))÷(20) = 208.875 assert.bnClose( - (await futuresMarket.liquidationPrice(trader, true)).price, + (await futuresMarket.liquidationPrice(trader)).price, toUnit(208.875), toUnit('0.001') ); // liqPrice = 250 + (162.5 − (1000 - 5))÷(-20) = 291.625 assert.bnClose( - (await futuresMarket.liquidationPrice(trader2, true)).price, + (await futuresMarket.liquidationPrice(trader2)).price, toUnit(291.625), toUnit('0.001') ); @@ -2901,13 +2895,13 @@ contract('FuturesMarket', accounts => { // liqMargin = max(100, 250 * 20 *0.03) + 250 * 20*0.0025 = 150 + 150 = 300 // liqPrice = 250 + (300 − (1000 - 15))÷(20) = 215.75 assert.bnClose( - (await futuresMarket.liquidationPrice(trader, true)).price, + (await futuresMarket.liquidationPrice(trader)).price, toUnit(215.75), toUnit('0.001') ); // liqPrice = 250 + (300 − (1000 - 5))÷(-20) = 284.75 assert.bnClose( - (await futuresMarket.liquidationPrice(trader2, true)).price, + (await futuresMarket.liquidationPrice(trader2)).price, toUnit(284.75), toUnit('0.001') ); @@ -2917,12 +2911,12 @@ contract('FuturesMarket', accounts => { await futuresMarketSettings.setLiquidationBufferRatio(toUnit('0'), { from: owner }); assert.bnClose( - (await futuresMarket.liquidationPrice(trader, true)).price, + (await futuresMarket.liquidationPrice(trader)).price, toUnit(200.75), toUnit('0.001') ); assert.bnClose( - (await futuresMarket.liquidationPrice(trader2, true)).price, + (await futuresMarket.liquidationPrice(trader2)).price, toUnit(299.75), toUnit('0.001') ); @@ -2938,9 +2932,6 @@ contract('FuturesMarket', accounts => { await futuresMarket.transferMargin(toUnit('500'), { from: trader2 }); await futuresMarket.modifyPosition(toUnit('-10'), { from: trader2 }); - const preLPrice1 = (await futuresMarket.liquidationPrice(trader, true))[0]; - const preLPrice2 = (await futuresMarket.liquidationPrice(trader2, true))[0]; - // One day of funding await fastForward(24 * 60 * 60); @@ -2948,18 +2939,14 @@ contract('FuturesMarket', accounts => { // trader 1 pays 30 * -0.05 = -1.5 base units of funding, and a $22.5 trading fee // liquidation price = pLast + (mLiq - m) / s + fPerUnit // liquidation price = 250 + (45 - (1500 - 22.5)) / 30 + 0.05 * 250 = 214.75 - let lPrice = await futuresMarket.liquidationPrice(trader, true); + let lPrice = await futuresMarket.liquidationPrice(trader); assert.bnClose(lPrice[0], toUnit(214.75), toUnit(0.001)); - lPrice = await futuresMarket.liquidationPrice(trader, false); - assert.bnClose(lPrice[0], preLPrice1, toUnit(0.001)); // liqMargin = max(20, 250 * 10 *0.0035) + 250 * 10*0.0025 = 26.25 // trader2 receives -10 * -0.05 = 0.5 base units of funding, and a $2.5 trading fee // liquidation price = 250 + (26.25 - (500 - 2.5)) / (-10) + 0.05 * 250 = 309.625 - lPrice = await futuresMarket.liquidationPrice(trader2, true); + lPrice = await futuresMarket.liquidationPrice(trader2); assert.bnClose(lPrice[0], toUnit(309.625), toUnit(0.001)); - lPrice = await futuresMarket.liquidationPrice(trader2, false); - assert.bnClose(lPrice[0], preLPrice2, toUnit(0.001)); }); it('Liquidation price reports invalidity properly', async () => { @@ -2971,7 +2958,7 @@ contract('FuturesMarket', accounts => { await futuresMarket.transferMargin(toUnit('1000'), { from: trader2 }); await futuresMarket.modifyPosition(toUnit('-20'), { from: trader2 }); - assert.isFalse((await futuresMarket.liquidationPrice(trader, true))[1]); + assert.isFalse((await futuresMarket.liquidationPrice(trader))[1]); await fastForward(60 * 60 * 24 * 7); // Stale the price @@ -2981,14 +2968,14 @@ contract('FuturesMarket', accounts => { // funding rate = -10/50 * 0.1 = -0.02 // trader 1 pays 30 * 7 * -0.02 = -4.2 units of funding, pays $22.5 exchange fee // Remaining margin = 250 + (45 - (1500 - 22.5))/30 + ( 7 * 0.02) * 250 = 237.25 - let lPrice = await futuresMarket.liquidationPrice(trader, true); + let lPrice = await futuresMarket.liquidationPrice(trader); assert.bnClose(lPrice[0], toUnit(237.25), toUnit(0.01)); assert.isTrue(lPrice[1]); // liqMargin = max(20, 250 * 20 * 0.0035) + 250 * 20*0.0025 = 32.5 // trader 2 receives -20 * 7 * -0.02 = 2.8 units of funding, pays $5 exchange fee // Remaining margin = 250 + (32.5 - (1000 - 5)) / (-20) + (7 * 0.02) * 250 = 333.125 - lPrice = await futuresMarket.liquidationPrice(trader2, true); + lPrice = await futuresMarket.liquidationPrice(trader2); assert.bnClose(lPrice[0], toUnit(333.125), toUnit(0.01)); assert.isTrue(lPrice[1]); }); @@ -2999,7 +2986,7 @@ contract('FuturesMarket', accounts => { }); it('No liquidation price on an empty position', async () => { - assert.bnEqual((await futuresMarket.liquidationPrice(noBalance, true))[0], toUnit(0)); + assert.bnEqual((await futuresMarket.liquidationPrice(noBalance))[0], toUnit(0)); }); }); @@ -3010,7 +2997,7 @@ contract('FuturesMarket', accounts => { await futuresMarket.transferMargin(toUnit('1000'), { from: trader }); await futuresMarket.modifyPosition(toUnit('20'), { from: trader }); - price = (await futuresMarket.liquidationPrice(trader, true)).price; + price = (await futuresMarket.liquidationPrice(trader)).price; await setPrice(baseAsset, price.sub(toUnit(1))); // The reason the price is imprecise is that the previously queried // liquidation price was calculated using: @@ -3196,7 +3183,7 @@ contract('FuturesMarket', accounts => { // fee 40*250*0.003 = 30 // Remaining margin = 250 + (60 - (1000 - 30)) / (40)= 227.25 assert.isFalse(await futuresMarket.canLiquidate(trader)); - const liqPrice = (await futuresMarket.liquidationPrice(trader, true)).price; + const liqPrice = (await futuresMarket.liquidationPrice(trader)).price; assert.bnClose(liqPrice, toUnit('227.25'), toUnit('0.01')); const newPrice = liqPrice.sub(toUnit(1)); @@ -3263,7 +3250,7 @@ contract('FuturesMarket', accounts => { // liqMargin = max(20, 250 * 40 * 0.0035) + 250 * 40*0.0025 = 60 // fee 40*250*0.003 = 30 // Remaining margin = 250 + (60 - (1000 - 30)) / (40)= 227.25 - const liqPrice = (await futuresMarket.liquidationPrice(trader, true)).price; + const liqPrice = (await futuresMarket.liquidationPrice(trader)).price; assert.bnClose(liqPrice, toUnit('227.25'), toUnit('0.01')); const newPrice = liqPrice.sub(toUnit(0.5)); @@ -3300,7 +3287,7 @@ contract('FuturesMarket', accounts => { // liqMargin = max(20, 250 * 20 * 0.0035) + 250 * 20*0.0025 = 32.5 // fee 20*250*0.001 = 5 // Remaining margin = 250 + (32.5 - (1000 - 5)) / (-20)= 298.125 - const liqPrice = (await futuresMarket.liquidationPrice(trader3, true)).price; + const liqPrice = (await futuresMarket.liquidationPrice(trader3)).price; assert.bnClose(liqPrice, toUnit('298.125'), toUnit('0.01')); const newPrice = liqPrice.add(toUnit(1)); @@ -3364,7 +3351,7 @@ contract('FuturesMarket', accounts => { // liqMargin = max(20, 250 * 20 * 0.0035) + 250 * 20*0.0025 = 32.5 // fee 20*250*0.001 = 5 // Remaining margin = 250 + (32.5 - (1000 - 5)) / (-20)= 298.125 - const liqPrice = (await futuresMarket.liquidationPrice(trader3, true)).price; + const liqPrice = (await futuresMarket.liquidationPrice(trader3)).price; assert.bnClose(liqPrice, toUnit('298.125'), toUnit('0.01')); const newPrice = liqPrice.add(toUnit(0.5)); @@ -3400,7 +3387,7 @@ contract('FuturesMarket', accounts => { it('Transfers an updated fee upon liquidation', async () => { const { size: positionSize, id: positionId } = await futuresMarket.positions(trader); // Move the price to a non-liquidating point - let price = (await futuresMarket.liquidationPrice(trader, true)).price; + let price = (await futuresMarket.liquidationPrice(trader)).price; const newPrice = price.add(toUnit('1')); await setPrice(baseAsset, newPrice); @@ -3411,7 +3398,7 @@ contract('FuturesMarket', accounts => { await futuresMarketSettings.setMinKeeperFee(toUnit('100'), { from: owner }); assert.isTrue(await futuresMarket.canLiquidate(trader)); - price = (await futuresMarket.liquidationPrice(trader, true)).price; + price = (await futuresMarket.liquidationPrice(trader)).price; // liquidate the position const tx = await futuresMarket.liquidatePosition(trader, { from: noBalance }); diff --git a/test/contracts/FuturesMarketData.js b/test/contracts/FuturesMarketData.js index 8db715992d..3391d58e54 100644 --- a/test/contracts/FuturesMarketData.js +++ b/test/contracts/FuturesMarketData.js @@ -218,7 +218,7 @@ contract('FuturesMarketData', accounts => { assert.bnEqual(details2.remainingMargin, remaining.marginRemaining); const accessible = await futuresMarket.accessibleMargin(trader1); assert.bnEqual(details2.accessibleMargin, accessible.marginAccessible); - const lp = await futuresMarket.liquidationPrice(trader1, true); + const lp = await futuresMarket.liquidationPrice(trader1); assert.bnEqual(details2.liquidationPrice, lp[0]); assert.equal(details.canLiquidatePosition, await futuresMarket.canLiquidate(trader1)); }); From 2a6a7624d851c8685ed0577f117b7fcb660ff9d3 Mon Sep 17 00:00:00 2001 From: artdgn Date: Fri, 4 Feb 2022 00:32:07 +1100 Subject: [PATCH 06/10] refactor some unused views from views mixin --- contracts/FuturesMarketBase.sol | 7 - contracts/FuturesMarketSettings.sol | 28 ++-- contracts/MixinFuturesMarketSettings.sol | 26 ---- contracts/MixinFuturesViews.sol | 123 ++++-------------- contracts/interfaces/IFuturesMarket.sol | 30 ----- .../test-helpers/TestableFuturesMarket.sol | 45 ++++++- test/contracts/FuturesMarket.js | 9 +- 7 files changed, 90 insertions(+), 178 deletions(-) diff --git a/contracts/FuturesMarketBase.sol b/contracts/FuturesMarketBase.sol index 3f31b4177d..2d5e85b408 100644 --- a/contracts/FuturesMarketBase.sol +++ b/contracts/FuturesMarketBase.sol @@ -215,13 +215,6 @@ contract FuturesMarketBase is MixinFuturesMarketSettings, IFuturesMarketBaseType /* ---------- Market Details ---------- */ - function _assetPrice() internal view returns (uint price, bool invalid) { - (price, invalid) = _exchangeCircuitBreaker().rateWithInvalid(baseAsset); - // Ensure we catch uninitialised rates or suspended state / synth - invalid = invalid || price == 0 || _systemStatus().synthSuspended(baseAsset); - return (price, invalid); - } - /* * The size of the skew relative to the size of the market skew scaler. * This value can be outside of [-1, 1] values. diff --git a/contracts/FuturesMarketSettings.sol b/contracts/FuturesMarketSettings.sol index 8adabc6a30..907e2e06c5 100644 --- a/contracts/FuturesMarketSettings.sol +++ b/contracts/FuturesMarketSettings.sol @@ -103,18 +103,26 @@ contract FuturesMarketSettings is Owned, MixinFuturesMarketSettings, IFuturesMar external view returns ( - uint _takerFee, - uint _makerFee, - uint _takerFeeNextPrice, - uint _makerFeeNextPrice, - uint _nextPriceConfirmWindow, - uint _maxLeverage, - uint _maxMarketValueUSD, - uint _maxFundingRate, - uint _skewScaleUSD + uint takerFee, + uint makerFee, + uint takerFeeNextPrice, + uint makerFeeNextPrice, + uint nextPriceConfirmWindow, + uint maxLeverage, + uint maxMarketValueUSD, + uint maxFundingRate, + uint skewScaleUSD ) { - return _parameters(_baseAsset); + takerFee = _takerFee(_baseAsset); + makerFee = _makerFee(_baseAsset); + takerFeeNextPrice = _takerFeeNextPrice(_baseAsset); + makerFeeNextPrice = _makerFeeNextPrice(_baseAsset); + nextPriceConfirmWindow = _nextPriceConfirmWindow(_baseAsset); + maxLeverage = _maxLeverage(_baseAsset); + maxMarketValueUSD = _maxMarketValueUSD(_baseAsset); + maxFundingRate = _maxFundingRate(_baseAsset); + skewScaleUSD = _skewScaleUSD(_baseAsset); } /* diff --git a/contracts/MixinFuturesMarketSettings.sol b/contracts/MixinFuturesMarketSettings.sol index eb0e39133f..2e7c2fc36d 100644 --- a/contracts/MixinFuturesMarketSettings.sol +++ b/contracts/MixinFuturesMarketSettings.sol @@ -94,32 +94,6 @@ contract MixinFuturesMarketSettings is MixinResolver { return _parameter(_baseAsset, PARAMETER_MAX_FUNDING_RATE); } - function _parameters(bytes32 _baseAsset) - internal - view - returns ( - uint takerFee, - uint makerFee, - uint takerFeeNextPrice, - uint makerFeeNextPrice, - uint nextPriceConfirmWindow, - uint maxLeverage, - uint maxMarketValueUSD, - uint maxFundingRate, - uint skewScaleUSD - ) - { - takerFee = _takerFee(_baseAsset); - makerFee = _makerFee(_baseAsset); - takerFeeNextPrice = _takerFeeNextPrice(_baseAsset); - makerFeeNextPrice = _makerFeeNextPrice(_baseAsset); - nextPriceConfirmWindow = _nextPriceConfirmWindow(_baseAsset); - maxLeverage = _maxLeverage(_baseAsset); - maxMarketValueUSD = _maxMarketValueUSD(_baseAsset); - maxFundingRate = _maxFundingRate(_baseAsset); - skewScaleUSD = _skewScaleUSD(_baseAsset); - } - function _minKeeperFee() internal view returns (uint) { return _flexibleStorage().getUIntValue(SETTING_CONTRACT_NAME, SETTING_MIN_KEEPER_FEE); } diff --git a/contracts/MixinFuturesViews.sol b/contracts/MixinFuturesViews.sol index 21c4d162f9..1e0f935d48 100644 --- a/contracts/MixinFuturesViews.sol +++ b/contracts/MixinFuturesViews.sol @@ -11,85 +11,37 @@ contract MixinFuturesViews is FuturesMarketBase { /* * The current base price from the oracle, and whether that price was invalid. Zero prices count as invalid. */ - function assetPrice() external view returns (uint price, bool invalid) { - return _assetPrice(); + function assetPrice() public view returns (uint price, bool invalid) { + (price, invalid) = _exchangeCircuitBreaker().rateWithInvalid(baseAsset); + // Ensure we catch uninitialised rates or suspended state / synth + invalid = invalid || price == 0 || _systemStatus().synthSuspended(baseAsset); + return (price, invalid); } - function _marketSizes() internal view returns (uint long, uint short) { + /* + * Sizes of the long and short sides of the market (in sUSD) + */ + function marketSizes() public view returns (uint long, uint short) { int size = int(marketSize); int skew = marketSkew; return (_abs(size.add(skew).div(2)), _abs(size.sub(skew).div(2))); } - /* - * The total number of base units on each side of the market. - */ - function marketSizes() external view returns (uint long, uint short) { - return _marketSizes(); - } - - /* - * The remaining units on each side of the market left to be filled before hitting the cap. - */ - function _maxOrderSizes(uint price) internal view returns (uint, uint) { - (uint long, uint short) = _marketSizes(); - int sizeLimit = int(_maxMarketValueUSD(baseAsset)).divideDecimal(int(price)); - return (uint(sizeLimit.sub(_min(int(long), sizeLimit))), uint(sizeLimit.sub(_min(int(short), sizeLimit)))); - } - - /* - * The maximum size in base units of an order on each side of the market that will not exceed the max market value. - */ - function maxOrderSizes() - external - view - returns ( - uint long, - uint short, - bool invalid - ) - { - (uint price, bool isInvalid) = _assetPrice(); - (uint longSize, uint shortSize) = _maxOrderSizes(price); - return (longSize, shortSize, isInvalid); - } - /* * The debt contributed by this market to the overall system. * The total market debt is equivalent to the sum of remaining margins in all open positions. */ function marketDebt() external view returns (uint debt, bool invalid) { - (uint price, bool isInvalid) = _assetPrice(); + (uint price, bool isInvalid) = assetPrice(); return (_marketDebt(price), isInvalid); } - /* - * The basic settings of this market, which determine trading fees and funding rate behaviour. - */ - function parameters() - external - view - returns ( - uint takerFee, - uint makerFee, - uint takerFeeNextPrice, - uint makerFeeNextPrice, - uint nextPriceConfirmWindow, - uint maxLeverage, - uint maxMarketValueUSD, - uint maxFundingRate, - uint skewScaleUSD - ) - { - return _parameters(baseAsset); - } - /* * The current funding rate as determined by the market skew; this is returned as a percentage per day. * If this is positive, shorts pay longs, if it is negative, longs pay shorts. */ function currentFundingRate() external view returns (int) { - (uint price, ) = _assetPrice(); + (uint price, ) = assetPrice(); return _currentFundingRate(price); } @@ -98,19 +50,10 @@ contract MixinFuturesViews is FuturesMarketBase { * been persisted in the funding sequence. */ function unrecordedFunding() external view returns (int funding, bool invalid) { - (uint price, bool isInvalid) = _assetPrice(); + (uint price, bool isInvalid) = assetPrice(); return (_unrecordedFunding(price), isInvalid); } - /* - * Computes the net funding that was accrued between any two funding sequence indices. - * If endIndex is equal to the funding sequence length, then unrecorded funding will be included. - */ - function netFundingPerUnit(uint startIndex, uint endIndex) external view returns (int funding, bool invalid) { - (uint price, bool isInvalid) = _assetPrice(); - return (_netFundingPerUnit(startIndex, endIndex, price), isInvalid); - } - /* * The number of entries in the funding sequence. */ @@ -122,7 +65,7 @@ contract MixinFuturesViews is FuturesMarketBase { * The notional value of a position is its size multiplied by the current price. Margin and leverage are ignored. */ function notionalValue(address account) external view returns (int value, bool invalid) { - (uint price, bool isInvalid) = _assetPrice(); + (uint price, bool isInvalid) = assetPrice(); return (_notionalValue(positions[account].size, price), isInvalid); } @@ -130,7 +73,7 @@ contract MixinFuturesViews is FuturesMarketBase { * The PnL of a position is the change in its notional value. Funding is not taken into account. */ function profitLoss(address account) external view returns (int pnl, bool invalid) { - (uint price, bool isInvalid) = _assetPrice(); + (uint price, bool isInvalid) = assetPrice(); return (_profitLoss(positions[account], price), isInvalid); } @@ -138,7 +81,7 @@ contract MixinFuturesViews is FuturesMarketBase { * The funding accrued in a position since it was opened; this does not include PnL. */ function accruedFunding(address account) external view returns (int funding, bool invalid) { - (uint price, bool isInvalid) = _assetPrice(); + (uint price, bool isInvalid) = assetPrice(); return (_accruedFunding(positions[account], fundingSequence.length, price), isInvalid); } @@ -146,7 +89,7 @@ contract MixinFuturesViews is FuturesMarketBase { * The initial margin plus profit and funding; returns zero balance if losses exceed the initial margin. */ function remainingMargin(address account) external view returns (uint marginRemaining, bool invalid) { - (uint price, bool isInvalid) = _assetPrice(); + (uint price, bool isInvalid) = assetPrice(); return (_remainingMargin(positions[account], fundingSequence.length, price), isInvalid); } @@ -155,22 +98,10 @@ contract MixinFuturesViews is FuturesMarketBase { * true value slightly. */ function accessibleMargin(address account) external view returns (uint marginAccessible, bool invalid) { - (uint price, bool isInvalid) = _assetPrice(); + (uint price, bool isInvalid) = assetPrice(); return (_accessibleMargin(positions[account], fundingSequence.length, price), isInvalid); } - /** - * The minimal margin at which liquidation can happen. Is the sum of liquidationBuffer and liquidationFee. - * Reverts if position size is 0. - * @param account address of the position account - * @return lMargin liquidation margin to maintain in sUSD fixed point decimal units - */ - function liquidationMargin(address account) external view returns (uint lMargin) { - require(positions[account].size != 0, "0 size position"); - (uint price, ) = _assetPrice(); - return _liquidationMargin(int(positions[account].size), price); - } - /* * The price at which a position is subject to liquidation; otherwise the price at which the user's remaining * margin has run out. When they have just enough margin left to pay a liquidator, then they are liquidated. @@ -179,7 +110,7 @@ contract MixinFuturesViews is FuturesMarketBase { * A position's accurate liquidation price can move around slightly due to accrued funding. */ function liquidationPrice(address account) external view returns (uint price, bool invalid) { - (uint aPrice, bool isInvalid) = _assetPrice(); + (uint aPrice, bool isInvalid) = assetPrice(); uint liqPrice = _liquidationPrice(positions[account], aPrice); return (liqPrice, isInvalid); } @@ -192,7 +123,7 @@ contract MixinFuturesViews is FuturesMarketBase { * in sUSD fixed point decimal units or 0 if account is not liquidatable. */ function liquidationFee(address account) external view returns (uint) { - (uint price, bool invalid) = _assetPrice(); + (uint price, bool invalid) = assetPrice(); if (!invalid && _canLiquidate(positions[account], fundingSequence.length, price)) { return _liquidationFee(int(positions[account].size), price); } else { @@ -207,20 +138,10 @@ contract MixinFuturesViews is FuturesMarketBase { * True if and only if a position is ready to be liquidated. */ function canLiquidate(address account) external view returns (bool) { - (uint price, bool invalid) = _assetPrice(); + (uint price, bool invalid) = assetPrice(); return !invalid && _canLiquidate(positions[account], fundingSequence.length, price); } - /* - * Equivalent to the position's notional value divided by its remaining margin. - */ - function currentLeverage(address account) external view returns (int leverage, bool invalid) { - (uint price, bool isInvalid) = _assetPrice(); - Position storage position = positions[account]; - uint remainingMargin_ = _remainingMargin(position, fundingSequence.length, price); - return (_currentLeverage(position, price, remainingMargin_), isInvalid); - } - /* * Reports the fee for submitting an order of a given size. Orders that increase the skew will be more * expensive than ones that decrease it. Dynamic fee is added according to the recent volatility @@ -230,7 +151,7 @@ contract MixinFuturesViews is FuturesMarketBase { * too high due to recent volatility. */ function orderFee(int sizeDelta) external view returns (uint fee, bool invalid) { - (uint price, bool isInvalid) = _assetPrice(); + (uint price, bool isInvalid) = assetPrice(); (uint dynamicFeeRate, bool tooVolatile) = _dynamicFeeRate(); TradeParams memory params = TradeParams({ @@ -259,7 +180,7 @@ contract MixinFuturesViews is FuturesMarketBase { ) { bool invalid; - (price, invalid) = _assetPrice(); + (price, invalid) = assetPrice(); if (invalid) { return (0, 0, 0, 0, 0, Status.InvalidPrice); } diff --git a/contracts/interfaces/IFuturesMarket.sol b/contracts/interfaces/IFuturesMarket.sol index 9d0d2375f7..eed2bb0bf6 100644 --- a/contracts/interfaces/IFuturesMarket.sol +++ b/contracts/interfaces/IFuturesMarket.sol @@ -32,38 +32,12 @@ interface IFuturesMarket { function marketSizes() external view returns (uint long, uint short); - function maxOrderSizes() - external - view - returns ( - uint long, - uint short, - bool invalid - ); - function marketDebt() external view returns (uint debt, bool isInvalid); - function parameters() - external - view - returns ( - uint takerFee, - uint makerFee, - uint takerFeeNextPrice, - uint makerFeeNextPrice, - uint nextPriceConfirmWindow, - uint maxLeverage, - uint maxMarketValueUSD, - uint maxFundingRate, - uint skewScaleUSD - ); - function currentFundingRate() external view returns (int fundingRate); function unrecordedFunding() external view returns (int funding, bool invalid); - function netFundingPerUnit(uint startIndex, uint endIndex) external view returns (int funding, bool invalid); - function fundingSequenceLength() external view returns (uint length); /* ---------- Position Details ---------- */ @@ -80,14 +54,10 @@ interface IFuturesMarket { function liquidationPrice(address account) external view returns (uint price, bool invalid); - function liquidationMargin(address account) external view returns (uint lMargin); - function liquidationFee(address account) external view returns (uint); function canLiquidate(address account) external view returns (bool); - function currentLeverage(address account) external view returns (int leverage, bool invalid); - function orderFee(int sizeDelta) external view returns (uint fee, bool invalid); function postTradeDetails(int sizeDelta, address sender) diff --git a/contracts/test-helpers/TestableFuturesMarket.sol b/contracts/test-helpers/TestableFuturesMarket.sol index 3e5ca7f1fa..d7dfca807c 100644 --- a/contracts/test-helpers/TestableFuturesMarket.sol +++ b/contracts/test-helpers/TestableFuturesMarket.sol @@ -10,11 +10,54 @@ contract TestableFuturesMarket is FuturesMarket { } function proportionalSkew() external view returns (int) { - (uint price, ) = _assetPrice(); + (uint price, ) = assetPrice(); return _proportionalSkew(price); } function maxFundingRate() external view returns (uint) { return _maxFundingRate(baseAsset); } + + /* + * The maximum size in base units of an order on each side of the market that will not exceed the max market value. + */ + function maxOrderSizes() + external + view + returns ( + uint long, + uint short, + bool invalid + ) + { + uint price; + (price, invalid) = assetPrice(); + int sizeLimit = int(_maxMarketValueUSD(baseAsset)).divideDecimal(int(price)); + (uint longSize, uint shortSize) = marketSizes(); + long = uint(sizeLimit.sub(_min(int(longSize), sizeLimit))); + short = uint(sizeLimit.sub(_min(int(shortSize), sizeLimit))); + return (long, short, invalid); + } + + /** + * The minimal margin at which liquidation can happen. Is the sum of liquidationBuffer and liquidationFee. + * Reverts if position size is 0. + * @param account address of the position account + * @return lMargin liquidation margin to maintain in sUSD fixed point decimal units + */ + function liquidationMargin(address account) external view returns (uint lMargin) { + require(positions[account].size != 0, "0 size position"); + (uint price, ) = assetPrice(); + return _liquidationMargin(int(positions[account].size), price); + } + + /* + * Equivalent to the position's notional value divided by its remaining margin. + */ + function currentLeverage(address account) external view returns (int leverage, bool invalid) { + (uint price, bool isInvalid) = assetPrice(); + Position storage position = positions[account]; + uint remainingMargin_ = _remainingMargin(position, fundingSequence.length, price); + return (_currentLeverage(position, price, remainingMargin_), isInvalid); + } } diff --git a/test/contracts/FuturesMarket.js b/test/contracts/FuturesMarket.js index 8487e8a196..a36e44458e 100644 --- a/test/contracts/FuturesMarket.js +++ b/test/contracts/FuturesMarket.js @@ -180,8 +180,8 @@ contract('FuturesMarket', accounts => { }); it('static parameters are set properly at construction', async () => { - const parameters = await futuresMarket.parameters(); assert.equal(await futuresMarket.baseAsset(), baseAsset); + const parameters = await futuresMarketSettings.parameters(baseAsset); assert.bnEqual(parameters.takerFee, takerFee); assert.bnEqual(parameters.makerFee, makerFee); assert.bnEqual(parameters.takerFeeNextPrice, takerFeeNextPrice); @@ -202,7 +202,7 @@ contract('FuturesMarket', accounts => { }); it('market size and skew', async () => { - const minScale = (await futuresMarket.parameters()).skewScaleUSD; + const minScale = (await futuresMarketSettings.parameters(baseAsset)).skewScaleUSD; const price = 100; let sizes = await futuresMarket.marketSizes(); let marketSkew = await futuresMarket.marketSkew(); @@ -2386,7 +2386,10 @@ contract('FuturesMarket', accounts => { assert.bnEqual(await futuresMarket.currentFundingRate(), toUnit(0)); - const minScale = divideDecimal((await futuresMarket.parameters()).skewScaleUSD, price); + const minScale = divideDecimal( + (await futuresMarketSettings.parameters(baseAsset)).skewScaleUSD, + price + ); const maxFundingRate = await futuresMarket.maxFundingRate(); // Market is 24 units long skewed (24 / 100000) await futuresMarket.modifyPosition(toUnit('24'), { from: trader }); From f2745d59a9d8cd9e425a6c0f437a3f29b4e8d41f Mon Sep 17 00:00:00 2001 From: artdgn Date: Fri, 4 Feb 2022 14:40:54 +1100 Subject: [PATCH 07/10] remove with-price-bounds methods --- contracts/FuturesMarketBase.sol | 51 ----------- contracts/interfaces/IFuturesMarket.sol | 8 -- test/contracts/FuturesMarket.js | 108 ------------------------ 3 files changed, 167 deletions(-) diff --git a/contracts/FuturesMarketBase.sol b/contracts/FuturesMarketBase.sol index 2d5e85b408..d63e74795f 100644 --- a/contracts/FuturesMarketBase.sol +++ b/contracts/FuturesMarketBase.sol @@ -939,38 +939,6 @@ contract FuturesMarketBase is MixinFuturesMarketSettings, IFuturesMarketBaseType _modifyPosition(msg.sender, params); } - function _revertIfPriceOutsideBounds( - uint price, - uint minPrice, - uint maxPrice - ) internal view { - _revertIfError(price < minPrice || maxPrice < price, Status.PriceOutOfBounds); - } - - /* - * Adjust the sender's position size, but with an acceptable slippage range in case - * the price updates while the transaction is in flight. - * Reverts if the oracle price is outside the specified bounds, or the resulting position is too large, - * outside the max leverage, or is liquidating. - */ - function modifyPositionWithPriceBounds( - int sizeDelta, - uint minPrice, - uint maxPrice - ) external { - uint price = _assetPriceRequireChecks(); - _revertIfPriceOutsideBounds(price, minPrice, maxPrice); - TradeParams memory params = - TradeParams({ - sizeDelta: sizeDelta, - price: price, - fundingIndex: _recomputeFunding(price), - takerFee: _takerFee(baseAsset), - makerFee: _makerFee(baseAsset) - }); - _modifyPosition(msg.sender, params); - } - /* * Submit an order to close a position. */ @@ -989,25 +957,6 @@ contract FuturesMarketBase is MixinFuturesMarketSettings, IFuturesMarketBaseType _modifyPosition(msg.sender, params); } - /* - * Submit an order to close a position; reverts if the asset price is outside the specified bounds. - */ - function closePositionWithPriceBounds(uint minPrice, uint maxPrice) external { - int size = positions[msg.sender].size; - _revertIfError(size == 0, Status.NoPositionOpen); - uint price = _assetPriceRequireChecks(); - _revertIfPriceOutsideBounds(price, minPrice, maxPrice); - TradeParams memory params = - TradeParams({ - sizeDelta: -size, - price: price, - fundingIndex: _recomputeFunding(price), - takerFee: _takerFee(baseAsset), - makerFee: _makerFee(baseAsset) - }); - _modifyPosition(msg.sender, params); - } - function _liquidatePosition( address account, address liquidator, diff --git a/contracts/interfaces/IFuturesMarket.sol b/contracts/interfaces/IFuturesMarket.sol index eed2bb0bf6..79b6e71372 100644 --- a/contracts/interfaces/IFuturesMarket.sol +++ b/contracts/interfaces/IFuturesMarket.sol @@ -88,15 +88,7 @@ interface IFuturesMarket { function executeNextPriceOrder(address account) external; - function modifyPositionWithPriceBounds( - int sizeDelta, - uint minPrice, - uint maxPrice - ) external; - function closePosition() external; - function closePositionWithPriceBounds(uint minPrice, uint maxPrice) external; - function liquidatePosition(address account) external; } diff --git a/test/contracts/FuturesMarket.js b/test/contracts/FuturesMarket.js index a36e44458e..7c444d7699 100644 --- a/test/contracts/FuturesMarket.js +++ b/test/contracts/FuturesMarket.js @@ -167,9 +167,7 @@ contract('FuturesMarket', accounts => { 'transferMargin', 'withdrawAllMargin', 'modifyPosition', - 'modifyPositionWithPriceBounds', 'closePosition', - 'closePositionWithPriceBounds', 'liquidatePosition', 'recomputeFunding', 'submitNextPriceOrder', @@ -990,55 +988,6 @@ contract('FuturesMarket', accounts => { 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); - - const margin = toUnit('1000'); - const minPrice = multiplyDecimal(startPrice, toUnit(1).sub(toUnit('0.01'))); - const maxPrice = multiplyDecimal(startPrice, toUnit(1).add(toUnit('0.01'))); - - await futuresMarket.transferMargin(margin, { from: trader }); - await futuresMarket.modifyPositionWithPriceBounds(toUnit('1'), minPrice, maxPrice, { - from: trader, - }); - - // Slips +1%. - await setPrice(baseAsset, maxPrice.add(toBN(1))); - await assert.revert( - futuresMarket.modifyPositionWithPriceBounds(toUnit('-1'), minPrice, maxPrice, { - from: trader, - }), - 'Price out of acceptable range' - ); - await assert.revert( - futuresMarket.closePositionWithPriceBounds(minPrice, maxPrice, { - from: trader, - }), - 'Price out of acceptable range' - ); - - // Slips -1%. - await setPrice(baseAsset, minPrice.sub(toBN(1))); - await assert.revert( - futuresMarket.modifyPositionWithPriceBounds(toUnit('1'), minPrice, maxPrice, { - from: trader, - }), - 'Price out of acceptable range' - ); - await assert.revert( - futuresMarket.closePositionWithPriceBounds(minPrice, maxPrice, { - from: trader, - }), - 'Price out of acceptable range' - ); - - await setPrice(baseAsset, startPrice); - await futuresMarket.closePositionWithPriceBounds(minPrice, maxPrice, { - from: trader, - }); - }); - it('Cannot modify a position if it is liquidating', async () => { await transferMarginAndModifyPosition({ market: futuresMarket, @@ -3573,28 +3522,10 @@ contract('FuturesMarket', accounts => { ); }); - it('then modifyPositionWithPriceBounds reverts', async () => { - await assert.revert( - futuresMarket.modifyPositionWithPriceBounds(toUnit('1'), toUnit('0.9'), toUnit('1.2'), { - from: trader, - }), - 'Invalid price' - ); - }); - it('then closePosition reverts', async () => { await assert.revert(futuresMarket.closePosition({ from: trader }), 'Invalid price'); }); - it('then closePositionWithPriceBounds reverts', async () => { - await assert.revert( - futuresMarket.closePositionWithPriceBounds(toUnit('0.9'), toUnit('1.2'), { - from: trader, - }), - 'Invalid price' - ); - }); - it('then liquidatePosition reverts', async () => { await assert.revert( futuresMarket.liquidatePosition(trader, { from: trader }), @@ -3650,27 +3581,10 @@ contract('FuturesMarket', accounts => { assert.bnEqual(await exchangeCircuitBreaker.lastExchangeRate(baseAsset), newPrice); }); - it('after modifyPositionWithPriceBounds', async () => { - await futuresMarket.modifyPositionWithPriceBounds( - toUnit('1'), - toUnit('50'), - toUnit('200'), - { from: trader } - ); - assert.bnEqual(await exchangeCircuitBreaker.lastExchangeRate(baseAsset), newPrice); - }); - it('after closePosition', async () => { await futuresMarket.closePosition({ from: trader }); assert.bnEqual(await exchangeCircuitBreaker.lastExchangeRate(baseAsset), newPrice); }); - - it('after closePositionWithPriceBounds reverts', async () => { - await futuresMarket.closePositionWithPriceBounds(toUnit('50'), toUnit('200'), { - from: trader, - }); - assert.bnEqual(await exchangeCircuitBreaker.lastExchangeRate(baseAsset), newPrice); - }); }); }); @@ -3710,19 +3624,7 @@ contract('FuturesMarket', accounts => { futuresMarket.modifyPosition(toUnit('1'), { from: trader }), revertMessage ); - await assert.revert( - futuresMarket.modifyPositionWithPriceBounds(toUnit('1'), toUnit('105'), toUnit('115'), { - from: trader, - }), - revertMessage - ); await assert.revert(futuresMarket.closePosition({ from: trader }), revertMessage); - await assert.revert( - futuresMarket.closePositionWithPriceBounds(toUnit('105'), toUnit('115'), { - from: trader, - }), - revertMessage - ); }); it('margin modifying actions do not revert', async () => { @@ -3802,16 +3704,6 @@ contract('FuturesMarket', accounts => { await futuresMarket.modifyPosition(toUnit('1'), { from: trader }); await futuresMarket.closePosition({ from: trader }); - await futuresMarket.modifyPositionWithPriceBounds( - toUnit('1'), - toUnit('100'), - toUnit('115'), - { from: trader } - ); - await futuresMarket.closePositionWithPriceBounds(toUnit('100'), toUnit('115'), { - from: trader, - }); - await futuresMarket.transferMargin(toUnit('1000'), { from: trader }); await futuresMarket.withdrawAllMargin({ from: trader }); }); From 76ae94d3794e1d1e3997efd45feb4eb0efbdad79 Mon Sep 17 00:00:00 2001 From: artdgn Date: Fri, 4 Feb 2022 18:09:45 +1100 Subject: [PATCH 08/10] additional minor stuff --- contracts/FuturesMarketBase.sol | 43 ++++++++--------------- contracts/MixinFuturesNextPriceOrders.sol | 10 +++--- 2 files changed, 20 insertions(+), 33 deletions(-) diff --git a/contracts/FuturesMarketBase.sol b/contracts/FuturesMarketBase.sol index d63e74795f..a9a1930429 100644 --- a/contracts/FuturesMarketBase.sol +++ b/contracts/FuturesMarketBase.sol @@ -259,19 +259,13 @@ contract FuturesMarketBase is MixinFuturesMarketSettings, IFuturesMarketBaseType uint endIndex, uint price ) internal view returns (int) { - int result; - // If the end index is not later than the start index, no funding has accrued. if (endIndex <= startIndex) { return 0; } // Determine whether we should include unrecorded funding. - if (endIndex == fundingSequence.length) { - result = _nextFundingEntry(price); - } else { - result = fundingSequence[endIndex]; - } + int result = endIndex == fundingSequence.length ? _nextFundingEntry(price) : fundingSequence[endIndex]; // Compute the net difference between start and end indices. return result.sub(fundingSequence[startIndex]); @@ -466,28 +460,19 @@ contract FuturesMarketBase is MixinFuturesMarketSettings, IFuturesMarketBaseType return proportionalFee > minFee ? proportionalFee : minFee; // not using _max() helper because it's for signed ints } - /** - * The margin buffer to maintain above the liquidation fee. The buffer is proportional to the position - * size. The buffer should prevent liquidation happenning at negative margin (due to next price being worse) - * so that stakers would not leak value to liquidators through minting rewards that are not from the - * account's margin. - * @param positionSize size of position in fixed point decimal baseAsset units - * @param price price of single baseAsset unit in sUSD fixed point decimal units - * @return lBuffer liquidation buffer to be paid to liquidator in sUSD fixed point decimal units - */ - function _liquidationBuffer(int positionSize, uint price) internal view returns (uint lBuffer) { - // size * price * buffer-ratio - return _abs(positionSize).multiplyDecimal(price).multiplyDecimal(_liquidationBufferRatio()); - } - /** * The minimal margin at which liquidation can happen. Is the sum of liquidationBuffer and liquidationFee * @param positionSize size of position in fixed point decimal baseAsset units * @param price price of single baseAsset unit in sUSD fixed point decimal units * @return lMargin liquidation margin to maintain in sUSD fixed point decimal units + * @dev The liquidation margin contains a buffer that is proportional to the position + * size. The buffer should prevent liquidation happenning at negative margin (due to next price being worse) + * so that stakers would not leak value to liquidators through minting rewards that are not from the + * account's margin. */ function _liquidationMargin(int positionSize, uint price) internal view returns (uint lMargin) { - return _liquidationBuffer(positionSize, price).add(_liquidationFee(positionSize, price)); + uint liquidationBuffer = _abs(positionSize).multiplyDecimal(price).multiplyDecimal(_liquidationBufferRatio()); + return liquidationBuffer.add(_liquidationFee(positionSize, price)); } function _canLiquidate( @@ -928,15 +913,16 @@ contract FuturesMarketBase is MixinFuturesMarketSettings, IFuturesMarketBaseType */ function modifyPosition(int sizeDelta) external { uint price = _assetPriceRequireChecks(); - TradeParams memory params = + _modifyPosition( + msg.sender, TradeParams({ sizeDelta: sizeDelta, price: price, fundingIndex: _recomputeFunding(price), takerFee: _takerFee(baseAsset), makerFee: _makerFee(baseAsset) - }); - _modifyPosition(msg.sender, params); + }) + ); } /* @@ -946,15 +932,16 @@ contract FuturesMarketBase is MixinFuturesMarketSettings, IFuturesMarketBaseType int size = positions[msg.sender].size; _revertIfError(size == 0, Status.NoPositionOpen); uint price = _assetPriceRequireChecks(); - TradeParams memory params = + _modifyPosition( + msg.sender, TradeParams({ sizeDelta: -size, price: price, fundingIndex: _recomputeFunding(price), takerFee: _takerFee(baseAsset), makerFee: _makerFee(baseAsset) - }); - _modifyPosition(msg.sender, params); + }) + ); } function _liquidatePosition( diff --git a/contracts/MixinFuturesNextPriceOrders.sol b/contracts/MixinFuturesNextPriceOrders.sol index dc426705d1..691fae0577 100644 --- a/contracts/MixinFuturesNextPriceOrders.sol +++ b/contracts/MixinFuturesNextPriceOrders.sol @@ -186,17 +186,17 @@ contract MixinFuturesNextPriceOrders is FuturesMarketBase { // the correct price for the past round (uint pastPrice, ) = _exchangeRates().rateAndTimestampAtRound(baseAsset, order.targetRoundId); - // set up the trade params - TradeParams memory params = + // execute or revert + _modifyPosition( + account, TradeParams({ sizeDelta: order.sizeDelta, // using the pastPrice from the target roundId price: pastPrice, // the funding is applied only from order confirmation time fundingIndex: fundingIndex, // using the next-price fees takerFee: _takerFeeNextPrice(baseAsset), makerFee: _makerFeeNextPrice(baseAsset) - }); - // execute or revert - _modifyPosition(account, params); + }) + ); // remove stored order delete nextPriceOrders[account]; From b3edc214ced3d870d10612ee15ad54c0ae906bed Mon Sep 17 00:00:00 2001 From: artdgn Date: Mon, 7 Feb 2022 22:00:03 +1100 Subject: [PATCH 09/10] simplify passing around fundingIndex --- contracts/FuturesMarketBase.sol | 126 ++++++------------ contracts/MixinFuturesNextPriceOrders.sol | 8 +- contracts/MixinFuturesViews.sol | 12 +- .../test-helpers/TestableFuturesMarket.sol | 2 +- 4 files changed, 52 insertions(+), 96 deletions(-) diff --git a/contracts/FuturesMarketBase.sol b/contracts/FuturesMarketBase.sol index a9a1930429..28db6ea66b 100644 --- a/contracts/FuturesMarketBase.sol +++ b/contracts/FuturesMarketBase.sol @@ -151,7 +151,6 @@ contract FuturesMarketBase is MixinFuturesMarketSettings, IFuturesMarketBaseType struct TradeParams { int sizeDelta; uint price; - uint fundingIndex; uint takerFee; uint makerFee; } @@ -251,24 +250,12 @@ contract FuturesMarketBase is MixinFuturesMarketSettings, IFuturesMarketBaseType * last entry and the unrecorded funding, so the sequence accumulates running total over the market's lifetime. */ function _nextFundingEntry(uint price) internal view returns (int funding) { - return int(fundingSequence[fundingSequence.length.sub(1)]).add(_unrecordedFunding(price)); + return int(fundingSequence[_latestFundingIndex()]).add(_unrecordedFunding(price)); } - function _netFundingPerUnit( - uint startIndex, - uint endIndex, - uint price - ) internal view returns (int) { - // If the end index is not later than the start index, no funding has accrued. - if (endIndex <= startIndex) { - return 0; - } - - // Determine whether we should include unrecorded funding. - int result = endIndex == fundingSequence.length ? _nextFundingEntry(price) : fundingSequence[endIndex]; - + function _netFundingPerUnit(uint startIndex, uint price) internal view returns (int) { // Compute the net difference between start and end indices. - return result.sub(fundingSequence[startIndex]); + return _nextFundingEntry(price).sub(fundingSequence[startIndex]); } /* ---------- Position Details ---------- */ @@ -321,28 +308,20 @@ contract FuturesMarketBase is MixinFuturesMarketSettings, IFuturesMarketBaseType return int(position.size).multiplyDecimal(priceShift); } - function _accruedFunding( - Position memory position, - uint endFundingIndex, - uint price - ) internal view returns (int funding) { + function _accruedFunding(Position memory position, uint price) internal view returns (int funding) { uint lastModifiedIndex = position.lastFundingIndex; if (lastModifiedIndex == 0) { return 0; // The position does not exist -- no funding. } - int net = _netFundingPerUnit(lastModifiedIndex, endFundingIndex, price); + int net = _netFundingPerUnit(lastModifiedIndex, price); return int(position.size).multiplyDecimal(net); } /* * The initial margin of a position, plus any PnL and funding it has accrued. The resulting value may be negative. */ - function _marginPlusProfitFunding( - Position memory position, - uint endFundingIndex, - uint price - ) internal view returns (int) { - int funding = _accruedFunding(position, endFundingIndex, price); + function _marginPlusProfitFunding(Position memory position, uint price) internal view returns (int) { + int funding = _accruedFunding(position, price); return int(position.margin).add(_profitLoss(position, price)).add(funding); } @@ -354,11 +333,10 @@ contract FuturesMarketBase is MixinFuturesMarketSettings, IFuturesMarketBaseType */ 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); + int newMargin = _marginPlusProfitFunding(position, price).add(marginDelta); if (newMargin < 0) { return (0, Status.InsufficientMargin); } @@ -374,22 +352,14 @@ contract FuturesMarketBase is MixinFuturesMarketSettings, IFuturesMarketBaseType return (uMargin, Status.Ok); } - function _remainingMargin( - Position memory position, - uint endFundingIndex, - uint price - ) internal view returns (uint) { - int remaining = _marginPlusProfitFunding(position, endFundingIndex, price); + function _remainingMargin(Position memory position, uint price) internal view returns (uint) { + int remaining = _marginPlusProfitFunding(position, price); // If the margin went past zero, the position should have been liquidated - return zero remaining margin. return uint(_max(0, remaining)); } - function _accessibleMargin( - Position memory position, - uint fundingIndex, - uint price - ) internal view returns (uint) { + function _accessibleMargin(Position memory position, uint price) internal view returns (uint) { // Ugly solution to rounding safety: leave up to an extra tenth of a cent in the account/leverage // This should guarantee that the value returned here can always been withdrawn, but there may be // a little extra actually-accessible value left over, depending on the position size and margin. @@ -406,7 +376,7 @@ contract FuturesMarketBase is MixinFuturesMarketSettings, IFuturesMarketBaseType inaccessible = inaccessible.add(milli); } - uint remaining = _remainingMargin(position, fundingIndex, price); + uint remaining = _remainingMargin(position, price); if (remaining <= inaccessible) { return 0; } @@ -423,7 +393,7 @@ contract FuturesMarketBase is MixinFuturesMarketSettings, IFuturesMarketBaseType } // price = lastPrice + (liquidationMargin - margin) / positionSize - netAccrued - int fundingPerUnit = _netFundingPerUnit(position.lastFundingIndex, fundingSequence.length, currentPrice); + int fundingPerUnit = _netFundingPerUnit(position.lastFundingIndex, currentPrice); // minimum margin beyond which position can be liqudiated uint liqMargin = _liquidationMargin(positionSize, currentPrice); @@ -475,17 +445,13 @@ contract FuturesMarketBase is MixinFuturesMarketSettings, IFuturesMarketBaseType return liquidationBuffer.add(_liquidationFee(positionSize, price)); } - function _canLiquidate( - Position memory position, - uint fundingIndex, - uint price - ) internal view returns (bool) { + function _canLiquidate(Position memory position, uint price) internal view returns (bool) { // No liquidating empty positions. if (position.size == 0) { return false; } - return _remainingMargin(position, fundingIndex, price) <= _liquidationMargin(int(position.size), price); + return _remainingMargin(position, price) <= _liquidationMargin(int(position.size), price); } function _currentLeverage( @@ -522,6 +488,10 @@ contract FuturesMarketBase is MixinFuturesMarketSettings, IFuturesMarketBaseType return _exchanger().dynamicFeeRateForExchange(sUSD, baseAsset); } + function _latestFundingIndex() internal view returns (uint) { + return fundingSequence.length.sub(1); // at least one element is pushed in constructor + } + function _postTradeDetails(Position memory oldPos, TradeParams memory params) internal view @@ -537,7 +507,7 @@ contract FuturesMarketBase is MixinFuturesMarketSettings, IFuturesMarketBaseType } // The order is not submitted if the user's existing position needs to be liquidated. - if (_canLiquidate(oldPos, params.fundingIndex, params.price)) { + if (_canLiquidate(oldPos, params.price)) { return (oldPos, 0, Status.CanLiquidate); } @@ -554,14 +524,14 @@ contract FuturesMarketBase is MixinFuturesMarketSettings, IFuturesMarketBaseType // Deduct the fee. // It is an error if the realised margin minus the fee is negative or subject to liquidation. - (uint newMargin, Status status) = _realisedMargin(oldPos, params.fundingIndex, params.price, -int(fee)); + (uint newMargin, Status status) = _realisedMargin(oldPos, params.price, -int(fee)); if (_isError(status)) { return (oldPos, 0, status); } Position memory newPos = Position({ id: oldPos.id, - lastFundingIndex: uint64(params.fundingIndex), + lastFundingIndex: uint64(_latestFundingIndex()), margin: uint128(newMargin), lastPrice: uint128(params.price), size: int128(newSize) @@ -756,7 +726,6 @@ contract FuturesMarketBase is MixinFuturesMarketSettings, IFuturesMarketBaseType function _transferMargin( int marginDelta, uint price, - uint fundingIndex, address sender ) internal { // Transfer no tokens if marginDelta is 0 @@ -781,29 +750,29 @@ contract FuturesMarketBase is MixinFuturesMarketSettings, IFuturesMarketBaseType } Position storage position = positions[sender]; - _updatePositionMargin(position, fundingIndex, price, marginDelta); + _updatePositionMargin(position, price, marginDelta); // Emit relevant events if (marginDelta != 0) { emit MarginTransferred(sender, marginDelta); } - emit PositionModified(position.id, sender, position.margin, position.size, 0, price, fundingIndex, 0); + emit PositionModified(position.id, sender, position.margin, position.size, 0, price, _latestFundingIndex(), 0); } // udpates the stored position margin in place (on the stored position) function _updatePositionMargin( Position storage position, - uint fundingIndex, uint price, int marginDelta ) internal { Position memory oldPosition = position; // Determine new margin, ensuring that the result is positive. - (uint margin, Status status) = _realisedMargin(oldPosition, fundingIndex, price, marginDelta); + (uint margin, Status status) = _realisedMargin(oldPosition, price, marginDelta); _revertIfError(status); // Update the debt correction. int positionSize = position.size; + uint fundingIndex = _latestFundingIndex(); _applyDebtCorrection( Position(0, uint64(fundingIndex), uint128(margin), uint128(price), int128(positionSize)), Position(0, position.lastFundingIndex, position.margin, position.lastPrice, int128(positionSize)) @@ -838,8 +807,8 @@ contract FuturesMarketBase is MixinFuturesMarketSettings, IFuturesMarketBaseType */ function transferMargin(int marginDelta) external { uint price = _assetPriceRequireChecks(); - uint fundingIndex = _recomputeFunding(price); - _transferMargin(marginDelta, price, fundingIndex, msg.sender); + _recomputeFunding(price); + _transferMargin(marginDelta, price, msg.sender); } /* @@ -849,9 +818,9 @@ contract FuturesMarketBase is MixinFuturesMarketSettings, IFuturesMarketBaseType function withdrawAllMargin() external { address sender = msg.sender; uint price = _assetPriceRequireChecks(); - uint fundingIndex = _recomputeFunding(price); - int marginDelta = -int(_accessibleMargin(positions[sender], fundingIndex, price)); - _transferMargin(marginDelta, price, fundingIndex, sender); + _recomputeFunding(price); + int marginDelta = -int(_accessibleMargin(positions[sender], price)); + _transferMargin(marginDelta, price, sender); } function _modifyPosition(address sender, TradeParams memory params) internal { @@ -877,6 +846,7 @@ contract FuturesMarketBase is MixinFuturesMarketSettings, IFuturesMarketBaseType // Record the trade uint64 id = oldPosition.id; + uint fundingIndex = _latestFundingIndex(); if (newPosition.size == 0) { // If the position is being closed, we no longer need to track these details. delete position.id; @@ -892,7 +862,7 @@ contract FuturesMarketBase is MixinFuturesMarketSettings, IFuturesMarketBaseType position.id = id; position.size = newPosition.size; position.lastPrice = uint128(params.price); - position.lastFundingIndex = uint64(params.fundingIndex); + position.lastFundingIndex = uint64(fundingIndex); } // emit the modification event emit PositionModified( @@ -902,7 +872,7 @@ contract FuturesMarketBase is MixinFuturesMarketSettings, IFuturesMarketBaseType newPosition.size, params.sizeDelta, params.price, - params.fundingIndex, + fundingIndex, fee ); } @@ -913,15 +883,10 @@ contract FuturesMarketBase is MixinFuturesMarketSettings, IFuturesMarketBaseType */ function modifyPosition(int sizeDelta) external { uint price = _assetPriceRequireChecks(); + _recomputeFunding(price); _modifyPosition( msg.sender, - TradeParams({ - sizeDelta: sizeDelta, - price: price, - fundingIndex: _recomputeFunding(price), - takerFee: _takerFee(baseAsset), - makerFee: _makerFee(baseAsset) - }) + TradeParams({sizeDelta: sizeDelta, price: price, takerFee: _takerFee(baseAsset), makerFee: _makerFee(baseAsset)}) ); } @@ -932,28 +897,22 @@ contract FuturesMarketBase is MixinFuturesMarketSettings, IFuturesMarketBaseType int size = positions[msg.sender].size; _revertIfError(size == 0, Status.NoPositionOpen); uint price = _assetPriceRequireChecks(); + _recomputeFunding(price); _modifyPosition( msg.sender, - TradeParams({ - sizeDelta: -size, - price: price, - fundingIndex: _recomputeFunding(price), - takerFee: _takerFee(baseAsset), - makerFee: _makerFee(baseAsset) - }) + TradeParams({sizeDelta: -size, price: price, takerFee: _takerFee(baseAsset), makerFee: _makerFee(baseAsset)}) ); } function _liquidatePosition( address account, address liquidator, - uint fundingIndex, uint price ) internal { Position storage position = positions[account]; // get remaining margin for sending any leftover buffer to fee pool - uint remMargin = _remainingMargin(position, fundingIndex, price); + uint remMargin = _remainingMargin(position, price); // Record updates to market size and debt. int positionSize = position.size; @@ -961,6 +920,7 @@ contract FuturesMarketBase is MixinFuturesMarketSettings, IFuturesMarketBaseType marketSkew = int128(int(marketSkew).sub(positionSize)); marketSize = uint128(uint(marketSize).sub(_abs(positionSize))); + uint fundingIndex = _latestFundingIndex(); _applyDebtCorrection( Position(0, uint64(fundingIndex), 0, uint128(price), 0), Position(0, position.lastFundingIndex, position.margin, position.lastPrice, int128(positionSize)) @@ -989,11 +949,11 @@ contract FuturesMarketBase is MixinFuturesMarketSettings, IFuturesMarketBaseType */ function liquidatePosition(address account) external { uint price = _assetPriceRequireChecks(); - uint fundingIndex = _recomputeFunding(price); + _recomputeFunding(price); - _revertIfError(!_canLiquidate(positions[account], fundingIndex, price), Status.CannotLiquidate); + _revertIfError(!_canLiquidate(positions[account], price), Status.CannotLiquidate); - _liquidatePosition(account, msg.sender, fundingIndex, price); + _liquidatePosition(account, msg.sender, price); } /* ========== EVENTS ========== */ diff --git a/contracts/MixinFuturesNextPriceOrders.sol b/contracts/MixinFuturesNextPriceOrders.sol index 691fae0577..237e7e56c9 100644 --- a/contracts/MixinFuturesNextPriceOrders.sol +++ b/contracts/MixinFuturesNextPriceOrders.sol @@ -43,7 +43,6 @@ contract MixinFuturesNextPriceOrders is FuturesMarketBase { TradeParams({ sizeDelta: sizeDelta, price: price, - fundingIndex: fundingIndex, takerFee: _takerFeeNextPrice(baseAsset), makerFee: _makerFeeNextPrice(baseAsset) }); @@ -53,7 +52,7 @@ contract MixinFuturesNextPriceOrders is FuturesMarketBase { // deduct fees from margin uint commitDeposit = _nextPriceCommitDeposit(params); uint keeperDeposit = _minKeeperFee(); - _updatePositionMargin(position, fundingIndex, price, -int(commitDeposit + keeperDeposit)); + _updatePositionMargin(position, price, -int(commitDeposit + keeperDeposit)); // emit event for modidying the position (subtracting the fees from margin) emit PositionModified(position.id, msg.sender, position.margin, position.size, 0, price, fundingIndex, 0); @@ -104,7 +103,7 @@ contract MixinFuturesNextPriceOrders is FuturesMarketBase { Position storage position = positions[account]; uint price = _assetPriceRequireChecks(); uint fundingIndex = _recomputeFunding(price); - _updatePositionMargin(position, fundingIndex, price, int(order.keeperDeposit)); + _updatePositionMargin(position, price, int(order.keeperDeposit)); // emit event for modidying the position (add the fee to margin) emit PositionModified(position.id, account, position.margin, position.size, 0, price, fundingIndex, 0); @@ -180,7 +179,7 @@ contract MixinFuturesNextPriceOrders is FuturesMarketBase { uint fundingIndex = _recomputeFunding(currentPrice); // refund the commitFee (and possibly the keeperFee) to the margin before executing the order // if the order later fails this is reverted of course - _updatePositionMargin(position, fundingIndex, currentPrice, int(toRefund)); + _updatePositionMargin(position, currentPrice, int(toRefund)); // emit event for modidying the position (refunding fee/s) emit PositionModified(position.id, account, position.margin, position.size, 0, currentPrice, fundingIndex, 0); @@ -192,7 +191,6 @@ contract MixinFuturesNextPriceOrders is FuturesMarketBase { TradeParams({ sizeDelta: order.sizeDelta, // using the pastPrice from the target roundId price: pastPrice, // the funding is applied only from order confirmation time - fundingIndex: fundingIndex, // using the next-price fees takerFee: _takerFeeNextPrice(baseAsset), makerFee: _makerFeeNextPrice(baseAsset) }) diff --git a/contracts/MixinFuturesViews.sol b/contracts/MixinFuturesViews.sol index 1e0f935d48..52f0ab1103 100644 --- a/contracts/MixinFuturesViews.sol +++ b/contracts/MixinFuturesViews.sol @@ -82,7 +82,7 @@ contract MixinFuturesViews is FuturesMarketBase { */ function accruedFunding(address account) external view returns (int funding, bool invalid) { (uint price, bool isInvalid) = assetPrice(); - return (_accruedFunding(positions[account], fundingSequence.length, price), isInvalid); + return (_accruedFunding(positions[account], price), isInvalid); } /* @@ -90,7 +90,7 @@ contract MixinFuturesViews is FuturesMarketBase { */ function remainingMargin(address account) external view returns (uint marginRemaining, bool invalid) { (uint price, bool isInvalid) = assetPrice(); - return (_remainingMargin(positions[account], fundingSequence.length, price), isInvalid); + return (_remainingMargin(positions[account], price), isInvalid); } /* @@ -99,7 +99,7 @@ contract MixinFuturesViews is FuturesMarketBase { */ function accessibleMargin(address account) external view returns (uint marginAccessible, bool invalid) { (uint price, bool isInvalid) = assetPrice(); - return (_accessibleMargin(positions[account], fundingSequence.length, price), isInvalid); + return (_accessibleMargin(positions[account], price), isInvalid); } /* @@ -124,7 +124,7 @@ contract MixinFuturesViews is FuturesMarketBase { */ function liquidationFee(address account) external view returns (uint) { (uint price, bool invalid) = assetPrice(); - if (!invalid && _canLiquidate(positions[account], fundingSequence.length, price)) { + if (!invalid && _canLiquidate(positions[account], price)) { return _liquidationFee(int(positions[account].size), price); } else { // theoretically we can calculate a value, but this value is always incorrect because @@ -139,7 +139,7 @@ contract MixinFuturesViews is FuturesMarketBase { */ function canLiquidate(address account) external view returns (bool) { (uint price, bool invalid) = assetPrice(); - return !invalid && _canLiquidate(positions[account], fundingSequence.length, price); + return !invalid && _canLiquidate(positions[account], price); } /* @@ -157,7 +157,6 @@ contract MixinFuturesViews is FuturesMarketBase { TradeParams({ sizeDelta: sizeDelta, price: price, - fundingIndex: 0, // doesn't matter for fee calculation takerFee: _takerFee(baseAsset), makerFee: _makerFee(baseAsset) }); @@ -189,7 +188,6 @@ contract MixinFuturesViews is FuturesMarketBase { TradeParams({ sizeDelta: sizeDelta, price: price, - fundingIndex: fundingSequence.length, takerFee: _takerFee(baseAsset), makerFee: _makerFee(baseAsset) }); diff --git a/contracts/test-helpers/TestableFuturesMarket.sol b/contracts/test-helpers/TestableFuturesMarket.sol index d7dfca807c..84ff484c41 100644 --- a/contracts/test-helpers/TestableFuturesMarket.sol +++ b/contracts/test-helpers/TestableFuturesMarket.sol @@ -57,7 +57,7 @@ contract TestableFuturesMarket is FuturesMarket { function currentLeverage(address account) external view returns (int leverage, bool invalid) { (uint price, bool isInvalid) = assetPrice(); Position storage position = positions[account]; - uint remainingMargin_ = _remainingMargin(position, fundingSequence.length, price); + uint remainingMargin_ = _remainingMargin(position, price); return (_currentLeverage(position, price, remainingMargin_), isInvalid); } } From 2057ffb1e62d01f3972f948f7d1a844b7812cfd9 Mon Sep 17 00:00:00 2001 From: artdgn Date: Tue, 8 Feb 2022 15:36:52 +1100 Subject: [PATCH 10/10] address preliminary audit fixes --- contracts/FuturesMarket.sol | 11 +-- contracts/FuturesMarketBase.sol | 71 ++++++++++++------- .../test-helpers/TestableFuturesMarket.sol | 2 +- 3 files changed, 48 insertions(+), 36 deletions(-) diff --git a/contracts/FuturesMarket.sol b/contracts/FuturesMarket.sol index 988cff5460..bb7eec3be2 100644 --- a/contracts/FuturesMarket.sol +++ b/contracts/FuturesMarket.sol @@ -29,14 +29,12 @@ import "./interfaces/IFuturesMarket.sol"; * As the funding rate is the same (but negated) on both sides of the market, there is an excess quantity of * funding being charged, which is collected by the debt pool, and serves to reduce the system debt. * - * To combat front-running, the system does not confirm a user's order until the next price is received from - * the oracle. Therefore opening a position is a three stage procedure: depositing margin, submitting an order, - * and waiting for that order to be confirmed. The last transaction is performed by a keeper, - * once a price update is detected. - * * The contract architecture is as follows: * * - FuturesMarket.sol: one of these exists per asset. Margin is maintained isolated per market. + * this contract is composed of several mixins: `base` contains all the core logic, + * `nextPrice` contains the next-price order flows, and `views` contains logic + * that is only used by external / manager contracts. * * - FuturesMarketManager.sol: the manager keeps track of which markets exist, and is the main window between * futures markets and the rest of the system. It accumulates the total debt @@ -47,9 +45,6 @@ import "./interfaces/IFuturesMarket.sol"; * the base asset, these settings determine the behaviour of each market. * See that contract for descriptions of the meanings of each setting. * - * Each futures market and the manager operates behind a proxy, and for efficiency they communicate with one another - * using their underlying implementations. - * * Technical note: internal functions within the FuturesMarket contract assume the following: * * - prices passed into them are valid; diff --git a/contracts/FuturesMarketBase.sol b/contracts/FuturesMarketBase.sol index 28db6ea66b..1e220662d9 100644 --- a/contracts/FuturesMarketBase.sol +++ b/contracts/FuturesMarketBase.sol @@ -223,12 +223,7 @@ contract FuturesMarketBase is MixinFuturesMarketSettings, IFuturesMarketBaseType // marketSize is in baseAsset units so we need to convert from USD units require(price > 0, "price can't be zero"); uint skewScaleBaseAsset = _skewScaleUSD(baseAsset).divideDecimal(price); - - // parameters may not be set, don't divide by zero - if (skewScaleBaseAsset == 0) { - return 0; - } - + require(skewScaleBaseAsset != 0, "skewScale is zero"); // don't divide by zero return int(marketSkew).divideDecimal(int(skewScaleBaseAsset)); } @@ -331,7 +326,7 @@ contract FuturesMarketBase is MixinFuturesMarketSettings, IFuturesMarketBaseType * 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( + function _recomputeMarginWithDelta( Position memory position, uint price, int marginDelta @@ -403,8 +398,10 @@ contract FuturesMarketBase is MixinFuturesMarketSettings, IFuturesMarketBaseType // Hence, expanding the definition of remainingMargin the exact price // at which a position can first be liquidated is: // margin + profitLoss + funding = liquidationMargin - // profitLoss = (price - last-price) * positionSize - // price = lastPrice + (liquidationMargin - margin) / positionSize - netFundingPerUnit + // substitute with: profitLoss = (price - last-price) * positionSize + // and also with: funding = netFundingPerUnit * positionSize + // we get: margin + (price - last-price) * positionSize + netFundingPerUnit * positionSize = liquidationMargin + // moving around: price = lastPrice + (liquidationMargin - margin) / positionSize - netFundingPerUnit int result = int(position.lastPrice).add(int(liqMargin).sub(int(position.margin)).divideDecimal(positionSize)).sub( fundingPerUnit @@ -511,8 +508,6 @@ contract FuturesMarketBase is MixinFuturesMarketSettings, IFuturesMarketBaseType return (oldPos, 0, Status.CanLiquidate); } - int newSize = int(oldPos.size).add(params.sizeDelta); - // get the dynamic fee rate SIP-184 (uint dynamicFeeRate, bool tooVolatile) = _dynamicFeeRate(); if (tooVolatile) { @@ -524,21 +519,27 @@ contract FuturesMarketBase is MixinFuturesMarketSettings, IFuturesMarketBaseType // Deduct the fee. // It is an error if the realised margin minus the fee is negative or subject to liquidation. - (uint newMargin, Status status) = _realisedMargin(oldPos, params.price, -int(fee)); + (uint newMargin, Status status) = _recomputeMarginWithDelta(oldPos, params.price, -int(fee)); if (_isError(status)) { return (oldPos, 0, status); } + + // construct new position Position memory newPos = Position({ id: oldPos.id, lastFundingIndex: uint64(_latestFundingIndex()), margin: uint128(newMargin), lastPrice: uint128(params.price), - size: int128(newSize) + size: int128(int(oldPos.size).add(params.sizeDelta)) }); - // Check that the user has sufficient margin given their order. - // We don't check the margin requirement if the position size is decreasing + // always allow to decrease a position, otherwise a margin of minInitialMargin cen never + // decrease a position as as the price goes against them. + // we also add the paid out fee for the minInitialMargin because otherwise minInitialMargin + // is never the actual minMargin, because the first trade will always deduct + // a fee (so the margin that otherwise would need to be transferred would have to include the future + // fee as well, making the UX and definition of min-margin confusing). bool positionDecreasing = _sameSide(oldPos.size, newPos.size) && _abs(newPos.size) < _abs(oldPos.size); if (!positionDecreasing) { // minMargin + fee <= margin is equivalent to minMargin <= margin - fee @@ -548,11 +549,21 @@ contract FuturesMarketBase is MixinFuturesMarketSettings, IFuturesMarketBaseType } } - // Check that the maximum leverage is not exceeded (ignoring the fee). + // check that new position margin is above liquidation margin + // (above, in _recomputeMarginWithDelta() we checked the old position, here we check the new one) + // Liquidation margin is considered without a fee, because it wouldn't make sense to allow + // a trade that will make the position liquidatable. + if (newMargin <= _liquidationMargin(newPos.size, params.price)) { + return (newPos, 0, Status.CanLiquidate); + } + + // Check that the maximum leverage is not exceeded when considering new margin including the paid fee. + // The paid fee is considered for the benefit of UX of allowed max leverage, otherwise, the actual + // max leverage is always below the max leverage parameter since the fee paid for a trade reduces the margin. // We'll allow a little extra headroom for rounding errors. { // stack too deep - int leverage = newSize.multiplyDecimal(int(params.price)).divideDecimal(int(newMargin.add(fee))); + int leverage = int(newPos.size).multiplyDecimal(int(params.price)).divideDecimal(int(newMargin.add(fee))); if (_maxLeverage(baseAsset).add(uint(_UNIT) / 100) < _abs(leverage)) { return (oldPos, 0, Status.MaxLeverageExceeded); } @@ -600,9 +611,7 @@ contract FuturesMarketBase is MixinFuturesMarketSettings, IFuturesMarketBaseType // True if and only if two positions a and b are on the same side of the market; // that is, if they have the same sign, or either of them is zero. function _sameSide(int a, int b) internal pure returns (bool) { - // Since we only care about the sign of the product, we don't care about overflow and - // aren't using SignedSafeDecimalMath - return 0 <= a * b; + return (a >= 0) == (b >= 0); } /* @@ -679,9 +688,14 @@ contract FuturesMarketBase is MixinFuturesMarketSettings, IFuturesMarketBaseType */ function _positionDebtCorrection(Position memory position) internal view returns (int) { /** - Debt correction calculation from the SIP https://sips.synthetix.io/sips/sip-80/ + This method only returns the correction term for the debt calculation of the position, and not it's + debt. This is needed for keeping track of the _markedDebt() in an efficient manner to allow O(1) marketDebt + calculation in _marketDebt(). + + Explanation of the full market debt calculation from the SIP https://sips.synthetix.io/sips/sip-80/: + The overall market debt is the sum of the remaining margin in all positions. The intuition is that - the debt of a single positino is the value withdrawn upon closing that position. + the debt of a single position is the value withdrawn upon closing that position. single position remaining margin = initial-margin + profit-loss + accrued-funding = = initial-margin + q * (price - last-prise) + q * funding-accrued-per-unit @@ -694,7 +708,9 @@ contract FuturesMarketBase is MixinFuturesMarketSettings, IFuturesMarketBaseType = skew (price + funding) + sum( initial-margin + q * last-price + q * initial-funding ) The last term: sum( initial-margin + q * last-price + q * initial-funding ) being the position debt correction - that is tracked with each position change. And the first term is calculated globally in _marketDebt(), + that is tracked with each position change using this method. + + The first term and the full debt calculation using current skew, price, and funding is calculated globally in _marketDebt(). */ return int(position.margin).sub( @@ -767,7 +783,7 @@ contract FuturesMarketBase is MixinFuturesMarketSettings, IFuturesMarketBaseType ) internal { Position memory oldPosition = position; // Determine new margin, ensuring that the result is positive. - (uint margin, Status status) = _realisedMargin(oldPosition, price, marginDelta); + (uint margin, Status status) = _recomputeMarginWithDelta(oldPosition, price, marginDelta); _revertIfError(status); // Update the debt correction. @@ -787,12 +803,13 @@ contract FuturesMarketBase is MixinFuturesMarketSettings, IFuturesMarketBaseType // The user can always decrease their margin if they have no position, or as long as: // * they have sufficient margin to do so - // * the resulting margin would not be lower than the minimum margin + // * the resulting margin would not be lower than the liquidation margin or min initial margin // * the resulting leverage is lower than the maximum leverage if (marginDelta < 0) { _revertIfError( - margin < _minInitialMargin() || - _maxLeverage(baseAsset) < _abs(_currentLeverage(position, price, margin)), + (margin < _minInitialMargin()) || + (margin <= _liquidationMargin(position.size, price)) || + (_maxLeverage(baseAsset) < _abs(_currentLeverage(position, price, margin))), Status.InsufficientMargin ); } diff --git a/contracts/test-helpers/TestableFuturesMarket.sol b/contracts/test-helpers/TestableFuturesMarket.sol index 84ff484c41..97527420d1 100644 --- a/contracts/test-helpers/TestableFuturesMarket.sol +++ b/contracts/test-helpers/TestableFuturesMarket.sol @@ -46,7 +46,7 @@ contract TestableFuturesMarket is FuturesMarket { * @return lMargin liquidation margin to maintain in sUSD fixed point decimal units */ function liquidationMargin(address account) external view returns (uint lMargin) { - require(positions[account].size != 0, "0 size position"); + require(positions[account].size != 0, "0 size position"); // reverts because otherwise minKeeperFee is returned (uint price, ) = assetPrice(); return _liquidationMargin(int(positions[account].size), price); }