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

[Utility] Foundational bugs, tests, code cleanup and improvements (2/3) #504

Closed
16 of 41 tasks
jessicadaugherty opened this issue Feb 13, 2023 · 0 comments · Fixed by #550
Closed
16 of 41 tasks

[Utility] Foundational bugs, tests, code cleanup and improvements (2/3) #504

jessicadaugherty opened this issue Feb 13, 2023 · 0 comments · Fixed by #550
Assignees
Labels
bug Something isn't working - expected behaviour is incorrect code health Nice to have code improvement documentation Improvements or additions to documentation

Comments

@jessicadaugherty
Copy link
Contributor

jessicadaugherty commented Feb 13, 2023

Objective

Partial refactor, cleanup, and code improvement of the utility module to set a foundation for M3.

Origin Document

Follow-up work to #475. Address the items not refactored in #475 as noted on PR #503.

When work on #443 started, while also looking at #325, it became evident that the logic in the utility module is hard to follow and understand while also being filled with minor hard-to-find bugs.

Goals

  • Unblock development: Create a foundation on which we design utility protocols and implement new features
  • Reduce cognitive overhead: Improve code readability and navigation for both internal and external contributors
  • Enable onboarding other engineers: Clarify both source code and tests through comment and source code improvements

Deliverable

  • A PR that updates the portions of the utility module noted above
  • Documentation & explanation of the changes that were made and next steps
  • Reviewing and updating the following files where appropriate:
    • ├── application.go
    • ├── application_test.go
    • ├── block.go
    • ├── block_test.go
    • ├── context.go
    • ├── doc
    • │   ├── CHANGELOG.md
    • │   ├── PROTOCOL_RELAY.md
    • │   ├── PROTOCOL_SESSION.md
    • │   └── README.md
    • ├── gov.go
    • ├── gov_test.go
    • ├── message_handler.go
    • ├── message_test.go
    • ├── module.go
    • ├── module_test.go
    • ├── service
    • │   └── service.go
    • ├── session.go
    • ├── transaction.go
    • ├── transaction_test.go
    • ├── types
    • │   ├── proto
    • │   │   ├── message.proto
    • │   │   ├── stake_status.proto
    • │   │   ├── transaction.proto
    • │   │   ├── tx_result.proto
    • │   │   └── utility_messages.proto
    • │   ├── transaction.go
    • │   ├── transaction_test.go
    • │   ├── tx_result.go
    • └── validator.go

Non-goals / Non-deliverables

  • Updating the entire module in a single PR (PR would be too large)
  • Adding new features to the utility module
  • Increasing/improving the test coverage to the po

General issue deliverables

  • Update the appropriate CHANGELOG(s)
  • Update any relevant local/global README(s)
  • Update relevant source code tree explanations
  • Add or update any relevant or supporting mermaid diagrams

Testing Methodology

  • All tests: make test_all
  • LocalNet: verify a LocalNet is still functioning correctly by following the instructions at docs/development/README.md

Creator: @jessicadaugherty
Co-owner: @Olshansk

@jessicadaugherty jessicadaugherty converted this from a draft issue Feb 13, 2023
@jessicadaugherty jessicadaugherty added bug Something isn't working - expected behaviour is incorrect documentation Improvements or additions to documentation code health Nice to have code improvement labels Feb 13, 2023
@jessicadaugherty jessicadaugherty moved this to Up Next in V1 Dashboard Feb 13, 2023
@Olshansk Olshansk moved this from Up Next to In Progress in V1 Dashboard Feb 22, 2023
@jessicadaugherty jessicadaugherty changed the title [Utility] Foundational bugs, tests, code cleanup and improvements (2/2) [Utility] Foundational bugs, tests, code cleanup and improvements (2/3) Feb 22, 2023
@Olshansk Olshansk moved this from In Progress to In Review in V1 Dashboard Feb 27, 2023
Olshansk added a commit that referenced this issue Feb 28, 2023
…3) (#550)

## Description

The second of three changes necessary to refactor and improve the utility module to enable implementation of future, more complex, interfaces.

While the list of changes in #503 was more bug & testing focused, this PR was more readability & documentation focused in preparation for M3.

## Issue

Fixes #504

## Type of change

Please mark the relevant option(s):

- [ ] New feature, functionality or library
- [x] Bug fix
- [x] Code health or cleanup
- [x] Major breaking change
- [ ] Documentation
- [ ] Other <!-- add details here if it a different type of change -->

## List of changes

- Fixed bug where we were not removing txs from the mempool of replicas
- Improved the readability of the `Block` lifecycle and `Message` validation
- Replace unnecessary functions in such as `Store()`, `getStoreAndHeight()` and a few otherwise
- Moved the `Transaction` proto to the shared module
- Added documentation to `TxResult` result and removed `DefaultTxResult` and 
- Added extensive documentation to functions involved in the Block lifecycle
- Removed unnecessary fields from the `BlockHeader`
- Renamed `GenericParam` to `ServiceURL`
- Consolidated `StakeStatus` in the shared package
- Renamed governance parametesr related to rate limitting Applications
- Renamed `applyTx` to `hydrateTx` and added documentation on its functionality

## Testing

- [x] `make develop_test`
- [x] [LocalNet](https://github.com/pokt-network/pocket/blob/main/docs/development/README.md) w/ all of the steps outlined in the `README`

## Required Checklist

- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have tested my changes using the available tooling
- [x] I have updated the corresponding CHANGELOG

### If Applicable Checklist

- [ ] I have updated the corresponding README(s); local and/or global
- [ ] I have added tests that prove my fix is effective or that my feature works
- [ ] I have added, or updated, [mermaid.js](https://mermaid-js.github.io) diagrams in the corresponding README(s)
- [ ] I have added, or updated, documentation and [mermaid.js](https://mermaid-js.github.io) diagrams in `shared/docs/*` if I updated `shared/*`README(s)

---

Co-authored-by: Alessandro De Blasis <alex@deblasis.net>
@github-project-automation github-project-automation bot moved this from In Review to Done in V1 Dashboard Feb 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working - expected behaviour is incorrect code health Nice to have code improvement documentation Improvements or additions to documentation
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants