From 3fa45912037f248d849321dbbb090aed6c383092 Mon Sep 17 00:00:00 2001 From: andy Date: Tue, 17 Jun 2025 13:27:07 -0400 Subject: [PATCH 01/10] remove unused fns --- .../contracts/contracts/MarketRegistry.sol | 63 +------------------ packages/contracts/lib/forge-std | 2 +- 2 files changed, 4 insertions(+), 61 deletions(-) diff --git a/packages/contracts/contracts/MarketRegistry.sol b/packages/contracts/contracts/MarketRegistry.sol index b8762f13d..f7146248f 100644 --- a/packages/contracts/contracts/MarketRegistry.sol +++ b/packages/contracts/contracts/MarketRegistry.sol @@ -325,27 +325,7 @@ contract MarketRegistry is _revokeStakeholder(_marketId, _lenderAddress, true); } - /** - * @notice Removes a borrower from a market via delegated revocation. - * @dev See {_revokeStakeholderViaDelegation}. - */ - /* function revokeLender( - uint256 _marketId, - address _lenderAddress, - uint8 _v, - bytes32 _r, - bytes32 _s - ) external { - _revokeStakeholderViaDelegation( - _marketId, - _lenderAddress, - true, - _v, - _r, - _s - ); - } */ - + /** * @notice Allows a lender to voluntarily leave a market. * @param _marketId The market ID to leave. @@ -405,27 +385,7 @@ contract MarketRegistry is _revokeStakeholder(_marketId, _borrowerAddress, false); } - /** - * @notice Removes a borrower from a market via delegated revocation. - * @dev See {_revokeStakeholderViaDelegation}. - */ - /* function revokeBorrower( - uint256 _marketId, - address _borrowerAddress, - uint8 _v, - bytes32 _r, - bytes32 _s - ) external { - _revokeStakeholderViaDelegation( - _marketId, - _borrowerAddress, - false, - _v, - _r, - _s - ); - }*/ - + /** * @notice Allows a borrower to voluntarily leave a market. * @param _marketId The market ID to leave. @@ -1181,24 +1141,7 @@ contract MarketRegistry is } - /* function _revokeStakeholderViaDelegation( - uint256 _marketId, - address _stakeholderAddress, - bool _isLender, - uint8 _v, - bytes32 _r, - bytes32 _s - ) internal { - bytes32 uuid = _revokeStakeholderVerification( - _marketId, - _stakeholderAddress, - _isLender - ); - // NOTE: Disabling the call to revoke the attestation on EAS contracts - // address attestor = markets[_marketId].owner; - // tellerAS.revokeByDelegation(uuid, attestor, _v, _r, _s); - } */ - + /** * @notice Removes a stakeholder (borrower/lender) from a market. * @param _marketId The market ID to remove the lender from. diff --git a/packages/contracts/lib/forge-std b/packages/contracts/lib/forge-std index 73a504d2c..2f6762e4f 160000 --- a/packages/contracts/lib/forge-std +++ b/packages/contracts/lib/forge-std @@ -1 +1 @@ -Subproject commit 73a504d2cf6f37b7ce285b479f4c681f76e95f1b +Subproject commit 2f6762e4f73f3d835457c220b5f62dfeeb6f6341 From 0beba5abf77c334aadb12fa680d446c820ac05e6 Mon Sep 17 00:00:00 2001 From: andy Date: Tue, 17 Jun 2025 13:33:41 -0400 Subject: [PATCH 02/10] remove attestation by delegation --- .../contracts/contracts/MarketRegistry.sol | 160 +----------------- packages/contracts/lib/forge-std | 2 +- .../tests/MarketRegistry_Override.sol | 13 +- .../contracts/tests/MarketRegistry_Test.sol | 34 +--- 4 files changed, 10 insertions(+), 199 deletions(-) diff --git a/packages/contracts/contracts/MarketRegistry.sol b/packages/contracts/contracts/MarketRegistry.sol index b8762f13d..8aae8e26e 100644 --- a/packages/contracts/contracts/MarketRegistry.sol +++ b/packages/contracts/contracts/MarketRegistry.sol @@ -294,28 +294,7 @@ contract MarketRegistry is _attestStakeholder(_marketId, _lenderAddress, _expirationTime, true); } - /** - * @notice Adds a lender to a market via delegated attestation. - * @dev See {_attestStakeholderViaDelegation}. - */ - function attestLender( - uint256 _marketId, - address _lenderAddress, - uint256 _expirationTime, - uint8 _v, - bytes32 _r, - bytes32 _s - ) external { - _attestStakeholderViaDelegation( - _marketId, - _lenderAddress, - _expirationTime, - true, - _v, - _r, - _s - ); - } + /** * @notice Removes a lender from an market. @@ -325,26 +304,7 @@ contract MarketRegistry is _revokeStakeholder(_marketId, _lenderAddress, true); } - /** - * @notice Removes a borrower from a market via delegated revocation. - * @dev See {_revokeStakeholderViaDelegation}. - */ - /* function revokeLender( - uint256 _marketId, - address _lenderAddress, - uint8 _v, - bytes32 _r, - bytes32 _s - ) external { - _revokeStakeholderViaDelegation( - _marketId, - _lenderAddress, - true, - _v, - _r, - _s - ); - } */ + /** * @notice Allows a lender to voluntarily leave a market. @@ -372,28 +332,7 @@ contract MarketRegistry is _attestStakeholder(_marketId, _borrowerAddress, _expirationTime, false); } - /** - * @notice Adds a borrower to a market via delegated attestation. - * @dev See {_attestStakeholderViaDelegation}. - */ - function attestBorrower( - uint256 _marketId, - address _borrowerAddress, - uint256 _expirationTime, - uint8 _v, - bytes32 _r, - bytes32 _s - ) external { - _attestStakeholderViaDelegation( - _marketId, - _borrowerAddress, - _expirationTime, - false, - _v, - _r, - _s - ); - } + /** * @notice Removes a borrower from an market. @@ -405,26 +344,7 @@ contract MarketRegistry is _revokeStakeholder(_marketId, _borrowerAddress, false); } - /** - * @notice Removes a borrower from a market via delegated revocation. - * @dev See {_revokeStakeholderViaDelegation}. - */ - /* function revokeBorrower( - uint256 _marketId, - address _borrowerAddress, - uint8 _v, - bytes32 _r, - bytes32 _s - ) external { - _revokeStakeholderViaDelegation( - _marketId, - _borrowerAddress, - false, - _v, - _r, - _s - ); - }*/ + /** * @notice Allows a borrower to voluntarily leave a market. @@ -1063,58 +983,7 @@ contract MarketRegistry is _isLender ); } - - /** - * @notice Adds a stakeholder (lender or borrower) to a market via delegated attestation. - * @dev The signature must match that of the market owner. - * @param _marketId The market ID to add a lender to. - * @param _stakeholderAddress The address of the lender to add to the market. - * @param _expirationTime The expiration time of the attestation. - * @param _isLender Boolean indicating if the stakeholder is a lender. Otherwise it is a borrower. - * @param _v Signature value - * @param _r Signature value - * @param _s Signature value - */ - function _attestStakeholderViaDelegation( - uint256 _marketId, - address _stakeholderAddress, - uint256 _expirationTime, - bool _isLender, - uint8 _v, - bytes32 _r, - bytes32 _s - ) - internal - virtual - withAttestingSchema( - _isLender ? lenderAttestationSchemaId : borrowerAttestationSchemaId - ) - { - // NOTE: block scope to prevent stack too deep! - bytes32 uuid; - { - bytes memory data = abi.encode(_marketId, _stakeholderAddress); - address attestor = _getMarketOwner(_marketId); - // Submit attestation for stakeholder to join a market (attestation must be signed by market owner) - uuid = tellerAS.attestByDelegation( - _stakeholderAddress, - _attestingSchemaId, // set by the modifier - _expirationTime, - 0, - data, - attestor, - _v, - _r, - _s - ); - } - _attestStakeholderVerification( - _marketId, - _stakeholderAddress, - uuid, - _isLender - ); - } + /** * @notice Adds a stakeholder (borrower/lender) to a market. @@ -1180,24 +1049,7 @@ contract MarketRegistry is // tellerAS.revoke(uuid); } - - /* function _revokeStakeholderViaDelegation( - uint256 _marketId, - address _stakeholderAddress, - bool _isLender, - uint8 _v, - bytes32 _r, - bytes32 _s - ) internal { - bytes32 uuid = _revokeStakeholderVerification( - _marketId, - _stakeholderAddress, - _isLender - ); - // NOTE: Disabling the call to revoke the attestation on EAS contracts - // address attestor = markets[_marketId].owner; - // tellerAS.revokeByDelegation(uuid, attestor, _v, _r, _s); - } */ + /** * @notice Removes a stakeholder (borrower/lender) from a market. diff --git a/packages/contracts/lib/forge-std b/packages/contracts/lib/forge-std index 73a504d2c..2f6762e4f 160000 --- a/packages/contracts/lib/forge-std +++ b/packages/contracts/lib/forge-std @@ -1 +1 @@ -Subproject commit 73a504d2cf6f37b7ce285b479f4c681f76e95f1b +Subproject commit 2f6762e4f73f3d835457c220b5f62dfeeb6f6341 diff --git a/packages/contracts/tests/MarketRegistry_Override.sol b/packages/contracts/tests/MarketRegistry_Override.sol index 27360ffc1..469dabeda 100644 --- a/packages/contracts/tests/MarketRegistry_Override.sol +++ b/packages/contracts/tests/MarketRegistry_Override.sol @@ -188,18 +188,7 @@ contract MarketRegistry_Override is MarketRegistry { ) internal override { attestStakeholderVerificationWasCalled = true; } - - function _attestStakeholderViaDelegation( - uint256 _marketId, - address _stakeholderAddress, - uint256 _expirationTime, - bool _isLender, - uint8 _v, - bytes32 _r, - bytes32 _s - ) internal override { - attestStakeholderViaDelegationWasCalled = true; - } + function _revokeStakeholder( uint256 _marketId, diff --git a/packages/contracts/tests/MarketRegistry_Test.sol b/packages/contracts/tests/MarketRegistry_Test.sol index 210540211..8a125fe83 100644 --- a/packages/contracts/tests/MarketRegistry_Test.sol +++ b/packages/contracts/tests/MarketRegistry_Test.sol @@ -367,22 +367,7 @@ FNDA:0,MarketRegistry._attestStakeholderViaDelegation function test_attestLender_expired() public {} - function test_attestLenderDelegated() public { - marketRegistry.attestLender( - marketId, - address(lender), - expirationTime, - v, - r, - s - ); - - assertEq( - marketRegistry.attestStakeholderViaDelegationWasCalled(), - true, - "Attest stakeholder via delegation was not called" - ); - } + function test_attestBorrower() public { marketRegistry.attestBorrower( @@ -398,22 +383,7 @@ FNDA:0,MarketRegistry._attestStakeholderViaDelegation ); } - function test_attestBorrowerDelegated() public { - marketRegistry.attestBorrower( - marketId, - address(lender), - expirationTime, - v, - r, - s - ); - - assertEq( - marketRegistry.attestStakeholderViaDelegationWasCalled(), - true, - "Attest stakeholder via delegation was not called" - ); - } + function test_revokeLender() public { marketRegistry.revokeLender(marketId, address(lender)); From 9dbd837d5662b11250d0603c1a2496b93ee87d9a Mon Sep 17 00:00:00 2001 From: andy Date: Tue, 17 Jun 2025 13:39:27 -0400 Subject: [PATCH 03/10] remove unused fns from collateralmanager --- .../contracts/contracts/CollateralManager.sol | 37 +------------------ .../interfaces/ICollateralManager.sol | 21 +---------- packages/contracts/lib/forge-std | 2 +- .../tests/CollateralManager_Test.sol | 32 +++++++++++----- .../tests/TellerV2/TellerV2_Test.sol | 2 +- 5 files changed, 28 insertions(+), 66 deletions(-) diff --git a/packages/contracts/contracts/CollateralManager.sol b/packages/contracts/contracts/CollateralManager.sol index c587aea29..68a317ea9 100644 --- a/packages/contracts/contracts/CollateralManager.sol +++ b/packages/contracts/contracts/CollateralManager.sol @@ -149,24 +149,7 @@ contract CollateralManager is OwnableUpgradeable, ICollateralManager { } } - /** - * @notice Checks the validity of a borrower's collateral balance and commits it to a bid. - * @param _bidId The id of the associated bid. - * @param _collateralInfo Additional information about the collateral asset. - * @return validation_ Boolean indicating if the collateral balance was validated. - */ - function commitCollateral( - uint256 _bidId, - Collateral calldata _collateralInfo - ) public onlyTellerV2 returns (bool validation_) { - address borrower = tellerV2.getLoanBorrower(_bidId); - require(borrower != address(0), "Loan has no borrower"); - validation_ = _checkBalance(borrower, _collateralInfo); - if (validation_) { - _commitCollateral(_bidId, _collateralInfo); - } - } - + /** * @notice Re-checks the validity of a borrower's collateral balance committed to a bid. * @param _bidId The id of the associated bid. @@ -294,23 +277,7 @@ contract CollateralManager is OwnableUpgradeable, ICollateralManager { ); } - /** - * @notice Withdraws deposited collateral from the created escrow of a bid that has been CLOSED after being defaulted. - * @param _bidId The id of the bid to withdraw collateral for. - */ - function lenderClaimCollateral(uint256 _bidId) external onlyTellerV2 whenProtocolNotPaused { - if (isBidCollateralBacked(_bidId)) { - BidState bidState = tellerV2.getBidState(_bidId); - - require( - bidState == BidState.CLOSED, - "Loan has not been liquidated" - ); - - _withdraw(_bidId, tellerV2.getLoanLender(_bidId)); - emit CollateralClaimed(_bidId); - } - } + /** * @notice Withdraws deposited collateral from the created escrow of a bid that has been CLOSED after being defaulted. diff --git a/packages/contracts/contracts/interfaces/ICollateralManager.sol b/packages/contracts/contracts/interfaces/ICollateralManager.sol index fa047facd..b48172be3 100644 --- a/packages/contracts/contracts/interfaces/ICollateralManager.sol +++ b/packages/contracts/contracts/interfaces/ICollateralManager.sol @@ -15,17 +15,7 @@ interface ICollateralManager { Collateral[] calldata _collateralInfo ) external returns (bool validation_); - /** - * @notice Checks the validity of a borrower's collateral balance and commits it to a bid. - * @param _bidId The id of the associated bid. - * @param _collateralInfo Additional information about the collateral asset. - * @return validation_ Boolean indicating if the collateral balance was validated. - */ - function commitCollateral( - uint256 _bidId, - Collateral calldata _collateralInfo - ) external returns (bool validation_); - + function checkBalances( address _borrowerAddress, Collateral[] calldata _collateralInfo @@ -72,14 +62,7 @@ interface ICollateralManager { */ function revalidateCollateral(uint256 _bidId) external returns (bool); - /** - * @notice Sends the deposited collateral to a lender of a bid. - * @notice Can only be called by the protocol. - * @param _bidId The id of the liquidated bid. - */ - function lenderClaimCollateral(uint256 _bidId) external; - - + /** * @notice Sends the deposited collateral to a lender of a bid. diff --git a/packages/contracts/lib/forge-std b/packages/contracts/lib/forge-std index 73a504d2c..2f6762e4f 160000 --- a/packages/contracts/lib/forge-std +++ b/packages/contracts/lib/forge-std @@ -1 +1 @@ -Subproject commit 73a504d2cf6f37b7ce285b479f4c681f76e95f1b +Subproject commit 2f6762e4f73f3d835457c220b5f62dfeeb6f6341 diff --git a/packages/contracts/tests/CollateralManager_Test.sol b/packages/contracts/tests/CollateralManager_Test.sol index 255e22b34..9d5137893 100644 --- a/packages/contracts/tests/CollateralManager_Test.sol +++ b/packages/contracts/tests/CollateralManager_Test.sol @@ -534,7 +534,7 @@ contract CollateralManager_Test is Testable { tellerV2Mock.setGlobalBidState(BidState.CLOSED); vm.expectRevert("Sender not authorized"); - collateralManager.lenderClaimCollateral(bidId); + collateralManager.lenderClaimCollateralWithRecipient(bidId,address(lender)); } function test_lenderClaimCollateral() public { @@ -548,7 +548,7 @@ contract CollateralManager_Test is Testable { vm.expectEmit(true, false, false, false); emit CollateralClaimed(bidId); vm.prank(address(tellerV2Mock)); - collateralManager.lenderClaimCollateral(bidId); + collateralManager.lenderClaimCollateralWithRecipient(bidId,address(lender)); assertEq( collateralManager.withdrawInternalWasCalledToRecipient(), @@ -1259,18 +1259,21 @@ contract CollateralManager_Test is Testable { function test_commit_collateral_single() public { uint256 bidId = 0; - Collateral memory collateral = Collateral({ + Collateral[] memory collateralArray = new Collateral[](1); + + collateralArray[0] = Collateral({ _collateralType: CollateralType.ERC20, _amount: 1000, _tokenId: 0, _collateralAddress: address(wethMock) }); + tellerV2Mock.setBorrower(address(borrower)); collateralManager.setCheckBalanceGlobalValid(true); vm.prank(address(tellerV2Mock)); - collateralManager.commitCollateral(bidId, collateral); + collateralManager.commitCollateral(bidId, collateralArray); assertTrue( collateralManager.commitCollateralInternalWasCalled(), @@ -1281,51 +1284,60 @@ contract CollateralManager_Test is Testable { function test_commit_collateral_single_invalid_bid() public { uint256 bidId = 0; - Collateral memory collateral = Collateral({ + Collateral[] memory collateralArray = new Collateral[](1); + + collateralArray[0] = Collateral({ _collateralType: CollateralType.ERC20, _amount: 1000, _tokenId: 0, _collateralAddress: address(wethMock) }); + collateralManager.setCheckBalanceGlobalValid(true); vm.prank(address(tellerV2Mock)); vm.expectRevert("Loan has no borrower"); - collateralManager.commitCollateral(bidId, collateral); + collateralManager.commitCollateral(bidId, collateralArray); } function test_commit_collateral_single_not_teller() public { uint256 bidId = 0; - Collateral memory collateral = Collateral({ + Collateral[] memory collateralArray = new Collateral[](1); + + collateralArray[0] = Collateral({ _collateralType: CollateralType.ERC20, _amount: 1000, _tokenId: 0, _collateralAddress: address(wethMock) }); + tellerV2Mock.setBorrower(address(borrower)); collateralManager.setCheckBalanceGlobalValid(true); vm.expectRevert("Sender not authorized"); - collateralManager.commitCollateral(bidId, collateral); + collateralManager.commitCollateral(bidId, collateralArray); } function test_commit_collateral_single_invalid() public { uint256 bidId = 0; - Collateral memory collateral = Collateral({ + Collateral[] memory collateralArray = new Collateral[](1); + + collateralArray[0] = Collateral({ _collateralType: CollateralType.ERC20, _amount: 1000, _tokenId: 0, _collateralAddress: address(wethMock) }); + tellerV2Mock.setBorrower(address(borrower)); collateralManager.setCheckBalanceGlobalValid(false); vm.prank(address(tellerV2Mock)); - collateralManager.commitCollateral(bidId, collateral); + collateralManager.commitCollateral(bidId, collateralArray); assertFalse( collateralManager.commitCollateralInternalWasCalled(), diff --git a/packages/contracts/tests/TellerV2/TellerV2_Test.sol b/packages/contracts/tests/TellerV2/TellerV2_Test.sol index 4849effec..50d5c0305 100644 --- a/packages/contracts/tests/TellerV2/TellerV2_Test.sol +++ b/packages/contracts/tests/TellerV2/TellerV2_Test.sol @@ -262,7 +262,7 @@ contract TellerV2_Test is Testable { // The malicious borrower performs the attack by frontrunning the tx and updating the bid collateral amount vm.prank(address(borrower)); vm.expectRevert(); - collateralManager.commitCollateral(bidId, info); + collateralManager.commitCollateral(bidId, collateralInfo); // The lender is now victim to the frontrunning and accepts the malicious bid /* acceptBid(bidId); From 02f57e922d8b704c0f1e6684af1df93fe409988d Mon Sep 17 00:00:00 2001 From: andy Date: Tue, 17 Jun 2025 13:41:49 -0400 Subject: [PATCH 04/10] move decrement above interaction --- packages/contracts/contracts/escrow/CollateralEscrowV1.sol | 6 +++++- packages/contracts/lib/forge-std | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/contracts/contracts/escrow/CollateralEscrowV1.sol b/packages/contracts/contracts/escrow/CollateralEscrowV1.sol index 1d1247cae..c3f6d6baf 100644 --- a/packages/contracts/contracts/escrow/CollateralEscrowV1.sol +++ b/packages/contracts/contracts/escrow/CollateralEscrowV1.sol @@ -93,13 +93,17 @@ contract CollateralEscrowV1 is OwnableUpgradeable, ICollateralEscrowV1 { collateral._amount >= _amount, "No collateral balance for asset" ); + + //this comes first to mitigate re-entrancy via checks-effects-interact pattern + collateral._amount -= _amount; + _withdrawCollateral( collateral, _collateralAddress, _amount, _recipient ); - collateral._amount -= _amount; + emit CollateralWithdrawn(_collateralAddress, _amount, _recipient); } diff --git a/packages/contracts/lib/forge-std b/packages/contracts/lib/forge-std index 73a504d2c..2f6762e4f 160000 --- a/packages/contracts/lib/forge-std +++ b/packages/contracts/lib/forge-std @@ -1 +1 @@ -Subproject commit 73a504d2cf6f37b7ce285b479f4c681f76e95f1b +Subproject commit 2f6762e4f73f3d835457c220b5f62dfeeb6f6341 From 0db9d31b95276b74e0106de69b20ce0603a81ce0 Mon Sep 17 00:00:00 2001 From: andy Date: Tue, 17 Jun 2025 13:42:44 -0400 Subject: [PATCH 05/10] remove payable --- packages/contracts/contracts/escrow/CollateralEscrowV1.sol | 2 +- packages/contracts/lib/forge-std | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/contracts/contracts/escrow/CollateralEscrowV1.sol b/packages/contracts/contracts/escrow/CollateralEscrowV1.sol index 1d1247cae..3ec49ac7f 100644 --- a/packages/contracts/contracts/escrow/CollateralEscrowV1.sol +++ b/packages/contracts/contracts/escrow/CollateralEscrowV1.sol @@ -54,7 +54,7 @@ contract CollateralEscrowV1 is OwnableUpgradeable, ICollateralEscrowV1 { address _collateralAddress, uint256 _amount, uint256 _tokenId - ) external payable virtual onlyOwner { + ) external virtual onlyOwner { require(_amount > 0, "Deposit amount cannot be zero"); _depositCollateral( _collateralType, diff --git a/packages/contracts/lib/forge-std b/packages/contracts/lib/forge-std index 73a504d2c..2f6762e4f 160000 --- a/packages/contracts/lib/forge-std +++ b/packages/contracts/lib/forge-std @@ -1 +1 @@ -Subproject commit 73a504d2cf6f37b7ce285b479f4c681f76e95f1b +Subproject commit 2f6762e4f73f3d835457c220b5f62dfeeb6f6341 From 175573acf791c0eb5ffd82d295bb2acd1ae45346 Mon Sep 17 00:00:00 2001 From: andy Date: Tue, 17 Jun 2025 13:44:29 -0400 Subject: [PATCH 06/10] fix tests --- .../contracts/interfaces/escrow/ICollateralEscrowV1.sol | 2 +- packages/contracts/tests/CollateralManager_Test.sol | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/contracts/contracts/interfaces/escrow/ICollateralEscrowV1.sol b/packages/contracts/contracts/interfaces/escrow/ICollateralEscrowV1.sol index d8440f337..977863ccb 100644 --- a/packages/contracts/contracts/interfaces/escrow/ICollateralEscrowV1.sol +++ b/packages/contracts/contracts/interfaces/escrow/ICollateralEscrowV1.sol @@ -26,7 +26,7 @@ interface ICollateralEscrowV1 { address _collateralAddress, uint256 _amount, uint256 _tokenId - ) external payable; + ) external; /** * @notice Withdraws a collateral asset from the escrow. diff --git a/packages/contracts/tests/CollateralManager_Test.sol b/packages/contracts/tests/CollateralManager_Test.sol index 255e22b34..5c0dde977 100644 --- a/packages/contracts/tests/CollateralManager_Test.sol +++ b/packages/contracts/tests/CollateralManager_Test.sol @@ -1479,7 +1479,7 @@ contract CollateralEscrowV1_Mock is CollateralEscrowV1 { address _collateralAddress, uint256 _amount, uint256 _tokenId - ) external payable override { + ) external override { depositAssetWasCalled = true; } From b0899f61a1f099b7d293b520c17c97bf0c1f1896 Mon Sep 17 00:00:00 2001 From: andy Date: Tue, 17 Jun 2025 13:52:39 -0400 Subject: [PATCH 07/10] improve efficiency of collateralmanager --- packages/contracts/contracts/CollateralManager.sol | 13 ++++++++----- packages/contracts/lib/forge-std | 2 +- .../contracts/tests/CollateralManager_Override.sol | 6 +++--- .../contracts/tests/CollateralManager_Test.sol | 14 +++++++------- 4 files changed, 19 insertions(+), 16 deletions(-) diff --git a/packages/contracts/contracts/CollateralManager.sol b/packages/contracts/contracts/CollateralManager.sol index c587aea29..9879793a5 100644 --- a/packages/contracts/contracts/CollateralManager.sol +++ b/packages/contracts/contracts/CollateralManager.sol @@ -199,8 +199,9 @@ contract CollateralManager is OwnableUpgradeable, ICollateralManager { */ function deployAndDeposit(uint256 _bidId) external onlyTellerV2 { if (isBidCollateralBacked(_bidId)) { - //attempt deploy a new collateral escrow contract if there is not already one. Otherwise fetch it. - (address proxyAddress, ) = _deployEscrow(_bidId); + //attempt deploy a new collateral escrow contract for this bid if there is not already one. Otherwise fetch it. + (address proxyAddress, address borrower) = _deployEscrow(_bidId); + _escrows[_bidId] = proxyAddress; //for each bid collateral associated with this loan, deposit the collateral into escrow @@ -213,7 +214,9 @@ contract CollateralManager is OwnableUpgradeable, ICollateralManager { _bidId, _bidCollaterals[_bidId].collateralInfo[ _bidCollaterals[_bidId].collateralAddresses.at(i) - ] + ], + proxyAddress, + borrower ); } @@ -385,12 +388,12 @@ contract CollateralManager is OwnableUpgradeable, ICollateralManager { * @param collateralInfo The collateral info to deposit. */ - function _deposit(uint256 _bidId, Collateral memory collateralInfo) + function _deposit(uint256 _bidId, Collateral memory collateralInfo, address escrowAddress, address borrower ) internal virtual { require(collateralInfo._amount > 0, "Collateral not validated"); - (address escrowAddress, address borrower) = _deployEscrow(_bidId); + ICollateralEscrowV1 collateralEscrow = ICollateralEscrowV1( escrowAddress ); diff --git a/packages/contracts/lib/forge-std b/packages/contracts/lib/forge-std index 73a504d2c..2f6762e4f 160000 --- a/packages/contracts/lib/forge-std +++ b/packages/contracts/lib/forge-std @@ -1 +1 @@ -Subproject commit 73a504d2cf6f37b7ce285b479f4c681f76e95f1b +Subproject commit 2f6762e4f73f3d835457c220b5f62dfeeb6f6341 diff --git a/packages/contracts/tests/CollateralManager_Override.sol b/packages/contracts/tests/CollateralManager_Override.sol index 6f6b05cd4..bd7360394 100644 --- a/packages/contracts/tests/CollateralManager_Override.sol +++ b/packages/contracts/tests/CollateralManager_Override.sol @@ -40,10 +40,10 @@ contract CollateralManager_Override is CollateralManager { super._commitCollateral(bidId, collateralInfo); } - function _depositSuper(uint256 _bidId, Collateral memory _collateralInfo) + function _depositSuper(uint256 _bidId, Collateral memory _collateralInfo, address _escrow, address _borrower ) public { - super._deposit(_bidId, _collateralInfo); + super._deposit(_bidId, _collateralInfo, _escrow, _borrower); } function _withdrawSuper(uint256 _bidId, address _receiver) public { @@ -123,7 +123,7 @@ contract CollateralManager_Override is CollateralManager { checks_ = new bool[](0); } - function _deposit(uint256 _bidId, Collateral memory collateralInfo) + function _deposit(uint256 _bidId, Collateral memory collateralInfo, address escrow, address borrower ) internal override { diff --git a/packages/contracts/tests/CollateralManager_Test.sol b/packages/contracts/tests/CollateralManager_Test.sol index 255e22b34..acf169e8e 100644 --- a/packages/contracts/tests/CollateralManager_Test.sol +++ b/packages/contracts/tests/CollateralManager_Test.sol @@ -163,7 +163,7 @@ contract CollateralManager_Test is Testable { collateral._amount, collateral._tokenId ); - collateralManager._depositSuper(bidId, collateral); + collateralManager._depositSuper(bidId, collateral,address(escrowImplementation),address(borrower)); } function test_deposit_collateral_erc20_not_validated() public { @@ -191,7 +191,7 @@ contract CollateralManager_Test is Testable { ); vm.expectRevert("Collateral not validated"); - collateralManager._depositSuper(bidId, collateral); + collateralManager._depositSuper(bidId, collateral,address(escrowImplementation),address(borrower)); } function test_deposit_erc20() public { @@ -227,7 +227,7 @@ contract CollateralManager_Test is Testable { collateral._tokenId ); vm.prank(address(borrower)); - collateralManager._depositSuper(bidId, collateral); + collateralManager._depositSuper(bidId, collateral,address(escrowImplementation),address(borrower)); assertEq( escrowImplementation.depositAssetWasCalled(), @@ -267,7 +267,7 @@ contract CollateralManager_Test is Testable { collateral._tokenId ); vm.prank(address(borrower)); - collateralManager._depositSuper(bidId, collateral); + collateralManager._depositSuper(bidId, collateral,address(escrowImplementation),address(borrower)); assertEq( escrowImplementation.depositAssetWasCalled(), @@ -300,7 +300,7 @@ contract CollateralManager_Test is Testable { vm.expectRevert("Collateral not validated"); vm.prank(address(borrower)); - collateralManager._depositSuper(bidId, collateral); + collateralManager._depositSuper(bidId, collateral,address(escrowImplementation),address(borrower)); } function test_deposit_erc1155() public { @@ -334,7 +334,7 @@ contract CollateralManager_Test is Testable { collateral._tokenId ); vm.prank(address(borrower)); - collateralManager._depositSuper(bidId, collateral); + collateralManager._depositSuper(bidId, collateral,address(escrowImplementation),address(borrower)); assertEq( escrowImplementation.depositAssetWasCalled(), @@ -365,7 +365,7 @@ contract CollateralManager_Test is Testable { vm.expectRevert("Collateral not validated"); vm.prank(address(borrower)); - collateralManager._depositSuper(bidId, collateral); + collateralManager._depositSuper(bidId, collateral,address(escrowImplementation),address(borrower)); } /*function test_deposit_invalid_bid() public { From 4d265437c6fa92d8678760198aa869b7448bfd1d Mon Sep 17 00:00:00 2001 From: andy Date: Tue, 17 Jun 2025 13:56:09 -0400 Subject: [PATCH 08/10] _checkbalances internal call --- packages/contracts/contracts/CollateralManager.sol | 2 +- packages/contracts/lib/forge-std | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/contracts/contracts/CollateralManager.sol b/packages/contracts/contracts/CollateralManager.sol index c587aea29..7c96f90d7 100644 --- a/packages/contracts/contracts/CollateralManager.sol +++ b/packages/contracts/contracts/CollateralManager.sol @@ -138,7 +138,7 @@ contract CollateralManager is OwnableUpgradeable, ICollateralManager { ) public onlyTellerV2 returns (bool validation_) { address borrower = tellerV2.getLoanBorrower(_bidId); require(borrower != address(0), "Loan has no borrower"); - (validation_, ) = checkBalances(borrower, _collateralInfo); + (validation_, ) = _checkBalances(borrower, _collateralInfo, false); //if the collateral info is valid, call commitCollateral for each one if (validation_) { diff --git a/packages/contracts/lib/forge-std b/packages/contracts/lib/forge-std index 73a504d2c..2f6762e4f 160000 --- a/packages/contracts/lib/forge-std +++ b/packages/contracts/lib/forge-std @@ -1 +1 @@ -Subproject commit 73a504d2cf6f37b7ce285b479f4c681f76e95f1b +Subproject commit 2f6762e4f73f3d835457c220b5f62dfeeb6f6341 From 44c17082956d807b69f26458dd07c16475b9ea41 Mon Sep 17 00:00:00 2001 From: andy Date: Tue, 17 Jun 2025 14:03:44 -0400 Subject: [PATCH 09/10] use safe xfer in collateral manager --- packages/contracts/contracts/CollateralManager.sol | 11 ++++++++++- packages/contracts/lib/forge-std | 2 +- .../contracts/tests/CollateralManager_Override.sol | 2 +- packages/contracts/tests/CollateralManager_Test.sol | 2 +- 4 files changed, 13 insertions(+), 4 deletions(-) diff --git a/packages/contracts/contracts/CollateralManager.sol b/packages/contracts/contracts/CollateralManager.sol index c587aea29..4891958f4 100644 --- a/packages/contracts/contracts/CollateralManager.sol +++ b/packages/contracts/contracts/CollateralManager.sol @@ -18,7 +18,16 @@ import { Collateral, CollateralType, ICollateralEscrowV1 } from "./interfaces/es import "./interfaces/ITellerV2.sol"; import "./interfaces/IProtocolPausingManager.sol"; import "./interfaces/IHasProtocolPausingManager.sol"; + +import "@openzeppelin/contracts/token/ERC20/ERC20.sol"; +import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; + + + contract CollateralManager is OwnableUpgradeable, ICollateralManager { + + using SafeERC20 for ERC20; + /* Storage */ using EnumerableSetUpgradeable for EnumerableSetUpgradeable.AddressSet; ITellerV2 public tellerV2; @@ -396,7 +405,7 @@ contract CollateralManager is OwnableUpgradeable, ICollateralManager { ); // Pull collateral from borrower & deposit into escrow if (collateralInfo._collateralType == CollateralType.ERC20) { - IERC20Upgradeable(collateralInfo._collateralAddress).transferFrom( + ERC20(collateralInfo._collateralAddress).safeTransferFrom( borrower, address(this), collateralInfo._amount diff --git a/packages/contracts/lib/forge-std b/packages/contracts/lib/forge-std index 73a504d2c..2f6762e4f 160000 --- a/packages/contracts/lib/forge-std +++ b/packages/contracts/lib/forge-std @@ -1 +1 @@ -Subproject commit 73a504d2cf6f37b7ce285b479f4c681f76e95f1b +Subproject commit 2f6762e4f73f3d835457c220b5f62dfeeb6f6341 diff --git a/packages/contracts/tests/CollateralManager_Override.sol b/packages/contracts/tests/CollateralManager_Override.sol index 6f6b05cd4..f8c30404b 100644 --- a/packages/contracts/tests/CollateralManager_Override.sol +++ b/packages/contracts/tests/CollateralManager_Override.sol @@ -16,7 +16,7 @@ import "./tokens/TestERC721Token.sol"; import "./tokens/TestERC1155Token.sol"; import "../contracts/mock/TellerV2SolMock.sol"; -import "../contracts/CollateralManager.sol"; +import {CollateralManager} from "../contracts/CollateralManager.sol"; contract CollateralManager_Override is CollateralManager { bool public checkBalancesWasCalled; diff --git a/packages/contracts/tests/CollateralManager_Test.sol b/packages/contracts/tests/CollateralManager_Test.sol index 255e22b34..a086f8399 100644 --- a/packages/contracts/tests/CollateralManager_Test.sol +++ b/packages/contracts/tests/CollateralManager_Test.sol @@ -15,7 +15,7 @@ import "./tokens/TestERC721Token.sol"; import "./tokens/TestERC1155Token.sol"; import "../contracts/mock/TellerV2SolMock.sol"; -import "../contracts/CollateralManager.sol"; +import {CollateralManager,CollateralType} from "../contracts/CollateralManager.sol"; import "../contracts/pausing/ProtocolPausingManager.sol"; import "./CollateralManager_Override.sol"; From b07ba2e96ce46343cf7f997b533e6826d6d04c39 Mon Sep 17 00:00:00 2001 From: andy Date: Mon, 14 Jul 2025 17:38:29 -0400 Subject: [PATCH 10/10] opt config --- packages/contracts/hardhat.config.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/contracts/hardhat.config.ts b/packages/contracts/hardhat.config.ts index f0c4c0e23..05f7c95b0 100644 --- a/packages/contracts/hardhat.config.ts +++ b/packages/contracts/hardhat.config.ts @@ -217,7 +217,7 @@ export default { polygon: process.env.POLYGONSCAN_VERIFY_API_KEY, arbitrumOne: process.env.ARBISCAN_VERIFY_API_KEY, base: process.env.BASESCAN_VERIFY_API_KEY, - optimism: process.env.OPTIMISMSCAN_VERIFY_API_KEY, + optimism: process.env.ETHERSCANV2_VERIFY_API_KEY, mantle: process.env.MANTLE_VERIFY_API_KEY ?? 'xyz', clarity: '', //none ? @@ -241,7 +241,7 @@ export default { network: 'optimism', chainId: 10, urls: { - apiURL: 'https://api-optimistic.etherscan.io/api', + apiURL: 'https://api.etherscan.io/v2/api?chainid=10', //using the v2 api browserURL: 'https://optimistic.etherscan.io', }, },