Skip to content

feat(x/gov): change lazy dynamic deposit computation in favor of endblocker per-tick update #104

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.

Sign up for GitHub

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

Closed
Parent: feat(x/gov): dynamic deposit throttler
wants to merge 1 commit into from
Closed

feat(x/gov): change lazy dynamic deposit computation in favor of endblocker per-tick update #104

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions x/gov/abci.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,9 @@ func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) {
)
return false
})

keeper.UpdateMinInitialDeposit(ctx, true)
keeper.UpdateMinDeposit(ctx, true)
}

// executes handle(msg) and recovers from panic.
Expand Down
53 changes: 32 additions & 21 deletions x/gov/keeper/min_deposit.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,7 @@ func (keeper Keeper) IncrementActiveProposalsNumber(ctx sdk.Context) {
activeProposalsNumber := keeper.GetActiveProposalsNumber(ctx) + 1
keeper.SetActiveProposalsNumber(ctx, activeProposalsNumber)

currMinDeposit := keeper.GetMinDeposit(ctx)
keeper.SetLastMinDeposit(ctx, currMinDeposit, ctx.BlockTime())
keeper.UpdateMinDeposit(ctx, false)
}

// DecrementActiveProposalsNumber decrements the number of active proposals by one
Expand All @@ -48,8 +47,7 @@ func (keeper Keeper) DecrementActiveProposalsNumber(ctx sdk.Context) {
panic("number of active proposals should never be negative")
}

currMinDeposit := keeper.GetMinDeposit(ctx)
keeper.SetLastMinDeposit(ctx, currMinDeposit, ctx.BlockTime())
keeper.UpdateMinDeposit(ctx, false)
}

// SetLastMinDeposit updates the last min deposit and last min deposit time.
Expand Down Expand Up @@ -78,18 +76,33 @@ func (keeper Keeper) GetLastMinDeposit(ctx sdk.Context) (sdk.Coins, time.Time) {

// GetMinDeposit returns the (dynamic) minimum deposit currently required for a proposal
func (keeper Keeper) GetMinDeposit(ctx sdk.Context) sdk.Coins {
minDeposit, _ := keeper.GetLastMinDeposit(ctx)

if minDeposit.Empty() {
// ValidateBasic prevents an empty FloorValue
// (and thus an empty deposit), if LastMinDeposit is empty
// it means it was never set, so we return the floor value
return keeper.GetParams(ctx).MinDepositThrottler.GetFloorValue()
}

return minDeposit
}

// UpdateMinDeposit updates the minimum deposit required for a proposal
func (keeper Keeper) UpdateMinDeposit(ctx sdk.Context, checkElapsedTime bool) {
params := keeper.GetParams(ctx)
minDepositFloor := sdk.Coins(params.MinDepositThrottler.FloorValue)
tick := params.MinDepositThrottler.UpdatePeriod
lastMinDeposit, lastMinDepositTime := keeper.GetLastMinDeposit(ctx)
if checkElapsedTime && ctx.BlockTime().Sub(lastMinDepositTime).Nanoseconds() < tick.Nanoseconds() {
return
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no additional case in the unit tests for this new condition, is it because it is already covered or is it a miss ?


minDepositFloor := sdk.Coins(params.MinDepositThrottler.FloorValue)
targetActiveProposals := math.NewIntFromUint64(params.MinDepositThrottler.TargetActiveProposals)
k := params.MinDepositThrottler.SensitivityTargetDistance
var a sdk.Dec

numActiveProposals := math.NewIntFromUint64(keeper.GetActiveProposalsNumber(ctx))
lastMinDeposit, lastMinDepositTime := keeper.GetLastMinDeposit(ctx)
// get number of ticks passed since last update
ticksPassed := ctx.BlockTime().Sub(lastMinDepositTime).Nanoseconds() / tick.Nanoseconds()

if numActiveProposals.GT(targetActiveProposals) {
a = sdk.MustNewDecFromStr(params.MinDepositThrottler.IncreaseRatio)
} else {
Expand All @@ -107,26 +120,24 @@ func (keeper Keeper) GetMinDeposit(ctx sdk.Context) sdk.Coins {
c := a.Mul(b)
percChange = percChange.Add(c)
}
if ticksPassed != 0 {
percChange = percChange.Power(uint64(ticksPassed))
}

minDeposit := sdk.Coins{}
newMinDeposit := sdk.Coins{}
minDepositFloorDenomsSeen := make(map[string]bool)
for _, lastMinDepositCoin := range lastMinDeposit {
minDepositFloorCoinAmt := minDepositFloor.AmountOf(lastMinDepositCoin.Denom)
if minDepositFloorCoinAmt.IsZero() {
// minDepositFloor was changed and this coin was removed,
// reflect this also in the current min deposit, i.e. remove
// this coin
// minDepositFloor was changed since last update,
// and this coin was removed.
// reflect this also in the current min initial deposit,
// i.e. remove this coin
continue
}
minDepositFloorDenomsSeen[lastMinDepositCoin.Denom] = true
minDepositCoinAmt := lastMinDepositCoin.Amount.ToLegacyDec().Mul(percChange).TruncateInt()
if minDepositCoinAmt.LT(minDepositFloorCoinAmt) {
minDeposit = append(minDeposit, sdk.NewCoin(lastMinDepositCoin.Denom, minDepositFloorCoinAmt))
newMinDeposit = append(newMinDeposit, sdk.NewCoin(lastMinDepositCoin.Denom, minDepositFloorCoinAmt))
} else {
minDeposit = append(minDeposit, sdk.NewCoin(lastMinDepositCoin.Denom, minDepositCoinAmt))
newMinDeposit = append(newMinDeposit, sdk.NewCoin(lastMinDepositCoin.Denom, minDepositCoinAmt))
}
}

Expand All @@ -135,12 +146,12 @@ func (keeper Keeper) GetMinDeposit(ctx sdk.Context) sdk.Coins {
if _, seen := minDepositFloorDenomsSeen[minDepositFloorCoin.Denom]; !seen {
minDepositCoinAmt := minDepositFloorCoin.Amount.ToLegacyDec().Mul(percChange).TruncateInt()
if minDepositCoinAmt.LT(minDepositFloorCoin.Amount) {
minDeposit = append(minDeposit, minDepositFloorCoin)
newMinDeposit = append(newMinDeposit, minDepositFloorCoin)
} else {
minDeposit = append(minDeposit, sdk.NewCoin(minDepositFloorCoin.Denom, minDepositCoinAmt))
newMinDeposit = append(newMinDeposit, sdk.NewCoin(minDepositFloorCoin.Denom, minDepositCoinAmt))
}
}
}

return minDeposit
keeper.SetLastMinDeposit(ctx, newMinDeposit, ctx.BlockTime())
}
16 changes: 16 additions & 0 deletions x/gov/keeper/min_deposit_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ func TestGetMinDeposit(t *testing.T) {
setup: func(ctx sdk.Context, k *keeper.Keeper) {
k.SetActiveProposalsNumber(ctx, 0)
k.SetLastMinDeposit(ctx, minDepositFloor, minDepositTimeFromTicks(0))
k.UpdateMinDeposit(ctx, false)
},
expectedMinDeposit: minDepositFloor.String(),
},
Expand All @@ -64,6 +65,7 @@ func TestGetMinDeposit(t *testing.T) {
setup: func(ctx sdk.Context, k *keeper.Keeper) {
k.SetActiveProposalsNumber(ctx, N)
k.SetLastMinDeposit(ctx, minDepositFloor, minDepositTimeFromTicks(0))
k.UpdateMinDeposit(ctx, false)
},
expectedMinDeposit: minDepositFloor.String(),
},
Expand All @@ -72,6 +74,7 @@ func TestGetMinDeposit(t *testing.T) {
setup: func(ctx sdk.Context, k *keeper.Keeper) {
k.SetActiveProposalsNumber(ctx, N+1)
k.SetLastMinDeposit(ctx, minDepositFloor, minDepositTimeFromTicks(0))
k.UpdateMinDeposit(ctx, false)
},
expectedMinDeposit: "10500000stake",
},
Expand All @@ -84,6 +87,7 @@ func TestGetMinDeposit(t *testing.T) {
),
minDepositTimeFromTicks(0),
)
k.UpdateMinDeposit(ctx, false)
},
expectedMinDeposit: "10500000stake",
},
Expand All @@ -92,6 +96,7 @@ func TestGetMinDeposit(t *testing.T) {
setup: func(ctx sdk.Context, k *keeper.Keeper) {
k.SetActiveProposalsNumber(ctx, N-1)
k.SetLastMinDeposit(ctx, minDepositFloorX2, minDepositTimeFromTicks(0))
k.UpdateMinDeposit(ctx, false)
},
expectedMinDeposit: "19500000stake",
},
Expand All @@ -100,6 +105,7 @@ func TestGetMinDeposit(t *testing.T) {
setup: func(ctx sdk.Context, k *keeper.Keeper) {
k.SetActiveProposalsNumber(ctx, N)
k.SetLastMinDeposit(ctx, minDepositFloorX2, minDepositTimeFromTicks(0))
k.UpdateMinDeposit(ctx, false)
},
expectedMinDeposit: minDepositFloorX2.String(),
},
Expand All @@ -108,6 +114,7 @@ func TestGetMinDeposit(t *testing.T) {
setup: func(ctx sdk.Context, k *keeper.Keeper) {
k.SetActiveProposalsNumber(ctx, N+1)
k.SetLastMinDeposit(ctx, minDepositFloorX2, minDepositTimeFromTicks(0))
k.UpdateMinDeposit(ctx, false)
},
expectedMinDeposit: "21000000stake",
},
Expand All @@ -116,6 +123,7 @@ func TestGetMinDeposit(t *testing.T) {
setup: func(ctx sdk.Context, k *keeper.Keeper) {
k.SetActiveProposalsNumber(ctx, N-1)
k.SetLastMinDeposit(ctx, minDepositFloorX2, minDepositTimeFromTicks(1))
k.UpdateMinDeposit(ctx, true)
},
expectedMinDeposit: "19500000stake",
},
Expand All @@ -124,6 +132,7 @@ func TestGetMinDeposit(t *testing.T) {
setup: func(ctx sdk.Context, k *keeper.Keeper) {
k.SetActiveProposalsNumber(ctx, N)
k.SetLastMinDeposit(ctx, minDepositFloorX2, minDepositTimeFromTicks(1))
k.UpdateMinDeposit(ctx, true)
},
expectedMinDeposit: minDepositFloorX2.String(),
},
Expand All @@ -132,6 +141,7 @@ func TestGetMinDeposit(t *testing.T) {
setup: func(ctx sdk.Context, k *keeper.Keeper) {
k.SetActiveProposalsNumber(ctx, N+1)
k.SetLastMinDeposit(ctx, minDepositFloorX2, minDepositTimeFromTicks(1))
k.UpdateMinDeposit(ctx, true)
},
expectedMinDeposit: "21000000stake",
},
Expand All @@ -140,6 +150,8 @@ func TestGetMinDeposit(t *testing.T) {
setup: func(ctx sdk.Context, k *keeper.Keeper) {
k.SetActiveProposalsNumber(ctx, N-1)
k.SetLastMinDeposit(ctx, minDepositFloorX2, minDepositTimeFromTicks(2))
k.UpdateMinDeposit(ctx.WithBlockTime(minDepositTimeFromTicks(1)), true)
k.UpdateMinDeposit(ctx, true)
},
expectedMinDeposit: "19012500stake",
},
Expand All @@ -148,6 +160,8 @@ func TestGetMinDeposit(t *testing.T) {
setup: func(ctx sdk.Context, k *keeper.Keeper) {
k.SetActiveProposalsNumber(ctx, N)
k.SetLastMinDeposit(ctx, minDepositFloorX2, minDepositTimeFromTicks(2))
k.UpdateMinDeposit(ctx.WithBlockTime(minDepositTimeFromTicks(1)), true)
k.UpdateMinDeposit(ctx, true)
},
expectedMinDeposit: minDepositFloorX2.String(),
},
Expand All @@ -156,6 +170,8 @@ func TestGetMinDeposit(t *testing.T) {
setup: func(ctx sdk.Context, k *keeper.Keeper) {
k.SetActiveProposalsNumber(ctx, N+1)
k.SetLastMinDeposit(ctx, minDepositFloorX2, minDepositTimeFromTicks(2))
k.UpdateMinDeposit(ctx.WithBlockTime(minDepositTimeFromTicks(1)), true)
k.UpdateMinDeposit(ctx, true)
},
expectedMinDeposit: "22050000stake",
},
Expand Down
49 changes: 30 additions & 19 deletions x/gov/keeper/min_initial_deposit.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,7 @@ func (keeper Keeper) IncrementInactiveProposalsNumber(ctx sdk.Context) {
inactiveProposalsNumber := keeper.GetInactiveProposalsNumber(ctx) + 1
keeper.SetInactiveProposalsNumber(ctx, inactiveProposalsNumber)

currMinInitialDeposit := keeper.GetMinInitialDeposit(ctx)
keeper.SetLastMinInitialDeposit(ctx, currMinInitialDeposit, ctx.BlockTime())
keeper.UpdateMinInitialDeposit(ctx, false)
}

// DecrementInactiveProposalsNumber decrements the number of inactive proposals by one
Expand All @@ -48,8 +47,7 @@ func (keeper Keeper) DecrementInactiveProposalsNumber(ctx sdk.Context) {
panic("number of inactive proposals should never be negative")
}

currMinInitialDeposit := keeper.GetMinInitialDeposit(ctx)
keeper.SetLastMinInitialDeposit(ctx, currMinInitialDeposit, ctx.BlockTime())
keeper.UpdateMinInitialDeposit(ctx, false)
}

// SetLastMinInitialDeposit updates the last min initial deposit and last min
Expand Down Expand Up @@ -80,18 +78,33 @@ func (keeper Keeper) GetLastMinInitialDeposit(ctx sdk.Context) (sdk.Coins, time.
// GetMinInitialDeposit returns the (dynamic) minimum initial deposit currently required for
// proposal submission
func (keeper Keeper) GetMinInitialDeposit(ctx sdk.Context) sdk.Coins {
minInitialDeposit, _ := keeper.GetLastMinInitialDeposit(ctx)

if minInitialDeposit.Empty() {
// ValidateBasic prevents an empty FloorValue
// (and thus an empty deposit), if LastMinInitialDeposit is empty
// it means it was never set, so we return the floor value
return keeper.GetParams(ctx).MinInitialDepositThrottler.GetFloorValue()
}

return minInitialDeposit
}

// UpdateMinInitialDeposit updates the min initial deposit required for proposal submission
func (keeper Keeper) UpdateMinInitialDeposit(ctx sdk.Context, checkElapsedTime bool) {
params := keeper.GetParams(ctx)
minInitialDepositFloor := sdk.Coins(params.MinInitialDepositThrottler.FloorValue)
tick := params.MinInitialDepositThrottler.UpdatePeriod
lastMinInitialDeposit, lastMinInitialDepositTime := keeper.GetLastMinInitialDeposit(ctx)
if checkElapsedTime && ctx.BlockTime().Sub(lastMinInitialDepositTime).Nanoseconds() < tick.Nanoseconds() {
return
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same question here


minInitialDepositFloor := sdk.Coins(params.MinInitialDepositThrottler.FloorValue)
targetInactiveProposals := math.NewIntFromUint64(params.MinInitialDepositThrottler.TargetProposals)
k := params.MinInitialDepositThrottler.SensitivityTargetDistance
var a sdk.Dec

numInactiveProposals := math.NewIntFromUint64(keeper.GetInactiveProposalsNumber(ctx))
lastMinInitialDeposit, lastMinInitialDepositTime := keeper.GetLastMinInitialDeposit(ctx)
// get number of ticks passed since last update
ticksPassed := ctx.BlockTime().Sub(lastMinInitialDepositTime).Nanoseconds() / tick.Nanoseconds()

if numInactiveProposals.GT(targetInactiveProposals) {
a = sdk.MustNewDecFromStr(params.MinInitialDepositThrottler.IncreaseRatio)
} else {
Expand All @@ -109,26 +122,24 @@ func (keeper Keeper) GetMinInitialDeposit(ctx sdk.Context) sdk.Coins {
c := a.Mul(b)
percChange = percChange.Add(c)
}
if ticksPassed != 0 {
percChange = percChange.Power(uint64(ticksPassed))
}

minInitialDeposit := sdk.Coins{}
newMinInitialDeposit := sdk.Coins{}
minInitialDepositFloorDenomsSeen := make(map[string]bool)
for _, lastMinInitialDepositCoin := range lastMinInitialDeposit {
minInitialDepositFloorCoinAmt := minInitialDepositFloor.AmountOf(lastMinInitialDepositCoin.Denom)
if minInitialDepositFloorCoinAmt.IsZero() {
// minInitialDepositFloor was changed and this coin was removed,
// minInitialDepositFloor was changed since last update,
// and this coin was removed.
// reflect this also in the current min initial deposit,
// i.e. remove this coin
continue
}
minInitialDepositFloorDenomsSeen[lastMinInitialDepositCoin.Denom] = true
minInitialDepositCoinAmt := lastMinInitialDepositCoin.Amount.ToLegacyDec().Mul(percChange).TruncateInt()
if minInitialDepositCoinAmt.LT(minInitialDepositFloorCoinAmt) {
minInitialDeposit = append(minInitialDeposit, sdk.NewCoin(lastMinInitialDepositCoin.Denom, minInitialDepositFloorCoinAmt))
newMinInitialDeposit = append(newMinInitialDeposit, sdk.NewCoin(lastMinInitialDepositCoin.Denom, minInitialDepositFloorCoinAmt))
} else {
minInitialDeposit = append(minInitialDeposit, sdk.NewCoin(lastMinInitialDepositCoin.Denom, minInitialDepositCoinAmt))
newMinInitialDeposit = append(newMinInitialDeposit, sdk.NewCoin(lastMinInitialDepositCoin.Denom, minInitialDepositCoinAmt))
}
}

Expand All @@ -137,12 +148,12 @@ func (keeper Keeper) GetMinInitialDeposit(ctx sdk.Context) sdk.Coins {
if _, seen := minInitialDepositFloorDenomsSeen[minInitialDepositFloorCoin.Denom]; !seen {
minInitialDepositCoinAmt := minInitialDepositFloorCoin.Amount.ToLegacyDec().Mul(percChange).TruncateInt()
if minInitialDepositCoinAmt.LT(minInitialDepositFloorCoin.Amount) {
minInitialDeposit = append(minInitialDeposit, minInitialDepositFloorCoin)
newMinInitialDeposit = append(newMinInitialDeposit, minInitialDepositFloorCoin)
} else {
minInitialDeposit = append(minInitialDeposit, sdk.NewCoin(minInitialDepositFloorCoin.Denom, minInitialDepositCoinAmt))
newMinInitialDeposit = append(newMinInitialDeposit, sdk.NewCoin(minInitialDepositFloorCoin.Denom, minInitialDepositCoinAmt))
}
}
}

return minInitialDeposit
keeper.SetLastMinInitialDeposit(ctx, newMinInitialDeposit, ctx.BlockTime())
}
Loading
Loading