Support migrations (changing storage after upgrading behavior)

[maraoz]

Remember that a migration could take more than one tx due to block gas limit. Think of supporting pause-the-world migrations vs lazy migrations done on the fly.

[maraoz]

Already supported by upgradeToAndCall in core repo (See: https://github.com/zeppelinos/core/blob/master/contracts/upgradeability/OwnedUpgradeabilityProxy.sol#L70). Consider adding a more standardized way of migrating after an upgrade (Maybe an Initializable or Migratable interface?)

[spalladino]

Consider upgrading a 721 token from basic to full as an example use case

[frangio]

We've ran into several different types of "migrations". The Basic to Full 721 example is actually one of the most complicated. I'll elaborate on these different types.

Generalized constructors are pieces of code that run when a contract is upgraded to a new version. Such code may be necessary to initialize new state variables, emit events, etcetera. The name refers to the fact that these are like the constructor that runs when a contract is deployed, which is the particular case when a contract is upgraded from not existing to the first version.

Simple migrations are the kind that transform data in the previous version's state variables into a new format necessary for the newer version. An example could be URIs for metadata of an NFT, migrated from one protocol to another. This kind of migration can be performed lazily: for each datum, the migration to the new format happens when it is accessed for the first time.

Full migrations are necessary when a new version needs data from the contract's history that isn't stored in the already available state variables. This is the case of Basic to Full 721, because the Full version adds an ownedTokens mapping which is (practically) impossible to recover from the state variables in the Basic version. It's unclear how to solve this problem. We may need to use trustless off-chain computation for it.

I made these names up, and there may be other kinds of migration-like things that I missed.

[spalladino]

There's another kind of migration that we ran into today, let's call it Migration initializer (horrible name, please suggest an alternative!).

A generalized constructor as defined above (ie an initialize method, only callable if an initialized flag is not set) is needed when creating a new proxy. This method typically receives a few parameters, which are set as initial (and sometimes immutable) values (see here for an example). This was abstracted into an Initializable base contract.

Now, when migrating to a new version, sometimes there is a need to either modify some of those values, or set new ones, which are also passed in as arguments. However, this method cannot depend on the initialized flag, since it is already set. This means that each new version must have its own initialized flag, which renders the Initializable contract useless.

We need to provide a reliable mechanism to ensure that the initializer for each version is properly called, and is called exactly once, even when upgrading from a V1 to a V3 (ie skipping a version).

[spalladino]

Consider reviewing the gemstone objects DB to see how object migrations are managed there, and try to draw inspiration from there (if it applies).

[spalladino]

Gemstone provides a default migration mechanism in which instance variables are migrated to variables on the destination class which share the same name. It is possible to provide a custom mapping, as well as custom code to calculate new fields from the original ones (see Instance Variable Mappings).

[spalladino]

Regarding the generalized constructors as defined by @frangio (which I misunderstood and redefined in a previous comment as migration initializer), I'm thinking about a Migratable interface that keeps track of the migrations run.

contract Migratable {
  mapping migrated(string => bool);

  modifier isInitializer(string version) {
    require(!migrated[version]);
    _;
    migrated[version] = true;
  }

  modifier isMigration(string from, string to) {
    require(migrated[from] && !migrated[to]);
    _;
    migrated[to] = true;
  }
}

For comparison, this is the Initializable code we have so far:

contract Initializable {
  bool private _initialized;

  modifier isInitializer() {
    require(!_initialized);
    _;
    _initialized = true;
  }
}

Usage could be something like:

contract MyContract is Migratable {
  uint256 value;
  function initialize(uint256 initValue) public isInitializer("v1") {
    value = initValue;
  }
}

contract MyContractV2 is MyContract {
  function migrate(uint256 newValue) public isMigration("v1", "v2") {
    value = newValue;
  }
}

@frangio @facuspagnuolo if you think it's worthwhile, I can submit a PR with this to zeppelinos/core along with a few tests. I'd also like to know what you had in mind about this.

[spalladino]

This approach could also be modified to handle large migrations, which require several transactions to be run by a migrator until the state is properly recalculated. One idea could be to have an intermediate MigrationContract (between V1 and V2) which only has the migration methods, and has no business logic (so it "pauses" the contract while it's set).

contract ComplexMigration is Migratable {
  address migrator;
  string from;
  string to;

  function ComplexMigration(string _from, string _to) {
    from = _from;
    to = _to;
  }

  function startMigration(address handler) {
    require(migrated[from]);
    migrator = handler;
  }

  function endMigration() onlyMigrator {
    migrated[to] = true;
  }

  modifier onlyMigrator() {
    require(msg.sender == migrator);
    _;
  }
}

Usage could be:

contract MyContractStorage {
  // Definition of the contract storage
}

contract MyContract is MyContractStorage {
  // Some initial logic
}

// Note that the migration contract does not extend from MyContract,
// so it does not implement any of its methods, effectively *pausing*
// the entire contract until the migration is finished
contract MyComplexMigrationToV2 is MyContractStorage, ComplexMigration {
  function MyComplexMigrationToV2()
    ComplexMigration("v1", "v2") { }

  function migrateBatch(bytes data) onlyMigrator {
    // ... migrate part of the storage, to be called multiple times
  }
}

contract MyContractV2 is MyContractStorage {
  // New logic be here
}

So an updater first upgrades the proxy implementation to the MigrationContract, runs all the migration operations needed (while the contract is effectively paused), and then moves to the next contract version once the storage is up-to-date.

[frangio]

The Migratable interface looks pretty good to me!

ComplexMigration feels like a big hack but it's actually pretty interesting. There's one thing missing though, which is the actual "pausing": there should be a fallback function that rejects every transaction. Otherwise they'll all succeed but not do anything. Definitely worth experimenting with!

[spalladino]

there should be a fallback function that rejects every transaction. Otherwise they'll all succeed but not do anything.

I think that if there is no fallback function defined, then all not-understood calls are rejected. But we can check to be sure.

[facuspagnuolo]

I agree with @frangio, let's start with the Migratable interface

[frangio]

if there is no fallback function defined, then all not-understood calls are rejected

Completely. I don't know what I was thinking. :sweat_smile:

[spalladino]

Will start working on Migratable, and leave ComplexMigrations for later.