fix: update pending nonces when aborting a cctx through MsgAbortStuckCCTX

changelog.md

@@ -14,6 +14,7 @@
 
 * [3206](https://github.com/zeta-chain/node/pull/3206) - skip Solana unsupported transaction version to not block inbound observation
 * [3184](https://github.com/zeta-chain/node/pull/3184) - zetaclient should not retry if inbound vote message validation fails
+* [3230](https://github.com/zeta-chain/node/pull/3230) - update pending nonces when aborting a cctx through MsgAbortStuckCCTX
 
 ## v23.0.0
 

e2e/utils/zetacore.go

@@ -113,7 +113,7 @@ func WaitCctxsMinedByInboundHash(
 		allFound := true
 		for j, cctx := range res.CrossChainTxs {
 			cctx := cctx
-			if !IsTerminalStatus(cctx.CctxStatus.Status) {
+			if !cctx.CctxStatus.Status.IsTerminalStatus() {
 				// prevent spamming logs
 				if i%20 == 0 {
 					logger.Info(
@@ -170,7 +170,7 @@ func WaitCCTXMinedByIndex(
 		}
 
 		cctx := res.CrossChainTx
-		if !IsTerminalStatus(cctx.CctxStatus.Status) {
+		if !cctx.CctxStatus.Status.IsTerminalStatus() {
 			// prevent spamming logs
 			if i%20 == 0 {
 				logger.Info(
@@ -299,12 +299,6 @@ func WaitCctxByInboundHash(
 	}
 }
 
-func IsTerminalStatus(status crosschaintypes.CctxStatus) bool {
-	return status == crosschaintypes.CctxStatus_OutboundMined ||
-		status == crosschaintypes.CctxStatus_Aborted ||
-		status == crosschaintypes.CctxStatus_Reverted
-}
-
 // WaitForBlockHeight waits until the block height reaches the given height
 func WaitForBlockHeight(
 	ctx context.Context,

x/crosschain/keeper/cctx.go

@@ -12,16 +12,29 @@ import (
 )
 
 // SetCctxAndNonceToCctxAndInboundHashToCctx does the following things in one function:
-// 1. set the cctx in the store
-// 2. set the mapping inboundHash -> cctxIndex , one inboundHash can be connected to multiple cctxindex
-// 3. set the mapping nonce => cctx
+
+// 1. Set the Nonce to Cctx mapping
+// A new mapping between a nonce and a cctx index should be created only when we add a new outbound to an existing cctx.
+// When adding a new outbound , the only two conditions are
+// - The cctx is in CctxStatus_PendingOutbound , which means the first outbound has been added, and we need to set the nonce for that
+// - The cctx is in CctxStatus_PendingRevert , which means the second outbound has been added, and we need to set the nonce for that
+
+// 2. Set the cctx in the store
+
+// 3. set the mapping inboundHash -> cctxIndex
+// A new value is added to the mapping when a single inbound hash is connected to multiple cctx indexes
+
 // 4. update the zeta accounting
+// Zeta-accounting is updated aborted cctxs of cointtype zeta.When a cctx is aborted it means that `GetAbortedAmount`
+//of zeta is locked and cannot be used.
+
 func (k Keeper) SetCctxAndNonceToCctxAndInboundHashToCctx(
 	ctx sdk.Context,
 	cctx types.CrossChainTx,
 	tssPubkey string,
 ) {
 	// set mapping nonce => cctxIndex
+
 	if cctx.CctxStatus.Status == types.CctxStatus_PendingOutbound ||
 		cctx.CctxStatus.Status == types.CctxStatus_PendingRevert {
 		k.GetObserverKeeper().SetNonceToCctx(ctx, observerTypes.NonceToCctx{
@@ -34,6 +47,7 @@ func (k Keeper) SetCctxAndNonceToCctxAndInboundHashToCctx(
 	}
 
 	k.SetCrossChainTx(ctx, cctx)
+
 	// set mapping inboundHash -> cctxIndex
 	in, _ := k.GetInboundHashToCctx(ctx, cctx.InboundParams.ObservedHash)
 	in.InboundHash = cctx.InboundParams.ObservedHash

x/crosschain/keeper/msg_server_abort_stuck_cctx.go

@@ -36,19 +36,15 @@ func (k msgServer) AbortStuckCCTX(
 	}
 
 	// check if the cctx is pending
-	isPending := cctx.CctxStatus.Status == types.CctxStatus_PendingOutbound ||
-		cctx.CctxStatus.Status == types.CctxStatus_PendingInbound ||
-		cctx.CctxStatus.Status == types.CctxStatus_PendingRevert
-	if !isPending {
+	if !cctx.CctxStatus.Status.IsPendingStatus() {
 		return nil, types.ErrStatusNotPending
 	}
 
-	cctx.CctxStatus = &types.Status{
-		Status:        types.CctxStatus_Aborted,
-		StatusMessage: AbortMessage,
-	}
+	// update the status
+	cctx.CctxStatus.UpdateStatusAndErrorMessages(types.CctxStatus_Aborted, AbortMessage, "")
 
-	k.SetCrossChainTx(ctx, cctx)
+	// Save out outbound, we do not need to provide the tss-pubkey as NonceToCctx is not updated
+	k.SaveOutbound(ctx, &cctx, "")
 
 	return &types.MsgAbortStuckCCTXResponse{}, nil
 }

x/crosschain/keeper/msg_server_abort_stuck_cctx_test.go

@@ -4,6 +4,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/require"
+	observertypes "github.com/zeta-chain/node/x/observer/types"
 
 	keepertest "github.com/zeta-chain/node/testutil/keeper"
 	"github.com/zeta-chain/node/testutil/sample"
@@ -14,6 +15,7 @@ import (
 
 func TestMsgServer_AbortStuckCCTX(t *testing.T) {
 	t.Run("can abort a cctx in pending inbound", func(t *testing.T) {
+		// Arrange
 		k, ctx, _, _ := keepertest.CrosschainKeeperWithMocks(t, keepertest.CrosschainMockOptions{
 			UseAuthorityMock: true,
 		})
@@ -28,24 +30,39 @@ func TestMsgServer_AbortStuckCCTX(t *testing.T) {
 			Status:        crosschaintypes.CctxStatus_PendingInbound,
 			StatusMessage: "pending inbound",
 		}
+		cctx.GetCurrentOutboundParam().TssNonce = 1
+
 		k.SetCrossChainTx(ctx, *cctx)
+		k.GetObserverKeeper().SetPendingNonces(ctx, observertypes.PendingNonces{
+			NonceLow:  0,
+			NonceHigh: 10,
+			ChainId:   cctx.GetCurrentOutboundParam().ReceiverChainId,
+			Tss:       cctx.GetCurrentOutboundParam().TssPubkey,
+		})
 
 		// abort the cctx
 		msg := crosschaintypes.MsgAbortStuckCCTX{
 			Creator:   admin,
 			CctxIndex: sample.GetCctxIndexFromString("cctx_index"),
 		}
 		keepertest.MockCheckAuthorization(&authorityMock.Mock, &msg, nil)
+		// Act
 		_, err := msgServer.AbortStuckCCTX(ctx, &msg)
 
+		// Assert
 		require.NoError(t, err)
 		cctxFound, found := k.GetCrossChainTx(ctx, sample.GetCctxIndexFromString("cctx_index"))
 		require.True(t, found)
 		require.Equal(t, crosschaintypes.CctxStatus_Aborted, cctxFound.CctxStatus.Status)
-		require.Equal(t, crosschainkeeper.AbortMessage, cctxFound.CctxStatus.StatusMessage)
+		require.Contains(t, cctxFound.CctxStatus.StatusMessage, crosschainkeeper.AbortMessage)
+		pendingNonces, found := k.GetObserverKeeper().
+			GetPendingNonces(ctx, cctx.GetCurrentOutboundParam().TssPubkey, cctx.GetCurrentOutboundParam().ReceiverChainId)
+		require.True(t, found)
+		require.Equal(t, pendingNonces.NonceLow, int64(cctx.GetCurrentOutboundParam().TssNonce+1))
 	})
 
 	t.Run("can abort a cctx in pending outbound", func(t *testing.T) {
+		// Arrange
 		k, ctx, _, _ := keepertest.CrosschainKeeperWithMocks(t, keepertest.CrosschainMockOptions{
 			UseAuthorityMock: true,
 		})
@@ -59,26 +76,42 @@ func TestMsgServer_AbortStuckCCTX(t *testing.T) {
 			Status:        crosschaintypes.CctxStatus_PendingOutbound,
 			StatusMessage: "pending outbound",
 		}
+		cctx.GetCurrentOutboundParam().TssNonce = 1
+
 		k.SetCrossChainTx(ctx, *cctx)
+		k.GetObserverKeeper().SetPendingNonces(ctx, observertypes.PendingNonces{
+			NonceLow:  0,
+			NonceHigh: 10,
+			ChainId:   cctx.GetCurrentOutboundParam().ReceiverChainId,
+			Tss:       cctx.GetCurrentOutboundParam().TssPubkey,
+		})
 
-		// abort the cctx
 		msg := crosschaintypes.MsgAbortStuckCCTX{
 			Creator:   admin,
 			CctxIndex: sample.GetCctxIndexFromString("cctx_index"),
 		}
 		keepertest.MockCheckAuthorization(&authorityMock.Mock, &msg, nil)
+
+		// Act
 		_, err := msgServer.AbortStuckCCTX(ctx, &msg)
 
+		// Assert
 		require.NoError(t, err)
 		cctxFound, found := k.GetCrossChainTx(ctx, sample.GetCctxIndexFromString("cctx_index"))
 		require.True(t, found)
 		require.Equal(t, crosschaintypes.CctxStatus_Aborted, cctxFound.CctxStatus.Status)
-		require.Equal(t, crosschainkeeper.AbortMessage, cctxFound.CctxStatus.StatusMessage)
+		require.Contains(t, cctxFound.CctxStatus.StatusMessage, crosschainkeeper.AbortMessage)
 		// ensure the last update timestamp is updated
 		require.Equal(t, cctxFound.CctxStatus.LastUpdateTimestamp, ctx.BlockTime().Unix())
+		pendingNonces, found := k.GetObserverKeeper().
+			GetPendingNonces(ctx, cctx.GetCurrentOutboundParam().TssPubkey, cctx.GetCurrentOutboundParam().ReceiverChainId)
+		require.True(t, found)
+		require.Equal(t, pendingNonces.NonceLow, int64(cctx.GetCurrentOutboundParam().TssNonce+1))
+
 	})
 
 	t.Run("can abort a cctx in pending revert", func(t *testing.T) {
+		// Arrange
 		k, ctx, _, _ := keepertest.CrosschainKeeperWithMocks(t, keepertest.CrosschainMockOptions{
 			UseAuthorityMock: true,
 		})
@@ -93,21 +126,35 @@ func TestMsgServer_AbortStuckCCTX(t *testing.T) {
 			Status:        crosschaintypes.CctxStatus_PendingRevert,
 			StatusMessage: "pending revert",
 		}
+		cctx.GetCurrentOutboundParam().TssNonce = 1
+
 		k.SetCrossChainTx(ctx, *cctx)
+		k.GetObserverKeeper().SetPendingNonces(ctx, observertypes.PendingNonces{
+			NonceLow:  0,
+			NonceHigh: 10,
+			ChainId:   cctx.GetCurrentOutboundParam().ReceiverChainId,
+			Tss:       cctx.GetCurrentOutboundParam().TssPubkey,
+		})
 
 		// abort the cctx
 		msg := crosschaintypes.MsgAbortStuckCCTX{
 			Creator:   admin,
 			CctxIndex: sample.GetCctxIndexFromString("cctx_index"),
 		}
 		keepertest.MockCheckAuthorization(&authorityMock.Mock, &msg, nil)
+		// Act
 		_, err := msgServer.AbortStuckCCTX(ctx, &msg)
 
+		// Assert
 		require.NoError(t, err)
 		cctxFound, found := k.GetCrossChainTx(ctx, sample.GetCctxIndexFromString("cctx_index"))
 		require.True(t, found)
 		require.Equal(t, crosschaintypes.CctxStatus_Aborted, cctxFound.CctxStatus.Status)
-		require.Equal(t, crosschainkeeper.AbortMessage, cctxFound.CctxStatus.StatusMessage)
+		require.Contains(t, cctxFound.CctxStatus.StatusMessage, crosschainkeeper.AbortMessage)
+		pendingNonces, found := k.GetObserverKeeper().
+			GetPendingNonces(ctx, cctx.GetCurrentOutboundParam().TssPubkey, cctx.GetCurrentOutboundParam().ReceiverChainId)
+		require.True(t, found)
+		require.Equal(t, pendingNonces.NonceLow, int64(cctx.GetCurrentOutboundParam().TssNonce+1))
 	})
 
 	t.Run("cannot abort a cctx in pending outbound if not admin", func(t *testing.T) {

x/crosschain/keeper/outbound_tracker_test.go

@@ -42,19 +42,29 @@ func TestOutboundTrackerGet(t *testing.T) {
 	}
 }
 func TestOutboundTrackerRemove(t *testing.T) {
-	k, ctx, _, _ := keepertest.CrosschainKeeper(t)
-	items := createNOutboundTracker(k, ctx, 10)
-	for _, item := range items {
-		k.RemoveOutboundTrackerFromStore(ctx,
-			item.ChainId,
-			item.Nonce,
-		)
-		_, found := k.GetOutboundTracker(ctx,
-			item.ChainId,
-			item.Nonce,
-		)
-		require.False(t, found)
-	}
+	t.Run("Remove tracker if it exists", func(t *testing.T) {
+		keeper, ctx, _, _ := keepertest.CrosschainKeeper(t)
+		items := createNOutboundTracker(keeper, ctx, 10)
+		for _, item := range items {
+			keeper.RemoveOutboundTrackerFromStore(ctx,
+				item.ChainId,
+				item.Nonce,
+			)
+			_, found := keeper.GetOutboundTracker(ctx,
+				item.ChainId,
+				item.Nonce,
+			)
+			require.False(t, found)
+		}
+	})
+
+	t.Run("Do nothing if tracker doesn't exist", func(t *testing.T) {
+		keeper, ctx, _, _ := keepertest.CrosschainKeeper(t)
+		require.NotPanics(t, func() {
+			keeper.RemoveOutboundTrackerFromStore(ctx, 1, 1)
+		})
+	})
+
 }
 
 func TestOutboundTrackerGetAll(t *testing.T) {

x/crosschain/types/status.go

@@ -86,3 +86,11 @@ func stateTransitionMap() map[CctxStatus][]CctxStatus {
 	}
 	return stateTransitionMap
 }
+
+func (c CctxStatus) IsTerminalStatus() bool {
+	return c == CctxStatus_Aborted || c == CctxStatus_Reverted || c == CctxStatus_OutboundMined
+}
+
+func (c CctxStatus) IsPendingStatus() bool {
+	return c == CctxStatus_PendingInbound || c == CctxStatus_PendingOutbound || c == CctxStatus_PendingRevert
+}

x/crosschain/types/status_test.go

@@ -174,3 +174,45 @@ func TestStatus_ChangeStatus(t *testing.T) {
 		)
 	})
 }
+
+func TestCctxStatus_IsTerminalStatus(t *testing.T) {
+	tests := []struct {
+		name     string
+		status   types.CctxStatus
+		expected bool
+	}{
+		{"PendingInbound", types.CctxStatus_PendingInbound, false},
+		{"PendingOutbound", types.CctxStatus_PendingOutbound, false},
+		{"OutboundMined", types.CctxStatus_OutboundMined, true},
+		{"Reverted", types.CctxStatus_Reverted, true},
+		{"Aborted", types.CctxStatus_Aborted, true},
+		{"PendingRevert", types.CctxStatus_PendingRevert, false},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			assert.Equal(t, tc.expected, tc.status.IsTerminalStatus())
+		})
+	}
+}
+
+func TestCctxStatus_IsPendingStatus(t *testing.T) {
+	tests := []struct {
+		name     string
+		status   types.CctxStatus
+		expected bool
+	}{
+		{"PendingInbound", types.CctxStatus_PendingInbound, true},
+		{"PendingOutbound", types.CctxStatus_PendingOutbound, true},
+		{"OutboundMined", types.CctxStatus_OutboundMined, false},
+		{"Reverted", types.CctxStatus_Reverted, false},
+		{"Aborted", types.CctxStatus_Aborted, false},
+		{"PendingRevert", types.CctxStatus_PendingRevert, true},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			assert.Equal(t, tc.expected, tc.status.IsPendingStatus())
+		})
+	}
+}