diff --git a/Makefile b/Makefile index 4cb7375fae..75bd9e9f92 100644 --- a/Makefile +++ b/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 ############################################################################### diff --git a/changelog.md b/changelog.md index b2b61d30cc..bfe1c6b7d0 100644 --- a/changelog.md +++ b/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 diff --git a/cmd/zetaclientd-supervisor/lib.go b/cmd/zetaclientd-supervisor/lib.go index cf9b883ea6..71f492e88b 100644 --- a/cmd/zetaclientd-supervisor/lib.go +++ b/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{}) diff --git a/cmd/zetaclientd/keygen_tss.go b/cmd/zetaclientd/keygen_tss.go index 71beeb17fe..8d677a2680 100644 --- a/cmd/zetaclientd/keygen_tss.go +++ b/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, diff --git a/cmd/zetae2e/local/local.go b/cmd/zetae2e/local/local.go index 20e6c72f8e..3b935c4791 100644 --- a/cmd/zetae2e/local/local.go +++ b/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) } diff --git a/x/crosschain/keeper/cctx_orchestrator_validate_inbound.go b/x/crosschain/keeper/cctx_orchestrator_validate_inbound.go index 2d1dfb030d..4cda69a5c3 100644 --- a/x/crosschain/keeper/cctx_orchestrator_validate_inbound.go +++ b/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 diff --git a/x/crosschain/keeper/cctx_orchestrator_validate_inbound_test.go b/x/crosschain/keeper/cctx_orchestrator_validate_inbound_test.go index 2666869b18..0f09c61025 100644 --- a/x/crosschain/keeper/cctx_orchestrator_validate_inbound_test.go +++ b/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) }) }