Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Run ValidateBasic with autocli tx cmd #20282

Open
1 task done
mmsqe opened this issue May 6, 2024 · 1 comment
Open
1 task done

Run ValidateBasic with autocli tx cmd #20282

mmsqe opened this issue May 6, 2024 · 1 comment

Comments

@mmsqe
Copy link
Contributor

mmsqe commented May 6, 2024

Is there an existing issue for this?

  • I have searched the existing issues

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)

@mmsqe mmsqe added the T:Bug label May 6, 2024
@julienrbrt
Copy link
Member

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!

@julienrbrt julienrbrt changed the title [Bug]: fail to do ValidateBasic with autocli tx cmd Run ValidateBasic with autocli tx cmd May 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: 📋 Backlog
Development

No branches or pull requests

2 participants