Script to verify upgrade did not corrupt old contract state

[zajck]

What does the script do?

1. Checkouts the contract files at specfied tag (e.g. "v2.0.0"). Technically it would even work with commit instead of tag
2. Calls task "deploy-suite", which compiles and deploys old version locally
3. Populates the old contracts with data
4. Stores the contract state
5. Checkouts new contracts at chosen tag. If tag doesn't exist yet, HEAD or any other commit can be used
6. Calls task "upgrade-facets"
7. Verifies that state after the upgrade matches the state before th upgrade.

This script uses actual other scripts that are used in deployment/upgrade process. Therefore *it's important that config files scripts/config/protocol-parameters.js, scripts/config/facet-deploy.js and scripts/config/facet-upgrade.js represent the actual values that would be used in deployment/upgrade.

Verification process:

• Verify that state right after the upgrade is the same as before
  • Verify variables that have getters defined in the interfaces
  • Verify variables without external getters (using getStorageAt)
  • Verify whole state by using debug_traceTransaction (might be added later)
• Verfiy that adding new data after the upgrade does not corrupt the old data
• Verify that actions that use data from before the upgrade still work as expected
• Verify breaking changes, i.e. calls that should be broken after the upgrade really behave differently than before the update

Last point is specific for each upgrade, while other will probably can be used in every upgrade. Currently everything is in one file (test/upgrade/2.0.0-2.1.0.js) but I migth split it into two - one for general upgrade test and one for upgrade specific tests.

[hswopeams]

This looks like a really good start. I think it would be a good idea to have a test file that is generic and then we can have a test file geared toward a specific upgrade. If it's possible to break them out that way, that would be my preference.

[anajuliabit]

The test is failing when I ran

  1) [@skip-on-coverage] After facet upgrade, everything is still operational
       "before all" hook in "[@skip-on-coverage] After facet upgrade, everything is still operational":
     Error: types/values length mismatch (count={"types":0,"values":1}, value={"types":[],"values":[["0xaaea2fdc2fe9e42a5c77e98666352fc2dbf7b32b9cbf91944089d3602b1a941d","0x90b9d701120ba03749f7021324e5fc97438c847380b773bb897ffa9fabab647c","0x6adb0d9c3c70c5c3ec9332cf7e4c4d2c33eb29cb3e55037c79434ef3fada4044","0x0ae2126d006c21c5824ec10ee0dc64d5ef05f858080dd1a92e3dbe198b5f8499","0xa7ef6a7cecd210eaf489268d95bf65e4565805cc51de6ea0cf0c4b7bf801fec0","0xa880fd89e679d8c813150b269255a8a9ae46984310cddeb8f344a77bbb4cb0c8","0x1227dbbba1af7882df0c2f368ac78fb2c624a77dcfa783b3512a331d08541945","0x1843b3a936e72dc3423a7820b79df54578eb2321480ad2f0c6191b7a2c500174","0x4e534c9650f9ac7d5c03f8c48b0522522a613d6214bf7ba579412924ab0f9295","0xfa92792a82bab95011388f166520331cc3b3a016362e01f3df45575f206a088c","0x125e35ecb0e80f32093bffe0ee126e07e3081113653234a73157ad1a9428e7b5","0xda14451cc7bdf6d3eb088cbf8cf16395d91652dfeb49155fea8e548623cf58fb","0x0eb1de1c910b5f6b3e081ea9d70ea55b1f63e8687221289e0fd95bb63b7f0267","0x7c016ad538b2b5ce85140e48fb6abe43e7ab43b31b114aae967303c96a22f901","0x04f63e12403cb2deb13216a684596fc0a94e03dd09a917ea334ad614d4265c56","0x20a68d25bd6cc258f86e9470a6f721a0c0d78f136b2cf060800568be00bae41b","0x42443efdfc55a8186beaa746c4c9cb35eb3548b30d041a7e394ad7056aaeb83a","0xae707f1e32af7522193cb7fc73109343421ddc883a6a0a4dfbc867ddfc7224ba","0xf7d95f3bb0dbce2f73cdc7ba81489e84901014795d94cdaa35d28446803e96e4","0xfb50e2350c3bb4f09d6e5db77ea5f63dcedbad1f5d95b8ec5b3baec8bfc13e94","0xa5c1674e620c21e2f21f5c1faae4ce84afbd8427e2ff31514412c9ce0737725a","0xdfdcd6135be7ca49767c5f46cf5299f807d20465cfa6bbfd2c53e78c1f5d5d43","0xb4dcefaf4091c503ee1183c6892061bd8b0d7bbfd691734068c3408c6080c512","0x65f65c948d22c455a7cdc716108ceb3e47b011fff5dcd04af5d01259045312cb","0xbed3ac5024241532a75d94ae773c337b68153785c80a53cc5d1fca9cc67edd1c","0x1f317d1c833d4ab29c44bcc446c31b0c3b34e8e9bd89a352476c6aa134ce819d","0x58477db8afd1115f4a8e27ae4284ae8c1b4b9487b287273dca9d2adee8cfbf3d","0x492edd2840bfa58042eb1b8390dfb5353ea588da111ce69dcd63abdbc07de001","0x18170b73b12e66f1bfce19ffd5cd452a578d79a836db53be0d8ac67e8eaceb69","0x229f69432b8d68fefb34adffa3fb8b5c570c343c26221a6f6f12a8d430f7b3c3","0x7b02365d7cb10a5594ae9167f695d5f2b000c870c3e1234bce9b92ff11e81f1c","0x1c6d6b5ed1e326c8b9c8e98d9fbb0a5eb39512bcf9fb33d81c6f3db58599d526","0x79a5fea918bca323054271090b522a46b6d6cb3ccc95209defbcf4c45d492429","0xa07fcc70c56b1cb2044c9a67496d53c6f7917ba096b643debdf405e55e5e49cd","0x685ef733ac3593f8717a833b61899824a7dbdd369abcc9654f645969504f9c5b","0x11ec86930fbd8bd9e4eefe71220f517bf8452e8fa6bfa392f855308c0676c991","0x4ff2e05649742b3d27ffcc7d11ff8dbcc4d0505fb4e5fb809e3ad610a624a6d7","0xc1c96af89a34084bccc5d426d6b90b7261d20475e2cf693fa4b4b23274d43730","0x20dead55d3c8b1b471af73c8ce8e057cd9347260c6ee844343951965ce4bb5e5","0xcb5fcf36e049de1f1a0a7b504e72ef51aa215f0fff1ce6df3c50b5d7374d3ef1","0x44c64d38103a262e536c7d44b00850ee7d39568e0a5e23a07cc0db96d8ca588b","0xdf3ce320c0a4e7295bf81a5ad49769fcb25784d66f5d4a39b978501163fda0dd","0xcc00c0613629c16c77d0f383611bdc1dd0be75cce1266e5bb757a2c1b5fbf349","0xdb7af92f84da228054781ac9bf5783f28ebcc1d9ac26648188455e8b32e0f98e","0x4915907bc4b6b677a3fb6bb7862f7f63f7ef6bed8cfb0ccebd43a02556656376","0x12b52cf58b53505833cfe90f93fce86e29e512b64543a64e345eb11dde029659","0x34fa96a697bb35f3a16a0ecd1fbbca12f084d2fe335fc54eb4a7032092c74c33","0x3635882429ecf7427e1d090edff7038f6d8b32e69247f4dfcc8bb1aea18e7617","0x1b002277f50ea47601c7446146c19b0603fc425d8b3db37aa07e12271713db19","0x3e03b0f6491639aad746484d776881edb4b355319b1a7fbf77e1a05d5c0139a2","0x088177c89f07128f658d1605a229e33d6cea2f974744e08c17a7a400db6cb91d","0x0568624426a2ae4a64888e7e5938460834a3ed753a7f3726360d325908d47399","0x0b1bb60854d611e75ddd006cf363e547eec8e9b3f43d0bcc4979ab78d09eb6b8","0x97a6f155d7dafa8b1117ef2d0236f849f55cddfec075df22bbe28961298faae7","0xb44d17ebf635b4c05bbf970f2099e25db03cec15fcbf5e2585b95c3bb2512362","0x6574e3baf5c3a80b6ede618c3c5c4db8242490b3f9432dd9cb566a5099f0e2ed","0xa290249cf9da6ed767f95dfdebfd34bfc08232a805970f973e56369975f04d6f","0x3f4ba83af89dc9793996d9e56b8abe6dc88cd97c9c2bb23027806e9c1ffd54dc"]]}, code=INVALID_ARGUMENT, version=abi/5.6.4)
      at Logger.makeError (node_modules/@ethersproject/logger/src.ts/index.ts:261:28)
      at Logger.throwError (node_modules/@ethersproject/logger/src.ts/index.ts:273:20)
      at AbiCoder.encode (node_modules/@ethersproject/abi/src.ts/abi-coder.ts:101:20)
      at Interface._encodeParams (node_modules/@ethersproject/abi/src.ts/interface.ts:323:31)
      at Interface.encodeFunctionData (node_modules/@ethersproject/abi/src.ts/interface.ts:378:18)
      at deployProtocolHandlerFacetsWithArgs (scripts/util/deploy-protocol-handler-facets.js:64:57)
      at runMicrotasks (<anonymous>)
      at processTicksAndRejections (node:internal/process/task_queues:96:5)
      at main (scripts/deploy-suite.js:166:34)
      at SimpleTaskDefinition.action (hardhat.config.js:53:5)

[zajck]

@anajuliabit I believe test is failing for you, since your configs are not properly set.

In v2.0.0 metaTransactionHandler had no init args. Therefore scripts/config/facet-deploy.js must be sth like

const noArgFacetNames = [
  "AccountHandlerFacet",
  "SellerHandlerFacet",
  "BuyerHandlerFacet",
  "DisputeResolverHandlerFacet",
  "AgentHandlerFacet",
  "BundleHandlerFacet",
  "DisputeHandlerFacet",
  "ExchangeHandlerFacet",
  "FundsHandlerFacet",
  "GroupHandlerFacet",
  "OfferHandlerFacet",
  "OrchestrationHandlerFacet",
  "TwinHandlerFacet",
  "PauseHandlerFacet",
  "MetaTransactionsHandlerFacet"
];

async function getFacets() {

  return {
    noArgFacets: noArgFacetNames,
    argFacets: {  },
  };
}

And scripts/config/facet-upgrade.js should match the upgrade that is actually used for upgrade to 2.1.0 i.e.

    addOrUpgrade: ["ERC165Facet", "AccountHandlerFacet", "SellerHandlerFacet", "DisputeResolverHandlerFacet"],
    remove: [],
    skipSelectors: {},
    initArgs: {},
    skipInit: ["ERC165Facet"],

[hswopeams]

Looks good. I just have a few questions and comments.

[anajuliabit]

IMO we should define the facets to deploy and facets to upgrade on separate files instead of using facet-deploy.js and facet-upgrade.js. If we are going to keep specific test files for each version I think they should be handled as immutable and NOT depends on changing config values every time we want to execute a different version test.

[anajuliabit]

LGTM

[anajuliabit]

Please put getMappingStorageFunction at utils.js so we can use in others files as well

[cliffhall]

Looks good, will approve after @anajuliabit's request is handled.

[cliffhall]

LGTM! 👍