Skip to content

Commit

Permalink
Fix/audit fixes round 8 (#95)
Browse files Browse the repository at this point in the history 
* fix: validate that window is not zero i nthe valdiate method

* fix: I-10. maximumSpent isn’t used on Custodian.generateOrder and can be removed

* fix: activeUserDataHash unused in BNPLHelper

* fix: M-3. Error-prone string casting for tokenURI

* fix: strategist fee unused

* fix: remove mentions of loan manager

* fix: remove mention of fee

* fix: remove address(this) in params for call to transferSpentItemsSelf

* fix: gas optimization of assignment

* fix: gas snapshot
  • Loading branch information
@dangerousfood
dangerousfood authored Jan 25, 2024
1 parent a653644 commit 748326b
Show file tree
Hide file tree
Showing 11 changed files with 120 additions and 140 deletions.
159 changes: 79 additions & 80 deletions .gas-snapshot
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,94 +16,94 @@ IntegrationTestCaveats:testRefinanceWCaveatsInvalidSalt() (gas: 377260)
IntegrationTestCaveats:testRefinanceWLenderApproval() (gas: 402978)
ModuleTesting:testFixedTermDutchAuctionSettlement() (gas: 438123)
ModuleTesting:testFixedTermDutchAuctionSettlementAuctionNotStarted() (gas: 441499)
ModuleTesting:testFixedTermDutchAuctionSettlementGetSettlementAuctionExpired() (gas: 440948)
ModuleTesting:testFixedTermDutchAuctionSettlementNotValid() (gas: 437157)
ModuleTesting:testFixedTermDutchAuctionSettlementValid() (gas: 438044)
ModuleTesting:testModuleValidation() (gas: 1274344)
ModuleTesting:testFixedTermDutchAuctionSettlementGetSettlementAuctionExpired() (gas: 440892)
ModuleTesting:testFixedTermDutchAuctionSettlementNotValid() (gas: 437177)
ModuleTesting:testFixedTermDutchAuctionSettlementValid() (gas: 438084)
ModuleTesting:testModuleValidation() (gas: 1262975)
PausableNonReentrantImpl:test() (gas: 2464)
PausableNonReentrantImpl:testReentrancy() (gas: 2757)
TestBorrowerEnforcer:testBERevertAdditionalTransfersFromBorrower() (gas: 76462)
TestBorrowerEnforcer:testBERevertInvalidLoanTerms() (gas: 81160)
TestBorrowerEnforcer:testBEValidLoanTerms() (gas: 72257)
TestBorrowerEnforcer:testBEValidLoanTermsAnyIssuer() (gas: 72343)
TestCustodian:testCannotLazyMintTwice() (gas: 82123)
TestCustodian:testCannotMintInvalidLoanInvalidCustodian() (gas: 72495)
TestCustodian:testCannotMintInvalidLoanValidCustodian() (gas: 77961)
TestCustodian:testCustodianCannotBeAuthorized() (gas: 142120)
TestCustodian:testCustodySelector() (gas: 2731962)
TestCustodian:testCannotLazyMintTwice() (gas: 82105)
TestCustodian:testCannotMintInvalidLoanInvalidCustodian() (gas: 72477)
TestCustodian:testCannotMintInvalidLoanValidCustodian() (gas: 77943)
TestCustodian:testCustodianCannotBeAuthorized() (gas: 141967)
TestCustodian:testCustodySelector() (gas: 2717132)
TestCustodian:testDefaultCustodySelectorRevert() (gas: 72478)
TestCustodian:testGenerateOrderInvalidPostRepayment() (gas: 173134)
TestCustodian:testGenerateOrderInvalidPostSettlement() (gas: 163297)
TestCustodian:testGenerateOrderRepay() (gas: 177292)
TestCustodian:testGenerateOrderRepayAsRepayApprovedBorrower() (gas: 193778)
TestCustodian:testGenerateOrderRepayERC1155AndERC20() (gas: 876355)
TestCustodian:testGenerateOrderRepayERC1155AndERC20HandlerAuthorized() (gas: 804921)
TestCustodian:testGenerateOrderRepayInvalidHookAddress() (gas: 97670)
TestCustodian:testGenerateOrderRepayInvalidHookReturnType() (gas: 92031)
TestCustodian:testGenerateOrderRepayNotBorrower() (gas: 106909)
TestCustodian:testGenerateOrderSettlement() (gas: 155008)
TestCustodian:testGenerateOrderSettlementHandlerAuthorized() (gas: 160405)
TestCustodian:testGenerateOrderSettlementNoActiveLoan() (gas: 163475)
TestCustodian:testGenerateOrderSettlementUnauthorized() (gas: 101874)
TestCustodian:testGenerateOrdersWithLoanStartAtBlockTimestampInvalidLoan() (gas: 461703)
TestCustodian:testGetBorrower() (gas: 78621)
TestCustodian:testInvalidAction() (gas: 173464)
TestCustodian:testInvalidActionRepayInActiveLoan() (gas: 130152)
TestCustodian:testInvalidActionSettleActiveLoan() (gas: 130090)
TestCustodian:testGenerateOrderInvalidPostRepayment() (gas: 172899)
TestCustodian:testGenerateOrderInvalidPostSettlement() (gas: 163178)
TestCustodian:testGenerateOrderRepay() (gas: 177059)
TestCustodian:testGenerateOrderRepayAsRepayApprovedBorrower() (gas: 193527)
TestCustodian:testGenerateOrderRepayERC1155AndERC20() (gas: 875677)
TestCustodian:testGenerateOrderRepayERC1155AndERC20HandlerAuthorized() (gas: 804495)
TestCustodian:testGenerateOrderRepayInvalidHookAddress() (gas: 97556)
TestCustodian:testGenerateOrderRepayInvalidHookReturnType() (gas: 91917)
TestCustodian:testGenerateOrderRepayNotBorrower() (gas: 106827)
TestCustodian:testGenerateOrderSettlement() (gas: 154891)
TestCustodian:testGenerateOrderSettlementHandlerAuthorized() (gas: 160288)
TestCustodian:testGenerateOrderSettlementNoActiveLoan() (gas: 163374)
TestCustodian:testGenerateOrderSettlementUnauthorized() (gas: 101809)
TestCustodian:testGenerateOrdersWithLoanStartAtBlockTimestampInvalidLoan() (gas: 461511)
TestCustodian:testGetBorrower() (gas: 78603)
TestCustodian:testInvalidAction() (gas: 179690)
TestCustodian:testInvalidActionRepayInActiveLoan() (gas: 130036)
TestCustodian:testInvalidActionSettleActiveLoan() (gas: 129973)
TestCustodian:testInvalidEncodedData() (gas: 26160)
TestCustodian:testMintWithApprovalSetAsBorrower() (gas: 366758)
TestCustodian:testMintWithApprovalSetAsBorrowerInvalidLoan() (gas: 64523)
TestCustodian:testMintWithApprovalSetAsBorrower() (gas: 366740)
TestCustodian:testMintWithApprovalSetAsBorrowerInvalidLoan() (gas: 64505)
TestCustodian:testMintWithApprovalSetNotAuthorized() (gas: 66842)
TestCustodian:testName() (gas: 7099)
TestCustodian:testName() (gas: 7077)
TestCustodian:testNonPayableFunctions() (gas: 215289)
TestCustodian:testOnlySeaport() (gas: 17918)
TestCustodian:testPreviewOrderNoActiveLoan() (gas: 105688)
TestCustodian:testPreviewOrderRepay() (gas: 230231)
TestCustodian:testPreviewOrderSettlement() (gas: 191959)
TestCustodian:testPreviewOrderSettlementInvalidFufliller() (gas: 108249)
TestCustodian:testPreviewOrderSettlementInvalidRepayer() (gas: 116960)
TestCustodian:testRatifyOrder() (gas: 184113)
TestCustodian:testSeaportMetadata() (gas: 8644)
TestCustodian:testPreviewOrderNoActiveLoan() (gas: 105670)
TestCustodian:testPreviewOrderRepay() (gas: 229793)
TestCustodian:testPreviewOrderSettlement() (gas: 191754)
TestCustodian:testPreviewOrderSettlementInvalidFufliller() (gas: 108161)
TestCustodian:testPreviewOrderSettlementInvalidRepayer() (gas: 116854)
TestCustodian:testRatifyOrder() (gas: 183880)
TestCustodian:testSeaportMetadata() (gas: 8588)
TestCustodian:testSupportsInterface() (gas: 9428)
TestCustodian:testSymbol() (gas: 7216)
TestCustodian:testTokenURI() (gas: 85150)
TestCustodian:testSymbol() (gas: 7194)
TestCustodian:testTokenURI() (gas: 85244)
TestCustodian:testTokenURIInvalidLoan() (gas: 13179)
TestLenderEnforcer:testLERevertAdditionalTransfersFromLender() (gas: 76455)
TestLenderEnforcer:testLERevertInvalidLoanTerms() (gas: 81096)
TestLenderEnforcer:testLEValidLoanTerms() (gas: 72169)
TestLenderEnforcer:testLEValidLoanTermsAnyBorrower() (gas: 72234)
TestLenderEnforcer:testLEValidLoanTermsWithAdditionalTransfers() (gas: 73525)
TestLoanCombinations:testLoan20For721SimpleInterestDutchFixedRepay() (gas: 592934)
TestLoanCombinations:testLoan20for20SimpleInterestDutchFixedRepay() (gas: 600147)
TestLoanCombinations:testLoan721for20SimpleInterestDutchFixedRepay() (gas: 590365)
TestLoanCombinations:testLoanAstariaSettlementRepay() (gas: 580182)
TestNewLoan:testBuyNowPayLater() (gas: 3018529)
TestNewLoan:testNewLoanAs1271ProxyAccountSender() (gas: 874195)
TestNewLoan:testNewLoanAs1271ProxyAccountThirdPartyFiller() (gas: 885220)
TestLoanCombinations:testLoan20For721SimpleInterestDutchFixedRepay() (gas: 592496)
TestLoanCombinations:testLoan20for20SimpleInterestDutchFixedRepay() (gas: 599709)
TestLoanCombinations:testLoan721for20SimpleInterestDutchFixedRepay() (gas: 589927)
TestLoanCombinations:testLoanAstariaSettlementRepay() (gas: 579744)
TestNewLoan:testBuyNowPayLater() (gas: 3018513)
TestNewLoan:testNewLoanAs1271ProxyAccountSender() (gas: 874179)
TestNewLoan:testNewLoanAs1271ProxyAccountThirdPartyFiller() (gas: 885204)
TestNewLoan:testNewLoanERC721CollateralDefaultTerms2() (gas: 429635)
TestNewLoan:testNewLoanRefinance() (gas: 590070)
TestNewLoan:testNewLoanViaOriginatorBorrowerApprovalAndLenderApproval() (gas: 324676)
TestNewLoan:testNewLoanViaOriginatorLenderApproval() (gas: 383472)
TestNewLoan:testSettleLoan() (gas: 642281)
TestNewLoan:testNewLoanRefinance() (gas: 589971)
TestNewLoan:testNewLoanViaOriginatorBorrowerApprovalAndLenderApproval() (gas: 324707)
TestNewLoan:testNewLoanViaOriginatorLenderApproval() (gas: 383503)
TestNewLoan:testSettleLoan() (gas: 642156)
TestPausableNonReentrant:testNotOwner() (gas: 21276)
TestPausableNonReentrant:testPauseAndUnpause() (gas: 22643)
TestPausableNonReentrant:testReentrancy() (gas: 15404)
TestPausableNonReentrant:testUnpauseWhenNotPaused() (gas: 12604)
TestRepayLoan:testRepayLoanApprovedRepayer() (gas: 667145)
TestRepayLoan:testRepayLoanBase() (gas: 599975)
TestRepayLoan:testRepayLoanApprovedRepayer() (gas: 666673)
TestRepayLoan:testRepayLoanBase() (gas: 599539)
TestRepayLoan:testRepayLoanGenerateOrderNotSeaport() (gas: 438777)
TestRepayLoan:testRepayLoanInSettlement() (gas: 585811)
TestRepayLoan:testRepayLoanInvalidRepayer() (gas: 604010)
TestRepayLoan:testRepayLoanThatDoesNotExist() (gas: 858771)
TestSimpleInterestPricing:test_calculateInterest() (gas: 881296)
TestSimpleInterestPricing:test_getPaymentConsideration() (gas: 928510)
TestSimpleInterestPricing:test_getRefinanceConsideration() (gas: 919314)
TestRepayLoan:testRepayLoanInSettlement() (gas: 585633)
TestRepayLoan:testRepayLoanInvalidRepayer() (gas: 603643)
TestRepayLoan:testRepayLoanThatDoesNotExist() (gas: 857698)
TestSimpleInterestPricing:test_calculateInterest() (gas: 869870)
TestSimpleInterestPricing:test_getPaymentConsideration() (gas: 916886)
TestSimpleInterestPricing:test_getRefinanceConsideration() (gas: 907796)
TestStarport:testAcquireTokensFail() (gas: 60473)
TestStarport:testAcquireTokensSuccess() (gas: 163042)
TestStarport:testAcquireTokensSuccess() (gas: 162844)
TestStarport:testActive() (gas: 69358)
TestStarport:testAdditionalTransfers() (gas: 300755)
TestStarport:testAdditionalTransfersOriginate() (gas: 275540)
TestStarport:testAdditionalTransfersRefinance() (gas: 218206)
TestStarport:testAdditionalTransfersRefinance() (gas: 218107)
TestStarport:testApplyRefinanceConsiderationToLoanMalformed() (gas: 129484)
TestStarport:testCannotIssueSameLoanTwice() (gas: 364125)
TestStarport:testCannotOriginateWhilePaused() (gas: 73479)
Expand All @@ -112,16 +112,16 @@ TestStarport:testCannotSettleUnlessValidCustodian() (gas: 71007)
TestStarport:testCaveatEnforcerRevert() (gas: 102601)
TestStarport:testDefaultFeeRake1() (gas: 383458)
TestStarport:testDefaultFeeRake2() (gas: 445821)
TestStarport:testDefaultFeeRakeExoticDebt() (gas: 394564)
TestStarport:testDefaultFeeRakeExoticDebt() (gas: 394368)
TestStarport:testEIP712Signing() (gas: 83109)
TestStarport:testExcessiveFeeRake() (gas: 19992)
TestStarport:testExoticDebtWithCustomPricingAndRepayment() (gas: 1237783)
TestStarport:testExoticDebtWithCustomPricingAndSettlement() (gas: 1692826)
TestStarport:testExoticDebtWithNoCaveatsNotAsBorrower() (gas: 376831)
TestStarport:testExoticDebtWithCustomPricingAndSettlement() (gas: 1695822)
TestStarport:testExoticDebtWithNoCaveatsNotAsBorrower() (gas: 376639)
TestStarport:testIncrementCaveatNonce() (gas: 35117)
TestStarport:testInitializedFlagSetProperly() (gas: 67372)
TestStarport:testInvalidAdditionalTransfersOriginate() (gas: 230426)
TestStarport:testInvalidAdditionalTransfersRefinance() (gas: 170766)
TestStarport:testInvalidAdditionalTransfersRefinance() (gas: 170667)
TestStarport:testInvalidAmountCollateral() (gas: 165968)
TestStarport:testInvalidAmountCollateral721() (gas: 165968)
TestStarport:testInvalidItemType() (gas: 151916)
Expand All @@ -133,23 +133,22 @@ TestStarport:testNonDefaultCustodianCustodyCallSuccess() (gas: 290392)
TestStarport:testNonPayableFunctions() (gas: 114479)
TestStarport:testOverrideFeeRake() (gas: 379873)
TestStarport:testPause() (gas: 18127)
TestStarport:testRefinancePostRepaymentFails() (gas: 127864)
TestStarport:testRefinancePostRepaymentFails() (gas: 127765)
TestStarport:testStargateGetOwner() (gas: 8851)
TestStarport:testTokenNoCodeCollateral() (gas: 150707)
TestStarport:testTokenNoCodeDebt() (gas: 180980)
TestStarport:testUnpause() (gas: 17187)
TestStrategistOriginator:testEncodeWithAccountCounter() (gas: 12330)
TestStrategistOriginator:testGetStrategistData() (gas: 1790990)
TestStrategistOriginator:testIncrementCounterAsStrategist() (gas: 38488)
TestStrategistOriginator:testIncrementCounterNotAuthorized() (gas: 13423)
TestStrategistOriginator:testInvalidCollateral() (gas: 210380)
TestStrategistOriginator:testInvalidDeadline() (gas: 216201)
TestStrategistOriginator:testInvalidDebt() (gas: 212088)
TestStrategistOriginator:testInvalidDebtAmountAskingMoreThanOffered() (gas: 212462)
TestStrategistOriginator:testInvalidDebtAmountOfferingZero() (gas: 212792)
TestStrategistOriginator:testInvalidDebtAmountRequestingZero() (gas: 212727)
TestStrategistOriginator:testInvalidDebtLength() (gas: 211382)
TestStrategistOriginator:testInvalidOffer() (gas: 427161)
TestStrategistOriginator:testInvalidSigner() (gas: 214520)
TestStrategistOriginator:testSetStrategist() (gas: 17884)
TestStrategistOriginator:testWithdraw() (gas: 167966)
TestStrategistOriginator:testEncodeWithAccountCounter() (gas: 12429)
TestStrategistOriginator:testIncrementCounterAsStrategist() (gas: 38457)
TestStrategistOriginator:testIncrementCounterNotAuthorized() (gas: 13468)
TestStrategistOriginator:testInvalidCollateral() (gas: 210516)
TestStrategistOriginator:testInvalidDeadline() (gas: 216337)
TestStrategistOriginator:testInvalidDebt() (gas: 212224)
TestStrategistOriginator:testInvalidDebtAmountAskingMoreThanOffered() (gas: 212708)
TestStrategistOriginator:testInvalidDebtAmountOfferingZero() (gas: 212928)
TestStrategistOriginator:testInvalidDebtAmountRequestingZero() (gas: 212863)
TestStrategistOriginator:testInvalidDebtLength() (gas: 211518)
TestStrategistOriginator:testInvalidOffer() (gas: 427273)
TestStrategistOriginator:testInvalidSigner() (gas: 214678)
TestStrategistOriginator:testSetStrategist() (gas: 17852)
TestStrategistOriginator:testWithdraw() (gas: 167733)
20 changes: 7 additions & 13 deletions src/BNPLHelper.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,17 +77,11 @@ contract BNPLHelper is IFlashLoanRecipient, Ownable {
error InvalidUserDataProvided();
error SenderNotVault();

/*´:°•.°+.*•´.*:˚.°*.˚•´.°:°•.°•.*•´.*:˚.°*.˚•´.°:°•.°+.*•´.*:*/
/* CONSTANTS AND IMMUTABLES */
/*.•°:°.´+˚.*°.˚:*.´•*.+°.•°:´*.´•*.•°.•°:°.´:•˚°.*°.˚:*.´+°.•*/

address private VAULT;

/*´:°•.°+.*•´.*:˚.°*.˚•´.°:°•.°•.*•´.*:˚.°*.˚•´.°:°•.°+.*•´.*:*/
/* STORAGE */
/*.•°:°.´+˚.*°.˚:*.´•*.+°.•°:´*.´•*.•°.•°:°.´:•˚°.*°.˚:*.´+°.•*/

bytes32 private activeUserDataHash;
address private vault;

/*´:°•.°+.*•´.*:˚.°*.˚•´.°:°•.°•.*•´.*:˚.°*.˚•´.°:°•.°+.*•´.*:*/
/* STRUCTS */
Expand All @@ -105,13 +99,13 @@ contract BNPLHelper is IFlashLoanRecipient, Ownable {
Fulfillment[] fulfillments;
}

constructor(address vault, address owner) {
VAULT = vault;
constructor(address _vault, address owner) {
vault = _vault;
_initializeOwner(owner);
}

function makeFlashLoan(address[] calldata tokens, uint256[] calldata amounts, bytes calldata userData) external {
IVault(VAULT).flashLoan(this, tokens, amounts, userData);
IVault(vault).flashLoan(this, tokens, amounts, userData);
}

function receiveFlashLoan(
Expand Down Expand Up @@ -140,7 +134,7 @@ contract BNPLHelper is IFlashLoanRecipient, Ownable {
identifier: 0,
token: tokens[i],
from: execution.borrower,
to: VAULT,
to: vault,
amount: amounts[i] + feeAmounts[i]
});
unchecked {
Expand All @@ -152,7 +146,7 @@ contract BNPLHelper is IFlashLoanRecipient, Ownable {
);
}

function setFlashVault(address vault) external onlyOwner {
VAULT = vault;
function setFlashVault(address _vault) external onlyOwner {
vault = _vault;
}
}
11 changes: 6 additions & 5 deletions src/Custodian.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import {LibString} from "solady/src/utils/LibString.sol";

contract Custodian is ERC721, ContractOffererInterface {
using {StarportLib.getId} for Starport.Loan;
using {LibString.concat} for string;

/*´:°•.°+.*•´.*:˚.°*.˚•´.°:°•.°•.*•´.*:˚.°*.˚•´.°:°•.°+.*•´.*:*/
/* CUSTOM ERRORS */
Expand Down Expand Up @@ -124,7 +125,7 @@ contract Custodian is ERC721, ContractOffererInterface {
if (!_exists(loanId)) {
revert InvalidLoan();
}
return string(abi.encodePacked("https://astaria.xyz/loans/", LibString.toString(loanId)));
return string("https://astaria.xyz/metadata/loan/").concat(LibString.toString(loanId));
}

/**
Expand Down Expand Up @@ -210,23 +211,23 @@ contract Custodian is ERC721, ContractOffererInterface {
/**
* @dev Generates the order for this contract offerer
* @param fulfiller The address of the contract fulfiller
* @param maximumSpent The maximum amount of items to be spent by the order
* @param context The context of the order
* @return offer The items spent by the order
* @return consideration The items received by the order
*/
function generateOrder(
address fulfiller,
SpentItem[] calldata,
SpentItem[] calldata maximumSpent,
SpentItem[] calldata,
bytes calldata context // encoded based on the schemaID
) external onlySeaport returns (SpentItem[] memory offer, ReceivedItem[] memory consideration) {
(Command memory close) = abi.decode(context, (Command));
Starport.Loan memory loan = close.loan;
if (loan.start == block.timestamp) {
revert InvalidLoan();
}
if (close.action == Actions.Repayment && Status(loan.terms.status).isActive(loan, close.extraData)) {
bool loanActive = Status(loan.terms.status).isActive(loan, close.extraData);
if (close.action == Actions.Repayment && loanActive) {
if (fulfiller != getBorrower(loan) && fulfiller != _getApproved(loan.getId())) {
revert InvalidRepayer();
}
Expand All @@ -240,7 +241,7 @@ contract Custodian is ERC721, ContractOffererInterface {

_settleLoan(loan);
_postRepaymentExecute(loan, fulfiller);
} else if (close.action == Actions.Settlement && !Status(loan.terms.status).isActive(loan, close.extraData)) {
} else if (close.action == Actions.Settlement && !loanActive) {
address authorized;
_beforeGetSettlementConsideration(loan);
(consideration, authorized) = Settlement(loan.terms.settlement).getSettlementConsideration(loan);
Expand Down
Loading

0 comments on commit 748326b

Please sign in to comment.