Bugfix and refactor NewCreateConsumerChainProposal (#337)

no err returned
cosmos · Sep 7, 2022 · 5e3ff16 · 5e3ff16
1 parent b81a07d
commit 5e3ff16
Show file tree

Hide file tree

Showing 4 changed files with 40 additions and 62 deletions.
diff --git a/tests/e2e/channel_init_test.go b/tests/e2e/channel_init_test.go
@@ -710,8 +710,7 @@ func (suite *ProviderTestSuite) TestConsumerChainProposalHandler() {
 				initialHeight := clienttypes.NewHeight(2, 3)
 				// ctx blocktime is after proposal's spawn time
 				ctx = suite.providerChain.GetContext().WithBlockTime(time.Now().Add(time.Hour))
-				content, err = types.NewCreateConsumerChainProposal("title", "description", "chainID", initialHeight, []byte("gen_hash"), []byte("bin_hash"), time.Now())
-				suite.Require().NoError(err)
+				content = types.NewCreateConsumerChainProposal("title", "description", "chainID", initialHeight, []byte("gen_hash"), []byte("bin_hash"), time.Now())
 			}, true,
 		},
 		{
@@ -804,8 +803,7 @@ func (suite *ProviderKeeperTestSuite) TestCreateConsumerChainProposal() {
 			"valid create consumer chain proposal: spawn time reached", func(suite *ProviderKeeperTestSuite) {
 				// ctx blocktime is after proposal's spawn time
 				ctx = suite.providerChain.GetContext().WithBlockTime(time.Now().Add(time.Hour))
-				content, err := types.NewCreateConsumerChainProposal("title", "description", chainID, initialHeight, []byte("gen_hash"), []byte("bin_hash"), time.Now())
-				suite.Require().NoError(err)
+				content := types.NewCreateConsumerChainProposal("title", "description", chainID, initialHeight, []byte("gen_hash"), []byte("bin_hash"), time.Now())
 				proposal, ok = content.(*types.CreateConsumerChainProposal)
 				suite.Require().True(ok)
 				proposal.LockUnbondingOnTimeout = lockUbdOnTimeout
@@ -815,8 +813,7 @@ func (suite *ProviderKeeperTestSuite) TestCreateConsumerChainProposal() {
 			"valid proposal: spawn time has not yet been reached", func(suite *ProviderKeeperTestSuite) {
 				// ctx blocktime is before proposal's spawn time
 				ctx = suite.providerChain.GetContext().WithBlockTime(time.Now())
-				content, err := types.NewCreateConsumerChainProposal("title", "description", chainID, initialHeight, []byte("gen_hash"), []byte("bin_hash"), time.Now().Add(time.Hour))
-				suite.Require().NoError(err)
+				content := types.NewCreateConsumerChainProposal("title", "description", chainID, initialHeight, []byte("gen_hash"), []byte("bin_hash"), time.Now().Add(time.Hour))
 				proposal, ok = content.(*types.CreateConsumerChainProposal)
 				suite.Require().True(ok)
 				proposal.LockUnbondingOnTimeout = lockUbdOnTimeout

diff --git a/x/ccv/provider/client/proposal_handler.go b/x/ccv/provider/client/proposal_handler.go
@@ -64,12 +64,9 @@ Where proposal.json contains:
 				return err
 			}
 
-			content, err := types.NewCreateConsumerChainProposal(
+			content := types.NewCreateConsumerChainProposal(
 				proposal.Title, proposal.Description, proposal.ChainId, proposal.InitialHeight,
 				proposal.GenesisHash, proposal.BinaryHash, proposal.SpawnTime)
-			if err != nil {
-				return err
-			}
 
 			from := clientCtx.GetFromAddress()
 
@@ -149,12 +146,9 @@ func postProposalHandlerFn(clientCtx client.Context) http.HandlerFunc {
 			return
 		}
 
-		content, err := types.NewCreateConsumerChainProposal(
+		content := types.NewCreateConsumerChainProposal(
 			req.Title, req.Description, req.ChainId, req.InitialHeight,
 			req.GenesisHash, req.BinaryHash, req.SpawnTime)
-		if rest.CheckBadRequestError(w, err) {
-			return
-		}
 
 		msg, err := govtypes.NewMsgSubmitProposal(content, req.Deposit, req.Proposer)
 		if rest.CheckBadRequestError(w, err) {

diff --git a/x/ccv/provider/types/proposal.go b/x/ccv/provider/types/proposal.go
@@ -24,7 +24,7 @@ func init() {
 }
 
 // NewCreateConsumerChainProposal creates a new create consumerchain proposal.
-func NewCreateConsumerChainProposal(title, description, chainID string, initialHeight clienttypes.Height, genesisHash, binaryHash []byte, spawnTime time.Time) (govtypes.Content, error) {
+func NewCreateConsumerChainProposal(title, description, chainID string, initialHeight clienttypes.Height, genesisHash, binaryHash []byte, spawnTime time.Time) govtypes.Content {
 	return &CreateConsumerChainProposal{
 		Title:         title,
 		Description:   description,
@@ -33,7 +33,7 @@ func NewCreateConsumerChainProposal(title, description, chainID string, initialH
 		GenesisHash:   genesisHash,
 		BinaryHash:    binaryHash,
 		SpawnTime:     spawnTime,
-	}, nil
+	}
 }
 
 // GetTitle returns the title of a create consumerchain proposal.

diff --git a/x/ccv/provider/types/proposal_test.go b/x/ccv/provider/types/proposal_test.go
@@ -18,72 +18,61 @@ import (
 )
 
 func TestValidateBasic(t *testing.T) {
-	var (
-		proposal govtypes.Content
-		err      error
-	)
 	initialHeight := clienttypes.NewHeight(2, 3)
 
 	testCases := []struct {
 		name     string
-		malleate func()
+		proposal govtypes.Content
 		expPass  bool
 	}{
 		{
-			"success", func() {
-				proposal, err = types.NewCreateConsumerChainProposal("title", "description", "chainID", initialHeight, []byte("gen_hash"), []byte("bin_hash"), time.Now())
-				require.NoError(t, err)
-			}, true,
+			"success",
+			types.NewCreateConsumerChainProposal("title", "description", "chainID", initialHeight, []byte("gen_hash"), []byte("bin_hash"), time.Now()),
+			true,
 		},
 		{
-			"fails validate abstract - empty title", func() {
-				proposal, err = types.NewCreateConsumerChainProposal(" ", "description", "chainID", initialHeight, []byte("gen_hash"), []byte("bin_hash"), time.Now())
-				require.NoError(t, err)
-			}, false,
+			"fails validate abstract - empty title",
+			types.NewCreateConsumerChainProposal(" ", "description", "chainID", initialHeight, []byte("gen_hash"), []byte("bin_hash"), time.Now()),
+			false,
 		},
 		{
-			"chainID is empty", func() {
-				proposal, err = types.NewCreateConsumerChainProposal("title", "description", " ", initialHeight, []byte("gen_hash"), []byte("bin_hash"), time.Now())
-				require.NoError(t, err)
-			}, false,
+			"chainID is empty",
+			types.NewCreateConsumerChainProposal("title", "description", " ", initialHeight, []byte("gen_hash"), []byte("bin_hash"), time.Now()),
+			false,
 		},
 		{
-			"initial height is zero", func() {
-				proposal = &types.CreateConsumerChainProposal{
-					Title:         "title",
-					Description:   "description",
-					ChainId:       "chainID",
-					InitialHeight: clienttypes.Height{},
-					GenesisHash:   []byte("gen_hash"),
-					BinaryHash:    []byte("bin_hash"),
-					SpawnTime:     time.Now(),
-				}
-			}, false,
+			"initial height is zero",
+			&types.CreateConsumerChainProposal{
+				Title:         "title",
+				Description:   "description",
+				ChainId:       "chainID",
+				InitialHeight: clienttypes.Height{},
+				GenesisHash:   []byte("gen_hash"),
+				BinaryHash:    []byte("bin_hash"),
+				SpawnTime:     time.Now(),
+			},
+			false,
 		},
 		{
-			"genesis hash is empty", func() {
-				proposal, err = types.NewCreateConsumerChainProposal("title", "description", "chainID", initialHeight, []byte(""), []byte("bin_hash"), time.Now())
-				require.NoError(t, err)
-			}, false,
+			"genesis hash is empty",
+			types.NewCreateConsumerChainProposal("title", "description", "chainID", initialHeight, []byte(""), []byte("bin_hash"), time.Now()),
+			false,
 		},
 		{
-			"binary hash is empty", func() {
-				proposal, err = types.NewCreateConsumerChainProposal("title", "description", "chainID", initialHeight, []byte("gen_hash"), []byte(""), time.Now())
-				require.NoError(t, err)
-			}, false,
+			"binary hash is empty",
+			types.NewCreateConsumerChainProposal("title", "description", "chainID", initialHeight, []byte("gen_hash"), []byte(""), time.Now()),
+			false,
 		},
 		{
-			"time is zero", func() {
-				proposal, err = types.NewCreateConsumerChainProposal("title", "description", "chainID", initialHeight, []byte("gen_hash"), []byte("bin_hash"), time.Time{})
-				require.NoError(t, err)
-			}, false,
+			"time is zero",
+			types.NewCreateConsumerChainProposal("title", "description", "chainID", initialHeight, []byte("gen_hash"), []byte("bin_hash"), time.Time{}),
+			false,
 		},
 	}
 
 	for _, tc := range testCases {
-		tc.malleate()
 
-		err := proposal.ValidateBasic()
+		err := tc.proposal.ValidateBasic()
 		if tc.expPass {
 			require.NoError(t, err, "valid case: %s should not return error. got %w", tc.name, err)
 		} else {
@@ -93,8 +82,7 @@ func TestValidateBasic(t *testing.T) {
 }
 
 func TestMarshalCreateConsumerChainProposal(t *testing.T) {
-	content, err := types.NewCreateConsumerChainProposal("title", "description", "chainID", clienttypes.NewHeight(0, 1), []byte("gen_hash"), []byte("bin_hash"), time.Now().UTC())
-	require.NoError(t, err)
+	content := types.NewCreateConsumerChainProposal("title", "description", "chainID", clienttypes.NewHeight(0, 1), []byte("gen_hash"), []byte("bin_hash"), time.Now().UTC())
 
 	cccp, ok := content.(*types.CreateConsumerChainProposal)
 	require.True(t, ok)
@@ -122,8 +110,7 @@ func TestMarshalCreateConsumerChainProposal(t *testing.T) {
 func TestCreateConsumerChainProposalString(t *testing.T) {
 	initialHeight := clienttypes.NewHeight(2, 3)
 	spawnTime := time.Now()
-	proposal, err := types.NewCreateConsumerChainProposal("title", "description", "chainID", initialHeight, []byte("gen_hash"), []byte("bin_hash"), spawnTime)
-	require.NoError(t, err)
+	proposal := types.NewCreateConsumerChainProposal("title", "description", "chainID", initialHeight, []byte("gen_hash"), []byte("bin_hash"), spawnTime)
 
 	expect := fmt.Sprintf(`CreateConsumerChain Proposal
 	Title: title