Use TaoCurrency for pallet balances balance type#2417
Use TaoCurrency for pallet balances balance type#2417gztensor wants to merge 8 commits intodevnet-readyfrom
Conversation
There was a problem hiding this comment.
my comments are just stylistic preferences. additionally i didn't understand why you needed to add this _u64 specifier everywhere.
other than that i think this change is safe, because there is an entrypoint for this Balance type (the alias) so if you change it, the rest is compiler-driven refactoring. the inconsistency might appear only in places outside of this type or where we had a mixed type errors (alpha/tao used interchangeably). the first thing is unlikely for the balance value, the second can have a place. so far there still persists this issue with StakeThreshold, but i didn't see anything else in your PR
gztensor
added
the
skip-cargo-audit
| impl From<i32> for $currency_type { | ||
| fn from(value: i32) -> Self { | ||
| Self(value.unsigned_abs().into()) | ||
| } | ||
| } |
There was a problem hiding this comment.
This is a lossy conversion, maybe it should use try_from?
| fn rem(self, rhs: Self) -> Self::Output { | ||
| let lhs_u64: u64 = self.into(); | ||
| let rhs_u64: u64 = rhs.into(); | ||
| (lhs_u64 % rhs_u64).into() |
| impl RemAssign for TaoCurrency { | ||
| #[allow(clippy::arithmetic_side_effects)] | ||
| fn rem_assign(&mut self, rhs: Self) { | ||
| *self = *self % rhs; |
There was a problem hiding this comment.
rhs == 0 panics, do we want to keep the behavior as close as possible to the normal one maybe?
| pub struct ExistentialDeposit; | ||
| impl frame_support::traits::Get<TaoCurrency> for ExistentialDeposit { | ||
| fn get() -> TaoCurrency { | ||
| TaoCurrency::new(500) | ||
| } | ||
| } |
There was a problem hiding this comment.
Could use parameter_types! for automatic implem
l0r1s
left a comment
l0r1s
left a comment
There was a problem hiding this comment.
Small remarks around the code that can panic.
It adds a lot of type safety and pave the way for more, great stuff!
Note for later: I was thinking about the naming of TaoCurrency and AlphaCurrency, a more appropriate name to match with Substrate primitives would be TaoBalance and AlphaBalance with the CustomBalance trait (as supertrait of Balance) because Currency as actually a full fledged system where you can make transfer, slash, burn, emit, etc...
Good point! I renamed the following: Currency -> Token |
3 participants
Description
Replace u64 with TaoCurrency for pallet balances balance and all over the subtensor code.
Type of Change
Checklist
./scripts/fix_rust.shto ensure my code is formatted and linted correctly