Skip to content

Commit

Permalink
fix(x/gov): α must be strictly between 0 and 1 (#70)
Browse files Browse the repository at this point in the history
Also add more tests cases
  • Loading branch information
tbruyelle authored and giunatale committed Jan 14, 2025
1 parent 56931a9 commit cadfb64
Show file tree
Hide file tree
Showing 5 changed files with 244 additions and 16 deletions.
20 changes: 20 additions & 0 deletions x/gov/genesis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ func TestInitGenesis(t *testing.T) {
Options: v1.NewNonSplitVoteOption(v1.OptionNo),
},
}
utcTime = time.Now().UTC()
depositEndTime = time.Now().Add(time.Hour * 8)
votingStartTime = time.Now()
votingEndTime = time.Now().Add(time.Hour * 24)
Expand Down Expand Up @@ -171,6 +172,25 @@ func TestInitGenesis(t *testing.T) {
t.Helper()
p := s.GovKeeper.GetParams(ctx)
assert.Equal(t, *params, p)
lmdCoins, lmdTime := s.GovKeeper.GetLastMinDeposit(ctx)
assert.EqualValues(t, p.MinDepositThrottler.FloorValue, lmdCoins)
assert.Equal(t, ctx.BlockTime(), lmdTime)
},
},
{
name: "ok: genesis with last min deposit",
genesis: v1.GenesisState{
Params: params,
LastMinDeposit: &v1.LastMinDeposit{
Value: sdk.NewCoins(sdk.NewInt64Coin("xxx", 1)),
Time: &utcTime,
},
},
assert: func(t *testing.T, ctx sdk.Context, s suite) {
t.Helper()
lmdCoins, lmdTime := s.GovKeeper.GetLastMinDeposit(ctx)
assert.EqualValues(t, sdk.NewCoins(sdk.NewInt64Coin("xxx", 1)), lmdCoins)
assert.Equal(t, utcTime, lmdTime)
},
},
{
Expand Down
17 changes: 17 additions & 0 deletions x/gov/keeper/min_deposit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,23 @@ import (
v1 "github.com/atomone-hub/atomone/x/gov/types/v1"
)

func TestActiveProposalNumber(t *testing.T) {
assert := assert.New(t)
k, _, _, ctx := setupGovKeeper(t)

assert.EqualValues(0, k.GetActiveProposalsNumber(ctx))

k.IncrementActiveProposalsNumber(ctx)
k.IncrementActiveProposalsNumber(ctx)
assert.EqualValues(2, k.GetActiveProposalsNumber(ctx))

k.DecrementActiveProposalsNumber(ctx)
assert.EqualValues(1, k.GetActiveProposalsNumber(ctx))

k.SetActiveProposalsNumber(ctx, 42)
assert.EqualValues(42, k.GetActiveProposalsNumber(ctx))
}

func TestGetMinDeposit(t *testing.T) {
var (
minDepositFloor = v1.DefaultMinDepositFloor
Expand Down
4 changes: 2 additions & 2 deletions x/gov/keeper/msg_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1120,7 +1120,7 @@ func (suite *KeeperTestSuite) TestMsgUpdateParams() {
}
},
expErr: true,
expErrMsg: "quorum timeout must not be nil: 0",
expErrMsg: "quorum timeout must not be nil",
},
{
name: "negative quorum timeout",
Expand Down Expand Up @@ -1169,7 +1169,7 @@ func (suite *KeeperTestSuite) TestMsgUpdateParams() {
}
},
expErr: true,
expErrMsg: "max voting period extension must not be nil: 0",
expErrMsg: "max voting period extension must not be nil",
},
{
name: "voting period extension below voting period - quorum timeout",
Expand Down
191 changes: 189 additions & 2 deletions x/gov/types/v1/genesis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package v1_test

import (
"testing"
"time"

"github.com/stretchr/testify/require"

Expand Down Expand Up @@ -40,7 +41,16 @@ func TestValidateGenesis(t *testing.T) {
expErrMsg: "starting proposal id must be greater than 0",
},
{
name: "invalid min deposit",
name: "min deposit throttler is nil",
genesisState: func() *v1.GenesisState {
params1 := params
params1.MinDepositThrottler = nil
return v1.NewGenesisState(v1.DefaultStartingProposalID, params1)
},
expErrMsg: "min deposit throttler must not be nil",
},
{
name: "invalid min deposit throttler floor",
genesisState: func() *v1.GenesisState {
params1 := params
minDepositThrottler1 := *params.MinDepositThrottler
Expand All @@ -52,7 +62,184 @@ func TestValidateGenesis(t *testing.T) {

return v1.NewGenesisState(v1.DefaultStartingProposalID, params1)
},
expErrMsg: "invalid minimum deposit",
expErrMsg: "invalid minimum deposit floor",
},
{
name: "min deposit throttler update period is nil",
genesisState: func() *v1.GenesisState {
params1 := params
mdt := *params.MinDepositThrottler
mdt.UpdatePeriod = nil
params1.MinDepositThrottler = &mdt
return v1.NewGenesisState(v1.DefaultStartingProposalID, params1)
},
expErrMsg: "minimum deposit update period must not be nil",
},
{
name: "min deposit throttler update period is 0",
genesisState: func() *v1.GenesisState {
params1 := params
mdt := *params.MinDepositThrottler
d := time.Duration(0)
mdt.UpdatePeriod = &d
params1.MinDepositThrottler = &mdt
return v1.NewGenesisState(v1.DefaultStartingProposalID, params1)
},
expErrMsg: "minimum deposit update period must be positive: 0s",
},
{
name: "min deposit throttler update period is greater than voting period",
genesisState: func() *v1.GenesisState {
params1 := params
mdt := *params.MinDepositThrottler
d := *params.VotingPeriod + 1
mdt.UpdatePeriod = &d
params1.MinDepositThrottler = &mdt
return v1.NewGenesisState(v1.DefaultStartingProposalID, params1)
},
expErrMsg: "minimum deposit update period must be less than or equal to the voting period: 504h0m0.000000001s",
},
{
name: "min deposit throttler sensitivity target distance is 0",
genesisState: func() *v1.GenesisState {
params1 := params
mdt := *params.MinDepositThrottler
mdt.SensitivityTargetDistance = 0
params1.MinDepositThrottler = &mdt
return v1.NewGenesisState(v1.DefaultStartingProposalID, params1)
},
expErrMsg: "minimum deposit sensitivity target distance must be positive: 0",
},
{
name: "invalid min deposit throttler increase ratio",
genesisState: func() *v1.GenesisState {
params1 := params
mdt := *params.MinDepositThrottler
mdt.IncreaseRatio = ""
params1.MinDepositThrottler = &mdt
return v1.NewGenesisState(v1.DefaultStartingProposalID, params1)
},
expErrMsg: "invalid minimum deposit increase ratio: decimal string cannot be empty",
},
{
name: "min deposit throttler increase ratio is 0",
genesisState: func() *v1.GenesisState {
params1 := params
mdt := *params.MinDepositThrottler
mdt.IncreaseRatio = "0"
params1.MinDepositThrottler = &mdt
return v1.NewGenesisState(v1.DefaultStartingProposalID, params1)
},
expErrMsg: "minimum deposit increase ratio must be positive: 0.000000000000000000",
},
{
name: "min deposit throttler increase ratio is 1",
genesisState: func() *v1.GenesisState {
params1 := params
mdt := *params.MinDepositThrottler
mdt.IncreaseRatio = "1"
params1.MinDepositThrottler = &mdt
return v1.NewGenesisState(v1.DefaultStartingProposalID, params1)
},
expErrMsg: "minimum deposit increase ratio too large: 1.000000000000000000",
},
{
name: "invalid min deposit throttler decrease ratio",
genesisState: func() *v1.GenesisState {
params1 := params
mdt := *params.MinDepositThrottler
mdt.DecreaseRatio = ""
params1.MinDepositThrottler = &mdt
return v1.NewGenesisState(v1.DefaultStartingProposalID, params1)
},
expErrMsg: "invalid minimum deposit decrease ratio: decimal string cannot be empty",
},
{
name: "min deposit throttler decrease ratio is 0",
genesisState: func() *v1.GenesisState {
params1 := params
mdt := *params.MinDepositThrottler
mdt.DecreaseRatio = "0"
params1.MinDepositThrottler = &mdt
return v1.NewGenesisState(v1.DefaultStartingProposalID, params1)
},
expErrMsg: "minimum deposit decrease ratio must be positive: 0.000000000000000000",
},
{
name: "min deposit throttler decrease ratio is 1",
genesisState: func() *v1.GenesisState {
params1 := params
mdt := *params.MinDepositThrottler
mdt.DecreaseRatio = "1"
params1.MinDepositThrottler = &mdt
return v1.NewGenesisState(v1.DefaultStartingProposalID, params1)
},
expErrMsg: "minimum deposit decrease ratio too large: 1.000000000000000000",
},
{
name: "min deposit is deprecated",
genesisState: func() *v1.GenesisState {
params1 := params
params1.MinDeposit = sdk.NewCoins(sdk.NewInt64Coin("xxx", 1))

return v1.NewGenesisState(v1.DefaultStartingProposalID, params1)
},
expErrMsg: "manually setting min deposit is deprecated in favor of a dynamic min deposit",
},
{
name: "quorum timeout is nil",
genesisState: func() *v1.GenesisState {
params1 := params
params1.QuorumCheckCount = 1
params1.QuorumTimeout = nil

return v1.NewGenesisState(v1.DefaultStartingProposalID, params1)
},
expErrMsg: "quorum timeout must not be nil",
},
{
name: "quorum timeout is negative",
genesisState: func() *v1.GenesisState {
params1 := params
params1.QuorumCheckCount = 1
d := time.Duration(-1)
params1.QuorumTimeout = &d

return v1.NewGenesisState(v1.DefaultStartingProposalID, params1)
},
expErrMsg: "quorum timeout must be 0 or greater: -1ns",
},
{
name: "quorum timeout is equal to voting period",
genesisState: func() *v1.GenesisState {
params1 := params
params1.QuorumCheckCount = 1
params1.QuorumTimeout = params.VotingPeriod

return v1.NewGenesisState(v1.DefaultStartingProposalID, params1)
},
expErrMsg: "quorum timeout 504h0m0s must be strictly less than the voting period 504h0m0s",
},
{
name: "max voting period extension is nil",
genesisState: func() *v1.GenesisState {
params1 := params
params1.QuorumCheckCount = 1
params1.MaxVotingPeriodExtension = nil
return v1.NewGenesisState(v1.DefaultStartingProposalID, params1)
},
expErrMsg: "max voting period extension must not be nil",
},
{
name: "max voting period extension less than voting period - quorum time out",
genesisState: func() *v1.GenesisState {
params1 := params
params1.QuorumCheckCount = 1
d := time.Duration(-1)
params1.MaxVotingPeriodExtension = &d
return v1.NewGenesisState(v1.DefaultStartingProposalID, params1)
},
expErrMsg: "max voting period extension -1ns must be greater than or equal to the difference between the voting period 504h0m0s and the quorum timeout 480h0m0s",
},
{
name: "invalid max deposit period",
Expand Down
28 changes: 16 additions & 12 deletions x/gov/types/v1/params.go
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ func (p Params) ValidateBasic() error {
if p.QuorumCheckCount > 0 {
// If quorum check is enabled, validate quorum check params
if p.QuorumTimeout == nil {
return fmt.Errorf("quorum timeout must not be nil: %d", p.QuorumTimeout)
return fmt.Errorf("quorum timeout must not be nil")
}
if p.QuorumTimeout.Seconds() < 0 {
return fmt.Errorf("quorum timeout must be 0 or greater: %s", p.QuorumTimeout)
Expand All @@ -290,52 +290,56 @@ func (p Params) ValidateBasic() error {
}

if p.MaxVotingPeriodExtension == nil {
return fmt.Errorf("max voting period extension must not be nil: %d", p.MaxVotingPeriodExtension)
return fmt.Errorf("max voting period extension must not be nil")
}
if p.MaxVotingPeriodExtension.Nanoseconds() < p.VotingPeriod.Nanoseconds()-p.QuorumTimeout.Nanoseconds() {
return fmt.Errorf("max voting period extension %s must be greater than or equal to the difference between the voting period %s and the quorum timeout %s", p.MaxVotingPeriodExtension, p.VotingPeriod, p.QuorumTimeout)
}
}

if p.MinDepositThrottler == nil {
return fmt.Errorf("min deposit throttler must not be nil")
}

if minDepositFloor := sdk.Coins(p.MinDepositThrottler.FloorValue); minDepositFloor.Empty() || !minDepositFloor.IsValid() {
return fmt.Errorf("invalid minimum deposit floor: %s", minDepositFloor)
}

if p.MinDepositThrottler.UpdatePeriod == nil {
return fmt.Errorf("minimum deposit update period must not be nil: %d", p.MinDepositThrottler.UpdatePeriod)
return fmt.Errorf("minimum deposit update period must not be nil")
}

if p.MinDepositThrottler.UpdatePeriod.Seconds() <= 0 {
return fmt.Errorf("minimum deposit update period must be positive: %d", p.MinDepositThrottler.UpdatePeriod)
return fmt.Errorf("minimum deposit update period must be positive: %s", p.MinDepositThrottler.UpdatePeriod)
}

if p.MinDepositThrottler.UpdatePeriod.Seconds() > p.VotingPeriod.Seconds() {
return fmt.Errorf("minimum deposit update period must be less than or equal to the voting period: %d", p.MinDepositThrottler.UpdatePeriod)
return fmt.Errorf("minimum deposit update period must be less than or equal to the voting period: %s", p.MinDepositThrottler.UpdatePeriod)
}

if p.MinDepositThrottler.SensitivityTargetDistance == 0 {
return fmt.Errorf("minimum deposit sensitivity target distance must be positive: %d", p.MinDepositThrottler.SensitivityTargetDistance)
}

minDepositIncreaseRation, err := sdk.NewDecFromStr(p.MinDepositThrottler.IncreaseRatio)
minDepositIncreaseRatio, err := sdk.NewDecFromStr(p.MinDepositThrottler.IncreaseRatio)
if err != nil {
return fmt.Errorf("invalid minimum deposit increase ratio: %w", err)
}
if minDepositIncreaseRation.IsNegative() {
return fmt.Errorf("minimum deposit increase ratio must be positive: %s", minDepositIncreaseRation)
if !minDepositIncreaseRatio.IsPositive() {
return fmt.Errorf("minimum deposit increase ratio must be positive: %s", minDepositIncreaseRatio)
}
if minDepositIncreaseRation.GT(math.LegacyOneDec()) {
return fmt.Errorf("minimum deposit increase ratio too large: %s", minDepositIncreaseRation)
if minDepositIncreaseRatio.GTE(math.LegacyOneDec()) {
return fmt.Errorf("minimum deposit increase ratio too large: %s", minDepositIncreaseRatio)
}

minDepositDecreaseRatio, err := sdk.NewDecFromStr(p.MinDepositThrottler.DecreaseRatio)
if err != nil {
return fmt.Errorf("invalid minimum deposit decrease ratio: %w", err)
}
if minDepositDecreaseRatio.IsNegative() {
if !minDepositDecreaseRatio.IsPositive() {
return fmt.Errorf("minimum deposit decrease ratio must be positive: %s", minDepositDecreaseRatio)
}
if minDepositDecreaseRatio.GT(math.LegacyOneDec()) {
if minDepositDecreaseRatio.GTE(math.LegacyOneDec()) {
return fmt.Errorf("minimum deposit decrease ratio too large: %s", minDepositDecreaseRatio)
}

Expand Down

0 comments on commit cadfb64

Please sign in to comment.