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

runtime: remove some Emerald references #306

Merged
merged 3 commits into from
Feb 4, 2023
Merged

Conversation

pro-wh
Copy link
Collaborator

@pro-wh pro-wh commented Feb 3, 2023

constructing a runtime analyzer now takes an analyzer.Runtime parameter

cross reference #305

last (?) step of https://app.clickup.com/t/3u9y285

analyzer/runtime/runtime.go Outdated Show resolved Hide resolved
Copy link
Contributor

@mitjat mitjat left a comment

Choose a reason for hiding this comment

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

Hah I have an almost identical commit on my machine :)
Thank you!
LGTM pending the unresolved comments.

analyzer/runtime/runtime.go Outdated Show resolved Hide resolved
@mitjat
Copy link
Contributor

mitjat commented Feb 3, 2023

As per the comment https://github.com/oasisprotocol/oasis-indexer/pull/306/files#r1095643489, here are the two tests that I ran to verify that this PR basically resolves https://app.clickup.com/t/3u9y285 and makes us ready to use multiple analyzers.

In both tests, I ran reindex_and_run.sh regression test with the config posted below, once with sapphire commented out and once present.

Test 1 - config:
Both analyzers scan the same blocks

  analyzers:
    emerald:
      from: 1_003_298  # round at Damask genesis
      to: 1_003_308  # 10 blocks
    sapphire:
      from: 1_003_298  # round at Damask genesis
      to: 1_003_308  # 10 blocks

No differences in emerald queries on the resulting DB. This strongly suggests no erroneous double dead-reckoning, or similar overlap between analyzers' output.

Test 2 - config:
Analyzers overlap in half of the blocks

  analyzers:
    emerald:
      from: 1_003_298  # round at Damask genesis
      to: 1_003_318  # 20 blocks
    sapphire:
      from: 1_003_308  # emerald + 10
      to: 1_003_328  # emerald + 10

No difference in any queries, except /consensus/accounts which presents a union of all true consensus accounts and accounts encountered in runtimes (via the address_preimages table). This makes sense, as the "sapphire" analyzer scanned 10 extra blocks, which involved not-seen-before eth addresses.

@pro-wh
Copy link
Collaborator Author

pro-wh commented Feb 4, 2023

thanks for the testing!

@pro-wh pro-wh merged commit 3597069 into main Feb 4, 2023
@pro-wh pro-wh deleted the pro-wh/feature/rtparam branch February 4, 2023 00:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants