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

sdk.Router should be stateful #5455

Closed
4 tasks
sunnya97 opened this issue Dec 30, 2019 · 2 comments · Fixed by #5452
Closed
4 tasks

sdk.Router should be stateful #5455

sunnya97 opened this issue Dec 30, 2019 · 2 comments · Fixed by #5452
Labels
C:baseapp S:proposed T: API Breaking Breaking changes that impact APIs and the SDK only (not state machine).

Comments

@sunnya97
Copy link
Member

sunnya97 commented Dec 30, 2019

Summary

The sdk.Router interface should be generalized to allow for more stateful router implementations.

Problem Definition

The basic default router included in baseapp is a stateless router i.e. the mapping from path strings to handlers is static and defined in the application codebase (in the app.go). However, the requirement to remain stateless is limiting in that it restricts the ability to do more complex modifications of routes.

Some examples of advanced router functionality that need statefulness:

  • Allowing governance or permissioned entities to enable or disable msg routes (requires to be able to read current state of param store)
  • Allowing governance to switch the handler that a specific route points to for upgrade purposes

Proposal

  • Update the sdk.Router interface's .Route() function to take in an sdk.Context as an argument
  • Keep the baseapp.Router implementation as the stateless design it is right now
  • Baseapp uses baseapp.Router by default unless intentionally passed a new router by an application's app.go
  • Add baseapp.WithRouter() function to allow an app.go to pass in its own router implementation.

For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@alexanderbez
Copy link
Contributor

ref: #5452

@alexanderbez
Copy link
Contributor

Given that we can still simply use the current/default stateless router, I don't have any strong objections against providing the option to use stateful routers. Unless anyone can state strong objective reasons for not doing so, I think we can go forward with this proposal 👍

Do you intend to have #5452 close this issue?

@alexanderbez alexanderbez added C:baseapp S:proposed T: API Breaking Breaking changes that impact APIs and the SDK only (not state machine). labels Jan 1, 2020
@PumpkinSeed PumpkinSeed mentioned this issue Jan 2, 2020
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:baseapp S:proposed T: API Breaking Breaking changes that impact APIs and the SDK only (not state machine).
Projects
None yet
2 participants