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

Update Hello and Swap to authenticated calls #218

Merged
merged 61 commits into from
Nov 24, 2024
Merged
Show file tree
Hide file tree
Changes from 54 commits
Commits
Show all changes
61 commits
Select commit Hold shift + click to select a range
2b827bc
Update hello to auth calls
fadeev Nov 8, 2024
afa11f1
remove omnichain examples
fadeev Nov 8, 2024
b6a444d
wip
fadeev Nov 8, 2024
a5e15a1
swap works
fadeev Nov 8, 2024
50e7217
swap
fadeev Nov 9, 2024
f8357ad
deps and remove omnichain swap
fadeev Nov 11, 2024
b0f3d1b
nft: update toolkit
fadeev Nov 11, 2024
3150e9c
nft: sender and update localnet
fadeev Nov 11, 2024
d687d9c
hello: rename contracts to universal and connected
fadeev Nov 11, 2024
ff28b85
rename hello to calls
fadeev Nov 11, 2024
e5f6c5f
Merge branch 'main' into auth-call-hello-swap
fadeev Nov 11, 2024
b6886bf
rename to call
fadeev Nov 11, 2024
d420ab7
rename to call
fadeev Nov 11, 2024
936d399
call: depositAndCall
fadeev Nov 11, 2024
7476874
call: add deposit function
fadeev Nov 11, 2024
c2410f0
call: universal withdraw
fadeev Nov 11, 2024
ed3e800
call: withdraw and call arbitrary
fadeev Nov 11, 2024
68217a5
call: fix withdraw and call
fadeev Nov 12, 2024
c5e2348
onCall: remove return
fadeev Nov 12, 2024
5e05fb0
call: deposit/depositAndCall ERC-20s
fadeev Nov 12, 2024
deffb4d
returns bytes 4
fadeev Nov 12, 2024
382e08c
slither
fadeev Nov 12, 2024
f05a97c
call: reverts
fadeev Nov 12, 2024
f7b583a
hello example
fadeev Nov 13, 2024
5eb0da9
remove unused improts
fadeev Nov 13, 2024
1d72aee
slither
fadeev Nov 13, 2024
5b6234f
wip
fadeev Nov 15, 2024
44f7d72
nft: fix localnet
fadeev Nov 15, 2024
57ceeb8
rename test.sh to localnet.sh
fadeev Nov 16, 2024
6513b52
rename test.sh to localnet.sh
fadeev Nov 16, 2024
c1ab35e
rename test.sh to localnet.sh
fadeev Nov 16, 2024
8331f22
token: consistency
fadeev Nov 17, 2024
01700ef
wip
fadeev Nov 17, 2024
29bb170
fix
fadeev Nov 17, 2024
0b7ac4c
immutable
fadeev Nov 17, 2024
34e87b4
import types
fadeev Nov 17, 2024
822cb23
fix revert
fadeev Nov 17, 2024
85f91be
nft: rename sender to receiver
fadeev Nov 18, 2024
1495357
replace system contract with uniswap router
fadeev Nov 18, 2024
1df6664
token: deploy task
fadeev Nov 18, 2024
f5ad522
token: replace system contract with uniswap router
fadeev Nov 18, 2024
051ee41
default addresses
fadeev Nov 18, 2024
cfaf6fb
default addresses
fadeev Nov 18, 2024
9985dc7
add localnet to gitignore
fadeev Nov 18, 2024
757c438
remove default uniswap address
fadeev Nov 18, 2024
0cbaa24
nft: withdrawAndCall
fadeev Nov 18, 2024
13f3299
nft: return extra tokens to sender, handle a case where receiver is n…
fadeev Nov 18, 2024
75438c3
token: return to sender
fadeev Nov 18, 2024
aa55fa3
nft/token: handle revert on zetachain
fadeev Nov 18, 2024
fd03f00
fix
fadeev Nov 18, 2024
91b8121
remove placeholder event
fadeev Nov 18, 2024
1bda9b7
token: zero address validation
fadeev Nov 19, 2024
84beed4
swap: zero address validation
fadeev Nov 19, 2024
4cfaa55
immutable gas limit
fadeev Nov 19, 2024
16d8c13
rename counterparty
fadeev Nov 20, 2024
5013b00
rename counterparty
fadeev Nov 20, 2024
6a15652
rename counterparty
fadeev Nov 20, 2024
c89c6da
gateway revert
fadeev Nov 20, 2024
786d629
gateway revert
fadeev Nov 20, 2024
a6d9809
safeTransferFrom
fadeev Nov 20, 2024
83ab10b
revert
fadeev Nov 20, 2024
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
43 changes: 0 additions & 43 deletions .github/workflows/publish-npm.yml

This file was deleted.

8 changes: 6 additions & 2 deletions .github/workflows/slither.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,14 @@ jobs:
include:
- project: "examples/hello"
file: "hello.sarif"
- project: "examples/call"
file: "call.sarif"
- project: "examples/swap"
file: "swap.sarif"
- project: "omnichain/swap"
file: "omnichain-swap.sarif"
- project: "examples/nft"
file: "nft.sarif"
- project: "examples/token"
file: "token.sarif"
permissions:
contents: read
security-events: write
Expand Down
6 changes: 3 additions & 3 deletions .github/workflows/test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ jobs:
- name: Set up Test Matrix
id: set-matrix
run: |
test_dirs=$(find examples/*/scripts -type f -name 'test.sh' -exec dirname {} \; | xargs dirname)
test_dirs=$(find examples/*/scripts -type f -name 'localnet.sh' -exec dirname {} \; | xargs dirname)
matrix_json=$(echo "$test_dirs" | jq -R '{"example-dir": .}' | jq -s . | jq -c .)
echo "matrix=$matrix_json" >> $GITHUB_OUTPUT

Expand All @@ -39,5 +39,5 @@ jobs:
run: |
cd "${{ matrix.example-dir }}"
yarn
chmod +x ./scripts/test.sh
./scripts/test.sh localnet
chmod +x ./scripts/localnet.sh
./scripts/localnet.sh start
File renamed without changes.
File renamed without changes.
4 changes: 3 additions & 1 deletion omnichain/swap/.gitignore → examples/call/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,6 @@ artifacts
out
cache_forge

access_token
access_token

localnet.json
File renamed without changes.
3 changes: 3 additions & 0 deletions examples/call/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# Hello Example

Tutorial: https://www.zetachain.com/docs/developers/tutorials/call/
104 changes: 104 additions & 0 deletions examples/call/contracts/Connected.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;

import {RevertContext} from "@zetachain/protocol-contracts/contracts/Revert.sol";
import "@zetachain/protocol-contracts/contracts/evm/GatewayEVM.sol";

contract Connected {
GatewayEVM public immutable gateway;

event RevertEvent(string, RevertContext);
event HelloEvent(string, string);

error TransferFailed();
error ApprovalFailed();

modifier onlyGateway() {
require(msg.sender == address(gateway), "Caller is not the gateway");
andresaiello marked this conversation as resolved.
Show resolved Hide resolved
_;
}

constructor(address payable gatewayAddress) {
gateway = GatewayEVM(gatewayAddress);
}
Comment on lines +23 to +25
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 zero address validation in constructor.

The constructor should validate that the gateway address is not zero to prevent deployment with an invalid gateway.

     constructor(address payable gatewayAddress) {
+        if (gatewayAddress == address(0)) revert("Zero address");
         gateway = GatewayEVM(gatewayAddress);
     }
📝 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
constructor(address payable gatewayAddress) {
gateway = GatewayEVM(gatewayAddress);
}
constructor(address payable gatewayAddress) {
if (gatewayAddress == address(0)) revert("Zero address");
gateway = GatewayEVM(gatewayAddress);
}


function call(
address receiver,
bytes calldata message,
RevertOptions memory revertOptions
) external {
gateway.call(receiver, message, revertOptions);
}

function deposit(
address receiver,
RevertOptions memory revertOptions
) external payable {
gateway.deposit{value: msg.value}(receiver, revertOptions);
}

function deposit(
address receiver,
uint256 amount,
address asset,
RevertOptions memory revertOptions
) external {
if (!IERC20(asset).transferFrom(msg.sender, address(this), amount)) {
andresaiello marked this conversation as resolved.
Show resolved Hide resolved
revert TransferFailed();
}
if (!IERC20(asset).approve(address(gateway), amount)) {
revert ApprovalFailed();
}
gateway.deposit(receiver, amount, asset, revertOptions);
}

function depositAndCall(
address receiver,
uint256 amount,
address asset,
bytes calldata message,
RevertOptions memory revertOptions
) external {
if (!IERC20(asset).transferFrom(msg.sender, address(this), amount)) {
andresaiello marked this conversation as resolved.
Show resolved Hide resolved
revert TransferFailed();
}
if (!IERC20(asset).approve(address(gateway), amount)) {
revert ApprovalFailed();
}
gateway.depositAndCall(receiver, amount, asset, message, revertOptions);
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Use SafeERC20 for token operations.

The current implementation uses unsafe ERC20 operations. Some tokens (like USDT) don't follow the standard return value pattern, which could cause the transfers to fail silently.

+import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
+import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";

 contract Connected {
+    using SafeERC20 for IERC20;
     // ...
     function deposit(
         address receiver,
         uint256 amount,
         address asset,
         RevertOptions memory revertOptions
     ) external {
-        if (!IERC20(asset).transferFrom(msg.sender, address(this), amount)) {
-            revert TransferFailed();
-        }
-        if (!IERC20(asset).approve(address(gateway), amount)) {
-            revert ApprovalFailed();
-        }
+        IERC20(asset).safeTransferFrom(msg.sender, address(this), amount);
+        IERC20(asset).safeApprove(address(gateway), amount);
         gateway.deposit(receiver, amount, asset, revertOptions);
     }

Apply similar changes to the depositAndCall function.

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


function depositAndCall(
address receiver,
bytes calldata message,
RevertOptions memory revertOptions
) external payable {
gateway.depositAndCall{value: msg.value}(
receiver,
message,
revertOptions
);
}

function hello(string memory message) external payable {
emit HelloEvent("Hello on EVM", message);
}

function onCall(
MessageContext calldata context,
bytes calldata message
) external payable onlyGateway returns (bytes4) {
emit HelloEvent("Hello on EVM from onCall()", "hey");
return "";
}
Comment on lines +81 to +87
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Return value should be IMessageDispatcher.onCall.selector

The empty bytes4 return value in the onCall function is incorrect. Based on the codebase analysis, this function is a callback from the gateway contract and should return the function selector for IMessageDispatcher.onCall to indicate successful processing.

🔗 Analysis chain

Verify the expected return value for onCall.

The function returns an empty bytes4 value. Verify if this is the expected return value by the gateway contract.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check the expected return value in the gateway contract

# Search for onCall function definition in the gateway contract
ast-grep --pattern 'function onCall($$$) $$ returns (bytes4) { $$$ }'

Length of output: 71


Script:

#!/bin/bash
# Let's try a broader search for gateway interfaces or contracts
rg -l "gateway|Gateway" --type sol

# Then look for any interface or contract definitions related to gateway callbacks
ast-grep --pattern 'interface $_ {
  $$$
  function onCall($$$)
  $$$
}'

# Also search for any documentation or comments about gateway callbacks
rg -i "oncall.*return" --type sol -A 2 -B 2

Length of output: 213


Script:

#!/bin/bash
# Search for any files with .sol extension
fd -e sol

# Then search for gateway-related content in all files
rg "gateway|Gateway" -A 5 -B 5

# Look for onCall implementations
rg "function onCall" -A 5 -B 5

Length of output: 82222


function onRevert(
RevertContext calldata revertContext
) external onlyGateway {
emit RevertEvent("Revert on EVM", revertContext);
}

receive() external payable {}

fallback() external payable {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import "@zetachain/protocol-contracts/contracts/zevm/interfaces/IGatewayZEVM.sol";
import "@zetachain/protocol-contracts/contracts/zevm/GatewayZEVM.sol";

contract Hello is UniversalContract {
contract Universal is UniversalContract {
GatewayZEVM public immutable gateway;

event HelloEvent(string, string);
Expand All @@ -22,46 +22,56 @@
gateway = GatewayZEVM(gatewayAddress);
}

function onCall(
MessageContext calldata context,
address zrc20,
uint256 amount,
bytes calldata message
) external override onlyGateway {
string memory name = abi.decode(message, (string));
emit HelloEvent("Hello on ZetaChain", name);
}

function onRevert(
RevertContext calldata revertContext
) external override onlyGateway {
emit RevertEvent("Revert on ZetaChain", revertContext);
}

function call(
bytes memory receiver,
address zrc20,
bytes calldata message,
uint256 gasLimit,
CallOptions memory callOptions,
RevertOptions memory revertOptions
) external {
(, uint256 gasFee) = IZRC20(zrc20).withdrawGasFeeWithGasLimit(gasLimit);
(, uint256 gasFee) = IZRC20(zrc20).withdrawGasFeeWithGasLimit(
callOptions.gasLimit
);
if (!IZRC20(zrc20).transferFrom(msg.sender, address(this), gasFee))
revert TransferFailed();
IZRC20(zrc20).approve(address(gateway), gasFee);
gateway.call(receiver, zrc20, message, gasLimit, revertOptions);
gateway.call(receiver, zrc20, message, callOptions, revertOptions);
}

function withdraw(
bytes memory receiver,
uint256 amount,
address zrc20,
RevertOptions memory revertOptions
) external {
(address gasZRC20, uint256 gasFee) = IZRC20(zrc20).withdrawGasFee();
uint256 target = zrc20 == gasZRC20 ? amount + gasFee : amount;
if (!IZRC20(zrc20).transferFrom(msg.sender, address(this), target))
revert TransferFailed();
IZRC20(zrc20).approve(address(gateway), target);
if (zrc20 != gasZRC20) {
if (
andresaiello marked this conversation as resolved.
Show resolved Hide resolved
!IZRC20(gasZRC20).transferFrom(
msg.sender,
address(this),
gasFee
)
) revert TransferFailed();
IZRC20(gasZRC20).approve(address(gateway), gasFee);
}
gateway.withdraw(receiver, amount, zrc20, revertOptions);
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Critical: Handle approve return values to prevent silent failures.

The approve calls' return values are not checked, which could lead to silent failures. This is particularly critical for cross-chain operations.

Apply this pattern to all approve calls in the contract:

-        IZRC20(zrc20).approve(address(gateway), target);
+        if (!IZRC20(zrc20).approve(address(gateway), target)) {
+            revert TransferFailed();
+        }

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

🧰 Tools
🪛 GitHub Check: Slither

[warning] 43-65: Unused return
Universal.withdraw(bytes,uint256,address,RevertOptions) (contracts/Universal.sol#43-65) ignores return value by IZRC20(gasZRC20).approve(address(gateway),gasFee) (contracts/Universal.sol#62)


[warning] 43-65: Unused return
Universal.withdraw(bytes,uint256,address,RevertOptions) (contracts/Universal.sol#43-65) ignores return value by IZRC20(zrc20).approve(address(gateway),target) (contracts/Universal.sol#53)

}

Check warning

Code scanning / Slither

Unused return Medium

Check warning

Code scanning / Slither

Unused return Medium


function withdrawAndCall(
bytes memory receiver,
uint256 amount,
address zrc20,
bytes calldata message,
uint256 gasLimit,
CallOptions memory callOptions,
RevertOptions memory revertOptions
) external {
(address gasZRC20, uint256 gasFee) = IZRC20(zrc20)
.withdrawGasFeeWithGasLimit(gasLimit);
.withdrawGasFeeWithGasLimit(callOptions.gasLimit);
uint256 target = zrc20 == gasZRC20 ? amount + gasFee : amount;
if (!IZRC20(zrc20).transferFrom(msg.sender, address(this), target))
revert TransferFailed();
Expand All @@ -81,8 +91,24 @@
amount,
zrc20,
message,
gasLimit,
callOptions,
revertOptions
);
}

function onCall(
MessageContext calldata context,
address zrc20,
uint256 amount,
bytes calldata message
) external override onlyGateway {
string memory name = abi.decode(message, (string));
emit HelloEvent("Hello on ZetaChain", name);
}

function onRevert(
RevertContext calldata revertContext
) external onlyGateway {
emit RevertEvent("Revert on ZetaChain", revertContext);
}
}
File renamed without changes.
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import "./tasks/deploy";
import "./tasks/solana/interact";
import "./tasks/interact";
import "./tasks/swap";
import "./tasks/universalCall";
import "./tasks/connectedCall";
import "./tasks/connectedDeposit";
import "./tasks/connectedDepositAndCall";
import "./tasks/universalWithdraw";
import "./tasks/universalWithdrawAndCall";
import "@zetachain/localnet/tasks";
import "@nomicfoundation/hardhat-toolbox";
import "@zetachain/toolkit/tasks";
Expand All @@ -13,15 +16,7 @@ const config: HardhatUserConfig = {
networks: {
...getHardhatConfigNetworks(),
},
solidity: {
compilers: [
{ version: "0.5.10" /** For create2 factory */ },
{ version: "0.6.6" /** For uniswap v2 router*/ },
{ version: "0.5.16" /** For uniswap v2 core*/ },
{ version: "0.4.19" /** For weth*/ },
{ version: "0.8.7" },
],
},
solidity: "0.8.26",
};

export default config;
11 changes: 5 additions & 6 deletions omnichain/swap/package.json → examples/call/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
"test": "echo \"Error: no test specified\" && exit 1",
"lint:fix": "npx eslint . --ext .js,.ts --fix",
"lint": "npx eslint . --ext .js,.ts",
"deploy": "npx hardhat compile --force && npx hardhat deploy --network localhost --name Hello && npx hardhat deploy --network localhost --name ReceiverContract"
"deploy:localnet": "npx hardhat compile --force && npx hardhat deploy --network localhost --gateway 0x9A676e781A523b5d0C0e43731313A708CB607508 && npx hardhat deploy --name Echo --network localhost --gateway 0x9fE46736679d2D9a65F0992F2272dE9f3c7fa6e0"
},
"keywords": [],
"author": "",
Expand All @@ -28,9 +28,8 @@
"@types/node": ">=12.0.0",
"@typescript-eslint/eslint-plugin": "^5.59.9",
"@typescript-eslint/parser": "^5.59.9",
"@zetachain/localnet": "^1.0.1",
"@zetachain/protocol-contracts": "9.0.0",
"@zetachain/toolkit": "^10.0.0",
"@zetachain/localnet": "4.0.0-rc6",
"@zetachain/toolkit": "13.0.0-rc7",
"axios": "^1.3.6",
"chai": "^4.2.0",
"dotenv": "^16.0.3",
Expand All @@ -55,9 +54,9 @@
"packageManager": "[email protected]+sha1.1959a18351b811cdeedbd484a8f86c3cc3bbaf72",
"dependencies": {
"@coral-xyz/anchor": "0.30.0",
"@openzeppelin/contracts": "^4.9.6",
"@solana-developers/helpers": "^2.4.0",
"@solana/spl-memo": "^0.2.5",
"@solana/web3.js": "^1.95.2"
"@solana/web3.js": "^1.95.2",
"@zetachain/protocol-contracts": "11.0.0-rc3"
}
}
Loading
Loading