Partial Collateralization & Liquidation #382
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* update OpenVaultArgs; set vault type; add vaultLatestUpdate * fix build * fix tests * fix integration tests * add vault type to event * increase tests coverage * update comment Co-authored-by: aparnakr <aparnalocked@gmail.com> Co-authored-by: aparnakr <aparnalocked@gmail.com>
* update chainlink interface to V3 * add decimals() scaling
* update OpenVaultArgs; set vault type; add vaultLatestUpdate * fix build * fix tests * fix integration tests * add vault type to event * increase tests coverage * update chainlink interface to V3 * add historical price function * fix integration tests * add tests * add more tests * typo Co-authored-by: Anton Cheng <antonassocareer@gmail.com> * add decimals() scaling Co-authored-by: Anton Cheng <antonassocareer@gmail.com>
* update OpenVaultArgs; set vault type; add vaultLatestUpdate * fix build * fix tests * fix integration tests * add vault type to event * increase tests coverage * update chainlink interface to V3 * add historical price function * fix integration tests * add tests * add more tests * add sync function * just checking that vault is not under water at settling, and boom encountered stack tooo deep :p * change variables scope * add vault type to getVault() * fix tests to work with the getVault() modification * update getVault() to return latestUpdateTimestamp; add test for sync() * add owner functions * fix typo * set oracle deviation value * fix build * add vaultType to getExcessCollateral * check margin if naked vault and not expired yet * fix tests * add _findUpperBoundValue() * add getter functions * add test coverage * improve coverage * typo Co-authored-by: Anton Cheng <antonassocareer@gmail.com> * add decimals() scaling * re-order functions; increase coverage * _getNakedMarginRequired() * _getNakedMarginRequired() test * re-run CI * update package-lock.json * lint contracts * lint test files * improve coverage * more margin required tests * more margin required tests * fix bug * call tests * improve comment * add view function to get required margin for specific product * add backward compatibility for openVault action * more tests * refactor _getProductHash * refactor * test settling undercollateralized vault * tests for removing collateral from expired positions * updated harness * remove console.log() * clean Co-authored-by: Anton Cheng <antonassocareer@gmail.com> Co-authored-by: Shelly Grossman <shellygr151@gmail.com>
* update OpenVaultArgs; set vault type; add vaultLatestUpdate * fix build * fix tests * fix integration tests * add vault type to event * increase tests coverage * update chainlink interface to V3 * add historical price function * fix integration tests * add tests * add more tests * add sync function * just checking that vault is not under water at settling, and boom encountered stack tooo deep :p * change variables scope * add vault type to getVault() * fix tests to work with the getVault() modification * update getVault() to return latestUpdateTimestamp; add test for sync() * add owner functions * fix typo * set oracle deviation value * fix build * add vaultType to getExcessCollateral * check margin if naked vault and not expired yet * fix tests * add _findUpperBoundValue() * add getter functions * add test coverage * improve coverage * typo Co-authored-by: Anton Cheng <antonassocareer@gmail.com> * add decimals() scaling * re-order functions; increase coverage * _getNakedMarginRequired() * _getNakedMarginRequired() test * re-run CI * update package-lock.json * lint contracts * lint test files * improve coverage * more margin required tests * more margin required tests * fix bug * call tests * improve comment * add view function to get required margin for specific product * add backward compatibility for openVault action * more tests * refactor _getProductHash * refactor * test settling undercollateralized vault * add liquidate action args * liquidate action args tests * get liquidation price; check if vault is liquidatable * add comments * init tests * liquidate * fix tests * more tests * add comments * liquidation price testing * more tests * change AUCTION_TIME scaling * tests * tests * fix calculation; more tests * fix collateral to sell calculation * full liquidation tests * partially liquidation test * add function description * clear Co-authored-by: Anton Cheng <antonassocareer@gmail.com> * clear Co-authored-by: Anton Cheng <antonassocareer@gmail.com> * clear Co-authored-by: Anton Cheng <antonassocareer@gmail.com> * clear Co-authored-by: Anton Cheng <antonassocareer@gmail.com> * clear Co-authored-by: Anton Cheng <antonassocareer@gmail.com> * clear Co-authored-by: Anton Cheng <antonassocareer@gmail.com> * rename var; refactor util function * natspec comments * clean tests * require dust amount is greater than zero * set bignumber config * fix coverage * update circleci config * fix tests * clean code * clean Co-authored-by: Anton Cheng <antonassocareer@gmail.com> * clean * fix build * add requirement for spot shock value * fix typo * cap liquidation price * fix capping * fix tests * add tests for call position * add natspec comment * fix timestamp check * fix tests * fix tests Co-authored-by: Anton Cheng <antonassocareer@gmail.com>
aleone
reviewed
Apr 14, 2021
| * @param _owner vault owner address | ||
| * @param _vaultId vault id | ||
| */ | ||
| function sync(address _owner, uint256 _vaultId) external nonReentrant notFullyPaused { |
There was a problem hiding this comment.
Is it useful to have a canSync public function or just do via f/e calls or require devs monitoring via bots to do it themselves?
There was a problem hiding this comment.
what canSync() for ? to check if it is able to sync? if so it will be basically the same as sync() that will check the margin and return bool. We can keep it in the frontend as it can be done by checking the margin state.
There was a problem hiding this comment.
@aleone, it looks like the equivalent of canSync would be calling calculator.getExcessCollateral:
(, bool isValidVault) = calculator.getExcessCollateral(vault, typeVault);I think a separate canSync function could be useful later on, but don't think it's necessary right now (for example, a FE could poll the vault state for a UI).
aleone
reviewed
Apr 14, 2021
contracts/MarginCalculator.sol
Outdated
| /// @dev FixedPoint 0 | ||
| FPI.FixedPointInt internal ZERO = FPI.fromScaledUint(0, BASE); | ||
| /// @notice auction length | ||
| uint256 public constant AUCTION_TIME = 3600; |
There was a problem hiding this comment.
I lean towards this being adjustable. If we start having failed liquidations we may need to adjust this quickly.
I guess we could always deploy a new controller, set all the values and then migrate it in the address book. That would require a bunch of tx, but only one tx to migrate, so we don't have a broken/controller with non finalized values.
Curious other folks thoughts.
There was a problem hiding this comment.
If we do allow it to be set, we could set a min value - admin can prb steal a lot of margined funds by atomically:
-flashloan bunch of eth
-changing margin requirements
-setting auction_time to 0
-liquidating a bunch of folks (they might need a recursive operate function to do this have to mint+liquidate+burn)
-setting margin req back to normal + auction time back to normal
There was a problem hiding this comment.
To update this, we need to redeploy MarginCalculator not the Controller, and yeah it will require more txs but I really prefer keeping this constant, as it is more gas efficient, and still if we have failed liquidation w can still batch deployment and params config in one transaction.
aleone
reviewed
Apr 14, 2021
contracts/MarginCalculator.sol
Outdated
| * @dev can only be called by owner | ||
| * @param _deviation deviation value | ||
| */ | ||
| function setOracleDeviation(uint256 _deviation) external onlyOwner { |
There was a problem hiding this comment.
Might make sense that this should be a mapping per product as chainlink deviations are different between products. COMPUSD is 1% ETHUSD 0.5%, LINKUSD 1%, etc.
We could also just be conservative and use 2% or something.
There was a problem hiding this comment.
What do u think is better ? if keeping it conservative will work, I prefer that rather than changing code now.
aleone
reviewed
Apr 14, 2021
aleone
reviewed
Apr 14, 2021
aleone
reviewed
Apr 14, 2021
aleone
reviewed
Apr 14, 2021
aleone
reviewed
Apr 14, 2021
| @@ -150,7 +166,18 @@ library Actions { | |||
| require(_args.actionType == ActionType.OpenVault, "Actions: can only parse arguments for open vault actions"); | |||
| require(_args.owner != address(0), "Actions: cannot open vault for an invalid account"); | |||
|
|
|||
| return OpenVaultArgs({owner: _args.owner, vaultId: _args.vaultId}); | |||
| // if not _args.data included, vault type will be 0 by default | |||
| uint256 vaultType; | |||
There was a problem hiding this comment.
Like this for non breaking changes!
aleone
reviewed
Apr 14, 2021
aleone
reviewed
Apr 14, 2021
aleone
reviewed
Apr 14, 2021
* update OpenVaultArgs; set vault type; add vaultLatestUpdate * fix build * fix tests * fix integration tests * add vault type to event * increase tests coverage * update chainlink interface to V3 * add historical price function * fix integration tests * add tests * add more tests * add sync function * just checking that vault is not under water at settling, and boom encountered stack tooo deep :p * change variables scope * add vault type to getVault() * fix tests to work with the getVault() modification * update getVault() to return latestUpdateTimestamp; add test for sync() * add owner functions * fix typo * set oracle deviation value * fix build * add vaultType to getExcessCollateral * check margin if naked vault and not expired yet * fix tests * add _findUpperBoundValue() * add getter functions * add test coverage * improve coverage * typo Co-authored-by: Anton Cheng <antonassocareer@gmail.com> * add decimals() scaling * re-order functions; increase coverage * _getNakedMarginRequired() * _getNakedMarginRequired() test * re-run CI * update package-lock.json * lint contracts * lint test files * improve coverage * more margin required tests * more margin required tests * fix bug * call tests * improve comment * add view function to get required margin for specific product * add backward compatibility for openVault action * more tests * refactor _getProductHash * refactor * test settling undercollateralized vault * add liquidate action args * liquidate action args tests * get liquidation price; check if vault is liquidatable * add comments * init tests * liquidate * fix tests * more tests * add comments * liquidation price testing * more tests * change AUCTION_TIME scaling * tests * tests * fix calculation; more tests * fix collateral to sell calculation * full liquidation tests * partially liquidation test * add function description * clear Co-authored-by: Anton Cheng <antonassocareer@gmail.com> * clear Co-authored-by: Anton Cheng <antonassocareer@gmail.com> * clear Co-authored-by: Anton Cheng <antonassocareer@gmail.com> * clear Co-authored-by: Anton Cheng <antonassocareer@gmail.com> * clear Co-authored-by: Anton Cheng <antonassocareer@gmail.com> * clear Co-authored-by: Anton Cheng <antonassocareer@gmail.com> * rename var; refactor util function * natspec comments * clean tests * require dust amount is greater than zero * set bignumber config * fix coverage * update circleci config * fix tests * clean code * clean Co-authored-by: Anton Cheng <antonassocareer@gmail.com> * clean * fix build * add requirement for spot shock value * fix typo * cap liquidation price * fix capping * fix tests * add tests for call position * clean * add natspec comment * add natspec comment * init integration tests * more tests * fix timestamp check * more tests * fix tests * fix tests * more tests * init call integration tests * more tests * fix tests * more tests * add more tests for put and call naked margin positions * chore: undo vault type changes * fix tests * fix unit tests * fix integration tests for call Co-authored-by: Anton Cheng <antonassocareer@gmail.com>
antoncoding
previously approved these changes
Apr 22, 2021
haythemsellami
changed the title
…BoundValues() (#385) * add setUpperBoundValues() * update tests * fix unit tests * fix integration tests * integration-tests:fix collateral check after liquidation * assert liquidation price is greater than zero when testing liquidation scenarios * clean * unit tests:setUpperBoundValues() * improve margin calculator coverage * fix integration tests * updateUpperBoundValue() * improve updateUpperBoundValue() coverage
* improve comments * Improve comment in Controller * fix build failing * remove unused code
* improve comments * Improve comment in Controller * fix build failing * remove LiquidationStatus struct * revert package.json change * Fix wrong sign * improve comments * Improve comment in Controller * fix build failing * remove unused code * remove LiquidationStatus struct * revert package.json change * Fix wrong sign Co-authored-by: Haythem Sellami <sellamihaythem96@gmail.com>
* fix tests * update error delta
* refactor marginVault to reduce # of SLOAD * Apply change to addCollateral and removeCollateral * Simplify remove logic * keep the old if block
* add donate() * add tests * fix tests * add function comment
CruzMolina
reviewed
May 5, 2021
CruzMolina
reviewed
May 5, 2021
* undo _withdrawCollateral() changes and update revert messages * update error messages * update tests: * Update error code Co-authored-by: Anton Cheng <antonassocareer@gmail.com>
antoncoding
approved these changes
May 6, 2021
* add external _getMarginRequired() * update getMarginRequired()
haythemsellami
commented
May 6, 2021
haythemsellami
commented
May 6, 2021
| ) | ||
| { | ||
| (, bool isUnderCollat, uint256 price, uint256 dust) = _isLiquidatable(_owner, _vaultId, _roundId); | ||
| return (isUnderCollat, price, dust); |
There was a problem hiding this comment.
Here I used isUnderCollat instead of isLiquidatable to avoid the compiler warning re shadowing vars
* fix yearn pricer to work with the new OpynPricerInterface * update revert message Co-authored-by: Anton Cheng <antonassocareer@gmail.com> Co-authored-by: Anton Cheng <antonassocareer@gmail.com>
aparnakr
approved these changes
May 6, 2021
* refactor: productToTimeToExpiry -> timesToExpiryForProduct * refactor: ProductTimeToExpiryAdded -> TimeToExpiryAdded * refactor: getProductTimeToExpiry -> getTimesToExpiry * refactor: timeToExpiryToMaxPrice -> maxPriceAtTimeToExpiry * refactor: TimeToExpiryValueAdded -> MaxPriceAdded * refactor: getTimeToExpiryValue -> getMaxPrice * docs: misc spotShock mapping comment update
* fix: contracts path * fix harness contracts path
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Gamma Partial Collateralization
High Level Description
Code
Documentation