Enrich contract events to contain data needed by child-chain and watcher

[pgebal]

Update data stored in events so it contains everything needed by child-chain and watcher, so they do not have to get if from call data.

Motivation for the changes are:

• we can't rely on call data
• simplicity in elixir services

Changes come with a cost of increased gas.

References #659

Helpful link:
https://omisego.atlassian.net/wiki/spaces/FT/pages/667746350/Minimal+Contract+Events+Review

[souradeep-das]

The older version of the test helpers package could be uninstalled

[pgebal]

Thanks, I decided to stay with the old version and remove the new one. I noticed I didn't use the new version.

[souradeep-das]

@pgebal Can we do a clean uninstall then? looks like it's still there in package-lock

[thec00n]

@pgebal the tests do not pass I would like to see a gas report and compare the impact

[pgebal]

@kevsul please take a look at this commit: f31f72b

I deleted that check because web3py can't handle complex events (I guess because of bytes field). I tried to update web3py hoping that newer version handles that well, but there is a lot of dependencies to untangle. I can get back to that in the last week of the cycle to not block developments in elixir-omg now.

Removing this check does not invalidate the python test.
We have e2e and unit truffle tests that check if a proper event is emitted.

[pgebal]

@pgebal the tests do not pass I would like to see a gas report and compare the impact

yes, I've been playing with it, fixing now
@thec00n fixed

[kevsul]

@kevsul please take a look at this commit: f31f72b

I deleted that check because web3py can't handle complex events (I guess because of bytes field). I tried to update web3py hoping that newer version handles that well, but there is a lot of dependencies to untangle. I can get back to that in the last week of the cycle to not block developments in elixir-omg now.

Removing this check does not invalidate the python test.
We have e2e and unit truffle tests that check if a proper event is emitted.

I think that's fine.

[InoMurko]

hmmm, do we need rlpOutputTx ? I don't think we do. I thin it's only utxoPos that we pull out of calldata.

[InoMurko]

https://github.com/omgnetwork/elixir-omg/blob/306246168240d90f9f4cad63092a8f5ea243ea57/apps/omg_watcher/lib/omg_watcher/exit_processor.ex#L717

[InoMurko]

rlpOutputTx gets renamed into:

{"outputTxInclusionProof", :output_tx_inclusion_proof},
      {"rlpOutputTx", :output_tx},
      {"utxoPos", :utxo_pos}

so it's output_tx.

[pgebal]

We need it in our current code @InoMurko . Check fields.ex and exit processor core. We are currently getting it from call data. We could work around that by getting the tx from rocksdb storage.

[InoMurko]

yeah fields.ex is a mapping module for structural decoding of ABI data, it's not really used anywhere.

[pgebal]

@InoMurko @boolafish how about here: https://github.com/omgnetwork/elixir-omg/blob/a7b4188c938390b13d069f40d941507ca79faca3/apps/omg_watcher/lib/omg_watcher/exit_processor/exit_info.ex#L66
Isn't it the exiting tx bytes from call data?
Called here: https://github.com/omgnetwork/elixir-omg/blob/a7b4188c938390b13d069f40d941507ca79faca3/apps/omg_watcher/lib/omg_watcher/exit_processor/core.ex#L188

[pgebal]

During a call with @thec00n it was decided to remove: owner and rlpOutputTx fields from ExitStarted event and inputTxs from InFlightExitStarted.
I decided to keep owner field because it is an indexed field and makes life easier for a user if she wants to track her exits. We have analogous fields in some other exit events: InFlightExitStarted, piggyback events, etc.
@thec00n is it ok for you if we leave owner field? And please do a CR :+)

[thec00n]

I discussed this with @pgebal and we covered my concerns in terms of the increased gas usage. In general I think we should plan for a targeted feature team working on reducing gas costs before v2 goes live.