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

Migrate away from SimpleDispatchInfo #5686

Merged
merged 8 commits into from
Apr 22, 2020
Merged

Migrate away from SimpleDispatchInfo #5686

merged 8 commits into from
Apr 22, 2020

Conversation

kianenigma
Copy link
Contributor

@kianenigma kianenigma commented Apr 17, 2020

  • Deprecates SimpleDispatchInfo, replaced with impls for Weight, (Weight, Class) and (Weight, Class, PaysFee)
  • Revamps all the docs accordingly. Since the new weight system is being prepared and will be presented in Sub0, this is quite important, the old docs were very much out of date.

I brought this up (intention to deprecate SimpleDispatchInfo) and there were no objections, so I assume it is okay. SimpleDispatchInfo was verbose, not flexible (i.e. for each permutation of (weight, class, fee) you need a new enum variant) and does not have any added value. I always saw it as hasty mistake which was added in the first version of weights.

So all in all, If this is going to happen, better for it to happen now before 2.0.

This PR should change no logic except for in weight.rs. substitution regex:

// FixedNormal pays fee, default class is Normal, only replace weight
SimpleDispatchInfo::FixedNormal\((.*)\)
$1

// FixedOperational pays fee, no need to set third value
SimpleDispatchInfo::FixedOperational\((.*)\)
\($1, frame_support::weights::DispatchClass::Operational\)

// FixedMandatory pays fee, no need to set third value
SimpleDispatchInfo::FixedMandatory\((.*)\)
\($1, frame_support::weights::DispatchClass::Mandatory\)

fn some_root_operation(origin) {
let _ = frame_system::ensure_root(origin);
}
#[weight = SimpleDispatchInfo::InsecureFreeNormal]
#[weight = 0]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

interesting case not automatically migrated.

fn proxy_remove_vote(origin, index: ReferendumIndex) -> DispatchResult {
let who = ensure_signed(origin)?;
let target = Self::proxy(who).and_then(|a| a.as_active()).ok_or(Error::<T>::NotProxy)?;
Self::try_remove_vote(&target, index, UnvoteScope::Any)
}

/// Enact a proposal from a referendum. For now we just make the weight be the maximum.
#[weight = SimpleDispatchInfo::MaxNormal]
#[weight = Weight::max_value()]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Interesting case not handled by the regex.

@kianenigma kianenigma requested review from shawntabrizi and removed request for andresilva, pepyakin, sorpaas and Demi-Marie April 17, 2020 15:57
Copy link
Member

@shawntabrizi shawntabrizi left a comment

Choose a reason for hiding this comment

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

LGTM when you create polkadot pr

@kianenigma kianenigma added A8-mergeoncegreen and removed A0-please_review Pull request needs code review. labels Apr 20, 2020
@gnunicorn
Copy link
Contributor

tests failing.

@kianenigma
Copy link
Contributor Author

should be good now. @gnunicorn

@kianenigma
Copy link
Contributor Author

Updated the polkadot PR as well and both should be mergeable now.

@bkchr bkchr merged commit 3a36e51 into master Apr 22, 2020
@bkchr bkchr deleted the kiz-revamp-weight-infra branch April 22, 2020 07:20
sorpaas pushed a commit that referenced this pull request Nov 20, 2020
* Migrate away from SimpleDispatchInfo

* Fix imports

* Better doc

* Update lib.rs

Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants