From 107487ec8c65c9b2c0ae0874564d8cb1c12d2a5a Mon Sep 17 00:00:00 2001 From: Jamie Pickett Date: Thu, 20 Jul 2023 22:21:30 -0400 Subject: [PATCH 1/4] changed update and delete on chain quote to use hash and store in array hash and valid until timestamp --- .../peer-to-peer/DataTypesPeerToPeer.sol | 7 + contracts/peer-to-peer/QuoteHandler.sol | 34 ++++- .../peer-to-peer/interfaces/IQuoteHandler.sol | 20 ++- test/peer-to-peer/local-tests.ts | 11 +- test/peer-to-peer/mainnet-forked-tests.ts | 130 +++++++----------- 5 files changed, 108 insertions(+), 94 deletions(-) diff --git a/contracts/peer-to-peer/DataTypesPeerToPeer.sol b/contracts/peer-to-peer/DataTypesPeerToPeer.sol index 8a8dbd21..6d69addf 100644 --- a/contracts/peer-to-peer/DataTypesPeerToPeer.sol +++ b/contracts/peer-to-peer/DataTypesPeerToPeer.sol @@ -153,6 +153,13 @@ library DataTypesPeerToPeer { uint256 tokenAmount; } + struct QuoteHashAndValidDeadline { + // hash of on chain quote + bytes32 quoteHash; + // valid until timestamp + uint256 validUntil; + } + enum WhitelistState { // not whitelisted NOT_WHITELISTED, diff --git a/contracts/peer-to-peer/QuoteHandler.sol b/contracts/peer-to-peer/QuoteHandler.sol index 9f7a6ef0..92dc9b33 100644 --- a/contracts/peer-to-peer/QuoteHandler.sol +++ b/contracts/peer-to-peer/QuoteHandler.sol @@ -19,6 +19,8 @@ contract QuoteHandler is IQuoteHandler { mapping(address => mapping(bytes32 => bool)) public offChainQuoteIsInvalidated; mapping(address => mapping(bytes32 => bool)) public isOnChainQuote; + mapping(address => DataTypesPeerToPeer.QuoteHashAndValidDeadline[]) + internal quoteHashAndValidDeadlinePerVault; constructor(address _addressRegistry) { if (_addressRegistry == address(0)) { @@ -41,13 +43,21 @@ contract QuoteHandler is IQuoteHandler { if (isOnChainQuoteFromVault[onChainQuoteHash]) { revert Errors.OnChainQuoteAlreadyAdded(); } + // note: in case of a vault re-adding a prior invalidated quote, this does create duplicate entry in array + // but that should be very rare and not worth tracking index with extra storage variable + quoteHashAndValidDeadlinePerVault[lenderVault].push( + DataTypesPeerToPeer.QuoteHashAndValidDeadline({ + quoteHash: onChainQuoteHash, + validUntil: onChainQuote.generalQuoteInfo.validUntil + }) + ); isOnChainQuoteFromVault[onChainQuoteHash] = true; emit OnChainQuoteAdded(lenderVault, onChainQuote, onChainQuoteHash); } function updateOnChainQuote( address lenderVault, - DataTypesPeerToPeer.OnChainQuote calldata oldOnChainQuote, + bytes32 oldOnChainQuoteHash, DataTypesPeerToPeer.OnChainQuote calldata newOnChainQuote ) external { _checkIsRegisteredVaultAndSenderIsOwner(lenderVault); @@ -56,7 +66,6 @@ contract QuoteHandler is IQuoteHandler { } mapping(bytes32 => bool) storage isOnChainQuoteFromVault = isOnChainQuote[lenderVault]; - bytes32 oldOnChainQuoteHash = _hashOnChainQuote(oldOnChainQuote); bytes32 newOnChainQuoteHash = _hashOnChainQuote(newOnChainQuote); // this check will catch the case where the old quote is the same as the new quote if (isOnChainQuoteFromVault[newOnChainQuoteHash]) { @@ -65,6 +74,14 @@ contract QuoteHandler is IQuoteHandler { if (!isOnChainQuoteFromVault[oldOnChainQuoteHash]) { revert Errors.UnknownOnChainQuote(); } + // note: in case of a vault re-adding a prior invalidated quote, this does create duplicate entry in array + // but that should be very rare and not worth tracking index with extra storage variable + quoteHashAndValidDeadlinePerVault[lenderVault].push( + DataTypesPeerToPeer.QuoteHashAndValidDeadline({ + quoteHash: newOnChainQuoteHash, + validUntil: newOnChainQuote.generalQuoteInfo.validUntil + }) + ); isOnChainQuoteFromVault[oldOnChainQuoteHash] = false; emit OnChainQuoteDeleted(lenderVault, oldOnChainQuoteHash); @@ -78,12 +95,11 @@ contract QuoteHandler is IQuoteHandler { function deleteOnChainQuote( address lenderVault, - DataTypesPeerToPeer.OnChainQuote calldata onChainQuote + bytes32 onChainQuoteHash ) external { _checkIsRegisteredVaultAndSenderIsOwner(lenderVault); mapping(bytes32 => bool) storage isOnChainQuoteFromVault = isOnChainQuote[lenderVault]; - bytes32 onChainQuoteHash = _hashOnChainQuote(onChainQuote); if (!isOnChainQuoteFromVault[onChainQuoteHash]) { revert Errors.UnknownOnChainQuote(); } @@ -205,6 +221,16 @@ contract QuoteHandler is IQuoteHandler { ); } + function getQuoteHashAndValidDeadlinePerVault( + address lenderVault + ) + external + view + returns (DataTypesPeerToPeer.QuoteHashAndValidDeadline[] memory) + { + return quoteHashAndValidDeadlinePerVault[lenderVault]; + } + /** * @dev The passed signatures must be sorted such that recovered addresses are increasing. */ diff --git a/contracts/peer-to-peer/interfaces/IQuoteHandler.sol b/contracts/peer-to-peer/interfaces/IQuoteHandler.sol index d73a7856..ff302888 100644 --- a/contracts/peer-to-peer/interfaces/IQuoteHandler.sol +++ b/contracts/peer-to-peer/interfaces/IQuoteHandler.sol @@ -56,12 +56,12 @@ interface IQuoteHandler { * @notice function updates on chain quote * @dev function can only be called by vault owner * @param lenderVault address of the vault updating quote - * @param oldOnChainQuote data for the old onChain quote (See notes in DataTypesPeerToPeer.sol) + * @param oldOnChainQuoteHash quote hash for the old onChain quote marked for deletion * @param newOnChainQuote data for the new onChain quote (See notes in DataTypesPeerToPeer.sol) */ function updateOnChainQuote( address lenderVault, - DataTypesPeerToPeer.OnChainQuote calldata oldOnChainQuote, + bytes32 oldOnChainQuoteHash, DataTypesPeerToPeer.OnChainQuote calldata newOnChainQuote ) external; @@ -69,11 +69,11 @@ interface IQuoteHandler { * @notice function deletes on chain quote * @dev function can only be called by vault owner * @param lenderVault address of the vault deleting - * @param onChainQuote data for the onChain quote marked for deletion (See notes in DataTypesPeerToPeer.sol) + * @param onChainQuoteHash quote hash for the onChain quote marked for deletion */ function deleteOnChainQuote( address lenderVault, - DataTypesPeerToPeer.OnChainQuote calldata onChainQuote + bytes32 onChainQuoteHash ) external; /** @@ -163,4 +163,16 @@ interface IQuoteHandler { address lenderVault, bytes32 hashToCheck ) external view returns (bool); + + /** + * @notice function returns array of structs containing the on chain quote hash and validUntil timestamp + * @param lenderVault address of vault + * @return array of quote hash and validUntil data for every on chain quote in a vault + */ + function getQuoteHashAndValidDeadlinePerVault( + address lenderVault + ) + external + view + returns (DataTypesPeerToPeer.QuoteHashAndValidDeadline[] memory); } diff --git a/test/peer-to-peer/local-tests.ts b/test/peer-to-peer/local-tests.ts index 4f40b0d2..dcd0cabb 100644 --- a/test/peer-to-peer/local-tests.ts +++ b/test/peer-to-peer/local-tests.ts @@ -3354,10 +3354,13 @@ describe('Peer-to-Peer: Local Tests', function () { 'OnChainQuoteAdded' ) - await expect(quoteHandler.connect(lender).deleteOnChainQuote(lenderVault.address, onChainQuote)).to.emit( - quoteHandler, - 'OnChainQuoteDeleted' - ) + const quoteHashAndValidUntilArr = await quoteHandler.getQuoteHashAndValidDeadlinePerVault(lenderVault.address) + + expect(quoteHashAndValidUntilArr.length).to.equal(2) + + await expect( + quoteHandler.connect(lender).deleteOnChainQuote(lenderVault.address, quoteHashAndValidUntilArr[0].quoteHash) + ).to.emit(quoteHandler, 'OnChainQuoteDeleted') }) }) diff --git a/test/peer-to-peer/mainnet-forked-tests.ts b/test/peer-to-peer/mainnet-forked-tests.ts index b13b787b..87cc2e7a 100644 --- a/test/peer-to-peer/mainnet-forked-tests.ts +++ b/test/peer-to-peer/mainnet-forked-tests.ts @@ -879,6 +879,10 @@ describe('Peer-to-Peer: Forked Mainnet Tests', function () { 'OnChainQuoteAdded' ) + const quoteHashAndValidUntilArr = await quoteHandler.getQuoteHashAndValidDeadlinePerVault(lenderVault.address) + + expect(quoteHashAndValidUntilArr.length).to.equal(2) + let newOnChainQuote = { ...onChainQuote, generalQuoteInfo: { @@ -892,25 +896,35 @@ describe('Peer-to-Peer: Forked Mainnet Tests', function () { await addressRegistry.connect(team).setWhitelistState([usdc.address], 0) await expect( - quoteHandler.connect(lender).updateOnChainQuote(borrower.address, onChainQuote, newOnChainQuote) + quoteHandler + .connect(lender) + .updateOnChainQuote(borrower.address, quoteHashAndValidUntilArr[0].quoteHash, newOnChainQuote) ).to.be.revertedWithCustomError(quoteHandler, 'UnregisteredVault') await expect( - quoteHandler.connect(borrower).updateOnChainQuote(lenderVault.address, onChainQuote, newOnChainQuote) + quoteHandler + .connect(borrower) + .updateOnChainQuote(lenderVault.address, quoteHashAndValidUntilArr[0].quoteHash, newOnChainQuote) ).to.be.revertedWithCustomError(quoteHandler, 'InvalidSender') await expect( - quoteHandler.connect(lender).updateOnChainQuote(lenderVault.address, onChainQuote, newOnChainQuote) + quoteHandler + .connect(lender) + .updateOnChainQuote(lenderVault.address, quoteHashAndValidUntilArr[0].quoteHash, newOnChainQuote) ).to.be.revertedWithCustomError(quoteHandler, 'InvalidQuote') newOnChainQuote.generalQuoteInfo.loanToken = usdc.address await expect( - quoteHandler.connect(lender).updateOnChainQuote(lenderVault.address, onChainQuote, newOnChainQuote) + quoteHandler + .connect(lender) + .updateOnChainQuote(lenderVault.address, quoteHashAndValidUntilArr[0].quoteHash, newOnChainQuote) ).to.be.revertedWithCustomError(quoteHandler, 'NonWhitelistedToken') await addressRegistry.connect(team).setWhitelistState([compAddress], 1) await expect( - quoteHandler.connect(lender).updateOnChainQuote(lenderVault.address, onChainQuote, newOnChainQuote) + quoteHandler + .connect(lender) + .updateOnChainQuote(lenderVault.address, quoteHashAndValidUntilArr[0].quoteHash, newOnChainQuote) ).to.be.revertedWithCustomError(quoteHandler, 'NonWhitelistedToken') await addressRegistry.connect(team).setWhitelistState([usdc.address], 1) @@ -918,23 +932,27 @@ describe('Peer-to-Peer: Forked Mainnet Tests', function () { onChainQuote.generalQuoteInfo.loanToken = compAddress await expect( - quoteHandler.connect(lender).updateOnChainQuote(lenderVault.address, onChainQuote, newOnChainQuote) + quoteHandler.connect(lender).updateOnChainQuote(lenderVault.address, ZERO_BYTES32, newOnChainQuote) ).to.be.revertedWithCustomError(quoteHandler, 'UnknownOnChainQuote') onChainQuote.generalQuoteInfo.loanToken = usdc.address // should revert if you add new quote same as old quote await expect( - quoteHandler.connect(lender).updateOnChainQuote(lenderVault.address, onChainQuote, onChainQuote) + quoteHandler + .connect(lender) + .updateOnChainQuote(lenderVault.address, quoteHashAndValidUntilArr[0].quoteHash, onChainQuote) ).to.be.revertedWithCustomError(quoteHandler, 'OnChainQuoteAlreadyAdded') // should revert if you add new quote which is already added await expect( - quoteHandler.connect(lender).updateOnChainQuote(lenderVault.address, onChainQuote, otherOnChainQuote) + quoteHandler + .connect(lender) + .updateOnChainQuote(lenderVault.address, quoteHashAndValidUntilArr[0].quoteHash, otherOnChainQuote) ).to.be.revertedWithCustomError(quoteHandler, 'OnChainQuoteAlreadyAdded') const updateOnChainQuoteTransaction = await quoteHandler .connect(lender) - .updateOnChainQuote(lenderVault.address, onChainQuote, newOnChainQuote) + .updateOnChainQuote(lenderVault.address, quoteHashAndValidUntilArr[0].quoteHash, newOnChainQuote) const updateOnChainQuoteReceipt = await updateOnChainQuoteTransaction.wait() @@ -950,7 +968,17 @@ describe('Peer-to-Peer: Forked Mainnet Tests', function () { expect(borrowQuoteAddedEvent).to.be.not.undefined - await quoteHandler.connect(lender).updateOnChainQuote(lenderVault.address, newOnChainQuote, onChainQuote) + const quoteHashAndValidUntilArrAfterUpdate = await quoteHandler.getQuoteHashAndValidDeadlinePerVault( + lenderVault.address + ) + + expect(quoteHashAndValidUntilArrAfterUpdate.length).to.equal(3) + + await quoteHandler + .connect(lender) + .updateOnChainQuote(lenderVault.address, quoteHashAndValidUntilArrAfterUpdate[2].quoteHash, onChainQuote) + + expect(await quoteHandler.getQuoteHashAndValidDeadlinePerVault(lenderVault.address)).to.have.lengthOf(4) // borrower approves borrower gateway await weth.connect(borrower).approve(borrowerGateway.address, MAX_UINT256) @@ -1870,85 +1898,23 @@ describe('Peer-to-Peer: Forked Mainnet Tests', function () { 'OnChainQuoteAdded' ) - await expect( - quoteHandler.connect(lender).deleteOnChainQuote(borrower.address, onChainQuote) - ).to.be.revertedWithCustomError(quoteHandler, 'UnregisteredVault') - await expect( - quoteHandler.connect(borrower).deleteOnChainQuote(lenderVault.address, onChainQuote) - ).to.be.revertedWithCustomError(quoteHandler, 'InvalidSender') - onChainQuote.generalQuoteInfo.loanToken = weth.address - await expect(quoteHandler.connect(lender).deleteOnChainQuote(lenderVault.address, onChainQuote)).to.reverted + const quoteHashAndValidUntilArr = await quoteHandler.getQuoteHashAndValidDeadlinePerVault(lenderVault.address) - onChainQuote.generalQuoteInfo.loanToken = usdc.address - - await expect(quoteHandler.connect(lender).deleteOnChainQuote(lenderVault.address, onChainQuote)).to.emit( - quoteHandler, - 'OnChainQuoteDeleted' - ) - }) - - it('Should validate correctly the wrong deleteOnChainQuote', async function () { - const { addressRegistry, quoteHandler, lender, borrower, team, usdc, weth, lenderVault } = await setupTest() - - // lenderVault owner deposits usdc - await usdc.connect(lender).transfer(lenderVault.address, ONE_USDC.mul(100000)) - - // lenderVault owner gives quote - const blocknum = await ethers.provider.getBlockNumber() - const timestamp = (await ethers.provider.getBlock(blocknum)).timestamp - let quoteTuples = [ - { - loanPerCollUnitOrLtv: ONE_USDC.mul(1000), - interestRatePctInBase: BASE.mul(10).div(100), - upfrontFeePctInBase: BASE.mul(1).div(100), - tenor: ONE_DAY.mul(365) - }, - { - loanPerCollUnitOrLtv: ONE_USDC.mul(1000), - interestRatePctInBase: BASE.mul(20).div(100), - upfrontFeePctInBase: 0, - tenor: ONE_DAY.mul(180) - } - ] - let onChainQuote = { - generalQuoteInfo: { - collToken: weth.address, - loanToken: usdc.address, - oracleAddr: ZERO_ADDR, - minLoan: ONE_USDC.mul(1000), - maxLoan: MAX_UINT256, - validUntil: timestamp + 60, - earliestRepayTenor: 0, - borrowerCompartmentImplementation: ZERO_ADDR, - isSingleUse: false, - whitelistAddr: ZERO_ADDR, - isWhitelistAddrSingleBorrower: false - }, - quoteTuples: quoteTuples, - salt: ZERO_BYTES32 - } - await addressRegistry.connect(team).setWhitelistState([weth.address, usdc.address], 1) - - await expect(quoteHandler.connect(lender).addOnChainQuote(lenderVault.address, onChainQuote)).to.emit( - quoteHandler, - 'OnChainQuoteAdded' - ) + expect(quoteHashAndValidUntilArr.length).to.equal(1) await expect( - quoteHandler.connect(lender).deleteOnChainQuote(borrower.address, onChainQuote) + quoteHandler.connect(lender).deleteOnChainQuote(borrower.address, quoteHashAndValidUntilArr[0].quoteHash) ).to.be.revertedWithCustomError(quoteHandler, 'UnregisteredVault') await expect( - quoteHandler.connect(borrower).deleteOnChainQuote(lenderVault.address, onChainQuote) + quoteHandler.connect(borrower).deleteOnChainQuote(lenderVault.address, quoteHashAndValidUntilArr[0].quoteHash) ).to.be.revertedWithCustomError(quoteHandler, 'InvalidSender') - onChainQuote.generalQuoteInfo.loanToken = weth.address - await expect(quoteHandler.connect(lender).deleteOnChainQuote(lenderVault.address, onChainQuote)).to.reverted - - onChainQuote.generalQuoteInfo.loanToken = usdc.address + await expect( + quoteHandler.connect(lender).deleteOnChainQuote(lenderVault.address, ZERO_BYTES32) + ).to.revertedWithCustomError(quoteHandler, 'UnknownOnChainQuote') - await expect(quoteHandler.connect(lender).deleteOnChainQuote(lenderVault.address, onChainQuote)).to.emit( - quoteHandler, - 'OnChainQuoteDeleted' - ) + await expect( + quoteHandler.connect(lender).deleteOnChainQuote(lenderVault.address, quoteHashAndValidUntilArr[0].quoteHash) + ).to.emit(quoteHandler, 'OnChainQuoteDeleted') }) }) From 823acd3f402303ad3463e8e13a968f5384c5ee45 Mon Sep 17 00:00:00 2001 From: Jamie Pickett Date: Thu, 20 Jul 2023 22:34:40 -0400 Subject: [PATCH 2/4] updated function name --- contracts/peer-to-peer/QuoteHandler.sol | 10 +++++----- contracts/peer-to-peer/interfaces/IQuoteHandler.sol | 2 +- test/peer-to-peer/local-tests.ts | 2 +- test/peer-to-peer/mainnet-forked-tests.ts | 8 ++++---- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/contracts/peer-to-peer/QuoteHandler.sol b/contracts/peer-to-peer/QuoteHandler.sol index 92dc9b33..988c665f 100644 --- a/contracts/peer-to-peer/QuoteHandler.sol +++ b/contracts/peer-to-peer/QuoteHandler.sol @@ -20,7 +20,7 @@ contract QuoteHandler is IQuoteHandler { public offChainQuoteIsInvalidated; mapping(address => mapping(bytes32 => bool)) public isOnChainQuote; mapping(address => DataTypesPeerToPeer.QuoteHashAndValidDeadline[]) - internal quoteHashAndValidDeadlinePerVault; + internal quoteHashesAndValidUntilTimestampsPerVault; constructor(address _addressRegistry) { if (_addressRegistry == address(0)) { @@ -45,7 +45,7 @@ contract QuoteHandler is IQuoteHandler { } // note: in case of a vault re-adding a prior invalidated quote, this does create duplicate entry in array // but that should be very rare and not worth tracking index with extra storage variable - quoteHashAndValidDeadlinePerVault[lenderVault].push( + quoteHashesAndValidUntilTimestampsPerVault[lenderVault].push( DataTypesPeerToPeer.QuoteHashAndValidDeadline({ quoteHash: onChainQuoteHash, validUntil: onChainQuote.generalQuoteInfo.validUntil @@ -76,7 +76,7 @@ contract QuoteHandler is IQuoteHandler { } // note: in case of a vault re-adding a prior invalidated quote, this does create duplicate entry in array // but that should be very rare and not worth tracking index with extra storage variable - quoteHashAndValidDeadlinePerVault[lenderVault].push( + quoteHashesAndValidUntilTimestampsPerVault[lenderVault].push( DataTypesPeerToPeer.QuoteHashAndValidDeadline({ quoteHash: newOnChainQuoteHash, validUntil: newOnChainQuote.generalQuoteInfo.validUntil @@ -221,14 +221,14 @@ contract QuoteHandler is IQuoteHandler { ); } - function getQuoteHashAndValidDeadlinePerVault( + function getQuoteHashesAndValidUntilTimestampsPerVault( address lenderVault ) external view returns (DataTypesPeerToPeer.QuoteHashAndValidDeadline[] memory) { - return quoteHashAndValidDeadlinePerVault[lenderVault]; + return quoteHashesAndValidUntilTimestampsPerVault[lenderVault]; } /** diff --git a/contracts/peer-to-peer/interfaces/IQuoteHandler.sol b/contracts/peer-to-peer/interfaces/IQuoteHandler.sol index ff302888..b5075de5 100644 --- a/contracts/peer-to-peer/interfaces/IQuoteHandler.sol +++ b/contracts/peer-to-peer/interfaces/IQuoteHandler.sol @@ -169,7 +169,7 @@ interface IQuoteHandler { * @param lenderVault address of vault * @return array of quote hash and validUntil data for every on chain quote in a vault */ - function getQuoteHashAndValidDeadlinePerVault( + function getQuoteHashesAndValidUntilTimestampsPerVault( address lenderVault ) external diff --git a/test/peer-to-peer/local-tests.ts b/test/peer-to-peer/local-tests.ts index dcd0cabb..4dbdc93d 100644 --- a/test/peer-to-peer/local-tests.ts +++ b/test/peer-to-peer/local-tests.ts @@ -3354,7 +3354,7 @@ describe('Peer-to-Peer: Local Tests', function () { 'OnChainQuoteAdded' ) - const quoteHashAndValidUntilArr = await quoteHandler.getQuoteHashAndValidDeadlinePerVault(lenderVault.address) + const quoteHashAndValidUntilArr = await quoteHandler.getQuoteHashesAndValidUntilTimestampsPerVault(lenderVault.address) expect(quoteHashAndValidUntilArr.length).to.equal(2) diff --git a/test/peer-to-peer/mainnet-forked-tests.ts b/test/peer-to-peer/mainnet-forked-tests.ts index 87cc2e7a..c7c7a3ef 100644 --- a/test/peer-to-peer/mainnet-forked-tests.ts +++ b/test/peer-to-peer/mainnet-forked-tests.ts @@ -879,7 +879,7 @@ describe('Peer-to-Peer: Forked Mainnet Tests', function () { 'OnChainQuoteAdded' ) - const quoteHashAndValidUntilArr = await quoteHandler.getQuoteHashAndValidDeadlinePerVault(lenderVault.address) + const quoteHashAndValidUntilArr = await quoteHandler.getQuoteHashesAndValidUntilTimestampsPerVault(lenderVault.address) expect(quoteHashAndValidUntilArr.length).to.equal(2) @@ -968,7 +968,7 @@ describe('Peer-to-Peer: Forked Mainnet Tests', function () { expect(borrowQuoteAddedEvent).to.be.not.undefined - const quoteHashAndValidUntilArrAfterUpdate = await quoteHandler.getQuoteHashAndValidDeadlinePerVault( + const quoteHashAndValidUntilArrAfterUpdate = await quoteHandler.getQuoteHashesAndValidUntilTimestampsPerVault( lenderVault.address ) @@ -978,7 +978,7 @@ describe('Peer-to-Peer: Forked Mainnet Tests', function () { .connect(lender) .updateOnChainQuote(lenderVault.address, quoteHashAndValidUntilArrAfterUpdate[2].quoteHash, onChainQuote) - expect(await quoteHandler.getQuoteHashAndValidDeadlinePerVault(lenderVault.address)).to.have.lengthOf(4) + expect(await quoteHandler.getQuoteHashesAndValidUntilTimestampsPerVault(lenderVault.address)).to.have.lengthOf(4) // borrower approves borrower gateway await weth.connect(borrower).approve(borrowerGateway.address, MAX_UINT256) @@ -1898,7 +1898,7 @@ describe('Peer-to-Peer: Forked Mainnet Tests', function () { 'OnChainQuoteAdded' ) - const quoteHashAndValidUntilArr = await quoteHandler.getQuoteHashAndValidDeadlinePerVault(lenderVault.address) + const quoteHashAndValidUntilArr = await quoteHandler.getQuoteHashesAndValidUntilTimestampsPerVault(lenderVault.address) expect(quoteHashAndValidUntilArr.length).to.equal(1) From d73cfba197d95a4dda52bd2f8bf5a79607564c76 Mon Sep 17 00:00:00 2001 From: asardon Date: Fri, 21 Jul 2023 12:42:19 +0200 Subject: [PATCH 3/4] added fixes for myso-26 --- README.md | 6 +-- .../peer-to-peer/DataTypesPeerToPeer.sol | 2 +- contracts/peer-to-peer/LenderVaultImpl.sol | 4 ++ contracts/peer-to-peer/QuoteHandler.sol | 41 +++++++++++-------- .../interfaces/ILenderVaultImpl.sol | 6 +++ .../peer-to-peer/interfaces/IQuoteHandler.sol | 30 ++++++++++---- test/peer-to-peer/local-tests.ts | 9 +++- test/peer-to-peer/mainnet-forked-tests.ts | 17 ++++---- 8 files changed, 79 insertions(+), 36 deletions(-) diff --git a/README.md b/README.md index 73fbf8f5..07ac52d4 100644 --- a/README.md +++ b/README.md @@ -164,13 +164,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.72 | 94.74 | 98.72 | 98.75 | | + contracts\peer-to-peer\ | 99.73 | 94.74 | 98.78 | 98.76 | | 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 | 92.86 | 100 | 98.88 | 63,206 | - QuoteHandler.sol | 100 | 98.04 | 100 | 99.34 | 365 | + QuoteHandler.sol | 100 | 98.04 | 100 | 99.35 | 412 | contracts\peer-to-peer\callbacks\ | 100 | 75 | 88.89 | 96.88 | | BalancerV2Looping.sol | 100 | 100 | 100 | 100 | | UniV3Looping.sol | 100 | 100 | 100 | 100 | | @@ -245,6 +245,6 @@ File | % Stmts | % Branch | IFundingPoolImpl.sol | 100 | 100 | 100 | 100 | | ILoanProposalImpl.sol | 100 | 100 | 100 | 100 | | ---------------------------------------------------------|----------|----------|----------|----------|----------------| -All files | 98.99 | 88.83 | 98.63 | 96.8 | | +All files | 98.99 | 88.83 | 98.65 | 96.81 | | ---------------------------------------------------------|----------|----------|----------|----------|----------------| ``` diff --git a/contracts/peer-to-peer/DataTypesPeerToPeer.sol b/contracts/peer-to-peer/DataTypesPeerToPeer.sol index 6d69addf..942ae33a 100644 --- a/contracts/peer-to-peer/DataTypesPeerToPeer.sol +++ b/contracts/peer-to-peer/DataTypesPeerToPeer.sol @@ -153,7 +153,7 @@ library DataTypesPeerToPeer { uint256 tokenAmount; } - struct QuoteHashAndValidDeadline { + struct OnChainQuoteInfo { // hash of on chain quote bytes32 quoteHash; // valid until timestamp diff --git a/contracts/peer-to-peer/LenderVaultImpl.sol b/contracts/peer-to-peer/LenderVaultImpl.sol index a4af3893..3308f834 100644 --- a/contracts/peer-to-peer/LenderVaultImpl.sol +++ b/contracts/peer-to-peer/LenderVaultImpl.sol @@ -394,6 +394,10 @@ contract LenderVaultImpl is return _loans.length; } + function totalNumSigners() external view returns (uint256) { + return signers.length; + } + function getTokenBalancesAndLockedAmounts( address[] calldata tokens ) diff --git a/contracts/peer-to-peer/QuoteHandler.sol b/contracts/peer-to-peer/QuoteHandler.sol index 988c665f..a92e1ef8 100644 --- a/contracts/peer-to-peer/QuoteHandler.sol +++ b/contracts/peer-to-peer/QuoteHandler.sol @@ -19,8 +19,8 @@ contract QuoteHandler is IQuoteHandler { mapping(address => mapping(bytes32 => bool)) public offChainQuoteIsInvalidated; mapping(address => mapping(bytes32 => bool)) public isOnChainQuote; - mapping(address => DataTypesPeerToPeer.QuoteHashAndValidDeadline[]) - internal quoteHashesAndValidUntilTimestampsPerVault; + mapping(address => DataTypesPeerToPeer.OnChainQuoteInfo[]) + internal onChainQuoteHistory; constructor(address _addressRegistry) { if (_addressRegistry == address(0)) { @@ -43,10 +43,9 @@ contract QuoteHandler is IQuoteHandler { if (isOnChainQuoteFromVault[onChainQuoteHash]) { revert Errors.OnChainQuoteAlreadyAdded(); } - // note: in case of a vault re-adding a prior invalidated quote, this does create duplicate entry in array - // but that should be very rare and not worth tracking index with extra storage variable - quoteHashesAndValidUntilTimestampsPerVault[lenderVault].push( - DataTypesPeerToPeer.QuoteHashAndValidDeadline({ + // @dev: on-chain quote history is append only + onChainQuoteHistory[lenderVault].push( + DataTypesPeerToPeer.OnChainQuoteInfo({ quoteHash: onChainQuoteHash, validUntil: onChainQuote.generalQuoteInfo.validUntil }) @@ -74,10 +73,9 @@ contract QuoteHandler is IQuoteHandler { if (!isOnChainQuoteFromVault[oldOnChainQuoteHash]) { revert Errors.UnknownOnChainQuote(); } - // note: in case of a vault re-adding a prior invalidated quote, this does create duplicate entry in array - // but that should be very rare and not worth tracking index with extra storage variable - quoteHashesAndValidUntilTimestampsPerVault[lenderVault].push( - DataTypesPeerToPeer.QuoteHashAndValidDeadline({ + // @dev: on-chain quote history is append only + onChainQuoteHistory[lenderVault].push( + DataTypesPeerToPeer.OnChainQuoteInfo({ quoteHash: newOnChainQuoteHash, validUntil: newOnChainQuote.generalQuoteInfo.validUntil }) @@ -221,14 +219,23 @@ contract QuoteHandler is IQuoteHandler { ); } - function getQuoteHashesAndValidUntilTimestampsPerVault( + function getOnChainQuoteHistory( + address lenderVault, + uint256 idx + ) external view returns (DataTypesPeerToPeer.OnChainQuoteInfo memory) { + return onChainQuoteHistory[lenderVault][idx]; + } + + function getFullOnChainQuoteHistory( + address lenderVault + ) external view returns (DataTypesPeerToPeer.OnChainQuoteInfo[] memory) { + return onChainQuoteHistory[lenderVault]; + } + + function getOnChainQuoteHistoryLength( address lenderVault - ) - external - view - returns (DataTypesPeerToPeer.QuoteHashAndValidDeadline[] memory) - { - return quoteHashesAndValidUntilTimestampsPerVault[lenderVault]; + ) external view returns (uint256) { + return onChainQuoteHistory[lenderVault].length; } /** diff --git a/contracts/peer-to-peer/interfaces/ILenderVaultImpl.sol b/contracts/peer-to-peer/interfaces/ILenderVaultImpl.sol index 3533c0fc..2e22c02a 100644 --- a/contracts/peer-to-peer/interfaces/ILenderVaultImpl.sol +++ b/contracts/peer-to-peer/interfaces/ILenderVaultImpl.sol @@ -297,4 +297,10 @@ interface ILenderVaultImpl { * @return total number of loans */ function totalNumLoans() external view returns (uint256); + + /** + * @notice function returns total number of signers + * @return total number of signers + */ + function totalNumSigners() external view returns (uint256); } diff --git a/contracts/peer-to-peer/interfaces/IQuoteHandler.sol b/contracts/peer-to-peer/interfaces/IQuoteHandler.sol index b5075de5..eaa1d5d5 100644 --- a/contracts/peer-to-peer/interfaces/IQuoteHandler.sol +++ b/contracts/peer-to-peer/interfaces/IQuoteHandler.sol @@ -165,14 +165,30 @@ interface IQuoteHandler { ) external view returns (bool); /** - * @notice function returns array of structs containing the on chain quote hash and validUntil timestamp + * @notice function returns element of on-chain history * @param lenderVault address of vault - * @return array of quote hash and validUntil data for every on chain quote in a vault + * @return element of on-chain quote history */ - function getQuoteHashesAndValidUntilTimestampsPerVault( + function getOnChainQuoteHistory( + address lenderVault, + uint256 idx + ) external view returns (DataTypesPeerToPeer.OnChainQuoteInfo memory); + + /** + * @notice function returns array of structs containing the on-chain quote hash and validUntil timestamp + * @param lenderVault address of vault + * @return array of quote hash and validUntil data for on-chain quote history of a vault + */ + function getFullOnChainQuoteHistory( + address lenderVault + ) external view returns (DataTypesPeerToPeer.OnChainQuoteInfo[] memory); + + /** + * @notice function returns the number of on-chain quotes that were added or updated + * @param lenderVault address of vault + * @return number of on-chain quotes that were added or updated + */ + function getOnChainQuoteHistoryLength( address lenderVault - ) - external - view - returns (DataTypesPeerToPeer.QuoteHashAndValidDeadline[] memory); + ) external view returns (uint256); } diff --git a/test/peer-to-peer/local-tests.ts b/test/peer-to-peer/local-tests.ts index 4dbdc93d..37a71b61 100644 --- a/test/peer-to-peer/local-tests.ts +++ b/test/peer-to-peer/local-tests.ts @@ -142,7 +142,10 @@ async function generateOffChainQuote({ expect(recoveredAddr).to.equal(signer.address) // add signer + const preNumSigners = await lenderVault.totalNumSigners() await lenderVault.connect(lender).addSigners([signer.address]) + const postNumSigners = await lenderVault.totalNumSigners() + expect(postNumSigners.sub(preNumSigners)).to.be.equal(1) // lender add sig to quote and pass to borrower offChainQuote.compactSigs = customSignatures.length != 0 ? customSignatures : [compactSig] @@ -3343,6 +3346,8 @@ describe('Peer-to-Peer: Local Tests', function () { salt: ZERO_BYTES32 } + expect(await quoteHandler.getOnChainQuoteHistoryLength(lenderVault.address)).to.equal(0) + await expect(quoteHandler.connect(lender).addOnChainQuote(lenderVault.address, onChainQuote)).to.emit( quoteHandler, 'OnChainQuoteAdded' @@ -3354,9 +3359,11 @@ describe('Peer-to-Peer: Local Tests', function () { 'OnChainQuoteAdded' ) - const quoteHashAndValidUntilArr = await quoteHandler.getQuoteHashesAndValidUntilTimestampsPerVault(lenderVault.address) + const quoteHashAndValidUntilArr = await quoteHandler.getFullOnChainQuoteHistory(lenderVault.address) + const historyLen = await quoteHandler.getOnChainQuoteHistoryLength(lenderVault.address) expect(quoteHashAndValidUntilArr.length).to.equal(2) + expect(quoteHashAndValidUntilArr.length).to.equal(historyLen) await expect( quoteHandler.connect(lender).deleteOnChainQuote(lenderVault.address, quoteHashAndValidUntilArr[0].quoteHash) diff --git a/test/peer-to-peer/mainnet-forked-tests.ts b/test/peer-to-peer/mainnet-forked-tests.ts index c7c7a3ef..48d4a670 100644 --- a/test/peer-to-peer/mainnet-forked-tests.ts +++ b/test/peer-to-peer/mainnet-forked-tests.ts @@ -879,7 +879,7 @@ describe('Peer-to-Peer: Forked Mainnet Tests', function () { 'OnChainQuoteAdded' ) - const quoteHashAndValidUntilArr = await quoteHandler.getQuoteHashesAndValidUntilTimestampsPerVault(lenderVault.address) + const quoteHashAndValidUntilArr = await quoteHandler.getFullOnChainQuoteHistory(lenderVault.address) expect(quoteHashAndValidUntilArr.length).to.equal(2) @@ -968,9 +968,7 @@ describe('Peer-to-Peer: Forked Mainnet Tests', function () { expect(borrowQuoteAddedEvent).to.be.not.undefined - const quoteHashAndValidUntilArrAfterUpdate = await quoteHandler.getQuoteHashesAndValidUntilTimestampsPerVault( - lenderVault.address - ) + const quoteHashAndValidUntilArrAfterUpdate = await quoteHandler.getFullOnChainQuoteHistory(lenderVault.address) expect(quoteHashAndValidUntilArrAfterUpdate.length).to.equal(3) @@ -978,7 +976,7 @@ describe('Peer-to-Peer: Forked Mainnet Tests', function () { .connect(lender) .updateOnChainQuote(lenderVault.address, quoteHashAndValidUntilArrAfterUpdate[2].quoteHash, onChainQuote) - expect(await quoteHandler.getQuoteHashesAndValidUntilTimestampsPerVault(lenderVault.address)).to.have.lengthOf(4) + expect(await quoteHandler.getFullOnChainQuoteHistory(lenderVault.address)).to.have.lengthOf(4) // borrower approves borrower gateway await weth.connect(borrower).approve(borrowerGateway.address, MAX_UINT256) @@ -1898,9 +1896,11 @@ describe('Peer-to-Peer: Forked Mainnet Tests', function () { 'OnChainQuoteAdded' ) - const quoteHashAndValidUntilArr = await quoteHandler.getQuoteHashesAndValidUntilTimestampsPerVault(lenderVault.address) - + const quoteHashAndValidUntilArr = await quoteHandler.getFullOnChainQuoteHistory(lenderVault.address) + const onChainQuoteHistoryElem = await quoteHandler.getOnChainQuoteHistory(lenderVault.address, 0) expect(quoteHashAndValidUntilArr.length).to.equal(1) + expect(quoteHashAndValidUntilArr[0].quoteHash).to.be.equal(onChainQuoteHistoryElem.quoteHash) + expect(quoteHashAndValidUntilArr[0].validUntil).to.be.equal(onChainQuoteHistoryElem.validUntil) await expect( quoteHandler.connect(lender).deleteOnChainQuote(borrower.address, quoteHashAndValidUntilArr[0].quoteHash) @@ -5175,7 +5175,10 @@ describe('Peer-to-Peer: Forked Mainnet Tests', function () { // lenderVault owner deposits usdc await usdc.connect(lender).transfer(lenderVault.address, ONE_USDC.mul(100000)) + const preTotalNumSigners = await lenderVault.numSigners() await lenderVault.connect(lender).addSigners([team.address]) + const postTotalNumSigners = await lenderVault.numSigners() + expect(postTotalNumSigners.sub(preTotalNumSigners)).to.be.equal(1) // deploy chainlinkOracleContract const usdcEthChainlinkAddr = '0x986b5e1e1755e3c2440e960477f25201b0a8bbd4' From 102b2ef4d5c3bdfde4a76986d73a9d6bab7b92e9 Mon Sep 17 00:00:00 2001 From: Jamie Pickett Date: Fri, 21 Jul 2023 08:58:12 -0400 Subject: [PATCH 4/4] added bounds check in getter --- contracts/peer-to-peer/QuoteHandler.sol | 6 +++++- test/peer-to-peer/mainnet-forked-tests.ts | 5 +++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/contracts/peer-to-peer/QuoteHandler.sol b/contracts/peer-to-peer/QuoteHandler.sol index a92e1ef8..236b64f9 100644 --- a/contracts/peer-to-peer/QuoteHandler.sol +++ b/contracts/peer-to-peer/QuoteHandler.sol @@ -223,7 +223,11 @@ contract QuoteHandler is IQuoteHandler { address lenderVault, uint256 idx ) external view returns (DataTypesPeerToPeer.OnChainQuoteInfo memory) { - return onChainQuoteHistory[lenderVault][idx]; + if (idx < onChainQuoteHistory[lenderVault].length) { + return onChainQuoteHistory[lenderVault][idx]; + } else { + revert Errors.InvalidArrayIndex(); + } } function getFullOnChainQuoteHistory( diff --git a/test/peer-to-peer/mainnet-forked-tests.ts b/test/peer-to-peer/mainnet-forked-tests.ts index 48d4a670..b6eadf4d 100644 --- a/test/peer-to-peer/mainnet-forked-tests.ts +++ b/test/peer-to-peer/mainnet-forked-tests.ts @@ -1897,6 +1897,11 @@ describe('Peer-to-Peer: Forked Mainnet Tests', function () { ) const quoteHashAndValidUntilArr = await quoteHandler.getFullOnChainQuoteHistory(lenderVault.address) + // revert if index out of bounds + await expect(quoteHandler.getOnChainQuoteHistory(lenderVault.address, 5)).to.be.revertedWithCustomError( + quoteHandler, + 'InvalidArrayIndex' + ) const onChainQuoteHistoryElem = await quoteHandler.getOnChainQuoteHistory(lenderVault.address, 0) expect(quoteHashAndValidUntilArr.length).to.equal(1) expect(quoteHashAndValidUntilArr[0].quoteHash).to.be.equal(onChainQuoteHistoryElem.quoteHash)