-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
refactor!: Remove sdk.Router and refactor baseapp tests #13005
Merged
Merged
Changes from 38 commits
Commits
Show all changes
41 commits
Select commit
Hold shift + click to select a range
6cd4d17
test
facundomedica ab5c482
progress-ish
facundomedica e6bfe51
progress
facundomedica 3442d41
progress
facundomedica 9477892
make mocks
facundomedica 1d1fc8a
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into facu…
facundomedica 8e83997
progress
facundomedica 01850f8
test
facundomedica 6e1f828
progress
facundomedica 8b19f6e
progress
facundomedica 21db64f
Merge branch 'main' into facu/remove-router
facundomedica 08ae27b
Merge branch 'facu/remove-router' of https://github.com/cosmos/cosmos…
facundomedica 525449f
Merge branch 'main' into facu/remove-router
facundomedica 1689eea
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into facu…
facundomedica 59a8733
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into facu…
facundomedica e57a4e6
progress
facundomedica ce6927c
progress
facundomedica 284bfe9
progress
facundomedica 36587ad
Merge branch 'facu/remove-router' of https://github.com/cosmos/cosmos…
facundomedica 236183e
progress
facundomedica e573754
progress
facundomedica 6887aa5
fix mock tests
facundomedica efce7f9
progress
facundomedica 1ec7379
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into facu…
facundomedica 105dcdc
pretty much done, might need some tidying up
facundomedica 056b5eb
lint
facundomedica 27b73d3
re-enable blockgas test
facundomedica f0877f7
Merge branch 'main' into facu/remove-router
facundomedica 33c3c1b
remove router
facundomedica 428b98c
Merge branch 'facu/remove-router' of https://github.com/cosmos/cosmos…
facundomedica d91ed4a
gofumpt
facundomedica c0ae7f3
remove more references of Router
facundomedica 3855afc
fix
facundomedica 27383fc
Merge branch 'main' into facu/remove-router
facundomedica 4f2d76a
remove unused code
facundomedica 5284a5d
Merge branch 'facu/remove-router' of https://github.com/cosmos/cosmos…
facundomedica 75f7d2f
remove unused code
facundomedica cce106f
Merge branch 'main' into facu/remove-router
facundomedica 32a1452
Merge branch 'main' into facu/remove-router
facundomedica c3c767a
Merge branch 'main' into facu/remove-router
tac0turtle a1f2db0
Merge branch 'main' into facu/remove-router
facundomedica File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,7 +18,6 @@ import ( | |
storetypes "github.com/cosmos/cosmos-sdk/store/types" | ||
sdk "github.com/cosmos/cosmos-sdk/types" | ||
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" | ||
"github.com/cosmos/cosmos-sdk/x/auth/migrations/legacytx" | ||
) | ||
|
||
const ( | ||
|
@@ -49,7 +48,6 @@ type BaseApp struct { //nolint: maligned | |
db dbm.DB // common DB backend | ||
cms sdk.CommitMultiStore // Main (uncached) state | ||
storeLoader StoreLoader // function to handle store loading, may be overridden with SetStoreLoader() | ||
router sdk.Router // handle any kind of legacy message | ||
queryRouter sdk.QueryRouter // router for redirecting query calls | ||
grpcQueryRouter *GRPCQueryRouter // router for redirecting gRPC query calls | ||
msgServiceRouter *MsgServiceRouter // router for redirecting Msg service messages | ||
|
@@ -149,7 +147,6 @@ func NewBaseApp( | |
db: db, | ||
cms: store.NewCommitMultiStore(db), | ||
storeLoader: DefaultStoreLoader, | ||
router: NewRouter(), | ||
queryRouter: NewQueryRouter(), | ||
grpcQueryRouter: NewGRPCQueryRouter(), | ||
msgServiceRouter: NewMsgServiceRouter(), | ||
|
@@ -367,17 +364,6 @@ func (app *BaseApp) setIndexEvents(ie []string) { | |
} | ||
} | ||
|
||
// Router returns the legacy router of the BaseApp. | ||
func (app *BaseApp) Router() sdk.Router { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We probably should have added an API-breaking changelog for this baseapp removal |
||
if app.sealed { | ||
// We cannot return a Router when the app is sealed because we can't have | ||
// any routes modified which would cause unexpected routing behavior. | ||
panic("Router() on sealed BaseApp") | ||
} | ||
|
||
return app.router | ||
} | ||
|
||
// QueryRouter returns the QueryRouter of a BaseApp. | ||
func (app *BaseApp) QueryRouter() sdk.QueryRouter { return app.queryRouter } | ||
|
||
|
@@ -749,20 +735,6 @@ func (app *BaseApp) runMsgs(ctx sdk.Context, msgs []sdk.Msg, mode runTxMode) (*s | |
// ADR 031 request type routing | ||
msgResult, err = handler(ctx, msg) | ||
eventMsgName = sdk.MsgTypeURL(msg) | ||
} else if legacyMsg, ok := msg.(legacytx.LegacyMsg); ok { | ||
// legacy sdk.Msg routing | ||
// Assuming that the app developer has migrated all their Msgs to | ||
// proto messages and has registered all `Msg services`, then this | ||
// path should never be called, because all those Msgs should be | ||
// registered within the `msgServiceRouter` already. | ||
msgRoute := legacyMsg.Route() | ||
eventMsgName = legacyMsg.Type() | ||
handler := app.router.Route(ctx, msgRoute) | ||
if handler == nil { | ||
return nil, sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "unrecognized message route: %s; message index: %d", msgRoute, i) | ||
} | ||
|
||
msgResult, err = handler(ctx, msg) | ||
} else { | ||
return nil, sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "can't route message %+v", msg) | ||
} | ||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at this, the legacy query router can also be removed, since #12725. @facundomedica would you like to take care of this too?