-
Notifications
You must be signed in to change notification settings - Fork 283
Make generic #4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make generic #4
Conversation
| .expect("account nonce is a map") | ||
| .key(&account_id); | ||
| let client = (*self).clone(); | ||
| self.fetch_or_default(account_nonce_key).map(|nonce| { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could also allow user supplied nonce to allow client side incrementing, as you originally suggested.
| let mut rt = tokio::runtime::Runtime::new().unwrap(); | ||
| rt.block_on(f) | ||
| #[derive(Clone, PartialEq, Eq)] | ||
| struct Runtime; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the reason for defining this test runtime? Types look mostly the same except Balances::OnFreeBalanceZero
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to make sure that the only thing required for it to work is implementing srml_system::Trait and srml_balances::Trait. If we just use the Runtime, we might miss some other trait bounds.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's actually only srml_system::Trait which is required.
| srml_system::CheckEra<Runtime>, | ||
| srml_system::CheckNonce<Runtime>, | ||
| srml_system::CheckWeight<Runtime>, | ||
| srml_balances::TakeFees<Runtime>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we just used node_runtime::Runtime here we could at least not require the local srml_balances::Trait impl. Though I think I am missing why this is needed
| } | ||
|
|
||
| impl StorageMetadata { | ||
| pub fn map(&self) -> Option<StorageMap> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe rename this storage_map or something. map says to me Functor
ascjones
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But those are only minor things, overall LGTM
Depends on paritytech/substrate#3329