From fec4acd5578f908d115d543ecc4358db548a36dd Mon Sep 17 00:00:00 2001 From: Andrew Redden Date: Fri, 29 Dec 2023 12:07:06 +0100 Subject: [PATCH] closes AST-2635, AST-2717, AST-2716, AST-2715, AST-2707, AST-2710, AST-2711, AST-2709, AST-2713 --- ffi-scripts/test-origination-hash.ts | 3 +- ffi-scripts/test-sign-typed-data.ts | 1 + src/Custodian.sol | 35 +++++++++++++--------- src/Starport.sol | 21 ++++++++----- src/lib/RefStarportLib.sol | 6 ++-- src/lib/StarportLib.sol | 4 +-- src/pricing/SimpleInterestPricing.sol | 1 - src/settlement/DutchAuctionSettlement.sol | 7 +++-- src/status/FixedTermStatus.sol | 2 +- test/integration-testing/TestNewLoan.sol | 2 +- test/integration-testing/TestRepayLoan.sol | 2 +- test/unit-testing/ModuleTesting.t.sol | 15 ++++++++++ test/unit-testing/TestCustodian.sol | 22 ++++++++++++++ 13 files changed, 88 insertions(+), 33 deletions(-) diff --git a/ffi-scripts/test-origination-hash.ts b/ffi-scripts/test-origination-hash.ts index b3226a77..341afe88 100644 --- a/ffi-scripts/test-origination-hash.ts +++ b/ffi-scripts/test-origination-hash.ts @@ -44,6 +44,7 @@ const types = { const domain = (verifyingContract: Address, chainId: number) => ({ + name: "Starport", version: "0", chainId: chainId, verifyingContract @@ -51,7 +52,7 @@ const domain = (verifyingContract: Address, chainId: number) => ({ type caveatType = [`0x${string}`, `0x${string}`]; -const typeDataMessage = (account: Address, accountNonce: string, singleUse: number, salt: Hex, deadline: string, caveats: any) => ({ +const typeDataMessage = (account: Address, accountNonce: string, singleUse: boolean, salt: Hex, deadline: string, caveats: any) => ({ account: account, accountNonce: parseInt(accountNonce), singleUse: singleUse, salt: salt, deadline: deadline, caveats: caveats[0] }); diff --git a/ffi-scripts/test-sign-typed-data.ts b/ffi-scripts/test-sign-typed-data.ts index 0ea75102..da935fc3 100644 --- a/ffi-scripts/test-sign-typed-data.ts +++ b/ffi-scripts/test-sign-typed-data.ts @@ -51,6 +51,7 @@ const types = { const domain = (verifyingContract: Address, chainId: number) => ({ + name: "Starport", version: "0", chainId: chainId, verifyingContract diff --git a/src/Custodian.sol b/src/Custodian.sol index e9d36d26..edd7d6f1 100644 --- a/src/Custodian.sol +++ b/src/Custodian.sol @@ -48,7 +48,7 @@ contract Custodian is ERC721, ContractOffererInterface { /*´:°•.°+.*•´.*:˚.°*.˚•´.°:°•.°•.*•´.*:˚.°*.˚•´.°:°•.°+.*•´.*:*/ /* CUSTOM ERRORS */ /*.•°:°.´+˚.*°.˚:*.´•*.+°.•°:´*.´•*.•°.•°:°.´:•˚°.*°.˚:*.´+°.•*/ - + error CustodianCannotBeAuthorized(); error ImplementInChild(); error InvalidAction(); error InvalidFulfiller(); @@ -162,12 +162,7 @@ contract Custodian is ERC721, ContractOffererInterface { * @param loan The loan to mint a custody token for */ function mint(Starport.Loan calldata loan) external { - bytes memory encodedLoan = abi.encode(loan); - uint256 loanId = uint256(keccak256(encodedLoan)); - if (loan.custodian != address(this) || SP.closed(loanId)) { - revert InvalidLoan(); - } - _safeMint(loan.borrower, loanId, encodedLoan); + _validateAndMint(loan); } /** @@ -176,16 +171,22 @@ contract Custodian is ERC721, ContractOffererInterface { * @param approvedTo The address with pre approvals set */ function mintWithApprovalSet(Starport.Loan calldata loan, address approvedTo) external { - bytes memory encodedLoan = abi.encode(loan); - uint256 loanId = uint256(keccak256(encodedLoan)); - if (loan.custodian != address(this) || SP.closed(loanId)) { - revert InvalidLoan(); - } if (msg.sender != loan.borrower) { revert NotAuthorized(); } - _safeMint(loan.borrower, loanId, encodedLoan); - _approve(loan.borrower, approvedTo, loanId); + _approve(loan.borrower, approvedTo, _validateAndMint(loan)); + } + + /** + * @dev internal helper that validates and mints a custody token for a loan. + * @param loan The loan to mint a custody token for + */ + function _validateAndMint(Starport.Loan calldata loan) internal returns (uint256 loanId) { + loanId = loan.getId(); + if (loan.custodian != address(this) || SP.closed(loanId)) { + revert InvalidLoan(); + } + _safeMint(loan.borrower, loanId); } /** @@ -244,6 +245,9 @@ contract Custodian is ERC721, ContractOffererInterface { address authorized; _beforeGetSettlementConsideration(loan); (consideration, authorized) = Settlement(loan.terms.settlement).getSettlementConsideration(loan); + if (authorized == address(this)) { + revert CustodianCannotBeAuthorized(); + } consideration = StarportLib.removeZeroAmountItems(consideration); _afterGetSettlementConsideration(loan); if (authorized == address(0) || fulfiller == authorized) { @@ -334,6 +338,9 @@ contract Custodian is ERC721, ContractOffererInterface { } else if (close.action == Actions.Settlement && !loanActive) { address authorized; (consideration, authorized) = Settlement(loan.terms.settlement).getSettlementConsideration(loan); + if (authorized == address(this)) { + revert CustodianCannotBeAuthorized(); + } consideration = StarportLib.removeZeroAmountItems(consideration); if (authorized == address(0) || fulfiller == authorized) { offer = loan.collateral; diff --git a/src/Starport.sol b/src/Starport.sol index 58d75122..d0adae00 100644 --- a/src/Starport.sol +++ b/src/Starport.sol @@ -94,13 +94,16 @@ contract Starport is PausableNonReentrant { bytes32 private constant _LOAN_EXISTS = 0x14ec57fc00000000000000000000000000000000000000000000000000000000; Stargate public immutable SG; + uint256 public immutable chainId; address public immutable defaultCustodian; bytes32 public immutable DEFAULT_CUSTODIAN_CODE_HASH; + bytes32 public immutable CACHED_DOMAIN_SEPARATOR; // Define the EIP712 domain and typeHash constants for generating signatures bytes32 public constant EIP_DOMAIN = - keccak256("EIP712Domain(" "string version," "uint256 chainId," "address verifyingContract" ")"); + keccak256("EIP712Domain(" "string name," "string version," "uint256 chainId," "address verifyingContract" ")"); bytes32 public constant VERSION = keccak256(bytes("0")); + bytes32 public constant NAME = keccak256(bytes("Starport")); bytes32 public constant INTENT_ORIGINATION_TYPEHASH = keccak256( "Origination(" "address account," "uint256 accountNonce," "bool singleUse," "bytes32 salt," "uint256 deadline," @@ -165,19 +168,25 @@ contract Starport is PausableNonReentrant { constructor(address seaport_, Stargate stargate_) { SG = stargate_; + chainId = block.chainid; + CACHED_DOMAIN_SEPARATOR = keccak256(abi.encode(EIP_DOMAIN, NAME, VERSION, block.chainid, address(this))); address custodian = address(new Custodian(this, seaport_)); bytes32 defaultCustodianCodeHash; assembly ("memory-safe") { defaultCustodianCodeHash := extcodehash(custodian) } - defaultCustodian = payable(custodian); + defaultCustodian = custodian; DEFAULT_CUSTODIAN_CODE_HASH = defaultCustodianCodeHash; _initializeOwner(msg.sender); } function domainSeparator() public view returns (bytes32) { - return keccak256(abi.encode(EIP_DOMAIN, VERSION, block.chainid, address(this))); + //return the cached domain separator if the chainId is the same + if (chainId == block.chainid) { + return CACHED_DOMAIN_SEPARATOR; + } + return keccak256(abi.encode(EIP_DOMAIN, NAME, VERSION, block.chainid, address(this))); } /*´:°•.°+.*•´.*:˚.°*.˚•´.°:°•.°•.*•´.*:˚.°*.˚•´.°:°•.°+.*•´.*:*/ @@ -502,10 +511,8 @@ contract Starport is PausableNonReentrant { assembly ("memory-safe") { codeHash := extcodehash(custodian) } - if ( - codeHash != DEFAULT_CUSTODIAN_CODE_HASH - && Custodian(payable(custodian)).custody(loan) != Custodian.custody.selector - ) { + if (codeHash != DEFAULT_CUSTODIAN_CODE_HASH && Custodian(custodian).custody(loan) != Custodian.custody.selector) + { revert InvalidCustodian(); } } diff --git a/src/lib/RefStarportLib.sol b/src/lib/RefStarportLib.sol index 91c4daa2..d1e989bb 100644 --- a/src/lib/RefStarportLib.sol +++ b/src/lib/RefStarportLib.sol @@ -56,12 +56,12 @@ library RefStarportLib { function validateSalt( mapping(address => mapping(bytes32 => bool)) storage usedSalts, - address borrower, + address validator, bytes32 salt ) internal { - if (usedSalts[borrower][salt]) { + if (usedSalts[validator][salt]) { revert InvalidSalt(); } - usedSalts[borrower][salt] = true; + usedSalts[validator][salt] = true; } } diff --git a/src/lib/StarportLib.sol b/src/lib/StarportLib.sol index 564b639d..7221ede2 100644 --- a/src/lib/StarportLib.sol +++ b/src/lib/StarportLib.sol @@ -105,11 +105,11 @@ library StarportLib { function validateSalt( mapping(address => mapping(bytes32 => bool)) storage usedSalts, - address borrower, + address validator, bytes32 salt ) internal { assembly ("memory-safe") { - mstore(0x0, borrower) + mstore(0x0, validator) mstore(0x20, usedSalts.slot) // usedSalts[borrower] diff --git a/src/pricing/SimpleInterestPricing.sol b/src/pricing/SimpleInterestPricing.sol index 7c2ae628..87165715 100644 --- a/src/pricing/SimpleInterestPricing.sol +++ b/src/pricing/SimpleInterestPricing.sol @@ -86,7 +86,6 @@ contract SimpleInterestPricing is BasePricing { if (oldDetails.decimals == newDetails.decimals && (newDetails.rate < oldDetails.rate)) { (repayConsideration, carryConsideration) = getPaymentConsideration(loan); - additionalConsideration = new AdditionalTransfer[](0); } else { revert InvalidRefinance(); } diff --git a/src/settlement/DutchAuctionSettlement.sol b/src/settlement/DutchAuctionSettlement.sol index ab28d8aa..998791ca 100644 --- a/src/settlement/DutchAuctionSettlement.sol +++ b/src/settlement/DutchAuctionSettlement.sol @@ -50,7 +50,7 @@ abstract contract DutchAuctionSettlement is Settlement, AmountDeriver { /* CUSTOM ERRORS */ /*.•°:°.´+˚.*°.˚:*.´•*.+°.•°:´*.´•*.•°.•°:°.´:•˚°.*°.˚:*.´+°.•*/ - error InvalidAmount(); + error AuctionNotStarted(); /*´:°•.°+.*•´.*:˚.°*.˚•´.°:°•.°•.*•´.*:˚.°*.˚•´.°:°•.°+.*•´.*:*/ /* CONSTRUCTOR */ @@ -116,6 +116,9 @@ abstract contract DutchAuctionSettlement is Settlement, AmountDeriver { uint256 start = getAuctionStart(loan); + if (start > block.timestamp) { + revert AuctionNotStarted(); + } // DutchAuction has failed, allow lender to redeem if (start + details.window < block.timestamp) { return (new ReceivedItem[](0), loan.issuer); @@ -134,7 +137,7 @@ abstract contract DutchAuctionSettlement is Settlement, AmountDeriver { loan, pricingDetails.rate, loan.start, block.timestamp, 0, pricingDetails.decimals ); - uint256 carry = interest.mulWad(pricingDetails.carryRate); + uint256 carry = interest * pricingDetails.carryRate / 10 ** pricingDetails.decimals; if (carry > 0 && loan.debt[0].amount + interest - carry < settlementPrice) { consideration = new ReceivedItem[](2); diff --git a/src/status/FixedTermStatus.sol b/src/status/FixedTermStatus.sol index 0c0c4790..8a0433e8 100644 --- a/src/status/FixedTermStatus.sol +++ b/src/status/FixedTermStatus.sol @@ -47,7 +47,7 @@ contract FixedTermStatus is Status { // @inheritdoc Status function isActive(Starport.Loan calldata loan, bytes calldata extraData) external view override returns (bool) { Details memory details = abi.decode(loan.terms.statusData, (Details)); - return loan.start + details.loanDuration > block.timestamp; + return loan.start + details.loanDuration >= block.timestamp; } function validate(Starport.Loan calldata loan) external view override returns (bytes4) { diff --git a/test/integration-testing/TestNewLoan.sol b/test/integration-testing/TestNewLoan.sol index c77857ff..c4c75bd4 100644 --- a/test/integration-testing/TestNewLoan.sol +++ b/test/integration-testing/TestNewLoan.sol @@ -377,7 +377,7 @@ contract TestNewLoan is StarportTest { // //default is 14 day term Starport.Loan memory activeLoan = testNewLoanERC721CollateralDefaultTerms2(); - skip(14 days); + skip(14 days + 1); minimumReceived.push( SpentItem({itemType: ItemType.ERC20, token: address(erc20s[0]), amount: 600 ether, identifier: 0}) diff --git a/test/integration-testing/TestRepayLoan.sol b/test/integration-testing/TestRepayLoan.sol index 2eeb8dd8..a19e6990 100644 --- a/test/integration-testing/TestRepayLoan.sol +++ b/test/integration-testing/TestRepayLoan.sol @@ -176,7 +176,7 @@ contract TestRepayLoan is StarportTest { _createLoan721Collateral20Debt({lender: lender.addr, borrowAmount: borrowAmount, terms: terms}); vm.startPrank(borrower.addr); - skip(14 days); + skip(14 days + 1); BasePricing.Details memory details = abi.decode(loan.terms.pricingData, (BasePricing.Details)); uint256 interest = SimpleInterestPricing(loan.terms.pricing).calculateInterest( 10 days, loan.debt[0].amount, details.rate, details.decimals diff --git a/test/unit-testing/ModuleTesting.t.sol b/test/unit-testing/ModuleTesting.t.sol index de61ab75..d4c22ffd 100644 --- a/test/unit-testing/ModuleTesting.t.sol +++ b/test/unit-testing/ModuleTesting.t.sol @@ -104,6 +104,21 @@ contract ModuleTesting is StarportTest, DeepEq { ); } + function testFixedTermDutchAuctionSettlementAuctionNotStarted() public { + Starport.Terms memory terms = Starport.Terms({ + status: address(fixedTermStatus), + settlement: address(dutchAuctionSettlement), //fixed term dutch auction + pricing: address(simpleInterestPricing), + pricingData: defaultPricingData, + settlementData: defaultSettlementData, + statusData: defaultStatusData + }); + Starport.Loan memory loan = + _createLoan721Collateral20Debt({lender: lender.addr, borrowAmount: 100, terms: terms}); + vm.expectRevert(DutchAuctionSettlement.AuctionNotStarted.selector); + FixedTermDutchAuctionSettlement(loan.terms.settlement).getSettlementConsideration(loan); + } + function testFixedTermDutchAuctionSettlementNotValid() public { DutchAuctionSettlement.Details memory details = DutchAuctionSettlement.Details({startingPrice: 1 ether, endingPrice: 10 ether, window: 7 days}); diff --git a/test/unit-testing/TestCustodian.sol b/test/unit-testing/TestCustodian.sol index 579ac467..41b5c0e7 100644 --- a/test/unit-testing/TestCustodian.sol +++ b/test/unit-testing/TestCustodian.sol @@ -613,4 +613,26 @@ contract TestCustodian is StarportTest, DeepEq, MockCall { seaportAddr, alice, new SpentItem[](0), activeDebt, abi.encode(Actions.Nothing, activeLoan) ); } + + function testCustodianCannotBeAuthorized() public { + mockStatusCall(activeLoan.terms.status, false); + mockSettlementCall(activeLoan.terms.settlement, new ReceivedItem[](0), address(custodian)); + + vm.expectRevert(abi.encodeWithSelector(Custodian.CustodianCannotBeAuthorized.selector)); + custodian.previewOrder( + seaportAddr, + alice, + new SpentItem[](0), + activeDebt, + abi.encode(Custodian.Command(Actions.Settlement, activeLoan, "")) + ); + + mockStatusCall(activeLoan.terms.status, false); + mockSettlementCall(activeLoan.terms.settlement, new ReceivedItem[](0), address(custodian)); + vm.prank(seaportAddr); + vm.expectRevert(abi.encodeWithSelector(Custodian.CustodianCannotBeAuthorized.selector)); + custodian.generateOrder( + alice, new SpentItem[](0), activeDebt, abi.encode(Custodian.Command(Actions.Settlement, activeLoan, "")) + ); + } }