Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Solve issue #331 by adding FirstEvent support in FireFly config #332

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

MKVEERENDRA
Copy link

This PR addresses issue #331, which was about adding support for the FirstEvent parameter in the FireFly configuration files.

Changes Made:
Added a new FirstEvent field support in the FireFly configuration (firefly_core_*.yml) files to allow specifying the starting event for blockchain-based processes.
Integrated this new parameter into the FireFly configuration logic during the stack initialization process.
Updated the stack setup and blockchain deployment flow to support FirstEvent, ensuring it can be configured when setting up FireFly for the first time.
The FirstEvent field is now configurable in the FireFlyConfig and is passed to the blockchain and smart contract initialization process.
Context and Fix:
The addition of the FirstEvent parameter gives users flexibility in defining which blockchain event the FireFly stack should use when starting up. This was missing in the previous configuration, causing issues for users who needed specific event handling.
This update ensures that FirstEvent is properly handled, stored in the configuration, and used during smart contract and blockchain node setup.
How to Test:
Start the FireFly stack with the new configuration changes.
Verify that the FirstEvent field is correctly passed to the blockchain during FireFly stack initialization.
Ensure that the expected event triggers the FireFly workflow appropriately, depending on the FirstEvent configuration value.
Issue Comment (on issue #331):
I have solved issue #331 by adding support for the FirstEvent parameter in the FireFly configuration. This allows users to specify which event should be used during FireFly stack initialization, enabling more flexible event handling for blockchain-based processes.

The fix is now available in the pull request https://github.com/MKVEERENDRA/firefly-cli1, and it has been tested to ensure the correct configuration of the FirstEvent value during the FireFly setup.

@MKVEERENDRA
Copy link
Author

I Fixed it

@@ -104,5 +104,6 @@ This command will start a stack and run it in the background.

func init() {
startCmd.Flags().BoolVarP(&startOptions.NoRollback, "no-rollback", "b", false, "Do not automatically rollback changes if first time setup fails")
startCmd.Flags().StringVarP(&startOptions.FirstEvent, "first-event", "f", "0", "Specify the starting block for event processing (default: 0)")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few things:

  • Move this to the init command
  • Would suggest renaming to --from-block and I wouldn't put a shortcut for this one of -f
  • I would make this super obvious that is only applies to multiparty as a contract listener gets created for that contract... It might be better as well to not create the contract listener at all
Suggested change
startCmd.Flags().StringVarP(&startOptions.FirstEvent, "first-event", "f", "0", "Specify the starting block for event processing (default: 0)")
startCmd.Flags().StringVarP(&startOptions.FirstEvent, "--from-block", "f", "0", "Specify the starting block for event processing (default: 0)")

if err := s.patchFireFlyCoreConfigs(configDir, member, newConfig); err != nil {
return messages, err
}
if s.Stack.MultipartyEnabled {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indentation looks odd

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and not sure why code was removed to write to the state file her

for _, member := range s.Stack.Members {
orgConfig := s.blockchainProvider.GetOrgConfig(s.Stack, member)
newConfig.Namespaces.Predefined[0].DefaultKey = orgConfig.Key
if s.Stack.MultipartyEnabled {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we still need this MultipartyEnabled wrapping this section


for _, member := range s.Stack.Members {
orgConfig := s.blockchainProvider.GetOrgConfig(s.Stack, member)
newConfig.Namespaces.Predefined[0].DefaultKey = orgConfig.Key
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need this key as well

Comment on lines +987 to +1012
newConfig := &types.FireflyConfig{
Namespaces: &types.NamespacesConfig{
Default: "default",
Predefined: []*types.Namespace{
{
Name: "default",
Description: "Default predefined namespace",
Plugins: []string{"database0", "blockchain0", "dataexchange0", "sharedstorage0"},
Multiparty: &types.MultipartyConfig{
Enabled: true,
Org: orgConfig,
Node: &types.NodeConfig{
Name: member.NodeName,
},
Contract: []*types.ContractConfig{
{
Location: contractLocation,
FirstEvent: firstEvent, // Now configurable
Options: optionsMap,
},
},
},
},
},
},
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do not want to replace the whole config - only the contract multiparty config section

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants