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

refactor!: Remove sdk.Router and refactor baseapp tests #13005

Merged
merged 41 commits into from
Aug 30, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
6cd4d17
test
facundomedica Aug 21, 2022
ab5c482
progress-ish
facundomedica Aug 23, 2022
e6bfe51
progress
facundomedica Aug 23, 2022
3442d41
progress
facundomedica Aug 23, 2022
9477892
make mocks
facundomedica Aug 23, 2022
1d1fc8a
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into facu…
facundomedica Aug 23, 2022
8e83997
progress
facundomedica Aug 23, 2022
01850f8
test
facundomedica Aug 24, 2022
6e1f828
progress
facundomedica Aug 24, 2022
8b19f6e
progress
facundomedica Aug 24, 2022
21db64f
Merge branch 'main' into facu/remove-router
facundomedica Aug 24, 2022
08ae27b
Merge branch 'facu/remove-router' of https://github.com/cosmos/cosmos…
facundomedica Aug 24, 2022
525449f
Merge branch 'main' into facu/remove-router
facundomedica Aug 24, 2022
1689eea
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into facu…
facundomedica Aug 24, 2022
59a8733
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into facu…
facundomedica Aug 24, 2022
e57a4e6
progress
facundomedica Aug 24, 2022
ce6927c
progress
facundomedica Aug 24, 2022
284bfe9
progress
facundomedica Aug 24, 2022
36587ad
Merge branch 'facu/remove-router' of https://github.com/cosmos/cosmos…
facundomedica Aug 25, 2022
236183e
progress
facundomedica Aug 25, 2022
e573754
progress
facundomedica Aug 25, 2022
6887aa5
fix mock tests
facundomedica Aug 26, 2022
efce7f9
progress
facundomedica Aug 26, 2022
1ec7379
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into facu…
facundomedica Aug 29, 2022
105dcdc
pretty much done, might need some tidying up
facundomedica Aug 29, 2022
056b5eb
lint
facundomedica Aug 29, 2022
27b73d3
re-enable blockgas test
facundomedica Aug 29, 2022
f0877f7
Merge branch 'main' into facu/remove-router
facundomedica Aug 29, 2022
33c3c1b
remove router
facundomedica Aug 29, 2022
428b98c
Merge branch 'facu/remove-router' of https://github.com/cosmos/cosmos…
facundomedica Aug 29, 2022
d91ed4a
gofumpt
facundomedica Aug 29, 2022
c0ae7f3
remove more references of Router
facundomedica Aug 29, 2022
3855afc
fix
facundomedica Aug 29, 2022
27383fc
Merge branch 'main' into facu/remove-router
facundomedica Aug 30, 2022
4f2d76a
remove unused code
facundomedica Aug 30, 2022
5284a5d
Merge branch 'facu/remove-router' of https://github.com/cosmos/cosmos…
facundomedica Aug 30, 2022
75f7d2f
remove unused code
facundomedica Aug 30, 2022
cce106f
Merge branch 'main' into facu/remove-router
facundomedica Aug 30, 2022
32a1452
Merge branch 'main' into facu/remove-router
facundomedica Aug 30, 2022
c3c767a
Merge branch 'main' into facu/remove-router
tac0turtle Aug 30, 2022
a1f2db0
Merge branch 'main' into facu/remove-router
facundomedica Aug 30, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 45 additions & 0 deletions baseapp/abci_test.go
Original file line number Diff line number Diff line change
@@ -1,19 +1,27 @@
package baseapp

import (
"encoding/json"
"os"
"testing"

"github.com/stretchr/testify/require"
abci "github.com/tendermint/tendermint/abci/types"
"github.com/tendermint/tendermint/libs/log"
tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
dbm "github.com/tendermint/tm-db"

pruningtypes "github.com/cosmos/cosmos-sdk/pruning/types"
"github.com/cosmos/cosmos-sdk/snapshots"
snapshottypes "github.com/cosmos/cosmos-sdk/snapshots/types"
"github.com/cosmos/cosmos-sdk/testutil"
sdk "github.com/cosmos/cosmos-sdk/types"
)

func defaultLogger() log.Logger {
return log.NewTMLogger(log.NewSyncWriter(os.Stdout)).With("module", "sdk/app")
}

func TestGetBlockRentionHeight(t *testing.T) {
logger := defaultLogger()
db := dbm.NewMemDB()
Expand Down Expand Up @@ -164,3 +172,40 @@ func TestBaseAppCreateQueryContext(t *testing.T) {
})
}
}

type paramStore struct {
db *dbm.MemDB
}

func (ps *paramStore) Set(_ sdk.Context, key []byte, value interface{}) {
bz, err := json.Marshal(value)
if err != nil {
panic(err)
}

ps.db.Set(key, bz)
}

func (ps *paramStore) Has(_ sdk.Context, key []byte) bool {
ok, err := ps.db.Has(key)
if err != nil {
panic(err)
}

return ok
}

func (ps *paramStore) Get(_ sdk.Context, key []byte, ptr interface{}) {
bz, err := ps.db.Get(key)
if err != nil {
panic(err)
}

if len(bz) == 0 {
return
}

if err := json.Unmarshal(bz, ptr); err != nil {
panic(err)
}
}
28 changes: 0 additions & 28 deletions baseapp/baseapp.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -149,7 +147,6 @@ func NewBaseApp(
db: db,
cms: store.NewCommitMultiStore(db),
storeLoader: DefaultStoreLoader,
router: NewRouter(),
queryRouter: NewQueryRouter(),
Copy link
Contributor

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?

grpcQueryRouter: NewGRPCQueryRouter(),
msgServiceRouter: NewMsgServiceRouter(),
Expand Down Expand Up @@ -367,17 +364,6 @@ func (app *BaseApp) setIndexEvents(ie []string) {
}
}

// Router returns the legacy router of the BaseApp.
func (app *BaseApp) Router() sdk.Router {
Copy link
Contributor

Choose a reason for hiding this comment

The 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 }

Expand Down Expand Up @@ -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)
}
Expand Down
Loading