From 1ab7f2f737c63034840e9c235a4c095ce4515f13 Mon Sep 17 00:00:00 2001 From: Tanmay Date: Thu, 7 Mar 2024 13:35:55 -0500 Subject: [PATCH 1/6] remove observerslash amount from param keys for emissions --- x/emissions/abci.go | 5 ++++- x/emissions/types/keys.go | 2 ++ x/emissions/types/params.go | 2 +- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/x/emissions/abci.go b/x/emissions/abci.go index 2fb0561bb6..7e0137800a 100644 --- a/x/emissions/abci.go +++ b/x/emissions/abci.go @@ -113,7 +113,10 @@ func DistributeObserverRewards(ctx sdk.Context, amount sdkmath.Int, keeper keepe continue } if observerRewardUnits < 0 { - slashAmount := keeper.GetParams(ctx).ObserverSlashAmount + slashAmount, ok := sdkmath.NewIntFromString(types.ObserverSlashAmount) + if !ok { + continue + } keeper.SlashObserverEmission(ctx, observerAddress.String(), slashAmount) finalDistributionList = append(finalDistributionList, &types.ObserverEmission{ EmissionType: types.EmissionType_Slash, diff --git a/x/emissions/types/keys.go b/x/emissions/types/keys.go index 9f1fa705fc..72c375fc18 100644 --- a/x/emissions/types/keys.go +++ b/x/emissions/types/keys.go @@ -29,6 +29,8 @@ const ( EmissionScheduledYears = 4 AvgBlockTime = "5.7" + + ObserverSlashAmount = "100000000000000000" ) func KeyPrefix(p string) []byte { diff --git a/x/emissions/types/params.go b/x/emissions/types/params.go index aa896f77e8..9849a02faa 100644 --- a/x/emissions/types/params.go +++ b/x/emissions/types/params.go @@ -54,7 +54,7 @@ func (p *Params) ParamSetPairs() paramtypes.ParamSetPairs { paramtypes.NewParamSetPair(KeyPrefix(ParamObserverEmissionPercentage), &p.ObserverEmissionPercentage, validateObserverEmissionPercentage), paramtypes.NewParamSetPair(KeyPrefix(ParamTssSignerEmissionPercentage), &p.TssSignerEmissionPercentage, validateTssEmissonPercentage), paramtypes.NewParamSetPair(KeyPrefix(ParamDurationFactorConstant), &p.DurationFactorConstant, validateDurationFactorConstant), - paramtypes.NewParamSetPair(KeyPrefix(ParamObserverSlashAmount), &p.ObserverSlashAmount, validateObserverSlashAmount), + //paramtypes.NewParamSetPair(KeyPrefix(ParamObserverSlashAmount), &p.ObserverSlashAmount, validateObserverSlashAmount), } } From cd9bfcbc8b959f5f9ceb3cbe38000799e70f6a47 Mon Sep 17 00:00:00 2001 From: Tanmay Date: Thu, 7 Mar 2024 13:37:25 -0500 Subject: [PATCH 2/6] fix lint --- x/emissions/types/params.go | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/x/emissions/types/params.go b/x/emissions/types/params.go index 9849a02faa..af02d3c84e 100644 --- a/x/emissions/types/params.go +++ b/x/emissions/types/params.go @@ -72,16 +72,6 @@ func (p Params) String() string { return string(out) } -func validateObserverSlashAmount(i interface{}) error { - v, ok := i.(sdkmath.Int) - if !ok { - return fmt.Errorf("invalid parameter type: %T", i) - } - if v.LT(sdk.ZeroInt()) { - return fmt.Errorf("slash amount cannot be less than 0") - } - return nil -} func validateDurationFactorConstant(i interface{}) error { _, ok := i.(string) if !ok { From 5b29a2cf1cb5de2c252e906bdfdfe6b018ebac57 Mon Sep 17 00:00:00 2001 From: lumtis Date: Thu, 7 Mar 2024 20:08:56 +0100 Subject: [PATCH 3/6] fix: test --- x/emissions/genesis_test.go | 6 +++++- x/emissions/keeper/params_test.go | 1 - 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/x/emissions/genesis_test.go b/x/emissions/genesis_test.go index e1bebddee5..2314281c92 100644 --- a/x/emissions/genesis_test.go +++ b/x/emissions/genesis_test.go @@ -1,6 +1,7 @@ package emissions_test import ( + sdk "github.com/cosmos/cosmos-sdk/types" "testing" "github.com/stretchr/testify/require" @@ -12,8 +13,11 @@ import ( ) func TestGenesis(t *testing.T) { + params := types.DefaultParams() + params.ObserverSlashAmount = sdk.Int{} + genesisState := types.GenesisState{ - Params: types.DefaultParams(), + Params: params, WithdrawableEmissions: []types.WithdrawableEmissions{ sample.WithdrawableEmissions(t), sample.WithdrawableEmissions(t), diff --git a/x/emissions/keeper/params_test.go b/x/emissions/keeper/params_test.go index f861a944a3..bcf1794b62 100644 --- a/x/emissions/keeper/params_test.go +++ b/x/emissions/keeper/params_test.go @@ -26,7 +26,6 @@ func TestKeeper_GetParams(t *testing.T) { ObserverEmissionPercentage: "00.25", TssSignerEmissionPercentage: "00.25", DurationFactorConstant: "0.001877876953694702", - ObserverSlashAmount: sdkmath.NewInt(100000000000000000), }, isPanic: "", }, From d6e779effb1c66c1c971f9b8ccc8f59266cb78d0 Mon Sep 17 00:00:00 2001 From: lumtis Date: Fri, 8 Mar 2024 15:06:58 +0100 Subject: [PATCH 4/6] add tracked task --- x/emissions/abci.go | 4 ++++ x/emissions/types/keys.go | 4 ++++ x/emissions/types/params.go | 3 +++ 3 files changed, 11 insertions(+) diff --git a/x/emissions/abci.go b/x/emissions/abci.go index 7e0137800a..8e009356a0 100644 --- a/x/emissions/abci.go +++ b/x/emissions/abci.go @@ -113,10 +113,14 @@ func DistributeObserverRewards(ctx sdk.Context, amount sdkmath.Int, keeper keepe continue } if observerRewardUnits < 0 { + + // TODO : Replace hardcoded slash amount with a parameter + // https://github.com/zeta-chain/node/pull/1861 slashAmount, ok := sdkmath.NewIntFromString(types.ObserverSlashAmount) if !ok { continue } + keeper.SlashObserverEmission(ctx, observerAddress.String(), slashAmount) finalDistributionList = append(finalDistributionList, &types.ObserverEmission{ EmissionType: types.EmissionType_Slash, diff --git a/x/emissions/types/keys.go b/x/emissions/types/keys.go index 72c375fc18..21b07cb854 100644 --- a/x/emissions/types/keys.go +++ b/x/emissions/types/keys.go @@ -30,6 +30,10 @@ const ( EmissionScheduledYears = 4 AvgBlockTime = "5.7" + // ObserverSlashAmount is the amount of tokens to be slashed from observer in case of incorrect vote + // it is set to 0.1 ZETA + // TODO: replace this with a parameter + // https://github.com/zeta-chain/node/pull/1861 ObserverSlashAmount = "100000000000000000" ) diff --git a/x/emissions/types/params.go b/x/emissions/types/params.go index af02d3c84e..aa73e16ab9 100644 --- a/x/emissions/types/params.go +++ b/x/emissions/types/params.go @@ -54,6 +54,9 @@ func (p *Params) ParamSetPairs() paramtypes.ParamSetPairs { paramtypes.NewParamSetPair(KeyPrefix(ParamObserverEmissionPercentage), &p.ObserverEmissionPercentage, validateObserverEmissionPercentage), paramtypes.NewParamSetPair(KeyPrefix(ParamTssSignerEmissionPercentage), &p.TssSignerEmissionPercentage, validateTssEmissonPercentage), paramtypes.NewParamSetPair(KeyPrefix(ParamDurationFactorConstant), &p.DurationFactorConstant, validateDurationFactorConstant), + + // TODO: enable this param + // https://github.com/zeta-chain/node/pull/1861 //paramtypes.NewParamSetPair(KeyPrefix(ParamObserverSlashAmount), &p.ObserverSlashAmount, validateObserverSlashAmount), } } From d841d563367e8fae5a374e85789f1ef98d04d225 Mon Sep 17 00:00:00 2001 From: lumtis Date: Fri, 8 Mar 2024 15:25:24 +0100 Subject: [PATCH 5/6] fix tests --- changelog.md | 4 ++++ x/emissions/genesis_test.go | 3 ++- x/emissions/keeper/params_test.go | 16 +--------------- x/emissions/types/params.go | 12 +++++------- 4 files changed, 12 insertions(+), 23 deletions(-) diff --git a/changelog.md b/changelog.md index d4b8145915..db924b3ee5 100644 --- a/changelog.md +++ b/changelog.md @@ -30,6 +30,10 @@ * [1787](https://github.com/zeta-chain/node/pull/1787) - add unit tests for cross-chain evm hooks and e2e test failed withdraw to BTC legacy address * [1840](https://github.com/zeta-chain/node/pull/1840) - fix code coverage test failures ignored in CI +### Fixes + +* [1861](https://github.com/zeta-chain/node/pull/1861) - fix `ObserverSlashAmount` invalid read + ### Chores * [1814](https://github.com/zeta-chain/node/pull/1814) - fix code coverage ignore for protobuf generated files diff --git a/x/emissions/genesis_test.go b/x/emissions/genesis_test.go index 2314281c92..79edd3bf90 100644 --- a/x/emissions/genesis_test.go +++ b/x/emissions/genesis_test.go @@ -1,9 +1,10 @@ package emissions_test import ( - sdk "github.com/cosmos/cosmos-sdk/types" "testing" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/stretchr/testify/require" keepertest "github.com/zeta-chain/zetacore/testutil/keeper" "github.com/zeta-chain/zetacore/testutil/nullify" diff --git a/x/emissions/keeper/params_test.go b/x/emissions/keeper/params_test.go index bcf1794b62..e73031b824 100644 --- a/x/emissions/keeper/params_test.go +++ b/x/emissions/keeper/params_test.go @@ -3,7 +3,6 @@ package keeper_test import ( "testing" - sdkmath "cosmossdk.io/math" "github.com/stretchr/testify/require" keepertest "github.com/zeta-chain/zetacore/testutil/keeper" emissionstypes "github.com/zeta-chain/zetacore/x/emissions/types" @@ -40,7 +39,6 @@ func TestKeeper_GetParams(t *testing.T) { ObserverEmissionPercentage: "00.25", TssSignerEmissionPercentage: "00.25", DurationFactorConstant: "0.001877876953694702", - ObserverSlashAmount: sdkmath.NewInt(-10), }, isPanic: "slash amount cannot be less than 0", }, @@ -55,7 +53,6 @@ func TestKeeper_GetParams(t *testing.T) { ObserverEmissionPercentage: "00.25", TssSignerEmissionPercentage: "00.25", DurationFactorConstant: "0.001877876953694702", - ObserverSlashAmount: sdkmath.NewInt(100000000000000000), }, isPanic: "max bond factor cannot be higher that 0.25", }, @@ -70,7 +67,6 @@ func TestKeeper_GetParams(t *testing.T) { ObserverEmissionPercentage: "00.25", TssSignerEmissionPercentage: "00.25", DurationFactorConstant: "0.001877876953694702", - ObserverSlashAmount: sdkmath.NewInt(100000000000000000), }, isPanic: "min bond factor cannot be lower that 0.75", }, @@ -85,7 +81,6 @@ func TestKeeper_GetParams(t *testing.T) { ObserverEmissionPercentage: "00.25", TssSignerEmissionPercentage: "00.25", DurationFactorConstant: "0.001877876953694702", - ObserverSlashAmount: sdkmath.NewInt(100000000000000000), }, isPanic: "invalid block time", }, @@ -100,7 +95,6 @@ func TestKeeper_GetParams(t *testing.T) { ObserverEmissionPercentage: "00.25", TssSignerEmissionPercentage: "00.25", DurationFactorConstant: "0.001877876953694702", - ObserverSlashAmount: sdkmath.NewInt(100000000000000000), }, isPanic: "block time cannot be less than or equal to 0", }, @@ -115,7 +109,6 @@ func TestKeeper_GetParams(t *testing.T) { ObserverEmissionPercentage: "00.25", TssSignerEmissionPercentage: "00.25", DurationFactorConstant: "0.001877876953694702", - ObserverSlashAmount: sdkmath.NewInt(100000000000000000), }, isPanic: "target bond ratio cannot be more than 100 percent", }, @@ -130,7 +123,6 @@ func TestKeeper_GetParams(t *testing.T) { ObserverEmissionPercentage: "00.25", TssSignerEmissionPercentage: "00.25", DurationFactorConstant: "0.001877876953694702", - ObserverSlashAmount: sdkmath.NewInt(100000000000000000), }, isPanic: "target bond ratio cannot be less than 0 percent", }, @@ -145,7 +137,6 @@ func TestKeeper_GetParams(t *testing.T) { ObserverEmissionPercentage: "00.25", TssSignerEmissionPercentage: "00.25", DurationFactorConstant: "0.001877876953694702", - ObserverSlashAmount: sdkmath.NewInt(100000000000000000), }, isPanic: "validator emission percentage cannot be more than 100 percent", }, @@ -160,7 +151,6 @@ func TestKeeper_GetParams(t *testing.T) { ObserverEmissionPercentage: "00.25", TssSignerEmissionPercentage: "00.25", DurationFactorConstant: "0.001877876953694702", - ObserverSlashAmount: sdkmath.NewInt(100000000000000000), }, isPanic: "validator emission percentage cannot be less than 0 percent", }, @@ -175,7 +165,6 @@ func TestKeeper_GetParams(t *testing.T) { ObserverEmissionPercentage: "-00.25", TssSignerEmissionPercentage: "00.25", DurationFactorConstant: "0.001877876953694702", - ObserverSlashAmount: sdkmath.NewInt(100000000000000000), }, isPanic: "observer emission percentage cannot be less than 0 percent", }, @@ -190,7 +179,6 @@ func TestKeeper_GetParams(t *testing.T) { ObserverEmissionPercentage: "150.25", TssSignerEmissionPercentage: "00.25", DurationFactorConstant: "0.001877876953694702", - ObserverSlashAmount: sdkmath.NewInt(100000000000000000), }, isPanic: "observer emission percentage cannot be more than 100 percent", }, @@ -205,12 +193,11 @@ func TestKeeper_GetParams(t *testing.T) { ObserverEmissionPercentage: "00.25", TssSignerEmissionPercentage: "102.22", DurationFactorConstant: "0.001877876953694702", - ObserverSlashAmount: sdkmath.NewInt(100000000000000000), }, isPanic: "tss emission percentage cannot be more than 100 percent", }, { - name: "tss signer percentage too loo", + name: "tss signer percentage too low", params: emissionstypes.Params{ MaxBondFactor: "1.25", MinBondFactor: "0.75", @@ -220,7 +207,6 @@ func TestKeeper_GetParams(t *testing.T) { ObserverEmissionPercentage: "00.25", TssSignerEmissionPercentage: "-102.22", DurationFactorConstant: "0.001877876953694702", - ObserverSlashAmount: sdkmath.NewInt(100000000000000000), }, isPanic: "tss emission percentage cannot be less than 0 percent", }, diff --git a/x/emissions/types/params.go b/x/emissions/types/params.go index aa73e16ab9..96267b0a70 100644 --- a/x/emissions/types/params.go +++ b/x/emissions/types/params.go @@ -4,7 +4,6 @@ import ( "fmt" "strconv" - sdkmath "cosmossdk.io/math" sdk "github.com/cosmos/cosmos-sdk/types" paramtypes "github.com/cosmos/cosmos-sdk/x/params/types" "gopkg.in/yaml.v2" @@ -20,11 +19,6 @@ func ParamKeyTable() paramtypes.KeyTable { // NewParams creates a new Params instance func NewParams() Params { - defaultSlashAmount := sdk.ZeroInt() - intSlashAmount, ok := sdkmath.NewIntFromString("100000000000000000") - if ok { - defaultSlashAmount = intSlashAmount - } return Params{ MaxBondFactor: "1.25", MinBondFactor: "0.75", @@ -34,7 +28,11 @@ func NewParams() Params { ObserverEmissionPercentage: "00.25", TssSignerEmissionPercentage: "00.25", DurationFactorConstant: "0.001877876953694702", - ObserverSlashAmount: defaultSlashAmount, + + // ObserverSlashAmount is currently disabled + // TODO: enable this param + // https://github.com/zeta-chain/node/issues/1862 + ObserverSlashAmount: sdk.Int{}, } } From 941db31b1308f02c23edaf029e7fc46a0d736c95 Mon Sep 17 00:00:00 2001 From: lumtis Date: Fri, 8 Mar 2024 22:38:36 +0100 Subject: [PATCH 6/6] fix tests --- x/emissions/abci.go | 24 +++++++++++++++--------- x/emissions/abci_test.go | 4 +++- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/x/emissions/abci.go b/x/emissions/abci.go index 8e009356a0..96be4ae613 100644 --- a/x/emissions/abci.go +++ b/x/emissions/abci.go @@ -22,6 +22,14 @@ func BeginBlocker(ctx sdk.Context, keeper keeper.Keeper) { observerRewards := sdk.MustNewDecFromStr(keeper.GetParams(ctx).ObserverEmissionPercentage).Mul(blockRewards).TruncateInt() tssSignerRewards := sdk.MustNewDecFromStr(keeper.GetParams(ctx).TssSignerEmissionPercentage).Mul(blockRewards).TruncateInt() + // TODO : Replace hardcoded slash amount with a parameter + // https://github.com/zeta-chain/node/pull/1861 + slashAmount, ok := sdkmath.NewIntFromString(types.ObserverSlashAmount) + if !ok { + ctx.Logger().Error(fmt.Sprintf("Error while parsing observer slash amount %s", types.ObserverSlashAmount)) + return + } + // Use a tmpCtx, which is a cache-wrapped context to avoid writing to the store // We commit only if all three distributions are successful, if not the funds stay in the emission pool tmpCtx, commit := ctx.CacheContext() @@ -30,7 +38,7 @@ func BeginBlocker(ctx sdk.Context, keeper keeper.Keeper) { ctx.Logger().Error(fmt.Sprintf("Error while distributing validator rewards %s", err)) return } - err = DistributeObserverRewards(tmpCtx, observerRewards, keeper) + err = DistributeObserverRewards(tmpCtx, observerRewards, keeper, slashAmount) if err != nil { ctx.Logger().Error(fmt.Sprintf("Error while distributing observer rewards %s", err)) return @@ -63,7 +71,12 @@ func DistributeValidatorRewards(ctx sdk.Context, amount sdkmath.Int, bankKeeper // NotVoted or Unsuccessful votes are slashed // rewards given or slashed amounts are in azeta -func DistributeObserverRewards(ctx sdk.Context, amount sdkmath.Int, keeper keeper.Keeper) error { +func DistributeObserverRewards( + ctx sdk.Context, + amount sdkmath.Int, + keeper keeper.Keeper, + slashAmount sdkmath.Int, +) error { rewardsDistributer := map[string]int64{} totalRewardsUnits := int64(0) @@ -114,13 +127,6 @@ func DistributeObserverRewards(ctx sdk.Context, amount sdkmath.Int, keeper keepe } if observerRewardUnits < 0 { - // TODO : Replace hardcoded slash amount with a parameter - // https://github.com/zeta-chain/node/pull/1861 - slashAmount, ok := sdkmath.NewIntFromString(types.ObserverSlashAmount) - if !ok { - continue - } - keeper.SlashObserverEmission(ctx, observerAddress.String(), slashAmount) finalDistributionList = append(finalDistributionList, &types.ObserverEmission{ EmissionType: types.EmissionType_Slash, diff --git a/x/emissions/abci_test.go b/x/emissions/abci_test.go index 07240d1450..a4a66b34a0 100644 --- a/x/emissions/abci_test.go +++ b/x/emissions/abci_test.go @@ -153,6 +153,7 @@ func TestBeginBlocker(t *testing.T) { } func TestDistributeObserverRewards(t *testing.T) { + keepertest.SetConfig(false) observerSet := sample.ObserverSet(4) tt := []struct { @@ -286,8 +287,9 @@ func TestDistributeObserverRewards(t *testing.T) { ctx = ctx.WithBlockHeight(100) // Distribute the rewards and check if the rewards are distributed correctly - err = emissionsModule.DistributeObserverRewards(ctx, tc.totalRewardsForBlock, *k) + err = emissionsModule.DistributeObserverRewards(ctx, tc.totalRewardsForBlock, *k, tc.slashAmount) require.NoError(t, err) + for i, observer := range observerSet.ObserverList { observerEmission, found := k.GetWithdrawableEmission(ctx, observer) require.True(t, found, "withdrawable emission not found for observer %d", i)