omgnetwork / elixir-omg

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

Child chain should read MIN_EXIT_PERIOD from PlasmaFramework contract #1101

Closed kevsul closed 5 years ago

kevsul commented 5 years ago

Currently the childchain's exit period is set via a configuration variable. However, as discussed in Slack

unnawut  11 hours ago
Now that we're deeper into this. I wonder if there's any use case for an exit period on childchain longer than the plasma_contract's MIN_EXIT_PERIOD?

unnawut  11 hours ago
or if that should ever be allowed

Papa Paweł  11 hours ago
This is v good question! How much longer btw?

unnawut  11 hours ago
No idea at all. Just wondering what could happen if the config in elixir-omg and plasma-contracts misalign.
elixir-omg < plasma-contracts, I guess the contract will just safely reject the exit.
elixir-omg > plasma-contracts, it exits successfully, but just poorer UX from unneccessarily longer wait time? (edited) 

Papa Paweł  11 hours ago
I guess the contract will just safely reject the exit.
I don't think so contract knows nothing about elixir code so exit takes longer to finalize...
just poorer UX
Or security issue? - Watcher tells user there is still time to challenge where in fact... Oops
I wonder why elixir doesn't get the value calling the contract (edited) 

and here #1099 this could mean that the childchain is configured differently than the contracts.

A better solution would be for the childchain to get the value from the PlasmaFramework contract. It's a public member variable: PlasmaFramework.minExitPeriod

pdobacz commented 5 years ago

For some background (might soon become irrelevant, but still is) - the reason why it didn't use the value from the contracts, is that the elixir-omg needed to take care of the early deployments of contracts, and as a consequence, feed exit_period_seconds to plasma-contracts. A bit of a chicken-egg problem.

CC @pnowosie

InoMurko commented 5 years ago

https://github.com/omisego/elixir-omg/pull/1099

boolafish commented 5 years ago

I thought current contract deployment is pretty independent from elixir-omg now(?) or there is still sth elixir-omg should take care of?

InoMurko commented 5 years ago

We're deploying in in our tests @boolafish. Hopefully soon that will stop as well!

InoMurko commented 5 years ago

done here https://github.com/omisego/elixir-omg/commit/fb4a75fff8954ffffc4b49f62124dda6e51913ba