fix!: update x/collection,token proto #863

0Tech · 2023-01-16T06:03:58Z

Description

It would apply the client's requirements on x/collection,token proto.

Update the class-related part in the proto as requested by our client.
Remove proxy in the proto.
Replace all the uri variants into uri, in the proto.

Checklist:

I followed the contributing guidelines and code of conduct.
I have added a relevant changelog to CHANGELOG.md
I have added tests to cover my changes.
I have updated the documentation accordingly.
I have updated API documentation client/docs/swagger-ui/swagger.yaml

codecov · 2023-01-16T12:10:25Z

Codecov Report

Merging #863 (046dc94) into main (1deccf3) will increase coverage by 0.00%.
The diff coverage is 96.51%.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #863   +/-   ##
=======================================
  Coverage   62.37%   62.37%           
=======================================
  Files         653      653           
  Lines       79967    79976    +9     
=======================================
+ Hits        49876    49885    +9     
  Misses      27399    27399           
  Partials     2692     2692

Impacted Files	Coverage Δ
x/collection/codec.go	`44.59% <50.00%> (+0.75%)`	⬆️
x/token/codec.go	`57.89% <57.14%> (+1.13%)`	⬆️
x/collection/msgs.go	`96.72% <97.64%> (ø)`
x/collection/client/testutil/query.go	`100.00% <100.00%> (ø)`
x/collection/client/testutil/suite.go	`99.60% <100.00%> (ø)`
x/collection/client/testutil/tx.go	`100.00% <100.00%> (ø)`
x/collection/event.go	`100.00% <100.00%> (ø)`
x/collection/genesis.go	`93.20% <100.00%> (ø)`
x/collection/keeper/genesis.go	`97.04% <100.00%> (ø)`
x/collection/keeper/grpc_query.go	`89.95% <100.00%> (ø)`
... and 32 more

ByunghyunBang · 2023-01-20T04:29:46Z

Could check comment?
Fix approver -> holder, proxy -> operator

ex)

  // - ErrUnauthorized
  //   - the approver has not authorized `proxy`. ---> ###
  // - ErrInvalidRequest
  //   - `owner` does not owns `subject`.

token/v1/tx.proto

rpc AuthorizeOperator
rpc OperatorSend
rpc OperatorBurn
rpc Modify
message MsgOperatorSend
message MsgAuthorizeOperator
message MsgOperatorBurn

collection /v1/tx.proto

rpc OperatorSendFT
rpc OperatorSendNFT
rpc AuthorizeOperator
rpc RevokeOperator
rpc OperatorBurnFT
rpc OperatorBurnNFT
rpc Modify
rpc OperatorAttach
rpc OperatorDetach

0Tech · 2023-01-20T04:58:27Z

Could check comment?
Fix approver -> holder, proxy -> operator

The conditions on the errors would be wiped out from the comment, because we will introduce its own codespace (on #849). It would be obvious from the error code.

ByunghyunBang · 2023-01-20T09:59:29Z

Could you check these? approver and proxy are used here
Change approver -> holder, proxy -> operator

collection/v1/query.proto

rpc Approved(QueryApprovedRequest) returns (QueryApprovedResponse)
rpc Approvers(QueryApproversRequest) returns (QueryApproversResponse)
message QueryApprovedRequest
message QueryApprovedResponse
message QueryApproversRequest
message QueryApproversResponse

token/v1/query.proto

rpc Approved(QueryApprovedRequest) returns (QueryApprovedResponse)
rpc Approvers(QueryApproversRequest) returns (QueryApproversResponse)
message QueryApprovedRequest
message QueryApprovedResponse
message QueryApproversRequest
message QueryApproversResponse

example)

// QueryApprovedRequest is the request type for the Query/Approved RPC method
message QueryApprovedRequest {
  // contract id associated with the contract.
  string contract_id = 1;
  // address of the proxy which the authorization is granted to.
  string proxy = 2;
  // approver is the address of the approver of the authorization.
  string approver = 3;
}

// QueryApprovedResponse is the response type for the Query/Approved RPC method
message QueryApprovedResponse {
  bool approved = 1;
}

ryu1-sakai · 2023-01-20T09:28:00Z

proto/lbm/collection/v1/event.proto

-  // class id associated with the token class.
-  string class_id = 3;
+  // token id associated with the token class.
+  string token_id = 3;


(For confirmation) This field was changed from a 8-bytes ID to a 16 bytes ID, am I correct?

Yes, the change has been applied.

ryu1-sakai · 2023-01-20T09:35:31Z

proto/lbm/collection/v1/event.proto

  // changes of the attributes applied.
  repeated Attribute changes = 4 [(gogoproto.nullable) = false];


It's helpful for client developers to add a comment about possible attribute keys. For example,

Suggested change

// changes of the attributes applied.

repeated Attribute changes = 4 [(gogoproto.nullable) = false];

// changes of the attributes applied.

// Possible attribute keys are same as those of `MsgModify.changes`.

repeated Attribute changes = 4 [(gogoproto.nullable) = false];

I added the comments.

ryu1-sakai · 2023-01-20T09:53:16Z

proto/lbm/token/v1/tx.proto

-// MsgBurnFromResponse defines the Msg/BurnFrom response type.
-message MsgBurnFromResponse {}
+// MsgOperatorBurnResponse defines the Msg/OperatorBurn response type.
+message MsgOperatorBurnResponse {}

 // MsgModify defines the Msg/Modify request type.
 //


It's helpful for client developers to add a comment about possible fields of changes. For example,

// changes to apply. // possible fields: uri, meta, ... repeated Pair changes = 3 [(gogoproto.nullable) = false];

ryu1-sakai · 2023-01-20T09:56:37Z

proto/lbm/token/v1/event.proto

 //
 // Since: 0.46.0 (finschia)
 message EventModified {
-  // contract id associated with the token class.
+  // contract id associated with the contract.
  string contract_id = 1;
  // address which triggered the modify.
  string operator = 2;


It's helpful for client developers to add a comment about possible fields of changes. For example,

// changes on the metadata of the class. // Possible fields are same as those of `MsgModify.changes`. repeated Pair changes = 3 [(gogoproto.nullable) = false];

x/token/client/cli/tx.go

x/collection/client/cli/tx.go

* main: feat!: add MultiSend deactivation (Finschia#876) chore(deps): Bump github.com/gogo/protobuf in /ics23/go (Finschia#816) fix: do not create account on x/token,collection (Finschia#866) fix!: update x/collection,token proto (Finschia#863) # Conflicts: # CHANGELOG.md

0Tech added 3 commits January 16, 2023 13:43

Apply client feedbacks into x/collection

54ebfe9

Make x/collection buildable

f4af2f4

Update x/collection tests

2a4ec77

0Tech self-assigned this Jan 16, 2023

0Tech added C:x/collection C:x/token labels Jan 16, 2023

0Tech added 9 commits January 16, 2023 17:47

Update contract id related part in protos

1298db7

Make it buildable

a01944d

Update tests

aeee0c6

Update uri related part in protos

1ff9bab

Make the modules buildable

04fd7db

Update tests

1297949

Update operator related part in proto

a8379ca

Make the modules buildable

5ed8679

Update tests

1f3c886

Update CHANGELOG.md

006e37c

ryu1-sakai reviewed Jan 20, 2023

View reviewed changes

0Tech added 5 commits January 25, 2023 12:51

Update operator related queries in x/token

d22708c

Update operator related queries in x/collection

9b6b2a5

Update attributes

bec8a07

Merge branch 'main' into client_proto

77a75c1

Fix tests

2a6e2d8

0Tech marked this pull request as ready for review January 26, 2023 10:00

0Tech requested review from zemyblue, dudong2 and jaeseung-bae January 26, 2023 10:01

0Tech requested review from loin3 and tkxkd0159 January 26, 2023 10:01

jaeseung-bae reviewed Jan 26, 2023

View reviewed changes

Apply suggestions from code review

bcdb66a

0Tech requested a review from jaeseung-bae January 30, 2023 05:08

jaeseung-bae approved these changes Jan 31, 2023

View reviewed changes

Fix tests

fae2a1f

zemyblue approved these changes Feb 2, 2023

View reviewed changes

0Tech added 3 commits February 3, 2023 11:30

Add legacy information on TokenType

8f1743f

Add reference in the comments

1b522df

Apply suggestions from code review

046dc94

0Tech merged commit 3933ad1 into Finschia:main Feb 3, 2023

0Tech mentioned this pull request Feb 6, 2023

feat!: apply changes to replace Ostracon proto message with Tendermint #869

Merged

5 tasks

0Tech deleted the client_proto branch February 6, 2023 09:32

zemyblue mentioned this pull request Feb 12, 2023

apply the changes of lbm-sdk v0.47.0-alpha1 Finschia/finschia-js#67

Closed

8 tasks

zemyblue mentioned this pull request Apr 24, 2023

Release finschia-sdk v0.47.0 #988

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix!: update x/collection,token proto #863

fix!: update x/collection,token proto #863

0Tech commented Jan 16, 2023 •

edited

Loading

codecov bot commented Jan 16, 2023 •

edited

Loading

ByunghyunBang commented Jan 20, 2023 •

edited

Loading

0Tech commented Jan 20, 2023 •

edited

Loading

ByunghyunBang commented Jan 20, 2023 •

edited

Loading

ryu1-sakai Jan 20, 2023

0Tech Jan 25, 2023

ryu1-sakai Jan 20, 2023

0Tech Jan 25, 2023

ryu1-sakai Jan 20, 2023

ryu1-sakai Jan 20, 2023

		// changes of the attributes applied.
		repeated Attribute changes = 4 [(gogoproto.nullable) = false];

fix!: update x/collection,token proto #863

fix!: update x/collection,token proto #863

Conversation

0Tech commented Jan 16, 2023 • edited Loading

Description

Checklist:

codecov bot commented Jan 16, 2023 • edited Loading

Codecov Report

ByunghyunBang commented Jan 20, 2023 • edited Loading

token/v1/tx.proto

collection /v1/tx.proto

0Tech commented Jan 20, 2023 • edited Loading

ByunghyunBang commented Jan 20, 2023 • edited Loading

collection/v1/query.proto

token/v1/query.proto

ryu1-sakai Jan 20, 2023

Choose a reason for hiding this comment

0Tech Jan 25, 2023

Choose a reason for hiding this comment

ryu1-sakai Jan 20, 2023

Choose a reason for hiding this comment

0Tech Jan 25, 2023

Choose a reason for hiding this comment

ryu1-sakai Jan 20, 2023

Choose a reason for hiding this comment

ryu1-sakai Jan 20, 2023

Choose a reason for hiding this comment

0Tech commented Jan 16, 2023 •

edited

Loading

codecov bot commented Jan 16, 2023 •

edited

Loading

ByunghyunBang commented Jan 20, 2023 •

edited

Loading

0Tech commented Jan 20, 2023 •

edited

Loading

ByunghyunBang commented Jan 20, 2023 •

edited

Loading