From d6dd005be88b94d0f559356000fbf9efab551163 Mon Sep 17 00:00:00 2001 From: Oren Laadan Date: Wed, 12 Apr 2023 22:43:20 +0800 Subject: [PATCH] Restore "CNS-368: fixation: improve API for GetEntry,FindEntry,PutEntry"" This reverts commit a62abec1142365afc46f47a3fcff6b5f4d9938ca. (It restores commit f22c5c70c81af559f70e4debbde7c5f0f159a9a4). --- common/fixation_entry.go | 69 +++++++------------ common/fixation_entry_test.go | 22 ++---- common/types/errors.go | 3 +- x/plans/keeper/plan.go | 22 +++--- x/plans/keeper/plan_test.go | 9 +-- x/projects/keeper/creation.go | 22 +++--- .../keeper/msg_server_set_project_policy.go | 6 +- x/projects/keeper/project.go | 18 ++--- x/subscription/types/expected_keepers.go | 2 +- 9 files changed, 61 insertions(+), 112 deletions(-) diff --git a/common/fixation_entry.go b/common/fixation_entry.go index 3193c7a971..34666b5b0f 100644 --- a/common/fixation_entry.go +++ b/common/fixation_entry.go @@ -117,7 +117,12 @@ 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} @@ -262,82 +267,61 @@ func (fs *FixationStore) getUnmarshaledEntryForBlock(ctx sdk.Context, safeIndex return types.Entry{}, false } -// 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) { +// 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 { safeIndex, err := sanitizeIndex(index) if err != nil { details := map[string]string{"index": index} - return utils.LavaError(ctx, ctx.Logger(), "FindEntry_invalid_index", details, "invalid non-ascii entry"), false + utils.LavaError(ctx, ctx.Logger(), "FindEntry_invalid_index", details, "invalid non-ascii entry") + return false } - // get the unmarshaled entry for block entry, found := fs.getUnmarshaledEntryForBlock(ctx, safeIndex, block) if !found { - return types.ErrEntryNotFound, false + return false } - // 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 + fs.cdc.MustUnmarshal(entry.GetData(), entryData) + return true } -// 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) { +// GetEntry returns the latest entry by index and increments the refcount +func (fs *FixationStore) GetEntry(ctx sdk.Context, index string, entryData codec.ProtoMarshaler) bool { safeIndex, err := sanitizeIndex(index) if err != nil { details := map[string]string{"index": index} - return utils.LavaError(ctx, ctx.Logger(), "GetEntry_invalid_index", details, "invalid non-ascii entry"), false + utils.LavaError(ctx, ctx.Logger(), "GetEntry_invalid_index", details, "invalid non-ascii entry") + return false } block := uint64(ctx.BlockHeight()) - // get the unmarshaled entry for block entry, found := fs.getUnmarshaledEntryForBlock(ctx, safeIndex, block) if !found { - return types.ErrEntryNotFound, false + return 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 - } + fs.cdc.MustUnmarshal(entry.GetData(), entryData) entry.Refcount += 1 fs.setEntry(ctx, entry) - - return nil, true + return true } -// 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) { +// PutEntry finds the entry by index and block and decrements the refcount +func (fs *FixationStore) PutEntry(ctx sdk.Context, index string, block uint64) { safeIndex, err := sanitizeIndex(index) if err != nil { - details := map[string]string{"index": index} - return utils.LavaError(ctx, ctx.Logger(), "PutEntry_invalid_index", details, "invalid non-ascii entry"), false + panic("PutEntry with non-ascii index: " + index) } - // get the unmarshaled entry for block + entry, found := fs.getUnmarshaledEntryForBlock(ctx, safeIndex, block) if !found { - 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 + panic("PutEntry with unknown index: " + index) } if entry.Refcount == 0 { - 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 + panic("PutEntry with refcount zero index: " + index) } entry.Refcount -= 1 @@ -348,7 +332,6 @@ func (fs *FixationStore) PutEntry(ctx sdk.Context, index string, block uint64, e } fs.setEntry(ctx, entry) - return nil, true } // removeEntry removes an entry from the store diff --git a/common/fixation_entry_test.go b/common/fixation_entry_test.go index 612f6ea7e0..153a9c9a47 100644 --- a/common/fixation_entry_test.go +++ b/common/fixation_entry_test.go @@ -100,35 +100,23 @@ func testWithTemplate(t *testing.T, playbook []template, countObj int, countVS i require.NotNil(t, err, what) } case "find": - err, found := vs[play.store].FindEntry(ctx, play.index, block, &dummy) + 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": - err, found := vs[play.store].GetEntry(ctx, play.index, &dummy) + 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": - 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) - } + vs[play.store].PutEntry(ctx, play.index, block) case "block": ctx = ctx.WithBlockHeight(ctx.BlockHeight() + play.count) case "getall": @@ -269,9 +257,7 @@ 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, coin: 0 }, - // double put triggers error - { op: "put", name: "refcount entry #1", count: block0, fail: true }, + { op: "put", name: "refcount entry #1", count: block0 }, // 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 }, diff --git a/common/types/errors.go b/common/types/errors.go index 68d50fb667..639b7732b7 100644 --- a/common/types/errors.go +++ b/common/types/errors.go @@ -8,6 +8,5 @@ import ( // x/pairing module sentinel errors var ( - ErrEntryNotFound = sdkerrors.Register(MODULE_NAME, 1, "entry not found") - ErrInvalidIndex = sdkerrors.Register(MODULE_NAME, 2, "entry index is invalid") + ErrInvalidIndex = sdkerrors.Register(MODULE_NAME, 1, "entry index is invalid") ) diff --git a/x/plans/keeper/plan.go b/x/plans/keeper/plan.go index 47f440aafc..9bc7c8afee 100644 --- a/x/plans/keeper/plan.go +++ b/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. The function returns if the added plan is a first version plans +// AddPlan adds a new plan to the KVStore 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,37 +16,33 @@ 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, "could not add new plan fixated entry to storage") + return utils.LavaError(ctx, k.Logger(ctx), "AddPlan_add_fixated_entry_failed", details, err.Error()) } return nil } -// GetPlan gets a plan from the KVStore. It increases the plan's refCount by 1 +// GetPlan gets the latest plan from the KVStore and increments its refcount func (k Keeper) GetPlan(ctx sdk.Context, index string) (val types.Plan, found bool) { var plan types.Plan - err, _ := k.plansFs.GetEntry(ctx, index, &plan) - if err != nil { + if found := k.plansFs.GetEntry(ctx, index, &plan); !found { return types.Plan{}, false } return plan, true } -// FindPlan gets a plan from the KVStore. It does nothing to the plan's refCount +// FindPlan gets a plan with nearest-smaller block (without changing its refcount) func (k Keeper) FindPlan(ctx sdk.Context, index string, block uint64) (val types.Plan, found bool) { var plan types.Plan - err, _ := k.plansFs.FindEntry(ctx, index, block, &plan) - if err != nil { + if found := k.plansFs.FindEntry(ctx, index, block, &plan); !found { return types.Plan{}, false } return plan, true } -// 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 +// 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) } // GetAllPlanIndices gets from the KVStore all the plans' indices diff --git a/x/plans/keeper/plan_test.go b/x/plans/keeper/plan_test.go index fe71d7a772..e67f18ee98 100644 --- a/x/plans/keeper/plan_test.go +++ b/x/plans/keeper/plan_test.go @@ -16,9 +16,6 @@ import ( "github.com/stretchr/testify/require" ) -// Prevent strconv unused error -var _ = strconv.IntSize - type testStruct struct { ctx context.Context keepers *testkeeper.Keepers @@ -336,10 +333,8 @@ func TestPlansDeletion(t *testing.T) { require.Equal(t, testPlans[1], secondPlanFromStore) // decrease the old plans' refCount - 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) + ts.keepers.Plans.PutPlan(sdk.UnwrapSDKContext(ts.ctx), testPlans[0].GetIndex(), firstPlanBlockHeight) + ts.keepers.Plans.PutPlan(sdk.UnwrapSDKContext(ts.ctx), testPlans[1].GetIndex(), secondPlanBlockHeight) // advance an epoch and create an newer plan to add (and trigger the plan deletion) ts.advanceEpochUntilStale() diff --git a/x/projects/keeper/creation.go b/x/projects/keeper/creation.go index 44849f8899..5cc71ef021 100644 --- a/x/projects/keeper/creation.go +++ b/x/projects/keeper/creation.go @@ -19,9 +19,8 @@ func (k Keeper) CreateProject(ctx sdk.Context, subscriptionAddress string, proje var emptyProject types.Project blockHeight := uint64(ctx.BlockHeight()) - _, found := k.projectsFS.FindEntry(ctx, project.Index, blockHeight, &emptyProject) - // the project with the same name already exists if no error has returned - if found { + if found := k.projectsFS.FindEntry(ctx, project.Index, blockHeight, &emptyProject); found { + // the project with the same name already exists if no error has returned 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") } @@ -30,7 +29,11 @@ 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 { @@ -44,9 +47,8 @@ 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 - _, 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 { + 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 developerData.ProjectID = projectIndex developerData.Vrfpk = vrfpk err := k.developerKeysFS.AppendEntry(ctx, developerKey, blockHeight, &developerData) @@ -72,8 +74,7 @@ func (k Keeper) SnapshotSubscriptionProjects(ctx sdk.Context, subscriptionAddr s // 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 - err, found := k.projectsFS.FindEntry(ctx, projectID, uint64(ctx.BlockHeight()), &project) - if err != nil || !found { + if found := k.projectsFS.FindEntry(ctx, projectID, uint64(ctx.BlockHeight()), &project); !found { return utils.LavaError(ctx, ctx.Logger(), "SnapshotProject_project_not_found", map[string]string{"projectID": projectID}, "snapshot of project failed, project does not exist") } @@ -84,8 +85,7 @@ func (k Keeper) snapshotProject(ctx sdk.Context, projectID string) error { func (k Keeper) DeleteProject(ctx sdk.Context, projectID string) error { var project types.Project - err, found := k.projectsFS.FindEntry(ctx, projectID, uint64(ctx.BlockHeight()), &project) - if err != nil || !found { + if found := k.projectsFS.FindEntry(ctx, projectID, uint64(ctx.BlockHeight()), &project); !found { return utils.LavaError(ctx, ctx.Logger(), "DeleteProject_project_not_found", map[string]string{"projectID": projectID}, "project to delete was not found") } diff --git a/x/projects/keeper/msg_server_set_project_policy.go b/x/projects/keeper/msg_server_set_project_policy.go index 83a31d0c38..63c29f9d7a 100644 --- a/x/projects/keeper/msg_server_set_project_policy.go +++ b/x/projects/keeper/msg_server_set_project_policy.go @@ -14,8 +14,7 @@ func (k msgServer) SetProjectPolicy(goCtx context.Context, msg *types.MsgSetProj adminKey := msg.Creator var project types.Project - err, found := k.projectsFS.FindEntry(ctx, projectID, uint64(ctx.BlockHeight()), &project) - if err != nil || !found { + if found := k.projectsFS.FindEntry(ctx, projectID, uint64(ctx.BlockHeight()), &project); !found { return nil, utils.LavaError(ctx, ctx.Logger(), "SetProjectPolicy_project_not_found", map[string]string{"project": projectID}, "project id not found") } @@ -31,8 +30,7 @@ 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 } diff --git a/x/projects/keeper/project.go b/x/projects/keeper/project.go index 5c5be7967d..77560d352a 100644 --- a/x/projects/keeper/project.go +++ b/x/projects/keeper/project.go @@ -12,8 +12,7 @@ import ( func (k Keeper) GetProjectForBlock(ctx sdk.Context, projectID string, blockHeight uint64) (types.Project, error) { var project types.Project - err, found := k.projectsFS.FindEntry(ctx, projectID, blockHeight, &project) - if err != nil || !found { + if found := k.projectsFS.FindEntry(ctx, projectID, blockHeight, &project); !found { return project, utils.LavaError(ctx, ctx.Logger(), "GetProjectForBlock_not_found", map[string]string{"project": projectID, "blockHeight": strconv.FormatUint(blockHeight, 10)}, "project not found") } @@ -22,8 +21,7 @@ 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 - err, found := k.developerKeysFS.FindEntry(ctx, developerKey, blockHeight, &projectDeveloperData) - if err != nil || !found { + if found := k.developerKeysFS.FindEntry(ctx, developerKey, blockHeight, &projectDeveloperData); !found { return types.ProtoDeveloperData{}, fmt.Errorf("GetProjectIDForDeveloper_invalid_key, the requesting key is not registered to a project, developer: %s", developerKey) } return projectDeveloperData, nil @@ -36,12 +34,7 @@ func (k Keeper) GetProjectForDeveloper(ctx sdk.Context, developerKey string, blo return project, "", err } - err, found := k.projectsFS.FindEntry(ctx, projectDeveloperData.ProjectID, blockHeight, &project) - if err != nil { - return project, "", err - } - - if !found { + if found := k.projectsFS.FindEntry(ctx, projectDeveloperData.ProjectID, blockHeight, &project); !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") } @@ -50,8 +43,7 @@ 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 - err, found := k.projectsFS.FindEntry(ctx, projectID, uint64(ctx.BlockHeight()), &project) - if err != nil || !found { + if found := k.projectsFS.FindEntry(ctx, projectID, uint64(ctx.BlockHeight()), &project); !found { return utils.LavaError(ctx, ctx.Logger(), "AddProjectKeys_project_not_found", map[string]string{"project": projectID}, "project id not found") } @@ -62,7 +54,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 } diff --git a/x/subscription/types/expected_keepers.go b/x/subscription/types/expected_keepers.go index 19511f947f..bda71a55d7 100644 --- a/x/subscription/types/expected_keepers.go +++ b/x/subscription/types/expected_keepers.go @@ -34,6 +34,6 @@ type ProjectsKeeper interface { type PlansKeeper interface { GetPlan(ctx sdk.Context, index string) (planstypes.Plan, bool) - PutPlan(ctx sdk.Context, index string, block uint64) bool + PutPlan(ctx sdk.Context, index string, block uint64) // Methods imported from planskeeper should be defined here }