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

POW Registration allowance/disallowance #189

Merged
merged 4 commits into from
Nov 15, 2023
Merged

Conversation

Rubberbandits
Copy link
Contributor

This PR adds a sudo call which separately manages whether or not POW registration is enabled for a subnet.

Self::if_subnet_allows_registration(netuid),
Self::get_network_pow_registration_allowed(netuid),
Copy link
Contributor

Choose a reason for hiding this comment

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

is there some idiomatic reason for this name change?
I thjink the previous one has a good naming, maybe something like.... Self::subnet_allows_registrations(netuid) which could fit with the context and its semantics. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if_subnet_allows_registration was a duplicate of another function which was already implemented.

Copy link
Contributor

@ifrit98 ifrit98 left a comment

Choose a reason for hiding this comment

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

LGTM! Only question is on some failing tests. Assume these panics are from flaky tests or not updated?

image

@Rubberbandits
Copy link
Contributor Author

LGTM! Only question is on some failing tests. Assume these panics are from flaky tests or not updated?

image

Yep, i do need to update these! Good catch

@Rubberbandits
Copy link
Contributor Author

Burn adjustment tests fixed

Copy link
Contributor

@ifrit98 ifrit98 left a comment

Choose a reason for hiding this comment

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

LGTM!

Only issue is the test_registration_difficulty_adjustment fails.

@Rubberbandits
Copy link
Contributor Author

LGTM!

Only issue is the test_registration_difficulty_adjustment fails.

Fixed! Thank you for catching this.

@shibshib shibshib self-requested a review November 15, 2023 18:12
@Rubberbandits Rubberbandits merged commit 6d64fd1 into main Nov 15, 2023
@welikethestock welikethestock deleted the pow/toggle-registration branch November 29, 2023 00:18
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.

4 participants