feat: indexer for cross level messages on Arbitrum #9312
Conversation
| query = | ||
| from(msg in Message, | ||
| select: msg.originating_tx_blocknum, | ||
| where: msg.direction == :to_l2 and not is_nil(msg.originating_tx_blocknum), | ||
| order_by: [desc: msg.message_id], | ||
| limit: 1 | ||
| ) | ||
|
|
||
| query | ||
| |> Repo.one() | ||
| end |
There was a problem hiding this comment.
Seems like the base of the query is equal for the next 3 functions except sorting direction and field. I'd suggest to extract the base query to a function and re-use in that functions.
There was a problem hiding this comment.
The difference lies in the value of the direction field and the keyword list used in order_by. Introducing values for the keyword list and further handling would reduce code readability and negate the benefits of introducing the new function. Therefore, I would leave it as it is for now.
| ## Returns | ||
| - The binary representation of the hash. If the input is `nil`, returns the binary form of the default zero hash. | ||
| """ | ||
| @spec strhash_to_byteshash(EthereumJSONRPC.hash() | nil) :: binary() |
There was a problem hiding this comment.
Let's rename to string_hash_to_bytes_hash for readability?
| @@ -154,6 +168,70 @@ defmodule Indexer.Helper do | |||
| ] | |||
| end | |||
|
|
|||
| @doc """ | |||
There was a problem hiding this comment.
Let's add todo reminder here, that probably, it worth to migrate to this generic function in Optimism and Polygon Edge modules as well.
| enabled: ConfigHelper.parse_bool_env_var("INDEXER_ARBITRUM_BRIDGE_MESSAGES_TRACKING_ENABLED") | ||
|
|
||
| config :indexer, Indexer.Fetcher.Arbitrum.TrackingBatchesStatuses, | ||
| recheck_interval: ConfigHelper.parse_integer_env_var("INDEXER_ARBITRUM_BATCHES_TRACKING_RECHECK_INTERVAL", 20), |
There was a problem hiding this comment.
Perhaps, we could rework time variables to use native for this type ConfigHelper.parse_time_env_var parser function?
| add(:originator_address, :bytea, null: true) | ||
| add(:originating_tx_hash, :bytea, null: true) | ||
| add(:origination_timestamp, :"timestamp without time zone", null: true) | ||
| add(:originating_tx_blocknum, :bigint, null: true) |
There was a problem hiding this comment.
For the sake of consistency with the core DB schema I'd suggest making next renamings in the suffixes in fields names fro new tables:
- "tx" -> "transaction"
- "blocknum" -> "block_number"
| ) | ||
|
|
||
| add( | ||
| :confirm_id, |
There was a problem hiding this comment.
Do you think it could be renamed to confirmation id in order to change verb to noun?
| add(:after_acc, :bytea, null: false) | ||
|
|
||
| add( | ||
| :commit_id, |
There was a problem hiding this comment.
The same as for confirm_id: do you think it could be renamed to confirmation id in order to change verb to noun?
| add(:id, :integer, null: false, primary_key: true) | ||
| add(:hash, :bytea, null: false) | ||
| add(:block_number, :integer, null: false) | ||
| add(:timestamp, :"timestamp without time zone", null: false) |
There was a problem hiding this comment.
Is this block_timestamp in fact? Or it would be incorrect to rename it like this in your opinion?
There was a problem hiding this comment.
Although the value of this timestamp matches with the timestamp of the block where the corresponding transaction was included on L1, logically this is the timestamp of the message. This value will be used to display age of the message in the corresponding UI view. I don't see any reason to rename the field to block_timestamp.
| add(:message_id, :integer, null: false, primary_key: true) | ||
|
|
||
| add( | ||
| :execution_id, |
There was a problem hiding this comment.
I noticed at least 2 joins on execution_id field in the logic of Arbitrum fetchers. Taking this in mind does it make sense to add B-tree index for this field? Currently, I don't see any index for this field.
There was a problem hiding this comment.
Correct. These joins are in the following functions: l1_block_of_latest_execution and l1_block_of_earliest_execution. These functions are only invoked in the handler of the init_worker message in Indexer.Fetcher.Arbitrum.TrackingBatchesStatuses, which occurs only once during the fetcher initialization. This means that adding the new index will not significantly impact performance during general fetcher operations, only during startup. I would leave it as it is for now.
| add(:originator_address, :bytea, null: true) | ||
| add(:originating_tx_hash, :bytea, null: true) | ||
| add(:origination_timestamp, :"timestamp without time zone", null: true) | ||
| add(:originating_tx_blocknum, :bigint, null: true) |
There was a problem hiding this comment.
originating_tx_blocknum is used for sorting in. rollup_block_of_earliest_discovered_message_from_l2/0 function. Should we add (B-tree, single-column) index to speed-up that query at potentionally huge table.
There was a problem hiding this comment.
Correct. This function is invoked only in the handler of the init_worker message in the Indexer.Fetcher.Arbitrum.RollupMessagesCatchup module, which occurs only once during the fetcher initialization. This means that adding the new index will not significantly impact performance during general fetcher operations, only during startup. I would leave it as it is for now.
| |> Base.decode16!(case: :mixed) | ||
| end | ||
|
|
||
| defp json_txid_to_hash(hash) do |
There was a problem hiding this comment.
Let's replace "txid" suffix with "tx_id"?
There was a problem hiding this comment.
Also, in zksync module: json_txid_to_hash => json_tx_id_to_hash
| - An instance of `Explorer.Chain.Arbitrum.L1Batch` representing the batch containing | ||
| the specified rollup block number, or `nil` if no corresponding batch is found. | ||
| """ | ||
| @spec get_batch_by_rollup_block_num(FullBlock.block_number()) :: L1Batch | nil |
There was a problem hiding this comment.
suggestion: "get_batch_by_rollup_block_num" => "get_batch_by_rollup_block_number"
Motivation
The proposed changes aim to introduce the indexing Arbitrum cross-chain messages and track their status through indexing rollup transaction batches, rollup block confirmations.
Handling of re-orgs is intended to be done under subsequent PR.
Changelog
Enhancements
The block fetcher has been extended with the functionality to identify L1-to-L2 messages by parsing the block transactions and L2-to-L1 messages by parsing the transactions receipts.
To identify rollup transaction batches, rollup block confirmations, L2-to-L1 messages executions and track appearance of new L1-to-L2 messages, three new fetchers have been introduced:
Indexer.Fetcher.Arbitrum.TrackingBatchesStatuses: Identifies rollup transaction batches, rollup block confirmations and L2-to-L1 message executions. Responsible for both discovery new entities and catching up historical events.Indexer.Fetcher.Arbitrum.TrackingMessagesOnL1: Identifies new L1-to-L2 messages directed to an Arbitrum rollup on L1.Indexer.Fetcher.Arbitrum.RollupMessagesCatchup: For those messages which were not picked up by the catchup block fetcher (when an existing BS instance is upgraded), discovers L1-to-L2 messages by re-requesting historical block transactions and L2-to-L1 messages by parsing transactions receipts available in the database.Six new tables have been added to store relevant data:
arbitrum_crosslevel_messages: Keeps all cross-chain messages related to an Arbitrum rolluparbitrum_l1_batches: Contains details of batches.arbitrum_lifecycle_l1_transactions: Stores hashes and timestamps of L1 transactions.arbitrum_batch_l2_transactions: Records associations of rollup transactions with batches.arbitrum_batch_l2_blocks: Manages associations of rollup blocks with batches and confirmation transactions.arbitrum_l1_executions: Maintain links between L2-to-L1 messages and their executions on L1.Checklist for your Pull Request (PR)
CHANGELOG.mdwith this PRmasterin the Version column. Changes will be reflected in this table: https://docs.blockscout.com/for-developers/information-and-settings/env-variables.