From 040346295092104f2a6dec0e9a2d5585705d482d Mon Sep 17 00:00:00 2001 From: Tanmay Date: Tue, 20 Aug 2024 13:11:28 -0400 Subject: [PATCH 1/4] refactor: fix lint errors from govet (#2749) * fix lint * fix lint * add changelog --- app/ante/authz.go | 2 +- changelog.md | 1 + cmd/zetaclientd/start.go | 2 +- cmd/zetaclientd/version.go | 2 +- .../test_message_passing_evm_to_zevm.go | 3 +-- e2e/e2etests/test_migrate_tss.go | 13 ++++++------- e2e/runner/accounting.go | 2 +- e2e/runner/logger.go | 14 +++++++------- e2e/runner/report.go | 2 +- pkg/chains/conversion.go | 3 ++- .../keeper/msg_server_add_inbound_tracker.go | 6 +++--- .../keeper/msg_server_add_outbound_tracker.go | 6 +++--- .../types/message_update_rate_limiter_flags.go | 2 +- x/emissions/keeper/msg_server_update_params.go | 2 +- x/fungible/keeper/evm.go | 18 +++++++++--------- x/fungible/keeper/gas_price.go | 6 +++--- .../types/message_update_chain_params.go | 2 +- x/observer/types/message_vote_block_header.go | 2 +- 18 files changed, 44 insertions(+), 44 deletions(-) diff --git a/app/ante/authz.go b/app/ante/authz.go index 5c8a58c534..d310943771 100644 --- a/app/ante/authz.go +++ b/app/ante/authz.go @@ -34,7 +34,7 @@ func (ald AuthzLimiterDecorator) AnteHandle( next sdk.AnteHandler, ) (newCtx sdk.Context, err error) { if err := ald.checkDisabledMsgs(tx.GetMsgs(), false, 1); err != nil { - return ctx, errorsmod.Wrapf(errortypes.ErrUnauthorized, err.Error()) + return ctx, errorsmod.Wrap(errortypes.ErrUnauthorized, err.Error()) } return next(ctx, tx, simulate) } diff --git a/changelog.md b/changelog.md index d00252a146..d6791e1659 100644 --- a/changelog.md +++ b/changelog.md @@ -18,6 +18,7 @@ ### Refactor * [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 ### Tests diff --git a/cmd/zetaclientd/start.go b/cmd/zetaclientd/start.go index cf6a8e4d9e..0c0bbd6585 100644 --- a/cmd/zetaclientd/start.go +++ b/cmd/zetaclientd/start.go @@ -169,7 +169,7 @@ func start(_ *cobra.Command, _ []string) error { startLogger.Debug().Msgf("hotkeyPk %s", hotkeyPk.String()) if len(hotkeyPk.Bytes()) != 32 { errMsg := fmt.Sprintf("key bytes len %d != 32", len(hotkeyPk.Bytes())) - log.Error().Msgf(errMsg) + log.Error().Msg(errMsg) return errors.New(errMsg) } priKey := secp256k1.PrivKey(hotkeyPk.Bytes()[:32]) diff --git a/cmd/zetaclientd/version.go b/cmd/zetaclientd/version.go index 817d2752f7..15886d4b80 100644 --- a/cmd/zetaclientd/version.go +++ b/cmd/zetaclientd/version.go @@ -15,6 +15,6 @@ var VersionCmd = &cobra.Command{ } func Version(_ *cobra.Command, _ []string) error { - fmt.Printf(constant.Version) + fmt.Print(constant.Version) return nil } diff --git a/e2e/e2etests/test_message_passing_evm_to_zevm.go b/e2e/e2etests/test_message_passing_evm_to_zevm.go index 6dfb1b53c2..09dac29a73 100644 --- a/e2e/e2etests/test_message_passing_evm_to_zevm.go +++ b/e2e/e2etests/test_message_passing_evm_to_zevm.go @@ -1,7 +1,6 @@ package e2etests import ( - "fmt" "math/big" "github.com/ethereum/go-ethereum/accounts/abi/bind" @@ -56,7 +55,7 @@ func TestMessagePassingEVMtoZEVM(r *runner.E2ERunner, args []string) { cctx := utils.WaitCctxMinedByInboundHash(r.Ctx, receipt.TxHash.String(), r.CctxClient, r.Logger, r.CctxTimeout) utils.RequireCCTXStatus(r, cctx, cctxtypes.CctxStatus_OutboundMined) - r.Logger.Info(fmt.Sprintf("πŸ”„ Cctx mined for contract call chain zevm %s", cctx.Index)) + r.Logger.Info("πŸ”„ Cctx mined for contract call chain zevm %s", cctx.Index) // On finalization the Fungible module calls the onReceive function which in turn calls the onZetaMessage function on the destination contract receipt, err = r.ZEVMClient.TransactionReceipt(r.Ctx, ethcommon.HexToHash(cctx.GetCurrentOutboundParam().Hash)) diff --git a/e2e/e2etests/test_migrate_tss.go b/e2e/e2etests/test_migrate_tss.go index c72b876f0f..c1bb79c062 100644 --- a/e2e/e2etests/test_migrate_tss.go +++ b/e2e/e2etests/test_migrate_tss.go @@ -2,7 +2,6 @@ package e2etests import ( "context" - "fmt" "sort" "strconv" "time" @@ -146,9 +145,9 @@ func TestMigrateTSS(r *runner.E2ERunner, _ []string) { btcTSSBalanceNew += utxo.Amount } - r.Logger.Info(fmt.Sprintf("BTC Balance Old: %f", btcTSSBalanceOld*1e8)) - r.Logger.Info(fmt.Sprintf("BTC Balance New: %f", btcTSSBalanceNew*1e8)) - r.Logger.Info(fmt.Sprintf("Migrator amount : %s", cctxBTC.GetCurrentOutboundParam().Amount)) + r.Logger.Info("BTC Balance Old: %f", btcTSSBalanceOld*1e8) + r.Logger.Info("BTC Balance New: %f", btcTSSBalanceNew*1e8) + r.Logger.Info("Migrator amount : %s", cctxBTC.GetCurrentOutboundParam().Amount) // btcTSSBalanceNew should be less than btcTSSBalanceOld as there is some loss of funds during migration // #nosec G701 e2eTest - always in range @@ -165,9 +164,9 @@ func TestMigrateTSS(r *runner.E2ERunner, _ []string) { ethTSSBalanceNew, err := r.EVMClient.BalanceAt(context.Background(), r.TSSAddress, nil) require.NoError(r, err) - r.Logger.Info(fmt.Sprintf("TSS Balance Old: %s", ethTSSBalanceOld.String())) - r.Logger.Info(fmt.Sprintf("TSS Balance New: %s", ethTSSBalanceNew.String())) - r.Logger.Info(fmt.Sprintf("Migrator amount : %s", cctxETH.GetCurrentOutboundParam().Amount.String())) + r.Logger.Info("TSS Balance Old: %s", ethTSSBalanceOld.String()) + r.Logger.Info("TSS Balance New: %s", ethTSSBalanceNew.String()) + r.Logger.Info("Migrator amount : %s", cctxETH.GetCurrentOutboundParam().Amount.String()) // ethTSSBalanceNew should be less than ethTSSBalanceOld as there is some loss of funds during migration require.Equal(r, ethTSSBalanceNew.String(), cctxETH.GetCurrentOutboundParam().Amount.String()) diff --git a/e2e/runner/accounting.go b/e2e/runner/accounting.go index 7f357a69ab..7c9a0c5746 100644 --- a/e2e/runner/accounting.go +++ b/e2e/runner/accounting.go @@ -208,7 +208,7 @@ func (r *E2ERunner) checkZetaTSSBalance() error { } zetaSupply, _ := big.NewInt(0).SetString(result.Amount.Amount, 10) if zetaLocked.Cmp(zetaSupply) < 0 { - r.Logger.Info(fmt.Sprintf("ZETA: TSS balance (%d) < ZRC20 TotalSupply (%d)", zetaLocked, zetaSupply)) + r.Logger.Info("ZETA: TSS balance (%d) < ZRC20 TotalSupply (%d)", zetaLocked, zetaSupply) } else { r.Logger.Info("ZETA: TSS balance (%d) >= ZRC20 TotalSupply (%d)", zetaLocked, zetaSupply) } diff --git a/e2e/runner/logger.go b/e2e/runner/logger.go index 683154f056..0b970ab6f3 100644 --- a/e2e/runner/logger.go +++ b/e2e/runner/logger.go @@ -57,7 +57,7 @@ func (l *Logger) Print(message string, args ...interface{}) { text := fmt.Sprintf(message, args...) // #nosec G104 - we are not using user input - l.logger.Printf(l.getPrefixWithPadding() + loggerSeparator + text + "\n") + l.logger.Print(l.getPrefixWithPadding() + loggerSeparator + text + "\n") } // PrintNoPrefix prints a message to the logger without the prefix @@ -67,7 +67,7 @@ func (l *Logger) PrintNoPrefix(message string, args ...interface{}) { text := fmt.Sprintf(message, args...) // #nosec G104 - we are not using user input - l.logger.Printf(text + "\n") + _, _ = l.logger.Print(text + "\n") } // Info prints a message to the logger if verbose is true @@ -78,7 +78,7 @@ func (l *Logger) Info(message string, args ...interface{}) { if l.verbose { text := fmt.Sprintf(message, args...) // #nosec G104 - we are not using user input - l.logger.Printf(l.getPrefixWithPadding() + loggerSeparator + "[INFO]" + text + "\n") + _, _ = l.logger.Print(l.getPrefixWithPadding() + loggerSeparator + "[INFO]" + text + "\n") } } @@ -90,11 +90,11 @@ func (l *Logger) InfoLoud(message string, args ...interface{}) { if l.verbose { text := fmt.Sprintf(message, args...) // #nosec G104 - we are not using user input - l.logger.Printf(l.getPrefixWithPadding() + loggerSeparator + "[INFO] =======================================") + l.logger.Print(l.getPrefixWithPadding() + loggerSeparator + "[INFO] =======================================") // #nosec G104 - we are not using user input - l.logger.Printf(l.getPrefixWithPadding() + loggerSeparator + "[INFO]" + text + "\n") + l.logger.Print(l.getPrefixWithPadding() + loggerSeparator + "[INFO]" + text + "\n") // #nosec G104 - we are not using user input - l.logger.Printf(l.getPrefixWithPadding() + loggerSeparator + "[INFO] =======================================") + l.logger.Print(l.getPrefixWithPadding() + loggerSeparator + "[INFO] =======================================") } } @@ -105,7 +105,7 @@ func (l *Logger) Error(message string, args ...interface{}) { text := fmt.Sprintf(message, args...) // #nosec G104 - we are not using user input - l.logger.Printf(l.getPrefixWithPadding() + loggerSeparator + "[ERROR]" + text + "\n") + l.logger.Print(l.getPrefixWithPadding() + loggerSeparator + "[ERROR]" + text + "\n") } // CCTX prints a CCTX diff --git a/e2e/runner/report.go b/e2e/runner/report.go index d80024e0dd..31cd085fc6 100644 --- a/e2e/runner/report.go +++ b/e2e/runner/report.go @@ -59,7 +59,7 @@ func (r *E2ERunner) PrintTestReports(tr TestReports) { if err != nil { r.Logger.Print("Error rendering test report: %s", err) } - r.Logger.PrintNoPrefix(table) + r.Logger.PrintNoPrefix(table, "") } // NetworkReport is a struct that contains the report for the network used after running e2e tests diff --git a/pkg/chains/conversion.go b/pkg/chains/conversion.go index 949d39ebb7..3200b76693 100644 --- a/pkg/chains/conversion.go +++ b/pkg/chains/conversion.go @@ -4,6 +4,7 @@ import ( "encoding/hex" "fmt" + "cosmossdk.io/errors" "github.com/btcsuite/btcd/chaincfg/chainhash" ethcommon "github.com/ethereum/go-ethereum/common" ) @@ -39,7 +40,7 @@ func ParseAddressAndData(message string) (ethcommon.Address, []byte, error) { data, err := hex.DecodeString(message) if err != nil { - return ethcommon.Address{}, nil, fmt.Errorf("message should be a hex encoded string: " + err.Error()) + return ethcommon.Address{}, nil, errors.Wrap(err, "message should be a hex encoded string") } if len(data) < 20 { diff --git a/x/crosschain/keeper/msg_server_add_inbound_tracker.go b/x/crosschain/keeper/msg_server_add_inbound_tracker.go index ae35036e88..13ad2bad1b 100644 --- a/x/crosschain/keeper/msg_server_add_inbound_tracker.go +++ b/x/crosschain/keeper/msg_server_add_inbound_tracker.go @@ -60,7 +60,7 @@ func (k msgServer) AddInboundTracker( func verifyProofAndInboundBody(ctx sdk.Context, k msgServer, msg *types.MsgAddInboundTracker) error { txBytes, err := k.GetLightclientKeeper().VerifyProof(ctx, msg.Proof, msg.ChainId, msg.BlockHash, msg.TxIndex) if err != nil { - return types.ErrProofVerificationFail.Wrapf(err.Error()) + return types.ErrProofVerificationFail.Wrap(err.Error()) } // get chain params and tss addresses to verify the inTx body @@ -72,14 +72,14 @@ func verifyProofAndInboundBody(ctx sdk.Context, k msgServer, msg *types.MsgAddIn BitcoinChainId: msg.ChainId, }) if err != nil { - return observertypes.ErrTssNotFound.Wrapf(err.Error()) + return observertypes.ErrTssNotFound.Wrap(err.Error()) } if tss == nil { return observertypes.ErrTssNotFound.Wrapf("tss address nil") } if err := types.VerifyInboundBody(*msg, txBytes, *chainParams, *tss); err != nil { - return types.ErrTxBodyVerificationFail.Wrapf(err.Error()) + return types.ErrTxBodyVerificationFail.Wrap(err.Error()) } return nil diff --git a/x/crosschain/keeper/msg_server_add_outbound_tracker.go b/x/crosschain/keeper/msg_server_add_outbound_tracker.go index b82fba7f79..9fdf5ac48d 100644 --- a/x/crosschain/keeper/msg_server_add_outbound_tracker.go +++ b/x/crosschain/keeper/msg_server_add_outbound_tracker.go @@ -132,7 +132,7 @@ func (k msgServer) AddOutboundTracker( func verifyProofAndOutboundBody(ctx sdk.Context, k msgServer, msg *types.MsgAddOutboundTracker) error { txBytes, err := k.lightclientKeeper.VerifyProof(ctx, msg.Proof, msg.ChainId, msg.BlockHash, msg.TxIndex) if err != nil { - return types.ErrProofVerificationFail.Wrapf(err.Error()) + return types.ErrProofVerificationFail.Wrap(err.Error()) } // get tss address @@ -145,14 +145,14 @@ func verifyProofAndOutboundBody(ctx sdk.Context, k msgServer, msg *types.MsgAddO BitcoinChainId: bitcoinChainID, }) if err != nil { - return observertypes.ErrTssNotFound.Wrapf(err.Error()) + return observertypes.ErrTssNotFound.Wrap(err.Error()) } if tss == nil { return observertypes.ErrTssNotFound.Wrapf("tss address nil") } if err := types.VerifyOutboundBody(*msg, txBytes, *tss); err != nil { - return types.ErrTxBodyVerificationFail.Wrapf(err.Error()) + return types.ErrTxBodyVerificationFail.Wrap(err.Error()) } return nil diff --git a/x/crosschain/types/message_update_rate_limiter_flags.go b/x/crosschain/types/message_update_rate_limiter_flags.go index 85d3ccf7c4..f082ad4f58 100644 --- a/x/crosschain/types/message_update_rate_limiter_flags.go +++ b/x/crosschain/types/message_update_rate_limiter_flags.go @@ -43,7 +43,7 @@ func (msg *MsgUpdateRateLimiterFlags) ValidateBasic() error { return errorsmod.Wrapf(sdkerrors.ErrInvalidAddress, "invalid creator address (%s)", err) } if err := msg.RateLimiterFlags.Validate(); err != nil { - return errorsmod.Wrapf(ErrInvalidRateLimiterFlags, err.Error()) + return errorsmod.Wrap(ErrInvalidRateLimiterFlags, err.Error()) } return nil } diff --git a/x/emissions/keeper/msg_server_update_params.go b/x/emissions/keeper/msg_server_update_params.go index 8652e4bb20..a91e1683be 100644 --- a/x/emissions/keeper/msg_server_update_params.go +++ b/x/emissions/keeper/msg_server_update_params.go @@ -28,7 +28,7 @@ func (k msgServer) UpdateParams( ctx := sdk.UnwrapSDKContext(goCtx) err := k.SetParams(ctx, msg.Params) if err != nil { - return nil, errors.Wrapf(types.ErrUnableToSetParams, err.Error()) + return nil, errors.Wrap(types.ErrUnableToSetParams, err.Error()) } return &types.MsgUpdateParamsResponse{}, nil diff --git a/x/fungible/keeper/evm.go b/x/fungible/keeper/evm.go index 7e426d4646..5f4c61bebf 100644 --- a/x/fungible/keeper/evm.go +++ b/x/fungible/keeper/evm.go @@ -573,7 +573,7 @@ func (k Keeper) BalanceOfZRC4( ) (*big.Int, error) { zrc4ABI, err := zrc20.ZRC20MetaData.GetAbi() if err != nil { - return nil, cosmoserrors.Wrapf(types.ErrABIUnpack, err.Error()) + return nil, cosmoserrors.Wrap(types.ErrABIUnpack, err.Error()) } res, err := k.CallEVM(ctx, *zrc4ABI, types.ModuleAddressEVM, contract, BigIntZero, nil, false, false, "balanceOf", account) @@ -583,7 +583,7 @@ func (k Keeper) BalanceOfZRC4( unpacked, err := zrc4ABI.Unpack("balanceOf", res.Ret) if err != nil || len(unpacked) == 0 { - return nil, cosmoserrors.Wrapf(types.ErrABIUnpack, err.Error()) + return nil, cosmoserrors.Wrap(types.ErrABIUnpack, err.Error()) } balance, ok := unpacked[0].(*big.Int) @@ -601,7 +601,7 @@ func (k Keeper) TotalSupplyZRC4( ) (*big.Int, error) { abi, err := zrc20.ZRC20MetaData.GetAbi() if err != nil { - return nil, cosmoserrors.Wrapf(types.ErrABIUnpack, err.Error()) + return nil, cosmoserrors.Wrap(types.ErrABIUnpack, err.Error()) } res, err := k.CallEVM(ctx, *abi, types.ModuleAddressEVM, contract, BigIntZero, nil, false, false, "totalSupply") if err != nil { @@ -610,12 +610,12 @@ func (k Keeper) TotalSupplyZRC4( unpacked, err := abi.Unpack("totalSupply", res.Ret) if err != nil || len(unpacked) == 0 { - return nil, cosmoserrors.Wrapf(types.ErrABIUnpack, err.Error()) + return nil, cosmoserrors.Wrap(types.ErrABIUnpack, err.Error()) } totalSupply, ok := unpacked[0].(*big.Int) if !ok { - return nil, cosmoserrors.Wrapf(types.ErrABIUnpack, "failed to unpack total supply") + return nil, cosmoserrors.Wrap(types.ErrABIUnpack, "failed to unpack total supply") } return totalSupply, nil @@ -628,7 +628,7 @@ func (k Keeper) QueryChainIDFromContract( ) (*big.Int, error) { abi, err := zrc20.ZRC20MetaData.GetAbi() if err != nil { - return nil, cosmoserrors.Wrapf(types.ErrABIUnpack, err.Error()) + return nil, cosmoserrors.Wrap(types.ErrABIUnpack, err.Error()) } res, err := k.CallEVM(ctx, *abi, types.ModuleAddressEVM, contract, BigIntZero, nil, false, false, "CHAIN_ID") if err != nil { @@ -637,12 +637,12 @@ func (k Keeper) QueryChainIDFromContract( unpacked, err := abi.Unpack("CHAIN_ID", res.Ret) if err != nil || len(unpacked) == 0 { - return nil, cosmoserrors.Wrapf(types.ErrABIUnpack, err.Error()) + return nil, cosmoserrors.Wrap(types.ErrABIUnpack, err.Error()) } chainID, ok := unpacked[0].(*big.Int) if !ok { - return nil, cosmoserrors.Wrapf(types.ErrABIUnpack, "failed to unpack chain ID") + return nil, cosmoserrors.Wrap(types.ErrABIUnpack, "failed to unpack chain ID") } return chainID, nil @@ -684,7 +684,7 @@ func (k Keeper) CallEVM( if ok { errMes = fmt.Sprintf("%s, reason: %v", errMes, revertErr.ErrorData()) } - return resp, cosmoserrors.Wrapf(err, errMes) + return resp, cosmoserrors.Wrap(err, errMes) } return resp, nil } diff --git a/x/fungible/keeper/gas_price.go b/x/fungible/keeper/gas_price.go index 600e1171a2..88ff0d6704 100644 --- a/x/fungible/keeper/gas_price.go +++ b/x/fungible/keeper/gas_price.go @@ -42,7 +42,7 @@ func (k Keeper) SetGasPrice(ctx sdk.Context, chainid *big.Int, gasPrice *big.Int gasPrice, ) if err != nil { - return 0, cosmoserrors.Wrapf(types.ErrContractCall, err.Error()) + return 0, cosmoserrors.Wrap(types.ErrContractCall, err.Error()) } if res.Failed() { return res.GasUsed, cosmoserrors.Wrapf(types.ErrContractCall, "setGasPrice tx failed") @@ -78,7 +78,7 @@ func (k Keeper) SetGasCoin(ctx sdk.Context, chainid *big.Int, address ethcommon. address, ) if err != nil { - return cosmoserrors.Wrapf(types.ErrContractCall, err.Error()) + return cosmoserrors.Wrap(types.ErrContractCall, err.Error()) } if res.Failed() { return cosmoserrors.Wrapf(types.ErrContractCall, "setGasCoinZRC20 tx failed") @@ -114,7 +114,7 @@ func (k Keeper) SetGasZetaPool(ctx sdk.Context, chainid *big.Int, pool ethcommon pool, ) if err != nil { - return cosmoserrors.Wrapf(types.ErrContractCall, err.Error()) + return cosmoserrors.Wrap(types.ErrContractCall, err.Error()) } if res.Failed() { return cosmoserrors.Wrapf(types.ErrContractCall, "setGasZetaPool tx failed") diff --git a/x/observer/types/message_update_chain_params.go b/x/observer/types/message_update_chain_params.go index 52bd399f02..e9db71920b 100644 --- a/x/observer/types/message_update_chain_params.go +++ b/x/observer/types/message_update_chain_params.go @@ -45,7 +45,7 @@ func (msg *MsgUpdateChainParams) ValidateBasic() error { } if err := ValidateChainParams(msg.ChainParams); err != nil { - return cosmoserrors.Wrapf(ErrInvalidChainParams, err.Error()) + return cosmoserrors.Wrap(ErrInvalidChainParams, err.Error()) } return nil diff --git a/x/observer/types/message_vote_block_header.go b/x/observer/types/message_vote_block_header.go index ba52975251..383ed6464c 100644 --- a/x/observer/types/message_vote_block_header.go +++ b/x/observer/types/message_vote_block_header.go @@ -55,7 +55,7 @@ func (msg *MsgVoteBlockHeader) GetSignBytes() []byte { func (msg *MsgVoteBlockHeader) ValidateBasic() error { _, err := sdk.AccAddressFromBech32(msg.Creator) if err != nil { - return cosmoserrors.Wrapf(sdkerrors.ErrInvalidAddress, err.Error()) + return cosmoserrors.Wrap(sdkerrors.ErrInvalidAddress, err.Error()) } if len(msg.BlockHash) != 32 { From 1d5cee1ce1e903df1be5ec3d6c1374ac9393a870 Mon Sep 17 00:00:00 2001 From: Lucas Bertrand Date: Tue, 20 Aug 2024 21:16:22 +0200 Subject: [PATCH 2/4] chore: fix label used in feature request template (#2761) --- .github/ISSUE_TEMPLATE/feature_request.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/ISSUE_TEMPLATE/feature_request.md b/.github/ISSUE_TEMPLATE/feature_request.md index af9bb95f4a..6010eb826e 100644 --- a/.github/ISSUE_TEMPLATE/feature_request.md +++ b/.github/ISSUE_TEMPLATE/feature_request.md @@ -2,7 +2,7 @@ name: Feature Request about: Suggest an idea for this project title: '' -labels: 'feature' +labels: 'feature:idea' assignees: '' --- From 1ad2bde785c62698bce88c0f1171c985cb1f93c1 Mon Sep 17 00:00:00 2001 From: Tanmay Date: Thu, 22 Aug 2024 11:20:09 -0400 Subject: [PATCH 3/4] test: update tss address on connectors and custody contracts (#2661) * rebase develop * rebase develop * add connectors * debug fees * debug fees * auto remove tracker * rebase develop * debug crosschainswap * debud btc swap * enable deposit btc * enable deposit btc * ad changelog * update cctx timeout * update cctx timeout * move to unreleased * move to unreleased * move to unreleased * generate * refactor migration to tss-migration * add issue to track increase in DefaultCctxTimeout * move migration test to a separate file * generate files * generate files * increase DefaultCctxTimeout * rename to admin evm * add unit test for failed to set outbound info * reduce cctx time to try getting CI to run successfully * increase cctx timeout * increase timeout for tss migration tests --- .github/workflows/e2e.yml | 1 + Makefile | 3 +- changelog.md | 1 + cmd/zetae2e/local/local.go | 45 +------------- cmd/zetae2e/local/migration.go | 4 +- cmd/zetae2e/local/post_migration.go | 58 ------------------- cmd/zetae2e/local/tss_migration.go | 43 ++++++++++++++ .../localnet/orchestrator/start-zetae2e.sh | 42 +++++++++++++- e2e/e2etests/test_crosschain_swap.go | 12 ++-- ...age_passing_external_chains_revert_fail.go | 1 - e2e/e2etests/test_migrate_tss.go | 1 - e2e/e2etests/test_zrc20_swap.go | 2 +- e2e/runner/admin_evm.go | 38 ++++++++++++ e2e/utils/zetacore.go | 5 +- .../keeper/msg_server_migrate_tss_funds.go | 6 ++ .../msg_server_migrate_tss_funds_test.go | 54 ++++++++++++----- x/crosschain/types/cmd_cctxs.go | 2 +- x/crosschain/types/errors.go | 5 +- 18 files changed, 190 insertions(+), 133 deletions(-) delete mode 100644 cmd/zetae2e/local/post_migration.go create mode 100644 cmd/zetae2e/local/tss_migration.go create mode 100644 e2e/runner/admin_evm.go diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml index f5030b1152..4390bd1ede 100644 --- a/.github/workflows/e2e.yml +++ b/.github/workflows/e2e.yml @@ -168,6 +168,7 @@ jobs: - make-target: "start-tss-migration-test" runs-on: ubuntu-20.04 run: ${{ needs.matrix-conditionals.outputs.TSS_MIGRATION_TESTS == 'true' }} + timeout-minutes: 40 - make-target: "start-solana-test" runs-on: ubuntu-20.04 run: ${{ needs.matrix-conditionals.outputs.SOLANA_TESTS == 'true' }} diff --git a/Makefile b/Makefile index 51931ea89b..6328a808a1 100644 --- a/Makefile +++ b/Makefile @@ -262,7 +262,8 @@ start-stress-test: zetanode cd contrib/localnet/ && $(DOCKER_COMPOSE) --profile stress -f docker-compose.yml up -d start-tss-migration-test: zetanode - @echo "--> Starting migration test" + @echo "--> Starting tss migration test" + export LOCALNET_MODE=tss-migrate && \ export E2E_ARGS="--test-tss-migration" && \ cd contrib/localnet/ && $(DOCKER_COMPOSE) up -d diff --git a/changelog.md b/changelog.md index d6791e1659..98a09fb06e 100644 --- a/changelog.md +++ b/changelog.md @@ -22,6 +22,7 @@ ### Tests +* [2661](https://github.com/zeta-chain/node/pull/2661) - update connector and erc20Custody addresses in tss migration e2e tests * [2726](https://github.com/zeta-chain/node/pull/2726) - add e2e tests for deposit and call, deposit and revert ### Fixes diff --git a/cmd/zetae2e/local/local.go b/cmd/zetae2e/local/local.go index 3d923b32a8..ca345a86ca 100644 --- a/cmd/zetae2e/local/local.go +++ b/cmd/zetae2e/local/local.go @@ -9,7 +9,6 @@ import ( "github.com/fatih/color" "github.com/spf13/cobra" - "github.com/stretchr/testify/require" "golang.org/x/sync/errgroup" zetae2econfig "github.com/zeta-chain/zetacore/cmd/zetae2e/config" @@ -439,7 +438,7 @@ func localE2ETest(cmd *cobra.Command, _ []string) { logger.Print("βœ… e2e tests completed in %s", time.Since(testStartTime).String()) if testTSSMigration { - runTSSMigrationTest(deployerRunner, logger, verbose, conf) + TSSMigration(deployerRunner, logger, verbose, conf) } // Verify that there are no trackers left over after tests complete if !skipTrackerCheck { @@ -496,48 +495,6 @@ func waitKeygenHeight( } } -func runTSSMigrationTest(deployerRunner *runner.E2ERunner, logger *runner.Logger, verbose bool, conf config.Config) { - migrationStartTime := time.Now() - logger.Print("🏁 starting tss migration") - - response, err := deployerRunner.CctxClient.LastZetaHeight( - deployerRunner.Ctx, - &crosschaintypes.QueryLastZetaHeightRequest{}, - ) - require.NoError(deployerRunner, err) - err = deployerRunner.ZetaTxServer.UpdateKeygen(response.Height) - require.NoError(deployerRunner, err) - - // Generate new TSS - waitKeygenHeight(deployerRunner.Ctx, deployerRunner.CctxClient, deployerRunner.ObserverClient, logger, 0) - - // migration test is a blocking thread, we cannot run other tests in parallel - // The migration test migrates funds to a new TSS and then updates the TSS address on zetacore. - // The necessary restarts are done by the zetaclient supervisor - fn := migrationTestRoutine(conf, deployerRunner, verbose, e2etests.TestMigrateTSSName) - - if err := fn(); err != nil { - logger.Print("❌ %v", err) - logger.Print("❌ tss migration failed") - os.Exit(1) - } - - logger.Print("βœ… migration completed in %s ", time.Since(migrationStartTime).String()) - logger.Print("🏁 starting post migration tests") - - tests := []string{ - e2etests.TestBitcoinWithdrawSegWitName, - e2etests.TestEtherWithdrawName, - } - fn = postMigrationTestRoutine(conf, deployerRunner, verbose, tests...) - - if err := fn(); err != nil { - logger.Print("❌ %v", err) - logger.Print("❌ post migration tests failed") - os.Exit(1) - } -} - func must[T any](v T, err error) T { return testutil.Must(v, err) } diff --git a/cmd/zetae2e/local/migration.go b/cmd/zetae2e/local/migration.go index d6fab1b709..27d9682990 100644 --- a/cmd/zetae2e/local/migration.go +++ b/cmd/zetae2e/local/migration.go @@ -11,8 +11,8 @@ import ( "github.com/zeta-chain/zetacore/e2e/runner" ) -// migrationTestRoutine runs migration related e2e tests -func migrationTestRoutine( +// migrationRoutine runs migration related e2e tests +func migrationRoutine( conf config.Config, deployerRunner *runner.E2ERunner, verbose bool, diff --git a/cmd/zetae2e/local/post_migration.go b/cmd/zetae2e/local/post_migration.go deleted file mode 100644 index f339ce0bc1..0000000000 --- a/cmd/zetae2e/local/post_migration.go +++ /dev/null @@ -1,58 +0,0 @@ -package local - -import ( - "time" - - "github.com/fatih/color" - "github.com/pkg/errors" - - "github.com/zeta-chain/zetacore/e2e/config" - "github.com/zeta-chain/zetacore/e2e/e2etests" - "github.com/zeta-chain/zetacore/e2e/runner" -) - -// postMigrationTestRoutine runs post migration tests -func postMigrationTestRoutine( - conf config.Config, - deployerRunner *runner.E2ERunner, - verbose bool, - testNames ...string, -) func() error { - return func() (err error) { - account := conf.AdditionalAccounts.UserBitcoin - // initialize runner for post migration test - postMigrationRunner, err := initTestRunner( - "postMigration", - conf, - deployerRunner, - account, - runner.NewLogger(verbose, color.FgMagenta, "postMigrationRunner"), - ) - if err != nil { - return err - } - - postMigrationRunner.Logger.Print("πŸƒ starting postMigration tests") - startTime := time.Now() - - testsToRun, err := postMigrationRunner.GetE2ETestsToRunByName( - e2etests.AllE2ETests, - testNames..., - ) - if err != nil { - return errors.Wrap(err, "postMigrationRunner tests failed") - } - - if err := postMigrationRunner.RunE2ETests(testsToRun); err != nil { - return errors.Wrap(err, "postMigrationRunner tests failed") - } - - if err := postMigrationRunner.CheckBtcTSSBalance(); err != nil { - return err - } - - postMigrationRunner.Logger.Print("🍾 PostMigration tests completed in %s", time.Since(startTime).String()) - - return err - } -} diff --git a/cmd/zetae2e/local/tss_migration.go b/cmd/zetae2e/local/tss_migration.go new file mode 100644 index 0000000000..a7acf80daa --- /dev/null +++ b/cmd/zetae2e/local/tss_migration.go @@ -0,0 +1,43 @@ +package local + +import ( + "os" + "time" + + "github.com/stretchr/testify/require" + + "github.com/zeta-chain/zetacore/e2e/config" + "github.com/zeta-chain/zetacore/e2e/e2etests" + "github.com/zeta-chain/zetacore/e2e/runner" + crosschaintypes "github.com/zeta-chain/zetacore/x/crosschain/types" +) + +func TSSMigration(deployerRunner *runner.E2ERunner, logger *runner.Logger, verbose bool, conf config.Config) { + migrationStartTime := time.Now() + logger.Print("🏁 starting tss migration") + + response, err := deployerRunner.CctxClient.LastZetaHeight( + deployerRunner.Ctx, + &crosschaintypes.QueryLastZetaHeightRequest{}, + ) + require.NoError(deployerRunner, err) + err = deployerRunner.ZetaTxServer.UpdateKeygen(response.Height) + require.NoError(deployerRunner, err) + + // Generate new TSS + waitKeygenHeight(deployerRunner.Ctx, deployerRunner.CctxClient, deployerRunner.ObserverClient, logger, 0) + + // Run migration + // migrationRoutine runs migration e2e test , which migrates funds from the older TSS to the new one + // The zetaclient restarts required for this process are managed by the background workers in zetaclient (TSSListener) + fn := migrationRoutine(conf, deployerRunner, verbose, e2etests.TestMigrateTSSName) + + if err := fn(); err != nil { + logger.Print("❌ %v", err) + logger.Print("❌ tss migration failed") + os.Exit(1) + } + deployerRunner.UpdateTssAddressForConnector() + deployerRunner.UpdateTssAddressForErc20custody() + logger.Print("βœ… migration completed in %s ", time.Since(migrationStartTime).String()) +} diff --git a/contrib/localnet/orchestrator/start-zetae2e.sh b/contrib/localnet/orchestrator/start-zetae2e.sh index 4ee8192c6d..c3adda9753 100644 --- a/contrib/localnet/orchestrator/start-zetae2e.sh +++ b/contrib/localnet/orchestrator/start-zetae2e.sh @@ -107,6 +107,45 @@ fi ### Run zetae2e command depending on the option passed +# Mode migrate is used to run the e2e tests before and after the TSS migration +# It runs the e2e tests with the migrate flag which triggers a TSS migration at the end of the tests. Once the migrationis done the first e2e test is complete +# The second e2e test is run after the migration to ensure the network is still working as expected with the new tss address +if [ "$LOCALNET_MODE" == "tss-migrate" ]; then + if [[ ! -f deployed.yml ]]; then + zetae2e local $E2E_ARGS --setup-only --config config.yml --config-out deployed.yml --skip-header-proof + if [ $? -ne 0 ]; then + echo "e2e setup failed" + exit 1 + fi + else + echo "skipping e2e setup because it has already been completed" + fi + + echo "running e2e test before migrating TSS" + zetae2e local $E2E_ARGS --skip-setup --config deployed.yml --skip-header-proof + if [ $? -ne 0 ]; then + echo "first e2e failed" + exit 1 + fi + + echo "waiting 10 seconds for node to restart" + sleep 10 + + zetae2e local --skip-setup --config deployed.yml --skip-bitcoin-setup --light --skip-header-proof + ZETAE2E_EXIT_CODE=$? + if [ $ZETAE2E_EXIT_CODE -eq 0 ]; then + echo "E2E passed after migration" + exit 0 + else + echo "E2E failed after migration" + exit 1 + fi +fi + + +# Mode upgrade is used to run the e2e tests before and after the upgrade +# It runs the e2e tests , waits for the upgrade height to be reached, and then runs the e2e tests again once the ungrade is done. +# The second e2e test is run after the upgrade to ensure the network is still working as expected with the new version if [ "$LOCALNET_MODE" == "upgrade" ]; then # Run the e2e tests, then restart zetaclientd at upgrade height and run the e2e tests again @@ -185,8 +224,7 @@ if [ "$LOCALNET_MODE" == "upgrade" ]; then fi else - - # Run the e2e tests normally + # If no mode is passed, run the e2e tests normally echo "running e2e setup..." if [[ ! -f deployed.yml ]]; then diff --git a/e2e/e2etests/test_crosschain_swap.go b/e2e/e2etests/test_crosschain_swap.go index 798958a652..f41d3d8372 100644 --- a/e2e/e2etests/test_crosschain_swap.go +++ b/e2e/e2etests/test_crosschain_swap.go @@ -15,6 +15,8 @@ import ( ) func TestCrosschainSwap(r *runner.E2ERunner, _ []string) { + stop := r.MineBlocksIfLocalBitcoin() + defer stop() r.ZEVMAuth.GasLimit = 10000000 // TODO: move into setup and skip it if already initialized @@ -23,7 +25,7 @@ func TestCrosschainSwap(r *runner.E2ERunner, _ []string) { // if the tx fails due to already initialized, it will be ignored _, err := r.UniswapV2Factory.CreatePair(r.ZEVMAuth, r.ERC20ZRC20Addr, r.BTCZRC20Addr) if err != nil { - r.Logger.Print("ℹ️create pair error") + r.Logger.Print("ℹ️ create pair error") } txERC20ZRC20Approve, err := r.ERC20ZRC20.Approve(r.ZEVMAuth, r.UniswapV2RouterAddr, big.NewInt(1e18)) @@ -90,10 +92,6 @@ func TestCrosschainSwap(r *runner.E2ERunner, _ []string) { _, err = r.GenerateToAddressIfLocalBitcoin(10, r.BTCDeployerAddress) require.NoError(r, err) - // mine blocks if testing on regnet - stop := r.MineBlocksIfLocalBitcoin() - defer stop() - // cctx1 index acts like the inboundHash for the second cctx (the one that withdraws BTC) cctx2 := utils.WaitCctxMinedByInboundHash(r.Ctx, cctx1.Index, r.CctxClient, r.Logger, r.CctxTimeout) @@ -145,7 +143,9 @@ func TestCrosschainSwap(r *runner.E2ERunner, _ []string) { r.Logger.Info("memo length %d", len(memo)) amount := 0.1 - txid, err := r.SendToTSSFromDeployerWithMemo(amount, utxos[1:2], memo) + utxos, err = r.ListDeployerUTXOs() + require.NoError(r, err) + txid, err := r.SendToTSSFromDeployerWithMemo(amount, utxos[0:1], memo) require.NoError(r, err) cctx := utils.WaitCctxMinedByInboundHash(r.Ctx, txid.String(), r.CctxClient, r.Logger, r.CctxTimeout) diff --git a/e2e/e2etests/test_message_passing_external_chains_revert_fail.go b/e2e/e2etests/test_message_passing_external_chains_revert_fail.go index 8504aaca56..47957e5678 100644 --- a/e2e/e2etests/test_message_passing_external_chains_revert_fail.go +++ b/e2e/e2etests/test_message_passing_external_chains_revert_fail.go @@ -65,7 +65,6 @@ func TestMessagePassingRevertFailExternalChains(r *runner.E2ERunner, args []stri r.Logger.Info(" Zeta Value: %d", sentLog.ZetaValueAndGas) } } - // expect revert tx to fail cctx := utils.WaitCctxMinedByInboundHash(r.Ctx, receipt.TxHash.String(), r.CctxClient, r.Logger, r.CctxTimeout) receipt, err = r.EVMClient.TransactionReceipt(r.Ctx, ethcommon.HexToHash(cctx.GetCurrentOutboundParam().Hash)) diff --git a/e2e/e2etests/test_migrate_tss.go b/e2e/e2etests/test_migrate_tss.go index c1bb79c062..cc7286360b 100644 --- a/e2e/e2etests/test_migrate_tss.go +++ b/e2e/e2etests/test_migrate_tss.go @@ -159,7 +159,6 @@ func TestMigrateTSS(r *runner.E2ERunner, _ []string) { require.LessOrEqual(r, btcTSSBalanceNew*1e8, btcTSSBalanceOld*1e8) // ETH - r.TSSAddress = common.HexToAddress(newTss.Eth) ethTSSBalanceNew, err := r.EVMClient.BalanceAt(context.Background(), r.TSSAddress, nil) require.NoError(r, err) diff --git a/e2e/e2etests/test_zrc20_swap.go b/e2e/e2etests/test_zrc20_swap.go index 14febbe800..3008904306 100644 --- a/e2e/e2etests/test_zrc20_swap.go +++ b/e2e/e2etests/test_zrc20_swap.go @@ -19,7 +19,7 @@ func TestZRC20Swap(r *runner.E2ERunner, _ []string) { // if the tx fails due to already initialized, it will be ignored tx, err := r.UniswapV2Factory.CreatePair(r.ZEVMAuth, r.ERC20ZRC20Addr, r.ETHZRC20Addr) if err != nil { - r.Logger.Print("ℹ️create pair error") + r.Logger.Print("ℹ️ create pair error") } else { utils.MustWaitForTxReceipt(r.Ctx, r.ZEVMClient, tx, r.Logger, r.ReceiptTimeout) } diff --git a/e2e/runner/admin_evm.go b/e2e/runner/admin_evm.go new file mode 100644 index 0000000000..8b357a01f5 --- /dev/null +++ b/e2e/runner/admin_evm.go @@ -0,0 +1,38 @@ +package runner + +import ( + "fmt" + + "github.com/ethereum/go-ethereum/accounts/abi/bind" + "github.com/stretchr/testify/require" + + "github.com/zeta-chain/zetacore/e2e/utils" +) + +func (r *E2ERunner) UpdateTssAddressForConnector() { + require.NoError(r, r.SetTSSAddresses()) + + tx, err := r.ConnectorEth.UpdateTssAddress(r.EVMAuth, r.TSSAddress) + require.NoError(r, err) + r.Logger.Info(fmt.Sprintf("TSS Address Update Tx: %s", tx.Hash().String())) + receipt := utils.MustWaitForTxReceipt(r.Ctx, r.EVMClient, tx, r.Logger, r.ReceiptTimeout) + utils.RequireTxSuccessful(r, receipt) + + tssAddressOnConnector, err := r.ConnectorEth.TssAddress(&bind.CallOpts{Context: r.Ctx}) + require.NoError(r, err) + require.Equal(r, r.TSSAddress, tssAddressOnConnector) +} + +func (r *E2ERunner) UpdateTssAddressForErc20custody() { + require.NoError(r, r.SetTSSAddresses()) + + tx, err := r.ERC20Custody.UpdateTSSAddress(r.EVMAuth, r.TSSAddress) + require.NoError(r, err) + r.Logger.Info(fmt.Sprintf("TSS ERC20 Address Update Tx: %s", tx.Hash().String())) + receipt := utils.MustWaitForTxReceipt(r.Ctx, r.EVMClient, tx, r.Logger, r.ReceiptTimeout) + utils.RequireTxSuccessful(r, receipt) + + tssAddressOnCustody, err := r.ERC20Custody.TSSAddress(&bind.CallOpts{Context: r.Ctx}) + require.NoError(r, err) + require.Equal(r, r.TSSAddress, tssAddressOnCustody) +} diff --git a/e2e/utils/zetacore.go b/e2e/utils/zetacore.go index 9122860d75..b69d9fbc32 100644 --- a/e2e/utils/zetacore.go +++ b/e2e/utils/zetacore.go @@ -19,8 +19,11 @@ const ( EmergencyPolicyName = "emergency" AdminPolicyName = "admin" OperationalPolicyName = "operational" + // The timeout was increased from 4 to 6 , which allows for a higher success in test runs + // However this needs to be researched as to why the increase in timeout was needed. + // https://github.com/zeta-chain/node/issues/2690 - DefaultCctxTimeout = 6 * time.Minute + DefaultCctxTimeout = 8 * time.Minute ) // WaitCctxMinedByInboundHash waits until cctx is mined; returns the cctxIndex (the last one) diff --git a/x/crosschain/keeper/msg_server_migrate_tss_funds.go b/x/crosschain/keeper/msg_server_migrate_tss_funds.go index 7d48b48cdb..f1f625571a 100644 --- a/x/crosschain/keeper/msg_server_migrate_tss_funds.go +++ b/x/crosschain/keeper/msg_server_migrate_tss_funds.go @@ -106,6 +106,12 @@ func (k Keeper) initiateMigrateTSSFundsCCTX( return err } + // Set the CCTX and the nonce for the outbound migration + err = k.SetObserverOutboundInfo(ctx, chainID, &cctx) + if err != nil { + return errorsmod.Wrap(types.ErrUnableToSetOutboundInfo, err.Error()) + } + // The migrate funds can be run again to update the migration cctx index if the migration fails // This should be used after carefully calculating the amount again existingMigrationInfo, found := k.zetaObserverKeeper.GetFundMigrator(ctx, chainID) diff --git a/x/crosschain/keeper/msg_server_migrate_tss_funds_test.go b/x/crosschain/keeper/msg_server_migrate_tss_funds_test.go index a715fcf4e6..93e9a8fb98 100644 --- a/x/crosschain/keeper/msg_server_migrate_tss_funds_test.go +++ b/x/crosschain/keeper/msg_server_migrate_tss_funds_test.go @@ -26,6 +26,7 @@ func setupTssMigrationParams( amount sdkmath.Uint, setNewTss bool, setCurrentTSS bool, + setChainNonces bool, ) (string, string) { zk.ObserverKeeper.SetCrosschainFlags(ctx, observertypes.CrosschainFlags{ IsInboundEnabled: false, @@ -70,10 +71,12 @@ func setupTssMigrationParams( PriorityFees: []uint64{100, 300, 200}, MedianIndex: 1, }) - k.GetObserverKeeper().SetChainNonces(ctx, observertypes.ChainNonces{ - ChainId: chain.ChainId, - Nonce: 1, - }) + if setChainNonces { + k.GetObserverKeeper().SetChainNonces(ctx, observertypes.ChainNonces{ + ChainId: chain.ChainId, + Nonce: 1, + }) + } indexString := crosschaintypes.GetTssMigrationCCTXIndexString( currentTss.TssPubkey, newTss.TssPubkey, @@ -98,7 +101,7 @@ func TestKeeper_MigrateTSSFundsForChain(t *testing.T) { msgServer := keeper.NewMsgServerImpl(*k) - indexString, _ := setupTssMigrationParams(zk, k, ctx, chain, amount, true, true) + indexString, _ := setupTssMigrationParams(zk, k, ctx, chain, amount, true, true, true) gp, priorityFee, found := k.GetMedianGasValues(ctx, chain.ChainId) require.True(t, found) msg := crosschaintypes.MsgMigrateTssFunds{ @@ -134,7 +137,7 @@ func TestKeeper_MigrateTSSFundsForChain(t *testing.T) { authorityMock := keepertest.GetCrosschainAuthorityMock(t, k) msgServer := keeper.NewMsgServerImpl(*k) - indexString, _ := setupTssMigrationParams(zk, k, ctx, chain, amount, true, true) + indexString, _ := setupTssMigrationParams(zk, k, ctx, chain, amount, true, true, true) gp, priorityFee, found := k.GetMedianGasValues(ctx, chain.ChainId) require.True(t, found) @@ -373,7 +376,7 @@ func TestMsgServer_MigrateTssFunds(t *testing.T) { msgServer := keeper.NewMsgServerImpl(*k) - indexString, _ := setupTssMigrationParams(zk, k, ctx, chain, amount, true, true) + indexString, _ := setupTssMigrationParams(zk, k, ctx, chain, amount, true, true, true) msg := crosschaintypes.MsgMigrateTssFunds{ Creator: admin, @@ -405,7 +408,7 @@ func TestMsgServer_MigrateTssFunds(t *testing.T) { authorityMock := keepertest.GetCrosschainAuthorityMock(t, k) msgServer := keeper.NewMsgServerImpl(*k) - indexString, _ := setupTssMigrationParams(zk, k, ctx, chain, amount, true, true) + indexString, _ := setupTssMigrationParams(zk, k, ctx, chain, amount, true, true, true) msg := crosschaintypes.MsgMigrateTssFunds{ Creator: admin, @@ -433,7 +436,7 @@ func TestMsgServer_MigrateTssFunds(t *testing.T) { authorityMock := keepertest.GetCrosschainAuthorityMock(t, k) msgServer := keeper.NewMsgServerImpl(*k) - indexString, _ := setupTssMigrationParams(zk, k, ctx, chain, amount, false, true) + indexString, _ := setupTssMigrationParams(zk, k, ctx, chain, amount, false, true, true) msg := crosschaintypes.MsgMigrateTssFunds{ Creator: admin, @@ -460,7 +463,7 @@ func TestMsgServer_MigrateTssFunds(t *testing.T) { authorityMock := keepertest.GetCrosschainAuthorityMock(t, k) msgServer := keeper.NewMsgServerImpl(*k) - indexString, tssPubkey := setupTssMigrationParams(zk, k, ctx, chain, amount, true, true) + indexString, tssPubkey := setupTssMigrationParams(zk, k, ctx, chain, amount, true, true, true) k.GetObserverKeeper().SetPendingNonces(ctx, observertypes.PendingNonces{ NonceLow: 1, NonceHigh: 10, @@ -483,7 +486,7 @@ func TestMsgServer_MigrateTssFunds(t *testing.T) { require.False(t, found) }) - t.Run("unable to migrate funds when a pending cctx is presnt in migration info", func(t *testing.T) { + t.Run("unable to migrate funds when a pending cctx is present in migration info", func(t *testing.T) { k, ctx, _, zk := keepertest.CrosschainKeeperWithMocks(t, keepertest.CrosschainMockOptions{ UseAuthorityMock: true, }) @@ -494,7 +497,7 @@ func TestMsgServer_MigrateTssFunds(t *testing.T) { authorityMock := keepertest.GetCrosschainAuthorityMock(t, k) msgServer := keeper.NewMsgServerImpl(*k) - indexString, tssPubkey := setupTssMigrationParams(zk, k, ctx, chain, amount, true, true) + indexString, tssPubkey := setupTssMigrationParams(zk, k, ctx, chain, amount, true, true, true) k.GetObserverKeeper().SetPendingNonces(ctx, observertypes.PendingNonces{ NonceLow: 1, NonceHigh: 1, @@ -541,7 +544,7 @@ func TestMsgServer_MigrateTssFunds(t *testing.T) { msgServer := keeper.NewMsgServerImpl(*k) - indexString, _ := setupTssMigrationParams(zk, k, ctx, chain, amount, false, false) + indexString, _ := setupTssMigrationParams(zk, k, ctx, chain, amount, false, false, true) currentTss, found := k.GetObserverKeeper().GetTSS(ctx) require.True(t, found) newTss := sample.Tss() @@ -565,4 +568,29 @@ func TestMsgServer_MigrateTssFunds(t *testing.T) { require.False(t, found) }, ) + + t.Run("unable to process migration if SetObserverOutboundInfo fails", func(t *testing.T) { + k, ctx, _, zk := keepertest.CrosschainKeeperWithMocks(t, keepertest.CrosschainMockOptions{ + UseAuthorityMock: true, + }) + + admin := sample.AccAddress() + chain := getValidEthChain() + amount := sdkmath.NewUintFromString("10000000000000000000") + authorityMock := keepertest.GetCrosschainAuthorityMock(t, k) + + msgServer := keeper.NewMsgServerImpl(*k) + + _, _ = setupTssMigrationParams(zk, k, ctx, chain, amount, true, true, false) + msg := crosschaintypes.MsgMigrateTssFunds{ + Creator: admin, + ChainId: chain.ChainId, + Amount: amount, + } + keepertest.MockCheckAuthorization(&authorityMock.Mock, &msg, nil) + keepertest.MockGetChainListEmpty(&authorityMock.Mock) + + _, err := msgServer.MigrateTssFunds(ctx, &msg) + require.ErrorContains(t, err, crosschaintypes.ErrUnableToSetOutboundInfo.Error()) + }) } diff --git a/x/crosschain/types/cmd_cctxs.go b/x/crosschain/types/cmd_cctxs.go index 3f416c6d7d..5ba7495d22 100644 --- a/x/crosschain/types/cmd_cctxs.go +++ b/x/crosschain/types/cmd_cctxs.go @@ -250,7 +250,7 @@ func MigrateFundCmdCCTX( gasLimit, gasPrice, priorityFee.MulUint64(2).String(), - newTSSPubKey, + currentTSSPubKey, ), nil } diff --git a/x/crosschain/types/errors.go b/x/crosschain/types/errors.go index 279442d2c7..40182fe313 100644 --- a/x/crosschain/types/errors.go +++ b/x/crosschain/types/errors.go @@ -55,6 +55,7 @@ var ( 1156, "migration tx from an old tss address detected", ) - ErrValidatingInbound = errorsmod.Register(ModuleName, 1157, "unable to validate inbound") - ErrInvalidGasLimit = errorsmod.Register(ModuleName, 1158, "invalid gas limit") + 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") ) From c3d19297b4d345e9d7ff746f510a0e3e25eb6355 Mon Sep 17 00:00:00 2001 From: Tanmay Date: Mon, 26 Aug 2024 05:04:22 -0400 Subject: [PATCH 4/4] fix: modify message vote tss to allow operators to vote on ballots assosiated with discarded keygen . (#2674) * debud btc swap * add unit test for two ballots * fix tests * add more tests * add comments for tests * generate files * add response status to MsgVoteTSSResponse * fix lint errors --- changelog.md | 1 + x/observer/keeper/msg_server_vote_tss.go | 29 +- x/observer/keeper/msg_server_vote_tss_test.go | 524 +++++++++++++++++- 3 files changed, 525 insertions(+), 29 deletions(-) diff --git a/changelog.md b/changelog.md index 98a09fb06e..124ab31df2 100644 --- a/changelog.md +++ b/changelog.md @@ -28,6 +28,7 @@ ### Fixes * [2654](https://github.com/zeta-chain/node/pull/2654) - add validation for authorization list in when validating genesis state for authorization module +* [2674](https://github.com/zeta-chain/node/pull/2674) - allow operators to vote on ballots assosiated with discarded keygen without affecting the status of the current keygen. * [2672](https://github.com/zeta-chain/node/pull/2672) - check observer set for duplicates when adding a new observer or updating an existing one ## v19.0.0 diff --git a/x/observer/keeper/msg_server_vote_tss.go b/x/observer/keeper/msg_server_vote_tss.go index 617411d575..9455dfa25b 100644 --- a/x/observer/keeper/msg_server_vote_tss.go +++ b/x/observer/keeper/msg_server_vote_tss.go @@ -42,11 +42,6 @@ func (k msgServer) VoteTSS(goCtx context.Context, msg *types.MsgVoteTSS) (*types return &types.MsgVoteTSSResponse{}, errorsmod.Wrap(types.ErrKeygenNotFound, voteTSSid) } - // Use a separate transaction to update keygen status to pending when trying to change the TSS address. - if keygen.Status == types.KeygenStatus_KeyGenSuccess { - return &types.MsgVoteTSSResponse{}, errorsmod.Wrap(types.ErrKeygenCompleted, voteTSSid) - } - // GetBallot checks against the supported chains list before querying for Ballot. ballotCreated := false index := msg.Digest() @@ -94,6 +89,30 @@ func (k msgServer) VoteTSS(goCtx context.Context, msg *types.MsgVoteTSS) (*types }, nil } + // The ballot is finalized, we check if this is the correct ballot for updating the TSS + // The requirements are + // 1. The keygen is still pending + // 2. The keygen block number matches the ballot block number ,which makes sure this the correct ballot for the current keygen + + // Return without an error so the vote is added to the ballot + if keygen.Status != types.KeygenStatus_PendingKeygen { + // The response is used for testing only.Setting false for keygen success as the keygen has already been finalized and it doesnt matter what the final status is.We are just asserting that the keygen was previously finalized and is not in pending status. + return &types.MsgVoteTSSResponse{ + VoteFinalized: isFinalized, + BallotCreated: ballotCreated, + KeygenSuccess: false, + }, nil + } + + // For cases when an observer tries to vote for an older pending ballot, associated with a keygen that was discarded , we would return at this check while still adding the vote to the ballot + if msg.KeygenZetaHeight != keygen.BlockNumber { + return &types.MsgVoteTSSResponse{ + VoteFinalized: isFinalized, + BallotCreated: ballotCreated, + KeygenSuccess: false, + }, nil + } + // Set TSS only on success, set keygen either way. // Keygen block can be updated using a policy transaction if keygen fails. keygenSuccess := false diff --git a/x/observer/keeper/msg_server_vote_tss_test.go b/x/observer/keeper/msg_server_vote_tss_test.go index f44ceaa07d..2d9466e89c 100644 --- a/x/observer/keeper/msg_server_vote_tss_test.go +++ b/x/observer/keeper/msg_server_vote_tss_test.go @@ -1,6 +1,7 @@ package keeper_test import ( + "fmt" "math" "testing" @@ -16,19 +17,24 @@ import ( func TestMsgServer_VoteTSS(t *testing.T) { t.Run("fail if node account not found", func(t *testing.T) { + // ARRANGE k, ctx, _, _ := keepertest.ObserverKeeper(t) srv := keeper.NewMsgServerImpl(*k) + // ACT _, err := srv.VoteTSS(ctx, &types.MsgVoteTSS{ Creator: sample.AccAddress(), TssPubkey: sample.Tss().TssPubkey, KeygenZetaHeight: 42, Status: chains.ReceiveStatus_success, }) + + // ASSERT require.ErrorIs(t, err, sdkerrors.ErrorInvalidSigner) }) t.Run("fail if keygen is not found", func(t *testing.T) { + // ARRANGE k, ctx, _, _ := keepertest.ObserverKeeper(t) srv := keeper.NewMsgServerImpl(*k) @@ -36,16 +42,20 @@ func TestMsgServer_VoteTSS(t *testing.T) { nodeAcc := sample.NodeAccount() k.SetNodeAccount(ctx, *nodeAcc) + // ACT _, err := srv.VoteTSS(ctx, &types.MsgVoteTSS{ Creator: nodeAcc.Operator, TssPubkey: sample.Tss().TssPubkey, KeygenZetaHeight: 42, Status: chains.ReceiveStatus_success, }) + + // ASSERT require.ErrorIs(t, err, types.ErrKeygenNotFound) }) t.Run("fail if keygen already completed ", func(t *testing.T) { + // ARRANGE k, ctx, _, _ := keepertest.ObserverKeeper(t) srv := keeper.NewMsgServerImpl(*k) @@ -53,30 +63,43 @@ func TestMsgServer_VoteTSS(t *testing.T) { nodeAcc := sample.NodeAccount() keygen := sample.Keygen(t) keygen.Status = types.KeygenStatus_KeyGenSuccess + keygen.BlockNumber = 42 k.SetNodeAccount(ctx, *nodeAcc) k.SetKeygen(ctx, *keygen) + // ACT _, err := srv.VoteTSS(ctx, &types.MsgVoteTSS{ Creator: nodeAcc.Operator, TssPubkey: sample.Tss().TssPubkey, KeygenZetaHeight: 42, Status: chains.ReceiveStatus_success, }) - require.ErrorIs(t, err, types.ErrKeygenCompleted) + + // ASSERT + // keygen is already completed, but the vote can still be added if the operator has not voted yet + require.NoError(t, err) + ballot, found := k.GetBallot(ctx, fmt.Sprintf("%d-%s", 42, "tss-keygen")) + require.True(t, found) + require.EqualValues(t, types.BallotStatus_BallotFinalized_SuccessObservation, ballot.BallotStatus) + require.True(t, ballot.HasVoted(nodeAcc.Operator)) }) t.Run("can create a new ballot, vote success and finalize", func(t *testing.T) { + // ARRANGE k, ctx, _, _ := keepertest.ObserverKeeper(t) - ctx = ctx.WithBlockHeight(42) + finalizingHeight := int64(55) + ctx = ctx.WithBlockHeight(finalizingHeight) srv := keeper.NewMsgServerImpl(*k) // setup state nodeAcc := sample.NodeAccount() keygen := sample.Keygen(t) keygen.Status = types.KeygenStatus_PendingKeygen + keygen.BlockNumber = 42 k.SetNodeAccount(ctx, *nodeAcc) k.SetKeygen(ctx, *keygen) + // ACT // there is a single node account, so the ballot will be created and finalized in a single vote res, err := srv.VoteTSS(ctx, &types.MsgVoteTSS{ Creator: nodeAcc.Operator, @@ -84,8 +107,9 @@ func TestMsgServer_VoteTSS(t *testing.T) { KeygenZetaHeight: 42, Status: chains.ReceiveStatus_success, }) - require.NoError(t, err) + // ASSERT + require.NoError(t, err) // check response require.True(t, res.BallotCreated) require.True(t, res.VoteFinalized) @@ -95,10 +119,17 @@ func TestMsgServer_VoteTSS(t *testing.T) { newKeygen, found := k.GetKeygen(ctx) require.True(t, found) require.EqualValues(t, types.KeygenStatus_KeyGenSuccess, newKeygen.Status) - require.EqualValues(t, ctx.BlockHeight(), newKeygen.BlockNumber) + require.EqualValues(t, finalizingHeight, newKeygen.BlockNumber) + + // check tss updated + tss, found := k.GetTSS(ctx) + require.True(t, found) + require.Equal(t, tss.KeyGenZetaHeight, int64(42)) + require.Equal(t, tss.FinalizedZetaHeight, finalizingHeight) }) t.Run("can create a new ballot, vote failure and finalize", func(t *testing.T) { + // ARRANGE k, ctx, _, _ := keepertest.ObserverKeeper(t) ctx = ctx.WithBlockHeight(42) srv := keeper.NewMsgServerImpl(*k) @@ -106,10 +137,12 @@ func TestMsgServer_VoteTSS(t *testing.T) { // setup state nodeAcc := sample.NodeAccount() keygen := sample.Keygen(t) + keygen.BlockNumber = 42 keygen.Status = types.KeygenStatus_PendingKeygen k.SetNodeAccount(ctx, *nodeAcc) k.SetKeygen(ctx, *keygen) + // ACT // there is a single node account, so the ballot will be created and finalized in a single vote res, err := srv.VoteTSS(ctx, &types.MsgVoteTSS{ Creator: nodeAcc.Operator, @@ -117,8 +150,9 @@ func TestMsgServer_VoteTSS(t *testing.T) { KeygenZetaHeight: 42, Status: chains.ReceiveStatus_failed, }) - require.NoError(t, err) + // ASSERT + require.NoError(t, err) // check response require.True(t, res.BallotCreated) require.True(t, res.VoteFinalized) @@ -131,7 +165,135 @@ func TestMsgServer_VoteTSS(t *testing.T) { require.EqualValues(t, math.MaxInt64, newKeygen.BlockNumber) }) - t.Run("can create a new ballot, vote without finalizing, then add vote and finalizing", func(t *testing.T) { + t.Run( + "can create a new ballot, vote without finalizing, then add final vote to update keygen and set tss", + func(t *testing.T) { + // ARRANGE + k, ctx, _, _ := keepertest.ObserverKeeper(t) + finalizingHeight := int64(55) + ctx = ctx.WithBlockHeight(finalizingHeight) + srv := keeper.NewMsgServerImpl(*k) + + // setup state with 3 node accounts + nodeAcc1 := sample.NodeAccount() + nodeAcc2 := sample.NodeAccount() + nodeAcc3 := sample.NodeAccount() + keygen := sample.Keygen(t) + keygen.BlockNumber = 42 + keygen.Status = types.KeygenStatus_PendingKeygen + tss := sample.Tss() + k.SetNodeAccount(ctx, *nodeAcc1) + k.SetNodeAccount(ctx, *nodeAcc2) + k.SetNodeAccount(ctx, *nodeAcc3) + k.SetKeygen(ctx, *keygen) + + // ACT + // 1st vote: created ballot, but not finalized + res, err := srv.VoteTSS(ctx, &types.MsgVoteTSS{ + Creator: nodeAcc1.Operator, + TssPubkey: tss.TssPubkey, + KeygenZetaHeight: 42, + Status: chains.ReceiveStatus_success, + }) + require.NoError(t, err) + + // check response + require.True(t, res.BallotCreated) + require.False(t, res.VoteFinalized) + require.False(t, res.KeygenSuccess) + + // check keygen not updated + newKeygen, found := k.GetKeygen(ctx) + require.True(t, found) + require.EqualValues(t, types.KeygenStatus_PendingKeygen, newKeygen.Status) + + // 2nd vote: already created ballot, and not finalized + res, err = srv.VoteTSS(ctx, &types.MsgVoteTSS{ + Creator: nodeAcc2.Operator, + TssPubkey: tss.TssPubkey, + KeygenZetaHeight: 42, + Status: chains.ReceiveStatus_success, + }) + require.NoError(t, err) + + // check response + require.False(t, res.BallotCreated) + require.False(t, res.VoteFinalized) + require.False(t, res.KeygenSuccess) + + // check keygen not updated + newKeygen, found = k.GetKeygen(ctx) + require.True(t, found) + require.EqualValues(t, types.KeygenStatus_PendingKeygen, newKeygen.Status) + + // 3rd vote: finalize the ballot + res, err = srv.VoteTSS(ctx, &types.MsgVoteTSS{ + Creator: nodeAcc3.Operator, + TssPubkey: tss.TssPubkey, + KeygenZetaHeight: 42, + Status: chains.ReceiveStatus_success, + }) + require.NoError(t, err) + + // ASSERT + // check response + require.False(t, res.BallotCreated) + require.True(t, res.VoteFinalized) + require.True(t, res.KeygenSuccess) + + // check keygen updated + newKeygen, found = k.GetKeygen(ctx) + require.True(t, found) + require.EqualValues(t, types.KeygenStatus_KeyGenSuccess, newKeygen.Status) + require.EqualValues(t, ctx.BlockHeight(), newKeygen.BlockNumber) + + // check tss updated + tss, found = k.GetTSS(ctx) + require.True(t, found) + require.Equal(t, tss.KeyGenZetaHeight, int64(42)) + require.Equal(t, tss.FinalizedZetaHeight, finalizingHeight) + }, + ) + + t.Run("fail if voting fails", func(t *testing.T) { + // ARRANGE + k, ctx, _, _ := keepertest.ObserverKeeper(t) + ctx = ctx.WithBlockHeight(42) + srv := keeper.NewMsgServerImpl(*k) + + // setup state with two node accounts to not finalize the ballot + nodeAcc := sample.NodeAccount() + keygen := sample.Keygen(t) + keygen.Status = types.KeygenStatus_PendingKeygen + k.SetNodeAccount(ctx, *nodeAcc) + k.SetNodeAccount(ctx, *sample.NodeAccount()) + k.SetKeygen(ctx, *keygen) + + // add a first vote + res, err := srv.VoteTSS(ctx, &types.MsgVoteTSS{ + Creator: nodeAcc.Operator, + TssPubkey: sample.Tss().TssPubkey, + KeygenZetaHeight: 42, + Status: chains.ReceiveStatus_success, + }) + require.NoError(t, err) + require.False(t, res.VoteFinalized) + + // ACT + // vote again: voting should fail + _, err = srv.VoteTSS(ctx, &types.MsgVoteTSS{ + Creator: nodeAcc.Operator, + TssPubkey: sample.Tss().TssPubkey, + KeygenZetaHeight: 42, + Status: chains.ReceiveStatus_success, + }) + + // ASSERT + require.ErrorIs(t, err, types.ErrUnableToAddVote) + }) + + t.Run("can create a new ballot, without finalizing the older and then finalize older ballot", func(t *testing.T) { + // ARRANGE k, ctx, _, _ := keepertest.ObserverKeeper(t) ctx = ctx.WithBlockHeight(42) srv := keeper.NewMsgServerImpl(*k) @@ -148,6 +310,7 @@ func TestMsgServer_VoteTSS(t *testing.T) { k.SetNodeAccount(ctx, *nodeAcc3) k.SetKeygen(ctx, *keygen) + // ACT // 1st vote: created ballot, but not finalized res, err := srv.VoteTSS(ctx, &types.MsgVoteTSS{ Creator: nodeAcc1.Operator, @@ -186,7 +349,51 @@ func TestMsgServer_VoteTSS(t *testing.T) { require.True(t, found) require.EqualValues(t, types.KeygenStatus_PendingKeygen, newKeygen.Status) - // 3rd vote: finalize the ballot + keygen.Status = types.KeygenStatus_PendingKeygen + keygen.BlockNumber = 52 + k.SetKeygen(ctx, *keygen) + + // Start voting on a new ballot + tss2 := sample.Tss() + // 1st Vote on new ballot (acc1) + res, err = srv.VoteTSS(ctx, &types.MsgVoteTSS{ + Creator: nodeAcc1.Operator, + TssPubkey: tss2.TssPubkey, + KeygenZetaHeight: 52, + Status: chains.ReceiveStatus_success, + }) + require.NoError(t, err) + + // check response + require.True(t, res.BallotCreated) + require.False(t, res.VoteFinalized) + require.False(t, res.KeygenSuccess) + + // 2nd vote on new ballot: already created ballot, and not finalized (acc3) + res, err = srv.VoteTSS(ctx, &types.MsgVoteTSS{ + Creator: nodeAcc3.Operator, + TssPubkey: tss2.TssPubkey, + KeygenZetaHeight: 52, + Status: chains.ReceiveStatus_success, + }) + require.NoError(t, err) + + // check response + require.False(t, res.BallotCreated) + require.False(t, res.VoteFinalized) + require.False(t, res.KeygenSuccess) + + // check keygen status + newKeygen, found = k.GetKeygen(ctx) + require.True(t, found) + require.EqualValues(t, types.KeygenStatus_PendingKeygen, newKeygen.Status) + + // At this point, we have two ballots + // 1. Ballot for keygen 42 Voted : (acc1, acc2) + // 2. Ballot for keygen 52 Voted : (acc1, acc3) + + // 3rd vote on ballot 1: finalize the older ballot + res, err = srv.VoteTSS(ctx, &types.MsgVoteTSS{ Creator: nodeAcc3.Operator, TssPubkey: tss.TssPubkey, @@ -195,48 +402,317 @@ func TestMsgServer_VoteTSS(t *testing.T) { }) require.NoError(t, err) - // check response + // ASSERT + // Check response require.False(t, res.BallotCreated) require.True(t, res.VoteFinalized) - require.True(t, res.KeygenSuccess) + require.False(t, res.KeygenSuccess) + // Older ballot should be finalized which still keep keygen in pending state. + newKeygen, found = k.GetKeygen(ctx) + require.True(t, found) + require.EqualValues(t, types.KeygenStatus_PendingKeygen, newKeygen.Status) + + _, found = k.GetTSS(ctx) + require.False(t, found) + + oldBallot, found := k.GetBallot(ctx, fmt.Sprintf("%d-%s", 42, "tss-keygen")) + require.True(t, found) + require.EqualValues(t, types.BallotStatus_BallotFinalized_SuccessObservation, oldBallot.BallotStatus) + + newBallot, found := k.GetBallot(ctx, fmt.Sprintf("%d-%s", 52, "tss-keygen")) + require.True(t, found) + require.EqualValues(t, types.BallotStatus_BallotInProgress, newBallot.BallotStatus) + }) + + t.Run("can create a new ballot, vote without finalizing,then finalize newer ballot", func(t *testing.T) { + // ARRANGE + k, ctx, _, _ := keepertest.ObserverKeeper(t) + ctx = ctx.WithBlockHeight(42) + srv := keeper.NewMsgServerImpl(*k) + + // setup state with 3 node accounts + nodeAcc1 := sample.NodeAccount() + nodeAcc2 := sample.NodeAccount() + nodeAcc3 := sample.NodeAccount() + keygen := sample.Keygen(t) + keygen.Status = types.KeygenStatus_PendingKeygen + keygen.BlockNumber = 42 + tss := sample.Tss() + k.SetNodeAccount(ctx, *nodeAcc1) + k.SetNodeAccount(ctx, *nodeAcc2) + k.SetNodeAccount(ctx, *nodeAcc3) + k.SetKeygen(ctx, *keygen) + + // ACT + // 1st vote: created ballot, but not finalized + res, err := srv.VoteTSS(ctx, &types.MsgVoteTSS{ + Creator: nodeAcc1.Operator, + TssPubkey: tss.TssPubkey, + KeygenZetaHeight: 42, + Status: chains.ReceiveStatus_success, + }) + require.NoError(t, err) + + // check response + require.True(t, res.BallotCreated) + require.False(t, res.VoteFinalized) + require.False(t, res.KeygenSuccess) + + // check keygen not updated + newKeygen, found := k.GetKeygen(ctx) + require.True(t, found) + require.EqualValues(t, types.KeygenStatus_PendingKeygen, newKeygen.Status) + + // 2nd vote: already created ballot, and not finalized + res, err = srv.VoteTSS(ctx, &types.MsgVoteTSS{ + Creator: nodeAcc2.Operator, + TssPubkey: tss.TssPubkey, + KeygenZetaHeight: 42, + Status: chains.ReceiveStatus_success, + }) + require.NoError(t, err) + + // check response + require.False(t, res.BallotCreated) + require.False(t, res.VoteFinalized) + require.False(t, res.KeygenSuccess) // check keygen not updated newKeygen, found = k.GetKeygen(ctx) require.True(t, found) + require.EqualValues(t, types.KeygenStatus_PendingKeygen, newKeygen.Status) + + // Update keygen to 52 and start voting on new ballot + keygen.Status = types.KeygenStatus_PendingKeygen + keygen.BlockNumber = 52 + k.SetKeygen(ctx, *keygen) + + // Start voting on a new ballot + tss2 := sample.Tss() + // 1st Vote on new ballot (acc1) + res, err = srv.VoteTSS(ctx, &types.MsgVoteTSS{ + Creator: nodeAcc1.Operator, + TssPubkey: tss2.TssPubkey, + KeygenZetaHeight: 52, + Status: chains.ReceiveStatus_success, + }) + require.NoError(t, err) + + // check response + require.True(t, res.BallotCreated) + require.False(t, res.VoteFinalized) + require.False(t, res.KeygenSuccess) + + // 2nd vote on new ballot: already created ballot, and not finalized (acc3) + res, err = srv.VoteTSS(ctx, &types.MsgVoteTSS{ + Creator: nodeAcc3.Operator, + TssPubkey: tss2.TssPubkey, + KeygenZetaHeight: 52, + Status: chains.ReceiveStatus_success, + }) + require.NoError(t, err) + + // check response + require.False(t, res.BallotCreated) + require.False(t, res.VoteFinalized) + require.False(t, res.KeygenSuccess) + + // check keygen status + newKeygen, found = k.GetKeygen(ctx) + require.True(t, found) + require.EqualValues(t, types.KeygenStatus_PendingKeygen, newKeygen.Status) + + // At this point, we have two ballots + // 1. Ballot for keygen 42 Voted : (acc1, acc2) + // 2. Ballot for keygen 52 Voted : (acc1, acc3) + + // 3rd vote on ballot 2: finalize the newer ballot + + finalizingHeight := int64(55) + ctx = ctx.WithBlockHeight(finalizingHeight) + res, err = srv.VoteTSS(ctx, &types.MsgVoteTSS{ + Creator: nodeAcc2.Operator, + TssPubkey: tss2.TssPubkey, + KeygenZetaHeight: 52, + Status: chains.ReceiveStatus_success, + }) + require.NoError(t, err) + + // ASSERT + require.False(t, res.BallotCreated) + require.True(t, res.VoteFinalized) + require.True(t, res.KeygenSuccess) + // Newer ballot should be finalized which make keygen success + newKeygen, found = k.GetKeygen(ctx) + require.True(t, found) + require.EqualValues(t, finalizingHeight, newKeygen.BlockNumber) require.EqualValues(t, types.KeygenStatus_KeyGenSuccess, newKeygen.Status) - require.EqualValues(t, ctx.BlockHeight(), newKeygen.BlockNumber) + + tss, found = k.GetTSS(ctx) + require.True(t, found) + require.Equal(t, tss.KeyGenZetaHeight, int64(52)) + require.Equal(t, tss.FinalizedZetaHeight, finalizingHeight) + + oldBallot, found := k.GetBallot(ctx, fmt.Sprintf("%d-%s", 42, "tss-keygen")) + require.True(t, found) + require.EqualValues(t, types.BallotStatus_BallotInProgress, oldBallot.BallotStatus) + + newBallot, found := k.GetBallot(ctx, fmt.Sprintf("%d-%s", 52, "tss-keygen")) + require.True(t, found) + require.EqualValues(t, types.BallotStatus_BallotFinalized_SuccessObservation, newBallot.BallotStatus) }) - t.Run("fail if voting fails", func(t *testing.T) { + t.Run("add vote to a successful keygen", func(t *testing.T) { + // ARRANGE k, ctx, _, _ := keepertest.ObserverKeeper(t) ctx = ctx.WithBlockHeight(42) srv := keeper.NewMsgServerImpl(*k) - // setup state with two node accounts to not finalize the ballot - nodeAcc := sample.NodeAccount() + // setup state with 3 node accounts + nodeAcc1 := sample.NodeAccount() + nodeAcc2 := sample.NodeAccount() + nodeAcc3 := sample.NodeAccount() keygen := sample.Keygen(t) - keygen.Status = types.KeygenStatus_PendingKeygen - k.SetNodeAccount(ctx, *nodeAcc) - k.SetNodeAccount(ctx, *sample.NodeAccount()) + keygen.Status = types.KeygenStatus_KeyGenSuccess + tss := sample.Tss() + k.SetNodeAccount(ctx, *nodeAcc1) + k.SetNodeAccount(ctx, *nodeAcc2) + k.SetNodeAccount(ctx, *nodeAcc3) k.SetKeygen(ctx, *keygen) - // add a first vote + // ACT + // 1st vote: created ballot, but not finalized res, err := srv.VoteTSS(ctx, &types.MsgVoteTSS{ - Creator: nodeAcc.Operator, - TssPubkey: sample.Tss().TssPubkey, + Creator: nodeAcc1.Operator, + TssPubkey: tss.TssPubkey, KeygenZetaHeight: 42, Status: chains.ReceiveStatus_success, }) require.NoError(t, err) + + // check response + require.True(t, res.BallotCreated) require.False(t, res.VoteFinalized) + require.False(t, res.KeygenSuccess) - // vote again: voting should fail - _, err = srv.VoteTSS(ctx, &types.MsgVoteTSS{ - Creator: nodeAcc.Operator, - TssPubkey: sample.Tss().TssPubkey, + // check keygen not updated + newKeygen, found := k.GetKeygen(ctx) + require.True(t, found) + require.EqualValues(t, types.KeygenStatus_KeyGenSuccess, newKeygen.Status) + + // 2nd vote: already created ballot, and not finalized + res, err = srv.VoteTSS(ctx, &types.MsgVoteTSS{ + Creator: nodeAcc2.Operator, + TssPubkey: tss.TssPubkey, KeygenZetaHeight: 42, Status: chains.ReceiveStatus_success, }) - require.ErrorIs(t, err, types.ErrUnableToAddVote) + require.NoError(t, err) + + // check response + require.False(t, res.BallotCreated) + require.False(t, res.VoteFinalized) + require.False(t, res.KeygenSuccess) + + // check keygen not updated + newKeygen, found = k.GetKeygen(ctx) + require.True(t, found) + require.EqualValues(t, types.KeygenStatus_KeyGenSuccess, newKeygen.Status) + + // 3nd vote: already created ballot, and not finalized (acc3) + res, err = srv.VoteTSS(ctx, &types.MsgVoteTSS{ + Creator: nodeAcc3.Operator, + TssPubkey: tss.TssPubkey, + KeygenZetaHeight: 42, + Status: chains.ReceiveStatus_success, + }) + require.NoError(t, err) + + // check response + require.False(t, res.BallotCreated) + require.True(t, res.VoteFinalized) + require.False(t, res.KeygenSuccess) + + // check keygen not updated + newKeygen, found = k.GetKeygen(ctx) + require.True(t, found) + require.EqualValues(t, types.KeygenStatus_KeyGenSuccess, newKeygen.Status) + }) + + t.Run("add vote to a failed keygen ", func(t *testing.T) { + // ARRANGE + k, ctx, _, _ := keepertest.ObserverKeeper(t) + ctx = ctx.WithBlockHeight(42) + srv := keeper.NewMsgServerImpl(*k) + + // setup state with 3 node accounts + nodeAcc1 := sample.NodeAccount() + nodeAcc2 := sample.NodeAccount() + nodeAcc3 := sample.NodeAccount() + keygen := sample.Keygen(t) + keygen.Status = types.KeygenStatus_KeyGenFailed + tss := sample.Tss() + k.SetNodeAccount(ctx, *nodeAcc1) + k.SetNodeAccount(ctx, *nodeAcc2) + k.SetNodeAccount(ctx, *nodeAcc3) + k.SetKeygen(ctx, *keygen) + + // ACT + // 1st vote: created ballot, but not finalized + res, err := srv.VoteTSS(ctx, &types.MsgVoteTSS{ + Creator: nodeAcc1.Operator, + TssPubkey: tss.TssPubkey, + KeygenZetaHeight: 42, + Status: chains.ReceiveStatus_failed, + }) + require.NoError(t, err) + + // check response + require.True(t, res.BallotCreated) + require.False(t, res.VoteFinalized) + require.False(t, res.KeygenSuccess) + + // check keygen not updated + newKeygen, found := k.GetKeygen(ctx) + require.True(t, found) + require.EqualValues(t, types.KeygenStatus_KeyGenFailed, newKeygen.Status) + + // 2nd vote: already created ballot, and not finalized + res, err = srv.VoteTSS(ctx, &types.MsgVoteTSS{ + Creator: nodeAcc2.Operator, + TssPubkey: tss.TssPubkey, + KeygenZetaHeight: 42, + Status: chains.ReceiveStatus_failed, + }) + require.NoError(t, err) + + // check response + require.False(t, res.BallotCreated) + require.False(t, res.VoteFinalized) + require.False(t, res.KeygenSuccess) + + // check keygen not updated + newKeygen, found = k.GetKeygen(ctx) + require.True(t, found) + require.EqualValues(t, types.KeygenStatus_KeyGenFailed, newKeygen.Status) + + // 3nd vote: already created ballot, and not finalized (acc3) + res, err = srv.VoteTSS(ctx, &types.MsgVoteTSS{ + Creator: nodeAcc3.Operator, + TssPubkey: tss.TssPubkey, + KeygenZetaHeight: 42, + Status: chains.ReceiveStatus_failed, + }) + require.NoError(t, err) + + // check response + require.False(t, res.BallotCreated) + require.True(t, res.VoteFinalized) + require.False(t, res.KeygenSuccess) + + // check keygen not updated + newKeygen, found = k.GetKeygen(ctx) + require.True(t, found) + require.EqualValues(t, types.KeygenStatus_KeyGenFailed, newKeygen.Status) }) }