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.

Sign up for GitHub

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

feat: check withdraw fee in Swap, callMulti example, universal NFT/FT #229

Open
wants to merge 19 commits into
base: main
Choose a base branch
from
Open

feat: check withdraw fee in Swap, callMulti example, universal NFT/FT #229

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
c7ba7e5
update toolkit
fadeev Jan 10, 2025
142c36c
isInputAmountSufficient
fadeev Jan 13, 2025
926a9f9
switch target token to gasZRC20
fadeev Jan 13, 2025
d07edd7
errors
fadeev Jan 13, 2025
05d2288
update toolkit and networks
fadeev Jan 14, 2025
272329d
deps
fadeev Jan 14, 2025
21cd7be
wip
fadeev Jan 14, 2025
c0a77f4
wip
fadeev Jan 14, 2025
63b4a4b
wip
fadeev Jan 14, 2025
de77a5f
update hardhat config
fadeev Jan 14, 2025
4799aec
hardhat config
fadeev Jan 14, 2025
c181622
hardhat config
fadeev Jan 14, 2025
947d450
hardhat config
fadeev Jan 14, 2025
81325b3
call multi
fadeev Jan 15, 2025
a34b866
comment
fadeev Jan 15, 2025
d8732f7
add nft and token examples
fadeev Jan 15, 2025
0272470
wip
fadeev Jan 15, 2025
468bece
use bash
fadeev Jan 15, 2025
846d720
use bash
fadeev Jan 15, 2025
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
8 changes: 5 additions & 3 deletions examples/call/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
"@typescript-eslint/eslint-plugin": "^5.59.9",
"@typescript-eslint/parser": "^5.59.9",
"@zetachain/localnet": "4.0.0-rc6",
"@zetachain/toolkit": "13.0.0-rc7",
"@zetachain/toolkit": "13.0.0-rc12",
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Version inconsistency detected in example projects

The @zetachain/toolkit package has inconsistent versions across examples:

  • Version 13.0.0-rc12: hello, swap, call
  • Version 13.0.0-rc8: nft, token

Consider updating all examples to use the latest release candidate version 13.0.0-rc12 for consistency.

🔗 Analysis chain

Ensure version consistency across example projects.

The changes introduce multiple release candidate dependencies:

  • Updating @zetachain/toolkit to 13.0.0-rc12
  • Adding @zetachain/networks at 10.0.0-rc4
  • Adding @zetachain/protocol-contracts-solana at 2.0.0-rc1

Run this script to verify version consistency across all example projects:

Also applies to: 60-62

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check version consistency across example projects

# Find all package.json files in example directories
echo "Checking ZetaChain package versions across all examples:"
for pkg in "@zetachain/toolkit" "@zetachain/networks" "@zetachain/protocol-contracts" "@zetachain/protocol-contracts-solana"; do
  echo -e "\nVersions of $pkg:"
  fd --type f "package.json" examples/ --exec jq -r \
    --arg pkg "$pkg" \
    'if .dependencies[$pkg] then "  " + . + ": " + .dependencies[$pkg] elif .devDependencies[$pkg] then "  " + . + ": " + .devDependencies[$pkg] else empty end'
done

Length of output: 3335

Script:

#!/bin/bash
# Description: Check version consistency with corrected jq query

echo "Checking ZetaChain package versions across all examples:"
for pkg in "@zetachain/toolkit" "@zetachain/networks" "@zetachain/protocol-contracts" "@zetachain/protocol-contracts-solana"; do
  echo -e "\nVersions of $pkg:"
  fd --type f "package.json" examples/ --exec sh -c "echo -n '  {}:' && jq -r '.dependencies[\"$pkg\"] // .devDependencies[\"$pkg\"] // \"not found\"' {}"
done

Length of output: 2613

"axios": "^1.3.6",
"chai": "^4.2.0",
"dotenv": "^16.0.3",
Expand Down Expand Up @@ -57,6 +57,8 @@
"@solana-developers/helpers": "^2.4.0",
"@solana/spl-memo": "^0.2.5",
"@solana/web3.js": "^1.95.8",
"@zetachain/protocol-contracts": "11.0.0-rc3"
"@zetachain/networks": "10.0.0-rc4",
"@zetachain/protocol-contracts": "11.0.0-rc3",
"@zetachain/protocol-contracts-solana": "2.0.0-rc1"
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Remove or document unused Solana dependency

The @zetachain/protocol-contracts-solana dependency appears to be unused in the codebase. Either:

  • Remove the dependency if it's not needed
  • Document its intended usage and implement the Solana integration
🔗 Analysis chain

Verify Solana contract integration requirements.

The addition of @zetachain/protocol-contracts-solana suggests new Solana integration capabilities. Ensure this dependency is required and properly documented in the project's README.

Run this script to check for Solana-related configuration and documentation:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for Solana configuration and documentation

# Check for Solana configuration files
echo "Checking for Solana configuration files:"
fd -e yaml -e json "solana" examples/call/

# Check for Solana documentation
echo -e "\nChecking for Solana documentation:"
rg -i "solana" examples/call/README.md || echo "No Solana documentation found in README"

Length of output: 346

Script:

#!/bin/bash
# Check for imports and usage of Solana contracts
echo "Checking for Solana contract imports and usage:"
rg -i "@zetachain/protocol-contracts-solana" examples/call/ -A 2

echo -e "\nChecking for Solana-related code patterns:"
rg -i "solana" examples/call/ --type ts --type js --type tsx --type jsx

Length of output: 1163

}
}
}
1,219 changes: 798 additions & 421 deletions examples/call/yarn.lock
View file
Open in desktop

Large diffs are not rendered by default.

5 changes: 3 additions & 2 deletions examples/hello/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
"@typescript-eslint/eslint-plugin": "^5.59.9",
"@typescript-eslint/parser": "^5.59.9",
"@zetachain/localnet": "4.0.0-rc6",
"@zetachain/toolkit": "13.0.0-rc7",
"@zetachain/toolkit": "13.0.0-rc12",
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Update dependency versions to align with published releases

🔗 Analysis chain

Verify compatibility with release candidate dependencies.

The changes introduce release candidate versions of ZetaChain packages:

  • Updating @zetachain/toolkit to 13.0.0-rc12
  • Adding @zetachain/networks at 10.0.0-rc4

Run this script to check for breaking changes and compatibility:

Also applies to: 60-60

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for breaking changes in the release candidates

# Check the changelog or release notes for breaking changes
gh api repos/zeta-chain/toolkit/releases | jq -r '.[].body' | grep -i "breaking"
gh api repos/zeta-chain/networks/releases | jq -r '.[].body' | grep -i "breaking"

# Check if these versions are the latest available
for pkg in "@zetachain/toolkit" "@zetachain/networks"; do
  echo "Checking latest version for $pkg:"
  npm view "$pkg" versions --json | jq -r '.[]' | tail -n 5
done

Length of output: 737

"axios": "^1.3.6",
"chai": "^4.2.0",
"dotenv": "^16.0.3",
Expand Down Expand Up @@ -57,6 +57,7 @@
"@solana-developers/helpers": "^2.4.0",
"@solana/spl-memo": "^0.2.5",
"@solana/web3.js": "^1.95.8",
"@zetachain/networks": "10.0.0-rc4",
"@zetachain/protocol-contracts": "11.0.0-rc3"
}
}
}
1,219 changes: 798 additions & 421 deletions examples/hello/yarn.lock
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions examples/swap/.gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,5 @@ cache_forge
access_token

localnet.json

.openzeppelin
69 changes: 66 additions & 3 deletions examples/swap/contracts/Swap.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {SystemContract, IZRC20} from "@zetachain/toolkit/contracts/SystemContrac
import {SwapHelperLib} from "@zetachain/toolkit/contracts/SwapHelperLib.sol";
import {BytesHelperLib} from "@zetachain/toolkit/contracts/BytesHelperLib.sol";
import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import "@uniswap/v2-periphery/contracts/interfaces/IUniswapV2Router02.sol";

import {RevertContext, RevertOptions} from "@zetachain/protocol-contracts/contracts/Revert.sol";
import "@zetachain/protocol-contracts/contracts/zevm/interfaces/UniversalContract.sol";
Expand All @@ -31,7 +32,8 @@ contract Swap is
error InvalidAddress();
error Unauthorized();
error ApprovalFailed();
error TransferFailed();
error TransferFailed(string);
error InsufficientAmount(string);
fadeev marked this conversation as resolved.
Show resolved Hide resolved

event TokenSwap(
address sender,
Expand Down Expand Up @@ -73,6 +75,9 @@ contract Swap is
bool withdraw;
}

/**
* @notice Swap tokens from a connected chain to another connected chain or ZetaChain
*/
function onCall(
MessageContext calldata context,
address zrc20,
Expand Down Expand Up @@ -120,6 +125,9 @@ contract Swap is
withdraw(params, context.sender, gasFee, gasZRC20, out, zrc20);
}

/**
* @notice Swap tokens from ZetaChain optionally withdrawing to a connected chain
*/
function swap(
address inputToken,
uint256 amount,
Expand All @@ -133,7 +141,9 @@ contract Swap is
amount
);
if (!success) {
revert TransferFailed();
revert TransferFailed(
"Failed to transfer ZRC-20 tokens from the sender to the contract"
);
fadeev marked this conversation as resolved.
Show resolved Hide resolved
}

(uint256 out, address gasZRC20, uint256 gasFee) = handleGasAndSwap(
Expand Down Expand Up @@ -163,6 +173,9 @@ contract Swap is
);
}

/**
* @notice Swaps enough tokens to pay gas fees, then swaps the remainder to the target token
*/
function handleGasAndSwap(
address inputToken,
uint256 amount,
Expand All @@ -175,6 +188,13 @@ contract Swap is

(gasZRC20, gasFee) = IZRC20(targetToken).withdrawGasFee();

uint256 minInput = quoteMinInput(inputToken, targetToken);
if (amount < minInput) {
revert InsufficientAmount(
"The input amount is less than the min amount required to cover the withdraw gas fee"
);
Comment on lines +193 to +195
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Provide detailed error information using fixed-size data types

To offer precise feedback while maintaining gas efficiency, update the InsufficientAmount error and the corresponding revert statement to include the required and provided amounts as uint256 parameters.

Apply this diff:

             if (amount < minInput) {
-                revert InsufficientAmount(
-                    "The input amount is less than the min amount required to cover the withdraw gas fee"
-                );
+                revert InsufficientAmount(minInput, amount);
             }

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

}

if (gasZRC20 == inputToken) {
swapAmount = amount - gasFee;
} else {
Expand All @@ -198,6 +218,9 @@ contract Swap is
return (out, gasZRC20, gasFee);
}

/**
* @notice Transfer tokens to the recipient on ZetaChain or withdraw to a connected chain
*/
function withdraw(
Params memory params,
address sender,
Expand Down Expand Up @@ -237,11 +260,17 @@ contract Swap is
out
);
if (!success) {
revert TransferFailed();
revert TransferFailed(
Copy link
Collaborator

Choose a reason for hiding this comment

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

I should check but the good part of custom errors is that reduce extremely bytecode size because uses the hash instead of strings. if you hardcode a string as msg IMO will store the same size we don't want to... params in custom errors should be to extra info like values, not messages. If you want to differentiate two errores create a new one.

Copy link
Member Author

Choose a reason for hiding this comment

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

The problem with custom errors without messages is that when looking up a tx, it shows the error as 0xcfd6249e, for example. Unless you know what this selector maps to, it's hard to say why a call failed. See:

https://zetachain-athens.blockpi.network/lcd/v1/public/zeta-chain/crosschain/inboundHashToCctxData/0x3638ba14874ac71fbacd68614ba3ff169301269149678a3c01572187a17db872

A message on the other hand can be easily decoded.

Bytecode size isn't particularly important on ZetaChain, because the gas costs are low.

"Failed to transfer target tokens to the recipient on ZetaChain"
);
Comment on lines +263 to +265
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Align revert statement with updated TransferFailed error

Following the optimization of the TransferFailed error, update this revert statement to match the new definition and enhance gas efficiency.

Apply this diff:

                 if (!success) {
-                    revert TransferFailed(
-                        "Failed to transfer target tokens to the recipient on ZetaChain"
-                    );
+                    revert TransferFailed();
                 }
📝 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
revert TransferFailed(
"Failed to transfer target tokens to the recipient on ZetaChain"
);
revert TransferFailed();

}
}
}

/**
* @notice onRevert handles an edge-case when a swap fails when the recipient
* on the destination chain is a contract that cannot accept tokens.
*/
function onRevert(RevertContext calldata context) external onlyGateway {
(address sender, address zrc20) = abi.decode(
context.revertMessage,
Expand All @@ -267,6 +296,40 @@ contract Swap is
);
}

/**
* @notice Returns the minimum amount of input tokens required to cover the gas fee for withdrawal
*/
function quoteMinInput(
address inputToken,
address targetToken
) public view returns (uint256) {
(address gasZRC20, uint256 gasFee) = IZRC20(targetToken)
.withdrawGasFee();

if (inputToken == gasZRC20) {
return gasFee;
}

address zeta = IUniswapV2Router01(uniswapRouter).WETH();

address[] memory path;
if (inputToken == zeta || gasZRC20 == zeta) {
path = new address[](2);
path[0] = inputToken;
path[1] = gasZRC20;
} else {
path = new address[](3);
path[0] = inputToken;
path[1] = zeta;
path[2] = gasZRC20;
}

uint256[] memory amountsIn = IUniswapV2Router02(uniswapRouter)
.getAmountsIn(gasFee, path);

return amountsIn[0];
}

function _authorizeUpgrade(
address newImplementation
) internal override onlyOwner {}
Expand Down
4 changes: 2 additions & 2 deletions examples/swap/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@
"@solana-developers/helpers": "^2.4.0",
"@solana/spl-memo": "^0.2.5",
"@solana/web3.js": "^1.95.8",
"@zetachain/protocol-contracts": "11.0.0-rc3",
"@zetachain/toolkit": "13.0.0-rc8"
"@zetachain/networks": "10.0.0-rc4",
"@zetachain/toolkit": "13.0.0-rc12"
}
}
6 changes: 5 additions & 1 deletion examples/swap/scripts/localnet.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,16 +22,18 @@ CONTRACT_SWAP=$(npx hardhat deploy --name Swap --network localhost --gateway "$G
COMPANION=$(npx hardhat deploy-companion --gateway "$GATEWAY_ETHEREUM" --network localhost --json | jq -r '.contractAddress')

npx hardhat companion-swap \
--skip-checks \
--network localhost \
--contract "$COMPANION" \
--universal-contract "$CONTRACT_SWAP" \
--amount 1 \
--target "$ZRC20_BNB" \
--recipient "$SENDER"
--recipient "$SENDER"

npx hardhat localnet-check

npx hardhat companion-swap \
--skip-checks \
--network localhost \
--contract "$COMPANION" \
--universal-contract "$CONTRACT_SWAP" \
Expand All @@ -43,6 +45,7 @@ npx hardhat companion-swap \
npx hardhat localnet-check

npx hardhat evm-swap \
--skip-checks \
--network localhost \
--receiver "$CONTRACT_SWAP" \
--amount 1 \
Expand All @@ -52,6 +55,7 @@ npx hardhat evm-swap \
npx hardhat localnet-check

npx hardhat evm-swap \
--skip-checks \
--network localhost \
--receiver "$CONTRACT_SWAP" \
--amount 1 \
Expand Down
15 changes: 15 additions & 0 deletions examples/swap/tasks/companionSwap.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import ERC20_ABI from "@openzeppelin/contracts/build/contracts/ERC20.json";
import { task, types } from "hardhat/config";
import type { HardhatRuntimeEnvironment } from "hardhat/types";
import { isInputAmountSufficient } from "./evmSwap";
import { ZetaChainClient } from "@zetachain/toolkit/client";

const main = async (args: any, hre: HardhatRuntimeEnvironment) => {
const { ethers } = hre;
Expand All @@ -9,6 +11,18 @@ const main = async (args: any, hre: HardhatRuntimeEnvironment) => {
const factory = (await hre.ethers.getContractFactory("SwapCompanion")) as any;
const contract = factory.attach(args.contract).connect(signer);

const client = new ZetaChainClient({ network: "testnet", signer });

if (!args.skipChecks) {
await isInputAmountSufficient({
hre,
client,
amount: args.amount,
erc20: args.erc20,
target: args.target,
});
}

let tx;
if (args.erc20) {
const erc20Contract = new ethers.Contract(
Expand Down Expand Up @@ -58,4 +72,5 @@ task("companion-swap", "Swap native gas tokens", main)
.addOptionalParam("erc20", "ERC-20 token address")
.addParam("target", "ZRC-20 address of the token to swap for")
.addParam("amount", "Amount of tokens to swap")
.addFlag("skipChecks", "Skip checks for minimum amount")
.addParam("recipient", "Recipient address");
39 changes: 39 additions & 0 deletions examples/swap/tasks/evmSwap.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,51 @@ import type { HardhatRuntimeEnvironment } from "hardhat/types";

import { ZetaChainClient } from "@zetachain/toolkit/client";

export const isInputAmountSufficient = async ({
hre,
client,
erc20,
amount,
target,
}: any) => {
Comment on lines +6 to +12
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 proper TypeScript types instead of using 'any'.

Replace the 'any' type with a proper interface definition for better type safety and code maintainability.

+interface InputAmountCheckParams {
+  hre: HardhatRuntimeEnvironment;
+  client: ZetaChainClient;
+  erc20?: string;
+  amount: string;
+  target: string;
+}

-export const isInputAmountSufficient = async ({
-  hre,
-  client,
-  erc20,
-  amount,
-  target,
-}: any) => {
+export const isInputAmountSufficient = async ({
+  hre,
+  client,
+  erc20,
+  amount,
+  target,
+}: InputAmountCheckParams) => {

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

const inputZRC20 = await (erc20
? client.getZRC20FromERC20(erc20)
: client.getZRC20GasToken(hre.network.name));

const minAmount = await client.getWithdrawFeeInInputToken(inputZRC20, target);

const minAmountFormatted = hre.ethers.utils.formatUnits(
minAmount.amount,
minAmount.decimals
);

const value = hre.ethers.utils.parseUnits(amount, minAmount.decimals);
Comment on lines +19 to +24
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Ensure consistent decimal units when parsing amounts

When parsing the amount, you use minAmount.decimals as the decimal parameter. This assumes that the input token and the minAmount share the same decimals, which might not be the case if their decimals differ. To prevent incorrect amount comparisons due to mismatched decimal units, retrieve the decimals of the input token (inputZRC20) and use it consistently.

Apply this diff:

   const minAmountFormatted = hre.ethers.utils.formatUnits(
     minAmount.amount,
-    minAmount.decimals
+    inputTokenDecimals
   );

-  const value = hre.ethers.utils.parseUnits(amount, minAmount.decimals);
+  const value = hre.ethers.utils.parseUnits(amount, inputTokenDecimals);

Ensure that inputTokenDecimals corresponds to the decimals of inputZRC20, which can be retrieved as follows:

const inputTokenDecimals = minAmount.decimals; // Or fetch from inputZRC20 if necessary


if (value.lt(minAmount.amount)) {
throw new Error(
`Input amount ${amount} is less than minimum amount ${minAmountFormatted} required for a withdrawal to the destination chain`
);
}
};

export const evmDepositAndCall = async (
args: any,
hre: HardhatRuntimeEnvironment
) => {
try {
const [signer] = await hre.ethers.getSigners();
const client = new ZetaChainClient({ network: "testnet", signer });

if (!args.skipChecks) {
await isInputAmountSufficient({
hre,
client,
amount: args.amount,
erc20: args.erc20,
target: args.target,
});
}

const tx = await client.evmDepositAndCall({
amount: args.amount,
erc20: args.erc20,
Expand Down Expand Up @@ -72,6 +110,7 @@ task("evm-swap", "Swap tokens from EVM", evmDepositAndCall)
.addOptionalParam("revertMessage", "Revert message", "0x")
.addParam("amount", "amount of ETH to send with the transaction")
.addOptionalParam("erc20", "ERC-20 token address")
.addFlag("skipChecks", "Skip checks for minimum amount")
.addParam("target", "ZRC-20 address of the token to swap for")
.addParam("recipient", "Recipient address")
.addOptionalParam(
Expand Down
Loading
Loading