Add Borrowable Margin Pool (V2)

[chudnov]

Task: Feature Name:

High Level Description

Allow whitelisted ribbon vaults to create a short position and direct their collateral to the new borrowable margin pool (MarginPoolV2) instead of the unborrowable marginpool (MarginPool). This allows certain whitelisted parties (whitelisted directly on MarginPoolV2) to borrow the collateral backing the call option. The whitelisted parties will exclusively be market makers who also buy the call option. This allows the market maker to delta hedge in a more capital efficient manner since they have an uncollateralized loan to the spot asset (which they can then short). We will whitelist vaults of treasuries that agree to offer this uncollateralized loan in exchange for higher premiums on the call option.

Say Zebra DAO has a $ZBR token. They launch a treasury vault with ribbon finance and deposit 100 $ZBR to sell covered calls and earn premiums from market makers. The 100 $ZBR is deposited into MarginPoolV2 when oTokens are minted. Market Maker X has 10 ZBR oTokens (call options) that they bought from the vault's oToken auction. Whereas in vanilla MarginPool the collateral cannot be taken out until the end, in MarginPoolV2 the oToken buyer (market maker) can borrow 10 $ZBR from MarginPoolV2 using borrow() to hedge their positive delta, posting ZERO collateral. Notice the amount they can borrow is proportional to how many oTokens they bought. Once a month passes, they deposit 10 $ZBR back into the vault.

Controller.sol:

• setMarginPoolV2: adds margin pool v2 address
• donateV2: donates to v2 margin pool

MarginPool.sol

• setBorrowerWhitelistedStatus: sets whether an address is whitelisted to borrow
• setRibbonVaultWhitelistedStatus: sets whether ribbon vault address is whitelisted to create short oToken on MarginPoolV2
• setBuyerWhitelistedStatus: sets whether address is whitelisted as oToken buyer of oToken minted on MarginPoolV2
• setBorrowPCT: sets the borrow percentage of entire collateral on a per-asset basis
• borrow: allows whitelisted address (market maker) to borrow some of underlying collateral proportional to their oToken bal
• borrowable: checks how much an address can borrow of underlying
• repay: allows whitelisted address to repay their loan once oToken expires
• repayFor: allows any address to repay loan on behalf of another party

Code

• [ ] Unit test 100% coverage
• [ ] Does your code follow the naming and code documentation guidelines?

Documentation

• [ ] Is your code up to date with the spec?
• [ ] Have you added your tests to the testing doc?

[haythemsellami]

Left some comments, main concerns re figuring out which pool to redeem oToken from. I actually think upgrading the MarginPool and having one MarginPool that keep track of collateral that can be borrowed may be better

Also probably in that case whitelisting oToken buyer is not needed...

Something like 1 pool contract (but we need to migrate current pool funds to the new one, but can be done by adding a function in Controller that can only be called by owner for that to migrate current balance).

MarginPoolV2 is MarginPool, adding a mapping to keep track of deposited collateral that can be borrowed + this BorrowablePool implementation, what u think about that? @chuddster

[haythemsellami]

Closed by mistake...

[haythemsellami]

Left some comments, main concerns re figuring out which pool to redeem oToken from. I actually think upgrading the MarginPool and having one MarginPool that keep track of collateral that can be borrowed may be better

Also probably in that case whitelisting oToken buyer is not needed...

Something like 1 pool contract (but we need to migrate current pool funds to the new one, but can be done by adding a function in Controller that can only be called by owner for that to migrate current balance).

MarginPoolV2 is MarginPool, adding a mapping to keep track of deposited collateral that can be borrowed + this BorrowablePool implementation, what u think about that? @chuddster

If MM with 1oToken can't borrow collateral greater than the amount of collateral deposited to mint that 1oToken, right? I think it is safe to use the same MarginPool, and allow all oToken holder to redeem from the same pool(no need to whitelist oToken buyer or check which pool to redeem from), as we are locking the oToken from borrower, right?

[chudnov]

Left some comments, main concerns re figuring out which pool to redeem oToken from. I actually think upgrading the MarginPool and having one MarginPool that keep track of collateral that can be borrowed may be better

Also probably in that case whitelisting oToken buyer is not needed...

Something like 1 pool contract (but we need to migrate current pool funds to the new one, but can be done by adding a function in Controller that can only be called by owner for that to migrate current balance). MarginPoolV2 is MarginPool, adding a mapping to keep track of deposited collateral that can be borrowed + this BorrowablePool implementation, what u think about that? @chuddster

If MM with 1oToken can't borrow collateral greater than the amount of collateral deposited to mint that 1oToken, right? I think it is safe to use the same MarginPool, and allow all oToken holder to redeem from the same pool(no need to whitelist oToken buyer or check which pool to redeem from), as we are locking the oToken from borrower, right?

@haythem96 I think some more context will be helpful for you here: we are trying to build this pool out for select ribbon treasury partners to increase the yield on the options since the collateral can be borrowed. We want to decouple this into an entirely new margin pool for three reasons:

1. this allows opyn to continue to be the owner of the MarginPool
2. this allows Ribbon to be owner of new MarginPool instance and decide who is whitelisted, the borrowing percentages, etcetera etcetera since the collateral comes only from ribbon vaults that we whitelist
3. upgrading original MarginPool with this code allows the owner to potentially whitelist borrowing of oTokens with ETH / BTC / USDC as collateral. These are oTokens minted by Ribbon retail vaults and StakeDao retail vaults that don't consent to their collateral being borrowed. This upgrade allows for that to be a possibility. Confining these access controls to an entirely separate MarginPool that only has select altcoins in it minimizes risk.

For figuring out which pool to redeem oToken from the logic is !borrowablePool.isWhitelistedOTokenBuyer(msg.sender) OR ERC20Interface(collateral).balanceOf(address(borrowablePool)) < payout

Seems like this can work. The logic goes as follows:

1. You are a whitelisted oToken buyer from the borrowable pool. Only market makers that buy from the treasury vaults will be whitelisted here. a. A whitelisted oToken buyer which is a market maker, might still participate in other ribbon options vaults as buyers so the same whitelisted address may hold oTokens that have a claim to collateral in vanilla pool. Stuff like ETH, WBTC, etc. In this case we check if the borrowable pool holds that collateral which by definition it cannot since the treasury vaults whitelisted only hold specific altcoins in there. The only way ETH/BTC can b in the borrowable pool is through donations or direct transfers, in which case if enough has been transferred (> payout) it will pay it out to market maker. This just means that instead of the stray tokens being locked into the borrowable pool (via transfer, etc) - the same tokens end up being locked in the vanilla pool because that collateral will never be claimed. So this logic has no negative effects as far as I understand - it just switches where the stray collateral is located.
2. You are not a whitelisted oToken buyer from the borrowable pool. This has to mean that the only oTokens in your possession must have collateral in the vanilla pool, which is where you are redirected.

[haythemsellami]

@chuddster Most of it looks fine to me.

Based on the current implementation, agree on:

• Keeping the new whitelist functions in the new BorrowablePool, no need to upgrade Whitelist module...etc
• Current naming LGTM

I still think the vault type set when opening a vault should change, the proposed implementation in the comment is more generic and does not assume that user who will use the borrowable pool will not need to open spread/naked margin https://github.com/opynfinance/GammaProtocol/pull/459#discussion_r930842294

[haythemsellami]

@akwritescode Can you please check circleci, because some jobs can't be executed from outside opyn org members.

[haythemsellami]

When CI successfully run, based on coverage may need to add some tests, especially one to cover cases where Controller is involved

[chudnov]

When CI successfully run, based on coverage may need to add some tests, especially one to cover cases where Controller is involved

can you elaborate?

[chudnov]

@chuddster Most of it looks fine to me.

Based on the current implementation, agree on:

• Keeping the new whitelist functions in the new BorrowablePool, no need to upgrade Whitelist module...etc
• Current naming LGTM

I still think the vault type set when opening a vault should change, the proposed implementation in the comment is more generic and does not assume that user who will use the borrowable pool will not need to open spread/naked margin #459 (comment)

Yes in general I agree your approach is better design (letting owner pass it through) instead of overriding whatever value is passed. Currently we pass in vault type 0 in these treasury vaults which will be overridden with this upgraded controller. This implementation does not require any vaults to be redeployed - we can just call setWhitelistedOptionsVault(true) on all existing addresses and it will override the vault type to 2. We already have 4 treasury vaults that are deployed and have funds in them and we would have to redeploy all of them and pass in vault type 2. This would require all of the treasuries to migrate to a new contract which is undesirable. I agree its a bit hacky.

The only vaults that will be able to open a margin vault in the borrowable pool are ribbon vaults (we will whitelist them) - so we are only constraining ourselves if we ever want to do a naked margin vault.

[haythemsellami]

When CI successfully run, based on coverage may need to add some tests, especially one to cover cases where Controller is involved

can you elaborate?

Just meant when CI run, to check the coverage if there are some important missing paths not tested.