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.

Sign up for GitHub

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

Jump to bottom

feat(x/accounts): use router service from env #20003

Merged
merged 12 commits into from
Apr 15, 2024
Merged

feat(x/accounts): use router service from env #20003

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
dc130eb
feat(x/accounts): use router service from env
julienrbrt Apr 10, 2024
9d1d193
wiring
julienrbrt Apr 10, 2024
da19302
build
julienrbrt Apr 10, 2024
8b00811
Merge branch 'main' into julien/accounts-env
julienrbrt Apr 11, 2024
ca1be85
fix after merge
julienrbrt Apr 11, 2024
b4d12c1
Merge branch 'main' into julien/accounts-env
julienrbrt Apr 12, 2024
d5f61f7
Merge branch 'main' into julien/accounts-env
julienrbrt Apr 14, 2024
1b82624
Merge branch 'main' into julien/accounts-env
julienrbrt Apr 15, 2024
966a745
fixes
julienrbrt Apr 15, 2024
6527b05
Merge branch 'main' into julien/accounts-env
julienrbrt Apr 15, 2024
febc15f
fix api
julienrbrt Apr 15, 2024
cf2d7d4
fixes
julienrbrt Apr 15, 2024
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
4 changes: 1 addition & 3 deletions simapp/app.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -287,10 +287,8 @@ func NewSimApp(
// add keepers
accountsKeeper, err := accounts.NewKeeper(
appCodec,
runtime.NewEnvironment(runtime.NewKVStoreService(keys[accounts.StoreKey]), logger),
runtime.NewEnvironment(runtime.NewKVStoreService(keys[accounts.StoreKey]), logger, runtime.EnvWithRouterService(app.GRPCQueryRouter(), app.MsgServiceRouter())),
signingCtx.AddressCodec(),
app.MsgServiceRouter(),
app.GRPCQueryRouter(),
appCodec.InterfaceRegistry(),
// TESTING: do not add
accountstd.AddAccount("counter", counter.NewAccount),
Expand Down
9 changes: 4 additions & 5 deletions tests/integration/auth/keeper/msg_server_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package keeper_test

import (
"context"
"fmt"
"strings"
"testing"

Expand Down Expand Up @@ -84,10 +85,8 @@ func initFixture(t *testing.T) *fixture {
account := baseaccount.NewAccount("base", signing.NewHandlerMap(handler))
accountsKeeper, err := accounts.NewKeeper(
cdc,
runtime.NewEnvironment(runtime.NewKVStoreService(keys[accounts.StoreKey]), log.NewNopLogger()),
runtime.NewEnvironment(runtime.NewKVStoreService(keys[accounts.StoreKey]), log.NewNopLogger(), runtime.EnvWithRouterService(queryRouter, router)),
addresscodec.NewBech32Codec("cosmos"),
router,
queryRouter,
cdc.InterfaceRegistry(),
account,
)
Expand Down Expand Up @@ -158,7 +157,7 @@ func TestAsyncExec(t *testing.T) {
addrs := simtestutil.CreateIncrementalAccounts(2)
coins := sdk.NewCoins(sdk.NewCoin("stake", sdkmath.NewInt(10)))

assert.NilError(t, testutil.FundAccount(f.ctx, f.bankKeeper, addrs[0], sdk.NewCoins(sdk.NewInt64Coin("stake", 50))))
assert.NilError(t, testutil.FundAccount(f.ctx, f.bankKeeper, addrs[0], sdk.NewCoins(sdk.NewInt64Coin("stake", 500))))

msg := &banktypes.MsgSend{
FromAddress: addrs[0].String(),
Expand Down Expand Up @@ -276,7 +275,7 @@ func TestAsyncExec(t *testing.T) {
if tc.expErrMsg != "" {
for _, res := range result.Results {
if res.Error != "" {
assert.Assert(t, strings.Contains(res.Error, tc.expErrMsg))
assert.Assert(t, strings.Contains(res.Error, tc.expErrMsg), fmt.Sprintf("res.Error %s does not contain %s", res.Error, tc.expErrMsg))
}
continue
}
Expand Down
10 changes: 4 additions & 6 deletions x/accounts/depinject.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,10 @@ type ModuleInputs struct {
Cdc codec.Codec
Environment appmodule.Environment
AddressCodec address.Codec
ExecRouter MsgRouter
QueryRouter QueryRouter
Registry cdctypes.InterfaceRegistry

// TODO: Add a way to inject custom accounts.
// Currently only the base account is supported.
Comment on lines +39 to +40
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider providing more details or a timeline in the TODO comment.

Would you like assistance in defining a more detailed plan or creating a tracking issue for this TODO?

}

type ModuleOutputs struct {
Expand All @@ -61,10 +62,7 @@ func (s directHandler) GetSignBytes(_ context.Context, _ signing.SignerData, _ s
func ProvideModule(in ModuleInputs) ModuleOutputs {
handler := directHandler{}
account := baseaccount.NewAccount("base", signing.NewHandlerMap(handler))
accountskeeper, err := NewKeeper(
in.Cdc, in.Environment, in.AddressCodec,
in.ExecRouter, in.QueryRouter, in.Registry, account,
)
accountskeeper, err := NewKeeper(in.Cdc, in.Environment, in.AddressCodec, in.Registry, account)
if err != nil {
panic(err)
}
Expand Down
2 changes: 0 additions & 2 deletions x/accounts/genesis_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package accounts

import (
"context"
"testing"

"github.com/cosmos/gogoproto/types"
Expand All @@ -16,7 +15,6 @@ func TestGenesis(t *testing.T) {
acc, err := NewTestAccount(deps)
return "test", acc, err
})
k.queryRouter = mockQuery(func(ctx context.Context, req, resp implementation.ProtoMsg) error { return nil })
// we init two accounts of the same type

// we set counter to 10
Expand Down
48 changes: 5 additions & 43 deletions x/accounts/keeper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,19 +35,6 @@ var (
AccountByNumber = collections.NewPrefix(2)
)

// QueryRouter represents a router which can be used to route queries to the correct module.
// It returns the handler given the message name, if multiple handlers are returned, then
// it is up to the caller to choose which one to call.
type QueryRouter interface {
HybridHandlerByRequestName(name string) []func(ctx context.Context, req, resp implementation.ProtoMsg) error
}

// MsgRouter represents a router which can be used to route messages to the correct module.
type MsgRouter interface {
HybridHandlerByMsgName(msgName string) func(ctx context.Context, req, resp implementation.ProtoMsg) error
ResponseNameByMsgName(name string) string
}

type InterfaceRegistry interface {
RegisterInterface(name string, iface any, impls ...protoiface.MessageV1)
RegisterImplementations(iface any, impls ...protoiface.MessageV1)
Expand All @@ -57,18 +44,14 @@ func NewKeeper(
cdc codec.Codec,
env appmodule.Environment,
addressCodec address.Codec,
execRouter MsgRouter,
queryRouter QueryRouter,
ir InterfaceRegistry,
accounts ...accountstd.AccountCreatorFunc,
) (Keeper, error) {
sb := collections.NewSchemaBuilder(env.KVStoreService)
keeper := Keeper{
environment: env,
addressCodec: addressCodec,
msgRouter: execRouter,
codec: cdc,
queryRouter: queryRouter,
addressCodec: addressCodec,
makeSendCoinsMsg: defaultCoinsTransferMsgFunc(addressCodec),
Schema: collections.Schema{},
AccountNumber: collections.NewSequence(sb, AccountNumberKey, "account_number"),
Expand All @@ -95,8 +78,6 @@ type Keeper struct {
environment appmodule.Environment
addressCodec address.Codec
codec codec.Codec
msgRouter MsgRouter // todo use env
queryRouter QueryRouter // todo use env
makeSendCoinsMsg coinsTransferMsgFunc

accounts map[string]implementation.Implementation
Expand Down Expand Up @@ -343,17 +324,11 @@ func (k Keeper) sendAnyMessages(ctx context.Context, sender []byte, anyMessages
// SendModuleMessageUntyped can be used to send a message towards a module.
// It should be used when the response type is not known by the caller.
func (k Keeper) SendModuleMessageUntyped(ctx context.Context, sender []byte, msg implementation.ProtoMsg) (implementation.ProtoMsg, error) {
// we need to fetch the response type from the request message type.
// this is because the response type is not known.
respName := k.msgRouter.ResponseNameByMsgName(implementation.MessageName(msg))
if respName == "" {
return nil, fmt.Errorf("could not find response type for message %T", msg)
}
// get response type
resp, err := implementation.FindMessageByName(respName)
resp, err := k.environment.RouterService.MessageRouterService().InvokeUntyped(ctx, msg)
if err != nil {
return nil, err
}

// send the message
return resp, k.sendModuleMessage(ctx, sender, msg, resp)
}
Expand All @@ -373,27 +348,14 @@ func (k Keeper) sendModuleMessage(ctx context.Context, sender []byte, msg, msgRe
if !bytes.Equal(sender, wantSenders[0]) {
return fmt.Errorf("%w: sender does not match expected sender", ErrUnauthorized)
}
messageName := implementation.MessageName(msg)
handler := k.msgRouter.HybridHandlerByMsgName(messageName)
if handler == nil {
return fmt.Errorf("unknown message: %s", messageName)
}
return handler(ctx, msg, msgResp)
return k.environment.RouterService.MessageRouterService().InvokeTyped(ctx, msg, msgResp)
}

// queryModule is the entrypoint for an account to query a module.
// It will try to find the query handler for the given query and execute it.
// If multiple query handlers are found, it will return an error.
func (k Keeper) queryModule(ctx context.Context, queryReq, queryResp implementation.ProtoMsg) error {
queryName := implementation.MessageName(queryReq)
handlers := k.queryRouter.HybridHandlerByRequestName(queryName)
if len(handlers) == 0 {
return fmt.Errorf("unknown query: %s", queryName)
}
if len(handlers) > 1 {
return fmt.Errorf("multiple handlers for query: %s", queryName)
}
return handlers[0](ctx, queryReq, queryResp)
return k.environment.RouterService.QueryRouterService().InvokeTyped(ctx, queryReq, queryResp)
}

// maybeSendFunds will send the provided coins between the provided addresses, if amt
Expand Down
54 changes: 3 additions & 51 deletions x/accounts/keeper_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,29 +1,18 @@
package accounts
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tip

Codebase Verification

The removed mock functions mockQuery and mockExec from x/accounts/keeper_test.go are indeed referenced in other test files within the repository (x/bank/keeper/grpc_query_test.go and x/bank/keeper/keeper_test.go). This indicates that the elimination of these mock functions could potentially affect other test functionalities or dependencies within the test suite.

Analysis chain

Confirm that the elimination of redundant mock functions does not affect other test functionalities or dependencies within the test suite.

Scripts Executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for any references to eliminated mock functions in the test files to ensure they are not used elsewhere.
rg --type go --files-with-matches --glob '*_test.go' 'mock'

Length of output: 3896

Script:

#!/bin/bash
# First, identify the specific mock functions that were removed from x/accounts/keeper_test.go
git diff origin/main...HEAD -- x/accounts/keeper_test.go | grep -P '^\-.*mock.*\('

# Then, check if these specific mock functions are referenced in other test files
rg --type go --files-with-matches --glob '*_test.go' 'specific_mock_function_name_here'

Length of output: 673

Script:

#!/bin/bash
# Search for references to 'mockQuery' and 'mockExec' in the test files to ensure they are not used elsewhere.
rg --type go --files-with-matches --glob '*_test.go' 'mockQuery|mockExec'

Length of output: 137


import (
"context"
"testing"

"github.com/cosmos/gogoproto/types"
"github.com/stretchr/testify/require"
"google.golang.org/protobuf/proto"

bankv1beta1 "cosmossdk.io/api/cosmos/bank/v1beta1"
basev1beta1 "cosmossdk.io/api/cosmos/base/v1beta1"
"cosmossdk.io/collections"
"cosmossdk.io/x/accounts/accountstd"
"cosmossdk.io/x/accounts/internal/implementation"
)

func TestKeeper_Init(t *testing.T) {
m, ctx := newKeeper(t, accountstd.AddAccount("test", NewTestAccount))
m.queryRouter = mockQuery(func(ctx context.Context, req, resp implementation.ProtoMsg) error {
_, ok := req.(*bankv1beta1.QueryBalanceRequest)
require.True(t, ok)
_, ok = resp.(*bankv1beta1.QueryBalanceResponse)
require.True(t, ok)
return nil
})

t.Run("ok", func(t *testing.T) {
sender := []byte("sender")
Expand Down Expand Up @@ -52,7 +41,6 @@ func TestKeeper_Init(t *testing.T) {

func TestKeeper_Execute(t *testing.T) {
m, ctx := newKeeper(t, accountstd.AddAccount("test", NewTestAccount))
m.queryRouter = mockQuery(func(ctx context.Context, req, resp implementation.ProtoMsg) error { return nil })

// create account
sender := []byte("sender")
Expand All @@ -71,35 +59,14 @@ func TestKeeper_Execute(t *testing.T) {
})

t.Run("exec module", func(t *testing.T) {
m.msgRouter = mockExec(func(ctx context.Context, msg, msgResp implementation.ProtoMsg) error {
concrete, ok := msg.(*bankv1beta1.MsgSend)
require.True(t, ok)
require.Equal(t, concrete.ToAddress, "recipient")
_, ok = msgResp.(*bankv1beta1.MsgSendResponse)
require.True(t, ok)
return nil
})

m.msgRouter = mockExec(func(ctx context.Context, msg, msgResp implementation.ProtoMsg) error {
concrete, ok := msg.(*bankv1beta1.MsgSend)
require.True(t, ok)
require.Equal(t, concrete.FromAddress, string(accAddr))
_, ok = msgResp.(*bankv1beta1.MsgSendResponse)
require.True(t, ok)

resp, err := m.Execute(ctx, accAddr, sender, &types.Int64Value{Value: 1000}, nil)
require.NoError(t, err)
require.True(t, implementation.Equal(&types.Empty{}, resp))
return nil
})
resp, err := m.Execute(ctx, accAddr, sender, &types.Int64Value{Value: 1000}, nil)
require.NoError(t, err)
require.True(t, implementation.Equal(&types.Empty{}, resp))
})
}

func TestKeeper_Query(t *testing.T) {
m, ctx := newKeeper(t, accountstd.AddAccount("test", NewTestAccount))
m.queryRouter = mockQuery(func(ctx context.Context, req, resp implementation.ProtoMsg) error {
return nil
})

// create account
sender := []byte("sender")
Expand All @@ -118,21 +85,6 @@ func TestKeeper_Query(t *testing.T) {
})

t.Run("query module", func(t *testing.T) {
// we inject the module query function, which accepts only a specific type of message
// we force the response
m.queryRouter = mockQuery(func(ctx context.Context, req, resp implementation.ProtoMsg) error {
concrete, ok := req.(*bankv1beta1.QueryBalanceRequest)
require.True(t, ok)
require.Equal(t, string(accAddr), concrete.Address)
require.Equal(t, concrete.Denom, "atom")
copyResp := &bankv1beta1.QueryBalanceResponse{Balance: &basev1beta1.Coin{
Denom: "atom",
Amount: "1000",
}}
proto.Merge(resp.(proto.Message), copyResp)
return nil
})

resp, err := m.Query(ctx, accAddr, &types.StringValue{Value: "atom"})
require.NoError(t, err)
require.True(t, implementation.Equal(&types.Int64Value{Value: 1000}, resp))
Expand Down
10 changes: 0 additions & 10 deletions x/accounts/msg_server_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,29 +1,19 @@
package accounts

import (
"context"
"testing"

"github.com/stretchr/testify/require"
"google.golang.org/protobuf/proto"
"google.golang.org/protobuf/types/known/emptypb"
"google.golang.org/protobuf/types/known/wrapperspb"

bankv1beta1 "cosmossdk.io/api/cosmos/bank/v1beta1"
"cosmossdk.io/x/accounts/accountstd"
"cosmossdk.io/x/accounts/internal/implementation"
v1 "cosmossdk.io/x/accounts/v1"
)

func TestMsgServer(t *testing.T) {
k, ctx := newKeeper(t, accountstd.AddAccount("test", NewTestAccount))
k.queryRouter = mockQuery(func(ctx context.Context, req, resp implementation.ProtoMsg) error {
_, ok := req.(*bankv1beta1.QueryBalanceRequest)
require.True(t, ok)
proto.Merge(resp.(proto.Message), &bankv1beta1.QueryBalanceResponse{})
return nil
})

s := NewMsgServer(k)

// create
Expand Down
4 changes: 0 additions & 4 deletions x/accounts/query_server_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package accounts

import (
"context"
"testing"

"github.com/cosmos/gogoproto/types"
Expand All @@ -16,9 +15,6 @@ import (

func TestQueryServer(t *testing.T) {
k, ctx := newKeeper(t, accountstd.AddAccount("test", NewTestAccount))
k.queryRouter = mockQuery(func(ctx context.Context, req, resp implementation.ProtoMsg) error {
return nil
})

ms := NewMsgServer(k)
qs := NewQueryServer(k)
Expand Down
Loading
Loading