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

Modifications to make work with gossamer node #1

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

edwardmack
Copy link
Owner

This contains changes that enable zombienet to work with gossamer nodes, note these changes break zombienet so that it will not work with polkadot nodes.

Zombienet uses CLI flags to start the target node, since gossamer has different CLI arguments that polkadot, it is necessary to change these to make it work with gossamer. (Note, this is just for research, there are many different ways that we can make these work together, this is done to determine what the differences are.)

  • Changes to cmdGenerater.ts are where the zombienet code has been changed to work with gossamer nodes.
  • The chain folder was added for testing, this contains chain spec files for testing with gossamer.
  • Added example spawn file examples/0001-westend-local-network.toml to spawn a local network using the westend chain spec.
    • Note, the default_command parameter points to a local instance of gossamer, this would normally be just gossamer assuming gossamer is in your path, but I've pointed to my development path for gossamer since I've been updating gossamer as I've been testing.
    • Note: the --config flag in args attribute should not be necessary since zombienet normally starts nodes using CLI arguments and not a config file, but there seems to be a bug in gossamer where some config attributes from config file are necessary to start a node in role 4 (grandpa authority).
    • Note: zombienet expects the node to accept CLI argument --node-key to define the libp2p key used to setup networking, the current gossamer node does not have this flag, using gossamer node from branch ed/sombienet_testing has this CLI argument. https://github.com/ChainSafe/gossamer/tree/ed/zombienet_testing

You can start the westend local test network with 3 nodes using the following command: node javascript/packages/cli/dist/cli.js spawn --provider native examples/0001-westend-local-network.toml
To start with debugging logs use: DEBUG=zombie* node javascript/packages/cli/dist/cli.js spawn --provider native examples/0001-westend-local-network.toml
To start testing nodes using test file example/0001-westend-local-network.zndsl (Note most if these tests fail, but the testing framework is working), DEBUG=zombie* node javascript/packages/cli/dist/cli.js test --provider native examples/0001-westend-local-network.zndsl

Copy link

@timwu20 timwu20 left a comment

Choose a reason for hiding this comment

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

I don't think the added files in chain and examples should be included in this PR.

Comment on lines +256 to +257

// args.push("--no-mdns");
Copy link

Choose a reason for hiding this comment

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

why is this commented out?

Comment on lines -263 to -265
if (prometheus && !args.includes("--prometheus-external"))
args.push("--prometheus-external");

Copy link

Choose a reason for hiding this comment

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

is this because gossamer doesn't support this?

];
let portFlags
let finalArgs: string[]
if (command.includes("gossamer")) {
Copy link

Choose a reason for hiding this comment

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

I feel like this is all unnecessary if we just support the substrate flags. The command can be modified to run our binary easily.

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