Skip to content

Commit

Permalink
fix: review fixes (#383)
Browse files Browse the repository at this point in the history
  • Loading branch information
skosito authored Oct 14, 2024
1 parent 93c01e5 commit f7f6d9f
Show file tree
Hide file tree
Showing 108 changed files with 662 additions and 357 deletions.
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();

(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

0 comments on commit f7f6d9f

Please sign in to comment.