fix: enforce validation on SSP when creating GVG (#519)

* fix: enforce validation on SSP cnt when creating GVG * fix: fix cyclic dependency between virtual group and storage module * fix: not allow duplicated secondary SP * bump gnfd-cosmos-sdk to develop latest commit * adjust comment
bnb-chain · Nov 9, 2023 · ce13fea · ce13fea
1 parent 1626816
commit ce13fea
Show file tree

Hide file tree

Showing 11 changed files with 139 additions and 43 deletions.
diff --git a/app/app.go b/app/app.go
@@ -473,7 +473,6 @@ func New(
 		app.BankKeeper,
 		app.PaymentKeeper,
 	)
-	virtualgroupModule := virtualgroupmodule.NewAppModule(appCodec, app.VirtualgroupKeeper, app.SpKeeper)
 
 	app.PermissionmoduleKeeper = *permissionmodulekeeper.NewKeeper(
 		appCodec,
@@ -497,6 +496,9 @@ func New(
 	)
 	storageModule := storagemodule.NewAppModule(appCodec, app.StorageKeeper, app.AccountKeeper, app.BankKeeper, app.SpKeeper)
 
+	app.VirtualgroupKeeper.SetStorageKeeper(&app.StorageKeeper)
+	virtualgroupModule := virtualgroupmodule.NewAppModule(appCodec, app.VirtualgroupKeeper, app.SpKeeper)
+
 	app.ChallengeKeeper = *challengemodulekeeper.NewKeeper(
 		appCodec,
 		keys[challengemoduletypes.StoreKey],

diff --git a/e2e/tests/permission_test.go b/e2e/tests/permission_test.go
@@ -1925,7 +1925,7 @@ func (s *StorageTestSuite) TestExpiredAccountPolicyGCAndRePut() {
 		Actions: []types.ActionType{types.ACTION_DELETE_BUCKET},
 		Effect:  types.EFFECT_ALLOW,
 	}
-	expirationTime := time.Now().Add(5 * time.Second)
+	expirationTime := time.Now().Add(3 * time.Second)
 
 	msgPutBucketPolicy := storagetypes.NewMsgPutPolicy(owner.GetAddr(), types2.NewBucketGRN(bucketName).String(),
 		principal, []*types.Statement{bucketStatement}, &expirationTime)
@@ -1990,7 +1990,7 @@ func (s *StorageTestSuite) TestExpiredGroupPolicyGCAndRePut() {
 
 	principal := types.NewPrincipalWithGroupId(headGroupResponse.GroupInfo.Id)
 	// Put bucket policy for group
-	expirationTime := time.Now().Add(5 * time.Second)
+	expirationTime := time.Now().Add(3 * time.Second)
 
 	bucketStatement := &types.Statement{
 		Actions: []types.ActionType{types.ACTION_DELETE_BUCKET},

diff --git a/e2e/tests/storage_test.go b/e2e/tests/storage_test.go
@@ -1699,7 +1699,7 @@ func (s *StorageTestSuite) TestRejectSealObject() {
 //			secondarySPIDs = append(secondarySPIDs, ssp.Info.Id)
 //			secondarySPs = append(secondarySPs, ssp)
 //		}
-//		if len(secondarySPIDs) == 5 {
+//		if len(secondarySPIDs) == 6 {
 //			break
 //		}
 //	}

diff --git a/e2e/tests/virtualgroup_test.go b/e2e/tests/virtualgroup_test.go
@@ -170,6 +170,39 @@ func (s *VirtualGroupTestSuite) TestBasic() {
 		context.Background(),
 		&virtualgroupmoduletypes.QueryGlobalVirtualGroupRequest{GlobalVirtualGroupId: newGVG.Id})
 	s.Require().Error(err)
+
+	// test number of secondary SP doest not match onchain requirement
+	secondarySPIDs = append(secondarySPIDs, secondarySPIDs[0])
+	msgCreateGVG := virtualgroupmoduletypes.MsgCreateGlobalVirtualGroup{
+		StorageProvider: primarySP.OperatorKey.GetAddr().String(),
+		FamilyId:        virtualgroupmoduletypes.NoSpecifiedFamilyId,
+		SecondarySpIds:  secondarySPIDs,
+		Deposit: sdk.Coin{
+			Denom:  s.Config.Denom,
+			Amount: types.NewIntFromInt64WithDecimal(1, types.DecimalBNB),
+		},
+	}
+	s.SendTxBlockWithExpectErrorString(&msgCreateGVG, primarySP.OperatorKey, virtualgroupmoduletypes.ErrInvalidSecondarySPCount.Error())
+
+	// test GVG has duplicated secondary Sp
+	secondarySPIDs = make([]uint32, 0)
+	for _, ssp := range s.StorageProviders {
+		if ssp.Info.Id != primarySP.Info.Id {
+			secondarySPIDs = append(secondarySPIDs, ssp.Info.Id)
+		}
+	}
+	secondarySPIDs[len(secondarySPIDs)-1] = secondarySPIDs[0]
+	msgCreateGVG = virtualgroupmoduletypes.MsgCreateGlobalVirtualGroup{
+		StorageProvider: primarySP.OperatorKey.GetAddr().String(),
+		FamilyId:        virtualgroupmoduletypes.NoSpecifiedFamilyId,
+		SecondarySpIds:  secondarySPIDs,
+		Deposit: sdk.Coin{
+			Denom:  s.Config.Denom,
+			Amount: types.NewIntFromInt64WithDecimal(1, types.DecimalBNB),
+		},
+	}
+	s.SendTxBlockWithExpectErrorString(&msgCreateGVG, primarySP.OperatorKey, virtualgroupmoduletypes.ErrDuplicateSecondarySP.Error())
+
 }
 
 func (s *VirtualGroupTestSuite) TestSettle() {

diff --git a/go.mod b/go.mod
@@ -178,7 +178,7 @@ replace (
 	github.com/cometbft/cometbft => github.com/bnb-chain/greenfield-cometbft v0.0.0-20231030090949-99ef7dbd1e62
 	github.com/cometbft/cometbft-db => github.com/bnb-chain/greenfield-cometbft-db v0.8.1-alpha.1
 	github.com/confio/ics23/go => github.com/cosmos/cosmos-sdk/ics23/go v0.8.0
-	github.com/cosmos/cosmos-sdk => github.com/bnb-chain/greenfield-cosmos-sdk v1.0.2-0.20231101023808-a3c24a58eca0
+	github.com/cosmos/cosmos-sdk => github.com/bnb-chain/greenfield-cosmos-sdk v1.0.2-0.20231109071432-ddd4ce07ab5c
 	github.com/cosmos/iavl => github.com/bnb-chain/greenfield-iavl v0.20.1
 	github.com/syndtr/goleveldb => github.com/syndtr/goleveldb v1.0.1-0.20210819022825-2ae1ddf74ef7
 	github.com/wercker/journalhook => github.com/wercker/journalhook v0.0.0-20230927020745-64542ffa4117

diff --git a/go.sum b/go.sum
@@ -163,8 +163,8 @@ github.com/bnb-chain/greenfield-cometbft v0.0.0-20231030090949-99ef7dbd1e62 h1:p
 github.com/bnb-chain/greenfield-cometbft v0.0.0-20231030090949-99ef7dbd1e62/go.mod h1:43yICrTxu90VjEUpQN23bsqi9mua5m5sFQq/ekHwN9s=
 github.com/bnb-chain/greenfield-cometbft-db v0.8.1-alpha.1 h1:XcWulGacHVRiSCx90Q8Y//ajOrLNBQWR/KDB89dy3cU=
 github.com/bnb-chain/greenfield-cometbft-db v0.8.1-alpha.1/go.mod h1:ey1CiK4bYo1RBNJLRiVbYr5CMdSxci9S/AZRINLtppI=
-github.com/bnb-chain/greenfield-cosmos-sdk v1.0.2-0.20231101023808-a3c24a58eca0 h1:5BQrQRqQ0GTR3UgvTtlTCJAYPkWrjk9Yk9kRnmzzut0=
-github.com/bnb-chain/greenfield-cosmos-sdk v1.0.2-0.20231101023808-a3c24a58eca0/go.mod h1:ZWyfWX032fdHkICmEoJwylfqmL+Atf/QNVS8GzJq1Kc=
+github.com/bnb-chain/greenfield-cosmos-sdk v1.0.2-0.20231109071432-ddd4ce07ab5c h1:2/EgztaiMBMZQu3HBdfRjbAct/pENdwelFVLSSeD3j4=
+github.com/bnb-chain/greenfield-cosmos-sdk v1.0.2-0.20231109071432-ddd4ce07ab5c/go.mod h1:ZWyfWX032fdHkICmEoJwylfqmL+Atf/QNVS8GzJq1Kc=
 github.com/bnb-chain/greenfield-cosmos-sdk/api v0.0.0-20230816082903-b48770f5e210 h1:GHPbV2bC+gmuO6/sG0Tm8oGal3KKSRlyE+zPscDjlA8=
 github.com/bnb-chain/greenfield-cosmos-sdk/api v0.0.0-20230816082903-b48770f5e210/go.mod h1:vhsZxXE9tYJeYB5JR4hPhd6Pc/uPf7j1T8IJ7p9FdeM=
 github.com/bnb-chain/greenfield-cosmos-sdk/math v0.0.0-20230816082903-b48770f5e210 h1:FLVOn4+OVbsKi2+YJX5kmD27/4dRu4FW7xCXFhzDO5s=

diff --git a/x/virtualgroup/keeper/keeper.go b/x/virtualgroup/keeper/keeper.go
@@ -29,6 +29,7 @@ type (
 		accountKeeper types.AccountKeeper
 		bankKeeper    types.BankKeeper
 		paymentKeeper types.PaymentKeeper
+		storageKeeper types.StorageKeeper
 		// sequence
 		gvgSequence       sequence.Sequence[uint32]
 		gvgFamilySequence sequence.Sequence[uint32]
@@ -63,6 +64,10 @@ func NewKeeper(
 	return &k
 }
 
+func (k *Keeper) SetStorageKeeper(storageKeeper types.StorageKeeper) {
+	k.storageKeeper = storageKeeper
+}
+
 func (k Keeper) GetAuthority() string {
 	return k.authority
 }

diff --git a/x/virtualgroup/keeper/msg_server.go b/x/virtualgroup/keeper/msg_server.go
@@ -53,6 +53,20 @@ func (k msgServer) UpdateParams(goCtx context.Context, req *types.MsgUpdateParam
 
 func (k msgServer) CreateGlobalVirtualGroup(goCtx context.Context, req *types.MsgCreateGlobalVirtualGroup) (*types.MsgCreateGlobalVirtualGroupResponse, error) {
 	ctx := sdk.UnwrapSDKContext(goCtx)
+	if ctx.IsUpgraded(upgradetypes.Pampas) {
+		expectSecondarySPNum := int(k.storageKeeper.GetExpectSecondarySPNumForECObject(ctx, ctx.BlockTime().Unix()))
+		if len(req.GetSecondarySpIds()) != expectSecondarySPNum {
+			return nil, types.ErrInvalidSecondarySPCount.Wrapf("the number of secondary sp in the Global virtual group should be %d", expectSecondarySPNum)
+		}
+		spIdSet := make(map[uint32]struct{}, len(req.GetSecondarySpIds()))
+		for _, spId := range req.GetSecondarySpIds() {
+			if _, ok := spIdSet[spId]; ok {
+				return nil, types.ErrDuplicateSecondarySP.Wrapf("the SP(id=%d) is duplicate in the Global virtual group.", spId)
+			}
+			spIdSet[spId] = struct{}{}
+		}
+	}
+
 	var gvgStatisticsWithinSPs []*types.GVGStatisticsWithinSP
 
 	spOperatorAddr := sdk.MustAccAddressFromHex(req.StorageProvider)

diff --git a/x/virtualgroup/types/errors.go b/x/virtualgroup/types/errors.go
@@ -6,21 +6,22 @@ import (
 
 // x/virtualgroup module sentinel errors
 var (
-	ErrGVGFamilyNotExist      = errors.Register(ModuleName, 1100, "global virtual group family not exist.")
-	ErrGVGNotExistInFamily    = errors.Register(ModuleName, 1101, "global virtual group not exist in family.")
-	ErrGVGNotExist            = errors.Register(ModuleName, 1102, "global virtual group not exist.")
-	ErrGVGNotEmpty            = errors.Register(ModuleName, 1103, "the store size of gvg is not zero")
-	ErrGenSequenceIDError     = errors.Register(ModuleName, 1104, "generate sequence id error.")
-	ErrWithdrawAmountTooLarge = errors.Register(ModuleName, 1105, "withdrawal amount is too large.")
-	ErrSwapOutFailed          = errors.Register(ModuleName, 1106, "swap out failed.")
-	ErrLVGNotExist            = errors.Register(ModuleName, 1107, "local virtual group not exist.")
-	ErrSPCanNotExit           = errors.Register(ModuleName, 1108, "the sp can not exit now.")
-	ErrSettleFailed           = errors.Register(ModuleName, 1109, "fail to settle.")
-	ErrInvalidGVGCount        = errors.Register(ModuleName, 1120, "the count of global virtual group ids is invalid.")
-	ErrWithdrawFailed         = errors.Register(ModuleName, 1121, "with draw failed.")
-
-	ErrLimitationExceed    = errors.Register(ModuleName, 1123, "limitation exceed.")
-	ErrInsufficientStaking = errors.Register(ModuleName, 1125, "insufficient staking for gvg")
+	ErrGVGFamilyNotExist       = errors.Register(ModuleName, 1100, "global virtual group family not exist.")
+	ErrGVGNotExistInFamily     = errors.Register(ModuleName, 1101, "global virtual group not exist in family.")
+	ErrGVGNotExist             = errors.Register(ModuleName, 1102, "global virtual group not exist.")
+	ErrGVGNotEmpty             = errors.Register(ModuleName, 1103, "the store size of gvg is not zero")
+	ErrGenSequenceIDError      = errors.Register(ModuleName, 1104, "generate sequence id error.")
+	ErrWithdrawAmountTooLarge  = errors.Register(ModuleName, 1105, "withdrawal amount is too large.")
+	ErrSwapOutFailed           = errors.Register(ModuleName, 1106, "swap out failed.")
+	ErrLVGNotExist             = errors.Register(ModuleName, 1107, "local virtual group not exist.")
+	ErrSPCanNotExit            = errors.Register(ModuleName, 1108, "the sp can not exit now.")
+	ErrSettleFailed            = errors.Register(ModuleName, 1109, "fail to settle.")
+	ErrInvalidGVGCount         = errors.Register(ModuleName, 1120, "the count of global virtual group ids is invalid.")
+	ErrWithdrawFailed          = errors.Register(ModuleName, 1121, "with draw failed.")
+	ErrInvalidSecondarySPCount = errors.Register(ModuleName, 1122, "the number of secondary sp within the global virtual group is invalid.")
+	ErrLimitationExceed        = errors.Register(ModuleName, 1123, "limitation exceed.")
+	ErrDuplicateSecondarySP    = errors.Register(ModuleName, 1124, "the global virtual group has duplicate secondary sp.")
+	ErrInsufficientStaking     = errors.Register(ModuleName, 1125, "insufficient staking for gvg")
 
 	ErrInvalidDenom = errors.Register(ModuleName, 2000, "Invalid denom.")
 )
diff --git a/x/virtualgroup/types/expected_keepers.go b/x/virtualgroup/types/expected_keepers.go
@@ -44,3 +44,7 @@ type PaymentKeeper interface {
 	Withdraw(ctx sdk.Context, fromAddr, toAddr sdk.AccAddress, amount sdkmath.Int) error
 	IsEmptyNetFlow(ctx sdk.Context, account sdk.AccAddress) bool
 }
+
+type StorageKeeper interface {
+	GetExpectSecondarySPNumForECObject(ctx sdk.Context, time int64) (res uint32)
+}