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

Fix edge case goldToken lookup for epoch rewards at synced head #206

Merged
merged 2 commits into from
Sep 12, 2023

Conversation

eelanagaraj
Copy link
Contributor

@eelanagaraj eelanagaraj commented Aug 24, 2023

Description

Fixes a bug that occurs when the block/transaction endpoint is hit for the epoch rewards tx in an epoch block that is at the synced tip of Rosetta’s node. What happens:

  • the monitor service subscribes to block headers and only processes + updates the DB only once it receives the header
    block/transaction tries to compute epoch rewards for the last tx in the epoch block, which requires some information that is processed by the monitor service (which updates rosetta’s internal DB)
  • the endpoint tries to query the goldToken contract from the start of the following block (since this is assumed the safest correct address to filter contract logs on to get the transfer events, since the contract address wouldn’t change in between the last tx + start of the next block)
  • the bug occurs bc the next block hasn’t yet been received/processed, so errors on fetching the contract address. Retrying the request after the followup block has been received is totally fine, so the bug doesn’t seem tooo critical.
  • we see this very consistently when running tests on mycelo networks with shorter epochs, and where it’s possible to really sync to the tip pretty quickly

The fix for this in this PR just looks up the contract address from the tx index of the epoch block. Since this is the last tx in the block, there should not be any other txs that change the goldToken contract address between this and the start of the next block (old solution).

Tested

  • reconciliation checks locally with a mycelo testnet
  • reconciliation checks locally for several epochs for alfajores
  • plan: include this in the next rosetta tests run in k8s -- if this causes unexpected new issues, this does not need to be included in the present release, since this is a small edge case that will succeed on retries anyways, so not a huge issue.
    • so far past espresso HF

@eelanagaraj eelanagaraj marked this pull request as ready for review September 4, 2023 10:03
Copy link
Contributor

@palango palango left a comment

Choose a reason for hiding this comment

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

Not relying on the next block looks like the cleaner solution!

@eelanagaraj eelanagaraj merged commit cfcefa0 into master Sep 12, 2023
4 checks passed
@eelanagaraj eelanagaraj deleted the eelanagaraj/fix-rewards-bug branch September 12, 2023 09:45
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