Skip to content

Commit

Permalink
Separate account getters from client/context (#4579)
Browse files Browse the repository at this point in the history
Account getters are removed from client context. x/auth has the
queriers necessary for retrieving account information.
These functions should be removed since they are currently
redundant and don't provide any extra value.

Closes: #4543
  • Loading branch information
Alessio Treglia authored Jun 19, 2019
1 parent 84a2582 commit 1eb7706
Show file tree
Hide file tree
Showing 13 changed files with 184 additions and 109 deletions.
2 changes: 2 additions & 0 deletions .pending/breaking/sdk/4543-Account-getters
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
#4543 Account getters are no longer part of client.CLIContext() and have now moved
to reside in the auth-specific AccountRetriever.
9 changes: 8 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ COMMIT := $(shell git log -1 --format='%H')
LEDGER_ENABLED ?= true
BINDIR ?= $(GOPATH)/bin
SIMAPP = github.com/cosmos/cosmos-sdk/simapp
MOCKS_DIR = $(CURDIR)/tests/mocks

export GO111MODULE = on

Expand All @@ -33,6 +34,12 @@ dist:
@bash publish/dist.sh
@bash publish/publish.sh

mocks: $(MOCKS_DIR)
mockgen -source=x/auth/types/account_retriever.go -package mocks -destination tests/mocks/account_retriever.go

$(MOCKS_DIR):
mkdir -p $(MOCKS_DIR)

########################################
### Tools & dependencies

Expand Down Expand Up @@ -186,7 +193,7 @@ snapcraft-local.yaml: snapcraft-local.yaml.in
# To avoid unintended conflicts with file names, always add to .PHONY
# unless there is a reason not to.
# https://www.gnu.org/software/make/manual/html_node/Phony-Targets.html
.PHONY: build dist clean test test_unit test_cover lint \
.PHONY: build dist clean test test_unit test_cover lint mocks \
benchmark devdoc_init devdoc devdoc_save devdoc_update runsim \
format test_sim_app_nondeterminism test_sim_modules test_sim_app_fast \
test_sim_app_custom_genesis_fast test_sim_app_custom_genesis_multi_seed \
Expand Down
76 changes: 0 additions & 76 deletions client/context/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import (

"github.com/cosmos/cosmos-sdk/store/rootmulti"
sdk "github.com/cosmos/cosmos-sdk/types"
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
)

// GetNode returns an RPC client. If the context's client is not defined, an
Expand Down Expand Up @@ -63,26 +62,6 @@ func (ctx CLIContext) QuerySubspace(subspace []byte, storeName string) (res []sd
return
}

// GetAccount queries for an account given an address and a block height. An
// error is returned if the query or decoding fails.
func (ctx CLIContext) GetAccount(address []byte) (authtypes.Account, error) {
if ctx.AccDecoder == nil {
return nil, errors.New("account decoder required but not provided")
}

res, err := ctx.queryAccount(address)
if err != nil {
return nil, err
}

var account authtypes.Account
if err := ctx.Codec.UnmarshalJSON(res, &account); err != nil {
return nil, err
}

return account, nil
}

// GetFromAddress returns the from address from the context's name.
func (ctx CLIContext) GetFromAddress() sdk.AccAddress {
return ctx.FromAddress
Expand All @@ -93,61 +72,6 @@ func (ctx CLIContext) GetFromName() string {
return ctx.FromName
}

// GetAccountNumber returns the next account number for the given account
// address.
func (ctx CLIContext) GetAccountNumber(address []byte) (uint64, error) {
account, err := ctx.GetAccount(address)
if err != nil {
return 0, err
}

return account.GetAccountNumber(), nil
}

// GetAccountSequence returns the sequence number for the given account
// address.
func (ctx CLIContext) GetAccountSequence(address []byte) (uint64, error) {
account, err := ctx.GetAccount(address)
if err != nil {
return 0, err
}

return account.GetSequence(), nil
}

// EnsureAccountExists ensures that an account exists for a given context. An
// error is returned if it does not.
func (ctx CLIContext) EnsureAccountExists() error {
addr := ctx.GetFromAddress()
return ctx.EnsureAccountExistsFromAddr(addr)
}

// EnsureAccountExistsFromAddr ensures that an account exists for a given
// address. Instead of using the context's from name, a direct address is
// given. An error is returned if it does not.
func (ctx CLIContext) EnsureAccountExistsFromAddr(addr sdk.AccAddress) error {
_, err := ctx.queryAccount(addr)
return err
}

// queryAccount queries an account using custom query endpoint of auth module
// returns an error if result is `null` otherwise account data
func (ctx CLIContext) queryAccount(addr sdk.AccAddress) ([]byte, error) {
bz, err := ctx.Codec.MarshalJSON(authtypes.NewQueryAccountParams(addr))
if err != nil {
return nil, err
}

route := fmt.Sprintf("custom/%s/%s", ctx.AccountStore, authtypes.QueryAccount)

res, _, err := ctx.query(route, bz)
if err != nil {
return nil, err
}

return res, nil
}

// query performs a query to a Tendermint node with the provided store name
// and path. It returns the result and height of the query upon success
// or an error if the query fails.
Expand Down
3 changes: 2 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ require (
github.com/fortytw2/leaktest v1.3.0 // indirect
github.com/go-logfmt/logfmt v0.4.0 // indirect
github.com/gogo/protobuf v1.1.1
github.com/golang/mock v1.3.1-0.20190508161146-9fa652df1129
github.com/golang/protobuf v1.3.0
github.com/golang/snappy v0.0.1 // indirect
github.com/gorilla/mux v1.7.0
Expand Down Expand Up @@ -41,7 +42,7 @@ require (
github.com/tendermint/go-amino v0.15.0
github.com/tendermint/iavl v0.12.2
github.com/tendermint/tendermint v0.31.5
golang.org/x/crypto v0.0.0-20180904163835-0709b304e793
golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2
google.golang.org/grpc v1.19.0 // indirect
gopkg.in/yaml.v2 v2.2.2
)
Expand Down
8 changes: 8 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,10 @@ github.com/gogo/protobuf v1.1.1 h1:72R+M5VuhED/KujmZVcIquuo8mBgX4oVda//DQb3PXo=
github.com/gogo/protobuf v1.1.1/go.mod h1:r8qH/GZQm5c6nD/R0oafs1akxWv10x8SbQlK7atdtwQ=
github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b h1:VKtxabqXZkF25pY9ekfRL6a582T4P37/31XEstQ5p58=
github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q=
github.com/golang/mock v1.1.1 h1:G5FRp8JnTd7RQH5kemVNlMeyXQAztQ3mOWV95KxsXH8=
github.com/golang/mock v1.1.1/go.mod h1:oTYuIxOrZwtPieC+H1uAHpcLFnEyAGVDL/k47Jfbm0A=
github.com/golang/mock v1.3.1-0.20190508161146-9fa652df1129 h1:tT8iWCYw4uOem71yYA3htfH+LNopJvcqZQshm56G5L4=
github.com/golang/mock v1.3.1-0.20190508161146-9fa652df1129/go.mod h1:sBzyDLLjw3U8JLTeZvSv8jJB+tU5PVekmnlKIyFUx0Y=
github.com/golang/protobuf v1.2.0 h1:P3YflyNX/ehuJFLhxviNdFxQPkGK5cDcApsge1SqnvM=
github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U=
github.com/golang/protobuf v1.3.0 h1:kbxbvI4Un1LUWKxufD+BiE6AEExYYgkQLQmLFqA1LFk=
Expand Down Expand Up @@ -158,13 +161,17 @@ golang.org/x/net v0.0.0-20180906233101-161cd47e91fd/go.mod h1:mL1N/T3taQHkDXs73r
golang.org/x/net v0.0.0-20181114220301-adae6a3d119a/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
golang.org/x/net v0.0.0-20181201002055-351d144fa1fc h1:a3CU5tJYVj92DY2LaA1kUkrsqD5/3mLDhx2NcNqyW+0=
golang.org/x/net v0.0.0-20181201002055-351d144fa1fc/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
golang.org/x/net v0.0.0-20190311183353-d8887717615a h1:oWX7TPOiFAMXLq8o0ikBYfCJVlRHBcsciT5bXOrH628=
golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=
golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U=
golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.0.0-20181108010431-42b317875d0f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4 h1:YUO/7uOKsKeq9UokNS62b8FYywz3ker1l1vDZRCRefw=
golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.0.0-20190227155943-e225da77a7e6 h1:bjcUS9ztw9kFmmIxJInhon/0Is3p+EHBKNgquIzo1OI=
golang.org/x/sync v0.0.0-20190227155943-e225da77a7e6/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.0.0-20190423024810-112230192c58 h1:8gQV6CLnAEikrhgkHFbMAEhagSSnXWGV915qUMm9mrU=
golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sys v0.0.0-20180830151530-49385e6e1522/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20180905080454-ebe1bf3edb33/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20180909124046-d0be0721c37e/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
Expand All @@ -175,6 +182,7 @@ golang.org/x/text v0.3.0 h1:g61tztE5qeGQ89tm6NTjjM9VPIm088od1l6aSorWRWg=
golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
golang.org/x/tools v0.0.0-20190114222345-bf090417da8b h1:qMK98NmNCRVDIYFycQ5yVRkvgDUFfdP8Ip4KqmDEB7g=
golang.org/x/tools v0.0.0-20190114222345-bf090417da8b/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
golang.org/x/tools v0.0.0-20190425150028-36563e24a262/go.mod h1:RgjU9mgBXZiqYHBnxXauZ1Gv1EHHAz9KjViQ78xBX0Q=
google.golang.org/appengine v1.1.0/go.mod h1:EbEs0AVv82hx2wNQdGPgUI5lhzA/G0D9YwlJXL52JkM=
google.golang.org/genproto v0.0.0-20180817151627-c66870c02cf8 h1:Nw54tB0rB7hY/N0NQvRW8DG4Yk3Q6T9cu9RcFQDu1tc=
google.golang.org/genproto v0.0.0-20180817151627-c66870c02cf8/go.mod h1:JiN7NxoALGmiZfu7CAH4rXhgtRTLTxftemlI0sWmxmc=
Expand Down
39 changes: 39 additions & 0 deletions tests/mocks/account_retriever.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package mocks

import (
gomock "github.com/golang/mock/gomock"
reflect "reflect"
)

type MockNodeQuerier struct {
ctrl *gomock.Controller
recorder *MockNodeQuerierMockRecorder
}

type MockNodeQuerierMockRecorder struct {
mock *MockNodeQuerier
}

func NewMockNodeQuerier(ctrl *gomock.Controller) *MockNodeQuerier {
mock := &MockNodeQuerier{ctrl: ctrl}
mock.recorder = &MockNodeQuerierMockRecorder{mock}
return mock
}

func (m *MockNodeQuerier) EXPECT() *MockNodeQuerierMockRecorder {
return m.recorder
}

func (m *MockNodeQuerier) QueryWithData(path string, data []byte) ([]byte, int64, error) {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "QueryWithData", path, data)
ret0, _ := ret[0].([]byte)
ret1, _ := ret[1].(int64)
ret2, _ := ret[2].(error)
return ret0, ret1, ret2
}

func (mr *MockNodeQuerierMockRecorder) QueryWithData(path, data interface{}) *gomock.Call {
mr.mock.ctrl.T.Helper()
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "QueryWithData", reflect.TypeOf((*MockNodeQuerier)(nil).QueryWithData), path, data)
}
1 change: 1 addition & 0 deletions x/auth/alias.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ var (
NewTxBuilder = types.NewTxBuilder
NewTxBuilderFromCLI = types.NewTxBuilderFromCLI
MakeSignature = types.MakeSignature
NewAccountRetriever = types.NewAccountRetriever

// variable aliases
ModuleCdc = types.ModuleCdc
Expand Down
5 changes: 3 additions & 2 deletions x/auth/client/cli/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,17 +50,18 @@ func GetAccountCmd(cdc *codec.Codec) *cobra.Command {
RunE: func(cmd *cobra.Command, args []string) error {
cliCtx := context.NewCLIContext().
WithCodec(cdc).WithAccountDecoder(cdc)
accGetter := types.NewAccountRetriever(cliCtx)

key, err := sdk.AccAddressFromBech32(args[0])
if err != nil {
return err
}

if err = cliCtx.EnsureAccountExistsFromAddr(key); err != nil {
if err := accGetter.EnsureExists(key); err != nil {
return err
}

acc, err := cliCtx.GetAccount(key)
acc, err := accGetter.GetAccount(key)
if err != nil {
return err
}
Expand Down
8 changes: 1 addition & 7 deletions x/auth/client/cli/tx_multisign.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,13 +84,7 @@ func makeMultiSignCmd(cdc *codec.Codec) func(cmd *cobra.Command, args []string)
txBldr := types.NewTxBuilderFromCLI()

if !viper.GetBool(flagOffline) {
addr := multisigInfo.GetAddress()
accnum, err := cliCtx.GetAccountNumber(addr)
if err != nil {
return err
}

seq, err := cliCtx.GetAccountSequence(addr)
accnum, seq, err := types.NewAccountRetriever(cliCtx).GetAccountNumberSequence(multisigInfo.GetAddress())
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion x/auth/client/cli/tx_sign.go
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ func printAndValidateSigs(
// Validate the actual signature over the transaction bytes since we can
// reach out to a full node to query accounts.
if !offline && success {
acc, err := cliCtx.GetAccount(sigAddr)
acc, err := types.NewAccountRetriever(cliCtx).GetAccount(sigAddr)
if err != nil {
fmt.Printf("failed to get account: %s\n", sigAddr)
return false
Expand Down
35 changes: 14 additions & 21 deletions x/auth/client/utils/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -251,17 +251,12 @@ func populateAccountFromState(
txBldr authtypes.TxBuilder, cliCtx context.CLIContext, addr sdk.AccAddress,
) (authtypes.TxBuilder, error) {

accNum, err := cliCtx.GetAccountNumber(addr)
num, seq, err := authtypes.NewAccountRetriever(cliCtx).GetAccountNumberSequence(addr)
if err != nil {
return txBldr, err
}

accSeq, err := cliCtx.GetAccountSequence(addr)
if err != nil {
return txBldr, err
}

return txBldr.WithAccountNumber(accNum).WithSequence(accSeq), nil
return txBldr.WithAccountNumber(num).WithSequence(seq), nil
}

// GetTxEncoder return tx encoder from global sdk configuration if ones is defined.
Expand Down Expand Up @@ -302,30 +297,28 @@ func parseQueryResponse(cdc *codec.Codec, rawRes []byte) (uint64, error) {

// PrepareTxBuilder populates a TxBuilder in preparation for the build of a Tx.
func PrepareTxBuilder(txBldr authtypes.TxBuilder, cliCtx context.CLIContext) (authtypes.TxBuilder, error) {
if err := cliCtx.EnsureAccountExists(); err != nil {
from := cliCtx.GetFromAddress()

accGetter := authtypes.NewAccountRetriever(cliCtx)
if err := accGetter.EnsureExists(from); err != nil {
return txBldr, err
}

from := cliCtx.GetFromAddress()

txbldrAccNum, txbldrAccSeq := txBldr.AccountNumber(), txBldr.Sequence()
// TODO: (ref #1903) Allow for user supplied account number without
// automatically doing a manual lookup.
if txBldr.AccountNumber() == 0 {
accNum, err := cliCtx.GetAccountNumber(from)
if txbldrAccNum == 0 || txbldrAccSeq == 0 {
num, seq, err := authtypes.NewAccountRetriever(cliCtx).GetAccountNumberSequence(from)
if err != nil {
return txBldr, err
}
txBldr = txBldr.WithAccountNumber(accNum)
}

// TODO: (ref #1903) Allow for user supplied account sequence without
// automatically doing a manual lookup.
if txBldr.Sequence() == 0 {
accSeq, err := cliCtx.GetAccountSequence(from)
if err != nil {
return txBldr, err
if txbldrAccNum == 0 {
txBldr = txBldr.WithAccountNumber(num)
}
if txbldrAccSeq == 0 {
txBldr = txBldr.WithSequence(seq)
}
txBldr = txBldr.WithSequence(accSeq)
}

return txBldr, nil
Expand Down
Loading

0 comments on commit 1eb7706

Please sign in to comment.