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(chain-spec): adjust hrmp channels json structure #244

Merged
merged 5 commits into from
Aug 13, 2024

Conversation

evilrobot-01
Copy link
Contributor

When testing zombienet-sdk with cross-chain calls with Asset Hub, we found that the hrmp channels passed to the chainspec caused the following issue:

Error: "Invalid JSON blob: invalid type: map, expected a tuple of size 4 at line 1 column 2794"

This is because the values of HrmpChannelConfig serialise to a map instead of a tuple, as expected by https://github.com/paritytech/polkadot-sdk/blob/fc906d5d0fb7796ef54ba670101cf37b0aad6794/polkadot/runtime/parachains/src/hrmp.rs#L492. Tests were updated accordingly to facilitate this fix.

Additional commits were also added to enable some of Asset Hub on Polkadot functionality observed in the classic Zombienet repo, so that we are able to launch a local Polkadot network with Asset Hub should anyone require. Example configuration files can be found at the bottom of https://github.com/r0gue-io/pop-cli/pull/278/files for anyone interested, although that PR is still a work in progress and dependent on this one being accepted.

PS - I added tests where I could, but additional tests might require some minor refactoring to make it easier to test only the logic that is changed by this PR, which I am not sure what the appetite is? An example would be refactoring the file name generation in keystore.rs into its own function to more easily test the outputs without the scoped_fs.

@evilrobot-01 evilrobot-01 mentioned this pull request Aug 12, 2024
1 task
@pepoviola pepoviola self-requested a review August 12, 2024 21:55
Copy link
Collaborator

@pepoviola pepoviola left a comment

Choose a reason for hiding this comment

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

Looks good! Thanks for fixing this. And also, yes. We need to make some refactors to make the code more modular and easy to test.
Thx!!

@pepoviola
Copy link
Collaborator

Hi @evilrobot-01, can you format the code with

cargo +nightly fmt  --all

Thx!

@evilrobot-01
Copy link
Contributor Author

Yes, will do.

Apologies, I didn't want to mess with the formatting at the time and forgot to come back to it!

@pepoviola
Copy link
Collaborator

Yes, will do.

Apologies, I didn't want to mess with the formatting at the time and forgot to come back to it!

No problem 👍, thanks for your contribution and help :)

@pepoviola pepoviola merged commit b1aac0e into paritytech:main Aug 13, 2024
6 of 7 checks passed
@pepoviola
Copy link
Collaborator

Merged, thanks @evilrobot-01 🙌

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