Implement MVC E2E Feature Path F.1-4: E2E Feature Flags

0xBigBoss commented 1 year ago

Objective: Implement MVC E2E Feature Path F.1-4: E2E Feature Flags

Origin Document:

Purpose: Implement the governance-controlled parameters with the ability to add new features, enable, disable, and modify feature values dynamically.

Actors:

[x] Validator
[ ] Application
[ ] Servicer
[ ] Fisherman
[ ] Portal

Data Structures:

message MessageChangeGovParameter {
  bytes signer = 1; // signer must be the owner of the parameter or the ACL owner
  int64 height = 2;
  string param_name = 3;
  google.protobuf.Any param_value = 4;
}

Other structs to be identified during the POC or MVC stages.

Interfaces:

The persistence module currently has feature flags as part of it's interface.

// Flags
 GetIntFlag(paramName string, height int64) (int, bool, error)
 GetStringFlag(paramName string, height int64) (string, bool, error)
 GetBytesFlag(paramName string, height int64) ([]byte, bool, error)

There should be a simple boolean type flag included in this interface to allow for simple feature flags to be activated or deactivated.

The MessageChangeGovParameter message type needs to be handled in the utility module.

https://github.com/pokt-network/pocket/blob/12f9f498c8b05a91f4c08c6b27227d94013d9232/utility/unit_of_work/tx_message_handler.go

Other interfaces to be identified during the POC or MVC stages.

Diagram:

sequenceDiagram
    participant A as ACL Owner
    participant S as System
    A->>+S: Send request to change a gov. parameter
    Note over A,S: Request includes valid signer, height, param_name, and param_value
    S->>S: Validates the request
    S->>-A: Acknowledges the request
    Note over S: New parameter takes effect at specified height
    A->>+S: Request to see the new parameter value
    S->>-A: Returns the new parameter value

User Stories as Tests:

Happy Paths:

An ACL Owner requests to change a government parameter with a valid signer, a height, a param_name, and a param_value. The new parameter takes effect at the specified height. The ACL Owner is able to see the new value when requested.

Sad Path:

An ACL Owner requests to change a government parameter with an invalid signer, a valid height, a param_name, and a param_value. The ACL Owner receives an error message saying the signer is not the owner of the parameter or the ACL owner.
An ACL Owner requests to change a government parameter with a valid signer, an invalid height, a param_name, and a param_value. The ACL Owner receives an error message saying the height is not valid.
An ACL Owner requests to change a government parameter with a valid signer, a valid height, an invalid param_name, and a param_value. The ACL Owner receives an error message saying the parameter name does not exist.
An ACL Owner requests to change a government parameter with a valid signer, a valid height, a valid param_name, and an invalid param_value. The ACL Owner receives an error message saying the parameter value is not valid.

Deliverable

POC:
- [ ] A POC SPIKE to be closed out and split out into multiple PRs
MVP:
- [ ] A PR that adds or modifies relevant structures and interfaces; such as shared/core/types/proto, shared/modules, etc
- [ ] A PR that materializes an MVP of the feature along with unit tests
- [ ] A PR that introduces a new E2E tests with one happy and one sad path scenarios as described in the origin document (refer to e2e/README.md); this may require additions to the cli
- [ ] A PR that updates all pertinent documentation
PROD:
- [ ] One or more subsequent GitHub issues that track future work including, but not limited to:
- Enhancing test coverage
- Adding subsequent features
- Patching hacks or workarounds
- Enabling your imagination!

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

[ ] Task specific tests or benchmarks: make ...
[ ] New tests or benchmarks: make ...
[ ] All tests: make test_all
[ ] LocalNet: verify a LocalNet is still functioning correctly by following the instructions at docs/development/README.md
[ ] k8s LocalNet: verify a k8s LocalNet is still functioning correctly by following the instructions here

Creator: @0xBigBoss Co-owner: @Olshansk

Olshansk commented 1 year ago

LGTM. Scope and details are 🔥.

Made some small inline nits to the description but otherwise LGTM

Olshansk commented 1 year ago

@0xBigBoss Wanted to check in if there are any updates or progress on this?

Olshansk commented 1 year ago

@0xBigBoss Wanted to check in if there are any updates or progress on this?

Do you have an estimate on when you might start working on this?

Trying to plan ahead and carve out time for review in the coming weeks.

0xBigBoss commented 1 year ago

@0xBigBoss Wanted to check in if there are any updates or progress on this?

Do you have an estimate on when you might start working on this?

Trying to plan ahead and carve out time for review in the coming weeks.

Hey @Olshansk most likely not until next week, I am trying to outline the scope of this ticket while I am focusing on upgrades.

pokt-network / pocket