Merge pull request #103 from primitivefinance/fix/engine-lending

Fix/engine lending

contracts/PrimitiveEngine.sol

@@ -351,41 +351,42 @@ contract PrimitiveEngine is IPrimitiveEngine {
     /// @inheritdoc IPrimitiveEngineActions
     function borrow(
         bytes32 poolId,
-        address recipient,
         uint256 delLiquidity,
         uint256 maxPremium,
         bytes calldata data
-    ) external override lock {
+    ) external override lock returns (uint256 premium) {
+        // Source: Convex Payoff Approimation. https://stanford.edu/~guillean/papers/cfmm-lending.pdf. Section 5
         Reserve.Data storage reserve = reserves[poolId];
-        {
-            uint256 delLiquidity = delLiquidity;
-            require(reserve.float >= delLiquidity && delLiquidity > 0, "Insufficient float"); // fail early if not enough float to borrow
-
-            uint256 resLiquidity = reserve.liquidity; // global liquidity balance
-            uint256 delRisky = (delLiquidity * reserve.reserveRisky) / resLiquidity; // amount of risky asset
-            uint256 delStable = (delLiquidity * reserve.reserveStable) / resLiquidity; // amount of stable asset
-
-            {
-                uint256 preRisky = IERC20(risky).balanceOf(address(this));
-                uint256 preStable = IERC20(stable).balanceOf(address(this));
-
-                // trigger callback before position debt is increased, so liquidity can be removed
-                IERC20(stable).safeTransfer(msg.sender, delStable);
-                IPrimitiveLendingCallback(msg.sender).borrowCallback(delLiquidity, delRisky, delStable, data); // trigger the callback so we can remove liquidity
-                positions.borrow(poolId, delLiquidity); // increase liquidity + debt
-                // fails if risky asset balance is less than borrowed `delLiquidity`
-                reserve.remove(delRisky, delStable, delLiquidity, _blockTimestamp());
-                reserve.borrowFloat(delLiquidity);
-
-                uint256 postRisky = IERC20(risky).balanceOf(address(this));
-                uint256 postRiskless = IERC20(stable).balanceOf(address(this));
-
-                require(postRisky >= preRisky + (delLiquidity - delRisky), "IRY");
-                require(postRiskless >= preStable - delStable, "IRL");
-            }
+        require(reserve.float >= delLiquidity && delLiquidity > 0, "Insufficient float"); // fail early if not enough float to borrow
 
-            emit Borrowed(recipient, poolId, delLiquidity, maxPremium);
+        uint256 resLiquidity = reserve.liquidity; // global liquidity balance
+        uint256 delRisky = (delLiquidity * reserve.reserveRisky) / resLiquidity; // amount of risky asset
+        uint256 delStable = (delLiquidity * reserve.reserveStable) / resLiquidity; // amount of stable asset
+
+        {
+            // Balances before position creation
+            uint256 preRisky = IERC20(risky).balanceOf(address(this));
+            uint256 preStable = IERC20(stable).balanceOf(address(this));
+            // 0. Update position of `msg.sender` with `delLiquidity` units of debt and `risky` tokens
+            positions.borrow(poolId, delLiquidity);
+            // 1. Borrow `delLiquidity`: Reduce global reserve float, increase global debt
+            reserve.borrowFloat(delLiquidity);
+            // 2. Remove liquidity: Releases `risky` and `stable` tokens
+            reserve.remove(delRisky, delStable, delLiquidity, _blockTimestamp());
+            // 3. Sell `stable` tokens for `risky` tokens, agnostically, within the callback
+            IERC20(stable).safeTransfer(msg.sender, delStable); // transfer the stable tokens of the liquidity out to the `msg.sender`
+            IPrimitiveLendingCallback(msg.sender).borrowCallback(delLiquidity, delRisky, delStable, data);
+            // Check price impact tolerance
+            premium = delLiquidity - delRisky;
+            require(maxPremium >= premium, "Max");
+            // Check balances after position creation
+            uint256 postRisky = IERC20(risky).balanceOf(address(this));
+            uint256 postStable = IERC20(stable).balanceOf(address(this));
+            require(postRisky >= preRisky + premium, "IRY");
+            require(postStable >= preStable - delStable, "IRL");
         }
+
+        emit Borrowed(msg.sender, poolId, delLiquidity, maxPremium);
     }
 
     /// @inheritdoc IPrimitiveEngineActions
@@ -396,30 +397,41 @@ contract PrimitiveEngine is IPrimitiveEngine {
         uint256 delLiquidity,
         bool fromMargin,
         bytes calldata data
-    ) external override lock returns (uint256 deltaRisky, uint256 deltaStable) {
+    )
+        external
+        override
+        lock
+        returns (
+            uint256 delRisky,
+            uint256 delStable,
+            uint256 premium
+        )
+    {
         Reserve.Data storage reserve = reserves[poolId];
         Position.Data storage position = positions.fetch(owner, poolId);
         Margin.Data storage margin = margins[owner];
 
-        require(reserve.debt >= delLiquidity && position.liquidity >= delLiquidity, "ID");
+        // There is `delLiquidity` units of debt, which must be repaid using `delLiquidity` risky tokens.
+        position.repay(delLiquidity); // must have an open position, releases position.debt of risky
+        delRisky = (delLiquidity * reserve.reserveRisky) / reserve.liquidity; // amount of risky required to mint LP
+        delStable = (delLiquidity * reserve.reserveStable) / reserve.liquidity; // amount of stable required to mint LP
+        require(delRisky * delStable > 0, "Deltas are 0"); // fail early if 0 amounts
 
-        deltaRisky = (delLiquidity * reserve.reserveRisky) / reserve.liquidity;
-        deltaStable = (delLiquidity * reserve.reserveStable) / reserve.liquidity;
+        premium = delLiquidity - delRisky; // amount of excess risky, used to pay for stable side
 
+        // Update state
+        reserve.allocate(delRisky, delStable, delLiquidity, _blockTimestamp());
+        reserve.repayFloat(delLiquidity);
+        // Balances prior to callback/transfers
+        uint256 preRisky = IERC20(risky).balanceOf(address(this));
+        uint256 preStable = IERC20(stable).balanceOf(address(this));
         if (fromMargin) {
-            margins.withdraw(delLiquidity - deltaRisky, deltaStable); // reduce margin balance
-            reserve.allocate(deltaRisky, deltaStable, delLiquidity, _blockTimestamp()); // increase reserve liquidity
-            position.repay(delLiquidity); // reduce position debt
+            margins.withdraw(0, delStable); // pay stables from margin balance
+            margin.deposit(premium, 0); // receive remainder `premium` of risky to margin
         } else {
-            uint256 preStable = IERC20(stable).balanceOf(address(this));
-            IPrimitiveLendingCallback(msg.sender).repayFromExternalCallback(deltaStable, data);
-
-            require(IERC20(stable).balanceOf(address(this)) >= preStable + deltaStable, "IS");
-
-            reserve.allocate(deltaRisky, deltaStable, delLiquidity, _blockTimestamp());
-            reserve.repayFloat(delLiquidity);
-            position.repay(delLiquidity);
-            margin.deposit(delLiquidity - deltaRisky, uint256(0));
+            IERC20(risky).safeTransfer(msg.sender, premium); // This is a concerning line of code!
+            IPrimitiveLendingCallback(msg.sender).repayFromExternalCallback(delStable, data);
+            require(IERC20(stable).balanceOf(address(this)) >= preStable + delStable, "IS"); // fails if stable is not paid
         }
 
         emit Repaid(owner, poolId, delLiquidity);

contracts/interfaces/engine/IPrimitiveEngineActions.sol

@@ -116,29 +116,37 @@ interface IPrimitiveEngineActions {
 
     /// @notice Increases the `msg.sender`'s position's liquidity value and also adds the same to the debt value.
     /// @param  poolId          Keccak hash of the option parameters of a curve to interact with
-    /// @param  owner           Position owner to grant the borrowed liquidity shares
     /// @param  delLiquidity    Amount of liquidity to borrow and add as debt
     /// @param  maxPremium      Max amount of `premium` that can be collected from the `msg.sender` to collateralize the position
     /// @param  data            Arbitrary data that is passed to the borrowCallback function
+    /// @return premium         Price paid to open position
     function borrow(
         bytes32 poolId,
-        address owner,
         uint256 delLiquidity,
         uint256 maxPremium,
         bytes calldata data
-    ) external;
+    ) external returns (uint256 premium);
 
     /// @notice Reduces the `msg.sender`'s position's liquidity value and also reduces the same to the debt value.
     /// @param  poolId          Keccak hash of the option parameters of a curve to interact with
     /// @param  owner           Position owner to grant the borrowed liquidity shares
     /// @param  delLiquidity    Amount of liquidity to borrow and add as debt
     /// @param  fromMargin      Whether the `msg.sender` uses their margin balance, or must send tokens
     /// @param  data            Arbitrary data that is passed to the repayCallback function
+    /// @return delRisky        Amount of risky tokens allocated as liquidity to pay debt
+    /// delStable               Amount of stable tokens allocated as liquidity to pay debt
+    /// premium                 Amount of risky tokens paid to the `owner`'s margin account
     function repay(
         bytes32 poolId,
         address owner,
         uint256 delLiquidity,
         bool fromMargin,
         bytes calldata data
-    ) external returns (uint256, uint256);
+    )
+        external
+        returns (
+            uint256 delRisky,
+            uint256 delStable,
+            uint256 premium
+        );
 }

contracts/interfaces/engine/IPrimitiveEngineView.sol

@@ -19,16 +19,16 @@ interface IPrimitiveEngineView {
     ) external view returns (int128 reserveOfToken);
 
     /// @notice                 Uses the trading function to calc the invariant using token reserve values
-    /// @param  poolId             The hashed pool Id
-    /// @param  postR1          Amount of risky tokens in the pool's reserves
-    /// @param  postR2          Amount of stable tokens in the pool's reserves
-    /// @param  postLiquidity   Total supply of liquidity shares for the pool
+    /// @param  poolId          The hashed pool Id
+    /// @param  resRisky        Amount of risky tokens in the pool's reserves
+    /// @param  resStable       Amount of stable tokens in the pool's reserves
+    /// @param  resLiquidity    Total supply of liquidity shares for the pool
     /// @return                 Invariant calculated (which should be near 0)
     function calcInvariant(
         bytes32 poolId,
-        uint256 postR1,
-        uint256 postR2,
-        uint256 postLiquidity
+        uint256 resRisky,
+        uint256 resStable,
+        uint256 resLiquidity
     ) external view returns (int128);
 
     /// @notice Fetches the current invariant based on risky and stable token reserves of pool with `poolId`
@@ -91,17 +91,13 @@ interface IPrimitiveEngineView {
 
     /// @notice Fetches Position data struct using a position id
     /// @param  posId   Position id
-    /// @return balanceRisky    Risky balance of the position debt
-    /// balanceStable   Stable balance of the position debt
-    /// float           Liquidity shares that are marked for loans
+    /// @return float   Liquidity shares that are marked for loans
     /// liquidity       Liquidity shares in the position
-    /// debt            Liquidity shares in debt, must be repaid
+    /// debt            Liquidity shares in debt, must be repaid, also equal to risky balance of position
     function positions(bytes32 posId)
         external
         view
         returns (
-            uint128 balanceRisky,
-            uint128 balanceStable,
             uint128 float,
             uint128 liquidity,
             uint128 debt

contracts/libraries/Position.sol

@@ -7,17 +7,14 @@ pragma abicoder v2;
 /// @dev     This library is a generalized position data structure for any engine.
 
 import "./SafeCast.sol";
-import "hardhat/console.sol";
 
 library Position {
     using SafeCast for uint256;
 
     struct Data {
-        uint128 balanceRisky; // Balance of risky asset
-        uint128 balanceStable; // Balance of stable asset
         uint128 float; // Balance of loaned liquidity
         uint128 liquidity; // Balance of liquidity, which is negative if a debt exists
-        uint128 debt; // Balance of liquidity debt that must be paid back
+        uint128 debt; // Balance of liquidity debt that must be paid back, also balance of risky in position
     }
 
     /// @notice An Engine's mapping of position Ids to Data structs can be used to fetch any position.
@@ -35,6 +32,7 @@ library Position {
 
     /// @notice Add to the balance of liquidity
     function allocate(Data storage position, uint256 delLiquidity) internal returns (Data storage) {
+        require(position.debt == 0, "Debt");
         position.liquidity += delLiquidity.toUint128();
         return position;
     }
@@ -57,10 +55,8 @@ library Position {
         uint256 delLiquidity
     ) internal returns (Data storage) {
         Data storage position = fetch(positions, msg.sender, poolId);
-        uint128 liquidity = position.liquidity;
-        require(liquidity == 0, "Must borrow from 0");
+        require(position.liquidity == 0, "Must borrow from 0");
         position.debt += delLiquidity.toUint128(); // add the debt post position manipulation
-        position.balanceRisky += delLiquidity.toUint128();
         return position;
     }
 
@@ -87,10 +83,9 @@ library Position {
         return position;
     }
 
-    /// @notice Reduces `delLiquidity` of position.debt by reducing `delLiquidity` of position.liquidity
+    /// @notice Reduces `delLiquidity` of position.debt
     function repay(Data storage position, uint256 delLiquidity) internal returns (Data storage) {
-        position.liquidity -= delLiquidity.toUint128();
-        // FIX: Contract too large, position.debt -= delLiquidity.toUint128();
+        position.debt -= delLiquidity.toUint128();
         return position;
     }
 

contracts/test/engine/EngineBorrow.sol

@@ -10,6 +10,8 @@ contract EngineBorrow {
     address public stable;
     address public CALLER;
 
+    uint256 public dontPay = 1;
+
     constructor() {}
 
     function initialize(
@@ -29,7 +31,30 @@ contract EngineBorrow {
         bytes calldata data
     ) public {
         CALLER = msg.sender;
-        IPrimitiveEngine(engine).borrow(poolId, owner, delLiquidity, type(uint256).max, data);
+        IPrimitiveEngine(engine).borrow(poolId, delLiquidity, type(uint256).max, data);
+    }
+
+    function borrowMaxPremium(
+        bytes32 poolId,
+        address owner,
+        uint256 delLiquidity,
+        uint256 maxPremium,
+        bytes calldata data
+    ) public {
+        CALLER = msg.sender;
+        IPrimitiveEngine(engine).borrow(poolId, delLiquidity, maxPremium, data);
+    }
+
+    function borrowWithoutPaying(
+        bytes32 poolId,
+        address owner,
+        uint256 delLiquidity,
+        bytes calldata data
+    ) public {
+        CALLER = msg.sender;
+        dontPay = 0;
+        IPrimitiveEngine(engine).borrow(poolId, delLiquidity, type(uint256).max, data);
+        dontPay = 1;
     }
 
     function borrowCallback(
@@ -39,11 +64,34 @@ contract EngineBorrow {
         bytes calldata data
     ) public {
         uint256 riskyNeeded = delLiquidity - delRisky;
-
+        if (dontPay == 0) return;
         IERC20(risky).transferFrom(CALLER, msg.sender, riskyNeeded);
         IERC20(stable).transfer(CALLER, delStable);
     }
 
+    function repay(
+        bytes32 poolId,
+        address owner,
+        uint256 delLiquidity,
+        bool fromMargin,
+        bytes calldata data
+    )
+        external
+        returns (
+            uint256 delRisky,
+            uint256 delStable,
+            uint256 premium
+        )
+    {
+        CALLER = msg.sender;
+        IPrimitiveEngine(engine).repay(poolId, owner, delLiquidity, fromMargin, data);
+    }
+
+    function repayFromExternalCallback(uint256 delStable, bytes calldata data) external {
+        IERC20(stable).transferFrom(CALLER, msg.sender, delStable);
+        IERC20(risky).transfer(CALLER, IERC20(risky).balanceOf(address(this)));
+    }
+
     function getPosition(bytes32 poolId) public view returns (bytes32 posid) {
         posid = keccak256(abi.encodePacked(address(this), poolId));
     }

contracts/test/engine/EngineCreate.sol

@@ -51,8 +51,6 @@ contract EngineCreate {
         public
         view
         returns (
-            uint128 balanceRisky,
-            uint128 balanceStable,
             uint128 float,
             uint128 liquidity,
             uint128 debt

contracts/test/libraries/TestPosition.sol

@@ -28,8 +28,6 @@ contract TestPosition {
     function beforeEach(bytes32 poolId, uint256 liquidity) public {
         posId = Position.getPositionId(msg.sender, poolId);
         positions[posId] = Position.Data({
-            balanceRisky: 0,
-            balanceStable: 0,
             float: 0,
             liquidity: uint128(liquidity), // init with {liquidity} units of liquidity
             debt: 0
@@ -68,13 +66,13 @@ contract TestPosition {
         assert(post + uint128(amount) >= pre);
     }
 
-    /// @notice Increments debt and balanceRisky for a position
+    /// @notice Increments debt for a position
     function shouldBorrow(bytes32 poolId, uint256 amount) public {
         Position.Data memory pos = _shouldFetch(msg.sender, poolId);
-        uint128 pre = pos.balanceRisky;
+        uint128 pre = pos.debt;
         positions.borrow(poolId, amount);
         pos = _shouldFetch(msg.sender, poolId);
-        uint128 post = pos.balanceRisky;
+        uint128 post = pos.debt;
         assert(post >= uint128(amount) + pre);
     }
 

package.json

@@ -19,11 +19,12 @@
     "test:logs": "hardhat test --logs",
     "test:engine": "hardhat test ./test/unit/primitiveEngine",
     "test:factory": "hardhat test ./test/unit/primitiveFactory",
-    "test:swap": "hardhat test ./test/unit/primitiveEngine/effect/swap.ts",
     "test:create": "hardhat test ./test/unit/primitiveEngine/effect/create.ts",
     "test:deposit": "hardhat test ./test/unit/primitiveEngine/effect/deposit.ts",
     "test:allocate": "hardhat test ./test/unit/primitiveEngine/effect/allocate.ts",
     "test:remove": "hardhat test ./test/unit/primitiveEngine/effect/remove.ts",
+    "test:swap": "hardhat test ./test/unit/primitiveEngine/effect/swap.ts",
+    "test:borrow": "hardhat test ./test/unit/primitiveEngine/effect/borrow.ts",
     "test:lib": "hardhat test ./test/unit/libraries",
     "prepare": "husky install"
   },