Merge PR #4460: Gov Keys and Iterators

* refactor gov keys

* iterators and renamings

* invert queue key order

* changelog

* fix tests

* update alias.go

* Apply suggestions from code review

Co-Authored-By: Alexander Bezobchuk <alexanderbez@users.noreply.github.com>

* rename keys

* rename functions

* Apply suggestions from code review

Co-Authored-By: Alexander Bezobchuk <alexanderbez@users.noreply.github.com>

* address Aleks' comments

* fix test

* address Karoly's comments

Author: fedekunze

.pending/breaking/sdk/4437-bytes-gov-keys

@@ -0,0 +1 @@
+#4437 Replace governance module store keys to use `[]byte` instead of `string`.

.pending/improvements/sdk/4439-add-governance-

@@ -0,0 +1 @@
+#4439 Implement governance module iterators.

x/gov/alias.go

@@ -48,52 +48,61 @@ const (
 
 var (
 	// functions aliases
-	RegisterCodec                    = types.RegisterCodec
-	RegisterProposalTypeCodec        = types.RegisterProposalTypeCodec
-	ValidateAbstract                 = types.ValidateAbstract
-	ErrUnknownProposal               = types.ErrUnknownProposal
-	ErrInactiveProposal              = types.ErrInactiveProposal
-	ErrAlreadyActiveProposal         = types.ErrAlreadyActiveProposal
-	ErrAlreadyFinishedProposal       = types.ErrAlreadyFinishedProposal
-	ErrAddressNotStaked              = types.ErrAddressNotStaked
-	ErrInvalidProposalContent        = types.ErrInvalidProposalContent
-	ErrInvalidProposalType           = types.ErrInvalidProposalType
-	ErrInvalidVote                   = types.ErrInvalidVote
-	ErrInvalidGenesis                = types.ErrInvalidGenesis
-	ErrNoProposalHandlerExists       = types.ErrNoProposalHandlerExists
-	KeyProposal                      = types.KeyProposal
-	KeyDeposit                       = types.KeyDeposit
-	KeyVote                          = types.KeyVote
-	KeyDepositsSubspace              = types.KeyDepositsSubspace
-	KeyVotesSubspace                 = types.KeyVotesSubspace
-	PrefixActiveProposalQueueTime    = types.PrefixActiveProposalQueueTime
-	KeyActiveProposalQueueProposal   = types.KeyActiveProposalQueueProposal
-	PrefixInactiveProposalQueueTime  = types.PrefixInactiveProposalQueueTime
-	KeyInactiveProposalQueueProposal = types.KeyInactiveProposalQueueProposal
-	NewMsgSubmitProposal             = types.NewMsgSubmitProposal
-	NewMsgDeposit                    = types.NewMsgDeposit
-	NewMsgVote                       = types.NewMsgVote
-	NewProposal                      = types.NewProposal
-	ProposalStatusFromString         = types.ProposalStatusFromString
-	ValidProposalStatus              = types.ValidProposalStatus
-	NewTallyResult                   = types.NewTallyResult
-	NewTallyResultFromMap            = types.NewTallyResultFromMap
-	EmptyTallyResult                 = types.EmptyTallyResult
-	NewTextProposal                  = types.NewTextProposal
-	NewSoftwareUpgradeProposal       = types.NewSoftwareUpgradeProposal
-	RegisterProposalType             = types.RegisterProposalType
-	ContentFromProposalType          = types.ContentFromProposalType
-	IsValidProposalType              = types.IsValidProposalType
-	ProposalHandler                  = types.ProposalHandler
-	VoteOptionFromString             = types.VoteOptionFromString
-	ValidVoteOption                  = types.ValidVoteOption
+	RegisterCodec                 = types.RegisterCodec
+	RegisterProposalTypeCodec     = types.RegisterProposalTypeCodec
+	ValidateAbstract              = types.ValidateAbstract
+	NewDeposit                    = types.NewDeposit
+	ErrUnknownProposal            = types.ErrUnknownProposal
+	ErrInactiveProposal           = types.ErrInactiveProposal
+	ErrAlreadyActiveProposal      = types.ErrAlreadyActiveProposal
+	ErrAlreadyFinishedProposal    = types.ErrAlreadyFinishedProposal
+	ErrAddressNotStaked           = types.ErrAddressNotStaked
+	ErrInvalidProposalContent     = types.ErrInvalidProposalContent
+	ErrInvalidProposalType        = types.ErrInvalidProposalType
+	ErrInvalidVote                = types.ErrInvalidVote
+	ErrInvalidGenesis             = types.ErrInvalidGenesis
+	ErrNoProposalHandlerExists    = types.ErrNoProposalHandlerExists
+	ProposalKey                   = types.ProposalKey
+	ActiveProposalByTimeKey       = types.ActiveProposalByTimeKey
+	ActiveProposalQueueKey        = types.ActiveProposalQueueKey
+	InactiveProposalByTimeKey     = types.InactiveProposalByTimeKey
+	InactiveProposalQueueKey      = types.InactiveProposalQueueKey
+	DepositsKey                   = types.DepositsKey
+	DepositKey                    = types.DepositKey
+	VotesKey                      = types.VotesKey
+	VoteKey                       = types.VoteKey
+	SplitProposalKey              = types.SplitProposalKey
+	SplitActiveProposalQueueKey   = types.SplitActiveProposalQueueKey
+	SplitInactiveProposalQueueKey = types.SplitInactiveProposalQueueKey
+	SplitKeyDeposit               = types.SplitKeyDeposit
+	SplitKeyVote                  = types.SplitKeyVote
+	NewMsgSubmitProposal          = types.NewMsgSubmitProposal
+	NewMsgDeposit                 = types.NewMsgDeposit
+	NewMsgVote                    = types.NewMsgVote
+	NewProposal                   = types.NewProposal
+	ProposalStatusFromString      = types.ProposalStatusFromString
+	ValidProposalStatus           = types.ValidProposalStatus
+	NewTallyResult                = types.NewTallyResult
+	NewTallyResultFromMap         = types.NewTallyResultFromMap
+	EmptyTallyResult              = types.EmptyTallyResult
+	NewTextProposal               = types.NewTextProposal
+	NewSoftwareUpgradeProposal    = types.NewSoftwareUpgradeProposal
+	RegisterProposalType          = types.RegisterProposalType
+	ContentFromProposalType       = types.ContentFromProposalType
+	IsValidProposalType           = types.IsValidProposalType
+	ProposalHandler               = types.ProposalHandler
+	NewVote                       = types.NewVote
+	VoteOptionFromString          = types.VoteOptionFromString
+	ValidVoteOption               = types.ValidVoteOption
 
 	// variable aliases
 	ModuleCdc                   = types.ModuleCdc
-	KeyDelimiter                = types.KeyDelimiter
-	KeyNextProposalID           = types.KeyNextProposalID
-	PrefixActiveProposalQueue   = types.PrefixActiveProposalQueue
-	PrefixInactiveProposalQueue = types.PrefixInactiveProposalQueue
+	ProposalsKeyPrefix          = types.ProposalsKeyPrefix
+	ActiveProposalQueuePrefix   = types.ActiveProposalQueuePrefix
+	InactiveProposalQueuePrefix = types.InactiveProposalQueuePrefix
+	ProposalIDKey               = types.ProposalIDKey
+	DepositsKeyPrefix           = types.DepositsKeyPrefix
+	VotesKeyPrefix              = types.VotesKeyPrefix
 )
 
 type (

x/gov/deposit.go

@@ -0,0 +1,122 @@
+package gov
+
+import (
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/cosmos/cosmos-sdk/x/gov/types"
+)
+
+// GetDeposit gets the deposit of a specific depositor on a specific proposal
+func (keeper Keeper) GetDeposit(ctx sdk.Context, proposalID uint64, depositorAddr sdk.AccAddress) (deposit Deposit, found bool) {
+	store := ctx.KVStore(keeper.storeKey)
+	bz := store.Get(types.DepositKey(proposalID, depositorAddr))
+	if bz == nil {
+		return deposit, false
+	}
+
+	keeper.cdc.MustUnmarshalBinaryLengthPrefixed(bz, &deposit)
+	return deposit, true
+}
+
+func (keeper Keeper) setDeposit(ctx sdk.Context, proposalID uint64, depositorAddr sdk.AccAddress, deposit Deposit) {
+	store := ctx.KVStore(keeper.storeKey)
+	bz := keeper.cdc.MustMarshalBinaryLengthPrefixed(deposit)
+	store.Set(types.DepositKey(proposalID, depositorAddr), bz)
+}
+
+// AddDeposit adds or updates a deposit of a specific depositor on a specific proposal
+// Activates voting period when appropriate
+func (keeper Keeper) AddDeposit(ctx sdk.Context, proposalID uint64, depositorAddr sdk.AccAddress, depositAmount sdk.Coins) (sdk.Error, bool) {
+	// Checks to see if proposal exists
+	proposal, ok := keeper.GetProposal(ctx, proposalID)
+	if !ok {
+		return ErrUnknownProposal(keeper.codespace, proposalID), false
+	}
+
+	// Check if proposal is still depositable
+	if (proposal.Status != StatusDepositPeriod) && (proposal.Status != StatusVotingPeriod) {
+		return ErrAlreadyFinishedProposal(keeper.codespace, proposalID), false
+	}
+
+	// Send coins from depositor's account to DepositedCoinsAccAddr account
+	// TODO: Don't use an account for this purpose; it's clumsy and prone to misuse.
+	err := keeper.ck.SendCoins(ctx, depositorAddr, DepositedCoinsAccAddr, depositAmount)
+	if err != nil {
+		return err, false
+	}
+
+	// Update proposal
+	proposal.TotalDeposit = proposal.TotalDeposit.Add(depositAmount)
+	keeper.SetProposal(ctx, proposal)
+
+	// Check if deposit has provided sufficient total funds to transition the proposal into the voting period
+	activatedVotingPeriod := false
+	if proposal.Status == StatusDepositPeriod && proposal.TotalDeposit.IsAllGTE(keeper.GetDepositParams(ctx).MinDeposit) {
+		keeper.activateVotingPeriod(ctx, proposal)
+		activatedVotingPeriod = true
+	}
+
+	// Add or update deposit object
+	deposit, found := keeper.GetDeposit(ctx, proposalID, depositorAddr)
+	if found {
+		deposit.Amount = deposit.Amount.Add(depositAmount)
+	} else {
+		deposit = NewDeposit(proposalID, depositorAddr, depositAmount)
+	}
+
+	keeper.setDeposit(ctx, proposalID, depositorAddr, deposit)
+
+	return nil, activatedVotingPeriod
+}
+
+// GetAllDeposits returns all the deposits from the store
+func (keeper Keeper) GetAllDeposits(ctx sdk.Context) (deposits Deposits) {
+	keeper.IterateAllDeposits(ctx, func(deposit Deposit) bool {
+		deposits = append(deposits, deposit)
+		return false
+	})
+	return
+}
+
+// GetDeposits returns all the deposits from a proposal
+func (keeper Keeper) GetDeposits(ctx sdk.Context, proposalID uint64) (deposits Deposits) {
+	keeper.IterateDeposits(ctx, proposalID, func(deposit Deposit) bool {
+		deposits = append(deposits, deposit)
+		return false
+	})
+	return
+}
+
+// GetDepositsIterator gets all the deposits on a specific proposal as an sdk.Iterator
+func (keeper Keeper) GetDepositsIterator(ctx sdk.Context, proposalID uint64) sdk.Iterator {
+	store := ctx.KVStore(keeper.storeKey)
+	return sdk.KVStorePrefixIterator(store, types.DepositsKey(proposalID))
+}
+
+// RefundDeposits refunds and deletes all the deposits on a specific proposal
+func (keeper Keeper) RefundDeposits(ctx sdk.Context, proposalID uint64) {
+	store := ctx.KVStore(keeper.storeKey)
+
+	keeper.IterateDeposits(ctx, proposalID, func(deposit Deposit) bool {
+		err := keeper.ck.SendCoins(ctx, DepositedCoinsAccAddr, deposit.Depositor, deposit.Amount)
+		if err != nil {
+			panic(err)
+		}
+		store.Delete(DepositKey(proposalID, deposit.Depositor))
+		return false
+	})
+}
+
+// DeleteDeposits deletes all the deposits on a specific proposal without refunding them
+func (keeper Keeper) DeleteDeposits(ctx sdk.Context, proposalID uint64) {
+	store := ctx.KVStore(keeper.storeKey)
+
+	keeper.IterateDeposits(ctx, proposalID, func(deposit Deposit) bool {
+		err := keeper.ck.SendCoins(ctx, DepositedCoinsAccAddr, BurnedDepositCoinsAccAddr, deposit.Amount)
+		if err != nil {
+			panic(err)
+		}
+
+		store.Delete(DepositKey(proposalID, deposit.Depositor))
+		return false
+	})
+}

x/gov/endblocker.go

@@ -7,100 +7,85 @@ import (
 	"github.com/cosmos/cosmos-sdk/x/gov/tags"
 )
 
-// Called every block, process inflation, update validator set
+// EndBlocker called every block, process inflation, update validator set
 func EndBlocker(ctx sdk.Context, keeper Keeper) sdk.Tags {
 	logger := keeper.Logger(ctx)
 	resTags := sdk.NewTags()
 
-	inactiveIterator := keeper.InactiveProposalQueueIterator(ctx, ctx.BlockHeader().Time)
-	defer inactiveIterator.Close()
-	for ; inactiveIterator.Valid(); inactiveIterator.Next() {
-		var proposalID uint64
+	// delete inactive proposal from store and its deposits
+	keeper.IterateInactiveProposalsQueue(ctx, ctx.BlockHeader().Time, func(proposal Proposal) bool {
+		keeper.DeleteProposal(ctx, proposal.ProposalID)
+		keeper.DeleteDeposits(ctx, proposal.ProposalID)
 
-		keeper.cdc.MustUnmarshalBinaryLengthPrefixed(inactiveIterator.Value(), &proposalID)
-		inactiveProposal, ok := keeper.GetProposal(ctx, proposalID)
-		if !ok {
-			panic(fmt.Sprintf("proposal %d does not exist", proposalID))
-		}
-
-		keeper.DeleteProposal(ctx, proposalID)
-		keeper.DeleteDeposits(ctx, proposalID) // delete any associated deposits (burned)
-
-		resTags = resTags.AppendTag(tags.ProposalID, fmt.Sprintf("%d", proposalID))
+		resTags = resTags.AppendTag(tags.ProposalID, fmt.Sprintf("%d", proposal.ProposalID))
 		resTags = resTags.AppendTag(tags.ProposalResult, tags.ActionProposalDropped)
 
 		logger.Info(
 			fmt.Sprintf("proposal %d (%s) didn't meet minimum deposit of %s (had only %s); deleted",
-				inactiveProposal.ProposalID,
-				inactiveProposal.GetTitle(),
+				proposal.ProposalID,
+				proposal.GetTitle(),
 				keeper.GetDepositParams(ctx).MinDeposit,
-				inactiveProposal.TotalDeposit,
+				proposal.TotalDeposit,
 			),
 		)
-	}
+		return false
+	})
 
 	// fetch active proposals whose voting periods have ended (are passed the block time)
-	activeIterator := keeper.ActiveProposalQueueIterator(ctx, ctx.BlockHeader().Time)
-	defer activeIterator.Close()
-	for ; activeIterator.Valid(); activeIterator.Next() {
-		var proposalID uint64
-
-		keeper.cdc.MustUnmarshalBinaryLengthPrefixed(activeIterator.Value(), &proposalID)
-		activeProposal, ok := keeper.GetProposal(ctx, proposalID)
-		if !ok {
-			panic(fmt.Sprintf("proposal %d does not exist", proposalID))
-		}
-		passes, burnDeposits, tallyResults := tally(ctx, keeper, activeProposal)
-
+	keeper.IterateActiveProposalsQueue(ctx, ctx.BlockHeader().Time, func(proposal Proposal) bool {
 		var tagValue, logMsg string
 
+		passes, burnDeposits, tallyResults := tally(ctx, keeper, proposal)
+
 		if burnDeposits {
-			keeper.DeleteDeposits(ctx, activeProposal.ProposalID)
+			keeper.DeleteDeposits(ctx, proposal.ProposalID)
 		} else {
-			keeper.RefundDeposits(ctx, activeProposal.ProposalID)
+			keeper.RefundDeposits(ctx, proposal.ProposalID)
 		}
 
 		if passes {
-			handler := keeper.router.GetRoute(activeProposal.ProposalRoute())
+			handler := keeper.router.GetRoute(proposal.ProposalRoute())
 			cacheCtx, writeCache := ctx.CacheContext()
 
 			// The proposal handler may execute state mutating logic depending
 			// on the proposal content. If the handler fails, no state mutation
 			// is written and the error message is logged.
-			err := handler(cacheCtx, activeProposal.Content)
+			err := handler(cacheCtx, proposal.Content)
 			if err == nil {
-				activeProposal.Status = StatusPassed
+				proposal.Status = StatusPassed
 				tagValue = tags.ActionProposalPassed
 				logMsg = "passed"
 
 				// write state to the underlying multi-store
 				writeCache()
 			} else {
-				activeProposal.Status = StatusFailed
+				proposal.Status = StatusFailed
 				tagValue = tags.ActionProposalFailed
 				logMsg = fmt.Sprintf("passed, but failed on execution: %s", err.ABCILog())
 			}
 		} else {
-			activeProposal.Status = StatusRejected
+			proposal.Status = StatusRejected
 			tagValue = tags.ActionProposalRejected
 			logMsg = "rejected"
 		}
 
-		activeProposal.FinalTallyResult = tallyResults
+		proposal.FinalTallyResult = tallyResults
 
-		keeper.SetProposal(ctx, activeProposal)
-		keeper.RemoveFromActiveProposalQueue(ctx, activeProposal.VotingEndTime, activeProposal.ProposalID)
+		keeper.SetProposal(ctx, proposal)
+		keeper.RemoveFromActiveProposalQueue(ctx, proposal.ProposalID, proposal.VotingEndTime)
 
 		logger.Info(
 			fmt.Sprintf(
 				"proposal %d (%s) tallied; result: %s",
-				activeProposal.ProposalID, activeProposal.GetTitle(), logMsg,
+				proposal.ProposalID, proposal.GetTitle(), logMsg,
 			),
 		)
 
-		resTags = resTags.AppendTag(tags.ProposalID, fmt.Sprintf("%d", proposalID))
+		resTags = resTags.AppendTag(tags.ProposalID, fmt.Sprintf("%d", proposal.ProposalID))
 		resTags = resTags.AppendTag(tags.ProposalResult, tagValue)
-	}
+
+		return false
+	})
 
 	return resTags
 }

x/gov/endblocker_test.go

@@ -232,7 +232,7 @@ func TestTickPassedVotingPeriod(t *testing.T) {
 	proposal, ok := input.keeper.GetProposal(ctx, activeProposalID)
 	require.True(t, ok)
 	require.Equal(t, StatusVotingPeriod, proposal.Status)
-	depositsIterator := input.keeper.GetDeposits(ctx, proposalID)
+	depositsIterator := input.keeper.GetDepositsIterator(ctx, proposalID)
 	require.True(t, depositsIterator.Valid())
 	depositsIterator.Close()
 	activeQueue.Close()