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
2 changes: 2 additions & 0 deletions docs/openapi/openapi.swagger.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -58446,6 +58446,8 @@ definitions:
$ref: '#/definitions/crosschainCctxStatus'
status_message:
type: string
error_message:
type: string
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")
}
3 changes: 2 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 @@ -95,6 +95,7 @@ message OutboundParams {
message Status {
CctxStatus status = 1;
string status_message = 2;
string error_message = 6;
lumtis marked this conversation as resolved.
Show resolved Hide resolved
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
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,11 @@ export declare class Status extends Message<Status> {
*/
statusMessage: string;

/**
* @generated from field: string error_message = 6;
*/
errorMessage: string;

/**
* @generated from field: int64 lastUpdate_timestamp = 3;
*/
Expand Down
17 changes: 17 additions & 0 deletions x/crosschain/keeper/cctx.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,23 @@ func (k Keeper) GetCrossChainTx(ctx sdk.Context, index string) (val types.CrossC
return val, true
}

// GetCrossChainTxError returns the error message for a given cctx index.
func (k Keeper) GetCrossChainTxError(ctx sdk.Context, index string) (errMsg string, found bool) {
lumtis marked this conversation as resolved.
Show resolved Hide resolved
var cctx types.CrossChainTx

p := types.KeyPrefix(fmt.Sprintf("%s", types.CCTXKey))
store := prefix.NewStore(ctx.KVStore(k.storeKey), p)

b := store.Get(types.KeyPrefix(index))
if b == nil {
return "", false
}

k.cdc.MustUnmarshal(b, &cctx)

return cctx.CctxStatus.ErrorMessage, true
}
fbac marked this conversation as resolved.
Show resolved Hide resolved

// GetAllCrossChainTx returns all cctxs
func (k Keeper) GetAllCrossChainTx(ctx sdk.Context) (list []types.CrossChainTx) {
p := types.KeyPrefix(fmt.Sprintf("%s", types.CCTXKey))
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())
lumtis marked this conversation as resolved.
Show resolved Hide resolved
return types.CctxStatus_Aborted, err
}
commit()
Expand Down
2 changes: 1 addition & 1 deletion x/crosschain/keeper/cctx_gateway_zevm.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func (c CCTXGatewayZEVM) InitiateOutbound(

if err != nil && !isContractReverted {
// exceptional case; internal error; should abort CCTX
config.CCTX.SetAbort(err.Error())
config.CCTX.SetAbort("Aborted while trying to handle EVM deposit", err.Error())
fbac marked this conversation as resolved.
Show resolved Hide resolved
lumtis marked this conversation as resolved.
Show resolved Hide resolved
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 an error was raised while trying to revert cctx",
lumtis marked this conversation as resolved.
Show resolved Hide resolved
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")
} 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("Reverted as part of a successful outbound", "Revert executed")
case types.CctxStatus_PendingOutbound:
cctx.SetOutboundMined("Outbound succeeded, mined")
cctx.SetOutboundMined("")
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
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")
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

// 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
}
7 changes: 6 additions & 1 deletion x/crosschain/keeper/cctx_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,8 +169,13 @@ func TestCCTXs(t *testing.T) {
send, found := keeper.GetCrossChainTx(ctx, s.Index)
require.True(t, found)
require.Equal(t, s, send)
err, found := keeper.GetCrossChainTxError(ctx, s.Index)
require.True(t, found)
require.Equal(t, "", err)
}

err, found := keeper.GetCrossChainTxError(ctx, "does-not-exist")
require.False(t, found)
require.Equal(t, "", err)
})
}
}
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