Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] "Just in time liquidity" AKA "splits" vault #1

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
out/
cache/
node_modules/
.env
6 changes: 6 additions & 0 deletions .gitmodules
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[submodule "lib/forge-std"]
path = lib/forge-std
url = https://github.com/foundry-rs/forge-std
[submodule "lib/solmate"]
path = lib/solmate
url = https://github.com/rari-capital/solmate
9 changes: 9 additions & 0 deletions foundry.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
[default]
src = 'src'
out = 'out'
libs = ['lib']
remappings = [
'@openzeppelin/=lib/backed-protocol/node_modules/@openzeppelin/'
]

# See more config options https://github.com/foundry-rs/foundry/tree/master/config
1 change: 1 addition & 0 deletions lib/forge-std
Submodule forge-std added at 1680d7
1 change: 1 addition & 0 deletions lib/solmate
Submodule solmate added at 851ea3
4 changes: 4 additions & 0 deletions src/Contract.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

contract Contract {}
156 changes: 156 additions & 0 deletions src/LendController.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
// SPDX-License-Identifier: UNLICENSED
pragma solidity 0.8.13;

import {ERC721, ERC721TokenReceiver} from 'solmate/tokens/ERC721.sol';
import {SafeTransferLib, ERC20} from "solmate/utils/SafeTransferLib.sol";

import {INFTLoanFacilitator} from './interfaces/INFTLoanFacilitator.sol';

struct LoanTerms {
bool allowBuyouts;
bool allowLoanAmountIncrease;
uint16 interestRate;
uint128 amount;
uint32 durationSeconds;
address loanAsset;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this part of loan terms? it's included in the call

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need on L147

}

interface NFTLoanTermsSource {
function terms(uint256 tokenId, address nft, address loanAsset, bytes calldata data) external returns (LoanTerms memory);
}

contract LendController is ERC721TokenReceiver {
using SafeTransferLib for ERC20;

struct LenderSpec {
address lender;
uint256 amount;
bool pull;
}

struct BorrowRequest {
LenderSpec[] lendersSpecs;
address termsSource;
LoanTerms terms;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

surprised the borrow request specifies the terms rather than it being derived from the terms source, but makes sense so the terms returned from the source can be validated. think these likely should be diff structs though

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah was concerned about a race condition where terms contract is changed while borrow request tx is in the mempool. Agree could be different structs.

}

uint256 ONE = 1e18;
mapping(address => mapping(address => bool)) public termsApprovals;
mapping(address => mapping(address => uint256)) public lenderBalance;
mapping(uint256 => mapping(address => uint256)) public lenderLoanShares;

INFTLoanFacilitator public loanFacilitator;

constructor(INFTLoanFacilitator _facilitator) {
loanFacilitator = _facilitator;
}

event SetTermsApproval(address indexed from, address indexed termsContract, bool approved);

function setTermsApproval(address termsContract, bool approved) external {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we'll probably want to have an amount based allowance and decrementing against it as funds go out.

termsApprovals[msg.sender][termsContract] = approved;

emit SetTermsApproval(msg.sender, termsContract, approved);
}

function purchaseNFT() external {
// allows anyone to call in and seize a seize NFT that
// the controller holds the lend ticket for and pays the lenders
// purchase price must be >= totalOwed on loan
}

function liquidateNFT(uint256 loanId) external {
// TBD if controller will have one liquidation mechanism
// or will call out to the terms contract for implement
// update lenderBalance based on lenderLoanShares * sale value
}

// used to lend to an exisiting loan
function lend(
uint256 loanId,
uint256 tokenId,
address nft,
BorrowRequest memory request,
bytes calldata data
) external {
_lend(loanId, tokenId, nft, request, false, data);
}

function _lend(
uint256 loanId,
uint256 tokenId,
address nft,
BorrowRequest memory request,
bool skipIsBuyoutCheck,
bytes calldata data
) internal {
LoanTerms memory terms = NFTLoanTermsSource(request.termsSource).terms(tokenId, msg.sender, request.terms.loanAsset, '');

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this method make some checks as to whether terms from terms source match up with request.terms? ideally they're the same yes, but someone could maliciously pass mismatching?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will get checked in NFTLoanFacilitator, right? Must meet or beat.


ERC20 loanAsset = ERC20(request.terms.loanAsset);
uint256 lenderTotal;

for(uint i = 0; i < request.lendersSpecs.length; i++) {
LenderSpec memory info = request.lendersSpecs[i];
require(termsApprovals[info.lender][request.termsSource], 'lender has not approved terms source');

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you could potentially borrow from lenders with different terms contracts, where the total lent must be less than the min

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, but then need to do extra accounting on the way out? Can't assume everyone had the same rate, etc. But is interesting idea.


if(info.pull) {
ERC20(request.terms.loanAsset).safeTransferFrom(info.lender, address(this), info.amount);
} else {
lenderBalance[info.lender][address(loanAsset)] -= info.amount;
}
lenderTotal += info.amount;

lenderLoanShares[loanId][info.lender] = info.amount * ONE / terms.amount;
Copy link

@joshieDo joshieDo May 23, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lenderLoanShares[loanId] can be moved out of the loop for some gas savings for loops > 1 (example, example

mapping(address => uint256) storage loan = lenderLoanShares[loanId]
for(...) {
...
loan[info.lender] = info.amount * ONE / terms.amount;
}

and even accessing the fields on request might be better cached if used more than once, but i'm not 100% sure of that

}

loanAsset.approve(address(loanFacilitator), type(uint256).max);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ick from core

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agree, as I said below

Would also be nice if we used like a fulfillLend hook to allow the lender to provide the liquidity by any means. Not necessarily a transfer from msg.sender.


if(skipIsBuyoutCheck){
require(lenderTotal == terms.amount);
} else {
(,,, uint40 lastAccumulatedTimestamp,,,,,,,) = loanFacilitator.loanInfo(loanId);

if(lastAccumulatedTimestamp != 0) {
require(terms.allowBuyouts);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need to think if this is really the best way to do this / what we want to do. Alternatively could

  • require(terms.amount >= loanFacilitator.totalOwed(loanId))
  • or just not do any check? If the terms contract is willing to lend to this asset, it should be willing to buyout? Expecting to get buyout amount back on repayment. (Off chain will have to ensure that terms include a 10% improvement over existing).

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think cool to not do any check. borrow ticket holder could then use UI to see if theres any way they could get better terms on their loan

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well but this issue is, if you're a lender, are you OK more than X of your funds moving because you need to payback the previous lender, in addition to lending?

require(lenderTotal == terms.amount + loanFacilitator.interestOwed(loanId));
} else {
require(lenderTotal == terms.amount);
}
}

loanFacilitator.lend(
loanId,
terms.interestRate,
terms.amount,
terms.durationSeconds,
address(this)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this actually isn't quite right: we do not want to transfer the lend ticket to this contract. Because Backed has eager transfers for loan repayments, and we need to track whose funds are whose in the LendController, we will need to deploy a contract solely for the purpose of holding the lend ticket and receiving repayment. We will need one such contract for every lend ticket

Copy link
Contributor Author

@wilsoncusack wilsoncusack May 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-eager payouts are better for composability but worse for gas in the EOA case: eats into lender returns to have to come back and withdraw payback

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

with regards to collateral seizure, are any of the lenders able to call seizeCollateral? does this newly deployed contract need to then know about liquidation strategy?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the newly deployed contract would just allow the parent to make seize calls and send the collateral to itself/somewhere. parent (i.e. LendController) would control auction.

And yeah I think possibly anyone could call seizeCollateral, so long as what happens next (e.g. what kind of auction) is pre-agreed on

);
}

// used to create loan and lend to it in one transaction
function onERC721Received(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

using this onReceived hook allows us to prevent needing a multi call where users first createsLoan and then calls lend here. Although worse gas than multi call because NFT needs to go user -> LendController -> Loan Facilitator rather than user -> loan facilitator. But loan facilitator doesn't support multicall, anyway

Copy link
Contributor Author

@wilsoncusack wilsoncusack May 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be cool if the core backed contract, on loanCreate allowed the borrower to send the lend ticket somewhere via a safeTransferFrom, counting on an immediate loan fulfillment as the result of that transfer. So the LendController would receive the lend ticket, instead, check terms and just return loan payment.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would also be nice if we used like a fulfillLend hook to allow the lender to provide the liquidity by any means. Not necessarily a transfer from msg.sender.

address,
address from,
uint256 tokenId,
bytes calldata data
) external override returns (bytes4) {
BorrowRequest memory request = abi.decode(data, (BorrowRequest));

ERC721(msg.sender).setApprovalForAll(address(loanFacilitator), true);

uint256 loanId = loanFacilitator.createLoan(
tokenId,
msg.sender,
request.terms.interestRate,
request.terms.allowLoanAmountIncrease,
request.terms.amount,
request.terms.loanAsset,
request.terms.durationSeconds,
from
);

_lend(loanId, tokenId, msg.sender, request, true, data);

return ERC721TokenReceiver.onERC721Received.selector;
}
}
6 changes: 6 additions & 0 deletions src/Terms.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

contract LendController {

}
Loading