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.

Sign up for GitHub

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

fix exisiting unit tests #54

Merged
merged 17 commits into from
Oct 18, 2024
Merged

fix exisiting unit tests #54

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
44eca76
fix exisiting unit tests
aazhou1 Oct 2, 2024
ed1e49d
get rid of console log
aazhou1 Oct 2, 2024
3753572
add repo token holding value checks on all auction steps
aazhou1 Oct 2, 2024
a9d6939
add repo token holding value checks on all auction steps
aazhou1 Oct 2, 2024
11a9740
add more management pause tests
aazhou1 Oct 2, 2024
92f348e
add setting discount rate adapter test
aazhou1 Oct 2, 2024
3498f74
Merge branch 'master' into test-fixes
aazhou1 Oct 4, 2024
d194fe8
fix edit offer unit tests
aazhou1 Oct 4, 2024
e56aba1
add coverage for liquid ratio view function
aazhou1 Oct 7, 2024
2639471
add more unit test coverage
aazhou1 Oct 7, 2024
d0491cf
add revert test cases for submit auction offer
aazhou1 Oct 7, 2024
f985eb1
Merge branch 'master' into test-fixes
aazhou1 Oct 8, 2024
af6bfed
test coverage for sellRepoToken and simulateTransaction
aazhou1 Oct 8, 2024
f081366
additional library test coverage
aazhou1 Oct 9, 2024
35e910a
Merge branch 'master' into test-fixes
aazhou1 Oct 9, 2024
94cf5b9
Merge branch 'master' into test-fixes
aazhou1 Oct 10, 2024
aee7b5c
Merge branch 'master' into test-fixes
aazhou1 Oct 10, 2024
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: 14 additions & 0 deletions src/interfaces/term/ITermAuctionOfferLocker.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,20 @@ interface ITermAuctionOfferLocker {
bool isRevealed;
}

/// @dev TermAuctionRevealedOffer represents a revealed offer to offeror an amount of money for a specific interest rate
struct TermAuctionRevealedOffer {
/// @dev Unique identifier for this bid
bytes32 id;
/// @dev The address of the offeror
address offeror;
/// @dev The offered price as a percentage of the initial loaned amount vs amount returned at maturity. This stores 9 decimal places
uint256 offerPriceRevealed;
/// @dev The maximum amount of purchase tokens offered
uint256 amount;
/// @dev The address of the lent ERC20 token
address purchaseToken;
}

function termRepoId() external view returns (bytes32);

function termAuctionId() external view returns (bytes32);
Expand Down
96 changes: 96 additions & 0 deletions src/test/TestUSDCIntegration.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ import {MockUSDC} from "./mocks/MockUSDC.sol";
import {Setup, ERC20, IStrategyInterface} from "./utils/Setup.sol";
import {Strategy} from "../Strategy.sol";

import {IERC4626} from "@openzeppelin/contracts/interfaces/IERC4626.sol";
import {TermDiscountRateAdapter} from "../TermDiscountRateAdapter.sol";


contract TestUSDCIntegration is Setup {
uint256 internal constant TEST_REPO_TOKEN_RATE = 0.05e18;
uint256 public constant THREESIXTY_DAYCOUNT_SECONDS = 360 days;
Expand Down Expand Up @@ -150,6 +154,98 @@ contract TestUSDCIntegration is Setup {
assertTrue(offers[0] == offerId1 ? address(repoToken1WeekAuction) <= address(repoToken1MonthAuction) : address(repoToken1MonthAuction) <= address(repoToken1WeekAuction));
}

function testRemovingMaturedTokensWithRedemptionAttempt() public {
address testUser = vm.addr(0x11111);
mockUSDC.mint(testUser, 1e18);
repoToken1Month.mint(testUser, 1000e18);

vm.startPrank(testUser);
mockUSDC.approve(address(mockYearnVault), type(uint256).max);
mockYearnVault.deposit(1e18, testUser);
repoToken1Month.approve(address(strategy), type(uint256).max);
termStrategy.sellRepoToken(address(repoToken1Month), 1e6);
vm.stopPrank();

address[] memory holdings = termStrategy.repoTokenHoldings();
assertEq(holdings.length, 1);


vm.warp(block.timestamp + 5 weeks);
termStrategy.auctionClosed();

holdings = termStrategy.repoTokenHoldings();
assertEq(holdings.length, 0);
assertEq(repoToken1Month.balanceOf(address(strategy)), 0);
}
Comment on lines +191 to +213
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add redemption attempt to match the test function name

The test function testRemovingMaturedTokensWithRedemptionAttempt does not include an attempt to redeem the matured tokens, despite the implication in its name. Consider adding logic to attempt redemption and verify the outcome to fully cover the intended scenario.

Apply this diff to include the redemption attempt:

     assertEq(holdings.length, 0);
+    // Attempt to redeem the matured tokens
+    uint256 redeemedAmount = termStrategy.redeemMaturedTokens();
+    // Verify the redeemed amount is as expected
+    assertEq(redeemedAmount, expectedRedeemedAmount);
+    // Ensure the repo token balance is zero after redemption
+    assertEq(repoToken1Month.balanceOf(address(strategy)), 0);
 }

Replace expectedRedeemedAmount with the expected amount after redemption.

Committable suggestion was skipped due to low confidence.


function testRepoTokenBlacklist() public {
address testUser = vm.addr(0x11111);
vm.prank(testUser);
vm.expectRevert("!management");
termStrategy.setRepoTokenBlacklist(address(repoToken1Week), true);
vm.stopPrank();

vm.prank(management);
termStrategy.setRepoTokenBlacklist(address(repoToken1Week), true);
vm.stopPrank();

vm.prank(testUser);
vm.expectRevert(abi.encodeWithSelector(Strategy.RepoTokenBlacklisted.selector, address(repoToken1Week)));
termStrategy.sellRepoToken(address(repoToken1Week), 1e6);
}

function testPauses() public {
address testUser = vm.addr(0x11111);
mockUSDC.mint(testUser, 1e18);
vm.prank(testUser);
vm.expectRevert("!management");
termStrategy.pauseDeposit();
vm.expectRevert("!management");
termStrategy.unpauseDeposit();
vm.stopPrank();

vm.prank(management);
termStrategy.pauseDeposit();
vm.stopPrank();

vm.prank(testUser);
mockUSDC.approve(address(termStrategy), 1e6);

vm.prank(testUser);
vm.expectRevert(abi.encodeWithSelector(Strategy.DepositPaused.selector));
IERC4626(address(termStrategy)).deposit(1e6, testUser);
vm.stopPrank();

vm.prank(management);
termStrategy.unpauseDeposit();
vm.stopPrank();

vm.prank(testUser);
IERC4626(address(termStrategy)).deposit(1e6, testUser);
vm.stopPrank();
}

function testSetDiscountRateAdapter() public {
address testUser = vm.addr(0x11111);

TermDiscountRateAdapter invalid = new TermDiscountRateAdapter(address(0), adminWallet);
TermDiscountRateAdapter valid = new TermDiscountRateAdapter(address(termController), adminWallet);

vm.prank(testUser);
vm.expectRevert("!management");
termStrategy.setDiscountRateAdapter(address(valid));

vm.prank(management);
vm.expectRevert();
termStrategy.setDiscountRateAdapter(address(invalid));
Comment on lines +361 to +362
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Specify expected revert reason in vm.expectRevert()

In the test testSetDiscountRateAdapter, the vm.expectRevert() call before setting an invalid adapter address does not specify the expected revert reason or selector. For more precise testing, it's recommended to include the expected error message or error selector.

Apply this diff to specify the expected revert reason:

 vm.prank(management);
-vm.expectRevert();
+vm.expectRevert("InvalidTermAdapter(address)");
 termStrategy.setDiscountRateAdapter(address(invalid));

Replace "InvalidTermAdapter(address)" with the actual revert reason emitted by the setDiscountRateAdapter function when provided with an invalid address.

Committable suggestion was skipped due to low confidence.


vm.prank(management);
termStrategy.setDiscountRateAdapter(address(valid));
vm.stopPrank();

assertEq(address(valid), address(termStrategy.discountRateAdapter()));
}

function _getRepoTokenAmountGivenPurchaseTokenAmount(
uint256 purchaseTokenAmount,
MockTermRepoToken termRepoToken,
Expand Down
66 changes: 61 additions & 5 deletions src/test/TestUSDCOffers.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,9 @@ contract TestUSDCSubmitOffer is Setup {
assertEq(termStrategy.totalLiquidBalance(), initialState.totalLiquidBalance - 1e6);
// test: totalAssetValue = total liquid balance + pending offer amount
assertEq(termStrategy.totalAssetValue(), termStrategy.totalLiquidBalance() + 1e6);

uint256 repoTokenHoldingValue = termStrategy.getRepoTokenHoldingValue(address(repoToken1Week));
assertEq(repoTokenHoldingValue, 1e6);
}

function testEditOffer() public {
Expand All @@ -88,7 +91,8 @@ contract TestUSDCSubmitOffer is Setup {
uint256 offerDiff = offerAmount - 1e6;

assertEq(termStrategy.totalLiquidBalance(), initialState.totalLiquidBalance - offerDiff);
// test: totalAssetValue = total liquid balance + pending offer amount
uint256 repoTokenHoldingValue = termStrategy.getRepoTokenHoldingValue(address(repoToken1Week));
assertEq(repoTokenHoldingValue, 4e6);
assertEq(termStrategy.totalAssetValue(), termStrategy.totalLiquidBalance() + offerDiff);
}

Expand All @@ -107,6 +111,8 @@ contract TestUSDCSubmitOffer is Setup {

assertEq(termStrategy.totalLiquidBalance(), initialState.totalLiquidBalance);
assertEq(termStrategy.totalAssetValue(), termStrategy.totalLiquidBalance());
uint256 repoTokenHoldingValue = termStrategy.getRepoTokenHoldingValue(address(repoToken1Week));
assertEq(repoTokenHoldingValue, 0);
}

uint256 public constant THREESIXTY_DAYCOUNT_SECONDS = 360 days;
Expand Down Expand Up @@ -147,6 +153,9 @@ contract TestUSDCSubmitOffer is Setup {

repoToken1WeekAuction.auctionSuccess(offerIds, fillAmounts, repoTokenAmounts);

uint256 repoTokenHoldingValue = termStrategy.getRepoTokenHoldingValue(address(repoToken1Week));
assertEq(repoTokenHoldingValue, 1e6);

//console2.log("repoTokenAmounts[0]", repoTokenAmounts[0]);

// test: asset value should equal to initial asset value (liquid + pending offers)
Expand All @@ -159,6 +168,9 @@ contract TestUSDCSubmitOffer is Setup {

termStrategy.auctionClosed();

repoTokenHoldingValue = termStrategy.getRepoTokenHoldingValue(address(repoToken1Week));
assertEq(repoTokenHoldingValue, 1e6);

// test: asset value should equal to initial asset value (liquid + repo tokens)
assertEq(termStrategy.totalAssetValue(), initialState.totalAssetValue);

Expand Down Expand Up @@ -188,8 +200,14 @@ contract TestUSDCSubmitOffer is Setup {
fillAmount, repoToken1Week, TEST_REPO_TOKEN_RATE
);

uint256 repoTokenHoldingValue = termStrategy.getRepoTokenHoldingValue(address(repoToken1Week));
assertEq(repoTokenHoldingValue, 1e6);

repoToken1WeekAuction.auctionSuccess(offerIds, fillAmounts, repoTokenAmounts);

repoTokenHoldingValue = termStrategy.getRepoTokenHoldingValue(address(repoToken1Week));
assertEq(repoTokenHoldingValue, 0.5e6);

// test: asset value should equal to initial asset value (liquid + pending offers)
assertEq(termStrategy.totalAssetValue(), initialState.totalAssetValue);

Expand All @@ -200,6 +218,9 @@ contract TestUSDCSubmitOffer is Setup {

termStrategy.auctionClosed();

repoTokenHoldingValue = termStrategy.getRepoTokenHoldingValue(address(repoToken1Week));
assertEq(repoTokenHoldingValue, 0.5e6);

// test: asset value should equal to initial asset value (liquid + repo tokens)
assertEq(termStrategy.totalAssetValue(), initialState.totalAssetValue);

Expand All @@ -214,14 +235,49 @@ contract TestUSDCSubmitOffer is Setup {
assertEq(offers.length, 0);
}

function testCompleteAuctionCanceled() public {
function testAuctionCancelForWithdrawal() public {
bytes32 offerId1 = _submitOffer(bytes32("offer id hash 1"), 1e6);

uint256 repoTokenHoldingValue = termStrategy.getRepoTokenHoldingValue(address(repoToken1Week));
assertEq(repoTokenHoldingValue, 1e6);

repoToken1WeekAuction.auctionCancelForWithdrawal();

repoTokenHoldingValue = termStrategy.getRepoTokenHoldingValue(address(repoToken1Week));
assertEq(repoTokenHoldingValue, 1e6);


// test: check value before calling complete auction
termStrategy.auctionClosed();

repoTokenHoldingValue = termStrategy.getRepoTokenHoldingValue(address(repoToken1Week));
assertEq(repoTokenHoldingValue, 0);

bytes32[] memory offers = termStrategy.pendingOffers();

assertEq(offers.length, 0);
}

function testAuctionCancel() public {
bytes32 offerId1 = _submitOffer(bytes32("offer id hash 1"), 1e6);

repoToken1WeekAuction.auctionCanceled();
uint256 repoTokenHoldingValue = termStrategy.getRepoTokenHoldingValue(address(repoToken1Week));
assertEq(repoTokenHoldingValue, 1e6);

bytes32[] memory offerIds = new bytes32[](1);
offerIds[0] = offerId1;

repoToken1WeekAuction.auctionCancel(offerIds);

repoTokenHoldingValue = termStrategy.getRepoTokenHoldingValue(address(repoToken1Week));
assertEq(repoTokenHoldingValue, 0);

// test: check value before calling complete auction
termStrategy.auctionClosed();

repoTokenHoldingValue = termStrategy.getRepoTokenHoldingValue(address(repoToken1Week));
assertEq(repoTokenHoldingValue, 0);

bytes32[] memory offers = termStrategy.pendingOffers();

assertEq(offers.length, 0);
Expand All @@ -238,8 +294,8 @@ contract TestUSDCSubmitOffer is Setup {
bytes32[] memory offers = termStrategy.pendingOffers();

assertEq(offers.length, 2);
assertEq(offers[0], offerId2);
assertEq(offers[1], offerId1);
assertEq(offers[0], offerId1);
assertEq(offers[1], offerId2);
}

function testMultipleOffersFillAndNoFill() public {
Expand Down
14 changes: 14 additions & 0 deletions src/test/TestUSDCSellRepoToken.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,10 @@ contract TestUSDCSellRepoToken is Setup {
address(repoToken1Week), 0.05e18, repoTokenSellAmount
);

uint256 repoTokenHoldingValue = termStrategy.getRepoTokenHoldingValue(address(repoToken1Week));

assertEq(repoTokenHoldingValue, expectedProceeds);

assertEq(mockUSDC.balanceOf(testUser), expectedProceeds);
assertEq(termStrategy.totalLiquidBalance(), initialState.totalLiquidBalance - expectedProceeds);
assertEq(termStrategy.totalAssetValue(), initialState.totalAssetValue);
Expand Down Expand Up @@ -552,6 +556,10 @@ contract TestUSDCSellRepoToken is Setup {

termController.setOracleRate(repoToken2Week.termRepoId(), 0.05e18);

vm.expectRevert(abi.encodeWithSelector(RepoTokenList.InvalidRepoToken.selector, address(0)));
termStrategy.getRepoTokenConcentrationRatio(address(0));


uint256 concentrationLimit = termStrategy.getRepoTokenConcentrationRatio(address(repoToken2Week));

_sell1RepoTokenNoMintExpectRevert(
Expand Down Expand Up @@ -584,5 +592,11 @@ contract TestUSDCSellRepoToken is Setup {
2e18,
"Pausable: paused"
);

vm.prank(management);
termStrategy.unpauseStrategy();
vm.prank(testDepositor);
IERC4626(address(termStrategy)).deposit(depositAmount, testDepositor);
vm.stopPrank();
}
}
8 changes: 4 additions & 4 deletions src/test/TestUSDCSubmitOffer.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,10 +235,10 @@ contract TestUSDCSubmitOffer is Setup {
assertEq(offers.length, 0);
}

function testCompleteAuctionCanceled() public {
function testAuctionCancelForWithdrawal() public {
bytes32 offerId1 = _submitOffer(bytes32("offer id hash 1"), 1e6);

repoToken1WeekAuction.auctionCanceled();
repoToken1WeekAuction.auctionCancelForWithdrawal();

// test: check value before calling complete auction
termStrategy.auctionClosed();
Expand All @@ -259,8 +259,8 @@ contract TestUSDCSubmitOffer is Setup {
bytes32[] memory offers = termStrategy.pendingOffers();

assertEq(offers.length, 2);
assertEq(offers[0], offerId2);
assertEq(offers[1], offerId1);
assertEq(offers[0], offerId1);
assertEq(offers[1], offerId2);
}

function testMultipleOffersFillAndNoFill() public {
Expand Down
12 changes: 12 additions & 0 deletions src/test/mocks/MockFeesFactory.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// SPDX-License-Identifier: AGPL-3.0
pragma solidity ^0.8.18;

import {MockFactory} from "@tokenized-strategy/test/mocks/MockFactory.sol";

contract MockFeesFactory is MockFactory {
address public governance;

constructor(uint16 bps, address treasury) MockFactory(bps, treasury) {
governance = msg.sender;
}
Comment on lines +9 to +11
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider adding parameter validation and review governance initialization.

The constructor implementation is generally correct, but consider the following suggestions:

  1. Add input validation for the bps and treasury parameters to ensure they meet any required constraints.
  2. Review if msg.sender is always the intended governance address. If not, consider passing the governance address as a parameter.

Here's a potential refactor to address these points:

constructor(uint16 bps, address treasury, address _governance) MockFactory(bps, treasury) {
    require(bps > 0 && bps <= 10000, "Invalid bps value");
    require(treasury != address(0), "Invalid treasury address");
    require(_governance != address(0), "Invalid governance address");
    governance = _governance;
}

This refactor adds basic validation and allows for explicit setting of the governance address.

}
20 changes: 19 additions & 1 deletion src/test/mocks/MockTermAuction.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
pragma solidity ^0.8.18;

import {ITermAuction} from "../../interfaces/term/ITermAuction.sol";
import {ITermAuctionOfferLocker} from "../../interfaces/term/ITermAuctionOfferLocker.sol";
import {ITermRepoToken} from "../../interfaces/term/ITermRepoToken.sol";
import {ITermRepoServicer} from "../../interfaces/term/ITermRepoServicer.sol";
import {MockTermAuctionOfferLocker} from "./MockTermAuctionOfferLocker.sol";
Expand Down Expand Up @@ -45,7 +46,24 @@ contract MockTermAuction is ITermAuction {
}
}

function auctionCanceled() external {
function auctionCancelForWithdrawal() external {
auctionCancelledForWithdrawal = true;
}
Comment on lines +49 to 51
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add access control to auctionCancelForWithdrawal

The auctionCancelForWithdrawal function is marked as external and can be called by any address. If this function is intended to be restricted to authorized entities (e.g., contract owner or administrator), consider adding appropriate access control modifiers to prevent unauthorized access.


function auctionCancel(bytes32[] calldata offerIds) external {

uint256 i = 0;
// Return revealed offer funds.
for (i = 0; i < offerIds.length; ++i) {
ITermAuctionOfferLocker.TermAuctionOffer memory offer = MockTermAuctionOfferLocker(termAuctionOfferLocker).lockedOffer(offerIds[i]);

MockTermAuctionOfferLocker(termAuctionOfferLocker).unlockOfferPartial(
offer.id,
offer.offeror,
offer.amount
);
}


}
Comment on lines +53 to +68
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add access control to auctionCancel

The auctionCancel function is declared as external and allows cancellation of offers by accepting an array of offerIds. To prevent unauthorized cancellations, consider adding access control modifiers if this function should be restricted to specific roles.

}
5 changes: 5 additions & 0 deletions src/test/mocks/MockTermAuctionOfferLocker.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,4 +100,9 @@ contract MockTermAuctionOfferLocker is ITermAuctionOfferLocker {
delete lockedOffers[offerId];
}
}

function unlockOfferPartial(bytes32 offerId, address offeror, uint256 amount) external {
delete lockedOffers[offerId];
repoLocker.releasePurchaseTokens(offeror, amount);
}
}
Loading