CosmWasm / CosmWasm/cw-storage-plus
Introducing migration helpers for `storage-plus` primitives
- Dominant language
- Rust
- Stars
- 51
- Forks
- 34
- PR merge metrics
- No merged PRs in 30d
Description
Some migration helpers would be useful. Let's say we have an `Item` in our contract, and it changes structure in the new contract version. Now, to create a migration for that, we need to recreate the old item structure just for migration purposes, then load it from old place, transform it, and store it in a new place. It may look like this:
```rust
// This is probably somewhere in `state.rs`
struct Config {
admin: Addr,
period: u64,
}
const CONFIG: Item = Item::new("config");
// And this would be in some `contract.rs`
fn migrate(deps: DepsMut, env: Env, msg: MigrationMsg) {
struct OldConfig {
admin: Addr,
};
const OLD_CONFIG: Item = Item::new("config");
let OldConfig { admin } = OLD_CONFIG.load(deps.storage)?;
let config = Config {
admin,
period: msg.period,
};
CONFIG.save(deps.storage, &config)?;
}
```
It is not very difficult, but it was just an item. Now when it comes to `Map`, iteration has to be involved, for snapshot-related utilities it is basically impossible to migrate some changes without an understanding of the underlying structures of the helper.
My proposal is to add to all the utilities the `migrate` function with signatures like:
```rust
fn migrate(&self, storage: store: &mut dyn Storage, mut transform: FnMut(OldValue) -> T) -> StdResult<()>
where
OldValue: DeserializeOwned,
;
```
Also, we would need some possibility to have failable migrations:
```rust
fn try_migrate(&self, storage: store: &mut dyn Storage, mut transform: FnMut(OldValue) -> Result) -> Result<(), Error>
where
OldValue: DeserializeOwned,
Error: StdError,
;
```
`T` here is obviously the type stored in the container (for `Map` it would be the value type). The function would make sure to properly update all stored data from the old version to the new one. The `transform` is a `FnMut` to allow some tricks with usage of transformed values for some more complex migrations. The function should never require turbofish syntax, as the type should be elideable from the passed function signature (for closure you always can hint at the argument type).
Usage:
```rust
// This is probably somewhere in `state.rs`
struct Config {
admin: Addr,
period: u64,
}
const CONFIG: Item = Item::new("config");
// And this would be in some `contract.rs`
fn migrate(deps: DepsMut, env: Env, msg: MigrationMsg) {
struct OldConfig {
admin: Addr,
};
OLD_CONFIG.migrate(deps.storage, |config| Config {
admin: config.admin,
period: msg.period,
})?;
}
```
We still need to recreate the old config, but updating the value is easier - very similar to calling `update`. The reason why I propose to split this to `migrate` and `try_migrate` is to help type elision - it is often the case that conversion cannot fail, and we never return an error. In such cases, we have to provide the error either by turbofish syntax, or explicit return type for closure - both are unnecessary noise (btw - splitting `update` to `update` and `try_update` may be also smart, but not sure if it is worth as it is breaking change).
The additional question here is if we want (and if it is easily possible) to migrate the map keys this way or limit this functionality to values. Keys are obviously less probable to change, but technically there may be some cases for updating them. Not sure if updating the key is more complicated or anything (it should not be for map, but `IndexMap` kind of scares me).
Additionally, we can have additional functions like:
```rust
fn migrate_from(&self, storage: store: &mut dyn Storage) -> T) -> StdResult<()>
where
OldValue: Deserialize,
T: From,
;
```
Which would just use `Into` trait to perform the conversion (instead of providing transformation). However, this would probably require turobofish to call. However honestly - personally I would prefer calling `MY_MAP.migrate(deps.storage, OldConfig::into)?;` and type elision would do the job.
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.