Add translate function for storage prefixed map. #4555
Conversation
gui1117
added
the
A3-in_progress
frame/support/src/storage/mod.rs
Outdated
| .filter(|n| n.starts_with(&prefix[..])) | ||
| { | ||
| let value: OldValue = unhashed::get(&next_key) | ||
| // We failed to read the value. Stop the translation and return an error. |
There was a problem hiding this comment.
in case the value can't be decoded then we return an error, but what should we do then.
In the case of linkedmap if an element fails to decode during the translation then we end the linkedmap at the last correctly decoded element and we return an error.
What about here should we remove this value and continue to decode following values or should we stop at the first failure ?
There was a problem hiding this comment.
What about here should we remove this value and continue to decode following values
This makes sense. Stopping at the first failure means that half of the storage will be one type and the other half another type. This would also prevent us from ever being able to call translate_value again. Removing the value seems like the best thing we can do without breaking the module using this storage.
gui1117
added
A0-please_review
|
related comment in regard to which implementation in case of error: #4052 (comment) |
gui1117
added
A3-in_progress
|
now key that couldn't be decoded are removed and an error is returned with the number of key that failed to decode. EDIT: actually maybe better not to remove them and just let them untranslated in the storage, anyway this will only happen if the storage is corrupted or the translate function expect wrong type. in both case I don't think we can solve anything from our side. |
gui1117
added
A0-please_review
frame/support/src/storage/mod.rs
Outdated
| assert_eq!(MyStorage::iter().collect::<Vec<_>>(), vec![]); | ||
|
|
||
| // test migration | ||
| unhashed::put(&[&k[..], &vec![1][..]].concat(), &1u128); |
There was a problem hiding this comment.
You inserting a u128 and use u32 in the translate function. You should actually store a u32.
And then call iter() to make sure that an empty vec is returned.
Than you should to the translation.
There was a problem hiding this comment.
oh yes I updated tests
If you leave them untranslated, runtime code could really get messed up. Better to remove data then to have bad data where the runtime assumes it is good. imo |
gavofyork
added
A8-looksgood
and removed
A0-please_review
Add a function to migrate values from one type to one another.
In case we fail to migrate one element we return an error with the key that has failed to decoded.
We could also remove values that fail to decode and finish the migration and at the end returning an error saying there is some value that has been removed.