Skip to content

Commit

Permalink
feat: duration renderer for SIGN_MODE_TEXTUAL (#13312)
Browse files Browse the repository at this point in the history
* feat: value renderer for durations

* feat: dispatch for duration ValueRenderer

* docs: update duration rendering spec

* feat: error on malformed proto, review feedback, more tests

* fix: silence spurious int overflow warnings

* fix: supress spurious gosec warning on safe operation
  • Loading branch information
JimLarson authored Oct 7, 2022
1 parent 6d03dde commit 5f01f6f
Show file tree
Hide file tree
Showing 8 changed files with 436 additions and 39 deletions.
35 changes: 30 additions & 5 deletions docs/architecture/adr-050-sign-mode-textual-annex1.md
Original file line number Diff line number Diff line change
Expand Up @@ -218,12 +218,37 @@ as `2006-01-02T15:04:05.7Z`.
The timestamp with 1136214245 seconds and zero nanoseconds is rendered
as `2006-01-02T15:04:05Z`.

### `google.protobuf.Duration` (TODO)
### `google.protobuf.Duration`

- rendered in terms of weeks, days, hours, minutes and seconds as these time units can be measured independently of any calendar and duration values are in seconds (so months and years can't be used precisely)
- total seconds values included at the end so users have both pieces of information
- Ex:
- `1483530 seconds` -> `2 weeks, 3 days, 4 hours, 5 minutes, 30 seconds (1483530 seconds total)`
The duration proto expresses a raw number of seconds and nanoseconds.
This will be rendered as longer time units of days, hours, and minutes,
plus any remaining seconds, in that order.
Leading and trailing zero-quantity units will be omitted, but all
units in between nonzero units will be shown, e.g. ` 3 days, 0 hours, 0 minutes, 5 seconds`.

Even longer time units such as months or years are imprecise.
Weeks are precise, but not commonly used - `91 days` is more immediately
legible than `13 weeks`. Although `days` can be problematic,
e.g. noon to noon on subsequent days can be 23 or 25 hours depending on
daylight savings transitions, there is significant advantage in using
strict 24-hour days over using only hours (e.g. `91 days` vs `2184 hours`).

When nanoseconds are nonzero, they will be shown as fractional seconds,
with only the minimum number of digits, e.g `0.5 seconds`.

A duration of exactly zero is shown as `0 seconds`.

Units will be given as singular (no trailing `s`) when the quantity is exactly one,
and will be shown in plural otherwise.

Negative durations will be indicated with a leading minus sign (`-`).

Examples:

- `1 day`
- `30 days`
- `-1 day, 12 hours`
- `3 hours, 0 minutes, 53.025 seconds`

### bytes

Expand Down
19 changes: 19 additions & 0 deletions tx/textual/internal/testdata/duration.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
[
{"proto": {"seconds": 0, "nanos": 0}, "text": "0 seconds"},
{"proto": {"seconds": 1, "nanos": 0}, "text": "1 second"},
{"proto": {"seconds": 2, "nanos": 0}, "text": "2 seconds"},
{"proto": {"seconds": -1, "nanos": 0}, "text": "-1 second"},
{"proto": {"seconds": 0, "nanos": 500000000}, "text": "0.5 seconds"},
{"proto": {"seconds": 0, "nanos": -500000000}, "text": "-0.5 seconds"},
{"proto": {"seconds": -1, "nanos": -500000000}, "text": "-1.5 seconds"},
{"proto": {"seconds": 60, "nanos": 0}, "text": "1 minute"},
{"proto": {"seconds": 3600, "nanos": 0}, "text": "1 hour"},
{"proto": {"seconds": 86400, "nanos": 0}, "text": "1 day"},
{"proto": {"seconds": 604800, "nanos": 0}, "text": "7 days"},
{"proto": {"seconds": 1483530, "nanos": 0}, "text": "17 days, 4 hours, 5 minutes, 30 seconds"},
{"proto": {"seconds": 1468800, "nanos": 100000000}, "text": "17 days, 0 hours, 0 minutes, 0.1 seconds"},
{"proto": {"seconds": 1468860, "nanos": 100000000}, "text": "17 days, 0 hours, 1 minute, 0.1 seconds"},
{"proto": {"seconds": 1, "nanos": -1}, "error": true},
{"proto": {"seconds": -1, "nanos": 1}, "error": true},
{"proto": {"seconds": 1, "nanos": 2000000000}, "error": true}
]
2 changes: 2 additions & 0 deletions tx/textual/internal/testpb/1.proto
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ syntax = "proto3";
option go_package = "cosmossdk.io/tx/textual/internal/testpb";

import "google/protobuf/descriptor.proto";
import "google/protobuf/duration.proto";
import "google/protobuf/timestamp.proto";
import "cosmos_proto/cosmos.proto";
import "cosmos/base/v1beta1/coin.proto";
Expand All @@ -25,6 +26,7 @@ message A {
repeated cosmos.base.v1beta1.Coin COINS = 8;
bytes BYTES = 9;
google.protobuf.Timestamp TIMESTAMP = 10;
google.protobuf.Duration DURATION = 11;

// Fields that are not handled by SIGN_MODE_TEXTUAL.
sint32 SINT32 = 101;
Expand Down
164 changes: 130 additions & 34 deletions tx/textual/internal/testpb/1.pulsar.go

Large diffs are not rendered by default.

190 changes: 190 additions & 0 deletions tx/textual/valuerenderer/duration.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,190 @@
package valuerenderer

import (
"context"
"fmt"
"io"
"regexp"
"strconv"
"strings"

"google.golang.org/protobuf/reflect/protoreflect"
dpb "google.golang.org/protobuf/types/known/durationpb"
)

type durationValueRenderer struct{}

// NewDurationValueRenderer returns a ValueRenderer for protocol buffer Duration messages.
// It renders durations by grouping seconds into units of days (86400s), hours (3600s),
// and minutes(60s), plus the total seconds elapsed. E.g. a duration of 1483530s is
// formatted as "17 days, 4 hours, 5 minutes, 30 seconds".
// Note that the days are always 24 hours regardless of daylight savings changes.
func NewDurationValueRenderer() ValueRenderer {
return durationValueRenderer{}
}

const (
min_sec = 60
hour_sec = 60 * min_sec
day_sec = 24 * hour_sec
)

type factors struct {
days, hours, minutes, seconds int64
}

func factorSeconds(x int64) factors {
var f factors
f.days = x / day_sec
x -= f.days * day_sec
f.hours = x / hour_sec
x -= f.hours * hour_sec
f.minutes = x / min_sec
x -= f.minutes * min_sec
f.seconds = x
return f
}

func maybePlural(s string, plural bool) string {
if plural {
return s + "s"
}
return s
}

func formatSeconds(seconds int64, nanos int32) string {
var s string
if nanos == 0 {
s = fmt.Sprintf("%d", seconds)
} else {
frac := fmt.Sprintf("%09d", nanos)
frac = strings.TrimRight(frac, "0")
s = fmt.Sprintf("%d.%s", seconds, frac)
}
return s
}

// Format implements the ValueRenderer interface.
func (dr durationValueRenderer) Format(_ context.Context, v protoreflect.Value, w io.Writer) error {
// Reify the reflected message as a proto Duration
msg := v.Message().Interface()
duration, ok := msg.(*dpb.Duration)
if !ok {
return fmt.Errorf("expected Duration, got %T", msg)
}

// Bypass use of time.Duration, as the range is more limited than that of dpb.Duration.
// (Too bad the companies that produced both technologies didn't coordinate better!)

if err := duration.CheckValid(); err != nil {
return err
}

negative := false
if duration.Seconds < 0 || duration.Nanos < 0 {
negative = true
// copy to avoid side-effecting our input
d := *duration
duration = &d
duration.Seconds *= -1
duration.Nanos *= -1
}
factors := factorSeconds(duration.Seconds)
components := []string{}

if factors.days > 0 {
components = append(components, fmt.Sprintf("%d %s", factors.days, maybePlural("day", factors.days != 1)))
}
if factors.hours > 0 || (len(components) > 0 && (factors.minutes > 0 || factors.seconds > 0 || duration.Nanos > 0)) {
components = append(components, fmt.Sprintf("%d %s", factors.hours, maybePlural("hour", factors.hours != 1)))
}
if factors.minutes > 0 || (len(components) > 0 && (factors.seconds > 0 || duration.Nanos > 0)) {
components = append(components, fmt.Sprintf("%d %s", factors.minutes, maybePlural("minute", factors.minutes != 1)))
}
if factors.seconds > 0 || duration.Nanos > 0 {
components = append(components, formatSeconds(factors.seconds, duration.Nanos)+" "+maybePlural("second", factors.seconds != 1 || duration.Nanos > 0))
}

s := strings.Join(components, ", ")

if s == "" {
s = "0 seconds"
}

if negative {
s = "-" + s
}

_, err := w.Write([]byte(s))
return err
}

var (
durRegexp = regexp.MustCompile(`^(-)?(?:([0-9]+) days?)?(?:, )?(?:([0-9]+) hours?)?(?:, )?(?:([0-9]+) minutes?)?(?:, )?(?:([0-9]+)(?:\.([0-9]+))? seconds?)?$`)
)

// Parse implements the ValueRenderer interface.
func (dr durationValueRenderer) Parse(_ context.Context, r io.Reader) (protoreflect.Value, error) {
bz, err := io.ReadAll(r)
if err != nil {
return protoreflect.Value{}, err
}

parts := durRegexp.FindStringSubmatch(string(bz))
if parts == nil {
return protoreflect.Value{}, fmt.Errorf("bad duration format: %s", string(bz))
}

negative := parts[1] != ""
var days, hours, minutes, seconds, nanos int64

if parts[2] != "" {
days, err = strconv.ParseInt(parts[2], 10, 64)
if err != nil {
return protoreflect.Value{}, fmt.Errorf(`bad number "%s": %w`, parts[2], err)
}
}
if parts[3] != "" {
hours, err = strconv.ParseInt(parts[3], 10, 64)
if err != nil {
return protoreflect.Value{}, fmt.Errorf(`bad number "%s": %w`, parts[3], err)
}
}
if parts[4] != "" {
minutes, err = strconv.ParseInt(parts[4], 10, 64)
if err != nil {
return protoreflect.Value{}, fmt.Errorf(`bad number "%s": %w`, parts[4], err)
}
}
if parts[5] != "" {
seconds, err = strconv.ParseInt(parts[5], 10, 64)
if err != nil {
return protoreflect.Value{}, fmt.Errorf(`bad number "%s": %w`, parts[5], err)
}
if parts[6] != "" {
if len(parts[6]) > 9 {
return protoreflect.Value{}, fmt.Errorf(`too many nanos "%s"`, parts[6])
}
addZeros := 9 - len(parts[6])
text := parts[6] + strings.Repeat("0", addZeros)
nanos, err = strconv.ParseInt(text, 10, 32)
if err != nil {
return protoreflect.Value{}, fmt.Errorf(`bad number "%s": %w`, text, err)
}
}
}

dur := &dpb.Duration{}
dur.Seconds = days*day_sec + hours*hour_sec + minutes*min_sec + seconds
// #nosec G701
// Since there are 9 digits or fewer, this conversion is safe.
dur.Nanos = int32(nanos)

if negative {
dur.Seconds *= -1
dur.Nanos *= -1
}

msg := dur.ProtoReflect()
return protoreflect.ValueOfMessage(msg), nil
}
62 changes: 62 additions & 0 deletions tx/textual/valuerenderer/duration_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
package valuerenderer_test

import (
"context"
"encoding/json"
"fmt"
"os"
"strings"
"testing"

"cosmossdk.io/tx/textual/valuerenderer"
"github.com/stretchr/testify/require"

"google.golang.org/protobuf/proto"
"google.golang.org/protobuf/reflect/protoreflect"
dpb "google.golang.org/protobuf/types/known/durationpb"
)

type durationTest struct {
Proto *dpb.Duration
Text string
Error bool
}

func TestDurationJSON(t *testing.T) {
raw, err := os.ReadFile("../internal/testdata/duration.json")
require.NoError(t, err)

var testcases []durationTest
err = json.Unmarshal(raw, &testcases)
require.NoError(t, err)

for i, tc := range testcases {
t.Run(fmt.Sprintf("%d", i), func(t *testing.T) {
rend := valuerenderer.NewDurationValueRenderer()

if tc.Proto != nil {
wr := new(strings.Builder)
err = rend.Format(context.Background(), protoreflect.ValueOf(tc.Proto.ProtoReflect()), wr)
if tc.Error {
require.Error(t, err)
return
}
require.NoError(t, err)
require.Equal(t, tc.Text, wr.String())
}

rd := strings.NewReader(tc.Text)
val, err := rend.Parse(context.Background(), rd)
if tc.Error {
require.Error(t, err)
return
}
require.NoError(t, err)
msg := val.Message().Interface()
require.IsType(t, &dpb.Duration{}, msg)
duration := msg.(*dpb.Duration)
require.True(t, proto.Equal(duration, tc.Proto), "%v vs %v", duration, tc.Proto)

})
}
}
2 changes: 2 additions & 0 deletions tx/textual/valuerenderer/valuerenderer.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (

"google.golang.org/protobuf/proto"
"google.golang.org/protobuf/reflect/protoreflect"
"google.golang.org/protobuf/types/known/durationpb"
"google.golang.org/protobuf/types/known/timestamppb"

bankv1beta1 "cosmossdk.io/api/cosmos/bank/v1beta1"
Expand Down Expand Up @@ -99,6 +100,7 @@ func (r *Textual) init() {
if r.messages == nil {
r.messages = map[protoreflect.FullName]ValueRenderer{}
r.messages[(&basev1beta1.Coin{}).ProtoReflect().Descriptor().FullName()] = NewCoinsValueRenderer(r.coinMetadataQuerier)
r.messages[(&durationpb.Duration{}).ProtoReflect().Descriptor().FullName()] = NewDurationValueRenderer()
r.messages[(&timestamppb.Timestamp{}).ProtoReflect().Descriptor().FullName()] = NewTimestampValueRenderer()
}
}
Expand Down
1 change: 1 addition & 0 deletions tx/textual/valuerenderer/valuerenderer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ func TestDispatcher(t *testing.T) {
{"SDKDEC", false, valuerenderer.NewDecValueRenderer()},
{"BYTES", false, valuerenderer.NewBytesValueRenderer()},
{"TIMESTAMP", false, valuerenderer.NewTimestampValueRenderer()},
{"DURATION", false, valuerenderer.NewDurationValueRenderer()},
{"COIN", false, valuerenderer.NewCoinsValueRenderer(nil)},
{"COINS", false, valuerenderer.NewCoinsValueRenderer(nil)},
{"FLOAT", true, nil},
Expand Down

0 comments on commit 5f01f6f

Please sign in to comment.