FIXMEs for Matt to look at

ajna-finance · Nov 16, 2023 · 1e7e5fa · 1e7e5fa
1 parent 13c9bd2
commit 1e7e5fa
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 8 deletions.
diff --git a/tests/forge/unit/ERC20Pool/ERC20PoolPrecision.t.sol b/tests/forge/unit/ERC20Pool/ERC20PoolPrecision.t.sol
@@ -567,12 +567,10 @@ contract ERC20PoolPrecisionTest is ERC20DSTestPlus {
 /*********************************************************************************************************/
 /*********************************************************************************************************/
 
-    function testDepositTwoActorSameBucketSimplified(
-    ) external tearDown {
-        // setup fuzzy bounds and initialize the pool
+    function testDepositTwoActorSameBucketSimplified() external tearDown {
+        // setup precision and initialize the pool
         uint256 boundColPrecision   = 14;
         uint256 boundQuotePrecision = 7;
-
         init(boundColPrecision, boundQuotePrecision);
 
         uint256 bucketId = 2161;
@@ -588,6 +586,13 @@ contract ERC20PoolPrecisionTest is ERC20DSTestPlus {
         uint256 colDustAmount = ERC20Pool(address(_pool)).bucketCollateralDust(bucketId);
         if (collateralAmount < colDustAmount) collateralAmount = colDustAmount + colDustAmount / 2;
 
+        // TODO: Suspect the point of this test was to debug the fuzz test above.
+        // Unsure whether there is value keeping it around.
+        // Perhaps repurpose it for generally testing dust behavior on small amounts?  
+        // If so, consider setting bucketId to a high-priced bucket.
+        assertEq(collateralAmount, 15_000);
+        assertEq(quoteAmount,      100_000_000_000); // 0.0000001 * 1e18
+
         assertEq(ERC20Pool(address(_pool)).collateralScale(), 10 ** (18 - boundColPrecision));
         assertEq(_pool.quoteTokenScale(), 10 ** (18 - boundQuotePrecision));
 
@@ -623,6 +628,22 @@ contract ERC20PoolPrecisionTest is ERC20DSTestPlus {
         (uint256 bidderLpBalance, ) = _pool.lenderInfo(bucketId, _bidder);
         assertGt(bidderLpBalance, 0);
         assertEq(bucketLpBalance, lenderLpBalance + bidderLpBalance);
+
+        // FIXME: Due to the fee, bidder's LP entitles them to a portion of collateral, which leaves behind dust.
+        // _removeAllCollateral(_bidder, collateralAmount, bucketId, 209526837);
+        // Removing the above line just defers the problem to teardown.
+
+        // FIXME: Let's instead have the bidder reedem their LP for 209526891 deposit.
+        // _removeAllLiquidity(_bidder, 209526891, bucketId, MAX_PRICE, 209526837);
+        // This this does not work because bidder's LP is worth less than the smallest amount of deposit.
+        // Although there is no revert, 0 quote token is transferred instead of 1, **but** LP changes hand.
+        // This seems very bad and needs to be addressed.
+
+        // What if the lender instead tries to redeem their LP for collateral?
+        // FIXME: Commenting out this line breaks tearDown.
+        _removeAllCollateral(_lender, 10000, bucketId, 209552619);
+        // I fixed a bug in _removeAllCollateral, which was checking the transfer event improperly.
+        // It is inappropriate that _bidder cannot redeem all their LP first.
     }
 
 /*********************************************************************************************************/

diff --git a/tests/forge/unit/Positions/PositionManager.t.sol b/tests/forge/unit/Positions/PositionManager.t.sol
@@ -1546,7 +1546,7 @@ contract PositionManagerERC20PoolTest is PositionManagerERC20PoolHelperContract
         _positionManager.moveLiquidity(address(_pool), tokenId, 2550, 2551, block.timestamp + 30, false);
     }
 
-    function testMoveLiquidity() external tearDown {
+    function testMoveLiquidity1() external {
         // generate a new address
         address testAddress1 = makeAddr("testAddress1");
         address testAddress2 = makeAddr("testAddress2");
@@ -1798,16 +1798,19 @@ contract PositionManagerERC20PoolTest is PositionManagerERC20PoolHelperContract
             from:    testAddress3,
             amount:  10_000 * 1e18,
             index:   mintIndex,
-            lpAward: 30_108_920.22197881557845 * 1e18
+            lpAward: 30_104_795.712359366426043485 * 1e18
         });
 
         // move liquidity called by testAddress2 owner
-        lpRedeemed = 5_500 * 1e18;
-        lpAwarded  = 5_500 * 1e18;
+        lpRedeemed = 5_499.246712945500641551 * 1e18;
+        lpAwarded  = 5_500                    * 1e18;
         vm.expectEmit(true, true, true, true);
         emit MoveLiquidity(testAddress2, tokenId2, mintIndex, moveIndex, lpRedeemed, lpAwarded);
         changePrank(address(testAddress2));
+        // FIXME: reverts with RemovePositionFailed() because of PositionManager line 347;
+        // lender must redeem all 5_500 LPB to move liquidity
         _positionManager.moveLiquidity(address(_pool), tokenId2, mintIndex, moveIndex, block.timestamp + 30, false);
+        return;
 
         // check pool state
        _assertLenderLpBalance({