Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add max leverage params #453

Merged
merged 3 commits into from
Mar 28, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions proto/derivatives/perpetual_futures.proto
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ message PerpetualFuturesParams {
repeated Market markets = 4 [
(gogoproto.moretags) = "yaml:\"markets\""
];
uint32 max_leverage = 5 [
(gogoproto.moretags) = "yaml:\"max_leverage\""
];
}

message PerpetualFuturesPosition {
Expand Down
1 change: 1 addition & 0 deletions x/derivatives/keeper/perpetual_futures.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Expand Down
7 changes: 6 additions & 1 deletion x/derivatives/types/params.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ func DefaultPerpetualFuturesParams() PerpetualFuturesParams {
CommissionRate: sdk.MustNewDecFromStr("0.001"),
MarginMaintenanceRate: sdk.MustNewDecFromStr("0.5"),
ImaginaryFundingRateProportionalCoefficient: sdk.MustNewDecFromStr("0.0005"),
Markets: []*Market{},
Markets: []*Market{},
MaxLeverage: 30,
}
}

Expand Down Expand Up @@ -137,6 +138,10 @@ func validatePerpetualFutures(i interface{}) error {
return fmt.Errorf("invalid margin maintenance rate: %s", perpetualFuturesParams.MarginMaintenanceRate)
}

if perpetualFuturesParams.MaxLeverage == 0 {
return fmt.Errorf("max leverage must not be zero: %d", perpetualFuturesParams.MaxLeverage)
}

return nil
}

Expand Down
175 changes: 106 additions & 69 deletions x/derivatives/types/perpetual_futures.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 8 additions & 0 deletions x/derivatives/types/positions.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ func (m Position) IsValid(quoteTicker string) error {
return fmt.Errorf("position size is not valid")
}

if !pfPosition.PositionInstance.IsValidLeverage(params.PerpetualFutures.MaxLeverage) {
return fmt.Errorf("leverage is not valid")
}

return nil
}

Expand All @@ -50,6 +54,10 @@ func (m PerpetualFuturesPosition) IsValidPositionSize(quoteTicker string) bool {
return !marginMaintenanceRate.LT(sdk.OneDec())
}

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 {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on the provided code fragment, it seems to be written in Go programming language. Here's my brief code review:

  1. The code adds a new IsValidLeverage method in the PerpetualFuturesPositionInstance struct.

  2. The IsValidLeverage method checks if the leverage for a position instance is within the allowable range specified by maxLeverage.

  3. The IsValidLeverage method returns a boolean value based on the check result, meaning there are no bug risks in this implementation.

  4. The IsValid method calls this new IsValidLeverage method 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.
  5. 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.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Expand Down
Loading