Under-quarantine ExitGame cannot call Vault's withdraw, exit finalization time takes no effect on quarantine.

[pnowosie]

Moved one work-item from #172 as a separate issue.

What

It should check the quarantined period of the ExitGame. That is the protection period for the ExitGame to take any effect, and ExitGame would call withdraw function of vaults so we need to protect the vault from new ExitGame contracts..

[pnowosie]

Hey @boolafish after analyzing this task some questions emerged. The code is quite complex already I think we can benefit from discussing some concept here.

What is related to this task: 1: why we don't rely on quarantine protection when exit is enqueued? Checking already enqueued exit on finalization if it (ExitGame) can call withdraw seems to deny that it cannot be enqueued first place :question: Or I'm missing smth?

2: Also do we plan to support ExitGame bugfixing? For now, once exit game is registered for tx type no further contracts can be registered with this tx type again. So it would need a new tx type to register fixed exit game and transfer funds which are utxos from old tx type to new one (which disrupts exit priorities)

3: Can you please elaborate more on mappings exit game <-> tx type. How you can see it'll be used?

Thanks :exclamation:

CC: @InoMurko @pdobacz

[boolafish]

I will quickly answer 1. For 2,3 I will need my computer, I will answer them next week.

For 1. Because the ExitGame contract can be anything. An operator can just deploy a contract that can call the vault without even any exit starts. And that’s the main reason of this task.

We could let the vault check the exit status instead, but I think that is quite difficult. You would need to know: 1. The exit should be related with the token and amount it is claiming to withdraw (this is especially hard given the tx data structure can be unknown) 2 the exit must pass the queue period .

[boolafish]

Btw, please take a look at #197 too. I think the two tasks should use the same period.

[pdobacz]

@boolafish To expand on 1/:

why can't the quarantine consist in preventing the "new" ExitGame from calling anything (enqueue, withdraw or finalize) at all until the quarantine passes, instead of going through comparing the "able-to-exit" (as described here https://github.com/omisego/plasma-contracts/issues/172) ?

[boolafish]

If we put limit on ExitGame itself, how do we make sure that new ExitGame must follows the rule?

[pdobacz]

but we don't put that limit on the ExitGame, we still put it on whatever ExitGame is calling, or some Registry. Instead of preventing a "fresh" ExitGame enqueuing exits that would finalize too early, we would be preventing any actions at all.

Or maybe I'm missing something here

[boolafish]

we would be preventing any actions at all

This is the part I don’t know how to enforce aside from limiting via all callee function of ExitGame. Or do you have an idea in your mind?

[boolafish]

Instead of preventing a "fresh" ExitGame enqueuing exits that would finalize too early,

Ah....sorry I finally realized your question. I originally thought if we do that, when operator being fraud, user only need to mass start exit but doesn’t need to finish the processExits from queue within time. I think this is what current plasma promises. However as #197 pointed out this seems to be not possible so I gave up and basically do what you’re proposing.

I think current existing implementation is doing what you’re proposing as well and does not compare the exit data when enquirer.

[boolafish]

2: Also do we plan to support ExitGame bugfixing?

This is a good one, which I would really like to hear more voice from elixir dev side. Some background discussion can be checked here: https://github.com/omisego/plasma-contracts/issues/108. I think we have two options here: 1. Each bug fix results in new tx type. 2. We "bump the version" of a tx type ExitGame contract.

We chose (1) previously because I was assuming in Prod env, we don't put contract without audit, and (hopefully) assumes audit would prevent us from bug (?). With the assumption of only a few and unfrequent upgrades in Prod env, using new tx type provide user slightly better security as they can choose to opt-in to move to new tx type themselves. (Well...without good predicate isolation, that is more or less a false claim for security since a invalid new ExitGame contract can still steal different tx type's fund due to a single plasma fund pool. But UX wise, it is about do we prefer to ask user to sign-off they agree to move to new upgraded version or we force them using new software)

Okay, back to bug fix, I assume most of the bug fix would be happened in our development and staging env. I was thinking to use the following flow to handle our more frequent upgrade in non-prod env: proposal here. Of course, this would mean we need to handle different tx type <> ExitGame contract mapping in different env, which would cause some extra complexity.

However, as I recently re-visits this topic: comment here, I am still on the fence to use upgrade by bumping version or to use upgrade by new tx type. I can see how bumping version style decrease complexity in our software quite a lot and make our (dev) life easier. And without good predicate isolation, the biggest benefit of new tx type (security) decreases.

I had a slack thread on this topic, feel free to join the discussion: here or left comments here.

3: Can you please elaborate more on mappings exit game <-> tx type. How you can see it'll be used?

Currently I mostly see it being used in two things: 1) Add new feature, lead to new exit game and tx type. 2) upgrade existing tx type. Current design uses new tx type as an upgrade.

[pnowosie]

Hey @boolafish thanks for the comments! Let me just highlight important outcome which could be missed in other not-less important thoughts.

Here we decided to prevent quarantined contract to call anything before quarantine has passed. So the exit finality time is not considered here. I'll edit :issue: title accordingly.