cosmos / cosmos-sdk

:chains: A Framework for Building High Value Public Blockchains :sparkles:
https://cosmos.network/
Apache License 2.0
6.07k stars 3.48k forks source link

Run ValidateBasic with autocli tx cmd #20282

Open mmsqe opened 1 month ago

mmsqe commented 1 month ago

Is there an existing issue for this?

What happened?

Currently the cast back to proto.Message with dynamicpb doesn't make use of proto.RegisterType, which fails to execute ValidateBasic in client side.

Cosmos SDK Version

release/v0.50.x

How to reproduce?

  1. register tx cmd with ValidateBasic
  2. run register-encryption-key cmd and get code 1 from server instead of early fail in client side
    cronosd tx e2ee register-encryption-key age18289ar3tzcv8eye5s3232yccqa4lkxm2v6pqzw2hn6tlav6jg96s7xew20malformed -y --home node0 --from validator --gas-prices 100000000000basetcro --gas 250000 | jq
    {
    "height": "0",
    "txhash": "E650B8554F752BBE2B3235F4520412352A123E16ABBCE62297FF395A4118CE66",
    "codespace": "undefined",
    "code": 1,
    "data": "",
    "raw_log": "malformed recipient \"age18289ar3tzcv8eye5s3232yccqa4lkxm2v6pqzw2hn6tlav6jg96s7xew20malformed\": invalid character data part: s[62]=111",
    "logs": [],
    "info": "",
    "gas_wanted": "0",
    "gas_used": "0",
    "tx": null,
    "timestamp": "",
    "events": []
    }

    Proposal

    only work for simple type but not with nested type

diff --git a/client/v2/autocli/msg.go b/client/v2/autocli/msg.go
index 0b3143620f..11e36c2f1b 100644
--- a/client/v2/autocli/msg.go
+++ b/client/v2/autocli/msg.go
@@ -3,6 +3,8 @@ package autocli
 import (
    "context"
    "fmt"
+   "reflect"
+   "strings"

    "github.com/cockroachdb/errors"
    gogoproto "github.com/cosmos/gogoproto/proto"
@@ -16,6 +18,7 @@ import (
    "cosmossdk.io/client/v2/internal/flags"
    "cosmossdk.io/client/v2/internal/util"
    addresscodec "cosmossdk.io/core/address"
+   codectypes "github.com/cosmos/cosmos-sdk/codec/types"

    // the following will be extracted to a separate module
    // https://github.com/cosmos/cosmos-sdk/issues/14403
@@ -165,9 +168,25 @@ func (b *Builder) BuildMsgMethodCommand(descriptor protoreflect.MethodDescriptor
        // Here we use dynamicpb, to create a proto v1 compatible message.
        // The SDK codec will handle protov2 -> protov1 (marshal)
        msg := dynamicpb.NewMessage(input.Descriptor())
-       proto.Merge(msg, input.Interface())
+       any, err := codectypes.NewAnyWithValue(msg)
+       if err != nil {
+           return err
+       }
+       msgType := gogoproto.MessageType(strings.ReplaceAll(any.TypeUrl, "/", ""))
+       val := reflect.New(msgType.Elem())
+       elem := val.Elem()
+       input.Range(func(fd protoreflect.FieldDescriptor, v protoreflect.Value) bool {
+           f := elem.FieldByNameFunc(func(s string) bool {
+               return strings.ToLower(s) == string(fd.Name())
+           })
+           if f.IsValid() && f.CanSet() {
+               f.Set(reflect.ValueOf(v.Interface()))
+           }
+           return true
+       })
+       sdkMsg := val.Interface()

-       return clienttx.GenerateOrBroadcastTxCLI(clientCtx, cmd.Flags(), msg)
+       return clienttx.GenerateOrBroadcastTxCLI(clientCtx, cmd.Flags(), sdkMsg.(gogoproto.Message))
    }

    cmd, err := b.buildMethodCommandCommon(descriptor, options, execFunc)
julienrbrt commented 1 month ago

Right, that a very good point! We actually haven't considered that, as we've made ValidateBasic optional and removed all of our ValidateBasic. However, I agree that it is a great feature to add!