Fix bugs when running with an old config file

docs/reference/config.md

@@ -17,6 +17,12 @@ nav_order: 2
 ---
 
 
+## admin
+
+|Key|Description|Type|Default Value|
+|---|-----------|----|-------------|
+|enabled|Deprecated - use spi.enabled instead|`boolean`|`<nil>`
+
 ## api
 
 |Key|Description|Type|Default Value|

ffconfig/migrate/migrations.go

@@ -28,7 +28,6 @@ import (
 var migrations = map[string]func(root *ConfigItem){
 	"1.0.0": func(root *ConfigItem) {
 		root.Get("org").Get("identity").RenameTo("key")
-		root.Get("publicstorage").RenameTo("sharedstorage")
 		root.Get("tokens").Each().Get("connector").RenameTo("plugin").ReplaceValue("https", "fftokens")
 		root.Get("dataexchange").Get("type").ReplaceValue("https", "ffdx")
 		root.Get("dataexchange").Get("https").RenameTo("ffdx")
@@ -38,6 +37,10 @@ var migrations = map[string]func(root *ConfigItem){
 		root.Get("dataexchange").Get("type").SetIfEmpty("ffdx")
 	},
 
+	"1.0.4": func(root *ConfigItem) {
+		root.Get("publicstorage").RenameTo("sharedstorage")
+	},
+
 	"1.1.0": func(root *ConfigItem) {
 		root.Get("admin").RenameTo("spi")
 

ffconfig/migrate/migrations_test.go

@@ -34,7 +34,7 @@ publicstorage:
 	assert.Equal(t, `database:
   type: sqlite3
 dataexchange: {}
-sharedstorage:
+publicstorage:
   type: ipfs
 `, string(result))
 }
@@ -43,34 +43,26 @@ func TestMigratev1_0_0(t *testing.T) {
 	config := []byte(`database:
   type: sqlite3
 dataexchange: {}
-publicstorage:
-  type: ipfs
 `)
 	result, err := Run(config, "", "v1.0.0")
 	assert.NoError(t, err)
 	assert.Equal(t, `database:
   type: sqlite3
 dataexchange: {}
-sharedstorage:
-  type: ipfs
 `, string(result))
 }
 
 func TestMigrate1_0_3(t *testing.T) {
 	config := []byte(`database:
   type: sqlite3
 dataexchange: {}
-publicstorage:
-  type: ipfs
 `)
 	result, err := Run(config, "", "1.0.3")
 	assert.NoError(t, err)
 	assert.Equal(t, `database:
   type: sqlite3
 dataexchange:
   type: ffdx
-sharedstorage:
-  type: ipfs
 `, string(result))
 }
 
@@ -92,6 +84,24 @@ publicstorage:
 `, string(result))
 }
 
+func TestMigrate1_0_4(t *testing.T) {
+	config := []byte(`database:
+  type: sqlite3
+dataexchange: {}
+publicstorage:
+  type: ipfs
+`)
+	result, err := Run(config, "", "1.0.4")
+	assert.NoError(t, err)
+	assert.Equal(t, `database:
+  type: sqlite3
+dataexchange:
+  type: ffdx
+sharedstorage:
+  type: ipfs
+`, string(result))
+}
+
 func TestMigrate1_1_0(t *testing.T) {
 	config := []byte(`database:
   type: sqlite3

internal/apiserver/server.go

@@ -34,6 +34,7 @@ import (
 	"github.com/hyperledger/firefly-common/pkg/fftypes"
 	"github.com/hyperledger/firefly-common/pkg/httpserver"
 	"github.com/hyperledger/firefly-common/pkg/i18n"
+	"github.com/hyperledger/firefly-common/pkg/log"
 	"github.com/hyperledger/firefly/internal/coreconfig"
 	"github.com/hyperledger/firefly/internal/coremsgs"
 	"github.com/hyperledger/firefly/internal/events/eifactory"
@@ -109,6 +110,8 @@ func (as *apiServer) Serve(ctx context.Context, mgr namespace.Manager) (err erro
 			return err
 		}
 		go spiHTTPServer.ServeHTTP(ctx)
+	} else if config.GetBool(coreconfig.LegacyAdminEnabled) {
+		log.L(ctx).Warnf("Your config includes an 'admin' section, which should be renamed to 'spi' - SPI server will not be enabled until this is corrected")
 	}
 
 	if as.metricsEnabled {

internal/apiserver/server_test.go

@@ -97,6 +97,24 @@ func TestStartStopServer(t *testing.T) {
 	assert.NoError(t, err)
 }
 
+func TestStartLegacyAdminConfig(t *testing.T) {
+	coreconfig.Reset()
+	metrics.Clear()
+	InitConfig()
+	apiConfig.Set(httpserver.HTTPConfPort, 0)
+	spiConfig.Set(httpserver.HTTPConfPort, 0)
+	config.Set(coreconfig.UIPath, "test")
+	config.Set(coreconfig.LegacyAdminEnabled, true)
+	ctx, cancel := context.WithCancel(context.Background())
+	cancel() // server will immediately shut down
+	as := NewAPIServer()
+	mgr := &namespacemocks.Manager{}
+	mae := &spieventsmocks.Manager{}
+	mgr.On("SPIEvents").Return(mae)
+	err := as.Serve(ctx, mgr)
+	assert.NoError(t, err)
+}
+
 func TestStartAPIFail(t *testing.T) {
 	coreconfig.Reset()
 	metrics.Clear()

internal/blockchain/ethereum/ethereum.go

@@ -788,7 +788,15 @@ func (e *Ethereum) GetNetworkVersion(ctx context.Context, location *fftypes.JSON
 		return cached.Value().(int), nil
 	}
 
-	res, err := e.queryContractMethod(ctx, ethLocation.Address, networkVersionMethodABI, []interface{}{}, nil)
+	version, err = e.queryNetworkVersion(ctx, ethLocation.Address)
+	if err == nil {
+		e.cache.Set(cacheKey, version, e.cacheTTL)
+	}
+	return version, err
+}
+
+func (e *Ethereum) queryNetworkVersion(ctx context.Context, address string) (version int, err error) {
+	res, err := e.queryContractMethod(ctx, address, networkVersionMethodABI, []interface{}{}, nil)
 	if err != nil || !res.IsSuccess() {
 		// "Call failed" is interpreted as "method does not exist, default to version 1"
 		if strings.Contains(err.Error(), "FFEC100148") {
@@ -804,9 +812,6 @@ func (e *Ethereum) GetNetworkVersion(ctx context.Context, location *fftypes.JSON
 	switch result := output.Output.(type) {
 	case string:
 		version, err = strconv.Atoi(result)
-		if err == nil {
-			e.cache.Set(cacheKey, version, e.cacheTTL)
-		}
 	default:
 		err = i18n.NewError(ctx, coremsgs.MsgBadNetworkVersion, output.Output)
 	}

internal/blockchain/fabric/fabric.go

@@ -855,7 +855,15 @@ func (f *Fabric) GetNetworkVersion(ctx context.Context, location *fftypes.JSONAn
 		return cached.Value().(int), nil
 	}
 
-	res, err := f.queryContractMethod(ctx, fabricOnChainLocation.Channel, fabricOnChainLocation.Chaincode, networkVersionMethodName, f.signer, "", []*PrefixItem{}, map[string]interface{}{}, nil)
+	version, err = f.queryNetworkVersion(ctx, fabricOnChainLocation.Channel, fabricOnChainLocation.Chaincode)
+	if err == nil {
+		f.cache.Set(cacheKey, version, f.cacheTTL)
+	}
+	return version, err
+}
+
+func (f *Fabric) queryNetworkVersion(ctx context.Context, channel, chaincode string) (version int, err error) {
+	res, err := f.queryContractMethod(ctx, channel, chaincode, networkVersionMethodName, f.signer, "", []*PrefixItem{}, map[string]interface{}{}, nil)
 	if err != nil || !res.IsSuccess() {
 		// "Function not found" is interpreted as "default to version 1"
 		notFoundError := fmt.Sprintf("Function %s not found", networkVersionMethodName)
@@ -872,9 +880,6 @@ func (f *Fabric) GetNetworkVersion(ctx context.Context, location *fftypes.JSONAn
 	switch result := output.Result.(type) {
 	case float64:
 		version = int(result)
-		if err == nil {
-			f.cache.Set(cacheKey, version, f.cacheTTL)
-		}
 	default:
 		err = i18n.NewError(ctx, coremsgs.MsgBadNetworkVersion, output.Result)
 	}

internal/coreconfig/coreconfig.go

@@ -220,6 +220,8 @@ var (
 	GroupCacheLimit = ffc("group.cache.limit")
 	// GroupCacheTTL cache time-to-live for private group addresses
 	GroupCacheTTL = ffc("group.cache.ttl")
+	// LegacyAdminEnabled is the deprecated key that pre-dates spi.enabled
+	LegacyAdminEnabled = ffc("admin.enabled")
 	// SPIEnabled determines whether the admin interface will be enabled or not
 	SPIEnabled = ffc("spi.enabled")
 	// SPIWebSocketEventQueueLength is the maximum number of events that will queue up on the server side of each WebSocket connection before events start being dropped

internal/coremsgs/en_config_descriptions.go

@@ -31,6 +31,7 @@ var (
 	ConfigGlobalMigrationsDirectory = ffc("config.global.migrations.directory", "The directory containing the numerically ordered migration DDL files to apply to the database", i18n.StringType)
 	ConfigGlobalShutdownTimeout     = ffc("config.global.shutdownTimeout", "The maximum amount of time to wait for any open HTTP requests to finish before shutting down the HTTP server", i18n.TimeDurationType)
 
+	ConfigLegacyAdmin     = ffc("config.admin.enabled", "Deprecated - use spi.enabled instead", i18n.BooleanType)
 	ConfigSPIAddress      = ffc("config.spi.address", "The IP address on which the admin HTTP API should listen", "IP Address "+i18n.StringType)
 	ConfigSPIEnabled      = ffc("config.spi.enabled", "Enables the admin HTTP API", i18n.BooleanType)
 	ConfigSPIPort         = ffc("config.spi.port", "The port on which the admin HTTP API should listen", i18n.IntType)

internal/multiparty/manager.go

@@ -136,9 +136,15 @@ func (mm *multipartyManager) ConfigureContract(ctx context.Context) (err error)
 }
 
 func (mm *multipartyManager) configureContractCommon(ctx context.Context, migration bool) (err error) {
-	contracts := mm.namespace.Contracts
-	log.L(ctx).Infof("Resolving FireFly contract at index %d", contracts.Active.Index)
-	location, firstEvent, err := mm.resolveFireFlyContract(ctx, contracts.Active.Index)
+	if mm.namespace.Contracts == nil {
+		mm.namespace.Contracts = &core.MultipartyContracts{}
+	}
+	if mm.namespace.Contracts.Active == nil {
+		mm.namespace.Contracts.Active = &core.MultipartyContract{}
+	}
+	active := mm.namespace.Contracts.Active
+	log.L(ctx).Infof("Resolving FireFly contract at index %d", active.Index)
+	location, firstEvent, err := mm.resolveFireFlyContract(ctx, active.Index)
 	if err != nil {
 		return err
 	}
@@ -149,17 +155,17 @@ func (mm *multipartyManager) configureContractCommon(ctx context.Context, migrat
 	}
 
 	if !migration {
-		if !contracts.Active.Location.IsNil() && contracts.Active.Location.String() != location.String() {
-			log.L(ctx).Warnf("FireFly contract location changed from %s to %s", contracts.Active.Location, location)
+		if !active.Location.IsNil() && active.Location.String() != location.String() {
+			log.L(ctx).Warnf("FireFly contract location changed from %s to %s", active.Location, location)
 		}
 	}
 
 	subID, err := mm.blockchain.AddFireflySubscription(ctx, mm.namespace.Ref(), location, firstEvent)
 	if err == nil {
-		contracts.Active.Location = location
-		contracts.Active.FirstEvent = firstEvent
-		contracts.Active.Info.Subscription = subID
-		contracts.Active.Info.Version = version
+		active.Location = location
+		active.FirstEvent = firstEvent
+		active.Info.Subscription = subID
+		active.Info.Version = version
 		err = mm.database.UpsertNamespace(ctx, mm.namespace, true)
 	}
 	return err

internal/multiparty/manager_test.go

@@ -156,21 +156,25 @@ func TestConfigureContractLocationChanged(t *testing.T) {
 	assert.NoError(t, err)
 }
 
-func TestResolveContractDeprecatedConfig(t *testing.T) {
+func TestConfigureContractDeprecatedConfig(t *testing.T) {
 	mp := newTestMultipartyManager()
 	defer mp.cleanup(t)
+	mp.namespace.Contracts = nil
 
 	mp.mbi.On("GetAndConvertDeprecatedContractConfig", context.Background()).Return(fftypes.JSONAnyPtr(fftypes.JSONObject{
 		"address": "0x123",
 	}.String()), "0", nil)
+	mp.mbi.On("GetNetworkVersion", mock.Anything, mock.Anything).Return(1, nil)
+	mp.mbi.On("AddFireflySubscription", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return("test", nil)
+	mp.mdi.On("UpsertNamespace", mock.Anything, mock.AnythingOfType("*core.Namespace"), true).Return(nil)
 
-	loc, firstBlock, err := mp.resolveFireFlyContract(context.Background(), 0)
+	err := mp.ConfigureContract(context.Background())
 
 	location := fftypes.JSONAnyPtr(fftypes.JSONObject{
 		"address": "0x123",
 	}.String())
-	assert.Equal(t, location, loc)
-	assert.Equal(t, "0", firstBlock)
+	assert.Equal(t, location, mp.namespace.Contracts.Active.Location)
+	assert.Equal(t, "0", mp.namespace.Contracts.Active.FirstEvent)
 	assert.NoError(t, err)
 }
 

internal/namespace/manager.go

@@ -489,6 +489,7 @@ func (nm *namespaceManager) getTokensPlugins(ctx context.Context) (plugins map[s
 			if err = fftypes.ValidateFFNameField(ctx, name, "name"); err != nil {
 				return nil, err
 			}
+			nm.tokenRemoteNames[name] = name
 
 			plugin, err := tifactory.GetPlugin(ctx, pluginType)
 			if err != nil {

test/e2e/common.go

@@ -49,7 +49,18 @@ func WriteConfig(t *testing.T, configFile string, data map[string]interface{}) {
 }
 
 func AddNamespace(data map[string]interface{}, ns map[string]interface{}) {
+	if _, ok := data["namespaces"]; !ok {
+		data["namespaces"] = make(map[interface{}]interface{})
+	}
 	namespaces := data["namespaces"].(map[interface{}]interface{})
+	if _, ok := namespaces["default"]; !ok {
+		namespaces["default"] = "default"
+	}
+	if _, ok := namespaces["predefined"]; !ok {
+		namespaces["predefined"] = []interface{}{
+			map[string]interface{}{"name": namespaces["default"]},
+		}
+	}
 	predefined := namespaces["predefined"].([]interface{})
 	namespaces["predefined"] = append(predefined, ns)
 }