From 65f7680c8e909ff9a398f702bf53829bc8835dfc Mon Sep 17 00:00:00 2001 From: androolloyd Date: Tue, 14 Nov 2023 21:04:37 -0400 Subject: [PATCH] feat/exotic fee fixes coverage updates (#66) * chore: snapshot * fix exotic debt computation bug, and add a test * format & snapshot --------- Co-authored-by: androolloyd --- .gas-snapshot | 42 +++++++++++++----------- src/BNPLHelper.sol | 6 ++-- src/Starport.sol | 17 ++++++---- test/integration-testing/TestNewLoan.sol | 6 ++++ test/unit-testing/TestStarport.sol | 14 ++++++++ 5 files changed, 56 insertions(+), 29 deletions(-) diff --git a/.gas-snapshot b/.gas-snapshot index 0dc40d93..bf0a6e5e 100644 --- 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, μ: 447052, ~: 445400) +TestFuzzStarport:testFuzzRefinance(((address,uint256,uint256,uint256,uint256,(uint8,address,uint256,uint256)[],uint8),string,uint8,uint256,uint256,uint256)) (runs: 256, μ: 1290856, ~: 1287156) +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, μ: 757788, ~: 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, μ: 642387, ~: 640510) +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, μ: 736033, ~: 736927) +TestFuzzStarport:testFuzzSettlementSuccess(((address,uint256,uint256,uint256,uint256,(uint8,address,uint256,uint256)[],uint8),uint256)) (runs: 256, μ: 605831, ~: 602717) 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,14 +105,15 @@ 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) @@ -119,16 +121,16 @@ 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 index 39c32341..c2e1ff31 100644 --- 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 index b2ac9339..52f251c9 100644 --- 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 index 2b6bac5d..cbdb7050 100644 --- 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 index 2877b0f4..c98ef9bb 100644 --- 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);