Skip to content
This repository was archived by the owner on May 12, 2025. It is now read-only.

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

Merged
eelanagaraj merged 2 commits into
masterfrom
eelanagaraj/fix-rewards-bug
Sep 12, 2023
Merged

Fix edge case goldToken lookup for epoch rewards at synced head#206
eelanagaraj merged 2 commits into
masterfrom
eelanagaraj/fix-rewards-bug

Conversation

@eelanagaraj

@eelanagaraj eelanagaraj commented Aug 24, 2023

Copy link
Copy Markdown
Contributor

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
@eelanagaraj eelanagaraj requested a review from palango September 4, 2023 10:04

@palango palango left a comment

Copy link
Copy Markdown
Contributor

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
@eelanagaraj eelanagaraj deleted the eelanagaraj/fix-rewards-bug branch September 12, 2023 09:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants