feat/custodian settlement flow adjustment (#51)

* AstariaV1Pricing tests, coverage, and cleanup, new snapshot

* clean up if else to adopt {} braces

* remove console imports from core and modules

* newest snapshot

* always execute settlement flow even on repayments

* update snapshot and disable fuzz testing for snapshot check

* updates to the repayApproval flow, so its now using the underlying erc721 approvals, new method for borrowers to mint with approval set

* update coverage and add additional tests for reverts

* remove on Loan settled callback from starport, can be done via new postSettlement/postRepayment flows

* snapshot and remove loansettled callback import

* remove dead test

.github/workflows/lint_test.yml

@@ -32,7 +32,7 @@ jobs:
         run: forge test -vvv
 
       - name: Snapshot
-        run: forge snapshot --check
+        run: forge snapshot --check --no-match-path '*fuzz*'
 
 
 

src/Custodian.sol

@@ -49,9 +49,11 @@ contract Custodian is ERC721, ContractOffererInterface {
     error ImplementInChild();
     error InvalidAction();
     error InvalidFulfiller();
-    error InvalidHandlerExecution();
+    error InvalidPostSettlement();
+    error InvalidPostRepayment();
     error InvalidLoan();
     error InvalidRepayer();
+    error NotAuthorized();
     error NotSeaport();
     error NotEnteredViaSeaport();
     error NotStarport();
@@ -154,16 +156,24 @@ contract Custodian is ERC721, ContractOffererInterface {
 
         _safeMint(loan.borrower, loanId, encodedLoan);
     }
-
     /**
-     * @dev Set's approvals for who can repay a loan on behalf of the borrower.
+     * @dev Mints a custody token for a loan.
      *
-     * @param who              The address of the account to modify approval for
-     * @param approved         The approval status
+     * @param loan             The loan to mint a custody token for
+     * @param approvedTo       The address with pre approvals set
      */
-    function setRepayApproval(address who, bool approved) external {
-        repayApproval[msg.sender][who] = approved;
-        emit RepayApproval(msg.sender, who, approved);
+
+    function mintWithApprovalSet(Starport.Loan calldata loan, address approvedTo) external {
+        bytes memory encodedLoan = abi.encode(loan);
+        uint256 loanId = uint256(keccak256(encodedLoan));
+        if (loan.custodian != address(this) || !SP.active(loanId)) {
+            revert InvalidLoan();
+        }
+        if (msg.sender != loan.borrower) {
+            revert NotAuthorized();
+        }
+        _safeMint(loan.borrower, loanId, encodedLoan);
+        _approve(loan.borrower, approvedTo, loanId);
     }
 
     /**
@@ -208,7 +218,7 @@ contract Custodian is ERC721, ContractOffererInterface {
         }
         if (action == Actions.Repayment && Status(loan.terms.status).isActive(loan)) {
             address borrower = getBorrower(loan);
-            if (fulfiller != borrower && !repayApproval[borrower][fulfiller]) {
+            if (fulfiller != borrower && fulfiller != _getApproved(loan.getId())) {
                 revert InvalidRepayer();
             }
 
@@ -222,6 +232,7 @@ contract Custodian is ERC721, ContractOffererInterface {
             consideration = StarportLib.mergeSpentItemsToReceivedItems(payment, loan.issuer, carry, loan.originator);
 
             _settleLoan(loan);
+            _postRepaymentExecute(loan, fulfiller);
         } else if (action == Actions.Settlement && !Status(loan.terms.status).isActive(loan)) {
             address authorized;
             //add in originator fee
@@ -236,18 +247,11 @@ contract Custodian is ERC721, ContractOffererInterface {
                 _setOfferApprovalsWithSeaport(offer);
             } else if (authorized == loan.terms.settlement || authorized == loan.issuer) {
                 _moveCollateralToAuthorized(loan.collateral, authorized);
-                _beforeSettlementHandlerHook(loan);
-                if (
-                    authorized == loan.terms.settlement
-                        && Settlement(loan.terms.settlement).execute(loan, fulfiller) != Settlement.execute.selector
-                ) {
-                    revert InvalidHandlerExecution();
-                }
-                _afterSettlementHandlerHook(loan);
             } else {
                 revert InvalidFulfiller();
             }
             _settleLoan(loan);
+            _postSettlementExecute(loan, fulfiller);
         } else {
             revert InvalidAction();
         }
@@ -400,6 +404,40 @@ contract Custodian is ERC721, ContractOffererInterface {
         }
     }
 
+    /**
+     * @dev settle the loan with the LoanManager
+     *
+     * @param loan              The the loan that is settled
+     * @param fulfiller      The address executing seaport
+     */
+    function _postSettlementExecute(Starport.Loan memory loan, address fulfiller) internal virtual {
+        _beforeSettlementHandlerHook(loan);
+        if (
+            Settlement(loan.terms.settlement).postSettlement{gas: 100_000}(loan, fulfiller)
+                != Settlement.postSettlement.selector
+        ) {
+            revert InvalidPostSettlement();
+        }
+        _afterSettlementHandlerHook(loan);
+    }
+    /**
+     * @dev settle the loan with the LoanManager
+     *
+     * @param loan              The the loan that is settled
+     * @param fulfiller      The address executing seaport
+     */
+
+    function _postRepaymentExecute(Starport.Loan memory loan, address fulfiller) internal virtual {
+        _beforeSettlementHandlerHook(loan);
+        if (
+            Settlement(loan.terms.settlement).postRepayment{gas: 100_000}(loan, fulfiller)
+                != Settlement.postRepayment.selector
+        ) {
+            revert InvalidPostRepayment();
+        }
+        _afterSettlementHandlerHook(loan);
+    }
+
     /**
      * @dev settle the loan with the LoanManager
      *

src/Starport.sol

@@ -37,10 +37,6 @@ import {Ownable} from "solady/src/auth/Ownable.sol";
 import {SafeTransferLib} from "solady/src/utils/SafeTransferLib.sol";
 import {PausableNonReentrant} from "starport-core/lib/PausableNonReentrant.sol";
 
-interface LoanSettledCallback {
-    function onLoanSettled(Starport.Loan calldata loan) external;
-}
-
 contract Starport is ERC721, PausableNonReentrant {
     using FixedPointMathLib for uint256;
 
@@ -473,10 +469,6 @@ contract Starport is ERC721, PausableNonReentrant {
             _burn(tokenId);
         }
         _setExtraData(tokenId, uint8(FieldFlags.INACTIVE));
-
-        if (loan.issuer.code.length > 0) {
-            loan.issuer.call(abi.encodeWithSelector(LoanSettledCallback.onLoanSettled.selector, loan));
-        }
         emit Close(tokenId);
     }
 

src/settlement/AstariaV1Settlement.sol

@@ -6,7 +6,6 @@ import {BaseRecall} from "starport-core/status/BaseRecall.sol";
 import {DutchAuctionSettlement} from "starport-core/settlement/DutchAuctionSettlement.sol";
 import {StarportLib} from "starport-core/lib/StarportLib.sol";
 import {FixedPointMathLib} from "solady/src/utils/FixedPointMathLib.sol";
-
 import {Pricing} from "starport-core/pricing/Pricing.sol";
 import {BasePricing} from "starport-core/pricing/BasePricing.sol";
 
@@ -145,8 +144,19 @@ contract AstariaV1Settlement is DutchAuctionSettlement {
         }
     }
 
-    function execute(Starport.Loan calldata loan, address fulfiller) external virtual override returns (bytes4) {
-        revert ExecuteHandlerNotImplemented();
+    function postSettlement(Starport.Loan calldata loan, address fulfiller)
+        external
+        virtual
+        override
+        returns (bytes4)
+    {
+        //        TODO: do we need the commented out code if we dont care about reverts anyways, seems like extra gas
+        //        (address recaller, uint64 recallStart) = BaseRecall(loan.terms.status).recalls(loan.getId());
+        //        if (recallStart != 0 || recaller != address(0)) {
+        //we dont wanna revert if theres ever a halt in the underlying call, settlement must complete
+        loan.terms.status.call(abi.encodeWithSelector(BaseRecall.withdraw.selector, loan, fulfiller));
+        //        }
+        return Settlement.postSettlement.selector;
     }
 
     function validate(Starport.Loan calldata loan) external view virtual override returns (bool) {

src/settlement/DutchAuctionSettlement.sol

@@ -30,8 +30,17 @@ abstract contract DutchAuctionSettlement is Settlement, AmountDeriver {
         uint256 window;
     }
 
-    function execute(Starport.Loan calldata loan, address fulfiller) external virtual override returns (bytes4) {
-        return Settlement.execute.selector;
+    function postSettlement(Starport.Loan calldata loan, address fulfiller)
+        external
+        virtual
+        override
+        returns (bytes4)
+    {
+        return Settlement.postSettlement.selector;
+    }
+
+    function postRepayment(Starport.Loan calldata loan, address fulfiller) external virtual override returns (bytes4) {
+        return Settlement.postRepayment.selector;
     }
 
     function getAuctionStart(Starport.Loan calldata loan) public view virtual returns (uint256);

src/settlement/EnglishAuctionSettlement.sol

@@ -25,6 +25,7 @@ import {
     OrderType
 } from "seaport-types/src/lib/ConsiderationStructs.sol";
 import {Pricing} from "starport-core/pricing/Pricing.sol";
+import {Status} from "starport-core/status/Status.sol";
 
 contract EnglishAuctionSettlement is Settlement {
     using FixedPointMathLib for uint256;
@@ -55,11 +56,20 @@ contract EnglishAuctionSettlement is Settlement {
         return details.reservePrice.length == loan.debt.length;
     }
 
-    function execute(Starport.Loan calldata loan, address fulfiller) external virtual override returns (bytes4) {
+    function postSettlement(Starport.Loan calldata loan, address fulfiller)
+        external
+        virtual
+        override
+        returns (bytes4)
+    {
         if (fulfiller != address(this)) {
             revert("must liquidate via the handler to trigger english auction");
         }
-        return Settlement.execute.selector;
+        return Settlement.postSettlement.selector;
+    }
+
+    function postRepayment(Starport.Loan calldata loan, address fulfiller) external virtual override returns (bytes4) {
+        return Settlement.postRepayment.selector;
     }
 
     function getSettlement(Starport.Loan calldata loan)

src/settlement/FixedTermDutchAuctionSettlement.sol

@@ -18,8 +18,13 @@ contract FixedTermDutchAuctionSettlement is DutchAuctionSettlement {
         return loan.start + details.loanDuration;
     }
 
-    function execute(Starport.Loan calldata loan, address fulfiller) external virtual override returns (bytes4) {
-        return Settlement.execute.selector;
+    function postSettlement(Starport.Loan calldata loan, address fulfiller)
+        external
+        virtual
+        override
+        returns (bytes4)
+    {
+        return Settlement.postSettlement.selector;
     }
 
     function validate(Starport.Loan calldata loan) external view virtual override returns (bool) {

src/settlement/Settlement.sol

@@ -32,7 +32,9 @@ abstract contract Settlement is TokenReceiverInterface {
         SP = SP_;
     }
 
-    function execute(Starport.Loan calldata loan, address fulfiller) external virtual returns (bytes4);
+    function postSettlement(Starport.Loan calldata loan, address fulfiller) external virtual returns (bytes4);
+
+    function postRepayment(Starport.Loan calldata loan, address fulfiller) external virtual returns (bytes4);
 
     function validate(Starport.Loan calldata loan) external view virtual returns (bool);
 

src/status/BaseRecall.sol

@@ -117,7 +117,7 @@ abstract contract BaseRecall {
     }
 
     // transfers all stake to anyone who asks after the LM token is burned
-    function withdraw(Starport.Loan calldata loan, address payable receiver) external {
+    function withdraw(Starport.Loan calldata loan, address receiver) external {
         Details memory details = abi.decode(loan.terms.statusData, (Details));
         bytes memory encodedLoan = abi.encode(loan);
         uint256 loanId = uint256(keccak256(encodedLoan));
@@ -171,12 +171,11 @@ abstract contract BaseRecall {
         }
     }
 
-    function generateRecallConsideration(
-        Starport.Loan calldata loan,
-        uint256 proportion,
-        address from,
-        address payable to
-    ) external view returns (AdditionalTransfer[] memory consideration) {
+    function generateRecallConsideration(Starport.Loan calldata loan, uint256 proportion, address from, address to)
+        external
+        view
+        returns (AdditionalTransfer[] memory consideration)
+    {
         Details memory details = abi.decode(loan.terms.statusData, (Details));
         return _generateRecallConsideration(loan, 0, details.recallStakeDuration, proportion, from, to);
     }
@@ -187,7 +186,7 @@ abstract contract BaseRecall {
         uint256 end,
         uint256 proportion,
         address from,
-        address payable to
+        address to
     ) internal view returns (AdditionalTransfer[] memory additionalTransfers) {
         uint256[] memory stake = _getRecallStake(loan, start, end);
         additionalTransfers = new AdditionalTransfer[](stake.length);

test/AstariaV1Test.sol

@@ -29,12 +29,12 @@ contract AstariaV1Test is StarportTest {
 
         pricing = new AstariaV1Pricing(SP);
         settlement = new AstariaV1Settlement(SP);
-        hook = new AstariaV1Status(SP);
+        status = new AstariaV1Status(SP);
 
         lenderEnforcer = new AstariaV1LenderEnforcer();
 
         vm.startPrank(recaller.addr);
-        erc20s[0].approve(address(hook), 1e18);
+        erc20s[0].approve(address(status), 1e18);
         vm.stopPrank();
 
         // // 1% interest rate per second

test/StarportTest.sol

@@ -61,7 +61,6 @@ import {ERC721} from "solady/src/tokens/ERC721.sol";
 import {ERC1155} from "solady/src/tokens/ERC1155.sol";
 import {ContractOffererInterface} from "seaport-types/src/interfaces/ContractOffererInterface.sol";
 import {TokenReceiverInterface} from "starport-core/interfaces/TokenReceiverInterface.sol";
-import {LoanSettledCallback} from "starport-core/Starport.sol";
 import {Actions} from "starport-core/lib/StarportLib.sol";
 
 import {CaveatEnforcer} from "starport-core/enforcers/CaveatEnforcer.sol";
@@ -77,9 +76,7 @@ interface IWETH9 {
     function withdraw(uint256) external;
 }
 
-contract MockIssuer is LoanSettledCallback, TokenReceiverInterface {
-    function onLoanSettled(Starport.Loan memory loan) external {}
-
+contract MockIssuer is TokenReceiverInterface {
     function onERC721Received(address operator, address from, uint256 tokenId, bytes calldata data)
         external
         override
@@ -128,7 +125,7 @@ contract StarportTest is BaseOrderTest {
 
     Pricing pricing;
     Settlement settlement;
-    Status hook;
+    Status status;
 
     uint256 defaultLoanDuration = 14 days;
 
@@ -205,7 +202,7 @@ contract StarportTest is BaseOrderTest {
         SO = new StrategistOriginator(SP, strategist.addr, 1e16, address(this));
         pricing = new SimpleInterestPricing(SP);
         settlement = new FixedTermDutchAuctionSettlement(SP);
-        hook = new FixedTermStatus();
+        status = new FixedTermStatus();
         vm.label(address(erc721s[0]), "Collateral NFT");
         vm.label(address(erc721s[1]), "Collateral2 NFT");
         vm.label(address(erc20s[0]), "Debt ERC20");
@@ -484,7 +481,7 @@ contract StarportTest is BaseOrderTest {
             collateral: newCollateral,
             debt: newDebt,
             terms: Starport.Terms({
-                status: address(hook),
+                status: address(status),
                 settlement: address(settlement),
                 pricing: address(pricing),
                 pricingData: defaultPricingData,