Fix receivers on ValidateBasic funcs. (#413)

* [412]: Add some unit tests.

* [412]: Add changelog line.

* [412]: Fix receivers for some of the metadata ValidateBasic funcs.

* [412]: Fix more ValidateBasic receivers.

* [412]: Add calls to ConvertOptionalFields to the tops of the msg_server funcs that rely on it being done.

* [412]: Fix some more receivers.

* [412]: gofmt the new tests I just added.

CHANGELOG.md

@@ -59,6 +59,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
   * Export marker accounts as a base account entry and a separate marker module record
   * Add Marker module governance proposals, genesis, and marker operations to simulation testing [#94](https://github.com/provenance-io/provenance/issues/94)
 * Fix an encoding issue with the `--page-key` CLI arguments used in paged queries [#332](https://github.com/provenance-io/provenance/issues/332)
+* Fix handling of optional fields in Metadata Write messages [#412](https://github.com/provenance-io/provenance/issues/412)
 
 ### Improvements
 

x/metadata/handler_test.go

@@ -739,3 +739,86 @@ func (s MetadataHandlerTestSuite) TestAddAndDeleteScopeDataAccess() {
 		assert.Equal(t, scopeA.Owners, scopeC.Owners, "add Owners")
 	})
 }
+
+func (s MetadataHandlerTestSuite) TestIssue412WriteScopeOptionalField() {
+	ownerAddress := "cosmos1vz99nyd2er8myeugsr4xm5duwhulhp5ae4dvpa"
+	specIDStr := "scopespec1qjkyp28sldx5r9ueaxqc5adrc5wszy6nsh"
+	specUUIDStr := "ac40a8f0-fb4d-4197-99e9-818a75a3c51d"
+	specID, specIDErr := types.MetadataAddressFromBech32(specIDStr)
+	require.NoError(s.T(), specIDErr, "converting scopeIDStr to a metadata address")
+
+	s.T().Run("Ensure ID and UUID strings match", func(t *testing.T) {
+		specIDFromID, e1 := types.MetadataAddressFromBech32(specIDStr)
+		require.NoError(t, e1, "specIDFromIDStr")
+		specUUIDFromID, e2 := specIDFromID.ScopeSpecUUID()
+		require.NoError(t, e2, "specUUIDActualStr")
+		specUUIDStrActual := specUUIDFromID.String()
+		assert.Equal(t, specUUIDStr, specUUIDStrActual, "UUID strings")
+
+		specIDFFromUUID := types.ScopeSpecMetadataAddress(uuid.MustParse(specUUIDStr))
+		specIDStrActual := specIDFFromUUID.String()
+		assert.Equal(t, specIDStr, specIDStrActual, "ID strings")
+
+		assert.Equal(t, specIDFromID, specIDFFromUUID, "scope spec ids")
+	})
+
+	s.T().Run("Setting scope spec with just a spec ID", func(t *testing.T) {
+		msg := types.MsgWriteScopeSpecificationRequest{
+			Specification: types.ScopeSpecification{
+				SpecificationId: specID,
+				Description: &types.Description{
+					Name:        "io.p8e.contracts.examplekotlin.helloWorld",
+					Description: "A generic scope that allows for a lot of example hello world contracts.",
+				},
+				OwnerAddresses:  []string{ownerAddress},
+				PartiesInvolved: []types.PartyType{types.PartyType_PARTY_TYPE_OWNER},
+				ContractSpecIds: nil,
+			},
+			Signers:  []string{ownerAddress},
+			SpecUuid: "",
+		}
+		res, err := s.handler(s.ctx, &msg)
+		assert.NoError(t, err)
+		assert.NotNil(t, 0, res)
+	})
+
+	s.T().Run("Setting scope spec with just a UUID", func(t *testing.T) {
+		msg := types.MsgWriteScopeSpecificationRequest{
+			Specification: types.ScopeSpecification{
+				SpecificationId: nil,
+				Description: &types.Description{
+					Name:        "io.p8e.contracts.examplekotlin.helloWorld",
+					Description: "A generic scope that allows for a lot of example hello world contracts.",
+				},
+				OwnerAddresses:  []string{ownerAddress},
+				PartiesInvolved: []types.PartyType{types.PartyType_PARTY_TYPE_OWNER},
+				ContractSpecIds: nil,
+			},
+			Signers:  []string{ownerAddress},
+			SpecUuid: specUUIDStr,
+		}
+		res, err := s.handler(s.ctx, &msg)
+		assert.NoError(t, err)
+		assert.NotNil(t, 0, res)
+	})
+
+	s.T().Run("Setting scope spec with matching ID and UUID", func(t *testing.T) {
+		msg := types.MsgWriteScopeSpecificationRequest{
+			Specification: types.ScopeSpecification{
+				SpecificationId: specID,
+				Description: &types.Description{
+					Name:        "io.p8e.contracts.examplekotlin.helloWorld",
+					Description: "A generic scope that allows for a lot of example hello world contracts.",
+				},
+				OwnerAddresses:  []string{ownerAddress},
+				PartiesInvolved: []types.PartyType{types.PartyType_PARTY_TYPE_OWNER},
+				ContractSpecIds: nil,
+			},
+			Signers:  []string{ownerAddress},
+			SpecUuid: specUUIDStr,
+		}
+		res, err := s.handler(s.ctx, &msg)
+		assert.NoError(t, err)
+		assert.NotNil(t, 0, res)
+	})
+}

x/metadata/keeper/msg_server.go

@@ -33,6 +33,9 @@ func (k msgServer) WriteScope(
 	defer telemetry.MeasureSince(time.Now(), types.ModuleName, "tx", "WriteScope")
 	ctx := sdk.UnwrapSDKContext(goCtx)
 
+	//nolint:errcheck // the error was checked when msg.ValidateBasic was called before getting here.
+	msg.ConvertOptionalFields()
+
 	existing, _ := k.GetScope(ctx, msg.Scope.ScopeId)
 	if err := k.ValidateScopeUpdate(ctx, existing, msg.Scope, msg.Signers); err != nil {
 		return nil, err
@@ -185,6 +188,9 @@ func (k msgServer) WriteSession(
 	defer telemetry.MeasureSince(time.Now(), types.ModuleName, "tx", "WriteSession")
 	ctx := sdk.UnwrapSDKContext(goCtx)
 
+	//nolint:errcheck // the error was checked when msg.ValidateBasic was called before getting here.
+	msg.ConvertOptionalFields()
+
 	var existing *types.Session = nil
 	var existingAudit *types.AuditFields = nil
 	if e, found := k.GetSession(ctx, msg.Session.SessionId); found {
@@ -210,6 +216,9 @@ func (k msgServer) WriteRecord(
 	defer telemetry.MeasureSince(time.Now(), types.ModuleName, "tx", "WriteRecord")
 	ctx := sdk.UnwrapSDKContext(goCtx)
 
+	//nolint:errcheck // the error was checked when msg.ValidateBasic was called before getting here.
+	msg.ConvertOptionalFields()
+
 	scopeUUID, err := msg.Record.SessionId.ScopeUUID()
 	if err != nil {
 		return nil, err
@@ -256,6 +265,9 @@ func (k msgServer) WriteScopeSpecification(
 	defer telemetry.MeasureSince(time.Now(), types.ModuleName, "tx", "WriteScopeSpecification")
 	ctx := sdk.UnwrapSDKContext(goCtx)
 
+	//nolint:errcheck // the error was checked when msg.ValidateBasic was called before getting here.
+	msg.ConvertOptionalFields()
+
 	var existing *types.ScopeSpecification = nil
 	if e, found := k.GetScopeSpecification(ctx, msg.Specification.SpecificationId); found {
 		existing = &e
@@ -303,6 +315,9 @@ func (k msgServer) WriteContractSpecification(
 	defer telemetry.MeasureSince(time.Now(), types.ModuleName, "tx", "WriteContractSpecification")
 	ctx := sdk.UnwrapSDKContext(goCtx)
 
+	//nolint:errcheck // the error was checked when msg.ValidateBasic was called before getting here.
+	msg.ConvertOptionalFields()
+
 	var existing *types.ContractSpecification = nil
 	if e, found := k.GetContractSpecification(ctx, msg.Specification.SpecificationId); found {
 		existing = &e
@@ -447,6 +462,9 @@ func (k msgServer) WriteRecordSpecification(
 	defer telemetry.MeasureSince(time.Now(), types.ModuleName, "tx", "WriteRecordSpecification")
 	ctx := sdk.UnwrapSDKContext(goCtx)
 
+	//nolint:errcheck // the error was checked when msg.ValidateBasic was called before getting here.
+	msg.ConvertOptionalFields()
+
 	contractSpecID, err := msg.Specification.SpecificationId.AsContractSpecAddress()
 	if err != nil {
 		return nil, err

x/metadata/types/msg.go

@@ -118,7 +118,7 @@ func (msg MsgWriteScopeRequest) GetSignBytes() []byte {
 }
 
 // ValidateBasic performs a quick validity check
-func (msg *MsgWriteScopeRequest) ValidateBasic() error {
+func (msg MsgWriteScopeRequest) ValidateBasic() error {
 	if len(msg.Signers) < 1 {
 		return fmt.Errorf("at least one signer is required")
 	}
@@ -244,7 +244,7 @@ func (msg MsgAddScopeDataAccessRequest) GetSignBytes() []byte {
 }
 
 // ValidateBasic performs a quick validity check
-func (msg *MsgAddScopeDataAccessRequest) ValidateBasic() error {
+func (msg MsgAddScopeDataAccessRequest) ValidateBasic() error {
 	if !msg.ScopeId.IsScopeAddress() {
 		return fmt.Errorf("address is not a scope id: %v", msg.ScopeId.String())
 	}
@@ -300,7 +300,7 @@ func (msg MsgDeleteScopeDataAccessRequest) GetSignBytes() []byte {
 }
 
 // ValidateBasic performs a quick validity check
-func (msg *MsgDeleteScopeDataAccessRequest) ValidateBasic() error {
+func (msg MsgDeleteScopeDataAccessRequest) ValidateBasic() error {
 	if !msg.ScopeId.IsScopeAddress() {
 		return fmt.Errorf("address is not a scope id: %v", msg.ScopeId.String())
 	}
@@ -356,7 +356,7 @@ func (msg MsgAddScopeOwnerRequest) GetSignBytes() []byte {
 }
 
 // ValidateBasic performs a quick validity check
-func (msg *MsgAddScopeOwnerRequest) ValidateBasic() error {
+func (msg MsgAddScopeOwnerRequest) ValidateBasic() error {
 	if !msg.ScopeId.IsScopeAddress() {
 		return fmt.Errorf("address is not a scope id: %v", msg.ScopeId.String())
 	}
@@ -406,7 +406,7 @@ func (msg MsgDeleteScopeOwnerRequest) GetSignBytes() []byte {
 }
 
 // ValidateBasic performs a quick validity check
-func (msg *MsgDeleteScopeOwnerRequest) ValidateBasic() error {
+func (msg MsgDeleteScopeOwnerRequest) ValidateBasic() error {
 	if !msg.ScopeId.IsScopeAddress() {
 		return fmt.Errorf("address is not a scope id: %v", msg.ScopeId.String())
 	}
@@ -458,7 +458,7 @@ func (msg MsgWriteSessionRequest) GetSignBytes() []byte {
 }
 
 // ValidateBasic performs a quick validity check
-func (msg *MsgWriteSessionRequest) ValidateBasic() error {
+func (msg MsgWriteSessionRequest) ValidateBasic() error {
 	if len(msg.Signers) < 1 {
 		return fmt.Errorf("at least one signer is required")
 	}
@@ -535,7 +535,7 @@ func (msg MsgWriteRecordRequest) GetSignBytes() []byte {
 }
 
 // ValidateBasic performs a quick validity check
-func (msg *MsgWriteRecordRequest) ValidateBasic() error {
+func (msg MsgWriteRecordRequest) ValidateBasic() error {
 	if len(msg.Signers) < 1 {
 		return fmt.Errorf("at least one signer is required")
 	}
@@ -655,7 +655,7 @@ func (msg MsgWriteScopeSpecificationRequest) GetSignBytes() []byte {
 }
 
 // ValidateBasic performs a quick validity check
-func (msg *MsgWriteScopeSpecificationRequest) ValidateBasic() error {
+func (msg MsgWriteScopeSpecificationRequest) ValidateBasic() error {
 	if len(msg.Signers) < 1 {
 		return fmt.Errorf("at least one signer is required")
 	}
@@ -805,7 +805,7 @@ func (msg MsgWriteContractSpecificationRequest) GetSignBytes() []byte {
 }
 
 // ValidateBasic performs a quick validity check
-func (msg *MsgWriteContractSpecificationRequest) ValidateBasic() error {
+func (msg MsgWriteContractSpecificationRequest) ValidateBasic() error {
 	if len(msg.Signers) < 1 {
 		return fmt.Errorf("at least one signer is required")
 	}
@@ -908,7 +908,7 @@ func (msg MsgAddContractSpecToScopeSpecRequest) GetSignBytes() []byte {
 }
 
 // ValidateBasic performs a quick validity check
-func (msg *MsgAddContractSpecToScopeSpecRequest) ValidateBasic() error {
+func (msg MsgAddContractSpecToScopeSpecRequest) ValidateBasic() error {
 	if !msg.ContractSpecificationId.IsContractSpecificationAddress() {
 		return fmt.Errorf("address is not a contract specification id: %s", msg.ContractSpecificationId.String())
 	}
@@ -1000,7 +1000,7 @@ func (msg MsgWriteRecordSpecificationRequest) GetSignBytes() []byte {
 }
 
 // ValidateBasic performs a quick validity check
-func (msg *MsgWriteRecordSpecificationRequest) ValidateBasic() error {
+func (msg MsgWriteRecordSpecificationRequest) ValidateBasic() error {
 	if len(msg.Signers) < 1 {
 		return fmt.Errorf("at least one signer is required")
 	}

x/metadata/types/scope.go

@@ -34,7 +34,7 @@ func NewScope(
 }
 
 // ValidateBasic performs basic format checking of data within a scope
-func (s *Scope) ValidateBasic() error {
+func (s Scope) ValidateBasic() error {
 	prefix, err := VerifyMetadataAddressFormat(s.ScopeId)
 	if err != nil {
 		return err
@@ -208,7 +208,7 @@ func NewSession(name string, sessionID, contractSpecification MetadataAddress, p
 }
 
 // ValidateBasic performs basic format checking of data within a scope
-func (s *Session) ValidateBasic() error {
+func (s Session) ValidateBasic() error {
 	prefix, err := VerifyMetadataAddressFormat(s.SessionId)
 	if err != nil {
 		return err

x/metadata/types/specification.go

@@ -56,7 +56,7 @@ func NewScopeSpecification(
 }
 
 // ValidateBasic performs basic format checking of data in a ScopeSpecification
-func (s *ScopeSpecification) ValidateBasic() error {
+func (s ScopeSpecification) ValidateBasic() error {
 	prefix, err := VerifyMetadataAddressFormat(s.SpecificationId)
 	if err != nil {
 		return fmt.Errorf("invalid scope specification id: %w", err)
@@ -130,7 +130,7 @@ func NewContractSpecificationSourceHash(hash string) *ContractSpecification_Hash
 }
 
 // ValidateBasic performs basic format checking of data in a ContractSpecification
-func (s *ContractSpecification) ValidateBasic() error {
+func (s ContractSpecification) ValidateBasic() error {
 	prefix, err := VerifyMetadataAddressFormat(s.SpecificationId)
 	if err != nil {
 		return fmt.Errorf("invalid contract specification id: %w", err)
@@ -207,7 +207,7 @@ func NewRecordSpecification(
 }
 
 // ValidateBasic performs basic format checking of data in a RecordSpecification
-func (s *RecordSpecification) ValidateBasic() error {
+func (s RecordSpecification) ValidateBasic() error {
 	prefix, err := VerifyMetadataAddressFormat(s.SpecificationId)
 	if err != nil {
 		return fmt.Errorf("invalid record specification id: %w", err)
@@ -282,7 +282,7 @@ func NewInputSpecificationSourceHash(hash string) *InputSpecification_Hash {
 }
 
 // ValidateBasic performs basic format checking of data in a InputSpecification
-func (s *InputSpecification) ValidateBasic() error {
+func (s InputSpecification) ValidateBasic() error {
 	if len(s.Name) == 0 {
 		return errors.New("input specification name cannot be empty")
 	}
@@ -341,7 +341,7 @@ func NewDescription(name, description, websiteURL, iconURL string) *Description
 // e.g. If the name field is invalid in this description, and the path provided is "ScopeSpecification.Description",
 // the error message will contain "ScopeSpecification.Description.Name" and the problem.
 // Provide "" if there is no context you wish to provide.
-func (d *Description) ValidateBasic(path string) error {
+func (d Description) ValidateBasic(path string) error {
 	if len(d.Name) == 0 {
 		return fmt.Errorf("description %s cannot be empty", makeFieldString(path, "Name"))
 	}

x/name/types/proposal.go

@@ -34,19 +34,19 @@ func NewCreateRootNameProposal(title, description, name string, owner sdk.AccAdd
 }
 
 // GetTitle returns the title of a community pool spend proposal.
-func (crnp *CreateRootNameProposal) GetTitle() string { return crnp.Title }
+func (crnp CreateRootNameProposal) GetTitle() string { return crnp.Title }
 
 // GetDescription returns the description of a community pool spend proposal.
-func (crnp *CreateRootNameProposal) GetDescription() string { return crnp.Description }
+func (crnp CreateRootNameProposal) GetDescription() string { return crnp.Description }
 
 // ProposalRoute returns the routing key of a community pool spend proposal.
-func (crnp *CreateRootNameProposal) ProposalRoute() string { return RouterKey }
+func (crnp CreateRootNameProposal) ProposalRoute() string { return RouterKey }
 
 // ProposalType returns the type of a community pool spend proposal.
-func (crnp *CreateRootNameProposal) ProposalType() string { return ProposalTypeCreateRootName }
+func (crnp CreateRootNameProposal) ProposalType() string { return ProposalTypeCreateRootName }
 
 // ValidateBasic runs basic stateless validity checks
-func (crnp *CreateRootNameProposal) ValidateBasic() error {
+func (crnp CreateRootNameProposal) ValidateBasic() error {
 	err := govtypes.ValidateAbstract(crnp)
 	if err != nil {
 		return err