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

Filtering the --alice flag is too aggressive #1448

Closed
JoshOrndorff opened this issue Oct 24, 2023 · 7 comments · Fixed by #1455
Closed

Filtering the --alice flag is too aggressive #1448

JoshOrndorff opened this issue Oct 24, 2023 · 7 comments · Fixed by #1455

Comments

@JoshOrndorff
Copy link

Issue Description

I'm building a parachain with the Tuxedo runtime framework instead of frame. My work is in this pr: Off-Narrative-Labs/Tuxedo#130.

Unlike, FRAME, the Tuxedo runtime does not store the Aura keys in the chain spec, and therefore, zombienet cannot fetch them from the spec. It even prints a helpful warning about this ⚠ aura keys not found in runtimeConfig.

This is because for now we just hard code the keys in the runtime. And we rely on the --alice flag to insert the keys into the keystore. We have a session key management system in mind to build eventually, but it still will not match the same format as FRAME.

I'm not requesting zombienet to add full support for the exotic stuff I'm trying, but I do request that there is some way not to filter the --alice flag.

For example, maybe when you detect that the aura keys are not in the spec (there is already a warning for that, remember) then you don't filter the keystore related flags. Or maybe you add an option to zombienet like --dont-filter-cli-flags-i-know-what-im-doing.

In the meantime, I was able to hack around the problem by adding another cli flag that does the same thing but has a different name. But ideally that won't be required long term.

Steps to reproduce the issue

Steps to reproduce the issue

  1. Checkout the Tuxedo repository and build the collator
  2. Build the polkadot validator
  3. zombienet -p native spawn tuxedo/zombienet.toml

Describe the results you received

The --alice flag is removed from my args even when I explicitly include it.

Describe the results you expected

The flags I put in the config will be used (otherwise I wouldn't have put them there).

Zombienet version

1.3.71

Provider

Native

Provider version

## For binaries
polkadot v1.2.0
tuxedo parachain-template-node [eed24566b540b753f58d4c93e7dfe7bacdc22cdc](https://github.com/Off-Narrative-Labs/Tuxedo/pull/130/commits/eed24566b540b753f58d4c93e7dfe7bacdc22cdc)

Upstream Latest Release

Yes

Additional environment details

No response

Additional information

The Tuxedo parachain work is supported by a w3f grant. As was Tuxedo itself by a previous grant

Screenshots

No response

@pepoviola
Copy link
Collaborator

Hi @JoshOrndorff, nice to hear from you :) Yes, we may have a too aggressive filtering logic trying to minimize the friction and reduce the issues on parsing/generating the end cmd to execute. Using a flag like --dont-filter-cli-flags-i-know-what-im-doing and allow expert mode sounds good, but we can also check if the node name is alice and is set to be a validator before filtering --<well known name> flag. Did the last approach resolve your case?

@JoshOrndorff
Copy link
Author

Nice to see you as well!

I didn't totally understand your proposed alternative approach. Can you detail it a little bit?

In the end, I just need some reliable way to make sure that the --alice flag actually ends up in the cli parameters that get passed to the parachain binary. Anything that makes that possible is good for me.

@pepoviola
Copy link
Collaborator

Hi @JoshOrndorff, sorry about the delay. The change that I have in mind is to not filter flags like --<well know account (e.g alice, bob, etc) if the node name match with the account (and is a validator) in the network definition.

For example, if you use this network definition like this

[relaychain]
default_image = "docker.io/parity/polkadot:latest"
default_command = "/tmp/polkadot-sdk/target/release/polkadot"
default_args = [ "-lparachain=debug" ]

chain = "rococo-local"

  [[relaychain.nodes]]
  name = "alice"
  args = [ "--alice" ]

  [[relaychain.nodes]]
  name = "bob"
  validator = false
  args = [ "--bob" ]

  [[relaychain.nodes]]
  name = "david"
  args = [ "--dave" ]

We match the flag with the node name and we don't remove the flag for alice but we remove it for bob and david (since bob is set to not be a validator and david didn't match with the flag). I think this behavior will work on your use-case right?

Thx!

@JoshOrndorff
Copy link
Author

Okay, I see what you mean. This will indeed work for me and if you like this option, I'm fine with it.

@pepoviola
Copy link
Collaborator

pepoviola commented Oct 27, 2023

Hi @JoshOrndorff, the fix landed in main and I will create a new release (v1.3.74) later today.
Thx!!

@muraca
Copy link

muraca commented Nov 1, 2023

Hey @pepoviola here to confirm that everything works as expected. Thanks!

Before:
Before

After:
After

@pepoviola
Copy link
Collaborator

Thanks @muraca!!

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 a pull request may close this issue.

3 participants