refactor(gov)!: finalize collections migration (#16268)

Co-authored-by: unknown unknown <unknown@unknown>

CHANGELOG.md

@@ -235,6 +235,11 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * (x/authx) [#15284](https://github.com/cosmos/cosmos-sdk/pull/15284) `NewKeeper` now requires `codec.Codec`.
 * (x/gov) [#15284](https://github.com/cosmos/cosmos-sdk/pull/15284) `NewKeeper` now requires `codec.Codec`.
 
+* (x/gov) [#16268](https://github.com/cosmos/cosmos-sdk/pull/16268) Use collections for proposal state management (part 2):
+   * this finalizes the gov collections migration
+   * Removed: keeper `InsertActiveProposalsQueue`, `RemoveActiveProposalsQueue`, `InsertInactiveProposalsQueue`, `RemoveInactiveProposalsQueue`, `IterateInactiveProposalsQueue`, `IterateActiveProposalsQueue`, `ActiveProposalsQueueIterator`, `InactiveProposalsQueueIterator`
+   * Remove: types all the key related functions
+
 ### Client Breaking Changes
 
 * (x/staking) [#15701](https://github.com/cosmos/cosmos-sdk/pull/15701) `HistoricalInfoKey` now has a binary format.

collections/pair.go

@@ -216,6 +216,12 @@ func (p pairKeyCodec[K1, K2]) DecodeJSON(b []byte) (Pair[K1, K2], error) {
 	return Join(k1, k2), nil
 }
 
+// NewPrefixUntilPairRange defines a collection query which ranges until the provided Pair prefix.
+// Unstable: this API might change in the future.
+func NewPrefixUntilPairRange[K1, K2 any](prefix K1) *PairRange[K1, K2] {
+	return &PairRange[K1, K2]{end: RangeKeyPrefixEnd(PairPrefix[K1, K2](prefix))}
+}
+
 // NewPrefixedPairRange creates a new PairRange which will prefix over all the keys
 // starting with the provided prefix.
 func NewPrefixedPairRange[K1, K2 any](prefix K1) *PairRange[K1, K2] {

types/collections.go

@@ -1,6 +1,9 @@
 package types
 
 import (
+	"fmt"
+	"time"
+
 	"cosmossdk.io/collections"
 	collcodec "cosmossdk.io/collections/codec"
 	"cosmossdk.io/math"
@@ -28,6 +31,12 @@ var (
 
 	// IntValue represents a collections.ValueCodec to work with Int.
 	IntValue collcodec.ValueCodec[math.Int] = intValueCodec{}
+
+	// TimeKey represents a collections.KeyCodec to work with time.Time
+	// Deprecated: exists only for state compatibility reasons, should not
+	// be used for new storage keys using time. Please use the time KeyCodec
+	// provided in the collections package.
+	TimeKey collcodec.KeyCodec[time.Time] = timeKeyCodec{}
 )
 
 type addressUnion interface {
@@ -156,3 +165,46 @@ func (i intValueCodec) Stringify(value Int) string {
 func (i intValueCodec) ValueType() string {
 	return "math.Int"
 }
+
+type timeKeyCodec struct{}
+
+func (timeKeyCodec) Encode(buffer []byte, key time.Time) (int, error) {
+	return copy(buffer, FormatTimeBytes(key)), nil
+}
+
+var timeSize = len(FormatTimeBytes(time.Time{}))
+
+func (timeKeyCodec) Decode(buffer []byte) (int, time.Time, error) {
+	if len(buffer) != timeSize {
+		return 0, time.Time{}, fmt.Errorf("invalid time buffer buffer size")
+	}
+	t, err := ParseTimeBytes(buffer)
+	if err != nil {
+		return 0, time.Time{}, err
+	}
+	return timeSize, t, nil
+}
+
+func (timeKeyCodec) Size(key time.Time) int { return timeSize }
+
+func (timeKeyCodec) EncodeJSON(value time.Time) ([]byte, error) { return value.MarshalJSON() }
+
+func (timeKeyCodec) DecodeJSON(b []byte) (time.Time, error) {
+	t := time.Time{}
+	err := t.UnmarshalJSON(b)
+	return t, err
+}
+
+func (timeKeyCodec) Stringify(key time.Time) string { return key.String() }
+func (timeKeyCodec) KeyType() string                { return "sdk/time.Time" }
+func (t timeKeyCodec) EncodeNonTerminal(buffer []byte, key time.Time) (int, error) {
+	return t.Encode(buffer, key)
+}
+
+func (t timeKeyCodec) DecodeNonTerminal(buffer []byte) (int, time.Time, error) {
+	if len(buffer) < timeSize {
+		return 0, time.Time{}, fmt.Errorf("invalid time buffer size, wanted: %d at least, got: %d", timeSize, len(buffer))
+	}
+	return t.Decode(buffer[:timeSize])
+}
+func (t timeKeyCodec) SizeNonTerminal(key time.Time) int { return t.Size(key) }

types/collections_test.go

@@ -2,6 +2,7 @@ package types
 
 import (
 	"testing"
+	"time"
 
 	"cosmossdk.io/collections/colltest"
 )
@@ -22,4 +23,8 @@ func TestCollectionsCorrectness(t *testing.T) {
 	t.Run("AddressIndexingKey", func(t *testing.T) {
 		colltest.TestKeyCodec(t, AddressKeyAsIndexKey(AccAddressKey), AccAddress{0x2, 0x5, 0x8})
 	})
+
+	t.Run("Time", func(t *testing.T) {
+		colltest.TestKeyCodec(t, TimeKey, time.Time{})
+	})
 }

x/gov/abci.go

@@ -1,9 +1,11 @@
 package gov
 
 import (
+	"errors"
 	"fmt"
 	"time"
 
+	"cosmossdk.io/collections"
 	"github.com/cosmos/cosmos-sdk/telemetry"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/cosmos/cosmos-sdk/x/gov/keeper"
@@ -18,15 +20,20 @@ func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) error {
 	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.
-	err := keeper.IterateInactiveProposalsQueue(ctx, ctx.BlockHeader().Time, func(proposal v1.Proposal) error {
-		err := keeper.DeleteProposal(ctx, proposal.Id)
+	rng := collections.NewPrefixUntilPairRange[time.Time, uint64](ctx.BlockTime())
+	err := keeper.InactiveProposalsQueue.Walk(ctx, rng, func(key collections.Pair[time.Time, uint64], _ uint64) (bool, error) {
+		proposal, err := keeper.Proposals.Get(ctx, key.K2())
 		if err != nil {
-			return err
+			return false, err
+		}
+		err = keeper.DeleteProposal(ctx, proposal.Id)
+		if err != nil {
+			return false, err
 		}
 
 		params, err := keeper.Params.Get(ctx)
 		if err != nil {
-			return err
+			return false, err
 		}
 		if !params.BurnProposalDepositPrevote {
 			err = keeper.RefundAndDeleteDeposits(ctx, proposal.Id) // refund deposit if proposal got removed without getting 100% of the proposal
@@ -35,7 +42,7 @@ func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) error {
 		}
 
 		if err != nil {
-			return err
+			return false, err
 		}
 
 		// called when proposal become inactive
@@ -58,19 +65,25 @@ func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) error {
 			"total_deposit", sdk.NewCoins(proposal.TotalDeposit...).String(),
 		)
 
-		return nil
+		return false, nil
 	})
-	if err != nil {
+	if err != nil && !errors.Is(err, collections.ErrInvalidIterator) {
 		return err
 	}
 
 	// fetch active proposals whose voting periods have ended (are passed the block time)
-	return keeper.IterateActiveProposalsQueue(ctx, ctx.BlockHeader().Time, func(proposal v1.Proposal) error {
+	rng = collections.NewPrefixUntilPairRange[time.Time, uint64](ctx.BlockTime())
+	err = keeper.ActiveProposalsQueue.Walk(ctx, rng, func(key collections.Pair[time.Time, uint64], _ uint64) (bool, error) {
+		proposal, err := keeper.Proposals.Get(ctx, key.K2())
+		if err != nil {
+			return false, err
+		}
+
 		var tagValue, logMsg string
 
 		passes, burnDeposits, tallyResults, err := keeper.Tally(ctx, proposal)
 		if err != nil {
-			return err
+			return false, err
 		}
 
 		// If an expedited proposal fails, we do not want to update
@@ -85,13 +98,13 @@ func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) error {
 			}
 
 			if err != nil {
-				return err
+				return false, err
 			}
 		}
 
-		err = keeper.RemoveFromActiveProposalQueue(ctx, proposal.Id, *proposal.VotingEndTime)
+		err = keeper.ActiveProposalsQueue.Remove(ctx, collections.Join(*proposal.VotingEndTime, proposal.Id))
 		if err != nil {
-			return err
+			return false, err
 		}
 
 		switch {
@@ -154,14 +167,14 @@ func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) error {
 			proposal.Expedited = false
 			params, err := keeper.Params.Get(ctx)
 			if err != nil {
-				return err
+				return false, err
 			}
 			endTime := proposal.VotingStartTime.Add(*params.VotingPeriod)
 			proposal.VotingEndTime = &endTime
 
-			err = keeper.InsertActiveProposalQueue(ctx, proposal.Id, *proposal.VotingEndTime)
+			err = keeper.ActiveProposalsQueue.Set(ctx, collections.Join(*proposal.VotingEndTime, proposal.Id), proposal.Id)
 			if err != nil {
-				return err
+				return false, err
 			}
 
 			tagValue = types.AttributeValueExpeditedProposalRejected
@@ -176,7 +189,7 @@ func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) error {
 
 		err = keeper.SetProposal(ctx, proposal)
 		if err != nil {
-			return err
+			return false, err
 		}
 
 		// when proposal become active
@@ -200,6 +213,10 @@ func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) error {
 			),
 		)
 
-		return nil
+		return false, nil
 	})
+	if err != nil && !errors.Is(err, collections.ErrInvalidIterator) {
+		return err
+	}
+	return nil
 }