Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inomurko/reorg block getter #1554

Merged
merged 7 commits into from
Jun 2, 2020
Merged

Inomurko/reorg block getter #1554

merged 7 commits into from
Jun 2, 2020

Conversation

InoMurko
Copy link
Contributor

@InoMurko InoMurko commented Jun 1, 2020

Overview

I've observed two things:

:sys.get_state OMG.Watcher.BlockGetter
%OMG.Watcher.BlockGetter.Core{
  chain_status: :ok,
  config: %OMG.Watcher.BlockGetter.Core.Config{
    block_getter_loops_interval_ms: 500,
    block_getter_reorg_margin: 200,
    block_interval: 1000,
    child_chain_url: "http://childchain:80",
    maximum_block_withholding_time_ms: 54000000,
    maximum_number_of_pending_blocks: 4,
    maximum_number_of_unapplied_blocks: 50
  },
  events: [],
  last_applied_block: 241000,
  num_of_highest_block_being_downloaded: 241000,
  number_of_blocks_being_downloaded: 0,
  potential_block_withholdings: %{},
  synced_height: 10178703,
  unapplied_blocks: %{}
}
iex(watcher_info@127.0.0.1)14> :sys.get_state :depositor
{%OMG.EthereumEventListener.Core{
   cached: %{data: [], events_upper_bound: 10178693, request_max_size: 1000},
   ethereum_events_check_interval_ms: 8000,
   service_name: :depositor,
   synced_height: 10178693,
   synced_height_update_key: :last_depositor_eth_height
 },
 %{
   get_ethereum_events_callback: &OMG.Watcher.EthereumEventAggregator.deposit_created/2,
   process_events_callback: &OMG.State.deposit/1
 }}

So two services are syncing, but blockgetter is faster then depositor.
BlockGetter synced_height: 10178703
Depositor synced_height: 10178693

The consolidation PR #1376 introduced a behaviour where all services go through the same funnel to fetch ethereum logs while relying on :depositor reorg protection.

It was observed that :depositor and :block_getter do not have constraints with one another:

depositor: [finality_margin: deposit_finality_margin],
       block_getter: [
         waits_for: [depositor: :no_margin],
         finality_margin: 0
       ]

A following event could cause watcher to have issues:

  • block was formed on height 10
  • blockgetter gets that block + all other events (defined in OMG.Watcher.SyncSupervisor, EthereumEventAggregator, events)
  • deposits or exits or whatever event happens after the block formation
  • a reorg happens in the depositor reorg protection margin (10 blocks)

This would cause the ETS table of event aggregator to store same ethereum log on two different heights.

Changes

  • BlockGetter has it's own reorg protection so we're letting him sync separately from ethereum event aggregator

Testing

Same tests apply.

@InoMurko InoMurko marked this pull request as ready for review June 1, 2020 08:44
defp get_block_submitted_events(block_from, block_to),
do: EthereumEventAggregator.block_submitted(block_from, block_to)
defp get_block_submitted_events(block_from, block_to) do
RootChain.get_block_submitted_events(block_from, block_to)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@doc """
Returns lists of block submissions from Ethereum logs
"""
def get_block_submitted_events(from_height, to_height) do
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@InoMurko InoMurko changed the base branch from master to v1.0.0 June 1, 2020 09:57
Copy link
Contributor

@boolafish boolafish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't we also remove EthereumEventAggregator.block_submitted method function?

@InoMurko
Copy link
Contributor Author

InoMurko commented Jun 1, 2020

shouldn't we also remove EthereumEventAggregator.block_submitted method?

function! :))))

@InoMurko
Copy link
Contributor Author

InoMurko commented Jun 1, 2020

shouldn't we also remove EthereumEventAggregator.block_submitted method function?

done. thanks!

@InoMurko InoMurko merged commit 2bec5d3 into v1.0.0 Jun 2, 2020
@InoMurko InoMurko deleted the inomurko/reorg-block-getter branch June 2, 2020 04:50
@T-Dnzt T-Dnzt mentioned this pull request Jun 2, 2020
T-Dnzt pushed a commit that referenced this pull request Jun 2, 2020
* Inomurko/reorg block getter (#1554)

* dont store blockgetter events

* dont fetch blockgetter events through aggregator

* get_block_submitted_events

* get_block_submitted_events

* return tuple

* the right contract encoding

* unused api function

* prevent race condition for status cache (#1558)

* prevent race condition for status cache

* Changelog for v1.0.0 (#1556)

Co-authored-by: Ino Murko <ino.murko@outlook.com>
unnawut added a commit that referenced this pull request Jun 23, 2020
* Inomurko/reorg block getter (#1554)

* dont store blockgetter events

* dont fetch blockgetter events through aggregator

* get_block_submitted_events

* get_block_submitted_events

* return tuple

* the right contract encoding

* unused api function

* prevent race condition for status cache (#1558)

* prevent race condition for status cache

* Changelog for v1.0.0 (#1556)

* restart strategy (#1565)

* restart strategy

* restart strategy

* restart strategy

* Update changelog for v1.0.0

* global block get interval (#1576)

* Update changelog for v1.0.0

* feat: increase ExitProcessor timeouts (#1592)

* increase timeouts

* docs: changelog

Co-authored-by: Unnawut Leepaisalsuwanna <unnawut@omisego.co>

* chore: update watcher docker-compose to v1.0.1

* docs: small non-content fix to changelog

Co-authored-by: Ino Murko <ino.murko@outlook.com>
Co-authored-by: Thibault <thibault@omisego.co>
boolafish added a commit that referenced this pull request Jun 25, 2020
* feat: sync v1.0.1 changes back to master (#1599)

* Inomurko/reorg block getter (#1554)

* dont store blockgetter events

* dont fetch blockgetter events through aggregator

* get_block_submitted_events

* get_block_submitted_events

* return tuple

* the right contract encoding

* unused api function

* prevent race condition for status cache (#1558)

* prevent race condition for status cache

* Changelog for v1.0.0 (#1556)

* restart strategy (#1565)

* restart strategy

* restart strategy

* restart strategy

* Update changelog for v1.0.0

* global block get interval (#1576)

* Update changelog for v1.0.0

* feat: increase ExitProcessor timeouts (#1592)

* increase timeouts

* docs: changelog

Co-authored-by: Unnawut Leepaisalsuwanna <unnawut@omisego.co>

* chore: update watcher docker-compose to v1.0.1

* docs: small non-content fix to changelog

Co-authored-by: Ino Murko <ino.murko@outlook.com>
Co-authored-by: Thibault <thibault@omisego.co>

* Add block processing queue to watcher info (#1560)

* [WIP] initial queuing work

* refactor: queue processing

* catch db timeouts

* update existing tests

* add tests

* continue with tests

* finish tests

* Fix integration test

* refactor

* refactor

* refactor tests

* remove retry_count

* remove on exist genserver shutdown

* add telemetry queue length event

* sobelow skip BinToTerm

* remove pending block status

* naming

* fix tests

* fix tests

* rename config

* remove unused alias

* fix PR minor comments

* missing file

* Add Transaction filter by end_datetime (#1595)

* add new query

* add if case

* format

* refactor

* add test

* fix feature spec

* use address

* fix assert

* fix test

* dummy assert

* add get single tx

* add wait tx

* working waiter pooler

* add wait helper cabbage test

* test end_datetime querty

* add swagger generator spec

* use second

* fix allow constraints

* update constraints validator

* fix test

* fix test

* edit name

* add empty line'

* more fixes

* fix warning

* refactor use the elixir way

* improve test

* fix test and typo

* use cheap fn

* sort

* use any

* use any boolean return

* use Enum.all

* final credo

* format

* use default params

* update constaints test for end_datetime

* mix format

* @default_paging constant

* fix test end_datetime

* fix test and rename feature file'

* mix format

* Revert "explain analyze updates (#1569)" (#1601)

This reverts commit 3431f26.
This commit is intended to be only deployed to develop env, thus a short life commit.
Reverting this in preparation of release.

* release artifacts (#1597)

* release artifacts

Co-authored-by: Unnawut Leepaisalsuwanna <921194+unnawut@users.noreply.github.com>
Co-authored-by: Ino Murko <ino.murko@outlook.com>
Co-authored-by: Thibault <thibault@omisego.co>
Co-authored-by: Mederic <32560642+mederic-p@users.noreply.github.com>
Co-authored-by: Jarindr Thitadilaka <jarindr23@gmail.com>
boolafish added a commit that referenced this pull request Jul 7, 2020
* chore: merge master back to v1.0.2 (#1606)

* feat: sync v1.0.1 changes back to master (#1599)

* Inomurko/reorg block getter (#1554)

* dont store blockgetter events

* dont fetch blockgetter events through aggregator

* get_block_submitted_events

* get_block_submitted_events

* return tuple

* the right contract encoding

* unused api function

* prevent race condition for status cache (#1558)

* prevent race condition for status cache

* Changelog for v1.0.0 (#1556)

* restart strategy (#1565)

* restart strategy

* restart strategy

* restart strategy

* Update changelog for v1.0.0

* global block get interval (#1576)

* Update changelog for v1.0.0

* feat: increase ExitProcessor timeouts (#1592)

* increase timeouts

* docs: changelog

Co-authored-by: Unnawut Leepaisalsuwanna <unnawut@omisego.co>

* chore: update watcher docker-compose to v1.0.1

* docs: small non-content fix to changelog

Co-authored-by: Ino Murko <ino.murko@outlook.com>
Co-authored-by: Thibault <thibault@omisego.co>

* Add block processing queue to watcher info (#1560)

* [WIP] initial queuing work

* refactor: queue processing

* catch db timeouts

* update existing tests

* add tests

* continue with tests

* finish tests

* Fix integration test

* refactor

* refactor

* refactor tests

* remove retry_count

* remove on exist genserver shutdown

* add telemetry queue length event

* sobelow skip BinToTerm

* remove pending block status

* naming

* fix tests

* fix tests

* rename config

* remove unused alias

* fix PR minor comments

* missing file

* Add Transaction filter by end_datetime (#1595)

* add new query

* add if case

* format

* refactor

* add test

* fix feature spec

* use address

* fix assert

* fix test

* dummy assert

* add get single tx

* add wait tx

* working waiter pooler

* add wait helper cabbage test

* test end_datetime querty

* add swagger generator spec

* use second

* fix allow constraints

* update constraints validator

* fix test

* fix test

* edit name

* add empty line'

* more fixes

* fix warning

* refactor use the elixir way

* improve test

* fix test and typo

* use cheap fn

* sort

* use any

* use any boolean return

* use Enum.all

* final credo

* format

* use default params

* update constaints test for end_datetime

* mix format

* @default_paging constant

* fix test end_datetime

* fix test and rename feature file'

* mix format

* Revert "explain analyze updates (#1569)" (#1601)

This reverts commit 3431f26.
This commit is intended to be only deployed to develop env, thus a short life commit.
Reverting this in preparation of release.

* release artifacts (#1597)

* release artifacts

Co-authored-by: Unnawut Leepaisalsuwanna <921194+unnawut@users.noreply.github.com>
Co-authored-by: Ino Murko <ino.murko@outlook.com>
Co-authored-by: Thibault <thibault@omisego.co>
Co-authored-by: Mederic <32560642+mederic-p@users.noreply.github.com>
Co-authored-by: Jarindr Thitadilaka <jarindr23@gmail.com>

* docs: v1.0.2 change logs (#1611)

* chore: bump version in VERSION file (#1613)

Co-authored-by: Unnawut Leepaisalsuwanna <921194+unnawut@users.noreply.github.com>
Co-authored-by: Ino Murko <ino.murko@outlook.com>
Co-authored-by: Thibault <thibault@omisego.co>
Co-authored-by: Mederic <32560642+mederic-p@users.noreply.github.com>
Co-authored-by: Jarindr Thitadilaka <jarindr23@gmail.com>
@unnawut unnawut added the enhancement New feature or request label Aug 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants