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

refactor: Simplify node client stack (remove ConsensusSourceStorage) #554

Merged
merged 9 commits into from
Nov 7, 2023

Conversation

mitjat
Copy link
Contributor

@mitjat mitjat commented Nov 2, 2023

This PR removes the ConsensusSourceStorage interface and its only implementation ConsensusClient. Both of these were mostly thin wrappers around ConsensusApiLite (with which this PR replaces them) and have been slated for removal.

ConsensusClient had methods that fell into two categories:

  • Trivial wrappers around node RPCs (which ConsensusApiLite already provides). These make for a lot of boilerplate, as every RPC needed to be added in at least 4 layers: ConsensusApiLite, its implemenation(s), ConsensusSourceStorage, and ConsensusClient. I simply rerouted these calls directly to ConsensusApiLite.
  • Data fetching+restructuring: AllData() and methods that it called in turn (StakingData, GovernanceData, etc). These methods rearrange data in small but convenient ways, e.g. they introduce new structs that encompass the height, epoch, and events-grouped-by-type, which makes analyzer code more streamlined and readable. It's a little verbose but not too bad. I simply moved these from the storage module into the consensus analyer, and made the types private (since they are inteded strictly for analyzer's internal consumption).

@mitjat mitjat changed the title refactor: Remove ConsensusSourceStorage refactor: Simplify node client stack (remove ConsensusSourceStorage) Nov 2, 2023
@mitjat mitjat force-pushed the mitjat/remove-consensus-client branch 2 times, most recently from d073e0f to 12254da Compare November 4, 2023 03:51
@mitjat mitjat force-pushed the mitjat/remove-consensus-client branch from 12254da to bf33bf7 Compare November 4, 2023 03:52
@mitjat mitjat merged commit 02d919c into main Nov 7, 2023
6 checks passed
@mitjat mitjat deleted the mitjat/remove-consensus-client branch November 7, 2023 01:49
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