Skip to content

Commit

Permalink
review fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
@skosito
skosito committed Oct 14, 2024
1 parent 6fa64b6 commit 915a2b6
Show file tree
Hide file tree
Showing 13 changed files with 51 additions and 44 deletions.
4 changes: 2 additions & 2 deletions v2/contracts/evm/ERC20Custody.sol
View file
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
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ contract GatewayEVM is
/// @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) {
revertIfCallingOnRevert(data);
_revertIfCallingOnRevert(data);
(bool success, bytes memory result) = destination.call{ value: msg.value }(data);
if (!success) revert ExecutionFailed();

Expand All @@ -95,9 +95,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 @@ -178,18 +178,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
_execute(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 @@ -238,6 +238,7 @@ contract GatewayEVM is
if (revertOptions.callOnRevert) revert CallOnRevertNotSupported();
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 @@ -264,8 +265,9 @@ contract GatewayEVM is
if (revertOptions.callOnRevert) revert CallOnRevertNotSupported();
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 @@ -287,7 +289,7 @@ contract GatewayEVM is
if (revertOptions.callOnRevert) revert CallOnRevertNotSupported();
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 @@ -316,9 +318,9 @@ contract GatewayEVM is
if (revertOptions.callOnRevert) revert CallOnRevertNotSupported();
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 @@ -338,7 +340,7 @@ contract GatewayEVM is
{
if (revertOptions.callOnRevert) revert CallOnRevertNotSupported();
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 @@ -368,7 +370,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 @@ -378,7 +380,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 @@ -399,7 +401,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 @@ -414,7 +416,7 @@ contract GatewayEVM is
}

// @dev prevent spoofing onRevert functions
function revertIfCallingOnRevert(bytes calldata data) private pure {
function _revertIfCallingOnRevert(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
View file
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);

emit UpdatedZetaConnectorTSSAddress(tssAddress, newTSSAddress);

tssAddress = newTSSAddress;

emit UpdatedZetaConnectorTSSAddress(newTSSAddress);
}

/// @notice Pause contract.
Expand Down
2 changes: 1 addition & 1 deletion v2/contracts/evm/ZetaConnectorNonNative.sol
View file
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
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ interface IERC20CustodyEvents {

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

/// @title IERC20CustodyErrors
Expand Down
3 changes: 2 additions & 1 deletion v2/contracts/evm/interfaces/IGatewayEVM.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ interface IGatewayEVMEvents {

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

/// @title IGatewayEVMErrors
Expand Down
3 changes: 2 additions & 1 deletion v2/contracts/evm/interfaces/IZetaConnector.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,6 @@ interface IZetaConnectorEvents {

/// @notice Emitted when tss address is updated
/// @param newTSSAddress new tss address
event UpdatedZetaConnectorTSSAddress(address newTSSAddress);
/// @param oldTSSAddress old tss address
event UpdatedZetaConnectorTSSAddress(address oldTSSAddress, address newTSSAddress);
}
18 changes: 10 additions & 8 deletions v2/contracts/zevm/GatewayZEVM.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,21 +88,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, FUNGIBLE_MODULE_ADDRESS, gasFee)) {
revert GasFeeTransferFailed();
Expand All @@ -117,10 +117,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 @@ -145,6 +145,7 @@ contract GatewayZEVM is
if (revertOptions.callOnRevert) revert CallOnRevertNotSupported();
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 @@ -219,6 +220,7 @@ contract GatewayZEVM is
if (revertOptions.callOnRevert) revert CallOnRevertNotSupported();
if (receiver.length == 0) revert ZeroAddress();
if (amount == 0) revert InsufficientZetaAmount();
if (revertOptions.revertMessage.length > MAX_MESSAGE_SIZE) revert MessageSizeExceeded();

_transferZETA(amount, FUNGIBLE_MODULE_ADDRESS);
emit Withdrawn(msg.sender, chainId, receiver, address(zetaToken), amount, 0, 0, "", 0, revertOptions);
Expand All @@ -245,7 +247,7 @@ contract GatewayZEVM is
if (revertOptions.callOnRevert) revert CallOnRevertNotSupported();
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, FUNGIBLE_MODULE_ADDRESS);
emit Withdrawn(msg.sender, chainId, receiver, address(zetaToken), amount, 0, 0, message, 0, revertOptions);
Expand All @@ -271,7 +273,7 @@ contract GatewayZEVM is
if (revertOptions.callOnRevert) revert CallOnRevertNotSupported();
if (receiver.length == 0) revert ZeroAddress();
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();

(address gasZRC20, uint256 gasFee) = IZRC20(zrc20).withdrawGasFeeWithGasLimit(gasLimit);
if (!IZRC20(gasZRC20).transferFrom(msg.sender, FUNGIBLE_MODULE_ADDRESS, gasFee)) {
Expand Down
2 changes: 1 addition & 1 deletion v2/test/ERC20Custody.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ contract ERC20CustodyTest is Test, IGatewayEVMErrors, IGatewayEVMEvents, IReceiv

vm.startPrank(owner);
vm.expectEmit(true, true, true, true, address(custody));
emit UpdatedCustodyTSSAddress(newTSSAddress);
emit UpdatedCustodyTSSAddress(tssAddress, newTSSAddress);
custody.updateTSSAddress(newTSSAddress);
assertEq(newTSSAddress, custody.tssAddress());

Expand Down
14 changes: 7 additions & 7 deletions v2/test/GatewayEVM.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ contract GatewayEVMTest is Test, IGatewayEVMErrors, IGatewayEVMEvents, IReceiver

vm.startPrank(owner);
vm.expectEmit(true, true, true, true, address(gateway));
emit UpdatedGatewayTSSAddress(newTSSAddress);
emit UpdatedGatewayTSSAddress(tssAddress, newTSSAddress);
gateway.updateTSSAddress(newTSSAddress);
assertEq(newTSSAddress, gateway.tssAddress());

Expand Down Expand Up @@ -501,8 +501,8 @@ contract GatewayEVMInboundTest is Test, IGatewayEVMErrors, IGatewayEVMEvents, IR

function testDepositERC20ToCustodyWithPayloadFailsIfPayloadSizeExceeded() public {
uint256 amount = 100_000;
bytes memory payload = new bytes(512);
revertOptions.revertMessage = new bytes(512);
bytes memory payload = new bytes(gateway.MAX_PAYLOAD_SIZE() / 2);
revertOptions.revertMessage = new bytes(gateway.MAX_PAYLOAD_SIZE() / 2 + 1);

token.approve(address(gateway), amount);

Expand Down Expand Up @@ -576,8 +576,8 @@ contract GatewayEVMInboundTest is Test, IGatewayEVMErrors, IGatewayEVMEvents, IR

function testDepositEthToTssWithPayloadFailsIfPayloadSizeExceeded() public {
uint256 amount = 100_000;
bytes memory payload = new bytes(512);
revertOptions.revertMessage = new bytes(512);
bytes memory payload = new bytes(gateway.MAX_PAYLOAD_SIZE() / 2);
revertOptions.revertMessage = new bytes(gateway.MAX_PAYLOAD_SIZE() / 2 + 1);

vm.expectRevert(PayloadSizeExceeded.selector);
gateway.depositAndCall{ value: amount }(destination, payload, revertOptions);
Expand Down Expand Up @@ -608,8 +608,8 @@ contract GatewayEVMInboundTest is Test, IGatewayEVMErrors, IGatewayEVMEvents, IR
}

function testCallWithPayloadFailsIfPayloadSizeExceeded() public {
bytes memory payload = new bytes(512);
revertOptions.revertMessage = new bytes(512);
bytes memory payload = new bytes(gateway.MAX_PAYLOAD_SIZE() / 2);
revertOptions.revertMessage = new bytes(gateway.MAX_PAYLOAD_SIZE() / 2 + 1);

vm.expectRevert(PayloadSizeExceeded.selector);
gateway.call(destination, payload, revertOptions);
Expand Down
2 changes: 1 addition & 1 deletion v2/test/ZetaConnectorNative.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ contract ZetaConnectorNativeTest is

vm.startPrank(owner);
vm.expectEmit(true, true, true, true, address(zetaConnector));
emit UpdatedZetaConnectorTSSAddress(newTSSAddress);
emit UpdatedZetaConnectorTSSAddress(tssAddress, newTSSAddress);
zetaConnector.updateTSSAddress(newTSSAddress);
assertEq(newTSSAddress, zetaConnector.tssAddress());

Expand Down
4 changes: 2 additions & 2 deletions v2/test/utils/upgrades/ERC20CustodyUpgradeTest.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,9 @@ contract ERC20CustodyUpgradeTest 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
4 changes: 2 additions & 2 deletions v2/test/utils/upgrades/GatewayEVMUpgradeTest.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,9 @@ contract GatewayEVMUpgradeTest is
_revokeRole(TSS_ROLE, tssAddress);
_grantRole(TSS_ROLE, newTSSAddress);

emit UpdatedGatewayTSSAddress(tssAddress, newTSSAddress);

tssAddress = newTSSAddress;

emit UpdatedGatewayTSSAddress(newTSSAddress);
}

/// @notice Pause contract.
Expand Down

0 comments on commit 915a2b6

Please sign in to comment.