Introducing migration helpers for storage-plus primitives

[hashedone]

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:

// This is probably somewhere in `state.rs`
struct Config {
  admin: Addr,
  period: u64,
}

const CONFIG: Item<Config> = 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<OldConfig> = 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:

fn migrate<OldValue>(&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:

fn try_migrate<OldValue, Error>(&self, storage: store: &mut dyn Storage, mut transform: FnMut(OldValue) -> Result<T, Error>) -> 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:

// This is probably somewhere in `state.rs`
struct Config {
  admin: Addr,
  period: u64,
}

const CONFIG: Item<Config> = 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:

fn migrate_from<OldValue>(&self, storage: store: &mut dyn Storage) -> T) -> StdResult<()>
where
  OldValue: Deserialize,
  T: From<OldValue>,
;

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.

[ethanfrey]

How is this different from #11 ?

They both are descriptions of making migration helpers. I think we should decide on what we want to do and make one PR to close them both