feat(EMP contracts) Create callable if new position CR is above GCR

[nicholaspai]

Motivation

#1843

Summary

The GCR require statement was turned into an OR condition including the current condition (where create CR > GCR) and adding a new one (where resultant position CR > GCR).

Details

GCR unit tests updated as well.

I tested a deployment on Kovan of the updated EMPLib and the bytecode deployed fine. The deployedBytecode increased by 116 bytes from my local calculation.

Issue(s)

Fixes #1843

[coveralls]

Coverage remained the same at 92.683% when pulling 9c0be75 on npai/contracts-gcr into 7778e7a on master.

[mrice32]

Looks awesome! One nit.

[mrice32]

nit: if it doesn't increase the bytecode size (that's a big if :)), do you think this would be a little more readable if we separated this into intermediate variables?

bool newTokensAboveGCR = ...;
bool resultingPositionAboveGCR = ...;
require(newTokensAboveGCR || resultingPositionAboveGCR, "Must meet GCR")

[chrismaree]

this will definitely increase the bytecode but would make it easier to read. Curious how much it increase it by.

[nicholaspai]

I can experiment

[nicholaspai]

This increased bytecode by 30

[nicholaspai]

 // Either the new create ratio or the resultant position CR must be above the current GCR.
        bool resultantPositionCRGreaterThanGCR = _checkCollateralization(
            _getFeeAdjustedCollateral(positionData.rawCollateral).add(collateralAmount),
            positionData.tokensOutstanding.add(numTokens)
        );
        bool creationCRGreaterThanGCR = _checkCollateralization(collateralAmount, numTokens);
        require(
            (resultantPositionCRGreaterThanGCR || creationCRGreaterThanGCR),
            "New CR below GCR"
        );

[mrice32]

Not super opinionated here, but I'd probably err on the side of saving bytecode and making it a little less readable, just for the sake of making things easier down the road.

[nicholaspai]

agreed

[daywiss]

what is the reason you can't just check the resultant CR > GCR and skip the first check?

[nicholaspai]

what is the reason you can't just check the resultant CR > GCR and skip the first check?

@daywiss Imagine that the GCR is 3.0 and your CR is 2.0 (and the liquidation ratio is 1.2). In this situation, the (CR > GCR) check would require a lot of capital for you to get your CR > GCR. So, we should still enable the previous check (creation CR > GCR) for such sponsors since it can only increase the position CR.

In other words, we should only allow a position's CR to decrease IFF the resultant position's CR > GCR

[mrice32]

LGTM!

[chrismaree]

This looks great! nice and simple. Clean implementation and tests. Good work @nicholaspai

[chrismaree]

+1