sidestream-tech / unified-auctions-ui

Unified MakerDAO auctions
https://unified-auctions.makerdao.com
GNU Affero General Public License v3.0
16 stars 13 forks source link

Outline current state of the collateral simulations #501

Open valiafetisov opened 2 years ago

valiafetisov commented 2 years ago

Goal

List of supported and unsupported collaterals, list of potential fixes

Context

Since we're working on the collateral auctions simulations for quite some time already and faced different kinds of problems, we need to summarise somewhere its state and come up with a list of solutions to move forward and prioritise them. Therefore, let's summarise it here or even create/modify test script that will do it for us.

Assets

Issues where simulations were previously developed and improved:

Previous state: https://github.com/sidestream-tech/unified-auctions-ui/pull/479#issuecomment-1270594143

Tasks

valiafetisov commented 2 years ago
Collateral type State Reason
ETH-A ✅ supported
ETH-A ✅ supported
ETH-B ✅ supported
ETH-C ✅ supported
LINK-A ✅ supported
MANA-A ✅ supported
RENBTC-A ✅ supported
WBTC-A ✅ supported
WBTC-B ✅ supported
WBTC-C ✅ supported
MATIC-A ✅ supported
WSTETH-A ✅ supported
UNIV2USDCETH-A ✅ supported
AAVE-A ⚠️ not testable MCD_VAT.ilks().line == 0 Max debt set to zero
BAL-A ⚠️ not testable MCD_VAT.ilks().line == 0 Max debt set to zero
BAT-A ⚠️ not testable MCD_VAT.ilks().line == 0 Max debt set to zero
COMP-A ⚠️ not testable MCD_VAT.ilks().line == 0 Max debt set to zero
GUSD-A ⚠️ not testable MCD_VAT.ilks().line == 0 Max debt set to zero
KNC-A ⚠️ not testable MCD_VAT.ilks().line == 0 Max debt set to zero
LRC-A ⚠️ not testable MCD_VAT.ilks().line == 0 Max debt set to zero
PAXUSD-A ⚠️ not testable MCD_VAT.ilks().line == 0 Max debt set to zero
TUSD-A ⚠️ not testable MCD_VAT.ilks().line == 0 Max debt set to zero
UNI-A ⚠️ not testable MCD_VAT.ilks().line == 0 Max debt set to zero
USDC-A ⚠️ not testable MCD_VAT.ilks().line == 0 Max debt set to zero
USDC-B ⚠️ not testable MCD_VAT.ilks().line == 0 Max debt set to zero
USDT-A ⚠️ not testable MCD_VAT.ilks().line == 0 Max debt set to zero
ZRX-A ⚠️ not testable MCD_VAT.ilks().line == 0 Max debt set to zero
UNIV2DAIETH-A ⚠️ not testable MCD_VAT.ilks().line == 0 Max debt set to zero
UNIV2ETHUSDT-A ⚠️ not testable MCD_VAT.ilks().line == 0 Max debt set to zero
UNIV2WBTCDAI-A ⚠️ not testable MCD_VAT.ilks().line == 0 Max debt set to zero
UNIV2WBTCETH-A ⚠️ not testable MCD_VAT.ilks().line == 0 Max debt set to zero
UNIV2LINKETH-A ⚠️ not testable MCD_VAT.ilks().line == 0 Max debt set to zero
UNIV2UNIETH-A ⚠️ not testable MCD_VAT.ilks().line == 0 Max debt set to zero
UNIV2AAVEETH-A ⚠️ not testable MCD_VAT.ilks().line == 0 Max debt set to zero
UNIV2DAIUSDT-A ⚠️ not testable MCD_VAT.ilks().line == 0 Max debt set to zero
YFI-A ⚠️ not testable Vat/ceiling-exceeded Error during vault opening https://github.com/sidestream-tech/unified-auctions-ui/pull/500
CRVV1ETHSTETH-A ❌ not supported Lacking programmatic mean to open this type of vault. Opening is done via different mechanism based on cropper
UNIV2DAIUSDC-A ❌ not supported Dog/liquidation-limit-hit Liquidation limit too high
WSTETH-B ❌ not supported Dog/not-unsafe: Successful vault creation, but stability fees did not change after time warp
RETH-* ❌ not supported Join contract does not have enough collateral. Will be addressed in https://github.com/sidestream-tech/unified-auctions-ui/issues/495
valiafetisov commented 2 years ago

Solutions

LukSteib commented 2 years ago

MCD_VAT.ilks().line == 0 as well as Vat/ceiling-exceeded I assume can be fixed by setting MCD_VAT.line via calling file method (impersonating proper account)

One remark here: As this relates to all of the collateral types that have been offboarded (also the reason why MCD_VAT.ilks().line == 0) let's be mindful with our resources and not trying to make these work for now.

KirillDogadin-std commented 2 years ago

Dog/not-unsafe

same: impersonate, call file on jug.sol contract

propose a solution here?

See https://github.com/sidestream-tech/unified-auctions-ui/issues/503

KirillDogadin-std commented 2 years ago

Created #503, #504, #505 as sub-issues

valiafetisov commented 2 years ago

Thanks! Please link them to the appropriate points in https://github.com/sidestream-tech/unified-auctions-ui/issues/501#issuecomment-1282146224

From my perspective, CRVV1ETHSTETH-A support is quite important, as it will allow us to do long-outstanding https://github.com/sidestream-tech/unified-auctions-ui/issues/376. But maybe find and liquidate existing vault is more suitable approach here? Can you please compare implementation complexity of the two and maybe even propose a third one?

KirillDogadin-std commented 2 years ago

Sorry, i don't understand the things that you're asking to compare.

The first one i assume is #503 as it is. The second is "find&liquidate existing vault". I either don't see the common line/ground there, or i did not get one/both of the compared items correctly

valiafetisov commented 2 years ago

Sorry, i don't understand the things that you're asking to compare

The comment with solutions specifies two options to support CRVV1ETHSTETH-A, a) and b). #503 covers option b), not both of them. There are 3 questions to you: 1) Can you please outline option a)? 2) Can you please propose another options? 3) Can you please compare available options (at least a) with b)) in terms of complexity/effort?

Edited the comment you've linked

Please edit the links so they correspond to specific solutions, not problems, since there can be many solutions per problem

KirillDogadin-std commented 2 years ago

Option (a) is outlined for CRVV1ETHSTETH-A in the linked issue. Option (b) would be heavily dependant on the mechanism that we should have by the end of the current global scope (i.e. ideally having the api at hand that we can use to query for vaults of specific type). For now it seems like the solution would be to:

  1. iterate over every vault that CDP-MANAGER is aware of, use ilks to get the vault type
  2. Collect the vaults of the required type, figure out the vault address (since it's proxied address we need additional steps):
    • query the owner of the vault in CDP-MANAGER (it must redirect to the registry)
    • get the owner of the vault in the CDP-REGISTRY (it must be a ds-proxy constract)
    • filter the events in the CROPPER contract: NewProxy is the name, the vault owner (ds-proxy) address is the parameter to filter by.
    • impersonate the ds-proxy's owner account, draw debt from the vault via calling frob function in the CROPPER contract via this proxy
      • if needed, overwrite ds-proxy's owner balance via the existing functionality so it has more CRVV1ETHSTETH-A token
    • pass time (risk factor: if at some point the stability fees are 0, we have to be able to file the new values.)
    • liquidate.

the listed option seems like an easier thing to implement at first glance. The deal-breaker here is the fact that there's no need to deploy a ds-proxy contract into the fork.

KirillDogadin-std commented 2 years ago

adding the support for CRVV1ETHSTETH-A in #510

KirillDogadin-std commented 2 years ago

Summary of currently known proxies that are used with the supported collaterals one way or another:

  1. REN-BTC

a proxy-based token. This means that the logic of this token is specified in another contract, while the information is stored in the token(proxy) contract.

wallet --function_call--> REN-BTC --delegatecall--> REN-BTC-LOGIC
                      [info storage]               [logic storage]
  1. CRVV1ETHSTETH-A

a usual token. But the vault is proxied.

The vault can be created and instantly drawn from if one uses DSS-PROXY-ACTIONS type of contract (e.g. 0xa2f69F8B9B341CFE9BfBb3aaB5fe116C89C95bAF). One has to use DS-PROXY when making a call to it for security/safety reasons. Under the hood the following happens:


wallet -call-> DS-PROXY -call-> DSS-PROXY-ACTIIONS-CROPPER -> 1. CDP-REGISTRY -> CDP-MANAGER
                                                           -> 2. CROPPER-PROXY -delegatecall-> CROPPER-IMPLEMENTATION
KirillDogadin-std commented 2 years ago

Additionally, GUSD (gemini) token now is hidden behind the proxy. This means that we have two cases that require to fallback to increasing vat balance currently (instead of increasing wallet balnce) during the simulations:

  1. REN-BTC
  2. GUSD

    Updated https://github.com/sidestream-tech/unified-auctions-ui/issues/501#issuecomment-1282146224

KirillDogadin-std commented 2 years ago

Removing the fallback of increasing vat-balance instead of wallet for proxy-based tokens seems trickier than expected. There seems to be no consistent layout of where the data is stored. For instance, the G-USD token has a separate storage contract that is referenced by the logic one. But sadly the same layout does not appear to be present for ren-btc. Which essentially means that it's not exactly clear what's the uniform way of overwriting storage of the proxy-contract.

As of "what if it just works" - running balance slot discovery on the GUSD and RENBTC did not work out for both cases: overwriting the logic contract and the token contract/