From 4c8a6b706067d5159d80bab9d07d9164c61cced4 Mon Sep 17 00:00:00 2001 From: GregTheDev <40359730+0xgregthedev@users.noreply.github.com> Date: Sat, 18 Nov 2023 11:47:06 -0600 Subject: [PATCH] final optimizations and changes (#73) * final optimizations and changes * chore(bot): format & snapshot --------- Co-authored-by: 0xgregthedev <0xgregthedev@users.noreply.github.com> --- .gas-snapshot | 134 ++++++++++----------- src/Starport.sol | 42 +++---- src/enforcers/BorrowerEnforcer.sol | 11 +- src/lib/PausableNonReentrant.sol | 8 +- src/lib/StarportLib.sol | 90 +++++++------- src/settlement/DutchAuctionSettlement.sol | 2 +- src/settlement/Settlement.sol | 8 +- test/StarportTest.sol | 50 +++----- test/integration-testing/TestNewLoan.sol | 2 +- test/unit-testing/TestBorrowerEnforcer.sol | 4 +- test/utils/Bound.sol | 4 +- test/utils/Cast.sol | 10 +- test/utils/Expect.sol | 2 +- 13 files changed, 173 insertions(+), 194 deletions(-) diff --git a/.gas-snapshot b/.gas-snapshot index 4121ef5b..4729aad4 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -1,39 +1,39 @@ -IntegrationTestCaveats:testOriginateUnapprovedFulfiller() (gas: 369613) -IntegrationTestCaveats:testOriginateWBorrowerApproval() (gas: 323372) -IntegrationTestCaveats:testOriginateWCaveats() (gas: 297466) +IntegrationTestCaveats:testOriginateUnapprovedFulfiller() (gas: 366712) +IntegrationTestCaveats:testOriginateWBorrowerApproval() (gas: 320478) +IntegrationTestCaveats:testOriginateWCaveats() (gas: 294572) IntegrationTestCaveats:testOriginateWCaveatsExpired() (gas: 177232) IntegrationTestCaveats:testOriginateWCaveatsIncrementedNonce() (gas: 205798) -IntegrationTestCaveats:testOriginateWCaveatsInvalidSalt() (gas: 303717) -IntegrationTestCaveats:testOriginateWCaveatsInvalidSaltManual() (gas: 179719) -IntegrationTestCaveats:testOriginateWLenderApproval() (gas: 323448) -IntegrationTestCaveats:testRefinanceAsLender() (gas: 1088462) -IntegrationTestCaveats:testRefinanceCaveatFailure() (gas: 437545) -IntegrationTestCaveats:testRefinanceLoanStartAtBlockTimestampInvalidLoan() (gas: 384815) -IntegrationTestCaveats:testRefinanceUnapprovedFulfiller() (gas: 495425) -IntegrationTestCaveats:testRefinanceWCaveatsInvalidSalt() (gas: 414216) -IntegrationTestCaveats:testRefinanceWLenderApproval() (gas: 437545) -ModuleTesting:testFixedTermDutchAuctionSettlement() (gas: 436187) -ModuleTesting:testFixedTermDutchAuctionSettlementGetSettlementAuctionExpired() (gas: 439054) -ModuleTesting:testFixedTermDutchAuctionSettlementNotValid() (gas: 435177) -ModuleTesting:testFixedTermDutchAuctionSettlementValid() (gas: 435998) -ModuleTesting:testModuleValidation() (gas: 1272828) +IntegrationTestCaveats:testOriginateWCaveatsInvalidSalt() (gas: 300823) +IntegrationTestCaveats:testOriginateWCaveatsInvalidSaltManual() (gas: 179750) +IntegrationTestCaveats:testOriginateWLenderApproval() (gas: 320554) +IntegrationTestCaveats:testRefinanceAsLender() (gas: 1082795) +IntegrationTestCaveats:testRefinanceCaveatFailure() (gas: 434517) +IntegrationTestCaveats:testRefinanceLoanStartAtBlockTimestampInvalidLoan() (gas: 381921) +IntegrationTestCaveats:testRefinanceUnapprovedFulfiller() (gas: 489763) +IntegrationTestCaveats:testRefinanceWCaveatsInvalidSalt() (gas: 411188) +IntegrationTestCaveats:testRefinanceWLenderApproval() (gas: 431878) +ModuleTesting:testFixedTermDutchAuctionSettlement() (gas: 433286) +ModuleTesting:testFixedTermDutchAuctionSettlementGetSettlementAuctionExpired() (gas: 436153) +ModuleTesting:testFixedTermDutchAuctionSettlementNotValid() (gas: 432276) +ModuleTesting:testFixedTermDutchAuctionSettlementValid() (gas: 433097) +ModuleTesting:testModuleValidation() (gas: 1269927) PausableNonReentrantImpl:test() (gas: 2442) PausableNonReentrantImpl:testReentrancy() (gas: 2735) -TestBorrowerEnforcer:testBERevertAdditionalTransfers() (gas: 75662) -TestBorrowerEnforcer:testBERevertInvalidLoanTerms() (gas: 80937) +TestBorrowerEnforcer:testBERevertAdditionalTransfersFromBorrower() (gas: 76218) +TestBorrowerEnforcer:testBERevertInvalidLoanTerms() (gas: 81026) TestBorrowerEnforcer:testBEValidLoanTerms() (gas: 72021) TestBorrowerEnforcer:testBEValidLoanTermsAnyIssuer() (gas: 72085) TestCustodian:testCannotLazyMintTwice() (gas: 78796) TestCustodian:testCannotMintInvalidLoanInvalidCustodian() (gas: 69031) TestCustodian:testCannotMintInvalidLoanValidCustodian() (gas: 74555) -TestCustodian:testCustodySelector() (gas: 2908178) +TestCustodian:testCustodySelector() (gas: 2861655) TestCustodian:testDefaultCustodySelectorRevert() (gas: 72312) -TestCustodian:testGenerateOrderInvalidPostRepayment() (gas: 173324) +TestCustodian:testGenerateOrderInvalidPostRepayment() (gas: 173056) TestCustodian:testGenerateOrderInvalidPostSettlement() (gas: 163179) -TestCustodian:testGenerateOrderRepay() (gas: 177479) -TestCustodian:testGenerateOrderRepayAsRepayApprovedBorrower() (gas: 193948) -TestCustodian:testGenerateOrderRepayERC1155AndERC20() (gas: 874180) -TestCustodian:testGenerateOrderRepayERC1155AndERC20HandlerAuthorized() (gas: 803573) +TestCustodian:testGenerateOrderRepay() (gas: 177211) +TestCustodian:testGenerateOrderRepayAsRepayApprovedBorrower() (gas: 193680) +TestCustodian:testGenerateOrderRepayERC1155AndERC20() (gas: 868121) +TestCustodian:testGenerateOrderRepayERC1155AndERC20HandlerAuthorized() (gas: 797782) TestCustodian:testGenerateOrderRepayInvalidHookAddress() (gas: 97601) TestCustodian:testGenerateOrderRepayInvalidHookReturnType() (gas: 91984) TestCustodian:testGenerateOrderRepayNotBorrower() (gas: 106839) @@ -41,24 +41,24 @@ TestCustodian:testGenerateOrderSettlement() (gas: 154934) TestCustodian:testGenerateOrderSettlementHandlerAuthorized() (gas: 160331) TestCustodian:testGenerateOrderSettlementNoActiveLoan() (gas: 163342) TestCustodian:testGenerateOrderSettlementUnauthorized() (gas: 101813) -TestCustodian:testGenerateOrdersWithLoanStartAtBlockTimestampInvalidLoan() (gas: 461053) +TestCustodian:testGenerateOrdersWithLoanStartAtBlockTimestampInvalidLoan() (gas: 458156) TestCustodian:testGetBorrower() (gas: 78641) TestCustodian:testInvalidAction() (gas: 173284) TestCustodian:testInvalidActionRepayInActiveLoan() (gas: 130104) TestCustodian:testInvalidActionSettleActiveLoan() (gas: 130086) TestCustodian:testInvalidEncodedData() (gas: 26192) -TestCustodian:testMintWithApprovalSetAsBorrower() (gas: 362794) +TestCustodian:testMintWithApprovalSetAsBorrower() (gas: 359900) TestCustodian:testMintWithApprovalSetAsBorrowerInvalidLoan() (gas: 60792) TestCustodian:testMintWithApprovalSetNotAuthorized() (gas: 76759) TestCustodian:testName() (gas: 7121) TestCustodian:testNonPayableFunctions() (gas: 215173) TestCustodian:testOnlySeaport() (gas: 17829) TestCustodian:testPreviewOrderNoActiveLoan() (gas: 105805) -TestCustodian:testPreviewOrderRepay() (gas: 230692) +TestCustodian:testPreviewOrderRepay() (gas: 230156) TestCustodian:testPreviewOrderSettlement() (gas: 191848) TestCustodian:testPreviewOrderSettlementInvalidFufliller() (gas: 108234) TestCustodian:testPreviewOrderSettlementInvalidRepayer() (gas: 116961) -TestCustodian:testRatifyOrder() (gas: 184300) +TestCustodian:testRatifyOrder() (gas: 184032) TestCustodian:testSeaportMetadata() (gas: 8632) TestCustodian:testSupportsInterface() (gas: 9428) TestCustodian:testSymbol() (gas: 7105) @@ -69,64 +69,64 @@ TestLenderEnforcer:testLERevertInvalidLoanTerms() (gas: 80962) TestLenderEnforcer:testLEValidLoanTerms() (gas: 71955) TestLenderEnforcer:testLEValidLoanTermsAnyBorrower() (gas: 72087) TestLenderEnforcer:testLEValidLoanTermsWithAdditionalTransfers() (gas: 73310) -TestLoanCombinations:testLoan20For721SimpleInterestDutchFixedRepay() (gas: 589926) -TestLoanCombinations:testLoan20for20SimpleInterestDutchFixedRepay() (gas: 597139) -TestLoanCombinations:testLoan721for20SimpleInterestDutchFixedRepay() (gas: 588220) -TestLoanCombinations:testLoanAstariaSettlementRepay() (gas: 578030) -TestNewLoan:testBuyNowPayLater() (gas: 2872541) +TestLoanCombinations:testLoan20For721SimpleInterestDutchFixedRepay() (gas: 586757) +TestLoanCombinations:testLoan20for20SimpleInterestDutchFixedRepay() (gas: 593970) +TestLoanCombinations:testLoan721for20SimpleInterestDutchFixedRepay() (gas: 585051) +TestLoanCombinations:testLoanAstariaSettlementRepay() (gas: 574861) +TestNewLoan:testBuyNowPayLater() (gas: 2869372) TestNewLoan:testInvalidSenderBNPL() (gas: 1613720) TestNewLoan:testInvalidUserDataHashBNPL() (gas: 1616299) -TestNewLoan:testNewLoanAs1271ProxyAccountSender() (gas: 864172) -TestNewLoan:testNewLoanAs1271ProxyAccountThirdPartyFiller() (gas: 873557) -TestNewLoan:testNewLoanERC721CollateralDefaultTerms2() (gas: 427351) -TestNewLoan:testNewLoanRefinance() (gas: 588882) -TestNewLoan:testNewLoanViaOriginatorBorrowerApprovalAndLenderApproval() (gas: 328439) -TestNewLoan:testNewLoanViaOriginatorLenderApproval() (gas: 384733) -TestNewLoan:testSettleLoan() (gas: 639410) +TestNewLoan:testNewLoanAs1271ProxyAccountSender() (gas: 861278) +TestNewLoan:testNewLoanAs1271ProxyAccountThirdPartyFiller() (gas: 870656) +TestNewLoan:testNewLoanERC721CollateralDefaultTerms2() (gas: 424450) +TestNewLoan:testNewLoanRefinance() (gas: 583073) +TestNewLoan:testNewLoanViaOriginatorBorrowerApprovalAndLenderApproval() (gas: 325542) +TestNewLoan:testNewLoanViaOriginatorLenderApproval() (gas: 381839) +TestNewLoan:testSettleLoan() (gas: 636143) TestPausableNonReentrant:testNotOwner() (gas: 21254) TestPausableNonReentrant:testPauseAndUnpause() (gas: 22555) TestPausableNonReentrant:testReentrancy() (gas: 15360) TestPausableNonReentrant:testUnpauseWhenNotPaused() (gas: 12582) -TestRepayLoan:testRepayLoanApprovedRepayer() (gas: 661916) -TestRepayLoan:testRepayLoanBase() (gas: 598119) -TestRepayLoan:testRepayLoanGenerateOrderNotSeaport() (gas: 436382) -TestRepayLoan:testRepayLoanInSettlement() (gas: 583446) -TestRepayLoan:testRepayLoanInvalidRepayer() (gas: 601777) -TestRepayLoan:testRepayLoanThatDoesNotExist() (gas: 857426) +TestRepayLoan:testRepayLoanApprovedRepayer() (gas: 658479) +TestRepayLoan:testRepayLoanBase() (gas: 594682) +TestRepayLoan:testRepayLoanGenerateOrderNotSeaport() (gas: 433481) +TestRepayLoan:testRepayLoanInSettlement() (gas: 580179) +TestRepayLoan:testRepayLoanInvalidRepayer() (gas: 598608) +TestRepayLoan:testRepayLoanThatDoesNotExist() (gas: 853185) 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: 214275) +TestStarport:testAdditionalTransfers() (gas: 298120) +TestStarport:testAdditionalTransfersOriginate() (gas: 272838) +TestStarport:testAdditionalTransfersRefinance() (gas: 211072) TestStarport:testApplyRefinanceConsiderationToLoanMalformed() (gas: 121896) -TestStarport:testCannotIssueSameLoanTwice() (gas: 364572) +TestStarport:testCannotIssueSameLoanTwice() (gas: 358910) TestStarport:testCannotOriginateWhilePaused() (gas: 73457) TestStarport:testCannotSettleInvalidLoan() (gas: 74915) TestStarport:testCannotSettleUnlessValidCustodian() (gas: 70985) TestStarport:testCaveatEnforcerRevert() (gas: 99214) -TestStarport:testDefaultFeeRake() (gas: 361421) -TestStarport:testDefaultFeeRakeExoticDebt() (gas: 370933) -TestStarport:testExoticDebtWithNoCaveatsNotAsBorrower() (gas: 377404) +TestStarport:testDefaultFeeRake() (gas: 357730) +TestStarport:testDefaultFeeRakeExoticDebt() (gas: 367863) +TestStarport:testExoticDebtWithNoCaveatsNotAsBorrower() (gas: 374121) TestStarport:testIncrementCaveatNonce() (gas: 35208) TestStarport:testInitializedFlagSetProperly() (gas: 67393) -TestStarport:testInvalidAdditionalTransfersOriginate() (gas: 228469) -TestStarport:testInvalidAdditionalTransfersRefinance() (gas: 163974) -TestStarport:testInvalidAmountCollateral() (gas: 163481) -TestStarport:testInvalidAmountCollateral721() (gas: 163481) -TestStarport:testInvalidItemType() (gas: 149429) +TestStarport:testInvalidAdditionalTransfersOriginate() (gas: 227780) +TestStarport:testInvalidAdditionalTransfersRefinance() (gas: 163706) +TestStarport:testInvalidAmountCollateral() (gas: 163345) +TestStarport:testInvalidAmountCollateral721() (gas: 163345) +TestStarport:testInvalidItemType() (gas: 149293) TestStarport:testInvalidTransferLengthCollateral() (gas: 151635) -TestStarport:testInvalidTransferLengthDebt() (gas: 173436) -TestStarport:testInvalidateCaveatSalt() (gas: 33463) -TestStarport:testNonDefaultCustodianCustodyCallFails() (gas: 264447) -TestStarport:testNonDefaultCustodianCustodyCallSuccess() (gas: 290660) +TestStarport:testInvalidTransferLengthDebt() (gas: 173302) +TestStarport:testInvalidateCaveatSalt() (gas: 33494) +TestStarport:testNonDefaultCustodianCustodyCallFails() (gas: 261553) +TestStarport:testNonDefaultCustodianCustodyCallSuccess() (gas: 287766) TestStarport:testNonPayableFunctions() (gas: 112043) -TestStarport:testOverrideFeeRake() (gas: 357317) +TestStarport:testOverrideFeeRake() (gas: 354048) TestStarport:testPause() (gas: 18093) TestStarport:testRefinancePostRepaymentFails() (gas: 120803) -TestStarport:testTokenNoCodeCollateral() (gas: 148242) -TestStarport:testTokenNoCodeDebt() (gas: 178649) +TestStarport:testTokenNoCodeCollateral() (gas: 148106) +TestStarport:testTokenNoCodeDebt() (gas: 178379) TestStarport:testUnpause() (gas: 17198) TestStrategistOriginator:testEncodeWithAccountCounter() (gas: 12307) TestStrategistOriginator:testGetStrategistData() (gas: 1489933) @@ -139,6 +139,6 @@ TestStrategistOriginator:testInvalidDebtAmountAskingMoreThanOffered() (gas: 2117 TestStrategistOriginator:testInvalidDebtAmountOfferingZero() (gas: 212095) TestStrategistOriginator:testInvalidDebtAmountRequestingZero() (gas: 212049) TestStrategistOriginator:testInvalidDebtLength() (gas: 210678) -TestStrategistOriginator:testInvalidOffer() (gas: 427302) +TestStrategistOriginator:testInvalidOffer() (gas: 424408) TestStrategistOriginator:testInvalidSigner() (gas: 214006) TestStrategistOriginator:testSetStrategist() (gas: 17796) \ No newline at end of file diff --git a/src/Starport.sol b/src/Starport.sol index 5c848203..83b8013a 100644 --- a/src/Starport.sol +++ b/src/Starport.sol @@ -119,7 +119,7 @@ contract Starport is PausableNonReentrant { address custodian = address(new Custodian(this, seaport_)); bytes32 defaultCustodianCodeHash; - assembly { + assembly ("memory-safe") { defaultCustodianCodeHash := extcodehash(custodian) } defaultCustodian = payable(custodian); @@ -290,7 +290,7 @@ contract Starport is PausableNonReentrant { address custodian = loan.custodian; // Comparing the retrieved code hash with a known hash bytes32 codeHash; - assembly { + assembly ("memory-safe") { codeHash := extcodehash(custodian) } if ( @@ -325,10 +325,8 @@ contract Starport is PausableNonReentrant { ) internal pure { uint256 i = 0; for (; i < additionalTransfers.length;) { - if ( - additionalTransfers[i].from != borrower && additionalTransfers[i].from != lender - && additionalTransfers[i].from != fulfiller - ) { + address from = additionalTransfers[i].from; + if (from != borrower && from != lender && from != fulfiller) { revert UnauthorizedAdditionalTransferIncluded(); } unchecked { @@ -403,7 +401,7 @@ contract Starport is PausableNonReentrant { } function invalidateCaveatSalt(bytes32 salt) external { - invalidSalts[msg.sender][salt] = true; + invalidSalts.validateSalt(msg.sender, salt); emit CaveatSaltInvalidated(msg.sender, salt); } @@ -488,33 +486,35 @@ contract Starport is PausableNonReentrant { uint256 totalFeeItems; for (uint256 i = 0; i < debt.length;) { uint256 amount; - if (debt[i].itemType == ItemType.ERC20) { - Fee memory feeOverride = feeOverrides[debt[i].token]; - feeItems[i].identifier = 0; - amount = debt[i].amount.mulDiv( - !feeOverride.enabled ? defaultFeeRake : feeOverride.amount, 10 ** ERC20(debt[i].token).decimals() + SpentItem memory debtItem = debt[i]; + if (debtItem.itemType == ItemType.ERC20) { + Fee memory feeOverride = feeOverrides[debtItem.token]; + SpentItem memory feeItem = feeItems[i]; + feeItem.identifier = 0; + amount = debtItem.amount.mulDiv( + !feeOverride.enabled ? defaultFeeRake : feeOverride.amount, 10 ** ERC20(debtItem.token).decimals() ); if (amount > 0) { - feeItems[i].amount = amount; - feeItems[i].token = debt[i].token; - feeItems[i].itemType = debt[i].itemType; + feeItem.amount = amount; + feeItem.token = debtItem.token; + feeItem.itemType = debtItem.itemType; ++totalFeeItems; } } paymentToBorrower[i] = SpentItem({ - token: debt[i].token, - itemType: debt[i].itemType, - identifier: debt[i].identifier, - amount: debt[i].amount - amount + token: debtItem.token, + itemType: debtItem.itemType, + identifier: debtItem.identifier, + amount: debtItem.amount - amount }); unchecked { ++i; } } - assembly { + assembly ("memory-safe") { mstore(feeItems, totalFeeItems) } } @@ -528,8 +528,6 @@ contract Starport is PausableNonReentrant { loan.start = block.timestamp; loan.originator = loan.originator != address(0) ? loan.originator : msg.sender; - bytes memory encodedLoan = abi.encode(loan); - uint256 loanId = loan.getId(); if (active(loanId)) { revert LoanExists(); diff --git a/src/enforcers/BorrowerEnforcer.sol b/src/enforcers/BorrowerEnforcer.sol index 8b27155a..6c72aba9 100644 --- a/src/enforcers/BorrowerEnforcer.sol +++ b/src/enforcers/BorrowerEnforcer.sol @@ -39,7 +39,14 @@ contract BorrowerEnforcer is CaveatEnforcer { if (keccak256(abi.encode(loan)) != keccak256(abi.encode(details.loan))) revert InvalidLoanTerms(); - //Should additional transfers from the accounts other than the borrower be allowed? - if (additionalTransfers.length > 0) revert InvalidAdditionalTransfer(); + if (additionalTransfers.length > 0) { + uint256 i = 0; + for (; i < additionalTransfers.length;) { + if (additionalTransfers[i].from == loan.borrower) revert InvalidAdditionalTransfer(); + unchecked { + ++i; + } + } + } } } diff --git a/src/lib/PausableNonReentrant.sol b/src/lib/PausableNonReentrant.sol index 2d27bb86..32eeb72f 100644 --- a/src/lib/PausableNonReentrant.sol +++ b/src/lib/PausableNonReentrant.sol @@ -18,7 +18,7 @@ abstract contract PausableNonReentrant is Ownable { * @dev modifier to ensure that the contract is not paused or locked */ modifier pausableNonReentrant() { - assembly { + assembly ("memory-safe") { //If locked or paused, handle revert cases if gt(sload(_state.slot), _UNLOCKED) { if gt(sload(_state.slot), _LOCKED) { @@ -33,7 +33,7 @@ abstract contract PausableNonReentrant is Ownable { sstore(_state.slot, _LOCKED) } _; - assembly { + assembly ("memory-safe") { sstore(_state.slot, _UNLOCKED) } } @@ -42,7 +42,7 @@ abstract contract PausableNonReentrant is Ownable { * @dev Pause the contract if not paused or locked */ function pause() external onlyOwner { - assembly { + assembly ("memory-safe") { //If locked, prevent owner from overriding state if eq(sload(_state.slot), _LOCKED) { //Revert IsLocked @@ -58,7 +58,7 @@ abstract contract PausableNonReentrant is Ownable { * @dev unpause the contract if not paused or locked */ function unpause() external onlyOwner { - assembly { + assembly ("memory-safe") { //If not paused, prevent owner from overriding state if lt(sload(_state.slot), _PAUSED) { //Revert NotPaused diff --git a/src/lib/StarportLib.sol b/src/lib/StarportLib.sol index e8c4d4a3..ee52c4d8 100644 --- a/src/lib/StarportLib.sol +++ b/src/lib/StarportLib.sol @@ -47,7 +47,7 @@ library StarportLib { address borrower, bytes32 salt ) internal { - assembly { + assembly ("memory-safe") { mstore(0x0, borrower) mstore(0x20, usedSalts.slot) @@ -82,11 +82,12 @@ library StarportLib { uint256 j = 0; for (; i < payment.length;) { if (payment[i].amount > 0) { + SpentItem memory paymentItem = payment[i]; consideration[j] = ReceivedItem({ - itemType: payment[i].itemType, - identifier: payment[i].identifier, - amount: payment[i].amount, - token: payment[i].token, + itemType: paymentItem.itemType, + identifier: paymentItem.identifier, + amount: paymentItem.amount, + token: paymentItem.token, recipient: payable(paymentRecipient) }); @@ -103,11 +104,12 @@ library StarportLib { i = 0; for (; i < carry.length;) { if (carry[i].amount > 0) { + SpentItem memory carryItem = carry[i]; consideration[j] = ReceivedItem({ - itemType: carry[i].itemType, - identifier: carry[i].identifier, - amount: carry[i].amount, - token: carry[i].token, + itemType: carryItem.itemType, + identifier: carryItem.identifier, + amount: carryItem.amount, + token: carryItem.token, recipient: payable(carryRecipient) }); @@ -121,7 +123,7 @@ library StarportLib { } } - assembly { + assembly ("memory-safe") { mstore(consideration, j) } } @@ -135,12 +137,13 @@ library StarportLib { newConsideration = new ReceivedItem[](consideration.length); for (uint256 i = 0; i < consideration.length;) { if (consideration[i].amount > 0) { + ReceivedItem memory considerationItem = consideration[i]; newConsideration[j] = ReceivedItem({ - itemType: consideration[i].itemType, - identifier: consideration[i].identifier, - amount: consideration[i].amount, - token: consideration[i].token, - recipient: consideration[i].recipient + itemType: considerationItem.itemType, + identifier: considerationItem.identifier, + amount: considerationItem.amount, + token: considerationItem.token, + recipient: considerationItem.recipient }); unchecked { @@ -151,7 +154,7 @@ library StarportLib { ++i; } } - assembly { + assembly ("memory-safe") { mstore(newConsideration, j) } } @@ -159,24 +162,23 @@ library StarportLib { function transferAdditionalTransfersCalldata(AdditionalTransfer[] calldata transfers) internal { uint256 i = 0; for (; i < transfers.length;) { - if (transfers[i].token.code.length == 0) { + AdditionalTransfer calldata transfer = transfers[i]; + if (transfer.token.code.length == 0) { revert InvalidItemTokenNoCode(); } - if (transfers[i].itemType == ItemType.ERC20) { + if (transfer.itemType == ItemType.ERC20) { // erc20 transfer - if (transfers[i].amount > 0) { - SafeTransferLib.safeTransferFrom( - transfers[i].token, transfers[i].from, transfers[i].to, transfers[i].amount - ); + if (transfer.amount > 0) { + SafeTransferLib.safeTransferFrom(transfer.token, transfer.from, transfer.to, transfer.amount); } - } else if (transfers[i].itemType == ItemType.ERC721) { + } else if (transfer.itemType == ItemType.ERC721) { // erc721 transfer - ERC721(transfers[i].token).transferFrom(transfers[i].from, transfers[i].to, transfers[i].identifier); + ERC721(transfer.token).transferFrom(transfer.from, transfer.to, transfer.identifier); } else if (transfers[i].itemType == ItemType.ERC1155) { // erc1155 transfer - if (transfers[i].amount > 0) { - ERC1155(transfers[i].token).safeTransferFrom( - transfers[i].from, transfers[i].to, transfers[i].identifier, transfers[i].amount, new bytes(0) + if (transfer.amount > 0) { + ERC1155(transfer.token).safeTransferFrom( + transfer.from, transfer.to, transfer.identifier, transfer.amount, new bytes(0) ); } } else { @@ -191,24 +193,23 @@ library StarportLib { function transferAdditionalTransfers(AdditionalTransfer[] memory transfers) internal { uint256 i = 0; for (; i < transfers.length;) { - if (transfers[i].token.code.length == 0) { + AdditionalTransfer memory transfer = transfers[i]; + if (transfer.token.code.length == 0) { revert InvalidItemTokenNoCode(); } - if (transfers[i].itemType == ItemType.ERC20) { + if (transfer.itemType == ItemType.ERC20) { // erc20 transfer - if (transfers[i].amount > 0) { - SafeTransferLib.safeTransferFrom( - transfers[i].token, transfers[i].from, transfers[i].to, transfers[i].amount - ); + if (transfer.amount > 0) { + SafeTransferLib.safeTransferFrom(transfer.token, transfer.from, transfer.to, transfer.amount); } - } else if (transfers[i].itemType == ItemType.ERC721) { + } else if (transfer.itemType == ItemType.ERC721) { // erc721 transfer - ERC721(transfers[i].token).transferFrom(transfers[i].from, transfers[i].to, transfers[i].identifier); - } else if (transfers[i].itemType == ItemType.ERC1155) { + ERC721(transfer.token).transferFrom(transfer.from, transfer.to, transfer.identifier); + } else if (transfer.itemType == ItemType.ERC1155) { // erc1155 transfer - if (transfers[i].amount > 0) { - ERC1155(transfers[i].token).safeTransferFrom( - transfers[i].from, transfers[i].to, transfers[i].identifier, transfers[i].amount, new bytes(0) + if (transfer.amount > 0) { + ERC1155(transfer.token).safeTransferFrom( + transfer.from, transfer.to, transfer.identifier, transfer.amount, new bytes(0) ); } } else { @@ -270,15 +271,8 @@ library StarportLib { if (transfers.length > 0) { uint256 i = 0; for (; i < transfers.length;) { - _transferItem( - transfers[i].itemType, - transfers[i].token, - transfers[i].identifier, - transfers[i].amount, - from, - to, - safe - ); + SpentItem memory transfer = transfers[i]; + _transferItem(transfer.itemType, transfer.token, transfer.identifier, transfer.amount, from, to, safe); unchecked { ++i; } diff --git a/src/settlement/DutchAuctionSettlement.sol b/src/settlement/DutchAuctionSettlement.sol index 578f7230..0610be06 100644 --- a/src/settlement/DutchAuctionSettlement.sol +++ b/src/settlement/DutchAuctionSettlement.sol @@ -57,7 +57,7 @@ abstract contract DutchAuctionSettlement is Settlement, AmountDeriver { view virtual override - returns (ReceivedItem[] memory consideration, address restricted) + returns (ReceivedItem[] memory consideration, address authorized) { Details memory details = abi.decode(loan.terms.settlementData, (Details)); diff --git a/src/settlement/Settlement.sol b/src/settlement/Settlement.sol index 0d99a5ec..57eed356 100644 --- a/src/settlement/Settlement.sol +++ b/src/settlement/Settlement.sol @@ -48,15 +48,15 @@ abstract contract Settlement is Validation { /* * @dev helper to get the consideration for a loan - * @param loan The loan in question - * @return consideration The settlement consideration for the loan - * @return address The address of the authorized party (if any) + * @param loan The loan in question + * @return consideration The settlement consideration for the loan + * @return authorized The address of the authorized party (if any) */ function getSettlementConsideration(Starport.Loan calldata loan) public view virtual - returns (ReceivedItem[] memory consideration, address restricted); + returns (ReceivedItem[] memory consideration, address authorized); /* * @dev standard erc1155 received hook diff --git a/test/StarportTest.sol b/test/StarportTest.sol index b80e6b07..5954a95f 100644 --- a/test/StarportTest.sol +++ b/test/StarportTest.sol @@ -74,29 +74,25 @@ interface IWETH9 { } contract MockIssuer is TokenReceiverInterface { - function onERC721Received(address operator, address from, uint256 tokenId, bytes calldata data) - external - override - returns (bytes4) - { + function onERC721Received(address, address, uint256, bytes calldata) external pure override returns (bytes4) { return this.onERC721Received.selector; } - function onERC1155Received(address operator, address from, uint256 id, uint256 value, bytes calldata data) + function onERC1155Received(address, address, uint256, uint256, bytes calldata) external + pure override returns (bytes4) { return this.onERC1155Received.selector; } - function onERC1155BatchReceived( - address operator, - address from, - uint256[] calldata ids, - uint256[] calldata values, - bytes calldata data - ) external override returns (bytes4) { + function onERC1155BatchReceived(address, address, uint256[] calldata, uint256[] calldata, bytes calldata) + external + pure + override + returns (bytes4) + { return this.onERC1155BatchReceived.selector; } } @@ -222,17 +218,10 @@ contract StarportTest is BaseOrderTest { simpleInterestPricing = new SimpleInterestPricing(SP); } - function onERC721Received(address operator, address from, uint256 tokenId, bytes calldata data) - public - pure - override - returns (bytes4) - { + function onERC721Received(address, address, uint256, bytes calldata) public pure override returns (bytes4) { return this.onERC721Received.selector; } - // ConsiderationItem[] selectedCollateral; - // ConsiderationItem[] collateral20; SpentItem[] activeDebt; function _setApprovalsForSpentItems(address approver, SpentItem[] memory items) internal { @@ -254,7 +243,7 @@ contract StarportTest is BaseOrderTest { vm.stopPrank(); } - function _emptyCaveat() internal returns (CaveatEnforcer.SignedCaveats memory) { + function _emptyCaveat() internal pure returns (CaveatEnforcer.SignedCaveats memory) { return CaveatEnforcer.SignedCaveats({ signature: "", singleUse: true, @@ -483,14 +472,6 @@ contract StarportTest is BaseOrderTest { }); } - // function newLoan( - // Starport.Loan memory loan, - // bytes32 borrowerSalt, - // bytes32 lenderSalt - // ) internal returns (Starport.Loan memory originatedLoan) { - // newLoanSpecifySigner(loan, borrowerSalt, borrower, lenderSalt, lender); - // } - function refinanceLoan( Starport.Loan memory loan, bytes memory newPricingData, @@ -503,6 +484,7 @@ contract StarportTest is BaseOrderTest { function getRefinanceCaveat(Starport.Loan memory loan, bytes memory pricingData, address fulfiller) public + view returns (Starport.Loan memory) { (SpentItem[] memory considerationPayment, SpentItem[] memory carryPayment,) = @@ -616,15 +598,13 @@ contract StarportTest is BaseOrderTest { extraData: abi.encode(Custodian.Command(Actions.Settlement, activeLoan, "")) }); - // vm.recordLogs(); - vm.startPrank(borrower.addr); + vm.startPrank(fulfiller); consideration.fulfillAdvancedOrder({ advancedOrder: x, criteriaResolvers: new CriteriaResolver[](0), fulfillerConduitKey: bytes32(0), - recipient: address(this) + recipient: address(fulfiller) }); - // Vm.Log[] memory logs = vm.getRecordedLogs(); } function _repayLoan(Starport.Loan memory loan, address fulfiller) internal { @@ -655,7 +635,7 @@ contract StarportTest is BaseOrderTest { assertEq(originatorBefore + carry, originatorAfter, "carry: Borrower repayment was not correct"); } - function getOrderHash(address contractOfferer) public returns (bytes32) { + function getOrderHash(address contractOfferer) public view returns (bytes32) { uint256 counter = consideration.getContractOffererNonce(contractOfferer); return bytes32(counter ^ (uint256(uint160(contractOfferer)) << 96)); } diff --git a/test/integration-testing/TestNewLoan.sol b/test/integration-testing/TestNewLoan.sol index 7987e245..805a231e 100644 --- a/test/integration-testing/TestNewLoan.sol +++ b/test/integration-testing/TestNewLoan.sol @@ -382,7 +382,7 @@ contract TestNewLoan is StarportTest { SpentItem({itemType: ItemType.ERC20, token: address(erc20s[0]), amount: 600 ether, identifier: 0}) ); - (ReceivedItem[] memory settlementConsideration, address restricted) = + (ReceivedItem[] memory settlementConsideration, address authorized) = Settlement(activeLoan.terms.settlement).getSettlementConsideration(activeLoan); settlementConsideration = StarportLib.removeZeroAmountItems(settlementConsideration); ConsiderationItem[] memory consider = new ConsiderationItem[]( diff --git a/test/unit-testing/TestBorrowerEnforcer.sol b/test/unit-testing/TestBorrowerEnforcer.sol index 59947f54..21481f30 100644 --- a/test/unit-testing/TestBorrowerEnforcer.sol +++ b/test/unit-testing/TestBorrowerEnforcer.sol @@ -5,13 +5,13 @@ import {AdditionalTransfer, ItemType} from "starport-core/lib/StarportLib.sol"; import "forge-std/console.sol"; contract TestBorrowerEnforcer is StarportTest { - function testBERevertAdditionalTransfers() external { + function testBERevertAdditionalTransfersFromBorrower() external { AdditionalTransfer[] memory additionalTransfers = new AdditionalTransfer[](1); additionalTransfers[0] = AdditionalTransfer({ token: address(0), amount: 0, to: address(0), - from: address(0), + from: borrower.addr, identifier: 0, itemType: ItemType.ERC20 }); diff --git a/test/utils/Bound.sol b/test/utils/Bound.sol index 3b91f3b4..8c557216 100644 --- a/test/utils/Bound.sol +++ b/test/utils/Bound.sol @@ -105,13 +105,13 @@ abstract contract Bound is StdUtils { } function _toUint(address value) internal view returns (uint256 output) { - assembly { + assembly ("memory-safe") { output := value } } function _toAddress(uint256 value) internal view returns (address output) { - assembly { + assembly ("memory-safe") { output := value } } diff --git a/test/utils/Cast.sol b/test/utils/Cast.sol index 17633601..5945e3fd 100644 --- a/test/utils/Cast.sol +++ b/test/utils/Cast.sol @@ -7,25 +7,25 @@ import {Starport} from "starport-core/Starport.sol"; library Cast { function toUint(uint8 input) internal pure returns (uint256 ret) { - assembly { + assembly ("memory-safe") { ret := input } } function toUint(address input) internal pure returns (uint256 ret) { - assembly { + assembly ("memory-safe") { ret := input } } function toItemType(uint256 input) internal pure returns (ItemType ret) { - assembly { + assembly ("memory-safe") { ret := input } } function toStorage(ConsiderationItem[] memory a, ConsiderationItem[] storage b) internal { - assembly { + assembly ("memory-safe") { sstore(b.slot, mload(a)) } for (uint256 i; i < a.length; i++) { @@ -34,7 +34,7 @@ library Cast { } function toStorage(SpentItem[] memory a, SpentItem[] storage b) internal { - assembly { + assembly ("memory-safe") { sstore(b.slot, mload(a)) } diff --git a/test/utils/Expect.sol b/test/utils/Expect.sol index 1ed14c14..43e98656 100644 --- a/test/utils/Expect.sol +++ b/test/utils/Expect.sol @@ -7,7 +7,7 @@ abstract contract Expect is Test { function _expectRevert(SpentItem[] calldata items) internal { bool expectRevert; ItemType max = type(ItemType).max; - assembly { + assembly ("memory-safe") { let e := add(items.offset, mul(items.length, 0x80)) for { let i := items.offset } lt(i, e) { i := add(i, 0x80) } {