From 48f9052a468ce027453ccd8cf25a3d28dd83e65e Mon Sep 17 00:00:00 2001 From: Francisco de Borja Aranda Castillejo Date: Wed, 2 Oct 2024 19:04:51 +0200 Subject: [PATCH] add function to update error messages --- changelog.md | 1 + e2e/e2etests/test_eth_deposit_call.go | 4 +- e2e/e2etests/test_solana_deposit_refund.go | 4 +- testutil/sample/crosschain.go | 1 + .../keeper/msg_server_vote_inbound_tx_test.go | 2 +- x/crosschain/types/cctx.go | 11 ++--- x/crosschain/types/cctx_test.go | 4 +- x/crosschain/types/status.go | 41 +++++++++++-------- x/crosschain/types/status_test.go | 6 +-- 9 files changed, 43 insertions(+), 31 deletions(-) diff --git a/changelog.md b/changelog.md index dc83cb839b..4ca51d5db9 100644 --- a/changelog.md +++ b/changelog.md @@ -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 ### Tests diff --git a/e2e/e2etests/test_eth_deposit_call.go b/e2e/e2etests/test_eth_deposit_call.go index 272d4b3e13..46805f9f81 100644 --- a/e2e/e2etests/test_eth_deposit_call.go +++ b/e2e/e2etests/test_eth_deposit_call.go @@ -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.ErrorMessage, utils.ErrHashRevertFoo) + // Check the error carries the revert executed. + require.Contains(r, cctx.CctxStatus.ErrorMessage, "revert executed") } diff --git a/e2e/e2etests/test_solana_deposit_refund.go b/e2e/e2etests/test_solana_deposit_refund.go index e9155c9ddd..3176edee78 100644 --- a/e2e/e2etests/test_solana_deposit_refund.go +++ b/e2e/e2etests/test_solana_deposit_refund.go @@ -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") } diff --git a/testutil/sample/crosschain.go b/testutil/sample/crosschain.go index 8f09271e23..ff17525734 100644 --- a/testutil/sample/crosschain.go +++ b/testutil/sample/crosschain.go @@ -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, } diff --git a/x/crosschain/keeper/msg_server_vote_inbound_tx_test.go b/x/crosschain/keeper/msg_server_vote_inbound_tx_test.go index 66031760ee..a4f144c291 100644 --- a/x/crosschain/keeper/msg_server_vote_inbound_tx_test.go +++ b/x/crosschain/keeper/msg_server_vote_inbound_tx_test.go @@ -302,7 +302,7 @@ func TestStatus_UpdateCctxStatus(t *testing.T) { for _, test := range tt { test := test t.Run(test.Name, func(t *testing.T) { - test.Status.UpdateCctxStatus(test.NonErrStatus, false, test.Msg, "") + test.Status.UpdateCctxMessages(test.NonErrStatus, false, test.Msg, "") if test.IsErr { require.Equal(t, test.ErrStatus, test.Status.Status) } else { diff --git a/x/crosschain/types/cctx.go b/x/crosschain/types/cctx.go index 4e3822dfff..b3eb882cc2 100644 --- a/x/crosschain/types/cctx.go +++ b/x/crosschain/types/cctx.go @@ -172,27 +172,27 @@ func (m *CrossChainTx) AddOutbound( // SetAbort sets the CCTX status to Aborted with the given error message. func (m CrossChainTx) SetAbort(statusMsg, errorMsg string) { - m.CctxStatus.UpdateCctxStatus(CctxStatus_Aborted, true, statusMsg, errorMsg) + m.CctxStatus.UpdateCctxMessages(CctxStatus_Aborted, true, statusMsg, errorMsg) } // SetPendingRevert sets the CCTX status to PendingRevert with the given error message. func (m CrossChainTx) SetPendingRevert(statusMsg, errorMsg string) { - m.CctxStatus.UpdateCctxStatus(CctxStatus_PendingRevert, true, statusMsg, errorMsg) + m.CctxStatus.UpdateCctxMessages(CctxStatus_PendingRevert, true, statusMsg, errorMsg) } // SetPendingOutbound sets the CCTX status to PendingOutbound with the given error message. func (m CrossChainTx) SetPendingOutbound(statusMsg string) { - m.CctxStatus.UpdateCctxStatus(CctxStatus_PendingOutbound, false, statusMsg, "") + m.CctxStatus.UpdateCctxMessages(CctxStatus_PendingOutbound, false, statusMsg, "") } // SetOutboundMined sets the CCTX status to OutboundMined with the given error message. func (m CrossChainTx) SetOutboundMined(statusMsg string) { - m.CctxStatus.UpdateCctxStatus(CctxStatus_OutboundMined, false, statusMsg, "") + m.CctxStatus.UpdateCctxMessages(CctxStatus_OutboundMined, false, statusMsg, "") } // SetReverted sets the CCTX status to Reverted with the given error message. func (m CrossChainTx) SetReverted(statusMsg, errorMsg string) { - m.CctxStatus.UpdateCctxStatus(CctxStatus_Reverted, true, statusMsg, errorMsg) + m.CctxStatus.UpdateCctxMessages(CctxStatus_Reverted, true, statusMsg, errorMsg) } func (m CrossChainTx) GetCCTXIndexBytes() ([32]byte, error) { @@ -259,6 +259,7 @@ func NewCCTX(ctx sdk.Context, msg MsgVoteInbound, tssPubkey string) (CrossChainT status := &Status{ Status: CctxStatus_PendingInbound, StatusMessage: "", + ErrorMessage: "", CreatedTimestamp: ctx.BlockHeader().Time.Unix(), LastUpdateTimestamp: ctx.BlockHeader().Time.Unix(), IsAbortRefunded: false, diff --git a/x/crosschain/types/cctx_test.go b/x/crosschain/types/cctx_test.go index f49768f8a0..418b5cf475 100644 --- a/x/crosschain/types/cctx_test.go +++ b/x/crosschain/types/cctx_test.go @@ -1,6 +1,7 @@ package types_test import ( + "fmt" "math/rand" "testing" @@ -151,7 +152,8 @@ func Test_SetRevertOutboundValues(t *testing.T) { func TestCrossChainTx_SetAbort(t *testing.T) { cctx := sample.CrossChainTx(t, "test") cctx.CctxStatus.Status = types.CctxStatus_PendingOutbound - cctx.SetAbort("test", "test") + cctx.SetAbort("test", fmt.Sprintf("deposit: %s, error: %s", "foo", "bar")) + fmt.Printf("DEBUG: %+v\n", cctx.CctxStatus) require.Equal(t, types.CctxStatus_Aborted, cctx.CctxStatus.Status) require.Contains(t, cctx.CctxStatus.StatusMessage, "test") require.Contains(t, cctx.CctxStatus.ErrorMessage, "test") diff --git a/x/crosschain/types/status.go b/x/crosschain/types/status.go index 0e54c847d0..2a5a53e501 100644 --- a/x/crosschain/types/status.go +++ b/x/crosschain/types/status.go @@ -9,21 +9,14 @@ func (m *Status) AbortRefunded() { m.StatusMessage = "CCTX aborted and Refunded" } -// UpdateCctxStatus transitions the Status. -// In case of an error, ErrorMessage is updated. -// In case of no error, StatusMessage is updated. -func (m *Status) UpdateCctxStatus(newStatus CctxStatus, isError bool, statusMsg, errorMsg string) { - m.ChangeStatus(newStatus, statusMsg) - - if isError && errorMsg != "" { - m.ErrorMessage = errorMsg - } else if isError && errorMsg == "" { - m.ErrorMessage = "unknown error" - } +// UpdateCctxMessages transitions the Status and Error messages. +func (m *Status) UpdateCctxMessages(newStatus CctxStatus, isError bool, statusMsg, errorMsg string) { + m.UpdateStatusMessage(newStatus, statusMsg) + m.UpdateErrorMessage(isError, errorMsg) } -// ChangeStatus changes the status of the cross chain transaction. -func (m *Status) ChangeStatus(newStatus CctxStatus, statusMsg string) { +// UpdateStatusMessage updates cctx.status.status_message. +func (m *Status) UpdateStatusMessage(newStatus CctxStatus, statusMsg string) { if !m.ValidateTransition(newStatus) { m.StatusMessage = fmt.Sprintf( "Failed to transition status from %s to %s", @@ -35,15 +28,29 @@ func (m *Status) ChangeStatus(newStatus CctxStatus, statusMsg string) { return } - if statusMsg == "" { - m.StatusMessage = fmt.Sprintf("Status changed from %s to %s", m.Status.String(), newStatus.String()) - } else { - m.StatusMessage = statusMsg + m.StatusMessage = fmt.Sprintf("Status changed from %s to %s", m.Status.String(), newStatus.String()) + + if statusMsg != "" { + m.StatusMessage += fmt.Sprintf(" - %s", statusMsg) } m.Status = newStatus } +// UpdateErrorMessage updates cctx.status.error_message. +func (m *Status) UpdateErrorMessage(isError bool, errorMsg string) { + if !isError { + return + } + + errMsg := errorMsg + if errMsg == "" { + errMsg = "unknown error" + } + + m.ErrorMessage = errMsg +} + func (m *Status) ValidateTransition(newStatus CctxStatus) bool { stateTransitionMap := stateTransitionMap() oldStatus := m.Status diff --git a/x/crosschain/types/status_test.go b/x/crosschain/types/status_test.go index 97f2bcef4c..c302b074d9 100644 --- a/x/crosschain/types/status_test.go +++ b/x/crosschain/types/status_test.go @@ -140,7 +140,7 @@ func TestStatus_ChangeStatus(t *testing.T) { t.Run("should change status and msg if transition is valid", func(t *testing.T) { s := types.Status{Status: types.CctxStatus_PendingInbound} - s.ChangeStatus(types.CctxStatus_PendingOutbound, "msg") + s.UpdateStatusMessage(types.CctxStatus_PendingOutbound, "msg") assert.Equal(t, s.Status, types.CctxStatus_PendingOutbound) assert.Equal(t, s.StatusMessage, "msg") }) @@ -148,7 +148,7 @@ func TestStatus_ChangeStatus(t *testing.T) { t.Run("should change status if transition is valid", func(t *testing.T) { s := types.Status{Status: types.CctxStatus_PendingInbound} - s.ChangeStatus(types.CctxStatus_PendingOutbound, "") + s.UpdateStatusMessage(types.CctxStatus_PendingOutbound, "") fmt.Printf("%+v\n", s) assert.Equal(t, s.Status, types.CctxStatus_PendingOutbound) assert.Equal(t, s.StatusMessage, fmt.Sprintf( @@ -161,7 +161,7 @@ func TestStatus_ChangeStatus(t *testing.T) { t.Run("should change status to aborted and msg if transition is invalid", func(t *testing.T) { s := types.Status{Status: types.CctxStatus_PendingOutbound} - s.ChangeStatus(types.CctxStatus_PendingInbound, "msg") + s.UpdateStatusMessage(types.CctxStatus_PendingInbound, "msg") assert.Equal(t, s.Status, types.CctxStatus_Aborted) assert.Equal( t,