Update order replacement protos to include old order id #1664
Conversation
WalkthroughThe recent updates to the Changes
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 as PR comments)
Additionally, you can add CodeRabbit Configration 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: 1
Outside diff range and nitpick comments (1)
indexer/packages/v4-protos/src/codegen/dydxprotocol/indexer/events/events.ts (1)
Line range hint
75-75: Address static analysis findings regarding useless case clauses.The static analysis tool has identified several useless case clauses in switch statements. These should be reviewed and potentially removed to clean up the code and reduce complexity.
- case "UNRECOGNIZED": - default: - return FundingEventV1_Type.UNRECOGNIZED;Also applies to: 76-76, 95-95
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (2)
protocol/indexer/events/events.pb.gois excluded by!**/*.pb.goprotocol/indexer/off_chain_updates/types/off_chain_updates.pb.gois excluded by!**/*.pb.go
Files selected for processing (4)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/indexer/events/events.ts (4 hunks)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/indexer/off_chain_updates/off_chain_updates.ts (5 hunks)
- proto/dydxprotocol/indexer/events/events.proto (1 hunks)
- proto/dydxprotocol/indexer/off_chain_updates/off_chain_updates.proto (1 hunks)
Additional context used
Biome
indexer/packages/v4-protos/src/codegen/dydxprotocol/indexer/off_chain_updates/off_chain_updates.ts
[error] 82-82: Useless case clause. (lint/complexity/noUselessSwitchCase)
because the default clause is present:
Unsafe fix: Remove the useless case.
[error] 83-83: Useless case clause. (lint/complexity/noUselessSwitchCase)
because the default clause is present:
Unsafe fix: Remove the useless case.
[error] 99-99: Useless case clause. (lint/complexity/noUselessSwitchCase)
because the default clause is present:
Unsafe fix: Remove the useless case.
[error] 194-194: Useless case clause. (lint/complexity/noUselessSwitchCase)
because the default clause is present:
Unsafe fix: Remove the useless case.
[error] 195-195: Useless case clause. (lint/complexity/noUselessSwitchCase)
because the default clause is present:
Unsafe fix: Remove the useless case.
[error] 214-214: Useless case clause. (lint/complexity/noUselessSwitchCase)
because the default clause is present:
Unsafe fix: Remove the useless case.
indexer/packages/v4-protos/src/codegen/dydxprotocol/indexer/events/events.ts
[error] 75-75: Useless case clause. (lint/complexity/noUselessSwitchCase)
because the default clause is present:
Unsafe fix: Remove the useless case.
[error] 76-76: Useless case clause. (lint/complexity/noUselessSwitchCase)
because the default clause is present:
Unsafe fix: Remove the useless case.
[error] 95-95: Useless case clause. (lint/complexity/noUselessSwitchCase)
because the default clause is present:
Unsafe fix: Remove the useless case.
Additional comments not posted (4)
proto/dydxprotocol/indexer/off_chain_updates/off_chain_updates.proto (1)
91-98: The addition ofold_order_idinOrderReplaceV1is correctly implemented and well-positioned.proto/dydxprotocol/indexer/events/events.proto (1)
285-288: The addition ofold_order_idinLongTermOrderReplacementV1is correctly implemented and well-positioned.indexer/packages/v4-protos/src/codegen/dydxprotocol/indexer/off_chain_updates/off_chain_updates.ts (1)
281-294: The addition ofoldOrderIdinOrderReplaceV1andOrderReplaceV1SDKTypeinterfaces is correctly implemented. The field is properly initialized, and the encoding/decoding functions are updated accordingly.indexer/packages/v4-protos/src/codegen/dydxprotocol/indexer/events/events.ts (1)
722-733: Ensure the newoldOrderIdfield is properly documented and integrated.The addition of the
oldOrderIdfield in both the TypeScript interface and its SDK type is consistent with the PR's objective to handle order replacements more effectively by tracking old order IDs. This change should help maintain the integrity of order tracking and replacement within the system.
| if (message.oldOrderId !== undefined) { | ||
| IndexerOrderId.encode(message.oldOrderId, writer.uint32(10).fork()).ldelim(); | ||
| } | ||
|
|
||
| if (message.order !== undefined) { | ||
| IndexerOrder.encode(message.order, writer.uint32(10).fork()).ldelim(); | ||
| IndexerOrder.encode(message.order, writer.uint32(18).fork()).ldelim(); |
There was a problem hiding this comment.
Refactor encoding and decoding logic to improve clarity.
The encoding and decoding logic for the oldOrderId could be simplified or documented better to improve maintainability. Consider adding comments or refactoring for clarity.
2747 if (message.oldOrderId !== undefined) {
2748 IndexerOrderId.encode(message.oldOrderId, writer.uint32(10).fork()).ldelim();
2749 }
2750 // Encode the main order
2751 if (message.order !== undefined) {
2752 IndexerOrder.encode(message.order, writer.uint32(18).fork()).ldelim();
2768 case 1:
2769 message.oldOrderId = IndexerOrderId.decode(reader, reader.uint32());
2770 break;
2771 // Decode the main orderAlso applies to: 2768-2771
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.
| if (message.oldOrderId !== undefined) { | |
| IndexerOrderId.encode(message.oldOrderId, writer.uint32(10).fork()).ldelim(); | |
| } | |
| if (message.order !== undefined) { | |
| IndexerOrder.encode(message.order, writer.uint32(10).fork()).ldelim(); | |
| IndexerOrder.encode(message.order, writer.uint32(18).fork()).ldelim(); | |
| if (message.oldOrderId !== undefined) { | |
| IndexerOrderId.encode(message.oldOrderId, writer.uint32(10).fork()).ldelim(); | |
| } | |
| // Encode the main order | |
| if (message.order !== undefined) { | |
| IndexerOrder.encode(message.order, writer.uint32(18).fork()).ldelim(); | |
| } | |
| // ... other code ... | |
| case 1: | |
| message.oldOrderId = IndexerOrderId.decode(reader, reader.uint32()); | |
| break; | |
| // Decode the main order |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- proto/dydxprotocol/indexer/events/events.proto (1 hunks)
- proto/dydxprotocol/indexer/off_chain_updates/off_chain_updates.proto (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- proto/dydxprotocol/indexer/events/events.proto
- proto/dydxprotocol/indexer/off_chain_updates/off_chain_updates.proto
Changelist
Vault replaces orders and generates a brand new order ID for each replacement order so the order replacement proto needs to include the old order ID that we want to remove, along with the new order.
proto
check-bc-breakingCI step failing is okay because these messages are not currently sent or consumedTest Plan
[Describe how this PR was tested (if applicable)]
Author/Reviewer Checklist
state-breakinglabel.indexer-postgres-breakinglabel.PrepareProposalorProcessProposal, manually add the labelproposal-breaking.feature:[feature-name].backport/[branch-name].refactor,chore,bug.Summary by CodeRabbit
oldOrderIdin order events and off-chain updates. This allows users to track the original order ID during replacements.