Repository: hyperledger/firefly (go)

Issue:
Passing an invalid namespace when connecting websocket causes a crash
Connecting a websocket to FireFly with an invalid namespace can cause the FireFly process to crash.

```
[2022-12-13T18:33:49.176Z] ERROR Sending protocol error to client: FF10187: Namespace does not exist pid=1 websocket=f66111ee-f12a-4be5-a97e-8e30e9cf6cd0
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x20 pc=0xbfc156]

goroutine 574 [running]:
github.com/hyperledger/firefly/internal/namespace.(*namespaceManager).Authorize(0x76892d?, {0x1093520, 0xc00089aa80}, 0xc0007782d0)
	/firefly/internal/namespace/manager.go:1136 +0x36
github.com/hyperledger/firefly/internal/events/websockets.(*websocketConnection).authorizeMessage(0xc0005032b0, {0xc00003b308, 0x11})
	/firefly/internal/events/websockets/websocket_connection.go:357 +0x11b
github.com/hyperledger/firefly/internal/events/websockets.(*websocketConnection).receiveLoop(0xc0005032b0)
	/firefly/internal/events/websockets/websocket_connection.go:160 +0x3d9
created by github.com/hyperledger/firefly/internal/events/websockets.newConnection
	/firefly/internal/events/websockets/websocket_connection.go:79 +0x36a
```

Pull request description:
Add strong nil checking on orchestrator and redress test coverage
- Fixes #1116
   - I went the route of forcing `nil` checking on the `Operator()` method everywhere (apart from unit tests, where I added a `MustOrchestrator`)
- Addresses a couple of  areas where test coverage seems to have slipped

After fix:
![image](https://user-images.githubusercontent.com/6660217/210892935-f458ab7a-6b8e-46a4-9398-5cd2217c2764.png)


Patch:
diff --git a/.golangci.yml b/.golangci.yml
index 6c6e82f6d..00d61e57b 100644
--- a/.golangci.yml
+++ b/.golangci.yml
@@ -30,8 +30,9 @@ linters-settings:
       limitations under the License.
 linters:
   disable-all: false
+  disable:
+    - structcheck
   enable:
-    - bodyclose
     - deadcode
     - depguard
     - dogsled
@@ -51,10 +52,8 @@ linters:
     - nakedret
     - revive
     - staticcheck
-    - structcheck
     - stylecheck
     - typecheck
     - unconvert
-    - unparam
     - unused
     - varcheck
diff --git a/internal/apiserver/server.go b/internal/apiserver/server.go
index e4efb3ef8..902a58365 100644
--- a/internal/apiserver/server.go
+++ b/internal/apiserver/server.go
@@ -1,4 +1,4 @@
-// Copyright © 2022 Kaleido, Inc.
+// Copyright © 2023 Kaleido, Inc.
 //
 // SPDX-License-Identifier: Apache-2.0
 //
@@ -188,9 +188,9 @@ func (as *apiServer) swaggerGenerator(routes []*ffapi.Route, apiBaseURL string)
 func (as *apiServer) contractSwaggerGenerator(mgr namespace.Manager, apiBaseURL string) func(req *http.Request) (*openapi3.T, error) {
 	return func(req *http.Request) (*openapi3.T, error) {
 		vars := mux.Vars(req)
-		or := mgr.Orchestrator(vars["ns"])
-		if or == nil {
-			return nil, i18n.NewError(req.Context(), coremsgs.MsgNamespaceDoesNotExist)
+		or, err := mgr.Orchestrator(req.Context(), vars["ns"])
+		if err != nil {
+			return nil, err
 		}
 		cm := or.Contracts()
 		api, err := cm.GetContractAPI(req.Context(), apiBaseURL, vars["apiName"])
@@ -213,19 +213,16 @@ func (as *apiServer) contractSwaggerGenerator(mgr namespace.Manager, apiBaseURL
 func getOrchestrator(ctx context.Context, mgr namespace.Manager, tag string, r *ffapi.APIRequest) (or orchestrator.Orchestrator, err error) {
 	switch tag {
 	case routeTagDefaultNamespace:
-		or = mgr.Orchestrator(config.GetString(coreconfig.NamespacesDefault))
+		return mgr.Orchestrator(ctx, config.GetString(coreconfig.NamespacesDefault))
 	case routeTagNonDefaultNamespace:
 		vars := mux.Vars(r.Req)
 		if ns, ok := vars["ns"]; ok {
-			or = mgr.Orchestrator(ns)
+			return mgr.Orchestrator(ctx, ns)
 		}
-	default:
+	case routeTagGlobal:
 		return nil, nil
 	}
-	if or == nil {
-		return nil, i18n.NewError(ctx, coremsgs.MsgNamespaceDoesNotExist)
-	}
-	return or, nil
+	return nil, i18n.NewError(ctx, coremsgs.MsgMissingNamespace)
 }
 
 func (as *apiServer) routeHandler(hf *ffapi.HandlerFactory, mgr namespace.Manager, apiBaseURL string, route *ffapi.Route) http.HandlerFunc {
diff --git a/internal/apiserver/spi_routes.go b/internal/apiserver/spi_routes.go
index 059d791ba..ef5f7963c 100644
--- a/internal/apiserver/spi_routes.go
+++ b/internal/apiserver/spi_routes.go
@@ -1,4 +1,4 @@
-// Copyright © 2022 Kaleido, Inc.
+// Copyright © 2023 Kaleido, Inc.
 //
 // SPDX-License-Identifier: Apache-2.0
 //
@@ -20,11 +20,14 @@ import "github.com/hyperledger/firefly-common/pkg/ffapi"
 
 // The Service Provider Interface (SPI) allows external microservices (such as the FireFly Transaction Manager)
 // to act as augmented components to the core.
-var spiRoutes = []*ffapi.Route{
+var spiRoutes = append(globalRoutes([]*ffapi.Route{
 	spiGetNamespaceByName,
 	spiGetNamespaces,
 	spiGetOpByID,
-	spiGetOps,
 	spiPatchOpByID,
 	spiPostReset,
-}
+}),
+	namespacedRoutes([]*ffapi.Route{
+		spiGetOps,
+	})...,
+)
diff --git a/internal/coremsgs/en_error_messages.go b/internal/coremsgs/en_error_messages.go
index 4d38d05b9..18993da9f 100644
--- a/internal/coremsgs/en_error_messages.go
+++ b/internal/coremsgs/en_error_messages.go
@@ -274,4 +274,6 @@ var (
 	MsgErrorNameMustBeSet                 = ffe("FF10433", "The name of the error must be set", 400)
 	MsgContractErrorsResolveError         = ffe("FF10434", "Unable to resolve contract errors: %s", 400)
 	MsgUnknownInterfaceFormat             = ffe("FF10435", "Unknown interface format: %s", 400)
+	MsgUnknownNamespace                   = ffe("FF10436", "Unknown namespace '%s'", 404)
+	MsgMissingNamespace                   = ffe("FF10437", "Missing namespace in request", 400)
 )
diff --git a/internal/namespace/manager.go b/internal/namespace/manager.go
index c1a5b3450..c0f3b2c1d 100644
--- a/internal/namespace/manager.go
+++ b/internal/namespace/manager.go
@@ -1,4 +1,4 @@
-// Copyright © 2022 Kaleido, Inc.
+// Copyright © 2023 Kaleido, Inc.
 //
 // SPDX-License-Identifier: Apache-2.0
 //
@@ -77,7 +77,8 @@ type Manager interface {
 	WaitStop()
 	Reset(ctx context.Context)
 
-	Orchestrator(ns string) orchestrator.Orchestrator
+	Orchestrator(ctx context.Context, ns string) (orchestrator.Orchestrator, error)
+	MustOrchestrator(ns string) orchestrator.Orchestrator
 	SPIEvents() spievents.Manager
 	GetNamespaces(ctx context.Context) ([]*core.Namespace, error)
 	GetOperationByNamespacedID(ctx context.Context, nsOpID string) (*core.Operation, error)
@@ -1039,13 +1040,22 @@ func (nm *namespaceManager) SPIEvents() spievents.Manager {
 	return nm.adminEvents
 }
 
-func (nm *namespaceManager) Orchestrator(ns string) orchestrator.Orchestrator {
+func (nm *namespaceManager) Orchestrator(ctx context.Context, ns string) (orchestrator.Orchestrator, error) {
 	nm.nsMux.Lock()
 	defer nm.nsMux.Unlock()
-	if namespace, ok := nm.namespaces[ns]; ok {
-		return namespace.orchestrator
+	if namespace, ok := nm.namespaces[ns]; ok && namespace != nil {
+		return namespace.orchestrator, nil
 	}
-	return nil
+	return nil, i18n.NewError(ctx, coremsgs.MsgUnknownNamespace, ns)
+}
+
+// MustOrchestrator must only be called by code that is absolutely sure the orchestrator exists
+func (nm *namespaceManager) MustOrchestrator(ns string) orchestrator.Orchestrator {
+	or, err := nm.Orchestrator(context.Background(), ns)
+	if err != nil {
+		panic(err)
+	}
+	return or
 }
 
 func (nm *namespaceManager) GetNamespaces(ctx context.Context) ([]*core.Namespace, error) {
@@ -1063,9 +1073,9 @@ func (nm *namespaceManager) GetOperationByNamespacedID(ctx context.Context, nsOp
 	if err != nil {
 		return nil, err
 	}
-	or := nm.Orchestrator(ns)
-	if or == nil {
-		return nil, i18n.NewError(ctx, coremsgs.Msg404NotFound)
+	or, err := nm.Orchestrator(ctx, ns)
+	if err != nil {
+		return nil, err
 	}
 	return or.GetOperationByID(ctx, u.String())
 }
@@ -1075,10 +1085,11 @@ func (nm *namespaceManager) ResolveOperationByNamespacedID(ctx context.Context,
 	if err != nil {
 		return err
 	}
-	or := nm.Orchestrator(ns)
-	if or == nil {
-		return i18n.NewError(ctx, coremsgs.Msg404NotFound)
+	or, err := nm.Orchestrator(ctx, ns)
+	if err != nil {
+		return err
 	}
+
 	return or.Operations().ResolveOperationByID(ctx, u, op)
 }
 
@@ -1133,5 +1144,9 @@ func (nm *namespaceManager) getAuthPlugin(ctx context.Context) (plugins map[stri
 }
 
 func (nm *namespaceManager) Authorize(ctx context.Context, authReq *fftypes.AuthReq) error {
-	return nm.Orchestrator(authReq.Namespace).Authorize(ctx, authReq)
+	or, err := nm.Orchestrator(ctx, authReq.Namespace)
+	if err != nil {
+		return err
+	}
+	return or.Authorize(ctx, authReq)
 }
diff --git a/mocks/apiservermocks/ffi_swagger_gen.go b/mocks/apiservermocks/ffi_swagger_gen.go
index 6e0755e19..840eafffb 100644
--- a/mocks/apiservermocks/ffi_swagger_gen.go
+++ b/mocks/apiservermocks/ffi_swagger_gen.go
@@ -1,4 +1,4 @@
-// Code generated by mockery v2.15.0. DO NOT EDIT.
+// Code generated by mockery v2.14.1. DO NOT EDIT.
 
 package apiservermocks
 
diff --git a/mocks/apiservermocks/server.go b/mocks/apiservermocks/server.go
index ae9c9e355..8e2d7858d 100644
--- a/mocks/apiservermocks/server.go
+++ b/mocks/apiservermocks/server.go
@@ -1,4 +1,4 @@
-// Code generated by mockery v2.15.0. DO NOT EDIT.
+// Code generated by mockery v2.14.1. DO NOT EDIT.
 
 package apiservermocks
 
diff --git a/mocks/assetmocks/manager.go b/mocks/assetmocks/manager.go
index e6e6558f6..36bd9addc 100644
--- a/mocks/assetmocks/manager.go
+++ b/mocks/assetmocks/manager.go
@@ -1,4 +1,4 @@
-// Code generated by mockery v2.15.0. DO NOT EDIT.
+// Code generated by mockery v2.14.1. DO NOT EDIT.
 
 package assetmocks
 
diff --git a/mocks/batchmocks/manager.go b/mocks/batchmocks/manager.go
index 70fbdf309..3e987ac49 100644
--- a/mocks/batchmocks/manager.go
+++ b/mocks/batchmocks/manager.go
@@ -1,4 +1,4 @@
-// Code generated by mockery v2.15.0. DO NOT EDIT.
+// Code generated by mockery v2.14.1. DO NOT EDIT.
 
 package batchmocks
 
diff --git a/mocks/blockchainmocks/callbacks.go b/mocks/blockchainmocks/callbacks.go
index 54e3923ac..024268dcd 100644
--- a/mocks/blockchainmocks/callbacks.go
+++ b/mocks/blockchainmocks/callbacks.go
@@ -1,4 +1,4 @@
-// Code generated by mockery v2.15.0. DO NOT EDIT.
+// Code generated by mockery v2.14.1. DO NOT EDIT.
 
 package blockchainmocks
 
diff --git a/mocks/blockchainmocks/plugin.go b/mocks/blockchainmocks/plugin.go
index aafdf08f1..09319628f 100644
--- a/mocks/blockchainmocks/plugin.go
+++ b/mocks/blockchainmocks/plugin.go
@@ -1,4 +1,4 @@
-// Code generated by mockery v2.15.0. DO NOT EDIT.
+// Code generated by mockery v2.14.1. DO NOT EDIT.
 
 package blockchainmocks
 
diff --git a/mocks/broadcastmocks/manager.go b/mocks/broadcastmocks/manager.go
index d98c8d7df..770dc5361 100644
--- a/mocks/broadcastmocks/manager.go
+++ b/mocks/broadcastmocks/manager.go
@@ -1,4 +1,4 @@
-// Code generated by mockery v2.15.0. DO NOT EDIT.
+// Code generated by mockery v2.14.1. DO NOT EDIT.
 
 package broadcastmocks
 
diff --git a/mocks/cachemocks/manager.go b/mocks/cachemocks/manager.go
index f9bdc4bad..48d54cd19 100644
--- a/mocks/cachemocks/manager.go
+++ b/mocks/cachemocks/manager.go
@@ -1,4 +1,4 @@
-// Code generated by mockery v2.15.0. DO NOT EDIT.
+// Code generated by mockery v2.14.1. DO NOT EDIT.
 
 package cachemocks
 
diff --git a/mocks/contractmocks/manager.go b/mocks/contractmocks/manager.go
index 89be0cde6..8a574a80d 100644
--- a/mocks/contractmocks/manager.go
+++ b/mocks/contractmocks/manager.go
@@ -1,4 +1,4 @@
-// Code generated by mockery v2.15.0. DO NOT EDIT.
+// Code generated by mockery v2.14.1. DO NOT EDIT.
 
 package contractmocks
 
diff --git a/mocks/coremocks/operation_callbacks.go b/mocks/coremocks/operation_callbacks.go
index 049b7d64a..5e0700d0b 100644
--- a/mocks/coremocks/operation_callbacks.go
+++ b/mocks/coremocks/operation_callbacks.go
@@ -1,4 +1,4 @@
-// Code generated by mockery v2.15.0. DO NOT EDIT.
+// Code generated by mockery v2.14.1. DO NOT EDIT.
 
 package coremocks
 
diff --git a/mocks/databasemocks/callbacks.go b/mocks/databasemocks/callbacks.go
index 07a7d808b..7bf2d3286 100644
--- a/mocks/databasemocks/callbacks.go
+++ b/mocks/databasemocks/callbacks.go
@@ -1,4 +1,4 @@
-// Code generated by mockery v2.15.0. DO NOT EDIT.
+// Code generated by mockery v2.14.1. DO NOT EDIT.
 
 package databasemocks
 
diff --git a/mocks/databasemocks/plugin.go b/mocks/databasemocks/plugin.go
index 37694649e..4b1fea3d1 100644
--- a/mocks/databasemocks/plugin.go
+++ b/mocks/databasemocks/plugin.go
@@ -1,4 +1,4 @@
-// Code generated by mockery v2.15.0. DO NOT EDIT.
+// Code generated by mockery v2.14.1. DO NOT EDIT.
 
 package databasemocks
 
diff --git a/mocks/dataexchangemocks/callbacks.go b/mocks/dataexchangemocks/callbacks.go
index b5fa9002e..7ecda9c04 100644
--- a/mocks/dataexchangemocks/callbacks.go
+++ b/mocks/dataexchangemocks/callbacks.go
@@ -1,4 +1,4 @@
-// Code generated by mockery v2.15.0. DO NOT EDIT.
+// Code generated by mockery v2.14.1. DO NOT EDIT.
 
 package dataexchangemocks
 
diff --git a/mocks/dataexchangemocks/dx_event.go b/mocks/dataexchangemocks/dx_event.go
index 1220ec563..82dae91e1 100644
--- a/mocks/dataexchangemocks/dx_event.go
+++ b/mocks/dataexchangemocks/dx_event.go
@@ -1,4 +1,4 @@
-// Code generated by mockery v2.15.0. DO NOT EDIT.
+// Code generated by mockery v2.14.1. DO NOT EDIT.
 
 package dataexchangemocks
 
diff --git a/mocks/dataexchangemocks/plugin.go b/mocks/dataexchangemocks/plugin.go
index 7803ed134..01672b978 100644
--- a/mocks/dataexchangemocks/plugin.go
+++ b/mocks/dataexchangemocks/plugin.go
@@ -1,4 +1,4 @@
-// Code generated by mockery v2.15.0. DO NOT EDIT.
+// Code generated by mockery v2.14.1. DO NOT EDIT.
 
 package dataexchangemocks
 
diff --git a/mocks/datamocks/manager.go b/mocks/datamocks/manager.go
index e7d708d5d..4cce3323a 100644
--- a/mocks/datamocks/manager.go
+++ b/mocks/datamocks/manager.go
@@ -1,4 +1,4 @@
-// Code generated by mockery v2.15.0. DO NOT EDIT.
+// Code generated by mockery v2.14.1. DO NOT EDIT.
 
 package datamocks
 
diff --git a/mocks/definitionsmocks/handler.go b/mocks/definitionsmocks/handler.go
index e8895493b..7b1ab52f0 100644
--- a/mocks/definitionsmocks/handler.go
+++ b/mocks/definitionsmocks/handler.go
@@ -1,4 +1,4 @@
-// Code generated by mockery v2.15.0. DO NOT EDIT.
+// Code generated by mockery v2.14.1. DO NOT EDIT.
 
 package definitionsmocks
 
diff --git a/mocks/definitionsmocks/sender.go b/mocks/definitionsmocks/sender.go
index 6a4e06c2f..44a207e4d 100644
--- a/mocks/definitionsmocks/sender.go
+++ b/mocks/definitionsmocks/sender.go
@@ -1,4 +1,4 @@
-// Code generated by mockery v2.15.0. DO NOT EDIT.
+// Code generated by mockery v2.14.1. DO NOT EDIT.
 
 package definitionsmocks
 
diff --git a/mocks/eventmocks/event_manager.go b/mocks/eventmocks/event_manager.go
index f3e19212d..f580c1e6a 100644
--- a/mocks/eventmocks/event_manager.go
+++ b/mocks/eventmocks/event_manager.go
@@ -1,4 +1,4 @@
-// Code generated by mockery v2.15.0. DO NOT EDIT.
+// Code generated by mockery v2.14.1. DO NOT EDIT.
 
 package eventmocks
 
diff --git a/mocks/eventsmocks/callbacks.go b/mocks/eventsmocks/callbacks.go
index d9093fb25..c3e746d3a 100644
--- a/mocks/eventsmocks/callbacks.go
+++ b/mocks/eventsmocks/callbacks.go
@@ -1,4 +1,4 @@
-// Code generated by mockery v2.15.0. DO NOT EDIT.
+// Code generated by mockery v2.14.1. DO NOT EDIT.
 
 package eventsmocks
 
diff --git a/mocks/eventsmocks/plugin.go b/mocks/eventsmocks/plugin.go
index 8093bfd02..82e66cbab 100644
--- a/mocks/eventsmocks/plugin.go
+++ b/mocks/eventsmocks/plugin.go
@@ -1,4 +1,4 @@
-// Code generated by mockery v2.15.0. DO NOT EDIT.
+// Code generated by mockery v2.14.1. DO NOT EDIT.
 
 package eventsmocks
 
diff --git a/mocks/identitymanagermocks/manager.go b/mocks/identitymanagermocks/manager.go
index 4634e0781..db1833a82 100644
--- a/mocks/identitymanagermocks/manager.go
+++ b/mocks/identitymanagermocks/manager.go
@@ -1,4 +1,4 @@
-// Code generated by mockery v2.15.0. DO NOT EDIT.
+// Code generated by mockery v2.14.1. DO NOT EDIT.
 
 package identitymanagermocks
 
diff --git a/mocks/identitymocks/callbacks.go b/mocks/identitymocks/callbacks.go
index d4cc9a94c..1684659f0 100644
--- a/mocks/identitymocks/callbacks.go
+++ b/mocks/identitymocks/callbacks.go
@@ -1,4 +1,4 @@
-// Code generated by mockery v2.15.0. DO NOT EDIT.
+// Code generated by mockery v2.14.1. DO NOT EDIT.
 
 package identitymocks
 
diff --git a/mocks/identitymocks/plugin.go b/mocks/identitymocks/plugin.go
index 1d428704d..329ae5f13 100644
--- a/mocks/identitymocks/plugin.go
+++ b/mocks/identitymocks/plugin.go
@@ -1,4 +1,4 @@
-// Code generated by mockery v2.15.0. DO NOT EDIT.
+// Code generated by mockery v2.14.1. DO NOT EDIT.
 
 package identitymocks
 
diff --git a/mocks/metricsmocks/manager.go b/mocks/metricsmocks/manager.go
index 1b4154aba..17141a831 100644
--- a/mocks/metricsmocks/manager.go
+++ b/mocks/metricsmocks/manager.go
@@ -1,4 +1,4 @@
-// Code generated by mockery v2.15.0. DO NOT EDIT.
+// Code generated by mockery v2.14.1. DO NOT EDIT.
 
 package metricsmocks
 
diff --git a/mocks/multipartymocks/manager.go b/mocks/multipartymocks/manager.go
index abeaeba34..80a3277b8 100644
--- a/mocks/multipartymocks/manager.go
+++ b/mocks/multipartymocks/manager.go
@@ -1,4 +1,4 @@
-// Code generated by mockery v2.15.0. DO NOT EDIT.
+// Code generated by mockery v2.14.1. DO NOT EDIT.
 
 package multipartymocks
 
diff --git a/mocks/namespacemocks/manager.go b/mocks/namespacemocks/manager.go
index 5b02d258f..059e082bd 100644
--- a/mocks/namespacemocks/manager.go
+++ b/mocks/namespacemocks/manager.go
@@ -1,4 +1,4 @@
-// Code generated by mockery v2.15.0. DO NOT EDIT.
+// Code generated by mockery v2.14.1. DO NOT EDIT.
 
 package namespacemocks
 
@@ -94,8 +94,8 @@ func (_m *Manager) Init(ctx context.Context, cancelCtx context.CancelFunc, reset
 	return r0
 }
 
-// Orchestrator provides a mock function with given fields: ns
-func (_m *Manager) Orchestrator(ns string) orchestrator.Orchestrator {
+// MustOrchestrator provides a mock function with given fields: ns
+func (_m *Manager) MustOrchestrator(ns string) orchestrator.Orchestrator {
 	ret := _m.Called(ns)
 
 	var r0 orchestrator.Orchestrator
@@ -110,6 +110,29 @@ func (_m *Manager) Orchestrator(ns string) orchestrator.Orchestrator {
 	return r0
 }
 
+// Orchestrator provides a mock function with given fields: ctx, ns
+func (_m *Manager) Orchestrator(ctx context.Context, ns string) (orchestrator.Orchestrator, error) {
+	ret := _m.Called(ctx, ns)
+
+	var r0 orchestrator.Orchestrator
+	if rf, ok := ret.Get(0).(func(context.Context, string) orchestrator.Orchestrator); ok {
+		r0 = rf(ctx, ns)
+	} else {
+		if ret.Get(0) != nil {
+			r0 = ret.Get(0).(orchestrator.Orchestrator)
+		}
+	}
+
+	var r1 error
+	if rf, ok := ret.Get(1).(func(context.Context, string) error); ok {
+		r1 = rf(ctx, ns)
+	} else {
+		r1 = ret.Error(1)
+	}
+
+	return r0, r1
+}
+
 // Reset provides a mock function with given fields: ctx
 func (_m *Manager) Reset(ctx context.Context) {
 	_m.Called(ctx)
diff --git a/mocks/networkmapmocks/manager.go b/mocks/networkmapmocks/manager.go
index 1baf056bb..5d9f7dbaf 100644
--- a/mocks/networkmapmocks/manager.go
+++ b/mocks/networkmapmocks/manager.go
@@ -1,4 +1,4 @@
-// Code generated by mockery v2.15.0. DO NOT EDIT.
+// Code generated by mockery v2.14.1. DO NOT EDIT.
 
 package networkmapmocks
 
diff --git a/mocks/operationmocks/manager.go b/mocks/operationmocks/manager.go
index fc80e7ee1..6fd421a1d 100644
--- a/mocks/operationmocks/manager.go
+++ b/mocks/operationmocks/manager.go
@@ -1,4 +1,4 @@
-// Code generated by mockery v2.15.0. DO NOT EDIT.
+// Code generated by mockery v2.14.1. DO NOT EDIT.
 
 package operationmocks
 
diff --git a/mocks/orchestratormocks/orchestrator.go b/mocks/orchestratormocks/orchestrator.go
index f57a6b174..e3a7d7f80 100644
--- a/mocks/orchestratormocks/orchestrator.go
+++ b/mocks/orchestratormocks/orchestrator.go
@@ -1,4 +1,4 @@
-// Code generated by mockery v2.15.0. DO NOT EDIT.
+// Code generated by mockery v2.14.1. DO NOT EDIT.
 
 package orchestratormocks
 
diff --git a/mocks/privatemessagingmocks/manager.go b/mocks/privatemessagingmocks/manager.go
index e6648913c..7400ed42d 100644
--- a/mocks/privatemessagingmocks/manager.go
+++ b/mocks/privatemessagingmocks/manager.go
@@ -1,4 +1,4 @@
-// Code generated by mockery v2.15.0. DO NOT EDIT.
+// Code generated by mockery v2.14.1. DO NOT EDIT.
 
 package privatemessagingmocks
 
diff --git a/mocks/shareddownloadmocks/callbacks.go b/mocks/shareddownloadmocks/callbacks.go
index 833ebddb0..67f2546cc 100644
--- a/mocks/shareddownloadmocks/callbacks.go
+++ b/mocks/shareddownloadmocks/callbacks.go
@@ -1,4 +1,4 @@
-// Code generated by mockery v2.15.0. DO NOT EDIT.
+// Code generated by mockery v2.14.1. DO NOT EDIT.
 
 package shareddownloadmocks
 
diff --git a/mocks/shareddownloadmocks/manager.go b/mocks/shareddownloadmocks/manager.go
index d485e580e..698246330 100644
--- a/mocks/shareddownloadmocks/manager.go
+++ b/mocks/shareddownloadmocks/manager.go
@@ -1,4 +1,4 @@
-// Code generated by mockery v2.15.0. DO NOT EDIT.
+// Code generated by mockery v2.14.1. DO NOT EDIT.
 
 package shareddownloadmocks
 
diff --git a/mocks/sharedstoragemocks/callbacks.go b/mocks/sharedstoragemocks/callbacks.go
index 157d31c3f..cba3791df 100644
--- a/mocks/sharedstoragemocks/callbacks.go
+++ b/mocks/sharedstoragemocks/callbacks.go
@@ -1,4 +1,4 @@
-// Code generated by mockery v2.15.0. DO NOT EDIT.
+// Code generated by mockery v2.14.1. DO NOT EDIT.
 
 package sharedstoragemocks
 
diff --git a/mocks/sharedstoragemocks/plugin.go b/mocks/sharedstoragemocks/plugin.go
index 5efc84184..2c2a49278 100644
--- a/mocks/sharedstoragemocks/plugin.go
+++ b/mocks/sharedstoragemocks/plugin.go
@@ -1,4 +1,4 @@
-// Code generated by mockery v2.15.0. DO NOT EDIT.
+// Code generated by mockery v2.14.1. DO NOT EDIT.
 
 package sharedstoragemocks
 
diff --git a/mocks/spieventsmocks/manager.go b/mocks/spieventsmocks/manager.go
index f8f5a5c6b..0ae3113e7 100644
--- a/mocks/spieventsmocks/manager.go
+++ b/mocks/spieventsmocks/manager.go
@@ -1,4 +1,4 @@
-// Code generated by mockery v2.15.0. DO NOT EDIT.
+// Code generated by mockery v2.14.1. DO NOT EDIT.
 
 package spieventsmocks
 
diff --git a/mocks/syncasyncmocks/bridge.go b/mocks/syncasyncmocks/bridge.go
index d92529b87..2deb4f70c 100644
--- a/mocks/syncasyncmocks/bridge.go
+++ b/mocks/syncasyncmocks/bridge.go
@@ -1,4 +1,4 @@
-// Code generated by mockery v2.15.0. DO NOT EDIT.
+// Code generated by mockery v2.14.1. DO NOT EDIT.
 
 package syncasyncmocks
 
diff --git a/mocks/syncasyncmocks/sender.go b/mocks/syncasyncmocks/sender.go
index 266453cc7..97d17f0ca 100644
--- a/mocks/syncasyncmocks/sender.go
+++ b/mocks/syncasyncmocks/sender.go
@@ -1,4 +1,4 @@
-// Code generated by mockery v2.15.0. DO NOT EDIT.
+// Code generated by mockery v2.14.1. DO NOT EDIT.
 
 package syncasyncmocks
 
diff --git a/mocks/systemeventmocks/event_interface.go b/mocks/systemeventmocks/event_interface.go
index 10015954d..e5aa317ae 100644
--- a/mocks/systemeventmocks/event_interface.go
+++ b/mocks/systemeventmocks/event_interface.go
@@ -1,4 +1,4 @@
-// Code generated by mockery v2.15.0. DO NOT EDIT.
+// Code generated by mockery v2.14.1. DO NOT EDIT.
 
 package systemeventmocks
 
diff --git a/mocks/tokenmocks/callbacks.go b/mocks/tokenmocks/callbacks.go
index 4dd7e8f7c..d2618b9b0 100644
--- a/mocks/tokenmocks/callbacks.go
+++ b/mocks/tokenmocks/callbacks.go
@@ -1,4 +1,4 @@
-// Code generated by mockery v2.15.0. DO NOT EDIT.
+// Code generated by mockery v2.14.1. DO NOT EDIT.
 
 package tokenmocks
 
diff --git a/mocks/tokenmocks/plugin.go b/mocks/tokenmocks/plugin.go
index 1c5a0721d..d7939c470 100644
--- a/mocks/tokenmocks/plugin.go
+++ b/mocks/tokenmocks/plugin.go
@@ -1,4 +1,4 @@
-// Code generated by mockery v2.15.0. DO NOT EDIT.
+// Code generated by mockery v2.14.1. DO NOT EDIT.
 
 package tokenmocks
 
diff --git a/mocks/txcommonmocks/helper.go b/mocks/txcommonmocks/helper.go
index 9650446e6..2d289053e 100644
--- a/mocks/txcommonmocks/helper.go
+++ b/mocks/txcommonmocks/helper.go
@@ -1,4 +1,4 @@
-// Code generated by mockery v2.15.0. DO NOT EDIT.
+// Code generated by mockery v2.14.1. DO NOT EDIT.
 
 package txcommonmocks
 
diff --git a/mocks/wsmocks/ws_client.go b/mocks/wsmocks/ws_client.go
index f13e87019..814a6800e 100644
--- a/mocks/wsmocks/ws_client.go
+++ b/mocks/wsmocks/ws_client.go
@@ -1,4 +1,4 @@
-// Code generated by mockery v2.15.0. DO NOT EDIT.
+// Code generated by mockery v2.14.1. DO NOT EDIT.
 
 package wsmocks
 


Test patch:
diff --git a/internal/apiserver/route_get_namespace_test.go b/internal/apiserver/route_get_namespace_test.go
index ff98c31c7..4e08870b1 100644
--- a/internal/apiserver/route_get_namespace_test.go
+++ b/internal/apiserver/route_get_namespace_test.go
@@ -18,6 +18,7 @@ package apiserver
 
 import (
 	"context"
+	"fmt"
 	"net/http/httptest"
 	"testing"
 
@@ -48,8 +49,8 @@ func TestGetNamespaceInvalid(t *testing.T) {
 	req.Header.Set("Content-Type", "application/json; charset=utf-8")
 	res := httptest.NewRecorder()
 
-	mgr.On("Orchestrator", "BAD").Return(nil, nil)
+	mgr.On("Orchestrator", mock.Anything, "BAD").Return(nil, fmt.Errorf("pop"))
 	r.ServeHTTP(res, req)
 
-	assert.Equal(t, 404, res.Result().StatusCode)
+	assert.Equal(t, 500, res.Result().StatusCode)
 }
diff --git a/internal/apiserver/server_test.go b/internal/apiserver/server_test.go
index 04dab4ae2..9df25251a 100644
--- a/internal/apiserver/server_test.go
+++ b/internal/apiserver/server_test.go
@@ -35,6 +35,7 @@ import (
 	"github.com/hyperledger/firefly-common/pkg/httpserver"
 	"github.com/hyperledger/firefly-common/pkg/i18n"
 	"github.com/hyperledger/firefly/internal/coreconfig"
+	"github.com/hyperledger/firefly/internal/coremsgs"
 	"github.com/hyperledger/firefly/internal/metrics"
 	"github.com/hyperledger/firefly/mocks/apiservermocks"
 	"github.com/hyperledger/firefly/mocks/contractmocks"
@@ -53,9 +54,9 @@ func newTestServer() (*namespacemocks.Manager, *orchestratormocks.Orchestrator,
 	InitConfig()
 	mgr := &namespacemocks.Manager{}
 	o := &orchestratormocks.Orchestrator{}
-	mgr.On("Orchestrator", "default").Return(o).Maybe()
-	mgr.On("Orchestrator", "mynamespace").Return(o).Maybe()
-	mgr.On("Orchestrator", "ns1").Return(o).Maybe()
+	mgr.On("Orchestrator", mock.Anything, "default").Return(o, nil).Maybe()
+	mgr.On("Orchestrator", mock.Anything, "mynamespace").Return(o, nil).Maybe()
+	mgr.On("Orchestrator", mock.Anything, "ns1").Return(o, nil).Maybe()
 	config.Set(coreconfig.APIMaxFilterLimit, 100)
 	as := &apiServer{
 		apiTimeout:    5 * time.Second,
@@ -369,7 +370,7 @@ func TestContractAPISwaggerJSONBadNamespace(t *testing.T) {
 	s := httptest.NewServer(r)
 	defer s.Close()
 
-	mgr.On("Orchestrator", "BAD").Return(nil)
+	mgr.On("Orchestrator", mock.Anything, "BAD").Return(nil, i18n.NewError(context.Background(), coremsgs.MsgUnknownNamespace))
 
 	res, err := http.Get(fmt.Sprintf("http://%s/api/v1/namespaces/BAD/apis/my-api/api/swagger.json", s.Listener.Addr()))
 	assert.NoError(t, err)
@@ -395,7 +396,7 @@ func TestJSONBadNamespace(t *testing.T) {
 	s := httptest.NewServer(r)
 	defer s.Close()
 
-	mgr.On("Orchestrator", "BAD").Return(nil)
+	mgr.On("Orchestrator", mock.Anything, "BAD").Return(nil, i18n.NewError(context.Background(), coremsgs.MsgUnknownNamespace))
 
 	var b bytes.Buffer
 	req := httptest.NewRequest("GET", "/api/v1/namespaces/BAD/apis", &b)
@@ -413,7 +414,7 @@ func TestFormDataBadNamespace(t *testing.T) {
 	s := httptest.NewServer(r)
 	defer s.Close()
 
-	mgr.On("Orchestrator", "BAD").Return(nil)
+	mgr.On("Orchestrator", mock.Anything, "BAD").Return(nil, i18n.NewError(context.Background(), coremsgs.MsgUnknownNamespace))
 
 	var b bytes.Buffer
 	w := multipart.NewWriter(&b)
@@ -471,3 +472,8 @@ func TestFormDataDisabledRoute(t *testing.T) {
 
 	assert.Equal(t, 400, res.Result().StatusCode)
 }
+
+func TestGetOrchestratorMissingTag(t *testing.T) {
+	_, err := getOrchestrator(context.Background(), &namespacemocks.Manager{}, "", nil)
+	assert.Regexp(t, "FF10437", err)
+}
diff --git a/internal/blockchain/ethereum/ethereum_test.go b/internal/blockchain/ethereum/ethereum_test.go
index e532e3056..0ea79022a 100644
--- a/internal/blockchain/ethereum/ethereum_test.go
+++ b/internal/blockchain/ethereum/ethereum_test.go
@@ -296,6 +296,7 @@ func TestInitAndStartWithFFTM(t *testing.T) {
 	assert.Equal(t, `{"type":"listen","topic":"topic1"}`, startupMessage)
 	startupMessage = <-toServer
 	assert.Equal(t, `{"type":"listenreplies"}`, startupMessage)
+	fromServer <- `{"bad":"receipt"}`                 // bad receipt that cannot be handled - will be swallowed
 	fromServer <- `{"batchNumber":12345,"events":[]}` // empty batch, will be ignored, but acked
 	reply := <-toServer
 	assert.Equal(t, `{"type":"ack","topic":"topic1","batchNumber":12345}`, reply)
diff --git a/internal/blockchain/fabric/fabric_test.go b/internal/blockchain/fabric/fabric_test.go
index 265c453bd..53d4b3dba 100644
--- a/internal/blockchain/fabric/fabric_test.go
+++ b/internal/blockchain/fabric/fabric_test.go
@@ -135,6 +135,14 @@ func TestInitMissingURL(t *testing.T) {
 	assert.Regexp(t, "FF10138.*url", err)
 }
 
+func TestGenerateErrorSignatureNoOp(t *testing.T) {
+	e, cancel := newTestFabric()
+	defer cancel()
+	resetConf(e)
+
+	assert.Empty(t, e.GenerateErrorSignature(context.Background(), &fftypes.FFIErrorDefinition{}))
+}
+
 func TestInitMissingTopic(t *testing.T) {
 	e, cancel := newTestFabric()
 	defer cancel()
@@ -205,7 +213,8 @@ func TestInitAllNewStreamsAndWSEvent(t *testing.T) {
 	assert.Equal(t, `{"type":"listen","topic":"topic1"}`, startupMessage)
 	startupMessage = <-toServer
 	assert.Equal(t, `{"type":"listenreplies"}`, startupMessage)
-	fromServer <- `[]` // empty batch, will be ignored, but acked
+	fromServer <- `{"bad":"receipt"}` // will be ignored - no ack\
+	fromServer <- `[]`                // empty batch, will be ignored, but acked
 	reply := <-toServer
 	assert.Equal(t, `{"topic":"topic1","type":"ack"}`, reply)
 
diff --git a/internal/contracts/manager_test.go b/internal/contracts/manager_test.go
index f55f9e970..638b40fb4 100644
--- a/internal/contracts/manager_test.go
+++ b/internal/contracts/manager_test.go
@@ -138,6 +138,13 @@ func TestResolveFFI(t *testing.T) {
 				},
 			},
 		},
+		Errors: []*fftypes.FFIError{
+			{
+				FFIErrorDefinition: fftypes.FFIErrorDefinition{
+					Name: "changed",
+				},
+			},
+		},
 	}
 
 	err := cm.ResolveFFI(context.Background(), ffi)
@@ -419,6 +426,19 @@ func TestValidateFFI(t *testing.T) {
 				},
 			},
 		},
+		Errors: []*fftypes.FFIError{
+			{
+				FFIErrorDefinition: fftypes.FFIErrorDefinition{
+					Name: "sum",
+					Params: []*fftypes.FFIParam{
+						{
+							Name:   "z",
+							Schema: fftypes.JSONAnyPtr(`{"type": "integer", "details": {"type": "uint256"}}`),
+						},
+					},
+				},
+			},
+		},
 	}
 
 	mdi.On("GetFFI", context.Background(), "ns1", "math", "1.0.0").Return(nil, nil)
@@ -593,6 +613,64 @@ func TestValidateFFIBadEventParam(t *testing.T) {
 	assert.Regexp(t, "FF10319", err)
 }
 
+func TestValidateFFIBadError(t *testing.T) {
+	cm := newTestContractManager()
+	mdi := cm.database.(*databasemocks.Plugin)
+
+	ffi := &fftypes.FFI{
+		Name:      "math",
+		Version:   "1.0.0",
+		Namespace: "default",
+		Errors: []*fftypes.FFIError{
+			{
+				FFIErrorDefinition: fftypes.FFIErrorDefinition{
+					Name: "",
+					Params: []*fftypes.FFIParam{
+						{
+							Name:   "z",
+							Schema: fftypes.JSONAnyPtr(`{"type": "integer", "details": {"type": "uint256"}}`),
+						},
+					},
+				},
+			},
+		},
+	}
+
+	mdi.On("GetFFI", context.Background(), "ns1", "math", "1.0.0").Return(nil, nil)
+
+	err := cm.ResolveFFI(context.Background(), ffi)
+	assert.Regexp(t, "FF10433", err)
+}
+
+func TestValidateFFIBadErrorParam(t *testing.T) {
+	cm := newTestContractManager()
+	mdi := cm.database.(*databasemocks.Plugin)
+
+	ffi := &fftypes.FFI{
+		Name:      "math",
+		Version:   "1.0.0",
+		Namespace: "default",
+		Errors: []*fftypes.FFIError{
+			{
+				FFIErrorDefinition: fftypes.FFIErrorDefinition{
+					Name: "pop",
+					Params: []*fftypes.FFIParam{
+						{
+							Name:   "z",
+							Schema: fftypes.JSONAnyPtr(`{"type": "wrongness"`),
+						},
+					},
+				},
+			},
+		},
+	}
+
+	mdi.On("GetFFI", context.Background(), "ns1", "math", "1.0.0").Return(nil, nil)
+
+	err := cm.ResolveFFI(context.Background(), ffi)
+	assert.Regexp(t, "FF10332", err)
+}
+
 func TestAddContractListenerInline(t *testing.T) {
 	cm := newTestContractManager()
 	mbi := cm.blockchain.(*blockchainmocks.Plugin)
@@ -1260,6 +1338,26 @@ func TestGetFFIByIDWithChildren(t *testing.T) {
 	assert.Equal(t, "event1", ffi.Events[0].Name)
 }
 
+func TestGetFFIByIDWithChildrenErrorsFail(t *testing.T) {
+	cm := newTestContractManager()
+	mdb := cm.database.(*databasemocks.Plugin)
+
+	cid := fftypes.NewUUID()
+	mdb.On("GetFFIByID", mock.Anything, "ns1", cid).Return(&fftypes.FFI{
+		ID: cid,
+	}, nil)
+	mdb.On("GetFFIMethods", mock.Anything, "ns1", mock.Anything).Return([]*fftypes.FFIMethod{
+		{ID: fftypes.NewUUID(), Name: "method1"},
+	}, nil, nil)
+	mdb.On("GetFFIEvents", mock.Anything, "ns1", mock.Anything).Return([]*fftypes.FFIEvent{}, nil, nil)
+	mdb.On("GetFFIErrors", mock.Anything, "ns1", mock.Anything).Return(nil, fmt.Errorf("pop"))
+
+	_, err := cm.GetFFIByIDWithChildren(context.Background(), cid)
+
+	assert.EqualError(t, err, "pop")
+	mdb.AssertExpectations(t)
+}
+
 func TestGetFFIByIDWithChildrenEventsFail(t *testing.T) {
 	cm := newTestContractManager()
 	mdb := cm.database.(*databasemocks.Plugin)
@@ -1663,6 +1761,27 @@ func TestInvokeContractMethodNotFound(t *testing.T) {
 	assert.Regexp(t, "FF10315", err)
 }
 
+func TestInvokeContractErrorsFail(t *testing.T) {
+	cm := newTestContractManager()
+	mdb := cm.database.(*databasemocks.Plugin)
+	mim := cm.identity.(*identitymanagermocks.Manager)
+
+	req := &core.ContractCallRequest{
+		Type:       core.CallTypeInvoke,
+		Interface:  fftypes.NewUUID(),
+		Location:   fftypes.JSONAnyPtr(""),
+		MethodPath: "set",
+	}
+
+	mim.On("NormalizeSigningKey", mock.Anything, "", identity.KeyNormalizationBlockchainPlugin).Return("key-resolved", nil)
+	mdb.On("GetFFIMethod", mock.Anything, "ns1", req.Interface, req.MethodPath).Return(&fftypes.FFIMethod{Name: "set"}, nil)
+	mdb.On("GetFFIErrors", mock.Anything, "ns1", req.Interface).Return(nil, fmt.Errorf("pop"))
+
+	_, err := cm.InvokeContract(context.Background(), req, false)
+
+	assert.Regexp(t, "FF10434", err)
+}
+
 func TestInvokeContractMethodBadInput(t *testing.T) {
 	cm := newTestContractManager()
 	mim := cm.identity.(*identitymanagermocks.Manager)
diff --git a/internal/definitions/handler_contracts_test.go b/internal/definitions/handler_contracts_test.go
index d5cb0ee37..5764ebb23 100644
--- a/internal/definitions/handler_contracts_test.go
+++ b/internal/definitions/handler_contracts_test.go
@@ -72,6 +72,20 @@ func testFFI() *fftypes.FFI {
 				},
 			},
 		},
+		Errors: []*fftypes.FFIError{
+			{
+				ID: fftypes.NewUUID(),
+				FFIErrorDefinition: fftypes.FFIErrorDefinition{
+					Name: "event1",
+					Params: fftypes.FFIParams{
+						{
+							Name:   "result",
+							Schema: fftypes.JSONAnyPtr(`{"type": "integer"}`),
+						},
+					},
+				},
+			},
+		},
 	}
 }
 
@@ -100,6 +114,7 @@ func TestHandleFFIBroadcastOk(t *testing.T) {
 	mdi.On("UpsertFFI", mock.Anything, mock.Anything).Return(nil)
 	mdi.On("UpsertFFIMethod", mock.Anything, mock.Anything).Return(nil)
 	mdi.On("UpsertFFIEvent", mock.Anything, mock.Anything).Return(nil)
+	mdi.On("UpsertFFIError", mock.Anything, mock.Anything).Return(nil)
 	mdi.On("InsertEvent", mock.Anything, mock.Anything).Return(nil)
 	mcm := dh.contracts.(*contractmocks.Manager)
 	mcm.On("ResolveFFI", mock.Anything, mock.Anything).Return(nil)
@@ -184,6 +199,22 @@ func TestPersistFFIUpsertFFIEventFail(t *testing.T) {
 	mcm.AssertExpectations(t)
 }
 
+func TestPersistFFIUpsertFFIErrorFail(t *testing.T) {
+	dh, _ := newTestDefinitionHandler(t)
+	mdi := dh.database.(*databasemocks.Plugin)
+	mdi.On("UpsertFFI", mock.Anything, mock.Anything).Return(nil)
+	mdi.On("UpsertFFIMethod", mock.Anything, mock.Anything).Return(nil)
+	mdi.On("UpsertFFIEvent", mock.Anything, mock.Anything).Return(nil)
+	mdi.On("UpsertFFIError", mock.Anything, mock.Anything).Return(fmt.Errorf("pop"))
+	mcm := dh.contracts.(*contractmocks.Manager)
+	mcm.On("ResolveFFI", mock.Anything, mock.Anything).Return(nil)
+	retry, err := dh.persistFFI(context.Background(), testFFI())
+	assert.Regexp(t, "pop", err)
+	assert.True(t, retry)
+	mdi.AssertExpectations(t)
+	mcm.AssertExpectations(t)
+}
+
 func TestHandleFFIBroadcastValidateFail(t *testing.T) {
 	dh, bs := newTestDefinitionHandler(t)
 	ffi := testFFI()
diff --git a/internal/definitions/sender_contracts_test.go b/internal/definitions/sender_contracts_test.go
index f109b52f9..ce954408b 100644
--- a/internal/definitions/sender_contracts_test.go
+++ b/internal/definitions/sender_contracts_test.go
@@ -39,6 +39,7 @@ func TestDefineFFIResolveFail(t *testing.T) {
 	ffi := &fftypes.FFI{
 		Methods: []*fftypes.FFIMethod{{}},
 		Events:  []*fftypes.FFIEvent{{}},
+		Errors:  []*fftypes.FFIError{{}},
 	}
 
 	mcm := ds.contracts.(*contractmocks.Manager)
diff --git a/internal/events/aggregator_rewind_test.go b/internal/events/aggregator_rewind_test.go
index 8b4937bd1..7152d39cd 100644
--- a/internal/events/aggregator_rewind_test.go
+++ b/internal/events/aggregator_rewind_test.go
@@ -201,3 +201,15 @@ func TestPopRewindsDoublePopNoBlock(t *testing.T) {
 	assert.Empty(t, batchIDs)
 
 }
+
+func TestPopRewindsNoTapBlocked(t *testing.T) {
+
+	em := newTestEventManager(t)
+	defer em.cleanup(t)
+
+	em.aggregator.rewinder.loop2ShoulderTap <- true
+	em.aggregator.rewinder.stagedRewinds = []*rewind{{}}
+
+	_ = em.aggregator.rewinder.popRewinds()
+
+}
diff --git a/internal/events/aggregator_test.go b/internal/events/aggregator_test.go
index 9d48979bb..48c2aa588 100644
--- a/internal/events/aggregator_test.go
+++ b/internal/events/aggregator_test.go
@@ -1582,6 +1582,59 @@ func TestAttemptMessageDispatchTransferMismatch(t *testing.T) {
 
 }
 
+func TestAttemptMessageDispatchGetApprovalsFail(t *testing.T) {
+	ag := newTestAggregator()
+	defer ag.cleanup(t)
+
+	org1 := newTestOrg("org1")
+	ag.mim.On("FindIdentityForVerifier", ag.ctx, mock.Anything, mock.Anything).Return(org1, nil)
+
+	ag.mdi.On("GetTokenApprovals", ag.ctx, "ns1", mock.Anything).Return(nil, nil, fmt.Errorf("pop"))
+
+	msg := &core.Message{
+		Header: core.MessageHeader{
+			ID:        fftypes.NewUUID(),
+			Type:      core.MessageTypeApprovalBroadcast,
+			SignerRef: core.SignerRef{Key: "0x12345", Author: org1.DID},
+		},
+	}
+	msg.Hash = msg.Header.Hash()
+	_, dispatched, err := ag.attemptMessageDispatch(ag.ctx, msg, core.DataArray{}, nil, &batchState{}, &core.Pin{Signer: "0x12345"})
+	assert.EqualError(t, err, "pop")
+	assert.False(t, dispatched)
+
+}
+
+func TestAttemptMessageDispatchApprovalMismatch(t *testing.T) {
+	ag := newTestAggregator()
+	defer ag.cleanup(t)
+
+	org1 := newTestOrg("org1")
+
+	msg := &core.Message{
+		Header: core.MessageHeader{
+			ID:        fftypes.NewUUID(),
+			Type:      core.MessageTypeApprovalBroadcast,
+			SignerRef: core.SignerRef{Key: "0x12345", Author: org1.DID},
+		},
+	}
+	msg.Hash = msg.Header.Hash()
+
+	approvals := []*core.TokenApproval{{
+		Message:     msg.Header.ID,
+		MessageHash: fftypes.NewRandB32(),
+	}}
+
+	ag.mim.On("FindIdentityForVerifier", ag.ctx, mock.Anything, mock.Anything).Return(org1, nil)
+
+	ag.mdi.On("GetTokenApprovals", ag.ctx, "ns1", mock.Anything).Return(approvals, nil, nil)
+
+	_, dispatched, err := ag.attemptMessageDispatch(ag.ctx, msg, core.DataArray{}, nil, &batchState{}, &core.Pin{Signer: "0x12345"})
+	assert.NoError(t, err)
+	assert.False(t, dispatched)
+
+}
+
 func TestDefinitionBroadcastActionRejectCustomCorrelator(t *testing.T) {
 	ag := newTestAggregator()
 	defer ag.cleanup(t)
diff --git a/internal/events/tokens_approved_test.go b/internal/events/tokens_approved_test.go
index 3ee150aa5..a50c5c11d 100644
--- a/internal/events/tokens_approved_test.go
+++ b/internal/events/tokens_approved_test.go
@@ -308,3 +308,108 @@ func TestApprovedBlockchainEventFail(t *testing.T) {
 	assert.EqualError(t, err, "pop")
 
 }
+
+func TestTokensApprovedWithMessageReceived(t *testing.T) {
+	em := newTestEventManager(t)
+	defer em.cleanup(t)
+
+	mti := &tokenmocks.Plugin{}
+
+	info := fftypes.JSONObject{"some": "info"}
+	approval := &tokens.TokenApproval{
+		PoolLocator: "F1",
+		TokenApproval: core.TokenApproval{
+			Connector:  "erc1155",
+			Key:        "0x12345",
+			ProtocolID: "123",
+			Message:    fftypes.NewUUID(),
+		},
+		Event: &blockchain.Event{
+			BlockchainTXID: "0xffffeeee",
+			ProtocolID:     "0000/0000/0000",
+			Info:           info,
+		},
+	}
+	pool := &core.TokenPool{
+		ID:        fftypes.NewUUID(),
+		Namespace: "ns1",
+	}
+	message := &core.Message{
+		BatchID: fftypes.NewUUID(),
+	}
+
+	em.mdi.On("GetTokenApprovalByProtocolID", em.ctx, "ns1", "erc1155", "123").Return(nil, nil).Times(2)
+	em.mdi.On("GetTokenPoolByLocator", em.ctx, "ns1", "erc1155", "F1").Return(pool, nil).Times(2)
+	em.mth.On("InsertOrGetBlockchainEvent", em.ctx, mock.MatchedBy(func(e *core.BlockchainEvent) bool {
+		return e.Namespace == pool.Namespace && e.Name == approval.Event.Name
+	})).Return(nil, nil).Times(2)
+	em.mdi.On("InsertEvent", em.ctx, mock.MatchedBy(func(ev *core.Event) bool {
+		return ev.Type == core.EventTypeBlockchainEventReceived && ev.Namespace == pool.Namespace
+	})).Return(nil).Times(2)
+	em.mdi.On("UpsertTokenApproval", em.ctx, &approval.TokenApproval).Return(nil).Times(2)
+	em.mdi.On("UpdateTokenApprovals", em.ctx, mock.Anything, mock.Anything).Return(nil).Times(2)
+	em.mdi.On("GetMessageByID", em.ctx, "ns1", approval.Message).Return(nil, fmt.Errorf("pop")).Once()
+	em.mdi.On("GetMessageByID", em.ctx, "ns1", approval.Message).Return(message, nil).Once()
+	em.mdi.On("InsertEvent", em.ctx, mock.MatchedBy(func(ev *core.Event) bool {
+		return ev.Type == core.EventTypeApprovalConfirmed && ev.Reference == approval.LocalID && ev.Namespace == pool.Namespace
+	})).Return(nil).Once()
+
+	err := em.TokensApproved(mti, approval)
+	assert.NoError(t, err)
+
+	mti.AssertExpectations(t)
+}
+
+func TestTokensApprovedWithMessageSend(t *testing.T) {
+	em := newTestEventManager(t)
+	defer em.cleanup(t)
+
+	mti := &tokenmocks.Plugin{}
+
+	info := fftypes.JSONObject{"some": "info"}
+	approval := &tokens.TokenApproval{
+		PoolLocator: "F1",
+		TokenApproval: core.TokenApproval{
+			Connector:  "erc1155",
+			Key:        "0x12345",
+			ProtocolID: "123",
+			Message:    fftypes.NewUUID(),
+		},
+		Event: &blockchain.Event{
+			BlockchainTXID: "0xffffeeee",
+			ProtocolID:     "0000/0000/0000",
+			Info:           info,
+		},
+	}
+	pool := &core.TokenPool{
+		ID:        fftypes.NewUUID(),
+		Namespace: "ns1",
+	}
+	message := &core.Message{
+		BatchID: fftypes.NewUUID(),
+		State:   core.MessageStateStaged,
+	}
+
+	em.mdi.On("GetTokenApprovalByProtocolID", em.ctx, "ns1", "erc1155", "123").Return(nil, nil).Times(2)
+	em.mdi.On("GetTokenPoolByLocator", em.ctx, "ns1", "erc1155", "F1").Return(pool, nil).Times(2)
+	em.mth.On("InsertOrGetBlockchainEvent", em.ctx, mock.MatchedBy(func(e *core.BlockchainEvent) bool {
+		return e.Namespace == pool.Namespace && e.Name == approval.Event.Name
+	})).Return(nil, nil).Times(2)
+	em.mdi.On("InsertEvent", em.ctx, mock.MatchedBy(func(ev *core.Event) bool {
+		return ev.Type == core.EventTypeBlockchainEventReceived && ev.Namespace == pool.Namespace
+	})).Return(nil).Times(2)
+	em.mdi.On("UpsertTokenApproval", em.ctx, &approval.TokenApproval).Return(nil).Times(2)
+	em.mdi.On("UpdateTokenApprovals", em.ctx, mock.Anything, mock.Anything).Return(nil).Times(2)
+	em.mdi.On("GetMessageByID", em.ctx, "ns1", mock.Anything).Return(message, nil).Times(2)
+	em.mdi.On("ReplaceMessage", em.ctx, mock.MatchedBy(func(msg *core.Message) bool {
+		return msg.State == core.MessageStateReady
+	})).Return(fmt.Errorf("pop"))
+	em.mdi.On("InsertEvent", em.ctx, mock.MatchedBy(func(ev *core.Event) bool {
+		return ev.Type == core.EventTypeApprovalConfirmed && ev.Reference == approval.LocalID && ev.Namespace == pool.Namespace
+	})).Return(nil).Once()
+
+	err := em.TokensApproved(mti, approval)
+	assert.NoError(t, err)
+
+	mti.AssertExpectations(t)
+}
diff --git a/internal/namespace/manager_test.go b/internal/namespace/manager_test.go
index 77828f058..fdebdb770 100644
--- a/internal/namespace/manager_test.go
+++ b/internal/namespace/manager_test.go
@@ -171,9 +171,9 @@ func TestInit(t *testing.T) {
 	err := nm.Init(ctx, cancelCtx, nm.reset)
 	assert.NoError(t, err)
 
-	assert.Equal(t, mo, nm.Orchestrator("default"))
-	assert.Nil(t, nm.Orchestrator("unknown"))
-	assert.Nil(t, nm.Orchestrator(core.LegacySystemNamespace))
+	assert.Equal(t, mo, nm.MustOrchestrator("default"))
+	assert.Panics(t, func() { nm.MustOrchestrator("unknown") })
+	assert.Panics(t, func() { nm.MustOrchestrator(core.LegacySystemNamespace) })
 
 	mo.AssertExpectations(t)
 }
@@ -359,9 +359,9 @@ func TestInitVersion1(t *testing.T) {
 	err := nm.Init(ctx, cancelCtx, nm.reset)
 	assert.NoError(t, err)
 
-	assert.Equal(t, mo, nm.Orchestrator("default"))
-	assert.Nil(t, nm.Orchestrator("unknown"))
-	assert.NotNil(t, nm.Orchestrator(core.LegacySystemNamespace))
+	assert.Equal(t, mo, nm.MustOrchestrator("default"))
+	assert.Panics(t, func() { nm.MustOrchestrator("unknown") })
+	assert.NotNil(t, nm.MustOrchestrator(core.LegacySystemNamespace))
 
 	mo.AssertExpectations(t)
 }
@@ -408,8 +408,8 @@ func TestInitFFSystemWithTerminatedV1Contract(t *testing.T) {
 	err := nm.Init(ctx, cancelCtx, nm.reset)
 	assert.NoError(t, err)
 
-	assert.Equal(t, mo, nm.Orchestrator("default"))
-	assert.Nil(t, nm.Orchestrator("unknown"))
+	assert.Equal(t, mo, nm.MustOrchestrator("default"))
+	assert.Panics(t, func() { nm.MustOrchestrator("unknown") })
 
 	mo.AssertExpectations(t)
 }
@@ -476,8 +476,8 @@ func TestLegacyNamespaceConflictingPlugins(t *testing.T) {
 	err = nm.Init(ctx, cancelCtx, nm.reset)
 	assert.Regexp(t, "FF10421", err)
 
-	assert.Equal(t, mo, nm.Orchestrator("default"))
-	assert.Nil(t, nm.Orchestrator("unknown"))
+	assert.Equal(t, mo, nm.MustOrchestrator("default"))
+	assert.Panics(t, func() { nm.MustOrchestrator("unknown") })
 
 	mo.AssertExpectations(t)
 }
@@ -543,8 +543,8 @@ func TestLegacyNamespaceConflictingPluginsTooManyPlugins(t *testing.T) {
 	err = nm.Init(ctx, cancelCtx, nm.reset)
 	assert.Regexp(t, "FF10421", err)
 
-	assert.Equal(t, mo, nm.Orchestrator("default"))
-	assert.Nil(t, nm.Orchestrator("unknown"))
+	assert.Equal(t, mo, nm.MustOrchestrator("default"))
+	assert.Panics(t, func() { nm.MustOrchestrator("unknown") })
 
 	mo.AssertExpectations(t)
 }
@@ -611,8 +611,8 @@ func TestLegacyNamespaceMatchingPlugins(t *testing.T) {
 	err = nm.Init(ctx, cancelCtx, nm.reset)
 	assert.NoError(t, err)
 
-	assert.Equal(t, mo, nm.Orchestrator("default"))
-	assert.Nil(t, nm.Orchestrator("unknown"))
+	assert.Equal(t, mo, nm.MustOrchestrator("default"))
+	assert.Panics(t, func() { nm.MustOrchestrator("unknown") })
 
 	mo.AssertExpectations(t)
 }
@@ -1919,7 +1919,7 @@ func TestGetOperationByNamespacedIDNoOrchestrator(t *testing.T) {
 	opID := fftypes.NewUUID()
 
 	_, err := nm.GetOperationByNamespacedID(context.Background(), "bad:"+opID.String())
-	assert.Regexp(t, "FF10109", err)
+	assert.Regexp(t, "FF10436", err)
 
 	mo.AssertExpectations(t)
 }
@@ -1972,7 +1972,7 @@ func TestResolveOperationByNamespacedIDNoOrchestrator(t *testing.T) {
 	opID := fftypes.NewUUID()
 
 	err := nm.ResolveOperationByNamespacedID(context.Background(), "bad:"+opID.String(), &core.OperationUpdateDTO{})
-	assert.Regexp(t, "FF10109", err)
+	assert.Regexp(t, "FF10436", err)
 
 	mo.AssertExpectations(t)
 }
@@ -1994,6 +1994,15 @@ func TestAuthorize(t *testing.T) {
 	mo.AssertExpectations(t)
 }
 
+func TestAuthorizeBadNamespace(t *testing.T) {
+	nm := newTestNamespaceManager(true)
+	defer nm.cleanup(t)
+	err := nm.Authorize(context.Background(), &fftypes.AuthReq{
+		Namespace: "ns1",
+	})
+	assert.Regexp(t, "FF10436", err)
+}
+
 func TestValidateNonMultipartyConfig(t *testing.T) {
 	nm := newTestNamespaceManager(true)
 	defer nm.cleanup(t)
diff --git a/internal/orchestrator/persistence_events_test.go b/internal/orchestrator/persistence_events_test.go
index 3276082fa..5dab7a16b 100644
--- a/internal/orchestrator/persistence_events_test.go
+++ b/internal/orchestrator/persistence_events_test.go
@@ -93,6 +93,16 @@ func TestSubscriptionDeleted(t *testing.T) {
 	mem.AssertExpectations(t)
 }
 
+func TestGroupCreatedNOOP(t *testing.T) {
+	mem := &eventmocks.EventManager{}
+	o := &orchestrator{
+		namespace: &core.Namespace{Name: "ns1", NetworkName: "ns1"},
+		events:    mem,
+	}
+	o.HashCollectionNSEvent(database.CollectionGroups, core.ChangeEventTypeCreated, "ns1", fftypes.NewRandB32())
+	mem.AssertExpectations(t)
+}
+
 func TestOrderedUUIDCollectionWrongNS(t *testing.T) {
 	o := &orchestrator{
 		ctx:       context.Background(),