Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Deprecates the chill_other extrinsic in staking #14187

Closed
wants to merge 1 commit into from

Conversation

gpestana
Copy link
Contributor

@gpestana gpestana commented May 22, 2023

This PR refactors the chill and chill_other extrinsics in staking. The current logic of the chill extrinsic can be deprecated in lieu of chill_other, since the logic of chill is a subset of chill_other. With these changes, all the chilling can be performed by calling the chill extrinsic.

Changes:

  1. Removes chill_other extrinsic
  2. Refactors chill so that callers can chill own or third-party controller. The caller can pass a optional controller account ID, which has the same logic as the current fn chill_other
pub fn chill(
  origin: OriginFor<T>,
  controller: Option<T::AccountId>,
) -> DispatchResultWithPostInfo

To finish

  • Polkadot companion
  • Cumulus companion

Closes paritytech/polkadot-sdk#314

@gpestana gpestana added A0-please_review Pull request needs code review. C1-low PR touches the given topic and has a low impact on builders. D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed. B1-note_worthy Changes should be noted in the release notes T1-runtime This PR/Issue is related to the topic “runtime”. labels May 22, 2023
@gpestana gpestana requested review from kianenigma, Ank4n, rossbulat and a team May 22, 2023 11:06
@gpestana gpestana self-assigned this May 22, 2023
@gpestana gpestana changed the title Deprecates the chill extrinsic in staking Deprecates the chill_other extrinsic in staking May 22, 2023
let ledger = Self::ledger(&controller).ok_or(Error::<T>::NotController)?;
let stash = ledger.stash;

// caller controlls the controller.
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems comment is not correct. I believe this condition is to check if Nominator Preference is undecodeable?

Copy link
Contributor

@juangirini juangirini left a comment

Choose a reason for hiding this comment

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

These are breaking changes for anything using the old chill or chill_other. I suggest to follow the deprecation process proposed here.
I have updated the issue description to track this deprecation process.
In this context we need to keep both extrinsics, but replace any reference to chill in favour of chill_other.
In another PR, once we have confirmed that any examples or tutorials (if any) have been updated, we should add a deprecation warning to chill saying that chill_other should be used instead. This deprecation will be noted in the release log.
Only afterwards we should be good to delete chill, and eventually rename chill_other to chill.

@gpestana
Copy link
Contributor Author

I'm closing this PR for now as after a few discussions, we think that it's better to avoid having another breaking change in the API at this point. Since this deprecation is mostly cosmetic, let's pause it for now and wait until we have a more stable deprecation process in place.

@Polkadot-Forum
Copy link

This pull request has been mentioned on Polkadot Forum. There might be relevant details there:

https://forum.polkadot.network/t/stablising-v15-metadata/2819/9

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B1-note_worthy Changes should be noted in the release notes C1-low PR touches the given topic and has a low impact on builders. D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed. T1-runtime This PR/Issue is related to the topic “runtime”.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[Deprecation] Explore deprecating the staking chill extrinsic
4 participants