-
-
Notifications
You must be signed in to change notification settings - Fork 14
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
Feature/insert events for trade and fee #535
Feature/insert events for trade and fee #535
Conversation
(gogoproto.moretags) = "yaml:\"position_id\"" | ||
]; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code patch appears to be adding three protobuf messages that define fees related to perpetual future trading.
There are no obvious bug risks with this code and the additional fields are well-defined with appropriate tags. However, further context such as the intended use-case, and usage in other parts of the system could provide a more comprehensive review.
(gogoproto.moretags) = "yaml:\"position_id\"" | ||
]; | ||
} | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code patch is adding three new events related to fee earned by the protocol from traders in a derivatives trading platform. The events seem clear and straightforward, and the protobuf message definitions also look well-defined.
As for improvement suggestions, it may be useful to include more detailed information about the relevant parameters in the events, such as the size of the fees and how they are calculated, to make it easier for users to understand them without having to refer to external documentation.
Regarding risks, it would be worth verifying that there are no errors or inconsistencies in the calculations related to the fees or their distribution, as any issues in this regard could potentially lead to financial losses or other problems for traders using the platform.
_ = ctx.EventManager().EmitTypedEvent(&types.EventPerpetualFuturesImaginaryFundingFee{ | ||
Fee: sdk.NewCoin(position.RemainingMargin.Denom, imaginaryFundingFee.Neg()), | ||
PositionId: position.Id, | ||
}) | ||
} | ||
position.LastLeviedAt = ctx.BlockTime() | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code patch is emitting some custom events to the event manager. These emitted events are EventPerpetualFuturesTradingFee
, EventPerpetualFuturesLiquidationFee
, and EventPerpetualFuturesImaginaryFundingFee
. This is a good practice for logging purposes and makes debugging easier.
Regarding bugs, there seem to be no evident bugs within this code patch. However, further analysis would require a broader view of the entire codebase.
As for an improvement suggestion: It seems like multiple calls emit the same event with different values. Consider refactoring these similar code lines and extracting them into helper methods.
(gogoproto.moretags) = "yaml:\"position_id\"" | ||
]; | ||
} | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code patch seems to add new features regarding events emitted by the protocol and their corresponding protobuf messages. There don't seem to be any major bug risks in this code, but some suggestions for improvement could include adding more detailed comments and explanations for each event and its purpose, as well as potentially adjusting field names or tags if they do not conform to best practices or existing conventions.
close #533