Revert "CNS-368: fixation: improve API for GetEntry,FindEntry,PutEntry"

common/fixation_entry.go

@@ -117,12 +117,7 @@ func (fs *FixationStore) setEntry(ctx sdk.Context, entry types.Entry) {
 }
 
 // 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 {
 	safeIndex, err := sanitizeIndex(index)
 	if err != nil {
 		details := map[string]string{"index": index}
@@ -267,61 +262,82 @@ func (fs *FixationStore) getUnmarshaledEntryForBlock(ctx sdk.Context, safeIndex
 	return types.Entry{}, false
 }
 
-// FindEntry returns the entry by index and block without changing the refcount
-func (fs *FixationStore) FindEntry(ctx sdk.Context, index string, block uint64, entryData codec.ProtoMarshaler) bool {
+// FindEntry returns the entry with index and block without changing the refcount
+func (fs *FixationStore) FindEntry(ctx sdk.Context, index string, block uint64, entryData codec.ProtoMarshaler) (error, bool) {
 	safeIndex, err := sanitizeIndex(index)
 	if err != nil {
 		details := map[string]string{"index": index}
-		utils.LavaError(ctx, ctx.Logger(), "FindEntry_invalid_index", details, "invalid non-ascii entry")
-		return false
+		return utils.LavaError(ctx, ctx.Logger(), "FindEntry_invalid_index", details, "invalid non-ascii entry"), false
 	}
 
+	// get the unmarshaled entry for block
 	entry, found := fs.getUnmarshaledEntryForBlock(ctx, safeIndex, block)
 	if !found {
-		return false
+		return types.ErrEntryNotFound, false
 	}
 
-	fs.cdc.MustUnmarshal(entry.GetData(), entryData)
-	return true
+	// unmarshal the entry's data
+	err = fs.cdc.Unmarshal(entry.GetData(), entryData)
+	if err != nil {
+		return utils.LavaError(ctx, ctx.Logger(), "FindEntry_cant_unmarshal", map[string]string{"err": err.Error()}, "can't unmarshal entry data"), false
+	}
+
+	return nil, true
 }
 
-// GetEntry returns the latest entry by index and increments the refcount
-func (fs *FixationStore) GetEntry(ctx sdk.Context, index string, entryData codec.ProtoMarshaler) bool {
+// GetEntry returns the latest entry with index and increments the refcount
+func (fs *FixationStore) GetEntry(ctx sdk.Context, index string, entryData codec.ProtoMarshaler) (error, bool) {
 	safeIndex, err := sanitizeIndex(index)
 	if err != nil {
 		details := map[string]string{"index": index}
-		utils.LavaError(ctx, ctx.Logger(), "GetEntry_invalid_index", details, "invalid non-ascii entry")
-		return false
+		return utils.LavaError(ctx, ctx.Logger(), "GetEntry_invalid_index", details, "invalid non-ascii entry"), false
 	}
 
 	block := uint64(ctx.BlockHeight())
 
+	// get the unmarshaled entry for block
 	entry, found := fs.getUnmarshaledEntryForBlock(ctx, safeIndex, block)
 	if !found {
-		return false
+		return types.ErrEntryNotFound, false
 	}
 
-	fs.cdc.MustUnmarshal(entry.GetData(), entryData)
+	// unmarshal the entry's data
+	err = fs.cdc.Unmarshal(entry.GetData(), entryData)
+	if err != nil {
+		return utils.LavaError(ctx, ctx.Logger(), "GetEntry_cant_unmarshal", map[string]string{"err": err.Error()}, "can't unmarshal entry data"), false
+	}
 
 	entry.Refcount += 1
 	fs.setEntry(ctx, entry)
-	return true
+
+	return nil, true
 }
 
-// PutEntry finds the entry by index and block and decrements the refcount
-func (fs *FixationStore) PutEntry(ctx sdk.Context, index string, block uint64) {
+// get entry with index and block with ref decrease
+func (fs *FixationStore) PutEntry(ctx sdk.Context, index string, block uint64, entryData codec.ProtoMarshaler) (error, bool) {
 	safeIndex, err := sanitizeIndex(index)
 	if err != nil {
-		panic("PutEntry with non-ascii index: " + index)
+		details := map[string]string{"index": index}
+		return utils.LavaError(ctx, ctx.Logger(), "PutEntry_invalid_index", details, "invalid non-ascii entry"), false
 	}
-
+	// get the unmarshaled entry for block
 	entry, found := fs.getUnmarshaledEntryForBlock(ctx, safeIndex, block)
 	if !found {
-		panic("PutEntry with unknown index: " + index)
+		return types.ErrEntryNotFound, false
+	}
+
+	// unmarshal the entry's data
+	err = fs.cdc.Unmarshal(entry.GetData(), entryData)
+	if err != nil {
+		return utils.LavaError(ctx, ctx.Logger(), "GetEntry_cant_unmarshal", map[string]string{"err": err.Error()}, "can't unmarshal entry data"), false
 	}
 
 	if entry.Refcount == 0 {
-		panic("PutEntry with refcount zero index: " + index)
+		details := map[string]string{
+			"index":    index,
+			"refcount": strconv.FormatUint(entry.Refcount, 10),
+		}
+		return utils.LavaError(ctx, ctx.Logger(), "PutEntry_zero_count", details, "refcount already reached zero"), false
 	}
 
 	entry.Refcount -= 1
@@ -332,6 +348,7 @@ func (fs *FixationStore) PutEntry(ctx sdk.Context, index string, block uint64) {
 	}
 
 	fs.setEntry(ctx, entry)
+	return nil, true
 }
 
 // removeEntry removes an entry from the store

common/fixation_entry_test.go

@@ -100,23 +100,35 @@ func testWithTemplate(t *testing.T, playbook []template, countObj int, countVS i
 				require.NotNil(t, err, what)
 			}
 		case "find":
-			found := vs[play.store].FindEntry(ctx, play.index, block, &dummy)
+			err, found := vs[play.store].FindEntry(ctx, play.index, block, &dummy)
 			if !play.fail {
+				require.Nil(t, err, what)
 				require.True(t, found, what)
 				require.Equal(t, dummy, coins[play.coin], what)
 			} else {
+				require.NotNil(t, err, what)
 				require.False(t, found, what)
 			}
 		case "get":
-			found := vs[play.store].GetEntry(ctx, play.index, &dummy)
+			err, found := vs[play.store].GetEntry(ctx, play.index, &dummy)
 			if !play.fail {
+				require.Nil(t, err, what)
 				require.True(t, found, what)
 				require.Equal(t, dummy, coins[play.coin], what)
 			} else {
+				require.NotNil(t, err, what)
 				require.False(t, found, what)
 			}
 		case "put":
-			vs[play.store].PutEntry(ctx, play.index, block)
+			err, found := vs[play.store].PutEntry(ctx, play.index, block, &dummy)
+			if !play.fail {
+				require.Nil(t, err, what)
+				require.True(t, found, what)
+				require.Equal(t, dummy, coins[play.coin], what)
+			} else {
+				require.NotNil(t, err, what)
+				require.False(t, found, what)
+			}
 		case "block":
 			ctx = ctx.WithBlockHeight(ctx.BlockHeight() + play.count)
 		case "getall":
@@ -254,7 +266,9 @@ func TestGetAndPutEntry(t *testing.T) {
 		{ op: "append", name: "entry #2", count: block1, coin: 1 },
 		// entry #1 should not be deleted because it has refcount != zero);
 		{ op: "find", name: "entry #1", count: block0, coin: 0 },
-		{ op: "put", name: "refcount entry #1", count: block0 },
+		{ op: "put", name: "refcount entry #1", count: block0, coin: 0 },
+		// double put triggers error
+		{ op: "put", name: "refcount entry #1", count: block0, fail: true },
 		// entry #1 not deleted because not enough time with refcount = zero
 		{ op: "find", name: "entry #1", count: block0, coin: 0 },
 		{ op: "append", name: "entry #3", count: block2, coin: 2 },

common/types/errors.go

@@ -8,5 +8,6 @@ import (
 
 // x/pairing module sentinel errors
 var (
-	ErrInvalidIndex = sdkerrors.Register(MODULE_NAME, 1, "entry index is invalid")
+	ErrEntryNotFound = sdkerrors.Register(MODULE_NAME, 1, "entry not found")
+	ErrInvalidIndex  = sdkerrors.Register(MODULE_NAME, 2, "entry index is invalid")
 )

x/plans/keeper/plan.go

@@ -6,7 +6,7 @@ import (
 	"github.com/lavanet/lava/x/plans/types"
 )
 
-// AddPlan adds a new plan to the KVStore
+// AddPlan adds a new plan to the KVStore. The function returns if the added plan is a first version plans
 func (k Keeper) AddPlan(ctx sdk.Context, planToAdd types.Plan) error {
 	// overwrite the planToAdd's block field with the current block height
 	planToAdd.Block = uint64(ctx.BlockHeight())
@@ -16,33 +16,37 @@ func (k Keeper) AddPlan(ctx sdk.Context, planToAdd types.Plan) error {
 	err := k.plansFs.AppendEntry(ctx, planToAdd.GetIndex(), planToAdd.Block, &planToAdd)
 	if err != nil {
 		details := map[string]string{"planToAdd": planToAdd.String()}
-		return utils.LavaError(ctx, k.Logger(ctx), "AddPlan_add_fixated_entry_failed", details, err.Error())
+		return utils.LavaError(ctx, k.Logger(ctx), "AddPlan_add_fixated_entry_failed", details, "could not add new plan fixated entry to storage")
 	}
 
 	return nil
 }
 
-// GetPlan gets the latest plan from the KVStore and increments its refcount
+// GetPlan gets a plan from the KVStore. It increases the plan's refCount by 1
 func (k Keeper) GetPlan(ctx sdk.Context, index string) (val types.Plan, found bool) {
 	var plan types.Plan
-	if found := k.plansFs.GetEntry(ctx, index, &plan); !found {
+	err, _ := k.plansFs.GetEntry(ctx, index, &plan)
+	if err != nil {
 		return types.Plan{}, false
 	}
 	return plan, true
 }
 
-// FindPlan gets a plan with nearest-smaller block (without changing its refcount)
+// FindPlan gets a plan from the KVStore. It does nothing to the plan's refCount
 func (k Keeper) FindPlan(ctx sdk.Context, index string, block uint64) (val types.Plan, found bool) {
 	var plan types.Plan
-	if found := k.plansFs.FindEntry(ctx, index, block, &plan); !found {
+	err, _ := k.plansFs.FindEntry(ctx, index, block, &plan)
+	if err != nil {
 		return types.Plan{}, false
 	}
 	return plan, true
 }
 
-// PutPlan finds a plan with nearest-smaller block and decrements its refcount
-func (k Keeper) PutPlan(ctx sdk.Context, index string, block uint64) {
-	k.plansFs.PutEntry(ctx, index, block)
+// PutPlan gets a plan from the KVStore. It decreases the plan's refCount by 1
+func (k Keeper) PutPlan(ctx sdk.Context, index string, block uint64) bool {
+	var plan types.Plan
+	_, found := k.plansFs.PutEntry(ctx, index, block, &plan)
+	return found
 }
 
 // GetAllPlanIndices gets from the KVStore all the plans' indices

x/plans/keeper/plan_test.go

@@ -16,6 +16,9 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+// Prevent strconv unused error
+var _ = strconv.IntSize
+
 type testStruct struct {
 	ctx     context.Context
 	keepers *testkeeper.Keepers
@@ -333,8 +336,10 @@ func TestPlansDeletion(t *testing.T) {
 	require.Equal(t, testPlans[1], secondPlanFromStore)
 
 	// decrease the old plans' refCount
-	ts.keepers.Plans.PutPlan(sdk.UnwrapSDKContext(ts.ctx), testPlans[0].GetIndex(), firstPlanBlockHeight)
-	ts.keepers.Plans.PutPlan(sdk.UnwrapSDKContext(ts.ctx), testPlans[1].GetIndex(), secondPlanBlockHeight)
+	found = ts.keepers.Plans.PutPlan(sdk.UnwrapSDKContext(ts.ctx), testPlans[0].GetIndex(), firstPlanBlockHeight)
+	require.True(t, found)
+	found = ts.keepers.Plans.PutPlan(sdk.UnwrapSDKContext(ts.ctx), testPlans[1].GetIndex(), secondPlanBlockHeight)
+	require.True(t, found)
 
 	// advance an epoch and create an newer plan to add (and trigger the plan deletion)
 	ts.advanceEpochUntilStale()

x/projects/keeper/creation.go

@@ -19,8 +19,9 @@ func (k Keeper) CreateProject(ctx sdk.Context, subscriptionAddress string, proje
 	var emptyProject types.Project
 
 	blockHeight := uint64(ctx.BlockHeight())
-	if found := k.projectsFS.FindEntry(ctx, project.Index, blockHeight, &emptyProject); found {
-		// the project with the same name already exists if no error has returned
+	_, found := k.projectsFS.FindEntry(ctx, project.Index, blockHeight, &emptyProject)
+	// the project with the same name already exists if no error has returned
+	if found {
 		return utils.LavaError(ctx, ctx.Logger(), "CreateEmptyProject_already_exist", map[string]string{"subscription": subscriptionAddress}, "project already exist for the current subscription with the same name")
 	}
 
@@ -29,11 +30,7 @@ func (k Keeper) CreateProject(ctx sdk.Context, subscriptionAddress string, proje
 	project.Policy.MaxProvidersToPair = providers
 	project.Policy.GeolocationProfile = geolocation
 
-	project.AppendKey(types.ProjectKey{
-		Key:   adminAddress,
-		Types: []types.ProjectKey_KEY_TYPE{types.ProjectKey_ADMIN},
-		Vrfpk: vrfpk,
-	})
+	project.AppendKey(types.ProjectKey{Key: adminAddress, Types: []types.ProjectKey_KEY_TYPE{types.ProjectKey_ADMIN}, Vrfpk: vrfpk})
 
 	err := k.RegisterDeveloperKey(ctx, adminAddress, project.Index, blockHeight, vrfpk)
 	if err != nil {
@@ -46,8 +43,9 @@ func (k Keeper) CreateProject(ctx sdk.Context, subscriptionAddress string, proje
 
 func (k Keeper) RegisterDeveloperKey(ctx sdk.Context, developerKey string, projectIndex string, blockHeight uint64, vrfpk string) error {
 	var developerData types.ProtoDeveloperData
-	if found := k.developerKeysFS.FindEntry(ctx, developerKey, blockHeight, &developerData); !found {
-		// a developer key with this address is not registered, add it to the developer keys list
+	_, found := k.developerKeysFS.FindEntry(ctx, developerKey, blockHeight, &developerData)
+	// a developer key with this address is not registered, add it to the developer keys list
+	if !found {
 		developerData.ProjectID = projectIndex
 		developerData.Vrfpk = vrfpk
 		err := k.developerKeysFS.AppendEntry(ctx, developerKey, blockHeight, &developerData)
@@ -62,7 +60,8 @@ func (k Keeper) RegisterDeveloperKey(ctx sdk.Context, developerKey string, proje
 // snapshot project, create a snapshot of a project and reset the cu
 func (k Keeper) SnapshotProject(ctx sdk.Context, projectID string) error {
 	var project types.Project
-	if found := k.projectsFS.FindEntry(ctx, projectID, uint64(ctx.BlockHeight()), &project); !found {
+	err, found := k.projectsFS.FindEntry(ctx, projectID, uint64(ctx.BlockHeight()), &project)
+	if err != nil || !found {
 		return utils.LavaError(ctx, ctx.Logger(), "SnapshotProject_project_not_found", map[string]string{"projectID": projectID}, "snapshot of project failed, project does not exist")
 	}
 
@@ -73,7 +72,8 @@ func (k Keeper) SnapshotProject(ctx sdk.Context, projectID string) error {
 
 func (k Keeper) DeleteProject(ctx sdk.Context, projectID string) error {
 	var project types.Project
-	if found := k.projectsFS.FindEntry(ctx, projectID, uint64(ctx.BlockHeight()), &project); !found {
+	err, found := k.projectsFS.FindEntry(ctx, projectID, uint64(ctx.BlockHeight()), &project)
+	if err != nil || !found {
 		return utils.LavaError(ctx, ctx.Logger(), "DeleteProject_project_not_found", map[string]string{"projectID": projectID}, "project to delete was not found")
 	}
 

x/projects/keeper/msg_server_set_project_policy.go

@@ -14,7 +14,8 @@ func (k msgServer) SetProjectPolicy(goCtx context.Context, msg *types.MsgSetProj
 	adminKey := msg.Creator
 	var project types.Project
 
-	if found := k.projectsFS.FindEntry(ctx, projectID, uint64(ctx.BlockHeight()), &project); !found {
+	err, found := k.projectsFS.FindEntry(ctx, projectID, uint64(ctx.BlockHeight()), &project)
+	if err != nil || !found {
 		return nil, utils.LavaError(ctx, ctx.Logger(), "SetProjectPolicy_project_not_found", map[string]string{"project": projectID}, "project id not found")
 	}
 
@@ -30,7 +31,8 @@ func (k msgServer) SetProjectPolicy(goCtx context.Context, msg *types.MsgSetProj
 	}
 
 	// TODO this needs to be applied in the next epoch
-	err := k.projectsFS.AppendEntry(ctx, projectID, uint64(ctx.BlockHeight()), &project)
+	err = k.projectsFS.AppendEntry(ctx, projectID, uint64(ctx.BlockHeight()), &project)
+
 	if err != nil {
 		return nil, err
 	}

x/projects/keeper/project.go

@@ -12,7 +12,8 @@ import (
 func (k Keeper) GetProjectForBlock(ctx sdk.Context, projectID string, blockHeight uint64) (types.Project, error) {
 	var project types.Project
 
-	if found := k.projectsFS.FindEntry(ctx, projectID, blockHeight, &project); !found {
+	err, found := k.projectsFS.FindEntry(ctx, projectID, blockHeight, &project)
+	if err != nil || !found {
 		return project, utils.LavaError(ctx, ctx.Logger(), "GetProjectForBlock_not_found", map[string]string{"project": projectID, "blockHeight": strconv.FormatUint(blockHeight, 10)}, "project not found")
 	}
 
@@ -21,7 +22,8 @@ func (k Keeper) GetProjectForBlock(ctx sdk.Context, projectID string, blockHeigh
 
 func (k Keeper) GetProjectDeveloperData(ctx sdk.Context, developerKey string, blockHeight uint64) (types.ProtoDeveloperData, error) {
 	var projectDeveloperData types.ProtoDeveloperData
-	if found := k.developerKeysFS.FindEntry(ctx, developerKey, blockHeight, &projectDeveloperData); !found {
+	err, found := k.developerKeysFS.FindEntry(ctx, developerKey, blockHeight, &projectDeveloperData)
+	if err != nil || !found {
 		return types.ProtoDeveloperData{}, fmt.Errorf("GetProjectIDForDeveloper_invalid_key, the requesting key is not registered to a project, developer: %s", developerKey)
 	}
 	return projectDeveloperData, nil
@@ -34,7 +36,12 @@ func (k Keeper) GetProjectForDeveloper(ctx sdk.Context, developerKey string, blo
 		return project, "", err
 	}
 
-	if found := k.projectsFS.FindEntry(ctx, projectDeveloperData.ProjectID, blockHeight, &project); !found {
+	err, found := k.projectsFS.FindEntry(ctx, projectDeveloperData.ProjectID, blockHeight, &project)
+	if err != nil {
+		return project, "", err
+	}
+
+	if !found {
 		return project, "", utils.LavaError(ctx, ctx.Logger(), "GetProjectForDeveloper_project_not_found", map[string]string{"developer": developerKey, "project": projectDeveloperData.ProjectID}, "the developers project was not found")
 	}
 
@@ -43,7 +50,8 @@ func (k Keeper) GetProjectForDeveloper(ctx sdk.Context, developerKey string, blo
 
 func (k Keeper) AddKeysToProject(ctx sdk.Context, projectID string, adminKey string, projectKeys []types.ProjectKey) error {
 	var project types.Project
-	if found := k.projectsFS.FindEntry(ctx, projectID, uint64(ctx.BlockHeight()), &project); !found {
+	err, found := k.projectsFS.FindEntry(ctx, projectID, uint64(ctx.BlockHeight()), &project)
+	if err != nil || !found {
 		return utils.LavaError(ctx, ctx.Logger(), "AddProjectKeys_project_not_found", map[string]string{"project": projectID}, "project id not found")
 	}
 
@@ -54,7 +62,7 @@ func (k Keeper) AddKeysToProject(ctx sdk.Context, projectID string, adminKey str
 
 	// check that those keys are unique for developers
 	for _, projectKey := range projectKeys {
-		err := k.RegisterDeveloperKey(ctx, projectKey.Key, project.Index, uint64(ctx.BlockHeight()), projectKey.Vrfpk)
+		err = k.RegisterDeveloperKey(ctx, projectKey.Key, project.Index, uint64(ctx.BlockHeight()), projectKey.Vrfpk)
 		if err != nil {
 			return err
 		}

x/subscription/types/expected_keepers.go

@@ -33,6 +33,6 @@ type ProjectsKeeper interface {
 
 type PlansKeeper interface {
 	GetPlan(ctx sdk.Context, index string) (planstypes.Plan, bool)
-	PutPlan(ctx sdk.Context, index string, block uint64)
+	PutPlan(ctx sdk.Context, index string, block uint64) bool
 	// Methods imported from planskeeper should be defined here
 }