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: v2 contracts natspec #249

Merged
merged 12 commits into from
Jul 25, 2024
Merged

feat: v2 contracts natspec #249

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
5b5cc4b
access control for erc20 custody new
skosito Jul 23, 2024
f1cf789
cleanup erc20 custody new events and errors
skosito Jul 23, 2024
8b9669e
access control for gateway evm
skosito Jul 23, 2024
1851b89
move connector events to interface
skosito Jul 23, 2024
ba0f25f
add tss address to connector and fix uniswap integration test
skosito Jul 23, 2024
3f4990c
tests for tss sender in connector
skosito Jul 23, 2024
a686f66
remove access control todos from zevm contracts
skosito Jul 23, 2024
58160e8
generate
skosito Jul 23, 2024
63bfb60
evm natspecs
skosito Jul 23, 2024
0837639
zevm natspecs
skosito Jul 23, 2024
f3f9dfa
Merge branch 'main' into natspec
skosito Jul 25, 2024
a3719f0
fix test
skosito Jul 25, 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
34 changes: 24 additions & 10 deletions contracts/prototypes/evm/ERC20CustodyNew.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,18 @@ import "@openzeppelin/contracts/security/ReentrancyGuard.sol";
import "./IGatewayEVM.sol";
import "./IERC20CustodyNew.sol";

// As the current version, ERC20CustodyNew hold the ERC20s deposited on ZetaChain
// This version include a functionality allowing to call a contract
// ERC20Custody doesn't call smart contract directly, it passes through the Gateway contract
/// @title ERC20CustodyNew
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: are we going to keep the "New" suffixes in the contracts? Should we omit then as this contracts move to production?

/// @notice Holds the ERC20 tokens deposited on ZetaChain and includes functionality to call a contract.
/// @dev This contract does not call smart contracts directly, it passes through the Gateway contract.
contract ERC20CustodyNew is IERC20CustodyNewEvents, IERC20CustodyNewErrors, ReentrancyGuard {
using SafeERC20 for IERC20;

/// @notice Gateway contract.
IGatewayEVM public gateway;
/// @notice TSS address.
address public tssAddress;

// @dev Only TSS address allowed modifier.
/// @notice Only TSS address allowed modifier.
modifier onlyTSS() {
if (msg.sender != tssAddress) {
revert InvalidSender();
Expand All @@ -26,22 +28,30 @@ contract ERC20CustodyNew is IERC20CustodyNewEvents, IERC20CustodyNewErrors, Reen
}

constructor(address _gateway, address _tssAddress) {
if (_gateway == address(0) || _tssAddress == address(0)) {
if (_gateway == address(0) || _tssAddress == address(0)) {
revert ZeroAddress();
}
gateway = IGatewayEVM(_gateway);
tssAddress = _tssAddress;
}

// Withdraw is called by TSS address, it directly transfers the tokens to the destination address without contract call
/// @notice Withdraw directly transfers the tokens to the destination address without contract call.
/// @dev This function can only be called by the TSS address.
/// @param token Address of the ERC20 token.
/// @param to Destination address for the tokens.
/// @param amount Amount of tokens to withdraw.
function withdraw(address token, address to, uint256 amount) external nonReentrant onlyTSS {
IERC20(token).safeTransfer(to, amount);

emit Withdraw(token, to, amount);
}

// WithdrawAndCall is called by TSS address, it transfers the tokens and call a contract
// For this, it passes through the Gateway contract, it transfers the tokens to the Gateway contract and then calls the contract
/// @notice WithdrawAndCall transfers tokens to Gateway and call a contract through the Gateway.
/// @dev This function can only be called by the TSS address.
/// @param token Address of the ERC20 token.
/// @param to Address of the contract to call.
/// @param amount Amount of tokens to withdraw.
/// @param data Calldata to pass to the contract call.
function withdrawAndCall(address token, address to, uint256 amount, bytes calldata data) public nonReentrant onlyTSS {
// Transfer the tokens to the Gateway contract
IERC20(token).safeTransfer(address(gateway), amount);
Expand All @@ -52,8 +62,12 @@ contract ERC20CustodyNew is IERC20CustodyNewEvents, IERC20CustodyNewErrors, Reen
emit WithdrawAndCall(token, to, amount, data);
}

// WithdrawAndRevert is called by TSS address, it transfers the tokens and call a contract
// For this, it passes through the Gateway contract, it transfers the tokens to the Gateway contract and then calls the contract
/// @notice WithdrawAndRevert transfers tokens to Gateway and call a contract with a revert functionality through the Gateway.
/// @dev This function can only be called by the TSS address.
/// @param token Address of the ERC20 token.
/// @param to Address of the contract to call.
/// @param amount Amount of tokens to withdraw.
/// @param data Calldata to pass to the contract call.
function withdrawAndRevert(address token, address to, uint256 amount, bytes calldata data) public nonReentrant onlyTSS {
// Transfer the tokens to the Gateway contract
IERC20(token).safeTransfer(address(gateway), amount);
Expand Down
94 changes: 70 additions & 24 deletions contracts/prototypes/evm/GatewayEVM.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,13 @@
import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
import "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol";
import "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol";

import "./IGatewayEVM.sol";
import "./ZetaConnectorNewBase.sol";

/**
* @title GatewayEVM
* @notice The GatewayEVM contract is the endpoint to call smart contracts on external chains.
* @dev The contract doesn't hold any funds and should never have active allowances.
*/
/// @title GatewayEVM
/// @notice The GatewayEVM contract is the endpoint to call smart contracts on external chains.
/// @dev The contract doesn't hold any funds and should never have active allowances.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Should we open an issue to implement this logic specifically?

    fallback() external payable {
        revert();
    }

    receive() external payable {
        revert();
    }

Copy link
Member

Choose a reason for hiding this comment

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

Is receive for ERC777?
I think makes sense to add the fallback, at the end I don't think we can prevent sending ERC20, the assumption above is more that it can't receive and hold funds from the TSS or ERC20Custody

Copy link
Contributor Author

Choose a reason for hiding this comment

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

lets open issue, i also think we would need check if sender is tss and dont revert in that case? but receiving eth/erc20 cant cause issues right?

contract GatewayEVM is Initializable, OwnableUpgradeable, UUPSUpgradeable, IGatewayEVMErrors, IGatewayEVMEvents, ReentrancyGuardUpgradeable {
using SafeERC20 for IERC20;

Expand All @@ -27,15 +26,15 @@
/// @notice The address of the Zeta token contract.
address public zetaToken;

// @dev Only TSS address allowed modifier.
/// @notice Only TSS address allowed modifier.
modifier onlyTSS() {
if (msg.sender != tssAddress) {
revert InvalidSender();
}
_;
}

// @dev Only asset handler (custody, connector) allowed modifier.
/// @notice Only custody or connector address allowed modifier.
modifier onlyAssetHandler() {
if (msg.sender != custody && msg.sender != zetaConnector) {
revert InvalidSender();
Expand All @@ -61,17 +60,25 @@
zetaToken = _zetaToken;
}

/// @dev Authorizes the upgrade of the contract, sender must be owner.
/// @param newImplementation Address of the new implementation.
function _authorizeUpgrade(address newImplementation) internal override onlyOwner() {}

/// @dev Internal function to execute a call to a destination address.
/// @param destination Address to call.
/// @param data Calldata to pass to the call.
/// @return The result of the call.
function _execute(address destination, bytes calldata data) internal returns (bytes memory) {
(bool success, bytes memory result) = destination.call{value: msg.value}(data);
if (!success) revert ExecutionFailed();

return result;
}

// Called by the TSS
// Calling onRevert directly
/// @notice Transfers msg.value to destination contract and executes it's onRevert function.
/// @dev This function can only be called by the TSS address and it is payable.
/// @param destination Address to call.
/// @param data Calldata to pass to the call.
function executeRevert(address destination, bytes calldata data) public payable onlyTSS {
(bool success, bytes memory result) = destination.call{value: msg.value}("");
if (!success) revert ExecutionFailed();
Expand All @@ -80,9 +87,11 @@
emit Reverted(destination, msg.value, data);
}

// Called by the TSS
// Execution without ERC20 tokens, it is payable and can be used in the case of WithdrawAndCall for Gas ZRC20
// It can be also used for contract call without asset movement
/// @notice Executes a call to a destination address without ERC20 tokens.
/// @dev This function can only be called by the TSS address and it is payable.
/// @param destination Address to call.
/// @param data Calldata to pass to the call.
/// @return The result of the call.
function execute(address destination, bytes calldata data) external payable onlyTSS returns (bytes memory) {
bytes memory result = _execute(destination, data);

Expand All @@ -91,10 +100,13 @@
return result;
}

// Called by the ERC20Custody contract
// It call a function using ERC20 transfer
// Since the goal is to allow calling contract not designed for ZetaChain specifically, it uses ERC20 allowance system
// It provides allowance to destination contract and call destination contract. In the end, it remove remaining allowance and transfer remaining tokens back to the custody contract for security purposes
/// @notice Executes a call to a destination contract using ERC20 tokens.
/// @dev This function can only be called by the custody or connector address.
/// It uses the ERC20 allowance system, resetting gateway allowance at the end.
/// @param token Address of the ERC20 token.
/// @param to Address of the contract to call.
/// @param amount Amount of tokens to transfer.
/// @param data Calldata to pass to the call.
function executeWithERC20(
address token,
address to,
Expand All @@ -120,8 +132,12 @@
emit ExecutedWithERC20(token, to, amount, data);
}

// Called by the ERC20Custody contract
// Directly transfers ERC20 and calls onRevert
/// @notice Directly transfers ERC20 tokens and calls onRevert.
/// @dev This function can only be called by the custody or connector address.
/// @param token Address of the ERC20 token.
/// @param to Address of the contract to call.
/// @param amount Amount of tokens to transfer.
/// @param data Calldata to pass to the call.
function revertWithERC20(
address token,
address to,
Expand All @@ -136,7 +152,8 @@
emit RevertedWithERC20(token, to, amount, data);
}

// Deposit ETH to tss
/// @notice Deposits ETH to the TSS address.
/// @param receiver Address of the receiver.
function deposit(address receiver) external payable {
if (msg.value == 0) revert InsufficientETHAmount();
(bool deposited, ) = tssAddress.call{value: msg.value}("");
Expand All @@ -146,7 +163,10 @@
emit Deposit(msg.sender, receiver, msg.value, address(0), "");
}

// Deposit ERC20 tokens to custody/connector
/// @notice Deposits ERC20 tokens to the custody or connector contract.
/// @param receiver Address of the receiver.
/// @param amount Amount of tokens to deposit.
/// @param asset Address of the ERC20 token.
function deposit(address receiver, uint256 amount, address asset) external {
if (amount == 0) revert InsufficientERC20Amount();

Expand All @@ -155,7 +175,9 @@
emit Deposit(msg.sender, receiver, amount, asset, "");
}

// Deposit ETH to tss and call an omnichain smart contract
/// @notice Deposits ETH to the TSS address and calls an omnichain smart contract.
/// @param receiver Address of the receiver.
/// @param payload Calldata to pass to the call.
function depositAndCall(address receiver, bytes calldata payload) external payable {
if (msg.value == 0) revert InsufficientETHAmount();
(bool deposited, ) = tssAddress.call{value: msg.value}("");
Expand All @@ -165,7 +187,11 @@
emit Deposit(msg.sender, receiver, msg.value, address(0), payload);
}

// Deposit ERC20 tokens to custody/connector and call an omnichain smart contract
/// @notice Deposits ERC20 tokens to the custody or connector contract and calls an omnichain smart contract.
/// @param receiver Address of the receiver.
/// @param amount Amount of tokens to deposit.
/// @param asset Address of the ERC20 token.
/// @param payload Calldata to pass to the call.
function depositAndCall(address receiver, uint256 amount, address asset, bytes calldata payload) external {
if (amount == 0) revert InsufficientERC20Amount();

Expand All @@ -174,29 +200,45 @@
emit Deposit(msg.sender, receiver, amount, asset, payload);
}

// Call an omnichain smart contract without asset transfer
/// @notice Calls an omnichain smart contract without asset transfer.
/// @param receiver Address of the receiver.
/// @param payload Calldata to pass to the call.
function call(address receiver, bytes calldata payload) external {
emit Call(msg.sender, receiver, payload);
}

/// @notice Sets the custody contract address.
/// @param _custody Address of the custody contract.
function setCustody(address _custody) external onlyTSS {
if (custody != address(0)) revert CustodyInitialized();
if (_custody == address(0)) revert ZeroAddress();

custody = _custody;
}

function setConnector(address _zetaConnector) external onlyTSS {
/// @notice Sets the connector contract address.
/// @param _zetaConnector Address of the connector contract.
function setConnector(address _zetaConnector) external onlyTSS {

Check warning

Code scanning / Slither

Conformance to Solidity naming conventions Warning

Parameter GatewayEVM.setConnector(address)._zetaConnector is not in mixedCase
if (zetaConnector != address(0)) revert CustodyInitialized();
if (_zetaConnector == address(0)) revert ZeroAddress();

zetaConnector = _zetaConnector;
}

Check notice

Code scanning / Slither

Missing events access control Low

GatewayEVM.setConnector(address) should emit an event for:
- zetaConnector = _zetaConnector

/// @dev Resets the approval of a token for a specified address.
/// This is used to ensure that the approval is set to zero before setting it to a new value.
/// @param token Address of the ERC20 token.
/// @param to Address to reset the approval for.
/// @return True if the approval reset was successful, false otherwise.
function resetApproval(address token, address to) private returns (bool) {
return IERC20(token).approve(to, 0);
}

/// @dev Transfers tokens from the sender to the asset handler.
/// This function handles the transfer of tokens to either the connector or custody contract based on the asset type.
/// @param from Address of the sender.
/// @param token Address of the ERC20 token.
/// @param amount Amount of tokens to transfer.
function transferFromToAssetHandler(address from, address token, uint256 amount) private {
if (token == zetaToken) { // transfer to connector
// transfer amount to gateway
Expand All @@ -210,6 +252,10 @@
}
}

/// @dev Transfers tokens to the asset handler.
/// This function handles the transfer of tokens to either the connector or custody contract based on the asset type.
/// @param token Address of the ERC20 token.
/// @param amount Amount of tokens to transfer.
function transferToAssetHandler(address token, uint256 amount) private {
if (token == zetaToken) { // transfer to connector
// approve connector to handle tokens depending on connector version (eg. lock or burn)
Expand Down
Loading
Loading