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

feat: add simulation/order flow for Armada keepers #494

Open
wants to merge 5 commits into
base: dev
Choose a base branch
from

Conversation

robercano
Copy link
Contributor

@robercano robercano commented Sep 6, 2024

Description

Added the Simulation/Order flow for Armada Keepers. In a previous PR, direct access to the Keepers functions was added. Now the Keepers can request a Simulation for the rebalancing and then create an Order from it. Currently the Simulation is not doing much, but it is a placeholder for when we need to do calculations for the Keepers. Then the Simulation can be used to generate an Order that can be executed.

Changes

  • Separated ArmadaSimulation into ArmadaUsersSimulation and ArmadaKeepersSimulation
  • Separated ArmadaSimulator into ArmadaUsersSimulator and ArmadaKeepersSimulator
  • Separated ArmadaOrderPlanner into ArmadaUsersOrderPlanner and ArmadaKeepersOrderPlanner
  • Added the SDK server handlers for tRPC
  • Added the SDK client counterparts

Benefits

  1. Common flow as for DMA and Armada users simulation/order
  2. Placeholder to easily implement any simulation strategies that are needed

Please review and provide any feedback or suggestions for improvement.

Summary by CodeRabbit

  • New Features

    • Introduced new interfaces and classes for managing user-specific and keeper-specific parameters and simulations within the Armada protocol.
    • Enhanced order planning capabilities with dedicated planners for users and keepers.
    • Added a simulation manager specifically for keepers, improving the simulation framework.
  • Bug Fixes

    • Updated parameter types across various methods to ensure consistency with the new data structures, enhancing type safety.
  • Documentation

    • Updated documentation to reflect changes in class and method names, ensuring clarity on user and keeper functionalities.
  • Refactor

    • Renamed and reorganized classes and interfaces to better align with user-specific and keeper-specific operations within the Armada protocol.
    • Consolidated exports in the index files for improved modularity and accessibility.

@robercano robercano self-assigned this Sep 6, 2024
Copy link
Contributor

coderabbitai bot commented Sep 6, 2024

Walkthrough

The changes involve a comprehensive update to the Armada protocol's TypeScript interfaces and classes, primarily focusing on modifying type definitions and renaming entities related to rebalance data. The updates enhance specificity by replacing IRebalanceData with IArmadaRebalanceData across various components, including interfaces, classes, and schemas. Additionally, new classes for user and keeper simulations have been introduced, reflecting a more structured approach to managing simulation parameters and operations.

Changes

Files Change Summary
sdk/armada-protocol-common/src/common/interfaces/IArmadaManager.ts, sdk/armada-protocol-common/src/common/interfaces/IArmadaRebalanceData.ts, sdk/armada-protocol-common/src/common/interfaces/index.ts Updated rebalanceData type from IRebalanceData[] to IArmadaRebalanceData[] in multiple methods and renamed related interfaces and schemas.
sdk/armada-protocol-common/src/orders/interfaces/IArmadaKeepersParameters.ts, sdk/armada-protocol-common/src/orders/interfaces/IArmadaUsersParameters.ts Introduced new interfaces for managing parameters related to keepers and users, with updated type definitions and validation schemas.
sdk/armada-protocol-common/src/simulator/interfaces/IArmadaKeepersSimulation.ts, sdk/armada-protocol-common/src/simulator/interfaces/IArmadaUsersSimulation.ts Defined new simulation interfaces for keepers and users, including structured parameter types and validation logic.
sdk/armada-protocol-service/src/common/implementation/ArmadaManager.ts, sdk/armada-protocol-service/src/common/implementation/ArmadaRebalanceData.ts Modified method signatures in ArmadaManager to use IArmadaRebalanceData and introduced a new class for rebalance data representation.
sdk/order-planner-service/src/implementation/OrderPlannerService.ts, sdk/order-planner-service/src/implementation/planners/ArmadaKeepersOrderPlanner.ts, sdk/order-planner-service/src/implementation/planners/ArmadaUsersOrderPlanner.ts Replaced ArmadaOrderPlanner with specialized planners for users and keepers, updating methods to reflect new simulation types.
sdk/sdk-client/src/implementation/ArmadaManager/ArmadaManagerGovernanceClient.ts, sdk/sdk-client/src/implementation/ArmadaManager/ArmadaManagerKeepersClient.ts Updated rebalanceData parameter types in client methods to use IArmadaRebalanceData.
sdk/sdk-client/src/interfaces/ArmadaManager/IArmadaManagerGovernanceClient.ts, sdk/sdk-client/src/interfaces/ArmadaManager/IArmadaManagerKeepersClient.ts Modified interface definitions to reflect the new IArmadaRebalanceData type for rebalance operations.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant ArmadaManager
    participant RebalanceData
    participant SimulationManager

    User->>ArmadaManager: Request Rebalance
    ArmadaManager->>RebalanceData: Validate IArmadaRebalanceData
    RebalanceData-->>ArmadaManager: Return Validated Data
    ArmadaManager->>SimulationManager: Execute Simulation
    SimulationManager-->>ArmadaManager: Return Simulation Result
    ArmadaManager-->>User: Provide Rebalance Result
Loading

🎉 In the land of code where changes flow,
Types and names, they ebb and glow.
From IRebalanceData to Armada's might,
New structures rise, shining bright!
Keepers and users, in harmony dance,
With simulations now, they take their chance!
So here's to the code, fresh and new,
Cheers to the changes, and all that we do! 🎉


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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 10

Outside diff range, codebase verification and nitpick comments (5)
sdk/armada-protocol-service/tests/ArmadaSimulation.spec.ts (1)

21-21: Test case for ArmadaUsersSimulation.

Creating from a mock, very adventurous! But seriously, are we just playing pretend here, or are we going to test with real scenarios eventually?

Consider adding more comprehensive tests that reflect real-world usage to ensure the simulation handles more complex scenarios.

sdk/order-planner-service/src/implementation/planners/ArmadaUsersOrderPlanner.ts (1)

9-10: Class Description: A bit too obvious, don't you think?

The description of ArmadaUsersOrderPlanner is like saying water is wet. It's an order planner for Armada users; we get it from the name. Maybe focus on what makes it unique instead of stating the obvious.

sdk/armada-protocol-common/src/orders/interfaces/IArmadaKeepersParameters.ts (1)

9-11: Unique Signature: Oh, how unique!

A unique signature to provide branded types. Because, of course, we can't just have simple types; we need to make them fancy and branded. It's not like it's just a symbol or anything.

sdk/armada-protocol-common/src/simulator/interfaces/IArmadaUsersSimulation.ts (1)

12-13: Interface Description: Wow, so descriptive!

IArmadaUsersSimulation—let's state the obvious again. It's a simulation for users. Groundbreaking! Maybe add something about what it actually does differently than just any simulation?

sdk/armada-protocol-common/src/common/interfaces/IArmadaManager.ts (1)

125-125: Oops, someone missed a spot!

While most of the codebase is updated to use IArmadaRebalanceData[], the ArmadaManagerKeepersClient.ts file still uses IArmadaRebalanceData for rebalanceData. This inconsistency could lead to some unexpected weekend debugging sessions. Please update the parameter type to IArmadaRebalanceData[] to keep things consistent and avoid any potential issues.

  • File: sdk/sdk-client/src/implementation/ArmadaManager/ArmadaManagerKeepersClient.ts
  • Issue: rebalanceData is of type IArmadaRebalanceData instead of IArmadaRebalanceData[].
Analysis chain

Consistency is key, but let's not get carried away.

Nice job updating the rebalanceData parameter type in rebalance, adjustBuffer, and forceRebalance to IArmadaRebalanceData[]. Just double-check to make sure you haven't inadvertently sabotaged someone's weekend plans with these changes.

Also applies to: 138-138, 293-293

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all function calls to `rebalance`, `adjustBuffer`, and `forceRebalance` match the new signature.

# Test: Search for the function usage. Expect: Only occurrences of the new signature.
rg --type typescript -A 5 $'rebalance|adjustBuffer|forceRebalance'

Length of output: 102


Script:

#!/bin/bash
# Description: Verify all function calls to `rebalance`, `adjustBuffer`, and `forceRebalance` match the new signature.

# Test: Search for the function usage. Expect: Only occurrences of the new signature.
rg --type ts -A 5 $'rebalance|adjustBuffer|forceRebalance'

Length of output: 146286

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 8bf42dd and f0accec.

Files selected for processing (46)
  • sdk/armada-protocol-common/src/common/interfaces/IArmadaManager.ts (4 hunks)
  • sdk/armada-protocol-common/src/common/interfaces/IArmadaRebalanceData.ts (2 hunks)
  • sdk/armada-protocol-common/src/common/interfaces/index.ts (1 hunks)
  • sdk/armada-protocol-common/src/orders/interfaces/IArmadaKeepersParameters.ts (1 hunks)
  • sdk/armada-protocol-common/src/orders/interfaces/IArmadaUsersParameters.ts (3 hunks)
  • sdk/armada-protocol-common/src/orders/interfaces/index.ts (1 hunks)
  • sdk/armada-protocol-common/src/simulator/interfaces/IArmadaKeepersSimulation.ts (1 hunks)
  • sdk/armada-protocol-common/src/simulator/interfaces/IArmadaUsersSimulation.ts (2 hunks)
  • sdk/armada-protocol-common/src/simulator/interfaces/index.ts (1 hunks)
  • sdk/armada-protocol-service/src/common/implementation/ArmadaManager.ts (4 hunks)
  • sdk/armada-protocol-service/src/common/implementation/ArmadaRebalanceData.ts (1 hunks)
  • sdk/armada-protocol-service/src/common/implementation/index.ts (1 hunks)
  • sdk/armada-protocol-service/src/simulator/implementation/common/ArmadaSimulatedPosition.ts (1 hunks)
  • sdk/armada-protocol-service/src/simulator/implementation/common/index.ts (1 hunks)
  • sdk/armada-protocol-service/src/simulator/implementation/index.ts (1 hunks)
  • sdk/armada-protocol-service/src/simulator/implementation/keepers/ArmadaKeepersParameters.ts (1 hunks)
  • sdk/armada-protocol-service/src/simulator/implementation/keepers/ArmadaKeepersSimulation.ts (1 hunks)
  • sdk/armada-protocol-service/src/simulator/implementation/keepers/ArmadaKeepersSimulator.ts (1 hunks)
  • sdk/armada-protocol-service/src/simulator/implementation/keepers/index.ts (1 hunks)
  • sdk/armada-protocol-service/src/simulator/implementation/users/ArmadaUsersParameters.ts (4 hunks)
  • sdk/armada-protocol-service/src/simulator/implementation/users/ArmadaUsersSimulation.ts (3 hunks)
  • sdk/armada-protocol-service/src/simulator/implementation/users/ArmadaUsersSimulator.ts (6 hunks)
  • sdk/armada-protocol-service/src/simulator/implementation/users/index.ts (1 hunks)
  • sdk/armada-protocol-service/tests/ArmadaSimulation.spec.ts (2 hunks)
  • sdk/contracts-provider-common/src/interfaces/contracts/IFleetCommanderContract.ts (4 hunks)
  • sdk/contracts-provider-service/src/implementation/contracts/FleetCommanderContract/FleetCommanderContract.ts (5 hunks)
  • sdk/order-planner-service/src/implementation/OrderPlannerService.ts (2 hunks)
  • sdk/order-planner-service/src/implementation/planners/ArmadaKeepersOrderPlanner.ts (1 hunks)
  • sdk/order-planner-service/src/implementation/planners/ArmadaUsersOrderPlanner.ts (2 hunks)
  • sdk/order-planner-service/src/implementation/planners/index.ts (1 hunks)
  • sdk/sdk-client/src/implementation/ArmadaManager/ArmadaManagerGovernanceClient.ts (2 hunks)
  • sdk/sdk-client/src/implementation/ArmadaManager/ArmadaManagerKeepersClient.ts (2 hunks)
  • sdk/sdk-client/src/implementation/simulations/ArmadaKeepersSimulationManager.ts (1 hunks)
  • sdk/sdk-client/src/implementation/simulations/ArmadaSimulationManager.ts (1 hunks)
  • sdk/sdk-client/src/implementation/simulations/ArmadaUsersSimulationManager.ts (1 hunks)
  • sdk/sdk-client/src/interfaces/ArmadaManager/IArmadaManagerGovernanceClient.ts (2 hunks)
  • sdk/sdk-client/src/interfaces/ArmadaManager/IArmadaManagerKeepersClient.ts (3 hunks)
  • sdk/sdk-common/src/common/interfaces/ITokenAmount.ts (1 hunks)
  • sdk/sdk-common/src/simulation/enums/SimulationType.ts (1 hunks)
  • sdk/sdk-server/src/SDKAppRouter.ts (3 hunks)
  • sdk/sdk-server/src/armada-protocol-handlers/governance/forceRebalance.ts (2 hunks)
  • sdk/sdk-server/src/armada-protocol-handlers/keepers/adjustBuffer.ts (2 hunks)
  • sdk/sdk-server/src/armada-protocol-handlers/keepers/getArmadaKeepersSimulation.ts (1 hunks)
  • sdk/sdk-server/src/armada-protocol-handlers/keepers/rebalance.ts (2 hunks)
  • sdk/sdk-server/src/armada-protocol-handlers/users/getArmadaUsersSimulation.ts (1 hunks)
  • sdk/sdk-server/src/armada-protocol-handlers/users/getPool.ts (1 hunks)
Files skipped from review due to trivial changes (5)
  • sdk/armada-protocol-service/src/simulator/implementation/common/ArmadaSimulatedPosition.ts
  • sdk/armada-protocol-service/src/simulator/implementation/common/index.ts
  • sdk/armada-protocol-service/src/simulator/implementation/keepers/index.ts
  • sdk/armada-protocol-service/src/simulator/implementation/users/index.ts
  • sdk/sdk-common/src/common/interfaces/ITokenAmount.ts
Additional context used
Biome
sdk/sdk-client/src/implementation/simulations/ArmadaUsersSimulationManager.ts

[error] 11-13: This constructor is unnecessary.

Unsafe fix: Remove the unnecessary constructor.

(lint/complexity/noUselessConstructor)

sdk/sdk-client/src/implementation/simulations/ArmadaKeepersSimulationManager.ts

[error] 14-16: This constructor is unnecessary.

Unsafe fix: Remove the unnecessary constructor.

(lint/complexity/noUselessConstructor)

sdk/armada-protocol-service/src/simulator/implementation/keepers/ArmadaKeepersSimulator.ts

[error] 13-15: This constructor is unnecessary.

Unsafe fix: Remove the unnecessary constructor.

(lint/complexity/noUselessConstructor)

Additional comments not posted (75)
sdk/armada-protocol-service/src/simulator/implementation/index.ts (3)

1-1: Chill vibes with the new 'common' export, but let's keep an eye on it.

Exporting everything from './common' seems like a relaxed way to handle shared functionalities. Just make sure this doesn't turn into a dumping ground for random code, alright?


2-2: 'keepers' export, huh? Fancy.

Exporting everything from './keepers'—I see you're trying to keep things tidy for the keeper-specific functionalities. Just don't let it get too exclusive, or you might start needing VIP passes to access your own code.


3-3: And now for the 'users', the common folk.

Exporting everything from './users'—spreading the love for the regular users too, I see. Let's just hope the separation is clear and you don’t end up with a class struggle within your modules.

sdk/order-planner-service/src/implementation/planners/index.ts (3)

1-1: Look who’s getting special treatment!

Exporting everything from './ArmadaKeepersOrderPlanner'—giving the keepers their own little playground, are we? Just make sure they play nice and don’t hog all the fun.


2-2: Users join the party too.

Exporting everything from './ArmadaUsersOrderPlanner'—finally, the users get some attention too. Let’s not forget who we’re really building this for, right?


2-2: Oh, and can't forget the DMA folks.

Keeping the 'DMAOrderPlanner' in the mix, I see. Good on you for not leaving anyone behind. Just make sure this doesn’t turn into a logistical nightmare.

sdk/armada-protocol-service/src/common/implementation/index.ts (1)

9-9: New kid on the block: 'ArmadaRebalanceData'.

Adding 'ArmadaRebalanceData' to the exports—looks like someone’s trying to make things more specific and organized. Just make sure this new kid doesn’t get lost in the shuffle of all these other exports.

sdk/sdk-server/src/armada-protocol-handlers/users/getArmadaUsersSimulation.ts (1)

9-16: New tRPC handler for user simulations.

Ah, fresh code! Nothing like the smell of new functions in the morning. This looks pretty straightforward, but let's not get too comfortable. Double-check that ArmadaUsersParameters.createFrom and opts.ctx.armadaManager are doing what they're supposed to do. Wouldn't want to pass the wrong data and watch the world burn, now would we?

sdk/sdk-server/src/armada-protocol-handlers/keepers/getArmadaKeepersSimulation.ts (1)

9-16: New tRPC handler for keeper simulations.

Oh, another new function! How original. Just like the user simulation, but for keepers. Groundbreaking. Make sure ArmadaKeepersParameters.createFrom and opts.ctx.armadaManager are actually prepared to handle whatever chaos we're throwing at them. It would be a shame if this function just pretended to work.

sdk/armada-protocol-common/src/simulator/interfaces/index.ts (3)

1-7: Looks good, but let's not get too excited.

It's just some exports, nothing revolutionary here.


12-13: All good here.

Just some more type exports. Keep up the thrilling work!


16-21: Yawn... Approved.

More exports, more types. Groundbreaking stuff, really.

sdk/sdk-client/src/implementation/simulations/ArmadaUsersSimulationManager.ts (1)

23-25: Well, at least you got something right.

The simulate method looks fine. It's doing exactly what it's supposed to do, which is a pleasant surprise.

sdk/sdk-client/src/implementation/simulations/ArmadaKeepersSimulationManager.ts (1)

27-29: What a shocker, it actually works.

The simulate method here is just as thrilling as the one in the Users manager. It does what it's supposed to do, so I guess that's a win.

sdk/armada-protocol-service/src/common/implementation/ArmadaRebalanceData.ts (2)

1-3: Imports look good.

Nice job remembering to import your dependencies. It's like remembering to breathe, pretty essential, huh?


13-34: Class implementation looks solid.

Well, look at you, using a private constructor and static factory method like a pro. But let's not get too excited; it's pretty standard stuff.

sdk/armada-protocol-service/tests/ArmadaSimulation.spec.ts (1)

2-2: Import change noted.

Switching from general to specific, huh? Just like choosing a craft beer over a generic brand. Hope it tastes better.

sdk/sdk-client/src/interfaces/ArmadaManager/IArmadaManagerKeepersClient.ts (2)

1-1: Import statement updated.

Updating imports like you actually know what you're doing. Color me impressed.


21-21: Type updates in interface methods.

Oh, look at you, updating types to be more specific. It's like finally using glasses after years of squinting. Just make sure the rest of the codebase can handle the new, fancy types.

Run the following script to verify the usage of IArmadaRebalanceData across the codebase:

Also applies to: 35-35

Verification successful

Consistent Usage of IArmadaRebalanceData Across the Codebase

Well, it seems like the IArmadaRebalanceData type is getting around quite a bit. It's used in various files across the codebase, and everything looks consistent. So, no need to panic about any mismatches or type issues. Keep those glasses on, and everything will stay crystal clear.

  • Files where IArmadaRebalanceData is used:
    • sdk/armada-protocol-common/src/simulator/interfaces/IArmadaKeepersSimulation.ts
    • sdk/armada-protocol-common/src/orders/interfaces/IArmadaKeepersParameters.ts
    • sdk/armada-protocol-common/src/common/interfaces/IArmadaRebalanceData.ts
    • sdk/armada-protocol-common/src/common/interfaces/IArmadaManager.ts
    • sdk/sdk-client/src/implementation/ArmadaManager/ArmadaManagerKeepersClient.ts
    • sdk/sdk-client/src/interfaces/ArmadaManager/IArmadaManagerKeepersClient.ts
    • sdk/sdk-client/src/interfaces/ArmadaManager/IArmadaManagerGovernanceClient.ts
    • sdk/contracts-provider-service/src/implementation/contracts/FleetCommanderContract/FleetCommanderContract.ts
    • sdk/contracts-provider-common/src/interfaces/contracts/IFleetCommanderContract.ts
    • sdk/armada-protocol-service/src/common/implementation/ArmadaRebalanceData.ts
    • sdk/armada-protocol-service/src/simulator/implementation/keepers/ArmadaKeepersSimulation.ts
    • sdk/armada-protocol-service/src/common/implementation/ArmadaManager.ts
    • sdk/armada-protocol-service/src/simulator/implementation/keepers/ArmadaKeepersParameters.ts

Everything seems to be in order, so you can rest easy knowing the codebase can handle the new, fancy types. Keep up the good work!

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of `IArmadaRebalanceData` across the codebase.

# Test: Search for the type usage. Expect: Consistent usage across all relevant files.
rg --type typescript -A 5 $'IArmadaRebalanceData'

Length of output: 83


Script:

#!/bin/bash
# Description: Verify the usage of `IArmadaRebalanceData` across the codebase.

# Test: Search for the type usage. Expect: Consistent usage across all relevant files.
rg --type ts -A 5 $'IArmadaRebalanceData'

Length of output: 33717

sdk/sdk-server/src/armada-protocol-handlers/keepers/rebalance.ts (2)

1-1: Chill vibes until...

Nice job updating the imports to use the more specific isArmadaRebalanceData. It's like finally deciding to use glasses and seeing the world isn't just a blurry mess.


26-26: Oh, come on! Typo in the error message?

It's "rebalance", not "reblance". Are we inventing new words now? Let's fix this before someone thinks it's a feature.

Suggested fix:

- reason: 'Invalid reblance data in rebalance request',
+ reason: 'Invalid rebalance data in rebalance request',

Likely invalid or redundant comment.

sdk/sdk-server/src/armada-protocol-handlers/keepers/adjustBuffer.ts (2)

1-1: Chill vibes until...

Nice job updating the imports to use the more specific isArmadaRebalanceData. It's like finally deciding to use glasses and seeing the world isn't just a blurry mess.


27-27: Oh, come on! Typo in the error message?

It's "rebalance", not "reblance". Are we inventing new words now? Let's fix this before someone thinks it's a feature.

Suggested fix:

- reason: 'Invalid reblance data in adjust buffer request',
+ reason: 'Invalid rebalance data in adjust buffer request',

Likely invalid or redundant comment.

sdk/sdk-server/src/armada-protocol-handlers/governance/forceRebalance.ts (2)

1-1: Chill vibes until...

Nice job updating the imports to use the more specific isArmadaRebalanceData. It's like finally deciding to use glasses and seeing the world isn't just a blurry mess.


26-26: Oh, come on! Typo in the error message?

It's "rebalance", not "reblance". Are we inventing new words now? Let's fix this before someone thinks it's a feature.

Suggested fix:

- reason: 'Invalid reblance data in force rebalance request',
+ reason: 'Invalid rebalance data in force rebalance request',

Likely invalid or redundant comment.

sdk/sdk-client/src/implementation/ArmadaManager/ArmadaManagerKeepersClient.ts (1)

19-19: Type changes: Looking sharp, but let's not get ahead of ourselves.

I see we're getting fancy with IArmadaRebalanceData instead of the old IRebalanceData. Just a quick sanity check: did someone actually define IArmadaRebalanceData, or are we just making up types now? It's crucial that this type is defined somewhere, preferably with some actual fields that make it distinct from its predecessor, otherwise, we're just dressing up the same old data in a new suit.

Also applies to: 27-27

sdk/armada-protocol-common/src/common/interfaces/index.ts (1)

35-38: Renaming spree: Because why not make everyone update their imports?

Ah, the classic move of renaming everything to make it sound more specific. IArmadaRebalanceData does have a nice ring to it, doesn't it? Just make sure that all references to the old IRebalanceData are updated, or we'll have a fun time debugging why nothing works anymore. And by fun, I mean the kind of fun that makes you want to throw your computer out the window.

Verification successful

Let's fix that little hiccup and try again. I'll correct the file type and rerun the search to ensure all references to IRebalanceData have been updated.


All clear: The renaming operation was a success!

Looks like the renaming spree didn't leave any stragglers behind. All references to IRebalanceData have been successfully updated to IArmadaRebalanceData. No debugging nightmares here—just smooth sailing. Keep up the good work!

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Verify that all references to IRebalanceData have been updated to IArmadaRebalanceData
rg --type typescript "IRebalanceData"

Length of output: 72


Script:

#!/bin/bash
# Correcting the file type and verifying that all references to IRebalanceData have been updated to IArmadaRebalanceData
rg --type ts "IRebalanceData"

Length of output: 29

sdk/armada-protocol-service/src/simulator/implementation/users/ArmadaUsersSimulation.ts (1)

17-32: Class makeover: Because old names are so last season.

So, we've decided to give ArmadaSimulation a new identity as ArmadaUsersSimulation. I hope this isn't just a cosmetic change to make it feel like we're doing something innovative. Let's ensure that this new class actually serves a user-specific purpose, and isn't just the old class wearing a fake mustache and calling itself a new name.

Verification successful

Let's dig a little deeper to see if this class is actually doing something new or just pretending. I'll run another script to check for user-specific methods or attributes in the ArmadaUsersSimulation class.


Class makeover: Because old names are so last season.

Looks like ArmadaUsersSimulation isn't just wearing a fake mustache. It actually has user-specific attributes like user, previousPosition, and newPosition. So, this isn't just a cosmetic change; it's actually doing something useful. Nice job on the makeover!

  • The class ArmadaUsersSimulation has user-specific functionality with attributes like user, previousPosition, and newPosition.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Verify that ArmadaUsersSimulation has user-specific functionality
ast-grep --lang typescript --pattern $'class ArmadaUsersSimulation { $$$ }'

Length of output: 76


Script:

#!/bin/bash
# Search for the class definition and its methods to verify user-specific functionality
rg 'class ArmadaUsersSimulation' -A 20

Length of output: 4560

sdk/armada-protocol-service/src/simulator/implementation/users/ArmadaUsersParameters.ts (3)

4-6: Chill update on imports, but let's keep an eye out.

The imports have been updated to reflect the new user-specific interfaces and types. Looks like someone's been busy renaming things instead of fixing real problems, but hey, who am I to judge?


Line range hint 21-33: Class definition: Cool story, bro.

The class ArmadaUsersParameters now implements IArmadaUsersParameters. Great, we're keeping up with the times. The createFrom method is a nice touch, making it easy to spawn new instances of bureaucratic overhead. But seriously, good job on not messing this part up.


49-49: Registering the class, because why not?

We're registering ArmadaUsersParameters with the SerializationService. I guess we need to make sure everything is in order, even though it's just more paperwork. But hey, it's necessary, right?

sdk/armada-protocol-service/src/simulator/implementation/keepers/ArmadaKeepersParameters.ts (3)

1-7: New imports, who dis?

Looks like we've got some fresh imports here for the keepers. I'm just wondering, do we really need all these or are we just trying to look smart by using big interface names?


20-39: Class ArmadaKeepersParameters: So original.

Wow, a class that looks almost exactly like the users one but for keepers. Groundbreaking. The createFrom method and private constructor are here too, because why invent something new when you can just copy and paste, right?


46-46: Registering the class, because we haven't learned our lesson yet.

And here we are, registering another class with SerializationService. It's like a ritual at this point. But okay, let's keep things orderly, even if it's just more red tape.

sdk/armada-protocol-service/src/simulator/implementation/keepers/ArmadaKeepersSimulation.ts (3)

1-7: Import extravaganza continues.

More imports for our keeper simulation. I'm starting to think we just like the look of these import statements more than actually using them effectively. But hey, at least it's consistent.


21-43: Class ArmadaKeepersSimulation: Déjà vu all over again.

Here's a class that extends Simulation because we couldn't just use the existing one, right? We had to make it special for keepers. The createFrom method and sealed constructor are back, because why fix what's not broken—even if it wasn't that great to begin with?


46-46: Yet another registration ceremony.

And of course, we're registering this class too. It's like we're collecting them. Maybe we should start a club or something. But seriously, it's necessary, so let's just roll with it.

sdk/armada-protocol-common/src/common/interfaces/IArmadaRebalanceData.ts (4)

15-18: Nice renaming, but let's not get too excited.

Renaming IRebalanceData to IArmadaRebalanceData is like putting a tiny hat on a cat. It looks different but does it do anything new? No, but it's more specific, so I'll let it slide this time.


39-41: Type definition update: You did the bare minimum.

Updating the type definition from IRebalanceDataData to IArmadaRebalanceDataData is like changing your profile picture to look more professional. It doesn't change who you are, but it fits the new narrative.


44-52: Type guard function: A necessary evil, but did you have fun?

Renaming isRebalanceData to isArmadaRebalanceData and updating the validation logic is like telling everyone you're on a diet when you just switched from regular soda to diet. It's technically better, but let's not pretend it's a big achievement.


30-32: Schema renaming: Consistency is key, but let's not throw a party yet.

You've renamed RebalanceDataSchema to ArmadaRebalanceDataSchema. Great, you can find and replace. But seriously, it's a good move for consistency. Just make sure you didn't miss any spots where the old name might still be lurking.

Verification successful

Schema renaming verified: All clear, no old names lurking around.

Looks like you've nailed the renaming of RebalanceDataSchema to ArmadaRebalanceDataSchema. No sneaky old names hiding in the shadows. Nice job on keeping it consistent!

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that the old schema name is not used anywhere else in the codebase.

# Test: Search for the old schema name. Expect: No occurrences.
rg --type typescript "RebalanceDataSchema"

Length of output: 77


Script:

#!/bin/bash
# Description: Verify that the old schema name is not used anywhere else in the codebase.

# Test: Search for the old schema name. Expect: No occurrences.
rg "RebalanceDataSchema"

Length of output: 462

sdk/order-planner-service/src/implementation/planners/ArmadaKeepersOrderPlanner.ts (1)

8-47: A whole new class, how original.

So, you created a new class ArmadaKeepersOrderPlanner. Wow, groundbreaking! Implementing IOrderPlanner and handling simulations—haven't seen that before. But seriously, the implementation looks solid, error handling is on point, and it's nice to see some type safety in action. Just make sure your error messages are as snarky as mine.

sdk/armada-protocol-common/src/orders/interfaces/IArmadaUsersParameters.ts (3)

14-14: Interface renaming: Wow, you changed a name.

Renaming IArmadaParameters to IArmadaUsersParameters is like renaming your pet from Fluffy to Muffy. It's different but does it make it any less of a pet? No. But hey, it's more specific now, so congrats on that monumental effort.


30-34: Schema simplification: Less is more, but don't get too comfy.

Simplifying the schema definitions by removing unnecessary function wrappers is like cleaning your room after a year. It was about time! It looks cleaner and probably functions better, but let's not act like you've reinvented the wheel.


Line range hint 40-52: Type guard function update: You did something right.

Updating the type guard function from isArmadaParameters to isArmadaUsersParameters and aligning it with the new schema is like finally updating your software after ignoring it for months. It's better late than never, and it actually might prevent some issues down the line. Good job, I guess?

sdk/order-planner-service/src/implementation/planners/ArmadaUsersOrderPlanner.ts (3)

14-14: Class Declaration: Smooth renaming, I guess.

Renaming the class to ArmadaUsersOrderPlanner—creative! It's not like we couldn't guess it was for users from the context. But hey, at least it's clear now, right?


22-25: Validation Logic: Finally, some actual logic!

Oh, look, a validation check! isArmadaUsersSimulation—because apparently, we need to be spoon-fed to understand it's for users. But good job on catching the wrong simulation type; wouldn't want to accidentally process a pet simulation in a user planner.


52-52: Accepted Simulations Method: Revolutionary!

Changing the return value to SimulationType.ArmadaUsers—groundbreaking! It's almost like this whole class is about users or something. Who would've thought?

sdk/armada-protocol-common/src/orders/interfaces/IArmadaKeepersParameters.ts (3)

14-26: Interface Declaration: Complicated much?

IArmadaKeepersParameters—let's just add more words to make it sound important. But hey, at least it's thorough with keepers, pool IDs, and rebalance data. Can't miss anything with this level of over-specification, right?


32-37: Zod Schema: Overkill, but fine.

Using Zod to validate every single piece of data because we can't trust simple assignments. We need to make sure that every keeper, pool ID, and rebalance data is meticulously checked. Because, you know, mistakes are totally a new concept.


53-57: Type Guard: Just in case you forgot what you were doing.

isArmadaKeepersParameters—because we need a function to tell us if our parameters are what we think they are. It's not like we could use TypeScript for that, right? Oh wait...

sdk/armada-protocol-common/src/simulator/interfaces/IArmadaUsersSimulation.ts (3)

26-26: Type Property: Narrowing it down, aren't we?

Changed the type to SimulationType.ArmadaUsers because, obviously, we can't have users getting confused with other types of simulations. It's not like they can read or anything.


32-37: Zod Schema: Copy-paste but make it fancy.

ArmadaUsersSimulationSchema—let's just take the existing schema and slap "Users" on it to make it user-specific. Because changing one word totally changes its functionality.


50-53: Type Guard: Redundancy at its finest.

isArmadaUsersSimulation—because we definitely needed another function to tell us what we already know. It's not like the type system in TypeScript could handle this.

sdk/armada-protocol-service/src/simulator/implementation/keepers/ArmadaKeepersSimulator.ts (1)

26-33: Simulate method: Looks good, but let's not get too excited.

The method structure is clear, and it delegates the actual simulation work to a private method. This is a good separation of concerns. However, make sure the parameters are well-documented and validated before use to avoid any surprises.

sdk/armada-protocol-common/src/simulator/interfaces/IArmadaKeepersSimulation.ts (2)

19-31: Interface IArmadaKeepersSimulation: Well-documented and structured.

The interface is a textbook example of how to extend functionality while keeping things tidy. The use of a unique symbol to ensure type safety is like using a secret handshake. Good job on that!


36-44: Zod schema: The gatekeeper of data integrity.

Using Zod to enforce the structure of the simulation data is like having a bouncer at the door of a club. It ensures that only the right data gets in. Just make sure the custom validators are as tough as they need to be!

sdk/order-planner-service/src/implementation/OrderPlannerService.ts (1)

24-25: New planners in town: How original.

Adding specialized planners for different user types is like having a different bartender for beer and cocktails. It's a smart move, as long as they don't start mixing up the orders. Keep an eye on how these planners are integrated and used across the system to avoid any mix-ups.

sdk/armada-protocol-service/src/simulator/implementation/users/ArmadaUsersSimulator.ts (4)

16-16: Class Renaming: A Fresh Coat of Paint

Ah, the classic "let's rename things and pretend it's a big change" move. But hey, if it helps you sleep at night, who am I to judge? Just make sure the rest of the codebase got the memo about this new fancy name.


31-33: Parameter Type Updates: Fancy New Types, I See

You've updated the parameter types in the simulate method. I hope you didn't just find and replace without checking if everything else still plays nice together. That would be a rookie move, right?


62-64: Method _simulateDeposit: Keeping It Consistent

You've updated the _simulateDeposit method to use the new user-specific types. I'm impressed you remembered to change the return type too. It's almost like you know what you're doing!

Also applies to: 83-83


100-102: Method _simulateWithdraw: Consistency Is Key

Look at you, updating _simulateWithdraw with the new types as well. Consistency is such a rare gem these days. Let's just hope the logic inside these methods actually does what it's supposed to.

Also applies to: 121-121

sdk/sdk-client/src/implementation/ArmadaManager/ArmadaManagerGovernanceClient.ts (1)

95-95: Type Update in forceRebalance: Oh, You Fancy, Huh?

Changing parameter types like you change socks, huh? Just make sure this new IArmadaRebalanceData type actually adds value and isn't just a cosmetic change to make the docs look pretty.

sdk/sdk-server/src/SDKAppRouter.ts (1)

56-59: Router Configuration: Look at You Being All Organized

Splitting armada into users and keepers in the router configuration? Wow, someone's trying to be all neat and tidy. Just make sure this doesn't mess up the existing routes, or you'll have a fun time debugging.

sdk/sdk-client/src/interfaces/ArmadaManager/IArmadaManagerGovernanceClient.ts (1)

163-163: Well, look who's getting specific!

The update from IRebalanceData[] to IArmadaRebalanceData[] in the forceRebalance method is a nice touch for those who care about type safety. But let's not throw a party just yet. Make sure this doesn't break anything where it's used, unless you enjoy debugging late at night.

Verification successful

Type Change Verified!

The update to IArmadaRebalanceData[] in the forceRebalance method is consistent across the codebase. No late-night debugging sessions needed here. Everything's looking smooth.

  • Verified occurrences in:
    • sdk/sdk-server/src/SDKAppRouter.ts
    • sdk/sdk-server/src/armada-protocol-handlers/governance/forceRebalance.ts
    • sdk/sdk-client/src/interfaces/ArmadaManager/IArmadaManagerGovernanceClient.ts
    • sdk/sdk-client/src/implementation/ArmadaManager/ArmadaManagerGovernanceClient.ts
    • sdk/contracts-provider-service/src/implementation/contracts/FleetCommanderContract/FleetCommanderContract.ts
    • sdk/contracts-provider-common/src/interfaces/contracts/IFleetCommanderContract.ts
    • sdk/armada-protocol-common/src/common/interfaces/IArmadaManager.ts
    • sdk/armada-protocol-service/src/common/implementation/ArmadaManager.ts
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all function calls to `forceRebalance` match the new signature.

# Test: Search for the function usage. Expect: Only occurrences of the new signature.
rg --type typescript -A 5 $'forceRebalance'

Length of output: 77


Script:

#!/bin/bash
# Description: Verify all function calls to `forceRebalance` match the new signature.

# Test: Search for the function usage. Expect: Only occurrences of the new signature.
rg --type ts -A 5 $'forceRebalance'

Length of output: 64230

sdk/contracts-provider-common/src/interfaces/contracts/IFleetCommanderContract.ts (1)

82-82: Oh, someone's been busy updating types!

Updating the rebalanceData parameter type across rebalance, adjustBuffer, and forceRebalance methods to IArmadaRebalanceData[] is a bold move. Just make sure you haven't just made a bunch of developers cry by breaking their existing code.

Also applies to: 93-93, 223-223

Verification successful

Chill out, everything's under control! The type update to IArmadaRebalanceData[] for rebalance, adjustBuffer, and forceRebalance is consistent across the codebase. No developers will be crying over broken code today.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all function calls to `rebalance`, `adjustBuffer`, and `forceRebalance` match the new signature.

# Test: Search for the function usage. Expect: Only occurrences of the new signature.
rg --type typescript -A 5 $'rebalance|adjustBuffer|forceRebalance'

Length of output: 102


Script:

#!/bin/bash
# Description: Verify all function calls to `rebalance`, `adjustBuffer`, and `forceRebalance` match the new signature.

# Test: Search for the function usage. Expect: Only occurrences of the new signature.
rg --type ts -A 5 $'rebalance|adjustBuffer|forceRebalance'

Length of output: 146286

sdk/contracts-provider-service/src/implementation/contracts/FleetCommanderContract/FleetCommanderContract.ts (5)

22-22: Oh, look, a new import!

Just when I thought the day couldn't get any more exciting, here comes a new import for IArmadaRebalanceData. I hope everyone's ready for the thrilling world of type changes. 🎉


127-127: A wild rebalance method appears!

It uses IArmadaRebalanceData! How innovative... not. But hey, at least it's consistent with the PR's theme of renaming things for the sake of renaming. Good job on keeping it uniform, I guess.


139-139: Adjusting more than just the buffer, I see.

Changing the type here too? Groundbreaking. It's almost like someone found the find-and-replace feature in their IDE. But seriously, consistency is key, so this change is on point.


310-310: Forcing consistency, one method at a time.

Here we go again, forceRebalance also got the IArmadaRebalanceData treatment. It's like a parade of type changes. Let's throw confetti every time we see IArmadaRebalanceData replacing IRebalanceData.


349-349: Private method, public type change.

Even the hidden gems aren't safe from the type change. IArmadaRebalanceData making a sneaky appearance in _convertRebalanceDataToSolidity. It's like finding an Easter egg in a video game, but less fun.

sdk/armada-protocol-service/src/common/implementation/ArmadaManager.ts (4)

9-9: Introducing a new player in the type game.

IArmadaRebalanceData joins the party. It's like watching a new character being introduced in a long-running TV show. Let's hope it doesn't mess up the plot.


177-177: Rebalancing with style.

Changing the type in rebalance method? How original. It's almost as if someone said, "Let's do something wild and change the type here!" And everyone else just nodded.


190-190: Buffer adjustment or type adjustment?

Here we are, adjusting more than just buffers. Changing the type in adjustBuffer method too. It's like a two-for-one special at your favorite store.


344-344: Forcing the type change, aren't we?

And finally, forceRebalance gets the same treatment. It's like the grand finale of a fireworks show, except it's just another type change.

@codecov-commenter
Copy link

codecov-commenter commented Sep 6, 2024

Codecov Report

Attention: Patch coverage is 68.18182% with 7 lines in your changes missing coverage. Please review.

Project coverage is 82.84%. Comparing base (8825883) to head (aa13495).
Report is 3 commits behind head on dev.

Files with missing lines Patch % Lines
...plementation/planners/ArmadaKeepersOrderPlanner.ts 45.45% 6 Missing ⚠️
...implementation/planners/ArmadaUsersOrderPlanner.ts 75.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##              dev     #494      +/-   ##
==========================================
- Coverage   83.04%   82.84%   -0.21%     
==========================================
  Files         291      292       +1     
  Lines        2967     2978      +11     
  Branches      178      179       +1     
==========================================
+ Hits         2464     2467       +3     
- Misses        492      500       +8     
  Partials       11       11              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between f0accec and f7e3b80.

Files selected for processing (1)
  • sdk/armada-protocol-service/src/simulator/implementation/keepers/index.ts (1 hunks)
Files skipped from review due to trivial changes (1)
  • sdk/armada-protocol-service/src/simulator/implementation/keepers/index.ts

Copy link
Member

@halaprix halaprix left a comment

Choose a reason for hiding this comment

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

left two comments :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants