diff --git a/changelog.md b/changelog.md index fa1d0af435..e1b73cd957 100644 --- a/changelog.md +++ b/changelog.md @@ -20,6 +20,7 @@ * [2615](https://github.com/zeta-chain/node/pull/2615) - Refactor cleanup of outbound trackers * [2749](https://github.com/zeta-chain/node/pull/2749) - fix all lint errors from govet +* [2725](https://github.com/zeta-chain/node/pull/2725) - refactor SetCctxAndNonceToCctxAndInboundHashToCctx to receive tsspubkey as an argument ### Tests diff --git a/testutil/keeper/crosschain.go b/testutil/keeper/crosschain.go index 8e58c6527f..aae70a32f6 100644 --- a/testutil/keeper/crosschain.go +++ b/testutil/keeper/crosschain.go @@ -417,10 +417,6 @@ func MockVoteOnOutboundFailedBallot( Once() } -func MockGetOutbound(m *crosschainmocks.CrosschainObserverKeeper, ctx sdk.Context) { - m.On("GetTSS", ctx).Return(observertypes.TSS{}, true).Once() -} - func MockSaveOutbound( m *crosschainmocks.CrosschainObserverKeeper, ctx sdk.Context, @@ -431,7 +427,6 @@ func MockSaveOutbound( m.On("RemoveFromPendingNonces", ctx, tss.TssPubkey, cctx.GetCurrentOutboundParam().ReceiverChainId, mock.Anything). Return().Times(expectedNumberOfOutboundParams) - m.On("GetTSS", ctx).Return(observertypes.TSS{}, true) } func MockSaveOutboundNewRevertCreated( diff --git a/x/crosschain/genesis.go b/x/crosschain/genesis.go index 6a1fe1ed58..7d750bafa1 100644 --- a/x/crosschain/genesis.go +++ b/x/crosschain/genesis.go @@ -43,9 +43,12 @@ func InitGenesis(ctx sdk.Context, k keeper.Keeper, genState types.GenesisState) } // Set all the cross-chain txs - for _, elem := range genState.CrossChainTxs { - if elem != nil { - k.SetCctxAndNonceToCctxAndInboundHashToCctx(ctx, *elem) + tss, found := k.GetObserverKeeper().GetTSS(ctx) + if found { + for _, elem := range genState.CrossChainTxs { + if elem != nil { + k.SetCctxAndNonceToCctxAndInboundHashToCctx(ctx, *elem, tss.TssPubkey) + } } } for _, elem := range genState.FinalizedInbounds { diff --git a/x/crosschain/keeper/cctx.go b/x/crosschain/keeper/cctx.go index f9cf7ee570..9a1e1781df 100644 --- a/x/crosschain/keeper/cctx.go +++ b/x/crosschain/keeper/cctx.go @@ -16,11 +16,11 @@ import ( // 2. set the mapping inboundHash -> cctxIndex , one inboundHash can be connected to multiple cctxindex // 3. set the mapping nonce => cctx // 4. update the zeta accounting -func (k Keeper) SetCctxAndNonceToCctxAndInboundHashToCctx(ctx sdk.Context, cctx types.CrossChainTx) { - tss, found := k.zetaObserverKeeper.GetTSS(ctx) - if !found { - return - } +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 { @@ -29,7 +29,7 @@ func (k Keeper) SetCctxAndNonceToCctxAndInboundHashToCctx(ctx sdk.Context, cctx // #nosec G115 always in range Nonce: int64(cctx.GetCurrentOutboundParam().TssNonce), CctxIndex: cctx.Index, - Tss: tss.TssPubkey, + Tss: tssPubkey, }) } @@ -37,7 +37,7 @@ func (k Keeper) SetCctxAndNonceToCctxAndInboundHashToCctx(ctx sdk.Context, cctx // set mapping inboundHash -> cctxIndex in, _ := k.GetInboundHashToCctx(ctx, cctx.InboundParams.ObservedHash) in.InboundHash = cctx.InboundParams.ObservedHash - found = false + found := false for _, cctxIndex := range in.CctxIndex { if cctxIndex == cctx.Index { found = true diff --git a/x/crosschain/keeper/cctx_orchestrator_validate_inbound.go b/x/crosschain/keeper/cctx_orchestrator_validate_inbound.go index 4cda69a5c3..f2f10e23b6 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) + k.SetCctxAndNonceToCctxAndInboundHashToCctx(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 bbd4530484..fd798998c0 100644 --- a/x/crosschain/keeper/cctx_orchestrator_validate_inbound_test.go +++ b/x/crosschain/keeper/cctx_orchestrator_validate_inbound_test.go @@ -214,80 +214,6 @@ func TestKeeper_ValidateInbound(t *testing.T) { require.ErrorIs(t, err, types.ErrCannotFindReceiverNonce) }) - t.Run("does not set cctx if SetCctxAndNonceToCctxAndInboundHashToCctx fails", func(t *testing.T) { - k, ctx, _, _ := keepertest.CrosschainKeeperWithMocks(t, - keepertest.CrosschainMockOptions{ - UseObserverMock: true, - UseFungibleMock: true, - UseAuthorityMock: true, - }) - - // Setup mock data - observerMock := keepertest.GetCrosschainObserverMock(t, k) - authorityMock := keepertest.GetCrosschainAuthorityMock(t, k) - receiver := sample.EthAddress() - creator := sample.AccAddress() - amount := sdkmath.NewUint(42) - message := "test" - inboundBlockHeight := uint64(420) - inboundHash := sample.Hash() - gasLimit := uint64(100) - asset := "test-asset" - eventIndex := uint64(1) - cointType := coin.CoinType_ERC20 - tss := sample.Tss() - receiverChain := chains.Goerli - senderChain := chains.Goerli - sender := sample.EthAddress() - tssList := sample.TssList(3) - - // Set up mocks for CheckIfTSSMigrationTransfer - observerMock.On("GetAllTSS", ctx).Return(tssList) - observerMock.On("GetSupportedChainFromChainID", mock.Anything, senderChain.ChainId).Return(senderChain, true) - authorityMock.On("GetAdditionalChainList", ctx).Return([]chains.Chain{}) - // setup Mocks for GetTSS - observerMock.On("GetTSS", mock.Anything).Return(tss, true).Twice() - // setup Mocks for IsInboundEnabled - observerMock.On("IsInboundEnabled", ctx).Return(true) - // setup mocks for Initiate Outbound - observerMock.On("GetChainNonces", mock.Anything, mock.Anything). - Return(observerTypes.ChainNonces{Nonce: 1}, true) - observerMock.On("GetPendingNonces", mock.Anything, mock.Anything, mock.Anything). - 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 - observerMock.On("GetTSS", mock.Anything).Return(tss, false).Once() - - k.SetGasPrice(ctx, types.GasPrice{ - ChainId: senderChain.ChainId, - MedianIndex: 0, - Prices: []uint64{100}, - }) - - // call InitiateOutbound - msg := types.MsgVoteInbound{ - Creator: creator, - Sender: sender.String(), - SenderChainId: senderChain.ChainId, - Receiver: receiver.String(), - ReceiverChain: receiverChain.ChainId, - Amount: amount, - Message: message, - InboundHash: inboundHash.String(), - InboundBlockHeight: inboundBlockHeight, - GasLimit: gasLimit, - CoinType: cointType, - TxOrigin: sender.String(), - Asset: asset, - EventIndex: eventIndex, - } - - _, err := k.ValidateInbound(ctx, &msg, false) - require.NoError(t, err) - require.Len(t, k.GetAllCrossChainTx(ctx), 0) - }) - t.Run("fail if inbound is disabled", func(t *testing.T) { k, ctx, _, _ := keepertest.CrosschainKeeperWithMocks(t, keepertest.CrosschainMockOptions{ diff --git a/x/crosschain/keeper/cctx_test.go b/x/crosschain/keeper/cctx_test.go index c523814c69..0c99f979c9 100644 --- a/x/crosschain/keeper/cctx_test.go +++ b/x/crosschain/keeper/cctx_test.go @@ -24,6 +24,7 @@ func createNCctxWithStatus( ctx sdk.Context, n int, status types.CctxStatus, + tssPubkey string, ) []types.CrossChainTx { items := make([]types.CrossChainTx, n) for i := range items { @@ -39,13 +40,13 @@ func createNCctxWithStatus( items[i].OutboundParams = []*types.OutboundParams{{Amount: math.ZeroUint()}} items[i].RevertOptions = types.NewEmptyRevertOptions() - keeper.SetCctxAndNonceToCctxAndInboundHashToCctx(ctx, items[i]) + keeper.SetCctxAndNonceToCctxAndInboundHashToCctx(ctx, items[i], tssPubkey) } return items } // Keeper Tests -func createNCctx(keeper *keeper.Keeper, ctx sdk.Context, n int) []types.CrossChainTx { +func createNCctx(keeper *keeper.Keeper, ctx sdk.Context, n int, tssPubkey string) []types.CrossChainTx { items := make([]types.CrossChainTx, n) for i := range items { items[i].Creator = "any" @@ -79,10 +80,9 @@ func createNCctx(keeper *keeper.Keeper, ctx sdk.Context, n int) []types.CrossCha items[i].ZetaFees = math.OneUint() items[i].Index = fmt.Sprintf("%d", i) - items[i].RevertOptions = types.NewEmptyRevertOptions() - keeper.SetCctxAndNonceToCctxAndInboundHashToCctx(ctx, items[i]) + keeper.SetCctxAndNonceToCctxAndInboundHashToCctx(ctx, items[i], tssPubkey) } return items } @@ -125,24 +125,39 @@ func TestCCTXs(t *testing.T) { keeper, ctx, _, zk := keepertest.CrosschainKeeper(t) keeper.SetZetaAccounting(ctx, types.ZetaAccounting{AbortedZetaAmount: math.ZeroUint()}) var sends []types.CrossChainTx - zk.ObserverKeeper.SetTSS(ctx, sample.Tss()) + tss := sample.Tss() + zk.ObserverKeeper.SetTSS(ctx, tss) + sends = append( + sends, + createNCctxWithStatus( + keeper, + ctx, + tt.PendingInbound, + types.CctxStatus_PendingInbound, + tss.TssPubkey, + )...) + sends = append( + sends, + createNCctxWithStatus( + keeper, + ctx, + tt.PendingOutbound, + types.CctxStatus_PendingOutbound, + tss.TssPubkey, + )...) sends = append( sends, - createNCctxWithStatus(keeper, ctx, tt.PendingInbound, types.CctxStatus_PendingInbound)...) + createNCctxWithStatus(keeper, ctx, tt.PendingRevert, types.CctxStatus_PendingRevert, tss.TssPubkey)...) sends = append( sends, - createNCctxWithStatus(keeper, ctx, tt.PendingOutbound, types.CctxStatus_PendingOutbound)...) + createNCctxWithStatus(keeper, ctx, tt.Aborted, types.CctxStatus_Aborted, tss.TssPubkey)...) sends = append( sends, - createNCctxWithStatus(keeper, ctx, tt.PendingRevert, types.CctxStatus_PendingRevert)...) - sends = append(sends, createNCctxWithStatus(keeper, ctx, tt.Aborted, types.CctxStatus_Aborted)...) + createNCctxWithStatus(keeper, ctx, tt.OutboundMined, types.CctxStatus_OutboundMined, tss.TssPubkey)...) sends = append( sends, - createNCctxWithStatus(keeper, ctx, tt.OutboundMined, types.CctxStatus_OutboundMined)...) - sends = append(sends, createNCctxWithStatus(keeper, ctx, tt.Reverted, types.CctxStatus_Reverted)...) - //require.Equal(t, tt.PendingOutbound, len(keeper.GetAllCctxByStatuses(ctx, []types.CctxStatus{types.CctxStatus_PendingOutbound}))) - //require.Equal(t, tt.PendingInbound, len(keeper.GetAllCctxByStatuses(ctx, []types.CctxStatus{types.CctxStatus_PendingInbound}))) - //require.Equal(t, tt.PendingOutbound+tt.PendingRevert, len(keeper.GetAllCctxByStatuses(ctx, []types.CctxStatus{types.CctxStatus_PendingOutbound, types.CctxStatus_PendingRevert}))) + createNCctxWithStatus(keeper, ctx, tt.Reverted, types.CctxStatus_Reverted, tss.TssPubkey)...) + require.Equal(t, len(sends), len(keeper.GetAllCrossChainTx(ctx))) for _, s := range sends { send, found := keeper.GetCrossChainTx(ctx, s.Index) @@ -156,8 +171,9 @@ func TestCCTXs(t *testing.T) { func TestCCTXGetAll(t *testing.T) { keeper, ctx, _, zk := keepertest.CrosschainKeeper(t) - zk.ObserverKeeper.SetTSS(ctx, sample.Tss()) - items := createNCctx(keeper, ctx, 10) + tss := sample.Tss() + zk.ObserverKeeper.SetTSS(ctx, tss) + items := createNCctx(keeper, ctx, 10, tss.TssPubkey) cctx := keeper.GetAllCrossChainTx(ctx) c := make([]types.CrossChainTx, len(cctx)) for i, val := range cctx { @@ -170,9 +186,10 @@ func TestCCTXGetAll(t *testing.T) { func TestCCTXQuerySingle(t *testing.T) { keeper, ctx, _, zk := keepertest.CrosschainKeeper(t) - zk.ObserverKeeper.SetTSS(ctx, sample.Tss()) + tss := sample.Tss() + zk.ObserverKeeper.SetTSS(ctx, tss) wctx := sdk.WrapSDKContext(ctx) - msgs := createNCctx(keeper, ctx, 2) + msgs := createNCctx(keeper, ctx, 2, tss.TssPubkey) for _, tc := range []struct { desc string request *types.QueryGetCctxRequest @@ -213,9 +230,10 @@ func TestCCTXQuerySingle(t *testing.T) { func TestCCTXQueryPaginated(t *testing.T) { keeper, ctx, _, zk := keepertest.CrosschainKeeper(t) + tss := sample.Tss() zk.ObserverKeeper.SetTSS(ctx, sample.Tss()) wctx := sdk.WrapSDKContext(ctx) - msgs := createNCctx(keeper, ctx, 5) + msgs := createNCctx(keeper, ctx, 5, tss.TssPubkey) request := func(next []byte, offset, limit uint64, total bool) *types.QueryAllCctxRequest { return &types.QueryAllCctxRequest{ @@ -262,8 +280,9 @@ func TestCCTXQueryPaginated(t *testing.T) { func TestKeeper_RemoveCrossChainTx(t *testing.T) { keeper, ctx, _, zk := keepertest.CrosschainKeeper(t) - zk.ObserverKeeper.SetTSS(ctx, sample.Tss()) - txs := createNCctx(keeper, ctx, 5) + tss := sample.Tss() + zk.ObserverKeeper.SetTSS(ctx, tss) + txs := createNCctx(keeper, ctx, 5, tss.TssPubkey) keeper.RemoveCrossChainTx(ctx, txs[0].Index) txs = keeper.GetAllCrossChainTx(ctx) 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 c99bd9d9fc..da91836fe4 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 @@ -126,7 +126,11 @@ func TestInTxHashToCctxQueryPaginated(t *testing.T) { }) } -func createInTxHashToCctxWithCctxs(keeper *crosschainkeeper.Keeper, ctx sdk.Context) ([]types.CrossChainTx, +func createInTxHashToCctxWithCctxs( + ctx sdk.Context, + keeper *crosschainkeeper.Keeper, + tssPubkey string, +) ([]types.CrossChainTx, types.InboundHashToCctx) { cctxs := make([]types.CrossChainTx, 5) for i := range cctxs { @@ -136,7 +140,7 @@ func createInTxHashToCctxWithCctxs(keeper *crosschainkeeper.Keeper, ctx sdk.Cont 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]) + keeper.SetCctxAndNonceToCctxAndInboundHashToCctx(ctx, cctxs[i], tssPubkey) } var inboundHashToCctx types.InboundHashToCctx @@ -152,9 +156,10 @@ func createInTxHashToCctxWithCctxs(keeper *crosschainkeeper.Keeper, ctx sdk.Cont func TestKeeper_InTxHashToCctxDataQuery(t *testing.T) { keeper, ctx, _, zk := keepertest.CrosschainKeeper(t) wctx := sdk.WrapSDKContext(ctx) - zk.ObserverKeeper.SetTSS(ctx, sample.Tss()) + tss := sample.Tss() + zk.ObserverKeeper.SetTSS(ctx, tss) t.Run("can query all cctxs data with in tx hash", func(t *testing.T) { - cctxs, inboundHashToCctx := createInTxHashToCctxWithCctxs(keeper, ctx) + cctxs, inboundHashToCctx := createInTxHashToCctxWithCctxs(ctx, keeper, tss.TssPubkey) req := &types.QueryInboundHashToCctxDataRequest{ InboundHash: inboundHashToCctx.InboundHash, } 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 56bc0a5ea1..da1cd23618 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) + k.SetCctxAndNonceToCctxAndInboundHashToCctx(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 f1f625571a..7921686ab5 100644 --- a/x/crosschain/keeper/msg_server_migrate_tss_funds.go +++ b/x/crosschain/keeper/msg_server_migrate_tss_funds.go @@ -134,7 +134,7 @@ func (k Keeper) initiateMigrateTSSFundsCCTX( } } - k.SetCctxAndNonceToCctxAndInboundHashToCctx(ctx, cctx) + k.SetCctxAndNonceToCctxAndInboundHashToCctx(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 4cda11f633..98e73566ed 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) + k.SetCctxAndNonceToCctxAndInboundHashToCctx(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 3da780ccc1..d4505dd2f3 100644 --- a/x/crosschain/keeper/msg_server_vote_outbound_tx.go +++ b/x/crosschain/keeper/msg_server_vote_outbound_tx.go @@ -63,6 +63,16 @@ func (k msgServer) VoteOutbound( ) (*types.MsgVoteOutboundResponse, error) { ctx := sdk.UnwrapSDKContext(goCtx) + // Check if TSS exists + // It is almost impossible to reach this point without a TSS, + // as the check for TSS was already done when creating the inbound, but we check anyway. + // We also expect the tss.Pubkey to be the same as the one in the outbound params, + // As we would have processed all CCTXs before migrating + tss, found := k.zetaObserverKeeper.GetTSS(ctx) + if !found { + return nil, cosmoserrors.Wrap(types.ErrCannotFindTSSKeys, voteOutboundID) + } + // Validate the message params to verify it against an existing CCTX. cctx, err := k.ValidateOutboundMessage(ctx, *msg) if err != nil { @@ -92,6 +102,7 @@ func (k msgServer) VoteOutbound( // Set the finalized ballot to the current outbound params. cctx.SetOutboundBallotIndex(ballotIndex) + // If ballot is successful, the value received should be the out tx amount. err = cctx.AddOutbound(ctx, *msg, ballot.BallotStatus) if err != nil { @@ -101,13 +112,13 @@ func (k msgServer) VoteOutbound( // Fund the gas stability pool with the remaining funds. k.FundStabilityPool(ctx, &cctx) + // We use the current TSS pubkey to finalize the outbound. err = k.ValidateOutboundObservers(ctx, &cctx, ballot.BallotStatus, msg.ValueReceived.String()) if err != nil { - k.SaveFailedOutbound(ctx, &cctx, err.Error()) + k.SaveFailedOutbound(ctx, &cctx, err.Error(), tss.TssPubkey) return &types.MsgVoteOutboundResponse{}, nil } - - k.SaveSuccessfulOutbound(ctx, &cctx) + k.SaveSuccessfulOutbound(ctx, &cctx, tss.TssPubkey) return &types.MsgVoteOutboundResponse{}, nil } @@ -173,17 +184,17 @@ SaveFailedOutbound saves a failed outbound transaction.It does the following thi 2. Save the outbound */ -func (k Keeper) SaveFailedOutbound(ctx sdk.Context, cctx *types.CrossChainTx, errMessage string) { +func (k Keeper) SaveFailedOutbound(ctx sdk.Context, cctx *types.CrossChainTx, errMessage string, tssPubkey string) { cctx.SetAbort(errMessage) ctx.Logger().Error(errMessage) - k.SaveOutbound(ctx, cctx) + k.SaveOutbound(ctx, cctx, tssPubkey) } // SaveSuccessfulOutbound saves a successful outbound transaction. // This function does not set the CCTX status, therefore all successful outbound transactions need // to have their status set during processing -func (k Keeper) SaveSuccessfulOutbound(ctx sdk.Context, cctx *types.CrossChainTx) { - k.SaveOutbound(ctx, cctx) +func (k Keeper) SaveSuccessfulOutbound(ctx sdk.Context, cctx *types.CrossChainTx, tssPubkey string) { + k.SaveOutbound(ctx, cctx, tssPubkey) } /* @@ -197,16 +208,15 @@ SaveOutbound saves the outbound transaction.It does the following things in one 4. Set the cctx and nonce to cctx and inboundHash to cctx */ -func (k Keeper) SaveOutbound(ctx sdk.Context, cctx *types.CrossChainTx) { +func (k Keeper) SaveOutbound(ctx sdk.Context, cctx *types.CrossChainTx, tssPubkey string) { // #nosec G115 always in range for _, outboundParams := range cctx.OutboundParams { k.GetObserverKeeper(). RemoveFromPendingNonces(ctx, outboundParams.TssPubkey, outboundParams.ReceiverChainId, int64(outboundParams.TssNonce)) k.RemoveOutboundTrackerFromStore(ctx, outboundParams.ReceiverChainId, outboundParams.TssNonce) } - // This should set nonce to cctx only if a new revert is created. - k.SetCctxAndNonceToCctxAndInboundHashToCctx(ctx, *cctx) + k.SetCctxAndNonceToCctxAndInboundHashToCctx(ctx, *cctx, tssPubkey) } func (k Keeper) ValidateOutboundMessage(ctx sdk.Context, msg types.MsgVoteOutbound) (types.CrossChainTx, error) { @@ -228,12 +238,6 @@ func (k Keeper) ValidateOutboundMessage(ctx sdk.Context, msg types.MsgVoteOutbou ) } - // Do not process an outbound vote if TSS is not found. - _, found = k.zetaObserverKeeper.GetTSS(ctx) - if !found { - return types.CrossChainTx{}, cosmoserrors.Wrap(types.ErrCannotFindTSSKeys, voteOutboundID) - } - if cctx.GetCurrentOutboundParam().ReceiverChainId != msg.OutboundChain { return types.CrossChainTx{}, cosmoserrors.Wrapf( sdkerrors.ErrInvalidRequest, 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 0ac0d817ba..13ecdc672e 100644 --- a/x/crosschain/keeper/msg_server_vote_outbound_tx_test.go +++ b/x/crosschain/keeper/msg_server_vote_outbound_tx_test.go @@ -146,9 +146,6 @@ func TestKeeper_VoteOutbound(t *testing.T) { // Successfully mock VoteOnOutboundBallot keepertest.MockVoteOnOutboundSuccessBallot(observerMock, ctx, cctx, senderChain, observer) - // Successfully mock GetOutbound - keepertest.MockGetOutbound(observerMock, ctx) - // Successfully mock SaveSuccessfulOutbound expectedNumberOfOutboundParams := 1 keepertest.MockSaveOutbound(observerMock, ctx, cctx, tss, expectedNumberOfOutboundParams) @@ -176,6 +173,44 @@ func TestKeeper_VoteOutbound(t *testing.T) { require.Len(t, c.OutboundParams, expectedNumberOfOutboundParams) }) + 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, + }) + + // 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_PendingOutbound + k.SetCrossChainTx(ctx, *cctx) + observerMock.On("GetTSS", ctx).Return(observertypes.TSS{}, false).Once() + + 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.ErrCannotFindTSSKeys) + }) + t.Run("successfully vote on outbound tx, vote-type failed", func(t *testing.T) { k, ctx, _, zk := keepertest.CrosschainKeeperWithMocks(t, keepertest.CrosschainMockOptions{ UseObserverMock: true, @@ -201,9 +236,6 @@ func TestKeeper_VoteOutbound(t *testing.T) { // Successfully mock VoteOnOutboundBallot keepertest.MockVoteOnOutboundFailedBallot(observerMock, ctx, cctx, senderChain, observer) - // Successfully mock GetOutbound - keepertest.MockGetOutbound(observerMock, ctx) - // Successfully mock ProcessOutbound keepertest.MockGetRevertGasLimitForERC20(fungibleMock, asset, senderChain, 100) keepertest.MockPayGasAndUpdateCCTX(fungibleMock, observerMock, ctx, *k, senderChain, asset) @@ -264,9 +296,6 @@ func TestKeeper_VoteOutbound(t *testing.T) { // Successfully mock VoteOnOutboundBallot keepertest.MockVoteOnOutboundFailedBallot(observerMock, ctx, cctx, senderChain, observer) - // Successfully mock GetOutbound - keepertest.MockGetOutbound(observerMock, ctx) - // Mock Failed ProcessOutbound keepertest.MockGetRevertGasLimitForERC20(fungibleMock, asset, senderChain, 100) keepertest.MockPayGasAndUpdateCCTX(fungibleMock, observerMock, ctx, *k, senderChain, asset) @@ -334,9 +363,6 @@ func TestKeeper_VoteOutbound(t *testing.T) { // Successfully mock VoteOnOutboundBallot keepertest.MockVoteOnOutboundFailedBallot(observerMock, ctx, cctx, senderChain, observer) - // Successfully mock GetOutbound - keepertest.MockGetOutbound(observerMock, ctx) - // Fail ProcessOutbound so that changes are not committed to the state fungibleMock.On("GetForeignCoinFromAsset", mock.Anything, mock.Anything, mock.Anything). Return(fungibletypes.ForeignCoins{}, false) @@ -472,6 +498,7 @@ func TestKeeper_VoteOutbound(t *testing.T) { func TestKeeper_SaveFailedOutbound(t *testing.T) { t.Run("successfully save failed outbound", func(t *testing.T) { + //ARRANGE k, ctx, _, _ := keepertest.CrosschainKeeper(t) cctx := sample.CrossChainTx(t, "test") k.SetOutboundTracker(ctx, types.OutboundTracker{ @@ -481,7 +508,11 @@ func TestKeeper_SaveFailedOutbound(t *testing.T) { HashList: nil, }) cctx.CctxStatus.Status = types.CctxStatus_PendingOutbound - k.SaveFailedOutbound(ctx, cctx, sample.String()) + + //ACT + k.SaveFailedOutbound(ctx, cctx, sample.String(), sample.Tss().TssPubkey) + + //ASSERT require.Equal(t, cctx.CctxStatus.Status, types.CctxStatus_Aborted) _, found := k.GetOutboundTracker( ctx, @@ -492,6 +523,7 @@ func TestKeeper_SaveFailedOutbound(t *testing.T) { }) t.Run("successfully save failed outbound with multiple trackers", func(t *testing.T) { + //ARRANGE k, ctx, _, _ := keepertest.CrosschainKeeper(t) cctx := sample.CrossChainTx(t, "test") for _, outboundParams := range cctx.OutboundParams { @@ -503,9 +535,12 @@ func TestKeeper_SaveFailedOutbound(t *testing.T) { }) } cctx.CctxStatus.Status = types.CctxStatus_PendingOutbound - k.SaveFailedOutbound(ctx, cctx, sample.String()) - require.Equal(t, cctx.CctxStatus.Status, types.CctxStatus_Aborted) + //ACT + k.SaveFailedOutbound(ctx, cctx, sample.String(), sample.Tss().TssPubkey) + + //ASSERT + require.Equal(t, cctx.CctxStatus.Status, types.CctxStatus_Aborted) for _, outboundParams := range cctx.OutboundParams { _, found := k.GetOutboundTracker( ctx, @@ -514,12 +549,12 @@ func TestKeeper_SaveFailedOutbound(t *testing.T) { ) require.False(t, found) } - }) } func TestKeeper_SaveSuccessfulOutbound(t *testing.T) { t.Run("successfully save successful outbound", func(t *testing.T) { + //ARRANGE k, ctx, _, _ := keepertest.CrosschainKeeper(t) cctx := sample.CrossChainTx(t, "test") k.SetOutboundTracker(ctx, types.OutboundTracker{ @@ -529,8 +564,11 @@ func TestKeeper_SaveSuccessfulOutbound(t *testing.T) { HashList: nil, }) cctx.CctxStatus.Status = types.CctxStatus_PendingOutbound - k.SaveSuccessfulOutbound(ctx, cctx) + //ACT + k.SaveSuccessfulOutbound(ctx, cctx, sample.Tss().TssPubkey) + + //ASSERT _, found := k.GetOutboundTracker( ctx, cctx.GetCurrentOutboundParam().ReceiverChainId, @@ -540,6 +578,7 @@ func TestKeeper_SaveSuccessfulOutbound(t *testing.T) { }) t.Run("successfully save successful outbound with multiple trackers", func(t *testing.T) { + //ARRANGE k, ctx, _, _ := keepertest.CrosschainKeeper(t) cctx := sample.CrossChainTx(t, "test") for _, outboundParams := range cctx.OutboundParams { @@ -551,8 +590,11 @@ func TestKeeper_SaveSuccessfulOutbound(t *testing.T) { }) } cctx.CctxStatus.Status = types.CctxStatus_PendingOutbound - k.SaveSuccessfulOutbound(ctx, cctx) + //ACT + k.SaveSuccessfulOutbound(ctx, cctx, sample.Tss().TssPubkey) + + //ASSERT for _, outboundParams := range cctx.OutboundParams { _, found := k.GetOutboundTracker( ctx, @@ -566,8 +608,8 @@ func TestKeeper_SaveSuccessfulOutbound(t *testing.T) { func TestKeeper_SaveOutbound(t *testing.T) { t.Run("successfully save outbound", func(t *testing.T) { + //ARRANGE k, ctx, _, zk := keepertest.CrosschainKeeper(t) - // setup state for crosschain and observer modules cctx := sample.CrossChainTx(t, "test") cctx.CctxStatus.Status = types.CctxStatus_PendingOutbound @@ -577,7 +619,6 @@ func TestKeeper_SaveOutbound(t *testing.T) { Nonce: cctx.GetCurrentOutboundParam().TssNonce, HashList: nil, }) - zk.ObserverKeeper.SetPendingNonces(ctx, observertypes.PendingNonces{ NonceLow: int64(cctx.GetCurrentOutboundParam().TssNonce) - 1, NonceHigh: int64(cctx.GetCurrentOutboundParam().TssNonce) + 1, @@ -588,8 +629,11 @@ func TestKeeper_SaveOutbound(t *testing.T) { TssPubkey: cctx.GetCurrentOutboundParam().TssPubkey, }) + //ACT // Save outbound and assert all values are successfully saved - k.SaveOutbound(ctx, cctx) + k.SaveOutbound(ctx, cctx, cctx.GetCurrentOutboundParam().TssPubkey) + + //ASSERT _, found := k.GetOutboundTracker( ctx, cctx.GetCurrentOutboundParam().ReceiverChainId, @@ -616,8 +660,8 @@ func TestKeeper_SaveOutbound(t *testing.T) { }) t.Run("successfully save outbound with multiple trackers", func(t *testing.T) { + //ARRANGE k, ctx, _, zk := keepertest.CrosschainKeeper(t) - // setup state for crosschain and observer modules cctx := sample.CrossChainTx(t, "test") for _, outboundParams := range cctx.OutboundParams { @@ -635,15 +679,16 @@ func TestKeeper_SaveOutbound(t *testing.T) { }) } cctx.CctxStatus.Status = types.CctxStatus_PendingRevert - tssPubkey := cctx.GetCurrentOutboundParam().TssPubkey zk.ObserverKeeper.SetTSS(ctx, observertypes.TSS{ TssPubkey: tssPubkey, }) + //ACT // Save outbound and assert all values are successfully saved - k.SaveOutbound(ctx, cctx) + k.SaveOutbound(ctx, cctx, cctx.GetCurrentOutboundParam().TssPubkey) + //ASSERT for _, outboundParams := range cctx.OutboundParams { _, found := k.GetOutboundTracker( ctx, @@ -717,17 +762,6 @@ func TestKeeper_ValidateOutboundMessage(t *testing.T) { ) }) - t.Run("failed to validate outbound message if tss not found", func(t *testing.T) { - k, ctx, _, _ := keepertest.CrosschainKeeper(t) - cctx := sample.CrossChainTx(t, "test") - k.SetCrossChainTx(ctx, *cctx) - _, err := k.ValidateOutboundMessage(ctx, types.MsgVoteOutbound{ - CctxHash: cctx.Index, - OutboundTssNonce: cctx.GetCurrentOutboundParam().TssNonce, - }) - require.ErrorIs(t, err, types.ErrCannotFindTSSKeys) - }) - t.Run("failed to validate outbound message if chain does not match", func(t *testing.T) { k, ctx, _, zk := keepertest.CrosschainKeeper(t) cctx := sample.CrossChainTx(t, "test") diff --git a/x/crosschain/keeper/msg_server_whitelist_erc20.go b/x/crosschain/keeper/msg_server_whitelist_erc20.go index 45cf836061..62ce836643 100644 --- a/x/crosschain/keeper/msg_server_whitelist_erc20.go +++ b/x/crosschain/keeper/msg_server_whitelist_erc20.go @@ -154,7 +154,7 @@ func (k msgServer) WhitelistERC20( GasLimit: uint64(msg.GasLimit), } k.fungibleKeeper.SetForeignCoins(ctx, foreignCoin) - k.SetCctxAndNonceToCctxAndInboundHashToCctx(ctx, cctx) + k.SetCctxAndNonceToCctxAndInboundHashToCctx(ctx, cctx, tss.TssPubkey) commit()