PaymentExitGame deployment

[InoMurko]

The issue is that using a multisig contract for the maintainer account, truffle migration cannot be streamlined.

Why is that? That's because the constructor of PaymentExitGame expects both vaults (ETH and ERC20) to be registered:

await ethVault.setDepositVerifier(ethDepositVerifier.address, { from: maintainerAddress });
        await plasmaFramework.registerVault(
            config.registerKeys.vaultId.eth,
            ethVault.address,
            { from: maintainerAddress },
        );
        await erc20Vault.setDepositVerifier(erc20DepositVerifier.address, { from: maintainerAddress });
        await plasmaFramework.registerVault(
            config.registerKeys.vaultId.erc20,
            erc20Vault.address,
            { from: maintainerAddress },
        );

If that's not so the deployment migration fails with:

Error:  *** Deployment Failed ***

"PaymentExitGame" hit a require or revert statement with the following reason given:
   * Invalid ETH vault

    at /opt/Omise/plasma-contracts/plasma_framework/node_modules/truffle/build/webpack:/packages/deployer/src/deployment.js:364:1
    at runMicrotasks (<anonymous>)
    at processTicksAndRejections (internal/process/task_queues.js:93:5)
Truffle v5.1.32 (core: 5.1.32)
Node v12.10.0

But using a multisig address as the maintainerAddress means the deployment becomes async. We need to wait for confirmations.

What can we do? Can we empty the PaymentExitGame constructor and implement a function:

function linkExitGameWithVaults(PaymentExitGameArgs.Args)

which would then check what the constructor checks, but via the maintainer multisig?

[InoMurko]

For better imagination: https://github.com/omgnetwork/plasma-contracts/blob/5eff26fcc15f0c4d4bd4608e78fbda766562211f/plasma_framework/migrations/1040_setup.js#L84

[boolafish]

Seems possible to use extra init function (your linkExitGameWithVaults function) instead of constructor. The "routers" would need to empty their constructors to init function too.

On the other hands, fixing this in vault should be compared as an option too IMO. One can inject the first deposit verifier via constructor and change the function setDepositVerifier to updateDepositVerifier and simplify the logic there a bit to be without considering the case of first deposit verifier.

[kevsul]

I think moving things out of the constructors and into init functions is the way to enable a more async deployment.

@boolafish I don't really understand your proposal of fixing it in the vault. It's the calls to registerVault that are the preoblem - they aren't executed before the PaymentExitGame gets deployed. I don't see how changing setDepositVerifier would help?

[InoMurko]

@boolafish I don't really understand your proposal of fixing it in the vault. It's the calls to registerVault that are the preoblem - they aren't executed before the PaymentExitGame gets deployed. I don't see how changing setDepositVerifier would help?

I was just looking into that and wondering the same thing 😬

[boolafish]

Lol 😂 my bad Brain not working today

[InoMurko]

https://github.com/omgnetwork/plasma-contracts/pull/660/files

is this how you've imagined it? It's just a POC. Emptied constructors and created init functions on Payment exit game and both routers.

[boolafish]

yes pretty much it!

[thec00n]

Sorry to chime in so late ... I think there is a better approach to this. I propose to rewrite the contracts in a way that any deployment step does not require the maintainer. This will save us a lot of time in production deployments and it also makes them more failure resistant by removing any semi automated steps. Let's look at all the steps in the current deployment where the maintainer is currently used:

• Set version (https://github.com/omisego/plasma-contracts/blob/0495ef108ce269f785bf1c6db4f95bd671c2e987/plasma_framework/migrations/20_deploy_plasma_framework.js#L25): This could be done by calling setVersion from the constructor and requiring the deployer to submit the version through the constructor.
• Deploy Vaults (https://github.com/omisego/plasma-contracts/blob/0495ef108ce269f785bf1c6db4f95bd671c2e987/plasma_framework/migrations/30_deploy_and_register_eth_vault.js#L26 and https://github.com/omisego/plasma-contracts/blob/0495ef108ce269f785bf1c6db4f95bd671c2e987/plasma_framework/migrations/40_deploy_and_register_erc20_vault.js#L22) should be done from the deployer
• Set the deposit verifiers: There are discussions that we should remove deposit verifiers and simplify deposits significantly as they require RLP deposits using more gas and also are suboptimal for none standard ERC20s. In order to be able to move forward with this issue and not block development I suggest that the deployer should be able to set the initial DepositVerifier. setDepositVerifier() could be split up into 2 functions setInitialDepositVerifier which can be done by the deployer and can only be called once and replaceDepositVerifier which can only be called by maintainer following the quarantine rules.
• Register Vaults: In a similar way I would split up registerVault. registerInitialVault can be called by the deployer until _vaultQuarantine.immunitiesRemaining == 0. A maintainer can call registerNewVault post deployment following the quarantine rules.
• Register ExitGame: do the exit same thing with registerExitGame and split it up into registerInitialExitGame and registerNewExitGame