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.

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

refactor: prevent panicking with params #1864

Merged
merged 12 commits into from
Mar 15, 2024
1 change: 1 addition & 0 deletions changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
* [1783](https://github.com/zeta-chain/node/pull/1783) - refactor zetaclient metrics naming and structure
* [1774](https://github.com/zeta-chain/node/pull/1774) - split params and config in zetaclient
* [1831](https://github.com/zeta-chain/node/pull/1831) - removing unnecessary pointers in context structure
* [1864](https://github.com/zeta-chain/node/pull/1864) - prevent panic in param management
* [1848](https://github.com/zeta-chain/node/issues/1848) - create a method to observe deposits to tss address in one evm block
* [1885](https://github.com/zeta-chain/node/pull/1885) - change important metrics on port 8123 to be prometheus compatible
* [1863](https://github.com/zeta-chain/node/pull/1863) - remove duplicate ValidateChainParams function
Expand Down
38 changes: 27 additions & 11 deletions testutil/keeper/emissions.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,15 @@ import (
)

type EmissionMockOptions struct {
UseBankMock bool
UseStakingMock bool
UseObserverMock bool
UseAccountMock bool
UseBankMock bool
UseStakingMock bool
UseObserverMock bool
UseAccountMock bool
UseParamStoreMock bool
}

func EmissionsKeeper(t testing.TB) (*keeper.Keeper, sdk.Context, SDKKeepers, ZetaKeepers) {
return EmissionKeeperWithMockOptions(t, EmissionMockOptions{
UseBankMock: false,
UseStakingMock: false,
UseObserverMock: false,
})
return EmissionKeeperWithMockOptions(t, EmissionMockOptions{})
}
func EmissionKeeperWithMockOptions(
t testing.TB,
Expand Down Expand Up @@ -91,18 +88,31 @@ func EmissionKeeperWithMockOptions(
observerKeeper = emissionsmocks.NewEmissionObserverKeeper(t)
}

var paramStore types.ParamStore
if mockOptions.UseParamStoreMock {
mock := emissionsmocks.NewEmissionParamStore(t)
// mock this method for the keeper constructor
mock.On("HasKeyTable").Maybe().Return(true)
paramStore = mock
} else {
paramStore = sdkKeepers.ParamsKeeper.Subspace(types.ModuleName)
}

k := keeper.NewKeeper(
cdc,
storeKey,
memStoreKey,
sdkKeepers.ParamsKeeper.Subspace(types.ModuleName),
paramStore,
authtypes.FeeCollectorName,
bankKeeper,
stakingKeeper,
observerKeeper,
authKeeper,
)
k.SetParams(ctx, types.DefaultParams())

if !mockOptions.UseParamStoreMock {
k.SetParams(ctx, types.DefaultParams())
}

return k, ctx, sdkKeepers, zetaKeepers
}
Expand All @@ -112,3 +122,9 @@ func GetEmissionsBankMock(t testing.TB, keeper *keeper.Keeper) *emissionsmocks.E
require.True(t, ok)
return cbk
}

func GetEmissionsParamStoreMock(t testing.TB, keeper *keeper.Keeper) *emissionsmocks.EmissionParamStore {
m, ok := keeper.GetParamStore().(*emissionsmocks.EmissionParamStore)
require.True(t, ok)
return m
}
18 changes: 0 additions & 18 deletions testutil/keeper/mocks/crosschain/observer.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

76 changes: 76 additions & 0 deletions testutil/keeper/mocks/emissions/param_store.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

20 changes: 0 additions & 20 deletions testutil/keeper/mocks/fungible/observer.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions testutil/keeper/mocks/mocks.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,11 @@ type EmissionObserverKeeper interface {
emissionstypes.ObserverKeeper
}

//go:generate mockery --name EmissionParamStore --filename param_store.go --case underscore --output ./emissions
type EmissionParamStore interface {
emissionstypes.ParamStore
}

/**
* Observer Mocks
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import (
)

func setupVerificationParams(zk keepertest.ZetaKeepers, ctx sdk.Context, tx_index int64, chainID int64, header ethtypes.Header, headerRLP []byte, block *ethtypes.Block) {
params := zk.ObserverKeeper.GetParams(ctx)
params := zk.ObserverKeeper.GetParamsIfExists(ctx)
zk.ObserverKeeper.SetParams(ctx, params)
zk.ObserverKeeper.SetBlockHeader(ctx, common.BlockHeader{
Height: block.Number().Int64(),
Expand Down
1 change: 0 additions & 1 deletion x/crosschain/types/expected_keepers.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ type BankKeeper interface {
type ObserverKeeper interface {
GetObserverSet(ctx sdk.Context) (val observertypes.ObserverSet, found bool)
GetBallot(ctx sdk.Context, index string) (val observertypes.Ballot, found bool)
GetParams(ctx sdk.Context) (params observertypes.Params)
GetChainParamsByChainID(ctx sdk.Context, chainID int64) (params *observertypes.ChainParams, found bool)
GetNodeAccount(ctx sdk.Context, address string) (nodeAccount observertypes.NodeAccount, found bool)
GetAllNodeAccount(ctx sdk.Context) (nodeAccounts []observertypes.NodeAccount)
Expand Down
9 changes: 4 additions & 5 deletions x/emissions/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,10 @@ func BeginBlocker(ctx sdk.Context, keeper keeper.Keeper) {
ctx.Logger().Info(fmt.Sprintf("Block rewards %s are greater than emission pool balance %s", blockRewards.String(), emissionPoolBalance.String()))
return
}
validatorRewards := sdk.MustNewDecFromStr(keeper.GetParams(ctx).ValidatorEmissionPercentage).Mul(blockRewards).TruncateInt()
observerRewards := sdk.MustNewDecFromStr(keeper.GetParams(ctx).ObserverEmissionPercentage).Mul(blockRewards).TruncateInt()
tssSignerRewards := sdk.MustNewDecFromStr(keeper.GetParams(ctx).TssSignerEmissionPercentage).Mul(blockRewards).TruncateInt()

// Get the distribution of rewards
params := keeper.GetParamsIfExists(ctx)
validatorRewards, observerRewards, tssSignerRewards := types.GetRewardsDistributions(params)

// TODO : Replace hardcoded slash amount with a parameter
// https://github.com/zeta-chain/node/pull/1861
Expand Down Expand Up @@ -70,7 +71,6 @@ func DistributeValidatorRewards(ctx sdk.Context, amount sdkmath.Int, bankKeeper
// The total rewards are distributed equally among all Successful votes
// NotVoted or Unsuccessful votes are slashed
// rewards given or slashed amounts are in azeta

func DistributeObserverRewards(
ctx sdk.Context,
amount sdkmath.Int,
Expand Down Expand Up @@ -126,7 +126,6 @@ func DistributeObserverRewards(
continue
}
if observerRewardUnits < 0 {

keeper.SlashObserverEmission(ctx, observerAddress.String(), slashAmount)
finalDistributionList = append(finalDistributionList, &types.ObserverEmission{
EmissionType: types.EmissionType_Slash,
Expand Down
6 changes: 3 additions & 3 deletions x/emissions/abci_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,9 @@ func TestBeginBlocker(t *testing.T) {
emissionPool := sk.AuthKeeper.GetModuleAccount(ctx, emissionstypes.ModuleName).GetAddress()

blockRewards := emissionstypes.BlockReward
observerRewardsForABlock := blockRewards.Mul(sdk.MustNewDecFromStr(k.GetParams(ctx).ObserverEmissionPercentage)).TruncateInt()
validatorRewardsForABlock := blockRewards.Mul(sdk.MustNewDecFromStr(k.GetParams(ctx).ValidatorEmissionPercentage)).TruncateInt()
tssSignerRewardsForABlock := blockRewards.Mul(sdk.MustNewDecFromStr(k.GetParams(ctx).TssSignerEmissionPercentage)).TruncateInt()
observerRewardsForABlock := blockRewards.Mul(sdk.MustNewDecFromStr(k.GetParamsIfExists(ctx).ObserverEmissionPercentage)).TruncateInt()
validatorRewardsForABlock := blockRewards.Mul(sdk.MustNewDecFromStr(k.GetParamsIfExists(ctx).ValidatorEmissionPercentage)).TruncateInt()
tssSignerRewardsForABlock := blockRewards.Mul(sdk.MustNewDecFromStr(k.GetParamsIfExists(ctx).TssSignerEmissionPercentage)).TruncateInt()
distributedRewards := observerRewardsForABlock.Add(validatorRewardsForABlock).Add(tssSignerRewardsForABlock)

require.True(t, blockRewards.TruncateInt().GT(distributedRewards))
Expand Down
2 changes: 1 addition & 1 deletion x/emissions/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ func InitGenesis(ctx sdk.Context, k keeper.Keeper, genState types.GenesisState)
// ExportGenesis returns the emissions module's exported genesis.
func ExportGenesis(ctx sdk.Context, k keeper.Keeper) *types.GenesisState {
var genesis types.GenesisState
genesis.Params = k.GetParams(ctx)
genesis.Params = k.GetParamsIfExists(ctx)
genesis.WithdrawableEmissions = k.GetAllWithdrawableEmission(ctx)

return &genesis
Expand Down
10 changes: 5 additions & 5 deletions x/emissions/keeper/block_rewards_components.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@
return reservesFactor, bondFactor, durationFactor
}
func (k Keeper) GetBondFactor(ctx sdk.Context, stakingKeeper types.StakingKeeper) sdk.Dec {
targetBondRatio := sdk.MustNewDecFromStr(k.GetParams(ctx).TargetBondRatio)
maxBondFactor := sdk.MustNewDecFromStr(k.GetParams(ctx).MaxBondFactor)
minBondFactor := sdk.MustNewDecFromStr(k.GetParams(ctx).MinBondFactor)
targetBondRatio := sdk.MustNewDecFromStr(k.GetParamsIfExists(ctx).TargetBondRatio)
maxBondFactor := sdk.MustNewDecFromStr(k.GetParamsIfExists(ctx).MaxBondFactor)
minBondFactor := sdk.MustNewDecFromStr(k.GetParamsIfExists(ctx).MinBondFactor)

Check warning on line 22 in x/emissions/keeper/block_rewards_components.go

View check run for this annotation

Codecov / codecov/patch

x/emissions/keeper/block_rewards_components.go#L20-L22

Added lines #L20 - L22 were not covered by tests
kingpinXD marked this conversation as resolved.
Show resolved Hide resolved

currentBondedRatio := stakingKeeper.BondedRatio(ctx)
// Bond factor ranges between minBondFactor (0.75) to maxBondFactor (1.25)
Expand All @@ -37,10 +37,10 @@
}

func (k Keeper) GetDurationFactor(ctx sdk.Context) sdk.Dec {
avgBlockTime := sdk.MustNewDecFromStr(k.GetParams(ctx).AvgBlockTime)
avgBlockTime := sdk.MustNewDecFromStr(k.GetParamsIfExists(ctx).AvgBlockTime)

Check warning on line 40 in x/emissions/keeper/block_rewards_components.go

View check run for this annotation

Codecov / codecov/patch

x/emissions/keeper/block_rewards_components.go#L40

Added line #L40 was not covered by tests
kingpinXD marked this conversation as resolved.
Show resolved Hide resolved
NumberOfBlocksInAMonth := sdk.NewDec(types.SecsInMonth).Quo(avgBlockTime)
monthFactor := sdk.NewDec(ctx.BlockHeight()).Quo(NumberOfBlocksInAMonth)
logValueDec := sdk.MustNewDecFromStr(k.GetParams(ctx).DurationFactorConstant)
logValueDec := sdk.MustNewDecFromStr(k.GetParamsIfExists(ctx).DurationFactorConstant)

Check warning on line 43 in x/emissions/keeper/block_rewards_components.go

View check run for this annotation

Codecov / codecov/patch

x/emissions/keeper/block_rewards_components.go#L43

Added line #L43 was not covered by tests
// month * log(1 + 0.02 / 12)
fractionNumerator := monthFactor.Mul(logValueDec)
// (month * log(1 + 0.02 / 12) ) + 1
Expand Down
2 changes: 1 addition & 1 deletion x/emissions/keeper/grpc_query_params.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,5 @@ func (k Keeper) Params(c context.Context, req *types.QueryParamsRequest) (*types
}
ctx := sdk.UnwrapSDKContext(c)

return &types.QueryParamsResponse{Params: k.GetParams(ctx)}, nil
return &types.QueryParamsResponse{Params: k.GetParamsIfExists(ctx)}, nil
}
11 changes: 7 additions & 4 deletions x/emissions/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,14 @@ import (

"github.com/cosmos/cosmos-sdk/codec"
sdk "github.com/cosmos/cosmos-sdk/types"
paramtypes "github.com/cosmos/cosmos-sdk/x/params/types"
)

type (
Keeper struct {
cdc codec.BinaryCodec
storeKey storetypes.StoreKey
memKey storetypes.StoreKey
paramstore paramtypes.Subspace
paramStore types.ParamStore
feeCollectorName string
bankKeeper types.BankKeeper
stakingKeeper types.StakingKeeper
Expand All @@ -30,7 +29,7 @@ func NewKeeper(
cdc codec.BinaryCodec,
storeKey,
memKey storetypes.StoreKey,
ps paramtypes.Subspace,
ps types.ParamStore,
feeCollectorName string,
bankKeeper types.BankKeeper,
stakingKeeper types.StakingKeeper,
Expand All @@ -46,7 +45,7 @@ func NewKeeper(
cdc: cdc,
storeKey: storeKey,
memKey: memKey,
paramstore: ps,
paramStore: ps,
feeCollectorName: feeCollectorName,
bankKeeper: bankKeeper,
stakingKeeper: stakingKeeper,
Expand Down Expand Up @@ -78,3 +77,7 @@ func (k Keeper) GetObserverKeeper() types.ObserverKeeper {
func (k Keeper) GetAuthKeeper() types.AccountKeeper {
return k.authKeeper
}

func (k Keeper) GetParamStore() types.ParamStore {
return k.paramStore
}
9 changes: 5 additions & 4 deletions x/emissions/keeper/params.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,14 @@ import (
"github.com/zeta-chain/zetacore/x/emissions/types"
)

// GetParams get all parameters as types.Params
func (k Keeper) GetParams(ctx sdk.Context) (params types.Params) {
k.paramstore.GetParamSet(ctx, &params)
// GetParamsIfExists get all parameters as types.Params if they exist
// non existent parameters will return zero values
func (k Keeper) GetParamsIfExists(ctx sdk.Context) (params types.Params) {
k.paramStore.GetParamSetIfExists(ctx, &params)
return
}

// SetParams set the params
func (k Keeper) SetParams(ctx sdk.Context, params types.Params) {
k.paramstore.SetParamSet(ctx, &params)
k.paramStore.SetParamSet(ctx, &params)
}
Loading
Loading