[Utility][Morse Parity] Introduce non-custodial Staking #493

Olshansk · 2024-04-25T19:18:00Z

Objective

Feature parity with non-custodial staking from Morse.

Origin Document

[Living Ticket] Prepare tickets/work that need to be completed during Alpha TestNet #437
Pocket Forum RC-0.8.0
Pocket core non custodial CLI command

Non-custodial staking command in Morse:

 pocket nodes stake
 Stake the node into the network, making it available for service.
 
 Usage:
   pocket nodes stake [command]
 
 Available Commands:
   custodial     Stake a node in the network. Custodial stake uses the same address as operator/output for rewards/return of staked funds.
   non-custodial Stake a node in the network, non-custodial stake allows a different output address for rewards/return of staked funds. The signer may be the operator or the output address. The signer must specify the public key of the operator

Note that this is a precursor for [Utility][Morse Parity] Built In Revenue Share #496

Goals

Achieve non-custodial & custodial staking feature parity w/ Morse
Use mainline Cosmos SDK best practices to implement non-custodial staking
Create a better user-interface (that's easy for anyone to use) for non-custodial staking
Prepare for reward sharing in [Utility][Morse Parity] Built In Revenue Share #496
Create a better user interface than how non-custodial staking works (see below) by allowing to set the complex configurations via a yaml file (like we do elsewhere)

Deliverables

Research Morse: Study & understand how non-custodial staking works in Morse
Research Cosmos: Study & understand what (if any) parts of the Cosmos SDK we should use for non-custodial staking
Implement: Implement non-custodial staking in Shannon
CLI: Update the poktrolld to support non-custodial staking
Test: Add unit, integration and E2E tests
Tooling: Add Makefile targets to easily trigger (on LocalNet) and test (e2e) non-custodial staking
Document: Add a new page to dev.poktroll.com explaining how non-custodial staking works and how to use it
Comments: Add/update TODOs and comments alongside the source code so it is easier to follow.

Non-goals / Non-deliverables

Expanding beyond what Morse has for non-custodial staking for Suppliers
Adding delegation or non-custodial taking for any other actors

Estimated Days of Work

3 days

Disclaimer: This is the total projected number of estimated hours to completion & merge. The owner of this tickets is expected to use this GitHub issue to communicate with the core protocol team along the way, with update & feedback for each deliverable throughout the duration of this work._

Creator: @Olshansk
Co-Owners: @moatus

The text was updated successfully, but these errors were encountered:

## Summary This PR implements non-custodial staking by * Adding a `Supplier.OwnerAddress` that represents the `Supplier` owner * Modifying `stake` and `unstake` CLI * Ensure that only the owner is able to `stake`, `unstake`, `change owner`, `change operator` * Update supplier staking config parser * Have the owner receive the rewards. *NOTE: ~950LOC is proto auto-generated code* The PR will be ready after: - [ ] Validating non-custodial logic added in this PR - [ ] Merging #718 into this one - [ ] Merging #720 into this one - [ ] Merging #722 into this one ## Issue - #493 - https://www.notion.so/buildwithgrove/Non-custodial-staking-92a136174dac41279717e8b963672e38 ## Type of change Select one or more: - [x] New feature, functionality or library - [ ] Bug fix - [ ] Code health or cleanup - [ ] Documentation - [ ] Other (specify) ## Testing **Documentation changes** (only if making doc changes) - [ ] `make docusaurus_start`; only needed if you make doc changes **Local Testing** (only if making code changes) - [ ] **Unit Tests**: `make go_develop_and_test` - [ ] **LocalNet E2E Tests**: `make test_e2e` - See [quickstart guide](https://dev.poktroll.com/developer_guide/quickstart) for instructions **PR Testing** (only if making code changes) - [ ] **DevNet E2E Tests**: Add the `devnet-test-e2e` label to the PR. - **THIS IS VERY EXPENSIVE**, so only do it after all the reviews are complete. - Optionally run `make trigger_ci` if you want to re-trigger tests without any code changes - If tests fail, try re-running failed tests only using the GitHub UI as shown [here](https://github.com/pokt-network/poktroll/assets/1892194/607984e9-0615-4569-9452-4c730190c1d2) ## Sanity Checklist - [ ] I have tested my changes using the available tooling - [ ] I have commented my code - [ ] I have performed a self-review of my own code; both comments & source code - [ ] I create and reference any new tickets, if applicable - [ ] I have left TODOs throughout the codebase, if applicable  ## Summary by CodeRabbit - **New Features** - Introduced a new field `OwnerAddress` in the `Supplier` struct for improved ownership management. - Enhanced supplier configuration structures with `OwnerAddress` and `OperatorAddress` fields for better role management. - Added new validation methods to ensure correct ownership and operator address integrity. - **Bug Fixes** - Improved validation logic to ensure only authorized users can modify supplier details. - **Documentation** - Clarified comments in code to improve understanding of address management and supplier roles. - **Chores** - Added new error constants to enhance error handling and user feedback for supplier-related operations.

## Summary This PR is a follow-up to #716. * It refactors the existing unit/integration tests to add `Supplier.OwnerAddress` * Adapts the `Supplier` (un)staking and tests (staking config, (un)staking CLI and keeper methods) to comply with the `Supplier` non-custodial staking. ## Issue - #493 ## Type of change Select one or more: - [ ] New feature, functionality or library - [ ] Bug fix - [x] Code health or cleanup - [ ] Documentation - [ ] Other (specify) ## Testing **Documentation changes** (only if making doc changes) - [ ] `make docusaurus_start`; only needed if you make doc changes **Local Testing** (only if making code changes) - [x] **Unit Tests**: `make go_develop_and_test` - [ ] **LocalNet E2E Tests**: `make test_e2e` - See [quickstart guide](https://dev.poktroll.com/developer_guide/quickstart) for instructions **PR Testing** (only if making code changes) - [ ] **DevNet E2E Tests**: Add the `devnet-test-e2e` label to the PR. - **THIS IS VERY EXPENSIVE**, so only do it after all the reviews are complete. - Optionally run `make trigger_ci` if you want to re-trigger tests without any code changes - If tests fail, try re-running failed tests only using the GitHub UI as shown [here](https://github.com/pokt-network/poktroll/assets/1892194/607984e9-0615-4569-9452-4c730190c1d2) ## Sanity Checklist - [ ] I have tested my changes using the available tooling - [ ] I have commented my code - [ ] I have performed a self-review of my own code; both comments & source code - [ ] I create and reference any new tickets, if applicable - [ ] I have left TODOs throughout the codebase, if applicable  ## Summary by CodeRabbit - **New Features** - Enhanced readability of output messages in the Makefile. - Added `OwnerAddress` to the `Supplier` data model for better ownership tracking. - Introduced support for both owner and operator addresses in various staking and unstaking processes. - **Bug Fixes** - Updated tests for `MsgUnstakeSupplier` to correctly validate signer addresses and enhance robustness. - **Documentation** - Improved test coverage and clarity for supplier-related functionality, reflecting the new data model changes.

## Summary This PR renames all the `Supplier.Address` references to `Supplier.OperatorAddress`. It also include renaming `supplierAddress` to `supplierOperatorAddress` variables, methods in `Claim` and `Proof`, comments and error messages. This PR has a lot of changes but does not introduce any addition logic to the code base. ## Issue - #493 ## Type of change Select one or more: - [ ] New feature, functionality or library - [ ] Bug fix - [x] Code health or cleanup - [ ] Documentation - [ ] Other (specify) ## Testing **Documentation changes** (only if making doc changes) - [ ] `make docusaurus_start`; only needed if you make doc changes **Local Testing** (only if making code changes) - [x] **Unit Tests**: `make go_develop_and_test` - [ ] **LocalNet E2E Tests**: `make test_e2e` - See [quickstart guide](https://dev.poktroll.com/developer_guide/quickstart) for instructions **PR Testing** (only if making code changes) - [ ] **DevNet E2E Tests**: Add the `devnet-test-e2e` label to the PR. - **THIS IS VERY EXPENSIVE**, so only do it after all the reviews are complete. - Optionally run `make trigger_ci` if you want to re-trigger tests without any code changes - If tests fail, try re-running failed tests only using the GitHub UI as shown [here](https://github.com/pokt-network/poktroll/assets/1892194/607984e9-0615-4569-9452-4c730190c1d2) ## Sanity Checklist - [x] I have tested my changes using the available tooling - [ ] I have commented my code - [ ] I have performed a self-review of my own code; both comments & source code - [ ] I create and reference any new tickets, if applicable - [ ] I have left TODOs throughout the codebase, if applicable

## Summary *This PR is duplicate of #722 which got mistakingly merged into `refactor/non-custodial-staking-tests`* This PR renames all the Supplier.Address references to Supplier.OperatorAddress. It also include renaming supplierAddress to supplierOperatorAddress variables, methods in Claim and Proof, comments and error messages. This PR has a lot of changes but does not introduce any addition logic to the code base. ## Issue - #493 ## Type of change Select one or more: - [ ] New feature, functionality or library - [ ] Bug fix - [x] Code health or cleanup - [ ] Documentation - [ ] Other (specify) ## Testing **Documentation changes** (only if making doc changes) - [ ] `make docusaurus_start`; only needed if you make doc changes **Local Testing** (only if making code changes) - [x] **Unit Tests**: `make go_develop_and_test` - [x] **LocalNet E2E Tests**: `make test_e2e` - See [quickstart guide](https://dev.poktroll.com/developer_guide/quickstart) for instructions **PR Testing** (only if making code changes) - [ ] **DevNet E2E Tests**: Add the `devnet-test-e2e` label to the PR. - **THIS IS VERY EXPENSIVE**, so only do it after all the reviews are complete. - Optionally run `make trigger_ci` if you want to re-trigger tests without any code changes - If tests fail, try re-running failed tests only using the GitHub UI as shown [here](https://github.com/pokt-network/poktroll/assets/1892194/607984e9-0615-4569-9452-4c730190c1d2) ## Sanity Checklist - [x] I have tested my changes using the available tooling - [ ] I have commented my code - [x] I have performed a self-review of my own code; both comments & source code - [ ] I create and reference any new tickets, if applicable - [ ] I have left TODOs throughout the codebase, if applicable

## Summary Follow-up to #716 to document custodial and non-custodial staking configuration. ## Issue ![image](https://github.com/user-attachments/assets/671278e9-911f-43d9-b7eb-e7994b959b72) - #493 ## Type of change Select one or more: - [ ] New feature, functionality or library - [ ] Bug fix - [ ] Code health or cleanup - [x] Documentation - [ ] Other (specify) ## Testing **Documentation changes** (only if making doc changes) - [x] `make docusaurus_start`; only needed if you make doc changes **Local Testing** (only if making code changes) - [ ] **Unit Tests**: `make go_develop_and_test` - [ ] **LocalNet E2E Tests**: `make test_e2e` - See [quickstart guide](https://dev.poktroll.com/developer_guide/quickstart) for instructions **PR Testing** (only if making code changes) - [ ] **DevNet E2E Tests**: Add the `devnet-test-e2e` label to the PR. - **THIS IS VERY EXPENSIVE**, so only do it after all the reviews are complete. - Optionally run `make trigger_ci` if you want to re-trigger tests without any code changes - If tests fail, try re-running failed tests only using the GitHub UI as shown [here](https://github.com/pokt-network/poktroll/assets/1892194/607984e9-0615-4569-9452-4c730190c1d2) ## Sanity Checklist - [ ] I have tested my changes using the available tooling - [ ] I have commented my code - [ ] I have performed a self-review of my own code; both comments & source code - [ ] I create and reference any new tickets, if applicable - [ ] I have left TODOs throughout the codebase, if applicable  ## Summary by CodeRabbit - **New Features** - Enhanced command for supplier unstake, improving usability and flexibility. - Introduced new sections in documentation for "Staking types," detailing Custodial and Non-Custodial Staking. - **Documentation** - Expanded configuration documentation with detailed explanations of `owner_address` and `operator_address`. - **Config Updates** - Added `owner_address` and `operator_address` parameters to multiple staking configuration files for improved clarity. - **Improvements** - Streamlined configuration files by removing comments, enhancing clarity on staking roles.

## Summary This PR implements non-custodial staking by * Adding a `Supplier.OwnerAddress` that represents the `Supplier` owner * Modifying `stake` and `unstake` CLI * Ensure that only the owner is able to `stake`, `unstake`, `change owner`, `change operator` * Update supplier staking config parser * Have the owner receive the rewards. *NOTE: ~950LOC is proto auto-generated code* The PR will be ready after: - [ ] Validating non-custodial logic added in this PR - [ ] Merging #718 into this one - [ ] Merging #720 into this one - [ ] Merging #722 into this one ## Issue - #493 - https://www.notion.so/buildwithgrove/Non-custodial-staking-92a136174dac41279717e8b963672e38 ## Type of change Select one or more: - [x] New feature, functionality or library - [ ] Bug fix - [ ] Code health or cleanup - [ ] Documentation - [ ] Other (specify) ## Testing **Documentation changes** (only if making doc changes) - [ ] `make docusaurus_start`; only needed if you make doc changes **Local Testing** (only if making code changes) - [ ] **Unit Tests**: `make go_develop_and_test` - [ ] **LocalNet E2E Tests**: `make test_e2e` - See [quickstart guide](https://dev.poktroll.com/developer_guide/quickstart) for instructions **PR Testing** (only if making code changes) - [ ] **DevNet E2E Tests**: Add the `devnet-test-e2e` label to the PR. - **THIS IS VERY EXPENSIVE**, so only do it after all the reviews are complete. - Optionally run `make trigger_ci` if you want to re-trigger tests without any code changes - If tests fail, try re-running failed tests only using the GitHub UI as shown [here](https://github.com/pokt-network/poktroll/assets/1892194/607984e9-0615-4569-9452-4c730190c1d2) ## Sanity Checklist - [ ] I have tested my changes using the available tooling - [ ] I have commented my code - [ ] I have performed a self-review of my own code; both comments & source code - [ ] I create and reference any new tickets, if applicable - [ ] I have left TODOs throughout the codebase, if applicable  ## Summary by CodeRabbit - **New Features** - Introduced a new field `OwnerAddress` in the `Supplier` struct for improved ownership management. - Enhanced supplier configuration structures with `OwnerAddress` and `OperatorAddress` fields for better role management. - Added new validation methods to ensure correct ownership and operator address integrity. - **Bug Fixes** - Improved validation logic to ensure only authorized users can modify supplier details. - **Documentation** - Clarified comments in code to improve understanding of address management and supplier roles. - **Chores** - Added new error constants to enhance error handling and user feedback for supplier-related operations.

## Summary This PR is a follow-up to #716. * It refactors the existing unit/integration tests to add `Supplier.OwnerAddress` * Adapts the `Supplier` (un)staking and tests (staking config, (un)staking CLI and keeper methods) to comply with the `Supplier` non-custodial staking. ## Issue - #493 ## Type of change Select one or more: - [ ] New feature, functionality or library - [ ] Bug fix - [x] Code health or cleanup - [ ] Documentation - [ ] Other (specify) ## Testing **Documentation changes** (only if making doc changes) - [ ] `make docusaurus_start`; only needed if you make doc changes **Local Testing** (only if making code changes) - [x] **Unit Tests**: `make go_develop_and_test` - [ ] **LocalNet E2E Tests**: `make test_e2e` - See [quickstart guide](https://dev.poktroll.com/developer_guide/quickstart) for instructions **PR Testing** (only if making code changes) - [ ] **DevNet E2E Tests**: Add the `devnet-test-e2e` label to the PR. - **THIS IS VERY EXPENSIVE**, so only do it after all the reviews are complete. - Optionally run `make trigger_ci` if you want to re-trigger tests without any code changes - If tests fail, try re-running failed tests only using the GitHub UI as shown [here](https://github.com/pokt-network/poktroll/assets/1892194/607984e9-0615-4569-9452-4c730190c1d2) ## Sanity Checklist - [ ] I have tested my changes using the available tooling - [ ] I have commented my code - [ ] I have performed a self-review of my own code; both comments & source code - [ ] I create and reference any new tickets, if applicable - [ ] I have left TODOs throughout the codebase, if applicable  ## Summary by CodeRabbit - **New Features** - Enhanced readability of output messages in the Makefile. - Added `OwnerAddress` to the `Supplier` data model for better ownership tracking. - Introduced support for both owner and operator addresses in various staking and unstaking processes. - **Bug Fixes** - Updated tests for `MsgUnstakeSupplier` to correctly validate signer addresses and enhance robustness. - **Documentation** - Improved test coverage and clarity for supplier-related functionality, reflecting the new data model changes.

## Summary *This PR is duplicate of #722 which got mistakingly merged into `refactor/non-custodial-staking-tests`* This PR renames all the Supplier.Address references to Supplier.OperatorAddress. It also include renaming supplierAddress to supplierOperatorAddress variables, methods in Claim and Proof, comments and error messages. This PR has a lot of changes but does not introduce any addition logic to the code base. ## Issue - #493 ## Type of change Select one or more: - [ ] New feature, functionality or library - [ ] Bug fix - [x] Code health or cleanup - [ ] Documentation - [ ] Other (specify) ## Testing **Documentation changes** (only if making doc changes) - [ ] `make docusaurus_start`; only needed if you make doc changes **Local Testing** (only if making code changes) - [x] **Unit Tests**: `make go_develop_and_test` - [x] **LocalNet E2E Tests**: `make test_e2e` - See [quickstart guide](https://dev.poktroll.com/developer_guide/quickstart) for instructions **PR Testing** (only if making code changes) - [ ] **DevNet E2E Tests**: Add the `devnet-test-e2e` label to the PR. - **THIS IS VERY EXPENSIVE**, so only do it after all the reviews are complete. - Optionally run `make trigger_ci` if you want to re-trigger tests without any code changes - If tests fail, try re-running failed tests only using the GitHub UI as shown [here](https://github.com/pokt-network/poktroll/assets/1892194/607984e9-0615-4569-9452-4c730190c1d2) ## Sanity Checklist - [x] I have tested my changes using the available tooling - [ ] I have commented my code - [x] I have performed a self-review of my own code; both comments & source code - [ ] I create and reference any new tickets, if applicable - [ ] I have left TODOs throughout the codebase, if applicable

## Summary Follow-up to #716 to document custodial and non-custodial staking configuration. ## Issue ![image](https://github.com/user-attachments/assets/671278e9-911f-43d9-b7eb-e7994b959b72) - #493 ## Type of change Select one or more: - [ ] New feature, functionality or library - [ ] Bug fix - [ ] Code health or cleanup - [x] Documentation - [ ] Other (specify) ## Testing **Documentation changes** (only if making doc changes) - [x] `make docusaurus_start`; only needed if you make doc changes **Local Testing** (only if making code changes) - [ ] **Unit Tests**: `make go_develop_and_test` - [ ] **LocalNet E2E Tests**: `make test_e2e` - See [quickstart guide](https://dev.poktroll.com/developer_guide/quickstart) for instructions **PR Testing** (only if making code changes) - [ ] **DevNet E2E Tests**: Add the `devnet-test-e2e` label to the PR. - **THIS IS VERY EXPENSIVE**, so only do it after all the reviews are complete. - Optionally run `make trigger_ci` if you want to re-trigger tests without any code changes - If tests fail, try re-running failed tests only using the GitHub UI as shown [here](https://github.com/pokt-network/poktroll/assets/1892194/607984e9-0615-4569-9452-4c730190c1d2) ## Sanity Checklist - [ ] I have tested my changes using the available tooling - [ ] I have commented my code - [ ] I have performed a self-review of my own code; both comments & source code - [ ] I create and reference any new tickets, if applicable - [ ] I have left TODOs throughout the codebase, if applicable  ## Summary by CodeRabbit - **New Features** - Enhanced command for supplier unstake, improving usability and flexibility. - Introduced new sections in documentation for "Staking types," detailing Custodial and Non-Custodial Staking. - **Documentation** - Expanded configuration documentation with detailed explanations of `owner_address` and `operator_address`. - **Config Updates** - Added `owner_address` and `operator_address` parameters to multiple staking configuration files for improved clarity. - **Improvements** - Streamlined configuration files by removing comments, enhancing clarity on staking roles.

Olshansk added protocol General core protocol related changes community A ticket intended to potentially be picked up by a community member utility labels Apr 25, 2024

Olshansk added this to the Shannon MainNet milestone Apr 25, 2024

Olshansk added this to Shannon Apr 25, 2024

Olshansk mentioned this issue Apr 25, 2024

[Living Ticket] Prepare tickets/work that need to be completed during Alpha TestNet #437

Closed

36 tasks

Olshansk moved this to 🔖 Ready in Shannon Jul 3, 2024

Olshansk assigned bryanchriswhite Jul 4, 2024

Olshansk removed the community A ticket intended to potentially be picked up by a community member label Jul 4, 2024

Olshansk changed the title ~~[Utility] Introduce non-custodial Staking~~ [Utility][Morse Parity] Introduce non-custodial Staking Jul 4, 2024

Olshansk assigned red-0ne and unassigned bryanchriswhite Jul 25, 2024

red-0ne moved this from 🔖 Ready to 🏗 In progress in Shannon Jul 29, 2024

This was referenced Jul 30, 2024

[Supplier] Implement non-custodial staking #716

Merged

[Supplier] Refactor/non custodial staking tests #718

Merged

[Docs] Document (non-)custodial staking #720

Merged

[Supplier] Rename Supplier.Address to Supplier.OperatorAddress #722

Merged

Olshansk modified the milestones: Shannon MainNet, Shannon Beta TestNet Launch Aug 9, 2024

red-0ne moved this from 🏗 In progress to 👀 In review in Shannon Aug 12, 2024

red-0ne closed this as completed Aug 13, 2024

github-project-automation bot moved this from 👀 In review to ✅ Done in Shannon Aug 13, 2024

red-0ne mentioned this issue Aug 13, 2024

[Supplier] Rename Supplier.Address to Supplier.OperatorAddress #734

Merged

14 tasks

red-0ne mentioned this issue Aug 14, 2024

chore: Use updated poktroll types pokt-network/shannon-sdk#21

Merged

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Utility][Morse Parity] Introduce non-custodial Staking #493

[Utility][Morse Parity] Introduce non-custodial Staking #493

Olshansk commented Apr 25, 2024 •

edited

Loading

[Utility][Morse Parity] Introduce non-custodial Staking #493

[Utility][Morse Parity] Introduce non-custodial Staking #493

Comments

Olshansk commented Apr 25, 2024 • edited Loading

Objective

Origin Document

Goals

Deliverables

Non-goals / Non-deliverables

Estimated Days of Work

Olshansk commented Apr 25, 2024 •

edited

Loading