resolve comments 4

Makefile

@@ -256,7 +256,7 @@ start-stress-test: zetanode
 
 start-tss-migration-test: zetanode
 	@echo "--> Starting migration test"
-	export E2E_ARGS="--test-migration" && \
+	export E2E_ARGS="--test-tss-migration" && \
 	cd contrib/localnet/ && $(DOCKER) compose up -d
 
 ###############################################################################

changelog.md

@@ -80,7 +80,7 @@
 * [2369](https://github.com/zeta-chain/node/pull/2369) - fix random cross-chain swap failure caused by using tiny UTXO
 * [2549](https://github.com/zeta-chain/node/pull/2459) - add separate accounts for each policy in e2e tests
 * [2415](https://github.com/zeta-chain/node/pull/2415) - add e2e test for upgrade and test admin functionalities
-* [2440](https://github.com/zeta-chain/node/pull/2440) - Add e2e test for Tss migration
+* [2440](https://github.com/zeta-chain/node/pull/2440) - Add e2e test for TSS migration
 
 
 ### Fixes

cmd/zetaclientd-supervisor/lib.go

@@ -182,6 +182,7 @@ func (s *zetaclientdSupervisor) handleTSSUpdate(ctx context.Context) {
 	retryInterval := 5 * time.Second
 
 	// TODO : use retry library under pkg/retry
+	// https://github.com/zeta-chain/node/issues/2492
 	for i := 0; i < maxRetries; i++ {
 		client := observertypes.NewQueryClient(s.zetacoredConn)
 		tss, err := client.TSS(ctx, &observertypes.QueryGetTSSRequest{})

cmd/zetaclientd/keygen_tss.go

@@ -85,9 +85,9 @@ func GenerateTSS(
 				}
 				// Try keygen only once at a particular block, irrespective of whether it is successful or failure
 				triedKeygenAtBlock = true
-				newPubkey, err := keygenTss(ctx, keyGen, *keygenTssServer, zetaCoreClient, keygenLogger)
+				newPubkey, err := keygenTSS(ctx, keyGen, *keygenTssServer, zetaCoreClient, keygenLogger)
 				if err != nil {
-					keygenLogger.Error().Err(err).Msg("keygenTss error")
+					keygenLogger.Error().Err(err).Msg("keygenTSS error")
 					tssFailedVoteHash, err := zetaCoreClient.PostVoteTSS(ctx,
 						"", keyGen.BlockNumber, chains.ReceiveStatus_failed)
 					if err != nil {
@@ -122,10 +122,10 @@ func GenerateTSS(
 	return errors.New("unexpected state for TSS generation")
 }
 
-// keygenTss generates a new TSS using the keygen request and the TSS server.
+// keygenTSS generates a new TSS using the keygen request and the TSS server.
 // If the keygen is successful, the function returns the new TSS pubkey.
 // If the keygen is unsuccessful, the function posts blame and returns an error.
-func keygenTss(
+func keygenTSS(
 	ctx context.Context,
 	keyGen observertypes.Keygen,
 	tssServer tss.TssServer,

cmd/zetae2e/local/local.go

@@ -36,7 +36,7 @@ const (
 	flagLight             = "light"
 	flagSetupOnly         = "setup-only"
 	flagSkipSetup         = "skip-setup"
-	flagTestMigration     = "test-migration"
+	flagTestTSSMigration  = "test-tss-migration"
 	flagSkipBitcoinSetup  = "skip-bitcoin-setup"
 	flagSkipHeaderProof   = "skip-header-proof"
 )
@@ -69,7 +69,7 @@ func NewLocalCmd() *cobra.Command {
 	cmd.Flags().Bool(flagSkipSetup, false, "set to true to skip setup")
 	cmd.Flags().Bool(flagSkipBitcoinSetup, false, "set to true to skip bitcoin wallet setup")
 	cmd.Flags().Bool(flagSkipHeaderProof, false, "set to true to skip header proof tests")
-	cmd.Flags().Bool(flagTestMigration, false, "set to true to include a migration test at the end")
+	cmd.Flags().Bool(flagTestTSSMigration, false, "set to true to include a migration test at the end")
 
 	return cmd
 }
@@ -90,7 +90,7 @@ func localE2ETest(cmd *cobra.Command, _ []string) {
 		skipSetup         = must(cmd.Flags().GetBool(flagSkipSetup))
 		skipBitcoinSetup  = must(cmd.Flags().GetBool(flagSkipBitcoinSetup))
 		skipHeaderProof   = must(cmd.Flags().GetBool(flagSkipHeaderProof))
-		testMigration     = must(cmd.Flags().GetBool(flagTestMigration))
+		testTSSMigration  = must(cmd.Flags().GetBool(flagTestTSSMigration))
 	)
 
 	logger := runner.NewLogger(verbose, color.FgWhite, "setup")
@@ -329,7 +329,7 @@ func localE2ETest(cmd *cobra.Command, _ []string) {
 
 	logger.Print("✅ e2e tests completed in %s", time.Since(testStartTime).String())
 
-	if testMigration {
+	if testTSSMigration {
 		runTSSMigrationTest(deployerRunner, logger, verbose, conf)
 	}
 

x/crosschain/keeper/cctx_orchestrator_validate_inbound.go

@@ -1,7 +1,6 @@
 package keeper
 
 import (
-	"cosmossdk.io/errors"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 
 	"github.com/zeta-chain/zetacore/pkg/chains"
@@ -22,7 +21,7 @@ func (k Keeper) ValidateInbound(
 		return nil, types.ErrCannotFindTSSKeys
 	}
 
-	err := k.CheckIfMigrationTransfer(ctx, msg)
+	err := k.CheckIfTSSMigrationTransfer(ctx, msg)
 	if err != nil {
 		return nil, err
 	}
@@ -57,12 +56,12 @@ func (k Keeper) ValidateInbound(
 	return &cctx, nil
 }
 
-// CheckIfMigrationTransfer checks if the sender is a TSS address and returns an error if it is.
+// CheckIfTSSMigrationTransfer checks if the sender is a TSS address and returns an error if it is.
 // If the sender is an older TSS address, this means that it is a migration transfer, and we do not need to treat this as a deposit and process the CCTX
-func (k Keeper) CheckIfMigrationTransfer(ctx sdk.Context, msg *types.MsgVoteInbound) error {
+func (k Keeper) CheckIfTSSMigrationTransfer(ctx sdk.Context, msg *types.MsgVoteInbound) error {
 	additionalChains := k.GetAuthorityKeeper().GetAdditionalChainList(ctx)
 
-	historicalTssList := k.zetaObserverKeeper.GetAllTSS(ctx)
+	historicalTSSList := k.zetaObserverKeeper.GetAllTSS(ctx)
 	chain, found := k.zetaObserverKeeper.GetSupportedChainFromChainID(ctx, msg.SenderChainId)
 	if !found {
 		return observertypes.ErrSupportedChains.Wrapf("chain not found for chainID %d", msg.SenderChainId)
@@ -75,10 +74,10 @@ func (k Keeper) CheckIfMigrationTransfer(ctx sdk.Context, msg *types.MsgVoteInbo
 
 	switch {
 	case chains.IsEVMChain(chain.ChainId, additionalChains):
-		for _, tss := range historicalTssList {
+		for _, tss := range historicalTSSList {
 			ethTssAddress, err := crypto.GetTssAddrEVM(tss.TssPubkey)
 			if err != nil {
-				return errors.Wrap(types.ErrInvalidAddress, err.Error())
+				continue
 			}
 			if ethTssAddress.Hex() == msg.Sender {
 				return types.ErrMigrationFromOldTss
@@ -89,10 +88,10 @@ func (k Keeper) CheckIfMigrationTransfer(ctx sdk.Context, msg *types.MsgVoteInbo
 		if err != nil {
 			return err
 		}
-		for _, tss := range historicalTssList {
+		for _, tss := range historicalTSSList {
 			btcTssAddress, err := crypto.GetTssAddrBTC(tss.TssPubkey, bitcoinParams)
 			if err != nil {
-				return errors.Wrap(types.ErrInvalidAddress, err.Error())
+				continue
 			}
 			if btcTssAddress == msg.Sender {
 				return types.ErrMigrationFromOldTss

x/crosschain/keeper/cctx_orchestrator_validate_inbound_test.go

@@ -43,7 +43,7 @@ func TestKeeper_ValidateInbound(t *testing.T) {
 		sender := sample.EthAddress()
 		tssList := sample.TssList(3)
 
-		// Set up mocks for CheckIfMigrationTransfer
+		// 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{})
@@ -174,7 +174,7 @@ func TestKeeper_ValidateInbound(t *testing.T) {
 		sender := sample.EthAddress()
 		tssList := sample.TssList(3)
 
-		// Set up mocks for CheckIfMigrationTransfer
+		// 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{})
@@ -241,7 +241,7 @@ func TestKeeper_ValidateInbound(t *testing.T) {
 		sender := sample.EthAddress()
 		tssList := sample.TssList(3)
 
-		// Set up mocks for CheckIfMigrationTransfer
+		// 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{})
@@ -315,7 +315,7 @@ func TestKeeper_ValidateInbound(t *testing.T) {
 		sender := sample.EthAddress()
 		tssList := sample.TssList(3)
 
-		// Set up mocks for CheckIfMigrationTransfer
+		// 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{})
@@ -352,7 +352,7 @@ func TestKeeper_ValidateInbound(t *testing.T) {
 		require.ErrorIs(t, err, observerTypes.ErrInboundDisabled)
 	})
 
-	t.Run("fails when CheckIfMigrationTransfer fails", func(t *testing.T) {
+	t.Run("fails when CheckIfTSSMigrationTransfer fails", func(t *testing.T) {
 		k, ctx, _, _ := keepertest.CrosschainKeeperWithMocks(t,
 			keepertest.CrosschainMockOptions{
 				UseObserverMock:  true,
@@ -381,7 +381,7 @@ func TestKeeper_ValidateInbound(t *testing.T) {
 		// setup Mocks for GetTSS
 		observerMock.On("GetTSS", mock.Anything).Return(tssList[0], true)
 
-		// Set up mocks for CheckIfMigrationTransfer
+		// Set up mocks for CheckIfTSSMigrationTransfer
 		observerMock.On("GetAllTSS", ctx).Return(tssList)
 		observerMock.On("GetSupportedChainFromChainID", mock.Anything, senderChain.ChainId).Return(senderChain, false)
 		authorityMock.On("GetAdditionalChainList", ctx).Return([]chains.Chain{})
@@ -438,7 +438,7 @@ func TestKeeper_CheckMigration(t *testing.T) {
 			Sender:        sender,
 		}
 
-		err := k.CheckIfMigrationTransfer(ctx, &msg)
+		err := k.CheckIfTSSMigrationTransfer(ctx, &msg)
 		require.NoError(t, err)
 	})
 
@@ -465,7 +465,7 @@ func TestKeeper_CheckMigration(t *testing.T) {
 			Sender:        sender,
 		}
 
-		err := k.CheckIfMigrationTransfer(ctx, &msg)
+		err := k.CheckIfTSSMigrationTransfer(ctx, &msg)
 		require.NoError(t, err)
 	})
 
@@ -494,11 +494,11 @@ func TestKeeper_CheckMigration(t *testing.T) {
 			Sender:        sender,
 		}
 
-		err := k.CheckIfMigrationTransfer(ctx, &msg)
+		err := k.CheckIfTSSMigrationTransfer(ctx, &msg)
 		require.ErrorIs(t, err, observerTypes.ErrSupportedChains)
 	})
 
-	t.Run("fails when tss address is invalid for bitcoin chain", func(t *testing.T) {
+	t.Run("skips check when an older tss address is invalid for bitcoin chain", func(t *testing.T) {
 		k, ctx, _, _ := keepertest.CrosschainKeeperWithMocks(t,
 			keepertest.CrosschainMockOptions{
 				UseAuthorityMock: true,
@@ -522,11 +522,11 @@ func TestKeeper_CheckMigration(t *testing.T) {
 			Sender:        sender,
 		}
 
-		err := k.CheckIfMigrationTransfer(ctx, &msg)
-		require.ErrorIs(t, err, types.ErrInvalidAddress)
+		err := k.CheckIfTSSMigrationTransfer(ctx, &msg)
+		require.NoError(t, err)
 	})
 
-	t.Run("fails when tss address is invalid for evm chain", func(t *testing.T) {
+	t.Run("skips check when an older tss address is invalid for evm chain", func(t *testing.T) {
 		k, ctx, _, _ := keepertest.CrosschainKeeperWithMocks(t,
 			keepertest.CrosschainMockOptions{
 				UseAuthorityMock: true,
@@ -550,8 +550,8 @@ func TestKeeper_CheckMigration(t *testing.T) {
 			Sender:        sender,
 		}
 
-		err := k.CheckIfMigrationTransfer(ctx, &msg)
-		require.ErrorIs(t, err, types.ErrInvalidAddress)
+		err := k.CheckIfTSSMigrationTransfer(ctx, &msg)
+		require.NoError(t, err)
 	})
 
 	t.Run("fails when sender is a TSS address for evm chain for evm chain", func(t *testing.T) {
@@ -578,7 +578,7 @@ func TestKeeper_CheckMigration(t *testing.T) {
 			Sender:        sender.String(),
 		}
 
-		err = k.CheckIfMigrationTransfer(ctx, &msg)
+		err = k.CheckIfTSSMigrationTransfer(ctx, &msg)
 		require.ErrorIs(t, err, types.ErrMigrationFromOldTss)
 	})
 
@@ -608,7 +608,7 @@ func TestKeeper_CheckMigration(t *testing.T) {
 			Sender:        sender,
 		}
 
-		err = k.CheckIfMigrationTransfer(ctx, &msg)
+		err = k.CheckIfTSSMigrationTransfer(ctx, &msg)
 		require.ErrorIs(t, err, types.ErrMigrationFromOldTss)
 	})
 
@@ -639,7 +639,7 @@ func TestKeeper_CheckMigration(t *testing.T) {
 			Sender:        sender,
 		}
 
-		err := k.CheckIfMigrationTransfer(ctx, &msg)
+		err := k.CheckIfTSSMigrationTransfer(ctx, &msg)
 		require.ErrorContains(t, err, "no Bitcoin net params for chain ID: 999")
 	})
 
@@ -662,7 +662,7 @@ func TestKeeper_CheckMigration(t *testing.T) {
 			Sender:        sender,
 		}
 
-		err := k.CheckIfMigrationTransfer(ctx, &msg)
+		err := k.CheckIfTSSMigrationTransfer(ctx, &msg)
 		require.NoError(t, err)
 	})
 }