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 commission rate data corruption #144

Merged
12 commits merged into from
Nov 29, 2023
Merged

Fix commission rate data corruption #144

12 commits merged into from
Nov 29, 2023

Conversation

ghost
Copy link

@ghost ghost commented Nov 27, 2023

Purpose

Due to an earlier bug validators commissions rates wasn’t adjusted on chain parameter updates. This was fixed in PR
https://github.com/Concordium/concordium-scan/pull/136/files#

Corrupted data was however not fixed which is handled in this PR.

Error is now visible in the CCD scan on stagenet since we now compare the actual payday commission rates with the latest change received from the node.
https://github.com/Concordium/concordium-scan/pull/139/files
#143

In cases where the latest commission rate change was due to a chain update, the effect has never been updated in the database and indicators are wrongly shown.

Changes

  • Add migration job flow to main import job
  • Add migration job which updated commission rates
  • Add generic metric for block height and added to main process such that we can observe if any job blocks import. Also migrated contract read height to this metric.
  • Removed custom Source Class Name enricher and use the default from Serilog. The custom wasn’t printing classes with generics correctly.

Checklist

  • My code follows the style of this project.
  • The code compiles without warnings.
  • I have performed a self-review of the changes.
  • I have documented my code, in particular the intent of the
    hard-to-understand areas.
  • (If necessary) I have updated the CHANGELOG.

@ghost ghost requested a review from orhoj November 29, 2023 11:11
Copy link
Contributor

@orhoj orhoj left a comment

Choose a reason for hiding this comment

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

Going forward it would be nice to get a change like this split into two PR's:

  1. A renaming refactoring PR. No functionality is affected, but all the interface changes etc. are made.
  2. The actual migration script PR.

I know you might have determined the need for the refactoring while doing the migration, but the above would make it easier to review.

@ghost ghost merged commit 109dc25 into main Nov 29, 2023
@ghost ghost deleted the cbw-1484/fix-data-corruption branch November 29, 2023 16:27
This pull request was closed.
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.

1 participant