enrich emitted event InFlightExitChallenged

[InoMurko]

[ ] bug report
[ x] feature request

InFlightExitChallenged event log looks something like:

"InFlightExitChallenged(address,bytes32,uint256)"
%{
    "address" => "0x92ce4d7773c57d96210c46a07b89acf725057f21",
    "blockHash" => "0xcfffb9645dc8d73acc4c825b67ba62924c62402cc125564b655f469e0adeef32",
    "blockNumber" => "0x196",
    "data" => "0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff",
    "logIndex" => "0x0",
    "removed" => false,
    "topics" => ["0x687401968e501bda2d2d6f880dd1a0a56ff50b1787185ee0b6f4c3fb9fc417ab",
     "0x0000000000000000000000007ae8190d9968cbb3b52e56a56b2cd4cd5e15a44f",
     "0x7532528ec22439a9a1ed5f4fce6cd66d71625add6202cefb970c10d04f2d5091"],
    "transactionHash" => "0xd9e3b3aaff8156dab8b004882d3bce834ba842c95deff7ec97da8f942f870ab4",
    "transactionIndex" => "0x0"
  }

we decode and postprocess this into:

%{
    challenger: <<122, 232, 25, 13, 153, 104, 203, 179, 181, 46, 86, 165, 107,
      44, 212, 205, 94, 21, 164, 79>>,
    competitor_position: 115792089237316195423570985008687907853269984665640564039457584007913129639935,
    eth_height: 406,
    event_signature: "InFlightExitChallenged(address,bytes32,uint256)",
    log_index: 0,
    root_chain_txhash: <<217, 227, 179, 170, 255, 129, 86, 218, 184, 176, 4,
      136, 45, 59, 206, 131, 75, 168, 66, 201, 93, 239, 247, 236, 151, 218, 143,
      148, 47, 135, 10, 180>>,
    tx_hash: <<117, 50, 82, 142, 194, 36, 57, 169, 161, 237, 95, 79, 206, 108,
      214, 109, 113, 98, 90, 221, 98, 2, 206, 251, 151, 12, 16, 208, 79, 45, 80,
      145>>
  }

PS: naming we use is: [{"challenger", :challenger}, {"challengeTxPosition", :competitor_position}, {"txHash", :tx_hash}], where key is contracts naming, value is elixir-omg naming.

Now the problem is as follows:

this event alone is unusable to us and we must further enrich the data to:

%{
    
      competing_tx: <<248, 116, 1, 225, 160, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
        0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 59, 154, 202, 0, 238,
        237, 1, 235, 148, 198, 56, 110, 145, 88, ...>>,
      competing_tx_inclusion_proof: "",
      competing_tx_input_index: 0,
      competing_tx_pos: 0,
      competing_tx_sig: <<135, 150, 205, 47, 23, 196, 225, 163, 14, 99, 107,
        226, 239, 125, 163, 142, 45, 38, 37, 87, 165, 186, 252, 203, 197, 98,
        199, 6, 66, 2, 159, 127, 31, 39, 225, 8, 133, 11, 72, 94, 217, 104, 169,
        ...>>,
      in_flight_input_index: 0,
      in_flight_tx: <<248, 163, 1, 225, 160, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
        0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 59, 154, 202, 0, 248,
        92, 237, 1, ...>>,
      input_tx_bytes: <<248, 83, 1, 192, 238, 237, 1, 235, 148, 122, 232, 25,
        13, 153, 104, 203, 179, 181, 46, 86, 165, 107, 44, 212, 205, 94, 21,
        164, 79, 148, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, ...>>,
      input_utxo_pos: 1000000000
    
  }

we do this by calling the eth node method: eth_get_transaction_by_hash.

Could we expand the Event with the data we need to further retrieve from eth client?

[
      :input_tx_bytes,
      :input_utxo_pos,
      :in_flight_tx,
      :in_flight_input_index,
      :competing_tx,
      :competing_tx_input_index,
      :competing_tx_pos,
      :competing_tx_inclusion_proof,
      :competing_tx_sig
    ]

[kevsul]

In general we try to send as little info as possible in events, because it's all stored in the tx logs, which costs money. Some of those fields are quite large: input_tx_bytes, in_flight_tx, competing_tx, competing_tx_inclusion_proof, competing_tx_sig

But I think it can be done. A topic (indexed) field can be max 32 bytes, but other fields have no limit.

Is it worth it though? Is the benefit one less eth_get_transaction_by_hash call? We should calculate exactly how much extra gas this would cost per challenge (which in turn affects the bond sizes)

[InoMurko]

There are three instances where we need to enrich the event. I'll gather all of them below and we can asses whether it makes sense to proceed or not. The problematic part I think is where these events are not sufficient - in exits. So a large amount of exits detected in a block means we need to eth_get_transaction_by_hash for each separately - in a tight loop!

Events needing enrichment: InFlightExitChallenged(address,bytes32,uint256)

• calling function challengeInFlightExitNotCanonical to retrieve: "inputTx", "inputUtxoPos", "inFlightTx", "inFlightTxInputIndex", "competingTx", "competingTxInputIndex", "competingTxPos", "competingTxInclusionProof", "competingTxWitness"

ExitStarted(address,uint160)

• calling function startStandardExit to retrieve: :utxo_pos, :output_tx, :output_tx_inclusion_proof

InFlightExitStarted(address,bytes32)

• calling function startInFlightExit to retrieve: :in_flight_tx, :input_txs, :input_utxos_pos, :input_inclusion_proofs, :in_flight_tx_sigs

WDYT?

[InoMurko]

In event InFlightExitChallenged(address,bytes32,uint256) function challengeInFlightExitNotCanonical competing_tx_inclusion_proof is actually not used. pls confirm @pdobacz I've ran tests that remove this field and it seems OK.

[pdobacz]

@InoMurko

re field not being used - this could be totally possible, not all fields that the "enriching" adds are used. The enriching has been done "100%" for events that required it, that is it captured all of the call_data items.

---

Now more on the general topic: an alternative I would suggest is that we plan for a thorough rethinking of how we approach contract events (and in general - reading data from the contract) as opposed to including the bits that the current implementation of chch/watcher uses.

So following an old issue:

There's a couple of factors at play here:

1. data can be read in 3 ways:
   • read the state of the contract from the currently synced eth-height
   • read the log entries
   • read the call data
   • (added now) run helper transformation functions like the getInFlightExitId(bytes)
2. gas implications: generating logs is expensive but not that much. Persisting state of the contract (e.g. not deleteing it in order to read data later) is expensive too
3. What kind of data lives where: e.g. call_data won't ever hold anything that is dependent on the state of the contract
4. consistency, consistency, consistency

There might be data that would be useful to have in the events, but we're not currently getting it via the "enriching" mechanism, I think one example is exit_ids which could potentially be included in the events.

[InoMurko]

Similarly output_tx_inclusion_proof is not used for: ExitStarted(address,uint160)

calling function startStandardExit to retrieve: :utxo_pos, :output_tx, :output_tx_inclusion_proof

[InoMurko]

@kevsul @boolafish do you guys think this is feasible?

[boolafish]

hmmm, I am still not fully convinced we should include so many data in events, especially quite some data can probably be fetch from DB (eg. tx, inclusion proof) instead of from root chain at all.

Probably in-flight exit is the exception (and thus the main discussion here?) as we cannot assume the service (ch-ch/watcher) has the in-flight exit tx data. So in-flight exit data must to some degree rely on root chain data source.

So a large amount of exits detected in a block means we need to eth_get_transaction_by_hash for each separately - in a tight loop!

Can I understand a bit more what is the problematic part here? Is it the infura cost being too high? Or sudden huge spike might causing timeout of the process as it is looping around?

Due to the gas cost of startIFE, we could have around max 166 startIFE per eth block. (10,000,000 / 60,000). It is not crazy amount of number though it is not a really small amount of number too. Would this be okay even if we continue the eth_get_transaction_by_hash path?

---

But don't get me wrong, I do feel there is need to re-structure our events and is also trying to collect inputs for that. However, my feeling is that for txBytes it might be a bit too expensive and unnecessary to log it in event....for other data with limited size it might be easier (like those indexes)?

[InoMurko]

We have three events that we have to "enrich" with call data (eth_get_transaction_by_hash).

• exit_started
• in_flight_exit_started
• in_flight_exit_challenged

From what I'm able to gather, the data we need from the extra "call" is: SE I believe (exit_started):

• utxo_pos
• output_tx

IFE (in_flight_exit_started)

• in_flight_tx
• in_flight_tx_sigs
• input_txs: input_txs
• input_utxos_pos: input_utxos_pos

IFE (in_flight_exit_challenged)

• competing_tx
• tx_bytes
• competing_tx_input_index
• competing_tx_sig

The thing is, that if we cannot include all fields for an event it really doesn't make sense to one or two. What I mean is: if you add utxo_pos to the SE event I'll still need to get output_tx with an extra call. So retrieving one field or two is still one RPC call so it wouldn't solve the problem.

[boolafish]

Just flying over the issue I get the impression that the rationale here is to expand Logs in order to save RPC calls. If that is the case then I have strong reservations of refactoring the events with the result of increasing the gas costs or adding more net more to logs. Bear in mind that increasing the gas price for logs was actively discussed at the last devcon and it's likely to happen as logs are a major factor in increasing the requirements to run a node.

--- from @thec00n (moving comment from confluence)