[CT-700] separate indexer and grpc streaming events #1209
Conversation
WalkthroughThe recent updates aim to enhance the orderbook update mechanism in a decentralized trading protocol. A new Changes
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 (
|
| if err := subscription.srv.Send( | ||
| &clobtypes.StreamOrderbookUpdatesResponse{ | ||
| Updates: updatesToSend, | ||
| Snapshot: snapshot, | ||
| }, | ||
| ); err != nil { | ||
| idsToRemove = append(idsToRemove, id) | ||
| } |
There was a problem hiding this comment.
send one snapshot for all updates instead of one snapshot / clob pair
|
|
||
| k.GetGrpcStreamingManager().SendOrderbookUpdates(offchainUpdates, false) |
There was a problem hiding this comment.
decouple grpc events and indexer events
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files selected for processing (11)
- protocol/mocks/ClobKeeper.go (1 hunks)
- protocol/mocks/MemClobKeeper.go (1 hunks)
- protocol/streaming/grpc/grpc_streaming_manager.go (1 hunks)
- protocol/testutil/memclob/keeper.go (1 hunks)
- protocol/x/clob/keeper/keeper.go (1 hunks)
- protocol/x/clob/keeper/orders.go (1 hunks)
- protocol/x/clob/memclob/memclob.go (4 hunks)
- protocol/x/clob/memclob/memclob_grpc_streaming.go (5 hunks)
- protocol/x/clob/memclob/memclob_grpc_streaming_test.go (4 hunks)
- protocol/x/clob/types/clob_keeper.go (1 hunks)
- protocol/x/clob/types/mem_clob_keeper.go (1 hunks)
Additional comments: 13
protocol/x/clob/types/mem_clob_keeper.go (1)
- 118-121: The addition of the
SendOrderbookUpdatesmethod to theMemClobKeeperinterface aligns well with the PR objectives of enhancing the efficiency of streaming orderbook updates. The method signature is clear and appropriately takesoffchainUpdatesand asnapshotboolean parameter.protocol/x/clob/memclob/memclob_grpc_streaming_test.go (2)
- 24-24: The call to
clobKeeper.SendOrderbookUpdatesin the test setup aligns with the new method introduced in theMemClobKeeperinterface, ensuring that the tests are updated to reflect the changes in the implementation.- 48-50: Replacing calls to
GetOffchainUpdatesForOrderwithGetOrderbookUpdatesForOrderPlacementin the test assertions correctly reflects the renaming and functionality update in the implementation. This ensures that the tests remain relevant and accurate.protocol/x/clob/types/clob_keeper.go (1)
- 147-152: The addition of the
InitializeNewGrpcStreamsandSendOrderbookUpdatesmethods to theClobKeeperinterface aligns well with the PR objectives of enhancing the efficiency of streaming orderbook updates and managing gRPC streams. The method signatures are clear and appropriately defined.protocol/x/clob/memclob/memclob_grpc_streaming.go (2)
- 59-62: Renaming
GetOffchainUpdatesForOrdertoGetOrderbookUpdatesForOrderPlacementimproves clarity and aligns with the functionality of generating updates for order placements. This change enhances the readability and understanding of the code.- 92-128: The addition of
GetOrderbookUpdatesForOrderRemovalandGetOrderbookUpdatesForOrderUpdatefunctions is a significant improvement, providing specific functionalities for handling order removals and updates. These functions contribute to the overall goal of enhancing the efficiency and organization of streaming orderbook updates.protocol/streaming/grpc/grpc_streaming_manager.go (1)
- 106-120: The modifications to the
SendOrderbookUpdatesmethod, specifically the accumulation of updates before sending them to subscribers, align with the PR objectives of enhancing the efficiency of streaming orderbook updates. This approach can improve performance by reducing the frequency and volume of data transmitted.protocol/x/clob/keeper/keeper.go (2)
- 236-237: The modification in the
InitializeNewGrpcStreamsmethod to callSendOrderbookUpdatesof theKeeperstruct itself instead of thestreamingManagerdirectly is a good architectural change. It encapsulates the functionality within theKeeperand provides a clear separation of concerns.- 239-244: The addition of the
SendOrderbookUpdatesmethod in theKeeperstruct is necessary for the architectural change made in theInitializeNewGrpcStreamsmethod. This method aligns with the PR objectives of enhancing the efficiency of streaming orderbook updates.protocol/testutil/memclob/keeper.go (1)
- 515-519: The method
SendOrderbookUpdateshas been added to theFakeMemClobKeeperstruct but currently has an empty implementation. This is expected for mock or fake implementations used in testing. However, it's crucial to ensure that this method aligns with the interface it's supposed to mock or mimic, in terms of parameters and return types. If this method is intended for future expansion or specific testing scenarios, consider adding a comment to clarify its purpose or future intentions.protocol/mocks/MemClobKeeper.go (1)
- 437-440: The addition of the
SendOrderbookUpdatesmock function to theMemClobKeeperstruct is consistent with the changes outlined in the PR objectives. This mock function is essential for testing the newSendOrderbookUpdatesfunctionality in various components. It's important to ensure that the parameters and the way this mock function is set up align with the actual implementation ofSendOrderbookUpdatesin the corresponding interfaces. This ensures that tests using this mock will accurately reflect real-world usage and behavior.protocol/mocks/ClobKeeper.go (1)
- 1133-1136: The addition of the
SendOrderbookUpdatesmock function aligns with the PR objectives and seems well-structured for testing purposes. Ensure that this function is adequately utilized in unit tests to simulate the behavior of sending order book updates, both incremental and snapshot types.protocol/x/clob/memclob/memclob.go (1)
- 1989-2001: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1963-1998]
The logic for sending orderbook updates to gRPC streams after removing or updating an order in the orderbook is correctly implemented. However, it's essential to handle potential errors from
SendOrderbookUpdatesmethod calls and log them for debugging purposes. This ensures the reliability of the orderbook streaming functionality and aids in troubleshooting.- m.clobKeeper.SendOrderbookUpdates(orderbookUpdate, false) + if err := m.clobKeeper.SendOrderbookUpdates(orderbookUpdate, false); err != nil { + log.ErrorLog(ctx, "Failed to send orderbook updates for order removal/update", "error", err) + }
protocol/x/clob/memclob/memclob.go
Outdated
| // For order matches in the last block, send an orderbook update | ||
| // to the grpc streams. | ||
| // Note that fully filled orders are removed from the orderbook in `RemoveOrderIfFilled` above. | ||
| allUpdates := types.NewOffchainUpdates() | ||
| for _, orderId := range filledOrderIds { | ||
| if m.openOrders.hasOrder(ctx, orderId) { | ||
| orderbookUpdate := m.GetOrderbookUpdatesForOrderUpdate(ctx, orderId) | ||
| allUpdates.Append(orderbookUpdate) | ||
| } | ||
| } | ||
| m.clobKeeper.SendOrderbookUpdates(allUpdates, false) | ||
|
|
There was a problem hiding this comment.
The logic for sending orderbook updates to gRPC streams after processing filled orders is correctly placed. However, it's important to ensure that the SendOrderbookUpdates method handles potential errors gracefully and logs them for debugging purposes. This is crucial for maintaining the reliability of the orderbook streaming functionality.
- m.clobKeeper.SendOrderbookUpdates(allUpdates, false)
+ if err := m.clobKeeper.SendOrderbookUpdates(allUpdates, false); err != nil {
+ log.ErrorLog(ctx, "Failed to send orderbook updates", "error", err)
+ }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.
| // For order matches in the last block, send an orderbook update | |
| // to the grpc streams. | |
| // Note that fully filled orders are removed from the orderbook in `RemoveOrderIfFilled` above. | |
| allUpdates := types.NewOffchainUpdates() | |
| for _, orderId := range filledOrderIds { | |
| if m.openOrders.hasOrder(ctx, orderId) { | |
| orderbookUpdate := m.GetOrderbookUpdatesForOrderUpdate(ctx, orderId) | |
| allUpdates.Append(orderbookUpdate) | |
| } | |
| } | |
| m.clobKeeper.SendOrderbookUpdates(allUpdates, false) | |
| // For order matches in the last block, send an orderbook update | |
| // to the grpc streams. | |
| // Note that fully filled orders are removed from the orderbook in `RemoveOrderIfFilled` above. | |
| allUpdates := types.NewOffchainUpdates() | |
| for _, orderId := range filledOrderIds { | |
| if m.openOrders.hasOrder(ctx, orderId) { | |
| orderbookUpdate := m.GetOrderbookUpdatesForOrderUpdate(ctx, orderId) | |
| allUpdates.Append(orderbookUpdate) | |
| } | |
| } | |
| if err := m.clobKeeper.SendOrderbookUpdates(allUpdates, false); err != nil { | |
| log.ErrorLog(ctx, "Failed to send orderbook updates", "error", err) | |
| } |
protocol/x/clob/memclob/memclob.go
Outdated
|
|
||
| // Send an orderbook update to grpc streams. | ||
| orderbookUpdate := m.GetOrderbookUpdatesForOrderPlacement(ctx, newOrder) | ||
| m.clobKeeper.SendOrderbookUpdates(orderbookUpdate, false) |
There was a problem hiding this comment.
The logic for sending orderbook updates to gRPC streams after adding an order to the orderbook is correctly implemented. Similar to the previous comment, ensure that potential errors from SendOrderbookUpdates are handled and logged appropriately to maintain the reliability of the orderbook streaming functionality.
- m.clobKeeper.SendOrderbookUpdates(orderbookUpdate, false)
+ if err := m.clobKeeper.SendOrderbookUpdates(orderbookUpdate, false); err != nil {
+ log.ErrorLog(ctx, "Failed to send orderbook updates for order placement", "error", err)
+ }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.
| // Send an orderbook update to grpc streams. | |
| orderbookUpdate := m.GetOrderbookUpdatesForOrderPlacement(ctx, newOrder) | |
| m.clobKeeper.SendOrderbookUpdates(orderbookUpdate, false) | |
| // Send an orderbook update to grpc streams. | |
| orderbookUpdate := m.GetOrderbookUpdatesForOrderPlacement(ctx, newOrder) | |
| if err := m.clobKeeper.SendOrderbookUpdates(orderbookUpdate, false); err != nil { | |
| log.ErrorLog(ctx, "Failed to send orderbook updates for order placement", "error", err) | |
| } |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (2)
- protocol/x/clob/memclob/memclob_purge_invalid_memclob_state_test.go (5 hunks)
- protocol/x/clob/memclob/memclob_remove_order_test.go (1 hunks)
Additional comments: 7
protocol/x/clob/memclob/memclob_purge_invalid_memclob_state_test.go (5)
- 253-256: The changes from lines 253 to 256 introduce a mock setup for
SendOrderbookUpdatesin theTestPurgeInvalidMemclobStatefunction. This is a good practice for isolating the unit under test and controlling its environment. However, it's important to ensure that the mock's behavior closely mimics the real implementation to avoid false positives or negatives in the test results.- 343-346: The changes from lines 343 to 346 repeat the setup for
SendOrderbookUpdatesin theTestPurgeInvalidMemclobState_PanicsWhenCalledWithDuplicateCanceledStatefulOrderIdsfunction. Consistency in mock setup across different test cases is beneficial for maintainability. However, consider abstracting common mock setups into a helper function to reduce duplication and improve readability.- 376-379: The changes from lines 376 to 379 show the mock setup for
SendOrderbookUpdatesin theTestPurgeInvalidMemclobState_PanicsWhenNonStatefulOrderIsCanceledfunction. This is consistent with the previous setups. As mentioned earlier, abstracting common setups into a helper function could enhance the test code's maintainability and readability.- 407-410: The changes from lines 407 to 410 in the
TestPurgeInvalidMemclobState_PanicsWhenCalledWithDuplicateExpiredStatefulOrdersfunction again demonstrate the mock setup forSendOrderbookUpdates. The repetition of this setup across multiple test functions reinforces the suggestion to abstract common setups into a helper function.- 441-444: The changes from lines 441 to 444 in the
TestPurgeInvalidMemclobState_PanicsWhenCalledWithShortTermExpiredStatefulOrdersfunction show the same mock setup forSendOrderbookUpdates. This consistent setup across tests is good practice, but again, consider using a helper function to reduce code duplication and enhance clarity.protocol/x/clob/memclob/memclob_remove_order_test.go (2)
- 333-333: The addition of the
SendOrderbookUpdatesmock expectation is appropriate for testing the new functionality. Ensure that its usage is verified across all relevant test cases to confirm that order book updates are correctly sent when expected.- 333-333: The test cases provided cover a wide range of scenarios for removing orders from the order book. It's recommended to verify that all edge cases are covered and that the assertions correctly reflect the expected outcomes of each test case.
protocol/x/clob/memclob/memclob.go
Outdated
| allUpdates.Append(orderbookUpdate) | ||
| } | ||
| } | ||
| m.clobKeeper.SendOrderbookUpdates(allUpdates, false) |
There was a problem hiding this comment.
Would it make sense to move out of PurgeInvalidMemclobState to a new memclob function called in PCS of abci go? It seems that we depend on filledOrderIds, which is read off of processProposerMatchesEvents.OrderIdsFilledInLastBlock. I don't think sending updates is related to purging memclob state?
| @@ -86,3 +88,41 @@ func (m *MemClobPriceTimePriority) GetOffchainUpdatesForOrder( | |||
|
|
|||
| return offchainUpdates | |||
| } | |||
|
|
|||
| // GetOrderbookUpdatesForOrderRemoval returns a place order offchain message and | |||
There was a problem hiding this comment.
finish comment? same with below
There was a problem hiding this comment.
omg copilot failed me
protocol/x/clob/memclob/memclob.go
Outdated
|
|
||
| // Send an orderbook update to grpc streams. | ||
| orderbookUpdate := m.GetOrderbookUpdatesForOrderPlacement(ctx, newOrder) | ||
| m.clobKeeper.SendOrderbookUpdates(orderbookUpdate, false) |
There was a problem hiding this comment.
i don't feel too strongly about this comment but would it make more sense to push these orderbook updates down a callstack level into the func (m *memclobOpenOrders) mustRemoveOrder memclobOpenOrders functions? Don't think we do this currently but in the future if we ever do direct order removal operations on the memclob open orders function orderbook updates won't be emitted.
There was a problem hiding this comment.
Same with the removal below
There was a problem hiding this comment.
thought about this when I am implementing but memclobOpenOrders does not hold a reference to memclob and therefore does not hold to reference to clob keeper to send messages 😢
There was a problem hiding this comment.
let's keep it that way for now? memclob holding a reference to clob is already kind of ugly in my opinion so don't want to introduce another backwards pointer to the parent
| @@ -86,3 +88,41 @@ func (m *MemClobPriceTimePriority) GetOffchainUpdatesForOrder( | |||
|
|
|||
| return offchainUpdates | |||
| } | |||
|
|
|||
| // GetOrderbookUpdatesForOrderRemoval returns a place order offchain message and | |||
| func (m *MemClobPriceTimePriority) GetOrderbookUpdatesForOrderRemoval( | |||
There was a problem hiding this comment.
In MemclobPriceTimepriority object we store a bool that indicates if we should be emitting offchain events.
// ---- Fields for determining if off-chain update messages should be generated ----
generateOffchainUpdates bool
Can we do the same for orderbook updates? It would be nice to have all these no-op if this isn't a full node with streaming turned on.
| memClob := clobmodulememclob.NewMemClobPriceTimePriority(app.IndexerEventManager.Enabled()) | ||
| memClob.SetGenerateOrderbookUpdates(app.GrpcStreamingManager.Enabled()) |
There was a problem hiding this comment.
we should probably also get rid to the app.IndexerEventManager.Enabled() in the builder method and use the Set... method instead
There was a problem hiding this comment.
hard to add arguments to the constructor method since we have a ton of tests that need to be modified
There was a problem hiding this comment.
Review Status
Actionable comments generated: 4
Configuration used: CodeRabbit UI
Files selected for processing (6)
- protocol/app/app.go (1 hunks)
- protocol/mocks/MemClob.go (1 hunks)
- protocol/x/clob/abci.go (1 hunks)
- protocol/x/clob/memclob/memclob.go (6 hunks)
- protocol/x/clob/memclob/memclob_grpc_streaming.go (5 hunks)
- protocol/x/clob/types/memclob.go (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- protocol/x/clob/memclob/memclob.go
- protocol/x/clob/memclob/memclob_grpc_streaming.go
Additional comments: 1
protocol/app/app.go (1)
- 994-1001: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1-999]
Consider modularizing the initialization logic for different modules and services to improve the readability and maintainability of the code. Breaking down the large
Newfunction into smaller, more focused functions can make the code easier to understand and modify. Additionally, ensure consistent error handling and resource management practices throughout the file to prevent potential resource leaks and unhandled errors.
| clobFlags := clobflags.GetClobFlagValuesFromOptions(appOpts) | ||
| logger.Info("Parsed CLOB flags", "Flags", clobFlags) | ||
|
|
||
| memClob := clobmodulememclob.NewMemClobPriceTimePriority( | ||
| app.IndexerEventManager.Enabled() || app.GrpcStreamingManager.Enabled(), | ||
| ) | ||
| memClob := clobmodulememclob.NewMemClobPriceTimePriority(app.IndexerEventManager.Enabled()) | ||
| memClob.SetGenerateOrderbookUpdates(app.GrpcStreamingManager.Enabled()) | ||
|
|
||
| app.ClobKeeper = clobmodulekeeper.NewKeeper( | ||
| appCodec, |
There was a problem hiding this comment.
📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [726-726]
The gRPC server is initialized without specifying SSL/TLS credentials, which could expose the server to security vulnerabilities such as man-in-the-middle attacks. It's crucial to secure the gRPC connection by including credentials derived from an SSL certificate. You can create credentials using credentials.NewServerTLSFromFile("cert.pem", "cert.key") and include them in the server options.
- app.Server = daemonserver.NewServer(logger, grpc.NewServer(), &daemontypes.FileHandlerImpl{}, daemonFlags.Shared.SocketAddress)
+ creds, err := credentials.NewServerTLSFromFile("cert.pem", "cert.key")
+ if err != nil {
+ panic(err)
+ }
+ app.Server = daemonserver.NewServer(logger, grpc.NewServer(grpc.Creds(creds)), &daemontypes.FileHandlerImpl{}, daemonFlags.Shared.SocketAddress)| GetOrderbookUpdatesForOrderPlacement( | ||
| ctx sdk.Context, | ||
| order Order, | ||
| ) (offchainUpdates *OffchainUpdates) | ||
| GetOrderbookUpdatesForOrderRemoval( | ||
| ctx sdk.Context, | ||
| orderId OrderId, | ||
| ) (offchainUpdates *OffchainUpdates) | ||
| GetOrderbookUpdatesForOrderUpdate( | ||
| ctx sdk.Context, | ||
| orderId OrderId, | ||
| ) (offchainUpdates *OffchainUpdates) |
There was a problem hiding this comment.
Consider adding documentation comments for the newly introduced methods GetOrderbookUpdatesForOrderPlacement, GetOrderbookUpdatesForOrderRemoval, and GetOrderbookUpdatesForOrderUpdate. Documentation is crucial for maintaining code readability and helping other developers understand the purpose and usage of these methods.
| // For orders that are filled in the last block, send an orderbook update to the grpc streams. | ||
| if keeper.GetGrpcStreamingManager().Enabled() { | ||
| allUpdates := types.NewOffchainUpdates() | ||
| for _, orderId := range processProposerMatchesEvents.OrderIdsFilledInLastBlock { | ||
| if _, exists := keeper.MemClob.GetOrder(ctx, orderId); exists { | ||
| orderbookUpdate := keeper.MemClob.GetOrderbookUpdatesForOrderUpdate(ctx, orderId) | ||
| allUpdates.Append(orderbookUpdate) | ||
| } | ||
| } | ||
| keeper.SendOrderbookUpdates(allUpdates, false) | ||
| } |
There was a problem hiding this comment.
The changes from lines 169 to 179 introduce a loop to send orderbook updates for orders filled in the last block. Consider the following improvements:
- Add error handling for the retrieval of orderbook updates in case
GetOrderbookUpdatesForOrderUpdatefails. - Evaluate the performance impact of this loop, especially in scenarios with a high volume of orders. Consider batching updates or other optimization strategies if necessary.
| // GetOrderbookUpdatesForOrderPlacement provides a mock function with given fields: ctx, order | ||
| func (_m *MemClob) GetOrderbookUpdatesForOrderPlacement(ctx types.Context, order clobtypes.Order) *clobtypes.OffchainUpdates { | ||
| ret := _m.Called(ctx, order) | ||
|
|
||
| if len(ret) == 0 { | ||
| panic("no return value specified for GetOrderbookUpdatesForOrderPlacement") | ||
| } | ||
|
|
||
| var r0 *clobtypes.OffchainUpdates | ||
| if rf, ok := ret.Get(0).(func(types.Context, clobtypes.Order) *clobtypes.OffchainUpdates); ok { | ||
| r0 = rf(ctx, order) | ||
| } else { | ||
| if ret.Get(0) != nil { | ||
| r0 = ret.Get(0).(*clobtypes.OffchainUpdates) | ||
| } | ||
| } | ||
|
|
||
| return r0 | ||
| } | ||
|
|
||
| // GetOrderbookUpdatesForOrderRemoval provides a mock function with given fields: ctx, orderId | ||
| func (_m *MemClob) GetOrderbookUpdatesForOrderRemoval(ctx types.Context, orderId clobtypes.OrderId) *clobtypes.OffchainUpdates { | ||
| ret := _m.Called(ctx, orderId) | ||
|
|
||
| if len(ret) == 0 { | ||
| panic("no return value specified for GetOrderbookUpdatesForOrderRemoval") | ||
| } | ||
|
|
||
| var r0 *clobtypes.OffchainUpdates | ||
| if rf, ok := ret.Get(0).(func(types.Context, clobtypes.OrderId) *clobtypes.OffchainUpdates); ok { | ||
| r0 = rf(ctx, orderId) | ||
| } else { | ||
| if ret.Get(0) != nil { | ||
| r0 = ret.Get(0).(*clobtypes.OffchainUpdates) | ||
| } | ||
| } | ||
|
|
||
| return r0 | ||
| } | ||
|
|
||
| // GetOrderbookUpdatesForOrderUpdate provides a mock function with given fields: ctx, orderId | ||
| func (_m *MemClob) GetOrderbookUpdatesForOrderUpdate(ctx types.Context, orderId clobtypes.OrderId) *clobtypes.OffchainUpdates { | ||
| ret := _m.Called(ctx, orderId) | ||
|
|
||
| if len(ret) == 0 { | ||
| panic("no return value specified for GetOrderbookUpdatesForOrderUpdate") | ||
| } | ||
|
|
||
| var r0 *clobtypes.OffchainUpdates | ||
| if rf, ok := ret.Get(0).(func(types.Context, clobtypes.OrderId) *clobtypes.OffchainUpdates); ok { | ||
| r0 = rf(ctx, orderId) | ||
| } else { | ||
| if ret.Get(0) != nil { | ||
| r0 = ret.Get(0).(*clobtypes.OffchainUpdates) | ||
| } | ||
| } | ||
|
|
||
| return r0 | ||
| } |
There was a problem hiding this comment.
Consider adding documentation comments for the mock functions GetOrderbookUpdatesForOrderPlacement, GetOrderbookUpdatesForOrderRemoval, and GetOrderbookUpdatesForOrderUpdate. While these are mock functions, documentation can improve readability and help developers understand their purpose and usage in tests.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (2)
- protocol/x/clob/keeper/keeper.go (1 hunks)
- protocol/x/clob/memclob/memclob.go (6 hunks)
Files skipped from review as they are similar to previous changes (2)
- protocol/x/clob/keeper/keeper.go
- protocol/x/clob/memclob/memclob.go
* [CT-700] separate indexer and grpc streaming events * fix tests * comments * update
* [CT-700] separate indexer and grpc streaming events * fix tests * comments * update
* [CT-645] Move off chain updates and v1 to a different package (#1131) * [CT-645] Add protos for orderbook stream query service * move removal reasons to a separate package * [CT-645] Add protos for orderbook stream query service (#1133) * [CT-645] Add protos for orderbook stream query service * make update not nullable * fix build * [CT-644] instantiate grpc stream manager (#1134) * [CT-644] instantiate grpc stream manager * update type * update channel type * [CT-646] stream offchain updates through stream manager (#1138) * [CT-646] stream offchain updates through stream manager * comments * fix lint * get rid of finished * comments * comments * [CT-652] add command line flag for full node streaming (#1145) * [CT-647] construct the initial orderbook snapshot (#1147) * [CT-647] construct the initial orderbook snapshot * [CT-647] initialize new streams and send orderbook snapshot (#1152) * [CT-647] initialize new streams and send orderbook snapshot * use sync once * comments * [CT-700] separate indexer and grpc streaming events (#1209) * [CT-700] separate indexer and grpc streaming events * fix tests * comments * update * [CT-700] only send response when there is at least one update (#1216) * [CT-712] send order update when short term order state fill amounts are pruned (#1241) * [CT-712] send fill amount updates for reverted operations (#1240) * [CT-723] add block number + stage to grpc updates (#1252) * [CT-723] add block number + stage to grpc updates * add indexer changes * [CT-727] avoid state reads when sending updates (#1261)
…#1269) * [CT-700] separate indexer and grpc streaming events (#1209) * [CT-700] separate indexer and grpc streaming events * fix tests * comments * update * [CT-700] only send response when there is at least one update (#1216) * [CT-712] send order update when short term order state fill amounts are pruned (#1241) * [CT-712] send fill amount updates for reverted operations (#1240) * [CT-723] add block number + stage to grpc updates (#1252) * [CT-723] add block number + stage to grpc updates * add indexer changes * [CT-727] avoid state reads when sending updates (#1261) * fix lint| * [CT-712] send updates for both normal order matches and liquidation (#1280) * fix test * fix test * update type
* [CT-645] Move off chain updates and v1 to a different package (#1131) * [CT-645] Add protos for orderbook stream query service * move removal reasons to a separate package * [CT-645] Add protos for orderbook stream query service (#1133) * [CT-645] Add protos for orderbook stream query service * make update not nullable * fix build * [CT-644] instantiate grpc stream manager (#1134) * [CT-644] instantiate grpc stream manager * update type * update channel type * [CT-646] stream offchain updates through stream manager (#1138) * [CT-646] stream offchain updates through stream manager * comments * fix lint * get rid of finished * comments * comments * [CT-652] add command line flag for full node streaming (#1145) * [CT-647] construct the initial orderbook snapshot (#1147) * [CT-647] construct the initial orderbook snapshot * [CT-647] initialize new streams and send orderbook snapshot (#1152) * [CT-647] initialize new streams and send orderbook snapshot * use sync once * comments * [CT-700] separate indexer and grpc streaming events (#1209) * [CT-700] separate indexer and grpc streaming events * fix tests * comments * update * [CT-700] only send response when there is at least one update (#1216) * [CT-712] send order update when short term order state fill amounts are pruned (#1241) * [CT-712] send fill amount updates for reverted operations (#1240) * [CT-723] add block number + stage to grpc updates (#1252) * [CT-723] add block number + stage to grpc updates * add indexer changes * [CT-727] avoid state reads when sending updates (#1261)
* [OTE-221] Add query for PendingSendPacket (backport #1176) (#1221) --------- Co-authored-by: Teddy Ding <teddy@dydx.exchange> (cherry picked from commit e545bbf) # Conflicts: # indexer/packages/v4-protos/src/codegen/dydxprotocol/bundle.ts # indexer/packages/v4-protos/src/codegen/gogoproto/bundle.ts # indexer/packages/v4-protos/src/codegen/google/bundle.ts # protocol/go.mod * fix protos * update go.mod --------- Co-authored-by: Mohammed Affan <affanmd@nyu.edu> Co-authored-by: affan <affan@dydx.exchange> * [Backport v4.x] backport full node streaming to v4.x branch (#1270) * [CT-645] Move off chain updates and v1 to a different package (#1131) * [CT-645] Add protos for orderbook stream query service * move removal reasons to a separate package * [CT-645] Add protos for orderbook stream query service (#1133) * [CT-645] Add protos for orderbook stream query service * make update not nullable * fix build * [CT-644] instantiate grpc stream manager (#1134) * [CT-644] instantiate grpc stream manager * update type * update channel type * [CT-646] stream offchain updates through stream manager (#1138) * [CT-646] stream offchain updates through stream manager * comments * fix lint * get rid of finished * comments * comments * [CT-652] add command line flag for full node streaming (#1145) * [CT-647] construct the initial orderbook snapshot (#1147) * [CT-647] construct the initial orderbook snapshot * [CT-647] initialize new streams and send orderbook snapshot (#1152) * [CT-647] initialize new streams and send orderbook snapshot * use sync once * comments * [CT-700] separate indexer and grpc streaming events (#1209) * [CT-700] separate indexer and grpc streaming events * fix tests * comments * update * [CT-700] only send response when there is at least one update (#1216) * [CT-712] send order update when short term order state fill amounts are pruned (#1241) * [CT-712] send fill amount updates for reverted operations (#1240) * [CT-723] add block number + stage to grpc updates (#1252) * [CT-723] add block number + stage to grpc updates * add indexer changes * [CT-727] avoid state reads when sending updates (#1261) * [CT-712] send updates for both normal order matches and liquidation (#1280) (#1281) * Fix lib.ErrorLogWithError: properly pass in args (#1306) (#1310) (cherry picked from commit a91c1ca) Co-authored-by: Jonathan Fung <121899091+jonfung-dydx@users.noreply.github.com> * fix broken tests (#1312) (#1316) (cherry picked from commit 5ec37d2) Co-authored-by: Jonathan Fung <121899091+jonfung-dydx@users.noreply.github.com> --------- Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: Mohammed Affan <affanmd@nyu.edu> Co-authored-by: affan <affan@dydx.exchange> Co-authored-by: jayy04 <103467857+jayy04@users.noreply.github.com> Co-authored-by: Jonathan Fung <121899091+jonfung-dydx@users.noreply.github.com>
Changelist
[Describe or list the changes made in this PR]
Test 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.