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 1 commit
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
123 changes: 88 additions & 35 deletions examples/swap/contracts/Swap.sol
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ 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 {
Expand All @@ -20,6 +21,7 @@ contract Swap is UniversalContract {
error InvalidAddress();
error Unauthorized();
error ApprovalFailed();
error TransferFailed();

modifier onlyGateway() {
if (msg.sender != address(gateway)) revert Unauthorized();
Expand All @@ -41,6 +43,7 @@ contract Swap is UniversalContract {
struct Params {
address target;
bytes to;
bool withdraw;
}

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

if (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(
Expand All @@ -72,6 +85,42 @@ contract Swap is UniversalContract {
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
);

withdraw(
Params({
target: targetToken,
to: recipient,
withdraw: withdrawFlag
}),
msg.sender,
gasFee,
gasZRC20,
out,
inputToken
);
}

function handleGasAndSwap(
address inputToken,
uint256 amount,
Expand All @@ -97,55 +146,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 +210,7 @@ contract Swap is UniversalContract {
context.amount,
zrc20
);

gateway.withdraw(
abi.encodePacked(sender),
out,
Expand Down
51 changes: 51 additions & 0 deletions examples/swap/contracts/SwapCompanion.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
// 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;

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);
IERC20(asset).approve(address(gateway), amount);
gateway.depositAndCall(
universalSwapContract,
amount,
asset,
abi.encode(targetToken, recipient, withdraw),
RevertOptions(msg.sender, false, address(0), "", 0)
);
}
Fixed Show fixed Hide fixed

receive() external payable {}

fallback() external payable {}
}
Loading
Loading