diff --git a/changelog.md b/changelog.md index d522fc869d..680f3695ce 100644 --- a/changelog.md +++ b/changelog.md @@ -18,6 +18,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 * [3225](https://github.com/zeta-chain/node/pull/3225) - use separate database file names for btc signet and testnet4 * [3253](https://github.com/zeta-chain/node/pull/3253) - fix solana inbound version 0 queries and move tss keysign prior to relayer key checking diff --git a/e2e/utils/zetacore.go b/e2e/utils/zetacore.go index bf01d79ec8..681e3bca7d 100644 --- a/e2e/utils/zetacore.go +++ b/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.IsTerminal() { // 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.IsTerminal() { // 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, diff --git a/x/crosschain/genesis.go b/x/crosschain/genesis.go index 467f532a53..8ea365e059 100644 --- a/x/crosschain/genesis.go +++ b/x/crosschain/genesis.go @@ -1,6 +1,7 @@ package crosschain import ( + sdkmath "cosmossdk.io/math" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/zeta-chain/node/x/crosschain/keeper" @@ -10,7 +11,9 @@ import ( // InitGenesis initializes the crosschain module's state from a provided genesis // state. func InitGenesis(ctx sdk.Context, k keeper.Keeper, genState types.GenesisState) { - k.SetZetaAccounting(ctx, genState.ZetaAccounting) + // Always set the zeta accounting to zero at genesis. + // ZetaAccounting value is build by iterating through all the cctxs and adding the amount to the zeta accounting. + k.SetZetaAccounting(ctx, types.ZetaAccounting{AbortedZetaAmount: sdkmath.ZeroUint()}) // Set all the outbound tracker for _, elem := range genState.OutboundTrackerList { k.SetOutboundTracker(ctx, elem) @@ -47,7 +50,7 @@ func InitGenesis(ctx sdk.Context, k keeper.Keeper, genState types.GenesisState) if found { for _, elem := range genState.CrossChainTxs { if elem != nil { - k.SetCctxAndNonceToCctxAndInboundHashToCctx(ctx, *elem, tss.TssPubkey) + k.SaveCCTXUpdate(ctx, *elem, tss.TssPubkey) } } } diff --git a/x/crosschain/keeper/cctx.go b/x/crosschain/keeper/cctx.go index f2234362f0..19f71a78af 100644 --- a/x/crosschain/keeper/cctx.go +++ b/x/crosschain/keeper/cctx.go @@ -11,30 +11,41 @@ import ( observerTypes "github.com/zeta-chain/node/x/observer/types" ) -// 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 +// SaveCCTXUpdate does the following things in one function: + +// 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. Update the mapping inboundHash -> cctxIndex +// A new value is added to the mapping when a single inbound hash is connected to multiple cctx indexes +// If the inbound hash to cctx mapping does not exist, a new mapping is created and the cctx index is added to the list of cctx indexes + // 4. update the zeta accounting -func (k Keeper) SetCctxAndNonceToCctxAndInboundHashToCctx( +// 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) SaveCCTXUpdate( 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{ - ChainId: cctx.GetCurrentOutboundParam().ReceiverChainId, - // #nosec G115 always in range - Nonce: int64(cctx.GetCurrentOutboundParam().TssNonce), - CctxIndex: cctx.Index, - Tss: tssPubkey, - }) - } - + k.setNonceToCCTX(ctx, cctx, tssPubkey) k.SetCrossChainTx(ctx, cctx) - // set mapping inboundHash -> cctxIndex + k.updateInboundHashToCCTX(ctx, cctx) + k.updateZetaAccounting(ctx, cctx) +} + +// updateInboundHashToCCTX updates the mapping between an inbound hash and a cctx index. +// A new index is added to the list of cctx indexes if it is not already present +func (k Keeper) updateInboundHashToCCTX( + ctx sdk.Context, + cctx types.CrossChainTx, +) { in, _ := k.GetInboundHashToCctx(ctx, cctx.InboundParams.ObservedHash) in.InboundHash = cctx.InboundParams.ObservedHash found := false @@ -48,15 +59,42 @@ func (k Keeper) SetCctxAndNonceToCctxAndInboundHashToCctx( in.CctxIndex = append(in.CctxIndex, cctx.Index) } k.SetInboundHashToCctx(ctx, in) +} - if cctx.CctxStatus.Status == types.CctxStatus_Aborted && cctx.InboundParams.CoinType == coin.CoinType_Zeta { +// updateZetaAccounting updates the zeta accounting with the amount of zeta that was locked in an aborted cctx +func (k Keeper) updateZetaAccounting( + ctx sdk.Context, + cctx types.CrossChainTx, +) { + if cctx.CctxStatus.Status == types.CctxStatus_Aborted && + cctx.InboundParams.CoinType == coin.CoinType_Zeta && + cctx.CctxStatus.IsAbortRefunded == false { k.AddZetaAbortedAmount(ctx, GetAbortedAmount(cctx)) } } +// setNonceToCCTX updates the mapping between a nonce and a cctx index if the cctx is in a PendingOutbound or PendingRevert state +func (k Keeper) setNonceToCCTX( + 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{ + ChainId: cctx.GetCurrentOutboundParam().ReceiverChainId, + // #nosec G115 always in range + Nonce: int64(cctx.GetCurrentOutboundParam().TssNonce), + CctxIndex: cctx.Index, + Tss: tssPubkey, + }) + } +} + // SetCrossChainTx set a specific cctx in the store from its index func (k Keeper) SetCrossChainTx(ctx sdk.Context, cctx types.CrossChainTx) { - // only set the update timestamp if the block height is >0 to allow + // only set the updated timestamp if the block height is >0 to allow // for a genesis import if cctx.CctxStatus != nil && ctx.BlockHeight() > 0 { cctx.CctxStatus.LastUpdateTimestamp = ctx.BlockHeader().Time.Unix() diff --git a/x/crosschain/keeper/cctx_orchestrator_validate_inbound.go b/x/crosschain/keeper/cctx_orchestrator_validate_inbound.go index 826e21eca5..d4764d9637 100644 --- a/x/crosschain/keeper/cctx_orchestrator_validate_inbound.go +++ b/x/crosschain/keeper/cctx_orchestrator_validate_inbound.go @@ -51,7 +51,7 @@ func (k Keeper) ValidateInbound( if ok { cctx.InboundParams.ObservedHash = inCctxIndex } - k.SetCctxAndNonceToCctxAndInboundHashToCctx(ctx, cctx, tss.TssPubkey) + k.SaveCCTXUpdate(ctx, cctx, tss.TssPubkey) return &cctx, nil } diff --git a/x/crosschain/keeper/cctx_orchestrator_validate_inbound_test.go b/x/crosschain/keeper/cctx_orchestrator_validate_inbound_test.go index 492ef14db0..3227ef4e5f 100644 --- a/x/crosschain/keeper/cctx_orchestrator_validate_inbound_test.go +++ b/x/crosschain/keeper/cctx_orchestrator_validate_inbound_test.go @@ -58,7 +58,7 @@ func TestKeeper_ValidateInbound(t *testing.T) { Return(observerTypes.PendingNonces{NonceHigh: 1}, true) observerMock.On("SetChainNonces", mock.Anything, mock.Anything).Return(nil) observerMock.On("SetPendingNonces", mock.Anything, mock.Anything).Return(nil) - // setup Mocks for SetCctxAndNonceToCctxAndInboundHashToCctx + // setup Mocks for SaveCCTXUpdate observerMock.On("SetNonceToCctx", mock.Anything, mock.Anything).Return(nil) k.SetGasPrice(ctx, types.GasPrice{ diff --git a/x/crosschain/keeper/cctx_test.go b/x/crosschain/keeper/cctx_test.go index d1e3f6fafc..e532db9310 100644 --- a/x/crosschain/keeper/cctx_test.go +++ b/x/crosschain/keeper/cctx_test.go @@ -44,7 +44,7 @@ func createNCctxWithStatus( items[i].OutboundParams = []*types.OutboundParams{{Amount: math.ZeroUint(), CallOptions: &types.CallOptions{}}} items[i].RevertOptions = types.NewEmptyRevertOptions() - keeper.SetCctxAndNonceToCctxAndInboundHashToCctx(ctx, items[i], tssPubkey) + keeper.SaveCCTXUpdate(ctx, items[i], tssPubkey) } return items } @@ -88,7 +88,7 @@ func createNCctx(keeper *keeper.Keeper, ctx sdk.Context, n int, tssPubkey string items[i].Index = fmt.Sprintf("%d", i) items[i].RevertOptions = types.NewEmptyRevertOptions() - keeper.SetCctxAndNonceToCctxAndInboundHashToCctx(ctx, items[i], tssPubkey) + keeper.SaveCCTXUpdate(ctx, items[i], tssPubkey) } return items } @@ -453,3 +453,234 @@ func Test_NewCCTX(t *testing.T) { require.Equal(t, types.ProtocolContractVersion_V1, cctx.ProtocolContractVersion) }) } + +func TestKeeper_UpdateNonceToCCTX(t *testing.T) { + t.Run("should set nonce to cctx if status is PendingOutbound", func(t *testing.T) { + // Arrange + k, ctx, _, _ := keepertest.CrosschainKeeper(t) + chainID := chains.Ethereum.ChainId + nonce := uint64(10) + + cctx := types.CrossChainTx{Index: "test", + OutboundParams: []*types.OutboundParams{{ReceiverChainId: chainID, TssNonce: nonce}}, + CctxStatus: &types.Status{Status: types.CctxStatus_PendingOutbound}, + } + tssPubkey := "test-tss-pubkey" + + // Act + k.SetNonceToCCTX(ctx, cctx, tssPubkey) + + // Assert + nonceToCctx, found := k.GetObserverKeeper().GetNonceToCctx(ctx, tssPubkey, chainID, int64(nonce)) + require.True(t, found) + require.Equal(t, cctx.Index, nonceToCctx.CctxIndex) + require.Equal(t, tssPubkey, nonceToCctx.Tss) + require.Equal(t, chainID, nonceToCctx.ChainId) + }) + + t.Run("should set nonce to cctx if status is PendingRevert", func(t *testing.T) { + // Arrange + k, ctx, _, _ := keepertest.CrosschainKeeper(t) + chainID := chains.Ethereum.ChainId + nonce := uint64(10) + + cctx := types.CrossChainTx{Index: "test", + OutboundParams: []*types.OutboundParams{{ReceiverChainId: chainID, TssNonce: nonce}}, + CctxStatus: &types.Status{Status: types.CctxStatus_PendingRevert}, + } + tssPubkey := "test-tss-pubkey" + + // Act + k.SetNonceToCCTX(ctx, cctx, tssPubkey) + + // Assert + nonceToCctx, found := k.GetObserverKeeper().GetNonceToCctx(ctx, tssPubkey, chainID, int64(nonce)) + require.True(t, found) + require.Equal(t, cctx.Index, nonceToCctx.CctxIndex) + require.Equal(t, tssPubkey, nonceToCctx.Tss) + require.Equal(t, chainID, nonceToCctx.ChainId) + }) + + t.Run("should not set nonce to cctx if status is not PendingOutbound or PendingRevert", func(t *testing.T) { + // Arrange + k, ctx, _, _ := keepertest.CrosschainKeeper(t) + chainID := chains.Ethereum.ChainId + nonce := uint64(10) + + cctx := types.CrossChainTx{Index: "test", + OutboundParams: []*types.OutboundParams{{ReceiverChainId: chainID, TssNonce: nonce}}, + CctxStatus: &types.Status{Status: types.CctxStatus_Aborted}, + } + tssPubkey := "test-tss-pubkey" + + // Act + k.SetNonceToCCTX(ctx, cctx, tssPubkey) + + // Assert + _, found := k.GetObserverKeeper().GetNonceToCctx(ctx, tssPubkey, chainID, int64(nonce)) + require.False(t, found) + }) +} + +func TestKeeper_UpdateInboundHashToCCTX(t *testing.T) { + t.Run( + "should update inbound hash to cctx mapping if new cctx index is found for the same inbound hash", + func(t *testing.T) { + // Arrange + k, ctx, _, _ := keepertest.CrosschainKeeper(t) + inboundHash := sample.Hash().String() + index1 := sample.ZetaIndex(t) + index2 := sample.ZetaIndex(t) + + inboundHashToCctx := types.InboundHashToCctx{ + InboundHash: inboundHash, + CctxIndex: []string{index1}, + } + k.SetInboundHashToCctx(ctx, inboundHashToCctx) + cctx := types.CrossChainTx{Index: index2, InboundParams: &types.InboundParams{ObservedHash: inboundHash}} + + // Act + k.UpdateInboundHashToCCTX(ctx, cctx) + + // Assert + inboundHashToCctx, found := k.GetInboundHashToCctx(ctx, inboundHash) + require.True(t, found) + require.Equal(t, inboundHash, inboundHashToCctx.InboundHash) + require.Equal(t, 2, len(inboundHashToCctx.CctxIndex)) + require.Contains(t, inboundHashToCctx.CctxIndex, index1) + require.Contains(t, inboundHashToCctx.CctxIndex, index2) + }, + ) + + t.Run("should do nothing if the cctx index is already in the mapping", func(t *testing.T) { + // Arrange + k, ctx, _, _ := keepertest.CrosschainKeeper(t) + inboundHash := sample.Hash().String() + index := sample.ZetaIndex(t) + + inboundHashToCctx := types.InboundHashToCctx{ + InboundHash: inboundHash, + CctxIndex: []string{index}, + } + k.SetInboundHashToCctx(ctx, inboundHashToCctx) + cctx := types.CrossChainTx{Index: index, InboundParams: &types.InboundParams{ObservedHash: inboundHash}} + + // Act + k.UpdateInboundHashToCCTX(ctx, cctx) + + // Assert + inboundHashToCctx, found := k.GetInboundHashToCctx(ctx, inboundHash) + require.True(t, found) + require.Equal(t, inboundHash, inboundHashToCctx.InboundHash) + require.Equal(t, 1, len(inboundHashToCctx.CctxIndex)) + require.Contains(t, inboundHashToCctx.CctxIndex, index) + }) + + t.Run("should add cctx index to mapping if InboundHashToCctx is not found", func(t *testing.T) { + // Arrange + k, ctx, _, _ := keepertest.CrosschainKeeper(t) + inboundHash := sample.Hash().String() + index := sample.ZetaIndex(t) + + cctx := types.CrossChainTx{Index: index, InboundParams: &types.InboundParams{ObservedHash: inboundHash}} + + // Act + k.UpdateInboundHashToCCTX(ctx, cctx) + + // Assert + inboundHashToCctx, found := k.GetInboundHashToCctx(ctx, inboundHash) + require.True(t, found) + require.Equal(t, inboundHash, inboundHashToCctx.InboundHash) + require.Equal(t, 1, len(inboundHashToCctx.CctxIndex)) + require.Contains(t, inboundHashToCctx.CctxIndex, index) + }) +} + +func TestKeeper_UpdateZetaAccounting(t *testing.T) { + t.Run("should update zeta accounting if cctx is aborted and coin type is zeta", func(t *testing.T) { + // Arrange + k, ctx, _, _ := keepertest.CrosschainKeeper(t) + amount := sdkmath.NewUint(100) + cctx := types.CrossChainTx{ + InboundParams: &types.InboundParams{CoinType: coin.CoinType_Zeta}, + CctxStatus: &types.Status{ + IsAbortRefunded: false, + Status: types.CctxStatus_Aborted}, + OutboundParams: []*types.OutboundParams{{Amount: amount}}, + } + k.SetZetaAccounting(ctx, types.ZetaAccounting{AbortedZetaAmount: math.ZeroUint()}) + + // Act + k.UpdateZetaAccounting(ctx, cctx) + + // Assert + zetaAccounting, found := k.GetZetaAccounting(ctx) + require.True(t, found) + require.Equal(t, amount, zetaAccounting.AbortedZetaAmount) + }) + + t.Run("should not update zeta accounting if cctx is not aborted", func(t *testing.T) { + // Arrange + k, ctx, _, _ := keepertest.CrosschainKeeper(t) + amount := sdkmath.NewUint(100) + cctx := types.CrossChainTx{ + InboundParams: &types.InboundParams{CoinType: coin.CoinType_Zeta}, + CctxStatus: &types.Status{ + IsAbortRefunded: false, + Status: types.CctxStatus_PendingOutbound}, + OutboundParams: []*types.OutboundParams{{Amount: amount}}, + } + k.SetZetaAccounting(ctx, types.ZetaAccounting{AbortedZetaAmount: math.ZeroUint()}) + + // Act + k.UpdateZetaAccounting(ctx, cctx) + + // Assert + zetaAccounting, found := k.GetZetaAccounting(ctx) + require.True(t, found) + require.Equal(t, math.ZeroUint(), zetaAccounting.AbortedZetaAmount) + }) + + t.Run("should update to amount if zeta accounting is not set", func(t *testing.T) { + // Arrange + k, ctx, _, _ := keepertest.CrosschainKeeper(t) + amount := sdkmath.NewUint(100) + cctx := types.CrossChainTx{ + InboundParams: &types.InboundParams{CoinType: coin.CoinType_Zeta}, + CctxStatus: &types.Status{ + IsAbortRefunded: false, + Status: types.CctxStatus_Aborted}, + OutboundParams: []*types.OutboundParams{{Amount: amount}}, + } + + // Act + k.UpdateZetaAccounting(ctx, cctx) + + // Assert + zetaAccounting, found := k.GetZetaAccounting(ctx) + require.True(t, found) + require.Equal(t, amount, zetaAccounting.AbortedZetaAmount) + }) + + t.Run("should not update zeta accounting if the cctx is already refunded", func(t *testing.T) { + // Arrange + k, ctx, _, _ := keepertest.CrosschainKeeper(t) + amount := sdkmath.NewUint(100) + cctx := types.CrossChainTx{ + InboundParams: &types.InboundParams{CoinType: coin.CoinType_Zeta}, + CctxStatus: &types.Status{ + IsAbortRefunded: true, + Status: types.CctxStatus_Aborted}, + OutboundParams: []*types.OutboundParams{{Amount: amount}}, + } + k.SetZetaAccounting(ctx, types.ZetaAccounting{AbortedZetaAmount: math.ZeroUint()}) + + // Act + k.UpdateZetaAccounting(ctx, cctx) + + // Assert + zetaAccounting, found := k.GetZetaAccounting(ctx) + require.True(t, found) + require.Equal(t, math.ZeroUint(), zetaAccounting.AbortedZetaAmount) + }) +} diff --git a/x/crosschain/keeper/export_private_functions_test.go b/x/crosschain/keeper/export_private_functions_test.go new file mode 100644 index 0000000000..0611ed995d --- /dev/null +++ b/x/crosschain/keeper/export_private_functions_test.go @@ -0,0 +1,20 @@ +package keeper + +import ( + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/zeta-chain/node/x/crosschain/types" +) + +// These functions are exported for testing purposes + +func (k Keeper) UpdateZetaAccounting(ctx sdk.Context, cctx types.CrossChainTx) { + k.updateZetaAccounting(ctx, cctx) +} + +func (k Keeper) UpdateInboundHashToCCTX(ctx sdk.Context, cctx types.CrossChainTx) { + k.updateInboundHashToCCTX(ctx, cctx) +} + +func (k Keeper) SetNonceToCCTX(ctx sdk.Context, cctx types.CrossChainTx, tssPubkey string) { + k.setNonceToCCTX(ctx, cctx, tssPubkey) +} diff --git a/x/crosschain/keeper/grpc_query_inbound_hash_to_cctx_test.go b/x/crosschain/keeper/grpc_query_inbound_hash_to_cctx_test.go index 296b46a63b..87e81162f6 100644 --- a/x/crosschain/keeper/grpc_query_inbound_hash_to_cctx_test.go +++ b/x/crosschain/keeper/grpc_query_inbound_hash_to_cctx_test.go @@ -140,7 +140,7 @@ func createInTxHashToCctxWithCctxs( cctxs[i].InboundParams = &types.InboundParams{ObservedHash: fmt.Sprintf("%d", i), Amount: math.OneUint()} cctxs[i].CctxStatus = &types.Status{Status: types.CctxStatus_PendingInbound} cctxs[i].RevertOptions = types.NewEmptyRevertOptions() - keeper.SetCctxAndNonceToCctxAndInboundHashToCctx(ctx, cctxs[i], tssPubkey) + keeper.SaveCCTXUpdate(ctx, cctxs[i], tssPubkey) } var inboundHashToCctx types.InboundHashToCctx diff --git a/x/crosschain/keeper/msg_server_abort_stuck_cctx.go b/x/crosschain/keeper/msg_server_abort_stuck_cctx.go index 6ba922e804..7e2505b01e 100644 --- a/x/crosschain/keeper/msg_server_abort_stuck_cctx.go +++ b/x/crosschain/keeper/msg_server_abort_stuck_cctx.go @@ -36,19 +36,16 @@ 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.IsPending() { 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 / New outbound is not added + k.SaveOutbound(ctx, &cctx, "") return &types.MsgAbortStuckCCTXResponse{}, nil } diff --git a/x/crosschain/keeper/msg_server_abort_stuck_cctx_test.go b/x/crosschain/keeper/msg_server_abort_stuck_cctx_test.go index a43d034b34..5b9b29cb8e 100644 --- a/x/crosschain/keeper/msg_server_abort_stuck_cctx_test.go +++ b/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,7 +30,15 @@ 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{ @@ -36,16 +46,23 @@ func TestMsgServer_AbortStuckCCTX(t *testing.T) { 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,7 +126,15 @@ 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{ @@ -101,13 +142,19 @@ func TestMsgServer_AbortStuckCCTX(t *testing.T) { 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) { diff --git a/x/crosschain/keeper/msg_server_migrate_erc20_custody_funds.go b/x/crosschain/keeper/msg_server_migrate_erc20_custody_funds.go index c94ccc089a..56fa655ddd 100644 --- a/x/crosschain/keeper/msg_server_migrate_erc20_custody_funds.go +++ b/x/crosschain/keeper/msg_server_migrate_erc20_custody_funds.go @@ -83,7 +83,7 @@ func (k msgServer) MigrateERC20CustodyFunds( if err != nil { return nil, err } - k.SetCctxAndNonceToCctxAndInboundHashToCctx(ctx, cctx, tss.TssPubkey) + k.SaveCCTXUpdate(ctx, cctx, tss.TssPubkey) err = ctx.EventManager().EmitTypedEvent( &types.EventERC20CustodyFundsMigration{ diff --git a/x/crosschain/keeper/msg_server_migrate_tss_funds.go b/x/crosschain/keeper/msg_server_migrate_tss_funds.go index 8ecc4c47ff..9ca7f2e7ab 100644 --- a/x/crosschain/keeper/msg_server_migrate_tss_funds.go +++ b/x/crosschain/keeper/msg_server_migrate_tss_funds.go @@ -133,7 +133,7 @@ func (k Keeper) initiateMigrateTSSFundsCCTX( } } - k.SetCctxAndNonceToCctxAndInboundHashToCctx(ctx, cctx, currentTss.TssPubkey) + k.SaveCCTXUpdate(ctx, cctx, currentTss.TssPubkey) k.zetaObserverKeeper.SetFundMigrator(ctx, observertypes.TssFundMigratorInfo{ ChainId: chainID, MigrationCctxIndex: cctx.Index, diff --git a/x/crosschain/keeper/msg_server_update_erc20_custody_pause_status.go b/x/crosschain/keeper/msg_server_update_erc20_custody_pause_status.go index ebd1b9cf1b..0d58f60711 100644 --- a/x/crosschain/keeper/msg_server_update_erc20_custody_pause_status.go +++ b/x/crosschain/keeper/msg_server_update_erc20_custody_pause_status.go @@ -81,7 +81,7 @@ func (k msgServer) UpdateERC20CustodyPauseStatus( if err != nil { return nil, err } - k.SetCctxAndNonceToCctxAndInboundHashToCctx(ctx, cctx, tss.TssPubkey) + k.SaveCCTXUpdate(ctx, cctx, tss.TssPubkey) err = ctx.EventManager().EmitTypedEvent( &types.EventERC20CustodyPausing{ diff --git a/x/crosschain/keeper/msg_server_vote_outbound_tx.go b/x/crosschain/keeper/msg_server_vote_outbound_tx.go index 4bb65a3c8f..a940c9c7db 100644 --- a/x/crosschain/keeper/msg_server_vote_outbound_tx.go +++ b/x/crosschain/keeper/msg_server_vote_outbound_tx.go @@ -100,6 +100,14 @@ func (k msgServer) VoteOutbound( return &types.MsgVoteOutboundResponse{}, nil } + // If the CCTX is in a terminal state, we do not need to process it. + if cctx.CctxStatus.Status.IsTerminal() { + return &types.MsgVoteOutboundResponse{}, cosmoserrors.Wrap( + types.ErrCCTXAlreadyFinalized, + fmt.Sprintf("CCTX status %s", cctx.CctxStatus.Status), + ) + } + // Set the finalized ballot to the current outbound params. cctx.SetOutboundBallotIndex(ballotIndex) @@ -216,7 +224,7 @@ func (k Keeper) SaveOutbound(ctx sdk.Context, cctx *types.CrossChainTx, tssPubke k.RemoveOutboundTrackerFromStore(ctx, outboundParams.ReceiverChainId, outboundParams.TssNonce) } // This should set nonce to cctx only if a new revert is created. - k.SetCctxAndNonceToCctxAndInboundHashToCctx(ctx, *cctx, tssPubkey) + k.SaveCCTXUpdate(ctx, *cctx, tssPubkey) } func (k Keeper) ValidateOutboundMessage(ctx sdk.Context, msg types.MsgVoteOutbound) (types.CrossChainTx, error) { diff --git a/x/crosschain/keeper/msg_server_vote_outbound_tx_test.go b/x/crosschain/keeper/msg_server_vote_outbound_tx_test.go index cadc6eeac8..5c1415d2df 100644 --- a/x/crosschain/keeper/msg_server_vote_outbound_tx_test.go +++ b/x/crosschain/keeper/msg_server_vote_outbound_tx_test.go @@ -173,6 +173,88 @@ func TestKeeper_VoteOutbound(t *testing.T) { require.Len(t, c.OutboundParams, expectedNumberOfOutboundParams) }) + t.Run("unable to finalize an outbound if the cctx has already been aborted ", func(t *testing.T) { + k, ctx, _, zk := keepertest.CrosschainKeeperWithMocks(t, keepertest.CrosschainMockOptions{ + UseObserverMock: true, + }) + + // Setup mock data + observerMock := keepertest.GetCrosschainObserverMock(t, k) + receiver := sample.EthAddress() + amount := big.NewInt(42) + senderChain := getValidEthChain() + asset := "" + observer := sample.AccAddress() + tss := sample.Tss() + zk.ObserverKeeper.SetObserverSet(ctx, observertypes.ObserverSet{ObserverList: []string{observer}}) + cctx := GetERC20Cctx(t, receiver, senderChain, asset, amount) + cctx.GetCurrentOutboundParam().TssPubkey = tss.TssPubkey + cctx.CctxStatus.Status = types.CctxStatus_Aborted + k.SetCrossChainTx(ctx, *cctx) + observerMock.On("GetTSS", ctx).Return(observertypes.TSS{}, true).Once() + + // Successfully mock VoteOnOutboundBallot + keepertest.MockVoteOnOutboundSuccessBallot(observerMock, ctx, cctx, senderChain, observer) + + msgServer := keeper.NewMsgServerImpl(*k) + msg := types.MsgVoteOutbound{ + CctxHash: cctx.Index, + OutboundTssNonce: cctx.GetCurrentOutboundParam().TssNonce, + OutboundChain: cctx.GetCurrentOutboundParam().ReceiverChainId, + Status: chains.ReceiveStatus_success, + Creator: observer, + ObservedOutboundHash: sample.Hash().String(), + ValueReceived: cctx.GetCurrentOutboundParam().Amount, + ObservedOutboundBlockHeight: 10, + ObservedOutboundEffectiveGasPrice: math.NewInt(21), + ObservedOutboundGasUsed: 21, + CoinType: cctx.InboundParams.CoinType, + } + _, err := msgServer.VoteOutbound(ctx, &msg) + require.ErrorIs(t, err, types.ErrCCTXAlreadyFinalized) + }) + + t.Run("unable to finalize an outbound if the cctx has already been outboundmined", func(t *testing.T) { + k, ctx, _, zk := keepertest.CrosschainKeeperWithMocks(t, keepertest.CrosschainMockOptions{ + UseObserverMock: true, + }) + + // Setup mock data + observerMock := keepertest.GetCrosschainObserverMock(t, k) + receiver := sample.EthAddress() + amount := big.NewInt(42) + senderChain := getValidEthChain() + asset := "" + observer := sample.AccAddress() + tss := sample.Tss() + zk.ObserverKeeper.SetObserverSet(ctx, observertypes.ObserverSet{ObserverList: []string{observer}}) + cctx := GetERC20Cctx(t, receiver, senderChain, asset, amount) + cctx.GetCurrentOutboundParam().TssPubkey = tss.TssPubkey + cctx.CctxStatus.Status = types.CctxStatus_OutboundMined + k.SetCrossChainTx(ctx, *cctx) + observerMock.On("GetTSS", ctx).Return(observertypes.TSS{}, true).Once() + + // Successfully mock VoteOnOutboundBallot + keepertest.MockVoteOnOutboundSuccessBallot(observerMock, ctx, cctx, senderChain, observer) + + msgServer := keeper.NewMsgServerImpl(*k) + msg := types.MsgVoteOutbound{ + CctxHash: cctx.Index, + OutboundTssNonce: cctx.GetCurrentOutboundParam().TssNonce, + OutboundChain: cctx.GetCurrentOutboundParam().ReceiverChainId, + Status: chains.ReceiveStatus_success, + Creator: observer, + ObservedOutboundHash: sample.Hash().String(), + ValueReceived: cctx.GetCurrentOutboundParam().Amount, + ObservedOutboundBlockHeight: 10, + ObservedOutboundEffectiveGasPrice: math.NewInt(21), + ObservedOutboundGasUsed: 21, + CoinType: cctx.InboundParams.CoinType, + } + _, err := msgServer.VoteOutbound(ctx, &msg) + require.ErrorIs(t, err, types.ErrCCTXAlreadyFinalized) + }) + t.Run("vote on outbound tx fails if tss is not found", func(t *testing.T) { k, ctx, _, zk := keepertest.CrosschainKeeperWithMocks(t, keepertest.CrosschainMockOptions{ UseObserverMock: true, diff --git a/x/crosschain/keeper/msg_server_whitelist_erc20.go b/x/crosschain/keeper/msg_server_whitelist_erc20.go index 197310e16c..fff9feb92a 100644 --- a/x/crosschain/keeper/msg_server_whitelist_erc20.go +++ b/x/crosschain/keeper/msg_server_whitelist_erc20.go @@ -174,7 +174,7 @@ func (k msgServer) WhitelistERC20( GasLimit: uint64(msg.GasLimit), } k.fungibleKeeper.SetForeignCoins(ctx, foreignCoin) - k.SetCctxAndNonceToCctxAndInboundHashToCctx(ctx, cctx, tss.TssPubkey) + k.SaveCCTXUpdate(ctx, cctx, tss.TssPubkey) commit() diff --git a/x/crosschain/keeper/outbound_tracker_test.go b/x/crosschain/keeper/outbound_tracker_test.go index 7eddcc2cfe..ad535739d1 100644 --- a/x/crosschain/keeper/outbound_tracker_test.go +++ b/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) { diff --git a/x/crosschain/types/errors.go b/x/crosschain/types/errors.go index 40182fe313..ca217bd537 100644 --- a/x/crosschain/types/errors.go +++ b/x/crosschain/types/errors.go @@ -58,4 +58,5 @@ var ( ErrValidatingInbound = errorsmod.Register(ModuleName, 1157, "unable to validate inbound") ErrInvalidGasLimit = errorsmod.Register(ModuleName, 1158, "invalid gas limit") ErrUnableToSetOutboundInfo = errorsmod.Register(ModuleName, 1159, "unable to set outbound info") + ErrCCTXAlreadyFinalized = errorsmod.Register(ModuleName, 1160, "cctx already finalized") ) diff --git a/x/crosschain/types/status.go b/x/crosschain/types/status.go index d67ec08ac6..5ebdf5391c 100644 --- a/x/crosschain/types/status.go +++ b/x/crosschain/types/status.go @@ -86,3 +86,21 @@ func stateTransitionMap() map[CctxStatus][]CctxStatus { } return stateTransitionMap } + +// IsTerminal returns true if the status is terminal. +// The terminal states are +// CctxStatus_Aborted +// CctxStatus_Reverted +// CctxStatus_OutboundMined +func (c CctxStatus) IsTerminal() bool { + return c == CctxStatus_Aborted || c == CctxStatus_Reverted || c == CctxStatus_OutboundMined +} + +// IsPending returns true if the status is pending. +// The pending states are +// CctxStatus_PendingInbound +// CctxStatus_PendingOutbound +// CctxStatus_PendingRevert +func (c CctxStatus) IsPending() bool { + return !c.IsTerminal() +} diff --git a/x/crosschain/types/status_test.go b/x/crosschain/types/status_test.go index 5bb272d522..f808f6e296 100644 --- a/x/crosschain/types/status_test.go +++ b/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.IsTerminal()) + }) + } +} + +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.IsPending()) + }) + } +}