From 15daa6048c45a1595de6c38bc002921585b8fd34 Mon Sep 17 00:00:00 2001 From: Alex Shorsher Date: Mon, 11 Jul 2022 09:48:12 -0400 Subject: [PATCH 1/5] make blockchain plugin optional in managers Signed-off-by: Alex Shorsher --- internal/identity/identitymanager.go | 2 +- internal/identity/identitymanager_test.go | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/internal/identity/identitymanager.go b/internal/identity/identitymanager.go index 78d44b5107..2e209af2e0 100644 --- a/internal/identity/identitymanager.go +++ b/internal/identity/identitymanager.go @@ -122,7 +122,7 @@ func (im *identityManager) NormalizeSigningKey(ctx context.Context, inputKey str // If the caller is not confident that the blockchain plugin/connector should be used to resolve, // for example it might be a different blockchain (Eth vs Fabric etc.), or it has its own // verification/management of keys, it should set `assets.keyNormalization: "none"` in the config. - if keyNormalizationMode != KeyNormalizationBlockchainPlugin { + if keyNormalizationMode != KeyNormalizationBlockchainPlugin || im.blockchain == nil { return inputKey, nil } signer, err := im.normalizeKeyViaBlockchainPlugin(ctx, inputKey) diff --git a/internal/identity/identitymanager_test.go b/internal/identity/identitymanager_test.go index a99c911a71..901a87de48 100644 --- a/internal/identity/identitymanager_test.go +++ b/internal/identity/identitymanager_test.go @@ -501,6 +501,26 @@ func TestNormalizeSigningKeyDefaultNoBlockchainDefaultKeyFallback(t *testing.T) assert.Equal(t, "key123", resolvedKey) } +func TestNormalizeSigningKeyDefaultNoBlockchainInputFallback(t *testing.T) { + ctx, im := newTestIdentityManager(t) + im.blockchain = nil + im.defaultKey = "key123" + + resolvedKey, err := im.NormalizeSigningKey(ctx, "testKey", KeyNormalizationBlockchainPlugin) + assert.NoError(t, err) + assert.Equal(t, "testKey", resolvedKey) +} + +func TestNormalizeSigningKeyDefaultNoBlockchainDefaultKeyFallback(t *testing.T) { + ctx, im := newTestIdentityManager(t) + im.blockchain = nil + im.defaultKey = "key123" + + resolvedKey, err := im.NormalizeSigningKey(ctx, "", KeyNormalizationBlockchainPlugin) + assert.NoError(t, err) + assert.Equal(t, "key123", resolvedKey) +} + func TestNormalizeSigningKeyOrgFallbackOk(t *testing.T) { ctx, im := newTestIdentityManager(t) From 274d5f28397e00bfd499420f76d853dcb14b107c Mon Sep 17 00:00:00 2001 From: Alex Shorsher Date: Mon, 11 Jul 2022 15:05:11 -0400 Subject: [PATCH 2/5] add `remotename` to tokens plugin config Signed-off-by: Alex Shorsher --- docs/reference/config.md | 1 + internal/coremsgs/en_config_descriptions.go | 11 ++++++----- internal/tokens/tifactory/factory.go | 1 + 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/docs/reference/config.md b/docs/reference/config.md index 8d423c45e4..b62a378a14 100644 --- a/docs/reference/config.md +++ b/docs/reference/config.md @@ -1084,6 +1084,7 @@ nav_order: 2 |Key|Description|Type|Default Value| |---|-----------|----|-------------| |name|The name of the Tokens Connector. This will be used in the FireFly API path to refer to this specific Token Connector|`string`|`` +|remotename|The remote name of the Tokens Connector. This will be used in messages and events to refer to this specific Token Connector, if it differs from the name|`string`|`` |type|The type of the Tokens Connector plugin to use|`string`|`` ## plugins.tokens[].fftokens diff --git a/internal/coremsgs/en_config_descriptions.go b/internal/coremsgs/en_config_descriptions.go index 31c099deff..fbdd452a5f 100644 --- a/internal/coremsgs/en_config_descriptions.go +++ b/internal/coremsgs/en_config_descriptions.go @@ -306,11 +306,12 @@ var ( ConfigTokensPlugin = ffc("config.tokens[].plugin", "The name of the Tokens Connector plugin to use", i18n.StringType) ConfigTokensURL = ffc("config.tokens[].url", "The URL of the Token Connector", "URL "+i18n.StringType) - ConfigPluginTokens = ffc("config.plugins.tokens", "The tokens plugin configurations. This will be used to configure tokens connectors", i18n.StringType) - ConfigPluginTokensName = ffc("config.plugins.tokens[].name", "The name of the Tokens Connector. This will be used in the FireFly API path to refer to this specific Token Connector", i18n.StringType) - ConfigPluginTokensType = ffc("config.plugins.tokens[].type", "The type of the Tokens Connector plugin to use", i18n.StringType) - ConfigPluginTokensURL = ffc("config.plugins.tokens[].fftokens.url", "The URL of the Token Connector", "URL "+i18n.StringType) - ConfigPluginTokensProxyURL = ffc("config.plugins.tokens[].fftokens.proxy.url", "Optional HTTP proxy server to use when connecting to the Token Connector", "URL "+i18n.StringType) + ConfigPluginTokens = ffc("config.plugins.tokens", "The tokens plugin configurations. This will be used to configure tokens connectors", i18n.StringType) + ConfigPluginTokensName = ffc("config.plugins.tokens[].name", "The name of the Tokens Connector. This will be used in the FireFly API path to refer to this specific Token Connector", i18n.StringType) + ConfigPluginTokensRemoteName = ffc("config.plugins.tokens[].remotename", "The remote name of the Tokens Connector. This will be used in messages and events to refer to this specific Token Connector, if it differs from the name", i18n.StringType) + ConfigPluginTokensType = ffc("config.plugins.tokens[].type", "The type of the Tokens Connector plugin to use", i18n.StringType) + ConfigPluginTokensURL = ffc("config.plugins.tokens[].fftokens.url", "The URL of the Token Connector", "URL "+i18n.StringType) + ConfigPluginTokensProxyURL = ffc("config.plugins.tokens[].fftokens.proxy.url", "Optional HTTP proxy server to use when connecting to the Token Connector", "URL "+i18n.StringType) ConfigTokensProxyURL = ffc("config.tokens[].proxy.url", "Optional HTTP proxy server to use when connecting to the Token Connector", "URL "+i18n.StringType) diff --git a/internal/tokens/tifactory/factory.go b/internal/tokens/tifactory/factory.go index 83843bcbb6..e20a0caca0 100644 --- a/internal/tokens/tifactory/factory.go +++ b/internal/tokens/tifactory/factory.go @@ -34,6 +34,7 @@ var pluginsByName = map[string]func() tokens.Plugin{ func InitConfig(config config.ArraySection) { config.AddKnownKey(coreconfig.PluginConfigName) config.AddKnownKey(coreconfig.PluginConfigType) + config.AddKnownKey(coreconfig.NamespaceRemoteName) for name, plugin := range pluginsByName { plugin().InitConfig(config.SubSection(name)) } From 27dd821e52c56b2044898411ef6e8986690f4ac7 Mon Sep 17 00:00:00 2001 From: Alex Shorsher Date: Tue, 12 Jul 2022 16:25:11 -0400 Subject: [PATCH 3/5] translate between token plugin name & remote name - introduce a new `remotename` key for tokens plugins - `remotename` must be unique The definitions package now utilizes the `remotename` of tokens plugins. `defsender` will replace the local token plugin name with the remote name before sending the definition. The `defhandler` will then replace the `remotename` with the local name after receiving a broadcast. Signed-off-by: Alex Shorsher --- internal/coreconfig/coreconfig.go | 2 + internal/coremsgs/en_error_messages.go | 1 + internal/definitions/handler.go | 4 +- internal/definitions/handler_test.go | 6 +- internal/definitions/handler_tokenpool.go | 7 +++ .../definitions/handler_tokenpool_test.go | 3 +- internal/definitions/sender.go | 49 +++++++++------ internal/definitions/sender_test.go | 6 +- internal/definitions/sender_tokenpool.go | 8 +++ internal/definitions/sender_tokenpool_test.go | 1 + internal/identity/identitymanager_test.go | 20 ------- internal/namespace/manager.go | 29 ++++++--- internal/namespace/manager_test.go | 59 ++++++++++++++++++- internal/orchestrator/orchestrator.go | 7 ++- internal/tokens/tifactory/factory.go | 2 +- 15 files changed, 145 insertions(+), 59 deletions(-) diff --git a/internal/coreconfig/coreconfig.go b/internal/coreconfig/coreconfig.go index 1a19cd5c03..a9109d3719 100644 --- a/internal/coreconfig/coreconfig.go +++ b/internal/coreconfig/coreconfig.go @@ -29,6 +29,8 @@ const ( PluginConfigName = "name" // PluginConfigType is the type of the plugin to be loaded PluginConfigType = "type" + // PluginRemoteName is the plugin name to be sent in plugin calls + PluginRemoteName = "remotename" // NamespaceName is the short name for a pre-defined namespace NamespaceName = "name" // NamespaceName is the long description for a pre-defined namespace diff --git a/internal/coremsgs/en_error_messages.go b/internal/coremsgs/en_error_messages.go index c67896efe5..623cdab0a5 100644 --- a/internal/coremsgs/en_error_messages.go +++ b/internal/coremsgs/en_error_messages.go @@ -258,4 +258,5 @@ var ( MsgInvalidSubscriptionForNetwork = ffe("FF10416", "Subscription name '%s' is invalid according to multiparty network rules in effect (network version=%d)") MsgBlockchainNotConfigured = ffe("FF10417", "No blockchain plugin configured") MsgInvalidBatchPinEvent = ffe("FF10418", "BatchPin event is not valid - %s (%s): %s") + MsgDuplicatePluginRemoteName = ffe("FF10419", "Invalid %s plugin remote name: %s - remote names must be unique") ) diff --git a/internal/definitions/handler.go b/internal/definitions/handler.go index 56b0c1b43a..0d7abf2c61 100644 --- a/internal/definitions/handler.go +++ b/internal/definitions/handler.go @@ -86,9 +86,10 @@ type definitionHandler struct { identity identity.Manager assets assets.Manager contracts contracts.Manager // optional + tokenNames map[string]string // mapping of token connector remote name => name } -func newDefinitionHandler(ctx context.Context, ns string, multiparty bool, di database.Plugin, bi blockchain.Plugin, dx dataexchange.Plugin, dm data.Manager, im identity.Manager, am assets.Manager, cm contracts.Manager) (*definitionHandler, error) { +func newDefinitionHandler(ctx context.Context, ns string, multiparty bool, di database.Plugin, bi blockchain.Plugin, dx dataexchange.Plugin, dm data.Manager, im identity.Manager, am assets.Manager, cm contracts.Manager, tokenNames map[string]string) (*definitionHandler, error) { if di == nil || dm == nil || im == nil || am == nil { return nil, i18n.NewError(ctx, coremsgs.MsgInitializationNilDepError, "DefinitionHandler") } @@ -102,6 +103,7 @@ func newDefinitionHandler(ctx context.Context, ns string, multiparty bool, di da identity: im, assets: am, contracts: cm, + tokenNames: tokenNames, }, nil } diff --git a/internal/definitions/handler_test.go b/internal/definitions/handler_test.go index 9a8c968057..495dbea81f 100644 --- a/internal/definitions/handler_test.go +++ b/internal/definitions/handler_test.go @@ -41,8 +41,10 @@ func newTestDefinitionHandler(t *testing.T) (*definitionHandler, *testDefinition mim := &identitymanagermocks.Manager{} mam := &assetmocks.Manager{} mcm := &contractmocks.Manager{} + tokenNames := make(map[string]string) + tokenNames["remote1"] = "connector1" mbi.On("VerifierType").Return(core.VerifierTypeEthAddress).Maybe() - dh, _ := newDefinitionHandler(context.Background(), "ns1", false, mdi, mbi, mdx, mdm, mim, mam, mcm) + dh, _ := newDefinitionHandler(context.Background(), "ns1", false, mdi, mbi, mdx, mdm, mim, mam, mcm, tokenNames) return dh, newTestDefinitionBatchState(t) } @@ -68,7 +70,7 @@ func (bs *testDefinitionBatchState) assertNoFinalizers() { } func TestInitFail(t *testing.T) { - _, err := newDefinitionHandler(context.Background(), "", false, nil, nil, nil, nil, nil, nil, nil) + _, err := newDefinitionHandler(context.Background(), "", false, nil, nil, nil, nil, nil, nil, nil, nil) assert.Regexp(t, "FF10128", err) } diff --git a/internal/definitions/handler_tokenpool.go b/internal/definitions/handler_tokenpool.go index 84c1231c6d..c67e715ae3 100644 --- a/internal/definitions/handler_tokenpool.go +++ b/internal/definitions/handler_tokenpool.go @@ -33,6 +33,13 @@ func (dh *definitionHandler) handleTokenPoolBroadcast(ctx context.Context, state } pool := announce.Pool + // Map remote connector name -> local name + if localName, ok := dh.tokenNames[pool.Connector]; ok { + pool.Connector = localName + } else { + log.L(ctx).Infof("Could not find local name for token connector remote name: %s", pool.Connector) + } + pool.Message = msg.Header.ID return dh.handleTokenPoolDefinition(ctx, state, pool) } diff --git a/internal/definitions/handler_tokenpool_test.go b/internal/definitions/handler_tokenpool_test.go index a5d99fe8fd..8611e2224a 100644 --- a/internal/definitions/handler_tokenpool_test.go +++ b/internal/definitions/handler_tokenpool_test.go @@ -43,6 +43,7 @@ func newPoolAnnouncement() *core.TokenPoolAnnouncement { Type: core.TransactionTypeTokenPool, ID: fftypes.NewUUID(), }, + Connector: "remote1", } return &core.TokenPoolAnnouncement{ Pool: pool, @@ -78,7 +79,7 @@ func TestHandleDefinitionBroadcastTokenPoolActivateOK(t *testing.T) { mam := sh.assets.(*assetmocks.Manager) mdi.On("GetTokenPoolByID", context.Background(), "ns1", pool.ID).Return(nil, nil) mdi.On("UpsertTokenPool", context.Background(), mock.MatchedBy(func(p *core.TokenPool) bool { - return *p.ID == *pool.ID && p.Message == msg.Header.ID + return *p.ID == *pool.ID && p.Message == msg.Header.ID && p.Connector == "connector1" })).Return(nil) mam.On("ActivateTokenPool", context.Background(), mock.AnythingOfType("*core.TokenPool")).Return(nil) diff --git a/internal/definitions/sender.go b/internal/definitions/sender.go index 5477c6f62f..ea67c15851 100644 --- a/internal/definitions/sender.go +++ b/internal/definitions/sender.go @@ -46,15 +46,16 @@ type Sender interface { } type definitionSender struct { - ctx context.Context - namespace string - multiparty bool - database database.Plugin - broadcast broadcast.Manager // optional - identity identity.Manager - data data.Manager - contracts contracts.Manager // optional - handler *definitionHandler + ctx context.Context + namespace string + multiparty bool + database database.Plugin + broadcast broadcast.Manager // optional + identity identity.Manager + data data.Manager + contracts contracts.Manager // optional + handler *definitionHandler + tokenRemoteNames map[string]string // mapping of token connector name => remote name } // Definitions that get processed immediately will create a temporary batch state and then finalize it inline @@ -70,25 +71,35 @@ func fakeBatch(ctx context.Context, handler func(context.Context, *core.BatchSta return err } -func NewDefinitionSender(ctx context.Context, ns string, multiparty bool, di database.Plugin, bi blockchain.Plugin, dx dataexchange.Plugin, bm broadcast.Manager, im identity.Manager, dm data.Manager, am assets.Manager, cm contracts.Manager) (Sender, Handler, error) { +func NewDefinitionSender(ctx context.Context, ns string, multiparty bool, di database.Plugin, bi blockchain.Plugin, dx dataexchange.Plugin, bm broadcast.Manager, im identity.Manager, dm data.Manager, am assets.Manager, cm contracts.Manager, tokenRemoteNames map[string]string) (Sender, Handler, error) { if di == nil || im == nil || dm == nil { return nil, nil, i18n.NewError(ctx, coremsgs.MsgInitializationNilDepError, "DefinitionSender") } ds := &definitionSender{ - ctx: ctx, - namespace: ns, - multiparty: multiparty, - database: di, - broadcast: bm, - identity: im, - data: dm, - contracts: cm, + ctx: ctx, + namespace: ns, + multiparty: multiparty, + database: di, + broadcast: bm, + identity: im, + data: dm, + contracts: cm, + tokenRemoteNames: tokenRemoteNames, } - dh, err := newDefinitionHandler(ctx, ns, multiparty, di, bi, dx, dm, im, am, cm) + dh, err := newDefinitionHandler(ctx, ns, multiparty, di, bi, dx, dm, im, am, cm, reverseMap(tokenRemoteNames)) ds.handler = dh return ds, dh, err } +// reverseMap reverses the key/values of a given map +func reverseMap(orderedMap map[string]string) map[string]string { + reverseMap := make(map[string]string, len(orderedMap)) + for k, v := range orderedMap { + reverseMap[v] = k + } + return reverseMap +} + func (bm *definitionSender) Name() string { return "DefinitionSender" } diff --git a/internal/definitions/sender_test.go b/internal/definitions/sender_test.go index a53a4e617a..345a36b4ca 100644 --- a/internal/definitions/sender_test.go +++ b/internal/definitions/sender_test.go @@ -44,15 +44,17 @@ func newTestDefinitionSender(t *testing.T) (*definitionSender, func()) { mdm := &datamocks.Manager{} mam := &assetmocks.Manager{} mcm := &contractmocks.Manager{} + tokenRemoteNames := make(map[string]string) + tokenRemoteNames["connector1"] = "remote1" ctx, cancel := context.WithCancel(context.Background()) - ds, _, err := NewDefinitionSender(ctx, "ns1", false, mdi, mbi, mdx, mbm, mim, mdm, mam, mcm) + ds, _, err := NewDefinitionSender(ctx, "ns1", false, mdi, mbi, mdx, mbm, mim, mdm, mam, mcm, tokenRemoteNames) assert.NoError(t, err) return ds.(*definitionSender), cancel } func TestInitSenderFail(t *testing.T) { - _, _, err := NewDefinitionSender(context.Background(), "", false, nil, nil, nil, nil, nil, nil, nil, nil) + _, _, err := NewDefinitionSender(context.Background(), "", false, nil, nil, nil, nil, nil, nil, nil, nil, nil) assert.Regexp(t, "FF10128", err) } diff --git a/internal/definitions/sender_tokenpool.go b/internal/definitions/sender_tokenpool.go index b25fb6b184..4aa03451c3 100644 --- a/internal/definitions/sender_tokenpool.go +++ b/internal/definitions/sender_tokenpool.go @@ -19,10 +19,18 @@ package definitions import ( "context" + "github.com/hyperledger/firefly-common/pkg/log" "github.com/hyperledger/firefly/pkg/core" ) func (bm *definitionSender) DefineTokenPool(ctx context.Context, pool *core.TokenPoolAnnouncement, waitConfirm bool) error { + // Map token connector name -> remote name + if remoteName, exists := bm.tokenRemoteNames[pool.Pool.Connector]; exists { + pool.Pool.Connector = remoteName + } else { + log.L(ctx).Infof("Could not find remote name for token connector: %s", pool.Pool.Connector) + } + if bm.multiparty { if err := pool.Pool.Validate(ctx); err != nil { return err diff --git a/internal/definitions/sender_tokenpool_test.go b/internal/definitions/sender_tokenpool_test.go index 4e7ab01a15..db2d8957ef 100644 --- a/internal/definitions/sender_tokenpool_test.go +++ b/internal/definitions/sender_tokenpool_test.go @@ -96,6 +96,7 @@ func TestDefineTokenPoolOk(t *testing.T) { Type: core.TokenTypeNonFungible, Locator: "N1", Symbol: "COIN", + Connector: "connector1", }, } diff --git a/internal/identity/identitymanager_test.go b/internal/identity/identitymanager_test.go index 901a87de48..584caae84d 100644 --- a/internal/identity/identitymanager_test.go +++ b/internal/identity/identitymanager_test.go @@ -481,16 +481,6 @@ func TestNormalizeSigningKeyNoDefaultNoBlockchain(t *testing.T) { } -func TestNormalizeSigningKeyDefaultNoBlockchainInputFallback(t *testing.T) { - ctx, im := newTestIdentityManager(t) - im.blockchain = nil - im.defaultKey = "key123" - - resolvedKey, err := im.NormalizeSigningKey(ctx, "testKey", KeyNormalizationBlockchainPlugin) - assert.Regexp(t, "FF10417", err) - assert.Equal(t, "", resolvedKey) -} - func TestNormalizeSigningKeyDefaultNoBlockchainDefaultKeyFallback(t *testing.T) { ctx, im := newTestIdentityManager(t) im.blockchain = nil @@ -511,16 +501,6 @@ func TestNormalizeSigningKeyDefaultNoBlockchainInputFallback(t *testing.T) { assert.Equal(t, "testKey", resolvedKey) } -func TestNormalizeSigningKeyDefaultNoBlockchainDefaultKeyFallback(t *testing.T) { - ctx, im := newTestIdentityManager(t) - im.blockchain = nil - im.defaultKey = "key123" - - resolvedKey, err := im.NormalizeSigningKey(ctx, "", KeyNormalizationBlockchainPlugin) - assert.NoError(t, err) - assert.Equal(t, "key123", resolvedKey) -} - func TestNormalizeSigningKeyOrgFallbackOk(t *testing.T) { ctx, im := newTestIdentityManager(t) diff --git a/internal/namespace/manager.go b/internal/namespace/manager.go index cffb456322..ee13325a17 100644 --- a/internal/namespace/manager.go +++ b/internal/namespace/manager.go @@ -105,10 +105,11 @@ type namespaceManager struct { events map[string]eventsPlugin auth map[string]authPlugin } - metricsEnabled bool - metrics metrics.Manager - adminEvents spievents.Manager - utOrchestrator orchestrator.Orchestrator + metricsEnabled bool + metrics metrics.Manager + adminEvents spievents.Manager + utOrchestrator orchestrator.Orchestrator + tokenRemoteNames map[string]string } type blockchainPlugin struct { @@ -153,8 +154,9 @@ type authPlugin struct { func NewNamespaceManager(withDefaults bool) Manager { nm := &namespaceManager{ - namespaces: make(map[string]*namespace), - metricsEnabled: config.GetBool(coreconfig.MetricsEnabled), + namespaces: make(map[string]*namespace), + metricsEnabled: config.GetBool(coreconfig.MetricsEnabled), + tokenRemoteNames: make(map[string]string), } InitConfig(withDefaults) @@ -347,6 +349,8 @@ func (nm *namespaceManager) loadPlugins(ctx context.Context) (err error) { func (nm *namespaceManager) getTokensPlugins(ctx context.Context) (plugins map[string]tokensPlugin, err error) { plugins = make(map[string]tokensPlugin) + // Remote names must be unique + remoteNames := make(map[string]bool) tokensConfigArraySize := tokensConfig.ArraySize() for i := 0; i < tokensConfigArraySize; i++ { @@ -355,6 +359,16 @@ func (nm *namespaceManager) getTokensPlugins(ctx context.Context) (plugins map[s if err != nil { return nil, err } + remoteName := config.GetString(coreconfig.PluginRemoteName) + // If there is no remote name, use the plugin name + if remoteName == "" { + remoteName = name + } + if _, exists := remoteNames[remoteName]; exists { + return nil, i18n.NewError(ctx, coremsgs.MsgDuplicatePluginRemoteName, "tokens", remoteName) + } + remoteNames[remoteName] = true + nm.tokenRemoteNames[name] = remoteName plugin, err := tifactory.GetPlugin(ctx, pluginType) if err != nil { @@ -742,7 +756,8 @@ func (nm *namespaceManager) loadNamespace(ctx context.Context, name string, inde } config := orchestrator.Config{ - DefaultKey: conf.GetString(coreconfig.NamespaceDefaultKey), + DefaultKey: conf.GetString(coreconfig.NamespaceDefaultKey), + TokenRemoteNames: nm.tokenRemoteNames, } var p *orchestrator.Plugins var err error diff --git a/internal/namespace/manager_test.go b/internal/namespace/manager_test.go index 2b5aeeefc0..49854e7362 100644 --- a/internal/namespace/manager_test.go +++ b/internal/namespace/manager_test.go @@ -94,9 +94,10 @@ func newTestNamespaceManager(resetConfig bool) *testNamespaceManager { mev: &eventsmocks.Plugin{}, auth: &authmocks.Plugin{}, namespaceManager: namespaceManager{ - ctx: context.Background(), - namespaces: make(map[string]*namespace), - pluginNames: make(map[string]bool), + ctx: context.Background(), + namespaces: make(map[string]*namespace), + pluginNames: make(map[string]bool), + tokenRemoteNames: make(map[string]string), }, } nm.plugins.blockchain = map[string]blockchainPlugin{ @@ -671,6 +672,58 @@ func TestTokensPlugin(t *testing.T) { assert.NoError(t, err) } +func TestTokensPluginDuplicateRemoteName(t *testing.T) { + nm := newTestNamespaceManager(true) + defer nm.cleanup(t) + tifactory.InitConfig(tokensConfig) + viper.SetConfigType("yaml") + err := viper.ReadConfig(strings.NewReader(` + plugins: + tokens: + - name: test1 + remotename: remote1 + type: fftokens + fftokens: + url: http://tokens:3000 + - name: test2 + remotename: remote1 + type: fftokens + fftokens: + url: http://tokens:3000 + `)) + assert.NoError(t, err) + + plugins, err := nm.getTokensPlugins(context.Background()) + assert.Nil(t, plugins) + assert.Regexp(t, "FF10419", err) +} + +func TestMultipleTokensPluginsWithRemoteName(t *testing.T) { + nm := newTestNamespaceManager(true) + defer nm.cleanup(t) + tifactory.InitConfig(tokensConfig) + viper.SetConfigType("yaml") + err := viper.ReadConfig(strings.NewReader(` + plugins: + tokens: + - name: test1 + remotename: remote1 + type: fftokens + fftokens: + url: http://tokens:3000 + - name: test2 + remotename: remote2 + type: fftokens + fftokens: + url: http://tokens:3000 + `)) + assert.NoError(t, err) + + plugins, err := nm.getTokensPlugins(context.Background()) + assert.Equal(t, 2, len(plugins)) + assert.NoError(t, err) +} + func TestTokensPluginNoType(t *testing.T) { nm := newTestNamespaceManager(true) defer nm.cleanup(t) diff --git a/internal/orchestrator/orchestrator.go b/internal/orchestrator/orchestrator.go index 668928763c..e4cdbb0885 100644 --- a/internal/orchestrator/orchestrator.go +++ b/internal/orchestrator/orchestrator.go @@ -169,8 +169,9 @@ type Plugins struct { } type Config struct { - DefaultKey string - Multiparty multiparty.Config + DefaultKey string + Multiparty multiparty.Config + TokenRemoteNames map[string]string } type orchestrator struct { @@ -482,7 +483,7 @@ func (or *orchestrator) initManagers(ctx context.Context) (err error) { } if or.defsender == nil { - or.defsender, or.defhandler, err = definitions.NewDefinitionSender(ctx, or.namespace.LocalName, or.config.Multiparty.Enabled, or.database(), or.blockchain(), or.dataexchange(), or.broadcast, or.identity, or.data, or.assets, or.contracts) + or.defsender, or.defhandler, err = definitions.NewDefinitionSender(ctx, or.namespace.LocalName, or.config.Multiparty.Enabled, or.database(), or.blockchain(), or.dataexchange(), or.broadcast, or.identity, or.data, or.assets, or.contracts, or.config.TokenRemoteNames) if err != nil { return err } diff --git a/internal/tokens/tifactory/factory.go b/internal/tokens/tifactory/factory.go index e20a0caca0..099c6e027d 100644 --- a/internal/tokens/tifactory/factory.go +++ b/internal/tokens/tifactory/factory.go @@ -34,7 +34,7 @@ var pluginsByName = map[string]func() tokens.Plugin{ func InitConfig(config config.ArraySection) { config.AddKnownKey(coreconfig.PluginConfigName) config.AddKnownKey(coreconfig.PluginConfigType) - config.AddKnownKey(coreconfig.NamespaceRemoteName) + config.AddKnownKey(coreconfig.PluginRemoteName) for name, plugin := range pluginsByName { plugin().InitConfig(config.SubSection(name)) } From 2072caaf6e5d4d60ee4d7d03b693cf54f88a0ca0 Mon Sep 17 00:00:00 2001 From: Alex Shorsher Date: Thu, 14 Jul 2022 22:37:13 -0400 Subject: [PATCH 4/5] revert identity manager changes Signed-off-by: Alex Shorsher --- internal/identity/identitymanager.go | 2 +- internal/identity/identitymanager_test.go | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/internal/identity/identitymanager.go b/internal/identity/identitymanager.go index 2e209af2e0..78d44b5107 100644 --- a/internal/identity/identitymanager.go +++ b/internal/identity/identitymanager.go @@ -122,7 +122,7 @@ func (im *identityManager) NormalizeSigningKey(ctx context.Context, inputKey str // If the caller is not confident that the blockchain plugin/connector should be used to resolve, // for example it might be a different blockchain (Eth vs Fabric etc.), or it has its own // verification/management of keys, it should set `assets.keyNormalization: "none"` in the config. - if keyNormalizationMode != KeyNormalizationBlockchainPlugin || im.blockchain == nil { + if keyNormalizationMode != KeyNormalizationBlockchainPlugin { return inputKey, nil } signer, err := im.normalizeKeyViaBlockchainPlugin(ctx, inputKey) diff --git a/internal/identity/identitymanager_test.go b/internal/identity/identitymanager_test.go index 584caae84d..a99c911a71 100644 --- a/internal/identity/identitymanager_test.go +++ b/internal/identity/identitymanager_test.go @@ -481,24 +481,24 @@ func TestNormalizeSigningKeyNoDefaultNoBlockchain(t *testing.T) { } -func TestNormalizeSigningKeyDefaultNoBlockchainDefaultKeyFallback(t *testing.T) { +func TestNormalizeSigningKeyDefaultNoBlockchainInputFallback(t *testing.T) { ctx, im := newTestIdentityManager(t) im.blockchain = nil im.defaultKey = "key123" - resolvedKey, err := im.NormalizeSigningKey(ctx, "", KeyNormalizationBlockchainPlugin) - assert.NoError(t, err) - assert.Equal(t, "key123", resolvedKey) + resolvedKey, err := im.NormalizeSigningKey(ctx, "testKey", KeyNormalizationBlockchainPlugin) + assert.Regexp(t, "FF10417", err) + assert.Equal(t, "", resolvedKey) } -func TestNormalizeSigningKeyDefaultNoBlockchainInputFallback(t *testing.T) { +func TestNormalizeSigningKeyDefaultNoBlockchainDefaultKeyFallback(t *testing.T) { ctx, im := newTestIdentityManager(t) im.blockchain = nil im.defaultKey = "key123" - resolvedKey, err := im.NormalizeSigningKey(ctx, "testKey", KeyNormalizationBlockchainPlugin) + resolvedKey, err := im.NormalizeSigningKey(ctx, "", KeyNormalizationBlockchainPlugin) assert.NoError(t, err) - assert.Equal(t, "testKey", resolvedKey) + assert.Equal(t, "key123", resolvedKey) } func TestNormalizeSigningKeyOrgFallbackOk(t *testing.T) { From 17dfa7819a693ee56f3c83a9d15751dd8d6766ca Mon Sep 17 00:00:00 2001 From: Alex Shorsher Date: Fri, 15 Jul 2022 07:30:18 -0400 Subject: [PATCH 5/5] additional error handling for remote/local name Signed-off-by: Alex Shorsher --- internal/coremsgs/en_error_messages.go | 1 + internal/definitions/handler_tokenpool.go | 3 +- .../definitions/handler_tokenpool_test.go | 20 ++++++ internal/definitions/sender_tokenpool.go | 3 + internal/definitions/sender_tokenpool_test.go | 64 ++++++++++++++++++- 5 files changed, 88 insertions(+), 3 deletions(-) diff --git a/internal/coremsgs/en_error_messages.go b/internal/coremsgs/en_error_messages.go index 623cdab0a5..402ddbd6be 100644 --- a/internal/coremsgs/en_error_messages.go +++ b/internal/coremsgs/en_error_messages.go @@ -259,4 +259,5 @@ var ( MsgBlockchainNotConfigured = ffe("FF10417", "No blockchain plugin configured") MsgInvalidBatchPinEvent = ffe("FF10418", "BatchPin event is not valid - %s (%s): %s") MsgDuplicatePluginRemoteName = ffe("FF10419", "Invalid %s plugin remote name: %s - remote names must be unique") + MsgInvalidConnectorName = ffe("FF10420", "Could not find name %s for %s connector") ) diff --git a/internal/definitions/handler_tokenpool.go b/internal/definitions/handler_tokenpool.go index c67e715ae3..13e24a7a85 100644 --- a/internal/definitions/handler_tokenpool.go +++ b/internal/definitions/handler_tokenpool.go @@ -37,7 +37,8 @@ func (dh *definitionHandler) handleTokenPoolBroadcast(ctx context.Context, state if localName, ok := dh.tokenNames[pool.Connector]; ok { pool.Connector = localName } else { - log.L(ctx).Infof("Could not find local name for token connector remote name: %s", pool.Connector) + log.L(ctx).Infof("Could not find local name for token connector: %s", pool.Connector) + return HandlerResult{Action: ActionReject}, i18n.NewError(ctx, coremsgs.MsgInvalidConnectorName, pool.Connector, "token") } pool.Message = msg.Header.ID diff --git a/internal/definitions/handler_tokenpool_test.go b/internal/definitions/handler_tokenpool_test.go index 8611e2224a..f08a4d1c55 100644 --- a/internal/definitions/handler_tokenpool_test.go +++ b/internal/definitions/handler_tokenpool_test.go @@ -93,6 +93,26 @@ func TestHandleDefinitionBroadcastTokenPoolActivateOK(t *testing.T) { mdi.AssertExpectations(t) } +func TestHandleDefinitionBroadcastTokenPoolBadConnector(t *testing.T) { + sh, bs := newTestDefinitionHandler(t) + + announce := newPoolAnnouncement() + pool := announce.Pool + pool.Name = "//bad" + msg, data, err := buildPoolDefinitionMessage(announce) + assert.NoError(t, err) + + mam := sh.assets.(*assetmocks.Manager) + mam.On("ActivateTokenPool", context.Background(), mock.AnythingOfType("*core.TokenPool")).Return(nil) + + action, err := sh.HandleDefinitionBroadcast(context.Background(), &bs.BatchState, msg, data, fftypes.NewUUID()) + assert.Equal(t, HandlerResult{Action: ActionReject, CustomCorrelator: pool.ID}, action) + assert.Regexp(t, "FF10403", err) + + err = bs.RunPreFinalize(context.Background()) + assert.NoError(t, err) +} + func TestHandleDefinitionBroadcastTokenPoolGetPoolFail(t *testing.T) { sh, bs := newTestDefinitionHandler(t) diff --git a/internal/definitions/sender_tokenpool.go b/internal/definitions/sender_tokenpool.go index 4aa03451c3..3b3632d728 100644 --- a/internal/definitions/sender_tokenpool.go +++ b/internal/definitions/sender_tokenpool.go @@ -19,7 +19,9 @@ 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" ) @@ -29,6 +31,7 @@ func (bm *definitionSender) DefineTokenPool(ctx context.Context, pool *core.Toke pool.Pool.Connector = remoteName } else { log.L(ctx).Infof("Could not find remote name for token connector: %s", pool.Pool.Connector) + return i18n.NewError(ctx, coremsgs.MsgInvalidConnectorName, remoteName, "token") } if bm.multiparty { diff --git a/internal/definitions/sender_tokenpool_test.go b/internal/definitions/sender_tokenpool_test.go index db2d8957ef..878f2e9a81 100644 --- a/internal/definitions/sender_tokenpool_test.go +++ b/internal/definitions/sender_tokenpool_test.go @@ -18,10 +18,12 @@ package definitions import ( "context" + "fmt" "testing" "github.com/hyperledger/firefly-common/pkg/fftypes" "github.com/hyperledger/firefly/mocks/broadcastmocks" + "github.com/hyperledger/firefly/mocks/databasemocks" "github.com/hyperledger/firefly/mocks/datamocks" "github.com/hyperledger/firefly/mocks/identitymanagermocks" "github.com/hyperledger/firefly/mocks/sysmessagingmocks" @@ -49,7 +51,7 @@ func TestBroadcastTokenPoolInvalid(t *testing.T) { } err := ds.DefineTokenPool(context.Background(), pool, false) - assert.Regexp(t, "FF00140", err) + assert.Regexp(t, "FF10420", err) mdm.AssertExpectations(t) } @@ -73,7 +75,7 @@ func TestBroadcastTokenPoolInvalidNonMultiparty(t *testing.T) { } err := ds.DefineTokenPool(context.Background(), pool, false) - assert.Regexp(t, "FF00140", err) + assert.Regexp(t, "FF10420", err) mdm.AssertExpectations(t) } @@ -112,3 +114,61 @@ func TestDefineTokenPoolOk(t *testing.T) { mbm.AssertExpectations(t) mms.AssertExpectations(t) } + +func TestDefineTokenPoolNonMultipartyTokenPoolFail(t *testing.T) { + ds, cancel := newTestDefinitionSender(t) + defer cancel() + + mdm := ds.data.(*datamocks.Manager) + mbm := ds.broadcast.(*broadcastmocks.Manager) + mdi := ds.database.(*databasemocks.Plugin) + + pool := &core.TokenPoolAnnouncement{ + Pool: &core.TokenPool{ + ID: fftypes.NewUUID(), + Namespace: "ns1", + Name: "mypool", + Type: core.TokenTypeNonFungible, + Locator: "N1", + Symbol: "COIN", + Connector: "connector1", + }, + } + + mdi.On("GetTokenPoolByID", context.Background(), "ns1", pool.Pool.ID).Return(nil, fmt.Errorf("pop")) + + err := ds.DefineTokenPool(context.Background(), pool, false) + assert.Regexp(t, "pop", err) + + mdm.AssertExpectations(t) + mbm.AssertExpectations(t) +} + +func TestDefineTokenPoolBadName(t *testing.T) { + ds, cancel := newTestDefinitionSender(t) + defer cancel() + ds.multiparty = true + + mim := ds.identity.(*identitymanagermocks.Manager) + mbm := ds.broadcast.(*broadcastmocks.Manager) + mms := &sysmessagingmocks.MessageSender{} + + pool := &core.TokenPoolAnnouncement{ + Pool: &core.TokenPool{ + ID: fftypes.NewUUID(), + Namespace: "ns1", + Name: "///bad/////", + Type: core.TokenTypeNonFungible, + Locator: "N1", + Symbol: "COIN", + Connector: "connector1", + }, + } + + mim.On("ResolveInputSigningIdentity", mock.Anything, mock.Anything).Return(nil) + mbm.On("NewBroadcast", mock.Anything).Return(mms) + mms.On("Send", context.Background()).Return(nil) + + err := ds.DefineTokenPool(context.Background(), pool, false) + assert.Regexp(t, "FF00140", err) +}