-
Notifications
You must be signed in to change notification settings - Fork 59
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
feat: add zeta handling to v2 contracts #214
Changes from 88 commits
1f2625e
bd71007
3553a8f
791834b
c1bd47d
6aed43c
8a973c1
d562400
555ff9d
21b8ace
484096a
a0c8e5a
5b893d2
776caef
704382b
5e4766c
ce87491
049f956
278c327
9303311
cb0ca6c
b9e5bfd
9a785bc
430992a
a09cc0c
b39bc57
4ee9023
d784b8e
7e3ad3e
0b1f894
7db93dd
69d5763
022fee2
1e8af88
d94d8ff
290a84e
5817d9a
f7d18fd
51ff6a2
b857b9c
cbd7dcf
da9c0cd
6bd8c16
f2a3371
cc93e3e
5a5f76b
941949e
6275282
8c33743
fa96e30
b5a665a
badd4f8
d158c71
49223b2
4275805
caae687
0c1b370
76f284d
e62b051
b69926f
bb27a54
d80596e
e559c31
d0ad2f5
354e45a
3190026
5524b59
4f1b2c2
86b4d77
e881d7c
c4f89f8
4613a15
a9ff420
c845f68
065c096
4e3c0c3
00a835b
1925dd0
8e63c45
01648d1
b8cc11b
0e03d10
f3c9a0b
8036d17
0df0624
8457ec3
4875824
399f318
6592586
65b25d3
718787f
09e5d90
deee4d3
eb501de
572646b
141bafe
a68698e
61d2c91
b4b0ce3
6c50fae
1cc8ad5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -15,16 +15,21 @@ | |||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
address public custody; | ||||||||||||||||||||||||||||||||||||||||||||
address public tssAddress; | ||||||||||||||||||||||||||||||||||||||||||||
address public zetaConnector; | ||||||||||||||||||||||||||||||||||||||||||||
address public zeta; | ||||||||||||||||||||||||||||||||||||||||||||
skosito marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
constructor() {} | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
function initialize(address _tssAddress) public initializer { | ||||||||||||||||||||||||||||||||||||||||||||
function initialize(address _tssAddress, address _zeta) public initializer { | ||||||||||||||||||||||||||||||||||||||||||||
Check warning Code scanning / Slither Conformance to Solidity naming conventions Warning
Parameter GatewayEVM.initialize(address,address)._zeta is not in mixedCase
Check warning Code scanning / Slither Conformance to Solidity naming conventions Warning
Parameter GatewayEVM.initialize(address,address)._tssAddress is not in mixedCase
|
||||||||||||||||||||||||||||||||||||||||||||
__Ownable_init(); | ||||||||||||||||||||||||||||||||||||||||||||
__UUPSUpgradeable_init(); | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
if (_tssAddress == address(0)) revert ZeroAddress(); | ||||||||||||||||||||||||||||||||||||||||||||
if (_tssAddress == address(0) || _zeta == address(0)) { | ||||||||||||||||||||||||||||||||||||||||||||
revert ZeroAddress(); | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
tssAddress = _tssAddress; | ||||||||||||||||||||||||||||||||||||||||||||
zeta = _zeta; | ||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Initialization function modified to include zeta handling. The - function initialize(address _tssAddress, address _zeta) public initializer {
+ function initialize(address tssAddress, address zeta) public initializer { Committable suggestion
Suggested change
ToolsGitHub Check: Slither
|
||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
function _authorizeUpgrade(address newImplementation) internal override onlyOwner() {} | ||||||||||||||||||||||||||||||||||||||||||||
|
@@ -69,10 +74,14 @@ | |||||||||||||||||||||||||||||||||||||||||||
// Reset approval | ||||||||||||||||||||||||||||||||||||||||||||
if(!resetApproval(token, to)) revert ApprovalFailed(); | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
// Transfer any remaining tokens back to the custody contract | ||||||||||||||||||||||||||||||||||||||||||||
// Transfer any remaining tokens back to the custody/connector contract | ||||||||||||||||||||||||||||||||||||||||||||
uint256 remainingBalance = IERC20(token).balanceOf(address(this)); | ||||||||||||||||||||||||||||||||||||||||||||
if (remainingBalance > 0) { | ||||||||||||||||||||||||||||||||||||||||||||
IERC20(token).safeTransfer(address(custody), remainingBalance); | ||||||||||||||||||||||||||||||||||||||||||||
address destination = address(custody); | ||||||||||||||||||||||||||||||||||||||||||||
if (token == zeta) { | ||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will zetaToken value used specifically for this check in this contract? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. also used for:
|
||||||||||||||||||||||||||||||||||||||||||||
destination = address(zetaConnector); | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
skosito marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||
IERC20(token).safeTransfer(address(destination), remainingBalance); | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
emit ExecutedWithERC20(token, to, amount, data); | ||||||||||||||||||||||||||||||||||||||||||||
|
@@ -90,10 +99,15 @@ | |||||||||||||||||||||||||||||||||||||||||||
emit Deposit(msg.sender, receiver, msg.value, address(0), ""); | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
// Deposit ERC20 tokens to custody | ||||||||||||||||||||||||||||||||||||||||||||
// Deposit ERC20 tokens to custody/connector | ||||||||||||||||||||||||||||||||||||||||||||
function deposit(address receiver, uint256 amount, address asset) external { | ||||||||||||||||||||||||||||||||||||||||||||
if (amount == 0) revert InsufficientERC20Amount(); | ||||||||||||||||||||||||||||||||||||||||||||
IERC20(asset).safeTransferFrom(msg.sender, address(custody), amount); | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
address destination = address(custody); | ||||||||||||||||||||||||||||||||||||||||||||
if (asset == zeta) { | ||||||||||||||||||||||||||||||||||||||||||||
destination = address(zetaConnector); | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
IERC20(asset).safeTransferFrom(msg.sender, address(destination), amount); | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
emit Deposit(msg.sender, receiver, amount, asset, ""); | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
@@ -108,10 +122,15 @@ | |||||||||||||||||||||||||||||||||||||||||||
emit Deposit(msg.sender, receiver, msg.value, address(0), payload); | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
// Deposit ERC20 tokens to custody and call an omnichain smart contract | ||||||||||||||||||||||||||||||||||||||||||||
// Deposit ERC20 tokens to custody/connector and call an omnichain smart contract | ||||||||||||||||||||||||||||||||||||||||||||
function depositAndCall(address receiver, uint256 amount, address asset, bytes calldata payload) external { | ||||||||||||||||||||||||||||||||||||||||||||
if (amount == 0) revert InsufficientERC20Amount(); | ||||||||||||||||||||||||||||||||||||||||||||
IERC20(asset).safeTransferFrom(msg.sender, address(custody), amount); | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
address destination = address(custody); | ||||||||||||||||||||||||||||||||||||||||||||
if (asset == zeta) { | ||||||||||||||||||||||||||||||||||||||||||||
destination = address(zetaConnector); | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
IERC20(asset).safeTransferFrom(msg.sender, address(destination), amount); | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
emit Deposit(msg.sender, receiver, amount, asset, payload); | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
@@ -123,9 +142,18 @@ | |||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
function setCustody(address _custody) external { | ||||||||||||||||||||||||||||||||||||||||||||
if (custody != address(0)) revert CustodyInitialized(); | ||||||||||||||||||||||||||||||||||||||||||||
if (_custody == address(0)) revert ZeroAddress(); | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
custody = _custody; | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
function setConnector(address _zetaConnector) external { | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
if (zetaConnector != address(0)) revert CustodyInitialized(); | ||||||||||||||||||||||||||||||||||||||||||||
if (_zetaConnector == address(0)) revert ZeroAddress(); | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
zetaConnector = _zetaConnector; | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+149
to
+154
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. New function to set zetaConnector. The - function setConnector(address _zetaConnector) external {
+ function setConnector(address zetaConnector) external { Committable suggestion
Suggested change
ToolsGitHub Check: Slither
|
||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
function resetApproval(address token, address to) private returns (bool) { | ||||||||||||||||||||||||||||||||||||||||||||
return IERC20(token).approve(to, 0); | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -6,40 +6,31 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import "./interfaces.sol"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// NOTE: Purpose of this contract is to test upgrade process, the only difference should be name of Executed event | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// The Gateway contract is the endpoint to call smart contracts on external chains | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// The contract doesn't hold any funds and should never have active allowances | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
contract GatewayEVMUpgradeTest is Initializable, OwnableUpgradeable, UUPSUpgradeable { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
contract GatewayEVMUpgradeTest is Initializable, OwnableUpgradeable, UUPSUpgradeable, IGatewayEVMErrors, IGatewayEVMEvents { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
using SafeERC20 for IERC20; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
error ExecutionFailed(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
error SendFailed(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
error InsufficientETHAmount(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
error ZeroAddress(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
error ApprovalFailed(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
address public custody; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
address public tssAddress; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
address public zetaConnector; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
address public zetaAsset; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
event ExecutedV2(address indexed destination, uint256 value, bytes data); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
event ExecutedWithERC20(address indexed token, address indexed to, uint256 amount, bytes data); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
event SendERC20(bytes recipient, address indexed asset, uint256 amount); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
event Send(bytes recipient, uint256 amount); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// @custom:oz-upgrades-unsafe-allow constructor | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
constructor() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
_disableInitializers(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
constructor() {} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
function initialize(address _tssAddress) public initializer { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
function initialize(address _tssAddress, address _zetaAsset) public initializer { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Check notice Code scanning / Slither Missing zero address validation Low
GatewayEVMUpgradeTest.initialize(address,address)._zetaAsset lacks a zero-check on :
- zetaAsset = _zetaAsset Check warning Code scanning / Slither Conformance to Solidity naming conventions Warning
Parameter GatewayEVMUpgradeTest.initialize(address,address)._zetaAsset is not in mixedCase
Check warning Code scanning / Slither Conformance to Solidity naming conventions Warning
Parameter GatewayEVMUpgradeTest.initialize(address,address)._tssAddress is not in mixedCase
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
__Ownable_init(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
__UUPSUpgradeable_init(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (_tssAddress == address(0)) revert ZeroAddress(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
tssAddress = _tssAddress; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
zetaAsset = _zetaAsset; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Implement zero address validation and adjust parameter naming. The 27 function initialize(address _tssAddress, address _zetaAsset) public initializer {
28 __Ownable_init();
29 __UUPSUpgradeable_init();
30
31 if (_tssAddress == address(0) || _zetaAsset == address(0)) revert ZeroAddress();
32
33 tssAddress = _tssAddress;
34 zetaAsset = _zetaAsset;
35 }
ToolsGitHub Check: Slither
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
function _authorizeUpgrade(address newImplementation) internal override onlyOwner() {} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -72,47 +63,95 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||
address to, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
uint256 amount, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
bytes calldata data | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
) external returns (bytes memory) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
) public returns (bytes memory) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (amount == 0) revert InsufficientETHAmount(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// Approve the target contract to spend the tokens | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if(!IERC20(token).approve(to, 0)) revert ApprovalFailed(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if(!resetApproval(token, to)) revert ApprovalFailed(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if(!IERC20(token).approve(to, amount)) revert ApprovalFailed(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// Execute the call on the target contract | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
bytes memory result = _execute(to, data); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// Reset approval | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if(!IERC20(token).approve(to, 0)) revert ApprovalFailed(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if(!resetApproval(token, to)) revert ApprovalFailed(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// Transfer any remaining tokens back to the custody contract | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// Transfer any remaining tokens back to the custody/connector contract | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
uint256 remainingBalance = IERC20(token).balanceOf(address(this)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (remainingBalance > 0) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
IERC20(token).safeTransfer(address(custody), remainingBalance); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
address destination = address(custody); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (token == zetaAsset) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
destination = address(zetaConnector); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
IERC20(token).safeTransfer(address(destination), remainingBalance); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
emit ExecutedWithERC20(token, to, amount, data); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return result; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// Transfer specified token amount to ERC20Custody and emits event | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
function sendERC20(bytes calldata recipient, address token, uint256 amount) external { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
IERC20(token).safeTransferFrom(msg.sender, address(custody), amount); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// Deposit ETH to tss | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
function deposit(address receiver) external payable { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (msg.value == 0) revert InsufficientETHAmount(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
(bool deposited, ) = tssAddress.call{value: msg.value}(""); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
emit SendERC20(recipient, token, amount); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (deposited == false) revert DepositFailed(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
emit Deposit(msg.sender, receiver, msg.value, address(0), ""); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+94
to
+101
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. New function to deposit ETH. This function allows for the deposit of ETH to a specified receiver. It includes checks for zero deposits and handles the actual transfer. However, consider using high-level calls instead of low-level ones to improve security. - (bool deposited, ) = tssAddress.call{value: msg.value}("");
+ require(tssAddress.send(msg.value), "Deposit failed"); Committable suggestion
Suggested change
Comment on lines
+95
to
+101
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add The - (bool deposited, ) = tssAddress.call{value: msg.value}("");
+ require(tssAddress.send(msg.value), "Deposit failed"); Committable suggestion
Suggested change
ToolsGitHub Check: Slither
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Check notice Code scanning / Slither Reentrancy vulnerabilities Low
Reentrancy in GatewayEVMUpgradeTest.deposit(address):
External calls: - (deposited,None) = tssAddress.call{value: msg.value}() Event emitted after the call(s): - Deposit(msg.sender,receiver,msg.value,address(0),) Check warning Code scanning / Slither Boolean equality Warning Check warning Code scanning / Slither Low-level calls Warning |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// Transfer specified ETH amount to TSS address and emits event | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
function send(bytes calldata recipient, uint256 amount) external payable { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// Deposit ERC20 tokens to custody | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
function deposit(address receiver, uint256 amount, address asset) external { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (amount == 0) revert InsufficientERC20Amount(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
address destination = address(custody); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (asset == zetaAsset) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
destination = address(zetaConnector); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
IERC20(asset).safeTransferFrom(msg.sender, address(destination), amount); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
emit Deposit(msg.sender, receiver, amount, asset, ""); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Check notice Code scanning / Slither Reentrancy vulnerabilities Low
Reentrancy in GatewayEVMUpgradeTest.deposit(address,uint256,address):
External calls: - IERC20(asset).safeTransferFrom(msg.sender,address(destination),amount) Event emitted after the call(s): - Deposit(msg.sender,receiver,amount,asset,) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Address reentrancy concerns in the ERC20 deposit function. Similar to the ETH deposit function, this function also lacks the 104 function deposit(address receiver, uint256 amount, address asset) external nonReentrant {
105 if (amount == 0) revert InsufficientERC20Amount();
106
107 address destination = address(custody);
108 if (asset == zetaAsset) {
109 destination = address(zetaConnector);
110 }
111 IERC20(asset).safeTransferFrom(msg.sender, address(destination), amount);
112
113 emit Deposit(msg.sender, receiver, amount, asset, "");
114 } Committable suggestion
Suggested change
ToolsGitHub Check: Slither
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// Deposit ETH to tss and call an omnichain smart contract | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
function depositAndCall(address receiver, bytes calldata payload) external payable { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (msg.value == 0) revert InsufficientETHAmount(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
(bool deposited, ) = tssAddress.call{value: msg.value}(""); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
(bool sent, ) = tssAddress.call{value: msg.value}(""); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (deposited == false) revert DepositFailed(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
emit Deposit(msg.sender, receiver, msg.value, address(0), payload); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+118
to
+125
Check notice Code scanning / Slither Reentrancy vulnerabilities Low
Reentrancy in GatewayEVMUpgradeTest.depositAndCall(address,bytes):
External calls: - (deposited,None) = tssAddress.call{value: msg.value}() Event emitted after the call(s): - Deposit(msg.sender,receiver,msg.value,address(0),payload)
Comment on lines
+118
to
+125
Check warning Code scanning / Slither Boolean equality Warning
GatewayEVMUpgradeTest.depositAndCall(address,bytes) compares to a boolean constant:
-deposited == false
Comment on lines
+118
to
+125
Check warning Code scanning / Slither Low-level calls Warning
Comment on lines
+117
to
+125
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add The + modifier nonReentrant() {
+ require(!locked, "Reentrant call");
+ locked = true;
+ _;
+ locked = false;
+ }
ToolsGitHub Check: Slither
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// Deposit ERC20 tokens to custody and call an omnichain smart contract | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
function depositAndCall(address receiver, uint256 amount, address asset, bytes calldata payload) external { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (amount == 0) revert InsufficientERC20Amount(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
address destination = address(custody); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (asset == zetaAsset) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
destination = address(zetaConnector); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
IERC20(asset).safeTransferFrom(msg.sender, address(destination), amount); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (sent == false) revert SendFailed(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
emit Deposit(msg.sender, receiver, amount, asset, payload); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Check notice Code scanning / Slither Reentrancy vulnerabilities Low
Reentrancy in GatewayEVMUpgradeTest.depositAndCall(address,uint256,address,bytes):
External calls: - IERC20(asset).safeTransferFrom(msg.sender,address(destination),amount) Event emitted after the call(s): - Deposit(msg.sender,receiver,amount,asset,payload) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Address reentrancy concerns in To prevent reentrancy attacks, consider using the 127 function depositAndCall(address receiver, uint256 amount, address asset, bytes calldata payload) external nonReentrant {
128 if (amount == 0) revert InsufficientERC20Amount();
129
130 address destination = address(custody);
131 if (asset == zetaAsset) {
132 destination = address(zetaConnector);
133 }
134 IERC20(asset).safeTransferFrom(msg.sender, address(destination), amount);
135
136 emit Deposit(msg.sender, receiver, amount, asset, payload);
137 }
138 } Committable suggestion
Suggested change
ToolsGitHub Check: Slither
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
emit Send(recipient, msg.value); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// Call an omnichain smart contract without asset transfer | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
function call(address receiver, bytes calldata payload) external { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
emit Call(msg.sender, receiver, payload); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
function setCustody(address _custody) external { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (custody != address(0)) revert CustodyInitialized(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
custody = _custody; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
function setConnector(address _zetaConnector) external { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (zetaConnector != address(0)) revert CustodyInitialized(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
zetaConnector = _zetaConnector; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+150
to
+153
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add zero address validation for The + if (_zetaConnector == address(0)) revert ZeroAddress(); Committable suggestion
Suggested change
ToolsGitHub Check: Slither
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
function resetApproval(address token, address to) private returns (bool) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return IERC20(token).approve(to, 0); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Check warning Code scanning / Slither Missing inheritance Warning
GatewayEVMUpgradeTest should inherit from IGatewayEVM
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,51 @@ | ||||||||||||||||||||||||||||||||||||||||||||
// SPDX-License-Identifier: MIT | ||||||||||||||||||||||||||||||||||||||||||||
pragma solidity 0.8.7; | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
import "@openzeppelin/contracts/token/ERC20/IERC20.sol"; | ||||||||||||||||||||||||||||||||||||||||||||
import "./interfaces.sol"; | ||||||||||||||||||||||||||||||||||||||||||||
import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; | ||||||||||||||||||||||||||||||||||||||||||||
import "@openzeppelin/contracts/security/ReentrancyGuard.sol"; | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
contract ZetaConnectorNew is ReentrancyGuard{ | ||||||||||||||||||||||||||||||||||||||||||||
using SafeERC20 for IERC20; | ||||||||||||||||||||||||||||||||||||||||||||
error ZeroAddress(); | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
IGatewayEVM public immutable gateway; | ||||||||||||||||||||||||||||||||||||||||||||
IERC20 public immutable zeta; | ||||||||||||||||||||||||||||||||||||||||||||
skosito marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
event Withdraw(address indexed to, uint256 amount); | ||||||||||||||||||||||||||||||||||||||||||||
event WithdrawAndCall(address indexed to, uint256 amount, bytes data); | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
constructor(address _gateway, address _zeta) { | ||||||||||||||||||||||||||||||||||||||||||||
if (_gateway == address(0) || _zeta == address(0)) { | ||||||||||||||||||||||||||||||||||||||||||||
revert ZeroAddress(); | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
gateway = IGatewayEVM(_gateway); | ||||||||||||||||||||||||||||||||||||||||||||
zeta = IERC20(_zeta); | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
// Withdraw is called by TSS address, it directly transfers zeta to the destination address without contract call | ||||||||||||||||||||||||||||||||||||||||||||
// TODO: Finalize access control | ||||||||||||||||||||||||||||||||||||||||||||
// https://github.com/zeta-chain/protocol-contracts/issues/204 | ||||||||||||||||||||||||||||||||||||||||||||
function withdraw(address to, uint256 amount) external nonReentrant { | ||||||||||||||||||||||||||||||||||||||||||||
// TODO: mint? | ||||||||||||||||||||||||||||||||||||||||||||
skosito marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||
zeta.safeTransfer(to, amount); | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
emit Withdraw(to, amount); | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
// WithdrawAndCall is called by TSS address, it transfers zeta and call a contract | ||||||||||||||||||||||||||||||||||||||||||||
// For this, it passes through the Gateway contract, it transfers zeta to the Gateway contract and then calls the contract | ||||||||||||||||||||||||||||||||||||||||||||
// TODO: Finalize access control | ||||||||||||||||||||||||||||||||||||||||||||
// https://github.com/zeta-chain/protocol-contracts/issues/204 | ||||||||||||||||||||||||||||||||||||||||||||
function withdrawAndCall(address to, uint256 amount, bytes calldata data) public nonReentrant { | ||||||||||||||||||||||||||||||||||||||||||||
// TODO: mint? | ||||||||||||||||||||||||||||||||||||||||||||
// Transfer zeta to the Gateway contract | ||||||||||||||||||||||||||||||||||||||||||||
zeta.safeTransfer(address(gateway), amount); | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
// Forward the call to the Gateway contract | ||||||||||||||||||||||||||||||||||||||||||||
gateway.executeWithERC20(address(zeta), to, amount, data); | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
emit WithdrawAndCall(to, amount, data); | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
Check warning Code scanning / Slither Unused return Medium There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I might think we don't need a return for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Handle the unused return value from The method - gateway.executeWithERC20(address(zeta), to, amount, data);
+ bool success = gateway.executeWithERC20(address(zeta), to, amount, data);
+ require(success, "ExecuteWithERC20 failed"); Committable suggestion
Suggested change
ToolsGitHub Check: Slither
|
||||||||||||||||||||||||||||||||||||||||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have an issue to track adding the right comment to the contracts #204
But since we're starting to introduce many variables, this might make sense to try adding specs as we add the new variable?