Repository: hyperledger/firefly
Pull request #813 (2022-05-12 18:48:32)

Return definition rejection reasons as errors
There is always a reason, so return it and let the caller decide how to log it.

I chose to return it in `err` - could also choose to add a new field to `HandlerResult` instead, but this felt logical as it does represent an exceptional/error state.


Patch:
diff --git a/internal/coremsgs/en_error_messages.go b/internal/coremsgs/en_error_messages.go
index 4eb8e68ee..239006fb8 100644
--- a/internal/coremsgs/en_error_messages.go
+++ b/internal/coremsgs/en_error_messages.go
@@ -246,4 +246,15 @@ var (
 	MsgUnrecognizedNetworkAction          = ffe("FF10397", "Unrecognized network action: %s", 400)
 	MsgOverrideExistingFieldCustomOption  = ffe("FF10398", "Cannot override existing field with custom option named '%s'", 400)
 	MsgTerminateNotSupported              = ffe("FF10399", "The 'terminate' operation to mark a switchover of smart contracts is not supported on namespace %s", 400)
+	MsgDefRejectedBadPayload              = ffe("FF10400", "Rejected %s message '%s' - invalid payload")
+	MsgDefRejectedAuthorBlank             = ffe("FF10401", "Rejected %s message '%s' - author is blank")
+	MsgDefRejectedSignatureMismatch       = ffe("FF10402", "Rejected %s message '%s' - signature mismatch")
+	MsgDefRejectedValidateFail            = ffe("FF10403", "Rejected %s '%s' - validate failed: %s")
+	MsgDefRejectedIDMismatch              = ffe("FF10404", "Rejected %s '%s' - ID mismatch with existing record")
+	MsgDefRejectedLocationMismatch        = ffe("FF10405", "Rejected %s '%s' - location mismatch with existing record")
+	MsgDefRejectedSchemaFail              = ffe("FF10406", "Rejected %s '%s' - schema check: %s")
+	MsgDefRejectedConflict                = ffe("FF10407", "Rejected %s '%s' - conflicts with existing: %s")
+	MsgDefRejectedIdentityNotFound        = ffe("FF10408", "Rejected %s '%s' - identity not found: %s")
+	MsgDefRejectedWrongAuthor             = ffe("FF10409", "Rejected %s '%s' - wrong author: %s")
+	MsgDefRejectedHashMismatch            = ffe("FF10410", "Rejected %s '%s' - hash mismatch: %s != %s")
 )
diff --git a/internal/definitions/definition_handler.go b/internal/definitions/definition_handler.go
index 583179101..66ae478ee 100644
--- a/internal/definitions/definition_handler.go
+++ b/internal/definitions/definition_handler.go
@@ -19,6 +19,7 @@ package definitions
 import (
 	"context"
 	"encoding/json"
+	"fmt"
 
 	"github.com/hyperledger/firefly-common/pkg/fftypes"
 	"github.com/hyperledger/firefly-common/pkg/i18n"
@@ -120,7 +121,7 @@ func NewDefinitionHandler(ctx context.Context, di database.Plugin, bi blockchain
 
 func (dh *definitionHandlers) HandleDefinitionBroadcast(ctx context.Context, state DefinitionBatchState, msg *core.Message, data core.DataArray, tx *fftypes.UUID) (msgAction HandlerResult, err error) {
 	l := log.L(ctx)
-	l.Infof("Processing system definition broadcast '%s' [%s]", msg.Header.Tag, msg.Header.ID)
+	l.Infof("Processing system definition '%s' [%s]", msg.Header.Tag, msg.Header.ID)
 	switch msg.Header.Tag {
 	case core.SystemTagDefineDatatype:
 		return dh.handleDatatypeBroadcast(ctx, state, msg, data, tx)
@@ -143,20 +144,19 @@ func (dh *definitionHandlers) HandleDefinitionBroadcast(ctx context.Context, sta
 	case core.SystemTagDefineContractAPI:
 		return dh.handleContractAPIBroadcast(ctx, state, msg, data, tx)
 	default:
-		l.Warnf("Unknown SystemTag '%s' for definition ID '%s'", msg.Header.Tag, msg.Header.ID)
-		return HandlerResult{Action: ActionReject}, nil
+		return HandlerResult{Action: ActionReject}, fmt.Errorf("unknown system tag '%s' for definition ID '%s'", msg.Header.Tag, msg.Header.ID)
 	}
 }
 
 func (dh *definitionHandlers) getSystemBroadcastPayload(ctx context.Context, msg *core.Message, data core.DataArray, res core.Definition) (valid bool) {
 	l := log.L(ctx)
 	if len(data) != 1 {
-		l.Warnf("Unable to process system broadcast %s - expecting 1 attachment, found %d", msg.Header.ID, len(data))
+		l.Warnf("Unable to process system definition %s - expecting 1 attachment, found %d", msg.Header.ID, len(data))
 		return false
 	}
 	err := json.Unmarshal(data[0].Value.Bytes(), &res)
 	if err != nil {
-		l.Warnf("Unable to process system broadcast %s - unmarshal failed: %s", msg.Header.ID, err)
+		l.Warnf("Unable to process system definition %s - unmarshal failed: %s", msg.Header.ID, err)
 		return false
 	}
 	res.SetBroadcastMessage(msg.Header.ID)
diff --git a/internal/definitions/definition_handler_contracts.go b/internal/definitions/definition_handler_contracts.go
index 076cfb197..f6bf550c2 100644
--- a/internal/definitions/definition_handler_contracts.go
+++ b/internal/definitions/definition_handler_contracts.go
@@ -20,87 +20,79 @@ import (
 	"context"
 
 	"github.com/hyperledger/firefly-common/pkg/fftypes"
+	"github.com/hyperledger/firefly-common/pkg/i18n"
 	"github.com/hyperledger/firefly-common/pkg/log"
+	"github.com/hyperledger/firefly/internal/coremsgs"
 	"github.com/hyperledger/firefly/pkg/core"
 	"github.com/hyperledger/firefly/pkg/database"
 )
 
-func (dh *definitionHandlers) persistFFI(ctx context.Context, ffi *core.FFI) (valid bool, err error) {
-	if err := dh.contracts.ValidateFFIAndSetPathnames(ctx, ffi); err != nil {
+func (dh *definitionHandlers) persistFFI(ctx context.Context, ffi *core.FFI) (err error) {
+	if err = dh.contracts.ValidateFFIAndSetPathnames(ctx, ffi); err != nil {
 		log.L(ctx).Warnf("Unable to process FFI %s - validate failed: %s", ffi.ID, err)
-		return false, nil
+		return nil
 	}
 
 	err = dh.database.UpsertFFI(ctx, ffi)
 	if err != nil {
-		return false, err
+		return err
 	}
 
 	for _, method := range ffi.Methods {
 		err := dh.database.UpsertFFIMethod(ctx, method)
 		if err != nil {
-			return false, err
+			return err
 		}
 	}
 
 	for _, event := range ffi.Events {
 		err := dh.database.UpsertFFIEvent(ctx, event)
 		if err != nil {
-			return false, err
+			return err
 		}
 	}
 
-	return true, nil
+	return nil
 }
 
-func (dh *definitionHandlers) persistContractAPI(ctx context.Context, api *core.ContractAPI) (valid bool, err error) {
+func (dh *definitionHandlers) persistContractAPI(ctx context.Context, api *core.ContractAPI) (retry bool, err error) {
 	existing, err := dh.database.GetContractAPIByName(ctx, api.Namespace, api.Name)
 	if err != nil {
-		return false, err // retryable
+		return true, err
 	}
 	if existing != nil {
 		if !api.LocationAndLedgerEquals(existing) {
-			return false, nil // not retryable
+			return false, i18n.NewError(ctx, coremsgs.MsgDefRejectedLocationMismatch, "contract API", api.ID)
 		}
 	}
 	err = dh.database.UpsertContractAPI(ctx, api)
 	if err != nil {
 		if err == database.IDMismatch {
-			log.L(ctx).Errorf("Invalid contract API '%s'. ID mismatch with existing record", api.ID)
-			return false, nil // not retryable
+			return false, i18n.NewError(ctx, coremsgs.MsgDefRejectedIDMismatch, "contract API", api.ID)
 		}
-		log.L(ctx).Errorf("Failed to insert contract API '%s': %s", api.ID, err)
-		return false, err // retryable
+		return true, err
 	}
-	return true, nil
+	return false, nil
 }
 
 func (dh *definitionHandlers) handleFFIBroadcast(ctx context.Context, state DefinitionBatchState, msg *core.Message, data core.DataArray, tx *fftypes.UUID) (HandlerResult, error) {
 	l := log.L(ctx)
-	var broadcast core.FFI
-	valid := dh.getSystemBroadcastPayload(ctx, msg, data, &broadcast)
-	if valid {
-		if validationErr := broadcast.Validate(ctx, true); validationErr != nil {
-			l.Warnf("Unable to process contract definition broadcast %s - validate failed: %s", msg.Header.ID, validationErr)
-			valid = false
-		} else {
-			var err error
-			broadcast.Message = msg.Header.ID
-			valid, err = dh.persistFFI(ctx, &broadcast)
-			if err != nil {
-				return HandlerResult{Action: ActionRetry}, err
-			}
-		}
+	var ffi core.FFI
+	if valid := dh.getSystemBroadcastPayload(ctx, msg, data, &ffi); !valid {
+		return HandlerResult{Action: ActionReject}, i18n.NewError(ctx, coremsgs.MsgDefRejectedBadPayload, "contract interface", msg.Header.ID)
+	}
+	if err := ffi.Validate(ctx, true); err != nil {
+		return HandlerResult{Action: ActionReject}, i18n.NewError(ctx, coremsgs.MsgDefRejectedValidateFail, "contract interface", ffi.ID, err)
 	}
 
-	if !valid {
-		l.Warnf("Contract interface rejected id=%s author=%s", broadcast.ID, msg.Header.Author)
-		return HandlerResult{Action: ActionReject}, nil
+	ffi.Message = msg.Header.ID
+	if err := dh.persistFFI(ctx, &ffi); err != nil {
+		return HandlerResult{Action: ActionRetry}, err
 	}
 
-	l.Infof("Contract interface created id=%s author=%s", broadcast.ID, msg.Header.Author)
+	l.Infof("Contract interface created id=%s author=%s", ffi.ID, msg.Header.Author)
 	state.AddFinalize(func(ctx context.Context) error {
-		event := core.NewEvent(core.EventTypeContractInterfaceConfirmed, broadcast.Namespace, broadcast.ID, tx, broadcast.Topic())
+		event := core.NewEvent(core.EventTypeContractInterfaceConfirmed, ffi.Namespace, ffi.ID, tx, ffi.Topic())
 		return dh.database.InsertEvent(ctx, event)
 	})
 	return HandlerResult{Action: ActionConfirm}, nil
@@ -108,30 +100,25 @@ func (dh *definitionHandlers) handleFFIBroadcast(ctx context.Context, state Defi
 
 func (dh *definitionHandlers) handleContractAPIBroadcast(ctx context.Context, state DefinitionBatchState, msg *core.Message, data core.DataArray, tx *fftypes.UUID) (HandlerResult, error) {
 	l := log.L(ctx)
-	var broadcast core.ContractAPI
-	valid := dh.getSystemBroadcastPayload(ctx, msg, data, &broadcast)
-	if valid {
-		if validateErr := broadcast.Validate(ctx, true); validateErr != nil {
-			l.Warnf("Unable to process contract API broadcast %s - validate failed: %s", msg.Header.ID, validateErr)
-			valid = false
-		} else {
-			var err error
-			broadcast.Message = msg.Header.ID
-			valid, err = dh.persistContractAPI(ctx, &broadcast)
-			if err != nil {
-				return HandlerResult{Action: ActionRetry}, err
-			}
-		}
+	var api core.ContractAPI
+	if valid := dh.getSystemBroadcastPayload(ctx, msg, data, &api); !valid {
+		return HandlerResult{Action: ActionReject}, i18n.NewError(ctx, coremsgs.MsgDefRejectedBadPayload, "contract API", msg.Header.ID)
+	}
+	if err := api.Validate(ctx, true); err != nil {
+		return HandlerResult{Action: ActionReject}, i18n.NewError(ctx, coremsgs.MsgDefRejectedValidateFail, "contract API", api.ID, err)
 	}
 
-	if !valid {
-		l.Warnf("Contract API rejected id=%s author=%s", broadcast.ID, msg.Header.Author)
-		return HandlerResult{Action: ActionReject}, nil
+	api.Message = msg.Header.ID
+	if retry, err := dh.persistContractAPI(ctx, &api); err != nil {
+		if retry {
+			return HandlerResult{Action: ActionRetry}, err
+		}
+		return HandlerResult{Action: ActionReject}, err
 	}
 
-	l.Infof("Contract API created id=%s author=%s", broadcast.ID, msg.Header.Author)
+	l.Infof("Contract API created id=%s author=%s", api.ID, msg.Header.Author)
 	state.AddFinalize(func(ctx context.Context) error {
-		event := core.NewEvent(core.EventTypeContractAPIConfirmed, broadcast.Namespace, broadcast.ID, tx, core.SystemTopicDefinitions)
+		event := core.NewEvent(core.EventTypeContractAPIConfirmed, api.Namespace, api.ID, tx, core.SystemTopicDefinitions)
 		return dh.database.InsertEvent(ctx, event)
 	})
 	return HandlerResult{Action: ActionConfirm}, nil
diff --git a/internal/definitions/definition_handler_datatype.go b/internal/definitions/definition_handler_datatype.go
index 67ad1275d..5da9820e4 100644
--- a/internal/definitions/definition_handler_datatype.go
+++ b/internal/definitions/definition_handler_datatype.go
@@ -20,36 +20,29 @@ import (
 	"context"
 
 	"github.com/hyperledger/firefly-common/pkg/fftypes"
-	"github.com/hyperledger/firefly-common/pkg/log"
+	"github.com/hyperledger/firefly-common/pkg/i18n"
+	"github.com/hyperledger/firefly/internal/coremsgs"
 	"github.com/hyperledger/firefly/pkg/core"
 )
 
 func (dh *definitionHandlers) handleDatatypeBroadcast(ctx context.Context, state DefinitionBatchState, msg *core.Message, data core.DataArray, tx *fftypes.UUID) (HandlerResult, error) {
-	l := log.L(ctx)
-
 	var dt core.Datatype
 	valid := dh.getSystemBroadcastPayload(ctx, msg, data, &dt)
 	if !valid {
-		return HandlerResult{Action: ActionReject}, nil
+		return HandlerResult{Action: ActionReject}, i18n.NewError(ctx, coremsgs.MsgDefRejectedBadPayload, "datatype", msg.Header.ID)
 	}
-
 	if err := dt.Validate(ctx, true); err != nil {
-		l.Warnf("Unable to process datatype broadcast %s - validate failed: %s", msg.Header.ID, err)
-		return HandlerResult{Action: ActionReject}, nil
+		return HandlerResult{Action: ActionReject}, i18n.NewError(ctx, coremsgs.MsgDefRejectedValidateFail, "datatype", dt.ID, err)
 	}
-
 	if err := dh.data.CheckDatatype(ctx, dt.Namespace, &dt); err != nil {
-		l.Warnf("Unable to process datatype broadcast %s - schema check: %s", msg.Header.ID, err)
-		return HandlerResult{Action: ActionReject}, nil
+		return HandlerResult{Action: ActionReject}, i18n.NewError(ctx, coremsgs.MsgDefRejectedSchemaFail, "datatype", dt.ID, err)
 	}
 
 	existing, err := dh.database.GetDatatypeByName(ctx, dt.Namespace, dt.Name, dt.Version)
 	if err != nil {
-		return HandlerResult{Action: ActionRetry}, err // We only return database errors
-	}
-	if existing != nil {
-		l.Warnf("Unable to process datatype broadcast %s (%s:%s) - duplicate of %v", msg.Header.ID, dt.Namespace, dt, existing.ID)
-		return HandlerResult{Action: ActionReject}, nil
+		return HandlerResult{Action: ActionRetry}, err
+	} else if existing != nil {
+		return HandlerResult{Action: ActionReject}, i18n.NewError(ctx, coremsgs.MsgDefRejectedConflict, "datatype", dt.ID, existing.ID)
 	}
 
 	if err = dh.database.UpsertDatatype(ctx, &dt, false); err != nil {
diff --git a/internal/definitions/definition_handler_identity_claim.go b/internal/definitions/definition_handler_identity_claim.go
index c6d48640a..c4eed5466 100644
--- a/internal/definitions/definition_handler_identity_claim.go
+++ b/internal/definitions/definition_handler_identity_claim.go
@@ -21,27 +21,25 @@ import (
 	"fmt"
 
 	"github.com/hyperledger/firefly-common/pkg/fftypes"
+	"github.com/hyperledger/firefly-common/pkg/i18n"
 	"github.com/hyperledger/firefly-common/pkg/log"
+	"github.com/hyperledger/firefly/internal/coremsgs"
 	"github.com/hyperledger/firefly/pkg/core"
 	"github.com/hyperledger/firefly/pkg/database"
 )
 
 func (dh *definitionHandlers) handleIdentityClaimBroadcast(ctx context.Context, state DefinitionBatchState, msg *core.Message, data core.DataArray, verificationID *fftypes.UUID) (HandlerResult, error) {
 	var claim core.IdentityClaim
-	valid := dh.getSystemBroadcastPayload(ctx, msg, data, &claim)
-	if !valid {
-		return HandlerResult{Action: ActionReject}, nil
+	if valid := dh.getSystemBroadcastPayload(ctx, msg, data, &claim); !valid {
+		return HandlerResult{Action: ActionReject}, i18n.NewError(ctx, coremsgs.MsgDefRejectedBadPayload, "identity claim", msg.Header.ID)
 	}
-
 	return dh.handleIdentityClaim(ctx, state, msg, &claim, verificationID)
-
 }
 
-func (dh *definitionHandlers) verifyClaimSignature(ctx context.Context, msg *core.Message, identity *core.Identity, parent *core.Identity) (valid bool) {
-
+func (dh *definitionHandlers) verifyClaimSignature(ctx context.Context, msg *core.Message, identity *core.Identity, parent *core.Identity) error {
 	author := msg.Header.Author
 	if author == "" {
-		return false
+		return i18n.NewError(ctx, coremsgs.MsgDefRejectedAuthorBlank, "identity claim", msg.Header.ID)
 	}
 
 	var expectedSigner *core.Identity
@@ -53,12 +51,13 @@ func (dh *definitionHandlers) verifyClaimSignature(ctx context.Context, msg *cor
 		expectedSigner = identity
 	}
 
-	valid = author == expectedSigner.DID ||
+	valid := author == expectedSigner.DID ||
 		(expectedSigner.Type == core.IdentityTypeOrg && author == fmt.Sprintf("%s%s", core.FireFlyOrgDIDPrefix, expectedSigner.ID))
 	if !valid {
-		log.L(ctx).Warnf("Unable to process identity claim %s - signature mismatch type=%s author=%s expected=%s", msg.Header.ID, identity.Type, author, expectedSigner.DID)
+		log.L(ctx).Errorf("unable to process identity claim %s - signature mismatch type=%s author=%s expected=%s", msg.Header.ID, identity.Type, author, expectedSigner.DID)
+		return i18n.NewError(ctx, coremsgs.MsgDefRejectedSignatureMismatch, "identity claim", msg.Header.ID)
 	}
-	return valid
+	return nil
 }
 
 func (dh *definitionHandlers) getClaimVerifier(msg *core.Message, identity *core.Identity) *core.Verifier {
@@ -144,8 +143,8 @@ func (dh *definitionHandlers) handleIdentityClaim(ctx context.Context, state Def
 	}
 
 	// Check signature verification
-	if !dh.verifyClaimSignature(ctx, msg, identity, parent) {
-		return HandlerResult{Action: ActionReject}, nil
+	if err := dh.verifyClaimSignature(ctx, msg, identity, parent); err != nil {
+		return HandlerResult{Action: ActionReject}, err
 	}
 
 	existingIdentity, err := dh.database.GetIdentityByName(ctx, identity.Type, identity.Namespace, identity.Name)
@@ -157,8 +156,7 @@ func (dh *definitionHandlers) handleIdentityClaim(ctx context.Context, state Def
 	}
 	if existingIdentity != nil && !existingIdentity.IdentityBase.Equals(ctx, &identity.IdentityBase) {
 		// If the existing one matches - this is just idempotent replay. No action needed, just confirm
-		l.Warnf("Unable to process identity claim %s - conflict with existing: %v", msg.Header.ID, existingIdentity.ID)
-		return HandlerResult{Action: ActionReject}, nil
+		return HandlerResult{Action: ActionReject}, i18n.NewError(ctx, coremsgs.MsgDefRejectedConflict, "identity claim", identity.ID, existingIdentity.ID)
 	}
 
 	// Check uniqueness of verifier
@@ -168,8 +166,9 @@ func (dh *definitionHandlers) handleIdentityClaim(ctx context.Context, state Def
 		return HandlerResult{Action: ActionRetry}, err // retry database errors
 	}
 	if existingVerifier != nil && !existingVerifier.Identity.Equals(identity.ID) {
-		log.L(ctx).Warnf("Unable to process identity claim %s - verifier type=%s value=%s already registered: %v", msg.Header.ID, verifier.Type, verifier.Value, existingVerifier.Hash)
-		return HandlerResult{Action: ActionReject}, nil
+		verifierLabel := fmt.Sprintf("%s:%s", verifier.Type, verifier.Value)
+		existingVerifierLabel := fmt.Sprintf("%s:%s", verifier.Type, verifier.Value)
+		return HandlerResult{Action: ActionReject}, i18n.NewError(ctx, coremsgs.MsgDefRejectedConflict, "identity verifier", verifierLabel, existingVerifierLabel)
 	}
 
 	if parent != nil && identity.Type != core.IdentityTypeNode {
diff --git a/internal/definitions/definition_handler_identity_update.go b/internal/definitions/definition_handler_identity_update.go
index e79f0ccf8..627ff98b7 100644
--- a/internal/definitions/definition_handler_identity_update.go
+++ b/internal/definitions/definition_handler_identity_update.go
@@ -19,39 +19,33 @@ package definitions
 import (
 	"context"
 
-	"github.com/hyperledger/firefly-common/pkg/log"
+	"github.com/hyperledger/firefly-common/pkg/i18n"
+	"github.com/hyperledger/firefly/internal/coremsgs"
 	"github.com/hyperledger/firefly/pkg/core"
 	"github.com/hyperledger/firefly/pkg/database"
 )
 
 func (dh *definitionHandlers) handleIdentityUpdateBroadcast(ctx context.Context, state DefinitionBatchState, msg *core.Message, data core.DataArray) (HandlerResult, error) {
 	var update core.IdentityUpdate
-	valid := dh.getSystemBroadcastPayload(ctx, msg, data, &update)
-	if !valid {
-		return HandlerResult{Action: ActionReject}, nil
+	if valid := dh.getSystemBroadcastPayload(ctx, msg, data, &update); !valid {
+		return HandlerResult{Action: ActionReject}, i18n.NewError(ctx, coremsgs.MsgDefRejectedBadPayload, "identity update", msg.Header.ID)
 	}
-
-	// See if we find the message to which it refers
-	err := update.Identity.Validate(ctx)
-	if err != nil {
-		log.L(ctx).Warnf("Invalid identity update message %s: %v", msg.Header.ID, err)
-		return HandlerResult{Action: ActionReject}, nil
+	if err := update.Identity.Validate(ctx); err != nil {
+		return HandlerResult{Action: ActionReject}, i18n.NewError(ctx, coremsgs.MsgDefRejectedValidateFail, "identity update", msg.Header.ID, err)
 	}
 
-	// Get the existing identity (must be a confirmed identity to at the point an update is issued)
+	// Get the existing identity (must be a confirmed identity at the point an update is issued)
 	identity, err := dh.identity.CachedIdentityLookupByID(ctx, update.Identity.ID)
 	if err != nil {
 		return HandlerResult{Action: ActionRetry}, err
 	}
 	if identity == nil {
-		log.L(ctx).Warnf("Invalid identity update message %s - not found: %s", msg.Header.ID, update.Identity.ID)
-		return HandlerResult{Action: ActionReject}, nil
+		return HandlerResult{Action: ActionReject}, i18n.NewError(ctx, coremsgs.MsgDefRejectedIdentityNotFound, "identity update", msg.Header.ID, update.Identity.ID)
 	}
 
 	// Check the author matches
 	if identity.DID != msg.Header.Author {
-		log.L(ctx).Warnf("Invalid identity update message %s - wrong author: %s", msg.Header.ID, msg.Header.Author)
-		return HandlerResult{Action: ActionReject}, nil
+		return HandlerResult{Action: ActionReject}, i18n.NewError(ctx, coremsgs.MsgDefRejectedWrongAuthor, "identity update", msg.Header.ID, msg.Header.Author)
 	}
 
 	// Update the profile
diff --git a/internal/definitions/definition_handler_identity_verification.go b/internal/definitions/definition_handler_identity_verification.go
index 204c79bc7..41a0f2f13 100644
--- a/internal/definitions/definition_handler_identity_verification.go
+++ b/internal/definitions/definition_handler_identity_verification.go
@@ -19,7 +19,8 @@ package definitions
 import (
 	"context"
 
-	"github.com/hyperledger/firefly-common/pkg/log"
+	"github.com/hyperledger/firefly-common/pkg/i18n"
+	"github.com/hyperledger/firefly/internal/coremsgs"
 	"github.com/hyperledger/firefly/pkg/core"
 )
 
@@ -27,14 +28,11 @@ func (dh *definitionHandlers) handleIdentityVerificationBroadcast(ctx context.Co
 	var verification core.IdentityVerification
 	valid := dh.getSystemBroadcastPayload(ctx, verifyMsg, data, &verification)
 	if !valid {
-		return HandlerResult{Action: ActionReject}, nil
+		return HandlerResult{Action: ActionReject}, i18n.NewError(ctx, coremsgs.MsgDefRejectedBadPayload, "identity verification", verifyMsg.Header.ID)
 	}
-
-	// See if we find the message to which it refers
 	err := verification.Identity.Validate(ctx)
 	if err != nil || verification.Identity.Parent == nil || verification.Claim.ID == nil || verification.Claim.Hash == nil {
-		log.L(ctx).Warnf("Invalid verification message %s: %v", verifyMsg.Header.ID, err)
-		return HandlerResult{Action: ActionReject}, nil
+		return HandlerResult{Action: ActionReject}, i18n.NewError(ctx, coremsgs.MsgDefRejectedValidateFail, "identity verification", verifyMsg.Header.ID, err)
 	}
 
 	// Check the verification is signed by the correct org
@@ -43,12 +41,10 @@ func (dh *definitionHandlers) handleIdentityVerificationBroadcast(ctx context.Co
 		return HandlerResult{Action: ActionRetry}, err
 	}
 	if parent == nil {
-		log.L(ctx).Warnf("Invalid verification message %s - parent not found: %s", verifyMsg.Header.ID, verification.Identity.Parent)
-		return HandlerResult{Action: ActionReject}, nil
+		return HandlerResult{Action: ActionReject}, i18n.NewError(ctx, coremsgs.MsgDefRejectedIdentityNotFound, "identity verification", verifyMsg.Header.ID, verification.Identity.Parent)
 	}
 	if parent.DID != verifyMsg.Header.Author {
-		log.L(ctx).Warnf("Invalid verification message %s - parent '%s' does not match signer '%s'", verifyMsg.Header.ID, parent.DID, verifyMsg.Header.Author)
-		return HandlerResult{Action: ActionReject}, nil
+		return HandlerResult{Action: ActionReject}, i18n.NewError(ctx, coremsgs.MsgDefRejectedWrongAuthor, "identity verification", verifyMsg.Header.ID, verifyMsg.Header.Author)
 	}
 
 	// At this point, this is a valid verification, but we don't know if the claim has arrived.
@@ -65,8 +61,7 @@ func (dh *definitionHandlers) handleIdentityVerificationBroadcast(ctx context.Co
 
 	if claimMsg != nil {
 		if !claimMsg.Hash.Equals(verification.Claim.Hash) {
-			log.L(ctx).Warnf("Invalid verification message %s - hash mismatch claim=%s verification=%s", verifyMsg.Header.ID, claimMsg.Hash, verification.Claim.Hash)
-			return HandlerResult{Action: ActionReject}, nil
+			return HandlerResult{Action: ActionReject}, i18n.NewError(ctx, coremsgs.MsgDefRejectedHashMismatch, "identity verification", verifyMsg.Header.ID, claimMsg.Hash, verification.Claim.Hash)
 		}
 		data, foundAll, err := dh.data.GetMessageDataCached(ctx, claimMsg)
 		if err != nil {
diff --git a/internal/definitions/definition_handler_namespace.go b/internal/definitions/definition_handler_namespace.go
index 7c7486118..39b951713 100644
--- a/internal/definitions/definition_handler_namespace.go
+++ b/internal/definitions/definition_handler_namespace.go
@@ -20,31 +20,27 @@ import (
 	"context"
 
 	"github.com/hyperledger/firefly-common/pkg/fftypes"
-	"github.com/hyperledger/firefly-common/pkg/log"
+	"github.com/hyperledger/firefly-common/pkg/i18n"
+	"github.com/hyperledger/firefly/internal/coremsgs"
 	"github.com/hyperledger/firefly/pkg/core"
 )
 
 func (dh *definitionHandlers) handleNamespaceBroadcast(ctx context.Context, state DefinitionBatchState, msg *core.Message, data core.DataArray, tx *fftypes.UUID) (HandlerResult, error) {
-	l := log.L(ctx)
-
 	var ns core.Namespace
-	valid := dh.getSystemBroadcastPayload(ctx, msg, data, &ns)
-	if !valid {
-		return HandlerResult{Action: ActionReject}, nil
+	if valid := dh.getSystemBroadcastPayload(ctx, msg, data, &ns); !valid {
+		return HandlerResult{Action: ActionReject}, i18n.NewError(ctx, coremsgs.MsgDefRejectedBadPayload, "namespace", msg.Header.ID)
 	}
 	if err := ns.Validate(ctx, true); err != nil {
-		l.Warnf("Unable to process namespace broadcast %s - validate failed: %s", msg.Header.ID, err)
-		return HandlerResult{Action: ActionReject}, nil
+		return HandlerResult{Action: ActionReject}, i18n.NewError(ctx, coremsgs.MsgDefRejectedValidateFail, "namespace", ns.ID, err)
 	}
 
 	existing, err := dh.database.GetNamespace(ctx, ns.Name)
 	if err != nil {
-		return HandlerResult{Action: ActionRetry}, err // We only return database errors
+		return HandlerResult{Action: ActionRetry}, err
 	}
 	if existing != nil {
 		if existing.Type != core.NamespaceTypeLocal {
-			l.Warnf("Unable to process namespace broadcast %s (name=%s) - duplicate of %v", msg.Header.ID, existing.Name, existing.ID)
-			return HandlerResult{Action: ActionReject}, nil
+			return HandlerResult{Action: ActionReject}, i18n.NewError(ctx, coremsgs.MsgDefRejectedConflict, "namespace", ns.ID, existing.ID)
 		}
 		// Remove the local definition
 		if err = dh.database.DeleteNamespace(ctx, existing.ID); err != nil {
diff --git a/internal/definitions/definition_handler_network_node.go b/internal/definitions/definition_handler_network_node.go
index 10587d74d..a019b6de4 100644
--- a/internal/definitions/definition_handler_network_node.go
+++ b/internal/definitions/definition_handler_network_node.go
@@ -19,17 +19,15 @@ package definitions
 import (
 	"context"
 
-	"github.com/hyperledger/firefly-common/pkg/log"
+	"github.com/hyperledger/firefly-common/pkg/i18n"
+	"github.com/hyperledger/firefly/internal/coremsgs"
 	"github.com/hyperledger/firefly/pkg/core"
 )
 
 func (dh *definitionHandlers) handleDeprecatedNodeBroadcast(ctx context.Context, state DefinitionBatchState, msg *core.Message, data core.DataArray) (HandlerResult, error) {
-	l := log.L(ctx)
-
 	var nodeOld core.DeprecatedNode
-	valid := dh.getSystemBroadcastPayload(ctx, msg, data, &nodeOld)
-	if !valid {
-		return HandlerResult{Action: ActionReject}, nil
+	if valid := dh.getSystemBroadcastPayload(ctx, msg, data, &nodeOld); !valid {
+		return HandlerResult{Action: ActionReject}, i18n.NewError(ctx, coremsgs.MsgDefRejectedBadPayload, "node", msg.Header.ID)
 	}
 
 	owner, err := dh.identity.FindIdentityForVerifier(ctx, []core.IdentityType{core.IdentityTypeOrg}, core.LegacySystemNamespace, &core.VerifierRef{
@@ -40,8 +38,7 @@ func (dh *definitionHandlers) handleDeprecatedNodeBroadcast(ctx context.Context,
 		return HandlerResult{Action: ActionRetry}, err // We only return database errors
 	}
 	if owner == nil {
-		l.Warnf("Unable to process node broadcast %s - parent identity not found: %s", msg.Header.ID, nodeOld.Owner)
-		return HandlerResult{Action: ActionReject}, nil
+		return HandlerResult{Action: ActionReject}, i18n.NewError(ctx, coremsgs.MsgDefRejectedIdentityNotFound, "node", nodeOld.ID, nodeOld.Owner)
 	}
 
 	return dh.handleIdentityClaim(ctx, state, msg, nodeOld.AddMigratedParent(owner.ID), nil)
diff --git a/internal/definitions/definition_handler_network_org.go b/internal/definitions/definition_handler_network_org.go
index 298c63b8e..a3328a49a 100644
--- a/internal/definitions/definition_handler_network_org.go
+++ b/internal/definitions/definition_handler_network_org.go
@@ -19,6 +19,8 @@ package definitions
 import (
 	"context"
 
+	"github.com/hyperledger/firefly-common/pkg/i18n"
+	"github.com/hyperledger/firefly/internal/coremsgs"
 	"github.com/hyperledger/firefly/pkg/core"
 )
 
@@ -27,7 +29,7 @@ func (dh *definitionHandlers) handleDeprecatedOrganizationBroadcast(ctx context.
 	var orgOld core.DeprecatedOrganization
 	valid := dh.getSystemBroadcastPayload(ctx, msg, data, &orgOld)
 	if !valid {
-		return HandlerResult{Action: ActionReject}, nil
+		return HandlerResult{Action: ActionReject}, i18n.NewError(ctx, coremsgs.MsgDefRejectedBadPayload, "org", msg.Header.ID)
 	}
 
 	return dh.handleIdentityClaim(ctx, state, msg, orgOld.Migrated(), nil)
diff --git a/internal/definitions/definition_handler_tokenpool.go b/internal/definitions/definition_handler_tokenpool.go
index 9ff841e42..c3607c70f 100644
--- a/internal/definitions/definition_handler_tokenpool.go
+++ b/internal/definitions/definition_handler_tokenpool.go
@@ -19,30 +19,17 @@ package definitions
 import (
 	"context"
 
+	"github.com/hyperledger/firefly-common/pkg/i18n"
 	"github.com/hyperledger/firefly-common/pkg/log"
+	"github.com/hyperledger/firefly/internal/coremsgs"
 	"github.com/hyperledger/firefly/pkg/core"
 	"github.com/hyperledger/firefly/pkg/database"
 )
 
-func (dh *definitionHandlers) persistTokenPool(ctx context.Context, announce *core.TokenPoolAnnouncement) (valid bool, err error) {
-	pool := announce.Pool
-	pool.State = core.TokenPoolStatePending
-	err = dh.database.UpsertTokenPool(ctx, pool)
-	if err != nil {
-		if err == database.IDMismatch {
-			log.L(ctx).Errorf("Invalid token pool '%s'. ID mismatch with existing record", pool.ID)
-			return false, nil // not retryable
-		}
-		log.L(ctx).Errorf("Failed to insert token pool '%s': %s", pool.ID, err)
-		return false, err // retryable
-	}
-	return true, nil
-}
-
 func (dh *definitionHandlers) handleTokenPoolBroadcast(ctx context.Context, state DefinitionBatchState, msg *core.Message, data core.DataArray) (HandlerResult, error) {
 	var announce core.TokenPoolAnnouncement
 	if valid := dh.getSystemBroadcastPayload(ctx, msg, data, &announce); !valid {
-		return HandlerResult{Action: ActionReject}, nil
+		return HandlerResult{Action: ActionReject}, i18n.NewError(ctx, coremsgs.MsgDefRejectedBadPayload, "token pool", msg.Header.ID)
 	}
 
 	pool := announce.Pool
@@ -53,8 +40,7 @@ func (dh *definitionHandlers) handleTokenPoolBroadcast(ctx context.Context, stat
 	correlator := pool.ID
 
 	if err := pool.Validate(ctx); err != nil {
-		log.L(ctx).Warnf("Token pool '%s' rejected - validate failed: %s", pool.ID, err)
-		return HandlerResult{Action: ActionReject, CustomCorrelator: correlator}, nil
+		return HandlerResult{Action: ActionReject, CustomCorrelator: correlator}, i18n.NewError(ctx, coremsgs.MsgDefRejectedValidateFail, "token pool", pool.ID, err)
 	}
 
 	// Check if pool has already been confirmed on chain (and confirm the message if so)
@@ -65,10 +51,12 @@ func (dh *definitionHandlers) handleTokenPoolBroadcast(ctx context.Context, stat
 	}
 
 	// Create the pool in pending state
-	if valid, err := dh.persistTokenPool(ctx, &announce); err != nil {
+	pool.State = core.TokenPoolStatePending
+	if err := dh.database.UpsertTokenPool(ctx, pool); err != nil {
+		if err == database.IDMismatch {
+			return HandlerResult{Action: ActionReject, CustomCorrelator: correlator}, i18n.NewError(ctx, coremsgs.MsgDefRejectedIDMismatch, "token pool", pool.ID)
+		}
 		return HandlerResult{Action: ActionRetry}, err
-	} else if !valid {
-		return HandlerResult{Action: ActionReject, CustomCorrelator: correlator}, nil
 	}
 
 	// Message will remain unconfirmed, but plugin will be notified to activate the pool
diff --git a/internal/events/aggregator.go b/internal/events/aggregator.go
index 611c131da..ac1d7c169 100644
--- a/internal/events/aggregator.go
+++ b/internal/events/aggregator.go
@@ -539,6 +539,9 @@ func (ag *aggregator) attemptMessageDispatch(ctx context.Context, msg *core.Mess
 		if handlerResult.Action == definitions.ActionWait {
 			return "", false, nil
 		}
+		if handlerResult.Action == definitions.ActionReject {
+			log.L(ctx).Warnf("Definition broadcast rejected: %s", err)
+		}
 		customCorrelator = handlerResult.CustomCorrelator
 		valid = handlerResult.Action == definitions.ActionConfirm
 


Test patch:
diff --git a/internal/definitions/definition_handler_contracts_test.go b/internal/definitions/definition_handler_contracts_test.go
index e84d2cb1b..cba38a783 100644
--- a/internal/definitions/definition_handler_contracts_test.go
+++ b/internal/definitions/definition_handler_contracts_test.go
@@ -120,9 +120,8 @@ func TestPersistFFIValidateFFIFail(t *testing.T) {
 	dh, _ := newTestDefinitionHandler(t)
 	mcm := dh.contracts.(*contractmocks.Manager)
 	mcm.On("ValidateFFIAndSetPathnames", mock.Anything, mock.Anything).Return(fmt.Errorf("pop"))
-	valid, err := dh.persistFFI(context.Background(), testFFI())
+	err := dh.persistFFI(context.Background(), testFFI())
 	assert.NoError(t, err)
-	assert.False(t, valid)
 	mcm.AssertExpectations(t)
 }
 
@@ -138,7 +137,7 @@ func TestHandleFFIBroadcastReject(t *testing.T) {
 		},
 	}, core.DataArray{}, fftypes.NewUUID())
 	assert.Equal(t, HandlerResult{Action: ActionReject}, action)
-	assert.NoError(t, err)
+	assert.Error(t, err)
 	bs.assertNoFinalizers()
 }
 
@@ -148,7 +147,7 @@ func TestPersistFFIUpsertFFIFail(t *testing.T) {
 	mbi.On("UpsertFFI", mock.Anything, mock.Anything).Return(fmt.Errorf("pop"))
 	mcm := dh.contracts.(*contractmocks.Manager)
 	mcm.On("ValidateFFIAndSetPathnames", mock.Anything, mock.Anything).Return(nil)
-	_, err := dh.persistFFI(context.Background(), testFFI())
+	err := dh.persistFFI(context.Background(), testFFI())
 	assert.Regexp(t, "pop", err)
 	mbi.AssertExpectations(t)
 	mcm.AssertExpectations(t)
@@ -161,7 +160,7 @@ func TestPersistFFIUpsertFFIMethodFail(t *testing.T) {
 	mbi.On("UpsertFFIMethod", mock.Anything, mock.Anything).Return(fmt.Errorf("pop"))
 	mcm := dh.contracts.(*contractmocks.Manager)
 	mcm.On("ValidateFFIAndSetPathnames", mock.Anything, mock.Anything).Return(nil)
-	_, err := dh.persistFFI(context.Background(), testFFI())
+	err := dh.persistFFI(context.Background(), testFFI())
 	assert.Regexp(t, "pop", err)
 	mbi.AssertExpectations(t)
 	mcm.AssertExpectations(t)
@@ -175,7 +174,7 @@ func TestPersistFFIUpsertFFIEventFail(t *testing.T) {
 	mbi.On("UpsertFFIEvent", mock.Anything, mock.Anything).Return(fmt.Errorf("pop"))
 	mcm := dh.contracts.(*contractmocks.Manager)
 	mcm.On("ValidateFFIAndSetPathnames", mock.Anything, mock.Anything).Return(nil)
-	_, err := dh.persistFFI(context.Background(), testFFI())
+	err := dh.persistFFI(context.Background(), testFFI())
 	assert.Regexp(t, "pop", err)
 	mbi.AssertExpectations(t)
 	mcm.AssertExpectations(t)
@@ -198,7 +197,7 @@ func TestHandleFFIBroadcastValidateFail(t *testing.T) {
 		},
 	}, core.DataArray{data}, fftypes.NewUUID())
 	assert.Equal(t, HandlerResult{Action: ActionReject}, action)
-	assert.NoError(t, err)
+	assert.Error(t, err)
 	bs.assertNoFinalizers()
 }
 
@@ -250,6 +249,44 @@ func TestHandleContractAPIBroadcastOk(t *testing.T) {
 	mbi.AssertExpectations(t)
 }
 
+func TestHandleContractAPIBadPayload(t *testing.T) {
+	dh, bs := newTestDefinitionHandler(t)
+	data := &core.Data{
+		Value: fftypes.JSONAnyPtr("bad"),
+	}
+
+	action, err := dh.HandleDefinitionBroadcast(context.Background(), bs, &core.Message{
+		Header: core.MessageHeader{
+			Tag: core.SystemTagDefineContractAPI,
+		},
+	}, core.DataArray{data}, fftypes.NewUUID())
+	assert.Equal(t, HandlerResult{Action: ActionReject}, action)
+	assert.Regexp(t, "FF10400", err)
+}
+
+func TestHandleContractAPIIDMismatch(t *testing.T) {
+	dh, bs := newTestDefinitionHandler(t)
+
+	b, err := json.Marshal(testFFI())
+	assert.NoError(t, err)
+	data := &core.Data{
+		Value: fftypes.JSONAnyPtrBytes(b),
+	}
+
+	mbi := dh.database.(*databasemocks.Plugin)
+	mbi.On("GetContractAPIByName", mock.Anything, mock.Anything, mock.Anything).Return(nil, nil)
+	mbi.On("UpsertContractAPI", mock.Anything, mock.Anything, mock.Anything).Return(database.IDMismatch)
+
+	action, err := dh.HandleDefinitionBroadcast(context.Background(), bs, &core.Message{
+		Header: core.MessageHeader{
+			Tag: core.SystemTagDefineContractAPI,
+		},
+	}, core.DataArray{data}, fftypes.NewUUID())
+	assert.Equal(t, HandlerResult{Action: ActionReject}, action)
+	assert.Regexp(t, "FF10404", err)
+	mbi.AssertExpectations(t)
+}
+
 func TestPersistContractAPIGetFail(t *testing.T) {
 	dh, _ := newTestDefinitionHandler(t)
 	mbi := dh.database.(*databasemocks.Plugin)
@@ -265,9 +302,9 @@ func TestPersistContractAPIDifferentLocation(t *testing.T) {
 	dh, _ := newTestDefinitionHandler(t)
 	mbi := dh.database.(*databasemocks.Plugin)
 	mbi.On("GetContractAPIByName", mock.Anything, mock.Anything, mock.Anything).Return(existing, nil)
-	valid, err := dh.persistContractAPI(context.Background(), testContractAPI())
-	assert.False(t, valid)
-	assert.NoError(t, err)
+	retry, err := dh.persistContractAPI(context.Background(), testContractAPI())
+	assert.False(t, retry)
+	assert.Regexp(t, "FF10405", err)
 	mbi.AssertExpectations(t)
 }
 
@@ -281,17 +318,6 @@ func TestPersistContractAPIUpsertFail(t *testing.T) {
 	mbi.AssertExpectations(t)
 }
 
-func TestPersistContractAPIIDMismatch(t *testing.T) {
-	dh, _ := newTestDefinitionHandler(t)
-	mbi := dh.database.(*databasemocks.Plugin)
-	mbi.On("GetContractAPIByName", mock.Anything, mock.Anything, mock.Anything).Return(nil, nil)
-	mbi.On("UpsertContractAPI", mock.Anything, mock.Anything, mock.Anything).Return(database.IDMismatch)
-	valid, err := dh.persistContractAPI(context.Background(), testContractAPI())
-	assert.False(t, valid)
-	assert.NoError(t, err)
-	mbi.AssertExpectations(t)
-}
-
 func TestHandleContractAPIBroadcastValidateFail(t *testing.T) {
 	dh, bs := newTestDefinitionHandler(t)
 	api := testContractAPI()
@@ -309,7 +335,7 @@ func TestHandleContractAPIBroadcastValidateFail(t *testing.T) {
 		},
 	}, core.DataArray{data}, fftypes.NewUUID())
 	assert.Equal(t, HandlerResult{Action: ActionReject}, action)
-	assert.NoError(t, err)
+	assert.Error(t, err)
 	bs.assertNoFinalizers()
 }
 
diff --git a/internal/definitions/definition_handler_datatype_test.go b/internal/definitions/definition_handler_datatype_test.go
index ec3ee2980..cdbb4f132 100644
--- a/internal/definitions/definition_handler_datatype_test.go
+++ b/internal/definitions/definition_handler_datatype_test.go
@@ -129,7 +129,7 @@ func TestHandleDefinitionBroadcastDatatypeMissingID(t *testing.T) {
 		},
 	}, core.DataArray{data}, fftypes.NewUUID())
 	assert.Equal(t, HandlerResult{Action: ActionReject}, action)
-	assert.NoError(t, err)
+	assert.Error(t, err)
 	bs.assertNoFinalizers()
 }
 
@@ -159,7 +159,7 @@ func TestHandleDefinitionBroadcastBadSchema(t *testing.T) {
 		},
 	}, core.DataArray{data}, fftypes.NewUUID())
 	assert.Equal(t, HandlerResult{Action: ActionReject}, action)
-	assert.NoError(t, err)
+	assert.Error(t, err)
 
 	mdm.AssertExpectations(t)
 	bs.assertNoFinalizers()
@@ -184,7 +184,7 @@ func TestHandleDefinitionBroadcastMissingData(t *testing.T) {
 		},
 	}, core.DataArray{}, fftypes.NewUUID())
 	assert.Equal(t, HandlerResult{Action: ActionReject}, action)
-	assert.NoError(t, err)
+	assert.Error(t, err)
 	bs.assertNoFinalizers()
 }
 
@@ -288,7 +288,7 @@ func TestHandleDefinitionBroadcastDatatypeDuplicate(t *testing.T) {
 		},
 	}, core.DataArray{data}, fftypes.NewUUID())
 	assert.Equal(t, HandlerResult{Action: ActionReject}, action)
-	assert.NoError(t, err)
+	assert.Error(t, err)
 
 	mdm.AssertExpectations(t)
 	mbi.AssertExpectations(t)
diff --git a/internal/definitions/definition_handler_identity_claim_test.go b/internal/definitions/definition_handler_identity_claim_test.go
index eee45737f..b106c1a87 100644
--- a/internal/definitions/definition_handler_identity_claim_test.go
+++ b/internal/definitions/definition_handler_identity_claim_test.go
@@ -423,7 +423,7 @@ func TestHandleDefinitionIdentityClaimVerifierClash(t *testing.T) {
 
 	action, err := dh.HandleDefinitionBroadcast(ctx, bs, claimMsg, core.DataArray{claimData}, fftypes.NewUUID())
 	assert.Equal(t, HandlerResult{Action: ActionReject}, action)
-	assert.NoError(t, err)
+	assert.Error(t, err)
 
 	mim.AssertExpectations(t)
 	mdi.AssertExpectations(t)
@@ -471,7 +471,7 @@ func TestHandleDefinitionIdentityClaimIdentityClash(t *testing.T) {
 
 	action, err := dh.HandleDefinitionBroadcast(ctx, bs, claimMsg, core.DataArray{claimData}, fftypes.NewUUID())
 	assert.Equal(t, HandlerResult{Action: ActionReject}, action)
-	assert.NoError(t, err)
+	assert.Error(t, err)
 
 	mim.AssertExpectations(t)
 	mdi.AssertExpectations(t)
@@ -512,7 +512,7 @@ func TestHandleDefinitionIdentityMissingAuthor(t *testing.T) {
 
 	action, err := dh.HandleDefinitionBroadcast(ctx, bs, claimMsg, core.DataArray{claimData}, fftypes.NewUUID())
 	assert.Equal(t, HandlerResult{Action: ActionReject}, action)
-	assert.NoError(t, err)
+	assert.Error(t, err)
 
 	mim.AssertExpectations(t)
 	bs.assertNoFinalizers()
@@ -530,7 +530,7 @@ func TestHandleDefinitionIdentityClaimBadSignature(t *testing.T) {
 
 	action, err := dh.HandleDefinitionBroadcast(ctx, bs, claimMsg, core.DataArray{claimData}, fftypes.NewUUID())
 	assert.Equal(t, HandlerResult{Action: ActionReject}, action)
-	assert.NoError(t, err)
+	assert.Error(t, err)
 
 	mim.AssertExpectations(t)
 	bs.assertNoFinalizers()
@@ -581,7 +581,7 @@ func TestHandleDefinitionIdentityClaimBadData(t *testing.T) {
 
 	action, err := dh.HandleDefinitionBroadcast(ctx, bs, claimMsg, core.DataArray{}, fftypes.NewUUID())
 	assert.Equal(t, HandlerResult{Action: ActionReject}, action)
-	assert.NoError(t, err)
+	assert.Error(t, err)
 
 	bs.assertNoFinalizers()
 }
diff --git a/internal/definitions/definition_handler_identity_update_test.go b/internal/definitions/definition_handler_identity_update_test.go
index cb4af9c67..cc1208cbf 100644
--- a/internal/definitions/definition_handler_identity_update_test.go
+++ b/internal/definitions/definition_handler_identity_update_test.go
@@ -131,7 +131,7 @@ func TestHandleDefinitionIdentityInvalidIdentity(t *testing.T) {
 
 	action, err := dh.HandleDefinitionBroadcast(ctx, bs, updateMsg, core.DataArray{updateData}, fftypes.NewUUID())
 	assert.Equal(t, HandlerResult{Action: ActionReject}, action)
-	assert.NoError(t, err)
+	assert.Error(t, err)
 
 	mim.AssertExpectations(t)
 	bs.assertNoFinalizers()
@@ -148,7 +148,7 @@ func TestHandleDefinitionIdentityNotFound(t *testing.T) {
 
 	action, err := dh.HandleDefinitionBroadcast(ctx, bs, updateMsg, core.DataArray{updateData}, fftypes.NewUUID())
 	assert.Equal(t, HandlerResult{Action: ActionReject}, action)
-	assert.NoError(t, err)
+	assert.Error(t, err)
 
 	mim.AssertExpectations(t)
 	bs.assertNoFinalizers()
@@ -202,7 +202,7 @@ func TestHandleDefinitionIdentityValidateFail(t *testing.T) {
 
 	action, err := dh.HandleDefinitionBroadcast(ctx, bs, updateMsg, core.DataArray{updateData}, fftypes.NewUUID())
 	assert.Equal(t, HandlerResult{Action: ActionReject}, action)
-	assert.NoError(t, err)
+	assert.Error(t, err)
 
 	bs.assertNoFinalizers()
 }
@@ -227,7 +227,7 @@ func TestHandleDefinitionIdentityMissingData(t *testing.T) {
 
 	action, err := dh.HandleDefinitionBroadcast(ctx, bs, updateMsg, core.DataArray{}, fftypes.NewUUID())
 	assert.Equal(t, HandlerResult{Action: ActionReject}, action)
-	assert.NoError(t, err)
+	assert.Error(t, err)
 
 	bs.assertNoFinalizers()
 }
diff --git a/internal/definitions/definition_handler_identity_verification_test.go b/internal/definitions/definition_handler_identity_verification_test.go
index 1869c1266..1807dbb44 100644
--- a/internal/definitions/definition_handler_identity_verification_test.go
+++ b/internal/definitions/definition_handler_identity_verification_test.go
@@ -148,7 +148,7 @@ func TestHandleDefinitionIdentityVerificationClaimHashMismatchl(t *testing.T) {
 
 	action, err := dh.HandleDefinitionBroadcast(ctx, bs, verifyMsg, core.DataArray{verifyData}, fftypes.NewUUID())
 	assert.Equal(t, HandlerResult{Action: ActionReject}, action)
-	assert.NoError(t, err)
+	assert.Error(t, err)
 
 	mim.AssertExpectations(t)
 	mdi.AssertExpectations(t)
@@ -209,7 +209,7 @@ func TestHandleDefinitionIdentityVerificationWrongSigner(t *testing.T) {
 
 	action, err := dh.HandleDefinitionBroadcast(ctx, bs, verifyMsg, core.DataArray{verifyData}, fftypes.NewUUID())
 	assert.Equal(t, HandlerResult{Action: ActionReject}, action)
-	assert.NoError(t, err)
+	assert.Error(t, err)
 
 	mim.AssertExpectations(t)
 	bs.assertNoFinalizers()
@@ -226,7 +226,7 @@ func TestHandleDefinitionIdentityVerificationCheckParentNotFound(t *testing.T) {
 
 	action, err := dh.HandleDefinitionBroadcast(ctx, bs, verifyMsg, core.DataArray{verifyData}, fftypes.NewUUID())
 	assert.Equal(t, HandlerResult{Action: ActionReject}, action)
-	assert.NoError(t, err)
+	assert.Error(t, err)
 
 	mim.AssertExpectations(t)
 	bs.assertNoFinalizers()
@@ -271,7 +271,7 @@ func TestHandleDefinitionIdentityVerificationInvalidPayload(t *testing.T) {
 		},
 	}, core.DataArray{emptyObjectData}, fftypes.NewUUID())
 	assert.Equal(t, HandlerResult{Action: ActionReject}, action)
-	assert.NoError(t, err)
+	assert.Error(t, err)
 
 	bs.assertNoFinalizers()
 }
@@ -288,7 +288,7 @@ func TestHandleDefinitionIdentityVerificationInvalidData(t *testing.T) {
 		},
 	}, core.DataArray{}, fftypes.NewUUID())
 	assert.Equal(t, HandlerResult{Action: ActionReject}, action)
-	assert.NoError(t, err)
+	assert.Error(t, err)
 
 	bs.assertNoFinalizers()
 }
diff --git a/internal/definitions/definition_handler_namespace_test.go b/internal/definitions/definition_handler_namespace_test.go
index 31cd34646..4f3c3abeb 100644
--- a/internal/definitions/definition_handler_namespace_test.go
+++ b/internal/definitions/definition_handler_namespace_test.go
@@ -126,7 +126,7 @@ func TestHandleDefinitionBroadcastNSMissingData(t *testing.T) {
 		},
 	}, core.DataArray{}, fftypes.NewUUID())
 	assert.Equal(t, HandlerResult{Action: ActionReject}, action)
-	assert.NoError(t, err)
+	assert.Error(t, err)
 	bs.assertNoFinalizers()
 }
 
@@ -146,7 +146,7 @@ func TestHandleDefinitionBroadcastNSBadID(t *testing.T) {
 		},
 	}, core.DataArray{data}, fftypes.NewUUID())
 	assert.Equal(t, HandlerResult{Action: ActionReject}, action)
-	assert.NoError(t, err)
+	assert.Error(t, err)
 	bs.assertNoFinalizers()
 }
 
@@ -163,7 +163,7 @@ func TestHandleDefinitionBroadcastNSBadData(t *testing.T) {
 		},
 	}, core.DataArray{data}, fftypes.NewUUID())
 	assert.Equal(t, HandlerResult{Action: ActionReject}, action)
-	assert.NoError(t, err)
+	assert.Error(t, err)
 	bs.assertNoFinalizers()
 }
 
@@ -188,7 +188,7 @@ func TestHandleDefinitionBroadcastDuplicate(t *testing.T) {
 		},
 	}, core.DataArray{data}, fftypes.NewUUID())
 	assert.Equal(t, HandlerResult{Action: ActionReject}, action)
-	assert.NoError(t, err)
+	assert.Error(t, err)
 
 	mdi.AssertExpectations(t)
 	bs.assertNoFinalizers()
diff --git a/internal/definitions/definition_handler_network_node_test.go b/internal/definitions/definition_handler_network_node_test.go
index 93588a0d8..957644f1f 100644
--- a/internal/definitions/definition_handler_network_node_test.go
+++ b/internal/definitions/definition_handler_network_node_test.go
@@ -151,7 +151,7 @@ func TestHandleDeprecatedNodeDefinitionBadData(t *testing.T) {
 
 	action, err := dh.handleDeprecatedNodeBroadcast(ctx, bs, &core.Message{}, core.DataArray{})
 	assert.Equal(t, HandlerResult{Action: ActionReject}, action)
-	assert.NoError(t, err)
+	assert.Error(t, err)
 
 	bs.assertNoFinalizers()
 }
@@ -191,7 +191,7 @@ func TestHandleDeprecatedNodeDefinitionOrgNotFound(t *testing.T) {
 
 	action, err := dh.handleDeprecatedNodeBroadcast(ctx, bs, msg, core.DataArray{data})
 	assert.Equal(t, HandlerResult{Action: ActionReject}, action)
-	assert.NoError(t, err)
+	assert.Error(t, err)
 
 	mim.AssertExpectations(t)
 	bs.assertNoFinalizers()
diff --git a/internal/definitions/definition_handler_network_org_test.go b/internal/definitions/definition_handler_network_org_test.go
index f11323a3c..179f05618 100644
--- a/internal/definitions/definition_handler_network_org_test.go
+++ b/internal/definitions/definition_handler_network_org_test.go
@@ -127,7 +127,7 @@ func TestHandleDeprecatedOrgDefinitionBadData(t *testing.T) {
 
 	action, err := dh.handleDeprecatedOrganizationBroadcast(ctx, bs, &core.Message{}, core.DataArray{})
 	assert.Equal(t, HandlerResult{Action: ActionReject}, action)
-	assert.NoError(t, err)
+	assert.Error(t, err)
 
 	bs.assertNoFinalizers()
 }
diff --git a/internal/definitions/definition_handler_test.go b/internal/definitions/definition_handler_test.go
index a8387a694..f32c6d4c7 100644
--- a/internal/definitions/definition_handler_test.go
+++ b/internal/definitions/definition_handler_test.go
@@ -95,7 +95,7 @@ func TestHandleDefinitionBroadcastUnknown(t *testing.T) {
 		},
 	}, core.DataArray{}, fftypes.NewUUID())
 	assert.Equal(t, HandlerResult{Action: ActionReject}, action)
-	assert.NoError(t, err)
+	assert.Error(t, err)
 	bs.assertNoFinalizers()
 }
 
diff --git a/internal/definitions/definition_handler_tokenpool_test.go b/internal/definitions/definition_handler_tokenpool_test.go
index 8f2297239..2628a9afa 100644
--- a/internal/definitions/definition_handler_tokenpool_test.go
+++ b/internal/definitions/definition_handler_tokenpool_test.go
@@ -173,7 +173,7 @@ func TestHandleDefinitionBroadcastTokenPoolIDMismatch(t *testing.T) {
 
 	action, err := sh.HandleDefinitionBroadcast(context.Background(), bs, msg, data, fftypes.NewUUID())
 	assert.Equal(t, HandlerResult{Action: ActionReject, CustomCorrelator: pool.ID}, action)
-	assert.NoError(t, err)
+	assert.Error(t, err)
 
 	mdi.AssertExpectations(t)
 	bs.assertNoFinalizers()
@@ -238,7 +238,7 @@ func TestHandleDefinitionBroadcastTokenPoolValidateFail(t *testing.T) {
 
 	action, err := sh.HandleDefinitionBroadcast(context.Background(), bs, msg, data, fftypes.NewUUID())
 	assert.Equal(t, HandlerResult{Action: ActionReject}, action)
-	assert.NoError(t, err)
+	assert.Error(t, err)
 	bs.assertNoFinalizers()
 }
 
@@ -254,6 +254,6 @@ func TestHandleDefinitionBroadcastTokenPoolBadMessage(t *testing.T) {
 
 	action, err := sh.HandleDefinitionBroadcast(context.Background(), bs, msg, nil, fftypes.NewUUID())
 	assert.Equal(t, HandlerResult{Action: ActionReject}, action)
-	assert.NoError(t, err)
+	assert.Error(t, err)
 	bs.assertNoFinalizers()
 }