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.

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

feat(textual): Coin and Coins value renderers #12729

Merged
merged 28 commits into from
Sep 14, 2022
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
23a0b84
wip coins
amaury1093 Jul 26, 2022
ca0fe05
Make coin test pass
amaury1093 Jul 26, 2022
18ca1e1
Remove useless file
amaury1093 Jul 26, 2022
0bd34cc
Merge branch 'main' of ssh://github.com/cosmos/cosmos-sdk into am/127…
amaury1093 Jul 28, 2022
24437ff
Merge branch 'main' of ssh://github.com/cosmos/cosmos-sdk into am/127…
amaury1093 Aug 31, 2022
466195a
wip
amaury1093 Sep 1, 2022
40b82ad
Fix tests
amaury1093 Sep 1, 2022
a2adcea
Merge branch 'main' into am/12708-textual-coins
amaury1093 Sep 1, 2022
cf0396b
Small tweaks
amaury1093 Sep 1, 2022
c4276eb
reviews
amaury1093 Sep 5, 2022
bdba40c
Add comment
amaury1093 Sep 5, 2022
655e9da
Add back go mod
amaury1093 Sep 5, 2022
4592834
Add more coins test
amaury1093 Sep 5, 2022
6af6005
Update coins test
amaury1093 Sep 5, 2022
00c1371
Add more coins tests
amaury1093 Sep 5, 2022
e4bcc76
Reference todo issue
amaury1093 Sep 5, 2022
68b6b5a
Add metadata querier test
amaury1093 Sep 5, 2022
4670093
add more tests
amaury1093 Sep 5, 2022
5355e87
Fix test build
amaury1093 Sep 6, 2022
1fdbea5
Merge branch 'main' into am/12708-textual-coins
amaury1093 Sep 6, 2022
91d708b
Improve comments
amaury1093 Sep 8, 2022
9ced494
Update tx/textual/internal/testdata/coin.json
amaury1093 Sep 8, 2022
e905ace
Merge branch 'am/12708-textual-coins' of ssh://github.com/cosmos/cosm…
amaury1093 Sep 8, 2022
8687dc2
json formatting
amaury1093 Sep 8, 2022
838c3b3
add more test cases
amaury1093 Sep 14, 2022
7106edc
Merge branch 'main' of ssh://github.com/cosmos/cosmos-sdk into am/127…
amaury1093 Sep 14, 2022
850796c
go mod tidy
amaury1093 Sep 14, 2022
93b5537
address review
amaury1093 Sep 14, 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
6 changes: 6 additions & 0 deletions tx/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,15 @@ require (
require (
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/gogo/protobuf v1.3.2 // indirect
github.com/golang/protobuf v1.5.2 // indirect
github.com/kr/pretty v0.3.0 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
github.com/rogpeppe/go-internal v1.8.1 // indirect
golang.org/x/net v0.0.0-20220722155237-a158d28d115b // indirect
golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab // indirect
golang.org/x/text v0.3.7 // indirect
google.golang.org/genproto v0.0.0-20220519153652-3a47de7e79bd // indirect
google.golang.org/grpc v1.48.0 // indirect
gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
)
112 changes: 112 additions & 0 deletions tx/go.sum

Large diffs are not rendered by default.

26 changes: 26 additions & 0 deletions tx/textual/internal/testdata/coin.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
[
[{"amount": "1", "denom": "ucosm"}, { "denom": "COSM", "exponent": 6 }, "0.000001 COSM"],
[{"amount": "10", "denom": "ucosm"}, { "denom": "COSM", "exponent": 6 }, "0.00001 COSM"],
[{"amount": "100", "denom": "ucosm"}, { "denom": "COSM", "exponent": 6 }, "0.0001 COSM"],
[{"amount": "1000", "denom": "ucosm"}, { "denom": "COSM", "exponent": 6 }, "0.001 COSM"],
[{"amount": "10000", "denom": "ucosm"}, { "denom": "COSM", "exponent": 6 }, "0.01 COSM"],
[{"amount": "100000", "denom": "ucosm"}, { "denom": "COSM", "exponent": 6 }, "0.1 COSM"],
[{"amount": "1000000", "denom": "ucosm"}, { "denom": "COSM", "exponent": 6 }, "1 COSM"],
[{"amount": "10000000", "denom": "ucosm"}, { "denom": "COSM", "exponent": 6 }, "10 COSM"],
[{"amount": "1", "denom": "ucosm"}, { "denom": "COSM", "exponent": 2 }, "0.01 COSM"],
[{"amount": "10", "denom": "ucosm"}, { "denom": "COSM", "exponent": 2 }, "0.1 COSM"],
[{"amount": "100", "denom": "ucosm"}, { "denom": "COSM", "exponent": 2 }, "1 COSM"],
[{"amount": "1000", "denom": "ucosm"}, { "denom": "COSM", "exponent": 2 }, "10 COSM"],
[{"amount": "10000", "denom": "ucosm"}, { "denom": "COSM", "exponent": 2 }, "100 COSM"],
[{"amount": "100000", "denom": "ucosm"}, { "denom": "COSM", "exponent": 2 }, "1'000 COSM"],
[{"amount": "1000000", "denom": "ucosm"}, { "denom": "COSM", "exponent": 2 }, "10'000 COSM"],
[{"amount": "10000000", "denom": "ucosm"}, { "denom": "COSM", "exponent": 2 }, "100'000 COSM"],
[{"amount": "1", "denom": "point"}, { "denom": "POINT", "exponent": 0 }, "1 POINT"],
[{"amount": "10", "denom": "point"}, { "denom": "POINT", "exponent": 0 }, "10 POINT"],
[{"amount": "100", "denom": "point"}, { "denom": "POINT", "exponent": 0 }, "100 POINT"],
[{"amount": "1000", "denom": "point"}, { "denom": "POINT", "exponent": 0 }, "1'000 POINT"],
[{"amount": "10000", "denom": "point"}, { "denom": "POINT", "exponent": 0 }, "10'000 POINT"],
[{"amount": "100000", "denom": "point"}, { "denom": "POINT", "exponent": 0 }, "100'000 POINT"],
[{"amount": "1000000", "denom": "point"}, { "denom": "POINT", "exponent": 0 }, "1'000'000 POINT"],
[{"amount": "10000000", "denom": "point"}, { "denom": "POINT", "exponent": 0 }, "10'000'000 POINT"]
]
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a test cases for

  • zero amount
  • coin without metadata
  • metadata with entries other than display and coin names
  • metadata without the display or coin name
  • failures (see timestamp.json for examples)

Copy link
Contributor Author

@amaury1093 amaury1093 Sep 5, 2022

Choose a reason for hiding this comment

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

Added test cases for all items except "metadata with entries other than display and coin names". That would be a test-only-related json parsing error, which I don't think is really useful here?

Copy link
Contributor

Choose a reason for hiding this comment

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

Additional metadata entries would be valid JSON data, and shouldn't be an SDK error either. As I understand metadata, it should be perfectly valid to have:

"metadata": {
  "display": "buck",
  "denom_units": [
    {"denom": "ubuck", "exponent": 0},
    {"denom": "mill", "exponent": 3},
    {"denom": "cent", "exponent": 4},
    {"denom": "dime", "exponent": 5},
    {"denom": "buck", "exponent": 6},
    {"denom": "grand", "exponent": 9},
    {"denom": "dirksen", "exponent": 15}
  ]
}

Copy link
Contributor Author

@amaury1093 amaury1093 Sep 14, 2022

Choose a reason for hiding this comment

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

OK, I misunderstood, by metadata with entries other than display I understood adding e.g. a field foo: ... in the metadata struct.

Added test cases with unused items in the denom_units array.

52 changes: 52 additions & 0 deletions tx/textual/internal/testdata/coins.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
[
[
[
{ "amount": "1", "denom": "ucosm" },
{ "amount": "3", "denom": "ustake" }
],
{
"ucosm": { "denom": "COSM", "exponent": 6 },
"ustake": { "denom": "STAKE", "exponent": 6 }
},
"0.000001 COSM, 0.000003 STAKE"
],
[
[
{ "amount": "3", "denom": "ustake" },
{ "amount": "1", "denom": "ucosm" }
],
{
"ucosm": { "denom": "COSM", "exponent": 6 },
"ustake": { "denom": "STAKE", "exponent": 6 }
},
"0.000001 COSM, 0.000003 STAKE"
],
[
[
{ "amount": "1", "denom": "uaa" },
{ "amount": "2", "denom": "ubb" },
{ "amount": "3", "denom": "uatom" }
],
{
"uaa": { "denom": "AA", "exponent": 6 },
"ubb": { "denom": "BB", "exponent": 6 },
"uatom": { "denom": "atom", "exponent": 6 }
},
"0.000001 AA, 0.000002 BB, 0.000003 atom"
],
[
[
{ "amount": "4", "denom": "uxc1" },
{ "amount": "3", "denom": "uxc" },
{ "amount": "2", "denom": "uxb" },
{ "amount": "1", "denom": "uxa" }
],
{
"uxa": { "denom": "xA", "exponent": 6 },
"uxb": { "denom": "xB", "exponent": 6 },
"uxc": { "denom": "xC", "exponent": 6 },
"uxc1": { "denom": "xC1", "exponent": 6 }
},
"0.000001 xA, 0.000002 xB, 0.000003 xC, 0.000004 xC1"
]
Copy link
Contributor

Choose a reason for hiding this comment

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

Add test cases for an empty Coins and a Coins of a single Coin.

Also add cases with a failure in coins at the beginning, middle, and end or the coins array.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added.

Copy link
Contributor

Choose a reason for hiding this comment

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

Still needs empty coins and singleton coins.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added.

]
30 changes: 13 additions & 17 deletions tx/textual/internal/testpb/1.proto
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,9 @@ enum Enumeration {
Two = 1;
}

// A contains fields that are parseable by SIGN_MODE_TEXTUAL.
// A is used for testing value renderers.
message A {
// Fields that are parseable by SIGN_MODE_TEXTUAL.
uint32 UINT32 = 1;
uint64 UINT64 = 2;
int32 INT32 = 3;
Expand All @@ -24,20 +25,15 @@ message A {
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
// may be moved to A at some point.
message B {
int32 INT32 = 1;
sint32 SINT32 = 2;
int64 INT64 = 3;
sint64 SING64 = 4;
sfixed32 SFIXED32 = 5;
fixed32 FIXED32 = 6;
float FLOAT = 7;
sfixed64 SFIXED64 = 8;
fixed64 FIXED64 = 9;
double DOUBLE = 10;
map<string, B> MAP = 11;
}
// Fields that are not handled by SIGN_MODE_TEXTUAL.
sint32 SINT32 = 101;
sint64 SINT64 = 102;
sfixed32 SFIXED32 = 105;
fixed32 FIXED32 = 106;
float FLOAT = 107;
sfixed64 SFIXED64 = 108;
fixed64 FIXED64 = 109;
double DOUBLE = 110;
map<string, A> MAP = 111;
}
Loading