Migration messages approach

[hashedone]

Some thoughts on Migration messages in our system.

TL;DR by @ethanfrey

Some migrate messages need data, others don’t.
But if we want to enable a v1 -> v5 upgrade where the v2 and v4 steps need data, this does get confusing.
If v4-> v5 we need no data
If v2-> v5 we need one data
If v1 -> v5 we need 2 data.

Whole story:

We basically make it very similar to the execution message. But I think it is not exactly how it should be done. Basically every single thing there should be optional. Now here is my reasoning:
Basically we have our first version of imaginary contract tgrade-ac (stands for tgrade awesome contract), for simplicity we assume it is in version 1.0.0, it works all on main net.

Now we create update with some changes, we version it as 1.1.0, and the migration message is like:

struct MigrateMsg {
  metadata_for_1_1_0: u64,
}

Now some time have passed, and we created 1.2.0 version, and to migrate from 1.1.0 to 1.2.0 we need additional superfield (a string). So now migration message is...:

struct MigrateMsg {
  superfield: String,
}

But the message doesn't allow to upgrade 1.0.0 -> 1.2.0 . To do such migration we need to do two-step migration which is probably a gas waste. Better is to have message taking everything needed to do batch upgrade:

struct MigrateMsg {
  metadata_for_1_1_0: u64,
  superfield: String,
}

But it is problematic if one want to update contract from 1.1.0 (he need to pass some fake metadata field just to make msg parsing). Also if one want to update 1.0.0 to 1.1.0 the same problem is about the superfield.

My point is - the messages like this seems to be not very much SOLID (in particular: Open-closed principle seems to be kind of broken).

It is not this much a problem with core contracts, as they are basically always on the newest version, so we don't need any migrations besides previous to last version, but it is probably not true in general. Especially when ppl would write own contracts, and ppl would reuse others contracts, updates would not be this strict. I think we should already keep this thing in mind to create good examples and processes.

Just random thing to share about our methodology, ready to talk about this.

Possible solution idea by @ethanfrey:

Maybe we can embed them?

MigrateMsgV5 {
  previous: Option<MigrateMsgv4>
}

MigrateMsgV4 {
  // data for this step
  pub superfield: String,
  // last message with data field
  pub previous: Option<MigrateMsgv2>
}

MigrateMsgV3 {
  // no custom data (if v2->v3)

  // last message with data field
  pub previous: Option<MigrateMsgv2>
}

MigrateMsgV2 {
  pub metadata_for_1_1_0: u64,
  // no previous one
}

Or maybe use v4 / v2 instead of “previous”?

[hashedone]

My comment on Ethans idea - I am not a fan of v1/v2, because they are not directly relating to version migrating to/from. I think idea is decent, but I would just name it like: MigrateMsgV1_1_0, MigrateMsgV2_0_0 with data relevant for current migration. Now I would have migrate message like this:

struct MigrateMsg {
  V1_1_0: Option<MigrateMsgV1_1_0>,
  #[serde(transparent)]
  V2_0_0: MigrateMsgV2_0_0,
}

This would make most common message (updating from previous version) very direct, and all previous data would be embedded in its own block. It would also allow to quickly verify if all needed blocks are needed just by comparing to from/to migration versions. Seems very decent.

[ethanfrey]

Does #[serde(transparent)] work with our serde-json-wasm?

Maybe it is time to add some of these nice features

[hashedone]

@ethanfrey I don't know, but I think if not, it is nice thing to add. I think it has problem with enums, but it is because for enums it needs to try out every variant. For structs it just flattens structure so it should work. Any way - when I would have some breathing room I can always add it.