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

feat: improve revert handling #361

Merged
merged 23 commits into from
Sep 27, 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
2 changes: 2 additions & 0 deletions v2/contracts/Revert.sol
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,12 @@ struct RevertOptions {
}

/// @notice Struct containing revert context passed to onRevert.
/// @param sender Address of account that initiated smart contract call.
/// @param asset Address of asset, empty if it's gas token.
/// @param amount Amount specified with the transaction.
/// @param revertMessage Arbitrary data sent back in onRevert.
struct RevertContext {
address sender;
address asset;
uint64 amount;
bytes revertMessage;
Expand Down
10 changes: 7 additions & 3 deletions v2/contracts/evm/GatewayEVM.sol
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,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) {
revertIfAuthenticatedCall(data);
revertIfOnCallOrOnRevert(data);
(bool success, bytes memory result) = destination.call{ value: msg.value }(data);
if (!success) revert ExecutionFailed();

Expand All @@ -429,8 +429,8 @@ contract GatewayEVM is
return Callable(destination).onCall{ value: msg.value }(messageContext, data);
}

// @dev prevent calling onCall function reserved for authenticated calls
function revertIfAuthenticatedCall(bytes calldata data) private pure {
// @dev prevent spoofing onCall and onRevert functions
function revertIfOnCallOrOnRevert(bytes calldata data) private pure {
if (data.length >= 4) {
bytes4 functionSelector;
assembly {
Expand All @@ -440,6 +440,10 @@ contract GatewayEVM is
if (functionSelector == Callable.onCall.selector) {
revert NotAllowedToCallOnCall();
}

if (functionSelector == Revertable.onRevert.selector) {
revert NotAllowedToCallOnRevert();
}
}
}
}
3 changes: 3 additions & 0 deletions v2/contracts/evm/interfaces/IGatewayEVM.sol
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,9 @@ interface IGatewayEVMErrors {

/// @notice Error when trying to call onCall method using arbitrary call.
error NotAllowedToCallOnCall();

/// @notice Error when trying to call onRevert method using arbitrary call.
error NotAllowedToCallOnRevert();
}

/// @title IGatewayEVM
Expand Down
6 changes: 3 additions & 3 deletions v2/contracts/zevm/GatewayZEVM.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ pragma solidity 0.8.26;

import { CallOptions, IGatewayZEVM } from "./interfaces/IGatewayZEVM.sol";

import { RevertContext, RevertOptions } from "../../contracts/Revert.sol";
import { RevertContext, RevertOptions, Revertable } from "../../contracts/Revert.sol";
import "./interfaces/IWZETA.sol";
import { IZRC20 } from "./interfaces/IZRC20.sol";
import { UniversalContract, zContext } from "./interfaces/UniversalContract.sol";
Expand Down Expand Up @@ -474,7 +474,7 @@ contract GatewayZEVM is
function executeRevert(address target, RevertContext calldata revertContext) external onlyProtocol whenNotPaused {
if (target == address(0)) revert ZeroAddress();

UniversalContract(target).onRevert(revertContext);
Revertable(target).onRevert(revertContext);
Copy link

Choose a reason for hiding this comment

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

Using onRevert() in the GatewayEVm seems to be missing here, but looking at the state of the main branch it seems to have been merged in another PR.

}

/// @notice Deposit foreign coins into ZRC20 and revert a user-specified contract on ZEVM.
Expand All @@ -497,6 +497,6 @@ contract GatewayZEVM is
if (target == PROTOCOL_ADDRESS || target == address(this)) revert InvalidTarget();

if (!IZRC20(zrc20).deposit(target, amount)) revert ZRC20DepositFailed();
UniversalContract(target).onRevert(revertContext);
Revertable(target).onRevert(revertContext);
}
}
2 changes: 0 additions & 2 deletions v2/contracts/zevm/interfaces/UniversalContract.sol
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,4 @@ interface UniversalContract {
bytes calldata message
)
external;

function onRevert(RevertContext calldata revertContext) external;
skosito marked this conversation as resolved.
Show resolved Hide resolved
}
5 changes: 5 additions & 0 deletions v2/docs/src/SUMMARY.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
- [IGatewayEVMEvents](contracts/evm/interfaces/IGatewayEVM.sol/interface.IGatewayEVMEvents.md)
- [IGatewayEVMErrors](contracts/evm/interfaces/IGatewayEVM.sol/interface.IGatewayEVMErrors.md)
- [IGatewayEVM](contracts/evm/interfaces/IGatewayEVM.sol/interface.IGatewayEVM.md)
- [MessageContext](contracts/evm/interfaces/IGatewayEVM.sol/struct.MessageContext.md)
- [Callable](contracts/evm/interfaces/IGatewayEVM.sol/interface.Callable.md)
- [IZetaConnectorEvents](contracts/evm/interfaces/IZetaConnector.sol/interface.IZetaConnectorEvents.md)
- [IZetaNonEthNew](contracts/evm/interfaces/IZetaNonEthNew.sol/interface.IZetaNonEthNew.md)
- [ERC20Custody](contracts/evm/ERC20Custody.sol/contract.ERC20Custody.md)
Expand All @@ -21,6 +23,7 @@
- [IGatewayZEVMEvents](contracts/zevm/interfaces/IGatewayZEVM.sol/interface.IGatewayZEVMEvents.md)
- [IGatewayZEVMErrors](contracts/zevm/interfaces/IGatewayZEVM.sol/interface.IGatewayZEVMErrors.md)
- [IGatewayZEVM](contracts/zevm/interfaces/IGatewayZEVM.sol/interface.IGatewayZEVM.md)
- [CallOptions](contracts/zevm/interfaces/IGatewayZEVM.sol/struct.CallOptions.md)
- [ISystem](contracts/zevm/interfaces/ISystem.sol/interface.ISystem.md)
- [IWETH9](contracts/zevm/interfaces/IWZETA.sol/interface.IWETH9.md)
- [IZRC20](contracts/zevm/interfaces/IZRC20.sol/interface.IZRC20.md)
Expand All @@ -31,6 +34,8 @@
- [zContract](contracts/zevm/interfaces/UniversalContract.sol/interface.zContract.md)
- [UniversalContract](contracts/zevm/interfaces/UniversalContract.sol/interface.UniversalContract.md)
- [GatewayZEVM](contracts/zevm/GatewayZEVM.sol/contract.GatewayZEVM.md)
- [SystemContractErrors](contracts/zevm/SystemContract.sol/interface.SystemContractErrors.md)
- [SystemContract](contracts/zevm/SystemContract.sol/contract.SystemContract.md)
- [ZRC20Errors](contracts/zevm/ZRC20.sol/interface.ZRC20Errors.md)
- [ZRC20](contracts/zevm/ZRC20.sol/contract.ZRC20.md)
- [RevertOptions](contracts/Revert.sol/struct.RevertOptions.md)
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/92837ac9178ca835368558d37c2ae9322f290363/contracts/Revert.sol)
[Git Source](https://github.com/zeta-chain/protocol-contracts/blob/3a274ce7bad045a879c73669586611d35509cbce/contracts/Revert.sol)

Interface for contracts that support revertable calls.

Expand Down
4 changes: 3 additions & 1 deletion v2/docs/src/contracts/Revert.sol/struct.RevertContext.md
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
# RevertContext
[Git Source](https://github.com/zeta-chain/protocol-contracts/blob/92837ac9178ca835368558d37c2ae9322f290363/contracts/Revert.sol)
[Git Source](https://github.com/zeta-chain/protocol-contracts/blob/3a274ce7bad045a879c73669586611d35509cbce/contracts/Revert.sol)

Struct containing revert context passed to onRevert.


```solidity
struct RevertContext {
address sender;
address asset;
uint64 amount;
bytes revertMessage;
Expand All @@ -16,6 +17,7 @@ struct RevertContext {

|Name|Type|Description|
|----|----|-----------|
|`sender`|`address`|Address of account that initiated smart contract call.|
|`asset`|`address`|Address of asset, empty if it's gas token.|
|`amount`|`uint64`|Amount specified with the transaction.|
|`revertMessage`|`bytes`|Arbitrary data sent back in 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/92837ac9178ca835368558d37c2ae9322f290363/contracts/Revert.sol)
[Git Source](https://github.com/zeta-chain/protocol-contracts/blob/3a274ce7bad045a879c73669586611d35509cbce/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/92837ac9178ca835368558d37c2ae9322f290363/contracts/evm/ERC20Custody.sol)
[Git Source](https://github.com/zeta-chain/protocol-contracts/blob/3a274ce7bad045a879c73669586611d35509cbce/contracts/evm/ERC20Custody.sol)

**Inherits:**
[IERC20Custody](/contracts/evm/interfaces/IERC20Custody.sol/interface.IERC20Custody.md), ReentrancyGuard, AccessControl, Pausable
Expand Down
118 changes: 94 additions & 24 deletions v2/docs/src/contracts/evm/GatewayEVM.sol/contract.GatewayEVM.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# GatewayEVM
[Git Source](https://github.com/zeta-chain/protocol-contracts/blob/92837ac9178ca835368558d37c2ae9322f290363/contracts/evm/GatewayEVM.sol)
[Git Source](https://github.com/zeta-chain/protocol-contracts/blob/3a274ce7bad045a879c73669586611d35509cbce/contracts/evm/GatewayEVM.sol)

**Inherits:**
Initializable, AccessControlUpgradeable, UUPSUpgradeable, [IGatewayEVM](/contracts/evm/interfaces/IGatewayEVM.sol/interface.IGatewayEVM.md), ReentrancyGuardUpgradeable, PausableUpgradeable
Expand Down Expand Up @@ -107,28 +107,6 @@ function _authorizeUpgrade(address newImplementation) internal override onlyRole
|`newImplementation`|`address`|Address of the new implementation.|


### _execute

*Internal function to execute a call to a destination address.*


```solidity
function _execute(address destination, bytes calldata data) internal returns (bytes memory);
```
**Parameters**

|Name|Type|Description|
|----|----|-----------|
|`destination`|`address`|Address to call.|
|`data`|`bytes`|Calldata to pass to the call.|

**Returns**

|Name|Type|Description|
|----|----|-----------|
|`<none>`|`bytes`|The result of the call.|


### pause

Pause contract.
Expand Down Expand Up @@ -177,13 +155,14 @@ function executeRevert(

### execute

Executes a call to a destination address without ERC20 tokens.
Executes an authenticated call to a destination address without ERC20 tokens.

*This function can only be called by the TSS address and it is payable.*


```solidity
function execute(
MessageContext calldata messageContext,
address destination,
bytes calldata data
)
Expand All @@ -196,6 +175,39 @@ function execute(
```
**Parameters**

|Name|Type|Description|
|----|----|-----------|
|`messageContext`|`MessageContext`|Message context containing sender.|
|`destination`|`address`|Address to call.|
|`data`|`bytes`|Calldata to pass to the call.|

**Returns**

|Name|Type|Description|
|----|----|-----------|
|`<none>`|`bytes`|The result of the call.|


### execute

Executes an arbitrary call to a destination address without ERC20 tokens.

*This function can only be called by the TSS address and it is payable.*


```solidity
function execute(
address destination,
bytes calldata data
)
external
payable
onlyRole(TSS_ROLE)
whenNotPaused
returns (bytes memory);
```
**Parameters**

|Name|Type|Description|
|----|----|-----------|
|`destination`|`address`|Address to call.|
Expand Down Expand Up @@ -478,3 +490,61 @@ function transferToAssetHandler(address token, uint256 amount) private;
|`amount`|`uint256`|Amount of tokens to transfer.|


### _executeArbitraryCall

*Private function to execute an arbitrary call to a destination address.*


```solidity
function _executeArbitraryCall(address destination, bytes calldata data) private returns (bytes memory);
```
**Parameters**

|Name|Type|Description|
|----|----|-----------|
|`destination`|`address`|Address to call.|
|`data`|`bytes`|Calldata to pass to the call.|

**Returns**

|Name|Type|Description|
|----|----|-----------|
|`<none>`|`bytes`|The result of the call.|


### _executeAuthenticatedCall

*Private function to execute an authenticated call to a destination address.*


```solidity
function _executeAuthenticatedCall(
MessageContext calldata messageContext,
address destination,
bytes calldata data
)
private
returns (bytes memory);
```
**Parameters**

|Name|Type|Description|
|----|----|-----------|
|`messageContext`|`MessageContext`|Message context containing sender and arbitrary call flag.|
|`destination`|`address`|Address to call.|
|`data`|`bytes`|Calldata to pass to the call.|

**Returns**

|Name|Type|Description|
|----|----|-----------|
|`<none>`|`bytes`|The result of the call.|


### revertIfOnCallOrOnRevert


```solidity
function revertIfOnCallOrOnRevert(bytes calldata data) private pure;
```

Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# ZetaConnectorBase
[Git Source](https://github.com/zeta-chain/protocol-contracts/blob/92837ac9178ca835368558d37c2ae9322f290363/contracts/evm/ZetaConnectorBase.sol)
[Git Source](https://github.com/zeta-chain/protocol-contracts/blob/3a274ce7bad045a879c73669586611d35509cbce/contracts/evm/ZetaConnectorBase.sol)

**Inherits:**
[IZetaConnectorEvents](/contracts/evm/interfaces/IZetaConnector.sol/interface.IZetaConnectorEvents.md), ReentrancyGuard, Pausable, AccessControl
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# ZetaConnectorNative
[Git Source](https://github.com/zeta-chain/protocol-contracts/blob/92837ac9178ca835368558d37c2ae9322f290363/contracts/evm/ZetaConnectorNative.sol)
[Git Source](https://github.com/zeta-chain/protocol-contracts/blob/3a274ce7bad045a879c73669586611d35509cbce/contracts/evm/ZetaConnectorNative.sol)

**Inherits:**
[ZetaConnectorBase](/contracts/evm/ZetaConnectorBase.sol/abstract.ZetaConnectorBase.md)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# ZetaConnectorNonNative
[Git Source](https://github.com/zeta-chain/protocol-contracts/blob/92837ac9178ca835368558d37c2ae9322f290363/contracts/evm/ZetaConnectorNonNative.sol)
[Git Source](https://github.com/zeta-chain/protocol-contracts/blob/3a274ce7bad045a879c73669586611d35509cbce/contracts/evm/ZetaConnectorNonNative.sol)

**Inherits:**
[ZetaConnectorBase](/contracts/evm/ZetaConnectorBase.sol/abstract.ZetaConnectorBase.md)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# IERC20Custody
[Git Source](https://github.com/zeta-chain/protocol-contracts/blob/92837ac9178ca835368558d37c2ae9322f290363/contracts/evm/interfaces/IERC20Custody.sol)
[Git Source](https://github.com/zeta-chain/protocol-contracts/blob/3a274ce7bad045a879c73669586611d35509cbce/contracts/evm/interfaces/IERC20Custody.sol)

**Inherits:**
[IERC20CustodyEvents](/contracts/evm/interfaces/IERC20Custody.sol/interface.IERC20CustodyEvents.md), [IERC20CustodyErrors](/contracts/evm/interfaces/IERC20Custody.sol/interface.IERC20CustodyErrors.md)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# IERC20CustodyErrors
[Git Source](https://github.com/zeta-chain/protocol-contracts/blob/92837ac9178ca835368558d37c2ae9322f290363/contracts/evm/interfaces/IERC20Custody.sol)
[Git Source](https://github.com/zeta-chain/protocol-contracts/blob/3a274ce7bad045a879c73669586611d35509cbce/contracts/evm/interfaces/IERC20Custody.sol)

Interface for the errors used in the ERC20 custody contract.

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# IERC20CustodyEvents
[Git Source](https://github.com/zeta-chain/protocol-contracts/blob/92837ac9178ca835368558d37c2ae9322f290363/contracts/evm/interfaces/IERC20Custody.sol)
[Git Source](https://github.com/zeta-chain/protocol-contracts/blob/3a274ce7bad045a879c73669586611d35509cbce/contracts/evm/interfaces/IERC20Custody.sol)

Interface for the events emitted by the ERC20 custody contract.

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# Callable
[Git Source](https://github.com/zeta-chain/protocol-contracts/blob/3a274ce7bad045a879c73669586611d35509cbce/contracts/evm/interfaces/IGatewayEVM.sol)

Interface implemented by contracts receiving authenticated calls.


## Functions
### onCall


```solidity
function onCall(MessageContext calldata context, bytes calldata message) external payable returns (bytes memory);
```

Loading
Loading