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: add error field in cctx #2952

Merged
merged 13 commits into from
Oct 8, 2024
1 change: 1 addition & 0 deletions changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,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
lumtis marked this conversation as resolved.
Show resolved Hide resolved

### 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("aborted due to an internal error", err.Error())
fbac marked this conversation as resolved.
Show resolved Hide resolved
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(
"aborted because of an 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 @@
cctx.InboundParams.Amount,
)
if err != nil {
cctx.SetAbort(fmt.Sprintf("%s : %s", depositErr, err.Error()))
cctx.SetAbort(
"aborted because revert failed",
fmt.Sprintf("deposit error: %s, processing error: %s", depositErr, err.Error()))
fbac marked this conversation as resolved.
Show resolved Hide resolved
return types.CctxStatus_Aborted
}

Expand Down Expand Up @@ -122,7 +124,7 @@
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")
fbac marked this conversation as resolved.
Show resolved Hide resolved
} 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 @@
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 @@
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 @@
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("")
fbac marked this conversation as resolved.
Show resolved Hide resolved
default:
return
}
Expand Down Expand Up @@ -256,7 +258,7 @@
}

// 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")
fbac marked this conversation as resolved.
Show resolved Hide resolved
fbac marked this conversation as resolved.
Show resolved Hide resolved
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 @@
return fmt.Errorf("failed ZETARevertAndCallContract: %s", err.Error())
}

cctx.SetReverted("Outbound failed, revert executed")
cctx.SetReverted("", "outbound failed")
fbac marked this conversation as resolved.
Show resolved Hide resolved
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 @@
}

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

Check warning on line 341 in x/crosschain/keeper/cctx_orchestrator_validate_outbound.go

View check run for this annotation

Codecov / codecov/patch

x/crosschain/keeper/cctx_orchestrator_validate_outbound.go#L341

Added line #L341 was not covered by tests
fbac marked this conversation as resolved.
Show resolved Hide resolved

// process the revert on ZEVM
if err := k.fungibleKeeper.ProcessV2RevertDeposit(
Expand All @@ -354,7 +356,7 @@
}

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

Check warning on line 359 in x/crosschain/keeper/cctx_orchestrator_validate_outbound.go

View check run for this annotation

Codecov / codecov/patch

x/crosschain/keeper/cctx_orchestrator_validate_outbound.go#L359

Added line #L359 was not covered by tests

// add event for tendermint transaction hash format
if len(ctx.TxBytes()) > 0 {
Expand All @@ -367,7 +369,7 @@
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")

Check warning on line 372 in x/crosschain/keeper/cctx_orchestrator_validate_outbound.go

View check run for this annotation

Codecov / codecov/patch

x/crosschain/keeper/cctx_orchestrator_validate_outbound.go#L372

Added line #L372 was not covered by tests
}
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, false, 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
Loading