omni / tokenbridge-contracts

Smart contracts for TokenBridge
http://docs.tokenbridge.net
GNU General Public License v3.0
230 stars 227 forks source link

Earning interest with locked DAI #317

Closed maxaleks closed 4 years ago

maxaleks commented 4 years ago

Solutions

Some of the possible solutions to earn interest are DAI Savings Rate, Compound, rDAI and Pool DAI

Let's take a closer look at each of them.

1. DAI Savings Rate (DSR)

This feature will be available with the DAI update. It allows us to lock our DAI and earn interest without any risks. The MakerDAO provides us the contract with join/exit methods which we should call using DSProxy pattern (here is the guide on how to use the pattern)

Example transactions: join (3 DAI) -> join (4 DAI) -> exitAll (7 DAI + earned interest)

The implementation steps will look like this:

  1. deploy DSProxy contracts or use DSProxyFactory.build() in production
  2. approve DAI tokens for deployed proxy
  3. add the following code to the bridge contracts:
    
    function _join(uint256 _amount) internal {
    bytes memory _data = abi.encodeWithSignature("join(address,address,uint256)", daiJoinAddress, potAddress, _amount);
    _execute(_data);
    }

function _exit(uint256 _amount) internal { bytes memory _data = abi.encodeWithSignature("exit(address,address,uint256)", daiJoinAddress, potAddress, _amount); _execute(_data); }

function _exitAll() internal { bytes memory _data = abi.encodeWithSignature("exitAll(address,address)", daiJoinAddress, potAddress); _execute(_data); }

function _execute(bytes memory _data) internal { DSProxy(proxy).execute(dsrProxyAddress, _data); }


`dsrProxyAddress`, `daiJoinAddress` and `potAddress` are provided by MakerDAO

Costs:
1. `join` ~ 195,000 gas
2. `exit` ~ 175,000 gas

#### 2. Compound
I find this solution the most inappropriate, because we'll have different balances in DAI and cDAI, and the logic isn't as simple as in the next solutions which are wrappers of Compound

#### 3. rDAI
It's a wrapper of Compound and it has 2 advantages:
1. `1 rDAI = 1 DAI`
2. we can use a definition called `hat` to configure who is the beneficiary of the accumulated interest

The second point allows us not to add the logic of calculation and withdrawal of earned interest to the bridge contracts code. The `hat` can withdraw earned interest itself.
The interaction logic is pretty simple: we need to [set hat](https://github.com/rtoken-project/rtoken-contracts/blob/master/contracts/RToken.sol#L259) once and then just call [mint](https://github.com/rtoken-project/rtoken-contracts/blob/master/contracts/RToken.sol#L182)/[redeem](https://github.com/rtoken-project/rtoken-contracts/blob/master/contracts/RToken.sol#L219).

Costs:
1. `mint`   ~ 290,000 gas
2. `redeem` ~ 280,000 gas

#### 4. Pool DAI
The same as rDAI, but it allows us to create our own token. Unfortunately, the smart contracts are not audited and the last update on GitHub was 3 months ago

### Conclusion
I think the most appropriate solution is `rDAI`, because it allows us to set a `hat` and not care about the distribution of earned interest, and the interest in `Compound`, that is used by `rDAI`, theoretically will be greater than in `DSR`. But `rDAI` costs 1.5 times more than `DSR`. So we need to set priorities and decide what to choose  
varasev commented 4 years ago

I'm for DSR if that really takes less gas than rDai. But we should repeat gas measurement for rDai after Compound switches to MCD because the current rDai's gas consumption measured for its SCD implementation.

maxaleks commented 4 years ago

@varasev, yes, you're right. I will measure as soon as this happens

varasev commented 4 years ago

On the other hand, DSR is 2% at the moment vs rDai's 6%. If DSR is less profitable than rDai, then it's worth utilizing rDai. But DSR can probably change.

Compound had ~4% annual rate during the last month if I calculated that correctly.

akolotov commented 4 years ago

If we chose rDAI we should state clearly who and when calls createHat, mint and redeem. What amount will be specified for mint and how it is calculated. What amount will be specified for redeem?

maxaleks commented 4 years ago

I think this may work as follows:

  1. createHat will be called inside the initialize method
  2. convertDaiToRDai (mint) will be public. It will be called inside the relayTokens method and directly by any address (by oracle, maybe)
  3. convertRDaiToDai (redeem) will be called inside the onExecuteMessage method

What amount will be specified for mint and how it is calculated

All DAI will be converted to rDAI, won't they?

What amount will be specified for redeem?

The amount that should be transferred to the user if the bridge doesn't have enough DAI

Any other ideas?

varasev commented 4 years ago

I think @akolotov means that we maybe need to have some threshold above which DAI would be converted to rDai, right? To reduce the number of callings of mint function and so to reduce total gas consumption (like we are going to do with the swapTokens function in Migration to MCD logic).

maxaleks commented 4 years ago

OK, then I propose to make threshold equal to executionDailyLimit, and call convertDaiToRDai once a day. Implementation example:

function convertDaiToRDai() external {
  uint256 balance = dai.balanceOf(address(this));
  uint256 remainingLimit = executionDailyLimit().sub(totalExecutedPerDay(getCurrentDay()));
  if (balance > remainingLimit) {
    rdai.mint(balance.sub(remainingLimit));
  }
}

And call convertRDaiToDai inside onExecuteMessage:

function convertRDaiToDai() internal {
  uint256 balance = dai.balanceOf(address(this));
  uint256 remainingLimit = executionDailyLimit().sub(totalExecutedPerDay(getCurrentDay()));
  if (balance < remainingLimit) {
     rdai.redeem(remainingLimit.sub(balance));
  }
}
akolotov commented 4 years ago

createHat will be called inside the initialize method

So, who will be members of this hat? Can the list of members can be changed later?

All DAI will be converted to rDAI, won't they?

The amount that should be transferred to the user if the bridge doesn't have enough DAI

My understanding that Compound has a limit to withdraw funds. If rDai is a wrapper for Compound, this limit should be inherited. If so, the bridge will not be able to exchange rTokens to Dai above some value. It means that we need to either limit withdrawal (and we have your changes for this in the existing PR) and have a reserve in Dai to be able support withdrawals if the bridge balance is below of the threshold used in the relative limits feature.

akolotov commented 4 years ago

call convertDaiToRDai once a day

We need to describe a way how this call will be triggered by the oracle: automatically or not? Is the period of time "once a day" is configurable or not? Which validator node will be responsible for calling this method?

maxaleks commented 4 years ago

So, who will be members of this hat?

As Igor said, it will be a multisig for PoC, and later - a mediator on AMB bridge

Can the list of members can be changed later?

The list can't be changed, but a new hat can be created

My understanding that Compound has a limit to withdraw funds.

I haven't found any limits. There may be a bank run problem which is described here. If we worry about this then we shouldn't use Compound at all

We need to describe a way how this call will be triggered by the oracle: automatically or not? Is the period of time "once a day" is configurable or not? Which validator node will be responsible for calling this method?

I don't know how to incentivize validators to call the method (maybe some kind of reward for doing this?). If I don't miss anything this method can be called at any time because we don't worry about our big balance in DAI -- it doesn't block withdrawals. So it can be called at any time and with any time interval between calls

igorbarinov commented 4 years ago

I haven't found any limits. There may be a bank run problem which is described here. If we worry about this then we shouldn't use Compound at all

the problem doesn't exist for us in observable future

current utilization and liquidity in Compound

Utilization
52.63%
Market Liquidity
11,603,746 SAI

https://app.compound.finance/asset/cDAI

thus, available for withdrawing now 5,496,694.4802 Sai we have 36.92k Sai locked in the bridge.

igorbarinov commented 4 years ago

FYI for DSR, there is a wrapper like poolDai for Dai/cDai https://twitter.com/MartinLundfall/status/1197627208136658944?s=09 https://github.com/dapphub/chai

varasev commented 4 years ago

FYI for DSR, there is a wrapper like poolDai for Dai/cDai

Anyway, I guess we are not going to use DSR at the moment because of its low savings rate.

igorbarinov commented 4 years ago

No, but it's good to know that there is another form of integration of DSR from creators of MCD.

On Fri, Nov 22, 2019, 9:58 AM varasev notifications@github.com wrote:

FYI for DSR, there is a wrapper like poolDai for Dai/cDai

Anyway, I guess we are not going to use DSR at the moment because of its low savings rate.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/poanetwork/tokenbridge-contracts/issues/317?email_source=notifications&email_token=AADHVNB2VCWOZGCLD4LLLY3QU57IRA5CNFSM4JN7ENL2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEE4X2UY#issuecomment-557415763, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADHVNDTM2L466FFR4ZVPBDQU57IRANCNFSM4JN7ENLQ .

igorbarinov commented 4 years ago

One more alternative popped up in my Twitter for DSR, Dai-HRD https://medium.com/@keydonix/dai-hrd-token-hold-rated-dai-dfd3fe491d83

On Fri, Nov 22, 2019, 10:09 AM Igor Barinov igor@blocknotary.com wrote:

No, but it's good to know that there is another form of integration of DSR from creators of MCD.

On Fri, Nov 22, 2019, 9:58 AM varasev notifications@github.com wrote:

FYI for DSR, there is a wrapper like poolDai for Dai/cDai

Anyway, I guess we are not going to use DSR at the moment because of its low savings rate.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/poanetwork/tokenbridge-contracts/issues/317?email_source=notifications&email_token=AADHVNB2VCWOZGCLD4LLLY3QU57IRA5CNFSM4JN7ENL2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEE4X2UY#issuecomment-557415763, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADHVNDTM2L466FFR4ZVPBDQU57IRANCNFSM4JN7ENLQ .

igorbarinov commented 4 years ago

If there anything stopping us from starting working on the rDai option?

maxaleks commented 4 years ago

Only possible changes in architecture in this PR, because rDAI will only be used with relative daily limit. Awaiting for @akolotov final review

varasev commented 4 years ago

@maxaleks I think we could start in a new branch based on relative-daily-limit branch. If anything changes there, the changes could be applied to the new branch too.

maxaleks commented 4 years ago

Ok, then I'm starting to work on it

akolotov commented 4 years ago

the problem doesn't exist for us in observable future

@maxaleks since your investigation showed that there is no daily limit for rDai and Igor confirmed this, I don't think anymore that relative limits and earning interest for Dai features are tied together.

maxaleks commented 4 years ago

Ok, then will this feature be mandatory or optional? If optional then what solution will we choose: do checks in contracts on isRDaiUsed: true/false or extend contracts with additional logic? In the first case we will have a bigger bytecode for those who doesn't use rDai feature, and in the second case we will have 4 contracts: ForeignBridgeErcToNative, ForeignBridgeErcToNativeRelativeDailyLimit, ForeignBridgeErcToNativeWithRDai, ForeignBridgeErcToNativeRelativeDailyLimitWithRDai These issues are also being discussed in the comments on this PR

varasev commented 4 years ago

I guess that should be optional. Since this is about Foreign* contracts only (which code size is less than Home*'s), maybe it's better to have isRDaiUsed option. Do you agree, @akolotov ?

igorbarinov commented 4 years ago

@everyone Sorry for the push-notification, but we need to provide a quick update regarding the DAI > SAI transition and what this means for rDAI.

The rDAI token currently live on mainnet will now officially be called "rSAI" to reflect the Maker migration. The token is in fact redeemable for SAI, not DAI.

The symbol and name have been updated on etherscan only, with updates to the rDAI website, repos, etc. to be made soon. No changes to the contracts have been made, therefore if you call the contract directly you will still see the old names symbol=rDAI and name=Redeemable DAI. We do not believe it is necessary to perform an upgrade to the contract for such a minor change. This is a good time to also mention that our current strategy for upgrades will include plenty of prior notice and transparency to the community.

Just as a reminder, here are the addresses for the currently deployed contracts. No changes have been made since the mainnet launch on 2019-11-07 14:33:45 UTC. rSAI proxy contract (the one you use in your DAPP): 0xea8b224edd3e342deb514c4176c2e72bcce6fff9

rSAI logic contract deployed 2019-11-07 14:30:08 0x293908E6352b11a91bC08eeb335644C485D21170

More details on the MCD migration, and future plans for Multi-collateral rDAI will be released soon via Medium post.


from rDai Discord

akolotov commented 4 years ago

@maxaleks @varasev I have provided my thoughts on how to extend the bridge contracts functionality in the PR 306.

k1rill-fedoseev commented 4 years ago

There is a proposal to use DSR instead of rDai.

Reasoning:

Implementation details:

Gas costs for DSRConnector.sol:

igorbarinov commented 4 years ago

Thanks for the analysis Had you reviewed Chai, an ERC20 wrapper for DSR? https://github.com/dapphub/chai

k1rill-fedoseev commented 4 years ago

Yes, I have, but I didn't look deep. Maybe it will be a little bit simpler to use Chai for our purposes.

akolotov commented 4 years ago

Could you provide more clarification about DSR? Currently I see that there are two coefficients pot.drip() and pot.chi() that affect the amount of DAI to be withdrawn. What is the nature of these coefficients? Should we add additional logic on the token bridge contract to monitor these values to secure DAI deposited by the bridge users?

Regarding Chai is it possible to completely exit from Chai tokens back to Dai tokens?

akolotov commented 4 years ago

It is not possible to collect interest on the account that is different from the initial DAI owner. In our case, this implies that the interest will be redeemed to the bridge contract. Some additional logic is required in order to distinguish between bridge's DAI and the collected interest, so that the bridge owner will be able to withdraw only the collected interest part.

Could you provide an example of logic that could be implemented to separate earned interest from deposited Dai tokens?

k1rill-fedoseev commented 4 years ago

So, regarding DSR, the logic is described in Pot contract documentation.

There is nothing to be concerned about, the only use of this coefficient is to evaluate balances and values in DAI, rather than in internal numbers stored in Pot contract.

Regarding Chai usage, there are analogs of mint/burn functions, which call join/exit correspondently, so you can completely exit chai and receive DAI + interest back. There is also a useful function for withdraw operation which guarantees that the amount of withdrawn DAI will be greater than or equal to the specified amount. This is useful for dealing with possible truncation problems. Using Chai also allows us to pay interest directly in chai tokens, without withdrawing them. This will significantly reduce gas cost of such operation on the bridge side, interest recipient then can redeem DAI by his own by calling exit or draw functions.

Personally, I think that using Chai is the best solution.

k1rill-fedoseev commented 4 years ago

Could you provide an example of logic that could be implemented to separate earned interest from deposited Dai tokens?

I guess, we can remember a current invested number of DAI daiInvested on the bridge side, and then, pay at most balanceInDSR() - daiInvested as an interest. So, at any time, the invested amount will be enough to cover all bridged tokens.