Skip to content
This repository has been archived by the owner on Aug 1, 2024. It is now read-only.
/ cosmos-sdk Public archive
forked from cosmos/cosmos-sdk

Commit

Permalink
refactor(textual): Use Title/Content instead of Text (cosmos#14730)
Browse files Browse the repository at this point in the history
  • Loading branch information
amaury1093 authored and tsenart committed Apr 12, 2023
1 parent 8dac08c commit 294fd65
Show file tree
Hide file tree
Showing 30 changed files with 476 additions and 462 deletions.
18 changes: 11 additions & 7 deletions docs/architecture/adr-050-sign-mode-textual.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
* Dec 01, 2022: Link to examples in separate JSON file.
* Dec 06, 2022: Re-ordering of envelope screens.
* Dec 14, 2022: Mention exceptions for invertability.
* Jan 23, 2022: Switch Screen.Text to Title+Content.

## Status

Expand Down Expand Up @@ -126,10 +127,11 @@ in the rendering.

The SignDoc for `SIGN_MODE_TEXTUAL` is formed from a data structure like:

```
```go
type Screen struct {
Text string text // possibly size limited to, e.g. 255 characters
Indent uint8 // size limited to something small like 16 or 32
Title string // possibly size limited to, e.g. 64 characters
Content string // possibly size limited to, e.g. 255 characters
Indent uint8 // size limited to something small like 16 or 32
Expert bool
}

Expand All @@ -154,15 +156,17 @@ screens = [* screen]
;; Text defaults to the empty string, indent defaults to zero,
;; and expert defaults to false.
screen = {
? text_key: tstr,
? title_key: tstr,
? content_key: tstr,
? indent_key: uint,
? expert_key: bool,
}
;; Keys are small integers to keep the encoding small.
text_key = 1
indent_key = 2
expert_key = 3
title_key = 1
content_key = 2
indent_key = 3
expert_key = 4
```

## Details
Expand Down
4 changes: 2 additions & 2 deletions x/tx/textual/any.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func (ar anyValueRenderer) Format(ctx context.Context, v protoreflect.Value) ([]
}

screens := make([]Screen, 1+len(subscreens))
screens[0].Text = anymsg.GetTypeUrl()
screens[0].Content = anymsg.GetTypeUrl()
for i, subscreen := range subscreens {
subscreen.Indent++
screens[i+1] = subscreen
Expand All @@ -64,7 +64,7 @@ func (ar anyValueRenderer) Parse(ctx context.Context, screens []Screen) (protore
return nilValue, fmt.Errorf("bad indentation: want 0, got %d", screens[0].Indent)
}

msgType, err := protoregistry.GlobalTypes.FindMessageByURL(screens[0].Text)
msgType, err := protoregistry.GlobalTypes.FindMessageByURL(screens[0].Content)
if err != nil {
return nilValue, err
}
Expand Down
6 changes: 3 additions & 3 deletions x/tx/textual/bytes.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ func (vr bytesValueRenderer) Format(ctx context.Context, v protoreflect.Value) (

if len(bz) <= maxByteLen {
text := toHex(bz)
return []Screen{{Text: text}}, nil
return []Screen{{Content: text}}, nil
}

// For long bytes, we display the hash.
Expand All @@ -43,14 +43,14 @@ func (vr bytesValueRenderer) Format(ctx context.Context, v protoreflect.Value) (
h := hasher.Sum(nil)

text := fmt.Sprintf("%s%s", hashPrefix, toHex(h))
return []Screen{{Text: text}}, nil
return []Screen{{Content: text}}, nil
}

func (vr bytesValueRenderer) Parse(_ context.Context, screens []Screen) (protoreflect.Value, error) {
if len(screens) != 1 {
return nilValue, fmt.Errorf("expected single screen: %v", screens)
}
formatted := screens[0].Text
formatted := screens[0].Content

// If the formatted string starts with `SHA-256=`, then we can't actually
// invert to get the original bytes. In this case, we return empty bytes.
Expand Down
2 changes: 1 addition & 1 deletion x/tx/textual/bytes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func TestBytesJsonTestCases(t *testing.T) {
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)
require.Equal(t, tc.hex, screens[0].Content)

// Round trip
val, err := valrend.Parse(context.Background(), screens)
Expand Down
2 changes: 1 addition & 1 deletion x/tx/textual/coin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ func TestCoinJsonTestcases(t *testing.T) {

require.NoError(t, err)
require.Equal(t, 1, len(screens))
require.Equal(t, tc.Text, screens[0].Text)
require.Equal(t, tc.Text, screens[0].Content)

// Round trip.
value, err := vr.Parse(ctx, screens)
Expand Down
12 changes: 6 additions & 6 deletions x/tx/textual/coins.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func (vr coinsValueRenderer) Format(ctx context.Context, v protoreflect.Value) (
return nil, err
}

return []Screen{{Text: formatted}}, nil
return []Screen{{Content: formatted}}, nil
}

func (vr coinsValueRenderer) FormatRepeated(ctx context.Context, v protoreflect.Value) ([]Screen, error) {
Expand All @@ -74,19 +74,19 @@ func (vr coinsValueRenderer) FormatRepeated(ctx context.Context, v protoreflect.
return nil, err
}

return []Screen{{Text: formatted}}, nil
return []Screen{{Content: formatted}}, nil
}

func (vr coinsValueRenderer) Parse(ctx context.Context, screens []Screen) (protoreflect.Value, error) {
if len(screens) != 1 {
return nilValue, fmt.Errorf("expected single screen: %v", screens)
}

if screens[0].Text == emptyCoins {
if screens[0].Content == emptyCoins {
return protoreflect.ValueOfMessage((&basev1beta1.Coin{}).ProtoReflect()), nil
}

parsed, err := vr.parseCoins(ctx, screens[0].Text)
parsed, err := vr.parseCoins(ctx, screens[0].Content)
if err != nil {
return nilValue, err
}
Expand All @@ -99,11 +99,11 @@ func (vr coinsValueRenderer) ParseRepeated(ctx context.Context, screens []Screen
return fmt.Errorf("expected single screen: %v", screens)
}

if screens[0].Text == emptyCoins {
if screens[0].Content == emptyCoins {
return nil
}

parsed, err := vr.parseCoins(ctx, screens[0].Text)
parsed, err := vr.parseCoins(ctx, screens[0].Content)
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion x/tx/textual/coins_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func TestCoinsJsonTestcases(t *testing.T) {

require.NoError(t, err)
require.Equal(t, 1, len(screens))
require.Equal(t, tc.Text, screens[0].Text)
require.Equal(t, tc.Text, screens[0].Content)

// Round trip.
parsedValue := NewGenericList([]*basev1beta1.Coin{})
Expand Down
4 changes: 2 additions & 2 deletions x/tx/textual/dec.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,15 @@ func (vr decValueRenderer) Format(_ context.Context, v protoreflect.Value) ([]Sc
if err != nil {
return nil, err
}
return []Screen{{Text: formatted}}, nil
return []Screen{{Content: formatted}}, nil
}

func (vr decValueRenderer) Parse(_ context.Context, screens []Screen) (protoreflect.Value, error) {
if n := len(screens); n != 1 {
return nilValue, fmt.Errorf("expected 1 screen, got: %d", n)
}

parsed, err := parseDec(screens[0].Text)
parsed, err := parseDec(screens[0].Content)
if err != nil {
return nilValue, err
}
Expand Down
6 changes: 3 additions & 3 deletions x/tx/textual/duration.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ func (dr durationValueRenderer) Format(_ context.Context, v protoreflect.Value)
s = "-" + s
}

return []Screen{{Text: s}}, nil
return []Screen{{Content: s}}, nil
}

var durRegexp = regexp.MustCompile(`^(-)?(?:([0-9]+) days?)?(?:, )?(?:([0-9]+) hours?)?(?:, )?(?:([0-9]+) minutes?)?(?:, )?(?:([0-9]+)(?:\.([0-9]+))? seconds?)?$`)
Expand All @@ -125,9 +125,9 @@ func (dr durationValueRenderer) Parse(_ context.Context, screens []Screen) (prot
return protoreflect.Value{}, fmt.Errorf("expected single screen: %v", screens)
}

parts := durRegexp.FindStringSubmatch(screens[0].Text)
parts := durRegexp.FindStringSubmatch(screens[0].Content)
if parts == nil {
return protoreflect.Value{}, fmt.Errorf("bad duration format: %s", screens[0].Text)
return protoreflect.Value{}, fmt.Errorf("bad duration format: %s", screens[0].Content)
}

negative := parts[1] != ""
Expand Down
2 changes: 1 addition & 1 deletion x/tx/textual/duration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func TestDurationJSON(t *testing.T) {
}
require.NoError(t, err)
require.Equal(t, 1, len(screens))
require.Equal(t, tc.Text, screens[0].Text)
require.Equal(t, tc.Text, screens[0].Content)
}

val, err := rend.Parse(context.Background(), screens)
Expand Down
22 changes: 14 additions & 8 deletions x/tx/textual/encode.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,22 +7,25 @@ import (
)

var (
textKey = cbor.NewUint(1)
indentKey = cbor.NewUint(2)
expertKey = cbor.NewUint(3)
titleKey = cbor.NewUint(1)
contentKey = cbor.NewUint(2)
indentKey = cbor.NewUint(3)
expertKey = cbor.NewUint(4)
)

// encode encodes an array of screens according to the CDDL:
//
// screens = [* screen]
// screen = {
// ? text_key: tstr,
// ? title_key: tstr,
// ? content_key: tstr,
// ? indent_key: uint,
// ? expert_key: bool,
// }
// text_key = 1
// indent_key = 2
// expert_key = 3
// content_key = 2
// indent_key = 3
// expert_key = 4
//
// with empty values ("", 0, false) omitted from the screen map.
func encode(screens []Screen, w io.Writer) error {
Expand All @@ -35,8 +38,11 @@ func encode(screens []Screen, w io.Writer) error {

func (s Screen) Cbor() cbor.Cbor {
m := cbor.NewMap()
if s.Text != "" {
m = m.Add(textKey, cbor.NewText(s.Text))
if s.Title != "" {
m = m.Add(titleKey, cbor.NewText(s.Title))
}
if s.Content != "" {
m = m.Add(contentKey, cbor.NewText(s.Content))
}
if s.Indent > 0 {
// #nosec G701
Expand Down
4 changes: 2 additions & 2 deletions x/tx/textual/enum.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func (er enumValueRenderer) Format(_ context.Context, v protoreflect.Value) ([]S
return nil, fmt.Errorf("cannot get enum %s variant of number %d", er.ed.FullName(), v.Enum())
}

return []Screen{{Text: string(evd.Name())}}, nil
return []Screen{{Content: string(evd.Name())}}, nil

}

Expand All @@ -39,7 +39,7 @@ func (er enumValueRenderer) Parse(_ context.Context, screens []Screen) (protoref
return nilValue, fmt.Errorf("expected single screen: %v", screens)
}

formatted := screens[0].Text
formatted := screens[0].Content

// Loop through all enum variants until we find the one matching the
// formatted screen's one.
Expand Down
2 changes: 1 addition & 1 deletion x/tx/textual/enum_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func TestEnumJsonTestcases(t *testing.T) {
screens, err := valrend.Format(context.Background(), val)
require.NoError(t, err)
require.Equal(t, 1, len(screens))
require.Equal(t, tc.Text, screens[0].Text)
require.Equal(t, tc.Text, screens[0].Content)

// Round trip
parsedVal, err := valrend.Parse(context.Background(), screens)
Expand Down
4 changes: 2 additions & 2 deletions x/tx/textual/int.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,15 @@ func (vr intValueRenderer) Format(_ context.Context, v protoreflect.Value) ([]Sc
if err != nil {
return nil, err
}
return []Screen{{Text: formatted}}, nil
return []Screen{{Content: formatted}}, nil
}

func (vr intValueRenderer) Parse(_ context.Context, screens []Screen) (protoreflect.Value, error) {
if n := len(screens); n != 1 {
return nilValue, fmt.Errorf("expected 1 screen, got: %d", n)
}

parsedInt, err := parseInt(screens[0].Text)
parsedInt, err := parseInt(screens[0].Content)
if err != nil {
return nilValue, err
}
Expand Down
2 changes: 1 addition & 1 deletion x/tx/textual/int_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ func checkNumberTest(t *testing.T, r textual.ValueRenderer, pv protoreflect.Valu
require.Equal(t, 0, screens[0].Indent)
require.Equal(t, false, screens[0].Expert)

require.Equal(t, expected, screens[0].Text)
require.Equal(t, expected, screens[0].Content)

// Round trip.
value, err := r.Parse(context.Background(), screens)
Expand Down
38 changes: 19 additions & 19 deletions x/tx/textual/internal/testdata/any.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
"@type": "/Foo"
},
"screens": [
{"text": "/Foo"},
{"text": "Foo object", "indent": 1}
{"content": "/Foo"},
{"content": "Foo object", "indent": 1}
]
},
{
Expand All @@ -14,9 +14,9 @@
"full_name": "testing"
},
"screens": [
{"text": "/Foo"},
{"text": "Foo object", "indent": 1},
{"text": "Full name: testing", "indent": 2}
{"content": "/Foo"},
{"content": "Foo object", "indent": 1},
{"title": "Full name", "content": "testing", "indent": 2}
]
},
{
Expand All @@ -25,8 +25,8 @@
"value": "2006-01-02T15:04:05Z"
},
"screens": [
{"text": "/google.protobuf.Timestamp"},
{"text": "2006-01-02T15:04:05Z", "indent": 1}
{"content": "/google.protobuf.Timestamp"},
{"content": "2006-01-02T15:04:05Z", "indent": 1}
]
},
{
Expand All @@ -37,9 +37,9 @@
}
},
"screens": [
{"text": "/google.protobuf.Any"},
{"text": "/Foo", "indent": 1},
{"text": "Foo object", "indent": 2}
{"content": "/google.protobuf.Any"},
{"content": "/Foo", "indent": 1},
{"content": "Foo object", "indent": 2}
]
},
{
Expand All @@ -51,10 +51,10 @@
}
},
"screens": [
{"text": "/google.protobuf.Any"},
{"text": "/Foo", "indent": 1},
{"text": "Foo object", "indent": 2},
{"text": "Full name: testing", "indent": 3}
{"content": "/google.protobuf.Any"},
{"content": "/Foo", "indent": 1},
{"content": "Foo object", "indent": 2},
{"title": "Full name", "content": "testing", "indent": 3}
]
},
{
Expand All @@ -66,11 +66,11 @@
}
},
"screens": [
{"text": "/A"},
{"text": "A object", "indent": 1},
{"text": "ANY: /Foo", "indent": 2},
{"text": "Foo object", "indent": 3},
{"text": "Full name: testing", "indent": 4}
{"content": "/A"},
{"content": "A object", "indent": 1},
{"title": "ANY", "content": "/Foo", "indent": 2},
{"content": "Foo object", "indent": 3},
{"title": "Full name", "content": "testing", "indent": 4}
]
}
]
Loading

0 comments on commit 294fd65

Please sign in to comment.