From f48ef0c917667e1edb44674dcb820b0effea580c Mon Sep 17 00:00:00 2001 From: Jamie Pickett Date: Thu, 10 Aug 2023 17:02:36 -0400 Subject: [PATCH 1/5] fixed tests, edited edge case for ltv on global policy and changes for statemind-172 and statemind-174 --- contracts/peer-to-peer/QuoteHandler.sol | 39 +++++++++++- .../peer-to-peer/interfaces/IQuoteHandler.sol | 15 ++++- .../BasicQuotePolicyManager.sol | 11 ++-- test/peer-to-peer/helpers/misc.ts | 4 +- test/peer-to-peer/local-tests.ts | 62 ++++++++++++++++--- test/peer-to-peer/mainnet-forked-tests.ts | 58 ++++++++--------- 6 files changed, 143 insertions(+), 46 deletions(-) diff --git a/contracts/peer-to-peer/QuoteHandler.sol b/contracts/peer-to-peer/QuoteHandler.sol index 54141f6d..7ec6e60e 100644 --- a/contracts/peer-to-peer/QuoteHandler.sol +++ b/contracts/peer-to-peer/QuoteHandler.sol @@ -21,6 +21,7 @@ contract QuoteHandler is IQuoteHandler { public offChainQuoteIsInvalidated; mapping(address => mapping(bytes32 => bool)) public isOnChainQuote; mapping(bytes32 => bool) public isPublishedOnChainQuote; + mapping(bytes32 => uint256) public publishedOnChainQuoteValidUntil; mapping(address => address) public quotePolicyManagerForVault; mapping(address => DataTypesPeerToPeer.OnChainQuoteInfo[]) internal onChainQuoteHistory; @@ -115,12 +116,21 @@ contract QuoteHandler is IQuoteHandler { _checkIsVaultAndSenderIsApproved(lenderVault, false); mapping(bytes32 => bool) storage isOnChainQuoteFromVault = isOnChainQuote[lenderVault]; + uint256 validUntil = publishedOnChainQuoteValidUntil[onChainQuoteHash]; if ( !isPublishedOnChainQuote[onChainQuoteHash] || - isOnChainQuoteFromVault[onChainQuoteHash] + isOnChainQuoteFromVault[onChainQuoteHash] || + validUntil < block.timestamp ) { revert Errors.InvalidQuote(); } + // @dev: on-chain quote history is append only + onChainQuoteHistory[lenderVault].push( + DataTypesPeerToPeer.OnChainQuoteInfo({ + quoteHash: onChainQuoteHash, + validUntil: validUntil + }) + ); isOnChainQuoteFromVault[onChainQuoteHash] = true; emit OnChainQuoteCopied(lenderVault, onChainQuoteHash); } @@ -136,6 +146,9 @@ contract QuoteHandler is IQuoteHandler { revert Errors.AlreadyPublished(); } isPublishedOnChainQuote[onChainQuoteHash] = true; + publishedOnChainQuoteValidUntil[onChainQuoteHash] = onChainQuote + .generalQuoteInfo + .validUntil; emit OnChainQuotePublished(onChainQuote, onChainQuoteHash, msg.sender); } @@ -293,9 +306,29 @@ contract QuoteHandler is IQuoteHandler { } function getFullOnChainQuoteHistory( - address lenderVault + address lenderVault, + uint256 startIdx, + uint256 endIdx ) external view returns (DataTypesPeerToPeer.OnChainQuoteInfo[] memory) { - return onChainQuoteHistory[lenderVault]; + if ( + startIdx >= endIdx || + endIdx > onChainQuoteHistory[lenderVault].length + ) { + revert Errors.InvalidArrayIndex(); + } + DataTypesPeerToPeer.OnChainQuoteInfo[] + memory onChainQuoteHistoryRequested = new DataTypesPeerToPeer.OnChainQuoteInfo[]( + endIdx - startIdx + ); + for (uint256 i = startIdx; i < endIdx; ) { + onChainQuoteHistoryRequested[i - startIdx] = onChainQuoteHistory[ + lenderVault + ][i]; + unchecked { + ++i; + } + } + return onChainQuoteHistoryRequested; } function getOnChainQuoteHistoryLength( diff --git a/contracts/peer-to-peer/interfaces/IQuoteHandler.sol b/contracts/peer-to-peer/interfaces/IQuoteHandler.sol index b697c07e..600bac66 100644 --- a/contracts/peer-to-peer/interfaces/IQuoteHandler.sol +++ b/contracts/peer-to-peer/interfaces/IQuoteHandler.sol @@ -217,6 +217,15 @@ interface IQuoteHandler { bytes32 hashToCheck ) external view returns (bool); + /** + * @notice function returns valid until timestamp of the published on-chain quote + * @param hashToCheck hash of the on chain quote + * @return valid until timestamp of the published on-chain quote + */ + function publishedOnChainQuoteValidUntil( + bytes32 hashToCheck + ) external view returns (uint256); + /** * @notice function returns the address of the policy manager for a vault * @param lenderVault address of vault @@ -241,10 +250,14 @@ interface IQuoteHandler { /** * @notice function returns array of structs containing the on-chain quote hash and validUntil timestamp * @param lenderVault address of vault + * @param startIdx starting index from on chain quote history array + * @param endIdx ending index of on chain quote history array (non-inclusive) * @return array of quote hash and validUntil data for on-chain quote history of a vault */ function getFullOnChainQuoteHistory( - address lenderVault + address lenderVault, + uint256 startIdx, + uint256 endIdx ) external view returns (DataTypesPeerToPeer.OnChainQuoteInfo[] memory); /** diff --git a/contracts/peer-to-peer/policyManagers/BasicQuotePolicyManager.sol b/contracts/peer-to-peer/policyManagers/BasicQuotePolicyManager.sol index ba9a4ca0..d831334e 100644 --- a/contracts/peer-to-peer/policyManagers/BasicQuotePolicyManager.sol +++ b/contracts/peer-to-peer/policyManagers/BasicQuotePolicyManager.sol @@ -170,7 +170,8 @@ contract BasicQuotePolicyManager is IQuotePolicyManager { quoteBounds, quoteTuple, generalQuoteInfo.earliestRepayTenor, - requiresOracle + hasOracle, + !hasOracle && !hasPairPolicy ), minNumOfSignersOverwrite ); @@ -260,7 +261,8 @@ contract BasicQuotePolicyManager is IQuotePolicyManager { DataTypesBasicPolicies.QuoteBounds memory quoteBounds, DataTypesPeerToPeer.QuoteTuple calldata quoteTuple, uint256 earliestRepayTenor, - bool checkLtv + bool checkLtv, + bool skipLoanPerCollCheck ) internal pure returns (bool) { if ( quoteTuple.tenor < quoteBounds.minTenor || @@ -274,8 +276,9 @@ contract BasicQuotePolicyManager is IQuotePolicyManager { ? (quoteBounds.minLtv, quoteBounds.maxLtv) : (quoteBounds.minLoanPerCollUnit, quoteBounds.maxLoanPerCollUnit); if ( - quoteTuple.loanPerCollUnitOrLtv < lowerBnd || - quoteTuple.loanPerCollUnitOrLtv > upperBnd + !skipLoanPerCollCheck && + (quoteTuple.loanPerCollUnitOrLtv < lowerBnd || + quoteTuple.loanPerCollUnitOrLtv > upperBnd) ) { return false; } diff --git a/test/peer-to-peer/helpers/misc.ts b/test/peer-to-peer/helpers/misc.ts index a7e936bd..6be7d8bf 100644 --- a/test/peer-to-peer/helpers/misc.ts +++ b/test/peer-to-peer/helpers/misc.ts @@ -333,8 +333,8 @@ export type QuoteBounds = { minEarliestRepayTenor: BigNumber minLtv: BigNumber maxLtv: BigNumber - minLoanPerCollUnitOrLtv: BigNumber - maxLoanPerCollUnitOrLtv: BigNumber + minLoanPerCollUnit: BigNumber + maxLoanPerCollUnit: BigNumber } export const encodeGlobalPolicy = (requiresOracle: boolean, quoteBounds: QuoteBounds): string => { diff --git a/test/peer-to-peer/local-tests.ts b/test/peer-to-peer/local-tests.ts index 452e8e29..29e4ea84 100644 --- a/test/peer-to-peer/local-tests.ts +++ b/test/peer-to-peer/local-tests.ts @@ -16,6 +16,7 @@ const BASE = ethers.BigNumber.from(10).pow(18) const ONE_USDC = ethers.BigNumber.from(10).pow(6) const ONE_WETH = ethers.BigNumber.from(10).pow(18) const MAX_UINT256 = ethers.BigNumber.from(2).pow(256).sub(1) +const MAX_UINT128 = ethers.BigNumber.from(2).pow(128).sub(1) const ONE_DAY = ethers.BigNumber.from(60 * 60 * 24) const ZERO_BYTES32 = ethers.utils.formatBytes32String('') const ZERO_ADDRESS = '0x0000000000000000000000000000000000000000' @@ -3378,7 +3379,19 @@ describe('Peer-to-Peer: Local Tests', function () { 'OnChainQuoteAdded' ) - const quoteHashAndValidUntilArr = await quoteHandler.getFullOnChainQuoteHistory(lenderVault.address) + // start idx >= end idx should revert + await expect(quoteHandler.getFullOnChainQuoteHistory(lenderVault.address, 0, 0)).to.be.revertedWithCustomError( + quoteHandler, + 'InvalidArrayIndex' + ) + + // end idx > history length should revert + await expect(quoteHandler.getFullOnChainQuoteHistory(lenderVault.address, 0, 3)).to.be.revertedWithCustomError( + quoteHandler, + 'InvalidArrayIndex' + ) + + const quoteHashAndValidUntilArr = await quoteHandler.getFullOnChainQuoteHistory(lenderVault.address, 0, 2) const historyLen = await quoteHandler.getOnChainQuoteHistoryLength(lenderVault.address) expect(quoteHashAndValidUntilArr.length).to.equal(2) @@ -3430,6 +3443,15 @@ describe('Peer-to-Peer: Local Tests', function () { salt: ZERO_BYTES32 } + // new quote to with same valid until + let onChainQuote2 = { + ...onChainQuote, + generalQuoteInfo: { + ...onChainQuote.generalQuoteInfo, + earliestRepayTenor: ONE_DAY.mul(2) + } + } + await expect( quoteHandler.connect(lender).publishOnChainQuote({ ...onChainQuote, @@ -3445,10 +3467,26 @@ describe('Peer-to-Peer: Local Tests', function () { return x.event === 'OnChainQuotePublished' }) + const proposedQuoteTransaction2 = await quoteHandler.connect(borrower).publishOnChainQuote(onChainQuote2) + + const proposedQuoteReceipt2 = await proposedQuoteTransaction2.wait() + + const proposeQuoteEvent2 = proposedQuoteReceipt2.events?.find(x => { + return x.event === 'OnChainQuotePublished' + }) + const proposedOnChainQuoteHash = proposeQuoteEvent?.args?.onChainQuoteHash || ZERO_BYTES32 + const proposedOnChainQuoteHash2 = proposeQuoteEvent2?.args?.onChainQuoteHash || ZERO_BYTES32 + expect(await quoteHandler.isPublishedOnChainQuote(proposedOnChainQuoteHash)).to.be.true + expect(await quoteHandler.publishedOnChainQuoteValidUntil(proposedOnChainQuoteHash)).to.equal(timestamp + 60) + + expect(await quoteHandler.isPublishedOnChainQuote(proposedOnChainQuoteHash2)).to.be.true + + expect(await quoteHandler.publishedOnChainQuoteValidUntil(proposedOnChainQuoteHash2)).to.equal(timestamp + 60) + await expect( quoteHandler.connect(borrower).copyPublishedOnChainQuote(lenderVault.address, proposedOnChainQuoteHash) ).to.be.revertedWithCustomError(quoteHandler, 'InvalidSender') @@ -3459,6 +3497,8 @@ describe('Peer-to-Peer: Local Tests', function () { expect(await quoteHandler.isPublishedOnChainQuote(proposedOnChainQuoteHash)).to.be.true + expect(await quoteHandler.publishedOnChainQuoteValidUntil(proposedOnChainQuoteHash)).to.equal(timestamp + 60) + await expect( quoteHandler.connect(lender).copyPublishedOnChainQuote(lenderVault.address, proposedOnChainQuoteHash) ).to.be.revertedWithCustomError(quoteHandler, 'InvalidQuote') @@ -3471,6 +3511,14 @@ describe('Peer-to-Peer: Local Tests', function () { await expect( quoteHandler.connect(lender).addOnChainQuote(lenderVault.address, onChainQuote) ).to.be.revertedWithCustomError(quoteHandler, 'OnChainQuoteAlreadyAdded') + + // move forward in time to make quote invalid + await ethers.provider.send('evm_increaseTime', [61]) + + // second proposed quote copy should revert since valid until has passed + await expect( + quoteHandler.connect(lender).copyPublishedOnChainQuote(lenderVault.address, proposedOnChainQuoteHash2) + ).to.be.revertedWithCustomError(quoteHandler, 'InvalidQuote') }) }) @@ -6009,8 +6057,8 @@ describe('Peer-to-Peer: Local Tests', function () { minEarliestRepayTenor: ethers.BigNumber.from(0), minLtv: BASE.div(10), maxLtv: BASE.div(2), - minLoanPerCollUnit: 0, - maxLoanPerCollUnit: 0 + minLoanPerCollUnit: ONE_USDC.mul(500), + maxLoanPerCollUnit: ONE_USDC.mul(2000) } const globalPolicyData = encodeGlobalPolicy(false, quoteBounds) @@ -6171,10 +6219,10 @@ describe('Peer-to-Peer: Local Tests', function () { lenderVault.address, encodeGlobalPolicy(false, { ...quoteBounds, - minLtv: ethers.BigNumber.from(0), - maxLtv: ethers.BigNumber.from(0), - minLoanPerCollUnitOrLtv: 0, - maxLoanPerCollUnitOrLtv: MAX_UINT256 + minLtv: ethers.BigNumber.from(1), + maxLtv: ethers.BigNumber.from(1), + minLoanPerCollUnit: ethers.BigNumber.from(1), + maxLoanPerCollUnit: MAX_UINT128 }) ) diff --git a/test/peer-to-peer/mainnet-forked-tests.ts b/test/peer-to-peer/mainnet-forked-tests.ts index 8be18ad6..0674f54b 100644 --- a/test/peer-to-peer/mainnet-forked-tests.ts +++ b/test/peer-to-peer/mainnet-forked-tests.ts @@ -963,7 +963,7 @@ describe('Peer-to-Peer: Forked Mainnet Tests', function () { 'OnChainQuoteAdded' ) - const quoteHashAndValidUntilArr = await quoteHandler.getFullOnChainQuoteHistory(lenderVault.address) + const quoteHashAndValidUntilArr = await quoteHandler.getFullOnChainQuoteHistory(lenderVault.address, 0, 2) expect(quoteHashAndValidUntilArr.length).to.equal(2) @@ -1052,7 +1052,7 @@ describe('Peer-to-Peer: Forked Mainnet Tests', function () { expect(borrowQuoteAddedEvent).to.be.not.undefined - const quoteHashAndValidUntilArrAfterUpdate = await quoteHandler.getFullOnChainQuoteHistory(lenderVault.address) + const quoteHashAndValidUntilArrAfterUpdate = await quoteHandler.getFullOnChainQuoteHistory(lenderVault.address, 0, 3) expect(quoteHashAndValidUntilArrAfterUpdate.length).to.equal(3) @@ -1060,7 +1060,7 @@ describe('Peer-to-Peer: Forked Mainnet Tests', function () { .connect(lender) .updateOnChainQuote(lenderVault.address, quoteHashAndValidUntilArrAfterUpdate[2].quoteHash, onChainQuote) - expect(await quoteHandler.getFullOnChainQuoteHistory(lenderVault.address)).to.have.lengthOf(4) + expect(await quoteHandler.getFullOnChainQuoteHistory(lenderVault.address, 0, 4)).to.have.lengthOf(4) // borrower approves borrower gateway await weth.connect(borrower).approve(borrowerGateway.address, MAX_UINT256) @@ -1980,7 +1980,7 @@ describe('Peer-to-Peer: Forked Mainnet Tests', function () { 'OnChainQuoteAdded' ) - const quoteHashAndValidUntilArr = await quoteHandler.getFullOnChainQuoteHistory(lenderVault.address) + const quoteHashAndValidUntilArr = await quoteHandler.getFullOnChainQuoteHistory(lenderVault.address, 0, 1) // revert if index out of bounds await expect(quoteHandler.getOnChainQuoteHistory(lenderVault.address, 5)).to.be.revertedWithCustomError( quoteHandler, @@ -5328,11 +5328,11 @@ describe('Peer-to-Peer: Forked Mainnet Tests', function () { maxTenor: ONE_DAY.mul(11), minFee: BASE.mul(11).div(100), minApr: BASE.mul(11).div(100), - minEarliestRepayTenor: 0, + minEarliestRepayTenor: ethers.BigNumber.from(0), minLtv: BASE.mul(11).div(100), maxLtv: BASE.mul(11).div(100), - minLoanPerCollUnit: 0, - maxLoanPerCollUnit: 0 + minLoanPerCollUnit: ONE_USDC.mul(100), + maxLoanPerCollUnit: ONE_USDC.mul(10000) } let globalRequiresOracle = false let globalPolicyData = encodeGlobalPolicy(globalRequiresOracle, globalQuoteBounds) @@ -5440,7 +5440,7 @@ describe('Peer-to-Peer: Forked Mainnet Tests', function () { expect(actGlobalPolicyData.quoteBounds.minFee).to.be.equal(globalQuoteBounds.minFee) expect(actGlobalPolicyData.quoteBounds.minApr).to.be.equal(globalQuoteBounds.minApr) expect(actGlobalPolicyData.quoteBounds.minEarliestRepayTenor).to.be.equal(globalQuoteBounds.minEarliestRepayTenor) - expect(actGlobalPolicyData.quoteBounds.minLoanPerCollUnitOrLtv).to.be.equal(globalQuoteBounds.minLoanPerCollUnit) + expect(actGlobalPolicyData.quoteBounds.minLoanPerCollUnit).to.be.equal(globalQuoteBounds.minLoanPerCollUnit) expect(actGlobalPolicyData.quoteBounds.maxLoanPerCollUnit).to.be.equal(globalQuoteBounds.maxLoanPerCollUnit) expect(actGlobalPolicyData.quoteBounds.minLtv).to.be.equal(globalQuoteBounds.minLtv) expect(actGlobalPolicyData.quoteBounds.maxLtv).to.be.equal(globalQuoteBounds.maxLtv) @@ -5521,13 +5521,13 @@ describe('Peer-to-Peer: Forked Mainnet Tests', function () { const pairQuoteBounds = { minTenor: ONE_DAY.mul(10), maxTenor: ONE_DAY.mul(365), - minFee: 0, - minApr: 0, - minEarliestRepayTenor: 0, + minFee: ethers.BigNumber.from(0), + minApr: ethers.BigNumber.from(0), + minEarliestRepayTenor: ethers.BigNumber.from(0), minLtv: BASE.mul(1).div(100), maxLtv: BASE.sub(1), - minLoanPerCollUnit: 0, - maxLoanPerCollUnit: 0 + minLoanPerCollUnit: ethers.BigNumber.from(0), + maxLoanPerCollUnit: ethers.BigNumber.from(0) } const pairRequiresOracle = true const pairMinNumOfSignersOverwrite = 1 @@ -5612,25 +5612,25 @@ describe('Peer-to-Peer: Forked Mainnet Tests', function () { quoteTupleIdx = 3 selectedQuoteTuple = quoteTuples[quoteTupleIdx] proof = quoteTuplesTree.getProof(quoteTupleIdx) - await borrowerGateway - .connect(borrower) - .borrowWithOffChainQuote(lenderVault.address, borrowInstructions, offChainQuote, selectedQuoteTuple, proof) + // await borrowerGateway + // .connect(borrower) + // .borrowWithOffChainQuote(lenderVault.address, borrowInstructions, offChainQuote, selectedQuoteTuple, proof) // borrow should go through quoteTupleIdx = 4 selectedQuoteTuple = quoteTuples[quoteTupleIdx] proof = quoteTuplesTree.getProof(quoteTupleIdx) - await borrowerGateway - .connect(borrower) - .borrowWithOffChainQuote(lenderVault.address, borrowInstructions, offChainQuote, selectedQuoteTuple, proof) + // await borrowerGateway + // .connect(borrower) + // .borrowWithOffChainQuote(lenderVault.address, borrowInstructions, offChainQuote, selectedQuoteTuple, proof) // borrow should go through quoteTupleIdx = 5 selectedQuoteTuple = quoteTuples[quoteTupleIdx] proof = quoteTuplesTree.getProof(quoteTupleIdx) - await borrowerGateway - .connect(borrower) - .borrowWithOffChainQuote(lenderVault.address, borrowInstructions, offChainQuote, selectedQuoteTuple, proof) + // await borrowerGateway + // .connect(borrower) + // .borrowWithOffChainQuote(lenderVault.address, borrowInstructions, offChainQuote, selectedQuoteTuple, proof) // delete global policy await basicQuotePolicyManager.connect(lender).setGlobalPolicy(lenderVault.address, '0x') @@ -5640,9 +5640,9 @@ describe('Peer-to-Peer: Forked Mainnet Tests', function () { quoteTupleIdx = 5 selectedQuoteTuple = quoteTuples[quoteTupleIdx] proof = quoteTuplesTree.getProof(quoteTupleIdx) - await borrowerGateway - .connect(borrower) - .borrowWithOffChainQuote(lenderVault.address, borrowInstructions, offChainQuote, selectedQuoteTuple, proof) + // await borrowerGateway + // .connect(borrower) + // .borrowWithOffChainQuote(lenderVault.address, borrowInstructions, offChainQuote, selectedQuoteTuple, proof) // swap should initially revert quoteTupleIdx = 6 @@ -5656,7 +5656,7 @@ describe('Peer-to-Peer: Forked Mainnet Tests', function () { // update pair policy pairQuoteBounds.minTenor = ethers.BigNumber.from(0) - pairQuoteBounds.maxLoanPerCollUnitOrLtv = BASE + pairQuoteBounds.maxLoanPerCollUnit = BASE pairPolicyData = encodePairPolicy(pairRequiresOracle, pairMinNumOfSignersOverwrite, pairQuoteBounds) await basicQuotePolicyManager .connect(lender) @@ -5666,9 +5666,9 @@ describe('Peer-to-Peer: Forked Mainnet Tests', function () { quoteTupleIdx = 6 selectedQuoteTuple = quoteTuples[quoteTupleIdx] proof = quoteTuplesTree.getProof(quoteTupleIdx) - await borrowerGateway - .connect(borrower) - .borrowWithOffChainQuote(lenderVault.address, borrowInstructions, offChainQuote, selectedQuoteTuple, proof) + // await borrowerGateway + // .connect(borrower) + // .borrowWithOffChainQuote(lenderVault.address, borrowInstructions, offChainQuote, selectedQuoteTuple, proof) // delete pair policy await basicQuotePolicyManager.connect(lender).setPairPolicy(lenderVault.address, weth.address, usdc.address, '0x') From 01a2e95ceaf0963813b3aa3466309a9e0ab93ff9 Mon Sep 17 00:00:00 2001 From: Jamie Pickett Date: Thu, 10 Aug 2023 17:25:22 -0400 Subject: [PATCH 2/5] added statemind-176 changes --- contracts/Errors.sol | 1 + .../BasicQuotePolicyManager.sol | 6 +++++ test/peer-to-peer/local-tests.ts | 24 +++++++++++++++++++ test/peer-to-peer/mainnet-forked-tests.ts | 4 ++-- 4 files changed, 33 insertions(+), 2 deletions(-) diff --git a/contracts/Errors.sol b/contracts/Errors.sol index 751b8efe..9b1722b1 100644 --- a/contracts/Errors.sol +++ b/contracts/Errors.sol @@ -131,4 +131,5 @@ library Errors { error InvalidLoanPerCollOrLtv(); error InvalidMinApr(); error NoPolicy(); + error InvalidMinFee(); } diff --git a/contracts/peer-to-peer/policyManagers/BasicQuotePolicyManager.sol b/contracts/peer-to-peer/policyManagers/BasicQuotePolicyManager.sol index d831334e..0f7f9c10 100644 --- a/contracts/peer-to-peer/policyManagers/BasicQuotePolicyManager.sol +++ b/contracts/peer-to-peer/policyManagers/BasicQuotePolicyManager.sol @@ -247,6 +247,8 @@ contract BasicQuotePolicyManager is IQuotePolicyManager { revert Errors.InvalidTenors(); } if ( + quoteBounds.minLtv == 0 || + quoteBounds.minLoanPerCollUnit == 0 || quoteBounds.minLtv > quoteBounds.maxLtv || quoteBounds.minLoanPerCollUnit > quoteBounds.maxLoanPerCollUnit ) { @@ -255,6 +257,10 @@ contract BasicQuotePolicyManager is IQuotePolicyManager { if (quoteBounds.minApr + int(Constants.BASE) <= 0) { revert Errors.InvalidMinApr(); } + // @dev: if minFee = BASE, then only swaps will be allowed + if (quoteBounds.minFee > Constants.BASE) { + revert Errors.InvalidMinFee(); + } } function _isAllowedWithBounds( diff --git a/test/peer-to-peer/local-tests.ts b/test/peer-to-peer/local-tests.ts index 29e4ea84..ff673bbd 100644 --- a/test/peer-to-peer/local-tests.ts +++ b/test/peer-to-peer/local-tests.ts @@ -6142,12 +6142,36 @@ describe('Peer-to-Peer: Local Tests', function () { .setGlobalPolicy(lenderVault.address, encodeGlobalPolicy(false, { ...quoteBounds, minLtv: BASE })) ).to.be.revertedWithCustomError(basicPolicyManager, 'InvalidLoanPerCollOrLtv') + await expect( + basicPolicyManager + .connect(lender) + .setGlobalPolicy( + lenderVault.address, + encodeGlobalPolicy(false, { ...quoteBounds, minLtv: ethers.BigNumber.from(0) }) + ) + ).to.be.revertedWithCustomError(basicPolicyManager, 'InvalidLoanPerCollOrLtv') + + await expect( + basicPolicyManager + .connect(lender) + .setGlobalPolicy( + lenderVault.address, + encodeGlobalPolicy(false, { ...quoteBounds, minLoanPerCollUnit: ethers.BigNumber.from(0) }) + ) + ).to.be.revertedWithCustomError(basicPolicyManager, 'InvalidLoanPerCollOrLtv') + await expect( basicPolicyManager .connect(lender) .setGlobalPolicy(lenderVault.address, encodeGlobalPolicy(false, { ...quoteBounds, minApr: BASE.mul(-2) })) ).to.be.revertedWithCustomError(basicPolicyManager, 'InvalidMinApr') + await expect( + basicPolicyManager + .connect(lender) + .setGlobalPolicy(lenderVault.address, encodeGlobalPolicy(false, { ...quoteBounds, minFee: BASE.add(1) })) + ).to.be.revertedWithCustomError(basicPolicyManager, 'InvalidMinFee') + await basicPolicyManager.connect(lender).setGlobalPolicy(lenderVault.address, encodeGlobalPolicy(true, quoteBounds)) // borrow should fail without oracle address diff --git a/test/peer-to-peer/mainnet-forked-tests.ts b/test/peer-to-peer/mainnet-forked-tests.ts index 0674f54b..9946b17b 100644 --- a/test/peer-to-peer/mainnet-forked-tests.ts +++ b/test/peer-to-peer/mainnet-forked-tests.ts @@ -5526,8 +5526,8 @@ describe('Peer-to-Peer: Forked Mainnet Tests', function () { minEarliestRepayTenor: ethers.BigNumber.from(0), minLtv: BASE.mul(1).div(100), maxLtv: BASE.sub(1), - minLoanPerCollUnit: ethers.BigNumber.from(0), - maxLoanPerCollUnit: ethers.BigNumber.from(0) + minLoanPerCollUnit: ethers.BigNumber.from(1), + maxLoanPerCollUnit: ethers.BigNumber.from(1) } const pairRequiresOracle = true const pairMinNumOfSignersOverwrite = 1 From 49002ab82a2f9bcd93079160a2be802a244de35b Mon Sep 17 00:00:00 2001 From: asardon Date: Fri, 11 Aug 2023 02:50:49 +0200 Subject: [PATCH 3/5] added suggestions --- contracts/Errors.sol | 5 +- contracts/peer-to-peer/QuoteHandler.sol | 31 ++--- .../peer-to-peer/interfaces/IQuoteHandler.sol | 2 +- .../BasicQuotePolicyManager.sol | 61 ++++++---- .../policyManagers/DataTypesBasicPolicies.sol | 16 +-- test/peer-to-peer/helpers/misc.ts | 24 ++-- test/peer-to-peer/local-tests.ts | 110 ++++++++++-------- test/peer-to-peer/mainnet-forked-tests.ts | 73 +++++++----- 8 files changed, 185 insertions(+), 137 deletions(-) diff --git a/contracts/Errors.sol b/contracts/Errors.sol index 9b1722b1..f2dbfe51 100644 --- a/contracts/Errors.sol +++ b/contracts/Errors.sol @@ -127,8 +127,9 @@ library Errors { error AlreadyPublished(); error PolicyAlreadySet(); error NoPolicyToDelete(); - error InvalidTenors(); - error InvalidLoanPerCollOrLtv(); + error InvalidTenorBounds(); + error InvalidLtvBounds(); + error InvalidLoanPerCollBounds(); error InvalidMinApr(); error NoPolicy(); error InvalidMinFee(); diff --git a/contracts/peer-to-peer/QuoteHandler.sol b/contracts/peer-to-peer/QuoteHandler.sol index 7ec6e60e..94b3289b 100644 --- a/contracts/peer-to-peer/QuoteHandler.sol +++ b/contracts/peer-to-peer/QuoteHandler.sol @@ -24,7 +24,7 @@ contract QuoteHandler is IQuoteHandler { mapping(bytes32 => uint256) public publishedOnChainQuoteValidUntil; mapping(address => address) public quotePolicyManagerForVault; mapping(address => DataTypesPeerToPeer.OnChainQuoteInfo[]) - internal onChainQuoteHistory; + internal _onChainQuoteHistory; constructor(address _addressRegistry) { if (_addressRegistry == address(0)) { @@ -48,7 +48,7 @@ contract QuoteHandler is IQuoteHandler { revert Errors.OnChainQuoteAlreadyAdded(); } // @dev: on-chain quote history is append only - onChainQuoteHistory[lenderVault].push( + _onChainQuoteHistory[lenderVault].push( DataTypesPeerToPeer.OnChainQuoteInfo({ quoteHash: onChainQuoteHash, validUntil: onChainQuote.generalQuoteInfo.validUntil @@ -78,7 +78,7 @@ contract QuoteHandler is IQuoteHandler { revert Errors.UnknownOnChainQuote(); } // @dev: on-chain quote history is append only - onChainQuoteHistory[lenderVault].push( + _onChainQuoteHistory[lenderVault].push( DataTypesPeerToPeer.OnChainQuoteInfo({ quoteHash: newOnChainQuoteHash, validUntil: newOnChainQuote.generalQuoteInfo.validUntil @@ -125,7 +125,7 @@ contract QuoteHandler is IQuoteHandler { revert Errors.InvalidQuote(); } // @dev: on-chain quote history is append only - onChainQuoteHistory[lenderVault].push( + _onChainQuoteHistory[lenderVault].push( DataTypesPeerToPeer.OnChainQuoteInfo({ quoteHash: onChainQuoteHash, validUntil: validUntil @@ -298,30 +298,35 @@ contract QuoteHandler is IQuoteHandler { address lenderVault, uint256 idx ) external view returns (DataTypesPeerToPeer.OnChainQuoteInfo memory) { - if (idx < onChainQuoteHistory[lenderVault].length) { - return onChainQuoteHistory[lenderVault][idx]; + if (idx < _onChainQuoteHistory[lenderVault].length) { + return _onChainQuoteHistory[lenderVault][idx]; } else { revert Errors.InvalidArrayIndex(); } } - function getFullOnChainQuoteHistory( + function getOnChainQuoteHistorySlice( address lenderVault, uint256 startIdx, uint256 endIdx ) external view returns (DataTypesPeerToPeer.OnChainQuoteInfo[] memory) { - if ( - startIdx >= endIdx || - endIdx > onChainQuoteHistory[lenderVault].length - ) { + uint256 onChainQuoteHistoryLen = _onChainQuoteHistory[lenderVault] + .length; + if (startIdx > endIdx || startIdx >= onChainQuoteHistoryLen) { revert Errors.InvalidArrayIndex(); } + endIdx = endIdx < onChainQuoteHistoryLen + ? endIdx + : onChainQuoteHistoryLen; + if (startIdx == 0 && endIdx == onChainQuoteHistoryLen) { + return _onChainQuoteHistory[lenderVault]; + } DataTypesPeerToPeer.OnChainQuoteInfo[] memory onChainQuoteHistoryRequested = new DataTypesPeerToPeer.OnChainQuoteInfo[]( endIdx - startIdx ); for (uint256 i = startIdx; i < endIdx; ) { - onChainQuoteHistoryRequested[i - startIdx] = onChainQuoteHistory[ + onChainQuoteHistoryRequested[i - startIdx] = _onChainQuoteHistory[ lenderVault ][i]; unchecked { @@ -334,7 +339,7 @@ contract QuoteHandler is IQuoteHandler { function getOnChainQuoteHistoryLength( address lenderVault ) external view returns (uint256) { - return onChainQuoteHistory[lenderVault].length; + return _onChainQuoteHistory[lenderVault].length; } /** diff --git a/contracts/peer-to-peer/interfaces/IQuoteHandler.sol b/contracts/peer-to-peer/interfaces/IQuoteHandler.sol index 600bac66..a654234b 100644 --- a/contracts/peer-to-peer/interfaces/IQuoteHandler.sol +++ b/contracts/peer-to-peer/interfaces/IQuoteHandler.sol @@ -254,7 +254,7 @@ interface IQuoteHandler { * @param endIdx ending index of on chain quote history array (non-inclusive) * @return array of quote hash and validUntil data for on-chain quote history of a vault */ - function getFullOnChainQuoteHistory( + function getOnChainQuoteHistorySlice( address lenderVault, uint256 startIdx, uint256 endIdx diff --git a/contracts/peer-to-peer/policyManagers/BasicQuotePolicyManager.sol b/contracts/peer-to-peer/policyManagers/BasicQuotePolicyManager.sol index 0f7f9c10..fa482e56 100644 --- a/contracts/peer-to-peer/policyManagers/BasicQuotePolicyManager.sol +++ b/contracts/peer-to-peer/policyManagers/BasicQuotePolicyManager.sol @@ -101,6 +101,10 @@ contract BasicQuotePolicyManager is IQuotePolicyManager { currSinglePolicy.requiresOracle && singlePolicy.minNumOfSignersOverwrite == currSinglePolicy.minNumOfSignersOverwrite && + singlePolicy.minLoanPerCollUnit == + currSinglePolicy.minLoanPerCollUnit && + singlePolicy.maxLoanPerCollUnit == + currSinglePolicy.maxLoanPerCollUnit && _equalQuoteBounds( singlePolicy.quoteBounds, currSinglePolicy.quoteBounds @@ -109,6 +113,13 @@ contract BasicQuotePolicyManager is IQuotePolicyManager { revert Errors.PolicyAlreadySet(); } _checkNewQuoteBounds(singlePolicy.quoteBounds); + if ( + singlePolicy.minLoanPerCollUnit == 0 || + singlePolicy.minLoanPerCollUnit > + singlePolicy.maxLoanPerCollUnit + ) { + revert Errors.InvalidLoanPerCollBounds(); + } if (!_hasSingleQuotingPolicy[loanToken]) { _hasSingleQuotingPolicy[loanToken] = true; } @@ -146,19 +157,24 @@ contract BasicQuotePolicyManager is IQuotePolicyManager { // @dev: pair policy (if defined) takes precedence over global policy bool hasOracle = generalQuoteInfo.oracleAddr != address(0); + bool checkLoanPerColl; bool requiresOracle; + uint256[2] memory minMaxLoanPerCollUnit; DataTypesBasicPolicies.QuoteBounds memory quoteBounds; if (hasPairPolicy) { DataTypesBasicPolicies.PairPolicy memory singlePolicy = _pairQuotingPolicies[lenderVault][ generalQuoteInfo.collToken ][generalQuoteInfo.loanToken]; - requiresOracle = singlePolicy.requiresOracle; quoteBounds = singlePolicy.quoteBounds; + minMaxLoanPerCollUnit[0] = singlePolicy.minLoanPerCollUnit; + minMaxLoanPerCollUnit[1] = singlePolicy.maxLoanPerCollUnit; + requiresOracle = singlePolicy.requiresOracle; minNumOfSignersOverwrite = singlePolicy.minNumOfSignersOverwrite; + checkLoanPerColl = !hasOracle; } else { - requiresOracle = globalPolicy.requiresOracle; quoteBounds = globalPolicy.quoteBounds; + requiresOracle = globalPolicy.requiresOracle; } if (requiresOracle && !hasOracle) { @@ -168,10 +184,11 @@ contract BasicQuotePolicyManager is IQuotePolicyManager { return ( _isAllowedWithBounds( quoteBounds, + minMaxLoanPerCollUnit, quoteTuple, generalQuoteInfo.earliestRepayTenor, hasOracle, - !hasOracle && !hasPairPolicy + checkLoanPerColl ), minNumOfSignersOverwrite ); @@ -230,10 +247,7 @@ contract BasicQuotePolicyManager is IQuotePolicyManager { quoteBounds1.minFee == quoteBounds2.minFee && quoteBounds1.minApr == quoteBounds2.minApr && quoteBounds1.minLtv == quoteBounds2.minLtv && - quoteBounds1.maxLtv == quoteBounds2.maxLtv && - quoteBounds1.minLoanPerCollUnit == - quoteBounds2.minLoanPerCollUnit && - quoteBounds1.maxLoanPerCollUnit == quoteBounds2.maxLoanPerCollUnit + quoteBounds1.maxLtv == quoteBounds2.maxLtv ) { isEqual = true; } @@ -244,15 +258,12 @@ contract BasicQuotePolicyManager is IQuotePolicyManager { ) internal pure { // @dev: allow minTenor == 0 to enable swaps if (quoteBounds.minTenor > quoteBounds.maxTenor) { - revert Errors.InvalidTenors(); + revert Errors.InvalidTenorBounds(); } if ( - quoteBounds.minLtv == 0 || - quoteBounds.minLoanPerCollUnit == 0 || - quoteBounds.minLtv > quoteBounds.maxLtv || - quoteBounds.minLoanPerCollUnit > quoteBounds.maxLoanPerCollUnit + quoteBounds.minLtv == 0 || quoteBounds.minLtv > quoteBounds.maxLtv ) { - revert Errors.InvalidLoanPerCollOrLtv(); + revert Errors.InvalidLtvBounds(); } if (quoteBounds.minApr + int(Constants.BASE) <= 0) { revert Errors.InvalidMinApr(); @@ -265,10 +276,11 @@ contract BasicQuotePolicyManager is IQuotePolicyManager { function _isAllowedWithBounds( DataTypesBasicPolicies.QuoteBounds memory quoteBounds, + uint256[2] memory minMaxLoanPerCollUnit, DataTypesPeerToPeer.QuoteTuple calldata quoteTuple, uint256 earliestRepayTenor, bool checkLtv, - bool skipLoanPerCollCheck + bool checkLoanPerColl ) internal pure returns (bool) { if ( quoteTuple.tenor < quoteBounds.minTenor || @@ -277,14 +289,19 @@ contract BasicQuotePolicyManager is IQuotePolicyManager { return false; } - // @dev: if requires oracle check against LTV bounds, else against loan-per-coll bounds - (uint256 lowerBnd, uint256 upperBnd) = checkLtv - ? (quoteBounds.minLtv, quoteBounds.maxLtv) - : (quoteBounds.minLoanPerCollUnit, quoteBounds.maxLoanPerCollUnit); - if ( - !skipLoanPerCollCheck && - (quoteTuple.loanPerCollUnitOrLtv < lowerBnd || - quoteTuple.loanPerCollUnitOrLtv > upperBnd) + if (checkLtv) { + // @dev: check either against LTV bounds + if ( + quoteTuple.loanPerCollUnitOrLtv < quoteBounds.minLtv || + quoteTuple.loanPerCollUnitOrLtv > quoteBounds.maxLtv + ) { + return false; + } + } else if ( + // @dev: only check against absolute loan-per-coll bounds on pair policy and if no oracle + checkLoanPerColl && + (quoteTuple.loanPerCollUnitOrLtv < minMaxLoanPerCollUnit[0] || + quoteTuple.loanPerCollUnitOrLtv > minMaxLoanPerCollUnit[1]) ) { return false; } diff --git a/contracts/peer-to-peer/policyManagers/DataTypesBasicPolicies.sol b/contracts/peer-to-peer/policyManagers/DataTypesBasicPolicies.sol index 5b741a6c..4c8778f1 100644 --- a/contracts/peer-to-peer/policyManagers/DataTypesBasicPolicies.sol +++ b/contracts/peer-to-peer/policyManagers/DataTypesBasicPolicies.sol @@ -17,26 +17,26 @@ library DataTypesBasicPolicies { uint128 minLtv; // Allowed maximum LTV for the quote uint128 maxLtv; - // Allowed minimum loan per collateral unit or LTV for the quote - uint128 minLoanPerCollUnit; - // Allowed maximum loan per collateral unit or LTV for the quote - uint128 maxLoanPerCollUnit; } struct GlobalPolicy { + // Applicable general bounds + QuoteBounds quoteBounds; // Flag indicating if an oracle is required for the pair bool requiresOracle; - // Applicable global bounds - QuoteBounds quoteBounds; } struct PairPolicy { + // Applicable general bounds + QuoteBounds quoteBounds; + // Allowed minimum loan per collateral unit or LTV for the quote + uint128 minLoanPerCollUnit; + // Allowed maximum loan per collateral unit or LTV for the quote + uint128 maxLoanPerCollUnit; // Flag indicating if an oracle is required for the pair bool requiresOracle; // Minimum number of signers required for the pair (if zero ignored, otherwise overwrites vault min signers) // @dev: can overwrite signer threshold to be lower or higher than vault min signers uint8 minNumOfSignersOverwrite; - // Applicable global bounds - QuoteBounds quoteBounds; } } diff --git a/test/peer-to-peer/helpers/misc.ts b/test/peer-to-peer/helpers/misc.ts index 6be7d8bf..ac91552f 100644 --- a/test/peer-to-peer/helpers/misc.ts +++ b/test/peer-to-peer/helpers/misc.ts @@ -333,31 +333,33 @@ export type QuoteBounds = { minEarliestRepayTenor: BigNumber minLtv: BigNumber maxLtv: BigNumber - minLoanPerCollUnit: BigNumber - maxLoanPerCollUnit: BigNumber } -export const encodeGlobalPolicy = (requiresOracle: boolean, quoteBounds: QuoteBounds): string => { +export const encodeGlobalPolicy = (quoteBounds: QuoteBounds, requiresOracle: boolean): string => { return ethers.utils.defaultAbiCoder.encode( [ - 'bool requiresOracle', - 'tuple(uint32 minTenor, uint32 maxTenor, uint80 minFee, int80 minApr, uint32 minEarliestRepayTenor, uint128 minLtv, uint128 maxLtv, uint128 minLoanPerCollUnit, uint128 maxLoanPerCollUnit) quoteBounds' + 'tuple(uint32 minTenor, uint32 maxTenor, uint80 minFee, int80 minApr, uint32 minEarliestRepayTenor, uint128 minLtv, uint128 maxLtv) quoteBounds', + 'bool requiresOracle' ], - [requiresOracle, quoteBounds] + [quoteBounds, requiresOracle] ) } export const encodePairPolicy = ( + quoteBounds: QuoteBounds, + minLoanPerCollUnit: BigNumber, + maxLoanPerCollUnit: BigNumber, requiresOracle: boolean, - minNumOfSignersOverwrite: number, - quoteBounds: QuoteBounds + minNumOfSignersOverwrite: number ): string => { return ethers.utils.defaultAbiCoder.encode( [ + 'tuple(uint32 minTenor, uint32 maxTenor, uint80 minFee, int80 minApr, uint32 minEarliestRepayTenor, uint128 minLtv, uint128 maxLtv) quoteBounds', + 'uint128 minLoanPerCollUnit', + 'uint128 maxLoanPerCollUnit', 'bool requiresOracle', - 'uint8 minNumOfSignersOverwrite', - 'tuple(uint32 minTenor, uint32 maxTenor, uint80 minFee, int80 minApr, uint32 minEarliestRepayTenor, uint128 minLtv, uint128 maxLtv, uint128 minLoanPerCollUnit, uint128 maxLoanPerCollUnit) quoteBounds' + 'uint8 minNumOfSignersOverwrite' ], - [requiresOracle, minNumOfSignersOverwrite, quoteBounds] + [quoteBounds, minLoanPerCollUnit, maxLoanPerCollUnit, requiresOracle, minNumOfSignersOverwrite] ) } diff --git a/test/peer-to-peer/local-tests.ts b/test/peer-to-peer/local-tests.ts index ff673bbd..41846630 100644 --- a/test/peer-to-peer/local-tests.ts +++ b/test/peer-to-peer/local-tests.ts @@ -3368,6 +3368,12 @@ describe('Peer-to-Peer: Local Tests', function () { expect(await quoteHandler.getOnChainQuoteHistoryLength(lenderVault.address)).to.equal(0) + // check revert on empty history + await expect(quoteHandler.getOnChainQuoteHistorySlice(lenderVault.address, 0, 0)).to.be.revertedWithCustomError( + quoteHandler, + 'InvalidArrayIndex' + ) + await expect(quoteHandler.connect(lender).addOnChainQuote(lenderVault.address, onChainQuote)).to.emit( quoteHandler, 'OnChainQuoteAdded' @@ -3379,19 +3385,19 @@ describe('Peer-to-Peer: Local Tests', function () { 'OnChainQuoteAdded' ) - // start idx >= end idx should revert - await expect(quoteHandler.getFullOnChainQuoteHistory(lenderVault.address, 0, 0)).to.be.revertedWithCustomError( - quoteHandler, - 'InvalidArrayIndex' - ) + const emptyHistory1 = await quoteHandler.getOnChainQuoteHistorySlice(lenderVault.address, 0, 0) + expect(emptyHistory1.length).to.be.equal(0) - // end idx > history length should revert - await expect(quoteHandler.getFullOnChainQuoteHistory(lenderVault.address, 0, 3)).to.be.revertedWithCustomError( + const emptyHistory2 = await quoteHandler.getOnChainQuoteHistorySlice(lenderVault.address, 1, 1) + expect(emptyHistory2.length).to.be.equal(0) + + // start idx > history length should revert + await expect(quoteHandler.getOnChainQuoteHistorySlice(lenderVault.address, 2, 3)).to.be.revertedWithCustomError( quoteHandler, 'InvalidArrayIndex' ) - const quoteHashAndValidUntilArr = await quoteHandler.getFullOnChainQuoteHistory(lenderVault.address, 0, 2) + const quoteHashAndValidUntilArr = await quoteHandler.getOnChainQuoteHistorySlice(lenderVault.address, 0, 2) const historyLen = await quoteHandler.getOnChainQuoteHistoryLength(lenderVault.address) expect(quoteHashAndValidUntilArr.length).to.equal(2) @@ -6056,16 +6062,14 @@ describe('Peer-to-Peer: Local Tests', function () { minApr: ethers.BigNumber.from(0), minEarliestRepayTenor: ethers.BigNumber.from(0), minLtv: BASE.div(10), - maxLtv: BASE.div(2), - minLoanPerCollUnit: ONE_USDC.mul(500), - maxLoanPerCollUnit: ONE_USDC.mul(2000) + maxLtv: BASE.div(2) } - const globalPolicyData = encodeGlobalPolicy(false, quoteBounds) + const globalPolicyData = encodeGlobalPolicy(quoteBounds, false) - const pairPolicyData = encodePairPolicy(true, 1, quoteBounds) + const pairPolicyData = encodePairPolicy(quoteBounds, ONE_USDC.mul(500), ONE_USDC.mul(2000), false, 1) - //should revert if unregistered vault + // should revert if unregistered vault await expect( basicPolicyManager.connect(team).setPairPolicy(borrower.address, weth.address, usdc.address, pairPolicyData) ).to.be.revertedWithCustomError(basicPolicyManager, 'UnregisteredVault') @@ -6126,6 +6130,7 @@ describe('Peer-to-Peer: Local Tests', function () { basicPolicyManager.connect(lender).setGlobalPolicy(lenderVault.address, globalPolicyData) ).to.be.revertedWithCustomError(basicPolicyManager, 'PolicyAlreadySet') + console.log('ok0') await borrowerGateway .connect(borrower) .borrowWithOnChainQuote(lenderVault.address, borrowInstructions1, onChainQuote1, quoteTupleIdx1) @@ -6133,46 +6138,43 @@ describe('Peer-to-Peer: Local Tests', function () { await expect( basicPolicyManager .connect(lender) - .setGlobalPolicy(lenderVault.address, encodeGlobalPolicy(false, { ...quoteBounds, minTenor: ONE_DAY.mul(40) })) - ).to.be.revertedWithCustomError(basicPolicyManager, 'InvalidTenors') + .setGlobalPolicy(lenderVault.address, encodeGlobalPolicy({ ...quoteBounds, minTenor: ONE_DAY.mul(40) }, false)) + ).to.be.revertedWithCustomError(basicPolicyManager, 'InvalidTenorBounds') await expect( basicPolicyManager .connect(lender) - .setGlobalPolicy(lenderVault.address, encodeGlobalPolicy(false, { ...quoteBounds, minLtv: BASE })) - ).to.be.revertedWithCustomError(basicPolicyManager, 'InvalidLoanPerCollOrLtv') + .setGlobalPolicy(lenderVault.address, encodeGlobalPolicy({ ...quoteBounds, minLtv: BASE }, false)) + ).to.be.revertedWithCustomError(basicPolicyManager, 'InvalidLtvBounds') await expect( basicPolicyManager .connect(lender) .setGlobalPolicy( lenderVault.address, - encodeGlobalPolicy(false, { ...quoteBounds, minLtv: ethers.BigNumber.from(0) }) + encodeGlobalPolicy({ ...quoteBounds, minLtv: ethers.BigNumber.from(0) }, false) ) - ).to.be.revertedWithCustomError(basicPolicyManager, 'InvalidLoanPerCollOrLtv') + ).to.be.revertedWithCustomError(basicPolicyManager, 'InvalidLtvBounds') await expect( basicPolicyManager .connect(lender) - .setGlobalPolicy( - lenderVault.address, - encodeGlobalPolicy(false, { ...quoteBounds, minLoanPerCollUnit: ethers.BigNumber.from(0) }) - ) - ).to.be.revertedWithCustomError(basicPolicyManager, 'InvalidLoanPerCollOrLtv') + .setGlobalPolicy(lenderVault.address, encodeGlobalPolicy({ ...quoteBounds }, false)) + ).to.be.revertedWithCustomError(basicPolicyManager, 'PolicyAlreadySet') await expect( basicPolicyManager .connect(lender) - .setGlobalPolicy(lenderVault.address, encodeGlobalPolicy(false, { ...quoteBounds, minApr: BASE.mul(-2) })) + .setGlobalPolicy(lenderVault.address, encodeGlobalPolicy({ ...quoteBounds, minApr: BASE.mul(-2) }, false)) ).to.be.revertedWithCustomError(basicPolicyManager, 'InvalidMinApr') await expect( basicPolicyManager .connect(lender) - .setGlobalPolicy(lenderVault.address, encodeGlobalPolicy(false, { ...quoteBounds, minFee: BASE.add(1) })) + .setGlobalPolicy(lenderVault.address, encodeGlobalPolicy({ ...quoteBounds, minFee: BASE.add(1) }, false)) ).to.be.revertedWithCustomError(basicPolicyManager, 'InvalidMinFee') - await basicPolicyManager.connect(lender).setGlobalPolicy(lenderVault.address, encodeGlobalPolicy(true, quoteBounds)) + await basicPolicyManager.connect(lender).setGlobalPolicy(lenderVault.address, encodeGlobalPolicy(quoteBounds, true)) // borrow should fail without oracle address await expect( @@ -6183,7 +6185,7 @@ describe('Peer-to-Peer: Local Tests', function () { await basicPolicyManager .connect(lender) - .setGlobalPolicy(lenderVault.address, encodeGlobalPolicy(false, { ...quoteBounds, minTenor: ONE_DAY.mul(35) })) + .setGlobalPolicy(lenderVault.address, encodeGlobalPolicy({ ...quoteBounds, minTenor: ONE_DAY.mul(35) }, false)) // borrow fail if tenor less than min tenor await expect( @@ -6194,7 +6196,7 @@ describe('Peer-to-Peer: Local Tests', function () { await basicPolicyManager .connect(lender) - .setGlobalPolicy(lenderVault.address, encodeGlobalPolicy(false, { ...quoteBounds, maxTenor: ONE_DAY.mul(25) })) + .setGlobalPolicy(lenderVault.address, encodeGlobalPolicy({ ...quoteBounds, maxTenor: ONE_DAY.mul(25) }, false)) // borrow fail if tenor greater than max tenor await expect( @@ -6205,7 +6207,7 @@ describe('Peer-to-Peer: Local Tests', function () { await basicPolicyManager .connect(lender) - .setGlobalPolicy(lenderVault.address, encodeGlobalPolicy(false, { ...quoteBounds, minFee: BASE.mul(9).div(10) })) + .setGlobalPolicy(lenderVault.address, encodeGlobalPolicy({ ...quoteBounds, minFee: BASE.mul(9).div(10) }, false)) // borrow should fail if upfront fee is below min fee await expect( @@ -6216,7 +6218,7 @@ describe('Peer-to-Peer: Local Tests', function () { await basicPolicyManager .connect(lender) - .setGlobalPolicy(lenderVault.address, encodeGlobalPolicy(false, { ...quoteBounds, minApr: BASE.mul(10) })) + .setGlobalPolicy(lenderVault.address, encodeGlobalPolicy({ ...quoteBounds, minApr: BASE.mul(10) }, false)) // borrow should fail if apr is below min apr await expect( @@ -6227,13 +6229,17 @@ describe('Peer-to-Peer: Local Tests', function () { await basicPolicyManager.connect(lender).setGlobalPolicy( lenderVault.address, - encodeGlobalPolicy(false, { - ...quoteBounds, - minLtv: BASE.mul(4).div(5), - maxLtv: BASE.mul(9).div(10) - }) + encodeGlobalPolicy( + { + ...quoteBounds, + minLtv: BASE.mul(4).div(5), + maxLtv: BASE.mul(9).div(10) + }, + false + ) ) + console.log('ok1') // borrow should go through even if loanPerCollUnitOrLtv is below min LTV if no oracle required await borrowerGateway .connect(borrower) @@ -6241,15 +6247,17 @@ describe('Peer-to-Peer: Local Tests', function () { await basicPolicyManager.connect(lender).setGlobalPolicy( lenderVault.address, - encodeGlobalPolicy(false, { - ...quoteBounds, - minLtv: ethers.BigNumber.from(1), - maxLtv: ethers.BigNumber.from(1), - minLoanPerCollUnit: ethers.BigNumber.from(1), - maxLoanPerCollUnit: MAX_UINT128 - }) + encodeGlobalPolicy( + { + ...quoteBounds, + minLtv: ethers.BigNumber.from(1), + maxLtv: ethers.BigNumber.from(1) + }, + false + ) ) + console.log('ok2') // borrow should go through even if loanPerCollUnitOrLtv is above maxLtv if no oracle required await borrowerGateway .connect(borrower) @@ -6289,11 +6297,14 @@ describe('Peer-to-Peer: Local Tests', function () { await basicPolicyManager.connect(lender).setGlobalPolicy( lenderVault.address, - encodeGlobalPolicy(false, { - ...quoteBounds, - minApr: BASE.mul(-99).div(100), - minEarliestRepayTenor: ONE_DAY.mul(20) - }) + encodeGlobalPolicy( + { + ...quoteBounds, + minApr: BASE.mul(-99).div(100), + minEarliestRepayTenor: ONE_DAY.mul(20) + }, + false + ) ) // borrow with negative interest rate should fail if earliest min repay is too short @@ -6316,6 +6327,7 @@ describe('Peer-to-Peer: Local Tests', function () { 'OnChainQuoteAdded' ) + console.log('ok3') // borrow with negative interest rate should go through if earliest min repay is long enough await borrowerGateway .connect(borrower) diff --git a/test/peer-to-peer/mainnet-forked-tests.ts b/test/peer-to-peer/mainnet-forked-tests.ts index 9946b17b..939829d8 100644 --- a/test/peer-to-peer/mainnet-forked-tests.ts +++ b/test/peer-to-peer/mainnet-forked-tests.ts @@ -963,7 +963,7 @@ describe('Peer-to-Peer: Forked Mainnet Tests', function () { 'OnChainQuoteAdded' ) - const quoteHashAndValidUntilArr = await quoteHandler.getFullOnChainQuoteHistory(lenderVault.address, 0, 2) + const quoteHashAndValidUntilArr = await quoteHandler.getOnChainQuoteHistorySlice(lenderVault.address, 0, 2) expect(quoteHashAndValidUntilArr.length).to.equal(2) @@ -1052,7 +1052,7 @@ describe('Peer-to-Peer: Forked Mainnet Tests', function () { expect(borrowQuoteAddedEvent).to.be.not.undefined - const quoteHashAndValidUntilArrAfterUpdate = await quoteHandler.getFullOnChainQuoteHistory(lenderVault.address, 0, 3) + const quoteHashAndValidUntilArrAfterUpdate = await quoteHandler.getOnChainQuoteHistorySlice(lenderVault.address, 0, 3) expect(quoteHashAndValidUntilArrAfterUpdate.length).to.equal(3) @@ -1060,7 +1060,7 @@ describe('Peer-to-Peer: Forked Mainnet Tests', function () { .connect(lender) .updateOnChainQuote(lenderVault.address, quoteHashAndValidUntilArrAfterUpdate[2].quoteHash, onChainQuote) - expect(await quoteHandler.getFullOnChainQuoteHistory(lenderVault.address, 0, 4)).to.have.lengthOf(4) + expect(await quoteHandler.getOnChainQuoteHistorySlice(lenderVault.address, 0, 4)).to.have.lengthOf(4) // borrower approves borrower gateway await weth.connect(borrower).approve(borrowerGateway.address, MAX_UINT256) @@ -1980,7 +1980,7 @@ describe('Peer-to-Peer: Forked Mainnet Tests', function () { 'OnChainQuoteAdded' ) - const quoteHashAndValidUntilArr = await quoteHandler.getFullOnChainQuoteHistory(lenderVault.address, 0, 1) + const quoteHashAndValidUntilArr = await quoteHandler.getOnChainQuoteHistorySlice(lenderVault.address, 0, 1) // revert if index out of bounds await expect(quoteHandler.getOnChainQuoteHistory(lenderVault.address, 5)).to.be.revertedWithCustomError( quoteHandler, @@ -5335,7 +5335,7 @@ describe('Peer-to-Peer: Forked Mainnet Tests', function () { maxLoanPerCollUnit: ONE_USDC.mul(10000) } let globalRequiresOracle = false - let globalPolicyData = encodeGlobalPolicy(globalRequiresOracle, globalQuoteBounds) + let globalPolicyData = encodeGlobalPolicy(globalQuoteBounds, globalRequiresOracle) // check revert when trying to delete global policy although not yet set await expect( @@ -5428,7 +5428,7 @@ describe('Peer-to-Peer: Forked Mainnet Tests', function () { ).to.be.revertedWithCustomError(quoteHandler, 'QuoteViolatesPolicy') // add global policy to allow all pairs - globalPolicyData = encodeGlobalPolicy(globalRequiresOracle, globalQuoteBounds) + globalPolicyData = encodeGlobalPolicy(globalQuoteBounds, globalRequiresOracle) await basicQuotePolicyManager.connect(lender).setGlobalPolicy(lenderVault.address, globalPolicyData) // check global policy is set correctly @@ -5440,8 +5440,6 @@ describe('Peer-to-Peer: Forked Mainnet Tests', function () { expect(actGlobalPolicyData.quoteBounds.minFee).to.be.equal(globalQuoteBounds.minFee) expect(actGlobalPolicyData.quoteBounds.minApr).to.be.equal(globalQuoteBounds.minApr) expect(actGlobalPolicyData.quoteBounds.minEarliestRepayTenor).to.be.equal(globalQuoteBounds.minEarliestRepayTenor) - expect(actGlobalPolicyData.quoteBounds.minLoanPerCollUnit).to.be.equal(globalQuoteBounds.minLoanPerCollUnit) - expect(actGlobalPolicyData.quoteBounds.maxLoanPerCollUnit).to.be.equal(globalQuoteBounds.maxLoanPerCollUnit) expect(actGlobalPolicyData.quoteBounds.minLtv).to.be.equal(globalQuoteBounds.minLtv) expect(actGlobalPolicyData.quoteBounds.maxLtv).to.be.equal(globalQuoteBounds.maxLtv) @@ -5525,13 +5523,20 @@ describe('Peer-to-Peer: Forked Mainnet Tests', function () { minApr: ethers.BigNumber.from(0), minEarliestRepayTenor: ethers.BigNumber.from(0), minLtv: BASE.mul(1).div(100), - maxLtv: BASE.sub(1), - minLoanPerCollUnit: ethers.BigNumber.from(1), - maxLoanPerCollUnit: ethers.BigNumber.from(1) + maxLtv: BASE.sub(1) } + const minLoanPerColl = ethers.BigNumber.from(1) + const maxLoanPerColl = ethers.BigNumber.from(1) + const pairRequiresOracle = true const pairMinNumOfSignersOverwrite = 1 - let pairPolicyData = encodePairPolicy(pairRequiresOracle, pairMinNumOfSignersOverwrite, pairQuoteBounds) + let pairPolicyData = encodePairPolicy( + pairQuoteBounds, + minLoanPerColl, + maxLoanPerColl, + pairRequiresOracle, + pairMinNumOfSignersOverwrite + ) // check revert when trying to delete pair policy although not yet set await expect( @@ -5566,8 +5571,8 @@ describe('Peer-to-Peer: Forked Mainnet Tests', function () { expect(actPairPolicyData.quoteBounds.minFee).to.be.equal(pairQuoteBounds.minFee) expect(actPairPolicyData.quoteBounds.minApr).to.be.equal(pairQuoteBounds.minApr) expect(actPairPolicyData.quoteBounds.minEarliestRepayTenor).to.be.equal(pairQuoteBounds.minEarliestRepayTenor) - expect(actPairPolicyData.quoteBounds.minLoanPerCollUnit).to.be.equal(pairQuoteBounds.minLoanPerCollUnit) - expect(actPairPolicyData.quoteBounds.maxLoanPerCollUnit).to.be.equal(pairQuoteBounds.maxLoanPerCollUnit) + expect(actPairPolicyData.minLoanPerCollUnit).to.be.equal(minLoanPerColl) + expect(actPairPolicyData.maxLoanPerCollUnit).to.be.equal(maxLoanPerColl) expect(actPairPolicyData.quoteBounds.minLtv).to.be.equal(pairQuoteBounds.minLtv) expect(actPairPolicyData.quoteBounds.maxLtv).to.be.equal(pairQuoteBounds.maxLtv) @@ -5612,25 +5617,25 @@ describe('Peer-to-Peer: Forked Mainnet Tests', function () { quoteTupleIdx = 3 selectedQuoteTuple = quoteTuples[quoteTupleIdx] proof = quoteTuplesTree.getProof(quoteTupleIdx) - // await borrowerGateway - // .connect(borrower) - // .borrowWithOffChainQuote(lenderVault.address, borrowInstructions, offChainQuote, selectedQuoteTuple, proof) + await borrowerGateway + .connect(borrower) + .borrowWithOffChainQuote(lenderVault.address, borrowInstructions, offChainQuote, selectedQuoteTuple, proof) // borrow should go through quoteTupleIdx = 4 selectedQuoteTuple = quoteTuples[quoteTupleIdx] proof = quoteTuplesTree.getProof(quoteTupleIdx) - // await borrowerGateway - // .connect(borrower) - // .borrowWithOffChainQuote(lenderVault.address, borrowInstructions, offChainQuote, selectedQuoteTuple, proof) + await borrowerGateway + .connect(borrower) + .borrowWithOffChainQuote(lenderVault.address, borrowInstructions, offChainQuote, selectedQuoteTuple, proof) // borrow should go through quoteTupleIdx = 5 selectedQuoteTuple = quoteTuples[quoteTupleIdx] proof = quoteTuplesTree.getProof(quoteTupleIdx) - // await borrowerGateway - // .connect(borrower) - // .borrowWithOffChainQuote(lenderVault.address, borrowInstructions, offChainQuote, selectedQuoteTuple, proof) + await borrowerGateway + .connect(borrower) + .borrowWithOffChainQuote(lenderVault.address, borrowInstructions, offChainQuote, selectedQuoteTuple, proof) // delete global policy await basicQuotePolicyManager.connect(lender).setGlobalPolicy(lenderVault.address, '0x') @@ -5640,9 +5645,9 @@ describe('Peer-to-Peer: Forked Mainnet Tests', function () { quoteTupleIdx = 5 selectedQuoteTuple = quoteTuples[quoteTupleIdx] proof = quoteTuplesTree.getProof(quoteTupleIdx) - // await borrowerGateway - // .connect(borrower) - // .borrowWithOffChainQuote(lenderVault.address, borrowInstructions, offChainQuote, selectedQuoteTuple, proof) + await borrowerGateway + .connect(borrower) + .borrowWithOffChainQuote(lenderVault.address, borrowInstructions, offChainQuote, selectedQuoteTuple, proof) // swap should initially revert quoteTupleIdx = 6 @@ -5656,8 +5661,14 @@ describe('Peer-to-Peer: Forked Mainnet Tests', function () { // update pair policy pairQuoteBounds.minTenor = ethers.BigNumber.from(0) - pairQuoteBounds.maxLoanPerCollUnit = BASE - pairPolicyData = encodePairPolicy(pairRequiresOracle, pairMinNumOfSignersOverwrite, pairQuoteBounds) + pairQuoteBounds.maxLtv = BASE + pairPolicyData = encodePairPolicy( + pairQuoteBounds, + minLoanPerColl, + maxLoanPerColl, + pairRequiresOracle, + pairMinNumOfSignersOverwrite + ) await basicQuotePolicyManager .connect(lender) .setPairPolicy(lenderVault.address, weth.address, usdc.address, pairPolicyData) @@ -5666,9 +5677,9 @@ describe('Peer-to-Peer: Forked Mainnet Tests', function () { quoteTupleIdx = 6 selectedQuoteTuple = quoteTuples[quoteTupleIdx] proof = quoteTuplesTree.getProof(quoteTupleIdx) - // await borrowerGateway - // .connect(borrower) - // .borrowWithOffChainQuote(lenderVault.address, borrowInstructions, offChainQuote, selectedQuoteTuple, proof) + await borrowerGateway + .connect(borrower) + .borrowWithOffChainQuote(lenderVault.address, borrowInstructions, offChainQuote, selectedQuoteTuple, proof) // delete pair policy await basicQuotePolicyManager.connect(lender).setPairPolicy(lenderVault.address, weth.address, usdc.address, '0x') From a3d45aaf6435f98b5360143ef1f4632122a1bdeb Mon Sep 17 00:00:00 2001 From: asardon Date: Fri, 11 Aug 2023 03:30:13 +0200 Subject: [PATCH 4/5] updated test coverage --- README.md | 14 +++++----- test/peer-to-peer/local-tests.ts | 46 ++++++++++++++++++++++++++++---- 2 files changed, 48 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 89ec85ec..823250fc 100644 --- a/README.md +++ b/README.md @@ -170,13 +170,13 @@ File | % Stmts | % Branch | Helpers.sol | 100 | 50 | 100 | 100 | | contracts\interfaces\ | 100 | 100 | 100 | 100 | | IMysoTokenManager.sol | 100 | 100 | 100 | 100 | | - contracts\peer-to-peer\ | 99.74 | 94.61 | 98.84 | 98.66 | | - AddressRegistry.sol | 100 | 96.74 | 100 | 99.17 | 117 | + contracts\peer-to-peer\ | 99.75 | 94.5 | 98.82 | 98.2 | | + AddressRegistry.sol | 100 | 96.74 | 100 | 99.17 | 116 | BorrowerGateway.sol | 98.57 | 90.91 | 90.91 | 96.97 | 241,317,358 | DataTypesPeerToPeer.sol | 100 | 100 | 100 | 100 | | LenderVaultFactory.sol | 100 | 87.5 | 100 | 100 | | - LenderVaultImpl.sol | 100 | 93.1 | 100 | 98.92 | 64,207 | - QuoteHandler.sol | 100 | 96.83 | 100 | 98.91 | 266,478 | + LenderVaultImpl.sol | 100 | 93.1 | 100 | 98.91 | 64,207 | + QuoteHandler.sol | 100 | 96.32 | 100 | 97.46 |... 332,333,516 | contracts\peer-to-peer\callbacks\ | 100 | 75 | 88.89 | 96.88 | | BalancerV2Looping.sol | 100 | 100 | 100 | 100 | | UniV3Looping.sol | 100 | 100 | 100 | 100 | | @@ -238,8 +238,8 @@ File | % Stmts | % Branch | OracleLibrary.sol | 100 | 66.67 | 100 | 100 | | TickMath.sol | 100 | 80.43 | 100 | 84.09 |... 63,65,67,73 | TwapGetter.sol | 100 | 62.5 | 100 | 93.33 | 57 | - contracts\peer-to-peer\policyManagers\ | 100 | 100 | 100 | 100 | | - BasicQuotePolicyManager.sol | 100 | 100 | 100 | 100 | | + contracts\peer-to-peer\policyManagers\ | 100 | 94.44 | 100 | 98.95 | | + BasicQuotePolicyManager.sol | 100 | 94.44 | 100 | 98.95 | 121 | DataTypesBasicPolicies.sol | 100 | 100 | 100 | 100 | | contracts\peer-to-peer\wrappers\ERC20\ | 100 | 83.33 | 100 | 98.88 | | ERC20Wrapper.sol | 100 | 75 | 100 | 96.88 | 45 | @@ -257,6 +257,6 @@ File | % Stmts | % Branch | IFundingPoolImpl.sol | 100 | 100 | 100 | 100 | | ILoanProposalImpl.sol | 100 | 100 | 100 | 100 | | ---------------------------------------------------------|----------|----------|----------|----------|----------------| -All files | 99.07 | 89.56 | 98.74 | 96.97 | | +All files | 99.08 | 89.33 | 98.74 | 96.78 | | ---------------------------------------------------------|----------|----------|----------|----------|----------------| ``` diff --git a/test/peer-to-peer/local-tests.ts b/test/peer-to-peer/local-tests.ts index 41846630..946ac68f 100644 --- a/test/peer-to-peer/local-tests.ts +++ b/test/peer-to-peer/local-tests.ts @@ -6030,6 +6030,12 @@ describe('Peer-to-Peer: Local Tests', function () { interestRatePctInBase: BASE.div(10), upfrontFeePctInBase: BASE.div(10), tenor: ONE_DAY.mul(30) + }, + { + loanPerCollUnitOrLtv: ONE_USDC.mul(400), + interestRatePctInBase: BASE.div(10), + upfrontFeePctInBase: BASE.div(10), + tenor: ONE_DAY.mul(30) } ] let onChainQuote1 = { @@ -6067,7 +6073,8 @@ describe('Peer-to-Peer: Local Tests', function () { const globalPolicyData = encodeGlobalPolicy(quoteBounds, false) - const pairPolicyData = encodePairPolicy(quoteBounds, ONE_USDC.mul(500), ONE_USDC.mul(2000), false, 1) + const minLoanPerColl = quoteTuples1[1].loanPerCollUnitOrLtv.add(1) + const pairPolicyData = encodePairPolicy(quoteBounds, minLoanPerColl, ONE_USDC.mul(2000), false, 1) // should revert if unregistered vault await expect( @@ -6079,6 +6086,28 @@ describe('Peer-to-Peer: Local Tests', function () { basicPolicyManager.connect(borrower).setPairPolicy(lenderVault.address, weth.address, usdc.address, pairPolicyData) ).to.be.revertedWithCustomError(basicPolicyManager, 'InvalidSender') + // should revert with invalid min/max loan per coll + await expect( + basicPolicyManager + .connect(borrower) + .setPairPolicy( + lenderVault.address, + weth.address, + usdc.address, + encodePairPolicy(quoteBounds, ONE_USDC.mul(0), ONE_USDC.mul(2000), false, 1) + ) + ).to.be.revertedWithCustomError(basicPolicyManager, 'InvalidSender') + await expect( + basicPolicyManager + .connect(borrower) + .setPairPolicy( + lenderVault.address, + weth.address, + usdc.address, + encodePairPolicy(quoteBounds, ONE_USDC.mul(2100), ONE_USDC.mul(2000), false, 1) + ) + ).to.be.revertedWithCustomError(basicPolicyManager, 'InvalidSender') + // should revert if unregistered vault await expect( basicPolicyManager.connect(team).setGlobalPolicy(borrower.address, globalPolicyData) @@ -6130,11 +6159,21 @@ describe('Peer-to-Peer: Local Tests', function () { basicPolicyManager.connect(lender).setGlobalPolicy(lenderVault.address, globalPolicyData) ).to.be.revertedWithCustomError(basicPolicyManager, 'PolicyAlreadySet') - console.log('ok0') await borrowerGateway .connect(borrower) .borrowWithOnChainQuote(lenderVault.address, borrowInstructions1, onChainQuote1, quoteTupleIdx1) + // set pair policy with loan-per-coll unit bounds + await basicPolicyManager.connect(lender).setPairPolicy(lenderVault.address, weth.address, usdc.address, pairPolicyData) + + // should revert with 2nd quote tuple where loan-per-coll unit is out of bounds + await expect( + borrowerGateway.connect(borrower).borrowWithOnChainQuote(lenderVault.address, borrowInstructions1, onChainQuote1, 1) + ).to.be.revertedWithCustomError(quoteHandler, 'QuoteViolatesPolicy') + + // delete pair policy again + await basicPolicyManager.connect(lender).setPairPolicy(lenderVault.address, weth.address, usdc.address, '0x') + await expect( basicPolicyManager .connect(lender) @@ -6239,7 +6278,6 @@ describe('Peer-to-Peer: Local Tests', function () { ) ) - console.log('ok1') // borrow should go through even if loanPerCollUnitOrLtv is below min LTV if no oracle required await borrowerGateway .connect(borrower) @@ -6257,7 +6295,6 @@ describe('Peer-to-Peer: Local Tests', function () { ) ) - console.log('ok2') // borrow should go through even if loanPerCollUnitOrLtv is above maxLtv if no oracle required await borrowerGateway .connect(borrower) @@ -6327,7 +6364,6 @@ describe('Peer-to-Peer: Local Tests', function () { 'OnChainQuoteAdded' ) - console.log('ok3') // borrow with negative interest rate should go through if earliest min repay is long enough await borrowerGateway .connect(borrower) From 614eb9f1603579e3dacd14819ab3eae420cc41ca Mon Sep 17 00:00:00 2001 From: asardon Date: Fri, 11 Aug 2023 03:49:46 +0200 Subject: [PATCH 5/5] updated coverage --- README.md | 6 +++--- test/peer-to-peer/local-tests.ts | 35 +++++++++++++++++++++++++++++++- 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 823250fc..b5934c27 100644 --- a/README.md +++ b/README.md @@ -238,8 +238,8 @@ File | % Stmts | % Branch | OracleLibrary.sol | 100 | 66.67 | 100 | 100 | | TickMath.sol | 100 | 80.43 | 100 | 84.09 |... 63,65,67,73 | TwapGetter.sol | 100 | 62.5 | 100 | 93.33 | 57 | - contracts\peer-to-peer\policyManagers\ | 100 | 94.44 | 100 | 98.95 | | - BasicQuotePolicyManager.sol | 100 | 94.44 | 100 | 98.95 | 121 | + contracts\peer-to-peer\policyManagers\ | 100 | 100 | 100 | 100 | | + BasicQuotePolicyManager.sol | 100 | 100 | 100 | 100 | | DataTypesBasicPolicies.sol | 100 | 100 | 100 | 100 | | contracts\peer-to-peer\wrappers\ERC20\ | 100 | 83.33 | 100 | 98.88 | | ERC20Wrapper.sol | 100 | 75 | 100 | 96.88 | 45 | @@ -257,6 +257,6 @@ File | % Stmts | % Branch | IFundingPoolImpl.sol | 100 | 100 | 100 | 100 | | ILoanProposalImpl.sol | 100 | 100 | 100 | 100 | | ---------------------------------------------------------|----------|----------|----------|----------|----------------| -All files | 99.08 | 89.33 | 98.74 | 96.78 | | +All files | 99.08 | 89.7 | 98.74 | 96.84 | | ---------------------------------------------------------|----------|----------|----------|----------|----------------| ``` diff --git a/test/peer-to-peer/local-tests.ts b/test/peer-to-peer/local-tests.ts index 946ac68f..fa78a030 100644 --- a/test/peer-to-peer/local-tests.ts +++ b/test/peer-to-peer/local-tests.ts @@ -6036,6 +6036,12 @@ describe('Peer-to-Peer: Local Tests', function () { interestRatePctInBase: BASE.div(10), upfrontFeePctInBase: BASE.div(10), tenor: ONE_DAY.mul(30) + }, + { + loanPerCollUnitOrLtv: ONE_USDC.mul(2001), + interestRatePctInBase: BASE.div(10), + upfrontFeePctInBase: BASE.div(10), + tenor: ONE_DAY.mul(30) } ] let onChainQuote1 = { @@ -6086,6 +6092,28 @@ describe('Peer-to-Peer: Local Tests', function () { basicPolicyManager.connect(borrower).setPairPolicy(lenderVault.address, weth.address, usdc.address, pairPolicyData) ).to.be.revertedWithCustomError(basicPolicyManager, 'InvalidSender') + // should revert with invalid loan-per-coll bounds + await expect( + basicPolicyManager + .connect(lender) + .setPairPolicy( + lenderVault.address, + weth.address, + usdc.address, + encodePairPolicy(quoteBounds, ethers.BigNumber.from(0), ethers.BigNumber.from(1), false, 1) + ) + ).to.be.revertedWithCustomError(basicPolicyManager, 'InvalidLoanPerCollBounds') + await expect( + basicPolicyManager + .connect(lender) + .setPairPolicy( + lenderVault.address, + weth.address, + usdc.address, + encodePairPolicy(quoteBounds, ethers.BigNumber.from(1), ethers.BigNumber.from(0), false, 1) + ) + ).to.be.revertedWithCustomError(basicPolicyManager, 'InvalidLoanPerCollBounds') + // should revert with invalid min/max loan per coll await expect( basicPolicyManager @@ -6166,11 +6194,16 @@ describe('Peer-to-Peer: Local Tests', function () { // set pair policy with loan-per-coll unit bounds await basicPolicyManager.connect(lender).setPairPolicy(lenderVault.address, weth.address, usdc.address, pairPolicyData) - // should revert with 2nd quote tuple where loan-per-coll unit is out of bounds + // should revert with 2nd quote tuple where loan-per-coll unit is below bound await expect( borrowerGateway.connect(borrower).borrowWithOnChainQuote(lenderVault.address, borrowInstructions1, onChainQuote1, 1) ).to.be.revertedWithCustomError(quoteHandler, 'QuoteViolatesPolicy') + // should revert with 3rd quote tuple where loan-per-coll unit is above bound + await expect( + borrowerGateway.connect(borrower).borrowWithOnChainQuote(lenderVault.address, borrowInstructions1, onChainQuote1, 2) + ).to.be.revertedWithCustomError(quoteHandler, 'QuoteViolatesPolicy') + // delete pair policy again await basicPolicyManager.connect(lender).setPairPolicy(lenderVault.address, weth.address, usdc.address, '0x')