[Docs] Document (non-)custodial staking #720
Conversation
… refactor/non-custodial-staking-tests
… refactor/non-custodial-staking-tests
… refactor/non-custodial-staking-tests
red-0ne
added
documentation
red-0ne
changed the base branch from
feat/non-custodial-staking
to
refactor/non-custodial-staking-tests
…ests' into docs/non-custodial-staking
## 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 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## 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. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
red-0ne
changed the base branch from
refactor/non-custodial-staking-tests
to
main
WalkthroughThis update improves the documentation and configuration for staking parameters related to Suppliers in the Pocket Network. It clarifies custodial and non-custodial staking types, enhances command usability in the Makefile, and provides detailed key configuration elements within YAML files. The introduction of clear distinctions between addresses ensures users have the necessary information for effective implementation and management of staked Suppliers. Changes
Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (3)
Files skipped from review as they are similar to previous changes (2)
Additional context usedLanguageTool
Additional comments not posted (3)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (7)
localnet/poktrolld/config/supplier1_stake_config.yaml (2)
1-8: Enhance Documentation Clarity.The documentation provides a good overview of custodial and non-custodial staking. Consider adding examples to illustrate the differences in practical scenarios.
10-14: Clarify Owner Address Role.The explanation of the
owner_addressrole is clear. Ensure that any limitations or constraints on changing the owner address are well-documented in the surrounding documentation.docusaurus/docs/operate/configs/supplier_staking_config.md (5)
49-51: Well-Structured Staking Types Section.The new section on staking types is well-structured and provides clarity. Consider providing a visual diagram to further aid understanding.
66-72: Non-Custodial Staking Explanation is Clear.The explanation of non-custodial staking is thorough. Consider adding a section on best practices for securing the operator account.
Tools
LanguageTool
[misspelling] ~69-~69: This word is normally spelled as one.
Context: ...ses and submitting claims and proofs. Non-custodial staking is suitable forSupplier`s tha...(EN_COMPOUNDS_NON_CUSTODIAL)
89-89: Punctuation Consistency.Ensure consistent punctuation usage throughout the document. The use of underscores and commas should be reviewed for consistency.
Tools
LanguageTool
[uncategorized] ~89-~89: Loose punctuation mark.
Context: ...ation ###owner_addressRequired,Non-empty```yaml owner_address: <...(UNLIKELY_OPENING_PUNCTUATION)
95-97: Clarify Owner Address Constraints.The description of the
owner_addressrole is clear. Ensure that any constraints or limitations on changing the owner address are well-documented.Tools
LanguageTool
[uncategorized] ~95-~95: Possible missing comma found.
Context: ...ccount that owns theSuppliers staked funds which will be returned to this account ...(AI_HYDRA_LEO_MISSING_COMMA)
119-119: Punctuation Consistency.Ensure consistent punctuation usage throughout the document. The use of underscores and commas should be reviewed for consistency.
Tools
LanguageTool
[uncategorized] ~119-~119: Loose punctuation mark.
Context: ...:: ###operator_addressOptional,Non-empty```yaml operator_address...(UNLIKELY_OPENING_PUNCTUATION)
| The `operator_address` is the address that identifies the `Supplier`. | ||
| Its corresponding account is used for operational tasks such as signing `RelayResponse`s, | ||
| submitting `Claims` and `Proofs` as well as updating the `Supplier`'s info | ||
| (i.e. Adding or removing services, increasing the stake amount, etc). |
There was a problem hiding this comment.
Add Period to Abbreviation.
In American English, abbreviations like “etc.” require a period.
- etc)
+ etc.)Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| (i.e. Adding or removing services, increasing the stake amount, etc). | |
| (i.e. Adding or removing services, increasing the stake amount, etc.). |
Tools
LanguageTool
[style] ~128-~128: In American English, abbreviations like “etc.” require a period.
Context: ... services, increasing the stake amount, etc). The operator account can also be use...(ETC_PERIOD)
| ## Configuration | ||
|
|
||
| ### `owner_address` | ||
|
|
||
| _`Required`_, _`Non-empty`_ |
There was a problem hiding this comment.
Remove loose punctuation marks.
The underscores around "Required" and "Non-empty" should be removed or formatted consistently.
- _`Required`_, _`Non-empty`_
+ **Required**, **Non-empty**Also applies to: 119-119
Tools
LanguageTool
[uncategorized] ~89-~89: Loose punctuation mark.
Context: ...ation ###owner_addressRequired,Non-empty```yaml owner_address: <...(UNLIKELY_OPENING_PUNCTUATION)
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (4)
docusaurus/docs/operate/configs/supplier_staking_config.md (4)
71-71: Correct spelling for "non-custodial".The term "non-custodial" should be spelled as one word.
- Non-custodial + NoncustodialAlso applies to: 103-103
Tools
LanguageTool
[misspelling] ~71-~71: This word is normally spelled as one.
Context: ...ses and submitting claims and proofs. Non-custodial staking is suitable forSupplier`s tha...(EN_COMPOUNDS_NON_CUSTODIAL)
133-140: Consider adding a mermaid diagram.A mermaid diagram could visually represent the differences between custodial and non-custodial staking, enhancing clarity.
122-122: Remove loose punctuation marks.The underscores around "Optional" and "Non-empty" should be removed or formatted consistently.
- _`Optional`_, _`Non-empty`_ + **Optional**, **Non-empty**Tools
LanguageTool
[uncategorized] ~122-~122: Loose punctuation mark.
Context: ...:: ###operator_addressOptional,Non-empty```yaml operator_address...(UNLIKELY_OPENING_PUNCTUATION)
131-131: Add period to abbreviation.In American English, abbreviations like “etc.” require a period.
- etc) + etc.)Tools
LanguageTool
[style] ~131-~131: In American English, abbreviations like “etc.” require a period.
Context: ... services, increasing the stake amount, etc). The operator account can also be use...(ETC_PERIOD)
## 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 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## 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. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary Follow-up to #716 to document custodial and non-custodial staking configuration. ## Issue  - #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 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## 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. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Summary
Follow-up to #716 to document custodial and non-custodial staking configuration.
Issue
Type of change
Select one or more:
Testing
Documentation changes (only if making doc changes)
make docusaurus_start; only needed if you make doc changesLocal Testing (only if making code changes)
make go_develop_and_testmake test_e2ePR Testing (only if making code changes)
devnet-test-e2elabel to the PR.make trigger_ciif you want to re-trigger tests without any code changesSanity Checklist
Summary by CodeRabbit
New Features
Documentation
owner_addressandoperator_address.Config Updates
owner_addressandoperator_addressparameters to multiple staking configuration files for improved clarity.Improvements