From f0fc582f7bb615d07e12006b8ebbee83635017e7 Mon Sep 17 00:00:00 2001 From: Joseph Delong Date: Wed, 17 Jan 2024 18:28:40 -0600 Subject: [PATCH] Fix/audit fixes round 6 (#91) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: m-4 feeOverride should be mandatory for tokens that aren’t 18 decimals * fix: m-3 Error-prone string casting for tokenURI * fix: i-3 There are still mentions of the LoanManager contract * fix: i-8 Delete unused errors, or use them * fix: i-14 References to the old naming Loan Manager or LM instead of Starport --- .gas-snapshot | 11 +++---- src/BNPLHelper.sol | 7 +++-- src/Custodian.sol | 13 ++++---- src/Starport.sol | 38 ++++++++++-------------- src/enforcers/LenderEnforcer.sol | 1 - src/originators/StrategistOriginator.sol | 1 - test/integration-testing/TestNewLoan.sol | 2 +- test/unit-testing/TestCustodian.sol | 8 +++-- 8 files changed, 38 insertions(+), 43 deletions(-) diff --git a/.gas-snapshot b/.gas-snapshot index ef911bb2..5e00f50e 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -30,7 +30,7 @@ TestCustodian:testCannotLazyMintTwice() (gas: 82131) TestCustodian:testCannotMintInvalidLoanInvalidCustodian() (gas: 72437) TestCustodian:testCannotMintInvalidLoanValidCustodian() (gas: 77947) TestCustodian:testCustodianCannotBeAuthorized() (gas: 142196) -TestCustodian:testCustodySelector() (gas: 2697813) +TestCustodian:testCustodySelector() (gas: 2706027) TestCustodian:testDefaultCustodySelectorRevert() (gas: 72420) TestCustodian:testGenerateOrderInvalidPostRepayment() (gas: 173141) TestCustodian:testGenerateOrderInvalidPostSettlement() (gas: 163282) @@ -66,7 +66,7 @@ TestCustodian:testRatifyOrder() (gas: 184120) TestCustodian:testSeaportMetadata() (gas: 8644) TestCustodian:testSupportsInterface() (gas: 9428) TestCustodian:testSymbol() (gas: 7216) -TestCustodian:testTokenURI() (gas: 84130) +TestCustodian:testTokenURI() (gas: 84908) TestCustodian:testTokenURIInvalidLoan() (gas: 13157) TestLenderEnforcer:testLERevertAdditionalTransfersFromLender() (gas: 76455) TestLenderEnforcer:testLERevertInvalidLoanTerms() (gas: 81096) @@ -112,9 +112,10 @@ TestStarport:testCannotOriginateWhilePaused() (gas: 73567) TestStarport:testCannotSettleInvalidLoan() (gas: 74903) TestStarport:testCannotSettleUnlessValidCustodian() (gas: 70963) TestStarport:testCaveatEnforcerRevert() (gas: 100284) -TestStarport:testDefaultFeeRake1() (gas: 387818) -TestStarport:testDefaultFeeRake2() (gas: 450161) +TestStarport:testDefaultFeeRake1() (gas: 387812) +TestStarport:testDefaultFeeRake2() (gas: 450155) TestStarport:testDefaultFeeRakeExoticDebt() (gas: 397641) +TestStarport:testEIP712Signing() (gas: 83089) TestStarport:testExoticDebtWithCustomPricingAndRepayment() (gas: 1237839) TestStarport:testExoticDebtWithCustomPricingAndSettlement() (gas: 1692860) TestStarport:testExoticDebtWithNoCaveatsNotAsBorrower() (gas: 376819) @@ -131,7 +132,7 @@ TestStarport:testInvalidateCaveatSalt() (gas: 33450) TestStarport:testNonDefaultCustodianCustodyCallFails() (gas: 264255) TestStarport:testNonDefaultCustodianCustodyCallSuccess() (gas: 290380) TestStarport:testNonPayableFunctions() (gas: 112229) -TestStarport:testOverrideFeeRake() (gas: 384052) +TestStarport:testOverrideFeeRake() (gas: 384046) TestStarport:testPause() (gas: 18259) TestStarport:testRefinancePostRepaymentFails() (gas: 127973) TestStarport:testStargateGetOwner() (gas: 8808) diff --git a/src/BNPLHelper.sol b/src/BNPLHelper.sol index 95592acd..5e3a5bf9 100644 --- a/src/BNPLHelper.sol +++ b/src/BNPLHelper.sol @@ -74,7 +74,6 @@ contract BNPLHelper is IFlashLoanRecipient { /* CUSTOM ERRORS */ /*.•°:°.´+˚.*°.˚:*.´•*.+°.•°:´*.´•*.•°.•°:°.´:•˚°.*°.˚:*.´+°.•*/ - error DoNotSendETH(); error InvalidUserDataProvided(); error SenderNotVault(); @@ -95,7 +94,7 @@ contract BNPLHelper is IFlashLoanRecipient { /*.•°:°.´+˚.*°.˚:*.´•*.+°.•°:´*.´•*.•°.•°:°.´:•˚°.*°.˚:*.´+°.•*/ struct Execution { - address lm; + address starport; address seaport; address borrower; CaveatEnforcer.SignedCaveats borrowerCaveat; @@ -154,6 +153,8 @@ contract BNPLHelper is IFlashLoanRecipient { ++i; } } - Starport(execution.lm).originate(transfers, execution.borrowerCaveat, execution.lenderCaveat, execution.loan); + Starport(execution.starport).originate( + transfers, execution.borrowerCaveat, execution.lenderCaveat, execution.loan + ); } } diff --git a/src/Custodian.sol b/src/Custodian.sol index edd7d6f1..ab01c97a 100644 --- a/src/Custodian.sol +++ b/src/Custodian.sol @@ -124,8 +124,7 @@ contract Custodian is ERC721, ContractOffererInterface { if (!_exists(loanId)) { revert InvalidLoan(); } - //TODO: PLACEHOLDER that is semantically sound - return string.concat("", LibString.toString(loanId)); + return string(abi.encodePacked("https://astaria.xyz/loans/", LibString.toString(loanId))); } /** @@ -410,7 +409,7 @@ contract Custodian is ERC721, ContractOffererInterface { } /** - * @dev settle the loan with the LoanManager + * @dev settle the loan with Starport * @param loan The the loan that is settled * @param fulfiller The address executing seaport */ @@ -423,7 +422,7 @@ contract Custodian is ERC721, ContractOffererInterface { } /** - * @dev settle the loan with the LoanManager + * @dev settle the loan with Starport * @param loan The the loan that is settled * @param fulfiller The address executing seaport */ @@ -436,7 +435,7 @@ contract Custodian is ERC721, ContractOffererInterface { } /** - * @dev settle the loan with the LoanManager + * @dev settle the loan with Starport * @param loan The the loan to settle */ function _settleLoan(Starport.Loan memory loan) internal virtual { @@ -483,13 +482,13 @@ contract Custodian is ERC721, ContractOffererInterface { function _afterSettlementHandlerHook(Starport.Loan memory loan) internal virtual {} /** - * @dev Hook to call before the loan is settled with the LM + * @dev Hook to call before the loan is settled with the Starport * @param loan The loan being settled */ function _beforeSettleLoanHook(Starport.Loan memory loan) internal virtual {} /** - * @dev Hook to call after the loan is settled with the LM + * @dev Hook to call after the loan is settled with the Starport * @param loan The loan being settled */ function _afterSettleLoanHook(Starport.Loan memory loan) internal virtual {} diff --git a/src/Starport.sol b/src/Starport.sol index 1a8cd9fc..ad342b64 100644 --- a/src/Starport.sol +++ b/src/Starport.sol @@ -54,8 +54,6 @@ contract Starport is PausableNonReentrant { /* CUSTOM ERRORS */ /*.•°:°.´+˚.*°.˚:*.´•*.+°.•°:´*.´•*.•°.•°:°.´:•˚°.*°.˚:*.´+°.•*/ - error AdditionalTransferError(); - error CannotTransferLoans(); error CaveatDeadlineExpired(); error InvalidCaveat(); error InvalidCaveatLength(); @@ -64,7 +62,6 @@ contract Starport is PausableNonReentrant { error InvalidLoan(); error InvalidLoanState(); error InvalidPostRepayment(); - error InvalidRefinance(); error LoanExists(); error MalformedRefinance(); error NotLoanCustodian(); @@ -657,27 +654,24 @@ contract Starport is PausableNonReentrant { Fee memory feeOverride = feeOverrides[debtItem.token]; SpentItem memory feeItem = feeItems[totalFeeItems]; feeItem.identifier = 0; - uint8 decimals; - try ERC20(debtItem.token).decimals() returns (uint8 _decimals) { - decimals = _decimals; - } catch { - decimals = 18; - } - uint256 defaultFeeRake = defaultFeeRakeByDecimals[decimals]; - if (defaultFeeRake != 0 || feeOverride.enabled) { - amount = debtItem.amount.mulDivUp( - !feeOverride.enabled ? defaultFeeRake : feeOverride.amount, 10 ** decimals - ); - } + try ERC20(debtItem.token).decimals() returns (uint8 decimals) { + uint256 defaultFeeRake = defaultFeeRakeByDecimals[decimals]; - if (amount > 0) { - feeItem.amount = amount; - feeItem.token = debtItem.token; - feeItem.itemType = debtItem.itemType; + if (defaultFeeRake != 0 || feeOverride.enabled) { + amount = debtItem.amount.mulDivUp( + !feeOverride.enabled ? defaultFeeRake : feeOverride.amount, 10 ** decimals + ); + } - ++totalFeeItems; - } + if (amount > 0) { + feeItem.amount = amount; + feeItem.token = debtItem.token; + feeItem.itemType = debtItem.itemType; + + ++totalFeeItems; + } + } catch {} } paymentToBorrower[i] = SpentItem({ token: debtItem.token, @@ -700,7 +694,7 @@ contract Starport is PausableNonReentrant { } /** - * @dev Issues a LM token if needed, only owner can call + * @dev Changes loanId status to open for the specified loan * @param loan The loan to issue */ function _issueLoan(Loan memory loan) internal { diff --git a/src/enforcers/LenderEnforcer.sol b/src/enforcers/LenderEnforcer.sol index 82243d40..9fb82edb 100644 --- a/src/enforcers/LenderEnforcer.sol +++ b/src/enforcers/LenderEnforcer.sol @@ -32,7 +32,6 @@ import {CaveatEnforcer} from "../enforcers/CaveatEnforcer.sol"; import {AdditionalTransfer} from "../lib/StarportLib.sol"; contract LenderEnforcer is CaveatEnforcer { - error LenderOnlyEnforcer(); error InvalidLoanTerms(); error InvalidAdditionalTransfer(); diff --git a/src/originators/StrategistOriginator.sol b/src/originators/StrategistOriginator.sol index 15c4e2e8..ac88fb34 100644 --- a/src/originators/StrategistOriginator.sol +++ b/src/originators/StrategistOriginator.sol @@ -48,7 +48,6 @@ contract StrategistOriginator is Ownable, Originator, TokenReceiverInterface { error AdditionalTransferError(); error InvalidCollateral(); - error InvalidCustodian(); error InvalidDeadline(); error InvalidDebt(); error InvalidDebtAmount(); diff --git a/test/integration-testing/TestNewLoan.sol b/test/integration-testing/TestNewLoan.sol index c4c75bd4..1f83f90b 100644 --- a/test/integration-testing/TestNewLoan.sol +++ b/test/integration-testing/TestNewLoan.sol @@ -236,7 +236,7 @@ contract TestNewLoan is StarportTest { amounts, abi.encode( BNPLHelper.Execution({ - lm: address(SP), + starport: address(SP), seaport: address(seaport), borrower: borrower.addr, borrowerCaveat: signCaveatForAccount( diff --git a/test/unit-testing/TestCustodian.sol b/test/unit-testing/TestCustodian.sol index 41b5c0e7..7c9ffaf7 100644 --- a/test/unit-testing/TestCustodian.sol +++ b/test/unit-testing/TestCustodian.sol @@ -104,10 +104,12 @@ contract TestCustodian is StarportTest, DeepEq, MockCall { } function testTokenURI() public { - assertEq( - custodian.tokenURI(uint256(keccak256(abi.encode(activeLoan)))), - LibString.toString(uint256(keccak256(abi.encode(activeLoan)))) + string memory expected = string( + abi.encodePacked( + "https://astaria.xyz/loans/", LibString.toString(uint256(keccak256(abi.encode(activeLoan)))) + ) ); + assertEq(custodian.tokenURI(uint256(keccak256(abi.encode(activeLoan)))), expected); } function testTokenURIInvalidLoan() public {