From 9ee7ff04089383f40efe0525e045d15762547768 Mon Sep 17 00:00:00 2001 From: Pratik Patil Date: Tue, 2 Jan 2024 10:10:23 +0530 Subject: [PATCH 1/8] resolved todos in keeper.go (except 1) --- x/auth/keeper/keeper.go | 24 +++++------------------- 1 file changed, 5 insertions(+), 19 deletions(-) diff --git a/x/auth/keeper/keeper.go b/x/auth/keeper/keeper.go index 0c9078f441d1..c8c074d1c4f4 100644 --- a/x/auth/keeper/keeper.go +++ b/x/auth/keeper/keeper.go @@ -88,7 +88,7 @@ type AccountKeeper struct { storeService store.KVStoreService cdc codec.BinaryCodec permAddrs map[string]types.PermissionsForAddress - // TODO HV2 bech32Prefix? + // TODO HV2 bech32Prefix? Keeping this as it is for now. Check if it is interfering with heimdall address bech32Prefix string // The prototypical AccountI constructor. @@ -281,37 +281,23 @@ func (ak AccountKeeper) GetParams(ctx context.Context) (params types.Params) { // GetBlockProposer returns block proposer func (ak AccountKeeper) GetBlockProposer(ctx sdk.Context) (sdk.HeimdallAddress, bool) { - // TODO HV2 are these implementations equivalent for GetBlockProposer kvStore := ak.storeService.OpenKVStore(ctx) - isProposerPresent, _ := kvStore.Has(types.ProposerKey()) // TODO HV2 handle error? + isProposerPresent, _ := kvStore.Has(types.ProposerKey()) if !isProposerPresent { return sdk.HeimdallAddress{}, false } - blockProposerBytes, _ := kvStore.Get(types.ProposerKey()) // TODO HV2 handle error? + blockProposerBytes, _ := kvStore.Get(types.ProposerKey()) return sdk.BytesToHeimdallAddress(blockProposerBytes), true - - //store := ctx.KVStore(ak.key) - //if !store.Has(types.ProposerKey()) { - // return hmTypes.HeimdallAddress{}, false - //} - //bz := store.Get(types.ProposerKey()) - //return hmTypes.BytesToHeimdallAddress(bz), true } // SetBlockProposer sets block proposer func (ak AccountKeeper) SetBlockProposer(ctx sdk.Context, addr sdk.HeimdallAddress) { - // TODO HV2 are these implementations equivalent for SetBlockProposer kvStore := ak.storeService.OpenKVStore(ctx) - kvStore.Set(types.ProposerKey(), addr.Bytes()) // TODO HV2 handle error? - //store := ctx.KVStore(ak.key) - //store.Set(types.ProposerKey(), addr.Bytes()) + kvStore.Set(types.ProposerKey(), addr.Bytes()) } // RemoveBlockProposer removes block proposer from store func (ak AccountKeeper) RemoveBlockProposer(ctx sdk.Context) { - // TODO HV2 are these implementations equivalent for RemoveBlockProposer kvStore := ak.storeService.OpenKVStore(ctx) - kvStore.Delete(types.ProposerKey()) // TODO HV2 handle error? - //store := ctx.KVStore(ak.key) - //store.Delete(types.ProposerKey()) + kvStore.Delete(types.ProposerKey()) } From 596150b5a58624c505eaa95b363e1bd7c0a7eae4 Mon Sep 17 00:00:00 2001 From: Pratik Patil Date: Tue, 2 Jan 2024 14:38:18 +0530 Subject: [PATCH 2/8] updated x/auth/keeper/grpc_query.go with comments --- x/auth/keeper/grpc_query.go | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/x/auth/keeper/grpc_query.go b/x/auth/keeper/grpc_query.go index 7ccac936929b..18a1dbe4fb06 100644 --- a/x/auth/keeper/grpc_query.go +++ b/x/auth/keeper/grpc_query.go @@ -71,6 +71,7 @@ func (s queryServer) Account(ctx context.Context, req *types.QueryAccountRequest return nil, status.Error(codes.InvalidArgument, "Address cannot be empty") } + // PSP - TODO HV2 - we might need wo change this to use heimdall address addr, err := s.k.addressCodec.StringToBytes(req.Address) if err != nil { return nil, err @@ -88,6 +89,25 @@ func (s queryServer) Account(ctx context.Context, req *types.QueryAccountRequest return &types.QueryAccountResponse{Account: any}, nil } +// func queryAccount(ctx sdk.Context, req abci.RequestQuery, keeper AccountKeeper) ([]byte, sdk.Error) { +// var params types.QueryAccountParams +// if err := keeper.cdc.UnmarshalJSON(req.Data, ¶ms); err != nil { +// return nil, sdk.ErrInternal(fmt.Sprintf("failed to parse params: %s", err)) +// } + +// account := keeper.GetAccount(ctx, params.Address) +// if account == nil { +// return nil, sdk.ErrUnknownAddress(fmt.Sprintf("account %s does not exist", params.Address)) +// } + +// bz, err := codec.MarshalJSONIndent(keeper.cdc, account) +// if err != nil { +// return nil, sdk.ErrInternal(sdk.AppendMsgToErr("could not marshal result to JSON", err.Error())) +// } + +// return bz, nil +// } + // Params returns parameters of auth module func (s queryServer) Params(c context.Context, req *types.QueryParamsRequest) (*types.QueryParamsResponse, error) { if req == nil { @@ -99,6 +119,15 @@ func (s queryServer) Params(c context.Context, req *types.QueryParamsRequest) (* return &types.QueryParamsResponse{Params: params}, nil } +// func queryParams(ctx sdk.Context, _ abci.RequestQuery, keeper AccountKeeper) ([]byte, sdk.Error) { +// bz, err := jsoniter.ConfigFastest.Marshal(keeper.GetParams(ctx)) +// if err != nil { +// return nil, sdk.ErrInternal(sdk.AppendMsgToErr("could not marshal result to JSON", err.Error())) +// } + +// return bz, nil +// } + // ModuleAccounts returns all the existing Module Accounts func (s queryServer) ModuleAccounts(c context.Context, req *types.QueryModuleAccountsRequest) (*types.QueryModuleAccountsResponse, error) { if req == nil { From 838c4be99a16ee3defed323cd58a320e4f9d5eb8 Mon Sep 17 00:00:00 2001 From: Pratik Patil Date: Thu, 4 Jan 2024 10:19:26 +0530 Subject: [PATCH 3/8] added comments in auth/module.go --- x/auth/module.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/x/auth/module.go b/x/auth/module.go index 7120d46d9c8b..1b6217219e31 100644 --- a/x/auth/module.go +++ b/x/auth/module.go @@ -6,6 +6,7 @@ import ( "fmt" gwruntime "github.com/grpc-ecosystem/grpc-gateway/runtime" + "github.com/maticnetwork/heimdall/helper" modulev1 "cosmossdk.io/api/cosmos/auth/module/v1" "cosmossdk.io/core/address" @@ -93,7 +94,6 @@ type AppModule struct { // legacySubspace is used solely for migration of x/params managed parameters legacySubspace exported.Subspace - // TODO HV2 check contractCaller and processors in this whole file contractCaller helper.IContractCaller processors []types.AccountProcessor } @@ -214,8 +214,6 @@ type ModuleOutputs struct { AccountKeeper keeper.AccountKeeper Module appmodule.AppModule - - // TODO HV2 check contractCaller and processors in this whole file } func ProvideModule(in ModuleInputs) ModuleOutputs { @@ -239,10 +237,12 @@ func ProvideModule(in ModuleInputs) ModuleOutputs { in.AccountI = types.ProtoBaseAccount } + // PSP - TODO HV2 remove this if we decided to remove contractCallerfrom ModuleInputs if in.contractCaller == nil { in.contractCaller = helper.IContractCaller } + // PSP - TODO HV2 remove this if we decided to remove processors from ModuleInputs if in.processors == nil { in.processors = []types.AccountProcessor{} } From 916f37976c99442bee99ef66ec3e2df58dc69c06 Mon Sep 17 00:00:00 2001 From: Pratik Patil Date: Fri, 5 Jan 2024 09:57:55 +0530 Subject: [PATCH 4/8] removed depinject from auth.ModuleInputs --- x/auth/module.go | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/x/auth/module.go b/x/auth/module.go index 1b6217219e31..e2468581baf0 100644 --- a/x/auth/module.go +++ b/x/auth/module.go @@ -203,10 +203,6 @@ type ModuleInputs struct { // LegacySubspace is used solely for migration of x/params managed parameters LegacySubspace exported.Subspace `optional:"true"` - - // TODO HV2 is this depinject needed? - contractCaller helper.IContractCaller - processors []types.AccountProcessor } type ModuleOutputs struct { @@ -237,18 +233,8 @@ func ProvideModule(in ModuleInputs) ModuleOutputs { in.AccountI = types.ProtoBaseAccount } - // PSP - TODO HV2 remove this if we decided to remove contractCallerfrom ModuleInputs - if in.contractCaller == nil { - in.contractCaller = helper.IContractCaller - } - - // PSP - TODO HV2 remove this if we decided to remove processors from ModuleInputs - if in.processors == nil { - in.processors = []types.AccountProcessor{} - } - k := keeper.NewAccountKeeper(in.Cdc, in.StoreService, in.AccountI, maccPerms, in.AddressCodec, in.Config.Bech32Prefix, authority.String()) - m := NewAppModule(in.Cdc, k, in.RandomGenesisAccountsFn, in.LegacySubspace, in.contractCaller, in.processors) + m := NewAppModule(in.Cdc, k, in.RandomGenesisAccountsFn, in.LegacySubspace, nil, nil) return ModuleOutputs{AccountKeeper: k, Module: m} } From 4bc9f4c7ee53846d70c1f863fe2eabb0fe1f7f3e Mon Sep 17 00:00:00 2001 From: Pratik Patil Date: Fri, 5 Jan 2024 13:43:50 +0530 Subject: [PATCH 5/8] resolved TODOs in auth/keeper/genesis (created NewBaseAccount and passed to processor) --- x/auth/keeper/genesis.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/x/auth/keeper/genesis.go b/x/auth/keeper/genesis.go index bbeb3aa71965..8d41398dcf22 100644 --- a/x/auth/keeper/genesis.go +++ b/x/auth/keeper/genesis.go @@ -28,10 +28,12 @@ func (ak AccountKeeper) InitGenesis(ctx sdk.Context, data types.GenesisState, pr n := ak.NextAccountNumber(ctx) lastAccNum = &n } + + baseAcc := types.NewBaseAccount(acc.GetAddress(), acc.GetPubKey(), accNum, acc.GetSequence()) + // execute account processors for _, p := range processors { - // TODO HV2 fill processors here (check heimdall's auth/genesis.go) - // acc = p(&gacc, d) //nolint + acc = p(&acc, baseAcc) //nolint } ak.SetAccount(ctx, acc) } From 1c724f224dcd2862129ffbaa6cf80ab7df31932f Mon Sep 17 00:00:00 2001 From: Pratik Patil Date: Fri, 5 Jan 2024 14:37:07 +0530 Subject: [PATCH 6/8] removed some TODOs from x/auth/types/account.go --- x/auth/types/account.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/x/auth/types/account.go b/x/auth/types/account.go index 09649b1a8fff..ba3c675a9e4e 100644 --- a/x/auth/types/account.go +++ b/x/auth/types/account.go @@ -6,9 +6,10 @@ import ( "encoding/json" "errors" "fmt" + "strings" + "github.com/cometbft/cometbft/crypto" "github.com/cometbft/cometbft/crypto/secp256k1" - "strings" codectypes "github.com/cosmos/cosmos-sdk/codec/types" cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" @@ -332,13 +333,11 @@ func (ga GenesisAccounts) Contains(addr sdk.HeimdallAddress) bool { } // GenesisAccount defines a genesis account that embeds an AccountI with validation capabilities. -// TODO HV2 genesis account is different type GenesisAccount interface { sdk.AccountI Validate() error } -// TODO HV2 imported from heimdall. Needed? // AccountProcessor is an interface to process account as per module type AccountProcessor func(*GenesisAccount, *BaseAccount) sdk.AccountI From 90fc26623aca698d8037a68a92e79fa36d1d84e5 Mon Sep 17 00:00:00 2001 From: Pratik Patil Date: Fri, 5 Jan 2024 16:56:44 +0530 Subject: [PATCH 7/8] added few comments in x/auth/ante/ante.go --- x/auth/ante/ante.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/x/auth/ante/ante.go b/x/auth/ante/ante.go index 8373927596f9..7030fccd4cec 100644 --- a/x/auth/ante/ante.go +++ b/x/auth/ante/ante.go @@ -25,7 +25,12 @@ type HandlerOptions struct { // NewAnteHandler returns an AnteHandler that checks and increments sequence // numbers, checks signatures & account numbers, and deducts fees from the first // signer. -// TODO HV2 is this enough to reconcile with heimdall's auth/ante.go? ( + +// TODO HV2 is this enough to reconcile with heimdall's auth/ante.go? +// ^ we will nedd to add the following in `HandlerOptions` as NewAnteHandler() in heimdall is using them: +// - chainKeeper chainmanager.Keeper +// - contractCaller helper.IContractCaller +// - sigGasConsumer SignatureVerificationGasConsumer func NewAnteHandler(options HandlerOptions) (sdk.AnteHandler, error) { if options.AccountKeeper == nil { return nil, errorsmod.Wrap(sdkerrors.ErrLogic, "account keeper is required for ante builder") From 816abfa4686b23b209ff0088eef5efa65e02a924 Mon Sep 17 00:00:00 2001 From: Pratik Patil Date: Tue, 9 Jan 2024 14:59:28 +0530 Subject: [PATCH 8/8] few final comments and modifications --- proto/cosmos/auth/v1beta1/query.proto | 1 + types/account.go | 1 - x/auth/module_test.go | 2 +- x/auth/types/account.go | 25 ------------------------- 4 files changed, 2 insertions(+), 27 deletions(-) diff --git a/proto/cosmos/auth/v1beta1/query.proto b/proto/cosmos/auth/v1beta1/query.proto index 804f2ff08001..ca9978533775 100644 --- a/proto/cosmos/auth/v1beta1/query.proto +++ b/proto/cosmos/auth/v1beta1/query.proto @@ -113,6 +113,7 @@ message QueryAccountRequest { option (gogoproto.goproto_getters) = false; // address defines the address to query for. + // TODO HV2 change it to HeimdallAddress string address = 1 [(cosmos_proto.scalar) = "cosmos.AddressString"]; } diff --git a/types/account.go b/types/account.go index e8839131136e..9301a693ee7d 100644 --- a/types/account.go +++ b/types/account.go @@ -23,7 +23,6 @@ type AccountI interface { GetAddress() HeimdallAddress SetAddress(HeimdallAddress) error // errors if already set. - // TODO HV2 is this key what we want to use? GetPubKey() cryptotypes.PubKey // can return nil. SetPubKey(cryptotypes.PubKey) error diff --git a/x/auth/module_test.go b/x/auth/module_test.go index 2f24781c3f15..981df86c3c94 100644 --- a/x/auth/module_test.go +++ b/x/auth/module_test.go @@ -14,8 +14,8 @@ import ( "github.com/cosmos/cosmos-sdk/x/auth/types" ) -// // TODO HV2 inject heimdall app func TestItCreatesModuleAccountOnInitBlock(t *testing.T) { + t.Skip() // skipping as we are not using depinject var accountKeeper keeper.AccountKeeper app, err := simtestutil.SetupAtGenesis( depinject.Configs( diff --git a/x/auth/types/account.go b/x/auth/types/account.go index ba3c675a9e4e..cb87a0e3a9f7 100644 --- a/x/auth/types/account.go +++ b/x/auth/types/account.go @@ -2,14 +2,12 @@ package types import ( "bytes" - "encoding/hex" "encoding/json" "errors" "fmt" "strings" "github.com/cometbft/cometbft/crypto" - "github.com/cometbft/cometbft/crypto/secp256k1" codectypes "github.com/cosmos/cosmos-sdk/codec/types" cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" @@ -117,29 +115,6 @@ func (acc *BaseAccount) SetSequence(seq uint64) error { return nil } -// TODO HV2 It's should not be needed, but verify (proto implements it in `auth.pb.go`) -// String implements fmt.Stringer -func (acc BaseAccount) String() string { - var pubkey string - - if acc.PubKey != nil { - // pubkey = sdk.MustBech32ifyAccPub(acc.PubKey) - - var pubObject secp256k1.PubKey - - cdc.MustUnmarshalBinaryBare(acc.PubKey.Bytes(), &pubObject) - - pubkey = "0x" + hex.EncodeToString(pubObject[:]) - } - - return fmt.Sprintf(`Account: - Address: %s - Pubkey: %s - AccountNumber: %d - Sequence: %d`, - acc.Address, pubkey, acc.AccountNumber, acc.Sequence) -} - // Validate checks for errors on the account fields func (acc BaseAccount) Validate() error { if acc.Address == "" || acc.PubKey == nil {