CNS-291: fix PR issues

common/fixation_entry.go

@@ -61,13 +61,15 @@ type FixationStore struct {
 }
 
 // AppendEntry adds a new entry to the store
-func (fs FixationStore) AppendEntry(ctx sdk.Context, index string, block uint64, entryData codec.ProtoMarshaler) error {
+func (fs *FixationStore) AppendEntry(ctx sdk.Context, index string, block uint64, entryData codec.ProtoMarshaler) error {
 	// get the latest entry for this index
-	latestEntry := fs.getUnmarshaledEntryForBlock(ctx, index, block, types.DO_NOTHING)
+	latestEntry, err := fs.getUnmarshaledEntryForBlock(ctx, index, block, types.DO_NOTHING)
 
 	// if latest entry is not found, this is a first version entry
-	if latestEntry == nil {
+	if latestEntry == nil && types.ErrEntryNotFound.Is(err) {
 		fs.SetEntryIndex(ctx, index)
+	} else if err != nil {
+		return utils.LavaError(ctx, ctx.Logger(), "AppendEntry_get_entry_err", map[string]string{"err": err.Error(), "index": index, "block": strconv.FormatUint(block, 10)}, "error getting entry")
 	} else {
 		// make sure the new entry's block is not smaller than the latest entry's block
 		if block < latestEntry.GetBlock() {
@@ -76,30 +78,30 @@ func (fs FixationStore) AppendEntry(ctx sdk.Context, index string, block uint64,
 
 		// if the new entry's block is equal to the latest entry, overwrite the latest entry
 		if block == latestEntry.GetBlock() {
-			return fs.SetEntry(ctx, index, block, entryData)
+			return fs.ModifyEntry(ctx, index, block, entryData)
 		}
 	}
 
 	// marshal the new entry's data
-	b := fs.cdc.MustMarshal(entryData)
+	marshaledEntryData := fs.cdc.MustMarshal(entryData)
 	// create a new entry and marshal it
-	entry := types.Entry{Index: index, Block: block, Data: b, Refcount: 0}
-	bz := fs.cdc.MustMarshal(&entry)
+	entry := types.Entry{Index: index, Block: block, Data: marshaledEntryData, Refcount: 0}
+	marshaledEntry := fs.cdc.MustMarshal(&entry)
 
 	// get the relevant store
 	store := prefix.NewStore(ctx.KVStore(fs.storeKey), types.KeyPrefix(fs.createStoreKey(index)))
 	byteKey := types.KeyPrefix(createEntryKey(block))
 
 	// set the new entry to the store
-	store.Set(byteKey, bz)
+	store.Set(byteKey, marshaledEntry)
 
 	// delete old entries
 	fs.deleteStaleEntries(ctx, index)
 
 	return nil
 }
 
-func (fs FixationStore) deleteStaleEntries(ctx sdk.Context, index string) {
+func (fs *FixationStore) deleteStaleEntries(ctx sdk.Context, index string) {
 	// get the relevant store and init an iterator
 	store := prefix.NewStore(ctx.KVStore(fs.storeKey), types.KeyPrefix(fs.createStoreKey(index)))
 	iterator := sdk.KVStorePrefixIterator(store, []byte{})
@@ -128,29 +130,29 @@ func (fs FixationStore) deleteStaleEntries(ctx sdk.Context, index string) {
 	}
 }
 
-// SetEntry sets a specific entry in the store
-func (fs FixationStore) SetEntry(ctx sdk.Context, index string, block uint64, entryData codec.ProtoMarshaler) error {
+// ModifyEntry modifies an exisiting entry in the store
+func (fs *FixationStore) ModifyEntry(ctx sdk.Context, index string, block uint64, entryData codec.ProtoMarshaler) error {
 	// get the relevant store
 	store := prefix.NewStore(ctx.KVStore(fs.storeKey), types.KeyPrefix(fs.createStoreKey(index)))
 	byteKey := types.KeyPrefix(createEntryKey(block))
 
 	// marshal the new entry data
-	b := fs.cdc.MustMarshal(entryData)
+	marshaledEntryData := fs.cdc.MustMarshal(entryData)
 
 	// get the entry from the store
-	entry := fs.getUnmarshaledEntryForBlock(ctx, index, block, types.DO_NOTHING)
-	if entry == nil {
+	entry, err := fs.getUnmarshaledEntryForBlock(ctx, index, block, types.DO_NOTHING)
+	if err != nil {
 		return utils.LavaError(ctx, ctx.Logger(), "SetEntry_cant_find_entry", map[string]string{"fs.prefix": fs.prefix, "index": index, "block": strconv.FormatUint(block, 10)}, "can't set non-existent entry")
 	}
 
 	// update the entry's data
-	entry.Data = b
+	entry.Data = marshaledEntryData
 
 	// marshal the entry
-	bz := fs.cdc.MustMarshal(entry)
+	marshaledEntry := fs.cdc.MustMarshal(entry)
 
 	// set the entry
-	store.Set(byteKey, bz)
+	store.Set(byteKey, marshaledEntry)
 
 	return nil
 }
@@ -169,6 +171,8 @@ func handleRefAction(ctx sdk.Context, entry *types.Entry, refAction types.Refere
 	case types.SUB_REFERENCE:
 		if entry.GetRefcount() > 0 {
 			entry.Refcount -= 1
+		} else {
+			return utils.LavaError(ctx, ctx.Logger(), "handleRefAction_sub_ref_from_non_positive_count", map[string]string{"refCount": strconv.FormatUint(entry.GetRefcount(), 10)}, "refCount is not larger than zero. Can't subtract refcount")
 		}
 	case types.DO_NOTHING:
 	}
@@ -177,28 +181,28 @@ func handleRefAction(ctx sdk.Context, entry *types.Entry, refAction types.Refere
 }
 
 // GetStoreKey returns the Fixation store's store key
-func (fs FixationStore) GetStoreKey() sdk.StoreKey {
+func (fs *FixationStore) GetStoreKey() sdk.StoreKey {
 	return fs.storeKey
 }
 
 // GetCdc returns the Fixation store's codec
-func (fs FixationStore) GetCdc() codec.BinaryCodec {
+func (fs *FixationStore) GetCdc() codec.BinaryCodec {
 	return fs.cdc
 }
 
 // Getprefix returns the Fixation store's fixation key
-func (fs FixationStore) GetPrefix() string {
+func (fs *FixationStore) GetPrefix() string {
 	return fs.prefix
 }
 
 // Setprefix sets the Fixation store's fixation key
-func (fs FixationStore) SetPrefix(prefix string) FixationStore {
-	fs.prefix = prefix
-	return fs
+func (fs *FixationStore) WithPrefix(prefix string) *FixationStore {
+	editedFs := FixationStore{storeKey: fs.storeKey, cdc: fs.cdc, prefix: prefix}
+	return &editedFs
 }
 
 // getUnmarshaledEntryForBlock gets an entry by block. Block doesn't have to be precise, it gets the closest entry version
-func (fs FixationStore) getUnmarshaledEntryForBlock(ctx sdk.Context, index string, block uint64, refAction types.ReferenceAction) *types.Entry {
+func (fs *FixationStore) getUnmarshaledEntryForBlock(ctx sdk.Context, index string, block uint64, refAction types.ReferenceAction) (*types.Entry, error) {
 	// get the relevant store using index
 	store := prefix.NewStore(ctx.KVStore(fs.storeKey), types.KeyPrefix(fs.createStoreKey(index)))
 
@@ -209,33 +213,37 @@ func (fs FixationStore) getUnmarshaledEntryForBlock(ctx sdk.Context, index strin
 	// iterate over entries
 	for ; iterator.Valid(); iterator.Next() {
 		// unmarshal the entry
-		var val types.Entry
-		fs.cdc.MustUnmarshal(iterator.Value(), &val)
+		var entry types.Entry
+		fs.cdc.MustUnmarshal(iterator.Value(), &entry)
 
 		// if the entry's block is smaller than or equal to the requested block, unmarshal the entry's data
-		if val.GetBlock() <= block {
-			err := handleRefAction(ctx, &val, refAction)
+		// since the user might not know the precise block that the entry was created on, we get the closest one
+		// we get from the past since this was the entry that was valid in the requested block (assume there's an
+		// entry in block 100 and block 200 and the user asks for the version of block 199. Since the block 200's
+		// didn't exist yet, we get the entry from block 100)
+		if entry.GetBlock() <= block {
+			err := handleRefAction(ctx, &entry, refAction)
 			if err != nil {
-				return nil
+				return nil, err
 			}
 
-			return &val
+			return &entry, nil
 		}
 	}
 
-	return nil
+	return nil, types.ErrEntryNotFound
 }
 
-// GetEntryForBlock gets an entry by block. Block doesn't have to be precise, it gets the closest entry version
-func (fs FixationStore) GetEntry(ctx sdk.Context, index string, block uint64, entryData codec.ProtoMarshaler, refAction types.ReferenceAction) error {
+// GetEntryForBlock gets an entry by block. The entry is returned by reference on the entryData argument. Block doesn't have to be precise, it gets the closest entry version
+func (fs *FixationStore) GetEntry(ctx sdk.Context, index string, block uint64, entryData codec.ProtoMarshaler, refAction types.ReferenceAction) error {
 	// get the unmarshaled entry for block
-	entry := fs.getUnmarshaledEntryForBlock(ctx, index, block, refAction)
-	if entry == nil {
+	entry, err := fs.getUnmarshaledEntryForBlock(ctx, index, block, refAction)
+	if err != nil {
 		return utils.LavaError(ctx, ctx.Logger(), "GetEntry_cant_get_entry", map[string]string{}, "can't get entry")
 	}
 
 	// unmarshal the entry's data
-	err := fs.cdc.Unmarshal(entry.GetData(), entryData)
+	err = fs.cdc.Unmarshal(entry.GetData(), entryData)
 	if err != nil {
 		return utils.LavaError(ctx, ctx.Logger(), "GetEntry_cant_unmarshal", map[string]string{}, "can't unmarshal entry data")
 	}
@@ -244,7 +252,7 @@ func (fs FixationStore) GetEntry(ctx sdk.Context, index string, block uint64, en
 }
 
 // RemoveEntry removes an entry from the store
-func (fs FixationStore) removeEntry(ctx sdk.Context, index string, block uint64) {
+func (fs *FixationStore) removeEntry(ctx sdk.Context, index string, block uint64) {
 	// get the relevant store
 	store := prefix.NewStore(ctx.KVStore(fs.storeKey), types.KeyPrefix(fs.createStoreKey(index)))
 
@@ -255,29 +263,17 @@ func (fs FixationStore) removeEntry(ctx sdk.Context, index string, block uint64)
 	store.Delete(types.KeyPrefix(entryKey))
 }
 
-// getAllUnmarshaledEntries gets all the unmarshaled entries from the store (without entries' old versions)
-func (fs FixationStore) getAllEntries(ctx sdk.Context) []*types.Entry {
-	latestVersionEntryList := []*types.Entry{}
-
-	uniqueIndices := fs.GetAllEntryIndices(ctx)
-	for _, uniqueIndex := range uniqueIndices {
-		latestVersionEntry := fs.getUnmarshaledEntryForBlock(ctx, uniqueIndex, uint64(ctx.BlockHeight()), types.DO_NOTHING)
-		latestVersionEntryList = append(latestVersionEntryList, latestVersionEntry)
-	}
-
-	return latestVersionEntryList
-}
-
 // createEntryKey creates an entry key for the KVStore
 func createEntryKey(block uint64) string {
 	return strconv.FormatUint(block, 10)
 }
 
-func (fs FixationStore) createStoreKey(index string) string {
+func (fs *FixationStore) createStoreKey(index string) string {
 	return types.EntryKey + fs.prefix + index
 }
 
 // NewFixationStore returns a new FixationStore object
 func NewFixationStore(storeKey sdk.StoreKey, cdc codec.BinaryCodec, prefix string) *FixationStore {
-	return &FixationStore{storeKey: storeKey, cdc: cdc, prefix: prefix}
+	fs := FixationStore{storeKey: storeKey, cdc: cdc, prefix: prefix}
+	return &fs
 }

common/fixation_entry_index.go

@@ -19,7 +19,7 @@ entry index of a package object to the store, it acceses the store using the key
 // SetEntryIndex appends an entry index in the store with a new id and updates the count. It returns the index in the list of the added value (for example, if the first value of the list is added, it'll return 0 (the first index in the list))
 func (fs FixationStore) SetEntryIndex(ctx sdk.Context, index string) {
 	// get the index store with the fixation key
-	store := prefix.NewStore(ctx.KVStore(fs.GetStoreKey()), types.KeyPrefix(types.EntryIndexKey+fs.prefix))
+	store := prefix.NewStore(ctx.KVStore(fs.GetStoreKey()), types.KeyPrefix(fs.createEntryIndexStoreKey()))
 
 	// convert the index value to a byte array
 	appendedValue := []byte(index)
@@ -31,16 +31,16 @@ func (fs FixationStore) SetEntryIndex(ctx sdk.Context, index string) {
 // RemoveEntryIndex removes an EntryIndex from the store
 func (fs FixationStore) removeEntryIndex(ctx sdk.Context, index string) {
 	// get the index store with the fixation key
-	store := prefix.NewStore(ctx.KVStore(fs.GetStoreKey()), types.KeyPrefix(types.EntryIndexKey+fs.prefix))
+	store := prefix.NewStore(ctx.KVStore(fs.GetStoreKey()), types.KeyPrefix(fs.createEntryIndexStoreKey()))
 
 	// remove the index from the store
-	store.Delete(types.KeyPrefix(types.EntryIndexKey + fs.prefix + index))
+	store.Delete(types.KeyPrefix(fs.createEntryIndexKey(index)))
 }
 
 // GetAllEntryIndex returns all EntryIndex
 func (fs FixationStore) GetAllEntryIndices(ctx sdk.Context) []string {
 	// get the index store with the fixation key and init an iterator
-	store := prefix.NewStore(ctx.KVStore(fs.GetStoreKey()), types.KeyPrefix(types.EntryIndexKey+fs.prefix))
+	store := prefix.NewStore(ctx.KVStore(fs.GetStoreKey()), types.KeyPrefix(fs.createEntryIndexStoreKey()))
 	iterator := sdk.KVStorePrefixIterator(store, []byte{})
 	defer iterator.Close()
 
@@ -53,3 +53,11 @@ func (fs FixationStore) GetAllEntryIndices(ctx sdk.Context) []string {
 
 	return indexList
 }
+
+func (fs FixationStore) createEntryIndexStoreKey() string {
+	return types.EntryIndexKey + fs.prefix
+}
+
+func (fs FixationStore) createEntryIndexKey(index string) string {
+	return types.EntryIndexKey + fs.prefix + index
+}

common/fixation_entry_test.go

@@ -67,21 +67,32 @@ func TestFixationEntryAdditionAndRemoval(t *testing.T) {
 	// make sure that one entry's data is the same data that was used to create it
 	require.True(t, dummyCoin.IsEqual(dummyObj))
 
+	// advance the block height to +STALE_ENTRY_TIME (the entry should not be deleted yet)
+	ctx = ctx.WithBlockHeight(types.STALE_ENTRY_TIME + int64(blockToAddEntry))
+	dummyObj2 := sdk.Coin{Denom: "utest", Amount: sdk.OneInt()}
+	blockToAddEntryBeforeStale := uint64(ctx.BlockHeight())
+	err = vs.AppendEntry(ctx, dummyIndex, blockToAddEntryBeforeStale, &dummyObj2)
+	require.Nil(t, err)
+
+	// make sure the old entry was not deleted (check block)
+	err = vs.GetEntry(ctx, dummyIndex, blockToAddEntry, &dummyCoin, types.DO_NOTHING)
+	require.Nil(t, err)
+
 	// remove the entry by advancing over the STALE_ENTRY_TIME and appending a new one (append triggers the removal func)
 	ctx = ctx.WithBlockHeight(types.STALE_ENTRY_TIME + int64(blockToAddEntry) + 1)
-	dummyObj2 := sdk.Coin{Denom: "utest", Amount: sdk.OneInt()}
+	dummyObj3 := sdk.Coin{Denom: "utest", Amount: sdk.OneInt()}
 	blockToAddEntryAfterStale := uint64(ctx.BlockHeight())
-	err = vs.AppendEntry(ctx, dummyIndex, blockToAddEntryAfterStale, &dummyObj2)
+	err = vs.AppendEntry(ctx, dummyIndex, blockToAddEntryAfterStale, &dummyObj3)
 	require.Nil(t, err)
 
 	// make sure the old entry was deleted (check block)
 	err = vs.GetEntry(ctx, dummyIndex, blockToAddEntry, &dummyCoin, types.DO_NOTHING)
 	require.NotNil(t, err)
 
-	// get the latest version and make sure it's equal to dummyObj2
+	// get the latest version and make sure it's equal to dummyObj3
 	err = vs.GetEntry(ctx, dummyIndex, blockToAddEntryAfterStale, &dummyCoin, types.DO_NOTHING)
 	require.Nil(t, err)
-	require.True(t, dummyCoin.IsEqual(dummyObj2))
+	require.True(t, dummyCoin.IsEqual(dummyObj3))
 
 	// make sure dummy index is still in the entry index list
 	indexList = vs.GetAllEntryIndices(ctx)
@@ -163,17 +174,19 @@ func TestDifferentFixationKeys(t *testing.T) {
 	dummyIndex := "index"
 	dummyObj := sdk.Coin{Denom: "utest", Amount: sdk.ZeroInt()}
 	dummyObj2 := sdk.Coin{Denom: "utest", Amount: sdk.OneInt()}
-	blockToAddEntry := uint64(10)
+
 	// init FixationStore + context
 	vs, ctx := initCtxAndFixationStore(t)
-	vs2 := vs.SetPrefix("fix2")
+	vs2 := vs.WithPrefix("fix2")
 
 	// add the first dummy entry
-	err := vs.AppendEntry(ctx, dummyIndex, blockToAddEntry, &dummyObj)
+	blockToAddFirstEntry := uint64(10)
+	err := vs.AppendEntry(ctx, dummyIndex, blockToAddFirstEntry, &dummyObj)
 	require.Nil(t, err)
 
 	// add the second dummy entry
-	err = vs2.AppendEntry(ctx, dummyIndex, blockToAddEntry, &dummyObj2)
+	blockToAddSecondEntry := uint64(10)
+	err = vs2.AppendEntry(ctx, dummyIndex, blockToAddSecondEntry, &dummyObj2)
 	require.Nil(t, err)
 
 	// get all indices with original fixation key and dummyIndex. make sure there is one entry
@@ -182,7 +195,7 @@ func TestDifferentFixationKeys(t *testing.T) {
 
 	// verify the data matches the entry from original fixation key storage
 	var dummyCoin sdk.Coin
-	err = vs.GetEntry(ctx, dummyIndex, blockToAddEntry, &dummyCoin, types.DO_NOTHING)
+	err = vs.GetEntry(ctx, dummyIndex, blockToAddFirstEntry, &dummyCoin, types.DO_NOTHING)
 	require.Nil(t, err)
 	require.True(t, dummyCoin.IsEqual(dummyObj))
 	require.False(t, dummyCoin.Equal(dummyObj2))
@@ -192,10 +205,23 @@ func TestDifferentFixationKeys(t *testing.T) {
 	require.Equal(t, 1, len(indexList))
 
 	// verify the data matches the entry from original fixation key storage
-	err = vs2.GetEntry(ctx, dummyIndex, blockToAddEntry, &dummyCoin, types.DO_NOTHING)
+	err = vs2.GetEntry(ctx, dummyIndex, blockToAddSecondEntry, &dummyCoin, types.DO_NOTHING)
 	require.Nil(t, err)
 	require.True(t, dummyCoin.IsEqual(dummyObj2))
 	require.False(t, dummyCoin.Equal(dummyObj))
+
+	// advance enough blocks so the entry with the regular fixation key will be deleted with a new append, but the second entry (with "fix2" key) won't be deleted
+	ctx = ctx.WithBlockHeight(int64(blockToAddFirstEntry) + types.STALE_ENTRY_TIME + 1)
+
+	// append object to remove the first entry
+	dummyObj3 := sdk.Coin{Denom: "utest", Amount: sdk.OneInt().Add(sdk.OneInt())}
+	blockToAddEntryForRemoval := uint64(ctx.BlockHeight())
+	err = vs.AppendEntry(ctx, dummyIndex, blockToAddEntryForRemoval, &dummyObj3)
+	require.Nil(t, err)
+
+	// make sure the old entry was deleted (check block)
+	err = vs.GetEntry(ctx, dummyIndex, blockToAddEntryForRemoval, &dummyCoin, types.DO_NOTHING)
+	require.True(t, dummyCoin.IsEqual(dummyObj3))
 }
 
 // Test that the appended entries are sorted (first element is oldest)

common/types/constants.go

@@ -1,7 +1,8 @@
 package types
 
 const (
-	STALE_ENTRY_TIME int64 = 1440 // 1440 blocks (equivalent to 24 hours when block_time = 1min)
+	STALE_ENTRY_TIME int64  = 1440 // 1440 blocks (equivalent to 24 hours when block_time = 1min)
+	MODULE_NAME      string = "common"
 )
 
 // References action enum

common/types/errors.go

@@ -0,0 +1,12 @@
+package types
+
+// DONTCOVER
+
+import (
+	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
+)
+
+// x/pairing module sentinel errors
+var (
+	ErrEntryNotFound = sdkerrors.Register(MODULE_NAME, 1, "entry not found")
+)