From 227e4e393300e5935afb5728d91b705d15e87aeb Mon Sep 17 00:00:00 2001 From: Ian Lucas Date: Fri, 25 Oct 2024 12:05:52 -0400 Subject: [PATCH] chore: Utilize TestUsers in more tests. Remove duplicate loadTest code --- .../LiquidContinuousMultiTokenVaultTest.t.sol | 50 ++++++----- .../ERC1155/IMultiTokenVaultTestBase.t.sol | 82 ++++++------------- .../test/token/ERC1155/TestParamSet.t.sol | 18 ++++ ...uidContinuousMultiTokenVaultTestBase.t.sol | 33 +++++--- 4 files changed, 96 insertions(+), 87 deletions(-) diff --git a/packages/contracts/test/src/yield/LiquidContinuousMultiTokenVaultTest.t.sol b/packages/contracts/test/src/yield/LiquidContinuousMultiTokenVaultTest.t.sol index cce5f9b8..979e9105 100644 --- a/packages/contracts/test/src/yield/LiquidContinuousMultiTokenVaultTest.t.sol +++ b/packages/contracts/test/src/yield/LiquidContinuousMultiTokenVaultTest.t.sol @@ -40,12 +40,22 @@ contract LiquidContinuousMultiTokenVaultTest is LiquidContinuousMultiTokenVaultT ); } + // @dev [Oct 25, 2024] Succeeds with: from=1 and to=600 ; Fails with: from=1 and to=650 function test__LiquidContinuousMultiTokenVault__LoadTest() public { vm.skip(true); // load test - should only be run during perf testing + TestParamSet.TestParam[] memory loadTestParams = TestParamSet.toLoadSet(100_000 * _scale, 1, 600); - uint256 principal = 100_000 * _scale; + address carol = makeAddr("carol"); + _transferFromTokenOwner(_asset, carol, 1_000_000_000 * _scale); + (TestParamSet.TestUsers memory depositUsers1, TestParamSet.TestUsers memory redeemUsers1) = + _createTestUsers(carol); - _loadTestVault(_liquidVault, principal, 1, 1_000); // 1,000 works, 1800 too much for the vm + // ------------------- deposits w/ redeems per deposit ------------------- + // NB - test all of the deposits BEFORE redeems. verifies no side-effects from deposits when redeeming. + uint256[] memory sharesAtPeriods = _testDepositOnly(depositUsers1, _liquidVault, loadTestParams); + + // NB - test all of the redeems AFTER deposits. verifies no side-effects from deposits when redeeming. + _testRedeemOnly(redeemUsers1, _liquidVault, loadTestParams, sharesAtPeriods); } function test__LiquidContinuousMultiTokenVault__DepositRedeem() public { @@ -187,9 +197,10 @@ contract LiquidContinuousMultiTokenVaultTest is LiquidContinuousMultiTokenVaultT }); } - TestParamSet.TestUsers memory testUsers = TestParamSet.toSingletonUsers(alice); + (TestParamSet.TestUsers memory depositUsers, TestParamSet.TestUsers memory redeemUsers) = + _createTestUsers(alice); - _testDepositOnly(testUsers, _liquidVault, depositTestParams); + _testDepositOnly(depositUsers, _liquidVault, depositTestParams); // split our deposits into two "batches" of redeems (TestParamSet.TestParam[] memory redeemParams1, TestParamSet.TestParam[] memory redeemParams2) = @@ -199,17 +210,17 @@ contract LiquidContinuousMultiTokenVaultTest is LiquidContinuousMultiTokenVaultT // ------------ requestRedeem #1 ----------- uint256 redeemPeriod1 = 31; - _testRequestRedeemMultiDeposit(testUsers, _liquidVault, redeemParams1, 31); + _testRequestRedeemMultiDeposit(redeemUsers, _liquidVault, redeemParams1, 31); // ------------ requestRedeem #2 ------------ uint256 redeemPeriod2 = 41; - _testRequestRedeemMultiDeposit(testUsers, _liquidVault, redeemParams2, 41); + _testRequestRedeemMultiDeposit(redeemUsers, _liquidVault, redeemParams2, 41); // ------------ redeems ------------ // NB - call the redeem AFTER the multiple requestRedeems. verify multiple requestRedeems work. - _testRedeemAfterRequestRedeemMultiDeposit(testUsers, _liquidVault, redeemParams1, redeemPeriod1); + _testRedeemAfterRequestRedeemMultiDeposit(redeemUsers, _liquidVault, redeemParams1, redeemPeriod1); - _testRedeemAfterRequestRedeemMultiDeposit(testUsers, _liquidVault, redeemParams2, redeemPeriod2); + _testRedeemAfterRequestRedeemMultiDeposit(redeemUsers, _liquidVault, redeemParams2, redeemPeriod2); } function test__LiquidContinuousMultiTokenVault__RedeemMultiPeriodsPartialShares() public { @@ -226,9 +237,9 @@ contract LiquidContinuousMultiTokenVaultTest is LiquidContinuousMultiTokenVaultT }); } - TestParamSet.TestUsers memory testUsers = TestParamSet.toSingletonUsers(alice); + (TestParamSet.TestUsers memory depositUsers, TestParamSet.TestUsers memory redeemUsers) = _createTestUsers(bob); - _testDepositOnly(testUsers, _liquidVault, depositTestParams); + _testDepositOnly(depositUsers, _liquidVault, depositTestParams); uint256 partialShares = 1 * _scale; @@ -237,7 +248,7 @@ contract LiquidContinuousMultiTokenVaultTest is LiquidContinuousMultiTokenVaultT TestParamSet.TestParam[] memory redeemParams1 = depositTestParams._subset(0, 2); redeemParams1[2].principal = partialShares; - _testRequestRedeemMultiDeposit(testUsers, _liquidVault, redeemParams1, redeemPeriod1); + _testRequestRedeemMultiDeposit(redeemUsers, _liquidVault, redeemParams1, redeemPeriod1); // ------------ requestRedeem #2 ------------ uint256 redeemPeriod2 = 50; @@ -245,14 +256,13 @@ contract LiquidContinuousMultiTokenVaultTest is LiquidContinuousMultiTokenVaultT redeemParams2[0].principal = (depositTestParams[2].principal - partialShares); redeemParams2[2].principal = partialShares; - _testRequestRedeemMultiDeposit(testUsers, _liquidVault, redeemParams2, redeemPeriod2); + _testRequestRedeemMultiDeposit(redeemUsers, _liquidVault, redeemParams2, redeemPeriod2); // ------------ redeems ------------ // NB - call the redeem AFTER the multiple requestRedeems. verify multiple requestRedeems work. + _testRedeemAfterRequestRedeemMultiDeposit(redeemUsers, _liquidVault, redeemParams1, redeemPeriod1); - _testRedeemAfterRequestRedeemMultiDeposit(testUsers, _liquidVault, redeemParams1, redeemPeriod1); - - _testRedeemAfterRequestRedeemMultiDeposit(testUsers, _liquidVault, redeemParams2, redeemPeriod2); + _testRedeemAfterRequestRedeemMultiDeposit(redeemUsers, _liquidVault, redeemParams2, redeemPeriod2); } function test__LiquidContinuousMultiTokenVault__WithdrawAssetNotOwnerReverts() public { @@ -282,7 +292,7 @@ contract LiquidContinuousMultiTokenVaultTest is LiquidContinuousMultiTokenVaultT assertEq(50_416_666666, actualReturns, "principal + interest not correct for $50k deposit after 30 days"); } - function test__LiquidContinuousMultiTokenVault__ConvertToAssets() public { + function test__LiquidContinuousMultiTokenVault__TotalAssetsAndConvertToAssets() public { uint256 depositPeriod1 = 5; uint256 depositPeriod2 = depositPeriod1 + 1; uint256 redeemPeriod = 10; @@ -298,9 +308,7 @@ contract LiquidContinuousMultiTokenVaultTest is LiquidContinuousMultiTokenVaultT redeemPeriod: redeemPeriod }); - TestParamSet.TestUsers memory testUsers = TestParamSet.toSingletonUsers(alice); - - uint256[] memory shares = _testDepositOnly(testUsers, _liquidVault, testParams); + uint256[] memory shares = _testDepositOnly(TestParamSet.toSingletonUsers(alice), _liquidVault, testParams); uint256 totalShares = shares[0] + shares[1]; // -------------- deposit period1 -------------- @@ -463,10 +471,8 @@ contract LiquidContinuousMultiTokenVaultTest is LiquidContinuousMultiTokenVaultT redeemPeriod: redeemPeriod }); - TestParamSet.TestUsers memory testUsers = TestParamSet.toSingletonUsers(alice); - // deposit - uint256 shares = _testDepositOnly(testUsers, _liquidVault, testParam); + uint256 shares = _testDepositOnly(TestParamSet.toSingletonUsers(alice), _liquidVault, testParam); // request redeem _warpToPeriod(_liquidVault, redeemPeriod - _liquidVault.noticePeriod()); diff --git a/packages/contracts/test/test/token/ERC1155/IMultiTokenVaultTestBase.t.sol b/packages/contracts/test/test/token/ERC1155/IMultiTokenVaultTestBase.t.sol index abaa1f4f..bca86d3a 100644 --- a/packages/contracts/test/test/token/ERC1155/IMultiTokenVaultTestBase.t.sol +++ b/packages/contracts/test/test/token/ERC1155/IMultiTokenVaultTestBase.t.sol @@ -3,7 +3,6 @@ pragma solidity ^0.8.20; import { IMultiTokenVault } from "@credbull/token/ERC1155/IMultiTokenVault.sol"; import { IERC20 } from "@openzeppelin/contracts/interfaces/IERC20.sol"; -import { IERC20Metadata } from "@openzeppelin/contracts/token/ERC20/extensions/IERC20Metadata.sol"; import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol"; import { Timer } from "@credbull/timelock/Timer.sol"; import { TestParamSet } from "@test/test/token/ERC1155/TestParamSet.t.sol"; @@ -171,20 +170,20 @@ abstract contract IMultiTokenVaultTestBase is Test { /// @dev verify deposit. updates vault assets and shares. function _testDepositOnly( - TestParamSet.TestUsers memory testUsers, + TestParamSet.TestUsers memory depositUsers, IMultiTokenVault vault, TestParamSet.TestParam[] memory testParams ) internal virtual returns (uint256[] memory sharesAtPeriod_) { uint256[] memory sharesAtPeriod = new uint256[](testParams.length); for (uint256 i = 0; i < testParams.length; i++) { - sharesAtPeriod[i] = _testDepositOnly(testUsers, vault, testParams[i]); + sharesAtPeriod[i] = _testDepositOnly(depositUsers, vault, testParams[i]); } return sharesAtPeriod; } /// @dev verify deposit. updates vault assets and shares. function _testDepositOnly( - TestParamSet.TestUsers memory testUsers, + TestParamSet.TestUsers memory depositUsers, IMultiTokenVault vault, TestParamSet.TestParam memory testParam ) internal virtual returns (uint256 actualSharesAtPeriod_) { @@ -192,32 +191,32 @@ abstract contract IMultiTokenVaultTestBase is Test { // capture state before for validations uint256 prevVaultPeriodsElapsed = vault.currentPeriodsElapsed(); - uint256 prevReceiverVaultBalance = vault.sharesAtPeriod(testUsers.tokenReceiver, testParam.depositPeriod); + uint256 prevReceiverVaultBalance = vault.sharesAtPeriod(depositUsers.tokenReceiver, testParam.depositPeriod); // ------------------- deposit ------------------- _warpToPeriod(vault, testParam.depositPeriod); // warp to deposit period assertGe( - asset.balanceOf(testUsers.tokenOwner), + asset.balanceOf(depositUsers.tokenOwner), testParam.principal, _assertMsg("not enough assets for deposit ", vault, testParam.depositPeriod) ); - vm.prank(testUsers.tokenOwner); // tokenOwner here is the owner of the USDC + vm.prank(depositUsers.tokenOwner); // tokenOwner here is the owner of the USDC asset.approve(address(vault), testParam.principal); // grant the vault allowance - vm.prank(testUsers.tokenOwner); // tokenOwner here is the owner of the USDC - uint256 actualSharesAtPeriod = vault.deposit(testParam.principal, testUsers.tokenReceiver); // now deposit + vm.prank(depositUsers.tokenOwner); // tokenOwner here is the owner of the USDC + uint256 actualSharesAtPeriod = vault.deposit(testParam.principal, depositUsers.tokenReceiver); // now deposit assertEq( prevReceiverVaultBalance + actualSharesAtPeriod, - vault.sharesAtPeriod(testUsers.tokenReceiver, testParam.depositPeriod), + vault.sharesAtPeriod(depositUsers.tokenReceiver, testParam.depositPeriod), _assertMsg( "receiver did not receive the correct vault shares - sharesAtPeriod", vault, testParam.depositPeriod ) ); assertEq( prevReceiverVaultBalance + actualSharesAtPeriod, - vault.balanceOf(testUsers.tokenReceiver, testParam.depositPeriod), + vault.balanceOf(depositUsers.tokenReceiver, testParam.depositPeriod), _assertMsg("receiver did not receive the correct vault shares - balanceOf ", vault, testParam.depositPeriod) ); @@ -253,6 +252,7 @@ abstract contract IMultiTokenVaultTestBase is Test { // ------------------- prep redeem ------------------- uint256 assetBalanceBeforeRedeem = asset.balanceOf(redeemUsers.tokenReceiver); + uint256 shareBalanceBeforeRedeem = vault.balanceOf(redeemUsers.tokenOwner, testParam.depositPeriod); uint256 expectedReturns = _expectedReturns(sharesToRedeemAtPeriod, vault, testParam); _transferFromTokenOwner(asset, address(vault), expectedReturns); @@ -281,6 +281,13 @@ abstract contract IMultiTokenVaultTestBase is Test { _assertMsg("assets does not equal principal + yield", vault, testParam.depositPeriod) ); + // verify the token owner shares reduced + assertEq( + shareBalanceBeforeRedeem - sharesToRedeemAtPeriod, + vault.balanceOf(redeemUsers.tokenOwner, testParam.depositPeriod), + _assertMsg("shares not reduced by redeem amount", vault, testParam.depositPeriod) + ); + // verify the receiver has the USDC back assertApproxEqAbs( assetBalanceBeforeRedeem + testParam.principal + expectedReturns, @@ -338,46 +345,7 @@ abstract contract IMultiTokenVaultTestBase is Test { return assets; } - /// @dev performance / load test harness to execute a number of deposits first, and then redeem after - function _loadTestVault(IMultiTokenVault vault, uint256 principal, uint256 fromPeriod, uint256 toPeriod) internal { - address charlie = makeAddr("charlie"); - address david = makeAddr("david"); - - IERC20Metadata _asset = IERC20Metadata(vault.asset()); - uint256 _scale = 10 ** _asset.decimals(); - - _transferFromTokenOwner(_asset, charlie, 1_000_000_000 * _scale); - _transferFromTokenOwner(_asset, david, 1_000_000_000 * _scale); - - // "wastes" storage from 0 -> fromPeriod. but fine in test, and makes the depositPeriod clear - uint256[] memory charlieShares = new uint256[](toPeriod + 1); - uint256[] memory davidShares = new uint256[](toPeriod + 1); - - TestParamSet.TestUsers memory charlieTestUsers = TestParamSet.toSingletonUsers(charlie); - TestParamSet.TestUsers memory davidTestUsers = TestParamSet.toSingletonUsers(david); - - // ----------------------- deposits ----------------------- - for (uint256 i = fromPeriod; i < toPeriod; ++i) { - TestParamSet.TestParam memory depositTestParam = TestParamSet.TestParam({ - principal: principal, - depositPeriod: i, - redeemPeriod: 0 // not used in deposit flow - }); - charlieShares[i] = _testDepositOnly(charlieTestUsers, vault, depositTestParam); - davidShares[i] = _testDepositOnly(davidTestUsers, vault, depositTestParam); - } - - // ----------------------- redeems ----------------------- - for (uint256 i = fromPeriod; i < toPeriod; ++i) { - TestParamSet.TestParam memory redeemTestParam = - TestParamSet.TestParam({ principal: principal, depositPeriod: i, redeemPeriod: toPeriod }); - - _testRedeemOnly(charlieTestUsers, vault, redeemTestParam, charlieShares[i]); - _testRedeemOnly(davidTestUsers, vault, redeemTestParam, davidShares[i]); - } - } - - /// @dev /// @dev execute a redeem on the vault across multiple deposit periods. (if supported) + /// @dev vault redeem across multiple deposit periods. (if supported) function _vaultRedeemBatch( TestParamSet.TestUsers memory redeemUsers, IMultiTokenVault vault, @@ -392,7 +360,7 @@ abstract contract IMultiTokenVaultTestBase is Test { uint256 assets = 0; vm.startPrank(redeemUsers.tokenOperator); - // @dev - IMultiTokenVault we don't support redeeming across deposit periods. redeem period by period instead. + // IMultiTokenVault doesn't support redeeming across deposit periods. redeem period by period instead. for (uint256 i = 0; i < depositTestParams.length; ++i) { uint256 depositPeriod = depositTestParams[i].depositPeriod; uint256 sharesAtPeriod = @@ -437,18 +405,22 @@ abstract contract IMultiTokenVaultTestBase is Test { // simple scenario with only one user function _createTestUsers(address account) internal + virtual returns (TestParamSet.TestUsers memory depositUsers_, TestParamSet.TestUsers memory redeemUsers_) { + // Convert the address to a string and then to bytes + string memory accountStr = vm.toString(account); + TestParamSet.TestUsers memory depositUsers = TestParamSet.TestUsers({ tokenOwner: account, // owns tokens, can specify who can receive tokens - tokenReceiver: makeAddr("depositTokenReceiver"), // receiver of tokens from the tokenOwner - tokenOperator: makeAddr("depositTokenOperator") // granted allowance by tokenOwner to act on their behalf + tokenReceiver: makeAddr(string.concat("depositTokenReceiver-", accountStr)), // receiver of tokens from the tokenOwner + tokenOperator: makeAddr(string.concat("depositTokenOperator-", accountStr)) // granted allowance by tokenOwner to act on their behalf }); TestParamSet.TestUsers memory redeemUsers = TestParamSet.TestUsers({ tokenOwner: depositUsers.tokenReceiver, // on deposit, the tokenReceiver receives (owns) the tokens tokenReceiver: account, // virtuous cycle, the account receives the returns in the end - tokenOperator: makeAddr("redeemTokenOperator") // granted allowance by tokenOwner to act on their behalf + tokenOperator: makeAddr(string.concat("redeemTokenOperator-", accountStr)) // granted allowance by tokenOwner to act on their behalf }); return (depositUsers, redeemUsers); diff --git a/packages/contracts/test/test/token/ERC1155/TestParamSet.t.sol b/packages/contracts/test/test/token/ERC1155/TestParamSet.t.sol index 2dced5df..b6f4054d 100644 --- a/packages/contracts/test/test/token/ERC1155/TestParamSet.t.sol +++ b/packages/contracts/test/test/token/ERC1155/TestParamSet.t.sol @@ -44,6 +44,24 @@ library TestParamSet { return testParamsWithOffsets; } + // Generate and add multiple testParams with offsets + function toLoadSet(uint256 principal, uint256 fromPeriod, uint256 toPeriod) + internal + pure + returns (TestParam[] memory loadTestParams_) + { + TestParam[] memory loadTestParams = new TestParam[](toPeriod - fromPeriod); + + uint256 arrayIndex = 0; + for (uint256 i = fromPeriod; i < toPeriod; ++i) { + loadTestParams[arrayIndex] = + TestParamSet.TestParam({ principal: principal, depositPeriod: i, redeemPeriod: toPeriod }); + arrayIndex++; + } + + return loadTestParams; + } + // simple scenario with only one user function toSingletonUsers(address account) internal pure returns (TestUsers memory testUsers_) { TestUsers memory testUsers = TestUsers({ diff --git a/packages/contracts/test/test/yield/LiquidContinuousMultiTokenVaultTestBase.t.sol b/packages/contracts/test/test/yield/LiquidContinuousMultiTokenVaultTestBase.t.sol index c50be69a..f82c5929 100644 --- a/packages/contracts/test/test/yield/LiquidContinuousMultiTokenVaultTestBase.t.sol +++ b/packages/contracts/test/test/yield/LiquidContinuousMultiTokenVaultTestBase.t.sol @@ -180,15 +180,7 @@ abstract contract LiquidContinuousMultiTokenVaultTestBase is IMultiTokenVaultTes ) internal virtual returns (uint256 assets_) { LiquidContinuousMultiTokenVault liquidVault = LiquidContinuousMultiTokenVault(address(vault)); - // in LiquidContinuousMultiTokenVault - the tokenOwner and tokenOperator (aka controller) must be the same - TestParamSet.TestUsers memory redeemUsersOperatorIsOwner = TestParamSet.TestUsers({ - tokenOwner: redeemUsers.tokenOwner, - tokenReceiver: redeemUsers.tokenReceiver, - tokenOperator: redeemUsers.tokenOwner - }); - - uint256 assets = - super._testRedeemMultiDeposit(redeemUsersOperatorIsOwner, vault, depositTestParams, redeemPeriod); + uint256 assets = super._testRedeemMultiDeposit(redeemUsers, vault, depositTestParams, redeemPeriod); // verify the requestRedeems are released (uint256[] memory unlockDepositPeriods, uint256[] memory unlockAmounts) = @@ -215,7 +207,7 @@ abstract contract LiquidContinuousMultiTokenVaultTestBase is IMultiTokenVaultTes return _testRedeemAfterRequestRedeemMultiDeposit(redeemUsers, vault, depositTestParams, redeemPeriod); } - /// @dev execute a redeem on the vault across multiple deposit periods.~ + /// @dev redeem across multiple deposit periods function _vaultRedeemBatch( TestParamSet.TestUsers memory redeemUsers, IMultiTokenVault vault, @@ -263,6 +255,27 @@ abstract contract LiquidContinuousMultiTokenVaultTestBase is IMultiTokenVaultTes vm.warp(warpToTimeInSeconds); } + // simple scenario with only one user + function _createTestUsers(address account) + internal + virtual + override + returns (TestParamSet.TestUsers memory depositUsers_, TestParamSet.TestUsers memory redeemUsers_) + { + (TestParamSet.TestUsers memory depositUsers, TestParamSet.TestUsers memory redeemUsers) = + super._createTestUsers(account); + + // in LiquidContinuousMultiTokenVault - tokenOwner and tokenOperator (aka controller) must be the same + // because IComponentToken.redeem() does not have an `owner` parameter. // TODO - we should add this in with Plume ! + TestParamSet.TestUsers memory redeemUsersOperatorIsOwner = TestParamSet.TestUsers({ + tokenOwner: redeemUsers.tokenOwner, + tokenReceiver: redeemUsers.tokenReceiver, + tokenOperator: redeemUsers.tokenOwner + }); + + return (depositUsers, redeemUsersOperatorIsOwner); + } + function _setPeriod(address operator, LiquidContinuousMultiTokenVault vault, uint256 newPeriod) public { uint256 newPeriodInSeconds = newPeriod * 1 days; uint256 currentTime = Timer.timestamp();