Add root chain transaction hash to InvalidExit and UnchallengedExit events #1479
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 8 commits
April 20, 2020 17:12
…471-byzantine-reporting-enhancements
…471-byzantine-reporting-enhancements
kalouo
changed the title
jarindr
approved these changes
Apr 21, 2020
InoMurko
suggested changes
Apr 21, 2020
unnawut
reviewed
Apr 21, 2020
apps/omg_watcher/test/omg_watcher/exit_processor/persistence_test.exs
Outdated
Show resolved
Hide resolved
added 3 commits
April 22, 2020 08:07
…471-byzantine-reporting-enhancements
|
Also added dialyzer specs in |
unnawut
approved these changes
Apr 22, 2020
souradeep-das
approved these changes
Apr 22, 2020
InoMurko
suggested changes
Apr 22, 2020
| } | ||
|
|
||
| def new(contract_status, %{eth_height: eth_height, call_data: %{output_tx: txbytes}, exit_id: exit_id} = event) do | ||
| @spec new(map(), map()) :: t() | ||
| def new(contract_status, event) do |
There was a problem hiding this comment.
wait... this input is not a struct. it's a map. Who is calling this?
There was a problem hiding this comment.
my comment (to use a dot access modifier was strictly for internal type structs ExitInfo
| is_binary(currency) and is_binary(owner) and is_integer(exit_id) and is_binary(exiting_txbytes) and | ||
| is_boolean(is_active) do | ||
| @spec make_db_update({Utxo.Position.t(), t()}) :: Utxo.Position.db_t() | ||
| def make_db_update({position, event}) do |
There was a problem hiding this comment.
Suggested change
| def make_db_update({position, event}) do | |
| def make_db_update({position, exit_info}) do |
There was a problem hiding this comment.
Both make sense to me but exit_info is clearer. Changed in other methods for consistency except using exit_event for new/2
InoMurko
approved these changes
Apr 22, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
#1480
Overview
This adds a
rootchain_txhashproperty to theinvalid_exitandunchallenged_exitevents, 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
exit_processor/exit_info.exto persist the root chain transaction hash in the database.omg_watcher/event.exto add root chain transaction hash to the event struct.Testing
<<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.