feat(orm): add query proto codegen #13438

aaronc · 2022-10-03T16:23:40Z

Description

This starts the implementation of approach (C) in #11774 by generating _query.proto files for .proto files which have ORM definitions. It does this using a new protoc plugin called protoc-gen-go-cosmos-orm-proto.

Please review bank_query.proto and test_schema_query.proto as these are the outputs of the codegen.

The approach taken does not attempt to do any sort of logical queries as discussed in #11774 and instead provides direct index access in the most simple and complete way that we can easily implement now. More advanced things in #11774 could be implemented later, but this should be possible to deliver relatively quickly and should allow developers to completely skip writing gRPC queries manually if they use ORM.

Note that the implementation of these queries can provide merkle proofs because the mapping to state is well known.

I did need to disable pulsar codegen in this PR because it doesn't support proto3 optionals which are needed here (depends on cosmos/cosmos-proto#12). Fortunately, pulsar is 100% compatible with the official google codegen and there is no problem running ORM tests against the official codegen.

Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

confirmed the correct type prefix in the PR title
confirmed ! in the type prefix if API or client breaking change
confirmed all author checklist items have been addressed
reviewed state machine logic
reviewed API design and naming
reviewed documentation is accurate
reviewed tests and test coverage
manually tested (if applicable)

aaronc · 2022-10-03T17:40:16Z

orm/internal/codegen/codegen.go

+			continue
+		}
+
+		out, err := os.OpenFile(fmt.Sprintf("%s_query.proto", f.GeneratedFilenamePrefix), os.O_RDWR|os.O_TRUNC|os.O_CREATE, 0644)


I have to say this seems rather strange... 0644 is pretty standard 🤔

codecov · 2022-10-03T17:11:55Z

Codecov Report

Merging #13438 (3029d26) into main (5f01f6f) will increase coverage by 0.46%.
The diff coverage is n/a.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #13438      +/-   ##
==========================================
+ Coverage   54.01%   54.48%   +0.46%     
==========================================
  Files         648      676      +28     
  Lines       55425    57952    +2527     
==========================================
+ Hits        29940    31576    +1636     
- Misses      23086    23731     +645     
- Partials     2399     2645     +246

Impacted Files	Coverage Δ
x/distribution/simulation/operations.go	`80.64% <0.00%> (-9.68%)`	⬇️
crypto/keys/internal/ecdsa/privkey.go	`81.13% <0.00%> (-1.89%)`	⬇️
tx/textual/valuerenderer/dec.go
tx/textual/valuerenderer/int.go
tx/textual/valuerenderer/bytes.go
tx/textual/valuerenderer/valuerenderer.go
tx/textual/valuerenderer/duration.go
tx/textual/valuerenderer/timestamp.go
tx/textual/valuerenderer/coins.go
orm/model/ormtable/table_impl.go	`59.92% <0.00%> (ø)`
... and 34 more

orm/internal/codegen/codegen.go

+}
+
+func QueryProtoPluginRunner(p *protogen.Plugin) error {
+	p.SupportedFeatures = uint64(pluginpb.CodeGeneratorResponse_FEATURE_PROTO3_OPTIONAL)


aaronc · 2022-10-03T17:48:45Z

orm/internal/codegen/codegen.go

@@ -17,7 +19,8 @@
 	ormTablePkg = protogen.GoImportPath("github.com/cosmos/cosmos-sdk/orm/model/ormtable")
 )

-func PluginRunner(p *protogen.Plugin) error {
+func ORMPluginRunner(p *protogen.Plugin) error {
+	p.SupportedFeatures = uint64(pluginpb.CodeGeneratorResponse_FEATURE_PROTO3_OPTIONAL)


Umm.. the value of the const is 1. gosec should be able to statically inspect const values...

… aaronc/orm-query-proto-gen

technicallyty

LGTM. are there any plans to add constructor type functions to make the ergonomics of forming the list query requests a little easier?

currently, using ListBalance would require devs to write:

	req := ListBalanceRequest{
		Query:         		&ListBalanceRequest_PrefixQuery{
		PrefixQuery: 		&ListBalanceRequest_IndexKey{
		Key: 			&ListBalanceRequest_IndexKey_Denom_{
		Denom: 			&ListBalanceRequest_IndexKey_Denom{Denom: &denom}}}},
		Pagination:    		nil,
	}

it's not too bad, but definitely a little more complex than i'd prefer

tac0turtle

how come this is landing? orm is on pause till we have more time and ability to take things on.

aaronc · 2022-10-03T19:52:16Z

My thought is that these are mostly for non golang clients. Which reminds me that I need to add grpc-gateway annotations to this PR.

From golang it should be possible to use the ORM directly using ABCI query or something similar that supports iteration rather than this API.

Longer term it'd be nice to see logical queries (eg. SELECT amount, denom FROM balance where address = ?)

aaronc · 2022-10-03T19:54:17Z

how come this is landing? orm is on pause till we have more time and ability to take things on.

Because I have it ready and didn't want this to get lost in a pile of POC code. Do you want me to close until we have more bandwidth?

tac0turtle · 2022-10-03T20:14:27Z

how come this is landing? orm is on pause till we have more time and ability to take things on.

Because I have it ready and didn't want this to get lost in a pile of POC code. Do you want me to close until we have more bandwidth?

lets merge, just somewhat confusing when its not within in the scope of things we are working right now. All good

aaronc · 2022-10-03T20:34:58Z

how come this is landing? orm is on pause till we have more time and ability to take things on.

Because I have it ready and didn't want this to get lost in a pile of POC code. Do you want me to close until we have more bandwidth?

lets merge, just somewhat confusing when its not within in the scope of things we are working right now. All good

Sorry about that. Will give a heads up next time

## Description This starts the implementation of approach (C) in cosmos#11774 by generating `_query.proto` files for `.proto` files which have ORM definitions. It does this using a new protoc plugin called `protoc-gen-go-cosmos-orm-proto`. Please review `bank_query.proto` and `test_schema_query.proto` as these are the outputs of the codegen. The approach taken does not attempt to do any sort of logical queries as discussed in cosmos#11774 and instead provides direct index access in the most simple and complete way that we can easily implement now. More advanced things in cosmos#11774 could be implemented later, but this should be possible to deliver relatively quickly and should allow developers to completely skip writing gRPC queries manually if they use ORM. Note that the implementation of these queries can provide merkle proofs because the mapping to state is well known. I did need to disable pulsar codegen in this PR because it doesn't support proto3 optionals which are needed here (depends on cosmos/cosmos-proto#12). Fortunately, pulsar is 100% compatible with the official google codegen and there is no problem running ORM tests against the official codegen. --- ### Author Checklist *All items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues.* I have... - [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] added `!` to the type prefix if API or client breaking change - [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/main/CONTRIBUTING.md#pr-targeting)) - [ ] provided a link to the relevant issue or specification - [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/main/docs/building-modules) - [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/main/CONTRIBUTING.md#testing) - [ ] added a changelog entry to `CHANGELOG.md` - [ ] included comments for [documenting Go code](https://blog.golang.org/godoc) - [ ] updated the relevant documentation or specification - [ ] reviewed "Files changed" and left comments if necessary - [ ] confirmed all CI checks have passed ### Reviewers Checklist *All items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items.* I have... - [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] confirmed `!` in the type prefix if API or client breaking change - [ ] confirmed all author checklist items have been addressed - [ ] reviewed state machine logic - [ ] reviewed API design and naming - [ ] reviewed documentation is accurate - [ ] reviewed tests and test coverage - [ ] manually tested (if applicable)

feat(orm): add query proto codegen

ab112f9

github-actions bot added the C:orm label Oct 3, 2022

github-advanced-security bot found potential problems Oct 3, 2022

View reviewed changes

codegen

08f697d

aaronc added 2 commits October 3, 2022 13:24

doc comments

35c9811

fixes

0cbc766

aaronc marked this pull request as ready for review October 3, 2022 17:36

aaronc requested a review from a team as a code owner October 3, 2022 17:36

Merge branch 'main' into aaronc/orm-query-proto-gen

be5c00b

aaronc requested review from technicallyty and ryanchristo October 3, 2022 17:36

github-advanced-security bot found potential problems Oct 3, 2022

View reviewed changes

aaronc added 2 commits October 3, 2022 14:23

codegen

acaa854

Merge remote-tracking branch 'origin/aaronc/orm-query-proto-gen' into…

819a119

… aaronc/orm-query-proto-gen

technicallyty approved these changes Oct 3, 2022

View reviewed changes

tac0turtle reviewed Oct 3, 2022

View reviewed changes

Merge branch 'main' into aaronc/orm-query-proto-gen

a33d0c3

tac0turtle approved these changes Oct 4, 2022

View reviewed changes

tac0turtle and others added 3 commits October 4, 2022 22:45

Merge branch 'main' into aaronc/orm-query-proto-gen

e7ac662

Merge branch 'main' into aaronc/orm-query-proto-gen

50c1451

Merge branch 'main' into aaronc/orm-query-proto-gen

3029d26

julienrbrt approved these changes Oct 7, 2022

View reviewed changes

julienrbrt added the A:automerge Automatically merge PR once all prerequisites pass. label Oct 7, 2022

mergify bot merged commit 2151427 into main Oct 7, 2022

mergify bot deleted the aaronc/orm-query-proto-gen branch October 7, 2022 14:37

aaronc mentioned this pull request Dec 2, 2022

feat: add schema reflection NibiruChain/collections#8

Closed

aaronc mentioned this pull request Feb 9, 2023

feat(orm)!: implement gRPC query support #14965

Closed

19 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(orm): add query proto codegen #13438

feat(orm): add query proto codegen #13438

aaronc commented Oct 3, 2022 •

edited

Loading

aaronc Oct 3, 2022

codecov bot commented Oct 3, 2022 •

edited

Loading

aaronc Oct 3, 2022

technicallyty left a comment

tac0turtle left a comment

aaronc commented Oct 3, 2022

aaronc commented Oct 3, 2022

tac0turtle commented Oct 3, 2022

aaronc commented Oct 3, 2022 •

edited

Loading

feat(orm): add query proto codegen #13438

feat(orm): add query proto codegen #13438

Conversation

aaronc commented Oct 3, 2022 • edited Loading

Description

Author Checklist

Reviewers Checklist

aaronc Oct 3, 2022

Choose a reason for hiding this comment

codecov bot commented Oct 3, 2022 • edited Loading

Codecov Report

aaronc Oct 3, 2022

Choose a reason for hiding this comment

technicallyty left a comment

Choose a reason for hiding this comment

tac0turtle left a comment

Choose a reason for hiding this comment

aaronc commented Oct 3, 2022

aaronc commented Oct 3, 2022

tac0turtle commented Oct 3, 2022

aaronc commented Oct 3, 2022 • edited Loading

aaronc commented Oct 3, 2022 •

edited

Loading

codecov bot commented Oct 3, 2022 •

edited

Loading

aaronc commented Oct 3, 2022 •

edited

Loading