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

Syncer/Reconciler Refactor #15

Merged
merged 31 commits into from
May 8, 2020
Merged

Syncer/Reconciler Refactor #15

merged 31 commits into from
May 8, 2020

Conversation

patrick-ogrady
Copy link
Contributor

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

Problems

  • Multiple people expressed interest in using the syncer and reconciler packages in their own applications, however, these packages were tightly coupled to the other packages in rosetta-cli.
  • In the last few weeks, significant tech debt was created to support running validation from an arbitrary start index without needing to sync from scratch. This lead to tons of duplicate logic in both the reconciler and syncer in the stateful_x and stateless_x implementations.
  • Without unified logic, 2 commands were created to access the stateful (check:complete) and stateless syncer (check:quick). Having 2 commands to check validity was extremely confusing for new users.
  • For blockchains that had genesis allocations, it was necessary to generate a bootstrap balances file to set balances before beginning syncing. This was a cumbersome tool to build for many Node API implementers.

Solutions

  • Remove all internal dependencies from syncer and reconciler to prepare for a migration to rosetta-sdk-go. It is now possible to provide your own storage and communication implementation.
  • Abstract logic in syncer and reconciler to work regardless of whether starting from genesis or some arbitrary height. This made the implementation much simpler and removed plenty of redundant code.
  • Create new check command that determines how to prepare storage to sync from any index. If you start from genesis, it will perform a comprehensive check. If you start from an arbitrary height, limited checks will be run.
  • Storage will automatically fetch the balance of an account at the parent block before processing a block containing the account if it is not found. This means it is only necessary to create a bootstrap accounts file if it is not possible to lookup balance by block.

Future PR

  • Move syncer and reconciler packages to rosetta-sdk-go
  • Restore the inactive checking queue from a previous instantiation

@coveralls
Copy link

coveralls commented May 6, 2020

Pull Request Test Coverage Report for Build 885

  • 284 of 686 (41.4%) changed or added relevant lines in 3 files are covered.
  • 11 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+4.2%) to 51.414%

Changes Missing Coverage Covered Lines Changed/Added Lines %
internal/storage/block_storage.go 166 240 69.17%
internal/syncer/syncer.go 42 125 33.6%
internal/reconciler/reconciler.go 76 321 23.68%
Files with Coverage Reduction New Missed Lines %
internal/storage/block_storage.go 3 73.77%
internal/syncer/syncer.go 8 32.19%
Totals Coverage Status
Change from base Build 578: 4.2%
Covered Lines: 509
Relevant Lines: 990

💛 - Coveralls

@patrick-ogrady patrick-ogrady force-pushed the syncer-refactor branch 2 times, most recently from 344b410 to f832824 Compare May 7, 2020 16:01
@patrick-ogrady patrick-ogrady changed the title [WIP] Syncer/Reconciler refactor Syncer/Reconciler Refactor May 7, 2020
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
@patrick-ogrady patrick-ogrady force-pushed the syncer-refactor branch 2 times, most recently from 2c68956 to 665e677 Compare May 7, 2020 23:19
@patrick-ogrady patrick-ogrady merged commit aabdb99 into master May 8, 2020
@patrick-ogrady patrick-ogrady deleted the syncer-refactor branch May 8, 2020 15:07
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.

4 participants