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.

Sign up for GitHub

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

Jump to bottom

[Utility] Local Proof of Stake Code Health Preparation #111

Closed
9 tasks
Olshansk opened this issue Jul 14, 2022 · 2 comments
Closed
9 tasks

[Utility] Local Proof of Stake Code Health Preparation #111

Olshansk opened this issue Jul 14, 2022 · 2 comments
Assignees
@Olshansk @andrewnguyen22
Labels
code health Nice to have code improvement utility Utility specific changes
Milestone
M1: Pocket PoS (P...

Comments

@Olshansk
Copy link
Member

Olshansk commented Jul 14, 2022 •
edited
Loading

Objective

Reduce the code footprint of the utility module so it is more understandble to new readers while finding a balance between minimizing legacy complexity in the future and not over-generalizing the existing code.

Origin Document

PR73 (First iteration of a PostgreSQL based Persistence Schema) underwent several iterations in order to make the persistence module implementation have the optionality to remain verbose on a per-actor basis but also avoid code duplication and redundancy.

The first iteration of the utility module did not undergo the same iterations and therefore has some low-hanging fruit to simplify the codebase before further functionality is added.

Goals / Deliverables

  • Extract common actor functionality into general interfaces
  • Reduce the code complexity and code footprint so it is more approachable and maintainable
  • Identify and document/cleanup parts of the code that are unclear to new readers
  • Generalize both unit tests and functionality where possible
  • Maintain verbose entry points for each actor
    General:
  • Update the CHANGELOG
  • Update the README
  • Update the global documentation & references
  • Document small issues / TODOs along the way

Testing Methodology

  • Utility tests: make test_utility_*
  • All tests: make test_all
  • LocalNet: works following the instructions in docs/development

Non-goals

  • Adding any new functionality

Creator: @Olshansk
Co-Owners: @andrewnguyen22
@Olshansk Olshansk added utility Utility specific changes code health Nice to have code improvement priority:medium labels Jul 14, 2022
andrewnguyen22 pushed a commit that referenced this issue Jul 22, 2022
Merge remote-tracking branch 'origin/origin/104-persistence-v1-persis…
444f6b8 
…tence-foundation-interim-merge' into issue-#111
andrewnguyen22 pushed a commit that referenced this issue Jul 22, 2022
issue-#111
61c9178
@Olshansk Olshansk moved this to In Progress in V1 Dashboard Jul 26, 2022
andrewnguyen22 pushed a commit that referenced this issue Jul 27, 2022
see pull request changes on #111
38684af
andrewnguyen22 pushed a commit that referenced this issue Jul 30, 2022
@Olshansk
Issue #111 - Cleanup Utility Module (#119)
3f42f13 
# Objective
Reduce the code footprint of the utility module, so it is more understandable to new readers while finding a balance between minimizing legacy complexity in the future and not over-generalizing the existing code.

# Origin Document

PR73 ([First iteration of a PostgreSQL based Persistence Schema](#73)) underwent several iterations in order to make the [persistence module implementation](https://github.com/pokt-network/pocket/tree/main/persistence) have the optionality to remain verbose on a per-actor basis but also avoid code duplication and redundancy.

The first iteration of the [utility module](https://github.com/pokt-network/pocket/tree/main/persistence) did not undergo the same iterations and therefore has some low-hanging fruit to simplify the codebase before further functionality is added.

# Goals / Deliverables
- [x] Extract common actor functionality into general interfaces
- [x] Reduce the code complexity and code footprint so it is more approachable and maintainable
- [ ] Identify and document/cleanup parts of the code that are unclear to new readers
- [x] Generalize both unit tests and functionality where possible
- [x] Maintain verbose entry points for each actor
General:
- [x] Update the CHANGELOG
- [x] Update the README
- [x] Update the global documentation & references
- [x] Document small issues / TODOs along the way

## Testing Methodology
 - **Utility tests**: `make test_utility_*`
 - **All tests**: `make test_all`
 - **LocalNet**: works following the instructions in [docs/development](https://github.com/pokt-network/pocket/tree/main/docs/development)

## Non-goals
- Adding any new functionality

---

## Changes Made
- Removed transaction fees from the transaction structure as fees will be enforced at the state level
- Removed actor specific messages (besides DoubleSign) and added actorType field to the struct
- Removed pause messages and functionality as it is out of scope for the current POS iteration
- Removed session and test-scoring as it's out of scope for the current POS iteration
- Consolidated unit test functionality for actors
- Modified pre-persistence to match persistence for Update(actor), 'amountToAdd' is now just 'amount'

## Future work
- There's more documentation that is needed for this to be complete. In a separate issue, I'd like to tackle connecting Utility with Persistence.
- Further cleanup / refactoring / optimizations can be made, but that can be left to future iteration of the module.

---

Co-authored-by: Daniel Olshansky <olshansky.daniel@gmail.com>
Co-authored-by: Andrew Nguyen <andrewnguyen@Andrews-MacBook-Pro-2.local>
Co-authored-by: Andrew Nguyen <andrewnguyen@Andrews-MBP-2.lan>
Co-authored-by: Daniel Olshansky <olshansky@pokt.network>
@Olshansk Olshansk self-assigned this Aug 3, 2022
@andrewnguyen22 andrewnguyen22 mentioned this issue Aug 9, 2022
Merged
16 tasks
@Olshansk
Copy link
Member Author

@andrewnguyen22 Is there any other work you were thinking of doing here?

@Olshansk Olshansk moved this from In Progress to In Review in V1 Dashboard Aug 23, 2022
@Olshansk Olshansk moved this from In Review to In Progress in V1 Dashboard Aug 27, 2022
@Olshansk
Copy link
Member Author

Olshansk commented Sep 4, 2022

Closing this out as we tended to some of the work in other PRs and the scope is too large to capture. Follow up issues will be created where necessary.

@Olshansk Olshansk closed this as completed Sep 4, 2022
Repository owner moved this from In Progress to Done in V1 Dashboard Sep 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Olshansk Olshansk

@andrewnguyen22 andrewnguyen22

Labels
code health Nice to have code improvement utility Utility specific changes
Projects
V1 Dashboard
Status: Done
Milestone
M1: Pocket PoS (Proof of Stake)
Development

No branches or pull requests
2 participants
@Olshansk @andrewnguyen22