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

Initialize Reconciler with Previously Seen Addresses #26

Merged
merged 4 commits into from
May 13, 2020

Conversation

patrick-ogrady
Copy link
Contributor

@patrick-ogrady patrick-ogrady commented May 13, 2020

Motivation

When restarting the Reconciler, the record of previously seen accounts and the inactive reconciliation queue is discarded. Upon restart of the reconciler, there is no way to repopulate this record of previously seen AccountCurrency. In the rosetta-cli, this can lead to inactive reconciliation not checking all accounts if reconciliation is restarted using the same datastore (common practice).

Solution

  • Allow the Reconciler to be instantiated with previously seen accounts.
  • Add a method NewAccountSeen to the Reconciler.Handler that is called whenever a new account is observed. This allows the client to store these new accounts persistently (to later provide for reconciler instantiation).

@coveralls
Copy link

coveralls commented May 13, 2020

Pull Request Test Coverage Report for Build 1214

  • 56 of 70 (80.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+2.07%) to 68.318%

Changes Missing Coverage Covered Lines Changed/Added Lines %
reconciler/reconciler.go 28 42 66.67%
Totals Coverage Status
Change from base Build 1151: 2.07%
Covered Lines: 1007
Relevant Lines: 1474

💛 - Coveralls

@patrick-ogrady patrick-ogrady force-pushed the patrick/init-seen branch 2 times, most recently from b2bd661 to e4dbfe0 Compare May 13, 2020 02:14
niallo
niallo previously approved these changes May 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants