feat/Standardize Validate method for modules /w test cleanup: remove …

…unused internals, conduits (#72)

* test cleanup: remove unused internals, conduits

* standardize the validate method with a new validation abstract

* add test for the validate methods

* chore(bot): format & snapshot

---------

Co-authored-by: 0xgregthedev <[email protected]>

.gas-snapshot

@@ -13,9 +13,10 @@ IntegrationTestCaveats:testRefinanceUnapprovedFulfiller() (gas: 495425)
 IntegrationTestCaveats:testRefinanceWCaveatsInvalidSalt() (gas: 414216)
 IntegrationTestCaveats:testRefinanceWLenderApproval() (gas: 437545)
 ModuleTesting:testFixedTermDutchAuctionSettlement() (gas: 436187)
-ModuleTesting:testFixedTermDutchAuctionSettlementGetSettlementAuctionExpired() (gas: 438965)
-ModuleTesting:testFixedTermDutchAuctionSettlementNotValid() (gas: 435075)
-ModuleTesting:testFixedTermDutchAuctionSettlementValid() (gas: 435939)
+ModuleTesting:testFixedTermDutchAuctionSettlementGetSettlementAuctionExpired() (gas: 439054)
+ModuleTesting:testFixedTermDutchAuctionSettlementNotValid() (gas: 435177)
+ModuleTesting:testFixedTermDutchAuctionSettlementValid() (gas: 435998)
+ModuleTesting:testModuleValidation() (gas: 1272828)
 PausableNonReentrantImpl:test() (gas: 2442)
 PausableNonReentrantImpl:testReentrancy() (gas: 2735)
 TestBorrowerEnforcer:testBERevertAdditionalTransfers() (gas: 75662)
@@ -25,21 +26,21 @@ TestBorrowerEnforcer:testBEValidLoanTermsAnyIssuer() (gas: 72085)
 TestCustodian:testCannotLazyMintTwice() (gas: 78796)
 TestCustodian:testCannotMintInvalidLoanInvalidCustodian() (gas: 69031)
 TestCustodian:testCannotMintInvalidLoanValidCustodian() (gas: 74555)
-TestCustodian:testCustodySelector() (gas: 2830993)
+TestCustodian:testCustodySelector() (gas: 2908178)
 TestCustodian:testDefaultCustodySelectorRevert() (gas: 72312)
-TestCustodian:testGenerateOrderInvalidPostRepayment() (gas: 178774)
-TestCustodian:testGenerateOrderInvalidPostSettlement() (gas: 163188)
-TestCustodian:testGenerateOrderRepay() (gas: 182892)
-TestCustodian:testGenerateOrderRepayAsRepayApprovedBorrower() (gas: 199361)
-TestCustodian:testGenerateOrderRepayERC1155AndERC20() (gas: 882157)
-TestCustodian:testGenerateOrderRepayERC1155AndERC20HandlerAuthorized() (gas: 803591)
+TestCustodian:testGenerateOrderInvalidPostRepayment() (gas: 173324)
+TestCustodian:testGenerateOrderInvalidPostSettlement() (gas: 163179)
+TestCustodian:testGenerateOrderRepay() (gas: 177479)
+TestCustodian:testGenerateOrderRepayAsRepayApprovedBorrower() (gas: 193948)
+TestCustodian:testGenerateOrderRepayERC1155AndERC20() (gas: 874180)
+TestCustodian:testGenerateOrderRepayERC1155AndERC20HandlerAuthorized() (gas: 803573)
 TestCustodian:testGenerateOrderRepayInvalidHookAddress() (gas: 97601)
 TestCustodian:testGenerateOrderRepayInvalidHookReturnType() (gas: 91984)
-TestCustodian:testGenerateOrderRepayNotBorrower() (gas: 106802)
-TestCustodian:testGenerateOrderSettlement() (gas: 154943)
-TestCustodian:testGenerateOrderSettlementHandlerAuthorized() (gas: 160340)
-TestCustodian:testGenerateOrderSettlementNoActiveLoan() (gas: 163351)
-TestCustodian:testGenerateOrderSettlementUnauthorized() (gas: 101822)
+TestCustodian:testGenerateOrderRepayNotBorrower() (gas: 106839)
+TestCustodian:testGenerateOrderSettlement() (gas: 154934)
+TestCustodian:testGenerateOrderSettlementHandlerAuthorized() (gas: 160331)
+TestCustodian:testGenerateOrderSettlementNoActiveLoan() (gas: 163342)
+TestCustodian:testGenerateOrderSettlementUnauthorized() (gas: 101813)
 TestCustodian:testGenerateOrdersWithLoanStartAtBlockTimestampInvalidLoan() (gas: 461053)
 TestCustodian:testGetBorrower() (gas: 78641)
 TestCustodian:testInvalidAction() (gas: 173284)
@@ -53,11 +54,11 @@ TestCustodian:testName() (gas: 7121)
 TestCustodian:testNonPayableFunctions() (gas: 215173)
 TestCustodian:testOnlySeaport() (gas: 17829)
 TestCustodian:testPreviewOrderNoActiveLoan() (gas: 105805)
-TestCustodian:testPreviewOrderRepay() (gas: 241591)
-TestCustodian:testPreviewOrderSettlement() (gas: 191866)
-TestCustodian:testPreviewOrderSettlementInvalidFufliller() (gas: 108243)
+TestCustodian:testPreviewOrderRepay() (gas: 230692)
+TestCustodian:testPreviewOrderSettlement() (gas: 191848)
+TestCustodian:testPreviewOrderSettlementInvalidFufliller() (gas: 108234)
 TestCustodian:testPreviewOrderSettlementInvalidRepayer() (gas: 116961)
-TestCustodian:testRatifyOrder() (gas: 189713)
+TestCustodian:testRatifyOrder() (gas: 184300)
 TestCustodian:testSeaportMetadata() (gas: 8632)
 TestCustodian:testSupportsInterface() (gas: 9428)
 TestCustodian:testSymbol() (gas: 7105)
@@ -68,37 +69,37 @@ TestLenderEnforcer:testLERevertInvalidLoanTerms() (gas: 80962)
 TestLenderEnforcer:testLEValidLoanTerms() (gas: 71955)
 TestLenderEnforcer:testLEValidLoanTermsAnyBorrower() (gas: 72087)
 TestLenderEnforcer:testLEValidLoanTermsWithAdditionalTransfers() (gas: 73310)
-TestLoanCombinations:testLoan20For721SimpleInterestDutchFixedRepay() (gas: 599132)
-TestLoanCombinations:testLoan20for20SimpleInterestDutchFixedRepay() (gas: 606345)
-TestLoanCombinations:testLoan721for20SimpleInterestDutchFixedRepay() (gas: 597426)
-TestLoanCombinations:testLoanAstariaSettlementRepay() (gas: 587236)
+TestLoanCombinations:testLoan20For721SimpleInterestDutchFixedRepay() (gas: 589926)
+TestLoanCombinations:testLoan20for20SimpleInterestDutchFixedRepay() (gas: 597139)
+TestLoanCombinations:testLoan721for20SimpleInterestDutchFixedRepay() (gas: 588220)
+TestLoanCombinations:testLoanAstariaSettlementRepay() (gas: 578030)
 TestNewLoan:testBuyNowPayLater() (gas: 2872541)
 TestNewLoan:testInvalidSenderBNPL() (gas: 1613720)
 TestNewLoan:testInvalidUserDataHashBNPL() (gas: 1616299)
-TestNewLoan:testNewLoanAs1271ProxyAccountSender() (gas: 864104)
-TestNewLoan:testNewLoanAs1271ProxyAccountThirdPartyFiller() (gas: 873525)
+TestNewLoan:testNewLoanAs1271ProxyAccountSender() (gas: 864172)
+TestNewLoan:testNewLoanAs1271ProxyAccountThirdPartyFiller() (gas: 873557)
 TestNewLoan:testNewLoanERC721CollateralDefaultTerms2() (gas: 427351)
-TestNewLoan:testNewLoanRefinance() (gas: 588883)
+TestNewLoan:testNewLoanRefinance() (gas: 588882)
 TestNewLoan:testNewLoanViaOriginatorBorrowerApprovalAndLenderApproval() (gas: 328439)
 TestNewLoan:testNewLoanViaOriginatorLenderApproval() (gas: 384733)
-TestNewLoan:testSettleLoan() (gas: 639391)
+TestNewLoan:testSettleLoan() (gas: 639410)
 TestPausableNonReentrant:testNotOwner() (gas: 21254)
 TestPausableNonReentrant:testPauseAndUnpause() (gas: 22555)
 TestPausableNonReentrant:testReentrancy() (gas: 15360)
 TestPausableNonReentrant:testUnpauseWhenNotPaused() (gas: 12582)
-TestRepayLoan:testRepayLoanApprovedRepayer() (gas: 672743)
-TestRepayLoan:testRepayLoanBase() (gas: 608946)
+TestRepayLoan:testRepayLoanApprovedRepayer() (gas: 661916)
+TestRepayLoan:testRepayLoanBase() (gas: 598119)
 TestRepayLoan:testRepayLoanGenerateOrderNotSeaport() (gas: 436382)
-TestRepayLoan:testRepayLoanInSettlement() (gas: 583350)
-TestRepayLoan:testRepayLoanInvalidRepayer() (gas: 607116)
-TestRepayLoan:testRepayLoanThatDoesNotExist() (gas: 884491)
-TestSimpleInterestPricing:test_calculateInterest() (gas: 881308)
-TestSimpleInterestPricing:test_getPaymentConsideration() (gas: 928516)
-TestSimpleInterestPricing:test_getRefinanceConsideration() (gas: 919289)
+TestRepayLoan:testRepayLoanInSettlement() (gas: 583446)
+TestRepayLoan:testRepayLoanInvalidRepayer() (gas: 601777)
+TestRepayLoan:testRepayLoanThatDoesNotExist() (gas: 857426)
+TestSimpleInterestPricing:test_calculateInterest() (gas: 895930)
+TestSimpleInterestPricing:test_getPaymentConsideration() (gas: 943140)
+TestSimpleInterestPricing:test_getRefinanceConsideration() (gas: 933907)
 TestStarport:testActive() (gas: 69320)
 TestStarport:testAdditionalTransfers() (gas: 301282)
 TestStarport:testAdditionalTransfersOriginate() (gas: 276000)
-TestStarport:testAdditionalTransfersRefinance() (gas: 214276)
+TestStarport:testAdditionalTransfersRefinance() (gas: 214275)
 TestStarport:testApplyRefinanceConsiderationToLoanMalformed() (gas: 121896)
 TestStarport:testCannotIssueSameLoanTwice() (gas: 364572)
 TestStarport:testCannotOriginateWhilePaused() (gas: 73457)
@@ -111,7 +112,7 @@ TestStarport:testExoticDebtWithNoCaveatsNotAsBorrower() (gas: 377404)
 TestStarport:testIncrementCaveatNonce() (gas: 35208)
 TestStarport:testInitializedFlagSetProperly() (gas: 67393)
 TestStarport:testInvalidAdditionalTransfersOriginate() (gas: 228469)
-TestStarport:testInvalidAdditionalTransfersRefinance() (gas: 163975)
+TestStarport:testInvalidAdditionalTransfersRefinance() (gas: 163974)
 TestStarport:testInvalidAmountCollateral() (gas: 163481)
 TestStarport:testInvalidAmountCollateral721() (gas: 163481)
 TestStarport:testInvalidItemType() (gas: 149429)
@@ -123,7 +124,7 @@ TestStarport:testNonDefaultCustodianCustodyCallSuccess() (gas: 290660)
 TestStarport:testNonPayableFunctions() (gas: 112043)
 TestStarport:testOverrideFeeRake() (gas: 357317)
 TestStarport:testPause() (gas: 18093)
-TestStarport:testRefinancePostRepaymentFails() (gas: 120804)
+TestStarport:testRefinancePostRepaymentFails() (gas: 120803)
 TestStarport:testTokenNoCodeCollateral() (gas: 148242)
 TestStarport:testTokenNoCodeDebt() (gas: 178649)
 TestStarport:testUnpause() (gas: 17198)

src/lib/StarportLib.sol

@@ -75,7 +75,7 @@ library StarportLib {
         address paymentRecipient,
         SpentItem[] memory carry,
         address carryRecipient
-    ) public pure returns (ReceivedItem[] memory consideration) {
+    ) internal pure returns (ReceivedItem[] memory consideration) {
         consideration = new ReceivedItem[](payment.length + carry.length);
 
         uint256 i = 0;
@@ -128,7 +128,7 @@ library StarportLib {
 
     function removeZeroAmountItems(ReceivedItem[] memory consideration)
         internal
-        view
+        pure
         returns (ReceivedItem[] memory newConsideration)
     {
         uint256 j = 0;
@@ -144,7 +144,7 @@ library StarportLib {
                 });
 
                 unchecked {
-                    j++;
+                    ++j;
                 }
             }
             unchecked {

src/lib/Validation.sol

@@ -0,0 +1,13 @@
+pragma solidity ^0.8.17;
+
+import {Starport} from "starport-core/Starport.sol";
+
+abstract contract Validation {
+    /*
+    * @dev Validates the loan against the module
+    * @param loan The loan to validate
+    * @return bytes4 The validation result
+    */
+
+    function validate(Starport.Loan calldata) external view virtual returns (bytes4);
+}

src/pricing/BasePricing.sol

@@ -28,6 +28,7 @@ import {FixedPointMathLib} from "solady/src/utils/FixedPointMathLib.sol";
 
 import {StarportLib} from "starport-core/lib/StarportLib.sol";
 import {SpentItem} from "seaport-types/src/lib/ConsiderationStructs.sol";
+import {Validation} from "starport-core/lib/Validation.sol";
 
 abstract contract BasePricing is Pricing {
     using FixedPointMathLib for uint256;
@@ -86,8 +87,10 @@ abstract contract BasePricing is Pricing {
         }
     }
 
-    function validate(Starport.Loan calldata loan) external pure virtual override returns (bytes4) {
-        return Pricing.validate.selector;
+    // @inheritdoc Validation
+    function validate(Starport.Loan calldata loan) external view virtual override returns (bytes4) {
+        Details memory details = abi.decode(loan.terms.pricingData, (Details));
+        return (details.decimals > 0) ? Validation.validate.selector : bytes4(0xFFFFFFFF);
     }
 
     /**

src/pricing/Pricing.sol

@@ -23,8 +23,9 @@ pragma solidity ^0.8.17;
 import {Starport} from "starport-core/Starport.sol";
 import {SpentItem} from "seaport-types/src/lib/ConsiderationStructs.sol";
 import {AdditionalTransfer} from "starport-core/lib/StarportLib.sol";
+import {Validation} from "starport-core/lib/Validation.sol";
 
-abstract contract Pricing {
+abstract contract Pricing is Validation {
     Starport public immutable SP;
 
     error InvalidRefinance();
@@ -54,6 +55,4 @@ abstract contract Pricing {
         view
         virtual
         returns (SpentItem[] memory, SpentItem[] memory, AdditionalTransfer[] memory);
-
-    function validate(Starport.Loan calldata loan) external pure virtual returns (bytes4);
 }

src/pricing/SimpleInterestPricing.sol

@@ -27,12 +27,14 @@ import {Pricing} from "starport-core/pricing/Pricing.sol";
 import {AdditionalTransfer} from "starport-core/lib/StarportLib.sol";
 import {SpentItem, ReceivedItem} from "seaport-types/src/lib/ConsiderationStructs.sol";
 import {StarportLib} from "starport-core/lib/StarportLib.sol";
+import {Validation} from "starport-core/lib/Validation.sol";
 
 contract SimpleInterestPricing is BasePricing {
     using FixedPointMathLib for uint256;
 
     constructor(Starport SP_) Pricing(SP_) {}
 
+    // @inheritdoc BasePricing
     function calculateInterest(uint256 delta_t, uint256 amount, uint256 rate, uint256 decimals)
         public
         pure
@@ -42,6 +44,13 @@ contract SimpleInterestPricing is BasePricing {
         return StarportLib.calculateSimpleInterest(delta_t, amount, rate, decimals);
     }
 
+    // @inheritdoc Validation
+    function validate(Starport.Loan calldata loan) external view override returns (bytes4) {
+        Details memory details = abi.decode(loan.terms.pricingData, (Details));
+        return (details.decimals > 0) ? Validation.validate.selector : bytes4(0xFFFFFFFF);
+    }
+
+    // @inheritdoc Pricing
     function getRefinanceConsideration(Starport.Loan calldata loan, bytes memory newPricingData, address fulfiller)
         external
         view
@@ -56,7 +65,6 @@ contract SimpleInterestPricing is BasePricing {
         Details memory oldDetails = abi.decode(loan.terms.pricingData, (Details));
         Details memory newDetails = abi.decode(newPricingData, (Details));
 
-        //todo: figure out the proper flow for here
         if ((newDetails.rate < oldDetails.rate)) {
             (repayConsideration, carryConsideration) = getPaymentConsideration(loan);
             additionalConsideration = new AdditionalTransfer[](0);

src/settlement/DutchAuctionSettlement.sol

@@ -14,6 +14,7 @@ import {FixedPointMathLib} from "solady/src/utils/FixedPointMathLib.sol";
 import {Starport, Settlement} from "starport-core/settlement/Settlement.sol";
 
 import {BasePricing} from "starport-core/pricing/BasePricing.sol";
+import {Validation} from "starport-core/lib/Validation.sol";
 
 abstract contract DutchAuctionSettlement is Settlement, AmountDeriver {
     constructor(Starport SP_) Settlement(SP_) {}
@@ -106,9 +107,9 @@ abstract contract DutchAuctionSettlement is Settlement, AmountDeriver {
         });
     }
 
-    // @inheritdoc Settlement
-    function validate(Starport.Loan calldata loan) external view virtual override returns (bool) {
+    // @inheritdoc Validation
+    function validate(Starport.Loan calldata loan) external view virtual override returns (bytes4) {
         Details memory details = abi.decode(loan.terms.settlementData, (Details));
-        return details.startingPrice > details.endingPrice;
+        return (details.startingPrice > details.endingPrice) ? Validation.validate.selector : bytes4(0xFFFFFFFF);
     }
 }

src/settlement/Settlement.sol

@@ -23,8 +23,9 @@ pragma solidity ^0.8.17;
 import {Starport} from "starport-core/Starport.sol";
 
 import {ReceivedItem} from "seaport-types/src/lib/ConsiderationStructs.sol";
+import {Validation} from "starport-core/lib/Validation.sol";
 
-abstract contract Settlement {
+abstract contract Settlement is Validation {
     Starport public immutable SP;
 
     constructor(Starport SP_) {
@@ -45,13 +46,6 @@ abstract contract Settlement {
     */
     function postRepayment(Starport.Loan calldata loan, address fulfiller) external virtual returns (bytes4);
 
-    /*
-    * @dev helper to validate the details of a loan are valid for this settlement
-    * @param loan      The loan in question
-    * @return bool     Whether the loan is valid for this settlement
-    */
-    function validate(Starport.Loan calldata loan) external view virtual returns (bool);
-
     /*
     * @dev helper to get the consideration for a loan
     * @param loan      The loan in question

src/status/FixedTermStatus.sol

@@ -2,6 +2,7 @@ pragma solidity ^0.8.17;
 
 import {Starport} from "starport-core/Starport.sol";
 import {Status} from "starport-core/status/Status.sol";
+import {Validation} from "starport-core/lib/Validation.sol";
 
 contract FixedTermStatus is Status {
     struct Details {
@@ -13,4 +14,9 @@ contract FixedTermStatus is Status {
         Details memory details = abi.decode(loan.terms.statusData, (Details));
         return loan.start + details.loanDuration > block.timestamp;
     }
+
+    function validate(Starport.Loan calldata loan) external view override returns (bytes4) {
+        Details memory details = abi.decode(loan.terms.statusData, (Details));
+        return (details.loanDuration > 0) ? Validation.validate.selector : bytes4(0xFFFFFFFF);
+    }
 }

src/status/Status.sol

@@ -21,8 +21,9 @@
 pragma solidity ^0.8.17;
 
 import {Starport} from "starport-core/Starport.sol";
+import {Validation} from "starport-core/lib/Validation.sol";
 
-abstract contract Status {
+abstract contract Status is Validation {
     /*
     * @dev Returns true if the loan is still active, false otherwise.
     * @param loan The loan to check.