feat: remove create object & bucket approval #580

BarryTong65 · 2024-02-28T06:31:12Z

Description

Remove the create object approval & create bucket approval, and during the process of creating objects & buckets, the calls to the SP side are eliminated through the Go-sdk.

For CreateObject, validation for the object's existence has been added on chain, and the corresponding verify signature has been removed.

For CreateBucket, two gRPC queries have been added and used during the creation process in the Go-sdk to replace the previous logic of Pick VGF on the SP side:

QuerySpAvailableGlobalVirtualGroupFamilies
QuerySpOptimalGlobalVirtualGroupFamily

And the limitation on creating buckets under a user has been removed.

Rationale

Before

SP Pick VGF & Create GVG process

Current

Replace the process on the SP side of checking for available VGF & GVG during the create bucket approval. Virtual Group Manager will automatically check the available status of each SP's GVG & VGF and create GVG when the storage is full or the SP does not have any available GVG.

Example

N/A

Changes

Notable changes:

Remove the create object approval & create bucket approval, and during the process of creating objects & buckets, the calls to the SP side are eliminated through the Go-sdk.

Potential Impacts

Go-SDK
Storage Provider

alexgao001 · 2024-03-04T07:00:16Z

x/storage/keeper/keeper.go

@@ -567,6 +559,12 @@ func (k Keeper) CreateObject(
 		return sdkmath.ZeroUint(), err
 	}

+	// check object
+	_, found = k.GetObjectInfo(ctx, bucketName, objectName)


This check is already implemented below.

alexgao001 · 2024-03-04T07:01:20Z

x/storage/keeper/keeper.go

@@ -117,14 +117,6 @@ func (k Keeper) CreateBucket(
 		return sdkmath.ZeroUint(), errors.Wrap(types.ErrNoSuchStorageProvider, "the storage provider is not in service")
 	}

-	// check primary sp approval
-	if opts.PrimarySpApproval.ExpiredHeight < uint64(ctx.BlockHeight()) {


these code cant be deleted directly. Need to modify after new hardfork is introduced.

Why can't be deleted directly. With the new Go-SDK, verification no longer goes through SP to obtain, thus there is no expiredHeight.
In the old Go-SDK, creating a bucket would go through SP to get approval, but this is no longer necessary for ExpiredHeight verification.

fixed with IsUpgraded(upgradetypes.Pawnee)
the correct hard fork version will be updated before release

alexgao001 · 2024-03-04T07:08:12Z

x/virtualgroup/keeper/grpc_query.go

+	}, nil
+}
+
+func (k Keeper) QuerySpOptimalGlobalVirtualGroupFamily(goCtx context.Context, req *types.QuerySpOptimalGlobalVirtualGroupFamilyRequest) (*types.QuerySpOptimalGlobalVirtualGroupFamilyResponse, error) {


Can we make this API more flexible? Might define the strategy as enum.

fixed, an enum was created to select the strategy.

alexgao001 · 2024-03-04T07:14:23Z

x/virtualgroup/keeper/keeper.go

@@ -167,6 +167,7 @@ func (k Keeper) DeleteGVG(ctx sdk.Context, primarySp *sptypes.StorageProvider, g
 		k.paymentKeeper.IsEmptyNetFlow(ctx, sdk.MustAccAddressFromHex(gvgFamily.VirtualPaymentAddress)) &&
 		!ctx.IsUpgraded(upgradetypes.Manchurian) {
 		store.Delete(types.GetGVGFamilyKey(gvg.FamilyId))
+		k.DeleteGVGFamilyWithinSP(ctx, primarySp.Id, gvg.FamilyId)


The above GetGVGFamilyKey deletes this VGF. Logically, the list of VGFs under this SP should also have the corresponding VGF removed. Why is this not necessary?

alexgao001 · 2024-03-04T07:18:03Z

x/virtualgroup/keeper/keeper.go

+			break
+		}
+	}
+	if index != -1 {


this can be put into the for loop, once meet the desired element, do the manipulation immediately.

alexgao001 · 2024-03-04T07:19:00Z

x/virtualgroup/keeper/keeper.go

+	store.Set(types.GetGVGFamilyStatisticsWithinSPKey(vgfStatisticsWithinSP.StorageProviderId), bz)
+}
+
+func (k Keeper) BatchSetGVGFamilyStatisticsWithinSP(ctx sdk.Context, vgfStatisticsWithinSP []*types.GVGFamilyStatisticsWithinSP) {


need to populate data into store as new key is used.

The BatchSetGVGFamilyStatisticsWithinSP calls SetGVGFamilyStatisticsWithinSP to store it in the store, so there should be no need to add additional logic.

update the logic to set all GVGFamilyStatisticsWithinSP when executing the hardfork

alexgao001 · 2024-03-04T07:20:04Z

proto/greenfield/virtualgroup/types.proto

@@ -73,6 +73,13 @@ message GVGStatisticsWithinSP {
  uint32 break_redundancy_reqmt_gvg_count = 4;
 }

+message GVGFamilyStatisticsWithinSP {
+  // storage_provider_id defines the id of the sp which the statistics associated to
+  uint32 storage_provider_id = 1;


the storage_provider_id is not needed as the key is the sp_id already.

x/storage/keeper/keeper.go

alexgao001 · 2024-03-05T07:48:55Z

x/virtualgroup/keeper/keeper.go

@@ -289,6 +294,8 @@ func (k Keeper) SwapAsPrimarySP(ctx sdk.Context, primarySP, successorSP *sptypes

 	srcStat := k.MustGetGVGStatisticsWithinSP(ctx, primarySP.Id)
 	dstStat := k.GetOrCreateGVGStatisticsWithinSP(ctx, successorSP.Id)
+	srcVGFStat := k.MustGetGVGFamilyStatisticsWithinSP(ctx, primarySP.Id)


if this is executed before the hardfork, there is no stats yet.

alexgao001 · 2024-03-05T07:49:51Z

x/virtualgroup/keeper/keeper.go

@@ -358,6 +365,9 @@ func (k Keeper) SwapAsPrimarySP(ctx sdk.Context, primarySP, successorSP *sptypes
 	}
 	k.SetGVGStatisticsWithSP(ctx, srcStat)
 	k.SetGVGStatisticsWithSP(ctx, dstStat)
+	k.DeleteGVGFamilyWithinSP(ctx, srcVGFStat.SpId, family.Id)


if this is executed before the hardfork, there is no stats yet.

alexgao001 · 2024-03-05T07:53:51Z

x/virtualgroup/keeper/keeper.go

+		var gvgFamily types.GlobalVirtualGroupFamily
+		k.cdc.MustUnmarshal(iterator.Value(), &gvgFamily)
+		gvgFamilyStatistics := k.GetOrCreateGVGFamilyStatisticsWithinSP(ctx, gvgFamily.PrimarySpId)
+		if !slices.Contains(gvgFamilyStatistics.GlobalVirtualGroupFamilyIds, gvgFamily.Id) {


should't be any family identical to another

alexgao001 · 2024-03-05T10:59:36Z

proto/greenfield/virtualgroup/common.proto

+enum PickVGFStrategy {
+  option (gogoproto.goproto_enum_prefix) = false;
+
+  StrategyMaximizeFreeStoreSize = 0;


check for the style to define enum. It should be like XX_XX_XX

Updated to Strategy_Maximize_Free_Store_Size

should be all CAP

x/storage/keeper/keeper_object_test.go

alexgao001 · 2024-03-07T10:29:15Z

x/virtualgroup/keeper/keeper.go

@@ -358,6 +369,11 @@ func (k Keeper) SwapAsPrimarySP(ctx sdk.Context, primarySP, successorSP *sptypes
 	}
 	k.SetGVGStatisticsWithSP(ctx, srcStat)
 	k.SetGVGStatisticsWithSP(ctx, dstStat)
+	if ctx.IsUpgraded(upgradetypes.Serengeti) {
+		k.DeleteGVGFamilyWithinSP(ctx, srcVGFStat.SpId, family.Id)


can you rename this, a little confused..

rename to DeleteSpecificGVGFamilyStatisticsFromSP. If it is not appropriate, i will think about another one

alexgao001 · 2024-03-07T10:31:47Z

x/virtualgroup/keeper/keeper.go

+	store.Set(types.GetGVGFamilyStatisticsWithinSPKey(vgfStatisticsWithinSP.SpId), bz)
+}
+
+func (k Keeper) BatchSetGVGFamilyStatisticsWithinSP(ctx sdk.Context, vgfStatisticsWithinSP []*types.GVGFamilyStatisticsWithinSP) {


this is not being called

alexgao001 · 2024-03-07T10:32:59Z

x/virtualgroup/keeper/keeper.go

+		gvgFamilyStatistics := k.GetOrCreateGVGFamilyStatisticsWithinSP(ctx, gvgFamily.PrimarySpId)
+		k.Logger(ctx).Info(fmt.Sprintf("gvgFamilyStatistics1 gvgFamily id %v, sp id: %d", gvgFamilyStatistics.GlobalVirtualGroupFamilyIds, gvgFamilyStatistics.SpId))
+		gvgFamilyStatistics.GlobalVirtualGroupFamilyIds = append(gvgFamilyStatistics.GlobalVirtualGroupFamilyIds, gvgFamily.Id)
+		k.Logger(ctx).Info(fmt.Sprintf("gvgFamilyStatistics2 gvgFamily id %v, sp id: %d", gvgFamilyStatistics.GlobalVirtualGroupFamilyIds, gvgFamilyStatistics.SpId))


can you refine the log

this is only for debug log,deleted

alexgao001 · 2024-03-07T10:35:09Z

x/virtualgroup/types/errors.go

+	ErrSwapInFailed                = errors.Register(ModuleName, 1127, "swap in failed.")
+	ErrSwapInInfoNotExist          = errors.Register(ModuleName, 1128, "swap in info not exist.")
+	ErrGVGStatisticsNotExist       = errors.Register(ModuleName, 1129, "gvg statistics not exist.")
+	ErrGVGFamilyStatisticsNotExist = errors.Register(ModuleName, 1130, "vgf statistics not exist.")


might want the naming consistent like "global virtual group family..." as it is exposed to users..

BarryTong65 added the wip label Feb 28, 2024

BarryTong65 force-pushed the feat-remove-approval branch 2 times, most recently from 7c7277e to 3815ffd Compare February 29, 2024 03:25

feat: remove bucket get approval

ce591d8

BarryTong65 force-pushed the feat-remove-approval branch from 0998071 to ce591d8 Compare March 1, 2024 01:01

BarryTong65 added r4r and removed wip labels Mar 1, 2024

BarryTong65 requested review from unclezoro, keefel, alexgao001 and ruojunm March 1, 2024 01:58

fix: add e2e tests

5a07296

alexgao001 reviewed Mar 4, 2024

View reviewed changes

BarryTong65 added 5 commits March 5, 2024 10:58

fix: add enum strategy into req and fix e2e tests

aaedf3e

fix: add hardfork logic and remove delete vgf index

fbec6fa

fix: add hardfork logic and remove delete vgf index

c070716

fix: add hardfork logic and remove delete vgf index

4864fcd

fix: add hardfork logic and remove delete vgf index

4d95969

keefel reviewed Mar 5, 2024

View reviewed changes

x/storage/keeper/keeper.go Outdated Show resolved Hide resolved

alexgao001 reviewed Mar 5, 2024

View reviewed changes

BarryTong65 added 2 commits March 5, 2024 16:08

fix: UT

7109e4b

fix: delete and set GVGFamilyStatistics after hard fork

b8e69c7

alexgao001 reviewed Mar 5, 2024

View reviewed changes

BarryTong65 added 4 commits March 5, 2024 19:19

fix: strategy enum

9216e82

fix: strategy enum

ca4f9ad

fix: prepare for Serengeti hardfork

8de6d48

fix: prepare for Serengeti hardfork

0d68d99

keefel reviewed Mar 7, 2024

View reviewed changes

x/storage/keeper/keeper_object_test.go Outdated Show resolved Hide resolved

keefel previously approved these changes Mar 7, 2024

View reviewed changes

fix: go imports format

518fce9

BarryTong65 dismissed keefel’s stale review via 518fce9 March 7, 2024 07:54

fix: go imports format

d97b159

alexgao001 reviewed Mar 7, 2024

View reviewed changes

fix: rename function and change error description

00c8574

alexgao001 approved these changes Mar 7, 2024

View reviewed changes

keefel approved these changes Mar 7, 2024

View reviewed changes

BarryTong65 added this pull request to the merge queue Mar 7, 2024

Merged via the queue into develop with commit 6067133 Mar 7, 2024
7 checks passed

alexgao001 mentioned this pull request Mar 15, 2024

release: prepare for release v1.6.0 #593

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: remove create object & bucket approval #580

feat: remove create object & bucket approval #580

BarryTong65 commented Feb 28, 2024 •

edited

Loading

alexgao001 Mar 4, 2024

BarryTong65 Mar 5, 2024

alexgao001 Mar 4, 2024

BarryTong65 Mar 5, 2024

BarryTong65 Mar 5, 2024

alexgao001 Mar 4, 2024

BarryTong65 Mar 5, 2024

alexgao001 Mar 4, 2024

BarryTong65 Mar 5, 2024

BarryTong65 Mar 5, 2024

alexgao001 Mar 4, 2024

BarryTong65 Mar 5, 2024

alexgao001 Mar 4, 2024

BarryTong65 Mar 5, 2024

BarryTong65 Mar 5, 2024

alexgao001 Mar 4, 2024

BarryTong65 Mar 5, 2024

alexgao001 Mar 5, 2024

BarryTong65 Mar 5, 2024

alexgao001 Mar 5, 2024

BarryTong65 Mar 5, 2024

alexgao001 Mar 5, 2024 •

edited

Loading

BarryTong65 Mar 5, 2024

alexgao001 Mar 5, 2024

BarryTong65 Mar 5, 2024

alexgao001 Mar 7, 2024 •

edited

Loading

alexgao001 Mar 7, 2024

BarryTong65 Mar 7, 2024

alexgao001 Mar 7, 2024

BarryTong65 Mar 7, 2024

alexgao001 Mar 7, 2024

BarryTong65 Mar 7, 2024

alexgao001 Mar 7, 2024

BarryTong65 Mar 7, 2024

feat: remove create object & bucket approval #580

feat: remove create object & bucket approval #580

Conversation

BarryTong65 commented Feb 28, 2024 • edited Loading

Description

Rationale

Before

Current

Example

Changes

Potential Impacts

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexgao001 Mar 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexgao001 Mar 7, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BarryTong65 commented Feb 28, 2024 •

edited

Loading

alexgao001 Mar 5, 2024 •

edited

Loading

alexgao001 Mar 7, 2024 •

edited

Loading