From 5f723f999ccc9cfbbe7716a180f7c819ebf69d2b Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Sat, 18 Feb 2023 13:47:33 +0100 Subject: [PATCH 01/13] fix(oracle): power vote calculation --- x/oracle/abci_test.go | 108 ++++++++++++++++++++++++-------------- x/oracle/keeper/ballot.go | 5 +- 2 files changed, 71 insertions(+), 42 deletions(-) diff --git a/x/oracle/abci_test.go b/x/oracle/abci_test.go index 6223422c56..5dd5145c37 100644 --- a/x/oracle/abci_test.go +++ b/x/oracle/abci_test.go @@ -35,7 +35,7 @@ type IntegrationTestSuite struct { } const ( - initialPower = int64(10000) + initialPower = int64(1000) ) func (s *IntegrationTestSuite) SetupTest() { @@ -52,13 +52,14 @@ func (s *IntegrationTestSuite) SetupTest() { sh.Denom = bondDenom // mint and send coins to validator - require.NoError(app.BankKeeper.MintCoins(ctx, minttypes.ModuleName, initCoins)) + require.NoError(app.BankKeeper.MintCoins(ctx, minttypes.ModuleName, initCoins.MulInt(sdk.NewIntFromUint64(3)))) require.NoError(app.BankKeeper.SendCoinsFromModuleToAccount(ctx, minttypes.ModuleName, addr1, initCoins)) - require.NoError(app.BankKeeper.MintCoins(ctx, minttypes.ModuleName, initCoins)) require.NoError(app.BankKeeper.SendCoinsFromModuleToAccount(ctx, minttypes.ModuleName, addr2, initCoins)) + require.NoError(app.BankKeeper.SendCoinsFromModuleToAccount(ctx, minttypes.ModuleName, addr3, initCoins)) - sh.CreateValidatorWithValPower(valAddr1, valPubKey1, 7000, true) - sh.CreateValidatorWithValPower(valAddr2, valPubKey2, 3000, true) + sh.CreateValidatorWithValPower(valAddr1, valPubKey1, 699, true) + sh.CreateValidatorWithValPower(valAddr2, valPubKey2, 300, true) + sh.CreateValidatorWithValPower(valAddr3, valPubKey3, 1, true) staking.EndBlocker(ctx, *app.StakingKeeper) @@ -68,7 +69,7 @@ func (s *IntegrationTestSuite) SetupTest() { // Test addresses var ( - valPubKeys = simapp.CreateTestPubKeys(2) + valPubKeys = simapp.CreateTestPubKeys(3) valPubKey1 = valPubKeys[0] pubKey1 = secp256k1.GenPrivKey().PubKey() @@ -80,6 +81,11 @@ var ( addr2 = sdk.AccAddress(pubKey2.Address()) valAddr2 = sdk.ValAddress(pubKey2.Address()) + valPubKey3 = valPubKeys[2] + pubKey3 = secp256k1.GenPrivKey().PubKey() + addr3 = sdk.AccAddress(pubKey3.Address()) + valAddr3 = sdk.ValAddress(pubKey3.Address()) + initTokens = sdk.TokensFromConsensusPower(initialPower, sdk.DefaultPowerReduction) initCoins = sdk.NewCoins(sdk.NewCoin(bondDenom, initTokens)) ) @@ -92,12 +98,9 @@ func (s *IntegrationTestSuite) TestEndBlockerVoteThreshold() { voteBlockDiff := int64(app.OracleKeeper.VotePeriod(ctx)/2 + 1) var ( - val1Tuples types.ExchangeRateTuples - val2Tuples types.ExchangeRateTuples - val1PreVotes types.AggregateExchangeRatePrevote - val2PreVotes types.AggregateExchangeRatePrevote - val1Votes types.AggregateExchangeRateVote - val2Votes types.AggregateExchangeRateVote + val1Tuples types.ExchangeRateTuples + val2Tuples types.ExchangeRateTuples + val3Tuples types.ExchangeRateTuples ) for _, denom := range app.OracleKeeper.AcceptList(ctx) { val1Tuples = append(val1Tuples, types.ExchangeRateTuple{ @@ -108,36 +111,39 @@ func (s *IntegrationTestSuite) TestEndBlockerVoteThreshold() { Denom: denom.SymbolDenom, ExchangeRate: sdk.MustNewDecFromStr("0.5"), }) + val3Tuples = append(val3Tuples, types.ExchangeRateTuple{ + Denom: denom.SymbolDenom, + ExchangeRate: sdk.MustNewDecFromStr("0.6"), + }) } - val1PreVotes = types.AggregateExchangeRatePrevote{ - Hash: "hash1", - Voter: valAddr1.String(), - SubmitBlock: uint64(ctx.BlockHeight()), - } - val2PreVotes = types.AggregateExchangeRatePrevote{ - Hash: "hash2", - Voter: valAddr2.String(), - SubmitBlock: uint64(ctx.BlockHeight()), - } - - val1Votes = types.AggregateExchangeRateVote{ - ExchangeRateTuples: val1Tuples, - Voter: valAddr1.String(), - } - val2Votes = types.AggregateExchangeRateVote{ - ExchangeRateTuples: val2Tuples, - Voter: valAddr2.String(), + createVote := func(hash string, val sdk.ValAddress, rates types.ExchangeRateTuples, blockHeight uint64) (types.AggregateExchangeRatePrevote, types.AggregateExchangeRateVote) { + preVote := types.AggregateExchangeRatePrevote{ + Hash: "hash1", + Voter: val.String(), + SubmitBlock: uint64(blockHeight), + } + vote := types.AggregateExchangeRateVote{ + ExchangeRateTuples: rates, + Voter: val.String(), + } + return preVote, vote } + h := uint64(ctx.BlockHeight()) + val1PreVotes, val1Votes := createVote("hash1", valAddr1, val1Tuples, h) + val2PreVotes, val2Votes := createVote("hash2", valAddr2, val2Tuples, h) + val3PreVotes, val3Votes := createVote("hash3", valAddr3, val3Tuples, h) // total voting power per denom is 100% app.OracleKeeper.SetAggregateExchangeRatePrevote(ctx, valAddr1, val1PreVotes) app.OracleKeeper.SetAggregateExchangeRatePrevote(ctx, valAddr2, val2PreVotes) + app.OracleKeeper.SetAggregateExchangeRatePrevote(ctx, valAddr3, val3PreVotes) oracle.EndBlocker(ctx, app.OracleKeeper) ctx = ctx.WithBlockHeight(ctx.BlockHeight() + voteBlockDiff) app.OracleKeeper.SetAggregateExchangeRateVote(ctx, valAddr1, val1Votes) app.OracleKeeper.SetAggregateExchangeRateVote(ctx, valAddr2, val2Votes) + app.OracleKeeper.SetAggregateExchangeRateVote(ctx, valAddr3, val3Votes) oracle.EndBlocker(ctx, app.OracleKeeper) for _, denom := range app.OracleKeeper.AcceptList(ctx) { @@ -146,12 +152,13 @@ func (s *IntegrationTestSuite) TestEndBlockerVoteThreshold() { s.Require().Equal(sdk.MustNewDecFromStr("1.0"), rate) } - // update prevotes' block + // Test: only val2 votes. + // Total voting power per denom must be bigger than 30%. So if only val2 votes, + // we won't have a price for the denom update prevotes' block. ctx = ctx.WithBlockHeight(ctx.BlockHeight() + preVoteBlockDiff) - val1PreVotes.SubmitBlock = uint64(ctx.BlockHeight()) - val2PreVotes.SubmitBlock = uint64(ctx.BlockHeight()) + h = uint64(ctx.BlockHeight()) + val2PreVotes.SubmitBlock = h - // total voting power per denom is 30% app.OracleKeeper.SetAggregateExchangeRatePrevote(ctx, valAddr2, val2PreVotes) oracle.EndBlocker(ctx, app.OracleKeeper) @@ -165,12 +172,37 @@ func (s *IntegrationTestSuite) TestEndBlockerVoteThreshold() { s.Require().Equal(sdk.ZeroDec(), rate) } - // update prevotes' block + // Test: val2 and val3 votes. + // now we will have 30.1% of the power, so the we should have prices + ctx = ctx.WithBlockHeight(ctx.BlockHeight() + preVoteBlockDiff) + h = uint64(ctx.BlockHeight()) + val2PreVotes.SubmitBlock = h + val3PreVotes.SubmitBlock = h + + app.OracleKeeper.SetAggregateExchangeRatePrevote(ctx, valAddr2, val2PreVotes) + app.OracleKeeper.SetAggregateExchangeRatePrevote(ctx, valAddr3, val3PreVotes) + oracle.EndBlocker(ctx, app.OracleKeeper) + + ctx = ctx.WithBlockHeight(ctx.BlockHeight() + voteBlockDiff) + app.OracleKeeper.SetAggregateExchangeRateVote(ctx, valAddr2, val2Votes) + app.OracleKeeper.SetAggregateExchangeRateVote(ctx, valAddr3, val3Votes) + oracle.EndBlocker(ctx, app.OracleKeeper) + + for _, denom := range app.OracleKeeper.AcceptList(ctx) { + rate, err := app.OracleKeeper.GetExchangeRate(ctx, denom.SymbolDenom) + s.Require().NoError(err) + s.Require().Equal(sdk.MustNewDecFromStr("0.5"), rate) + } + + return + // Test: all validators vote again ctx = ctx.WithBlockHeight(ctx.BlockHeight() + preVoteBlockDiff) - val1PreVotes.SubmitBlock = uint64(ctx.BlockHeight()) - val2PreVotes.SubmitBlock = uint64(ctx.BlockHeight()) + h = uint64(ctx.BlockHeight()) + val1PreVotes.SubmitBlock = h + val2PreVotes.SubmitBlock = h + val3PreVotes.SubmitBlock = h - // umee has 100% power, and atom has 30% + // umee has 99.9% power, and atom has 30% val1Tuples = types.ExchangeRateTuples{ types.ExchangeRateTuple{ Denom: "umee", diff --git a/x/oracle/keeper/ballot.go b/x/oracle/keeper/ballot.go index ec707022ad..bb6320d8bc 100644 --- a/x/oracle/keeper/ballot.go +++ b/x/oracle/keeper/ballot.go @@ -21,13 +21,10 @@ func (k Keeper) OrganizeBallotByDenom( // organize ballot only for the active validators claim, ok := validatorClaimMap[vote.Voter] if ok { - power := claim.Power - for _, tuple := range vote.ExchangeRateTuples { - tmpPower := power votes[tuple.Denom] = append( votes[tuple.Denom], - types.NewVoteForTally(tuple.ExchangeRate, tuple.Denom, voterAddr, tmpPower), + types.NewVoteForTally(tuple.ExchangeRate, tuple.Denom, voterAddr, claim.Power), ) } } From 27a217c6442565356328bc9e27890b98295e50b8 Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Sat, 18 Feb 2023 13:59:49 +0100 Subject: [PATCH 02/13] finish test --- x/oracle/abci_test.go | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/x/oracle/abci_test.go b/x/oracle/abci_test.go index 5dd5145c37..1ef4308d49 100644 --- a/x/oracle/abci_test.go +++ b/x/oracle/abci_test.go @@ -92,7 +92,6 @@ var ( func (s *IntegrationTestSuite) TestEndBlockerVoteThreshold() { app, ctx := s.app, s.ctx - originalBlockHeight := ctx.BlockHeight() ctx = ctx.WithBlockHeight(1) preVoteBlockDiff := int64(app.OracleKeeper.VotePeriod(ctx) / 2) voteBlockDiff := int64(app.OracleKeeper.VotePeriod(ctx)/2 + 1) @@ -194,33 +193,27 @@ func (s *IntegrationTestSuite) TestEndBlockerVoteThreshold() { s.Require().Equal(sdk.MustNewDecFromStr("0.5"), rate) } - return - // Test: all validators vote again + // TODO: check reward distribution + + // Test: val1 and val2 vote again + // umee has 69.9% power, and atom has 30%, so we should have price for umee, but not for atom ctx = ctx.WithBlockHeight(ctx.BlockHeight() + preVoteBlockDiff) h = uint64(ctx.BlockHeight()) val1PreVotes.SubmitBlock = h val2PreVotes.SubmitBlock = h - val3PreVotes.SubmitBlock = h - // umee has 99.9% power, and atom has 30% - val1Tuples = types.ExchangeRateTuples{ + val1Votes.ExchangeRateTuples = types.ExchangeRateTuples{ types.ExchangeRateTuple{ Denom: "umee", ExchangeRate: sdk.MustNewDecFromStr("1.0"), }, } - val2Tuples = types.ExchangeRateTuples{ - types.ExchangeRateTuple{ - Denom: "umee", - ExchangeRate: sdk.MustNewDecFromStr("0.5"), - }, + val2Votes.ExchangeRateTuples = types.ExchangeRateTuples{ types.ExchangeRateTuple{ Denom: "atom", ExchangeRate: sdk.MustNewDecFromStr("0.5"), }, } - val1Votes.ExchangeRateTuples = val1Tuples - val2Votes.ExchangeRateTuples = val2Tuples app.OracleKeeper.SetAggregateExchangeRatePrevote(ctx, valAddr1, val1PreVotes) app.OracleKeeper.SetAggregateExchangeRatePrevote(ctx, valAddr2, val2PreVotes) @@ -237,8 +230,6 @@ func (s *IntegrationTestSuite) TestEndBlockerVoteThreshold() { rate, err = app.OracleKeeper.GetExchangeRate(ctx, "atom") s.Require().ErrorIs(err, sdkerrors.Wrap(types.ErrUnknownDenom, "atom")) s.Require().Equal(sdk.ZeroDec(), rate) - - ctx = ctx.WithBlockHeight(originalBlockHeight) } var exchangeRates = map[string][]sdk.Dec{ From b0ed3bd01f10d62124fdc1e9bc1d3fb87d79f325 Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Mon, 20 Feb 2023 16:53:34 +0100 Subject: [PATCH 03/13] oracle: add SetVoteThreshold method --- x/oracle/abci.go | 15 ++++++++------- x/oracle/abci_test.go | 8 ++++++-- x/oracle/keeper/params.go | 15 +++++++++++++-- x/oracle/types/params.go | 17 +++++++++++++++++ 4 files changed, 44 insertions(+), 11 deletions(-) diff --git a/x/oracle/abci.go b/x/oracle/abci.go index 6fe4f9d951..c6165c7207 100644 --- a/x/oracle/abci.go +++ b/x/oracle/abci.go @@ -1,6 +1,7 @@ package oracle import ( + "fmt" "strings" "time" @@ -38,16 +39,14 @@ func CalcPrices(ctx sdk.Context, params types.Params, k keeper.Keeper) error { validatorClaimMap := make(map[string]types.Claim) powerReduction := k.StakingKeeper.PowerReduction(ctx) // Calculate total validator power - var totalBondedValidatorPower int64 + var totalBondedPower int64 for _, v := range k.StakingKeeper.GetBondedValidatorsByPower(ctx) { - totalBondedValidatorPower += v.GetConsensusPower(powerReduction) + totalBondedPower += v.GetConsensusPower(powerReduction) } for _, v := range k.StakingKeeper.GetBondedValidatorsByPower(ctx) { addr := v.GetOperator() - validatorPowerRatio := sdk.NewDec(v.GetConsensusPower(powerReduction)).QuoInt64(totalBondedValidatorPower) - // Power is tracked as an int64 ranging from 0-100 - validatorPower := validatorPowerRatio.MulInt64(100).RoundInt64() - validatorClaimMap[addr.String()] = types.NewClaim(validatorPower, 0, 0, addr) + valPower := v.GetConsensusPower(powerReduction) + validatorClaimMap[addr.String()] = types.NewClaim(valPower, 0, 0, addr) } // voteTargets defines the symbol (ticker) denoms that we require votes on @@ -62,11 +61,13 @@ func CalcPrices(ctx sdk.Context, params types.Params, k keeper.Keeper) error { // NOTE: it filters out inactive or jailed validators ballotDenomSlice := k.OrganizeBallotByDenom(ctx, validatorClaimMap) + threshold := k.VoteThreshold(ctx).MulInt64(100).RoundInt().Int64() + fmt.Println(">>> threshold", threshold) // Iterate through ballots and update exchange rates; drop if not enough votes have been achieved. for _, ballotDenom := range ballotDenomSlice { // Convert ballot power to a percentage to compare with VoteThreshold param - if sdk.NewDecWithPrec(ballotDenom.Ballot.Power(), 2).LTE(k.VoteThreshold(ctx)) { + if ballotDenom.Ballot.Power()*100/totalBondedPower <= threshold { ctx.Logger().Info("Ballot voting power is under vote threshold, dropping ballot", "denom", ballotDenom) continue } diff --git a/x/oracle/abci_test.go b/x/oracle/abci_test.go index 1ef4308d49..9eebcdead2 100644 --- a/x/oracle/abci_test.go +++ b/x/oracle/abci_test.go @@ -57,12 +57,15 @@ func (s *IntegrationTestSuite) SetupTest() { require.NoError(app.BankKeeper.SendCoinsFromModuleToAccount(ctx, minttypes.ModuleName, addr2, initCoins)) require.NoError(app.BankKeeper.SendCoinsFromModuleToAccount(ctx, minttypes.ModuleName, addr3, initCoins)) - sh.CreateValidatorWithValPower(valAddr1, valPubKey1, 699, true) - sh.CreateValidatorWithValPower(valAddr2, valPubKey2, 300, true) + sh.CreateValidatorWithValPower(valAddr1, valPubKey1, 599, true) + sh.CreateValidatorWithValPower(valAddr2, valPubKey2, 400, true) sh.CreateValidatorWithValPower(valAddr3, valPubKey3, 1, true) staking.EndBlocker(ctx, *app.StakingKeeper) + err := app.OracleKeeper.SetVoteThreshold(ctx, sdk.MustNewDecFromStr("0.4")) + s.Require().NoError(err) + s.app = app s.ctx = ctx } @@ -194,6 +197,7 @@ func (s *IntegrationTestSuite) TestEndBlockerVoteThreshold() { } // TODO: check reward distribution + // https://github.com/umee-network/umee/issues/1853 // Test: val1 and val2 vote again // umee has 69.9% power, and atom has 30%, so we should have price for umee, but not for atom diff --git a/x/oracle/keeper/params.go b/x/oracle/keeper/params.go index bf6d0f47f1..74ac4d41be 100644 --- a/x/oracle/keeper/params.go +++ b/x/oracle/keeper/params.go @@ -19,13 +19,24 @@ func (k Keeper) VotePeriod(ctx sdk.Context) (res uint64) { return } -// VoteThreshold returns the minimum percentage of votes that must be received -// for a ballot to pass. +// VoteThreshold returns the minimum rate of combined validator power of votes +// that must be received for a ballot to pass. func (k Keeper) VoteThreshold(ctx sdk.Context) (res sdk.Dec) { k.paramSpace.Get(ctx, types.KeyVoteThreshold, &res) return } +// SetVoteThreshold sets min combined validator power voting on a denom to accept +// it as valid. +// TODO: this is used in tests, we should refactor the way how this is handled. +func (k Keeper) SetVoteThreshold(ctx sdk.Context, threshold sdk.Dec) error { + if err := types.ValidateVotingThreshold(threshold); err != nil { + return err + } + k.paramSpace.Set(ctx, types.KeyVoteThreshold, &threshold) + return nil +} + // RewardBand returns the ratio of allowable exchange rate error that a validator // can be rewarded. func (k Keeper) RewardBand(ctx sdk.Context) (res sdk.Dec) { diff --git a/x/oracle/types/params.go b/x/oracle/types/params.go index 59cc431633..abad5ab48c 100644 --- a/x/oracle/types/params.go +++ b/x/oracle/types/params.go @@ -4,10 +4,15 @@ import ( "fmt" sdk "github.com/cosmos/cosmos-sdk/types" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" paramstypes "github.com/cosmos/cosmos-sdk/x/params/types" "gopkg.in/yaml.v3" ) +var ( + oneDec = sdk.OneDec() +) + // Parameter keys var ( KeyVotePeriod = []byte("VotePeriod") @@ -378,3 +383,15 @@ func validateMaximumMedianStamps(i interface{}) error { return nil } + +func ValidateVotingThreshold(x sdk.Dec) error { + if !x.IsPositive() || x.GT(oneDec) { + return sdkerrors.ErrInvalidRequest.Wrap("threshold must be bigger than zero and <= 1") + } + i := x.MulInt64(100).RoundInt64() + x2 := sdk.NewDecWithPrec(i, 2) + if !x2.Equal(x) { + return sdkerrors.ErrInvalidRequest.Wrap("threshold precision must be maximum 2 decimals") + } + return nil +} From 286d6b29f534431b3263a4d302b1c9a13e8c46ce Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Mon, 20 Feb 2023 20:41:52 +0100 Subject: [PATCH 04/13] more VoteThreshold tests --- x/oracle/keeper/params.go | 2 +- x/oracle/types/params.go | 20 ++++++-------------- x/oracle/types/params_test.go | 34 ++++++++++++++++++++++++++++++++++ 3 files changed, 41 insertions(+), 15 deletions(-) diff --git a/x/oracle/keeper/params.go b/x/oracle/keeper/params.go index 74ac4d41be..b70ba1cff7 100644 --- a/x/oracle/keeper/params.go +++ b/x/oracle/keeper/params.go @@ -30,7 +30,7 @@ func (k Keeper) VoteThreshold(ctx sdk.Context) (res sdk.Dec) { // it as valid. // TODO: this is used in tests, we should refactor the way how this is handled. func (k Keeper) SetVoteThreshold(ctx sdk.Context, threshold sdk.Dec) error { - if err := types.ValidateVotingThreshold(threshold); err != nil { + if err := types.ValidateVoteThreshold(threshold); err != nil { return err } k.paramSpace.Set(ctx, types.KeyVoteThreshold, &threshold) diff --git a/x/oracle/types/params.go b/x/oracle/types/params.go index abad5ab48c..514c76059f 100644 --- a/x/oracle/types/params.go +++ b/x/oracle/types/params.go @@ -10,7 +10,8 @@ import ( ) var ( - oneDec = sdk.OneDec() + oneDec = sdk.OneDec() + minVoteThreshold = sdk.NewDecWithPrec(33, 2) // 0.33 ) // Parameter keys @@ -225,16 +226,7 @@ func validateVoteThreshold(i interface{}) error { if !ok { return fmt.Errorf("invalid parameter type: %T", i) } - - if v.LT(sdk.NewDecWithPrec(33, 2)) { - return fmt.Errorf("vote threshold must be bigger than 33%%: %s", v) - } - - if v.GT(sdk.OneDec()) { - return fmt.Errorf("vote threshold too large: %s", v) - } - - return nil + return ValidateVoteThreshold(v) } func validateRewardBand(i interface{}) error { @@ -384,9 +376,9 @@ func validateMaximumMedianStamps(i interface{}) error { return nil } -func ValidateVotingThreshold(x sdk.Dec) error { - if !x.IsPositive() || x.GT(oneDec) { - return sdkerrors.ErrInvalidRequest.Wrap("threshold must be bigger than zero and <= 1") +func ValidateVoteThreshold(x sdk.Dec) error { + if x.LTE(minVoteThreshold) || x.GT(oneDec) { + return sdkerrors.ErrInvalidRequest.Wrap("threshold must be bigger than 0.33 and <= 1") } i := x.MulInt64(100).RoundInt64() x2 := sdk.NewDecWithPrec(i, 2) diff --git a/x/oracle/types/params_test.go b/x/oracle/types/params_test.go index d6505fb78f..1c6e935bf7 100644 --- a/x/oracle/types/params_test.go +++ b/x/oracle/types/params_test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/stretchr/testify/require" + "gotest.tools/v3/assert" sdk "github.com/cosmos/cosmos-sdk/types" ) @@ -246,3 +247,36 @@ func TestParamsEqual(t *testing.T) { require.NotNil(t, p13.ParamSetPairs()) require.NotNil(t, p13.String()) } + +func TestValidateVotingThreshold(t *testing.T) { + tcs := []struct { + name string + t sdk.Dec + errMsg string + }{ + {"fail: negative", sdk.MustNewDecFromStr("-1"), "threshold must be"}, + {"fail: zero", sdk.ZeroDec(), "threshold must be"}, + {"fail: less than 0.33", sdk.MustNewDecFromStr("0.3"), "threshold must be"}, + {"fail: equal 0.33", sdk.MustNewDecFromStr("0.33"), "threshold must be"}, + {"fail: more than 1", sdk.MustNewDecFromStr("1.1"), "threshold must be"}, + {"fail: more than 1", sdk.MustNewDecFromStr("10"), "threshold must be"}, + {"fail: max precision 2", sdk.MustNewDecFromStr("0.333"), "maximum 2 decimals"}, + {"fail: max precision 2", sdk.MustNewDecFromStr("0.401"), "maximum 2 decimals"}, + {"fail: max precision 2", sdk.MustNewDecFromStr("0.409"), "maximum 2 decimals"}, + {"fail: max precision 2", sdk.MustNewDecFromStr("0.4009"), "maximum 2 decimals"}, + {"fail: max precision 2", sdk.MustNewDecFromStr("0.999"), "maximum 2 decimals"}, + + {"ok: 1", sdk.MustNewDecFromStr("1"), ""}, + {"ok: 0.34", sdk.MustNewDecFromStr("0.34"), ""}, + {"ok: 0.99", sdk.MustNewDecFromStr("0.99"), ""}, + } + + for _, tc := range tcs { + err := ValidateVoteThreshold(tc.t) + if tc.errMsg == "" { + assert.NilError(t, err, "test_case", tc.name) + } else { + assert.ErrorContains(t, err, tc.errMsg, tc.name) + } + } +} From 6c85d26a352ce26e7a012a08689ef74812d10fd5 Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Mon, 20 Feb 2023 20:44:04 +0100 Subject: [PATCH 05/13] lint --- x/oracle/types/params.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/x/oracle/types/params.go b/x/oracle/types/params.go index 514c76059f..b5b53c3c9b 100644 --- a/x/oracle/types/params.go +++ b/x/oracle/types/params.go @@ -376,6 +376,10 @@ func validateMaximumMedianStamps(i interface{}) error { return nil } +// ValidateVoteThreshold validates oracle exchange rates power vote threshold. +// Must be +// * a decimal value > 0.33 and <= 1. +// * max precision is 2 (so 0.501 is not allowed) func ValidateVoteThreshold(x sdk.Dec) error { if x.LTE(minVoteThreshold) || x.GT(oneDec) { return sdkerrors.ErrInvalidRequest.Wrap("threshold must be bigger than 0.33 and <= 1") From c49cc6b8d4d39b70d7df315bbf1d3c126d54159c Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Mon, 20 Feb 2023 22:02:34 +0100 Subject: [PATCH 06/13] cleanup --- x/oracle/abci.go | 10 +++++----- x/oracle/abci_test.go | 22 ++++++++++++++-------- x/oracle/types/params.go | 10 ++++++++-- 3 files changed, 27 insertions(+), 15 deletions(-) diff --git a/x/oracle/abci.go b/x/oracle/abci.go index c6165c7207..0ef8fc639c 100644 --- a/x/oracle/abci.go +++ b/x/oracle/abci.go @@ -1,7 +1,6 @@ package oracle import ( - "fmt" "strings" "time" @@ -61,13 +60,14 @@ func CalcPrices(ctx sdk.Context, params types.Params, k keeper.Keeper) error { // NOTE: it filters out inactive or jailed validators ballotDenomSlice := k.OrganizeBallotByDenom(ctx, validatorClaimMap) - threshold := k.VoteThreshold(ctx).MulInt64(100).RoundInt().Int64() - fmt.Println(">>> threshold", threshold) + threshold := k.VoteThreshold(ctx).MulInt64(types.MaxVoteThresholdMultiplier).TruncateInt64() // Iterate through ballots and update exchange rates; drop if not enough votes have been achieved. for _, ballotDenom := range ballotDenomSlice { - // Convert ballot power to a percentage to compare with VoteThreshold param - if ballotDenom.Ballot.Power()*100/totalBondedPower <= threshold { + // Calculate the rate as an integer value, scaled up using the same multiplayer as the + // `threshold` computed above + support := ballotDenom.Ballot.Power() * types.MaxVoteThresholdMultiplier / totalBondedPower + if support < threshold { ctx.Logger().Info("Ballot voting power is under vote threshold, dropping ballot", "denom", ballotDenom) continue } diff --git a/x/oracle/abci_test.go b/x/oracle/abci_test.go index 9eebcdead2..97502c5ee4 100644 --- a/x/oracle/abci_test.go +++ b/x/oracle/abci_test.go @@ -42,24 +42,30 @@ func (s *IntegrationTestSuite) SetupTest() { require := s.Require() isCheckTx := false app := umeeapp.Setup(s.T()) - ctx := app.BaseApp.NewContext(isCheckTx, tmproto.Header{ + ctx := app.NewContext(isCheckTx, tmproto.Header{ ChainID: fmt.Sprintf("test-chain-%s", tmrand.Str(4)), }) oracle.InitGenesis(ctx, app.OracleKeeper, *types.DefaultGenesisState()) + // validate setup... umeeapp.Setup creates one validator, with 1uumee self delegation + setupVals := app.StakingKeeper.GetBondedValidatorsByPower(ctx) + s.Require().Len(setupVals, 1) + s.Require().Equal(int64(1), setupVals[0].GetConsensusPower(app.StakingKeeper.PowerReduction(ctx))) + sh := teststaking.NewHelper(s.T(), ctx, *app.StakingKeeper) sh.Denom = bondDenom - // mint and send coins to validator + // mint and send coins to validators require.NoError(app.BankKeeper.MintCoins(ctx, minttypes.ModuleName, initCoins.MulInt(sdk.NewIntFromUint64(3)))) require.NoError(app.BankKeeper.SendCoinsFromModuleToAccount(ctx, minttypes.ModuleName, addr1, initCoins)) require.NoError(app.BankKeeper.SendCoinsFromModuleToAccount(ctx, minttypes.ModuleName, addr2, initCoins)) require.NoError(app.BankKeeper.SendCoinsFromModuleToAccount(ctx, minttypes.ModuleName, addr3, initCoins)) + // self delegate 999 in total ... 1 val with 1uumee is already created in umeeapp.Setup sh.CreateValidatorWithValPower(valAddr1, valPubKey1, 599, true) - sh.CreateValidatorWithValPower(valAddr2, valPubKey2, 400, true) - sh.CreateValidatorWithValPower(valAddr3, valPubKey3, 1, true) + sh.CreateValidatorWithValPower(valAddr2, valPubKey2, 398, true) + sh.CreateValidatorWithValPower(valAddr3, valPubKey3, 2, true) staking.EndBlocker(ctx, *app.StakingKeeper) @@ -154,9 +160,9 @@ func (s *IntegrationTestSuite) TestEndBlockerVoteThreshold() { s.Require().Equal(sdk.MustNewDecFromStr("1.0"), rate) } - // Test: only val2 votes. - // Total voting power per denom must be bigger than 30%. So if only val2 votes, - // we won't have a price for the denom update prevotes' block. + // Test: only val2 votes (has 39% vote power). + // Total voting power per denom must be bigger or equal than 40% (see SetupTest). + // So if only val2 votes, we won't have a price for the denom update prevotes' block. ctx = ctx.WithBlockHeight(ctx.BlockHeight() + preVoteBlockDiff) h = uint64(ctx.BlockHeight()) val2PreVotes.SubmitBlock = h @@ -175,7 +181,7 @@ func (s *IntegrationTestSuite) TestEndBlockerVoteThreshold() { } // Test: val2 and val3 votes. - // now we will have 30.1% of the power, so the we should have prices + // now we will have 40% of the power, so now we should have prices ctx = ctx.WithBlockHeight(ctx.BlockHeight() + preVoteBlockDiff) h = uint64(ctx.BlockHeight()) val2PreVotes.SubmitBlock = h diff --git a/x/oracle/types/params.go b/x/oracle/types/params.go index b5b53c3c9b..b92855442c 100644 --- a/x/oracle/types/params.go +++ b/x/oracle/types/params.go @@ -14,6 +14,12 @@ var ( minVoteThreshold = sdk.NewDecWithPrec(33, 2) // 0.33 ) +// maxium number of decimals allowed for VoteThreshold +const ( + MaxVoteThresholdPrecision = 2 + MaxVoteThresholdMultiplier = 100 // must be 10^MaxVoteThresholdPrecision +) + // Parameter keys var ( KeyVotePeriod = []byte("VotePeriod") @@ -384,8 +390,8 @@ func ValidateVoteThreshold(x sdk.Dec) error { if x.LTE(minVoteThreshold) || x.GT(oneDec) { return sdkerrors.ErrInvalidRequest.Wrap("threshold must be bigger than 0.33 and <= 1") } - i := x.MulInt64(100).RoundInt64() - x2 := sdk.NewDecWithPrec(i, 2) + i := x.MulInt64(100).TruncateInt64() + x2 := sdk.NewDecWithPrec(i, MaxVoteThresholdPrecision) if !x2.Equal(x) { return sdkerrors.ErrInvalidRequest.Wrap("threshold precision must be maximum 2 decimals") } From 28ceaf4e12aa59bf1bb13df23c8667320501ddb5 Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Mon, 20 Feb 2023 23:23:25 +0100 Subject: [PATCH 07/13] Update x/oracle/types/params.go Co-authored-by: Adam Moser <63419657+toteki@users.noreply.github.com> --- x/oracle/types/params.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/oracle/types/params.go b/x/oracle/types/params.go index b92855442c..93a4137f6d 100644 --- a/x/oracle/types/params.go +++ b/x/oracle/types/params.go @@ -388,7 +388,7 @@ func validateMaximumMedianStamps(i interface{}) error { // * max precision is 2 (so 0.501 is not allowed) func ValidateVoteThreshold(x sdk.Dec) error { if x.LTE(minVoteThreshold) || x.GT(oneDec) { - return sdkerrors.ErrInvalidRequest.Wrap("threshold must be bigger than 0.33 and <= 1") + return sdkerrors.ErrInvalidRequest.Wrapf("threshold must be bigger than %s and <= 1", minVoteThreshold) } i := x.MulInt64(100).TruncateInt64() x2 := sdk.NewDecWithPrec(i, MaxVoteThresholdPrecision) From 723b8c03e70ce2eae71faae895c80448488c1250 Mon Sep 17 00:00:00 2001 From: Adam Moser <63419657+toteki@users.noreply.github.com> Date: Mon, 20 Feb 2023 15:48:12 -0700 Subject: [PATCH 08/13] comment changes only (review) --- x/oracle/abci.go | 4 ++-- x/oracle/abci_test.go | 2 +- x/oracle/keeper/params.go | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/x/oracle/abci.go b/x/oracle/abci.go index 0ef8fc639c..f6bf74019f 100644 --- a/x/oracle/abci.go +++ b/x/oracle/abci.go @@ -64,8 +64,8 @@ func CalcPrices(ctx sdk.Context, params types.Params, k keeper.Keeper) error { // Iterate through ballots and update exchange rates; drop if not enough votes have been achieved. for _, ballotDenom := range ballotDenomSlice { - // Calculate the rate as an integer value, scaled up using the same multiplayer as the - // `threshold` computed above + // Calculate the portion of votes received as an integer, scaled up using the + // same multiplier as the `threshold` computed above support := ballotDenom.Ballot.Power() * types.MaxVoteThresholdMultiplier / totalBondedPower if support < threshold { ctx.Logger().Info("Ballot voting power is under vote threshold, dropping ballot", "denom", ballotDenom) diff --git a/x/oracle/abci_test.go b/x/oracle/abci_test.go index 97502c5ee4..062efef773 100644 --- a/x/oracle/abci_test.go +++ b/x/oracle/abci_test.go @@ -162,7 +162,7 @@ func (s *IntegrationTestSuite) TestEndBlockerVoteThreshold() { // Test: only val2 votes (has 39% vote power). // Total voting power per denom must be bigger or equal than 40% (see SetupTest). - // So if only val2 votes, we won't have a price for the denom update prevotes' block. + // So if only val2 votes, we won't have any prices next block. ctx = ctx.WithBlockHeight(ctx.BlockHeight() + preVoteBlockDiff) h = uint64(ctx.BlockHeight()) val2PreVotes.SubmitBlock = h diff --git a/x/oracle/keeper/params.go b/x/oracle/keeper/params.go index b70ba1cff7..a8261a31fc 100644 --- a/x/oracle/keeper/params.go +++ b/x/oracle/keeper/params.go @@ -19,7 +19,7 @@ func (k Keeper) VotePeriod(ctx sdk.Context) (res uint64) { return } -// VoteThreshold returns the minimum rate of combined validator power of votes +// VoteThreshold returns the minimum portion of combined validator power of votes // that must be received for a ballot to pass. func (k Keeper) VoteThreshold(ctx sdk.Context) (res sdk.Dec) { k.paramSpace.Get(ctx, types.KeyVoteThreshold, &res) From 7e1bfec974313c9a5ec2dbfabd0bdb75cea99820 Mon Sep 17 00:00:00 2001 From: toteki <63419657+toteki@users.noreply.github.com> Date: Mon, 20 Feb 2023 15:50:56 -0700 Subject: [PATCH 09/13] combine loops --- x/oracle/abci.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/x/oracle/abci.go b/x/oracle/abci.go index f6bf74019f..1fb20c79af 100644 --- a/x/oracle/abci.go +++ b/x/oracle/abci.go @@ -39,13 +39,11 @@ func CalcPrices(ctx sdk.Context, params types.Params, k keeper.Keeper) error { powerReduction := k.StakingKeeper.PowerReduction(ctx) // Calculate total validator power var totalBondedPower int64 - for _, v := range k.StakingKeeper.GetBondedValidatorsByPower(ctx) { - totalBondedPower += v.GetConsensusPower(powerReduction) - } for _, v := range k.StakingKeeper.GetBondedValidatorsByPower(ctx) { addr := v.GetOperator() - valPower := v.GetConsensusPower(powerReduction) - validatorClaimMap[addr.String()] = types.NewClaim(valPower, 0, 0, addr) + power := v.GetConsensusPower(powerReduction) + totalBondedPower += power + validatorClaimMap[addr.String()] = types.NewClaim(power, 0, 0, addr) } // voteTargets defines the symbol (ticker) denoms that we require votes on From 02aa72c7c79a88770f4b251eab66fa5eb676f45e Mon Sep 17 00:00:00 2001 From: toteki <63419657+toteki@users.noreply.github.com> Date: Mon, 20 Feb 2023 15:54:23 -0700 Subject: [PATCH 10/13] fix simulations --- x/oracle/simulations/genesis.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x/oracle/simulations/genesis.go b/x/oracle/simulations/genesis.go index ac53d07db7..ce84e9d957 100644 --- a/x/oracle/simulations/genesis.go +++ b/x/oracle/simulations/genesis.go @@ -30,9 +30,9 @@ func GenVotePeriod(r *rand.Rand) uint64 { return uint64(5 + r.Intn(100)) } -// GenVoteThreshold produces a randomized VoteThreshold in the range of [0.333, 0.666] +// GenVoteThreshold produces a randomized VoteThreshold in the range of [0.33, 0.66] func GenVoteThreshold(r *rand.Rand) sdk.Dec { - return sdk.NewDecWithPrec(333, 3).Add(sdk.NewDecWithPrec(int64(r.Intn(333)), 3)) + return sdk.NewDecWithPrec(33, 2).Add(sdk.NewDecWithPrec(int64(r.Intn(33)), 2)) } // GenRewardBand produces a randomized RewardBand in the range of [0.000, 0.100] From f46649d6474c495b3ca92823d3ab19c64732d6b3 Mon Sep 17 00:00:00 2001 From: toteki <63419657+toteki@users.noreply.github.com> Date: Mon, 20 Feb 2023 16:09:04 -0700 Subject: [PATCH 11/13] fix error test --- x/oracle/types/params_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x/oracle/types/params_test.go b/x/oracle/types/params_test.go index 1c6e935bf7..08621d9752 100644 --- a/x/oracle/types/params_test.go +++ b/x/oracle/types/params_test.go @@ -29,10 +29,10 @@ func TestValidateVoteThreshold(t *testing.T) { require.ErrorContains(t, err, "invalid parameter type: string") err = validateVoteThreshold(sdk.MustNewDecFromStr("0.31")) - require.ErrorContains(t, err, "vote threshold must be bigger than 33%: 0.310000000000000000") + require.ErrorContains(t, err, "threshold must be bigger than 0.330000000000000000 and <= 1") err = validateVoteThreshold(sdk.MustNewDecFromStr("40.0")) - require.ErrorContains(t, err, "vote threshold too large: 40.000000000000000000") + require.ErrorContains(t, err, "threshold must be bigger than 0.330000000000000000 and <= 1") err = validateVoteThreshold(sdk.MustNewDecFromStr("0.35")) require.Nil(t, err) From 7d41b95ac5f23e9ff945f056de021ca1443d7966 Mon Sep 17 00:00:00 2001 From: toteki <63419657+toteki@users.noreply.github.com> Date: Mon, 20 Feb 2023 16:24:04 -0700 Subject: [PATCH 12/13] fix sims? --- x/oracle/simulations/genesis.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x/oracle/simulations/genesis.go b/x/oracle/simulations/genesis.go index ce84e9d957..900dff9b3e 100644 --- a/x/oracle/simulations/genesis.go +++ b/x/oracle/simulations/genesis.go @@ -30,9 +30,9 @@ func GenVotePeriod(r *rand.Rand) uint64 { return uint64(5 + r.Intn(100)) } -// GenVoteThreshold produces a randomized VoteThreshold in the range of [0.33, 0.66] +// GenVoteThreshold produces a randomized VoteThreshold in the range of [0.34, 0.67] func GenVoteThreshold(r *rand.Rand) sdk.Dec { - return sdk.NewDecWithPrec(33, 2).Add(sdk.NewDecWithPrec(int64(r.Intn(33)), 2)) + return sdk.NewDecWithPrec(34, 2).Add(sdk.NewDecWithPrec(int64(r.Intn(33)), 2)) } // GenRewardBand produces a randomized RewardBand in the range of [0.000, 0.100] From befb8c223208a0e433b02c18e7c0d8d21d1a3acd Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Tue, 21 Feb 2023 00:38:37 +0100 Subject: [PATCH 13/13] review --- x/oracle/types/params_test.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/x/oracle/types/params_test.go b/x/oracle/types/params_test.go index 08621d9752..f91fc1130f 100644 --- a/x/oracle/types/params_test.go +++ b/x/oracle/types/params_test.go @@ -28,12 +28,6 @@ func TestValidateVoteThreshold(t *testing.T) { err := validateVoteThreshold("invalidSdkType") require.ErrorContains(t, err, "invalid parameter type: string") - err = validateVoteThreshold(sdk.MustNewDecFromStr("0.31")) - require.ErrorContains(t, err, "threshold must be bigger than 0.330000000000000000 and <= 1") - - err = validateVoteThreshold(sdk.MustNewDecFromStr("40.0")) - require.ErrorContains(t, err, "threshold must be bigger than 0.330000000000000000 and <= 1") - err = validateVoteThreshold(sdk.MustNewDecFromStr("0.35")) require.Nil(t, err) }