Relocate weight to paint + decouple from extensions #4124
Conversation
paint/elections-phragmen/Cargo.toml
Outdated
| @@ -8,7 +8,7 @@ edition = "2018" | |||
| codec = { package = "parity-scale-codec", version = "1.0.0", default-features = false, features = ["derive"] } | |||
| sr-primitives = { path = "../../primitives/sr-primitives", default-features = false } | |||
| phragmen = { package = "substrate-phragmen", path = "../../primitives/phragmen", default-features = false } | |||
| paint-support = { path = "../support", default-features = false } | |||
| support = { package = "paint-support", path = "../support", default-features = false } | |||
There was a problem hiding this comment.
as soon as names are settled, one should make a PR and unify all of such imports.
There was a problem hiding this comment.
I thought we no longer want to do renames?
There was a problem hiding this comment.
indeed, but see my note:
as soon as names are settled, one should make a PR and unify all of such imports.
at least now that almost everywhere we use paint-support as support, having consistency is better IMO + makes sed-ing much easier. As mentioned, I'm more than happy to kill all of these as soon as we are done with paint -> palette.
paint/elections-phragmen/Cargo.toml
Outdated
| @@ -8,7 +8,7 @@ edition = "2018" | |||
| codec = { package = "parity-scale-codec", version = "1.0.0", default-features = false, features = ["derive"] } | |||
| sr-primitives = { path = "../../primitives/sr-primitives", default-features = false } | |||
| phragmen = { package = "substrate-phragmen", path = "../../primitives/phragmen", default-features = false } | |||
| paint-support = { path = "../support", default-features = false } | |||
| support = { package = "paint-support", path = "../support", default-features = false } | |||
There was a problem hiding this comment.
I thought we no longer want to do renames?
paint/support/src/dispatch.rs
Outdated
| @@ -23,11 +23,11 @@ pub use paint_metadata::{ | |||
| FunctionMetadata, DecodeDifferent, DecodeDifferentArray, FunctionArgumentMetadata, | |||
| ModuleConstantMetadata, DefaultByte, DefaultByteGetter, ModuleErrorMetadata, ErrorMetadata | |||
| }; | |||
| pub use crate::weights::{ | |||
There was a problem hiding this comment.
Is it really worth to re-export? This stuff is in the same crate, I don't see a point - it will just introduce ambiguity.
There was a problem hiding this comment.
AFAIK: This stuff, similar to other re-exports of the module are needed to be accessed from within the macro. I learned this by example. @bkchr can probably either explain more or correct me.
paint/transaction-payment/src/lib.rs
Outdated
| @@ -56,7 +56,7 @@ type NegativeImbalanceOf<T> = | |||
|
|
|||
| pub trait Trait: system::Trait { | |||
| /// The currency type in which fees will be paid. | |||
| type Currency: Currency<Self::AccountId>; | |||
| type Currency: Currency<Self::AccountId> + Send + Sync; | |||
There was a problem hiding this comment.
Can you elaborate why is Send + Sync required? I wouldn't expect to see that in any part of runtime.
There was a problem hiding this comment.
Good point, I didn't really care much since I have seen these two in all SignedExtension type. For instance:
substrate/paint/system/src/lib.rs
Line 853 in 0f87c92
So I assumed it is okay. But I will look it up.
although, we use Send + Sync in Member which is used around the runtime, so just by observation, I would not assume that we ban them in runtime. Although it does makes sense that a single threaded runtime should not really need them.
There was a problem hiding this comment.
Needed due to AccountId being Member.
| @@ -713,6 +712,9 @@ pub trait SignedExtension: Codec + Debug + Sync + Send + Clone + Eq + PartialEq | |||
| /// The type that encodes information that can be passed from pre_dispatch to post-dispatch. | |||
| type Pre: Default; | |||
|
|
|||
| /// An opaque set of information attached to the transaction. | |||
| type Info: Copy; | |||
There was a problem hiding this comment.
Copy is a pretty strict, why not just Clone?
There was a problem hiding this comment.
I did face the dilemma and at the time the rational was: while we are making Type Info be generic but we currently know that we'll use it with a type that has Copy and this makes the syntax much much easier for now. If we see Copy as merely a marker that clone is cheap, I think it is fair to say that this bundled information will always be cheap to copy.
There was a problem hiding this comment.
naah made it clone, was easier than what I thought to refactor.
…ocate-weight-stuff
…ocate-weight-stuff
|
will need some minor refactoring in polkadot |
Co-Authored-By: Bastian Köcher <bkchr@users.noreply.github.com>
Co-Authored-By: Bastian Köcher <bkchr@users.noreply.github.com>
Makes
SignedExtensionindependent of the type of the weight data, allowing a potential user to actually not have any sort of weight or class by settingtype Info = ().Moves the weight related stuff to support, since they are part of our opinionated runtime.
Looks like we need a follow up PR to move all the extrinsic stuff to paint, but I'll do that in a follow up, since this has a bit of logic change in signed extension.