refactor: Improve txRaw decoder code (backport #9769)

server/grpc/gogoreflection/fix_registration.go

@@ -9,7 +9,6 @@ import (
 	_ "github.com/gogo/protobuf/gogoproto" // required so it does register the gogoproto file descriptor
 	gogoproto "github.com/gogo/protobuf/proto"
 
-	// nolint: staticcheck
 	"github.com/golang/protobuf/proto"
 	dpb "github.com/golang/protobuf/protoc-gen-go/descriptor"
 	_ "github.com/regen-network/cosmos-proto" // look above
@@ -86,7 +85,7 @@ func getFileDescriptor(filePath string) []byte {
 	if len(fd) != 0 {
 		return fd
 	}
-	// nolint: staticcheck
+
 	return proto.FileDescriptor(filePath)
 }
 
@@ -95,7 +94,7 @@ func getMessageType(name string) reflect.Type {
 	if typ != nil {
 		return typ
 	}
-	// nolint: staticcheck
+
 	return proto.MessageType(name)
 }
 
@@ -107,7 +106,7 @@ func getExtension(extID int32, m proto.Message) *gogoproto.ExtensionDesc {
 		}
 	}
 	// check into proto registry
-	// nolint: staticcheck
+
 	for id, desc := range proto.RegisteredExtensions(m) {
 		if id == extID {
 			return &gogoproto.ExtensionDesc{
@@ -133,7 +132,7 @@ func getExtensionsNumbers(m proto.Message) []int32 {
 	if len(out) != 0 {
 		return out
 	}
-	// nolint: staticcheck
+
 	protoExts := proto.RegisteredExtensions(m)
 	out = make([]int32, 0, len(protoExts))
 	for id := range protoExts {

server/grpc/gogoreflection/serverreflection.go

@@ -47,7 +47,6 @@ import (
 	"sort"
 	"sync"
 
-	// nolint: staticcheck
 	"github.com/golang/protobuf/proto"
 	dpb "github.com/golang/protobuf/protoc-gen-go/descriptor"
 	"google.golang.org/grpc"

x/auth/tx/decoder.go

@@ -16,7 +16,7 @@ import (
 func DefaultTxDecoder(cdc codec.ProtoCodecMarshaler) sdk.TxDecoder {
 	return func(txBytes []byte) (sdk.Tx, error) {
 		// Make sure txBytes follow ADR-027.
-		err := rejectNonADR027(txBytes)
+		err := rejectNonADR027TxRaw(txBytes)
 		if err != nil {
 			return nil, sdkerrors.Wrap(sdkerrors.ErrTxDecode, err.Error())
 		}
@@ -90,13 +90,14 @@ func DefaultJSONTxDecoder(cdc codec.ProtoCodecMarshaler) sdk.TxDecoder {
 	}
 }
 
-// rejectNonADR027 rejects txBytes that do not follow ADR-027. This function
+// rejectNonADR027TxRaw rejects txBytes that do not follow ADR-027. This is NOT
+// a generic ADR-027 checker, it only applies decoding TxRaw. Specifically, it
 // only checks that:
 // - field numbers are in ascending order (1, 2, and potentially multiple 3s),
-// - and varints as as short as possible.
-// All other ADR-027 edge cases (e.g. TxRaw fields having default values) will
-// not happen with TxRaw.
-func rejectNonADR027(txBytes []byte) error {
+// - and varints are as short as possible.
+// All other ADR-027 edge cases (e.g. default values) are not applicable with
+// TxRaw.
+func rejectNonADR027TxRaw(txBytes []byte) error {
 	// Make sure all fields are ordered in ascending order with this variable.
 	prevTagNum := protowire.Number(0)
 
@@ -105,21 +106,25 @@ func rejectNonADR027(txBytes []byte) error {
 		if m < 0 {
 			return fmt.Errorf("invalid length; %w", protowire.ParseError(m))
 		}
+		// TxRaw only has bytes fields.
 		if wireType != protowire.BytesType {
-			return fmt.Errorf("expected %d wire type, got %d", protowire.VarintType, wireType)
+			return fmt.Errorf("expected %d wire type, got %d", protowire.BytesType, wireType)
 		}
+		// Make sure fields are ordered in ascending order.
 		if tagNum < prevTagNum {
 			return fmt.Errorf("txRaw must follow ADR-027, got tagNum %d after tagNum %d", tagNum, prevTagNum)
 		}
 		prevTagNum = tagNum
 
 		// All 3 fields of TxRaw have wireType == 2, so their next component
-		// is a varint.
-		// We make sure that the varint is as short as possible.
+		// is a varint, so we can safely call ConsumeVarint here.
+		// Byte structure: <varint of bytes length><bytes sequence>
+		// Inner  fields are verified in `DefaultTxDecoder`
 		lengthPrefix, m := protowire.ConsumeVarint(txBytes[m:])
 		if m < 0 {
 			return fmt.Errorf("invalid length; %w", protowire.ParseError(m))
 		}
+		// We make sure that this varint is as short as possible.
 		n := varintMinLength(lengthPrefix)
 		if n != m {
 			return fmt.Errorf("length prefix varint for tagNum %d is not as short as possible, read %d, only need %d", tagNum, m, n)
@@ -141,23 +146,23 @@ func rejectNonADR027(txBytes []byte) error {
 func varintMinLength(n uint64) int {
 	switch {
 	// Note: 1<<N == 2**N.
-	case n < 1<<7:
+	case n < 1<<(7):
 		return 1
-	case n < 1<<14:
+	case n < 1<<(7*2):
 		return 2
-	case n < 1<<21:
+	case n < 1<<(7*3):
 		return 3
-	case n < 1<<28:
+	case n < 1<<(7*4):
 		return 4
-	case n < 1<<35:
+	case n < 1<<(7*5):
 		return 5
-	case n < 1<<42:
+	case n < 1<<(7*6):
 		return 6
-	case n < 1<<49:
+	case n < 1<<(7*7):
 		return 7
-	case n < 1<<56:
+	case n < 1<<(7*8):
 		return 8
-	case n < 1<<63:
+	case n < 1<<(7*9):
 		return 9
 	default:
 		return 10

x/auth/tx/service.go

@@ -6,7 +6,7 @@ import (
 	"strings"
 
 	gogogrpc "github.com/gogo/protobuf/grpc"
-	"github.com/golang/protobuf/proto" // nolint: staticcheck
+	"github.com/golang/protobuf/proto"
 	"github.com/grpc-ecosystem/grpc-gateway/runtime"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"