Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change checkBalances to 'view' #66

Open
wants to merge 4 commits into
base: merge-train-r4-test
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions packages/contracts/contracts/CollateralManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ contract CollateralManager is OwnableUpgradeable, ICollateralManager {
* @return validation_ Boolean indicating if the collateral balance was validated.
*/
function revalidateCollateral(uint256 _bidId)
external
external view
returns (bool validation_)
{
Collateral[] memory collateralInfos = getCollateralInfo(_bidId);
Expand All @@ -189,7 +189,7 @@ contract CollateralManager is OwnableUpgradeable, ICollateralManager {
function checkBalances(
address _borrowerAddress,
Collateral[] calldata _collateralInfo
) public returns (bool validated_, bool[] memory checks_) {
) public view returns (bool validated_, bool[] memory checks_) {
return _checkBalances(_borrowerAddress, _collateralInfo, false);
}

Expand Down Expand Up @@ -530,13 +530,13 @@ contract CollateralManager is OwnableUpgradeable, ICollateralManager {
* @notice Checks the validity of a borrower's multiple collateral balances.
* @param _borrowerAddress The address of the borrower holding the collateral.
* @param _collateralInfo Additional information about the collateral assets.
* @param _shortCircut if true, will return immediately until an invalid balance
* @param _shortCircuit if true, will return immediately until an invalid balance
*/
function _checkBalances(
address _borrowerAddress,
Collateral[] memory _collateralInfo,
bool _shortCircut
) internal virtual returns (bool validated_, bool[] memory checks_) {
bool _shortCircuit
) internal virtual view returns (bool validated_, bool[] memory checks_) {
checks_ = new bool[](_collateralInfo.length);
validated_ = true;
for (uint256 i; i < _collateralInfo.length; i++) {
Expand All @@ -548,7 +548,7 @@ contract CollateralManager is OwnableUpgradeable, ICollateralManager {
if (!isValidated) {
validated_ = false;
//if short circuit is true, return on the first invalid balance to save execution cycles. Values of checks[] will be invalid/undetermined if shortcircuit is true.
if (_shortCircut) {
if (_shortCircuit) {
return (validated_, checks_);
}
}
Expand All @@ -564,7 +564,7 @@ contract CollateralManager is OwnableUpgradeable, ICollateralManager {
function _checkBalance(
address _borrowerAddress,
Collateral memory _collateralInfo
) internal virtual returns (bool) {
) internal virtual view returns (bool) {
CollateralType collateralType = _collateralInfo._collateralType;

if (collateralType == CollateralType.ERC20) {
Expand Down
55 changes: 43 additions & 12 deletions packages/contracts/tests/CollateralManager_Override.sol
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ import "../contracts/mock/TellerV2SolMock.sol";
import "../contracts/CollateralManager.sol";

contract CollateralManager_Override is CollateralManager {

using EnumerableSetUpgradeable for EnumerableSetUpgradeable.AddressSet;

bool public checkBalancesWasCalled;
bool public checkBalanceWasCalled;
address public withdrawInternalWasCalledToRecipient;
Expand Down Expand Up @@ -66,6 +69,9 @@ contract CollateralManager_Override is CollateralManager {
Collateral[] memory _collateralInfo,
bool _shortCircut
) public returns (bool validated_, bool[] memory checks_) {

checkBalancesWasCalled = true;

return
super._checkBalances(
_borrowerAddress,
Expand All @@ -78,6 +84,9 @@ contract CollateralManager_Override is CollateralManager {
address _borrowerAddress,
Collateral memory _collateralInfo
) public returns (bool) {

checkBalanceWasCalled = true;

return super._checkBalance(_borrowerAddress, _collateralInfo);
}

Expand Down Expand Up @@ -112,32 +121,54 @@ contract CollateralManager_Override is CollateralManager {
return bidsCollateralBackedGlobally;
}



function _deposit(uint256 _bidId, Collateral memory collateralInfo)
internal
override
{
depositInternalWasCalled = true;
}

function forceSetBidCollateral(

uint256 _bidId,
Collateral[] memory newCollateralArray
) public {
CollateralInfo storage collateral = _bidCollaterals[_bidId];

for (uint256 i; i < newCollateralArray.length; i++) {
address collateralAddress = newCollateralArray[i]._collateralAddress;

collateral.collateralAddresses.add(collateralAddress );

collateral.collateralInfo[collateralAddress] = newCollateralArray[i];
}

}


/*
function _checkBalances(
address _borrowerAddress,
Collateral[] memory _collateralInfo,
bool _shortCircut
) internal override returns (bool validated_, bool[] memory checks_) {
checkBalancesWasCalled = true;
bool _shortCircuit
) internal override view returns (bool validated_, bool[] memory checks_) {
//checkBalancesWasCalled = true;

validated_ = checkBalanceGlobalValid;
checks_ = new bool[](0);
}

function _deposit(uint256 _bidId, Collateral memory collateralInfo)
internal
override
{
depositInternalWasCalled = true;
}

function _checkBalance(
address _borrowerAddress,
Collateral memory _collateralInfo
) internal override returns (bool) {
checkBalanceWasCalled = true;
) internal override view returns (bool) {
//checkBalanceWasCalled = true;

return checkBalanceGlobalValid;
}
}*/

//for mock purposes
function setGlobalEscrowProxyAddress(address _address) public {
Expand Down
128 changes: 121 additions & 7 deletions packages/contracts/tests/CollateralManager_Test.sol
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ contract CollateralManager_Test is Testable {
User private liquidator;

TestERC20Token wethMock;
TestERC20Token otherErc20Mock;
TestERC721Token erc721Mock;
TestERC1155Token erc1155Mock;

Expand Down Expand Up @@ -65,6 +66,7 @@ contract CollateralManager_Test is Testable {
);

wethMock = new TestERC20Token("wrappedETH", "WETH", 1e24, 18);
otherErc20Mock = new TestERC20Token("wrappedETH", "WETH", 1e24, 18);
erc721Mock = new TestERC721Token("ERC721", "ERC721");
erc1155Mock = new TestERC1155Token("ERC1155");

Expand Down Expand Up @@ -920,7 +922,83 @@ contract CollateralManager_Test is Testable {
assertTrue(valid);
}

function test_checkBalances_public() public {


function test_checkBalances_has_partial() public {
Collateral[] memory collateralArray = new Collateral[](2);


wethMock.transfer(address(borrower), 1000000);

uint256 borrowerBalance = otherErc20Mock.balanceOf(address(borrower));
assertEq( borrowerBalance, 0 ) ;

//does not have this
collateralArray[0] = Collateral({
_collateralType: CollateralType.ERC20,
_amount: 1000,
_tokenId: 0,
_collateralAddress: address(otherErc20Mock)
});

//does have this
collateralArray[1] = Collateral({
_collateralType: CollateralType.ERC20,
_amount: 1000,
_tokenId: 0,
_collateralAddress: address(wethMock)
});


(bool valid, bool[] memory checks) = collateralManager._checkBalancesSuper(
address(borrower),
collateralArray,
false
);

// assertFalse(checks[0]);
// assertTrue(checks[1]);

//check balances should return false (even with short circuit = false)
assertFalse(valid);
}


function test_checkBalances_has_both() public {
Collateral[] memory collateralArray = new Collateral[](2);


wethMock.transfer(address(borrower), 1000000);
otherErc20Mock.transfer(address(borrower), 1000000);

//does have this
collateralArray[0] = Collateral({
_collateralType: CollateralType.ERC20,
_amount: 1000,
_tokenId: 0,
_collateralAddress: address(otherErc20Mock)
});

//does have this
collateralArray[1] = Collateral({
_collateralType: CollateralType.ERC20,
_amount: 1000,
_tokenId: 0,
_collateralAddress: address(wethMock)
});


(bool valid, bool[] memory checks) = collateralManager._checkBalancesSuper(
address(borrower),
collateralArray,
false
);

//check balances should return true ( with short circuit = false)
assertTrue(valid);
}

/* function test_checkBalances_public() public {
Collateral[] memory collateralArray = new Collateral[](1);

collateralArray[0] = Collateral({
Expand All @@ -941,7 +1019,7 @@ contract CollateralManager_Test is Testable {
);
}

/* function test_checkBalance_internal_invalid_type() public {
function test_checkBalance_internal_invalid_type() public {

wethMock.transfer(address(borrower), 1000);

Expand Down Expand Up @@ -1070,6 +1148,9 @@ contract CollateralManager_Test is Testable {
function test_checkBalances_internal_short_circuit_valid() public {
bool shortCircuit = true;


wethMock.transfer(address(borrower), 1000000);

Collateral[] memory collateralArray = new Collateral[](2);
collateralArray[0] = Collateral({
_collateralType: CollateralType.ERC20,
Expand Down Expand Up @@ -1114,7 +1195,7 @@ contract CollateralManager_Test is Testable {
_collateralAddress: address(wethMock)
});

collateralManager.setCheckBalanceGlobalValid(false);
// collateralManager.setCheckBalanceGlobalValid(false);

(bool valid, bool[] memory checks) = collateralManager
._checkBalancesSuper(
Expand All @@ -1128,7 +1209,12 @@ contract CollateralManager_Test is Testable {
}

function test_checkBalances_internal_valid() public {


wethMock.transfer(address(borrower), 1000000);

Collateral[] memory collateralArray = new Collateral[](2);

collateralArray[0] = Collateral({
_collateralType: CollateralType.ERC20,
_amount: 1000,
Expand All @@ -1144,7 +1230,7 @@ contract CollateralManager_Test is Testable {

bool shortCircuit = false;

collateralManager.setCheckBalanceGlobalValid(true);
// collateralManager.setCheckBalanceGlobalValid(true);

(bool valid, bool[] memory checks) = collateralManager
._checkBalancesSuper(
Expand Down Expand Up @@ -1245,11 +1331,22 @@ contract CollateralManager_Test is Testable {
}

function test_revalidateCollateral_invalid() public {
Collateral[] memory collateralArray;


uint256 bidId = 0;

collateralManager.setCheckBalanceGlobalValid(false);

Collateral[] memory collateralArray = new Collateral[](1);

collateralArray[0] = Collateral({
_collateralType: CollateralType.ERC20,
_amount: 1e32,
_tokenId: 0,
_collateralAddress: address(wethMock)
});


collateralManager.forceSetBidCollateral(bidId ,collateralArray );

bool valid = collateralManager.revalidateCollateral(bidId);

Expand All @@ -1259,6 +1356,10 @@ contract CollateralManager_Test is Testable {
function test_commit_collateral_single() public {
uint256 bidId = 0;

wethMock.transfer(address(borrower), 1000000);



Collateral memory collateral = Collateral({
_collateralType: CollateralType.ERC20,
_amount: 1000,
Expand All @@ -1281,6 +1382,10 @@ contract CollateralManager_Test is Testable {
function test_commit_collateral_single_invalid_bid() public {
uint256 bidId = 0;

wethMock.transfer(address(borrower), 1000000);



Collateral memory collateral = Collateral({
_collateralType: CollateralType.ERC20,
_amount: 1000,
Expand All @@ -1297,6 +1402,10 @@ contract CollateralManager_Test is Testable {
function test_commit_collateral_single_not_teller() public {
uint256 bidId = 0;

wethMock.transfer(address(borrower), 1000000);



Collateral memory collateral = Collateral({
_collateralType: CollateralType.ERC20,
_amount: 1000,
Expand Down Expand Up @@ -1338,6 +1447,11 @@ contract CollateralManager_Test is Testable {

Collateral[] memory collateralArray = new Collateral[](1);


wethMock.transfer(address(borrower), 1000000);



collateralArray[0] = Collateral({
_collateralType: CollateralType.ERC20,
_amount: 1000,
Expand All @@ -1347,7 +1461,7 @@ contract CollateralManager_Test is Testable {

tellerV2Mock.setBorrower(address(borrower));

collateralManager.setCheckBalanceGlobalValid(true);
// collateralManager.setCheckBalanceGlobalValid(true);
vm.prank(address(tellerV2Mock));
collateralManager.commitCollateral(bidId, collateralArray);

Expand Down