F/add stake grants #20
Conversation
WalkthroughThe changes in this pull request involve updates to several documentation files and the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Exchange
participant Grantor
participant Grantee
User->>Exchange: Request Fee Discount
Exchange->>Grantor: Authorize Stake Grants
Grantor->>Exchange: Send 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: 6
🧹 Outside diff range and nitpick comments (11)
docs/develop/modules/injective/exchange/02_other_concepts.md (1)
71-73: LGTM! Documentation clearly explains stake grants mechanics.The new section provides accurate information about stake grants and their relationship to fee discount tiers.
Consider these improvements for better clarity:
- Add examples to illustrate the formula:
### Example If Account A has: - OwnStake: 1000 INJ - Received grant from Account B: 500 INJ - Granted to Account C: 200 INJ Total stake for fee discount = 1000 + 500 - 200 = 1300 INJ
- Add a subsection about grant management:
### Managing Stake Grants - To grant stake: Use `MsgAuthorizeStakeGrants` - To activate a received grant: Use `MsgActivateStakeGrant` - To revoke a grant: [Add revocation process]
- Highlight the validator limit more prominently:
### Important Note About Validators The 25-validator limit is crucial for stake grants: - Only INJ staked with ≤25 validators counts for `OwnStake` - Exceeding this limit will reduce usable stake for grants - Choose validators strategically to maximize grant potentialdocs/develop/modules/injective/exchange/09_events.md (3)
56-67: Consider adding field validation and type consistency.The new bankruptcy-related events look good, but could benefit from:
- Explicit field validation annotations for required fields
- Consistent type usage for
market_id(string vs bytes) across events- Decimal type annotations for price and fund fields
message EventMarketBeyondBankruptcy { - string market_id = 1; - string settle_price = 2; - string missing_market_funds = 3; + string market_id = 1 [(gogoproto.nullable) = false]; + string settle_price = 2 [ + (gogoproto.customtype) = "cosmossdk.io/math.LegacyDec", + (gogoproto.nullable) = false + ]; + string missing_market_funds = 3 [ + (gogoproto.customtype) = "cosmossdk.io/math.LegacyDec", + (gogoproto.nullable) = false + ]; }
137-138: Add field validation for deposit updates.Consider adding validation annotations to ensure the repeated field is not empty.
-message EventBatchDepositUpdate { repeated DepositUpdate deposit_updates = 1; } +message EventBatchDepositUpdate { + repeated DepositUpdate deposit_updates = 1 [(gogoproto.nullable) = false]; +}
188-240: Add field validations and documentation.The new events are well-structured, but consider:
- Adding validation annotations for required fields
- Documenting the expected format/content of the
descriptionfield inEventOrderCancelFail- Adding size constraints for repeated fields where appropriate
Example improvements:
message EventOrderCancelFail { - string market_id = 1; - string subaccount_id = 2; - string order_hash = 3; + string market_id = 1 [(gogoproto.nullable) = false]; + string subaccount_id = 2 [(gogoproto.nullable) = false]; + string order_hash = 3 [(gogoproto.nullable) = false]; string cid = 4; + // Human-readable description of why the order cancellation failed string description = 5; }docs/develop/modules/injective/exchange/05_messages.md (2)
447-448: Enhance documentation with examplesThe documentation would benefit from including examples showing:
- How to grant stake authorization
- How to revoke a grant by setting amount to zero
Example:
// Example: Granting stake authorization { "sender": "inj1...", "grants": [{ "grantee": "inj1...", "amount": "1000000000000000000" // 1 INJ }] } // Example: Revoking stake authorization { "sender": "inj1...", "grants": [{ "grantee": "inj1...", "amount": "0" }] }Also applies to: 458-460
465-466: Enhance documentation with context and examplesThe documentation should explain:
- When to use this message (e.g., after receiving a grant)
- The relationship between granter and sender
- The impact of activating a grant
Example:
// Example: Activating a stake grant { "sender": "inj1...", // The grantee who received the stake grant "granter": "inj1..." // The address that authorized the stake grant }Additionally, consider adding a note about whether:
- Multiple grants can be active simultaneously
- An existing active grant needs to be deactivated first
- There are any restrictions on activation/deactivation frequency
Also applies to: 476-477
docs/develop/modules/injective/exchange/03_state.md (3)
596-606: Consider enhancing documentation with an example.The
GrantAuthorizationtype is well-defined, but adding a usage example would make it more developer-friendly.Consider adding an example like:
// Example: // grantAuthorization := GrantAuthorization{ // Grantee: "inj1...", // Grantee's address // Amount: math.NewInt(1000000), // 1 INJ (assuming 6 decimals) // }
608-618: Clarify relationship with GrantAuthorization.The
ActiveGranttype is well-defined, but its documentation could better explain how it relates toGrantAuthorizationin the stake grant lifecycle.Consider adding clarification like:
`ActiveGrant` represents the portion of a `GrantAuthorization` that has been activated by the grantee. While `GrantAuthorization` tracks the authorized amount, `ActiveGrant` tracks how much of that authorization has been activated for use in fee discounts.
620-631: Document IsValid field and type relationships.The
EffectiveGranttype needs additional documentation to explain:
- The purpose of the
IsValidfield- How
NetGrantedStakerelates to the amounts inGrantAuthorizationandActiveGrantConsider adding:
The `IsValid` field indicates whether the grant is currently valid and can be used for fee discounts. `NetGrantedStake` represents the total stake amount authorized by the granter across all active grant authorizations, calculated as the sum of all `GrantAuthorization` amounts minus the sum of all `ActiveGrant` amounts for the same granter.docs/develop/modules/injective/exchange/04_state_transitions.md (2)
450-460: Enhance documentation for Stake Grant AuthorizationsWhile the steps are logically structured, the section could benefit from:
- An introductory paragraph explaining what stake grant authorizations are
- More detailed explanations for each step
- Examples of grant calculations
Consider adding an introduction like:
## Stake Grant Authorizations + +Stake grant authorizations allow a staker (granter) to authorize another address (grantee) +to utilize their staked tokens for fee discount purposes. This enables delegation of +staking benefits without transferring token ownership. + **Steps**🧰 Tools
🪛 Markdownlint
452-452: null
Emphasis used instead of a heading(MD036, no-emphasis-as-heading)
Line range hint
1-469: Consider adding cross-references between related sectionsThe document would benefit from cross-references between related sections, particularly:
- Link between Fee Discount Schedule Proposal and Stake Grant sections
- References to related message types and events
Consider adding "See also" sections at the end of related segments to improve navigation and understanding of the relationships between different components.
🧰 Tools
🪛 Markdownlint
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)
📜 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
🪛 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 (4)
docs/develop/modules/injective/exchange/10_params.md (1)
10-11: LGTM: Table formatting improvements
The table formatting changes improve readability with consistent column alignment.
docs/develop/modules/injective/exchange/09_events.md (2)
Line range hint 69-134: LGTM! Field annotation updates improve type safety.
The consistent addition of (gogoproto.nullable) = false annotations across messages improves type safety and makes the contract more explicit.
139-152: LGTM! Well-structured derivative order cancellation messages.
The messages are properly structured with appropriate annotations for decimal types and nullable fields.
docs/develop/modules/injective/exchange/04_state_transitions.md (1)
37-39: LGTM: Table of contents updated correctly
The new sections have been properly added to the table of contents, maintaining consistency with the document structure.
| | BinaryOptionsAtomicMarketOrderFeeMultiplier | math.LegacyDec | 2x | | ||
| | MinimalProtocolFeeRate | math.LegacyDec | 0.00001% | | ||
| | IsInstantDerivativeMarketLaunchEnabled | bool | false | | ||
| | PostOnlyModeHeightThreshold | int64 | 1000 | |
There was a problem hiding this comment.
💡 Codebase verification
Based on the investigation so far:
- The parameter was recently added in a commit by Hans Sun specifically to add it to the params documentation
- We couldn't find any actual implementation in the Go code
- The parameter appears only in the documentation file without any context or explanation
- There's no evidence linking this to stake grants
The PostOnlyModeHeightThreshold parameter addition requires clarification
This parameter appears to be added in isolation without proper implementation or documentation. It was added in a recent commit but:
- No implementation found in the codebase
- No explanation of its purpose or usage
- No connection to stake grants functionality
- Only appears in the parameters documentation
Please provide the implementation details and proper documentation for this parameter, or remove it if it was added unintentionally.
🔗 Analysis chain
Please verify the addition of PostOnlyModeHeightThreshold parameter
The new parameter PostOnlyModeHeightThreshold appears to be unrelated to the PR's primary objective of enhancing stake grants documentation. Could you please:
- Confirm if this parameter addition is intentional
- Provide documentation about its purpose and usage
- Explain how it relates to stake grants, if applicable
Let's check if this parameter is documented elsewhere:
Let me search for more context about this parameter in the codebase:
Let me check the git history to understand when and why this parameter was added:
Let me check the actual code implementation to understand this parameter better:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any documentation or references to PostOnlyModeHeightThreshold
rg -i "PostOnlyModeHeightThreshold" --type md
Length of output: 183
Script:
#!/bin/bash
# Search for PostOnlyModeHeightThreshold in all files to understand its implementation and purpose
rg -i "PostOnlyModeHeightThreshold" -A 3 -B 3
Length of output: 597
Script:
#!/bin/bash
# Check git history for this parameter
git log -p | grep -A 10 -B 10 "PostOnlyModeHeightThreshold"
# Also check if there are any related code changes in the PR
git diff origin/master | grep -A 10 -B 10 "PostOnlyModeHeightThreshold"
Length of output: 3797
Script:
#!/bin/bash
# Search for the parameter in Go files to find its implementation
rg -t go "PostOnlyModeHeightThreshold" -A 5 -B 5
Length of output: 197
| message EventNewConditionalDerivativeOrder { | ||
| string market_id = 1; | ||
| DerivativeOrder order = 2; | ||
| bytes hash = 3; | ||
| bool is_market = 4; | ||
| } | ||
|
|
||
| message EventCancelConditionalDerivativeOrder { | ||
| string market_id = 1; | ||
| bool isLimitCancel = 2; | ||
| DerivativeLimitOrder limit_order = 3 [ (gogoproto.nullable) = true ]; | ||
| DerivativeMarketOrder market_order = 4 [ (gogoproto.nullable) = true ]; | ||
| } | ||
|
|
||
| message EventConditionalDerivativeOrderTrigger { | ||
| bytes market_id = 1; | ||
| bool isLimitTrigger = 2; | ||
| bytes triggered_order_hash = 3; | ||
| bytes placed_order_hash = 4; | ||
| string triggered_order_cid = 5; | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Standardize type usage across conditional order events.
There are inconsistencies in type usage:
market_idis string inEventNewConditionalDerivativeOrderbut bytes inEventConditionalDerivativeOrderTrigger- Missing field validations for required fields
Consider standardizing the types and adding validations:
message EventConditionalDerivativeOrderTrigger {
- bytes market_id = 1;
+ string market_id = 1 [(gogoproto.nullable) = false];
bool isLimitTrigger = 2;
- bytes triggered_order_hash = 3;
- bytes placed_order_hash = 4;
+ bytes triggered_order_hash = 3 [(gogoproto.nullable) = false];
+ bytes placed_order_hash = 4 [(gogoproto.nullable) = false];
string triggered_order_cid = 5;
}📝 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.
| message EventNewConditionalDerivativeOrder { | |
| string market_id = 1; | |
| DerivativeOrder order = 2; | |
| bytes hash = 3; | |
| bool is_market = 4; | |
| } | |
| message EventCancelConditionalDerivativeOrder { | |
| string market_id = 1; | |
| bool isLimitCancel = 2; | |
| DerivativeLimitOrder limit_order = 3 [ (gogoproto.nullable) = true ]; | |
| DerivativeMarketOrder market_order = 4 [ (gogoproto.nullable) = true ]; | |
| } | |
| message EventConditionalDerivativeOrderTrigger { | |
| bytes market_id = 1; | |
| bool isLimitTrigger = 2; | |
| bytes triggered_order_hash = 3; | |
| bytes placed_order_hash = 4; | |
| string triggered_order_cid = 5; | |
| } | |
| message EventNewConditionalDerivativeOrder { | |
| string market_id = 1; | |
| DerivativeOrder order = 2; | |
| bytes hash = 3; | |
| bool is_market = 4; | |
| } | |
| message EventCancelConditionalDerivativeOrder { | |
| string market_id = 1; | |
| bool isLimitCancel = 2; | |
| DerivativeLimitOrder limit_order = 3 [ (gogoproto.nullable) = true ]; | |
| DerivativeMarketOrder market_order = 4 [ (gogoproto.nullable) = true ]; | |
| } | |
| message EventConditionalDerivativeOrderTrigger { | |
| string market_id = 1 [(gogoproto.nullable) = false]; | |
| bool isLimitTrigger = 2; | |
| bytes triggered_order_hash = 3 [(gogoproto.nullable) = false]; | |
| bytes placed_order_hash = 4 [(gogoproto.nullable) = false]; | |
| string triggered_order_cid = 5; | |
| } |
| type MsgAuthorizeStakeGrants struct { | ||
| Sender string | ||
| Granter string | ||
| } | ||
| ``` |
There was a problem hiding this comment.
Fix incorrect struct name and formatting
The struct name is incorrect and should be MsgActivateStakeGrant instead of MsgAuthorizeStakeGrants.
-type MsgAuthorizeStakeGrants struct {
- Sender string
- Granter string
+type MsgActivateStakeGrant struct {
+ Sender string
+ Granter string
}📝 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.
| type MsgAuthorizeStakeGrants struct { | |
| Sender string | |
| Granter string | |
| } | |
| ``` | |
| type MsgActivateStakeGrant struct { | |
| Sender string | |
| Granter string | |
| } |
🧰 Tools
🪛 Markdownlint
469-469: Column: 1
Hard tabs
(MD010, no-hard-tabs)
470-470: Column: 1
Hard tabs
(MD010, no-hard-tabs)
| ```go | ||
| type MsgAuthorizeStakeGrants struct { | ||
| Sender string | ||
| Grants []*GrantAuthorization | ||
| } | ||
| ``` |
There was a problem hiding this comment.
Fix struct name inconsistency and add type definition
The struct name in the code block should be MsgAuthorizeStakeGrants to match the section title. Also, the GrantAuthorization type definition is missing, which is crucial for understanding the Grants field.
type MsgAuthorizeStakeGrants struct {
- Sender string
- Grants []*GrantAuthorization
+ Sender string
+ Grants []*GrantAuthorization
}
+// GrantAuthorization defines the grant authorization structure
+type GrantAuthorization struct {
+ Grantee string
+ Amount math.Int
+}Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Markdownlint
451-451: Column: 1
Hard tabs
(MD010, no-hard-tabs)
452-452: Column: 1
Hard tabs
(MD010, no-hard-tabs)
| @@ -445,3 +446,24 @@ Derivative market param update is handled by `DerivativeMarketParamUpdateProposa | |||
| - Set the first fee paid bucket timestamp as the current block time | |||
| - Set New Fee Discount Schedule, delete it along with Market Qualifications | |||
| - Set New Market Qualifications | |||
|
|
|||
There was a problem hiding this comment.
Add missing steps for Fee Discount Schedule Proposal
The section "Fee Discount Schedule Proposal" appears incomplete. While the heading is added, the steps are missing from the section content.
Would you like me to help draft the missing steps for the Fee Discount Schedule Proposal section? The steps should align with the existing format and detail the process of proposing and implementing fee discount schedules.
| ## Stake Grant Activation | ||
|
|
||
| **Steps** | ||
|
|
||
| - Check to make sure grant from granter to grantee exists | ||
| - Check to make sure granter is not granting more than their total staked amount | ||
| - If grant amount is 0, delete the grant, otherwise write new grant amount to store | ||
| - Emit `EventGrantActivation` with grantee, granter, and amount |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve clarity of Stake Grant Activation section
The section needs more context about when and why stake grants are activated. Additionally, some steps need clarification:
- Step about checking grant existence should specify where to check
- The grant amount validation step needs more detail
- The deletion condition should be explained
Consider enhancing the section like this:
## Stake Grant Activation
+
+Stake grant activation is the process of enabling a previously authorized grant.
+Only one grant can be active at a time for a grantee.
**Steps**
-Check to make sure grant from granter to grantee exists
+- Verify the grant exists in the GrantAuthorization store for the granter-grantee pair
-Check to make sure granter is not granting more than their total staked amount
+- Validate that the granter's total granted amount (including this grant) does not exceed their total staked tokens
-If grant amount is 0, delete the grant, otherwise write new grant amount to store
+- If the grant amount is 0:
+ - Delete the grant from the GrantAuthorization store
+ - Clean up any associated metadata
+- Otherwise:
+ - Update the grant amount in the store
+ - Update the active grant status
- Emit `EventGrantActivation` with grantee, granter, and amount📝 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.
| ## Stake Grant Activation | |
| **Steps** | |
| - Check to make sure grant from granter to grantee exists | |
| - Check to make sure granter is not granting more than their total staked amount | |
| - If grant amount is 0, delete the grant, otherwise write new grant amount to store | |
| - Emit `EventGrantActivation` with grantee, granter, and amount | |
| ## Stake Grant Activation | |
| Stake grant activation is the process of enabling a previously authorized grant. | |
| Only one grant can be active at a time for a grantee. | |
| **Steps** | |
| - Verify the grant exists in the GrantAuthorization store for the granter-grantee pair | |
| - Validate that the granter's total granted amount (including this grant) does not exceed their total staked tokens | |
| - If the grant amount is 0: | |
| - Delete the grant from the GrantAuthorization store | |
| - Clean up any associated metadata | |
| - Otherwise: | |
| - Update the grant amount in the store | |
| - Update the active grant status | |
| - Emit `EventGrantActivation` with grantee, granter, and amount |
🧰 Tools
🪛 Markdownlint
464-464: null
Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
Added documentation on stake grants and updated some snippets
Summary by CodeRabbit
New Features
Documentation