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

fix: review fixes #383

Merged
merged 6 commits into from
Oct 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions v2/contracts/evm/ERC20Custody.sol
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,9 @@ contract ERC20Custody is
_grantRole(WITHDRAWER_ROLE, newTSSAddress);
_grantRole(WHITELISTER_ROLE, newTSSAddress);

tssAddress = newTSSAddress;
emit UpdatedCustodyTSSAddress(tssAddress, newTSSAddress);

emit UpdatedCustodyTSSAddress(newTSSAddress);
tssAddress = newTSSAddress;
}

/// @notice Unpause contract.
Expand Down
32 changes: 17 additions & 15 deletions v2/contracts/evm/GatewayEVM.sol
Original file line number Diff line number Diff line change
Expand Up @@ -82,9 +82,9 @@ contract GatewayEVM is
_revokeRole(TSS_ROLE, tssAddress);
_grantRole(TSS_ROLE, newTSSAddress);

tssAddress = newTSSAddress;
emit UpdatedGatewayTSSAddress(tssAddress, newTSSAddress);

emit UpdatedGatewayTSSAddress(newTSSAddress);
tssAddress = newTSSAddress;
}

/// @notice Pause contract.
Expand Down Expand Up @@ -191,18 +191,18 @@ contract GatewayEVM is
if (amount == 0) revert InsufficientERC20Amount();
if (to == address(0)) revert ZeroAddress();
// Approve the target contract to spend the tokens
if (!resetApproval(token, to)) revert ApprovalFailed();
if (!_resetApproval(token, to)) revert ApprovalFailed();
if (!IERC20(token).approve(to, amount)) revert ApprovalFailed();
// Execute the call on the target contract
_executeArbitraryCall(to, data);

// Reset approval
if (!resetApproval(token, to)) revert ApprovalFailed();
if (!_resetApproval(token, to)) revert ApprovalFailed();

// Transfer any remaining tokens back to the custody/connector contract
uint256 remainingBalance = IERC20(token).balanceOf(address(this));
if (remainingBalance > 0) {
transferToAssetHandler(token, remainingBalance);
_transferToAssetHandler(token, remainingBalance);
}

emit ExecutedWithERC20(token, to, amount, data);
Expand Down Expand Up @@ -250,6 +250,7 @@ contract GatewayEVM is
{
if (msg.value == 0) revert InsufficientETHAmount();
if (receiver == address(0)) revert ZeroAddress();
if (revertOptions.revertMessage.length > MAX_PAYLOAD_SIZE) revert PayloadSizeExceeded();
skosito marked this conversation as resolved.
Show resolved Hide resolved

(bool deposited,) = tssAddress.call{ value: msg.value }("");

Expand All @@ -275,8 +276,9 @@ contract GatewayEVM is
{
if (amount == 0) revert InsufficientERC20Amount();
if (receiver == address(0)) revert ZeroAddress();
if (revertOptions.revertMessage.length > MAX_PAYLOAD_SIZE) revert PayloadSizeExceeded();

transferFromToAssetHandler(msg.sender, asset, amount);
_transferFromToAssetHandler(msg.sender, asset, amount);

emit Deposited(msg.sender, receiver, amount, asset, "", revertOptions);
}
Expand All @@ -297,7 +299,7 @@ contract GatewayEVM is
{
if (msg.value == 0) revert InsufficientETHAmount();
if (receiver == address(0)) revert ZeroAddress();
if (payload.length + revertOptions.revertMessage.length >= MAX_PAYLOAD_SIZE) revert PayloadSizeExceeded();
if (payload.length + revertOptions.revertMessage.length > MAX_PAYLOAD_SIZE) revert PayloadSizeExceeded();

(bool deposited,) = tssAddress.call{ value: msg.value }("");

Expand Down Expand Up @@ -325,9 +327,9 @@ contract GatewayEVM is
{
if (amount == 0) revert InsufficientERC20Amount();
if (receiver == address(0)) revert ZeroAddress();
if (payload.length + revertOptions.revertMessage.length >= MAX_PAYLOAD_SIZE) revert PayloadSizeExceeded();
if (payload.length + revertOptions.revertMessage.length > MAX_PAYLOAD_SIZE) revert PayloadSizeExceeded();

transferFromToAssetHandler(msg.sender, asset, amount);
_transferFromToAssetHandler(msg.sender, asset, amount);

emit Deposited(msg.sender, receiver, amount, asset, payload, revertOptions);
}
Expand All @@ -346,7 +348,7 @@ contract GatewayEVM is
nonReentrant
{
if (receiver == address(0)) revert ZeroAddress();
if (payload.length + revertOptions.revertMessage.length >= MAX_PAYLOAD_SIZE) revert PayloadSizeExceeded();
if (payload.length + revertOptions.revertMessage.length > MAX_PAYLOAD_SIZE) revert PayloadSizeExceeded();

emit Called(msg.sender, receiver, payload, revertOptions);
}
Expand Down Expand Up @@ -376,7 +378,7 @@ contract GatewayEVM is
/// @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) {
function _resetApproval(address token, address to) private returns (bool) {
return IERC20(token).approve(to, 0);
}

Expand All @@ -386,7 +388,7 @@ contract GatewayEVM is
/// @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 {
function _transferFromToAssetHandler(address from, address token, uint256 amount) private {
if (token == zetaToken) {
// transfer to connector
// transfer amount to gateway
Expand All @@ -407,7 +409,7 @@ contract GatewayEVM is
/// type.
/// @param token Address of the ERC20 token.
/// @param amount Amount of tokens to transfer.
function transferToAssetHandler(address token, uint256 amount) private {
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 All @@ -426,7 +428,7 @@ contract GatewayEVM is
/// @param data Calldata to pass to the call.
/// @return The result of the call.
function _executeArbitraryCall(address destination, bytes calldata data) private returns (bytes memory) {
revertIfOnCallOrOnRevert(data);
_revertIfOnCallOrOnRevert(data);
(bool success, bytes memory result) = destination.call{ value: msg.value }(data);
if (!success) revert ExecutionFailed();

Expand All @@ -450,7 +452,7 @@ contract GatewayEVM is
}

// @dev prevent spoofing onCall and onRevert functions
function revertIfOnCallOrOnRevert(bytes calldata data) private pure {
function _revertIfOnCallOrOnRevert(bytes calldata data) private pure {
if (data.length >= 4) {
bytes4 functionSelector;
assembly {
Expand Down
4 changes: 2 additions & 2 deletions v2/contracts/evm/ZetaConnectorBase.sol
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,9 @@ abstract contract ZetaConnectorBase is
_grantRole(WITHDRAWER_ROLE, newTSSAddress);
_grantRole(TSS_ROLE, newTSSAddress);

tssAddress = newTSSAddress;
emit UpdatedZetaConnectorTSSAddress(tssAddress, newTSSAddress);

emit UpdatedZetaConnectorTSSAddress(newTSSAddress);
tssAddress = newTSSAddress;
}

/// @notice Pause contract.
Expand Down
2 changes: 1 addition & 1 deletion v2/contracts/evm/ZetaConnectorNonNative.sol
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ contract ZetaConnectorNonNative is ZetaConnectorBase {
}

/// @dev mints to provided account and checks if totalSupply will be exceeded
function _mintTo(address to, uint256 amount, bytes32 internalSendHash) internal {
function _mintTo(address to, uint256 amount, bytes32 internalSendHash) private {
if (amount + IERC20(zetaToken).totalSupply() > maxSupply) revert ExceedsMaxSupply();
IZetaNonEthNew(zetaToken).mint(address(to), amount, internalSendHash);
}
Expand Down
3 changes: 2 additions & 1 deletion v2/contracts/evm/interfaces/IERC20Custody.sol
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,9 @@ interface IERC20CustodyEvents {
event Deposited(bytes recipient, IERC20 indexed asset, uint256 amount, bytes message);

/// @notice Emitted when tss address is updated
/// @param oldTSSAddress old tss address
/// @param newTSSAddress new tss address
event UpdatedCustodyTSSAddress(address newTSSAddress);
event UpdatedCustodyTSSAddress(address oldTSSAddress, address newTSSAddress);
}

/// @title IERC20CustodyErrors
Expand Down
3 changes: 2 additions & 1 deletion v2/contracts/evm/interfaces/IGatewayEVM.sol
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,9 @@ interface IGatewayEVMEvents {
event Called(address indexed sender, address indexed receiver, bytes payload, RevertOptions revertOptions);

/// @notice Emitted when tss address is updated
/// @param oldTSSAddress old tss address
/// @param newTSSAddress new tss address
event UpdatedGatewayTSSAddress(address newTSSAddress);
event UpdatedGatewayTSSAddress(address oldTSSAddress, address newTSSAddress);
}

/// @title IGatewayEVMErrors
Expand Down
3 changes: 2 additions & 1 deletion v2/contracts/evm/interfaces/IZetaConnector.sol
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ interface IZetaConnectorEvents {
event WithdrawnAndReverted(address indexed to, uint256 amount, bytes data, RevertContext revertContext);

/// @notice Emitted when tss address is updated
/// @param oldTSSAddress old tss address
/// @param newTSSAddress new tss address
event UpdatedZetaConnectorTSSAddress(address newTSSAddress);
event UpdatedZetaConnectorTSSAddress(address oldTSSAddress, address newTSSAddress);
}
28 changes: 15 additions & 13 deletions v2/contracts/zevm/GatewayZEVM.sol
Original file line number Diff line number Diff line change
Expand Up @@ -87,21 +87,21 @@ contract GatewayZEVM is
_unpause();
}

/// @dev Internal function to withdraw ZRC20 tokens.
/// @dev Private function to withdraw ZRC20 tokens.
/// @param amount The amount of tokens to withdraw.
/// @param zrc20 The address of the ZRC20 token.
/// @return The gas fee for the withdrawal.
function _withdrawZRC20(uint256 amount, address zrc20) internal returns (uint256) {
function _withdrawZRC20(uint256 amount, address zrc20) private returns (uint256) {
// Use gas limit from zrc20
return _withdrawZRC20WithGasLimit(amount, zrc20, IZRC20(zrc20).GAS_LIMIT());
}

/// @dev Internal function to withdraw ZRC20 tokens with gas limit.
/// @dev Private function to withdraw ZRC20 tokens with gas limit.
/// @param amount The amount of tokens to withdraw.
/// @param zrc20 The address of the ZRC20 token.
/// @param gasLimit Gas limit.
/// @return The gas fee for the withdrawal.
function _withdrawZRC20WithGasLimit(uint256 amount, address zrc20, uint256 gasLimit) internal returns (uint256) {
function _withdrawZRC20WithGasLimit(uint256 amount, address zrc20, uint256 gasLimit) private returns (uint256) {
(address gasZRC20, uint256 gasFee) = IZRC20(zrc20).withdrawGasFeeWithGasLimit(gasLimit);
if (!IZRC20(gasZRC20).transferFrom(msg.sender, PROTOCOL_ADDRESS, gasFee)) {
revert GasFeeTransferFailed();
Expand All @@ -116,10 +116,10 @@ contract GatewayZEVM is
return gasFee;
}

/// @dev Internal function to transfer ZETA tokens.
/// @dev Private function to transfer ZETA tokens.
/// @param amount The amount of tokens to transfer.
/// @param to The address to transfer the tokens to.
function _transferZETA(uint256 amount, address to) internal {
function _transferZETA(uint256 amount, address to) private {
if (!IWETH9(zetaToken).transferFrom(msg.sender, address(this), amount)) revert FailedZetaSent();
IWETH9(zetaToken).withdraw(amount);
(bool sent,) = to.call{ value: amount }("");
Expand All @@ -143,6 +143,7 @@ contract GatewayZEVM is
{
if (receiver.length == 0) revert ZeroAddress();
if (amount == 0) revert InsufficientZRC20Amount();
if (revertOptions.revertMessage.length > MAX_MESSAGE_SIZE) revert MessageSizeExceeded();

uint256 gasFee = _withdrawZRC20(amount, zrc20);
emit Withdrawn(
Expand Down Expand Up @@ -181,7 +182,7 @@ contract GatewayZEVM is
if (receiver.length == 0) revert ZeroAddress();
if (amount == 0) revert InsufficientZRC20Amount();
if (gasLimit == 0) revert InsufficientGasLimit();
if (message.length + revertOptions.revertMessage.length >= MAX_MESSAGE_SIZE) revert MessageSizeExceeded();
if (message.length + revertOptions.revertMessage.length > MAX_MESSAGE_SIZE) revert MessageSizeExceeded();

uint256 gasFee = _withdrawZRC20WithGasLimit(amount, zrc20, gasLimit);
emit Withdrawn(
Expand Down Expand Up @@ -220,7 +221,7 @@ contract GatewayZEVM is
if (receiver.length == 0) revert ZeroAddress();
if (amount == 0) revert InsufficientZRC20Amount();
if (callOptions.gasLimit == 0) revert InsufficientGasLimit();
if (message.length + revertOptions.revertMessage.length >= MAX_MESSAGE_SIZE) revert MessageSizeExceeded();
if (message.length + revertOptions.revertMessage.length > MAX_MESSAGE_SIZE) revert MessageSizeExceeded();

uint256 gasFee = _withdrawZRC20WithGasLimit(amount, zrc20, callOptions.gasLimit);
emit Withdrawn(
Expand Down Expand Up @@ -253,6 +254,7 @@ contract GatewayZEVM is
{
if (receiver.length == 0) revert ZeroAddress();
if (amount == 0) revert InsufficientZetaAmount();
if (revertOptions.revertMessage.length > MAX_MESSAGE_SIZE) revert MessageSizeExceeded();

_transferZETA(amount, PROTOCOL_ADDRESS);
emit Withdrawn(
Expand Down Expand Up @@ -288,7 +290,7 @@ contract GatewayZEVM is
{
if (receiver.length == 0) revert ZeroAddress();
if (amount == 0) revert InsufficientZetaAmount();
if (message.length + revertOptions.revertMessage.length >= MAX_MESSAGE_SIZE) revert MessageSizeExceeded();
if (message.length + revertOptions.revertMessage.length > MAX_MESSAGE_SIZE) revert MessageSizeExceeded();

_transferZETA(amount, PROTOCOL_ADDRESS);
emit Withdrawn(
Expand Down Expand Up @@ -327,7 +329,7 @@ contract GatewayZEVM is
if (receiver.length == 0) revert ZeroAddress();
if (amount == 0) revert InsufficientZetaAmount();
if (callOptions.gasLimit == 0) revert InsufficientGasLimit();
if (message.length + revertOptions.revertMessage.length >= MAX_MESSAGE_SIZE) revert MessageSizeExceeded();
if (message.length + revertOptions.revertMessage.length > MAX_MESSAGE_SIZE) revert MessageSizeExceeded();

_transferZETA(amount, PROTOCOL_ADDRESS);
emit Withdrawn(
Expand All @@ -353,7 +355,7 @@ contract GatewayZEVM is
whenNotPaused
{
if (callOptions.gasLimit == 0) revert InsufficientGasLimit();
if (message.length + revertOptions.revertMessage.length >= MAX_MESSAGE_SIZE) revert MessageSizeExceeded();
if (message.length + revertOptions.revertMessage.length > MAX_MESSAGE_SIZE) revert MessageSizeExceeded();

_call(receiver, zrc20, message, callOptions, revertOptions);
}
Expand All @@ -376,7 +378,7 @@ contract GatewayZEVM is
whenNotPaused
{
if (gasLimit == 0) revert InsufficientGasLimit();
if (message.length + revertOptions.revertMessage.length >= MAX_MESSAGE_SIZE) revert MessageSizeExceeded();
if (message.length + revertOptions.revertMessage.length > MAX_MESSAGE_SIZE) revert MessageSizeExceeded();

_call(receiver, zrc20, message, CallOptions({ gasLimit: gasLimit, isArbitraryCall: true }), revertOptions);
}
Expand All @@ -388,7 +390,7 @@ contract GatewayZEVM is
CallOptions memory callOptions,
RevertOptions memory revertOptions
)
internal
private
{
if (receiver.length == 0) revert ZeroAddress();

Expand Down
2 changes: 1 addition & 1 deletion v2/docs/src/contracts/Revert.sol/interface.Revertable.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# Revertable
[Git Source](https://github.com/zeta-chain/protocol-contracts/blob/b0a690824216f461bd292d05ff57810c5c3ecafd/contracts/Revert.sol)
[Git Source](https://github.com/zeta-chain/protocol-contracts/blob/45df03a49b31cc5722a5bb6453b743fc8ac35d1f/contracts/Revert.sol)

Interface for contracts that support revertable calls.

Expand Down
2 changes: 1 addition & 1 deletion v2/docs/src/contracts/Revert.sol/struct.RevertContext.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# RevertContext
[Git Source](https://github.com/zeta-chain/protocol-contracts/blob/b0a690824216f461bd292d05ff57810c5c3ecafd/contracts/Revert.sol)
[Git Source](https://github.com/zeta-chain/protocol-contracts/blob/45df03a49b31cc5722a5bb6453b743fc8ac35d1f/contracts/Revert.sol)

Struct containing revert context passed to onRevert.

Expand Down
2 changes: 1 addition & 1 deletion v2/docs/src/contracts/Revert.sol/struct.RevertOptions.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# RevertOptions
[Git Source](https://github.com/zeta-chain/protocol-contracts/blob/b0a690824216f461bd292d05ff57810c5c3ecafd/contracts/Revert.sol)
[Git Source](https://github.com/zeta-chain/protocol-contracts/blob/45df03a49b31cc5722a5bb6453b743fc8ac35d1f/contracts/Revert.sol)

Struct containing revert options

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# ERC20Custody
[Git Source](https://github.com/zeta-chain/protocol-contracts/blob/b0a690824216f461bd292d05ff57810c5c3ecafd/contracts/evm/ERC20Custody.sol)
[Git Source](https://github.com/zeta-chain/protocol-contracts/blob/45df03a49b31cc5722a5bb6453b743fc8ac35d1f/contracts/evm/ERC20Custody.sol)

**Inherits:**
Initializable, UUPSUpgradeable, [IERC20Custody](/contracts/evm/interfaces/IERC20Custody.sol/interface.IERC20Custody.md), ReentrancyGuardUpgradeable, AccessControlUpgradeable, PausableUpgradeable
Expand Down
Loading
Loading