This repository was archived by the owner on Nov 15, 2023. It is now read-only.
Conversation
gui1117
added
A0-please_review
Contributor
Author
|
for the sake of clarity, now modules like treasury bounds its associated type like
|
gavofyork
reviewed
Feb 19, 2019
| spec_name: create_runtime_str!("node"), | ||
| impl_name: create_runtime_str!("substrate-node"), | ||
| authoring_version: 10, | ||
| spec_version: 29, |
Member
There was a problem hiding this comment.
no need to bump spec version - there are no logical changes.
gavofyork
approved these changes
Feb 19, 2019
Member
|
Minor change but otherwise good to go. |
gavofyork
added
A6-mustntgrumble
and removed
A0-please_review
Member
Sounds like a bug in our parity codec derive logic. Would be nice if you could create some reproducible example and open an issue in parity-codec. |
Contributor
Author
|
how did you get this error ? (EDIT: yes ok it is in first post: 🤦) I got a Sized issue when doing that. If I use Currency as the main trait bound and other I assign there inner type to the currency one then it fails to compiles. But if use ArithmeticType as the main trait bound and other I assigne to ArithmeticType::Type then it works. type Of<T> = <<T as F>::Type as Cur>::Type;
pub trait Arith {
type Type: parity_codec::HasCompact;
}
pub trait Cur {
type Type;
}
pub trait F {
type Type: Cur + Arith<Type = Of<Self>>;
}error is: |
Contributor
Author
pub trait F {
type T: parity_codec::HasCompact;
}
impl F for () {
type T = ();
}should we simply implement Compact<()> ? |
Contributor
Author
|
OK so diff --git a/src/codec.rs b/src/codec.rs
index 730640d..a44d61a 100644
--- a/src/codec.rs
+++ b/src/codec.rs
@@ -213,7 +213,7 @@ macro_rules! impl_from_compact {
}
}
-impl_from_compact! { u8, u16, u32, u64, u128 }
+impl_from_compact! { (), u8, u16, u32, u64, u128 }
/// Compact-encoded variant of &'a T. This is more space-efficient but less compute-efficient.
#[derive(Eq, PartialEq, Clone, Copy)]
@@ -277,6 +277,18 @@ impl<T: 'static> HasCompact for T where
type Type = Compact<T>;
}
+impl<'a> Encode for CompactRef<'a, ()> {
+ fn using_encoded<R, F: FnOnce(&[u8]) -> R>(&self, f: F) -> R {
+ f(&[])
+ }
+}
+
+impl Decode for Compact<()> {
+ fn decode<I: Input>(_input: &mut I) -> Option<Self> {
+ Some(Compact(()))
+ }
+}
+
// compact encoding:
// 0b00 00 00 00 / 00 00 00 00 / 00 00 00 00 / 00 00 00 00
// xx xx xx 00 (0 ... 2**6 - 1) (u8)solve this recursive error. TransferAsset can't bound SimpleArithmetic then because () doesn't implements Mul Add and things but at least error is understandable. and it can bound HasCompact and it compiles. Also if we do that we should also implement Encode for |
Contributor
Author
|
seems fine to me @bkchr |
Member
|
Okay :) Thanks for looking into the error again! |
MTDK1
pushed a commit
to bdevux/substrate
that referenced
this pull request
Apr 12, 2019
* remove amount associated * make a new trait to bound some arithmetics to balances or assets: It also remove arithmetic bounds of srml-support::traits::Currency. To update your code then use srml_support::traits::ArithmeticType like: `type Currency: ArithmeticType + Currency<Self::AccountId, Balance=BalanceOf<Self>>; ` with `type BalanceOf<T> = <<T as Trait>::Currency as ArithmeticType>::Type; ` * improve decl_storage when it explicit serde bound: basically don't try to be smarter than rust and just use where clause.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
related to #1819
Note: for some reasons we can't add SimpleArithmetic bound to TransferAsset::Amount:
result in
So and also I think it is better, I went for the second option:
I made a trait ArithmeticType for bound its associated type to some arithmetic stuff and else, and make balances::Module implementing it.
Also while doing so I run into an error of the bound of serde by
get_non_bound_serde_derive_typesbasically types like AssetOf bound T to serialize explicitly in our implementation and also<<T as Trait>::TransferAsset as ArithmeticType::Typedoesn't works.For this part I wonder why we try to be clever. Rust handle where clause very well already binding all types seems fine. AFAIK it is what we have done for parity-codec-derive also.
Last thing. I bound some arithmetic stuff to the associated type Balance of Currency in the past. It seems we should use this ArithmeticType instead so I replaced in modules.