add max leverage params #453
Conversation
|
Seems you are using me but didn't get OPENAI_API_KEY seted in Variables for this repo. you could follow readme for more information |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -69,6 +69,7 @@ func (k Keeper) OpenPerpetualFuturesPosition(ctx sdk.Context, positionId string, | |||
| RemainingMargin: margin, | |||
| } | |||
|
|
|||
| params := k.GetParams(ctx) | |||
| // General validation for the position creation | |||
| quoteTicker := k.GetPoolQuoteTicker(ctx) | |||
| if err := position.IsValid(quoteTicker); err != nil { | |||
There was a problem hiding this comment.
The code patch adds a call to k.GetParams(ctx) to retrieve parameters from the storage. Otherwise, it appears to be adding validation for position creation using IsValid() method.
Without seeing the full context of the code, it's hard to identify any potential bugs or improvements. However, given that this is just one method in a larger codebase, it would be helpful to ensure consistency in coding style, naming conventions, and error handling throughout the codebase. It's also important to write clear comments and commit messages to explain the purpose and intent of each change made.
| func (m PerpetualFuturesPositionInstance) IsValidLeverage(maxLeverage uint32) bool { | ||
| return m.Leverage > 0 && m.Leverage <= maxLeverage | ||
| } | ||
|
|
||
| func UnpackPositionInstance(positionAny types.Any) (PositionInstance, error) { | ||
| position := UnpackPerpetualFuturesPositionInstance(positionAny) | ||
| if position != nil { |
There was a problem hiding this comment.
Based on the provided code fragment, it seems to be written in Go programming language. Here's my brief code review:
-
The code adds a new
IsValidLeveragemethod in thePerpetualFuturesPositionInstancestruct. -
The
IsValidLeveragemethod checks if the leverage for a position instance is within the allowable range specified bymaxLeverage. -
The
IsValidLeveragemethod returns a boolean value based on the check result, meaning there are no bug risks in this implementation. -
The
IsValidmethod calls this newIsValidLeveragemethod to validate the leverage in addition to validating the position size:- If the leverage is not valid, it returns an error message.
- If the position size is not valid, it also returns an error message.
-
A possible improvement suggestion is to add some inline documentation, like comments, that explain what the methods do and how they work to make the code more readable and maintainable.
Overall, based on this code snippet, I see no obvious bug risks, and adding some inline documentation may improve the code quality even further.
|
Seems you are using me but didn't get OPENAI_API_KEY seted in Variables for this repo. you could follow readme for more information |
| quoteTicker := k.GetPoolQuoteTicker(ctx) | ||
| if err := position.IsValid(quoteTicker); err != nil { | ||
| params := k.GetParams(ctx) | ||
| if err := position.IsValid(params); err != nil { | ||
| return nil, err | ||
| } | ||
|
|
There was a problem hiding this comment.
The code patch appears to replace the validation of a perpetual futures position's validity with a call to a GetParams function that retrieves the module's parameters from the context. This implies that the validation of the position now relies on the module's parameters instead of the pool's quote ticker.
Without knowing the specifics of the IsValid method and the GetPoolQuoteTicker function, I cannot determine whether there are any bug risks or improvement suggestions for the change made in this patch. However, as long as the GetParams function correctly retrieves the necessary parameters for validating positions, this change could improve code simplicity and maintainability.
| func (m PerpetualFuturesPositionInstance) IsValidLeverage(maxLeverage uint32) bool { | ||
| return m.Leverage > 0 && m.Leverage <= maxLeverage | ||
| } | ||
|
|
||
| func UnpackPositionInstance(positionAny types.Any) (PositionInstance, error) { | ||
| position := UnpackPerpetualFuturesPositionInstance(positionAny) | ||
| if position != nil { |
There was a problem hiding this comment.
The code changes seem to update the IsValid function of the Position type to accept a Params parameter instead of just a quoteTicker string argument. This allows for more validation criteria to be passed in and checked before returning an error.
One improvement suggestion would be to add more descriptive error messages that specify which validation rule failed, rather than generic error messages. This will make it easier for developers to troubleshoot issues if/when they occur.
As for bug risks, without seeing the rest of the codebase it's difficult to determine whether there are any potential issues. However, as long as the parameters being passed into IsValid are correctly formatted and valid, the changes should not introduce any new bugs.
Issue: #408
The default max leverage is set
30.