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

feat!(runtime): removal of sudo #774

Merged
merged 6 commits into from
Jul 10, 2023
Merged

Conversation

DylanVerstraete
Copy link
Contributor

@DylanVerstraete DylanVerstraete commented Jul 5, 2023

Also order and number runtime pallets for future compatibility

  • remove storage with migration
  • fix ci
  • fix clients

@DylanVerstraete DylanVerstraete marked this pull request as ready for review July 5, 2023 15:05
Copy link
Contributor

@brandonpille brandonpille left a comment

Choose a reason for hiding this comment

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

Thanks for the changes! :)

Dao: pallet_dao::{Pallet, Call, Storage, Event<T>},
Utility: pallet_utility::{Pallet, Call, Event},
Historical: pallet_session::historical::{Pallet},
// System support
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you also fix the go client? It has a list of modules errors (in utils.go) and we map from id to error there. Will need to remove the id of Sudo pallet.

Copy link
Contributor Author

@DylanVerstraete DylanVerstraete Jul 10, 2023

Choose a reason for hiding this comment

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

Sure, but honestly having a staticly typed moduleErrors list is quite a hastle to maintain. This should actually come from the metadata.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree it's a hastle. Does the metadata also show the error information?

Copy link
Collaborator

@renauter renauter 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!
Meanwhile I see some remaining references to sudo in documentation in docs folder
And also in some chainSpec.json files in substrate-node/chainSpecs/...
Is it still valid?

@DylanVerstraete
Copy link
Contributor Author

DylanVerstraete commented Jul 10, 2023

@renauter references in the chainspecs need to stay since the chainspecs are genesis configurations and are static forever. I'll remove any references in the documentation.

@DylanVerstraete DylanVerstraete merged commit 215a12a into development Jul 10, 2023
1 check passed
@DylanVerstraete DylanVerstraete deleted the feat_remove_sudo branch July 10, 2023 12:13
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.

3 participants