Merge branch 'feat/custodian-handler-changes-cleanup' of https://gith…

…ub.com/astariaxyz/starport into feat/custodian-handler-changes

.gas-snapshot

@@ -6,60 +6,61 @@ EnforcerTest:testFailRateEnforcerMaxRateAndMaxCarryRate() (gas: 787942)
 EnforcerTest:testRateEnforcerBasic() (gas: 908012)
 EnforcerTest:testTermEnforcerBasic() (gas: 980340)
 TestAstariaV1Loan:testNewLoanERC721CollateralDefaultTermsRecall() (gas: 969012)
-TestAstariaV1Loan:testNewLoanERC721CollateralDefaultTermsRecallLender() (gas: 859249)
-TestAstariaV1Loan:testNewLoanERC721CollateralDefaultTermsRecallLiquidation() (gas: 906166)
+TestAstariaV1Loan:testNewLoanERC721CollateralDefaultTermsRecallLender() (gas: 859410)
+TestAstariaV1Loan:testNewLoanERC721CollateralDefaultTermsRecallLiquidation() (gas: 906274)
 TestCustodian:testCannotLazyMintTwice() (gas: 76591)
 TestCustodian:testCannotMintInvalidLoanInvalidCustodian() (gas: 66811)
 TestCustodian:testCannotMintInvalidLoanValidCustodian() (gas: 72394)
-TestCustodian:testCustodySelector() (gas: 11040)
-TestCustodian:testGenerateOrderInvalidHandlerExecution() (gas: 147048)
-TestCustodian:testGenerateOrderRepay() (gas: 188954)
-TestCustodian:testGenerateOrderRepayAsRepayApprovedBorrower() (gas: 214528)
-TestCustodian:testGenerateOrderRepayERC1155AndERC20AndNative() (gas: 1129198)
-TestCustodian:testGenerateOrderRepayERC1155AndERC20AndNativeHandlerAuthorized() (gas: 1027917)
-TestCustodian:testGenerateOrderRepayERC1155WithRevert() (gas: 530479)
-TestCustodian:testGenerateOrderRepayInvalidHookAddress() (gas: 104747)
-TestCustodian:testGenerateOrderRepayInvalidHookReturnType() (gas: 99150)
-TestCustodian:testGenerateOrderRepayNotBorrower() (gas: 110738)
-TestCustodian:testGenerateOrderSettlement() (gas: 164446)
-TestCustodian:testGenerateOrderSettlementHandlerAuthorized() (gas: 172953)
-TestCustodian:testGenerateOrderSettlementNoActiveLoan() (gas: 107356)
-TestCustodian:testGenerateOrderSettlementUnauthorized() (gas: 108680)
-TestCustodian:testGetBorrower() (gas: 76052)
+TestCustodian:testCustodySelector() (gas: 2790258)
+TestCustodian:testDefaultCustodySelectorRevert() (gas: 11673)
+TestCustodian:testGenerateOrderInvalidHandlerExecution() (gas: 147388)
+TestCustodian:testGenerateOrderRepay() (gas: 189283)
+TestCustodian:testGenerateOrderRepayAsRepayApprovedBorrower() (gas: 214857)
+TestCustodian:testGenerateOrderRepayERC1155AndERC20AndNative() (gas: 1130044)
+TestCustodian:testGenerateOrderRepayERC1155AndERC20AndNativeHandlerAuthorized() (gas: 1029768)
+TestCustodian:testGenerateOrderRepayERC1155WithRevert() (gas: 530761)
+TestCustodian:testGenerateOrderRepayInvalidHookAddress() (gas: 104841)
+TestCustodian:testGenerateOrderRepayInvalidHookReturnType() (gas: 99244)
+TestCustodian:testGenerateOrderRepayNotBorrower() (gas: 110879)
+TestCustodian:testGenerateOrderSettlement() (gas: 164511)
+TestCustodian:testGenerateOrderSettlementHandlerAuthorized() (gas: 173617)
+TestCustodian:testGenerateOrderSettlementNoActiveLoan() (gas: 107403)
+TestCustodian:testGenerateOrderSettlementUnauthorized() (gas: 108744)
+TestCustodian:testGetBorrower() (gas: 76099)
 TestCustodian:testName() (gas: 7098)
 TestCustodian:testNonPayableFunctions() (gas: 247767)
 TestCustodian:testOnlySeaport() (gas: 17931)
 TestCustodian:testPayableFunctions() (gas: 43468)
-TestCustodian:testPreviewOrderNoActiveLoan() (gas: 104868)
-TestCustodian:testPreviewOrderRepay() (gas: 243281)
-TestCustodian:testPreviewOrderSettlement() (gas: 195956)
-TestCustodian:testPreviewOrderSettlementInvalidFufliller() (gas: 106598)
-TestCustodian:testPreviewOrderSettlementInvalidRepayer() (gas: 112596)
-TestCustodian:testRatifyOrder() (gas: 195072)
+TestCustodian:testPreviewOrderNoActiveLoan() (gas: 104915)
+TestCustodian:testPreviewOrderRepay() (gas: 243798)
+TestCustodian:testPreviewOrderSettlement() (gas: 195979)
+TestCustodian:testPreviewOrderSettlementInvalidFufliller() (gas: 106672)
+TestCustodian:testPreviewOrderSettlementInvalidRepayer() (gas: 112759)
+TestCustodian:testRatifyOrder() (gas: 195401)
 TestCustodian:testSafeTransferReceive() (gas: 159065)
 TestCustodian:testSeaportMetadata() (gas: 8501)
 TestCustodian:testSetRepayApproval() (gas: 37907)
 TestCustodian:testSupportsInterface() (gas: 9428)
 TestCustodian:testSymbol() (gas: 7105)
 TestCustodian:testTokenURI() (gas: 64811)
 TestCustodian:testTokenURIInvalidLoan() (gas: 13196)
-TestExoticLoans:testSwap() (gas: 1334111)
+TestExoticLoans:testSwap() (gas: 1353956)
 TestLoanCombinations:testLoan20For721SimpleInterestDutchFixedRepay() (gas: 164)
 TestLoanCombinations:testLoan20for20SimpleInterestDutchFixedRepay() (gas: 539940)
-TestLoanCombinations:testLoan721for20SimpleInterestDutchFixedRepay() (gas: 756028)
+TestLoanCombinations:testLoan721for20SimpleInterestDutchFixedRepay() (gas: 756253)
 TestLoanCombinations:testLoanAstariaSettlementRepay() (gas: 575128)
-TestLoanCombinations:testLoanSimpleInterestEnglishFixed() (gas: 747673)
+TestLoanCombinations:testLoanSimpleInterestEnglishFixed() (gas: 747899)
 TestLoanManager:testGenerateOrder() (gas: 1005642)
 TestLoanManager:testGenerateOrderNotSeaport() (gas: 13074)
 TestLoanManager:testName() (gas: 7206)
 TestLoanManager:testSupportsInterface() (gas: 9579)
 TestLoanManager:testSymbol() (gas: 7148)
-TestNewLoan:testBuyNowPayLater() (gas: 1148679)
-TestNewLoan:testNewLoanERC721CollateralDefaultTerms2():((uint256,address,address,address,address,(uint8,address,uint256,uint256)[],(uint8,address,uint256,uint256)[],(address,bytes,address,bytes,address,bytes))) (gas: 979129)
-TestNewLoan:testNewLoanERC721CollateralDefaultTermsRefinance() (gas: 639900)
-TestNewLoan:testNewLoanERC721CollateralLessDebtThanOffered():((uint256,address,address,address,address,(uint8,address,uint256,uint256)[],(uint8,address,uint256,uint256)[],(address,bytes,address,bytes,address,bytes))) (gas: 979463)
-TestNewLoan:testSettleLoan() (gas: 1233491)
-TestRepayLoan:testRepayLoan() (gas: 694472)
+TestNewLoan:testBuyNowPayLater() (gas: 1148671)
+TestNewLoan:testNewLoanERC721CollateralDefaultTerms2():((uint256,address,address,address,address,(uint8,address,uint256,uint256)[],(uint8,address,uint256,uint256)[],(address,bytes,address,bytes,address,bytes))) (gas: 979121)
+TestNewLoan:testNewLoanERC721CollateralDefaultTermsRefinance() (gas: 639892)
+TestNewLoan:testNewLoanERC721CollateralLessDebtThanOffered():((uint256,address,address,address,address,(uint8,address,uint256,uint256)[],(uint8,address,uint256,uint256)[],(address,bytes,address,bytes,address,bytes))) (gas: 979455)
+TestNewLoan:testSettleLoan() (gas: 1233583)
+TestRepayLoan:testRepayLoan() (gas: 694754)
 TestStarLiteUtils:testEncodeReceivedWithRecipient() (gas: 17955)
 TestStarLiteUtils:testSpentToReceived() (gas: 17708)
 TestStarLiteUtils:testValidateSaltOpt(address,bytes32) (runs: 256, μ: 26479, ~: 26479)

src/Custodian.sol

@@ -27,17 +27,17 @@ import {ERC1155} from "solady/src/tokens/ERC1155.sol";
 import {ItemType, Schema, SpentItem, ReceivedItem} from "seaport-types/src/lib/ConsiderationStructs.sol";
 
 import {ContractOffererInterface} from "seaport-types/src/interfaces/ContractOffererInterface.sol";
+import {ConduitHelper} from "starport-core/ConduitHelper.sol";
 import {TokenReceiverInterface} from "starport-core/interfaces/TokenReceiverInterface.sol";
 import {FixedPointMathLib} from "solady/src/utils/FixedPointMathLib.sol";
 import {Originator} from "starport-core/originators/Originator.sol";
 import {SettlementHook} from "starport-core/hooks/SettlementHook.sol";
 import {SettlementHandler} from "starport-core/handlers/SettlementHandler.sol";
 import {Pricing} from "starport-core/pricing/Pricing.sol";
 import {LoanManager} from "starport-core/LoanManager.sol";
-import {ConduitHelper} from "starport-core/ConduitHelper.sol";
 import {StarPortLib} from "starport-core/lib/StarPortLib.sol";
 
-abstract contract Custodian is ContractOffererInterface, TokenReceiverInterface, ConduitHelper, ERC721 {
+contract Custodian is ERC721, ContractOffererInterface, ConduitHelper, TokenReceiverInterface {
     using {StarPortLib.getId} for LoanManager.Loan;
 
     LoanManager public immutable LM;
@@ -54,6 +54,7 @@ abstract contract Custodian is ContractOffererInterface, TokenReceiverInterface,
     error InvalidFulfiller();
     error InvalidHandlerExecution();
     error InvalidLoan();
+    error ImplementInChild();
 
     constructor(LoanManager LM_, address seaport_) {
         seaport = seaport_;
@@ -167,7 +168,9 @@ abstract contract Custodian is ContractOffererInterface, TokenReceiverInterface,
         bytes32[] calldata orderHashes,
         uint256 contractNonce,
         bytes calldata context
-    ) external virtual onlyLoanManager returns (bytes4 selector);
+    ) external virtual onlyLoanManager returns (bytes4 selector) {
+        revert ImplementInChild();
+    }
 
     //todo work with seaport
     function getSeaportMetadata() external pure returns (string memory, Schema[] memory schemas) {
@@ -220,12 +223,10 @@ abstract contract Custodian is ContractOffererInterface, TokenReceiverInterface,
             address authorized;
             (consideration, authorized) = SettlementHandler(loan.terms.handler).getSettlement(loan);
 
-            if (authorized == loan.terms.handler || (fulfiller != authorized && authorized == loan.issuer)) {} else if (
-                authorized != address(0) && fulfiller != authorized
-            ) {
-                revert InvalidFulfiller();
-            } else {
+            if (authorized == address(0) || fulfiller == authorized) {
                 offer = loan.collateral;
+            } else if (authorized == loan.terms.handler || authorized == loan.issuer) {} else {
+                revert InvalidFulfiller();
             }
         }
     }
@@ -291,23 +292,24 @@ abstract contract Custodian is ContractOffererInterface, TokenReceiverInterface,
             (consideration, authorized) = SettlementHandler(loan.terms.handler).getSettlement(loan);
             _afterSettlementHandlerHook(loan);
 
-            if (authorized == loan.terms.handler || (fulfiller != authorized && authorized == loan.issuer)) {
+            if (authorized == address(0) || fulfiller == authorized) {
+                offer = loan.collateral;
+                _beforeApprovalsSetHook(fulfiller, maximumSpent, context);
+                _setOfferApprovalsWithSeaport(offer);
+            } else if (authorized == loan.terms.handler || authorized == loan.issuer) {
                 _moveDebtToAuthorized(loan.collateral, authorized);
                 if (
                     authorized == loan.terms.handler
-                        && SettlementHandler(loan.terms.handler).execute(loan) != SettlementHandler.execute.selector
+                        && SettlementHandler(loan.terms.handler).execute(loan, fulfiller)
+                            != SettlementHandler.execute.selector
                 ) {
                     revert InvalidHandlerExecution();
                 }
-                _settleLoan(loan);
-            } else if (authorized != address(0) && fulfiller != authorized) {
-                revert InvalidFulfiller();
             } else {
-                offer = loan.collateral;
-                _beforeApprovalsSetHook(fulfiller, maximumSpent, context);
-                _setOfferApprovalsWithSeaport(offer);
-                _settleLoan(loan);
+                revert InvalidFulfiller();
             }
+
+            _settleLoan(loan);
         }
     }
 

src/LoanManager.sol

@@ -48,7 +48,7 @@ interface LoanSettledCallback {
     function onLoanSettled(LoanManager.Loan calldata loan) external;
 }
 
-contract LoanManager is ERC721, ContractOffererInterface, ConduitHelper, Ownable {
+contract LoanManager is ContractOffererInterface, ConduitHelper, Ownable, ERC721 {
     using FixedPointMathLib for uint256;
 
     using {StarPortLib.toReceivedItems} for SpentItem[];

src/handlers/AstariaV1SettlementHandler.sol

@@ -83,6 +83,10 @@ contract AstariaV1SettlementHandler is DutchAuctionHandler {
         consideration = _removeZeroAmounts(consideration);
     }
 
+    function execute(LoanManager.Loan calldata loan, address fulfiller) external virtual override returns (bytes4) {
+        return SettlementHandler.execute.selector;
+    }
+
     function validate(LoanManager.Loan calldata loan) external view virtual override returns (bool) {
         return true;
     }

src/handlers/DutchAuctionHandler.sol

@@ -32,6 +32,10 @@ abstract contract DutchAuctionHandler is SettlementHandler, AmountDeriver, Condu
         uint256 window;
     }
 
+    function execute(LoanManager.Loan calldata loan, address fulfiller) external virtual override returns (bytes4) {
+        return SettlementHandler.execute.selector;
+    }
+
     function _getAuctionStart(LoanManager.Loan memory loan) internal view virtual returns (uint256);
 
     function getSettlement(LoanManager.Loan calldata loan)

src/handlers/EnglishAuctionHandler.sol

@@ -55,8 +55,8 @@ contract EnglishAuctionHandler is SettlementHandler {
         return details.reservePrice.length == loan.debt.length;
     }
 
-    function execute(LoanManager.Loan calldata loan) external virtual override returns (bytes4 selector) {
-        selector = SettlementHandler.execute.selector;
+    function execute(LoanManager.Loan calldata loan, address fulfiller) external virtual override returns (bytes4) {
+        return SettlementHandler.execute.selector;
     }
 
     function getSettlement(LoanManager.Loan calldata loan)

src/handlers/FixedTermDutchAuctionHandler.sol

@@ -18,6 +18,10 @@ contract FixedTermDutchAuctionHandler is DutchAuctionHandler {
         return loan.start + details.loanDuration;
     }
 
+    function execute(LoanManager.Loan calldata loan, address fulfiller) external virtual override returns (bytes4) {
+        return SettlementHandler.execute.selector;
+    }
+
     function validate(LoanManager.Loan calldata loan) external view virtual override returns (bool) {
         return true;
     }

src/handlers/SettlementHandler.sol

@@ -32,9 +32,7 @@ abstract contract SettlementHandler is TokenReceiverInterface {
         LM = LM_;
     }
 
-    function execute(LoanManager.Loan calldata loan) external virtual returns (bytes4) {
-        return SettlementHandler.execute.selector;
-    }
+    function execute(LoanManager.Loan calldata loan, address fulfiller) external virtual returns (bytes4);
 
     function validate(LoanManager.Loan calldata loan) external view virtual returns (bool);
 

src/pricing/Pricing.sol

@@ -22,8 +22,6 @@ pragma solidity =0.8.17;
 
 import {LoanManager} from "starport-core/LoanManager.sol";
 import {ReceivedItem} from "seaport-types/src/lib/ConsiderationStructs.sol";
-import "forge-std/console.sol";
-import "seaport/lib/seaport-sol/src/lib/ReceivedItemLib.sol";
 
 abstract contract Pricing {
     LoanManager LM;

test/StarPortTest.sol

@@ -186,7 +186,7 @@ contract StarPortTest is BaseOrderTest {
         refinancer = makeAndAllocateAccount("refinancer");
 
         LM = new LoanManager(consideration);
-        custodian = new Custodian(LM, seaportAddr);
+        custodian = Custodian(payable(LM.defaultCustodian()));
         UO = new UniqueOriginator(LM, strategist.addr, 1e16, address(this));
         pricing = new SimpleInterestPricing(LM);
         handler = new FixedTermDutchAuctionHandler(LM);

test/TestCustodian.sol

@@ -4,6 +4,19 @@ import {MockCall} from "starport-test/utils/MockCall.sol";
 import "forge-std/Test.sol";
 import {StarPortLib} from "starport-core/lib/StarPortLib.sol";
 
+contract MockCustodian is Custodian {
+    constructor(LoanManager LM_, address seaport_) Custodian(LM_, seaport_) {}
+
+    function custody(
+        ReceivedItem[] calldata consideration,
+        bytes32[] calldata orderHashes,
+        uint256 contractNonce,
+        bytes calldata context
+    ) external virtual override onlyLoanManager returns (bytes4 selector) {
+        selector = Custodian.custody.selector;
+    }
+}
+
 contract TestCustodian is StarPortTest, DeepEq, MockCall {
     using Cast for *;
 
@@ -216,13 +229,20 @@ contract TestCustodian is StarPortTest, DeepEq, MockCall {
     }
 
     function testCustodySelector() public {
-        vm.prank(address(custodian.LM()));
+        MockCustodian custodianMock = new MockCustodian(LM, seaportAddr);
+        vm.prank(address(custodianMock.LM()));
         assert(
-            custodian.custody(new ReceivedItem[](0), new bytes32[](0), uint256(0), new bytes(0))
+            custodianMock.custody(new ReceivedItem[](0), new bytes32[](0), uint256(0), new bytes(0))
                 == Custodian.custody.selector
         );
     }
 
+    function testDefaultCustodySelectorRevert() public {
+        vm.prank(address(custodian.LM()));
+        vm.expectRevert(abi.encodeWithSelector(Custodian.ImplementInChild.selector));
+        custodian.custody(new ReceivedItem[](0), new bytes32[](0), uint256(0), new bytes(0));
+    }
+
     //TODO: add assertions
     function testGenerateOrderRepay() public {
         vm.prank(seaportAddr);

test/TestExoticLoans.t.sol

@@ -54,7 +54,7 @@ contract SwapHook is SettlementHook {
 contract SwapHandler is SettlementHandler {
     constructor(LoanManager LM_) SettlementHandler(LM_) {}
 
-    function execute(LoanManager.Loan calldata loan) external override returns (bytes4) {
+    function execute(LoanManager.Loan calldata loan, address fulfiller) external override returns (bytes4) {
         return bytes4(0);
     }