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

refactor: update cctx status to object to split error message into error and revert_error #3326

Open
wants to merge 16 commits into
base: develop
Choose a base branch
from
Open
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
6 changes: 6 additions & 0 deletions changelog.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# CHANGELOG

### Unreleases

### Refactor

* [3326](https://github.com/zeta-chain/node/pull/3326) - improve error messages for cctx status object

## v25.0.0

### Features
Expand Down
5 changes: 5 additions & 0 deletions docs/openapi/openapi.swagger.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -58515,6 +58515,11 @@ definitions:
type: string
format: int64
description: when the CCTX was created. only populated on new transactions.
error_message_revert:
type: string
title: |-
error_message_revert carries information about the revert outbound tx ,
which is created if the first outbound tx fails
kingpinXD marked this conversation as resolved.
Show resolved Hide resolved
zetacoreemissionsParams:
type: object
properties:
Expand Down
69 changes: 69 additions & 0 deletions docs/zetacore/status_message.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
# CCTX status message

The cctx object contains a field `cctx_status` , which has the following structure
```go
type Status struct {
Status CctxStatus `protobuf:"varint,1,opt,name=status,proto3,enum=zetachain.zetacore.crosschain.CctxStatus" json:"status,omitempty"`
StatusMessage string `protobuf:"bytes,2,opt,name=status_message,json=statusMessage,proto3" json:"status_message,omitempty"`
ErrorMessage string `protobuf:"bytes,6,opt,name=error_message,json=errorMessage,proto3" json:"error_message,omitempty"`
LastUpdateTimestamp int64 `protobuf:"varint,3,opt,name=lastUpdate_timestamp,json=lastUpdateTimestamp,proto3" json:"lastUpdate_timestamp,omitempty"`
IsAbortRefunded bool `protobuf:"varint,4,opt,name=isAbortRefunded,proto3" json:"isAbortRefunded,omitempty"`
CreatedTimestamp int64 `protobuf:"varint,5,opt,name=created_timestamp,json=createdTimestamp,proto3" json:"created_timestamp,omitempty"`
ErrorMessageRevert string `protobuf:"bytes,7,opt,name=error_message_revert,json=errorMessageRevert,proto3" json:"error_message_revert,omitempty"`
}
```

## Status
This is the most updated status for the cctx . This can be one of the following values
- `PendingInbound` : The cctx is pending for the inbound to be finalized , this is an intermediately status used by the protocol only
- `PendingOutbound` : This means that the inbound has been finalzied, and the outbound is pending
- `OutboundMined` : The outbound has been successfully mined. This is a terminal status
- `Aborted` : The cctx has been aborted. This is a terminal status
- `PendingRevert` : The the cctx failed at some step and is pending for the revert to be finalized
- `Reverted` : The cctx has been successfully reverted. This is a terminal status

### StatusMessage
The status message provides a some details about the current status.This is primiary meant for the user to quickly understand the status of the cctx.
### LastUpdateTimestamp
The last time the status was updated
### IsAbortRefunded
This is a boolean value which is true if the cctx has been refunded after being aborted or not .
### CreatedTimestamp
The time when the cctx was created
### ErrorMessage and ErrorMessageRevert
A cctx can have a maximum of two outbound params. We can refer to the first outbound as `outbound` and the second as `revert`.
- A normal flow for a cctx is to go from `PendingOutbound` -> `OutboundMined` , which creates a single outbound
- A cctx where the outbound fails has the transition `PendingOutbound` -> `PendingRevert` -> `Reverted` , which creates two outbounds
- Any of the above two flows can abort the cctx at some point that can create either one or two outbounds

- The `ErrorMessage` field only contains a value if the original outbound failed. It contains details about the error that caused the outbound to fail
- The `ErrorMessageRevert` field only contains a value if the revert outbound failed. It contains details about the error that caused the revert outbound to fail.

### Example values for StatusMessage field and how to interpret them
- `initiating outbound` : The inbound votes have been successfully finalized, and the protocol is starting the outbound process
- `outbound successfully mined` : The outbound was successfully mined
- `revert successfully processed` : The outbound failed , but the revert was successful
- `revert failed to be processed` : The revert failed. This message also means that the initial outbound has failed.

- `outbound failed` : The outbound failed, The protocol would try to create a revert either in the same block or schedule one to be picked up by zetaclient
- `outbound failed for admin tx` : The outbound failed for an admin transaction, in this case we do not revert the cctx
- `outbound failed unable to process` : The outbound processing failed at the protocol level. When this happens, the protocol sets the cctx to aborted.
- `outbound failed but the universal contract did not revert` : The outbound/deposit failed, but the contract did not revert,
this is most likely caused by an internal error in the protocol.The CCTX is this case is aborted. Users can try connecting with the zetachain team to get a refund
- `cctx aborted through MsgAbortStuckCCTX` : The cctx was aborted manually by an admin command


### Example values for ErrorMessage and ErrorMessageRevert fields and how to interpret them

- For a failed deposit, the ErrorMessage would contain the following fields. The protocol generates the fields tagged as internal.
```
- method: The method that was called by the protocol
- contract: The contract that his method was called on
- args:The argumets that were used for this call
- errorMessage[Internal]: Error message from the ZEVM call
- revertReason: Revert reason from the smart contract
```

- `outbound tx failed to be executed on connected chain` : `revert tx failed to be executed on connected chain` : The outbound/revert transaction failed to be executed on the connected chain.
- `coin type [CoinType] not supported for revert when source chain is Zetachain` : The coin type is not supported for revert when the source chain is Zetachain.
- `error from EVMDeposit: [Error_String]` : Error returned by the protocol when trying to deposit tokens( and optionally call a contract) on ZEVM. The error string should explain the cause
Original file line number Diff line number Diff line change
Expand Up @@ -52,5 +52,8 @@ func TestBitcoinDepositAndCallRevertWithDust(r *runner.E2ERunner, args []string)
cctx := utils.WaitCctxAbortedByInboundHash(r.Ctx, r, txHash.String(), r.CctxClient)

require.True(r, cctx.GetCurrentOutboundParam().Amount.Uint64() < constant.BTCWithdrawalDustAmount)
require.True(r, strings.Contains(cctx.CctxStatus.ErrorMessage, crosschaintypes.ErrInvalidWithdrawalAmount.Error()))
require.True(
r,
strings.Contains(cctx.CctxStatus.ErrorMessageRevert, crosschaintypes.ErrInvalidWithdrawalAmount.Error()),
)
}
12 changes: 12 additions & 0 deletions proto/zetachain/zetacore/crosschain/cross_chain_tx.proto
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,18 @@ message Status {
bool isAbortRefunded = 4;
// when the CCTX was created. only populated on new transactions.
int64 created_timestamp = 5;
// error_message_revert carries information about the revert outbound tx ,
// which is created if the first outbound tx fails
string error_message_revert = 7;
}

// StatusMessages is a message that carries status and error messages for a
// cctx Currently this is only used internally to pass messages when updating
// the status
message StatusMessages {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This struct is only used internally, and not saved to the state.
In my opinion, the code looks cleaner using this object rather than three strings as arguments. But it can be changed to that if needed

Copy link
Member

Choose a reason for hiding this comment

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

If it's used internally we don't need a proto definition and it can be an internal type

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Defined this at the proto level as we we can refactor the Status object in the future to use this directly instead of defining three fields .
I wanted to get reviews on this pr before creating an issue.

I can refactor to defining a type, though, if its still needed

string status_message = 1;
string error_message_outbound = 2;
string error_message_revert = 3;
}

// ProtocolContractVersion represents the version of the protocol contract used
Expand Down
46 changes: 46 additions & 0 deletions typescript/zetachain/zetacore/crosschain/cross_chain_tx_pb.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,14 @@ export declare class Status extends Message<Status> {
*/
createdTimestamp: bigint;

/**
* error_message_revert carries information about the revert outbound tx ,
* which is created if the first outbound tx fails
*
* @generated from field: string error_message_revert = 7;
*/
errorMessageRevert: string;

constructor(data?: PartialMessage<Status>);

static readonly runtime: typeof proto3;
Expand All @@ -410,6 +418,44 @@ export declare class Status extends Message<Status> {
static equals(a: Status | PlainMessage<Status> | undefined, b: Status | PlainMessage<Status> | undefined): boolean;
}

/**
* StatusMessages is a message that carries status and error messages for a
* cctx Currently this is only used internally to pass messages when updating
* the status
*
* @generated from message zetachain.zetacore.crosschain.StatusMessages
*/
export declare class StatusMessages extends Message<StatusMessages> {
/**
* @generated from field: string status_message = 1;
*/
statusMessage: string;

/**
* @generated from field: string error_message_outbound = 2;
*/
errorMessageOutbound: string;

/**
* @generated from field: string error_message_revert = 3;
*/
errorMessageRevert: string;

constructor(data?: PartialMessage<StatusMessages>);

static readonly runtime: typeof proto3;
static readonly typeName = "zetachain.zetacore.crosschain.StatusMessages";
static readonly fields: FieldList;

static fromBinary(bytes: Uint8Array, options?: Partial<BinaryReadOptions>): StatusMessages;

static fromJson(jsonValue: JsonValue, options?: Partial<JsonReadOptions>): StatusMessages;

static fromJsonString(jsonString: string, options?: Partial<JsonReadOptions>): StatusMessages;

static equals(a: StatusMessages | PlainMessage<StatusMessages> | undefined, b: StatusMessages | PlainMessage<StatusMessages> | undefined): boolean;
}

/**
* RevertOptions represents the options for reverting a cctx
*
Expand Down
5 changes: 4 additions & 1 deletion x/crosschain/keeper/cctx_gateway_observers.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,10 @@ func (c CCTXGatewayObservers) InitiateOutbound(
}()
if err != nil {
// do not commit anything here as the CCTX should be aborted
config.CCTX.SetAbort("internal error", err.Error())
config.CCTX.SetAbort(types.StatusMessages{
StatusMessage: "outbound failed unable to process",
ErrorMessageOutbound: fmt.Sprintf("unable to create outbound: %s", err.Error()),
})
return types.CctxStatus_Aborted, err
}
commit()
Expand Down
9 changes: 6 additions & 3 deletions x/crosschain/keeper/cctx_gateway_zevm.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package keeper

import (
"fmt"

sdk "github.com/cosmos/cosmos-sdk/types"

"github.com/zeta-chain/node/x/crosschain/types"
Expand Down Expand Up @@ -28,9 +30,10 @@ func (c CCTXGatewayZEVM) InitiateOutbound(

if err != nil && !isContractReverted {
// exceptional case; internal error; should abort CCTX
config.CCTX.SetAbort(
"error during deposit that is not smart contract revert",
err.Error())
config.CCTX.SetAbort(types.StatusMessages{
StatusMessage: "outbound failed but the universal contract did not revert",
ErrorMessageOutbound: fmt.Sprintf("failed to deposit tokens in ZEVM: %s", err.Error()),
})
return types.CctxStatus_Aborted, err
}

Expand Down
72 changes: 52 additions & 20 deletions x/crosschain/keeper/cctx_orchestrator_validate_outbound.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,20 +45,25 @@ func (k Keeper) ValidateOutboundZEVM(
if depositErr != nil && isContractReverted {
tmpCtxRevert, commitRevert := ctx.CacheContext()
// contract call reverted; should refund via a revert tx
err := k.processFailedOutboundOnExternalChain(
revertErr := k.processFailedOutboundOnExternalChain(
tmpCtxRevert,
cctx,
types.CctxStatus_PendingOutbound,
depositErr.Error(),
cctx.InboundParams.Amount,
)
if err != nil {
if revertErr != nil {
// Error here would mean the outbound tx failed and we also failed to create a revert tx.
// This is the only case where we set outbound and revert messages,
// as both the outbound and the revert failed in the same block
cctx.SetAbort(
"revert failed",
fmt.Sprintf("deposit error: %s, processing error: %s", depositErr.Error(), err.Error()))
types.StatusMessages{
StatusMessage: fmt.Sprintf("revert failed to be processed"),
ErrorMessageOutbound: depositErr.Error(),
ErrorMessageRevert: revertErr.Error(),
})
return types.CctxStatus_Aborted
}

commitRevert()
return types.CctxStatus_PendingRevert
}
Expand Down Expand Up @@ -108,7 +113,7 @@ func (k Keeper) ValidateOutboundObservers(
// 4. Set the finalization status of the current outbound tx to executed. If a revert tx is is created, the finalization status is not set, it would get set when the revert is processed via a subsequent transaction
//
// This function sets CCTX status , in cases where the outbound tx is successful, but tx itself fails
// This is done because SaveSuccessfulOutbound does not set the cctx status
// This is done because HandleValidOutbound does not set the cctx status
// For cases where the outbound tx is unsuccessful, the cctx status is automatically set to Aborted in the processFailedOutboundObservers function, so we can just return and error to trigger that
func (k Keeper) processFailedOutboundObservers(ctx sdk.Context, cctx *types.CrossChainTx, valueReceived string) error {
oldStatus := cctx.CctxStatus.Status
Expand All @@ -124,7 +129,9 @@ func (k Keeper) processFailedOutboundObservers(ctx sdk.Context, cctx *types.Cros
if cctx.InboundParams.CoinType == coin.CoinType_Cmd {
// if the cctx is of coin type cmd or the sender chain is zeta chain, then we do not revert, the cctx is aborted
cctx.GetCurrentOutboundParam().TxFinalizationStatus = types.TxFinalizationStatus_Executed
cctx.SetAbort("", "outbound failed for admin tx")
cctx.SetAbort(types.StatusMessages{
StatusMessage: "outbound failed for admin tx",
})
} else if chains.IsZetaChain(cctx.InboundParams.SenderChainId, k.GetAuthorityKeeper().GetAdditionalChainList(ctx)) {
switch cctx.InboundParams.CoinType {
// Try revert if the coin-type is ZETA
Expand All @@ -139,7 +146,11 @@ func (k Keeper) processFailedOutboundObservers(ctx sdk.Context, cctx *types.Cros
default:
{
cctx.GetCurrentOutboundParam().TxFinalizationStatus = types.TxFinalizationStatus_Executed
cctx.SetAbort("", "outbound failed for non-ZETA cctx")
cctx.SetAbort(types.StatusMessages{
StatusMessage: "outbound failed",
ErrorMessageOutbound: "outbound tx failed to be executed on connected chain",
ErrorMessageRevert: fmt.Sprintf("revert on ZetaChain is not supported for cctx v1 with coin type %s", cctx.InboundParams.CoinType),
})
}
}
} else {
Expand All @@ -158,7 +169,7 @@ func (k Keeper) processFailedOutboundOnExternalChain(
ctx sdk.Context,
cctx *types.CrossChainTx,
oldStatus types.CctxStatus,
revertMsg string,
errorMsg string,
inputAmount math.Uint,
) error {
switch oldStatus {
Expand Down Expand Up @@ -210,10 +221,16 @@ func (k Keeper) processFailedOutboundOnExternalChain(
return err
}
// Not setting the finalization status here, the required changes have been made while creating the revert tx
cctx.SetPendingRevert("", revertMsg)
cctx.SetPendingRevert(types.StatusMessages{
StatusMessage: "outbound failed",
kingpinXD marked this conversation as resolved.
Show resolved Hide resolved
ErrorMessageOutbound: errorMsg,
lumtis marked this conversation as resolved.
Show resolved Hide resolved
})
case types.CctxStatus_PendingRevert:
cctx.GetCurrentOutboundParam().TxFinalizationStatus = types.TxFinalizationStatus_Executed
cctx.SetAbort("aborted while processing failed outbound", "outbound and revert failed")
cctx.SetAbort(types.StatusMessages{
StatusMessage: "revert failed to be processed",
ErrorMessageRevert: errorMsg,
})
}
return nil
}
Expand All @@ -229,7 +246,7 @@ func (k Keeper) processFailedOutboundOnExternalChain(
// 3. Emit an event for the successful outbound transaction if flag is provided
//
// This function sets CCTX status, in cases where the outbound tx is successful, but tx itself fails
// This is done because SaveSuccessfulOutbound does not set the cctx status
// This is done because HandleValidOutbound does not set the cctx status
// For cases where the outbound tx is unsuccessful, the cctx status is automatically set to Aborted in the processFailedOutboundObservers function, so we can just return and error to trigger that
func (k Keeper) processSuccessfulOutbound(
ctx sdk.Context,
Expand All @@ -240,9 +257,9 @@ func (k Keeper) processSuccessfulOutbound(
oldStatus := cctx.CctxStatus.Status
switch oldStatus {
case types.CctxStatus_PendingRevert:
cctx.SetReverted("", "")
cctx.SetReverted(types.StatusMessages{StatusMessage: "revert successfully processed"})
case types.CctxStatus_PendingOutbound:
cctx.SetOutboundMined("")
cctx.SetOutboundMined(types.StatusMessages{StatusMessage: "outbound successfully mined"})
default:
return
}
Expand Down Expand Up @@ -270,8 +287,12 @@ func (k Keeper) processFailedZETAOutboundOnZEVM(ctx sdk.Context, cctx *types.Cro
return fmt.Errorf("failed AddRevertOutbound: %s", err.Error())
}

// Trying to revert the transaction this would get set to a finalized status in the same block as this does not need a TSS singing
cctx.SetPendingRevert("", "outbound failed")
// Trying to revert the transaction, this would get set to a finalized status in the same block as this does not need a TSS singing
// The outbound failed due to majority of observer voting failure.We do not have the exact reason for the failure available on chain.
cctx.SetPendingRevert(types.StatusMessages{
StatusMessage: "outbound failed",
kingpinXD marked this conversation as resolved.
Show resolved Hide resolved
ErrorMessageOutbound: "outbound failed to be executed on connected chain",
})
data, err := base64.StdEncoding.DecodeString(cctx.RelayedMessage)
if err != nil {
return fmt.Errorf("failed decoding relayed message: %s", err.Error())
Expand Down Expand Up @@ -305,7 +326,10 @@ func (k Keeper) processFailedZETAOutboundOnZEVM(ctx sdk.Context, cctx *types.Cro
return fmt.Errorf("failed ZETARevertAndCallContract: %s", err.Error())
}

cctx.SetReverted("", "outbound failed")
cctx.SetReverted(types.StatusMessages{
StatusMessage: "revert successfully processed",
})

if len(ctx.TxBytes()) > 0 {
// add event for tendermint transaction hash format
hash := tmbytes.HexBytes(tmtypes.Tx(ctx.TxBytes()).Hash())
Expand Down Expand Up @@ -350,7 +374,10 @@ func (k Keeper) processFailedOutboundV2(ctx sdk.Context, cctx *types.CrossChainT
}

// update status
cctx.SetPendingRevert("", "outbound failed")
cctx.SetPendingRevert(types.StatusMessages{
StatusMessage: "outbound failed",
ErrorMessageOutbound: "outbound tx failed to be executed on connected chain",
})

// process the revert on ZEVM
if err := k.fungibleKeeper.ProcessV2RevertDeposit(
Expand All @@ -368,7 +395,9 @@ func (k Keeper) processFailedOutboundV2(ctx sdk.Context, cctx *types.CrossChainT
}

// tx is reverted
cctx.SetReverted("", "outbound failed")
cctx.SetReverted(types.StatusMessages{
StatusMessage: "revert successfully processed",
})

// add event for tendermint transaction hash format
if len(ctx.TxBytes()) > 0 {
Expand All @@ -381,7 +410,10 @@ func (k Keeper) processFailedOutboundV2(ctx sdk.Context, cctx *types.CrossChainT
cctx.GetCurrentOutboundParam().TxFinalizationStatus = types.TxFinalizationStatus_Executed
case types.CctxStatus_PendingRevert:
cctx.GetCurrentOutboundParam().TxFinalizationStatus = types.TxFinalizationStatus_Executed
cctx.SetAbort("aborted while processing failed outbound", "outbound and revert failed")
cctx.SetAbort(types.StatusMessages{
StatusMessage: "revert failed to be processed",
ErrorMessageRevert: "revert tx failed to be executed on connected chain",
})
}
return nil
}
Loading
Loading