From 609478565006082c0b16e85890f1913edfdbae23 Mon Sep 17 00:00:00 2001 From: Daniel Wedul <72031080+dwedul-figure@users.noreply.github.com> Date: Fri, 6 Aug 2021 08:07:32 -0600 Subject: [PATCH] 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 | 1 + x/metadata/handler_test.go | 83 +++++++++++++++++++++++++++++++ x/metadata/keeper/msg_server.go | 18 +++++++ x/metadata/types/msg.go | 22 ++++---- x/metadata/types/scope.go | 4 +- x/metadata/types/specification.go | 10 ++-- x/name/types/proposal.go | 10 ++-- 7 files changed, 125 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f356443f37..fd3d662477 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/x/metadata/handler_test.go b/x/metadata/handler_test.go index bd60e774ac..1d41d008ee 100644 --- a/x/metadata/handler_test.go +++ b/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) + }) +} diff --git a/x/metadata/keeper/msg_server.go b/x/metadata/keeper/msg_server.go index 9e9f120d96..96413606b7 100644 --- a/x/metadata/keeper/msg_server.go +++ b/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 diff --git a/x/metadata/types/msg.go b/x/metadata/types/msg.go index 4eca1e937f..fee29235f7 100644 --- a/x/metadata/types/msg.go +++ b/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") } diff --git a/x/metadata/types/scope.go b/x/metadata/types/scope.go index 5f71f69a77..48f42545c6 100644 --- a/x/metadata/types/scope.go +++ b/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 diff --git a/x/metadata/types/specification.go b/x/metadata/types/specification.go index 8834b1854a..471d1023c3 100644 --- a/x/metadata/types/specification.go +++ b/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")) } diff --git a/x/name/types/proposal.go b/x/name/types/proposal.go index 043ea632d0..3d9f7ba2f9 100644 --- a/x/name/types/proposal.go +++ b/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