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.

Sign up for GitHub

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

Jump to bottom

feat: value renderer for timestamp protos #12860

Merged
merged 9 commits into from
Aug 23, 2022
Merged

feat: value renderer for timestamp protos #12860

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
00ee180
feat: value renderer for timestamp protos
JimLarson Aug 8, 2022
9915821
feat: dispatch for timestamp renderer, plus json testdata
JimLarson Aug 10, 2022
836d395
docs: update the ADR annex for timestamps
JimLarson Aug 10, 2022
c5b4f76
test: switch more timestamp tests to be data driven
JimLarson Aug 17, 2022
4c1b5bb
refactor: make message dispatch table-driven
JimLarson Aug 17, 2022
469c4f8
docs: extra example for timestamps
JimLarson Aug 17, 2022
b317220
tests: add cosmosjs timestamp test cases
JimLarson Aug 17, 2022
9501781
docs: add rationale for fractional second truncation
JimLarson Aug 19, 2022
48fd2d0
Merge remote-tracking branch 'upstream/main' into jimlarson/12709-tim…
JimLarson Aug 23, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion docs/architecture/adr-050-sign-mode-textual-annex1.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,20 @@ Object: /cosmos.gov.v1.Vote

### `google.protobuf.Timestamp`

Rendered as either ISO8601 (`2021-01-01T12:00:00Z`).
Rendered using [RFC 3339](https://www.rfc-editor.org/rfc/rfc3339) (a
simplification of ISO 8601), which is the current recommendation for portable
time values. The rendering always uses "Z" (UTC) as the timezone. It uses only
the necessary fractional digits of a second, omitting the fractional part
entirely if the timestamp has no fractional seconds. (The resulting timestamps
are not automatically sortable by standard lexicographic order, but we favor
the legibility of the shorter string.)

#### Examples

The timestamp with 1136214245 seconds and 700000000 nanoseconds is rendered
as `2006-01-02T15:04:05.7Z`.
amaury1093 marked this conversation as resolved.
Show resolved Hide resolved
The timestamp with 1136214245 seconds and zero nanoseconds is rendered
as `2006-01-02T15:04:05Z`.

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

Expand Down
82 changes: 82 additions & 0 deletions tx/textual/internal/testdata/timestamp.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
[
{
"proto": {"seconds": 1136214245},
"text": "2006-01-02T15:04:05Z"
},
{
"proto": {"seconds": 1136214245, "nanos": 123456789},
"text": "2006-01-02T15:04:05.123456789Z"
},
{
"proto": {"seconds": 1136214245, "nanos": 123000000},
"text": "2006-01-02T15:04:05.123Z"
},
{ "text": "", "error": true },
{ "text": " ", "error": true },
{ "text": "garbage", "error": true },
{ "text": "11/30/2007", "error": true },
{
"proto": {"seconds": 0, "nanos": 0},
"text": "1970-01-01T00:00:00Z"
},
{
"proto": {"seconds": 0, "nanos": 1},
"text": "1970-01-01T00:00:00.000000001Z"
},
{
"proto": {"seconds": 0, "nanos": 10},
"text": "1970-01-01T00:00:00.00000001Z"
},
{
"proto": {"seconds": 0, "nanos": 100},
"text": "1970-01-01T00:00:00.0000001Z"
},
{
"proto": {"seconds": 0, "nanos": 1000},
"text": "1970-01-01T00:00:00.000001Z"
},
{
"proto": {"seconds": 0, "nanos": 10000},
"text": "1970-01-01T00:00:00.00001Z"
},
{
"proto": {"seconds": 0, "nanos": 100000},
"text": "1970-01-01T00:00:00.0001Z"
},
{
"proto": {"seconds": 0, "nanos": 1000000},
"text": "1970-01-01T00:00:00.001Z"
},
{
"proto": {"seconds": 0, "nanos": 10000000},
"text": "1970-01-01T00:00:00.01Z"
},
{
"proto": {"seconds": 0, "nanos": 100000000},
"text": "1970-01-01T00:00:00.1Z"
},
{
"proto": {"seconds": 0, "nanos": 999999999},
"text": "1970-01-01T00:00:00.999999999Z"
},
{
"proto": {"seconds": 1, "nanos": 0},
"text": "1970-01-01T00:00:01Z"
},
{
"proto": {"seconds": 2, "nanos": 0},
"text": "1970-01-01T00:00:02Z"
},
{
"proto": {"seconds": 2, "nanos": 6},
"text": "1970-01-01T00:00:02.000000006Z"
},
{
"proto": {"seconds": 1657797740, "nanos": 983000000},
"text": "2022-07-14T11:22:20.983Z"
},
{
"proto": {"seconds": -1, "nanos": 0},
"text": "1969-12-31T23:59:59Z"
}
]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you use the tests from https://github.com/cosmos/cosmjs/pull/1147/files#diff-1aee49f4f44f595fc5e4c8a2f0e15fce0b30fab5ea6e9e8f1b2894a917b19fa5?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Dropped a duplicate and also dropped the out-of-range tests, as the straightforward Go implementation doesn't return errors for those cases. It looks like the underlying Go library is able to tolerate slightly out-of-spec inputs, and that shouldn't be an error, IMHO.

2 changes: 2 additions & 0 deletions tx/textual/internal/testpb/1.proto
View file
Open in desktop
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/timestamp.proto";
import "cosmos_proto/cosmos.proto";
import "cosmos/base/v1beta1/coin.proto";

Expand All @@ -22,6 +23,7 @@ message A {
cosmos.base.v1beta1.Coin COIN = 7;
repeated cosmos.base.v1beta1.Coin COINS = 8;
bytes BYTES = 9;
google.protobuf.Timestamp TIMESTAMP = 10;
}

// B contains fields that are not parseable by SIGN_MODE_TEXTUAL, some fields
Expand Down
Loading