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.

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

Jump to bottom

refactor(x/gov)!: Use KVStoreService, context.Context, return errors, better iterators #15988

Merged
merged 20 commits into from
May 5, 2023
Merged

refactor(x/gov)!: Use KVStoreService, context.Context, return errors, better iterators #15988

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
52df72d
progress
facundomedica Apr 28, 2023
6750d4e
progress
facundomedica Apr 28, 2023
45aa5ce
fix
facundomedica Apr 28, 2023
577da2e
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into facu…
facundomedica Apr 28, 2023
7f6d83e
fix
facundomedica Apr 28, 2023
96c2223
fix
facundomedica Apr 28, 2023
de49e2b
fix
facundomedica Apr 28, 2023
d3a2436
fix
facundomedica Apr 28, 2023
e65962c
fix lint
facundomedica May 2, 2023
d989d49
fix lint
facundomedica May 2, 2023
a526477
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into facu…
facundomedica May 2, 2023
25f26bf
use errors ErrStopIterating and remove own
facundomedica May 2, 2023
bdcf475
tidy
facundomedica May 2, 2023
5c05a8d
fix lint, cl ++
facundomedica May 2, 2023
a6c70a0
upgrading++
facundomedica May 2, 2023
2589a80
go mod tidy tests
facundomedica May 2, 2023
b82ffe0
tidy
facundomedica May 2, 2023
cac721c
fix test
facundomedica May 2, 2023
8ba5d38
fix
facundomedica May 2, 2023
88fc59d
Update x/gov/types/errors.go
facundomedica May 3, 2023
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
2 changes: 1 addition & 1 deletion simapp/app.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ func NewSimApp(
govConfig.MaxMetadataLen = 10000
*/
govKeeper := govkeeper.NewKeeper(
appCodec, keys[govtypes.StoreKey], app.AccountKeeper, app.BankKeeper,
appCodec, runtime.NewKVStoreService(keys[govtypes.StoreKey]), app.AccountKeeper, app.BankKeeper,
app.StakingKeeper, app.DistrKeeper, app.MsgServiceRouter(), govConfig, authtypes.NewModuleAddress(govtypes.ModuleName).String(),
)

Expand Down
16 changes: 9 additions & 7 deletions tests/integration/gov/genesis_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,8 @@ func TestImportExportQueues(t *testing.T) {
assert.NilError(t, err)
proposalID2 := proposal2.Id

votingStarted, err := s1.GovKeeper.AddDeposit(ctx, proposalID2, addrs[0], s1.GovKeeper.GetParams(ctx).MinDeposit)
params, _ := s1.GovKeeper.GetParams(ctx)
votingStarted, err := s1.GovKeeper.AddDeposit(ctx, proposalID2, addrs[0], params.MinDeposit)
assert.NilError(t, err)
assert.Assert(t, votingStarted)

Expand All @@ -105,7 +106,7 @@ func TestImportExportQueues(t *testing.T) {
distributionGenState := s1.DistrKeeper.ExportGenesis(ctx)

// export the state and import it into a new app
govGenState := gov.ExportGenesis(ctx, s1.GovKeeper)
govGenState, _ := gov.ExportGenesis(ctx, s1.GovKeeper)
genesisState := s1.appBuilder.DefaultGenesis()

genesisState[authtypes.ModuleName] = s1.cdc.MustMarshalJSON(authGenState)
Expand Down Expand Up @@ -144,8 +145,9 @@ func TestImportExportQueues(t *testing.T) {

ctx2 := s2.app.BaseApp.NewContext(false, cmtproto.Header{})

params, err = s2.GovKeeper.GetParams(ctx2)
// Jump the time forward past the DepositPeriod and VotingPeriod
ctx2 = ctx2.WithBlockTime(ctx2.BlockHeader().Time.Add(*s2.GovKeeper.GetParams(ctx2).MaxDepositPeriod).Add(*s2.GovKeeper.GetParams(ctx2).VotingPeriod))
ctx2 = ctx2.WithBlockTime(ctx2.BlockHeader().Time.Add(*params.MaxDepositPeriod).Add(*params.VotingPeriod))

// Make sure that they are still in the DepositPeriod and VotingPeriod respectively
proposal1, ok = s2.GovKeeper.GetProposal(ctx2, proposalID1)
Expand All @@ -156,15 +158,15 @@ func TestImportExportQueues(t *testing.T) {
assert.Assert(t, proposal2.Status == v1.StatusVotingPeriod)

macc := s2.GovKeeper.GetGovernanceAccount(ctx2)
assert.DeepEqual(t, sdk.Coins(s2.GovKeeper.GetParams(ctx2).MinDeposit), s2.BankKeeper.GetAllBalances(ctx2, macc.GetAddress()))
assert.DeepEqual(t, sdk.Coins(params.MinDeposit), s2.BankKeeper.GetAllBalances(ctx2, macc.GetAddress()))

// Run the endblocker. Check to make sure that proposal1 is removed from state, and proposal2 is finished VotingPeriod.
gov.EndBlocker(ctx2, s2.GovKeeper)

proposal1, ok = s2.GovKeeper.GetProposal(ctx2, proposalID1)
assert.Assert(t, ok == false)
assert.assert(t, ok == false)

proposal2, ok = s2.GovKeeper.GetProposal(ctx2, proposalID2)
assert.Assert(t, ok)
proposal2, err = s2.GovKeeper.GetProposal(ctx2, proposalID2)
assert.Assert(t, err == nil)
assert.Assert(t, proposal2.Status == v1.StatusRejected)
}
30 changes: 15 additions & 15 deletions tests/integration/gov/keeper/tally_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func TestTallyNoOneVotes(t *testing.T) {

proposal, ok := app.GovKeeper.GetProposal(ctx, proposalID)
assert.Assert(t, ok)
passes, burnDeposits, tallyResults := app.GovKeeper.Tally(ctx, proposal)
passes, burnDeposits, tallyResults, _ := app.GovKeeper.Tally(ctx, proposal)

assert.Assert(t, passes == false)
assert.Assert(t, burnDeposits == false)
Expand Down Expand Up @@ -57,7 +57,7 @@ func TestTallyNoQuorum(t *testing.T) {

proposal, ok := app.GovKeeper.GetProposal(ctx, proposalID)
assert.Assert(t, ok)
passes, burnDeposits, _ := app.GovKeeper.Tally(ctx, proposal)
passes, burnDeposits, _, _ := app.GovKeeper.Tally(ctx, proposal)
assert.Assert(t, passes == false)
assert.Assert(t, burnDeposits == false)
}
Expand All @@ -83,7 +83,7 @@ func TestTallyOnlyValidatorsAllYes(t *testing.T) {

proposal, ok := app.GovKeeper.GetProposal(ctx, proposalID)
assert.Assert(t, ok)
passes, burnDeposits, tallyResults := app.GovKeeper.Tally(ctx, proposal)
passes, burnDeposits, tallyResults, _ := app.GovKeeper.Tally(ctx, proposal)

assert.Assert(t, passes)
assert.Assert(t, burnDeposits == false)
Expand All @@ -110,7 +110,7 @@ func TestTallyOnlyValidators51No(t *testing.T) {

proposal, ok := app.GovKeeper.GetProposal(ctx, proposalID)
assert.Assert(t, ok)
passes, burnDeposits, _ := app.GovKeeper.Tally(ctx, proposal)
passes, burnDeposits, _, _ := app.GovKeeper.Tally(ctx, proposal)

assert.Assert(t, passes == false)
assert.Assert(t, burnDeposits == false)
Expand All @@ -136,7 +136,7 @@ func TestTallyOnlyValidators51Yes(t *testing.T) {

proposal, ok := app.GovKeeper.GetProposal(ctx, proposalID)
assert.Assert(t, ok)
passes, burnDeposits, tallyResults := app.GovKeeper.Tally(ctx, proposal)
passes, burnDeposits, tallyResults, _ := app.GovKeeper.Tally(ctx, proposal)

assert.Assert(t, passes)
assert.Assert(t, burnDeposits == false)
Expand Down Expand Up @@ -164,7 +164,7 @@ func TestTallyOnlyValidatorsVetoed(t *testing.T) {

proposal, ok := app.GovKeeper.GetProposal(ctx, proposalID)
assert.Assert(t, ok)
passes, burnDeposits, tallyResults := app.GovKeeper.Tally(ctx, proposal)
passes, burnDeposits, tallyResults, _ := app.GovKeeper.Tally(ctx, proposal)

assert.Assert(t, passes == false)
assert.Assert(t, burnDeposits)
Expand Down Expand Up @@ -192,7 +192,7 @@ func TestTallyOnlyValidatorsAbstainPasses(t *testing.T) {

proposal, ok := app.GovKeeper.GetProposal(ctx, proposalID)
assert.Assert(t, ok)
passes, burnDeposits, tallyResults := app.GovKeeper.Tally(ctx, proposal)
passes, burnDeposits, tallyResults, _ := app.GovKeeper.Tally(ctx, proposal)

assert.Assert(t, passes)
assert.Assert(t, burnDeposits == false)
Expand Down Expand Up @@ -220,7 +220,7 @@ func TestTallyOnlyValidatorsAbstainFails(t *testing.T) {

proposal, ok := app.GovKeeper.GetProposal(ctx, proposalID)
assert.Assert(t, ok)
passes, burnDeposits, tallyResults := app.GovKeeper.Tally(ctx, proposal)
passes, burnDeposits, tallyResults, _ := app.GovKeeper.Tally(ctx, proposal)

assert.Assert(t, passes == false)
assert.Assert(t, burnDeposits == false)
Expand Down Expand Up @@ -248,7 +248,7 @@ func TestTallyOnlyValidatorsNonVoter(t *testing.T) {

proposal, ok := app.GovKeeper.GetProposal(ctx, proposalID)
assert.Assert(t, ok)
passes, burnDeposits, tallyResults := app.GovKeeper.Tally(ctx, proposal)
passes, burnDeposits, tallyResults, _ := app.GovKeeper.Tally(ctx, proposal)

assert.Assert(t, passes == false)
assert.Assert(t, burnDeposits == false)
Expand Down Expand Up @@ -286,7 +286,7 @@ func TestTallyDelgatorOverride(t *testing.T) {

proposal, ok := app.GovKeeper.GetProposal(ctx, proposalID)
assert.Assert(t, ok)
passes, burnDeposits, tallyResults := app.GovKeeper.Tally(ctx, proposal)
passes, burnDeposits, tallyResults, _ := app.GovKeeper.Tally(ctx, proposal)

assert.Assert(t, passes == false)
assert.Assert(t, burnDeposits == false)
Expand Down Expand Up @@ -323,7 +323,7 @@ func TestTallyDelgatorInherit(t *testing.T) {

proposal, ok := app.GovKeeper.GetProposal(ctx, proposalID)
assert.Assert(t, ok)
passes, burnDeposits, tallyResults := app.GovKeeper.Tally(ctx, proposal)
passes, burnDeposits, tallyResults, _ := app.GovKeeper.Tally(ctx, proposal)

assert.Assert(t, passes)
assert.Assert(t, burnDeposits == false)
Expand Down Expand Up @@ -365,7 +365,7 @@ func TestTallyDelgatorMultipleOverride(t *testing.T) {

proposal, ok := app.GovKeeper.GetProposal(ctx, proposalID)
assert.Assert(t, ok)
passes, burnDeposits, tallyResults := app.GovKeeper.Tally(ctx, proposal)
passes, burnDeposits, tallyResults, _ := app.GovKeeper.Tally(ctx, proposal)

assert.Assert(t, passes == false)
assert.Assert(t, burnDeposits == false)
Expand Down Expand Up @@ -408,7 +408,7 @@ func TestTallyDelgatorMultipleInherit(t *testing.T) {

proposal, ok := app.GovKeeper.GetProposal(ctx, proposalID)
assert.Assert(t, ok)
passes, burnDeposits, tallyResults := app.GovKeeper.Tally(ctx, proposal)
passes, burnDeposits, tallyResults, _ := app.GovKeeper.Tally(ctx, proposal)

assert.Assert(t, passes == false)
assert.Assert(t, burnDeposits == false)
Expand Down Expand Up @@ -453,7 +453,7 @@ func TestTallyJailedValidator(t *testing.T) {

proposal, ok := app.GovKeeper.GetProposal(ctx, proposalID)
assert.Assert(t, ok)
passes, burnDeposits, tallyResults := app.GovKeeper.Tally(ctx, proposal)
passes, burnDeposits, tallyResults, _ := app.GovKeeper.Tally(ctx, proposal)

assert.Assert(t, passes)
assert.Assert(t, burnDeposits == false)
Expand Down Expand Up @@ -488,7 +488,7 @@ func TestTallyValidatorMultipleDelegations(t *testing.T) {

proposal, ok := app.GovKeeper.GetProposal(ctx, proposalID)
assert.Assert(t, ok)
passes, burnDeposits, tallyResults := app.GovKeeper.Tally(ctx, proposal)
passes, burnDeposits, tallyResults, _ := app.GovKeeper.Tally(ctx, proposal)

assert.Assert(t, passes)
assert.Assert(t, burnDeposits == false)
Expand Down
64 changes: 48 additions & 16 deletions x/gov/abci.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,30 @@ import (
)

// EndBlocker called every block, process inflation, update validator set.
func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) {
func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) error {
defer telemetry.ModuleMeasureSince(types.ModuleName, time.Now(), telemetry.MetricKeyEndBlocker)

logger := ctx.Logger().With("module", "x/"+types.ModuleName)
// delete dead proposals from store and returns theirs deposits.
// A proposal is dead when it's inactive and didn't get enough deposit on time to get into voting phase.
keeper.IterateInactiveProposalsQueue(ctx, ctx.BlockHeader().Time, func(proposal v1.Proposal) bool {
keeper.DeleteProposal(ctx, proposal.Id)
err := keeper.IterateInactiveProposalsQueue(ctx, ctx.BlockHeader().Time, func(proposal v1.Proposal) error {
err := keeper.DeleteProposal(ctx, proposal.Id)
if err != nil {
return err
}

params := keeper.GetParams(ctx)
params, err := keeper.GetParams(ctx)
if err != nil {
return err
}
if !params.BurnProposalDepositPrevote {
keeper.RefundAndDeleteDeposits(ctx, proposal.Id) // refund deposit if proposal got removed without getting 100% of the proposal
err = keeper.RefundAndDeleteDeposits(ctx, proposal.Id) // refund deposit if proposal got removed without getting 100% of the proposal
} else {
keeper.DeleteAndBurnDeposits(ctx, proposal.Id) // burn the deposit if proposal got removed without getting 100% of the proposal
err = keeper.DeleteAndBurnDeposits(ctx, proposal.Id) // burn the deposit if proposal got removed without getting 100% of the proposal
}

if err != nil {
return err
}

// called when proposal become inactive
Expand All @@ -48,28 +58,41 @@ func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) {
"total_deposit", sdk.NewCoins(proposal.TotalDeposit...).String(),
)

return false
return nil
})
if err != nil {
return err
}

// fetch active proposals whose voting periods have ended (are passed the block time)
keeper.IterateActiveProposalsQueue(ctx, ctx.BlockHeader().Time, func(proposal v1.Proposal) bool {
return keeper.IterateActiveProposalsQueue(ctx, ctx.BlockHeader().Time, func(proposal v1.Proposal) error {
var tagValue, logMsg string

passes, burnDeposits, tallyResults := keeper.Tally(ctx, proposal)
passes, burnDeposits, tallyResults, err := keeper.Tally(ctx, proposal)
if err != nil {
return err
}

// If an expedited proposal fails, we do not want to update
// the deposit at this point since the proposal is converted to regular.
// As a result, the deposits are either deleted or refunded in all cases
// EXCEPT when an expedited proposal fails.
if !(proposal.Expedited && !passes) {
if burnDeposits {
keeper.DeleteAndBurnDeposits(ctx, proposal.Id)
err = keeper.DeleteAndBurnDeposits(ctx, proposal.Id)
} else {
keeper.RefundAndDeleteDeposits(ctx, proposal.Id)
err = keeper.RefundAndDeleteDeposits(ctx, proposal.Id)
}

if err != nil {
return err
}
}

keeper.RemoveFromActiveProposalQueue(ctx, proposal.Id, *proposal.VotingEndTime)
err = keeper.RemoveFromActiveProposalQueue(ctx, proposal.Id, *proposal.VotingEndTime)
if err != nil {
return err
}

switch {
case passes:
Expand Down Expand Up @@ -129,11 +152,17 @@ func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) {
// once the regular voting period expires again, the tally is repeated
// according to the regular proposal rules.
proposal.Expedited = false
params := keeper.GetParams(ctx)
params, err := keeper.GetParams(ctx)
if err != nil {
return err
}
endTime := proposal.VotingStartTime.Add(*params.VotingPeriod)
proposal.VotingEndTime = &endTime

keeper.InsertActiveProposalQueue(ctx, proposal.Id, *proposal.VotingEndTime)
err = keeper.InsertActiveProposalQueue(ctx, proposal.Id, *proposal.VotingEndTime)
if err != nil {
return err
}

tagValue = types.AttributeValueExpeditedProposalRejected
logMsg = "expedited proposal converted to regular"
Expand All @@ -145,7 +174,10 @@ func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) {

proposal.FinalTallyResult = &tallyResults

keeper.SetProposal(ctx, proposal)
err = keeper.SetProposal(ctx, proposal)
if err != nil {
return err
}

// when proposal become active
keeper.Hooks().AfterProposalVotingPeriodEnded(ctx, proposal.Id)
Expand All @@ -168,6 +200,6 @@ func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) {
),
)

return false
return nil
})
}
Loading