Add support for CRVV1ETHSTETH collateral vaults.

[KirillDogadin-std]

Goal

Have vault module in core/ be able to operate with collaterals of type CRVV1ETHSTETH just as it does with all the other types (e.g. ETH-A). CRVV1ETHSTETH Vaults should be fetched, listed, mapped into VaultTransaction type. Simulations that involve interaction with collateral vaults should support CRVV1ETHSTETH as well.

Context

CRVV1ETHSTETH is handled differently on the chain. Usually vaults have the id that is handed out by CDP-Manager contract. With this one it is not the case. Which means that there is a different way of interaction with CRVV1ETHSTETH vaults.

Assets

#479 has added the simulations, #460 adds the support for vaults in the core

Tasks

• Investigate how interactions (open , liquidate, etc.) are handled on the chain.
  • report findings in this issue
• Propose a patch
• Implement the patch

[KirillDogadin-std]

22 days ago the following post was made:

#363 (comment)

it claimed that CRVV1ETHSTETH does not have numbers as vault ids.

the linke in the comment is https://tracker-vaults.makerdao.network/collateral/CRVV1ETHSTETH-A

following this link i find that now all the vaults there have their ids 🎉.

opening https://etherscan.io/address/0x5ef30b9986345249bc32d8928b7ee64de9435e39 and using 'Read Contract' tool i've inspected the vault with id 29456. I was able to fetch the owner and the collateral type.

• Seems like the issue has resolved itself and no action required to have this type supported for current and future blocks.
• Therefore the tests and simulations that involve vaults might just have to be run on blocks after the recent one.

I can investigate further about what has changed and what was the case in the past. The hypothesis/speculation would be that the CDP_MANAGER still had the knowledge about this vaults but the vaults tracker was not up-to-date. Instead the tracker listed the vault contract addresses. (each vault id in the cdp manager has the corresponding vault contract that holds funds via VAT). So i would guess that the tracker just listed the vault addresses before the update. Later it updated it and ids have been fetched.

• I think we should check the vault ids on blocks in the past to ensure that the hypothesis is correct

[valiafetisov]

Seems like the issue has resolved itself and no action required

Doesn't seem to be true https://unified-auctions.makerdao.com/vaults?network=mainnet&vault=29456

[LukSteib]

Doesn't seem to be true https://unified-auctions.makerdao.com/vaults?network=mainnet&vault=29456

Able to fetch the vault but currently not fetching its values correctly, right?

[KirillDogadin-std]

Alright, this seems ugly.

The storage of the vault address is done differently. Apparently opening the vault with this collateral absolutely requires a dss-proxy.

• In short, i've taken a look at transaction event log here and found out the following:
  • the user uses the dss-proxy to open the vault
  • the dss-proxy "reserves" the vault in the cdp manager via cdp-registry
    • consequently, the registry contract owns the "vault" in the cdp-manager
  • then funds are managed in the cropper contract and the vault proxy is generated in it
    • dss-proxy owns the vault proxy
    • user vallet owns dss-proxy
  • vault proxy creation is reported as the event.
  • it is possible to build the chain logical chain:
    • if cdp-registry owns the vault in the cdp manger
      • via events of cdp-registry find out which dss-proxy owns this vault
    • if we know the dss-proxy, we can filter the events of the cropper to detect the urn-proxy creation
    • if we know the urn-proxy address, we can look up its funds in the vat.

Todo/unanswered questions

• find out how exactly vault addresses are managed and what is the difference with the other vaults
  • "proxy" term means that the contract that executes the logic borrows the logic form another contract. (contract A exectues function defined in contract B)
  • the vault "open" function is called via the proxy that belongs to the specific wallet (owner is set)
  • the vault is opened via the tooling that is called "cdp-registry". It reserves the address in the actual CDP Manager, the CDP manager 'thinks' that the vault belongs to the registry. Hence all operations are possible through the cdp-registry since it has the rights. Obviously cdp-registry keeps track of who is the real owner - in this case ds-proxy is the real owner. ds-proxy once again keeps track of the owner. The chain becomes like this: CDP MANAGER < CDP REGISTRY < DS PROXY < HUMAN'S WALLET where < means "has access rights"
  • to find the address of the vault's owner one has to:
    • call owns in the cdp-manager, get redirected to the cdp-registry
    • call owns in the cdp-registry, get redirected to the proxy
    • call owner in the proxy, get redirected to the owner
• how is the vault opened via the dss-proxy (what should human do?)
  • use the ds-proxy https://github.com/makerdao/ds-proxy and provide necessary arguments (function name, contract address, arguments)
  • in the current case one can use this contract to open the vault. Seems like the correct function would be openLockGemAndDraw
• how does cropper contract manage the funds of the vault?
  • cropper contract itself is an abstraction mechanism that requires the "implementation" to be specified. It is also acting like a proxy (executes the functions of its implementation)
  • due to the contract being a proxy - it does not store information in public functions. Therefore all of the values of interest are logged as events.
  • seems like the cropper is a modification of join contract (join accepts, dispenses, holds actual funds & lets VAT know that you have added/removed coins). From the description of the cropper on the github page: "This is a collateral adapter for Dai that allows for the distribution of rewards given to holders of the collateral." From the quick look at the code it seems like the CropJoin gives rewards on adding/removing the collateral from/to vat. Have not been able to find full documentation on it at https://docs.makerdao.com.
• does cropper store any valuable information?
  • no, since it's a proxy contract it does not have much information that can be used (only implementation and access rights)
• is there another way to find out the information except for events? (and except for direct memory/slot access)
  • so far, does not seem so
• can one wallet have multiple dss-proxies? can one dss-proxy have multiple vault-proxies?
  • does not seem that there's a mechanism that prevents several proxies with the same owner.
  • the only limitation it seems is that there can be only one vault with type per owner (only one CRVV1ETHSTETH vault per owner)
  • access rights chain is VAULT PROXY < DS PROXY < USER where < is "is accessed by"

---

to find out the contents of the vault 29456:

1. open cdp manager contract, find the owner of this vault via owns and receive 0xBe0274664Ca7A68d6b5dF826FB3CcB7c620bADF3
2. open the address in the etherscan, find out that it's a cdp-registry contract. Look through the events and search for event that has the cdpi logged as 29456 (0x7310), extract the address of dss-proxy from there (should be 0x45799b667fd4abc96ac8b7b0e039502a72ebdcdf)
3. open cropper contract (use chainlog) and search for event NewProxy with the address 0x45799b667fd4abc96ac8b7b0e039502a72ebdcdf in it - extract the proxy address (should be 0x6625a9b63284f8913feeb3a87720fbddc4b276ab)
4. read the contents of the vault from vat contract via urns table. collateral is 0x435256563145544853544554482d41, the address of the urn is the one we found out in the previous step: 0x6625a9b63284f8913feeb3a87720fbddc4b276ab. Expected values for block 15717534 are:

  ink   uint256 :  154116476595071208910
  art   uint256 :  108666516936626029717812

this is aligned with the vaults tracker (~100k $ of debt)

[LukSteib]

Was able to follow the step-by-step guide above and ended up with same outcome:

[KirillDogadin-std]

Proposal on implementation

• modify the logic which fetches the information about the vault by its id to:
  • check if the owner is a contract How to determine whether an address is a contract address ethers-io/ethers.js#3084.
    • if not, proceed as before
    • else:
      • find out the owner-ds-proxy by owner variable of the cdp-registry (the contract that was stated to be the owner of the vault in the cdp-manager)
      • search through the events of the cropper contract and look for events with the name NewProxy and topic1 with the address of the owner-ds-proxy. Extract the urn address from topic2
      • we know the type of the vault from the manager, we know the urn address from the event, we know that there's one single vault of the collateral type per user. We've found the only vault address that exists.
      • we now can go fetch the balance of the vault (since it seems to be the only thing that is missing/wrong currently) from the vat by the collateral type and the vault address.

[valiafetisov]

check if the owner is a contract
find out the owner-ds-proxy by owner variable of the cdp-registry (the contract that was stated to be the owner of the vault in the cdp-manager)

Can you directly try to call owner.owns(vaultId) to check if it's a CdpRegistry contract?

search through the events

Since the event is indexed, this seems to be not as hacky as with other events.

we now can go fetch the balance of the vault (since it seems to be the only thing that is missing/wrong currently)

Should we then concentrate this change in a single place? A function to getVaultBalance that does those checks from above or directly gets the balance?

[KirillDogadin-std]

Can you directly try to call owner.owns(vaultId) to check if it's a CdpRegistry contract?

did not find a thing that would prevent this implementation way. But thinking about it more it seems like a more safe option to still do the direct comparison with the address of the contract that we get from the chainlog. Because the edge case: several other contracts could potentially have owns field and be stated as the owner.

Since the event is indexed, this seems to be not as hacky as with other events.

👍

Should we then concentrate this change in a single place? A function to getVaultBalance that does those checks from above or directly gets the balance?

👍

[valiafetisov]

still do the direct comparison with the address of the contract that we get from the chainlog

Maybe I'm missing the point, but I'm not against this. I am just saying that instead of check if the owner is a contract via linked await provider.getCode(address) you can directly call a method on this contract (which will fail if this is a regular wallet)

[KirillDogadin-std]

Maybe I'm missing the point, but I'm not against this. I am just saying that instead of check if the owner is a contract via linked await provider.getCode(address) you can directly call a method on this contract (which will fail if this is a regular wallet)

ok, makes sense, my point derived from yours is & was:

• i've realised that there's 3rd way: comparing the address of the owner directly with the cdp-registry address that we can fetch from the chainlog
• since you're not against, i will prefer this 3rd way

[KirillDogadin-std]

The edge case detected that is relevant for the simulation of this collateral and perhaps several others.

As mentioned before, the join contract here is of type "Cropper" which is a proxy contract that has an implementation specified in the other contract.

Specifically for CRVV1ETH here we have the following chain of join function call (the one that deposits the collateral into the protocol):

• join of Cropper is called
• the previous call is proxied into the CropJoin
• this call is also proxied into the SynthetixJoinImp which has address 0x6Eb33B161664EB5D1bB4B90ce7B35e2c57dF76bd and essentially immediately converts the transfered token into Lido

Therefore the edgecase description for the simulation of liquidation for CRVV1ETH sounds as follows:

• The join contract is storing all the received funds in Lido token instead of the CRVV1ETH. It performs the conversion right away. Hence we have an error during the simulation: the code expects the CRVV1ETH balance to be non-0 and to be the representative of how much value is stored in the join contract. This expectation is not met in fact. So there's the problem.

Possible approaches to solve this:

• modify the COLLATERALS constant and add information there about the token that the join contract holds. Although afai can see we need this information now only for simulation and the rest of the code does not benefit from this
• introduce a separate config that will only be relevant to this simulation and provide the address of the token that join contract holds (only for collaterals that need this overridden since they hold token that does not correspond to the collateral: ETH_JOIN will have no override because it holds ETH, CRVV1ETH will have the override becuse it holds Lido).

[valiafetisov]

Possible approaches to solve this

As far as I understand the solutions is the workaround for "withdrawing tokens from VAT". Maybe it was invalid assumption to start with (that it will be easier to modify VAT amounts) instead of token amounts? Would taking a different approach to possess desired amount of tokens (outlined in #495) solve this particular problem? Then, I would split the scope of this issue to fixing information retrieval for CRVV1ETH vaults and move on to #495.

[KirillDogadin-std]

Ok, let's scope this issue so that it can display and potentially liquidate the vaults of the type. the rest is up to discussion in the #495