diff --git a/core/contracts/financial-templates/common/FeePayer.sol b/core/contracts/financial-templates/common/FeePayer.sol index 2cdcb25f75..386ba1de6a 100644 --- a/core/contracts/financial-templates/common/FeePayer.sol +++ b/core/contracts/financial-templates/common/FeePayer.sol @@ -3,9 +3,11 @@ pragma experimental ABIEncoderV2; import "@openzeppelin/contracts/token/ERC20/IERC20.sol"; import "@openzeppelin/contracts/token/ERC20/SafeERC20.sol"; + import "../../common/implementation/Lockable.sol"; import "../../common/implementation/FixedPoint.sol"; import "../../common/implementation/Testable.sol"; + import "../../oracle/interfaces/StoreInterface.sol"; import "../../oracle/interfaces/FinderInterface.sol"; import "../../oracle/implementation/Constants.sol"; @@ -85,11 +87,11 @@ abstract contract FeePayer is Testable, Lockable { /** * @notice Pays UMA DVM regular fees (as a % of the collateral pool) to the Store contract. - * @dev These must be paid periodically for the life of the contract. If the contract has not paid its - * regular fee in a week or more then a late penalty is applied which is sent to the caller. - * This will revert if the amount of fees owed are greater than the pfc. An event is only fired if the fees charged are greater than 0. + * @dev These must be paid periodically for the life of the contract. If the contract has not paid its regular fee + * in a week or more then a late penalty is applied which is sent to the caller. This will revert if the amount of + * fees owed are greater than the pfc. An event is only fired if the fees charged are greater than 0. * @return totalPaid Amount of collateral that the contract paid (sum of the amount paid to the Store and caller). - * This will return 0 and exit early if there is no pfc, fees were already paid during the current block, or the fee rate is 0. + * This returns 0 and exit early if there is no pfc, fees were already paid during the current block, or the fee rate is 0. */ function payRegularFees() public nonReentrant() returns (FixedPoint.Unsigned memory totalPaid) { StoreInterface store = _getStore(); @@ -117,9 +119,9 @@ abstract contract FeePayer is Testable, Lockable { if (totalPaid.isEqual(0)) { return totalPaid; } - // If the effective fees paid as a % of the pfc is > 100%, then we need to reduce it and make the contract pay as much of the fee - // that it can (up to 100% of its pfc). We'll reduce the late penalty first and then the regular fee, which has the effect of paying - // the store first, followed by the caller if there is any fee remaining. + // If the effective fees paid as a % of the pfc is > 100%, then we need to reduce it and make the contract pay + // as much of the fee that it can (up to 100% of its pfc). We'll reduce the late penalty first and then the + // regular fee, which has the effect of paying the store first, followed by the caller if there is any fee remaining. if (totalPaid.isGreaterThan(collateralPool)) { FixedPoint.Unsigned memory deficit = totalPaid.sub(collateralPool); FixedPoint.Unsigned memory latePenaltyReduction = FixedPoint.min(latePenalty, deficit); @@ -141,11 +143,26 @@ abstract contract FeePayer is Testable, Lockable { if (latePenalty.isGreaterThan(0)) { collateralCurrency.safeTransfer(msg.sender, latePenalty.rawValue); } + return totalPaid; + } + + /** + * @notice Gets the current profit from corruption for this contract in terms of the collateral currency. + * @dev This is equivalent to the collateral pool available from which to pay fees. Therefore, derived contracts are + * expected to implement this so that pay-fee methods can correctly compute the owed fees as a % of PfC. + * @return pfc value for equal to the current profic from corrution denominated in collateral currency. + */ + function pfc() public view nonReentrantView() returns (FixedPoint.Unsigned memory) { + return _pfc(); } - // Pays UMA Oracle final fees of `amount` in `collateralCurrency` to the Store contract. Final fee is a flat fee charged for each price request. - // If payer is the contract, adjusts internal bookkeeping variables. If payer is not the contract, pulls in `amount` - // of collateral currency. + /**************************************** + * INTERNAL FUNCTIONS * + ****************************************/ + + // Pays UMA Oracle final fees of `amount` in `collateralCurrency` to the Store contract. Final fee is a flat fee + // charged for each price request. If payer is the contract, adjusts internal bookkeeping variables. If payer is not + // the contract, pulls in `amount` of collateral currency. function _payFinalFees(address payer, FixedPoint.Unsigned memory amount) internal { if (amount.isEqual(0)) { return; @@ -171,20 +188,6 @@ abstract contract FeePayer is Testable, Lockable { store.payOracleFeesErc20(address(collateralCurrency), amount); } - /** - * @notice Gets the current profit from corruption for this contract in terms of the collateral currency. - * @dev This will be equivalent to the collateral pool available from which to pay fees. - * Therefore, derived contracts are expected to implement this so that pay-fee methods - * can correctly compute the owed fees as a % of PfC. - */ - function pfc() public view nonReentrantView() returns (FixedPoint.Unsigned memory) { - return _pfc(); - } - - /**************************************** - * INTERNAL FUNCTIONS * - ****************************************/ - function _pfc() internal virtual view returns (FixedPoint.Unsigned memory); function _getStore() internal view returns (StoreInterface) { @@ -207,9 +210,8 @@ abstract contract FeePayer is Testable, Lockable { return rawCollateral.mul(cumulativeFeeMultiplier); } - // Converts a user-readable collateral value into a raw value that accounts for already-assessed - // fees. If any fees have been taken from this contract in the past, then the raw value will be - // larger than the user-readable value. + // Converts a user-readable collateral value into a raw value that accounts for already-assessed fees. If any fees + // have been taken from this contract in the past, then the raw value will be larger than the user-readable value. function _convertToRawCollateral(FixedPoint.Unsigned memory collateral) internal view diff --git a/core/contracts/financial-templates/common/SyntheticToken.sol b/core/contracts/financial-templates/common/SyntheticToken.sol index ed0aa2da4c..e1e944a75a 100644 --- a/core/contracts/financial-templates/common/SyntheticToken.sol +++ b/core/contracts/financial-templates/common/SyntheticToken.sol @@ -5,8 +5,7 @@ import "../../common/implementation/Lockable.sol"; /** * @title Burnable and mintable ERC20. - * @dev The contract deployer will initially be the only minter and burner as - * well as the owner who is capable of adding new roles. + * @dev The contract deployer will initially be the only minter, burner and owner capable of adding new roles. */ contract SyntheticToken is ExpandedERC20, Lockable { diff --git a/core/contracts/financial-templates/expiring-multiparty/ExpiringMultiPartyCreator.sol b/core/contracts/financial-templates/expiring-multiparty/ExpiringMultiPartyCreator.sol index 0957f1ea25..6297ec73ed 100644 --- a/core/contracts/financial-templates/expiring-multiparty/ExpiringMultiPartyCreator.sol +++ b/core/contracts/financial-templates/expiring-multiparty/ExpiringMultiPartyCreator.sol @@ -11,7 +11,12 @@ import "./ExpiringMultiPartyLib.sol"; /** * @title Expiring Multi Party Contract creator. * @notice Factory contract to create and register new instances of expiring multiparty contracts. - * Responsible for constraining the parameters used to construct a new EMP. + * Responsible for constraining the parameters used to construct a new EMP. This creator contains a number of constraints + * that are applied to newly created expiring multi party contract. These constraints can evolve over time and are + * initially constrained to conservative values in this first iteration. Technically there is nothing in the + * ExpiringMultiParty contract requiring these constraints. However, because `createExpiringMultiParty()` is intended + * to be the only way to create valid financial contracts that are registered with the DVM (via _registerContract), + we can enforce deployment configurations here. */ contract ExpiringMultiPartyCreator is ContractCreator, Testable, Lockable { using FixedPoint for FixedPoint.Unsigned; @@ -32,22 +37,11 @@ contract ExpiringMultiPartyCreator is ContractCreator, Testable, Lockable { FixedPoint.Unsigned disputerDisputeRewardPct; FixedPoint.Unsigned minSponsorTokens; } - - /** - * @notice Deployment Configuration Constraints. - * @dev: These constraints can evolve over time and are initially constrained to conservative values - * in this first iteration of an EMP creator. Technically there is nothing in the ExpiringMultiParty - * contract requiring these constraints. However, because `createExpiringMultiParty()` is intended to - * be the only way to create valid financial contracts that are **registered** with the - * DVM (via `_registerContract()`), we can enforce deployment configurations here. - **/ - // - Whitelist allowed collateral currencies. // Note: before an instantiation of ExpiringMultipartyCreator is approved to register contracts, voters should - // ensure that the ownership of this collateralTokenWhitelist has been renounced (so it is effectively - // frozen). One could also set the owner to the address of the Governor contract, but voters may find that option - // less preferable since it would force them to take a more active role in managing this financial contract - // template. + // ensure that the ownership of this collateralTokenWhitelist has been renounced (so it is effectively frozen). One + // could also set the owner to the address of the Governor contract, but voters may find that option less preferable + // since it would force them to take a more active role in managing this financial contract template. AddressWhitelist public collateralTokenWhitelist; // - Address of TokenFactory to pass into newly constructed ExpiringMultiParty contracts address public tokenFactoryAddress; @@ -109,7 +103,7 @@ contract ExpiringMultiPartyCreator is ContractCreator, Testable, Lockable { /** * @notice Creates an instance of expiring multi party and registers it within the registry. * @param params is a `ConstructorParams` object from ExpiringMultiParty. - * @return address of the deployed ExpiringMultiParty contract + * @return address of the deployed ExpiringMultiParty contract. */ function createExpiringMultiParty(Params memory params) public nonReentrant() returns (address) { address derivative = ExpiringMultiPartyLib.deploy(_convertParams(params)); diff --git a/core/contracts/financial-templates/expiring-multiparty/Liquidatable.sol b/core/contracts/financial-templates/expiring-multiparty/Liquidatable.sol index 899d4c3291..c68904a73f 100644 --- a/core/contracts/financial-templates/expiring-multiparty/Liquidatable.sol +++ b/core/contracts/financial-templates/expiring-multiparty/Liquidatable.sol @@ -4,9 +4,10 @@ pragma experimental ABIEncoderV2; import "@openzeppelin/contracts/math/SafeMath.sol"; import "@openzeppelin/contracts/token/ERC20/SafeERC20.sol"; -import "../../common/implementation/FixedPoint.sol"; import "./PricelessPositionManager.sol"; +import "../../common/implementation/FixedPoint.sol"; + /** * @title Liquidatable @@ -78,9 +79,8 @@ contract Liquidatable is PricelessPositionManager { // Total collateral in liquidation. FixedPoint.Unsigned public rawLiquidationCollateral; - // Immutable contract parameters. - - // Amount of time for pending liquidation before expiry + // Immutable contract parameters: + // Amount of time for pending liquidation before expiry. uint256 public liquidationLiveness; // Required collateral:TRV ratio for a position to be considered sufficiently collateralized. FixedPoint.Unsigned public collateralRequirement; @@ -88,10 +88,10 @@ contract Liquidatable is PricelessPositionManager { // Represented as a multiplier, for example 1.5e18 = "150%" and 0.05e18 = "5%" FixedPoint.Unsigned public disputeBondPct; // Percent of oraclePrice paid to sponsor in the Disputed state (i.e. following a successful dispute) - // Represented as a multiplier, see above + // Represented as a multiplier, see above. FixedPoint.Unsigned public sponsorDisputeRewardPct; // Percent of oraclePrice paid to disputer in the Disputed state (i.e. following a successful dispute) - // Represented as a multiplier, see above + // Represented as a multiplier, see above. FixedPoint.Unsigned public disputerDisputeRewardPct; /**************************************** @@ -181,7 +181,7 @@ contract Liquidatable is PricelessPositionManager { * synthetic tokens to retire the position's outstanding tokens. * @dev This method generates an ID that will uniquely identify liquidation for the sponsor. This contract must be * approved to spend at least `tokensLiquidated` of `tokenCurrency` and at least `finalFeeBond` of `collateralCurrency`. - * @param sponsor address to liquidate. + * @param sponsor address of the sponsor to liquidate. * @param minCollateralPerToken abort the liquidation if the position's collateral per token is below this value. * @param maxCollateralPerToken abort the liquidation if the position's collateral per token exceeds this value. * @param maxTokensToLiquidate max number of tokens to liquidate. @@ -213,8 +213,8 @@ contract Liquidatable is PricelessPositionManager { tokensLiquidated = FixedPoint.min(maxTokensToLiquidate, positionToLiquidate.tokensOutstanding); - // Starting values for the Position being liquidated. - // If withdrawal request amount is > position's collateral, then set this to 0, otherwise set it to (startCollateral - withdrawal request amount). + // Starting values for the Position being liquidated. If withdrawal request amount is > position's collateral, + // then set this to 0, otherwise set it to (startCollateral - withdrawal request amount). FixedPoint.Unsigned memory startCollateral = _getFeeAdjustedCollateral(positionToLiquidate.rawCollateral); FixedPoint.Unsigned memory startCollateralNetOfWithdrawal = FixedPoint.fromUnscaledUint(0); if (positionToLiquidate.withdrawalRequestAmount.isLessThanOrEqual(startCollateral)) { @@ -268,9 +268,8 @@ contract Liquidatable is PricelessPositionManager { _addCollateral(rawLiquidationCollateral, lockedCollateral.add(finalFeeBond)); // Construct liquidation object. - // Note: all dispute-related values are just zeroed out until a dispute occurs. - // liquidationId is the index of the new LiquidationData that we will push into the array, - // which is equal to the current length of the array pre-push. + // Note: All dispute-related values are zeroed out until a dispute occurs. liquidationId is the index of the new + // LiquidationData that is pushed into the array, which is equal to the current length of the array pre-push. liquidationId = liquidations[sponsor].length; liquidations[sponsor].push( LiquidationData({ @@ -329,8 +328,7 @@ contract Liquidatable is PricelessPositionManager { ); _addCollateral(rawLiquidationCollateral, disputeBondAmount); - // Request a price from DVM, - // Liquidation is pending dispute until DVM returns a price + // Request a price from DVM. Liquidation is pending dispute until DVM returns a price. disputedLiquidation.state = Status.PendingDispute; disputedLiquidation.disputer = msg.sender; @@ -344,12 +342,12 @@ contract Liquidatable is PricelessPositionManager { liquidationId, disputeBondAmount.rawValue ); - totalPaid = disputeBondAmount.add(disputedLiquidation.finalFee); - // Pay a final fee. + // Pay the final fee for requesting price from the DVM. _payFinalFees(msg.sender, disputedLiquidation.finalFee); + // Transfer the dispute bond amount from the caller to this contract. collateralCurrency.safeTransferFrom(msg.sender, address(this), disputeBondAmount.rawValue); } @@ -357,7 +355,7 @@ contract Liquidatable is PricelessPositionManager { * @notice After a dispute has settled or after a non-disputed liquidation has expired, * the sponsor, liquidator, and/or disputer can call this method to receive payments. * @dev If the dispute SUCCEEDED: the sponsor, liquidator, and disputer are eligible for payment. - * If the dispute FAILED: only the liquidator can receive payment + * If the dispute FAILED: only the liquidator can receive payment. * Once all collateral is withdrawn, delete the liquidation data. * @param liquidationId uniquely identifies the sponsor's liquidation. * @param sponsor address of the sponsor associated with the liquidation. @@ -378,12 +376,11 @@ contract Liquidatable is PricelessPositionManager { "Caller cannot withdraw rewards" ); - // Settles the liquidation if necessary. - // Note: this will fail if the price has not resolved yet. + // Settles the liquidation if necessary. This call will revert if the price has not resolved yet. _settle(liquidationId, sponsor); - // Calculate rewards as a function of the TRV. Note: all payouts are scaled by the unit collateral value so - // all payouts are charged the fees pro rata. + // Calculate rewards as a function of the TRV. + // Note: all payouts are scaled by the unit collateral value so all payouts are charged the fees pro rata. FixedPoint.Unsigned memory feeAttenuation = _getFeeAdjustedCollateral(liquidation.rawUnitCollateral); FixedPoint.Unsigned memory tokenRedemptionValue = liquidation .tokensOutstanding @@ -421,7 +418,7 @@ contract Liquidatable is PricelessPositionManager { // Pay LIQUIDATOR: TRV - dispute reward - sponsor reward // If TRV > Collateral, then subtract rewards from collateral // NOTE: This should never be below zero since we prevent (sponsorDisputePct+disputerDisputePct) >= 0 in - // the constructor when these params are set + // the constructor when these params are set. FixedPoint.Unsigned memory payToLiquidator = tokenRedemptionValue.sub(sponsorDisputeReward).sub( disputerDisputeReward ); @@ -429,7 +426,7 @@ contract Liquidatable is PricelessPositionManager { delete liquidation.liquidator; } - // Free up space once all collateral is withdrawn + // Free up space once all collateral is withdrawn by removing the liquidation object from the array. if ( liquidation.disputer == address(0) && liquidation.sponsor == address(0) && @@ -449,16 +446,30 @@ contract Liquidatable is PricelessPositionManager { withdrawalAmount = collateral.add(finalFee); delete liquidations[sponsor][liquidationId]; } - require(withdrawalAmount.isGreaterThan(0), "Invalid withdrawal amount"); + + // Decrease the total collateral held in liquidatable by the amount withdrawn. amountWithdrawn = _removeCollateral(rawLiquidationCollateral, withdrawalAmount); emit LiquidationWithdrawn(msg.sender, amountWithdrawn.rawValue, liquidation.state); + // Transfer amount withdrawn from this contract to the caller. collateralCurrency.safeTransfer(msg.sender, amountWithdrawn.rawValue); + + return amountWithdrawn; } - function getLiquidations(address sponsor) external view nonReentrantView() returns (LiquidationData[] memory) { + /** + * @notice Gets all liquidation information for a given sponsor address. + * @param sponsor address of the position sponsor. + * @return liquidationData array of all liquidation information for the given sponsor address. + */ + function getLiquidations(address sponsor) + external + view + nonReentrantView() + returns (LiquidationData[] memory liquidationData) + { return liquidations[sponsor]; } diff --git a/core/contracts/financial-templates/expiring-multiparty/PricelessPositionManager.sol b/core/contracts/financial-templates/expiring-multiparty/PricelessPositionManager.sol index 98a3129a86..2be6fa2d2d 100644 --- a/core/contracts/financial-templates/expiring-multiparty/PricelessPositionManager.sol +++ b/core/contracts/financial-templates/expiring-multiparty/PricelessPositionManager.sol @@ -33,8 +33,7 @@ contract PricelessPositionManager is FeePayer, AdministrateeInterface { * PRICELESS POSITION DATA STRUCTURES * ****************************************/ - // Enum to store the state of the PricelessPositionManager. Set on expiration, emergency shutdown, or initial - // settlement. + // Stores the state of the PricelessPositionManager. Set on expiration, emergency shutdown, or settlement. enum ContractState { Open, ExpiredPriceRequested, ExpiredPriceReceived } ContractState public contractState; @@ -281,9 +280,8 @@ contract PricelessPositionManager is FeePayer, AdministrateeInterface { /** * @notice Transfers `collateralAmount` of `collateralCurrency` from the sponsor's position to the sponsor. - * @dev Reverts if the withdrawal puts this position's collateralization ratio below the global - * collateralization ratio. In that case, use `requestWithdrawal`. Might not withdraw the full requested - * amount in order to account for precision loss. + * @dev Reverts if the withdrawal puts this position's collateralization ratio below the global collateralization + * ratio. In that case, use `requestWithdrawal`. Might not withdraw the full requested amount to account for precision loss. * @param collateralAmount is the amount of collateral to withdraw. * @return amountWithdrawn The actual amount of collateral withdrawn. */ @@ -305,15 +303,14 @@ contract PricelessPositionManager is FeePayer, AdministrateeInterface { emit Withdrawal(msg.sender, amountWithdrawn.rawValue); // Move collateral currency from contract to sender. - // Note that we move the amount of collateral that is decreased from rawCollateral (inclusive of fees) + // Note: that we move the amount of collateral that is decreased from rawCollateral (inclusive of fees) // instead of the user requested amount. This eliminates precision loss that could occur // where the user withdraws more collateral than rawCollateral is decremented by. collateralCurrency.safeTransfer(msg.sender, amountWithdrawn.rawValue); } /** - * @notice Starts a withdrawal request that, if passed, allows the sponsor to withdraw - * `collateralAmount` from their position. + * @notice Starts a withdrawal request that, if passed, allows the sponsor to withdraw` from their position. * @dev The request will be pending for `withdrawalLiveness`, during which the position can be liquidated. * @param collateralAmount the amount of collateral requested to withdraw */ @@ -480,12 +477,11 @@ contract PricelessPositionManager is FeePayer, AdministrateeInterface { } /** - * @notice After a contract has passed expiry all token holders can redeem their tokens for - * underlying at the prevailing price defined by the DVM from the `expire` function. - * @dev This burns all tokens from the caller of `tokenCurrency` and sends back the proportional - * amount of `collateralCurrency`. Might not redeem the full proportional amount of collateral - * in order to account for precision loss. This contract must be approved to spend `tokenCurrency` at least up to the - * caller's full balance. + * @notice After a contract has passed expiry all token holders can redeem their tokens for underlying at the + * prevailing price defined by the DVM from the `expire` function. + * @dev This burns all tokens from the caller of `tokenCurrency` and sends back the proportional amount of + * `collateralCurrency`. Might not redeem the full proportional amount of collateral in order to account for + * precision loss. This contract must be approved to spend `tokenCurrency` at least up to the caller's full balance. * @return amountWithdrawn The actual amount of collateral withdrawn. */ function settleExpired() @@ -555,8 +551,7 @@ contract PricelessPositionManager is FeePayer, AdministrateeInterface { /** * @notice Locks contract state in expired and requests oracle price. - * @dev this function can only be called once the contract is expired and cant be re-called - * due to the state modifiers applied on it. + * @dev this function can only be called once the contract is expired and can't be re-called. */ function expire() external onlyPostExpiration() onlyOpenState() fees() nonReentrant() { contractState = ContractState.ExpiredPriceRequested; @@ -598,16 +593,28 @@ contract PricelessPositionManager is FeePayer, AdministrateeInterface { * @dev This is necessary because the struct returned by the positions() method shows * rawCollateral, which isn't a user-readable value. * @param sponsor address whose collateral amount is retrieved. + * @return collateralAmount amount of collateral within a sponsors position. */ - function getCollateral(address sponsor) external view nonReentrantView() returns (FixedPoint.Unsigned memory) { + function getCollateral(address sponsor) + external + view + nonReentrantView() + returns (FixedPoint.Unsigned memory collateralAmount) + { // Note: do a direct access to avoid the validity check. return _getFeeAdjustedCollateral(positions[sponsor].rawCollateral); } /** * @notice Accessor method for the total collateral stored within the PricelessPositionManager. + * @return totalCollateral amount of all collateral within the Expiring Multi Party Contract. */ - function totalPositionCollateral() external view nonReentrantView() returns (FixedPoint.Unsigned memory) { + function totalPositionCollateral() + external + view + nonReentrantView() + returns (FixedPoint.Unsigned memory totalCollateral) + { return _getFeeAdjustedCollateral(rawTotalPositionCollateral); } @@ -714,7 +721,7 @@ contract PricelessPositionManager is FeePayer, AdministrateeInterface { return FixedPoint.Unsigned(uint256(oraclePrice)); } - // Reset withdrawal request by setting the withdrawl request and withdrawl timestamp to 0. + // Reset withdrawal request by setting the withdrawal request and withdrawal timestamp to 0. function _resetWithdrawalRequest(PositionData storage positionData) internal { positionData.withdrawalRequestAmount = FixedPoint.fromUnscaledUint(0); positionData.requestPassTimestamp = 0; @@ -775,9 +782,9 @@ contract PricelessPositionManager is FeePayer, AdministrateeInterface { ); } - // Note: This checks whether an already existing position has a pending withdrawal. This cannot be used on the `create` method - // because it is possible that `create` is called on a new position (i.e. one without any collateral or tokens outstanding) which would fail - // the `onlyCollateralizedPosition` modifier on `_getPositionData`. + // Note: This checks whether an already existing position has a pending withdrawal. This cannot be used on the + // `create` method because it is possible that `create` is called on a new position (i.e. one without any collateral + // or tokens outstanding) which would fail the `onlyCollateralizedPosition` modifier on `_getPositionData`. function _positionHasNoPendingWithdrawal(address sponsor) internal view { require(_getPositionData(sponsor).requestPassTimestamp == 0, "Pending withdrawal"); }