feat: add core module API #12239

aaronc · 2022-06-13T20:38:43Z

Description

Replaces #11340

This PR adds a new module core with a basic API for building modules with the new app wiring with zero implementation and an example module using it (see core/internal/example). It is intended to be an API with a smaller surface area exposed than what's currently in sdk.Context, types/, store/ etc. that apps can depend on not changing even if the underlying implementation changes substantially.

It attempts to cover very high-level module needs (for 95% of use cases) for:

store
events
block height, time, hash
gas
msg & query server registration
begin & end blockers

All context access is through the go standard context.Context so that each component doesn't need knowledge of the other things that might be in the context (unlike our current sdk.Context). The API is intended to be simple and independent of any particular Tendermint release (with lower-level APIs available for code that needs more direct Tendermint interaction).

It should be pretty straightforward for the current runtime module to implement all of the above interfaces on top of the existing sdk.Context and kv-store keys. This allows future runtime and store layers to change the underlying implementation substantially (i.e. ABCI++ or store/v2) without any API breakage for modules depending on core.

I didn't get to genesis handling or any CLI stuff and intend to do that in follow-up PRs.

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)

…e-api

api/cosmos/app/v1alpha1/config.pulsar.go

api/cosmos/app/v1alpha1/module.pulsar.go

api/cosmos/app/v1alpha1/query.pulsar.go

api/cosmos/base/abci/v1beta1/abci.pulsar.go

api/cosmos/base/query/v1beta1/pagination.pulsar.go

api/cosmos/app/runtime/v1alpha1/module.pulsar.go

api/cosmos/auth/module/v1/module.pulsar.go

api/cosmos/authz/module/v1/module.pulsar.go

api/cosmos/bank/module/v1/module.pulsar.go

tac0turtle

LGTM

core/appmodule/handler.go

core/blockinfo/service.go

core/appmodule/handler.go

Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com>

…e-api

runtime/validators.go

core/internal/buf.yaml

alexanderbez · 2022-10-11T16:51:23Z

core/appmodule/genesis.go

+	"google.golang.org/protobuf/runtime/protoiface"
+)
+
+type GenesisSource interface {


nit: godocs on interfaces and their methods

alexanderbez · 2022-10-11T16:52:22Z

core/internal/example/example.proto

+    use_package: {
+      name: "example"
+    }


Is this for app wiring? What does this mean?

It's sort of informational for now, but eventually the idea is that it helps clients know about protobuf package revisions (for instance when we add comments like Since 0.46 in .proto files). See https://github.com/cosmos/cosmos-sdk/blob/main/proto/cosmos/app/v1alpha1/module.proto#L25-L81.

Who and where is going to check this? Do we really need this in proto? It seams it's Cosmos SDK Go implementation specific, so why not using Go directly rather than going through proto?

amaury1093

I'm giving a concept ACK, namely the part where we have a core API for the sdk which aims to be stable, and extensible/customizable runtimes that satisfy the core API.

I'm still not convinced about a lot of the APIs (e.g. tiny services, handlers vs extension interfaces, overridable gas meters...), but I guess we can address those separately.

I can also approve the accompanying ADR, whichever you want to see go in first @aaronc.

amaury1093 · 2022-10-11T17:23:17Z

core/appmodule/service.go

+	event.Service
+	blockinfo.Service
+	gas.Service
+	RootInterModuleClient


Is there value in creating these subservices separately?

In practice, for a smooth transition, I envision module developers (including us) just replacing sdk.Context with this bundle service. We can also remove a lot of core/*/service.go packages, and just have a single one.

Yeah it might be possible to have one meta service. I think it is still nice to have clear separation of concerns and separate packages.

alexanderbez · 2022-10-12T16:16:28Z

I'm still not convinced about a lot of the APIs (e.g. tiny services, handlers vs extension interfaces, overridable gas meters...), but I guess we can address those separately.

I hear and understand this apprehension. @aaronc is there a way we can go with a more minimal API first and not introduce types/APIs which we don't immediately need? Like we can keep things around genesis stuff and the ABCI boundary.

aaronc · 2022-10-12T17:01:23Z

I'm still not convinced about a lot of the APIs (e.g. tiny services, handlers vs extension interfaces, overridable gas meters...), but I guess we can address those separately.

I hear and understand this apprehension. @aaronc is there a way we can go with a more minimal API first and not introduce types/APIs which we don't immediately need? Like we can keep things around genesis stuff and the ABCI boundary.

We can work on things one by one if that is easier. In that case, doing the micro-service approach actually makes sense because we can implement one by one until we have the full appmodule.Service ready. #13519 introduces the ADR 033 part of this API. We could maybe even start with that?

alexanderbez · 2022-10-12T17:12:00Z

@aaronc yes, let's start with a subset of the required/necessary APIs introduced in this PR 🙏

…e-api

aaronc · 2022-10-20T19:05:00Z

@AmauryM @alexanderbez I updated this PR to use extension interfaces instead of handlers based on discussions with @AmauryM related to #13281. I will start introducing pieces of this one by one so we can gradually build consensus on the right approach, but I want to keep the full picture here for reference.

alexanderbez · 2022-10-21T17:55:38Z

@aaronc so is this R4R again?

tac0turtle · 2022-10-21T17:57:05Z

seems like the idea is to piece meal it into main instead of one huge merge? if so can we close this for now

aaronc · 2022-10-21T18:04:53Z

seems like the idea is to piece meal it into main instead of one huge merge? if so can we close this for now

Yes, the idea is to piece meal it into main. I still think it's useful to at least have this in draft to refer to in those smaller PRs see the big picture. I think small PRs like #13607, while maybe the right stepping stone, don't convey enough context on their own.

robert-zaremba · 2022-11-07T23:11:15Z

core/appmodule/genesis.go

+type GenesisSource interface {
+	ReadMessage(protoiface.MessageV1) error
+	OpenReader(field string) (io.ReadCloser, error)
+	ReadRawJSON() (json.RawMessage, error)


is ReadMessage and ReadRawJSON going to read next record or all records? If latter, then we should have kind of an iterator pattern:.

robert-zaremba · 2022-11-07T23:15:13Z

core/appmodule/service.go

+// NOTE: If new core services are provided, they shouldn't be added to this
+// interface which would be API breaking, but rather a cosmossdk.io/core/appmodule/v2.Service
+// should be created which extends this Service interface with new services.
+type Service interface {


Does this represents an App?

robert-zaremba · 2022-11-07T23:23:43Z

core/store/store.go

+}
+
+// Iterator is an alias db's Iterator for convenience.
+type Iterator = dbm.Iterator


I think we should avoid creating aliases , unless needed for refactoring.

robert-zaremba · 2022-11-07T23:25:19Z

core/internal/example/example.proto

+    use_package: {
+      name: "example"
+    }


Who and where is going to check this? Do we really need this in proto? It seams it's Cosmos SDK Go implementation specific, so why not using Go directly rather than going through proto?

aaronc and others added 12 commits March 9, 2022 13:25

feat: add core module API

c59fef9

docs

8251594

add full example setup

204cd7a

docs

afcdbcd

docs

964f481

docs

80fb734

Merge branch 'master' into aaronc/core-api

91c7b8f

add extension resolver

40d493a

Merge remote-tracking branch 'origin/aaronc/core-api' into aaronc/cor…

b432e37

…e-api

update proto

963bbae

Merge branch 'main' of github.com:cosmos/cosmos-sdk into aaronc/core-api

81049cd

revert

68abaa7

github-advanced-security bot found potential problems Jun 13, 2022

View reviewed changes

aaronc added 3 commits June 13, 2022 17:11

updates

8ade043

docs

4de0773

docs

024f366

aaronc marked this pull request as ready for review June 13, 2022 21:30

aaronc requested a review from a team as a code owner June 13, 2022 21:30

Merge branch 'main' into aaronc/core-api

b7e175b

tac0turtle approved these changes Jun 14, 2022

View reviewed changes

tac0turtle reviewed Jun 14, 2022

View reviewed changes

core/appmodule/handler.go Outdated Show resolved Hide resolved

Merge branch 'main' into aaronc/core-api

33f99b2

alexanderbez reviewed Jun 14, 2022

View reviewed changes

core/blockinfo/service.go Outdated Show resolved Hide resolved

core/blockinfo/service.go Outdated Show resolved Hide resolved

core/appmodule/handler.go Outdated Show resolved Hide resolved

aaronc and others added 4 commits June 14, 2022 13:13

Update core/blockinfo/service.go

8e0f531

Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com>

Update core/blockinfo/service.go

3c6b412

Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com>

add ValidatorUpdateService

25acd02

Merge remote-tracking branch 'origin/aaronc/core-api' into aaronc/cor…

807c2e0

…e-api

aaronc commented Jun 14, 2022

View reviewed changes

runtime/validators.go Show resolved Hide resolved

Merge branch 'main' into aaronc/core-api

043b6fb

aaronc mentioned this pull request Jun 14, 2022

Lower boilerplate in grpc_query.go codegen #12257

Closed

4 tasks

julienrbrt reviewed Oct 5, 2022

View reviewed changes

core/internal/buf.yaml Show resolved Hide resolved

aaronc added 7 commits October 10, 2022 16:13

Merge branch 'main' of github.com:cosmos/cosmos-sdk into aaronc/core-api

3ccd485

fix import

5328e62

buf mod update

002d9d2

Merge branch 'main' of github.com:cosmos/cosmos-sdk into aaronc/core-api

b1ae3f9

fix tests

f9a903d

revert

481d107

remove empty file

6db3f0a

alexanderbez reviewed Oct 11, 2022

View reviewed changes

amaury1093 approved these changes Oct 11, 2022

View reviewed changes

Merge branch 'main' into aaronc/core-api

0438fdc

Merge branch 'main' into aaronc/core-api

8b27a49

aaronc added 4 commits October 20, 2022 14:25

updates

fdc9933

update core API to extension interfaces

f5dd33a

Merge remote-tracking branch 'origin/aaronc/core-api' into aaronc/cor…

0c28208

…e-api

update core API to extension interfaces

6e5b9c7

aaronc mentioned this pull request Oct 20, 2022

feat(core): add AppModule tag interface #13607

Merged

19 tasks

aaronc marked this pull request as draft October 21, 2022 18:05

robert-zaremba reviewed Nov 7, 2022

View reviewed changes

aaronc closed this Dec 1, 2022

This was referenced Dec 8, 2022

feat(core): add genesis API #14223

Merged

feat(core): add store API #14227

Merged

tac0turtle deleted the aaronc/core-api branch April 14, 2023 10:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add core module API #12239

feat: add core module API #12239

aaronc commented Jun 13, 2022 •

edited

Loading

tac0turtle left a comment •

edited

Loading

alexanderbez Oct 11, 2022

alexanderbez Oct 11, 2022

aaronc Oct 11, 2022

robert-zaremba Nov 7, 2022

amaury1093 left a comment

amaury1093 Oct 11, 2022

aaronc Oct 25, 2022

alexanderbez commented Oct 12, 2022

aaronc commented Oct 12, 2022

alexanderbez commented Oct 12, 2022

aaronc commented Oct 20, 2022

alexanderbez commented Oct 21, 2022

tac0turtle commented Oct 21, 2022

aaronc commented Oct 21, 2022

robert-zaremba Nov 7, 2022

robert-zaremba Nov 7, 2022

robert-zaremba Nov 7, 2022

robert-zaremba Nov 7, 2022

feat: add core module API #12239

feat: add core module API #12239

Conversation

aaronc commented Jun 13, 2022 • edited Loading

Description

Author Checklist

Reviewers Checklist

tac0turtle left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amaury1093 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexanderbez commented Oct 12, 2022

aaronc commented Oct 12, 2022

alexanderbez commented Oct 12, 2022

aaronc commented Oct 20, 2022

alexanderbez commented Oct 21, 2022

tac0turtle commented Oct 21, 2022

aaronc commented Oct 21, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aaronc commented Jun 13, 2022 •

edited

Loading

tac0turtle left a comment •

edited

Loading