carbon vortex 2.0 - peckshield audit fixes

bancorprotocol · May 27, 2024 · 01790f4 · 01790f4
1 parent d06152e
commit 01790f4
Show file tree

Hide file tree

Showing 2 changed files with 6 additions and 66 deletions.
diff --git a/contracts/vortex/CarbonVortex.sol b/contracts/vortex/CarbonVortex.sol
@@ -568,7 +568,7 @@ contract CarbonVortex is ICarbonVortex, Upgradeable, ReentrancyGuardUpgradeable,
         // revert if current price is not valid
         _validPrice(currentPrice);
         // calculate the trade input based on the current price
-        return MathEx.mulDivF(currentPrice.sourceAmount, targetAmount, currentPrice.targetAmount).toUint128();
+        return MathEx.mulDivC(currentPrice.sourceAmount, targetAmount, currentPrice.targetAmount).toUint128();
     }
 
     /**
@@ -625,11 +625,6 @@ contract CarbonVortex is ICarbonVortex, Upgradeable, ReentrancyGuardUpgradeable,
         // transfer the tokens to caller
         token.safeTransfer(msg.sender, targetAmount);
 
-        // if the target token is native, refund any excess native token to caller
-        if (_targetToken == NATIVE_TOKEN && msg.value > sourceAmount) {
-            payable(msg.sender).sendValue(msg.value - sourceAmount);
-        }
-
         // if no final target token is defined, transfer the target token to `transferAddress`
         if (Token.unwrap(_finalTargetToken) == address(0)) {
             // safe due to nonreenrant modifier (forwards all available gas if token is native)
@@ -643,6 +638,11 @@ contract CarbonVortex is ICarbonVortex, Upgradeable, ReentrancyGuardUpgradeable,
             _resetTrading(token, 0);
         }
 
+        // if the target token is native, refund any excess native token to caller
+        if (_targetToken == NATIVE_TOKEN && msg.value > sourceAmount) {
+            payable(msg.sender).sendValue(msg.value - sourceAmount);
+        }
+
         return sourceAmount;
     }
 

diff --git a/test/forge/CarbonVortex.t.sol b/test/forge/CarbonVortex.t.sol
@@ -1627,66 +1627,6 @@ contract CarbonVortexTest is TestFixture {
         assertEq(balanceSent, expectedSourceAmount);
     }
 
-    /// @dev test should revert if the trade requires 0 target token on finalTargetToken -> targetToken trades
-    function testShouldRevertIfTradeRequiresZeroTargetToken() public {
-        vm.prank(admin);
-        // set fees
-        uint256 accumulatedFees = 100 ether;
-        carbonController.testSetAccumulatedFees(targetToken, accumulatedFees);
-
-        vm.startPrank(user1);
-
-        // execute
-        Token[] memory tokens = new Token[](1);
-        tokens[0] = targetToken;
-        carbonVortex.execute(tokens);
-
-        // trade target for final target
-        uint128 targetAmount = 1 ether;
-
-        // advance time to a point where the price is very low
-        vm.warp(60 days);
-
-        // get the expected trade input for 1 ether of target token
-        uint128 expectedSourceAmount = carbonVortex.expectedTradeInput(targetToken, targetAmount);
-
-        // approve the source token
-        finalTargetToken.safeApprove(address(carbonVortex), expectedSourceAmount);
-        // trade
-        vm.expectRevert(ICarbonVortex.InvalidTrade.selector);
-        carbonVortex.trade(targetToken, 1);
-    }
-
-    /// @dev test should revert if the trade requires 0 tokens on targetToken -> token trades
-    function testShouldRevertIfTradeRequiresZeroTokens() public {
-        vm.prank(admin);
-
-        Token token = token1;
-        // set fees
-        uint256 accumulatedFees = 100 ether;
-        carbonController.testSetAccumulatedFees(token, accumulatedFees);
-
-        vm.startPrank(user1);
-
-        // execute
-        Token[] memory tokens = new Token[](1);
-        tokens[0] = token;
-        carbonVortex.execute(tokens);
-
-        // trade target for final target
-        uint128 targetAmount = 1 ether;
-
-        // advance time to a point where the price is very low
-        vm.warp(60 days);
-
-        // get the expected trade input for 1 ether of target token
-        uint128 expectedSourceAmount = carbonVortex.expectedTradeInput(token, targetAmount);
-
-        // trade
-        vm.expectRevert(ICarbonVortex.InvalidTrade.selector);
-        carbonVortex.trade{ value: expectedSourceAmount }(token, 1);
-    }
-
     /// @dev test should revert if user hasn't sent enough target tokens for trade
     function testShouldRevertIfUserHasntSentEnoughTargetTokenForTrade() public {
         vm.prank(admin);