omgnetwork / elixir-omg

OMG-Network repository of Watcher and Watcher Info
https://omg.network
Apache License 2.0
211 stars 59 forks source link

Child chain should include IFEd txs in blocks #994

Open pdobacz opened 5 years ago

pdobacz commented 5 years ago

Expanding on the original issue OMG-349 - Include IFEd txs in blocks?.

With the new learnings, it is apparent that the more involved model (called exoife below) turns out to be much better. Aside from the "benefit a/" known originally, there's 3 more (see comment below).

Originally, exoife was left as a nice-to-have with too little advantage, not worth the extra complication (see original issue summary). I think it is now a warranted feature.

Original ticket:

user story(ies)

As an operator, I want to include txs that reach me via IFE on ethereum. As a wallet provider, I want the child chain to help me prove IFEd txs canonic, if they're valid.

Summary

There are two approaches to handle IFEs in the child chain server 1) "sai" - spend all inputs - first take, currently implemented, for laziness sake 2) "exoife" - exec on IFE - potential modification

sai means that we spend all inputs of an IFEd txexoife means that we first try to exec (as if submitted via submit) the IFEd tx, and then spend all of its inputs.

Benefits of exoife: a/ better support for cases, where two actors make a payment jointly, and one of them IFEs before tx is submitted. With exoife the child chain would include the tx in a block, effectively canonizing it. If the receiving actor piggybacked the output already, they have a certainty, that within 2 weeks the tx will be theirs (they submit the inclusion proof and wait).

There's also a chance of the receiving actor to delay the piggyback, and if the tx is included within that delay, no 2 weeks waiting.

Mind however that this is an exotic scenario

Drawbacks of exoife: b/ more code. more complexity, because the "exec as if submitted" part might fail for various reasons: insufficient fees, full block (which is very random). There needs to be some additional thinking done on how to be predictable.

Draws: c/ consistence of behaviors - both approaches seem rather consistent in their behavior, levels of possible astonishment is similar d/ <obsolete, removed>

pdobacz commented 5 years ago

In addition to (a/) above (exoife allows for faster finality of honestly IFEing txs), we have 3 more benefits of exoife

e/ allows actors involved in an IFE, that aren't starting it, to piggyback less aggressively, since txs will get included

f/ forces the honest child chain to push an IFEing RC DEX MVP1 settlement, preventing gaming on forced withdrawal notification timing (see respective specs)

g/ allows smarter state and finalization validity management in the Watcher's ExitProcessor:

When IFE finalizes, the finalizations given in the event from the root chain contract hold all piggybacked inputs/outputs that are actually exiting. This means that only outputs are exiting for canonical IFE, and inputs for non-canonical IFEs. If we assume that the child chain includes (canonicalizes) all IFE txs, if they're valid (canonicalizable), then we can assume, that if an IFE finalizes canonically, this tx must have been included by the chch.

Failure to do so can be considered a failure of the chch. In such case, if someone piggybacks on such canonical IFEs output and then it finalizes, those outputs will not be found in the Watcher's UTXO set, leading to some byzantine event (potentially labeled unchallenged as well). Possibly, a new byzantine event must be defined, as this will neither be non_canonical_ife nor invalid_piggyback. (It can be a new invalid_exit_finalization, which could be useful more generally)

OTOH if the IFE tx is included (validly) it becomes strictly canonical, i.e. there's no older competitor. When such IFE finalizes and output is piggybacked, and is not found in Watcher's UTXO set, it means that the said output has been spent in a subsequent transaction, so an invalidly piggybacked IFE output was allowed to finalize. The failure is, again, a warranted invalid_piggyback + unchallenged_exit (or invalid_exit_finalization)

If an IFE tx finalizes "invalidly canonically", i.e. there has been a canonical competitor (included in the block), which prevented the canonicalization of said IFE tx by the child chain, it will mean that such IFE tx will attempt to exit outputs that never existed. This warrants a non_canonical_ife + unchallenged_exit condition (or invalid_exit_finalization).

pdobacz commented 5 years ago

Labeling as bug. The reason is that in case a canonical but not-included tx finalizes, it will make /status.get on the Watcher to emit spurious piggyback_available events.

This is mistakingly tested here. The mistake got missed, because the assertion accepts any byzantine event in the response, and it is the spurrious piggyback_available.

There is a tentative branch demonstrating an alternative set of behaviors to be tested, which should pass in exoife model (now, one of the would fail): https://github.com/omisego/elixir-omg/tree/temp/944_updated_IFE_finalization_test

pdobacz commented 4 years ago

Unlabelling this as bug because #1046 needed to resolve the problem, and it did. Canonical but not-included IFEs should now work anyway.

Still this remains here as an enhancement issue, motivated as explained in the main part of the description.

boolafish commented 4 years ago

Hmmm, interesting. I think to enable exoife, we would also need to make sure we can handle the case of competing IFE shows up. If we are aggressively including one of them to block, we would probably need to aggressively respond to the canonical challenge + make sure we do challenge the another IFE tx non-canonical. Otherwise a mismatch state from exited IFE and the Plasma chain would be ..... fun(?)

pdobacz commented 4 years ago

we would probably need to aggressively respond to the canonical challenge

Hm, this sounds like Watcher's responsibility (and we're doing it - Watcher will do its best to challenge the non-included tx with the included one, and not the other way around).

So from state resolution's perspective, this should be in the end resolved the same way as if the "first & canonical" IFE tx was originally included and IFEd afterwards.

boolafish commented 4 years ago

Hm, this sounds like Watcher's responsibility (and we're doing it - Watcher will do its best to challenge the non-included tx with the included one, and not the other way around).

Ah, I see. A side note is that when two competing IFE shows up, and both are not included to a block, we as the operator would kind of force make decision on the canonicity, probably depends on our discovery time/order.

BTW, do we need to worry on the edge case of our discovery time? Like we discovered the IFE tx really close to the end of IFE challenge period phase 1. I think in practice we got days so might be fine? Probably a bit tricky on our non prod env.

pdobacz commented 4 years ago

Ah, I see. A side note is that when two competing IFE shows up, and both are not included to a block, we as the operator would kind of force make decision on the canonicity, probably depends on our discovery time/order.

first come, first serve; the order is prescribed by the order of respective txs in the root chain.

BTW, do we need to worry on the edge case of our discovery time? Like we discovered the IFE tx really close to the end of IFE challenge period phase 1. I think in practice we got days so might be fine? Probably a bit tricky on our non prod env.

This is catered for with the SEs, by means of exit_sla_margin and unchallenged_exit events. The child chain must "discover" SEs (and sometimes when some issues get done) IFEs within a specific time frame.

If the discovery is "late" in this sense, and someone manages to invalidate an old SE, it triggers an unchallenged_exit event and causes a mass exit. More here