pba-content
pba-content copied to clipboard
[PBA2] FRAME Assignment common issues
The exam showed that students, again, despite passing the rust exam, make a lot of mistake in using Options and Results. This becomes crucial next to "no panic in Runtime" fact.
This example from shawn should be hidden in the exam:
Here is a small primer:
#[derive(Default)]
struct StorageValue {
value: Option<u32>,
}
impl StorageValue {
pub fn get(&self) -> Option<u32> {
self.value
}
pub fn set(mut self, v: Option<u32>) {
self.value = v;
}
pub fn put(mut self, v: u32) {
self.value = Some(v);
}
}
enum PalletError {
NoValueSet,
}
fn my_function() -> Result<(), PalletError> {
let MyStorage = StorageValue::default();
// Read some storage, if NONE return an error
let value = MyStorage.get().ok_or(PalletError::NoValueSet)?;
// Anti-pattern. Don't do this!
let maybe_value = MyStorage.get();
if maybe_value.is_none() {
return Err(PalletError::NoValueSet)
}
let value = maybe_value.unwrap();
// This is at least a little better, but still an anti-pattern
let value = maybe_value.expect("cannot panic because of `none` check right above. q.e.d");
// Do some logic only if a storage value exists, but don't return an error
// which would halt the whole extrinsic.
if let Some(value) = MyStorage.get() {
// do things with value
// If there is no value, then this code will never execute.
}
// This is okay... but probably means you should be using `ValueQuery` configuration.
// Can't really make a simple example here of `ValueQuery`. Check the presentations.
// So probably again an anti-pattern.
let value = MyStorage.get().unwrap_or_default();
Ok(())
}
Another example that I see really really ugly code written using try_mutate and try_mutate_exists, where it is not justified. I am worried that students:
- don't know the full list of functions and APIs of storage in mind when writing this
- think that just because
mutate_xhas a closure, it more advance.
We should make it clear that these functions were specifically useful for the days of pre-transactional-by-default.
Perhaps, we should even discourage using them unless if absolutely needed.
So we should have a whole module in the rust entrance exam covering options and results and crating test for those ( we test the tests they write somehow?)
Ideas for the Uni V2 dex that we could encourage students to explore:
- pair of assets, what do you do with balances / native currency?
- feeless?
- remove balances and only assets?
- replace native currency for fees with assets?
- liquidity pool is an asset...
- deterministic pool -> asset ID? or to assets managed by some key-less account?
- if assetID... then people could squat on IDs and block liquidity pool rewards token! Wat do? (instantiable pallets, one for user tokens, one for liquidity pool rewards... how stop userspace? call filters?)
- deterministic pool -> asset ID? or to assets managed by some key-less account?
Lots and lots of fun ways to push people to dive deeper with this project than a minimal impl.
replace native currency for fees with assets
I did this in a seminar a few years back. It was very enlightening to do so. Could be a good challenge for students.