Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge Swap and SwapToAnyToken into a single example #223

Merged
merged 14 commits into from
Dec 16, 2024
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
185 changes: 142 additions & 43 deletions examples/swap/contracts/Swap.sol
Original file line number Diff line number Diff line change
Expand Up @@ -9,30 +9,59 @@ import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import {RevertContext, RevertOptions} from "@zetachain/protocol-contracts/contracts/Revert.sol";
import "@zetachain/protocol-contracts/contracts/zevm/interfaces/UniversalContract.sol";
import "@zetachain/protocol-contracts/contracts/zevm/interfaces/IGatewayZEVM.sol";
import "@zetachain/protocol-contracts/contracts/zevm/interfaces/IWZETA.sol";
import {GatewayZEVM} from "@zetachain/protocol-contracts/contracts/zevm/GatewayZEVM.sol";

contract Swap is UniversalContract {
address public immutable uniswapRouter;
import {Initializable} from "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
import {OwnableUpgradeable} from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
import {UUPSUpgradeable} from "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol";

contract Swap is
UniversalContract,
Initializable,
UUPSUpgradeable,
OwnableUpgradeable
{
address public uniswapRouter;
GatewayZEVM public gateway;
uint256 constant BITCOIN = 18332;
uint256 public immutable gasLimit;
uint256 constant BITCOIN = 8332;
uint256 constant BITCOIN_TESTNET = 18332;
uint256 public gasLimit;

error InvalidAddress();
error Unauthorized();
error ApprovalFailed();
error TransferFailed();

event TokenSwap(
address sender,
bytes indexed recipient,
address indexed inputToken,
address indexed targetToken,
uint256 inputAmount,
uint256 outputAmount
);

modifier onlyGateway() {
if (msg.sender != address(gateway)) revert Unauthorized();
_;
}

constructor(
/// @custom:oz-upgrades-unsafe-allow constructor
constructor() {
_disableInitializers();
}

function initialize(
address payable gatewayAddress,
address uniswapRouterAddress,
uint256 gasLimitAmount
) {
uint256 gasLimitAmount,
address owner
) public initializer {
if (gatewayAddress == address(0) || uniswapRouterAddress == address(0))
revert InvalidAddress();
__UUPSUpgradeable_init();
__Ownable_init(owner);
uniswapRouter = uniswapRouterAddress;
gateway = GatewayZEVM(gatewayAddress);
gasLimit = gasLimitAmount;
Expand All @@ -41,6 +70,7 @@ contract Swap is UniversalContract {
struct Params {
address target;
bytes to;
bool withdraw;
}

function onCall(
Expand All @@ -49,29 +79,90 @@ contract Swap is UniversalContract {
uint256 amount,
bytes calldata message
) external onlyGateway {
Params memory params = Params({target: address(0), to: bytes("")});
if (context.chainID == BITCOIN) {
Params memory params = Params({
target: address(0),
to: bytes(""),
withdraw: true
});

if (context.chainID == BITCOIN_TESTNET || context.chainID == BITCOIN) {
params.target = BytesHelperLib.bytesToAddress(message, 0);
params.to = abi.encodePacked(
BytesHelperLib.bytesToAddress(message, 20)
);
if (message.length >= 41) {
params.withdraw = BytesHelperLib.bytesToBool(message, 40);
}
} else {
(address targetToken, bytes memory recipient) = abi.decode(
message,
(address, bytes)
);
(
address targetToken,
bytes memory recipient,
bool withdrawFlag
) = abi.decode(message, (address, bytes, bool));
params.target = targetToken;
params.to = recipient;
params.withdraw = withdrawFlag;
}

(uint256 out, address gasZRC20, uint256 gasFee) = handleGasAndSwap(
zrc20,
amount,
params.target
);
emit TokenSwap(
context.sender,
params.to,
zrc20,
params.target,
amount,
out
);
withdraw(params, context.sender, gasFee, gasZRC20, out, zrc20);
}

function swap(
address inputToken,
uint256 amount,
address targetToken,
bytes memory recipient,
bool withdrawFlag
) public {
Comment on lines +123 to +129
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add input validation for swap parameters

The function should validate that input addresses are not zero and amount is greater than 0.

    function swap(
        address inputToken,
        uint256 amount,
        address targetToken,
        bytes memory recipient,
        bool withdrawFlag
    ) public {
+       if (inputToken == address(0) || targetToken == address(0)) revert InvalidAddress();
+       if (amount == 0) revert("Invalid amount");
+       if (recipient.length == 0) revert("Invalid recipient");

Committable suggestion skipped: line range outside the PR's diff.

bool success = IZRC20(inputToken).transferFrom(
msg.sender,
address(this),
amount
);
if (!success) {
revert TransferFailed();
}

(uint256 out, address gasZRC20, uint256 gasFee) = handleGasAndSwap(
inputToken,
amount,
targetToken
);
emit TokenSwap(
msg.sender,
recipient,
inputToken,
targetToken,
amount,
out
);
withdraw(
Params({
target: targetToken,
to: recipient,
withdraw: withdrawFlag
}),
msg.sender,
gasFee,
gasZRC20,
out,
inputToken
);
}
Comment on lines +123 to +164
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Use safeTransferFrom for secure token transfers in the swap function.

To handle tokens that do not return a boolean value and to ensure robust error handling, use safeTransferFrom from the SafeERC20 library instead of transferFrom.

Apply this diff to modify the token transfer:

+import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
...
+using SafeERC20 for IERC20;
...
 function swap(
     address inputToken,
     uint256 amount,
     address targetToken,
     bytes memory recipient,
     bool withdrawFlag
 ) public {
-    bool success = IZRC20(inputToken).transferFrom(
-        msg.sender,
-        address(this),
-        amount
-    );
-    if (!success) {
-        revert TransferFailed();
-    }
+    IERC20(inputToken).safeTransferFrom(
+        msg.sender,
+        address(this),
+        amount
+    );

Remove the TransferFailed error definition if it's no longer used:

-error TransferFailed();

Committable suggestion skipped: line range outside the PR's diff.


function handleGasAndSwap(
address inputToken,
uint256 amount,
Expand All @@ -97,55 +188,58 @@ contract Swap is UniversalContract {
swapAmount = amount - inputForGas;
}

uint256 outputAmount = SwapHelperLib.swapExactTokensForTokens(
uint256 out = SwapHelperLib.swapExactTokensForTokens(
uniswapRouter,
inputToken,
swapAmount,
targetToken,
0
);
return (outputAmount, gasZRC20, gasFee);
return (out, gasZRC20, gasFee);
}

function withdraw(
Params memory params,
address sender,
uint256 gasFee,
address gasZRC20,
uint256 outputAmount,
uint256 out,
address inputToken
) public {
if (gasZRC20 == params.target) {
if (
!IZRC20(gasZRC20).approve(
address(gateway),
outputAmount + gasFee
)
) {
revert ApprovalFailed();
if (params.withdraw) {
if (gasZRC20 == params.target) {
if (!IZRC20(gasZRC20).approve(address(gateway), out + gasFee)) {
revert ApprovalFailed();
}
} else {
if (!IZRC20(gasZRC20).approve(address(gateway), gasFee)) {
revert ApprovalFailed();
}
if (!IZRC20(params.target).approve(address(gateway), out)) {
revert ApprovalFailed();
}
}
gateway.withdraw(
abi.encodePacked(params.to),
out,
params.target,
RevertOptions({
revertAddress: address(this),
callOnRevert: true,
abortAddress: address(0),
revertMessage: abi.encode(sender, inputToken),
onRevertGasLimit: gasLimit
})
);
Comment on lines +209 to +233
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Use safeApprove in the withdraw function to ensure safe approvals.

Replace direct approve calls with safeApprove to handle tokens that may not return a boolean value, ensuring the contract operates securely.

Apply this diff to update the approval logic:

 if (params.withdraw) {
     if (gasZRC20 == params.target) {
-        if (!IZRC20(gasZRC20).approve(address(gateway), out + gasFee)) {
-            revert ApprovalFailed();
-        }
+        IERC20(gasZRC20).safeApprove(address(gateway), out + gasFee);
     } else {
-        if (!IZRC20(gasZRC20).approve(address(gateway), gasFee)) {
-            revert ApprovalFailed();
-        }
-        if (!IZRC20(params.target).approve(address(gateway), out)) {
-            revert ApprovalFailed();
-        }
+        IERC20(gasZRC20).safeApprove(address(gateway), gasFee);
+        IERC20(params.target).safeApprove(address(gateway), out);
     }
     gateway.withdraw(
         abi.encodePacked(params.to),
         out,
         params.target,
         RevertOptions({
             revertAddress: address(this),
             callOnRevert: true,
             abortAddress: address(0),
             revertMessage: abi.encode(sender, inputToken),
             onRevertGasLimit: gasLimit
         })
     );
 }

Remove the ApprovalFailed error definition if it's no longer used:

-error ApprovalFailed();

Committable suggestion skipped: line range outside the PR's diff.

} else {
if (!IZRC20(gasZRC20).approve(address(gateway), gasFee)) {
revert ApprovalFailed();
}
if (
!IZRC20(params.target).approve(address(gateway), outputAmount)
) {
revert ApprovalFailed();
bool success = IWETH9(params.target).transfer(
address(uint160(bytes20(params.to))),
out
);
if (!success) {
revert TransferFailed();
}
Comment on lines +209 to 241
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add reentrancy protection to withdraw function

The function modifies state after external calls, which could be vulnerable to reentrancy attacks. Consider using OpenZeppelin's ReentrancyGuard.

+import {ReentrancyGuardUpgradeable} from "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol";

-contract Swap is UniversalContract, Initializable, UUPSUpgradeable, OwnableUpgradeable {
+contract Swap is UniversalContract, Initializable, UUPSUpgradeable, OwnableUpgradeable, ReentrancyGuardUpgradeable {
    // ...
-    function withdraw(
+    function withdraw(
        Params memory params,
        address sender,
        uint256 gasFee,
        address gasZRC20,
        uint256 out,
        address inputToken
-    ) public {
+    ) public nonReentrant {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (params.withdraw) {
if (gasZRC20 == params.target) {
if (!IZRC20(gasZRC20).approve(address(gateway), out + gasFee)) {
revert ApprovalFailed();
}
} else {
if (!IZRC20(gasZRC20).approve(address(gateway), gasFee)) {
revert ApprovalFailed();
}
if (!IZRC20(params.target).approve(address(gateway), out)) {
revert ApprovalFailed();
}
}
gateway.withdraw(
abi.encodePacked(params.to),
out,
params.target,
RevertOptions({
revertAddress: address(this),
callOnRevert: true,
abortAddress: address(0),
revertMessage: abi.encode(sender, inputToken),
onRevertGasLimit: gasLimit
})
);
} else {
if (!IZRC20(gasZRC20).approve(address(gateway), gasFee)) {
revert ApprovalFailed();
}
if (
!IZRC20(params.target).approve(address(gateway), outputAmount)
) {
revert ApprovalFailed();
bool success = IWETH9(params.target).transfer(
address(uint160(bytes20(params.to))),
out
);
if (!success) {
revert TransferFailed();
}
import {ReentrancyGuardUpgradeable} from "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol";
contract Swap is UniversalContract, Initializable, UUPSUpgradeable, OwnableUpgradeable, ReentrancyGuardUpgradeable {
// ...
function withdraw(
Params memory params,
address sender,
uint256 gasFee,
address gasZRC20,
uint256 out,
address inputToken
) public nonReentrant {
if (params.withdraw) {
if (gasZRC20 == params.target) {
if (!IZRC20(gasZRC20).approve(address(gateway), out + gasFee)) {
revert ApprovalFailed();
}
} else {
if (!IZRC20(gasZRC20).approve(address(gateway), gasFee)) {
revert ApprovalFailed();
}
if (!IZRC20(params.target).approve(address(gateway), out)) {
revert ApprovalFailed();
}
}
gateway.withdraw(
abi.encodePacked(params.to),
out,
params.target,
RevertOptions({
revertAddress: address(this),
callOnRevert: true,
abortAddress: address(0),
revertMessage: abi.encode(sender, inputToken),
onRevertGasLimit: gasLimit
})
);
} else {
bool success = IWETH9(params.target).transfer(
address(uint160(bytes20(params.to))),
out
);
if (!success) {
revert TransferFailed();
}

}
gateway.withdraw(
params.to,
outputAmount,
params.target,
RevertOptions({
revertAddress: address(this),
callOnRevert: true,
abortAddress: address(0),
revertMessage: abi.encode(sender, inputToken),
onRevertGasLimit: gasLimit
})
);
}

function onRevert(RevertContext calldata context) external onlyGateway {
Expand All @@ -158,6 +252,7 @@ contract Swap is UniversalContract {
context.amount,
zrc20
);

gateway.withdraw(
abi.encodePacked(sender),
out,
Expand All @@ -175,4 +270,8 @@ contract Swap is UniversalContract {
fallback() external payable {}

receive() external payable {}

function _authorizeUpgrade(
address newImplementation
) internal override onlyOwner {}
}
55 changes: 55 additions & 0 deletions examples/swap/contracts/SwapCompanion.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;

import "@zetachain/protocol-contracts/contracts/evm/GatewayEVM.sol";
import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";

contract SwapCompanion {
using SafeERC20 for IERC20;

error ApprovalFailed();

GatewayEVM public immutable gateway;

constructor(address payable gatewayAddress) {
gateway = GatewayEVM(gatewayAddress);
}

function swapNativeGas(
address universalSwapContract,
address targetToken,
bytes memory recipient,
bool withdraw
) public payable {
gateway.depositAndCall{value: msg.value}(
universalSwapContract,
abi.encode(targetToken, recipient, withdraw),
RevertOptions(msg.sender, false, address(0), "", 0)
);
}

function swapERC20(
address universalSwapContract,
address targetToken,
bytes memory recipient,
uint256 amount,
address asset,
bool withdraw
) public {
IERC20(asset).safeTransferFrom(msg.sender, address(this), amount);
if (!IERC20(asset).approve(address(gateway), amount)) {
revert ApprovalFailed();
}
gateway.depositAndCall(
universalSwapContract,
amount,
asset,
abi.encode(targetToken, recipient, withdraw),
RevertOptions(msg.sender, false, address(0), "", 0)
);
}
Comment on lines +31 to +50
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Use safeApprove instead of approve to ensure safe token approvals.

Since you're utilizing SafeERC20, it's recommended to use safeApprove instead of approve. This handles non-standard ERC20 tokens that do not return a boolean value and eliminates the need for manual error handling.

Apply this diff to modify the approval:

 IERC20(asset).safeTransferFrom(msg.sender, address(this), amount);
-if (!IERC20(asset).approve(address(gateway), amount)) {
-    revert ApprovalFailed();
-}
+IERC20(asset).safeApprove(address(gateway), amount);

Also, remove the ApprovalFailed error definition if it's no longer used:

-error ApprovalFailed();
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
function swapERC20(
address universalSwapContract,
address targetToken,
bytes memory recipient,
uint256 amount,
address asset,
bool withdraw
) public {
IERC20(asset).safeTransferFrom(msg.sender, address(this), amount);
if (!IERC20(asset).approve(address(gateway), amount)) {
revert ApprovalFailed();
}
gateway.depositAndCall(
universalSwapContract,
amount,
asset,
abi.encode(targetToken, recipient, withdraw),
RevertOptions(msg.sender, false, address(0), "", 0)
);
}
function swapERC20(
address universalSwapContract,
address targetToken,
bytes memory recipient,
uint256 amount,
address asset,
bool withdraw
) public {
IERC20(asset).safeTransferFrom(msg.sender, address(this), amount);
IERC20(asset).safeApprove(address(gateway), amount);
gateway.depositAndCall(
universalSwapContract,
amount,
asset,
abi.encode(targetToken, recipient, withdraw),
RevertOptions(msg.sender, false, address(0), "", 0)
);
}


receive() external payable {}

fallback() external payable {}
}
Loading
Loading