Closed SeanJCasey closed 4 years ago
@travs For the Registry, we also could be setting more state in the constructor (mlnToken, nativeAsset, assets, derivatives, etc). Is there a reason we prefer for the Council to do this rather than including it in deployment? If not, we can add those in there.
@travs Another note re: the way Shares
uses IDerivativePriceSource
: getPrice
is just a simple first implementation, because derivatives can have a basket of underlying assets (e.g., Uniswap LPs, Balancer pool tokens, etc). For those, we will actually need to refactor this function to something like getUnderlyingAssetRates
, and then loop through each of the return values to add to the gav.
It gets even trickier when you consider that these underlying assets can themselves be derivative assets (e.g., a Uniswap LP with cDai).
Anyways, I thought that it would be better to just start with the simplified case of 1 underlying asset.
Addresses issue (epic) #971 .
Also heavily refactors Registry to make it more abstract, which should help reduce the need to upgrade a Registry between fund factory, adapter, price feed, and other functionality releases.
This PR also contains the basic implementation of price calculations for "derivative" (as in cTokens and Chai) assets. This includes:
derivativeToPriceSource
mapping on the RegistryIDerivativePriceSource.sol
interface. This will be the interface inherited by all derivative price source oraclesIDerivativePriceSource
in gav calcuation in Shares^This is all abstract at this stage, and a concrete implementation will come next with the Chai integration PR
Some notes about changes to various contracts:
Registry
reserveMin
stuff fromRegistry
, shifting to what is hopefully better logic directly on KyberPriceFeedRegistry
(Registry
only needs to store the bytes32 fund name hash map to check for uniqueness).integrationTypes
as an array of strings, e.g.,['trading', 'lending']
. I did this to simulate an enum without limiting the options (so we add a new type to the registry later without re-deploying the contract)MAX_REGISTERED_ENTITIES
. I don't think this makes sense, especially not as a constant, since we can just remove entities if there is a problem with price updates (the updater needs to work with batches of assets also, which will be a task in the next release)Shares
calcGav
into a loop using a newcalcAssetGav
function, which conditionally uses the Kyber price feed or derivative price feedFundFactory
KyberPriceFeed
minReserve
forgetExpectedRate()
ingetKyberPrice()
; use1
instead (discussed in Kyber Discord)Tests