fix: use previewWithdraw(assetsDeposited) for excess shares (#7152)

Author: yorhodes

solidity/contracts/token/extensions/HypERC4626OwnerCollateral.sol

@@ -66,8 +66,10 @@ contract HypERC4626OwnerCollateral is HypERC4626Collateral {
      * @notice Allows the owner to redeem excess shares
      */
     function sweep() external onlyOwner {
+        // convert assetsDeposited to shares rounding up to ensure
+        // the owner cannot withdraw user collateral
         uint256 excessShares = vault.maxRedeem(address(this)) -
-            vault.convertToShares(assetDeposited);
+            vault.previewWithdraw(assetDeposited);
         uint256 assetsRedeemed = vault.redeem(
             excessShares,
             owner(),

solidity/test/token/HypERC20CollateralVaultDeposit.t.sol

@@ -238,6 +238,141 @@ contract HypERC4626OwnerCollateralTest is HypTokenTest {
         );
     }
 
+    function testERC4626VaultDeposit_ceilingRounding_reservesMoreShares()
+        public
+    {
+        // This test verifies the mathematical difference between convertToShares (floor)
+        // and previewWithdraw (ceiling) rounding when calculating shares for deposits.
+
+        uint256 transferAmount = 100e18;
+        uint256 rewardAmount = 1e18;
+
+        // Setup: Transfer from Alice to Bob
+        vm.prank(ALICE);
+        primaryToken.approve(address(localToken), transferAmount);
+        _performRemoteTransfer(0, transferAmount);
+
+        // Add yield to the vault (increases share value)
+        primaryToken.mintTo(address(vault), rewardAmount);
+
+        // Transfer back from Bob to Alice
+        vm.prank(BOB);
+        remoteToken.transferRemote(
+            ORIGIN,
+            BOB.addressToBytes32(),
+            transferAmount
+        );
+        _handleLocalTransfer(transferAmount);
+
+        // At this point, we have excess shares due to the yield
+        uint256 totalShares = vault.maxRedeem(
+            address(erc20CollateralVaultDeposit)
+        );
+        uint256 assetDeposited = erc20CollateralVaultDeposit.assetDeposited();
+
+        // Calculate what convertToShares (floor rounding) would give us
+        uint256 sharesFloor = vault.convertToShares(assetDeposited);
+
+        // Calculate what previewWithdraw (ceiling rounding) gives us
+        uint256 sharesCeiling = vault.previewWithdraw(assetDeposited);
+
+        // When there's rounding involved, ceiling should be >= floor
+        // and the excess shares should be: totalShares - sharesCeiling
+        uint256 excessSharesWithCeiling = totalShares - sharesCeiling;
+        uint256 excessSharesWithFloor = totalShares - sharesFloor;
+
+        // Verify the key difference: ceiling rounding calculates more shares to reserve
+        // for the deposited assets, which means fewer excess shares to sweep
+        assertLe(
+            excessSharesWithCeiling,
+            excessSharesWithFloor,
+            "Ceiling rounding should reserve more shares for deposits"
+        );
+
+        // Perform sweep and verify the amount swept is <= excessSharesWithFloor
+        // Record logs to capture the event
+        vm.recordLogs();
+        erc20CollateralVaultDeposit.sweep();
+
+        // Get the logs and extract the ExcessSharesSwept event
+        Vm.Log[] memory logs = vm.getRecordedLogs();
+        bool foundEvent = false;
+        uint256 sweptShares;
+
+        for (uint256 i = 0; i < logs.length; i++) {
+            // ExcessSharesSwept event signature: ExcessSharesSwept(uint256,uint256)
+            if (
+                logs[i].topics[0] ==
+                keccak256("ExcessSharesSwept(uint256,uint256)")
+            ) {
+                foundEvent = true;
+                // Decode the event data (amount is first parameter, assetsRedeemed is second)
+                (sweptShares, ) = abi.decode(logs[i].data, (uint256, uint256));
+                break;
+            }
+        }
+
+        assertTrue(
+            foundEvent,
+            "ExcessSharesSwept event should have been emitted"
+        );
+        assertLe(
+            sweptShares,
+            excessSharesWithFloor,
+            "Swept amount should be <= excessSharesWithFloor"
+        );
+    }
+
+    function testERC4626VaultDeposit_sweep_usesCeilingRounding() public {
+        // This test verifies that sweep() correctly sweeps excess shares after yield accrual
+        // and leaves no shares behind when assetDeposited is 0.
+
+        uint256 transferAmount = 100e18;
+        uint256 rewardAmount = 1e18;
+
+        // Setup: Transfer from Alice to Bob
+        vm.prank(ALICE);
+        primaryToken.approve(address(localToken), transferAmount);
+        _performRemoteTransfer(0, transferAmount);
+
+        // Add yield to the vault (increases share value)
+        primaryToken.mintTo(address(vault), rewardAmount);
+
+        // Transfer back from Bob to Alice
+        vm.prank(BOB);
+        remoteToken.transferRemote(
+            ORIGIN,
+            BOB.addressToBytes32(),
+            transferAmount
+        );
+        _handleLocalTransfer(transferAmount);
+
+        uint256 ownerBalanceBefore = primaryToken.balanceOf(
+            erc20CollateralVaultDeposit.owner()
+        );
+
+        // Call sweep() which should use previewWithdraw (ceiling rounding)
+        erc20CollateralVaultDeposit.sweep();
+
+        uint256 ownerBalanceAfter = primaryToken.balanceOf(
+            erc20CollateralVaultDeposit.owner()
+        );
+        uint256 sweptAmount = ownerBalanceAfter - ownerBalanceBefore;
+
+        // The swept amount should be positive (we did sweep excess shares)
+        assertGt(sweptAmount, 0, "Should have swept excess shares");
+
+        // After sweep, we should have no shares remaining (assetDeposited is 0)
+        uint256 remainingShares = vault.maxRedeem(
+            address(erc20CollateralVaultDeposit)
+        );
+        assertEq(
+            remainingShares,
+            0,
+            "Should have no shares remaining after sweep with no deposits"
+        );
+    }
+
     function testERC4626VaultDeposit_TransferFromSender_CorrectMetadata()
         public
     {