From 84a542c61d1b142bf8f3c681cb5efc68aa61320a Mon Sep 17 00:00:00 2001 From: Alex Towle Date: Tue, 20 Feb 2024 23:03:03 -0600 Subject: [PATCH 1/2] Fixed the bug in `removeLiquidity` when the destination is invoked --- contracts/src/internal/HyperdriveLP.sol | 6 +- .../NegativeInterestLongFeeTest.t.sol | 3 + .../NegativeInterestShortFeeTest.t.sol | 2 + .../hyperdrive/NonstandardDecimals.sol | 1 + .../hyperdrive/ReentrancyTest.t.sol | 5 + test/units/hyperdrive/AddLiquidityTest.t.sol | 29 +++++ test/units/hyperdrive/CloseLongTest.t.sol | 105 ++++++++++----- test/units/hyperdrive/CloseShortTest.t.sol | 111 +++++++++++----- test/units/hyperdrive/FeeTest.t.sol | 4 + test/units/hyperdrive/InitializeTest.t.sol | 29 ++++- test/units/hyperdrive/OpenLongTest.t.sol | 107 +++++++++++----- test/units/hyperdrive/OpenShortTest.t.sol | 121 +++++++++++++----- .../RedeemWithdrawalSharesTest.t.sol | 39 ++++++ .../hyperdrive/RemoveLiquidityTest.t.sol | 50 +++++++- test/utils/HyperdriveTest.sol | 50 +++++--- 15 files changed, 512 insertions(+), 150 deletions(-) diff --git a/contracts/src/internal/HyperdriveLP.sol b/contracts/src/internal/HyperdriveLP.sol index a6757e316..2861c6227 100644 --- a/contracts/src/internal/HyperdriveLP.sol +++ b/contracts/src/internal/HyperdriveLP.sol @@ -291,6 +291,7 @@ abstract contract HyperdriveLP is // Redeem as many of the withdrawal shares as possible. uint256 withdrawalSharesRedeemed; (proceeds, withdrawalSharesRedeemed) = _redeemWithdrawalSharesInternal( + _options.destination, _lpShares, vaultSharePrice, _minOutputPerShare, @@ -346,6 +347,7 @@ abstract contract HyperdriveLP is // Redeem as many of the withdrawal shares as possible. (proceeds, withdrawalSharesRedeemed) = _redeemWithdrawalSharesInternal( + msg.sender, _withdrawalShares, vaultSharePrice, _minOutputPerShare, @@ -372,6 +374,7 @@ abstract contract HyperdriveLP is /// withdrawal pool's proceeds. This function redeems the maximum /// amount of the specified withdrawal shares given the amount of /// withdrawal shares ready to withdraw. + /// @param _source The address that owns the withdrawal shares to redeem. /// @param _withdrawalShares The withdrawal shares to redeem. /// @param _sharePrice The share price. /// @param _minOutputPerShare The minimum amount of base the LP expects to @@ -381,6 +384,7 @@ abstract contract HyperdriveLP is /// @return withdrawalSharesRedeemed The amount of withdrawal shares that /// were redeemed. function _redeemWithdrawalSharesInternal( + address _source, uint256 _withdrawalShares, uint256 _sharePrice, uint256 _minOutputPerShare, @@ -399,7 +403,7 @@ abstract contract HyperdriveLP is // We burn the shares from the user. _burn( AssetId._WITHDRAWAL_SHARE_ASSET_ID, - msg.sender, + _source, withdrawalSharesRedeemed ); diff --git a/test/integrations/hyperdrive/NegativeInterestLongFeeTest.t.sol b/test/integrations/hyperdrive/NegativeInterestLongFeeTest.t.sol index 9d915aa54..f6f996f84 100644 --- a/test/integrations/hyperdrive/NegativeInterestLongFeeTest.t.sol +++ b/test/integrations/hyperdrive/NegativeInterestLongFeeTest.t.sol @@ -152,6 +152,7 @@ contract NegativeInterestLongFeeTest is HyperdriveTest { basePaid, DepositOverrides({ asBase: true, + destination: bob, depositAmount: basePaid, minSharePrice: 0, minSlippage: 0, // TODO: This should never go below the base amount. Investigate this. @@ -353,6 +354,7 @@ contract NegativeInterestLongFeeTest is HyperdriveTest { basePaid, DepositOverrides({ asBase: true, + destination: bob, depositAmount: basePaid, minSharePrice: 0, minSlippage: 0, // TODO: This should never go below the base amount. Investigate this. @@ -558,6 +560,7 @@ contract NegativeInterestLongFeeTest is HyperdriveTest { basePaid, DepositOverrides({ asBase: true, + destination: bob, depositAmount: basePaid, minSharePrice: 0, minSlippage: 0, // TODO: This should never go below the base amount. Investigate this. diff --git a/test/integrations/hyperdrive/NegativeInterestShortFeeTest.t.sol b/test/integrations/hyperdrive/NegativeInterestShortFeeTest.t.sol index 5c7a54a26..1f7bc6761 100644 --- a/test/integrations/hyperdrive/NegativeInterestShortFeeTest.t.sol +++ b/test/integrations/hyperdrive/NegativeInterestShortFeeTest.t.sol @@ -345,6 +345,7 @@ contract NegativeInterestShortFeeTest is HyperdriveTest { shortAmount, DepositOverrides({ asBase: true, + destination: bob, // NOTE: Roughly double deposit amount needed to cover 100% flat fee depositAmount: shortAmount * 2, minSharePrice: 0, @@ -557,6 +558,7 @@ contract NegativeInterestShortFeeTest is HyperdriveTest { shortAmount, DepositOverrides({ asBase: true, + destination: bob, // NOTE: Roughly double deposit amount needed to cover 100% flat fee depositAmount: shortAmount * 2, minSharePrice: 0, diff --git a/test/integrations/hyperdrive/NonstandardDecimals.sol b/test/integrations/hyperdrive/NonstandardDecimals.sol index 0e1c7fdd6..33757592e 100644 --- a/test/integrations/hyperdrive/NonstandardDecimals.sol +++ b/test/integrations/hyperdrive/NonstandardDecimals.sol @@ -523,6 +523,7 @@ contract NonstandardDecimalsTest is HyperdriveTest { // checking for reverts. DepositOverrides memory overrides = DepositOverrides({ asBase: true, + destination: celine, depositAmount: testParams.contribution, minSharePrice: 0, // unused minSlippage: spotAPRBefore - 0.015e18, // min spot rate of .5% diff --git a/test/integrations/hyperdrive/ReentrancyTest.t.sol b/test/integrations/hyperdrive/ReentrancyTest.t.sol index 47fd80f5f..cde310081 100644 --- a/test/integrations/hyperdrive/ReentrancyTest.t.sol +++ b/test/integrations/hyperdrive/ReentrancyTest.t.sol @@ -297,6 +297,7 @@ contract ReentrancyTest is HyperdriveTest { // the ETH receiver will receive a refund. DepositOverrides({ asBase: true, + destination: _trader, depositAmount: CONTRIBUTION + 1, minSharePrice: 0, minSlippage: 0, @@ -326,6 +327,7 @@ contract ReentrancyTest is HyperdriveTest { // the ETH receiver will receive a refund. DepositOverrides({ asBase: true, + destination: _trader, depositAmount: CONTRIBUTION + 1, minSharePrice: 0, minSlippage: 0, @@ -393,6 +395,7 @@ contract ReentrancyTest is HyperdriveTest { // the ETH receiver will receive a refund. DepositOverrides({ asBase: true, + destination: _trader, depositAmount: BASE_PAID + 1, minSharePrice: 0, minSlippage: 0, @@ -433,6 +436,7 @@ contract ReentrancyTest is HyperdriveTest { // ETH receiver will receive a refund. DepositOverrides({ asBase: true, + destination: _trader, // NOTE: Roughly double deposit amount needed to cover 100% flat fee depositAmount: BOND_AMOUNT * 2, minSharePrice: 0, @@ -452,6 +456,7 @@ contract ReentrancyTest is HyperdriveTest { BOND_AMOUNT, DepositOverrides({ asBase: true, + destination: _trader, // NOTE: Roughly double deposit amount needed to cover 100% flat fee depositAmount: BOND_AMOUNT * 2, minSharePrice: 0, diff --git a/test/units/hyperdrive/AddLiquidityTest.t.sol b/test/units/hyperdrive/AddLiquidityTest.t.sol index 7c286f807..1104ed2ae 100644 --- a/test/units/hyperdrive/AddLiquidityTest.t.sol +++ b/test/units/hyperdrive/AddLiquidityTest.t.sol @@ -519,6 +519,35 @@ contract AddLiquidityTest is HyperdriveTest { ); } + function test_add_liquidity_destination() external { + // Initialize the pool with a large amount of capital. + uint256 fixedRate = 0.05e18; + uint256 contribution = 500_000_000e18; + initialize(alice, fixedRate, contribution); + + // Bob adds liquidity and sends the LP shares to Celine. + uint256 lpShares = addLiquidity( + bob, + contribution, + DepositOverrides({ + asBase: true, + destination: celine, + depositAmount: contribution, + minSharePrice: 0, // min lp share price of 0 + minSlippage: 0, // min spot rate of 0 + maxSlippage: type(uint256).max, // max spot rate of uint256 max + extraData: new bytes(0) // unused + }) + ); + + // Ensure that the correct event was emitted. + verifyAddLiquidityEvent(celine, lpShares, contribution); + + // Ensure that Celine received the LP shares. + assertEq(hyperdrive.balanceOf(AssetId._LP_ASSET_ID, bob), 0); + assertEq(hyperdrive.balanceOf(AssetId._LP_ASSET_ID, celine), lpShares); + } + function verifyAddLiquidity( address lp, uint256 contribution diff --git a/test/units/hyperdrive/CloseLongTest.t.sol b/test/units/hyperdrive/CloseLongTest.t.sol index cdd1d0e6c..1b590fd99 100644 --- a/test/units/hyperdrive/CloseLongTest.t.sol +++ b/test/units/hyperdrive/CloseLongTest.t.sol @@ -880,6 +880,37 @@ contract CloseLongTest is HyperdriveTest { assertGt(maxFlatFeeState.shareReserves, zeroFeeState.shareReserves); } + function test_close_long_destination() external { + // Initialize the pool with a large amount of capital. + uint256 fixedRate = 0.05e18; + uint256 contribution = 500_000_000e18; + initialize(alice, fixedRate, contribution); + + // Bob opens a long. + uint256 basePaid = 1_000_000e18; + (uint256 maturityTime, uint256 bondAmount) = openLong(bob, basePaid); + + // Bob closes his long and sends the proceeds to Celine. + uint256 baseProceeds = closeLong( + bob, + maturityTime, + bondAmount, + WithdrawalOverrides({ + asBase: true, + destination: celine, + minSlippage: 0, + extraData: new bytes(0) + }) + ); + + // Ensure that the correct event was emitted. + verifyCloseLongEvent(celine, maturityTime, bondAmount, baseProceeds); + + // Ensure that the proceeds were sent to Celine. + assertEq(baseToken.balanceOf(bob), 0); + assertEq(baseToken.balanceOf(celine), baseProceeds); + } + struct TestCase { IHyperdrive.PoolInfo poolInfoBefore; uint256 traderBaseBalanceBefore; @@ -893,41 +924,12 @@ contract CloseLongTest is HyperdriveTest { function verifyCloseLong(TestCase memory testCase) internal { // Ensure that one `CloseLong` event was emitted with the correct // arguments. - { - VmSafe.Log[] memory logs = vm.getRecordedLogs().filterLogs( - CloseLong.selector - ); - assertEq(logs.length, 1); - VmSafe.Log memory log = logs[0]; - assertEq(address(uint160(uint256(log.topics[1]))), bob); - assertEq( - uint256(log.topics[2]), - AssetId.encodeAssetId( - AssetId.AssetIdPrefix.Long, - testCase.maturityTime - ) - ); - ( - uint256 eventMaturityTime, - uint256 eventBaseAmount, - uint256 eventVaultShareAmount, - bool eventAsBase, - uint256 eventBondAmount - ) = abi.decode( - log.data, - (uint256, uint256, uint256, bool, uint256) - ); - assertEq(eventMaturityTime, testCase.maturityTime); - assertEq(eventBaseAmount, testCase.baseProceeds); - assertEq( - eventVaultShareAmount, - testCase.baseProceeds.divDown( - hyperdrive.getPoolInfo().vaultSharePrice - ) - ); - assertEq(eventAsBase, true); - assertEq(eventBondAmount, testCase.bondAmount); - } + verifyCloseLongEvent( + bob, + testCase.bondAmount, + testCase.baseProceeds, + testCase.maturityTime + ); // Ensure that the correct amount of base was transferred. assertEq( @@ -1044,4 +1046,37 @@ contract CloseLongTest is HyperdriveTest { ); assertEq(poolInfoAfter.shortAverageMaturityTime, 0); } + + function verifyCloseLongEvent( + address destination, + uint256 maturityTime, + uint256 bondAmount, + uint256 baseProceeds + ) internal { + VmSafe.Log[] memory logs = vm.getRecordedLogs().filterLogs( + CloseLong.selector + ); + assertEq(logs.length, 1); + VmSafe.Log memory log = logs[0]; + assertEq(address(uint160(uint256(log.topics[1]))), destination); + assertEq( + uint256(log.topics[2]), + AssetId.encodeAssetId(AssetId.AssetIdPrefix.Long, maturityTime) + ); + ( + uint256 eventMaturityTime, + uint256 eventBaseAmount, + uint256 eventVaultShareAmount, + bool eventAsBase, + uint256 eventBondAmount + ) = abi.decode(log.data, (uint256, uint256, uint256, bool, uint256)); + assertEq(eventMaturityTime, maturityTime); + assertEq(eventBaseAmount, baseProceeds); + assertEq( + eventVaultShareAmount, + baseProceeds.divDown(hyperdrive.getPoolInfo().vaultSharePrice) + ); + assertEq(eventAsBase, true); + assertEq(eventBondAmount, bondAmount); + } } diff --git a/test/units/hyperdrive/CloseShortTest.t.sol b/test/units/hyperdrive/CloseShortTest.t.sol index 175354783..03d2f302f 100644 --- a/test/units/hyperdrive/CloseShortTest.t.sol +++ b/test/units/hyperdrive/CloseShortTest.t.sol @@ -607,6 +607,7 @@ contract CloseShortTest is HyperdriveTest { 10e18, DepositOverrides({ asBase: false, + destination: bob, // NOTE: Roughly double deposit amount needed to cover 100% flat fee depositAmount: 10e18 * 2, minSharePrice: 0, @@ -643,6 +644,7 @@ contract CloseShortTest is HyperdriveTest { 10e18, DepositOverrides({ asBase: false, + destination: bob, // NOTE: Roughly double deposit amount needed to cover 100% flat fee depositAmount: 10e18 * 2, minSharePrice: 0, @@ -691,6 +693,7 @@ contract CloseShortTest is HyperdriveTest { 10e18, DepositOverrides({ asBase: false, + destination: bob, // NOTE: Roughly double deposit amount needed to cover 100% flat fee depositAmount: 10e18 * 2, minSharePrice: 0, @@ -726,6 +729,7 @@ contract CloseShortTest is HyperdriveTest { 10e18, DepositOverrides({ asBase: false, + destination: bob, // NOTE: Roughly double deposit amount needed to cover 100% flat fee depositAmount: 10e18 * 2, minSharePrice: 0, @@ -849,6 +853,37 @@ contract CloseShortTest is HyperdriveTest { assertGe(shortProceeds1, shortProceeds4); } + function test_close_short_destination() external { + // Initialize the pool with a large amount of capital. + uint256 fixedRate = 0.05e18; + uint256 contribution = 500_000_000e18; + initialize(alice, fixedRate, contribution); + + // Bob opens a short. + uint256 shortAmount = 1_000_000e18; + (uint256 maturityTime, ) = openShort(bob, shortAmount); + + // Bob closes his short and sends the proceeds to Celine. + uint256 baseProceeds = closeShort( + bob, + maturityTime, + shortAmount, + WithdrawalOverrides({ + asBase: true, + destination: celine, + minSlippage: 0, + extraData: new bytes(0) + }) + ); + + // Ensure that the correct event was emitted. + verifyCloseShortEvent(celine, maturityTime, shortAmount, baseProceeds); + + // Ensure that the proceeds were sent to Celine. + assertEq(baseToken.balanceOf(bob), 0); + assertEq(baseToken.balanceOf(celine), baseProceeds); + } + struct TestCase { IHyperdrive.PoolInfo poolInfoBefore; uint256 bobBaseBalanceBefore; @@ -862,41 +897,12 @@ contract CloseShortTest is HyperdriveTest { function verifyCloseShort(TestCase memory testCase) internal { // Ensure that one `CloseShort` event was emitted with the correct // arguments. - { - VmSafe.Log[] memory logs = vm.getRecordedLogs().filterLogs( - CloseShort.selector - ); - assertEq(logs.length, 1); - VmSafe.Log memory log = logs[0]; - assertEq(address(uint160(uint256(log.topics[1]))), bob); - assertEq( - uint256(log.topics[2]), - AssetId.encodeAssetId( - AssetId.AssetIdPrefix.Short, - testCase.maturityTime - ) - ); - ( - uint256 eventMaturityTime, - uint256 eventBaseAmount, - uint256 eventVaultShareAmount, - bool eventAsBase, - uint256 eventBondAmount - ) = abi.decode( - log.data, - (uint256, uint256, uint256, bool, uint256) - ); - assertEq(eventMaturityTime, testCase.maturityTime); - assertEq(eventBaseAmount, testCase.baseProceeds); - assertEq( - eventVaultShareAmount, - testCase.baseProceeds.divDown( - hyperdrive.getPoolInfo().vaultSharePrice - ) - ); - assertEq(eventAsBase, true); - assertEq(eventBondAmount, testCase.bondAmount); - } + verifyCloseShortEvent( + bob, + testCase.maturityTime, + testCase.bondAmount, + testCase.baseProceeds + ); // Ensure that the correct amount of base was transferred from // Hyperdrive to Bob. @@ -1035,4 +1041,39 @@ contract CloseShortTest is HyperdriveTest { assertEq(poolInfoAfter.longAverageMaturityTime, 0); assertEq(poolInfoAfter.shortAverageMaturityTime, 0); } + + function verifyCloseShortEvent( + address destination, + uint256 maturityTime, + uint256 bondAmount, + uint256 baseProceeds + ) internal { + // Ensure that one `CloseShort` event was emitted with the correct + // arguments. + VmSafe.Log[] memory logs = vm.getRecordedLogs().filterLogs( + CloseShort.selector + ); + assertEq(logs.length, 1); + VmSafe.Log memory log = logs[0]; + assertEq(address(uint160(uint256(log.topics[1]))), destination); + assertEq( + uint256(log.topics[2]), + AssetId.encodeAssetId(AssetId.AssetIdPrefix.Short, maturityTime) + ); + ( + uint256 eventMaturityTime, + uint256 eventBaseAmount, + uint256 eventVaultShareAmount, + bool eventAsBase, + uint256 eventBondAmount + ) = abi.decode(log.data, (uint256, uint256, uint256, bool, uint256)); + assertEq(eventMaturityTime, maturityTime); + assertEq(eventBaseAmount, baseProceeds); + assertEq( + eventVaultShareAmount, + baseProceeds.divDown(hyperdrive.getPoolInfo().vaultSharePrice) + ); + assertEq(eventAsBase, true); + assertEq(eventBondAmount, bondAmount); + } } diff --git a/test/units/hyperdrive/FeeTest.t.sol b/test/units/hyperdrive/FeeTest.t.sol index 2624443db..efebd1bb1 100644 --- a/test/units/hyperdrive/FeeTest.t.sol +++ b/test/units/hyperdrive/FeeTest.t.sol @@ -195,6 +195,7 @@ contract FeeTest is HyperdriveTest { basePaid, DepositOverrides({ asBase: true, + destination: bob, depositAmount: basePaid, minSharePrice: 0, minSlippage: 0, @@ -238,6 +239,7 @@ contract FeeTest is HyperdriveTest { basePaid, DepositOverrides({ asBase: true, + destination: bob, depositAmount: basePaid, minSharePrice: 0, minSlippage: 0, @@ -306,6 +308,7 @@ contract FeeTest is HyperdriveTest { basePaid, DepositOverrides({ asBase: true, + destination: bob, depositAmount: basePaid, minSharePrice: 0, minSlippage: 0, @@ -369,6 +372,7 @@ contract FeeTest is HyperdriveTest { basePaid, DepositOverrides({ asBase: true, + destination: bob, depositAmount: basePaid, minSharePrice: 0, minSlippage: 0, diff --git a/test/units/hyperdrive/InitializeTest.t.sol b/test/units/hyperdrive/InitializeTest.t.sol index 453ea76c3..d89e03571 100644 --- a/test/units/hyperdrive/InitializeTest.t.sol +++ b/test/units/hyperdrive/InitializeTest.t.sol @@ -70,7 +70,7 @@ contract InitializeTest is HyperdriveTest { ); } - function test_initialize_success( + function test_initialize( uint256 initialVaultSharePrice, uint256 checkpointDuration, uint256 checkpointsPerTerm, @@ -124,6 +124,33 @@ contract InitializeTest is HyperdriveTest { assertEq(lpShares, hyperdrive.balanceOf(AssetId._LP_ASSET_ID, alice)); } + function test_initialize_destination() external { + // Alice initializes the pool and sends the lp shares to Celine. + uint256 fixedRate = 0.5e18; + uint256 contribution = 100_000_000e18; + uint256 lpShares = initialize( + alice, + fixedRate, + contribution, + DepositOverrides({ + asBase: true, + destination: celine, + depositAmount: contribution, + minSharePrice: 0, // unused + minSlippage: 0, // unused + maxSlippage: type(uint256).max, // unused + extraData: new bytes(0) // unused + }) + ); + + // Ensure that Celine was invoked in the event. + verifyInitializeEvent(celine, lpShares, contribution, fixedRate); + + // Ensure that Celine received the LP shares. + assertEq(hyperdrive.balanceOf(AssetId._LP_ASSET_ID, alice), 0); + assertEq(hyperdrive.balanceOf(AssetId._LP_ASSET_ID, celine), lpShares); + } + function verifyInitializeEvent( address provider, uint256 expectedLpShares, diff --git a/test/units/hyperdrive/OpenLongTest.t.sol b/test/units/hyperdrive/OpenLongTest.t.sol index 2c9eb6f38..a8b702f23 100644 --- a/test/units/hyperdrive/OpenLongTest.t.sol +++ b/test/units/hyperdrive/OpenLongTest.t.sol @@ -235,6 +235,49 @@ contract OpenLongTest is HyperdriveTest { ); } + function test_open_long_destination() external { + uint256 apr = 0.05e18; + + // Initialize the pools with a large amount of capital. + uint256 contribution = 500_000_000e18; + initialize(alice, apr, contribution); + + // Open a long and send the position to celine. + uint256 baseAmount = 10e18; + (uint256 maturityTime, uint256 bondAmount) = openLong( + bob, + baseAmount, + DepositOverrides({ + asBase: true, + destination: celine, + depositAmount: baseAmount, + minSharePrice: 0, // min vault share price of 0 + minSlippage: baseAmount, // min bond proceeds of baseAmount + maxSlippage: type(uint256).max, // unused + extraData: new bytes(0) // unused + }) + ); + + // Ensure that the correct event was emitted. + verifyOpenLongEvent(celine, maturityTime, bondAmount, baseAmount); + + // Ensure that the position was sent to celine. + assertEq( + hyperdrive.balanceOf( + AssetId.encodeAssetId(AssetId.AssetIdPrefix.Long, maturityTime), + bob + ), + 0 + ); + assertEq( + hyperdrive.balanceOf( + AssetId.encodeAssetId(AssetId.AssetIdPrefix.Long, maturityTime), + celine + ), + bondAmount + ); + } + function test_open_long_with_small_amount() external { uint256 apr = 0.05e18; @@ -351,36 +394,7 @@ contract OpenLongTest is HyperdriveTest { ) internal { // Ensure that one `OpenLong` event was emitted with the correct // arguments. - { - VmSafe.Log[] memory logs = vm.getRecordedLogs().filterLogs( - OpenLong.selector - ); - assertEq(logs.length, 1); - VmSafe.Log memory log = logs[0]; - assertEq(address(uint160(uint256(log.topics[1]))), bob); - assertEq( - uint256(log.topics[2]), - AssetId.encodeAssetId(AssetId.AssetIdPrefix.Long, maturityTime) - ); - ( - uint256 eventMaturityTime, - uint256 eventBaseAmount, - uint256 eventVaultShareAmount, - bool eventAsBase, - uint256 eventBondAmount - ) = abi.decode( - log.data, - (uint256, uint256, uint256, bool, uint256) - ); - assertEq(eventMaturityTime, maturityTime); - assertEq(eventBaseAmount, baseAmount); - assertEq( - eventVaultShareAmount, - baseAmount.divDown(hyperdrive.getPoolInfo().vaultSharePrice) - ); - assertEq(eventAsBase, true); - assertEq(eventBondAmount, bondAmount); - } + verifyOpenLongEvent(bob, maturityTime, bondAmount, baseAmount); // Verify that the open long updated the state correctly. _verifyOpenLong( @@ -517,4 +531,37 @@ contract OpenLongTest is HyperdriveTest { ); assertEq(poolInfoAfter.shortAverageMaturityTime, 0); } + + function verifyOpenLongEvent( + address destination, + uint256 maturityTime, + uint256 bondAmount, + uint256 baseAmount + ) internal { + VmSafe.Log[] memory logs = vm.getRecordedLogs().filterLogs( + OpenLong.selector + ); + assertEq(logs.length, 1); + VmSafe.Log memory log = logs[0]; + assertEq(address(uint160(uint256(log.topics[1]))), destination); + assertEq( + uint256(log.topics[2]), + AssetId.encodeAssetId(AssetId.AssetIdPrefix.Long, maturityTime) + ); + ( + uint256 eventMaturityTime, + uint256 eventBaseAmount, + uint256 eventVaultShareAmount, + bool eventAsBase, + uint256 eventBondAmount + ) = abi.decode(log.data, (uint256, uint256, uint256, bool, uint256)); + assertEq(eventMaturityTime, maturityTime); + assertEq(eventBaseAmount, baseAmount); + assertEq( + eventVaultShareAmount, + baseAmount.divDown(hyperdrive.getPoolInfo().vaultSharePrice) + ); + assertEq(eventAsBase, true); + assertEq(eventBondAmount, bondAmount); + } } diff --git a/test/units/hyperdrive/OpenShortTest.t.sol b/test/units/hyperdrive/OpenShortTest.t.sol index aad5c4b18..13bdcc0ff 100644 --- a/test/units/hyperdrive/OpenShortTest.t.sol +++ b/test/units/hyperdrive/OpenShortTest.t.sol @@ -180,6 +180,55 @@ contract OpenShortTest is HyperdriveTest { ); } + function test_open_short_destination() external { + uint256 apr = 0.05e18; + + // Initialize the pool with a large amount of capital. + uint256 contribution = 500_000_000e18; + initialize(alice, apr, contribution); + + // Short a small amount of bonds. + uint256 shortAmount = 10e18; + (uint256 maturityTime, uint256 basePaid) = openShort( + bob, + shortAmount, + DepositOverrides({ + asBase: true, + destination: celine, + depositAmount: shortAmount, + minSharePrice: 0, // min vault share price of 0 + minSlippage: shortAmount, // min bond proceeds of baseAmount + maxSlippage: type(uint256).max, // unused + extraData: new bytes(0) // unused + }) + ); + + // Ensure that the correct event was emitted. + verifyOpenShortEvent(celine, maturityTime, shortAmount, basePaid); + + // Ensure that the position was sent to celine. + assertEq( + hyperdrive.balanceOf( + AssetId.encodeAssetId( + AssetId.AssetIdPrefix.Short, + maturityTime + ), + bob + ), + 0 + ); + assertEq( + hyperdrive.balanceOf( + AssetId.encodeAssetId( + AssetId.AssetIdPrefix.Short, + maturityTime + ), + celine + ), + shortAmount + ); + } + function test_open_short_with_small_amount() external { uint256 apr = 0.05e18; @@ -243,6 +292,7 @@ contract OpenShortTest is HyperdriveTest { bondAmount = (hyperdrive.calculateMaxShort() * 90) / 100; DepositOverrides memory depositOverrides = DepositOverrides({ asBase: false, + destination: bob, depositAmount: bondAmount * 2, minSharePrice: 0, minSlippage: 0, @@ -378,38 +428,7 @@ contract OpenShortTest is HyperdriveTest { ) internal { // Ensure that one `OpenShort` event was emitted with the correct // arguments. - { - VmSafe.Log[] memory logs = vm.getRecordedLogs().filterLogs( - OpenShort.selector - ); - assertEq(logs.length, 1); - VmSafe.Log memory log = logs[0]; - assertEq(address(uint160(uint256(log.topics[1]))), bob); - assertEq( - uint256(log.topics[2]), - AssetId.encodeAssetId(AssetId.AssetIdPrefix.Short, maturityTime) - ); - ( - uint256 eventMaturityTime, - uint256 eventBaseAmount, - uint256 eventVaultShareAmount, - bool eventAsBase, - uint256 eventBaseProceeds, - uint256 eventBondAmount - ) = abi.decode( - log.data, - (uint256, uint256, uint256, bool, uint256, uint256) - ); - assertEq(eventMaturityTime, maturityTime); - assertEq(eventBaseAmount, basePaid); - assertEq( - eventVaultShareAmount, - basePaid.divDown(hyperdrive.getPoolInfo().vaultSharePrice) - ); - assertEq(eventAsBase, true); - assertEq(eventBaseProceeds, shortAmount - basePaid); - assertEq(eventBondAmount, shortAmount); - } + verifyOpenShortEvent(bob, maturityTime, shortAmount, basePaid); // Verify that Hyperdrive received the max loss and that Bob received // the short tokens. @@ -491,4 +510,42 @@ contract OpenShortTest is HyperdriveTest { 5 ); } + + function verifyOpenShortEvent( + address destination, + uint256 maturityTime, + uint256 shortAmount, + uint256 basePaid + ) internal { + VmSafe.Log[] memory logs = vm.getRecordedLogs().filterLogs( + OpenShort.selector + ); + assertEq(logs.length, 1); + VmSafe.Log memory log = logs[0]; + assertEq(address(uint160(uint256(log.topics[1]))), destination); + assertEq( + uint256(log.topics[2]), + AssetId.encodeAssetId(AssetId.AssetIdPrefix.Short, maturityTime) + ); + ( + uint256 eventMaturityTime, + uint256 eventBaseAmount, + uint256 eventVaultShareAmount, + bool eventAsBase, + uint256 eventBaseProceeds, + uint256 eventBondAmount + ) = abi.decode( + log.data, + (uint256, uint256, uint256, bool, uint256, uint256) + ); + assertEq(eventMaturityTime, maturityTime); + assertEq(eventBaseAmount, basePaid); + assertEq( + eventVaultShareAmount, + basePaid.divDown(hyperdrive.getPoolInfo().vaultSharePrice) + ); + assertEq(eventAsBase, true); + assertEq(eventBaseProceeds, shortAmount - basePaid); + assertEq(eventBondAmount, shortAmount); + } } diff --git a/test/units/hyperdrive/RedeemWithdrawalSharesTest.t.sol b/test/units/hyperdrive/RedeemWithdrawalSharesTest.t.sol index fbfd7c607..493af50a8 100644 --- a/test/units/hyperdrive/RedeemWithdrawalSharesTest.t.sol +++ b/test/units/hyperdrive/RedeemWithdrawalSharesTest.t.sol @@ -288,6 +288,7 @@ contract RedeemWithdrawalSharesTest is HyperdriveTest { withdrawalShares, WithdrawalOverrides({ asBase: true, + destination: alice, minSlippage: expectedSharePrice, extraData: new bytes(0) }) @@ -319,6 +320,44 @@ contract RedeemWithdrawalSharesTest is HyperdriveTest { ); } + function test_redeem_withdrawal_shares_destination() external { + // Initialize the pool. + uint256 lpShares = initialize(alice, 0.02e18, 500_000_000e18); + + // Bob opens a large short. + uint256 shortAmount = HyperdriveUtils.calculateMaxShort(hyperdrive); + (uint256 maturityTime, ) = openShort(bob, shortAmount); + + // Alice removes her liquidity. + (, uint256 withdrawalShares) = removeLiquidity(alice, lpShares); + + // The term passes and no interest accrues. + advanceTime(POSITION_DURATION, 0); + + // Bob closes his short. + closeShort(bob, maturityTime, shortAmount); + + // Alice redeems her withdrawal shares and sends the proceeds to Celine. + (uint256 baseProceeds, uint256 sharesRedeemed) = redeemWithdrawalShares( + alice, + withdrawalShares, + WithdrawalOverrides({ + asBase: true, + destination: celine, + minSlippage: 0, + extraData: new bytes(0) + }) + ); + assertGt(baseProceeds, 0); + + // Ensure that a `RedeemWithdrawalShares` event was emitted. + verifyRedeemWithdrawalSharesEvent(celine, sharesRedeemed, baseProceeds); + + // Ensure that Celine received the base proceeds. + assertEq(baseToken.balanceOf(alice), 0); + assertEq(baseToken.balanceOf(celine), baseProceeds); + } + function verifyRedeemWithdrawalSharesEvent( address provider, uint256 expectedSharesRedeemed, diff --git a/test/units/hyperdrive/RemoveLiquidityTest.t.sol b/test/units/hyperdrive/RemoveLiquidityTest.t.sol index c3ba35651..6a7319c6a 100644 --- a/test/units/hyperdrive/RemoveLiquidityTest.t.sol +++ b/test/units/hyperdrive/RemoveLiquidityTest.t.sol @@ -1,6 +1,9 @@ // SPDX-License-Identifier: Apache-2.0 pragma solidity 0.8.20; +// FIXME +import { console2 as console } from "forge-std/console2.sol"; + import { stdError } from "forge-std/StdError.sol"; import { VmSafe } from "forge-std/Vm.sol"; import { IHyperdrive } from "contracts/src/interfaces/IHyperdrive.sol"; @@ -104,6 +107,49 @@ contract RemoveLiquidityTest is HyperdriveTest { _test_remove_liquidity(testCase); } + function test_remove_liquidity_destination() external { + // Initialize the pool with a large amount of capital. + uint256 apr = 0.05e18; + uint256 contribution = 100_000_000e18; + uint256 lpShares = initialize(alice, apr, contribution); + + // Bob opens a max short. + openShort(bob, hyperdrive.calculateMaxShort()); + + // Alice removes her liquidity and sends the proceeds to celine. + (uint256 baseProceeds, uint256 withdrawalShares) = removeLiquidity( + alice, + lpShares, + WithdrawalOverrides({ + asBase: false, + destination: celine, + minSlippage: 0, + extraData: new bytes(0) + }) + ); + assertGt(withdrawalShares, 0); + + // Ensure that the correct event was emitted. + verifyRemoveLiquidityEvent( + celine, + lpShares, + baseProceeds, + withdrawalShares + ); + + // Ensure that the proceeds were sent to celine. + assertEq(baseToken.balanceOf(alice), 0); + assertEq(baseToken.balanceOf(celine), baseProceeds); + assertEq( + hyperdrive.balanceOf(AssetId._WITHDRAWAL_SHARE_ASSET_ID, alice), + 0 + ); + assertEq( + hyperdrive.balanceOf(AssetId._WITHDRAWAL_SHARE_ASSET_ID, celine), + withdrawalShares + ); + } + function test_remove_liquidity_long_trade() external { TestCase memory testCase = TestCase({ initializer: alice, @@ -238,6 +284,7 @@ contract RemoveLiquidityTest is HyperdriveTest { // Ensure that the correct event was emitted. verifyRemoveLiquidityEvent( + alice, testCase.initialLpShares, testCase.initialLpBaseProceeds, testCase.initialLpWithdrawalShares @@ -268,6 +315,7 @@ contract RemoveLiquidityTest is HyperdriveTest { } function verifyRemoveLiquidityEvent( + address destination, uint256 expectedLpShares, uint256 expectedBaseAmount, uint256 expectedWithdrawalShares @@ -277,7 +325,7 @@ contract RemoveLiquidityTest is HyperdriveTest { ); assertEq(logs.length, 1); VmSafe.Log memory log = logs[0]; - assertEq(address(uint160(uint256(log.topics[1]))), alice); + assertEq(address(uint160(uint256(log.topics[1]))), destination); ( uint256 lpShares, uint256 baseAmount, diff --git a/test/utils/HyperdriveTest.sol b/test/utils/HyperdriveTest.sol index d77df5984..54a7b36f0 100644 --- a/test/utils/HyperdriveTest.sol +++ b/test/utils/HyperdriveTest.sol @@ -168,6 +168,10 @@ contract HyperdriveTest is IHyperdriveEvents, BaseTest { struct DepositOverrides { // A boolean flag specifying whether or not the underlying should be used. bool asBase; + // The destination address. + address destination; + // The extra data to pass to the yield source. + bytes extraData; // The amount of tokens the action should prepare to deposit. Note that // the actual deposit amount will still be specified by the action being // called; however, this is the amount that will be minted as a @@ -184,19 +188,19 @@ contract HyperdriveTest is IHyperdriveEvents, BaseTest { // This is the slippage parameter that defines an upper bound on the // quantity being measured. It may not be used by some actions. uint256 maxSlippage; - // The extra data to pass to the yield source. - bytes extraData; } // Overrides for functions that initiate withdrawals. struct WithdrawalOverrides { // A boolean flag specifying whether or not the underlying should be used. bool asBase; + // The destination address. + address destination; + // The extra data to pass to the yield source. + bytes extraData; // This is the slippage parameter that defines a lower bound on the // quantity being measured. uint256 minSlippage; - // The extra data to pass to the yield source. - bytes extraData; } function initialize( @@ -231,7 +235,7 @@ contract HyperdriveTest is IHyperdriveEvents, BaseTest { contribution, apr, IHyperdrive.Options({ - destination: lp, + destination: overrides.destination, asBase: overrides.asBase, extraData: overrides.extraData }) @@ -251,6 +255,7 @@ contract HyperdriveTest is IHyperdriveEvents, BaseTest { contribution, DepositOverrides({ asBase: true, + destination: lp, depositAmount: contribution, minSharePrice: 0, // unused minSlippage: 0, // unused @@ -273,6 +278,7 @@ contract HyperdriveTest is IHyperdriveEvents, BaseTest { contribution, DepositOverrides({ asBase: asBase, + destination: lp, depositAmount: contribution, minSharePrice: 0, // unused minSlippage: 0, // unused @@ -302,7 +308,7 @@ contract HyperdriveTest is IHyperdriveEvents, BaseTest { overrides.minSlippage, // min spot rate overrides.maxSlippage, // max spot rate IHyperdrive.Options({ - destination: lp, + destination: overrides.destination, asBase: overrides.asBase, extraData: overrides.extraData }) @@ -317,7 +323,7 @@ contract HyperdriveTest is IHyperdriveEvents, BaseTest { overrides.minSlippage, // min spot rate overrides.maxSlippage, // max spot rate IHyperdrive.Options({ - destination: lp, + destination: overrides.destination, asBase: overrides.asBase, extraData: overrides.extraData }) @@ -335,6 +341,7 @@ contract HyperdriveTest is IHyperdriveEvents, BaseTest { contribution, DepositOverrides({ asBase: true, + destination: lp, depositAmount: contribution, minSharePrice: 0, // unused minSlippage: 0, // min spot rate of 0 @@ -355,6 +362,7 @@ contract HyperdriveTest is IHyperdriveEvents, BaseTest { contribution, DepositOverrides({ asBase: asBase, + destination: lp, depositAmount: contribution, minSharePrice: 0, // min lp share price of 0 minSlippage: 0, // min spot rate of 0 @@ -378,7 +386,7 @@ contract HyperdriveTest is IHyperdriveEvents, BaseTest { shares, overrides.minSlippage, // min lp share price IHyperdrive.Options({ - destination: lp, + destination: overrides.destination, asBase: overrides.asBase, extraData: overrides.extraData }) @@ -395,6 +403,7 @@ contract HyperdriveTest is IHyperdriveEvents, BaseTest { shares, WithdrawalOverrides({ asBase: true, + destination: lp, minSlippage: 0, // min lp share price of 0 extraData: new bytes(0) // unused }) @@ -412,6 +421,7 @@ contract HyperdriveTest is IHyperdriveEvents, BaseTest { shares, WithdrawalOverrides({ asBase: asBase, + destination: lp, minSlippage: 0, // min base proceeds of 0 extraData: new bytes(0) // unused }) @@ -432,7 +442,7 @@ contract HyperdriveTest is IHyperdriveEvents, BaseTest { shares, overrides.minSlippage, // min output per share IHyperdrive.Options({ - destination: lp, + destination: overrides.destination, asBase: overrides.asBase, extraData: overrides.extraData }) @@ -449,6 +459,7 @@ contract HyperdriveTest is IHyperdriveEvents, BaseTest { shares, WithdrawalOverrides({ asBase: true, + destination: lp, minSlippage: 0, // min output per share of 0 extraData: new bytes(0) // unused }) @@ -466,6 +477,7 @@ contract HyperdriveTest is IHyperdriveEvents, BaseTest { shares, WithdrawalOverrides({ asBase: asBase, + destination: lp, minSlippage: 0, // min output per share of 0 extraData: new bytes(0) // unused }) @@ -492,7 +504,7 @@ contract HyperdriveTest is IHyperdriveEvents, BaseTest { overrides.minSlippage, // min bond proceeds overrides.minSharePrice, // min vault share price IHyperdrive.Options({ - destination: trader, + destination: overrides.destination, asBase: overrides.asBase, extraData: overrides.extraData }) @@ -506,7 +518,7 @@ contract HyperdriveTest is IHyperdriveEvents, BaseTest { overrides.minSlippage, // min bond proceeds overrides.minSharePrice, // min vault share price IHyperdrive.Options({ - destination: trader, + destination: overrides.destination, asBase: overrides.asBase, extraData: overrides.extraData }) @@ -524,6 +536,7 @@ contract HyperdriveTest is IHyperdriveEvents, BaseTest { baseAmount, DepositOverrides({ asBase: true, + destination: trader, depositAmount: baseAmount, minSharePrice: 0, // min vault share price of 0 minSlippage: baseAmount, // min bond proceeds of baseAmount @@ -544,6 +557,7 @@ contract HyperdriveTest is IHyperdriveEvents, BaseTest { baseAmount, DepositOverrides({ asBase: asBase, + destination: trader, depositAmount: baseAmount, minSharePrice: 0, // min vault share price of 0 minSlippage: baseAmount, // min bond proceeds of baseAmount @@ -569,7 +583,7 @@ contract HyperdriveTest is IHyperdriveEvents, BaseTest { bondAmount, overrides.minSlippage, // min base proceeds IHyperdrive.Options({ - destination: trader, + destination: overrides.destination, asBase: overrides.asBase, extraData: overrides.extraData }) @@ -587,6 +601,7 @@ contract HyperdriveTest is IHyperdriveEvents, BaseTest { maturityTime, bondAmount, WithdrawalOverrides({ + destination: trader, asBase: true, minSlippage: 0, // min base proceeds of 0 extraData: new bytes(0) // unused @@ -607,6 +622,7 @@ contract HyperdriveTest is IHyperdriveEvents, BaseTest { bondAmount, WithdrawalOverrides({ asBase: asBase, + destination: trader, minSlippage: 0, // min base proceeds of 0 extraData: new bytes(0) // unused }) @@ -636,7 +652,7 @@ contract HyperdriveTest is IHyperdriveEvents, BaseTest { overrides.maxSlippage, // max base payment overrides.minSharePrice, // min vault share price IHyperdrive.Options({ - destination: trader, + destination: overrides.destination, asBase: overrides.asBase, extraData: overrides.extraData }) @@ -649,7 +665,7 @@ contract HyperdriveTest is IHyperdriveEvents, BaseTest { overrides.maxSlippage, // max base payment overrides.minSharePrice, // min vault share price IHyperdrive.Options({ - destination: trader, + destination: overrides.destination, asBase: overrides.asBase, extraData: overrides.extraData }) @@ -670,6 +686,7 @@ contract HyperdriveTest is IHyperdriveEvents, BaseTest { bondAmount, DepositOverrides({ asBase: true, + destination: trader, depositAmount: bondAmount, minSharePrice: 0, // min vault share price of 0 minSlippage: 0, // unused @@ -690,6 +707,7 @@ contract HyperdriveTest is IHyperdriveEvents, BaseTest { bondAmount, DepositOverrides({ asBase: asBase, + destination: trader, depositAmount: bondAmount, minSharePrice: 0, // min vault share price of 0 minSlippage: 0, // unused @@ -715,7 +733,7 @@ contract HyperdriveTest is IHyperdriveEvents, BaseTest { bondAmount, overrides.minSlippage, // min base proceeds IHyperdrive.Options({ - destination: trader, + destination: overrides.destination, asBase: overrides.asBase, extraData: overrides.extraData }) @@ -734,6 +752,7 @@ contract HyperdriveTest is IHyperdriveEvents, BaseTest { bondAmount, WithdrawalOverrides({ asBase: true, + destination: trader, minSlippage: 0, // min base proceeds of 0 extraData: new bytes(0) // unused }) @@ -753,6 +772,7 @@ contract HyperdriveTest is IHyperdriveEvents, BaseTest { bondAmount, WithdrawalOverrides({ asBase: asBase, + destination: trader, minSlippage: 0, // min base proceeds of 0 extraData: new bytes(0) // unused }) From 3deb8935eb429672d6b4663b2e3c1a2a98af9b9c Mon Sep 17 00:00:00 2001 From: Alex Towle Date: Wed, 21 Feb 2024 11:58:11 -0600 Subject: [PATCH 2/2] Addressed nit and fixed tests --- test/units/hyperdrive/CloseLongTest.t.sol | 4 ++-- test/units/hyperdrive/RemoveLiquidityTest.t.sol | 5 +---- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/test/units/hyperdrive/CloseLongTest.t.sol b/test/units/hyperdrive/CloseLongTest.t.sol index 1b590fd99..8809b6459 100644 --- a/test/units/hyperdrive/CloseLongTest.t.sol +++ b/test/units/hyperdrive/CloseLongTest.t.sol @@ -926,9 +926,9 @@ contract CloseLongTest is HyperdriveTest { // arguments. verifyCloseLongEvent( bob, + testCase.maturityTime, testCase.bondAmount, - testCase.baseProceeds, - testCase.maturityTime + testCase.baseProceeds ); // Ensure that the correct amount of base was transferred. diff --git a/test/units/hyperdrive/RemoveLiquidityTest.t.sol b/test/units/hyperdrive/RemoveLiquidityTest.t.sol index 6a7319c6a..005408dad 100644 --- a/test/units/hyperdrive/RemoveLiquidityTest.t.sol +++ b/test/units/hyperdrive/RemoveLiquidityTest.t.sol @@ -1,9 +1,6 @@ // SPDX-License-Identifier: Apache-2.0 pragma solidity 0.8.20; -// FIXME -import { console2 as console } from "forge-std/console2.sol"; - import { stdError } from "forge-std/StdError.sol"; import { VmSafe } from "forge-std/Vm.sol"; import { IHyperdrive } from "contracts/src/interfaces/IHyperdrive.sol"; @@ -121,7 +118,7 @@ contract RemoveLiquidityTest is HyperdriveTest { alice, lpShares, WithdrawalOverrides({ - asBase: false, + asBase: true, destination: celine, minSlippage: 0, extraData: new bytes(0)