chore: improve validation of pool proposals (backport #74) (#94)

Co-authored-by: Troy Kessler <[email protected]>
KYVENetwork · Jun 14, 2023 · adf2448 · adf2448
1 parent 3567b4e
commit adf2448
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 27 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -21,6 +21,7 @@
 ### Improvements
 
 - (`x/bundles`) [#62](https://github.com/KYVENetwork/chain/pull/62) Payout storage cost directly to the bundle uploader.
+- (`x/pool`) [#74](https://github.com/KYVENetwork/chain/pull/74) Improve parameter validation in pool proposals.
 - (`x/stakers`) [#46](https://github.com/KYVENetwork/chain/pull/46) Allow protocol validator commission rewards to be claimed.
 
 ### Client Breaking

diff --git a/x/pool/keeper/msg_server_update_pool_test.go b/x/pool/keeper/msg_server_update_pool_test.go
@@ -322,33 +322,12 @@ var _ = Describe("msg_server_update_pool.go", Ordered, func() {
 		Expect(pool.Name).To(BeEmpty())
 	})
 
-	It("Update pool with invalid UploadInterval", func() {
-		// ARRANGE
-		msg := &types.MsgUpdatePool{
-			Authority: gov,
-			Id:        1,
-			Payload:   "{\"UploadInterval\": 0}",
-		}
-
-		p, _ := BuildGovernanceTxs(s, []sdk.Msg{msg})
-
-		// ACT
-		_ = s.RunTxError(&p)
-		s.Commit()
-
-		// ASSERT
-		pool, found := s.App().PoolKeeper.GetPool(s.Ctx(), 0)
-
-		Expect(found).To(BeTrue())
-		Expect(pool.Name).To(BeEmpty())
-	})
-
 	It("Update pool with invalid OperatingCost", func() {
 		// ARRANGE
 		msg := &types.MsgUpdatePool{
 			Authority: gov,
 			Id:        1,
-			Payload:   "{\"OperatingCost\": 0}",
+			Payload:   "{\"OperatingCost\": -1}",
 		}
 
 		p, _ := BuildGovernanceTxs(s, []sdk.Msg{msg})
@@ -369,7 +348,7 @@ var _ = Describe("msg_server_update_pool.go", Ordered, func() {
 		msg := &types.MsgUpdatePool{
 			Authority: gov,
 			Id:        1,
-			Payload:   "{\"MinDelegation\": 0}",
+			Payload:   "{\"MinDelegation\": -1}",
 		}
 
 		p, _ := BuildGovernanceTxs(s, []sdk.Msg{msg})

diff --git a/x/pool/types/msgs.go b/x/pool/types/msgs.go
@@ -35,14 +35,18 @@ func (msg *MsgCreatePool) ValidateBasic() error {
 		return errors.Wrapf(errorsTypes.ErrInvalidRequest, "invalid upload interval")
 	}
 
-	if err := util.ValidatePositiveNumber(msg.OperatingCost); err != nil {
+	if err := util.ValidateNumber(msg.OperatingCost); err != nil {
 		return errors.Wrapf(errorsTypes.ErrInvalidRequest, "invalid operating cost")
 	}
 
-	if err := util.ValidatePositiveNumber(msg.MinDelegation); err != nil {
+	if err := util.ValidateNumber(msg.MinDelegation); err != nil {
 		return errors.Wrapf(errorsTypes.ErrInvalidRequest, "invalid minimum delegation")
 	}
 
+	if err := util.ValidatePositiveNumber(msg.MaxBundleSize); err != nil {
+		return errors.Wrapf(errorsTypes.ErrInvalidRequest, "invalid max bundle size")
+	}
+
 	return nil
 }
 
@@ -84,17 +88,23 @@ func (msg *MsgUpdatePool) ValidateBasic() error {
 	}
 
 	if payload.OperatingCost != nil {
-		if err := util.ValidatePositiveNumber(*payload.OperatingCost); err != nil {
+		if err := util.ValidateNumber(*payload.OperatingCost); err != nil {
 			return errors.Wrapf(errorsTypes.ErrInvalidRequest, "invalid operating cost")
 		}
 	}
 
 	if payload.MinDelegation != nil {
-		if err := util.ValidatePositiveNumber(*payload.MinDelegation); err != nil {
+		if err := util.ValidateNumber(*payload.MinDelegation); err != nil {
 			return errors.Wrapf(errorsTypes.ErrInvalidRequest, "invalid minimum delegation")
 		}
 	}
 
+	if payload.MaxBundleSize != nil {
+		if err := util.ValidatePositiveNumber(*payload.MaxBundleSize); err != nil {
+			return errors.Wrapf(errorsTypes.ErrInvalidRequest, "invalid max bundle size")
+		}
+	}
+
 	return nil
 }