fix exotic debt computation bug, and add a test

AstariaXYZ · Nov 14, 2023 · 6997da5 · 6997da5
1 parent 4554785
commit 6997da5
Show file tree

Hide file tree

Showing 5 changed files with 56 additions and 29 deletions.
diff --git a/.gas-snapshot b/.gas-snapshot
@@ -62,12 +62,12 @@ TestCustodian:testSupportsInterface() (gas: 9428)
 TestCustodian:testSymbol() (gas: 7105)
 TestCustodian:testTokenURI() (gas: 67024)
 TestCustodian:testTokenURIInvalidLoan() (gas: 13151)
-TestFuzzStarport:testFuzzNewOrigination((address,uint256,uint256,uint256,uint256,(uint8,address,uint256,uint256)[],uint8)) (runs: 256, μ: 445408, ~: 442561)
-TestFuzzStarport:testFuzzRefinance(((address,uint256,uint256,uint256,uint256,(uint8,address,uint256,uint256)[],uint8),string,uint8,uint256,uint256,uint256)) (runs: 256, μ: 1291763, ~: 1288019)
-TestFuzzStarport:testFuzzRepaymentFails(((address,uint256,uint256,uint256,uint256,(uint8,address,uint256,uint256)[],uint8),(uint8,address,uint256,uint256)[10],(uint8,address,uint256,uint256)[10],address[3],uint256)) (runs: 256, μ: 761212, ~: 762045)
-TestFuzzStarport:testFuzzRepaymentSuccess(((address,uint256,uint256,uint256,uint256,(uint8,address,uint256,uint256)[],uint8),(uint8,address,uint256,uint256)[10],(uint8,address,uint256,uint256)[10],address[3],uint256)) (runs: 256, μ: 644506, ~: 641869)
-TestFuzzStarport:testFuzzSettlementFails(((address,uint256,uint256,uint256,uint256,(uint8,address,uint256,uint256)[],uint8),(uint8,address,uint256,uint256)[10],(uint8,address,uint256,uint256)[10],address[3],uint256)) (runs: 256, μ: 739572, ~: 739346)
-TestFuzzStarport:testFuzzSettlementSuccess(((address,uint256,uint256,uint256,uint256,(uint8,address,uint256,uint256)[],uint8),uint256)) (runs: 256, μ: 603888, ~: 602287)
+TestFuzzStarport:testFuzzNewOrigination((address,uint256,uint256,uint256,uint256,(uint8,address,uint256,uint256)[],uint8)) (runs: 256, μ: 446207, ~: 442561)
+TestFuzzStarport:testFuzzRefinance(((address,uint256,uint256,uint256,uint256,(uint8,address,uint256,uint256)[],uint8),string,uint8,uint256,uint256,uint256)) (runs: 256, μ: 1291623, ~: 1288019)
+TestFuzzStarport:testFuzzRepaymentFails(((address,uint256,uint256,uint256,uint256,(uint8,address,uint256,uint256)[],uint8),(uint8,address,uint256,uint256)[10],(uint8,address,uint256,uint256)[10],address[3],uint256)) (runs: 256, μ: 757271, ~: 760814)
+TestFuzzStarport:testFuzzRepaymentSuccess(((address,uint256,uint256,uint256,uint256,(uint8,address,uint256,uint256)[],uint8),(uint8,address,uint256,uint256)[10],(uint8,address,uint256,uint256)[10],address[3],uint256)) (runs: 256, μ: 641897, ~: 639803)
+TestFuzzStarport:testFuzzSettlementFails(((address,uint256,uint256,uint256,uint256,(uint8,address,uint256,uint256)[],uint8),(uint8,address,uint256,uint256)[10],(uint8,address,uint256,uint256)[10],address[3],uint256)) (runs: 256, μ: 735517, ~: 736927)
+TestFuzzStarport:testFuzzSettlementSuccess(((address,uint256,uint256,uint256,uint256,(uint8,address,uint256,uint256)[],uint8),uint256)) (runs: 256, μ: 604657, ~: 602287)
 TestLenderEnforcer:testLERevertAdditionalTransfersFromLender() (gas: 76210)
 TestLenderEnforcer:testLERevertInvalidLoanTerms() (gas: 80962)
 TestLenderEnforcer:testLEValidLoanTerms() (gas: 71955)
@@ -77,14 +77,15 @@ TestLoanCombinations:testLoan20For721SimpleInterestDutchFixedRepay() (gas: 59222
 TestLoanCombinations:testLoan20for20SimpleInterestDutchFixedRepay() (gas: 599433)
 TestLoanCombinations:testLoan721for20SimpleInterestDutchFixedRepay() (gas: 590484)
 TestLoanCombinations:testLoanAstariaSettlementRepay() (gas: 580325)
-TestNewLoan:testBuyNowPayLater() (gas: 2864927)
-TestNewLoan:testInvalidUserDataHashBNPL() (gas: 1612321)
-TestNewLoan:testNewLoanERC721CollateralDefaultTerms2() (gas: 420265)
+TestNewLoan:testBuyNowPayLater() (gas: 2868927)
+TestNewLoan:testInvalidSenderBNPL() (gas: 1613720)
+TestNewLoan:testInvalidUserDataHashBNPL() (gas: 1616299)
+TestNewLoan:testNewLoanERC721CollateralDefaultTerms2() (gas: 420243)
 TestNewLoan:testNewLoanERC721CollateralLessDebtThanOffered() (gas: 2348)
-TestNewLoan:testNewLoanRefinanceNew() (gas: 185)
-TestNewLoan:testNewLoanViaOriginatorBorrowerApprovalAndLenderApproval() (gas: 328384)
+TestNewLoan:testNewLoanRefinanceNew() (gas: 163)
+TestNewLoan:testNewLoanViaOriginatorBorrowerApprovalAndLenderApproval() (gas: 328494)
 TestNewLoan:testNewLoanViaOriginatorLenderApproval() (gas: 382896)
-TestNewLoan:testSettleLoan() (gas: 632342)
+TestNewLoan:testSettleLoan() (gas: 632320)
 TestPausableNonReentrant:testNotOwner() (gas: 21254)
 TestPausableNonReentrant:testPauseAndUnpause() (gas: 22555)
 TestPausableNonReentrant:testReentrancy() (gas: 15360)
@@ -104,31 +105,32 @@ TestStarport:testAdditionalTransfers() (gas: 299521)
 TestStarport:testAdditionalTransfersOriginate() (gas: 274239)
 TestStarport:testAdditionalTransfersRefinance() (gas: 214181)
 TestStarport:testApplyRefinanceConsiderationToLoanMalformed() (gas: 78162)
-TestStarport:testCannotIssueSameLoanTwice() (gas: 361073)
+TestStarport:testCannotIssueSameLoanTwice() (gas: 361051)
 TestStarport:testCannotOriginateWhilePaused() (gas: 73544)
 TestStarport:testCannotSettleInvalidLoan() (gas: 75003)
 TestStarport:testCannotSettleUnlessValidCustodian() (gas: 71073)
-TestStarport:testCaveatEnforcerRevert() (gas: 99173)
-TestStarport:testDefaultFeeRake() (gas: 359826)
+TestStarport:testCaveatEnforcerRevert() (gas: 99151)
+TestStarport:testDefaultFeeRake() (gas: 359891)
+TestStarport:testDefaultFeeRakeExoticDebt() (gas: 369406)
 TestStarport:testExoticDebtWithNoCaveatsNotAsBorrower() (gas: 375643)
-TestStarport:testIncrementCaveatNonce() (gas: 34627)
+TestStarport:testIncrementCaveatNonce() (gas: 34605)
 TestStarport:testInitializedFlagSetProperly() (gas: 67393)
 TestStarport:testInvalidAdditionalTransfersOriginate() (gas: 226782)
 TestStarport:testInvalidAdditionalTransfersRefinance() (gas: 163953)
 TestStarport:testInvalidAmountCollateral() (gas: 161794)
 TestStarport:testInvalidAmountCollateral721() (gas: 161794)
 TestStarport:testInvalidItemType() (gas: 147742)
 TestStarport:testInvalidTransferLengthCollateral() (gas: 149948)
-TestStarport:testInvalidTransferLengthDebt() (gas: 171771)
-TestStarport:testInvalidateCaveatSalt() (gas: 32791)
+TestStarport:testInvalidTransferLengthDebt() (gas: 171749)
+TestStarport:testInvalidateCaveatSalt() (gas: 32858)
 TestStarport:testNonDefaultCustodianCustodyCallFails() (gas: 262686)
 TestStarport:testNonDefaultCustodianCustodyCallSuccess() (gas: 288899)
 TestStarport:testNonPayableFunctions() (gas: 111955)
-TestStarport:testOverrideFeeRake() (gas: 355703)
+TestStarport:testOverrideFeeRake() (gas: 355780)
 TestStarport:testPause() (gas: 18158)
 TestStarport:testRefinancePostRepaymentFails() (gas: 120713)
 TestStarport:testTokenNoCodeCollateral() (gas: 146555)
-TestStarport:testTokenNoCodeDebt() (gas: 176876)
+TestStarport:testTokenNoCodeDebt() (gas: 176962)
 TestStarport:testUnpause() (gas: 17263)
 TestStarportLib:testValidateSalt(address,bytes32) (runs: 256, μ: 33628, ~: 33628)
 TestStrategistOriginator:testEncodeWithAccountCounter() (gas: 12307)

diff --git a/src/BNPLHelper.sol b/src/BNPLHelper.sol
@@ -54,7 +54,7 @@ contract BNPLHelper is IFlashLoanRecipient {
         Fulfillment[] fulfillments;
     }
 
-    error SenderNotSelf();
+    error SenderNotVault();
     error DoNotSendETH();
     error InvalidUserDataProvided();
 
@@ -70,7 +70,9 @@ contract BNPLHelper is IFlashLoanRecipient {
         uint256[] calldata feeAmounts,
         bytes calldata userData
     ) external override {
-        require(msg.sender == vault);
+        if (msg.sender != vault) {
+            revert SenderNotVault();
+        }
 
         if (activeUserDataHash != keccak256(userData)) {
             revert InvalidUserDataProvided();

diff --git a/src/Starport.sol b/src/Starport.sol
@@ -489,18 +489,14 @@ contract Starport is PausableNonReentrant {
         paymentToBorrower = new SpentItem[](debt.length);
         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;
-                uint256 amount = debt[i].amount.mulDiv(
+                amount = debt[i].amount.mulDiv(
                     !feeOverride.enabled ? defaultFeeRake : feeOverride.amount, 10 ** ERC20(debt[i].token).decimals()
                 );
-                paymentToBorrower[i] = SpentItem({
-                    token: debt[i].token,
-                    itemType: debt[i].itemType,
-                    identifier: debt[i].identifier,
-                    amount: debt[i].amount - amount
-                });
+
                 if (amount > 0) {
                     feeItems[i].amount = amount;
                     feeItems[i].token = debt[i].token;
@@ -509,10 +505,17 @@ contract Starport is PausableNonReentrant {
                     ++totalFeeItems;
                 }
             }
+            paymentToBorrower[i] = SpentItem({
+                token: debt[i].token,
+                itemType: debt[i].itemType,
+                identifier: debt[i].identifier,
+                amount: debt[i].amount - amount
+            });
             unchecked {
                 ++i;
             }
         }
+
         assembly {
             mstore(feeItems, totalFeeItems)
         }

diff --git a/test/integration-testing/TestNewLoan.sol b/test/integration-testing/TestNewLoan.sol
@@ -317,6 +317,12 @@ contract TestNewLoan is StarportTest {
         helper.receiveFlashLoan(new address[](0), new uint256[](0), new uint256[](0), bytes(""));
     }
 
+    function testInvalidSenderBNPL() public {
+        BNPLHelper helper = new BNPLHelper();
+        vm.expectRevert(abi.encodeWithSelector(BNPLHelper.SenderNotVault.selector));
+        helper.receiveFlashLoan(new address[](0), new uint256[](0), new uint256[](0), bytes(""));
+    }
+
     function testNewLoanViaOriginatorLenderApproval() public {
         Starport.Loan memory loan = generateDefaultLoanTerms();
 

diff --git a/test/unit-testing/TestStarport.sol b/test/unit-testing/TestStarport.sol
@@ -387,6 +387,20 @@ contract TestStarport is StarportTest, DeepEq {
         assertEq(erc20s[0].balanceOf(feeReceiver), loan.debt[0].amount * 1e17 / 1e18, "fee receiver not paid properly");
     }
 
+    function testDefaultFeeRakeExoticDebt() public {
+        assertEq(SP.defaultFeeRake(), 0);
+        address feeReceiver = address(20);
+        SP.setFeeData(feeReceiver, 1e17); //10% fees
+
+        Starport.Loan memory originationDetails = _generateOriginationDetails(
+            _getERC721SpentItem(erc721s[0], uint256(2)), _getERC1155SpentItem(erc1155s[1]), lender.addr
+        );
+
+        Starport.Loan memory loan =
+            newLoan(originationDetails, bytes32(bytes32(msg.sig)), bytes32(bytes32(msg.sig)), lender.addr);
+        assertEq(erc20s[0].balanceOf(feeReceiver), 0, "fee receiver not paid properly");
+    }
+
     function testOverrideFeeRake() public {
         assertEq(SP.defaultFeeRake(), 0);
         address feeReceiver = address(20);