refactor(textual): Hash bytes when len >35 (cosmos#14598)

docs/architecture/adr-050-sign-mode-textual-annex1.md

@@ -256,20 +256,25 @@ Examples:
 
 ### bytes
 
-* Bytes of length shorter or equal to 32 are rendered in hexadecimal, all capital letters, without the `0x` prefix.
-* Bytes of length greater than 32 are hashed using SHA256. The rendered text is `SHA-256=`, followed by the 32-byte hash, in hexadecimal, all capital letters, without the `0x` prefix.
+* Bytes of length shorter or equal to 35 are rendered in hexadecimal, all capital letters, without the `0x` prefix.
+* Bytes of length greater than 35 are hashed using SHA256. The rendered text is `SHA-256=`, followed by the 32-byte hash, in hexadecimal, all capital letters, without the `0x` prefix.
 * The hexadecimal string is finally separated into groups of 4 digits, with a space `' '` as separator. If the bytes length is odd, the 2 remaining hexadecimal characters are at the end.
 
-Note: Data longer than 32 bytes are not rendered in a way that can be inverted. See ADR-050's [section about invertability](./adr-050-sign-mode-textual.md#invertible-rendering) for a discussion.
+The number 35 was chosen because it is the longest length where the hashed-and-prefixed representation is longer than the original data directly formatted, using the 3 rules above. More specifically:
+- a 35-byte array will have 70 hex characters, plus 17 space characters, resulting in 87 characters.
+- byte arrays starting from length 36 will be be hashed to 32 bytes, which is 64 hex characters plus 15 spaces, and with the `SHA-256=` prefix, it takes 87 characters.
+Also, secp256k1 public keys have length 33, so their Textual representation is not their hashed value, which we would like to avoid.
+
+Note: Data longer than 35 bytes are not rendered in a way that can be inverted. See ADR-050's [section about invertability](./adr-050-sign-mode-textual.md#invertible-rendering) for a discussion.
 
 #### Examples
 
 Inputs are displayed as byte arrays.
 
 * `[0]`: `00`
 * `[0,1,2]`: `0001 02`
-* `[0,1,2,..,31]`: `0001 0203 0405 0607 0809 0A0B 0C0D 0E0F 1011 1213 1415 1617 1819 1A1B 1C1D 1E1F` 
-* `[0,1,2,..,32]`: `SHA-256=5D8F CFEF A9AE EB71 1FB8 ED1E 4B7D 5C8A 9BAF A46E 8E76 E68A A18A DCE5 A10D F6AB`
+* `[0,1,2,..,34]`: `0001 0203 0405 0607 0809 0A0B 0C0D 0E0F 1011 1213 1415 1617 1819 1A1B 1C1D 1E1F 2021 22`
+* `[0,1,2,..,35]`: `SHA-256=5D7E 2D9B 1DCB C85E 7C89 0036 A2CF 2F9F E7B6 6554 F2DF 08CE C6AA 9C0A 25C9 9C21`
 
 ### address bytes
 

tx/textual/bytes.go

@@ -13,7 +13,7 @@ import (
 
 var (
 	hashPrefix = "SHA-256="
-	maxByteLen = 32 // Maximum byte length to be displayed as is. Longer than this, we hash.
+	maxByteLen = 35 // Maximum byte length to be displayed as is. Longer than this, we hash.
 )
 
 // NewBytesValueRenderer returns a ValueRenderer for Protobuf bytes, which are

tx/textual/bytes_test.go

@@ -23,22 +23,24 @@ func TestBytesJsonTestCases(t *testing.T) {
 	textual := textual.NewTextual(nil)
 
 	for _, tc := range testcases {
-		valrend, err := textual.GetFieldValueRenderer(fieldDescriptorFromName("BYTES"))
-		require.NoError(t, err)
-
-		screens, err := valrend.Format(context.Background(), protoreflect.ValueOfBytes(tc.base64))
-		require.NoError(t, err)
-		require.Equal(t, 1, len(screens))
-		require.Equal(t, tc.hex, screens[0].Text)
-
-		// Round trip
-		val, err := valrend.Parse(context.Background(), screens)
-		require.NoError(t, err)
-		if len(tc.base64) > 32 {
-			require.Equal(t, 0, len(val.Bytes()))
-		} else {
-			require.Equal(t, tc.base64, val.Bytes())
-		}
+		t.Run(tc.hex, func(t *testing.T) {
+			valrend, err := textual.GetFieldValueRenderer(fieldDescriptorFromName("BYTES"))
+			require.NoError(t, err)
+
+			screens, err := valrend.Format(context.Background(), protoreflect.ValueOfBytes(tc.base64))
+			require.NoError(t, err)
+			require.Equal(t, 1, len(screens))
+			require.Equal(t, tc.hex, screens[0].Text)
+
+			// Round trip
+			val, err := valrend.Parse(context.Background(), screens)
+			require.NoError(t, err)
+			if len(tc.base64) > 35 {
+				require.Equal(t, 0, len(val.Bytes()))
+			} else {
+				require.Equal(t, tc.base64, val.Bytes())
+			}
+		})
 	}
 }
 

tx/textual/internal/testdata/bytes.json

@@ -7,6 +7,6 @@
 	["Zm9vYg==", "666F 6F62"],
 	["Zm9vYmE=", "666F 6F62 61"],
 	["Zm9vYmFy", "666F 6F62 6172"],
-	["AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8=", "0001 0203 0405 0607 0809 0A0B 0C0D 0E0F 1011 1213 1415 1617 1819 1A1B 1C1D 1E1F"],
-	["AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8g", "SHA-256=5D8F CFEF A9AE EB71 1FB8 ED1E 4B7D 5C8A 9BAF A46E 8E76 E68A A18A DCE5 A10D F6AB"]
+	["AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8gISI=", "0001 0203 0405 0607 0809 0A0B 0C0D 0E0F 1011 1213 1415 1617 1819 1A1B 1C1D 1E1F 2021 22"],
+	["AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8gISIj", "SHA-256=5D7E 2D9B 1DCB C85E 7C89 0036 A2CF 2F9F E7B6 6554 F2DF 08CE C6AA 9C0A 25C9 9C21"]
 ]

tx/textual/internal/testdata/tx.json

@@ -54,7 +54,7 @@
 			{ "text": "Address: cosmos1ulav3hsenupswqfkw2y3sup5kgtqwnvqa8eyhs" },
 			{ "text": "Public key: /cosmos.crypto.secp256k1.PubKey", "expert": true },
 			{ "text": "PubKey object", "indent": 1, "expert": true },
-			{ "text": "Key: SHA-256=F795 2F7F 4120 C816 DFCF F060 E486 734B 0145 590B 1968 3856 DD00 3074 BD0D 146C", "indent": 2, "expert": true },
+			{ "text": "Key: 02EB DD7F E4FD EB76 DC8A 205E F65D 790C D30E 8A37 5A5C 2528 EB3A 923A F1FB 4D79 4D", "indent": 2, "expert": true },
 			{ "text": "This transaction has 1 Message" },
 			{ "text": "Message (1/1): /cosmos.bank.v1beta1.MsgSend", "indent": 1 },
 			{ "text": "MsgSend object", "indent": 2 },
@@ -123,7 +123,7 @@
 			{ "text": "Address: cosmos1ulav3hsenupswqfkw2y3sup5kgtqwnvqa8eyhs" },
 			{ "text": "Public key: /cosmos.crypto.secp256k1.PubKey", "expert": true },
 			{ "text": "PubKey object", "indent": 1, "expert": true },
-			{ "text": "Key: SHA-256=F795 2F7F 4120 C816 DFCF F060 E486 734B 0145 590B 1968 3856 DD00 3074 BD0D 146C", "indent": 2, "expert": true },
+			{ "text": "Key: 02EB DD7F E4FD EB76 DC8A 205E F65D 790C D30E 8A37 5A5C 2528 EB3A 923A F1FB 4D79 4D", "indent": 2, "expert": true },
 			{ "text": "This transaction has 1 Message" },
 			{ "text": "Message (1/1): /cosmos.bank.v1beta1.MsgSend", "indent": 1 },
 			{ "text": "MsgSend object", "indent": 2 },
@@ -193,7 +193,7 @@
 			{ "text": "Address: cosmos1ulav3hsenupswqfkw2y3sup5kgtqwnvqa8eyhs" },
 			{ "text": "Public key: /cosmos.crypto.secp256k1.PubKey", "expert": true },
 			{ "text": "PubKey object", "indent": 1, "expert": true },
-			{ "text": "Key: SHA-256=F795 2F7F 4120 C816 DFCF F060 E486 734B 0145 590B 1968 3856 DD00 3074 BD0D 146C", "indent": 2, "expert": true },
+			{ "text": "Key: 02EB DD7F E4FD EB76 DC8A 205E F65D 790C D30E 8A37 5A5C 2528 EB3A 923A F1FB 4D79 4D", "indent": 2, "expert": true },
 			{ "text": "This transaction has 1 Message" },
 			{ "text": "Message (1/1): /cosmos.gov.v1.MsgVote", "indent": 1 },
 			{ "text": "MsgVote object", "indent": 2 },
@@ -339,7 +339,7 @@
 			{ "text": "Address: cosmos1ulav3hsenupswqfkw2y3sup5kgtqwnvqa8eyhs" },
 			{ "text": "Public key: /cosmos.crypto.secp256k1.PubKey", "expert": true },
 			{ "text": "PubKey object", "indent": 1, "expert": true },
-			{ "text": "Key: SHA-256=F795 2F7F 4120 C816 DFCF F060 E486 734B 0145 590B 1968 3856 DD00 3074 BD0D 146C", "indent": 2, "expert": true },
+			{ "text": "Key: 02EB DD7F E4FD EB76 DC8A 205E F65D 790C D30E 8A37 5A5C 2528 EB3A 923A F1FB 4D79 4D", "indent": 2, "expert": true },
 			{ "text": "This transaction has 2 Messages" },
 			{ "text": "Message (1/2): /cosmos.authz.v1beta1.MsgExec", "indent": 1 },
 			{ "text": "MsgExec object", "indent": 2 },
@@ -377,10 +377,10 @@
 			{ "text": "Public keys: 2 Any", "indent": 4, "expert": true },
 			{ "text": "Public keys (1/2): /cosmos.crypto.secp256k1.PubKey", "indent": 5, "expert": true },
 			{ "text": "PubKey object", "indent": 6, "expert": true },
-			{ "text": "Key: SHA-256=8560 3B16 9613 06E3 9D23 BEE0 3156 7D77 F9BB 5977 A249 529F 8D9C AE2D 71CA 0ADE", "indent": 7, "expert": true },
+			{ "text": "Key: 0257 4EBE 0BFC 754F 5967 3BA1 1B27 500F 9158 ADE2 E17E 1A01 82B0 CA8B C652 4DB0 93", "indent": 7, "expert": true },
 			{ "text": "Public keys (2/2): /cosmos.crypto.ed25519.PubKey", "indent": 5, "expert": true },
 			{ "text": "PubKey object", "indent": 6, "expert": true },
-			{ "text": "Key: SHA-256=2DEF 8D73 17D8 7F1F A337 CA21 9CDE 80EC C602 6605 1328 B964 E27D 056A 94DD AC64", "indent": 7, "expert": true },
+			{ "text": "Key: 0315 0C47 F18A A327 16A6 547E DA8B 7369 067D CE11 D141 6245 B778 756C F835 9678 77", "indent": 7, "expert": true },
 			{ "text": "End of Public keys", "indent": 4, "expert": true },
 			{ "text": "Mode info: ModeInfo object", "indent": 2, "expert": true },
 			{ "text": "Multi: Multi object", "indent": 3, "expert": true },

tx/textual/tx_test.go

@@ -6,20 +6,17 @@ import (
 	"os"
 	"testing"
 
-	"github.com/cosmos/cosmos-proto/any"
 	"github.com/google/go-cmp/cmp"
 	"github.com/stretchr/testify/require"
 	"google.golang.org/protobuf/encoding/protojson"
 	"google.golang.org/protobuf/proto"
 	"google.golang.org/protobuf/reflect/protoreflect"
 	"google.golang.org/protobuf/testing/protocmp"
-	"google.golang.org/protobuf/types/known/anypb"
 
 	_ "cosmossdk.io/api/cosmos/auth/v1beta1"
 	_ "cosmossdk.io/api/cosmos/authz/v1beta1"
 	bankv1beta1 "cosmossdk.io/api/cosmos/bank/v1beta1"
 	_ "cosmossdk.io/api/cosmos/crypto/ed25519"
-	"cosmossdk.io/api/cosmos/crypto/multisig"
 	_ "cosmossdk.io/api/cosmos/crypto/secp256k1"
 	_ "cosmossdk.io/api/cosmos/gov/v1"
 	txv1beta1 "cosmossdk.io/api/cosmos/tx/v1beta1"
@@ -102,15 +99,9 @@ func TestTxJsonTestcases(t *testing.T) {
 			parsedAuthInfo := &txv1beta1.AuthInfo{}
 			err = proto.Unmarshal(parsedTextualData.AuthInfoBytes, parsedAuthInfo)
 			require.NoError(t, err)
-			// Remove the non-parsable fields, i.e. the hashed bytes
-			for i, si := range txAuthInfo.SignerInfos {
-				txAuthInfo.SignerInfos[i].PublicKey = removePkKeys(t, si.PublicKey)
-			}
 			diff = cmp.Diff(txAuthInfo, parsedAuthInfo, protocmp.Transform())
 			require.Empty(t, diff)
 
-			// Remove the non-parsable fields, i.e. the hashed public key
-			removePkKeys(t, signerData.PubKey)
 			diff = cmp.Diff(
 				signerData,
 				signerDataFromProto(parsedTextualData.SignerData),
@@ -156,27 +147,3 @@ func signerDataFromProto(d *textualpb.SignerData) signing.SignerData {
 		PubKey:        d.PubKey,
 	}
 }
-
-// removePkKeys takes a public key Any, decodes it, and recursively removes all
-// the "key" fields (hashed by textual) inside it.
-func removePkKeys(t *testing.T, pkAny *anypb.Any) *anypb.Any {
-	pk, err := anypb.UnmarshalNew(pkAny, proto.UnmarshalOptions{})
-	require.NoError(t, err)
-	m := pk.ProtoReflect().Interface()
-	switch m := m.(type) {
-	case *multisig.LegacyAminoPubKey:
-		newAnys := make([]*anypb.Any, len(m.PublicKeys))
-		for i, any := range m.PublicKeys {
-			newAnys[i] = removePkKeys(t, any)
-		}
-
-		m.PublicKeys = newAnys
-		newMultisigAny, err := any.New(m)
-		require.NoError(t, err)
-
-		return newMultisigAny
-	default:
-		pkAny.Value = nil
-		return pkAny
-	}
-}