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

Remove sudo from Moonriver (and Alphanet temporarily) #650

Merged
merged 6 commits into from
Jul 30, 2021

Conversation

JoshOrndorff
Copy link
Contributor

This PR removes pallet sudo from the moonriver runtime. After this PR the only ways to dispatch with root origin will be through democracy or council.

@girazoki
Copy link
Collaborator

girazoki commented Jul 29, 2021

Remember to remove traces in the Rust integration tests. I see

assert_eq!(
		<moonriver_runtime::Sudo as StorageInfoTrait>::storage_info(),
		vec![StorageInfo {
			prefix: prefix(b"Sudo", b"Key"),
			max_values: Some(1),
			max_size: Some(20),
		}]
	);

In Moonriver integration tests

@crystalin crystalin added the A0-pleasereview Pull request needs code review. label Jul 29, 2021
@crystalin crystalin requested a review from notlesh July 29, 2021 18:03
@crystalin crystalin added the B7-runtimenoteworthy Changes should be noted in any runtime-upgrade release notes label Jul 29, 2021
@crystalin
Copy link
Collaborator

@JoshOrndorff we need to set the pallet index in moonbase I believe, can you do that ?

@crystalin crystalin added A8-mergeoncegreen Pull request is reviewed well. and removed A0-pleasereview Pull request needs code review. labels Jul 30, 2021
Copy link
Contributor

@notlesh notlesh left a comment

Choose a reason for hiding this comment

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

LGTM, however, there are a lot of changes around the it("should be able to register many accounts - batch : " test that seem unnecessary. Probably not a big deal if we simply intend to git revert this soon.

@girazoki
Copy link
Collaborator

Yeah still I know I added a sudo-dependent test yesterday. I used sudo to enact a democracy proposal. Didnt realize about this PR. I can try to change it to make it be approved by regular means, or do you want to do it @JoshOrndorff ?

@crystalin
Copy link
Collaborator

Ideally we would want to move those tests to use democracy,
But in short term, we decided to disable them until we restore Sudo on Alphanet (probably in a week)

@JoshOrndorff JoshOrndorff changed the title Remove sudo from Moonriver Remove sudo from Moonriver (and Alphanet temporarily) Jul 30, 2021
@JoshOrndorff JoshOrndorff merged commit 2386b45 into master Jul 30, 2021
@JoshOrndorff JoshOrndorff deleted the joshy-moonriver-no-sudo branch July 30, 2021 17:47
JoshOrndorff added a commit that referenced this pull request Jul 30, 2021
@JoshOrndorff
Copy link
Contributor Author

I prepared this one to restore sudo to moonbase. #652

I'll just leave it open until we're ready for it.

crystalin pushed a commit that referenced this pull request Aug 5, 2021
* Revert "Remove sudo from Moonriver (and Alphanet temporarily) (#650)"

This reverts commit 2386b45.

* remove sudo from moonriver

(cherry picked from commit 89d17d5)

* also remove it from the integration tests

(cherry picked from commit d831ff0)

* restore pallet indeces

* Revert "Skipping introduced test with sudo (#653)"

This reverts commit 4f0bcad.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A8-mergeoncegreen Pull request is reviewed well. B7-runtimenoteworthy Changes should be noted in any runtime-upgrade release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants