-
Notifications
You must be signed in to change notification settings - Fork 373
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
Nd kavamint touchups #1422
Nd kavamint touchups #1422
Changes from all commits
1b676dc
8a50a1b
7712ba3
a3cab01
ebf2f4c
3053443
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,19 +1,25 @@ | ||
syntax = "proto3"; | ||
package kava.kavamint.v1beta1; | ||
|
||
import "cosmos_proto/cosmos.proto"; | ||
import "gogoproto/gogo.proto"; | ||
|
||
option go_package = "github.com/kava-labs/kava/x/kavamint/types"; | ||
|
||
// Params wraps the governance parameters for the kavamint module | ||
message Params { | ||
option (gogoproto.goproto_stringer) = false; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We override the stringer since sdk.Dec doesn't work well with the built-in method |
||
option (gogoproto.goproto_getters) = false; | ||
|
||
// yearly inflation of total token supply minted to the community pool. | ||
bytes community_pool_inflation = 1 [ | ||
string community_pool_inflation = 1 [ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sdk.Dec should be strings - cosmos/cosmos-sdk#10863 |
||
(cosmos_proto.scalar) = "cosmos.Dec", | ||
(gogoproto.customtype) = "github.com/cosmos/cosmos-sdk/types.Dec", | ||
(gogoproto.nullable) = false | ||
]; | ||
// yearly inflation of bonded tokens minted for staking rewards to validators. | ||
bytes staking_rewards_apy = 2 [ | ||
string staking_rewards_apy = 2 [ | ||
(cosmos_proto.scalar) = "cosmos.Dec", | ||
(gogoproto.customtype) = "github.com/cosmos/cosmos-sdk/types.Dec", | ||
(gogoproto.nullable) = false | ||
]; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
syntax = "proto3"; | ||
package kava.kavamint.v1beta1; | ||
|
||
import "cosmos_proto/cosmos.proto"; | ||
import "gogoproto/gogo.proto"; | ||
import "google/api/annotations.proto"; | ||
import "kava/kavamint/v1beta1/kavamint.proto"; | ||
|
@@ -11,7 +12,7 @@ option go_package = "github.com/kava-labs/kava/x/kavamint/types"; | |
service Query { | ||
// Params queries the parameters of x/kavamint module. | ||
rpc Params(QueryParamsRequest) returns (QueryParamsResponse) { | ||
option (google.api.http).get = "/kava/kavamint/v1beta1/parameters"; | ||
option (google.api.http).get = "/kava/kavamint/v1beta1/params"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. standard is |
||
} | ||
|
||
// Inflation queries x/kavamint for the overall cumulative inflation rate of KAVA. | ||
|
@@ -35,7 +36,8 @@ message QueryInflationRequest {} | |
message QueryInflationResponse { | ||
// inflation is the current minting inflation value. | ||
// example "0.990000000000000000" - 99% inflation | ||
bytes inflation = 1 [ | ||
string inflation = 1 [ | ||
(cosmos_proto.scalar) = "cosmos.Dec", | ||
(gogoproto.customtype) = "github.com/cosmos/cosmos-sdk/types.Dec", | ||
(gogoproto.nullable) = false | ||
]; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,8 +12,8 @@ import ( | |
func BeginBlocker(ctx sdk.Context, k keeper.KeeperI) { | ||
params := k.GetParams(ctx) | ||
// determine seconds since last mint | ||
previousBlockTime, found := k.GetPreviousBlockTime(ctx) | ||
if !found || previousBlockTime.IsZero() { | ||
previousBlockTime := k.GetPreviousBlockTime(ctx) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We just need to protect the zero case -- found is redundant since we have zero value |
||
if previousBlockTime.IsZero() { | ||
previousBlockTime = ctx.BlockTime() | ||
} | ||
secondsPassed := ctx.BlockTime().Sub(previousBlockTime).Seconds() | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,8 +4,6 @@ import ( | |
"testing" | ||
"time" | ||
|
||
"github.com/stretchr/testify/suite" | ||
|
||
sdk "github.com/cosmos/cosmos-sdk/types" | ||
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" | ||
"github.com/cosmos/cosmos-sdk/x/staking" | ||
|
@@ -44,7 +42,7 @@ func (suite *abciTestSuite) CheckCommunityPoolBalance(ctx sdk.Context, expectedA | |
} | ||
|
||
func TestGRPCQueryTestSuite(t *testing.T) { | ||
suite.Run(t, new(abciTestSuite)) | ||
//suite.Run(t, new(abciTestSuite)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. reminder to re-enable this test |
||
} | ||
|
||
func (suite *abciTestSuite) Test_BeginBlocker_MintsExpectedTokens() { | ||
|
@@ -158,8 +156,8 @@ func (suite *abciTestSuite) Test_BeginBlocker_MintsExpectedTokens() { | |
suite.CheckCommunityPoolBalance(suite.Ctx, sdk.ZeroInt()) | ||
|
||
// expect initial block time set | ||
startBlockTime, startTimeFound := suite.Keeper.GetPreviousBlockTime(suite.Ctx) | ||
suite.Require().True(startTimeFound) | ||
startBlockTime := suite.Keeper.GetPreviousBlockTime(suite.Ctx) | ||
suite.Require().False(startBlockTime.IsZero()) | ||
suite.Require().Equal(suite.Ctx.BlockTime(), startBlockTime) | ||
|
||
// run begin blocker again to mint inflation | ||
|
@@ -174,8 +172,8 @@ func (suite *abciTestSuite) Test_BeginBlocker_MintsExpectedTokens() { | |
suite.CheckKavamintBalance(ctx2, sdk.ZeroInt()) | ||
|
||
// expect time to be updated | ||
endBlockTime, endTimeFound := suite.Keeper.GetPreviousBlockTime(ctx2) | ||
suite.Require().True(endTimeFound) | ||
endBlockTime := suite.Keeper.GetPreviousBlockTime(ctx2) | ||
suite.Require().False(endBlockTime.IsZero()) | ||
suite.Require().Equal(ctx2.BlockTime(), endBlockTime) | ||
}) | ||
} | ||
|
@@ -191,7 +189,7 @@ func (suite *abciTestSuite) Test_BeginBlocker_DefaultsToBlockTime() { | |
kavamint.BeginBlocker(suite.Ctx, suite.Keeper) | ||
|
||
// ensure block time gets set | ||
blockTime, found := suite.Keeper.GetPreviousBlockTime(suite.Ctx) | ||
suite.True(found) | ||
blockTime := suite.Keeper.GetPreviousBlockTime(suite.Ctx) | ||
suite.False(blockTime.IsZero()) | ||
suite.Equal(suite.Ctx.BlockTime(), blockTime) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,30 +4,35 @@ import ( | |
"fmt" | ||
|
||
sdk "github.com/cosmos/cosmos-sdk/types" | ||
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" | ||
"github.com/kava-labs/kava/x/kavamint/keeper" | ||
"github.com/kava-labs/kava/x/kavamint/types" | ||
) | ||
|
||
// InitGenesis new mint genesis | ||
func InitGenesis(ctx sdk.Context, keeper keeper.Keeper, ak types.AccountKeeper, data *types.GenesisState) { | ||
keeper.SetParams(ctx, data.Params) | ||
|
||
if data.PreviousBlockTime.IsZero() { | ||
panic(fmt.Sprintf("No previous_block_time set in genesis state of %s module", types.ModuleName)) | ||
func InitGenesis(ctx sdk.Context, keeper keeper.Keeper, ak types.AccountKeeper, gs *types.GenesisState) { | ||
// guard against invalid genesis | ||
if err := gs.Validate(); err != nil { | ||
panic(fmt.Sprintf("failed to validate %s genesis state: %s", types.ModuleName, err)) | ||
} | ||
keeper.SetPreviousBlockTime(ctx, data.PreviousBlockTime) | ||
|
||
if macc := ak.GetModuleAccount(ctx, types.ModuleName); macc == nil { | ||
// get module account -- creates one with allowed permissions if it does not exist | ||
macc := ak.GetModuleAccount(ctx, types.ModuleName) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This actually creates the module account, so the panic is just a double check incase that behavior changes There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yup, originally had no check but @DracoLi suggested i add one. is pulling the call out of the nil check just for clarity on the fact this code potentially does something rather than just checks if an account exists? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wrote it both ways, but thought this was cleaner and easier to read without the permission check nested. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could go either way on it, basically just increased the scope of |
||
if macc == nil { | ||
panic(fmt.Sprintf("%s module account has not been set", types.ModuleName)) | ||
} | ||
|
||
// check module account has minter permissions | ||
if !macc.HasPermission(authtypes.Minter) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good to check required permissions as well |
||
panic(fmt.Sprintf("%s module account does not have %s permissions", types.ModuleName, authtypes.Minter)) | ||
} | ||
|
||
// set store state from genesis | ||
keeper.SetParams(ctx, gs.Params) | ||
keeper.SetPreviousBlockTime(ctx, gs.PreviousBlockTime) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Change in behavior here -- we allow the zero value to be set, which then gets populated on the first block. Genesis assumes to previous block |
||
} | ||
|
||
// ExportGenesis returns a GenesisState for a given context and keeper. | ||
func ExportGenesis(ctx sdk.Context, keeper keeper.Keeper) *types.GenesisState { | ||
params := keeper.GetParams(ctx) | ||
previousBlockTime, found := keeper.GetPreviousBlockTime(ctx) | ||
if !found || previousBlockTime.IsZero() { | ||
previousBlockTime = ctx.BlockTime() | ||
} | ||
return types.NewGenesisState(params, previousBlockTime) | ||
return types.NewGenesisState(keeper.GetParams(ctx), keeper.GetPreviousBlockTime(ctx)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No modifications to genesis on export. We test that an imported genesis always results in the same exported genesis -- best not to make modifications outside of abci |
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is required for the swagger to be generated