Skip to content

Commit

Permalink
refactor: add error field in cctx (#2952)
Browse files Browse the repository at this point in the history
* feat: add error field in cctx

* feat: divide messages into status and error

* fix unit tests

* add function to update error messages

* add query to get cctx error message

* fix error messages

* reviews suggestions

* include new generated files

* make every msg lowercase

* applied suggestions
  • Loading branch information
fbac authored Oct 8, 2024
1 parent 1a32f05 commit 45a9b05
Show file tree
Hide file tree
Showing 23 changed files with 278 additions and 166 deletions.
1 change: 1 addition & 0 deletions changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
* [2826](https://github.com/zeta-chain/node/pull/2826) - remove unused code from emissions module and add new parameter for fixed block reward amount
* [2890](https://github.com/zeta-chain/node/pull/2890) - refactor `MsgUpdateChainInfo` to accept a single chain, and add `MsgRemoveChainInfo` to remove a chain
* [2899](https://github.com/zeta-chain/node/pull/2899) - remove btc deposit fee v1 and improve unit tests
* [2952](https://github.com/zeta-chain/node/pull/2952) - add error_message to cctx.status

### Tests

Expand Down
8 changes: 8 additions & 0 deletions docs/openapi/openapi.swagger.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -58446,6 +58446,14 @@ definitions:
$ref: '#/definitions/crosschainCctxStatus'
status_message:
type: string
description: |-
status_message carries information about the status transitions:
why they were triggered, old and new status.
error_message:
type: string
description: |-
error_message carries information about the error that caused the tx
to be PendingRevert, Reverted or Aborted.
lastUpdate_timestamp:
type: string
format: int64
Expand Down
4 changes: 2 additions & 2 deletions e2e/e2etests/test_eth_deposit_call.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,6 @@ func TestEtherDepositAndCall(r *runner.E2ERunner, args []string) {

r.Logger.Info("Cross-chain call to reverter reverted")

// check the status message contains revert error hash in case of revert
require.Contains(r, cctx.CctxStatus.StatusMessage, utils.ErrHashRevertFoo)
// Check the error carries the revert executed.
require.Contains(r, cctx.CctxStatus.ErrorMessage, "revert executed")
}
4 changes: 2 additions & 2 deletions e2e/e2etests/test_solana_deposit_refund.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,6 @@ func TestSolanaDepositAndCallRefund(r *runner.E2ERunner, args []string) {
r.Logger.CCTX(*cctx, "solana_deposit_and_refund")
utils.RequireCCTXStatus(r, cctx, crosschaintypes.CctxStatus_Reverted)

// check the status message contains revert error hash in case of revert
require.Contains(r, cctx.CctxStatus.StatusMessage, utils.ErrHashRevertFoo)
// Check the error carries the revert executed.
require.Contains(r, cctx.CctxStatus.ErrorMessage, "revert executed")
}
7 changes: 6 additions & 1 deletion proto/zetachain/zetacore/crosschain/cross_chain_tx.proto
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ enum CctxStatus {
Reverted = 5; // inbound reverted.
Aborted =
6; // inbound tx error or invalid paramters and cannot revert; just abort.
// But the amount can be refunded to zetachain using and admin proposal
// But the amount can be refunded to zetachain using and admin proposal
}

enum TxFinalizationStatus {
Expand Down Expand Up @@ -94,7 +94,12 @@ message OutboundParams {

message Status {
CctxStatus status = 1;
// status_message carries information about the status transitions:
// why they were triggered, old and new status.
string status_message = 2;
// error_message carries information about the error that caused the tx
// to be PendingRevert, Reverted or Aborted.
string error_message = 6;
int64 lastUpdate_timestamp = 3;
bool isAbortRefunded = 4;
// when the CCTX was created. only populated on new transactions.
Expand Down
1 change: 1 addition & 0 deletions testutil/sample/crosschain.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ func Status(t *testing.T, index string) *types.Status {
return &types.Status{
Status: types.CctxStatus(r.Intn(100)),
StatusMessage: String(),
ErrorMessage: String(),
CreatedTimestamp: createdAt,
LastUpdateTimestamp: createdAt,
}
Expand Down
11 changes: 11 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 @@ -355,10 +355,21 @@ export declare class Status extends Message<Status> {
status: CctxStatus;

/**
* status_message carries information about the status transitions:
* why they were triggered, old and new status.
*
* @generated from field: string status_message = 2;
*/
statusMessage: string;

/**
* error_message carries information about the error that caused the tx
* to be PendingRevert, Reverted or Aborted.
*
* @generated from field: string error_message = 6;
*/
errorMessage: string;

/**
* @generated from field: int64 lastUpdate_timestamp = 3;
*/
Expand Down
2 changes: 1 addition & 1 deletion x/crosschain/keeper/cctx_gateway_observers.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ func (c CCTXGatewayObservers) InitiateOutbound(
}()
if err != nil {
// do not commit anything here as the CCTX should be aborted
config.CCTX.SetAbort(err.Error())
config.CCTX.SetAbort("internal error", err.Error())
return types.CctxStatus_Aborted, err
}
commit()
Expand Down
4 changes: 3 additions & 1 deletion x/crosschain/keeper/cctx_gateway_zevm.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@ func (c CCTXGatewayZEVM) InitiateOutbound(

if err != nil && !isContractReverted {
// exceptional case; internal error; should abort CCTX
config.CCTX.SetAbort(err.Error())
config.CCTX.SetAbort(
"error during deposit that is not smart contract revert",
err.Error())
return types.CctxStatus_Aborted, err
}

Expand Down
26 changes: 14 additions & 12 deletions x/crosschain/keeper/cctx_orchestrator_validate_outbound.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,9 @@ func (k Keeper) ValidateOutboundZEVM(
cctx.InboundParams.Amount,
)
if err != nil {
cctx.SetAbort(fmt.Sprintf("%s : %s", depositErr, err.Error()))
cctx.SetAbort(
"revert failed",
fmt.Sprintf("deposit error: %s, processing error: %s", depositErr.Error(), err.Error()))
return types.CctxStatus_Aborted
}

Expand Down Expand Up @@ -122,7 +124,7 @@ 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, cmd cctx reverted")
cctx.SetAbort("", "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 @@ -137,7 +139,7 @@ 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("", "outbound failed for non-ZETA cctx")
}
}
} else {
Expand Down Expand Up @@ -195,10 +197,10 @@ 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("", revertMsg)
case types.CctxStatus_PendingRevert:
cctx.GetCurrentOutboundParam().TxFinalizationStatus = types.TxFinalizationStatus_Executed
cctx.SetAbort("Outbound failed: revert failed; abort TX")
cctx.SetAbort("aborted while processing failed outbound", "outbound and revert failed")
}
return nil
}
Expand All @@ -225,9 +227,9 @@ func (k Keeper) processSuccessfulOutbound(
oldStatus := cctx.CctxStatus.Status
switch oldStatus {
case types.CctxStatus_PendingRevert:
cctx.SetReverted("Outbound succeeded, revert executed")
cctx.SetReverted("", "revert executed")
case types.CctxStatus_PendingOutbound:
cctx.SetOutboundMined("Outbound succeeded, mined")
cctx.SetOutboundMined("")
default:
return
}
Expand Down Expand Up @@ -256,7 +258,7 @@ func (k Keeper) processFailedZETAOutboundOnZEVM(ctx sdk.Context, cctx *types.Cro
}

// 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 revert")
cctx.SetPendingRevert("", "outbound failed")
data, err := base64.StdEncoding.DecodeString(cctx.RelayedMessage)
if err != nil {
return fmt.Errorf("failed decoding relayed message: %s", err.Error())
Expand Down Expand Up @@ -290,7 +292,7 @@ func (k Keeper) processFailedZETAOutboundOnZEVM(ctx sdk.Context, cctx *types.Cro
return fmt.Errorf("failed ZETARevertAndCallContract: %s", err.Error())
}

cctx.SetReverted("Outbound failed, revert executed")
cctx.SetReverted("", "outbound failed")
if len(ctx.TxBytes()) > 0 {
// add event for tendermint transaction hash format
hash := tmbytes.HexBytes(tmtypes.Tx(ctx.TxBytes()).Hash())
Expand Down Expand Up @@ -336,7 +338,7 @@ func (k Keeper) processFailedOutboundV2(ctx sdk.Context, cctx *types.CrossChainT
}

// update status
cctx.SetPendingRevert("Outbound failed, trying revert")
cctx.SetPendingRevert("", "outbound failed")

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

// tx is reverted
cctx.SetReverted("Outbound failed, revert executed")
cctx.SetReverted("", "outbound failed")

// add event for tendermint transaction hash format
if len(ctx.TxBytes()) > 0 {
Expand All @@ -367,7 +369,7 @@ 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("Outbound failed: revert failed; abort TX")
cctx.SetAbort("aborted while processing failed outbound", "outbound and revert failed")
}
return nil
}
1 change: 0 additions & 1 deletion x/crosschain/keeper/cctx_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,6 @@ func TestCCTXs(t *testing.T) {
require.True(t, found)
require.Equal(t, s, send)
}

})
}
}
Expand Down
20 changes: 10 additions & 10 deletions x/crosschain/keeper/initiate_outbound_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ func TestKeeper_InitiateOutboundZEVMDeposit(t *testing.T) {
require.ErrorContains(t, err, "deposit error")
require.Equal(t, types.CctxStatus_Aborted, cctx.CctxStatus.Status)
require.Equal(t, types.CctxStatus_Aborted, newStatus)
require.Equal(t, "deposit error", cctx.CctxStatus.StatusMessage)
require.Equal(t, "deposit error", cctx.CctxStatus.ErrorMessage)
})

t.Run(
Expand Down Expand Up @@ -111,7 +111,7 @@ func TestKeeper_InitiateOutboundZEVMDeposit(t *testing.T) {
require.Equal(t, types.CctxStatus_Aborted, newStatus)
require.Contains(
t,
cctx.CctxStatus.StatusMessage,
cctx.CctxStatus.ErrorMessage,
"chain not supported",
)
},
Expand Down Expand Up @@ -151,7 +151,7 @@ func TestKeeper_InitiateOutboundZEVMDeposit(t *testing.T) {
require.Equal(t, types.CctxStatus_Aborted, newStatus)
require.Contains(
t,
cctx.CctxStatus.StatusMessage,
cctx.CctxStatus.ErrorMessage,
"GetRevertGasLimit: foreign coin not found for sender chain",
)
})
Expand Down Expand Up @@ -194,7 +194,7 @@ func TestKeeper_InitiateOutboundZEVMDeposit(t *testing.T) {
require.Equal(t, types.CctxStatus_Aborted, newStatus)
require.Contains(
t,
cctx.CctxStatus.StatusMessage,
cctx.CctxStatus.ErrorMessage,
"chain not supported",
)
},
Expand Down Expand Up @@ -239,7 +239,7 @@ func TestKeeper_InitiateOutboundZEVMDeposit(t *testing.T) {
require.Equal(t, types.CctxStatus_Aborted, newStatus)
require.Contains(
t,
cctx.CctxStatus.StatusMessage,
cctx.CctxStatus.ErrorMessage,
"chain not supported",
)
},
Expand Down Expand Up @@ -284,7 +284,7 @@ func TestKeeper_InitiateOutboundZEVMDeposit(t *testing.T) {
require.NoError(t, err)
require.Equal(t, types.CctxStatus_Aborted, cctx.CctxStatus.Status)
require.Equal(t, types.CctxStatus_Aborted, newStatus)
require.Contains(t, cctx.CctxStatus.StatusMessage, "cannot find receiver chain nonce")
require.Contains(t, cctx.CctxStatus.ErrorMessage, "cannot find receiver chain nonce")
})

t.Run("unable to process zevm deposit HandleEVMDeposit revert successfully", func(t *testing.T) {
Expand Down Expand Up @@ -321,7 +321,7 @@ func TestKeeper_InitiateOutboundZEVMDeposit(t *testing.T) {
require.NoError(t, err)
require.Equal(t, types.CctxStatus_PendingRevert, cctx.CctxStatus.Status)
require.Equal(t, types.CctxStatus_PendingRevert, newStatus)
require.Equal(t, errDeposit.Error(), cctx.CctxStatus.StatusMessage)
require.Equal(t, errDeposit.Error(), cctx.CctxStatus.ErrorMessage)
require.Equal(t, updatedNonce, cctx.GetCurrentOutboundParam().TssNonce)
})

Expand Down Expand Up @@ -361,7 +361,7 @@ func TestKeeper_InitiateOutboundZEVMDeposit(t *testing.T) {
require.Equal(t, types.CctxStatus_Aborted, newStatus)
require.Contains(
t,
cctx.CctxStatus.StatusMessage,
cctx.CctxStatus.ErrorMessage,
"cannot revert a revert tx",
)
},
Expand Down Expand Up @@ -427,7 +427,7 @@ func TestKeeper_InitiateOutboundProcessCrosschainMsgPassing(t *testing.T) {
require.ErrorIs(t, err, observertypes.ErrSupportedChains)
require.Equal(t, types.CctxStatus_Aborted, cctx.CctxStatus.Status)
require.Equal(t, types.CctxStatus_Aborted, newStatus)
require.Equal(t, observertypes.ErrSupportedChains.Error(), cctx.CctxStatus.StatusMessage)
require.Equal(t, observertypes.ErrSupportedChains.Error(), cctx.CctxStatus.ErrorMessage)
})

t.Run("unable to process crosschain msg passing UpdateNonce fails", func(t *testing.T) {
Expand Down Expand Up @@ -456,7 +456,7 @@ func TestKeeper_InitiateOutboundProcessCrosschainMsgPassing(t *testing.T) {
require.ErrorContains(t, err, "cannot find receiver chain nonce")
require.Equal(t, types.CctxStatus_Aborted, cctx.CctxStatus.Status)
require.Equal(t, types.CctxStatus_Aborted, newStatus)
require.Contains(t, cctx.CctxStatus.StatusMessage, "cannot find receiver chain nonce")
require.Contains(t, cctx.CctxStatus.ErrorMessage, "cannot find receiver chain nonce")
})
}

Expand Down
3 changes: 1 addition & 2 deletions x/crosschain/keeper/msg_server_migrate_tss_funds.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
tmbytes "github.com/cometbft/cometbft/libs/bytes"
tmtypes "github.com/cometbft/cometbft/types"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/errors"

authoritytypes "github.com/zeta-chain/node/x/authority/types"
"github.com/zeta-chain/node/x/crosschain/types"
Expand All @@ -26,7 +25,7 @@ func (k msgServer) MigrateTssFunds(
// check if authorized
err := k.GetAuthorityKeeper().CheckAuthorization(ctx, msg)
if err != nil {
return nil, errors.Wrap(authoritytypes.ErrUnauthorized, err.Error())
return nil, errorsmod.Wrap(authoritytypes.ErrUnauthorized, err.Error())
}

if k.zetaObserverKeeper.IsInboundEnabled(ctx) {
Expand Down
4 changes: 2 additions & 2 deletions x/crosschain/keeper/msg_server_vote_inbound_tx_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ func TestKeeper_VoteInbound(t *testing.T) {
})
}

func TestStatus_ChangeStatus(t *testing.T) {
func TestStatus_UpdateCctxStatus(t *testing.T) {
tt := []struct {
Name string
Status types.Status
Expand Down Expand Up @@ -302,7 +302,7 @@ func TestStatus_ChangeStatus(t *testing.T) {
for _, test := range tt {
test := test
t.Run(test.Name, func(t *testing.T) {
test.Status.ChangeStatus(test.NonErrStatus, test.Msg)
test.Status.UpdateStatusAndErrorMessages(test.NonErrStatus, test.Msg, "")
if test.IsErr {
require.Equal(t, test.ErrStatus, test.Status.Status)
} else {
Expand Down
2 changes: 1 addition & 1 deletion x/crosschain/keeper/msg_server_vote_outbound_tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ SaveFailedOutbound saves a failed outbound transaction.It does the following thi
*/

func (k Keeper) SaveFailedOutbound(ctx sdk.Context, cctx *types.CrossChainTx, errMessage string, tssPubkey string) {
cctx.SetAbort(errMessage)
cctx.SetAbort("", errMessage)
ctx.Logger().Error(errMessage)
k.SaveOutbound(ctx, cctx, tssPubkey)
}
Expand Down
1 change: 1 addition & 0 deletions x/crosschain/migrations/v5/migrate.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ func SetZetaAccounting(

return nil
}

func GetAbortedAmount(cctx types.CrossChainTx) sdkmath.Uint {
if cctx.OutboundParams != nil && !cctx.GetCurrentOutboundParam().Amount.IsZero() {
return cctx.GetCurrentOutboundParam().Amount
Expand Down
Loading

0 comments on commit 45a9b05

Please sign in to comment.