From 3dfbfbc606ab87ff8f8de4de61e9ff4f671eb600 Mon Sep 17 00:00:00 2001 From: Owen Hu Date: Wed, 22 Mar 2023 23:00:31 +0700 Subject: [PATCH 1/3] chore: refine crosschain module --- proto/cosmos/crosschain/v1/crosschain.proto | 1 + x/crosschain/keeper/grpc_query.go | 3 +++ x/crosschain/keeper/keeper.go | 1 + 3 files changed, 5 insertions(+) diff --git a/proto/cosmos/crosschain/v1/crosschain.proto b/proto/cosmos/crosschain/v1/crosschain.proto index 2d8622e999..2c66dee4c8 100644 --- a/proto/cosmos/crosschain/v1/crosschain.proto +++ b/proto/cosmos/crosschain/v1/crosschain.proto @@ -6,5 +6,6 @@ option go_package = "github.com/cosmos/cosmos-sdk/x/crosschain/types"; // Params holds parameters for the cross chain module. message Params { // initial balance to mint for crosschain module when the chain starts + // todo(quality): Use `sdk.Int` instead of `string` for `init_module_balance`. string init_module_balance = 1; } diff --git a/x/crosschain/keeper/grpc_query.go b/x/crosschain/keeper/grpc_query.go index 63a16d2e5e..1eaa98fd4b 100644 --- a/x/crosschain/keeper/grpc_query.go +++ b/x/crosschain/keeper/grpc_query.go @@ -21,6 +21,7 @@ func (k Keeper) Params(c context.Context, _ *types.QueryParamsRequest) (*types.Q func (k Keeper) CrossChainPackage(c context.Context, req *types.QueryCrossChainPackageRequest) (*types.QueryCrossChainPackageResponse, error) { ctx := sdk.UnwrapSDKContext(c) + // todo(quality): check if req is nil pack, err := k.GetCrossChainPackage(ctx, sdk.ChannelID(req.ChannelId), req.Sequence) if err != nil { return nil, err @@ -33,6 +34,7 @@ func (k Keeper) CrossChainPackage(c context.Context, req *types.QueryCrossChainP // SendSequence returns the send sequence of the channel func (k Keeper) SendSequence(c context.Context, req *types.QuerySendSequenceRequest) (*types.QuerySendSequenceResponse, error) { ctx := sdk.UnwrapSDKContext(c) + // todo(quality): check if req is nil sequence := k.GetSendSequence(ctx, sdk.ChannelID(req.ChannelId)) return &types.QuerySendSequenceResponse{ @@ -43,6 +45,7 @@ func (k Keeper) SendSequence(c context.Context, req *types.QuerySendSequenceRequ // ReceiveSequence returns the receive sequence of the channel func (k Keeper) ReceiveSequence(c context.Context, req *types.QueryReceiveSequenceRequest) (*types.QueryReceiveSequenceResponse, error) { ctx := sdk.UnwrapSDKContext(c) + // todo(quality): check if req is nil sequence := k.GetReceiveSequence(ctx, sdk.ChannelID(req.ChannelId)) return &types.QueryReceiveSequenceResponse{ diff --git a/x/crosschain/keeper/keeper.go b/x/crosschain/keeper/keeper.go index aa60e74305..3c8e7cdcbf 100644 --- a/x/crosschain/keeper/keeper.go +++ b/x/crosschain/keeper/keeper.go @@ -41,6 +41,7 @@ func NewKeeper( } } +// todo(quality): is it better to pass keeper by pointer? It can reduce the cost of copying the whole struct. // Logger inits the logger for cross chain module func (k Keeper) Logger(ctx sdk.Context) log.Logger { return ctx.Logger().With("module", "x/"+types.ModuleName) From 6248c34b787fd0ffc9e8739627a18ef952966c99 Mon Sep 17 00:00:00 2001 From: yutianwu Date: Thu, 23 Mar 2023 10:43:41 +0800 Subject: [PATCH 2/3] fix comments --- proto/cosmos/crosschain/v1/crosschain.proto | 10 +++- x/crosschain/keeper/grpc_query.go | 18 +++++-- x/crosschain/keeper/keeper.go | 16 +++--- x/crosschain/types/crosschain.pb.go | 59 +++++++++++---------- x/crosschain/types/genesis.go | 10 +--- x/crosschain/types/params.go | 23 +++++--- x/oracle/types/oracle.pb.go | 4 +- 7 files changed, 78 insertions(+), 62 deletions(-) diff --git a/proto/cosmos/crosschain/v1/crosschain.proto b/proto/cosmos/crosschain/v1/crosschain.proto index 2c66dee4c8..0c91f0d15c 100644 --- a/proto/cosmos/crosschain/v1/crosschain.proto +++ b/proto/cosmos/crosschain/v1/crosschain.proto @@ -1,11 +1,17 @@ syntax = "proto3"; package cosmos.crosschain.v1; +import "cosmos_proto/cosmos.proto"; +import "gogoproto/gogo.proto"; + option go_package = "github.com/cosmos/cosmos-sdk/x/crosschain/types"; // Params holds parameters for the cross chain module. message Params { // initial balance to mint for crosschain module when the chain starts - // todo(quality): Use `sdk.Int` instead of `string` for `init_module_balance`. - string init_module_balance = 1; + string init_module_balance = 1 [ + (cosmos_proto.scalar) = "cosmos.Int", + (gogoproto.customtype) = "github.com/cosmos/cosmos-sdk/types.Int", + (gogoproto.nullable) = false + ]; } diff --git a/x/crosschain/keeper/grpc_query.go b/x/crosschain/keeper/grpc_query.go index 1eaa98fd4b..b24e07faf2 100644 --- a/x/crosschain/keeper/grpc_query.go +++ b/x/crosschain/keeper/grpc_query.go @@ -5,6 +5,8 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/crosschain/types" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" ) var _ types.QueryServer = Keeper{} @@ -19,9 +21,11 @@ func (k Keeper) Params(c context.Context, _ *types.QueryParamsRequest) (*types.Q // CrossChainPackage returns the specified cross chain package func (k Keeper) CrossChainPackage(c context.Context, req *types.QueryCrossChainPackageRequest) (*types.QueryCrossChainPackageResponse, error) { - ctx := sdk.UnwrapSDKContext(c) + if req == nil { + return nil, status.Error(codes.InvalidArgument, "invalid request") + } - // todo(quality): check if req is nil + ctx := sdk.UnwrapSDKContext(c) pack, err := k.GetCrossChainPackage(ctx, sdk.ChannelID(req.ChannelId), req.Sequence) if err != nil { return nil, err @@ -33,8 +37,11 @@ func (k Keeper) CrossChainPackage(c context.Context, req *types.QueryCrossChainP // SendSequence returns the send sequence of the channel func (k Keeper) SendSequence(c context.Context, req *types.QuerySendSequenceRequest) (*types.QuerySendSequenceResponse, error) { + if req == nil { + return nil, status.Error(codes.InvalidArgument, "invalid request") + } + ctx := sdk.UnwrapSDKContext(c) - // todo(quality): check if req is nil sequence := k.GetSendSequence(ctx, sdk.ChannelID(req.ChannelId)) return &types.QuerySendSequenceResponse{ @@ -44,8 +51,11 @@ func (k Keeper) SendSequence(c context.Context, req *types.QuerySendSequenceRequ // ReceiveSequence returns the receive sequence of the channel func (k Keeper) ReceiveSequence(c context.Context, req *types.QueryReceiveSequenceRequest) (*types.QueryReceiveSequenceResponse, error) { + if req == nil { + return nil, status.Error(codes.InvalidArgument, "invalid request") + } + ctx := sdk.UnwrapSDKContext(c) - // todo(quality): check if req is nil sequence := k.GetReceiveSequence(ctx, sdk.ChannelID(req.ChannelId)) return &types.QueryReceiveSequenceResponse{ diff --git a/x/crosschain/keeper/keeper.go b/x/crosschain/keeper/keeper.go index 3c8e7cdcbf..fa50549d23 100644 --- a/x/crosschain/keeper/keeper.go +++ b/x/crosschain/keeper/keeper.go @@ -6,6 +6,7 @@ import ( "fmt" "math/big" + sdkmath "cosmossdk.io/math" "github.com/tendermint/tendermint/libs/log" "github.com/cosmos/cosmos-sdk/codec" @@ -41,7 +42,6 @@ func NewKeeper( } } -// todo(quality): is it better to pass keeper by pointer? It can reduce the cost of copying the whole struct. // Logger inits the logger for cross chain module func (k Keeper) Logger(ctx sdk.Context) log.Logger { return ctx.Logger().With("module", "x/"+types.ModuleName) @@ -57,7 +57,7 @@ func (k Keeper) InitGenesis(ctx sdk.Context, state *types.GenesisState, bankKeep err := bankKeeper.MintCoins(ctx, types.ModuleName, sdk.Coins{sdk.Coin{ Denom: bondDenom, - Amount: sdk.NewIntFromBigInt(initModuleBalance), + Amount: initModuleBalance, }}) if err != nil { panic(fmt.Sprintf("mint initial cross chain module balance error, err=%s", err.Error())) @@ -65,14 +65,10 @@ func (k Keeper) InitGenesis(ctx sdk.Context, state *types.GenesisState, bankKeep } // GetInitModuleBalance returns the initial balance of cross chain module -func (k Keeper) GetInitModuleBalance(ctx sdk.Context) *big.Int { - var initModuleBlaanceParam string - k.paramSpace.Get(ctx, types.KeyParamInitModuleBalance, &initModuleBlaanceParam) - moduleBalance, valid := big.NewInt(0).SetString(initModuleBlaanceParam, 10) - if !valid { - panic(fmt.Errorf("invalid init module balance: %s", initModuleBlaanceParam)) - } - return moduleBalance +func (k Keeper) GetInitModuleBalance(ctx sdk.Context) sdkmath.Int { + var initModuleBalanceParam sdkmath.Int + k.paramSpace.Get(ctx, types.KeyParamInitModuleBalance, &initModuleBalanceParam) + return initModuleBalanceParam } // SetParams sets the params of cross chain module diff --git a/x/crosschain/types/crosschain.pb.go b/x/crosschain/types/crosschain.pb.go index 727c0836fc..b0a7778e74 100644 --- a/x/crosschain/types/crosschain.pb.go +++ b/x/crosschain/types/crosschain.pb.go @@ -5,6 +5,9 @@ package types import ( fmt "fmt" + _ "github.com/cosmos/cosmos-proto" + github_com_cosmos_cosmos_sdk_types "github.com/cosmos/cosmos-sdk/types" + _ "github.com/gogo/protobuf/gogoproto" proto "github.com/gogo/protobuf/proto" io "io" math "math" @@ -25,7 +28,7 @@ const _ = proto.GoGoProtoPackageIsVersion3 // please upgrade the proto package // Params holds parameters for the cross chain module. type Params struct { // initial balance to mint for crosschain module when the chain starts - InitModuleBalance string `protobuf:"bytes,1,opt,name=init_module_balance,json=initModuleBalance,proto3" json:"init_module_balance,omitempty"` + InitModuleBalance github_com_cosmos_cosmos_sdk_types.Int `protobuf:"bytes,1,opt,name=init_module_balance,json=initModuleBalance,proto3,customtype=github.com/cosmos/cosmos-sdk/types.Int" json:"init_module_balance"` } func (m *Params) Reset() { *m = Params{} } @@ -61,13 +64,6 @@ func (m *Params) XXX_DiscardUnknown() { var xxx_messageInfo_Params proto.InternalMessageInfo -func (m *Params) GetInitModuleBalance() string { - if m != nil { - return m.InitModuleBalance - } - return "" -} - func init() { proto.RegisterType((*Params)(nil), "cosmos.crosschain.v1.Params") } @@ -77,18 +73,22 @@ func init() { } var fileDescriptor_d7b94a7254cf916a = []byte{ - // 175 bytes of a gzipped FileDescriptorProto + // 227 bytes of a gzipped FileDescriptorProto 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0xe2, 0x52, 0x4d, 0xce, 0x2f, 0xce, 0xcd, 0x2f, 0xd6, 0x4f, 0x2e, 0xca, 0x2f, 0x2e, 0x4e, 0xce, 0x48, 0xcc, 0xcc, 0xd3, 0x2f, 0x33, 0x44, 0xe2, 0xe9, 0x15, 0x14, 0xe5, 0x97, 0xe4, 0x0b, 0x89, 0x40, 0x94, 0xe9, 0x21, 0x49, 0x94, - 0x19, 0x2a, 0x59, 0x70, 0xb1, 0x05, 0x24, 0x16, 0x25, 0xe6, 0x16, 0x0b, 0xe9, 0x71, 0x09, 0x67, - 0xe6, 0x65, 0x96, 0xc4, 0xe7, 0xe6, 0xa7, 0x94, 0xe6, 0xa4, 0xc6, 0x27, 0x25, 0xe6, 0x24, 0xe6, - 0x25, 0xa7, 0x4a, 0x30, 0x2a, 0x30, 0x6a, 0x70, 0x06, 0x09, 0x82, 0xa4, 0x7c, 0xc1, 0x32, 0x4e, - 0x10, 0x09, 0x27, 0xcf, 0x13, 0x8f, 0xe4, 0x18, 0x2f, 0x3c, 0x92, 0x63, 0x7c, 0xf0, 0x48, 0x8e, - 0x71, 0xc2, 0x63, 0x39, 0x86, 0x0b, 0x8f, 0xe5, 0x18, 0x6e, 0x3c, 0x96, 0x63, 0x88, 0xd2, 0x4f, - 0xcf, 0x2c, 0xc9, 0x28, 0x4d, 0xd2, 0x4b, 0xce, 0xcf, 0xd5, 0x87, 0xb9, 0x0d, 0x4c, 0xe9, 0x16, - 0xa7, 0x64, 0xeb, 0x57, 0x20, 0x3b, 0xb4, 0xa4, 0xb2, 0x20, 0xb5, 0x38, 0x89, 0x0d, 0xec, 0x42, - 0x63, 0x40, 0x00, 0x00, 0x00, 0xff, 0xff, 0xf7, 0xe6, 0x93, 0xeb, 0xca, 0x00, 0x00, 0x00, + 0x19, 0x4a, 0x49, 0x42, 0x44, 0xe3, 0xc1, 0x6a, 0xf4, 0xa1, 0x4a, 0xc0, 0x1c, 0x29, 0x91, 0xf4, + 0xfc, 0xf4, 0x7c, 0x88, 0x38, 0x88, 0x05, 0x11, 0x55, 0x2a, 0xe3, 0x62, 0x0b, 0x48, 0x2c, 0x4a, + 0xcc, 0x2d, 0x16, 0xca, 0xe1, 0x12, 0xce, 0xcc, 0xcb, 0x2c, 0x89, 0xcf, 0xcd, 0x4f, 0x29, 0xcd, + 0x49, 0x8d, 0x4f, 0x4a, 0xcc, 0x49, 0xcc, 0x4b, 0x4e, 0x95, 0x60, 0x54, 0x60, 0xd4, 0xe0, 0x74, + 0xb2, 0x39, 0x71, 0x4f, 0x9e, 0xe1, 0xd6, 0x3d, 0x79, 0xb5, 0xf4, 0xcc, 0x92, 0x8c, 0xd2, 0x24, + 0xbd, 0xe4, 0xfc, 0x5c, 0x7d, 0x98, 0x3b, 0xc1, 0x94, 0x6e, 0x71, 0x4a, 0xb6, 0x7e, 0x49, 0x65, + 0x41, 0x6a, 0xb1, 0x9e, 0x67, 0x5e, 0xc9, 0xa5, 0x2d, 0xba, 0x5c, 0x50, 0xcb, 0x3d, 0xf3, 0x4a, + 0x82, 0x04, 0x41, 0x06, 0xfb, 0x82, 0xcd, 0x75, 0x82, 0x18, 0xeb, 0xe4, 0x79, 0xe2, 0x91, 0x1c, + 0xe3, 0x85, 0x47, 0x72, 0x8c, 0x0f, 0x1e, 0xc9, 0x31, 0x4e, 0x78, 0x2c, 0xc7, 0x70, 0xe1, 0xb1, + 0x1c, 0xc3, 0x8d, 0xc7, 0x72, 0x0c, 0x51, 0xfa, 0x78, 0xad, 0xa8, 0x40, 0x0e, 0x17, 0xb0, 0x7d, + 0x49, 0x6c, 0x60, 0x9f, 0x18, 0x03, 0x02, 0x00, 0x00, 0xff, 0xff, 0xd9, 0x23, 0x9a, 0x6c, 0x39, + 0x01, 0x00, 0x00, } func (m *Params) Marshal() (dAtA []byte, err error) { @@ -111,13 +111,16 @@ func (m *Params) MarshalToSizedBuffer(dAtA []byte) (int, error) { _ = i var l int _ = l - if len(m.InitModuleBalance) > 0 { - i -= len(m.InitModuleBalance) - copy(dAtA[i:], m.InitModuleBalance) - i = encodeVarintCrosschain(dAtA, i, uint64(len(m.InitModuleBalance))) - i-- - dAtA[i] = 0xa + { + size := m.InitModuleBalance.Size() + i -= size + if _, err := m.InitModuleBalance.MarshalTo(dAtA[i:]); err != nil { + return 0, err + } + i = encodeVarintCrosschain(dAtA, i, uint64(size)) } + i-- + dAtA[i] = 0xa return len(dAtA) - i, nil } @@ -138,10 +141,8 @@ func (m *Params) Size() (n int) { } var l int _ = l - l = len(m.InitModuleBalance) - if l > 0 { - n += 1 + l + sovCrosschain(uint64(l)) - } + l = m.InitModuleBalance.Size() + n += 1 + l + sovCrosschain(uint64(l)) return n } @@ -210,7 +211,9 @@ func (m *Params) Unmarshal(dAtA []byte) error { if postIndex > l { return io.ErrUnexpectedEOF } - m.InitModuleBalance = string(dAtA[iNdEx:postIndex]) + if err := m.InitModuleBalance.Unmarshal(dAtA[iNdEx:postIndex]); err != nil { + return err + } iNdEx = postIndex default: iNdEx = preIndex diff --git a/x/crosschain/types/genesis.go b/x/crosschain/types/genesis.go index 889659066b..4afef0135a 100644 --- a/x/crosschain/types/genesis.go +++ b/x/crosschain/types/genesis.go @@ -2,7 +2,6 @@ package types import ( "fmt" - "math/big" ) // NewGenesisState creates a new GenesisState object @@ -23,13 +22,8 @@ func DefaultGenesisState() *GenesisState { // ValidateGenesis validates the cross chain genesis parameters func ValidateGenesis(data GenesisState) error { - balance, valid := big.NewInt(0).SetString(data.Params.InitModuleBalance, 10) - if !valid { - return fmt.Errorf("invalid module balance, is %s", data.Params.InitModuleBalance) - } - - if balance.Cmp(big.NewInt(0)) < 0 { - return fmt.Errorf("init module balance should be positive, is %s", balance.String()) + if !data.Params.InitModuleBalance.IsPositive() { + return fmt.Errorf("init module balance should be positive, is %s", data.Params.InitModuleBalance.String()) } return nil diff --git a/x/crosschain/types/params.go b/x/crosschain/types/params.go index aa065ec2ff..05198e68dd 100644 --- a/x/crosschain/types/params.go +++ b/x/crosschain/types/params.go @@ -2,12 +2,20 @@ package types import ( "fmt" - "math/big" + sdkmath "cosmossdk.io/math" paramtypes "github.com/cosmos/cosmos-sdk/x/params/types" ) -const DefaultInitModuleBalance string = "2000000000000000000000000" // 2M +var DefaultInitModuleBalance sdkmath.Int + +func init() { + initModuleBalance, ok := sdkmath.NewIntFromString("2000000000000000000000000") // 2M + if !ok { + panic("invalid init module balance") + } + DefaultInitModuleBalance = initModuleBalance +} var KeyParamInitModuleBalance = []byte("InitModuleBalance") @@ -29,18 +37,17 @@ func (p *Params) ParamSetPairs() paramtypes.ParamSetPairs { } func validateInitModuleBalance(i interface{}) error { - v, ok := i.(string) + v, ok := i.(sdkmath.Int) if !ok { return fmt.Errorf("invalid parameter type: %T", i) } - balance, valid := big.NewInt(0).SetString(v, 10) - if !valid { - return fmt.Errorf("invalid module balance, is %s", v) + if v.IsNil() { + return fmt.Errorf("init module balance should not be nil") } - if balance.Cmp(big.NewInt(0)) < 0 { - return fmt.Errorf("init module balance should be positive, is %s", v) + if !v.IsPositive() { + return fmt.Errorf("init module balance should be positive, is %s", v.String()) } return nil diff --git a/x/oracle/types/oracle.pb.go b/x/oracle/types/oracle.pb.go index 66de90e8bb..6c901b6e93 100644 --- a/x/oracle/types/oracle.pb.go +++ b/x/oracle/types/oracle.pb.go @@ -24,9 +24,9 @@ const _ = proto.GoGoProtoPackageIsVersion3 // please upgrade the proto package // Params holds parameters for the oracle module. type Params struct { - // Timeout for the in turn relayer + // Timeout for the in turn relayer in seconds RelayerTimeout uint64 `protobuf:"varint,1,opt,name=relayer_timeout,json=relayerTimeout,proto3" json:"relayer_timeout,omitempty"` - // RelayInterval is for in-turn relayer + // RelayInterval is for in-turn relayer in seconds RelayerInterval uint64 `protobuf:"varint,2,opt,name=relayer_interval,json=relayerInterval,proto3" json:"relayer_interval,omitempty"` // Reward share for the relayer sends the claim message, // the other relayers signed the bls message will share the reward evenly. From 7c45d46a399a9be60a7f3d67c1fd40856260a6e8 Mon Sep 17 00:00:00 2001 From: yutianwu Date: Thu, 23 Mar 2023 10:59:23 +0800 Subject: [PATCH 3/3] check init module balance --- x/crosschain/types/genesis.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/crosschain/types/genesis.go b/x/crosschain/types/genesis.go index 4afef0135a..026426d2d9 100644 --- a/x/crosschain/types/genesis.go +++ b/x/crosschain/types/genesis.go @@ -22,7 +22,7 @@ func DefaultGenesisState() *GenesisState { // ValidateGenesis validates the cross chain genesis parameters func ValidateGenesis(data GenesisState) error { - if !data.Params.InitModuleBalance.IsPositive() { + if data.Params.InitModuleBalance.IsNil() || !data.Params.InitModuleBalance.IsPositive() { return fmt.Errorf("init module balance should be positive, is %s", data.Params.InitModuleBalance.String()) }