[2] Add root chain transaction hash to InvalidExit and UnchallengedExit events

[kalouo]

The original PR broke the Watcher because existing DB entries did not have newly added keys.

This is the original PR with some additions:

• defaults root_chain_txhash to nil for previously existing database entries without this key.
• writes a test (exit_info_test.exs) to test for this and effectively prevent adding a new key without similar handling.

Test with mix test test/omg_watcher/exit_processor/exit_info_test.exs

Original PR

#1480

Overview

This adds a rootchain_txhash property to the invalid_exit and unchallenged_exit events, referring to the Ethereum transaction hash of the exit in question.

As seen by running the cabbage tests locally:

{
        "details": {
          "amount": 12000000000000000000,
          "currency": "0x0000000000000000000000000000000000000000",
          "eth_height": 899,
          "name": "unchallenged_exit",
          "owner": "0x37d5c1fbfa32b6582b425d5ea8d724478dac0aee",
          "root_chain_txhash": "0x5ebfbd6caff8d53e74590a646ffa5018e8776445c78ed735e29bdb52e55edd9c",
          "utxo_pos": 2001000000000
        },
        "event": "unchallenged_exit"
      },
      {
        "details": {
          "amount": 12000000000000000000,
          "currency": "0x0000000000000000000000000000000000000000",
          "eth_height": 899,
          "name": "invalid_exit",
          "owner": "0x37d5c1fbfa32b6582b425d5ea8d724478dac0aee",
          "root_chain_txhash": "0x5ebfbd6caff8d53e74590a646ffa5018e8776445c78ed735e29bdb52e55edd9c",
          "utxo_pos": 2001000000000
        },
        "event": "invalid_exit"
      }

Changes

• Changes in exit_processor/exit_info.ex to persist the root chain transaction hash in the database.
• Changes in omg_watcher/event.ex to add root chain transaction hash to the event struct.

Testing

• No tests added but exit test helper updated to include a transaction hash of <<0::256. Open question whether we'd like a more advanced logic there in this PR.

Note to @DmitryDao to update documentation here and here once this is merged.

[coveralls]

Coverage increased (+0.08%) to 77.983% when pulling d6d9158 on 1471-byzantine-reporting-enhancements into 8189b81 on master.

[pnowosie]

Great work! I'm picking at the test values - nothing really significant.

[InoMurko]

You've said in the comment:

As seen by running the cabbage tests locally:

Do cabbage tests still work if root_chain_txhash is nil. Meaning, for old entries without root_chain_txhash?

[kalouo]

You've said in the comment:

As seen by running the cabbage tests locally:

Do cabbage tests still work if root_chain_txhash is nil. Meaning, for old entries without root_chain_txhash?

The cabbage tests would be persisting ExitInfo structs as per the new specification in the DB so I don't believe you would encounter a scenario where the root_chain_txhash is nil in the cabbage tests.

[InoMurko]

But does the API still work if they are nil?

[kalouo]

But does the API still work if they are nil?

Not sure if I'm addressing your question here but you did draw my attention to one thing.

The byzantine_events array returned by /status.get contains events as defined in event.ex, created in ExitInfo.make_event_data/3.

I just updated the relevant event modules to accept nil for the root_chain_txhash (thanks for prompting that)

If that is what you meant I believe the API should be good. Not sure if it's worth writing a cabbage test for this but I can simulate locally and confirm.

[kalouo]

Confirming that /status.get can return events with root_chain_txhash equal to nil if none is persisted in the DB.

[InoMurko]

this should have been a case clause, not an if