Squashed commit of the following:

commit b28e176
Author: Shawn Marshall-Spitzbart <[email protected]>
Date:   Mon Dec 5 17:11:41 2022 -0800

    requested key format changes (#560)

    changes

commit 5b8d2c7
Author: Shawn Marshall-Spitzbart <[email protected]>
Date:   Mon Dec 5 13:12:55 2022 -0800

    log

x/ccv/provider/keeper/relay.go

@@ -253,6 +253,7 @@ func (k Keeper) OnRecvSlashPacket(ctx sdk.Context, packet channeltypes.Packet, d
 	k.QueuePendingSlashPacketEntry(ctx, providertypes.NewSlashPacketEntry(
 		ctx.BlockTime(), // recv time
 		chainID,         // consumer chain id that sent the packet
+		packet.Sequence, // IBC sequence number of the packet
 		data.Validator.Address))
 
 	// Queue slash packet data in the same (consumer chain specific) queue as vsc matured packet data,

x/ccv/provider/keeper/throttle.go

@@ -163,8 +163,8 @@ func (k Keeper) GetSlashMeterAllowance(ctx sdktypes.Context) sdktypes.Int {
 
 	roundedInt := sdktypes.NewInt(decFrac.MulInt(totalPower).RoundInt64())
 	if roundedInt.IsZero() {
-		// TODO: Log warning that replenish fraction is too small to
-		// add any allowance to the meter, considering bankers rounding.
+		k.Logger(ctx).Info("slash meter replenish fraction is too small " +
+			"to add any allowance to the meter, considering bankers rounding")
 
 		// Return non-zero allowance to guarantee some slash packets are eventually handled
 		return sdktypes.NewInt(1)
@@ -180,10 +180,10 @@ func (k Keeper) GetSlashMeterAllowance(ctx sdktypes.Context) sdktypes.Int {
 // related to jailing/tombstoning over time. This "parent" queue is used to coordinate the order of slash packet handling
 // between chains, whereas the chain specific queue is used to coordinate the order of slash and vsc matured packets
 // relevant to each chain.
-func (k Keeper) QueuePendingSlashPacketEntry(ctx sdktypes.Context, entry providertypes.SlashPacketEntry) {
+func (k Keeper) QueuePendingSlashPacketEntry(ctx sdktypes.Context,
+	entry providertypes.SlashPacketEntry) {
 	store := ctx.KVStore(k.storeKey)
 	key := providertypes.PendingSlashPacketEntryKey(entry)
-	// Note: Val address is stored as value to assist in debugging. This could be removed for efficiency.
 	store.Set(key, entry.ValAddr)
 }
 
@@ -206,9 +206,9 @@ func (k Keeper) IteratePendingSlashPacketEntries(ctx sdktypes.Context,
 	iterator := sdktypes.KVStorePrefixIterator(store, []byte{providertypes.PendingSlashPacketEntryBytePrefix})
 	defer iterator.Close()
 	for ; iterator.Valid(); iterator.Next() {
-		recvTime, chainID := providertypes.ParsePendingSlashPacketEntryKey(iterator.Key())
+		recvTime, chainID, ibcSeqNum := providertypes.ParsePendingSlashPacketEntryKey(iterator.Key())
 		valAddr := iterator.Value()
-		entry := providertypes.NewSlashPacketEntry(recvTime, chainID, valAddr)
+		entry := providertypes.NewSlashPacketEntry(recvTime, chainID, ibcSeqNum, valAddr)
 		stop := cb(entry)
 		if stop {
 			break

x/ccv/provider/keeper/throttle_test.go

@@ -616,24 +616,28 @@ func TestPendingSlashPacketEntries(t *testing.T) {
 	defer ctrl.Finish()
 
 	// Consistent time for "now"
-	now := time.Now()
+	now := time.Now().UTC()
 
 	entries := providerKeeper.GetAllPendingSlashPacketEntries(ctx)
 	require.Equal(t, 0, len(entries))
 
 	// Queue 3 entries for chainIDs 0, 1, 2
 	for i := 0; i < 3; i++ {
-		entry := providertypes.NewSlashPacketEntry(now,
-			fmt.Sprintf("chain-%d", i), ed25519.GenPrivKey().PubKey().Address())
+		entry := providertypes.NewSlashPacketEntry(now.Local(),
+			fmt.Sprintf("chain-%d", i),
+			8, // all with seq = 8
+			ed25519.GenPrivKey().PubKey().Address())
 		providerKeeper.QueuePendingSlashPacketEntry(ctx, entry)
 	}
 	entries = providerKeeper.GetAllPendingSlashPacketEntries(ctx)
 	require.Equal(t, 3, len(entries))
 
 	// Queue 3 entries for chainIDs 0, 1, 2 an hour later
 	for i := 0; i < 3; i++ {
-		entry := providertypes.NewSlashPacketEntry(now.Add(time.Hour),
-			fmt.Sprintf("chain-%d", i), ed25519.GenPrivKey().PubKey().Address())
+		entry := providertypes.NewSlashPacketEntry(now.Add(time.Hour).Local(),
+			fmt.Sprintf("chain-%d", i),
+			9, // all with seq = 9
+			ed25519.GenPrivKey().PubKey().Address())
 		providerKeeper.QueuePendingSlashPacketEntry(ctx, entry)
 	}
 
@@ -653,8 +657,10 @@ func TestPendingSlashPacketEntries(t *testing.T) {
 
 	// Queue 3 entries for chainIDs 5, 6, 7 another hour later
 	for i := 0; i < 3; i++ {
-		entry := providertypes.NewSlashPacketEntry(now.Add(2*time.Hour),
-			fmt.Sprintf("chain-%d", i+5), ed25519.GenPrivKey().PubKey().Address())
+		entry := providertypes.NewSlashPacketEntry(now.Add(2*time.Hour).Local(),
+			fmt.Sprintf("chain-%d", i+5),
+			7697, // all with seq = 7697
+			ed25519.GenPrivKey().PubKey().Address())
 		providerKeeper.QueuePendingSlashPacketEntry(ctx, entry)
 	}
 
@@ -676,6 +682,29 @@ func TestPendingSlashPacketEntries(t *testing.T) {
 	require.True(t, slices.Contains(thirdChainIdSet, "chain-6"))
 	require.True(t, slices.Contains(thirdChainIdSet, "chain-7"))
 
+	// Assert each field is as expected for all 9 entries
+	for idx, entry := range entries {
+		switch idx {
+		case 0, 1, 2:
+			require.Equal(t, now, entry.RecvTime)
+			require.Equal(t, fmt.Sprintf("chain-%d", idx), entry.ConsumerChainID)
+			require.Equal(t, uint64(8), entry.IbcSeqNum)
+			require.NotEmpty(t, entry.ValAddr)
+		case 3, 4, 5:
+			require.Equal(t, now.Add(time.Hour), entry.RecvTime)
+			require.Equal(t, fmt.Sprintf("chain-%d", idx-3), entry.ConsumerChainID)
+			require.Equal(t, uint64(9), entry.IbcSeqNum)
+			require.NotEmpty(t, entry.ValAddr)
+		case 6, 7, 8:
+			require.Equal(t, now.Add(2*time.Hour), entry.RecvTime)
+			require.Equal(t, fmt.Sprintf("chain-%d", idx-6+5), entry.ConsumerChainID)
+			require.Equal(t, uint64(7697), entry.IbcSeqNum)
+			require.NotEmpty(t, entry.ValAddr)
+		default:
+			t.Fatalf("unexpected entry index %d", idx)
+		}
+	}
+
 	// Test the callback break functionality of the iterator
 	entries = []providertypes.SlashPacketEntry{}
 	providerKeeper.IteratePendingSlashPacketEntries(ctx, func(entry providertypes.SlashPacketEntry) bool {
@@ -701,13 +730,13 @@ func TestPendingSlashPacketEntryDeletion(t *testing.T) {
 
 	// Instantiate entries in the expected order we wish to get them back as (ordered by recv time)
 	entries = []providertypes.SlashPacketEntry{}
-	entries = append(entries, providertypes.NewSlashPacketEntry(now, "chain-0", ed25519.GenPrivKey().PubKey().Address()))
-	entries = append(entries, providertypes.NewSlashPacketEntry(now.Add(time.Hour).UTC(), "chain-1", ed25519.GenPrivKey().PubKey().Address()))
-	entries = append(entries, providertypes.NewSlashPacketEntry(now.Add(2*time.Hour).Local(), "chain-2", ed25519.GenPrivKey().PubKey().Address()))
-	entries = append(entries, providertypes.NewSlashPacketEntry(now.Add(3*time.Hour).In(time.FixedZone("UTC-8", -8*60*60)), "chain-3", ed25519.GenPrivKey().PubKey().Address()))
-	entries = append(entries, providertypes.NewSlashPacketEntry(now.Add(4*time.Hour).Local(), "chain-4", ed25519.GenPrivKey().PubKey().Address()))
-	entries = append(entries, providertypes.NewSlashPacketEntry(now.Add(5*time.Hour).UTC(), "chain-5", ed25519.GenPrivKey().PubKey().Address()))
-	entries = append(entries, providertypes.NewSlashPacketEntry(now.Add(6*time.Hour).Local(), "chain-6", ed25519.GenPrivKey().PubKey().Address()))
+	entries = append(entries, providertypes.NewSlashPacketEntry(now, "chain-0", 1, ed25519.GenPrivKey().PubKey().Address()))
+	entries = append(entries, providertypes.NewSlashPacketEntry(now.Add(time.Hour).UTC(), "chain-1", 178, ed25519.GenPrivKey().PubKey().Address()))
+	entries = append(entries, providertypes.NewSlashPacketEntry(now.Add(2*time.Hour).Local(), "chain-2", 89, ed25519.GenPrivKey().PubKey().Address()))
+	entries = append(entries, providertypes.NewSlashPacketEntry(now.Add(3*time.Hour).In(time.FixedZone("UTC-8", -8*60*60)), "chain-3", 23423, ed25519.GenPrivKey().PubKey().Address()))
+	entries = append(entries, providertypes.NewSlashPacketEntry(now.Add(4*time.Hour).Local(), "chain-4", 323, ed25519.GenPrivKey().PubKey().Address()))
+	entries = append(entries, providertypes.NewSlashPacketEntry(now.Add(5*time.Hour).UTC(), "chain-5", 18, ed25519.GenPrivKey().PubKey().Address()))
+	entries = append(entries, providertypes.NewSlashPacketEntry(now.Add(6*time.Hour).Local(), "chain-6", 2, ed25519.GenPrivKey().PubKey().Address()))
 
 	// Instantiate shuffled copy of above slice
 	shuffledEntries := append([]providertypes.SlashPacketEntry{}, entries...)

x/ccv/provider/types/keys.go

@@ -275,38 +275,41 @@ func ParsePendingPacketDataKey(key []byte) (string, uint64, error) {
 }
 
 // PendingSlashPacketEntryKey returns the key for storing a pending slash packet entry.
-//
-// Note: It's not expected for a single consumer chain to send a slash packet for the same validator more than once
-// in the same block. Hence why this key should ber unique per slash packet. However, if a malicious consumer did send
-// duplicate slash packets in the same block, the slash packet entry would simply be overwritten.
 func PendingSlashPacketEntryKey(packetEntry SlashPacketEntry) []byte {
 	timeBz := sdk.FormatTimeBytes(packetEntry.RecvTime)
 	timeBzL := len(timeBz)
 	return AppendMany(
 		[]byte{PendingSlashPacketEntryBytePrefix},
 		sdk.Uint64ToBigEndian(uint64(timeBzL)),
 		timeBz,
-		HashBytes(packetEntry.ValAddr),
+		sdk.Uint64ToBigEndian(packetEntry.IbcSeqNum),
 		[]byte(packetEntry.ConsumerChainID),
 	)
 }
 
 // ParsePendingSlashPacketEntryKey returns the received time and chainID for a pending slash packet entry key
-func ParsePendingSlashPacketEntryKey(bz []byte) (time.Time, string) {
+func ParsePendingSlashPacketEntryKey(bz []byte) (
+	recvTime time.Time, consumerChainID string, ibcSeqNum uint64) {
+
 	// Prefix is in first byte
 	expectedPrefix := []byte{PendingSlashPacketEntryBytePrefix}
 	if prefix := bz[:1]; !bytes.Equal(prefix, expectedPrefix) {
 		panic(fmt.Sprintf("invalid prefix; expected: %X, got: %X", expectedPrefix, prefix))
 	}
+
 	// 8 bytes for uint64 storing timestamp length
 	timeBzL := sdk.BigEndianToUint64(bz[1:9])
 	recvTime, err := sdk.ParseTimeBytes(bz[9 : 9+timeBzL])
 	if err != nil {
 		panic(err)
 	}
-	// ChainID is stored after 32 byte hashed validator address
-	chainID := string(bz[9+int(timeBzL)+32:])
-	return recvTime, chainID
+
+	ibcSeqNum = sdk.BigEndianToUint64(bz[9+timeBzL : 9+timeBzL+8])
+
+	// ChainID is stored after 8 byte ibc seq num
+	chainID := string(bz[9+timeBzL+8:])
+
+	return recvTime, chainID, ibcSeqNum
 }
 
 // ConsumerValidatorsKey returns the key under which the

x/ccv/provider/types/keys_test.go

@@ -179,19 +179,20 @@ func TestPendingPacketDataKeyAndParse(t *testing.T) {
 func TestPendingSlashPacketEntryKeyAndParse(t *testing.T) {
 	now := time.Now()
 	entries := []providertypes.SlashPacketEntry{}
-	entries = append(entries, providertypes.NewSlashPacketEntry(now, "chain-0", ed25519.GenPrivKey().PubKey().Address()))
-	entries = append(entries, providertypes.NewSlashPacketEntry(now.Add(2*time.Hour), "chain-7896978", ed25519.GenPrivKey().PubKey().Address()))
-	entries = append(entries, providertypes.NewSlashPacketEntry(now.Add(3*time.Hour), "chain-1", ed25519.GenPrivKey().PubKey().Address()))
+	entries = append(entries, providertypes.NewSlashPacketEntry(now, "chain-0", 2, ed25519.GenPrivKey().PubKey().Address()))
+	entries = append(entries, providertypes.NewSlashPacketEntry(now.Add(2*time.Hour), "chain-7896978", 3, ed25519.GenPrivKey().PubKey().Address()))
+	entries = append(entries, providertypes.NewSlashPacketEntry(now.Add(3*time.Hour), "chain-1", 4723894, ed25519.GenPrivKey().PubKey().Address()))
 
 	for _, entry := range entries {
 		key := providertypes.PendingSlashPacketEntryKey(entry)
 		require.NotEmpty(t, key)
 		timeBzL := len(sdk.FormatTimeBytes(entry.RecvTime))
-		// This key should be of set length: prefix + 8 + timeBzL + hashed valAddr + chainID
-		require.Equal(t, 1+8+timeBzL+32+len(entry.ConsumerChainID), len(key))
-		parsedRecvTime, parsedChainID := providertypes.ParsePendingSlashPacketEntryKey(key)
+		// This key should be of set length: prefix + 8 + timeBzL + 8 + chainID
+		require.Equal(t, 1+8+timeBzL+8+len(entry.ConsumerChainID), len(key))
+		parsedRecvTime, parsedChainID, parsedIBCSeqNum := providertypes.ParsePendingSlashPacketEntryKey(key)
 		require.Equal(t, entry.RecvTime, parsedRecvTime)
 		require.Equal(t, entry.ConsumerChainID, parsedChainID)
+		require.Equal(t, entry.IbcSeqNum, parsedIBCSeqNum)
 	}
 }
 

x/ccv/provider/types/throttle.go

@@ -7,18 +7,27 @@ import (
 // A persisted queue entry indicating that a slash packet data instance needs to be handled
 type SlashPacketEntry struct {
 	// Block time that slash packet was received by provider chain.
+	// This field is used for store key iteration ordering.
 	RecvTime time.Time
 	// The consumer that sent the packet.
 	ConsumerChainID string
+	// The IBC sequence number of the recv packet.
+	// This field is used in the store key to ensure uniqueness.
+	IbcSeqNum uint64
 	// The byte address of the validator being slashed.
+	// This field is used to obtain validator power in HandlePendingSlashPackets.
+	// It is not used in the store key, but is persisted in value bytes,
+	// see QueuePendingSlashPacketEntry.
 	ValAddr []byte
 }
 
 // NewSlashPacketEntry creates a new SlashPacketEntry.
-func NewSlashPacketEntry(recvTime time.Time, consumerChainID string, valAddr []byte) SlashPacketEntry {
+func NewSlashPacketEntry(recvTime time.Time, consumerChainID string,
+	ibcSeqNum uint64, valAddr []byte) SlashPacketEntry {
 	return SlashPacketEntry{
 		RecvTime:        recvTime.UTC(), // UTC prevents serialization inconsistencies
 		ConsumerChainID: consumerChainID,
+		IbcSeqNum:       ibcSeqNum,
 		ValAddr:         valAddr,
 	}
 }