F/add stake grants #13
Conversation
WalkthroughThe pull request introduces several updates across various documentation files related to the exchange module. Key changes include the addition of new sections and types that clarify the mechanics of staking, fee discounts, and grant authorizations. New message types and event definitions have been introduced to enhance functionality and consistency in event handling. Additionally, the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Exchange
participant Grantor
participant Grantee
User->>Exchange: Request Fee Discount
Exchange->>Grantor: Authorize Grant
Grantor->>Exchange: Confirm Grant Authorization
Exchange->>Grantee: Activate Stake Grant
Grantee->>Exchange: Use Fee Discount
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 using PR comments)
Other keywords and placeholders
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 and nitpick comments (9)
docs/develop/modules/injective/exchange/02_other_concepts.md (1)
71-73: Excellent addition to the documentation!The new "Stake Delegations/Grants" section provides valuable information on how staked INJ requirements for fee discount tiers can be met through grants. The explanation is clear, concise, and relevant to the "Fee Discounts" section.
Consider adding a comma after "grants" in the last sentence for improved readability:
-Granted stake cannot be regranted. +Granted stake cannot be regranted, and grants cannot be chained.This minor change enhances clarity by emphasizing that both regranting and chaining of grants are not allowed.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~73-~73: Possible missing comma found.
Context: ...nts, stake with 25 or fewer validators. Granted stake cannot be regranted.(AI_HYDRA_LEO_MISSING_COMMA)
docs/develop/modules/injective/exchange/05_messages.md (3)
442-445: LGTM! Consider a minor enhancement.The clarification for the
Amountfield improves the documentation. To further enhance clarity, consider adding an example value or range for theAmountfield.
444-460: LGTM! Consider adding an example.The introduction of
MsgAuthorizeStakeGrantsis well-documented and aligns with the PR objectives. To enhance understanding, consider adding a simple example of how to use this message type, particularly for granting and revoking.🧰 Tools
🪛 Markdownlint
451-451: Column: 1
Hard tabs(MD010, no-hard-tabs)
452-452: Column: 1
Hard tabs(MD010, no-hard-tabs)
456-456: null
Emphasis used instead of a heading(MD036, no-emphasis-as-heading)
456-456: Consider using headings for field descriptions.For consistency and improved structure, consider using level 3 headings (###) for "Fields description" sections instead of emphasis (**). This change would align with Markdown best practices and improve the overall document structure.
Also applies to: 474-474
🧰 Tools
🪛 Markdownlint
456-456: null
Emphasis used instead of a heading(MD036, no-emphasis-as-heading)
docs/develop/modules/injective/exchange/03_state.md (2)
622-629: LGTM: EffectiveGrant structure is well-defined, but documentation could be improved.The
EffectiveGrantstructure is correctly implemented with appropriate field types. However, the documentation could be more detailed to explain the purpose of theIsValidfield and how it's used.Consider expanding the documentation to include:
- The purpose of the
IsValidfield.- How the
NetGrantedStakeis calculated or updated.- Any conditions that might cause the grant to become invalid.
595-631: Great addition to the exchange module documentation!The new structures (
GrantAuthorization,ActiveGrant, andEffectiveGrant) effectively enhance the documentation related to stake grants for trading fee discounts. These additions are well-integrated into the existing documentation and provide a clear mechanism for managing stake grants.To further improve the documentation:
- Consider adding a brief introduction to the concept of stake grants and how they relate to trading fee discounts at the beginning of this new section.
- It might be helpful to include a small example or use case to illustrate how these structures work together in practice.
Overall, these changes significantly improve the clarity and comprehensiveness of the exchange module documentation.
docs/develop/modules/injective/exchange/04_state_transitions.md (3)
37-39: Consider improving list formatting for consistency.The new additions to the list of state transitions are valuable. However, to maintain consistency with the rest of the list, consider using hyphens (-) instead of bullet points (•) for these new items.
452-460: LGTM! Consider adding an introductory sentence.The steps for Stake Grant Authorizations are well-documented and comprehensive. To enhance clarity, consider adding a brief introductory sentence before the steps, such as:
"The following steps outline the process for authorizing stake grants:"
This addition would provide context for the reader before diving into the specific steps.
🧰 Tools
🪛 Markdownlint
452-452: null
Emphasis used instead of a heading(MD036, no-emphasis-as-heading)
464-469: LGTM! Consider adding an introductory sentence.The steps for Stake Grant Activation are well-documented and comprehensive. To enhance clarity, consider adding a brief introductory sentence before the steps, such as:
"The following steps describe the process for activating a stake grant:"
This addition would provide context for the reader before diving into the specific steps.
🧰 Tools
🪛 Markdownlint
464-464: null
Emphasis used instead of a heading(MD036, no-emphasis-as-heading)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (7)
- .gitignore (1 hunks)
- docs/develop/modules/injective/exchange/02_other_concepts.md (1 hunks)
- docs/develop/modules/injective/exchange/03_state.md (1 hunks)
- docs/develop/modules/injective/exchange/04_state_transitions.md (23 hunks)
- docs/develop/modules/injective/exchange/05_messages.md (1 hunks)
- docs/develop/modules/injective/exchange/09_events.md (4 hunks)
- docs/develop/modules/injective/exchange/10_params.md (1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .gitignore
🧰 Additional context used
🪛 LanguageTool
docs/develop/modules/injective/exchange/02_other_concepts.md
[uncategorized] ~73-~73: Possible missing comma found.
Context: ...nts, stake with 25 or fewer validators. Granted stake cannot be regranted.(AI_HYDRA_LEO_MISSING_COMMA)
🪛 Markdownlint
docs/develop/modules/injective/exchange/04_state_transitions.md
127-127: null
Emphasis used instead of a heading(MD036, no-emphasis-as-heading)
165-165: null
Emphasis used instead of a heading(MD036, no-emphasis-as-heading)
196-196: null
Emphasis used instead of a heading(MD036, no-emphasis-as-heading)
245-245: null
Emphasis used instead of a heading(MD036, no-emphasis-as-heading)
452-452: null
Emphasis used instead of a heading(MD036, no-emphasis-as-heading)
464-464: null
Emphasis used instead of a heading(MD036, no-emphasis-as-heading)
docs/develop/modules/injective/exchange/05_messages.md
451-451: Column: 1
Hard tabs(MD010, no-hard-tabs)
452-452: Column: 1
Hard tabs(MD010, no-hard-tabs)
469-469: Column: 1
Hard tabs(MD010, no-hard-tabs)
470-470: Column: 1
Hard tabs(MD010, no-hard-tabs)
456-456: null
Emphasis used instead of a heading(MD036, no-emphasis-as-heading)
474-474: null
Emphasis used instead of a heading(MD036, no-emphasis-as-heading)
🔇 Additional comments (21)
docs/develop/modules/injective/exchange/10_params.md (3)
10-11: LGTM: Improved table formattingThe changes to the table header and alignment enhance the readability and consistency of the parameter documentation.
12-35: LGTM: Consistent formatting for existing parametersThe formatting of existing parameters has been adjusted to match the new table structure, maintaining consistency throughout the document.
36-36: Please provide more context for the new parameterThe addition of
PostOnlyModeHeightThresholdis noted, but more information is needed to understand its purpose and impact on the exchange module. Consider the following improvements:
- Add a brief description of what this parameter does and how it affects the system.
- Explain the significance of the example value
1000. Is this a block height, a time duration, or something else?- If possible, provide a link to more detailed documentation about this parameter and its usage.
These additions would greatly enhance the clarity and usefulness of the documentation for users and developers.
To ensure consistency and completeness, let's check if this parameter is mentioned in other relevant files:
docs/develop/modules/injective/exchange/02_other_concepts.md (1)
71-73: Summary: Valuable addition to the Fee Discounts documentationThe new "Stake Delegations/Grants" section is a well-integrated and valuable addition to the document. It clearly explains the mechanics of how staked INJ requirements for fee discount tiers can be satisfied through grants, which aligns perfectly with the PR objectives of enhancing documentation for stake grants.
Key points covered:
- Calculation of total staked INJ value for fee discounts
- Limitations on grant activation and usage
- Considerations for staking with validators
This addition significantly improves the clarity and comprehensiveness of the fee discount system documentation.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~73-~73: Possible missing comma found.
Context: ...nts, stake with 25 or fewer validators. Granted stake cannot be regranted.(AI_HYDRA_LEO_MISSING_COMMA)
docs/develop/modules/injective/exchange/09_events.md (12)
56-60: LGTM: New event message for market beyond bankruptcy scenario.The
EventMarketBeyondBankruptcymessage provides crucial information when a market goes beyond bankruptcy. The fields (market_id, settle_price, missing_market_funds) are well-chosen to represent the state of the market in this scenario.
62-66: LGTM: New event message for all positions haircut scenario.The
EventAllPositionsHaircutmessage effectively captures the necessary information when all positions in a market undergo a haircut. The fields (market_id, settle_price, missing_funds_rate) provide a clear picture of the market state and the extent of the haircut.
69-69: LGTM: Improved field definition in EventBinaryOptionsMarketUpdate.The modification to make the
marketfield explicitly non-nullable ([ (gogoproto.nullable) = false ]) enhances the clarity and robustness of the message definition. This change aligns with best practices for protocol buffer message definitions.
86-86: LGTM: Improved field definitions in multiple event messages.The modifications to
EventCancelSpotOrder,EventSpotMarketUpdate,EventPerpetualMarketUpdate, andEventExpiryFuturesMarketUpdatemessages explicitly specify the nullability of their fields. This enhances the clarity and robustness of these message definitions, aligning with best practices for protocol buffer messages.Also applies to: 90-90, 94-96, 100-102
166-171: LGTM: New event message for conditional derivative orders.The
EventNewConditionalDerivativeOrdermessage effectively captures the necessary information for new conditional derivative orders. The fields (market_id, order, hash, is_market) provide a comprehensive representation of the order details.
173-178: LGTM: New event message for cancelling conditional derivative orders.The
EventCancelConditionalDerivativeOrdermessage effectively captures the necessary information for cancelling conditional derivative orders. The fields (market_id, isLimitCancel, limit_order, market_order) provide a clear picture of the cancelled order details.
180-186: LGTM: New event message for triggered conditional derivative orders.The
EventConditionalDerivativeOrderTriggermessage effectively captures the necessary information when a conditional derivative order is triggered. The fields (market_id, isLimitTrigger, triggered_order_hash, placed_order_hash, triggered_order_cid) provide a comprehensive view of both the triggered order and the resulting placed order.
188-193: LGTM: New event message for failed orders.The
EventOrderFailmessage effectively captures the necessary information for failed orders. The fields (account, hashes, flags, cids) provide a comprehensive view of the failed orders, allowing for proper error handling and reporting.
195-213: LGTM: New event messages for orderbook updates and fee multipliers.The new messages (
EventAtomicMarketOrderFeeMultipliersUpdated,EventOrderbookUpdate,OrderbookUpdate, andOrderbook) provide a well-structured representation of orderbook updates and fee multiplier changes. The nested structure allows for efficient representation of complex orderbook data. These additions enhance the system's ability to track and report important market events.
215-232: LGTM: New event messages for grant authorizations and activations.The new messages (
EventGrantAuthorizations,EventGrantActivation, andEventInvalidGrant) provide a well-structured representation of grant-related events. These additions enhance the system's ability to track and report important authorization events, improving the overall functionality of the grant system.
234-240: LGTM: New event message for failed order cancellations.The
EventOrderCancelFailmessage effectively captures the necessary information for failed order cancellations. The fields (market_id, subaccount_id, order_hash, cid, description) provide a comprehensive view of the failed cancellation, including a description field for additional context. This addition enhances the system's ability to track and report order cancellation failures.
Line range hint
56-240: Overall assessment: Significant improvements to the exchange module's event system.The changes in this file substantially enhance the exchange module's event system. New messages have been added to cover scenarios such as market beyond bankruptcy, all positions haircut, conditional derivative orders, order failures, orderbook updates, and grant authorizations. Existing messages have been improved with explicit nullability specifications. These additions and modifications provide a more comprehensive and robust event reporting system, which will likely improve the module's observability and error handling capabilities.
The new message definitions are well-structured and consistent, following best practices for protocol buffer messages. They provide detailed information for various market events, which should facilitate better monitoring and management of the exchange system.
These changes align well with the PR objectives of enhancing documentation related to stake grants and updating code snippets. The new event messages, particularly those related to grants, directly support this goal.
docs/develop/modules/injective/exchange/05_messages.md (1)
442-443: LGTM! Clear and informative update.The changes to the
MsgBatchUpdateOrdersdescription provide valuable clarity on the order of operations and the atomic nature of the process. This improvement aligns well with the PR's objective of enhancing documentation.docs/develop/modules/injective/exchange/03_state.md (2)
598-604: LGTM: GrantAuthorization structure is well-defined and documented.The
GrantAuthorizationstructure is correctly implemented with appropriate field types. The preceding documentation clearly explains its purpose in tracking grantee's address and authorized stake amount for trading fee discounts.
610-616: LGTM: ActiveGrant structure is well-defined and documented.The
ActiveGrantstructure is correctly implemented with appropriate field types. The preceding documentation clearly explains its purpose in tracking granter's address and activated stake amount for trading fee discounts.docs/develop/modules/injective/exchange/04_state_transitions.md (2)
450-451: LGTM! New section added as expected.The addition of the "Stake Grant Authorizations" section is consistent with the PR objectives and maintains the document's formatting style.
462-463: LGTM! New section added as expected.The addition of the "Stake Grant Activation" section is consistent with the PR objectives and maintains the document's formatting style.
|
|
||
| ## Msg/ActivateStakeGrant | ||
|
|
||
| `MsgActivateStakeGrant` is a message used to select/activate a stake grant for fee discount purposes. | ||
|
|
||
| ```go | ||
| type MsgAuthorizeStakeGrants struct { | ||
| Sender string | ||
| Granter string | ||
| } | ||
| ``` | ||
|
|
||
| **Fields description** | ||
|
|
||
| - `Sender` describes the creator of this msg. | ||
| - `Granter` describes the address of the granter. |
There was a problem hiding this comment.
LGTM! Fix struct name and consider adding an example.
The introduction of MsgActivateStakeGrant is well-documented and complements the MsgAuthorizeStakeGrants functionality. However, there's an error in the struct name in the code block.
Please update the struct name in the code block:
-type MsgAuthorizeStakeGrants struct {
+type MsgActivateStakeGrant struct {
Sender string
Granter string
}Consider adding a brief example demonstrating how to use this message type to activate a stake grant.
📝 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.
| ## Msg/ActivateStakeGrant | |
| `MsgActivateStakeGrant` is a message used to select/activate a stake grant for fee discount purposes. | |
| ```go | |
| type MsgAuthorizeStakeGrants struct { | |
| Sender string | |
| Granter string | |
| } | |
| ``` | |
| **Fields description** | |
| - `Sender` describes the creator of this msg. | |
| - `Granter` describes the address of the granter. | |
| ## Msg/ActivateStakeGrant | |
| `MsgActivateStakeGrant` is a message used to select/activate a stake grant for fee discount purposes. | |
| ```go | |
| type MsgActivateStakeGrant struct { | |
| Sender string | |
| Granter string | |
| } | |
| ``` | |
| **Fields description** | |
| - `Sender` describes the creator of this msg. | |
| - `Granter` describes the address of the granter. |
🧰 Tools
🪛 Markdownlint
469-469: Column: 1
Hard tabs(MD010, no-hard-tabs)
470-470: Column: 1
Hard tabs(MD010, no-hard-tabs)
474-474: null
Emphasis used instead of a heading(MD036, no-emphasis-as-heading)
|
Please re-open and finish the PR against |
|
Reopened under #20 |
2 participants
Added documentation on stake grants and updated some snippets
Summary by CodeRabbit
New Features
Documentation
Bug Fixes