From 4c0c3ad6f957f41ea056d32e879d1ecc69d6bea9 Mon Sep 17 00:00:00 2001 From: Milap Sheth Date: Thu, 13 Jan 2022 16:13:36 +0000 Subject: [PATCH] fix: misc validation fixes (#1204) * fix: more validation checks * fix: vald always starts listening to the latest block * doc updates * Revert "fix: vald always starts listening to the latest block" This reverts commit 93446eacfe0f0c567022ec73f04fdfa8d5c92078. * log fix * commnets * flag check --- cmd/axelard/cmd/vald/start.go | 6 ++--- docs/cli/axelard_tx_tss_start-keygen.md | 2 +- docs/cli/axelard_vald-start.md | 2 +- x/evm/keeper/msg_server.go | 6 ++--- x/nexus/keeper/msg_server.go | 2 +- x/tss/abci.go | 11 +++++----- x/tss/client/cli/tx.go | 8 +++++-- x/tss/exported/types.go | 29 +++++++++++++++++++++++++ x/tss/types/params.go | 4 ++-- 9 files changed, 52 insertions(+), 18 deletions(-) diff --git a/cmd/axelard/cmd/vald/start.go b/cmd/axelard/cmd/vald/start.go index c8970fe8b..946aa682a 100644 --- a/cmd/axelard/cmd/vald/start.go +++ b/cmd/axelard/cmd/vald/start.go @@ -95,8 +95,8 @@ func GetValdCommand() *cobra.Command { } valAddr := serverCtx.Viper.GetString("validator-addr") - if valAddr == "" { - return fmt.Errorf("validator address not set") + if _, err := sdk.ValAddressFromBech32(valAddr); err != nil { + return sdkerrors.Wrap(err, "invalid validator operator address") } valdHome := filepath.Join(cliCtx.HomeDir, "vald") @@ -158,7 +158,7 @@ func setPersistentFlags(cmd *cobra.Command) { cmd.PersistentFlags().String("tofnd-host", defaultConf.Host, "host name for tss daemon") cmd.PersistentFlags().String("tofnd-port", defaultConf.Port, "port for tss daemon") cmd.PersistentFlags().String("tofnd-recovery", "", "json file with recovery request") - cmd.PersistentFlags().String("validator-addr", "", "the address of the validator operator") + cmd.PersistentFlags().String("validator-addr", "", "the address of the validator operator, i.e axelarvaloper1..") cmd.PersistentFlags().String(flags.FlagChainID, app.Name, "The network chain ID") } diff --git a/docs/cli/axelard_tx_tss_start-keygen.md b/docs/cli/axelard_tx_tss_start-keygen.md index 948d2ea12..15461cd9f 100644 --- a/docs/cli/axelard_tx_tss_start-keygen.md +++ b/docs/cli/axelard_tx_tss_start-keygen.md @@ -21,7 +21,7 @@ axelard tx tss start-keygen [flags] --generate-only Build an unsigned transaction and write it to STDOUT (when enabled, the local Keybase is not accessible) -h, --help help for start-keygen --id string unique ID for new key (required) - --key-role string role of the key to be generated (default "master") + --key-role string role of the key to be generated --key-type string type of the key to be generated (default "multisig") --keyring-backend string Select keyring's backend (os|file|kwallet|pass|test|memory) (default "file") --keyring-dir string The client Keyring directory; if omitted, the default 'home' directory will be used diff --git a/docs/cli/axelard_vald-start.md b/docs/cli/axelard_vald-start.md index 6cd0481b9..73431bae5 100644 --- a/docs/cli/axelard_vald-start.md +++ b/docs/cli/axelard_vald-start.md @@ -31,7 +31,7 @@ axelard vald-start [flags] --tofnd-host string host name for tss daemon (default "localhost") --tofnd-port string port for tss daemon (default "50051") --tofnd-recovery string json file with recovery request - --validator-addr string the address of the validator operator + --validator-addr string the address of the validator operator, i.e axelarvaloper1.. -y, --yes Skip tx broadcasting prompt confirmation ``` diff --git a/x/evm/keeper/msg_server.go b/x/evm/keeper/msg_server.go index 9a27dc336..c498e12cf 100644 --- a/x/evm/keeper/msg_server.go +++ b/x/evm/keeper/msg_server.go @@ -660,7 +660,7 @@ func (s msgServer) VoteConfirmChain(c context.Context, req *types.VoteConfirmCha )) if poll.Is(vote.Pending) { - return &types.VoteConfirmChainResponse{Log: fmt.Sprintf("not enough votes to confirm chain in %s yet", pendingChain.Chain.Name)}, nil + return &types.VoteConfirmChainResponse{Log: fmt.Sprintf("not enough votes to confirm chain %s yet", pendingChain.Chain.Name)}, nil } if poll.Is(vote.Failed) { @@ -673,7 +673,7 @@ func (s msgServer) VoteConfirmChain(c context.Context, req *types.VoteConfirmCha return nil, fmt.Errorf("result of poll %s has wrong type, expected bool, got %T", req.PollKey.String(), poll.GetResult()) } - s.Logger(ctx).Info(fmt.Sprintf("EVM chain confirmation result is %t", confirmed.Value)) + s.Logger(ctx).Info(fmt.Sprintf("EVM chain %s confirmation result is %t", pendingChain.Chain.Name, confirmed.Value)) s.DeletePendingChain(ctx, pendingChain.Chain.Name) // handle poll result @@ -878,7 +878,7 @@ func (s msgServer) VoteConfirmToken(c context.Context, req *types.VoteConfirmTok return nil, fmt.Errorf("result of poll %s has wrong type, expected bool, got %T", req.PollKey.String(), poll.GetResult()) } - s.Logger(ctx).Info(fmt.Sprintf("token deployment confirmation result is %t", confirmed.Value)) + s.Logger(ctx).Info(fmt.Sprintf("token %s deployment confirmation result on chain %s is %t", req.Asset, chain.Name, confirmed.Value)) // handle poll result event := sdk.NewEvent(types.EventTypeTokenConfirmation, diff --git a/x/nexus/keeper/msg_server.go b/x/nexus/keeper/msg_server.go index 28a7fa79b..8ae0510c8 100644 --- a/x/nexus/keeper/msg_server.go +++ b/x/nexus/keeper/msg_server.go @@ -13,7 +13,7 @@ import ( var _ types.MsgServiceServer = msgServer{} -const allChain = "*" +const allChain = ":all:" type msgServer struct { types.Nexus diff --git a/x/tss/abci.go b/x/tss/abci.go index d9dce825e..47ac61ada 100644 --- a/x/tss/abci.go +++ b/x/tss/abci.go @@ -189,14 +189,14 @@ func timeoutMultisigKeygen(ctx sdk.Context, multiSigKeygenQueue utils.SequenceKV participant := v.GetSDKValidator().GetOperator() if !multisigKeyInfo.DoesParticipate(participant) { - ctx.Logger().Debug(fmt.Sprintf("absent pub keys from %s for multisig keygen %s", participant, keyID)) + ctx.Logger().Info(fmt.Sprintf("absent pub keys from %s for multisig keygen %s", participant, keyID)) k.PenalizeCriminal(ctx, participant, tofnd.CRIME_TYPE_NON_MALICIOUS) } } k.DeleteSnapshotCounterForKeyID(ctx, keyID) k.DeleteMultisigKeygen(ctx, keyID) - ctx.Logger().Debug(fmt.Sprintf("multisig keygen %s timed out", keyID)) + ctx.Logger().Info(fmt.Sprintf("multisig keygen %s timed out", keyID)) ctx.EventManager().EmitEvent( sdk.NewEvent( types.EventTypeKeygen, @@ -209,6 +209,7 @@ func timeoutMultisigKeygen(ctx sdk.Context, multiSigKeygenQueue utils.SequenceKV multiSigKeygenQueue.Dequeue(0, &keyIDStr) } } + func handleMultisigSigns(ctx sdk.Context, sequenceQueue utils.SequenceKVQueue, k types.TSSKeeper) { var sigIDStr gogoprototypes.StringValue i := uint64(0) @@ -242,7 +243,7 @@ func handleMultisigSigns(ctx sdk.Context, sequenceQueue utils.SequenceKVQueue, k SigStatus: exported.SigStatus_Signed, }) - ctx.Logger().Debug(fmt.Sprintf("multisig sign %s completed", sigID)) + ctx.Logger().Info(fmt.Sprintf("multisig sign %s completed", sigID)) ctx.EventManager().EmitEvent( sdk.NewEvent( types.EventTypeSign, @@ -259,7 +260,7 @@ func handleMultisigSigns(ctx sdk.Context, sequenceQueue utils.SequenceKVQueue, k for _, participant := range participants { val, _ := sdk.ValAddressFromBech32(participant) if !multisigSignInfo.DoesParticipate(val) { - ctx.Logger().Debug(fmt.Sprintf("signatures from %s absent for multisig sign %s", participant, sigID)) + ctx.Logger().Info(fmt.Sprintf("signatures from %s absent for multisig sign %s", participant, sigID)) k.PenalizeCriminal(ctx, val, tofnd.CRIME_TYPE_NON_MALICIOUS) } } @@ -276,7 +277,7 @@ func handleMultisigSigns(ctx sdk.Context, sequenceQueue utils.SequenceKVQueue, k } } - ctx.Logger().Debug(fmt.Sprintf("multisig sign %s timed out", sigID)) + ctx.Logger().Info(fmt.Sprintf("multisig sign %s timed out", sigID)) ctx.EventManager().EmitEvent(sdk.NewEvent( types.EventTypeSign, sdk.NewAttribute(sdk.AttributeKeyModule, types.ModuleName), diff --git a/x/tss/client/cli/tx.go b/x/tss/client/cli/tx.go index 03e5c3b55..0ecf0af0c 100644 --- a/x/tss/client/cli/tx.go +++ b/x/tss/client/cli/tx.go @@ -42,10 +42,14 @@ func getCmdKeygenStart() *cobra.Command { keyID := cmd.Flags().String("id", "", "unique ID for new key (required)") if cmd.MarkFlagRequired("id") != nil { - panic("flag not set") + panic("id flag not set") + } + + keyRoleStr := cmd.Flags().String("key-role", "", "role of the key to be generated") + if cmd.MarkFlagRequired("key-role") != nil { + panic("key-role flag not set") } - keyRoleStr := cmd.Flags().String("key-role", exported.MasterKey.SimpleString(), "role of the key to be generated") keyTypeStr := cmd.Flags().String("key-type", exported.Multisig.SimpleString(), "type of the key to be generated") cmd.RunE = func(cmd *cobra.Command, args []string) error { diff --git a/x/tss/exported/types.go b/x/tss/exported/types.go index 6c00b765d..58878daef 100644 --- a/x/tss/exported/types.go +++ b/x/tss/exported/types.go @@ -64,10 +64,39 @@ func (m Key) Validate() error { return err } + if pub := m.GetECDSAKey(); pub != nil { + if _, err := pub.GetPubKey(); err != nil { + return fmt.Errorf("invalid pub key") + } + } + + if pubkeys := m.GetMultisigKey(); pubkeys != nil { + if pubkeys.GetThreshold() <= 0 { + return fmt.Errorf("invalid threshold") + } + + pubs, err := pubkeys.GetPubKey() + if err != nil { + return fmt.Errorf("invalid multisig pub key") + } + + if int64(len(pubs)) < pubkeys.GetThreshold() { + return fmt.Errorf("invalid number of multisig pub keys") + } + } + + if m.GetECDSAKey() == nil && m.GetMultisigKey() == nil { + return fmt.Errorf("pubkey cannot be nil") + } + if m.RotationCount < 0 { return fmt.Errorf("rotation count must be >=0") } + if err := utils.ValidateString(m.Chain); err != nil { + return sdkerrors.Wrap(err, "invalid chain") + } + if m.SnapshotCounter < 0 { return fmt.Errorf("snapshot counter must be >=0") } diff --git a/x/tss/types/params.go b/x/tss/types/params.go index e2e1ef05f..1a1d8561f 100644 --- a/x/tss/types/params.go +++ b/x/tss/types/params.go @@ -16,12 +16,12 @@ const ( // Parameter keys var ( - KeyKeyRequirements = []byte("keyRequirements") + KeyKeyRequirements = []byte("KeyRequirements") KeySuspendDurationInBlocks = []byte("SuspendDurationInBlocks") KeyHeartbeatPeriodInBlocks = []byte("HeartbeatPeriodInBlocks") KeyMaxMissedBlocksPerWindow = []byte("MaxMissedBlocksPerWindow") KeyUnbondingLockingKeyRotationCount = []byte("UnbondingLockingKeyRotationCount") - KeyExternalMultisigThreshold = []byte("externalMultisigThreshold") + KeyExternalMultisigThreshold = []byte("ExternalMultisigThreshold") KeyMaxSignQueueSize = []byte("MaxSignQueueSize") MaxSimultaneousSignShares = []byte("MaxSimultaneousSignShares") KeyTssSignedBlocksWindow = []byte("TssSignedBlocksWindow")