Fix/audit fixes round 6 (#91)

* 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
AstariaXYZ · Jan 18, 2024 · f0fc582 · f0fc582
1 parent 95f64d1
commit f0fc582
Show file tree

Hide file tree

Showing 8 changed files with 38 additions and 43 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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 {