-
Notifications
You must be signed in to change notification settings - Fork 115
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
[OTE-877] deprecate OI indexer update event #2499
Conversation
WalkthroughThe changes in this pull request focus on deprecating certain event interfaces and enhancing the documentation within the indexer module. Key modifications include the introduction of the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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 (
|
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.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (3)
proto/dydxprotocol/indexer/events/events.proto (1)
Line range hint
553-554
: Typo in Comment: Incorrect Message NameThe comment above
LiquidityTierUpsertEventV2
incorrectly referencesLiquidationEventV2
instead ofLiquidityTierUpsertEventV2
. This could cause confusion for developers reading the code.Apply this diff to correct the comment:
-// LiquidationEventV2 message contains all the information needed to update -// the liquidity tiers. It contains all the fields from V1 along with the -// open interest caps. +// LiquidityTierUpsertEventV2 message contains all the information needed to update +// the liquidity tiers. It contains all the fields from V1 along with the +// open interest caps.indexer/packages/v4-protos/src/codegen/dydxprotocol/indexer/events/events.ts (2)
Line range hint
1420-1424
: Correct the interface name in the commentThe comment refers to
LiquidationEventV2
, but the interface isLiquidityTierUpsertEventV2
. Update the comment to reflect the correct interface name.Apply this diff:
- /** - * LiquidationEventV2 message contains all the information needed to update + /** + * LiquidityTierUpsertEventV2 message contains all the information needed to update * the liquidity tiers. It contains all the fields from V1 along with the * open interest caps. */
Line range hint
1452-1456
: Correct the interface name in the commentEnsure that the comment for
LiquidityTierUpsertEventV2SDKType
matches the interface name.Apply this diff:
- /** - * LiquidationEventV2 message contains all the information needed to update + /** + * LiquidityTierUpsertEventV2SDKType message contains all the information needed to update * the liquidity tiers. It contains all the fields from V1 along with the * open interest caps. */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
protocol/indexer/events/events.pb.go
is excluded by!**/*.pb.go
📒 Files selected for processing (9)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/indexer/events/events.ts (1 hunks)
- proto/dydxprotocol/indexer/events/events.proto (1 hunks)
- protocol/x/clob/e2e/long_term_orders_test.go (0 hunks)
- protocol/x/clob/e2e/short_term_orders_test.go (0 hunks)
- protocol/x/perpetuals/abci.go (0 hunks)
- protocol/x/perpetuals/abci_test.go (0 hunks)
- protocol/x/perpetuals/keeper/perpetual.go (0 hunks)
- protocol/x/perpetuals/keeper/perpetual_test.go (0 hunks)
- protocol/x/perpetuals/types/types.go (0 hunks)
💤 Files with no reviewable changes (7)
- protocol/x/clob/e2e/long_term_orders_test.go
- protocol/x/clob/e2e/short_term_orders_test.go
- protocol/x/perpetuals/abci.go
- protocol/x/perpetuals/abci_test.go
- protocol/x/perpetuals/keeper/perpetual.go
- protocol/x/perpetuals/keeper/perpetual_test.go
- protocol/x/perpetuals/types/types.go
🧰 Additional context used
🔇 Additional comments (4)
proto/dydxprotocol/indexer/events/events.proto (4)
537-541
: Properly DeprecatedOpenInterestUpdateEventV1
The
OpenInterestUpdateEventV1
message is correctly marked as deprecated using theoption deprecated = true;
statement within the message. This appropriately signals to users that this message should no longer be used and helps maintain backward compatibility.
545-549
: Properly DeprecatedOpenInterestUpdate
The
OpenInterestUpdate
message is also correctly marked as deprecated withoption deprecated = true;
. This ensures that users are aware that this message is deprecated and should migrate to newer alternatives.
Line range hint
565-569
: Addition ofRegisterAffiliateEventV1
is CorrectThe
RegisterAffiliateEventV1
message is properly defined with appropriate fields forreferee
andaffiliate
. The use of(cosmos_proto.scalar) = "cosmos.AddressString"
ensures correct handling of address strings in the Cosmos SDK.
Line range hint
571-577
:UpsertVaultEventV1
Message Addition is Well-StructuredThe
UpsertVaultEventV1
message is correctly structured, with clear fields foraddress
,clob_pair_id
, andstatus
. UtilizingVaultStatus
for thestatus
field is appropriate and maintains consistency with existing protocol definitions.
|
||
/** @deprecated */ | ||
|
||
export interface OpenInterestUpdate { | ||
/** The ID of the perpetual market. */ | ||
perpetualId: number; | ||
/** The new open interest value for the perpetual market. */ | ||
|
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.
Include @deprecated
annotation within the JSDoc comment block
For OpenInterestUpdate
, the @deprecated
tag should be placed inside the JSDoc comment block.
Apply this diff:
/**
* OpenInterestUpdate contains a single open interest update for a perpetual
* Deprecated.
+ * @deprecated
*/
- /** @deprecated */
export interface OpenInterestUpdate {
perpetualId: number;
/** The new open interest value for the perpetual market. */
📝 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.
/** @deprecated */ | |
export interface OpenInterestUpdate { | |
/** The ID of the perpetual market. */ | |
perpetualId: number; | |
/** The new open interest value for the perpetual market. */ | |
/** | |
* OpenInterestUpdate contains a single open interest update for a perpetual | |
* Deprecated. | |
* @deprecated | |
*/ | |
export interface OpenInterestUpdate { | |
perpetualId: number; | |
/** The new open interest value for the perpetual market. */ |
/** | ||
* OpenInterestUpdateEventV1 is used for open interest update events | ||
* Deprecated. | ||
*/ | ||
|
||
/** @deprecated */ |
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.
Combine @deprecated
annotation with the JSDoc comment block
Similarly, include the @deprecated
tag within the JSDoc comment block for OpenInterestUpdateEventV1SDKType
.
Apply this diff:
/**
* OpenInterestUpdateEventV1 is used for open interest update events
* Deprecated.
+ * @deprecated
*/
- /** @deprecated */
export interface OpenInterestUpdateEventV1SDKType {
open_interest_updates: OpenInterestUpdateSDKType[];
}
📝 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.
/** | |
* OpenInterestUpdateEventV1 is used for open interest update events | |
* Deprecated. | |
*/ | |
/** @deprecated */ | |
/** | |
* OpenInterestUpdateEventV1 is used for open interest update events | |
* Deprecated. | |
* @deprecated | |
*/ | |
export interface OpenInterestUpdateEventV1SDKType { | |
open_interest_updates: OpenInterestUpdateSDKType[]; | |
} |
/** | ||
* OpenInterestUpdateEventV1 is used for open interest update events | ||
* Deprecated. | ||
*/ | ||
|
||
/** @deprecated */ |
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.
Combine @deprecated
annotation with the JSDoc comment block
The @deprecated
tag for OpenInterestUpdateEventV1
should be included within the JSDoc comment block to properly document the deprecation.
Apply this diff:
/**
* OpenInterestUpdateEventV1 is used for open interest update events
* Deprecated.
+ * @deprecated
*/
- /** @deprecated */
export interface OpenInterestUpdateEventV1 {
openInterestUpdates: OpenInterestUpdate[];
}
📝 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.
/** | |
* OpenInterestUpdateEventV1 is used for open interest update events | |
* Deprecated. | |
*/ | |
/** @deprecated */ | |
/** | |
* OpenInterestUpdateEventV1 is used for open interest update events | |
* Deprecated. | |
* @deprecated | |
*/ | |
export interface OpenInterestUpdateEventV1 { | |
openInterestUpdates: OpenInterestUpdate[]; | |
} |
💡 Codebase verification
Deprecated Interfaces Still in Use
The deprecated interfaces OpenInterestUpdateEventV1
and OpenInterestUpdate
are still being used in the file indexer/services/ender/__tests__/helpers/constants.ts
. Please remove these usages to complete the deprecation process.
🔗 Analysis chain
Verify that deprecated interfaces are no longer used in the codebase
Now that OpenInterestUpdateEventV1
and OpenInterestUpdate
have been deprecated, ensure that they are not used elsewhere in the codebase.
Run the following script to check for usage:
Also applies to: 1399-1405
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for usage of deprecated interfaces in TypeScript files
# Expected: No occurrences of the deprecated interfaces
rg --type ts 'OpenInterestUpdateEventV1|OpenInterestUpdate' --glob '!events.ts'
Length of output: 726
|
||
/** @deprecated */ | ||
|
||
export interface OpenInterestUpdateSDKType { | ||
/** The ID of the perpetual market. */ | ||
perpetual_id: number; | ||
/** The new open interest value for the perpetual market. */ |
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.
Include @deprecated
annotation within the JSDoc comment block
Similarly, for OpenInterestUpdateSDKType
, include the @deprecated
tag within the JSDoc comment block.
Apply this diff:
/**
* OpenInterestUpdate contains a single open interest update for a perpetual
* Deprecated.
+ * @deprecated
*/
- /** @deprecated */
export interface OpenInterestUpdateSDKType {
perpetual_id: number;
/** The new open interest value for the perpetual market. */
📝 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.
/** @deprecated */ | |
export interface OpenInterestUpdateSDKType { | |
/** The ID of the perpetual market. */ | |
perpetual_id: number; | |
/** The new open interest value for the perpetual market. */ | |
/** | |
* OpenInterestUpdate contains a single open interest update for a perpetual | |
* Deprecated. | |
* @deprecated | |
*/ | |
export interface OpenInterestUpdateSDKType { | |
perpetual_id: number; | |
/** The new open interest value for the perpetual market. */ |
@Mergifyio backport release/protocol/v6.x |
@Mergifyio backport release/protocol/v7.x |
✅ Backports have been created
|
✅ Backports have been created
|
(cherry picked from commit 4208b7b)
(cherry picked from commit 4208b7b) # Conflicts: # protocol/indexer/events/events.pb.go
Changelist
[Describe or list the changes made in this PR]
Test Plan
[Describe how this PR was tested (if applicable)]
Author/Reviewer Checklist
state-breaking
label.indexer-postgres-breaking
label.PrepareProposal
orProcessProposal
, manually add the labelproposal-breaking
.feature:[feature-name]
.backport/[branch-name]
.refactor
,chore
,bug
.Summary by CodeRabbit
LiquidityTierUpsertEventV2
for enhanced liquidity tier management.OpenInterestUpdateEventV1
andOpenInterestUpdate
.