BE-632 | Include limit order balances in passthrough/portfolio-assets #552
Conversation
adding backport mergify action and created A:backport/v27.x label
* BE-586 | Claimbot prototype Init * BE-586 | WIP * BE-586 | WIP * BE-595 | Clean up * BE-586 | Add docs * BE-586 | Clean up * BE-586 | Add docs * BE-596 | Tests init * BE-586 | Add tests for slices, orderbook packages * BE-586 | claimbot/tx tests * BE-586 | claimbot/order.go tests * BE-586 | Requested changes * BE-586 | Process block orderbooks, tests * BE-586 | Requested changes * BE-586 | Config update * BE-586 | OrderBookClient use slices.Split for pagination Cleans up OrderBookClient by reusing slices.Split instead of duplicating splitting slices into chunks logic in some of the methods. * BE-586 | Clean up * BE-586 | Fix fillbot docker-compose Fixes errors running fillbot via docker-compose * BE-586 | Docs, docker compose fixes * BE-586 | Run fillbot via docker-compose * BE-586 | Run claimbot via docker-compose, clean up * BE-586 | Cleanup * BE-586 | Named logger * BE-586 | Requested changes * BE-586 | Logging failing tx * BE-586 | Increase gas adjustment * BE-586 | Error logging fix * BE-586 | Trace name update * BE-586 | Requested changes #1 * BE-586 | Requested changes #2 * BE-586 | Sequence number update * BE-586 | added tests * BE-586 | Suggested improvements
* refactor: gas estimation APIs * changelog * lint
* feat: simulate swap as part of quotes * try adding e2e test * updates * lint * fix test * swagger * attempt fix grpc wiring * fix e2e sim test * updates
Includes limit order balances in total balance for passthrough/portfolio-assets
|
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 golangci-lint (1.62.2)level=warning msg="The linter 'exportloopref' is deprecated (since v1.60.2) due to: Since Go1.22 (loopvar) this linter is no longer relevant. Replaced by copyloopvar." WalkthroughThe pull request introduces significant enhancements to the order book functionality within the application. Key changes include the integration of a new Changes
Suggested reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 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
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (8)
domain/mvc/orderbook.go (1)
20-20: Enhance error documentation clarity.While the added documentation explains the boolean return value, it could be more explicit about the relationship between the error and boolean returns.
Consider this improved documentation:
-// If there was an error fetching the orders, the second return value will be true indicating best effort. +// Returns (orders, bestEffort, error) where bestEffort is true if some orders were retrieved despite errors.domain/orderbook/order.go (2)
101-106: Refactor OrderByDirection to handle OrderDirection consistently.The method is using
NewDirectionto convert the string field fromOrderstruct, which is a workaround for the inconsistent type usage. This could be simplified once theOrderstruct is updated to use theOrderDirectiontype.func (o Orders) OrderByDirection(direction OrderDirection) Orders { var result Orders for _, v := range o { - if NewDirection(v.OrderDirection).Is(direction) { + if v.OrderDirection.Is(direction) { result = append(result, v) } } return result }
146-159: Enhance documentation for calculation methods.While the implementation looks correct, the methods would benefit from more detailed documentation explaining:
- The business logic behind each calculation
- The units/denomination of the returned values
- Example scenarios showing how these calculations are used
-// ClaimableAmountOfOSMO calculates the claimable amount of base asset +// ClaimableAmount calculates the remaining claimable amount from filled orders +// that haven't been claimed yet. +// For example, if an order is 80% filled and 30% claimed: +// - TotalFilled = 0.8 +// - PercentClaimed = 0.3 +// - ClaimableAmount = 0.8 - (0.8 * 0.3) = 0.56 func (o LimitOrder) ClaimableAmount() osmomath.Dec { -// GetQuoteFillableAmount calculates the amount of quote asset left to be filled +// QuoteAssetFillableAmount calculates the remaining quote asset amount that can still +// be filled for this order. This represents the difference between the initially +// placed quantity and the amount that has been filled so far. func (o LimitOrder) QuoteAssetFillableAmount() osmomath.Dec { -// GetBaseAssetFillableAmount calculates the amount of base asset left to be filled +// BaseAssetFillableAmount calculates the remaining base asset amount that can still +// be filled for this order. This represents the difference between the initial +// quantity and the total amount that has been filled so far. func (o LimitOrder) BaseAssetFillableAmount() osmomath.Dec {orderbook/usecase/orderbook_usecase.go (2)
487-487: Consider caching the direction conversionWhile the type-safe direction handling is good,
NewDirectionis called twice for each order - once during direction checking and again when creating the LimitOrder struct. Consider caching the result of the first conversion to avoid redundant processing.- OrderDirection: orderbookdomain.NewDirection(order.OrderDirection), + direction := orderbookdomain.NewDirection(order.OrderDirection) // Use direction in the checks above and here + OrderDirection: direction,
542-547: Consider pre-allocating the result sliceThe
appendoperation in the return statement might cause unnecessary slice reallocations. Consider pre-allocating the result slice with an estimated capacity based on the lengths of askClaimable and bidClaimable.- return append(askClaimable, bidClaimable...), nil + result := make([]orderbookdomain.ClaimableOrder, 0, len(askClaimable)+len(bidClaimable)) + result = append(result, askClaimable...) + result = append(result, bidClaimable...) + return result, nilpassthrough/usecase/passthrough_usecase.go (1)
90-92: Document the reasoning behind job count constants.Consider adding comments to explain how these numbers are calculated:
-const ( - numFinalResultJobs = 8 - totalAssetCompositionNumJobs = 7 +// Constants for job counts +const ( + // Total number of final result jobs: + // 1. User balances + // 2. Total assets + // 3. Unstaking + // 4. Staked + // 5. Unclaimed rewards + // 6. Pooled + // 7. In-locks + // 8. Limit orders + numFinalResultJobs = 8 + + // Total number of jobs for asset composition: + // 1. User balances + // 2. Unstaking + // 3. Staked + // 4. In-locks + // 5. Unclaimed rewards + // 6. Pooled + // 7. Limit orders + totalAssetCompositionNumJobs = 7app/sidecar_query_server.go (2)
188-191: Ensure Proper Error Handling During InitializationWhile initializing
wasmQueryClient,orderBookAPIClient,orderBookRepository, andorderBookUseCase, there is no error handling in place. If any of these initializations fail internally, it could lead to runtime panics or unexpected behavior.Consider adding error checks if these functions can return errors. For example, if
orderbookusecase.Newreturns an error:orderBookUseCase, err := orderbookusecase.New(orderBookRepository, orderBookAPIClient, poolsUseCase, tokensUseCase, logger) if err != nil { return nil, err }Ensure that you handle errors appropriately to maintain robustness.
188-194: Improve Code Readability by Renaming Local VariablesThe variable
passthroughUseCaseis used both as a package name (import alias) and as a local variable. This can lead to confusion and reduce code readability.Consider renaming the local variable to avoid shadowing the package name:
-passthroughUseCase := passthroughUseCase.NewPassThroughUsecase(...) +passThroughUC := passthroughUseCase.NewPassThroughUsecase(...)This change clarifies the distinction between the package and the variable, improving maintainability.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (3)
domain/orderbook/order_test.gois excluded by!**/*_test.gopassthrough/usecase/export_test.gois excluded by!**/*_test.gopassthrough/usecase/passthrough_usecase_test.gois excluded by!**/*_test.go
📒 Files selected for processing (5)
app/sidecar_query_server.go(1 hunks)domain/mvc/orderbook.go(1 hunks)domain/orderbook/order.go(5 hunks)orderbook/usecase/orderbook_usecase.go(4 hunks)passthrough/usecase/passthrough_usecase.go(7 hunks)
🔇 Additional comments (7)
domain/mvc/orderbook.go (1)
21-21: Verify implementation of updated GetActiveOrders signature.
The addition of the error return value is a good practice for explicit error handling. However, we should verify that all implementations of this interface have been updated accordingly.
✅ Verification successful
Let me gather more information to verify the implementation details.
All implementations correctly handle the error return value
Based on the code analysis:
- The interface implementation in
OrderbookUseCaseImpl.GetActiveOrdersreturns([]orderbookdomain.LimitOrder, bool, error) - The gRPC client implementation in
orderbookClientImpl.GetActiveOrdersreturns(orderbookdomain.Orders, uint64, error) - All callers properly handle the error:
passthrough_usecase.gochecks and handles the errorpassthrough_handler.gochecks error and returns appropriate HTTP responseorderbook_usecase.gopropagates the error with proper error wrapping
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find all implementations of GetActiveOrders to verify they match the new signature
# Search for interface implementations
ast-grep --pattern 'type $_ struct {
$$$
}
func ($_ $_) GetActiveOrders(ctx context.Context, address string) $$$'
# Search for any direct calls to GetActiveOrders to ensure they handle the new error return
rg -A 2 'GetActiveOrders\(ctx,'
Length of output: 1356
Script:
#!/bin/bash
# Search for the interface definition and any other struct implementations
ast-grep --pattern 'type $_ interface {
$$$
GetActiveOrders(ctx context.Context, address string) $$$
}'
# Search for the orderBookClient implementation
ast-grep --pattern 'func ($_ $_) GetActiveOrders(ctx context.Context, $_, $_ string) $$$'
# Look for the actual implementation in the repository
rg -A 5 'func.*GetActiveOrders.*\('
Length of output: 8915
domain/orderbook/order.go (1)
146-149: LGTM: Changes align well with PR objectives.
The new calculation methods directly support the PR's goal of including limit order balances in portfolio assets by providing precise calculations for:
- Claimable amounts from filled orders
- Remaining fillable amounts in both quote and base assets
Also applies to: 151-154, 156-159
orderbook/usecase/orderbook_usecase.go (2)
391-392: Great improvement in order direction handling!
The replacement of string literals with orderbookdomain.NewDirection(order.OrderDirection).Is() checks enhances type safety and reduces the risk of errors from typos in string literals. This change aligns well with the PR's goal of cleaning up hardcoded order direction strings.
Also applies to: 464-465
Line range hint 464-471: Verify balance calculation for partially filled orders
Given the PR's focus on limit order balances and the uncertainty about remaining balances after partial fills, please verify that the output calculation correctly handles partially filled orders:
- For bid orders:
output = placedQuantity.Quo(price.Dec()) - For ask orders:
output = placedQuantity.Mul(price.Dec())
The calculation uses placedQuantity instead of the remaining quantity (quantity). This might need adjustment depending on whether we want to show the total original balance or just the remaining claimable balance.
✅ Verification successful
Let me gather more information about the balance calculation logic.
Based on the code analysis, I can now provide a final response about the balance calculation logic.
Balance calculation for partially filled orders is correct
The implementation correctly uses placedQuantity for balance calculation as it represents the total order size, which is the right approach for showing claimable balances:
- For bid orders:
output = placedQuantity.Quo(price.Dec())- Correctly calculates the total base asset amount that can be claimed - For ask orders:
output = placedQuantity.Mul(price.Dec())- Correctly calculates the total quote asset amount that can be claimed
The code also properly tracks:
- Remaining quantity through
quantity - Fill status via
percentFilled = totalFilled.Quo(placedQuantity) - Claim status via
percentClaimed = placedQuantity.Sub(quantity).Quo(placedQuantity)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for tests verifying partial fill scenarios
rg -A 10 "TestPartialFill|partial.*fill" --type go
Length of output: 3769
Script:
#!/bin/bash
# Search for order placement and balance calculation logic
ast-grep --pattern 'func $_($$$) {
$$$
placedQuantity := $$$
$$$
}'
# Search for quantity-related fields and methods
rg -A 5 "Quantity|quantity" --type go orderbook/
Length of output: 18749
passthrough/usecase/passthrough_usecase.go (3)
20-25: LGTM: Clean dependency injection implementation.
The addition of orderbookUseCase follows good practices by using interface type and maintaining consistency with existing field patterns.
289-292: LGTM: Clean integration of limit orders processing.
The limit orders job is properly integrated into the existing concurrent processing pipeline, maintaining consistency with the established pattern.
Also applies to: 400-400
248-258: Verify order direction handling.
The PR objectives mention cleaning up hardcoded order direction strings, but the current implementation doesn't show explicit handling of order directions. Let's verify the order direction handling in the codebase:
✅ Verification successful
Let me gather more information about the OrderDirection type and its usage in the codebase.
Let me search for more specific information about how order directions are handled in the active orders context.
Based on the comprehensive search results, I can now generate the final response:
Order direction handling is properly implemented and not hardcoded
The codebase shows a well-structured implementation of order directions:
- Order directions are defined as constants (
DirectionBidandDirectionAsk) indomain/orderbook/order.go - The
OrderDirectiontype is properly typed as a string with validation methods - The
GetActiveOrdersimplementation handles orders regardless of their direction - Test cases cover both bid and ask directions with proper type checking
The PR's objective of cleaning up hardcoded order direction strings has been achieved through:
- Using typed constants instead of raw strings
- Proper encapsulation in the domain model
- Consistent usage across the codebase
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for hardcoded order direction strings that might need cleanup
echo "Searching for hardcoded order direction strings..."
rg -i "\"(buy|sell|ask|bid)\"" --type go
# Search for order direction type usage
echo "Searching for OrderDirection type usage..."
ast-grep --pattern 'type OrderDirection'
Length of output: 6160
Script:
#!/bin/bash
# Search for OrderDirection type definition and its usage
echo "Searching for OrderDirection type and its usage..."
rg -A 5 "type OrderDirection" --type go
# Search for GetActiveOrders implementation to understand order direction handling
echo "Searching for GetActiveOrders implementation..."
ast-grep --pattern 'GetActiveOrders($$$) {
$$$
}'
Length of output: 655
Script:
#!/bin/bash
# Search for GetActiveOrders implementation with more context
echo "Searching for GetActiveOrders usage and implementation..."
rg -A 10 "GetActiveOrders" --type go
# Search for order direction handling in the orderbook usecase
echo "Searching for order direction handling in orderbook..."
rg -A 5 "OrderDirection" --type go --type-add go:*.{go,proto} --no-filename
Length of output: 53037
WalkthroughThe pull request introduces several enhancements to the order book functionality within the application. Key modifications include the integration of new components related to order book operations in the Changes
Possibly related PRs
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
Documentation and Community
|
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Outside diff range and nitpick comments (5)
domain/orderbook/order.go (2)
9-31: LGTM! Consider adding validation in NewDirection.The introduction of
OrderDirectiontype improves type safety and maintainability. The implementation is clean and well-documented.Consider adding validation in
NewDirectionto ensure only valid directions are created:func NewDirection(direction string) OrderDirection { + switch direction { + case string(DirectionBid), string(DirectionAsk): + return OrderDirection(direction) + default: + panic(fmt.Sprintf("invalid order direction: %s", direction)) + } - return OrderDirection(direction) }
119-137: LGTM! Consider adding field documentation.The
LimitOrderstruct fields are well-organized and cover all necessary data for limit order processing. However, some fields lack documentation explaining their purpose and units.Consider adding documentation for fields like
PercentClaimed,TotalFilled,Output, etc., to clarify their purpose and units.orderbook/usecase/orderbook_usecase.go (3)
391-391: Remove unnecessary empty line.This empty line doesn't serve any purpose and can be removed to maintain code cleanliness.
Line range hint
391-503: Consider breaking down CreateFormattedLimitOrder into smaller functions.The
CreateFormattedLimitOrdermethod is quite long and handles multiple responsibilities:
- Asset metadata retrieval
- Tick processing
- Quantity calculations
- Price normalization
- Status determination
Consider extracting some of these responsibilities into helper functions to improve readability and maintainability.
Line range hint
464-469: Add inline documentation for price calculations.The price and output calculations are complex and would benefit from inline documentation explaining:
- The relationship between price and direction
- Why the calculations differ for bid vs ask orders
- The significance of normalization
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (3)
domain/orderbook/order_test.gois excluded by!**/*_test.gopassthrough/usecase/export_test.gois excluded by!**/*_test.gopassthrough/usecase/passthrough_usecase_test.gois excluded by!**/*_test.go
📒 Files selected for processing (5)
app/sidecar_query_server.go(1 hunks)domain/mvc/orderbook.go(1 hunks)domain/orderbook/order.go(5 hunks)orderbook/usecase/orderbook_usecase.go(4 hunks)passthrough/usecase/passthrough_usecase.go(7 hunks)
🔇 Additional comments (13)
domain/mvc/orderbook.go (2)
20-21: LGTM! Clear documentation of error handling behavior.
The added comment clearly documents the meaning of the boolean return value, which is crucial for understanding the best-effort nature of the operation.
20-21: Verify error handling in implementations.
The addition of the error return parameter is a good practice for proper error propagation. However, we should ensure all implementations handle errors consistently.
✅ Verification successful
Based on the search results, I can now generate the final response as we have found the implementation and test cases showing error handling.
Error handling is properly implemented across the codebase
The implementation in orderbook/usecase/orderbook_usecase.go correctly handles and propagates errors:
- Returns errors with proper wrapping (e.g.,
types.FailedGetAllCanonicalOrderbookPoolIDsError) - Maintains the contract of returning
([]orderbookdomain.LimitOrder, bool, error)
Test cases in passthrough/delivery/http/passthrough_handler_test.go verify both success and error scenarios:
- Success case returns orders with
falsefor best effort andnilerror - Error case properly propagates the error and returns
nilorders
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find all implementations of GetActiveOrders to verify error handling
# Test: Search for implementations of GetActiveOrders. Expect: All implementations should handle and return errors properly
# Find all implementations
ast-grep --pattern 'func ($struct $_) GetActiveOrders(ctx context.Context, address string) ([]orderbookdomain.LimitOrder, bool, error) {
$$$
}'
# Find any old implementations that might need updating
ast-grep --pattern 'func ($struct $_) GetActiveOrders(ctx context.Context, address string) ([]orderbookdomain.LimitOrder, bool) {
$$$
}'
Length of output: 288
Script:
#!/bin/bash
# Let's try a different approach using ripgrep to find implementations
rg "GetActiveOrders.*\(ctx context\.Context, address string\)" -A 10
# Also search for the interface definition to understand the contract
rg "type.*interface.*{" -A 20 "orderbook.go"
Length of output: 11206
orderbook/usecase/orderbook_usecase.go (2)
392-392: LGTM! Improved type safety with NewDirection.
The consistent use of orderbookdomain.NewDirection() instead of string comparisons enhances type safety and reduces the likelihood of errors. This change aligns well with the PR's objective of cleaning up hardcoded order direction strings.
Also applies to: 464-464, 487-487
542-547: LGTM! Consistent use of direction constants.
The use of orderbookdomain.DirectionAsk and orderbookdomain.DirectionBid constants is consistent with the improved type safety approach.
Given the PR objectives to include limit order balances in portfolio assets, let's verify the balance calculations:
app/sidecar_query_server.go (1)
188-192: Initialization of order book components looks good
The initialization of wasmQueryClient, orderBookAPIClient, orderBookRepository, and orderBookUseCase is correct and enhances the server's capabilities to manage order book operations.
passthrough/usecase/passthrough_usecase.go (8)
22-22: Addition of orderbookUseCase to passthroughUseCase struct
The orderbookUseCase field has been added to the passthroughUseCase struct, aligning with the integration of limit order functionalities.
37-37: Definition of limitOrdersCategoryName constant
Defining limitOrdersCategoryName as "limit-orders" ensures consistency when referencing this category throughout the codebase.
90-92: Verify the correctness of job count constants
The constants have been updated:
numFinalResultJobsset to8totalAssetCompositionNumJobsset to7
Please confirm that these constants accurately reflect the updated number of jobs after including limit orders. This ensures proper synchronization and prevents potential issues with job handling.
109-117: Expansion of NewPassThroughUsecase constructor
The constructor now includes the orderbookUseCase parameter, and it is appropriately assigned within the passthroughUseCase struct.
120-125: Correct assignment of new dependencies
All new parameters, including orderbookUseCase, are correctly assigned to the struct fields, ensuring proper initialization.
248-259: Validate order.BaseAsset.Symbol for correctness
Ensure that order.BaseAsset.Symbol corresponds to a valid denomination recognized by the system. Incorrect or unrecognized symbols may cause issues in downstream processing.
Please verify that order.BaseAsset.Symbol consistently represents a valid denom and consider adding validation or mapping if necessary.
289-292: Inclusion of limit-orders category in fetch jobs
Adding the limit-orders category to fetchJobs ensures that limit order balances are fetched and included in the portfolio assets.
400-400: Update to include limit orders in the final job aggregation
The comment now reflects the addition of the limit orders category:
// 7. In-locks
-// 8. [placeholder for future categories]
+// 8. Limit orders| // ClaimableAmountOfOSMO calculates the claimable amount of base asset | ||
| func (o LimitOrder) ClaimableAmount() osmomath.Dec { | ||
| return o.TotalFilled.Sub(o.TotalFilled.Mul(o.PercentClaimed)) | ||
| } | ||
|
|
||
| // GetQuoteFillableAmount calculates the amount of quote asset left to be filled | ||
| func (o LimitOrder) QuoteAssetFillableAmount() osmomath.Dec { | ||
| return o.PlacedQuantity.Sub(o.Output.Mul(o.PercentFilled)) | ||
| } | ||
|
|
||
| // GetBaseAssetFillableAmount calculates the amount of base asset left to be filled | ||
| func (o LimitOrder) BaseAssetFillableAmount() osmomath.Dec { | ||
| return o.Quantity.Sub(o.TotalFilled) | ||
| } |
There was a problem hiding this comment.
💡 Codebase verification
Missing test coverage for fillable amount calculations
While ClaimableAmount() is well-tested with various scenarios, the QuoteAssetFillableAmount() and BaseAssetFillableAmount() methods lack test coverage. These methods perform critical calculations for determining remaining fillable amounts and should be thoroughly tested:
QuoteAssetFillableAmount: Calculates remaining quote asset usingPlacedQuantity.Sub(o.Output.Mul(o.PercentFilled))BaseAssetFillableAmount: Calculates remaining base asset usingQuantity.Sub(o.TotalFilled)
🔗 Analysis chain
Verify calculation methods against requirements.
The calculation methods look correct and align with the PR objectives. For example, ClaimableAmount correctly calculates the remaining claimable amount as demonstrated in the PR description (e.g., 0.5 USD - 0.125 USD = 0.375 USD).
Let's verify the usage of these calculation methods:
Let me verify the test coverage to ensure these calculation methods are properly tested.
Let me check for tests of the remaining two calculation methods.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for usages of the new calculation methods to ensure they're used correctly
# in the portfolio assets endpoint
rg -A 5 "ClaimableAmount|QuoteAssetFillableAmount|BaseAssetFillableAmount" --type go
Length of output: 2115
Script:
#!/bin/bash
# Check test coverage for these calculation methods
cat domain/orderbook/order_test.go
Length of output: 7545
Script:
#!/bin/bash
# Search for tests of QuoteAssetFillableAmount and BaseAssetFillableAmount
ast-grep --pattern 'func Test$_(t *testing.T) {
$$$
QuoteAssetFillableAmount()
$$$
}'
ast-grep --pattern 'func Test$_(t *testing.T) {
$$$
BaseAssetFillableAmount()
$$$
}'
Length of output: 186
I don't fully understand what happened here - could you elaborate? |
User balances only contain the user balances while the latter contains all of the value from pools, unclaimed rewards etc |
|
Do you mind please updating the docs with the limit order data too please? sqs/docs/architecture/passthrough.md Lines 17 to 46 in f4188d5 |
|
Let's also extend this test: Line 58 in a9e6b46 |
|
deividaspetraitis
added
A:backport/v28.x
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (3)
app/sidecar_query_server.go(1 hunks)orderbook/usecase/orderbook_usecase.go(3 hunks)passthrough/usecase/passthrough_usecase.go(7 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- orderbook/usecase/orderbook_usecase.go
🔇 Additional comments (8)
app/sidecar_query_server.go (2)
196-199: LGTM! Clean initialization of order book components.
The initialization sequence follows clean architecture patterns with proper dependency injection.
202-202: Remove unnecessary error check after NewPassThroughUsecase.
The function does not return an error, but the code might be checking err afterward.
passthrough/usecase/passthrough_usecase.go (6)
20-22: LGTM! Clean addition of order book use case field.
The field is properly placed and correctly typed.
37-37: LGTM! Consistent constant naming.
The constant follows the established naming pattern.
90-92: LGTM! Job counts properly updated.
Constants correctly reflect the addition of limit orders category.
109-125: LGTM! Constructor properly updated.
The constructor signature and implementation correctly handle the new dependency.
294-297: LGTM! Fetch job properly configured.
The limit orders fetch job follows the established pattern.
405-405: LGTM! Comment updated to include limit orders.
Documentation accurately reflects the addition of limit orders.
| getLimitOrderCoins := func(ctx context.Context, address string) (sdk.Coins, error) { | ||
| orders, _, err := p.orderbookUseCase.GetActiveOrders(ctx, address) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| var limitOrdersCoins sdk.Coins | ||
| for _, order := range orders { | ||
| limitOrdersCoins = limitOrdersCoins.Add(sdk.Coin{ | ||
| Denom: order.BaseAsset.Symbol, | ||
| Amount: order.ClaimableAmount().TruncateInt(), | ||
| }) | ||
| } | ||
|
|
||
| return limitOrdersCoins, err | ||
| } |
There was a problem hiding this comment.
Fix error return in getLimitOrderCoins.
The function returns err on line 262, but this variable was already checked and handled. The function should return nil as the error.
Apply this diff:
- return limitOrdersCoins, err
+ return limitOrdersCoins, nil📝 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.
| getLimitOrderCoins := func(ctx context.Context, address string) (sdk.Coins, error) { | |
| orders, _, err := p.orderbookUseCase.GetActiveOrders(ctx, address) | |
| if err != nil { | |
| return nil, err | |
| } | |
| var limitOrdersCoins sdk.Coins | |
| for _, order := range orders { | |
| limitOrdersCoins = limitOrdersCoins.Add(sdk.Coin{ | |
| Denom: order.BaseAsset.Symbol, | |
| Amount: order.ClaimableAmount().TruncateInt(), | |
| }) | |
| } | |
| return limitOrdersCoins, err | |
| } | |
| getLimitOrderCoins := func(ctx context.Context, address string) (sdk.Coins, error) { | |
| orders, _, err := p.orderbookUseCase.GetActiveOrders(ctx, address) | |
| if err != nil { | |
| return nil, err | |
| } | |
| var limitOrdersCoins sdk.Coins | |
| for _, order := range orders { | |
| limitOrdersCoins = limitOrdersCoins.Add(sdk.Coin{ | |
| Denom: order.BaseAsset.Symbol, | |
| Amount: order.ClaimableAmount().TruncateInt(), | |
| }) | |
| } | |
| return limitOrdersCoins, nil | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
domain/orderbook/order.go(5 hunks)passthrough/usecase/passthrough_usecase.go(7 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- domain/orderbook/order.go
🔇 Additional comments (4)
passthrough/usecase/passthrough_usecase.go (4)
15-15: LGTM: Clean integration of orderbook functionality
The import and struct field additions are well-organized and follow Go conventions.
Also applies to: 21-23
110-126: LGTM: Constructor updated correctly
The constructor signature and implementation are properly updated to include the new orderbook use case.
310-313: LGTM: Job configuration properly updated
The limit orders job is correctly integrated into the existing job configuration structure.
91-93: Verify constant values consistency
The incremented values for numFinalResultJobs and totalAssetCompositionNumJobs need verification to ensure they match the actual number of jobs after adding limit orders support.
✅ Verification successful
Let me gather more information to verify the job counts accurately.
Based on the code analysis, I can now provide the final response:
Constants correctly reflect the number of jobs
The constant values are accurate:
-
numFinalResultJobs = 8matches the 8 categories being processed:- User balances
- Total assets
- Pooled
- Staked
- Unclaimed rewards
- Pooled
- In-locks
- Limit orders
-
totalAssetCompositionNumJobs = 7matches the number of jobs that contribute to total assets composition, as commented in the code:- User balances
- Pooled
- In-locks
- Staked
- Unclaimed rewards
- Pooled
- Limit orders
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Count the actual number of jobs in the code
echo "Checking job counts..."
rg -U "fetchJobs.*\[\]fetchBalancesPortfolioAssetsJob.*\{(?s:.)*?\}" | grep -c "name:"
Length of output: 143
Script:
#!/bin/bash
# Search for all job-related constants and their usage
echo "Searching for job-related code..."
rg "Job" -A 5 -B 5 passthrough/usecase/passthrough_usecase.go
# Search for any job creation or initialization
echo -e "\nSearching for job initialization..."
rg "jobs\s*:=\s*\[" -A 20 passthrough/usecase/passthrough_usecase.go
# Look for any job type definitions
echo -e "\nSearching for job type definitions..."
ast-grep --pattern 'type $_Job struct {
$$$
}'
Length of output: 7291
| getLimitOrderCoins := func(ctx context.Context, address string) (sdk.Coins, error) { | ||
| orders, _, err := p.orderbookUseCase.GetActiveOrders(ctx, address) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| var limitOrdersCoins sdk.Coins | ||
| for _, order := range orders { | ||
| denom, err := func() (string, error) { | ||
| switch order.OrderDirection { | ||
| case orderbookdomain.DirectionAsk: | ||
| return order.BaseAsset.Symbol, nil | ||
| case orderbookdomain.DirectionBid: | ||
| return order.QuoteAsset.Symbol, nil | ||
| default: | ||
| return "", fmt.Errorf("unknown order direction: %s", order.OrderDirection) | ||
| } | ||
| }() | ||
| if err != nil { | ||
| p.logger.Error("unable to get denom for limit order", zap.Error(err)) | ||
| continue | ||
| } | ||
|
|
||
| limitOrdersCoins = limitOrdersCoins.Add(sdk.Coin{ | ||
| Denom: denom, | ||
| Amount: order.Quantity.TruncateInt(), | ||
| }) | ||
| } | ||
|
|
||
| return limitOrdersCoins, err | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Multiple improvements needed for getLimitOrderCoins
- Error handling issue on line 278
- Performance optimization opportunity
- Missing function documentation
Apply these improvements:
+// getLimitOrderCoins returns the total coins from active limit orders for the given address.
+// For ask orders, it returns the base asset quantity, and for bid orders, it returns the quote asset quantity.
getLimitOrderCoins := func(ctx context.Context, address string) (sdk.Coins, error) {
orders, _, err := p.orderbookUseCase.GetActiveOrders(ctx, address)
if err != nil {
return nil, err
}
- var limitOrdersCoins sdk.Coins
+ // Preallocate slice with capacity of orders
+ limitOrdersCoins := make(sdk.Coins, 0, len(orders))
for _, order := range orders {
denom, err := func() (string, error) {
switch order.OrderDirection {
case orderbookdomain.DirectionAsk:
return order.BaseAsset.Symbol, nil
case orderbookdomain.DirectionBid:
return order.QuoteAsset.Symbol, nil
default:
return "", fmt.Errorf("unknown order direction: %s", order.OrderDirection)
}
}()
if err != nil {
p.logger.Error("unable to get denom for limit order", zap.Error(err))
continue
}
limitOrdersCoins = limitOrdersCoins.Add(sdk.Coin{
Denom: denom,
Amount: order.Quantity.TruncateInt(),
})
}
- return limitOrdersCoins, err
+ return limitOrdersCoins, nil
}📝 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.
| getLimitOrderCoins := func(ctx context.Context, address string) (sdk.Coins, error) { | |
| orders, _, err := p.orderbookUseCase.GetActiveOrders(ctx, address) | |
| if err != nil { | |
| return nil, err | |
| } | |
| var limitOrdersCoins sdk.Coins | |
| for _, order := range orders { | |
| denom, err := func() (string, error) { | |
| switch order.OrderDirection { | |
| case orderbookdomain.DirectionAsk: | |
| return order.BaseAsset.Symbol, nil | |
| case orderbookdomain.DirectionBid: | |
| return order.QuoteAsset.Symbol, nil | |
| default: | |
| return "", fmt.Errorf("unknown order direction: %s", order.OrderDirection) | |
| } | |
| }() | |
| if err != nil { | |
| p.logger.Error("unable to get denom for limit order", zap.Error(err)) | |
| continue | |
| } | |
| limitOrdersCoins = limitOrdersCoins.Add(sdk.Coin{ | |
| Denom: denom, | |
| Amount: order.Quantity.TruncateInt(), | |
| }) | |
| } | |
| return limitOrdersCoins, err | |
| } | |
| // getLimitOrderCoins returns the total coins from active limit orders for the given address. | |
| // For ask orders, it returns the base asset quantity, and for bid orders, it returns the quote asset quantity. | |
| getLimitOrderCoins := func(ctx context.Context, address string) (sdk.Coins, error) { | |
| orders, _, err := p.orderbookUseCase.GetActiveOrders(ctx, address) | |
| if err != nil { | |
| return nil, err | |
| } | |
| // Preallocate slice with capacity of orders | |
| limitOrdersCoins := make(sdk.Coins, 0, len(orders)) | |
| for _, order := range orders { | |
| denom, err := func() (string, error) { | |
| switch order.OrderDirection { | |
| case orderbookdomain.DirectionAsk: | |
| return order.BaseAsset.Symbol, nil | |
| case orderbookdomain.DirectionBid: | |
| return order.QuoteAsset.Symbol, nil | |
| default: | |
| return "", fmt.Errorf("unknown order direction: %s", order.OrderDirection) | |
| } | |
| }() | |
| if err != nil { | |
| p.logger.Error("unable to get denom for limit order", zap.Error(err)) | |
| continue | |
| } | |
| limitOrdersCoins = limitOrdersCoins.Add(sdk.Coin{ | |
| Denom: denom, | |
| Amount: order.Quantity.TruncateInt(), | |
| }) | |
| } | |
| return limitOrdersCoins, nil | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
passthrough/usecase/passthrough_usecase.go (2)
110-126: Consider using a config struct for constructor parametersThe constructor's parameter list is growing long. Consider introducing a config struct to improve maintainability and make future additions easier.
+type PassThroughConfig struct { + PassthroughGRPCClient passthroughdomain.PassthroughGRPCClient + PoolsUseCase mvc.PoolsUsecase + TokensUseCase mvc.TokensUsecase + OrderbookUseCase mvc.OrderBookUsecase + LiquidityPricer domain.LiquidityPricer + DefaultQuoteDenom string + Logger log.Logger +} -func NewPassThroughUsecase( - passthroughGRPCClient passthroughdomain.PassthroughGRPCClient, - poolsUseCase mvc.PoolsUsecase, - tokensUseCase mvc.TokensUsecase, - orderbookUseCase mvc.OrderBookUsecase, - liquidityPricer domain.LiquidityPricer, - defaultQuoteDenom string, - logger log.Logger, -) *passthroughUseCase { +func NewPassThroughUsecase(config PassThroughConfig) *passthroughUseCase { return &passthroughUseCase{ - passthroughGRPCClient: passthroughGRPCClient, - poolsUseCase: poolsUseCase, - tokensUseCase: tokensUseCase, - orderbookUseCase: orderbookUseCase, - defaultQuoteDenom: defaultQuoteDenom, - liquidityPricer: liquidityPricer, - logger: logger, + passthroughGRPCClient: config.PassthroughGRPCClient, + poolsUseCase: config.PoolsUseCase, + tokensUseCase: config.TokensUseCase, + orderbookUseCase: config.OrderbookUseCase, + defaultQuoteDenom: config.DefaultQuoteDenom, + liquidityPricer: config.LiquidityPricer, + logger: config.Logger, } }
260-264: Consider a more concise implementationThe callback function can be simplified since it's just forwarding the channel results.
-getLimitOrderCoins := func(ctx context.Context, address string) (sdk.Coins, error) { - limitOrdersResult := <-limitOrdersChan - return limitOrdersResult.coins, limitOrdersResult.err -} +getLimitOrderCoins := func(ctx context.Context, address string) (result coinsResult) { + return <-limitOrdersChan +}
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
passthrough/usecase/passthrough_usecase.go(10 hunks)
🔇 Additional comments (4)
passthrough/usecase/passthrough_usecase.go (4)
15-15: LGTM: Clean integration of orderbook functionality
The orderbook integration follows the existing pattern and maintains consistency with the codebase structure.
Also applies to: 23-23
145-147: LGTM: Clean channel setup
The limit orders channel setup follows the existing patterns and includes proper cleanup.
193-200: LGTM: Clean goroutine implementation
The goroutine follows the established pattern and correctly handles the channel communication.
294-297: LGTM: Clean fetch job addition
The limit orders fetch job is correctly integrated into the existing jobs slice.
| // getLimitOrderCoins returns the user's limit order coins | ||
| func (p *passthroughUseCase) getLimitOrderCoins(ctx context.Context, address string) (sdk.Coins, error) { | ||
| var err error | ||
| orders, _, err := p.orderbookUseCase.GetActiveOrders(ctx, address) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| var limitOrdersCoins sdk.Coins | ||
| for _, order := range orders { | ||
| denom, derr := func() (string, error) { | ||
| switch order.OrderDirection { | ||
| case orderbookdomain.DirectionAsk: | ||
| return order.BaseAsset.Symbol, nil | ||
| case orderbookdomain.DirectionBid: | ||
| return order.QuoteAsset.Symbol, nil | ||
| default: | ||
| return "", fmt.Errorf("unknown order direction: %s", order.OrderDirection) | ||
| } | ||
| }() | ||
| if err != nil { | ||
| err = derr // update error | ||
| p.logger.Error("unable to get denom for limit order", zap.Error(err)) | ||
| continue | ||
| } | ||
|
|
||
| limitOrdersCoins = limitOrdersCoins.Add(sdk.Coin{ | ||
| Denom: denom, | ||
| Amount: order.Quantity.TruncateInt(), | ||
| }) | ||
| } | ||
|
|
||
| return limitOrdersCoins, err | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Multiple improvements needed for getLimitOrderCoins
- Remove the redundant error variable declaration
- Fix error handling to avoid shadowing
- Add function documentation
Apply these improvements:
+// getLimitOrderCoins returns the user's limit order coins.
+// For ask orders, it returns the base asset quantity.
+// For bid orders, it returns the quote asset quantity.
+// Returns error if fails to get active orders or if an unknown order direction is encountered.
func (p *passthroughUseCase) getLimitOrderCoins(ctx context.Context, address string) (sdk.Coins, error) {
- var err error
orders, _, err := p.orderbookUseCase.GetActiveOrders(ctx, address)
if err != nil {
return nil, err
}
var limitOrdersCoins sdk.Coins
for _, order := range orders {
denom, derr := func() (string, error) {
switch order.OrderDirection {
case orderbookdomain.DirectionAsk:
return order.BaseAsset.Symbol, nil
case orderbookdomain.DirectionBid:
return order.QuoteAsset.Symbol, nil
default:
return "", fmt.Errorf("unknown order direction: %s", order.OrderDirection)
}
}()
- if err != nil {
- err = derr
+ if derr != nil {
p.logger.Error("unable to get denom for limit order", zap.Error(err))
continue
}
limitOrdersCoins = limitOrdersCoins.Add(sdk.Coin{
Denom: denom,
Amount: order.Quantity.TruncateInt(),
})
}
- return limitOrdersCoins, err
+ return limitOrdersCoins, nil
}📝 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.
| // getLimitOrderCoins returns the user's limit order coins | |
| func (p *passthroughUseCase) getLimitOrderCoins(ctx context.Context, address string) (sdk.Coins, error) { | |
| var err error | |
| orders, _, err := p.orderbookUseCase.GetActiveOrders(ctx, address) | |
| if err != nil { | |
| return nil, err | |
| } | |
| var limitOrdersCoins sdk.Coins | |
| for _, order := range orders { | |
| denom, derr := func() (string, error) { | |
| switch order.OrderDirection { | |
| case orderbookdomain.DirectionAsk: | |
| return order.BaseAsset.Symbol, nil | |
| case orderbookdomain.DirectionBid: | |
| return order.QuoteAsset.Symbol, nil | |
| default: | |
| return "", fmt.Errorf("unknown order direction: %s", order.OrderDirection) | |
| } | |
| }() | |
| if err != nil { | |
| err = derr // update error | |
| p.logger.Error("unable to get denom for limit order", zap.Error(err)) | |
| continue | |
| } | |
| limitOrdersCoins = limitOrdersCoins.Add(sdk.Coin{ | |
| Denom: denom, | |
| Amount: order.Quantity.TruncateInt(), | |
| }) | |
| } | |
| return limitOrdersCoins, err | |
| } | |
| // getLimitOrderCoins returns the user's limit order coins. | |
| // For ask orders, it returns the base asset quantity. | |
| // For bid orders, it returns the quote asset quantity. | |
| // Returns error if fails to get active orders or if an unknown order direction is encountered. | |
| func (p *passthroughUseCase) getLimitOrderCoins(ctx context.Context, address string) (sdk.Coins, error) { | |
| orders, _, err := p.orderbookUseCase.GetActiveOrders(ctx, address) | |
| if err != nil { | |
| return nil, err | |
| } | |
| var limitOrdersCoins sdk.Coins | |
| for _, order := range orders { | |
| denom, derr := func() (string, error) { | |
| switch order.OrderDirection { | |
| case orderbookdomain.DirectionAsk: | |
| return order.BaseAsset.Symbol, nil | |
| case orderbookdomain.DirectionBid: | |
| return order.QuoteAsset.Symbol, nil | |
| default: | |
| return "", fmt.Errorf("unknown order direction: %s", order.OrderDirection) | |
| } | |
| }() | |
| if derr != nil { | |
| p.logger.Error("unable to get denom for limit order", zap.Error(err)) | |
| continue | |
| } | |
| limitOrdersCoins = limitOrdersCoins.Add(sdk.Coin{ | |
| Denom: denom, | |
| Amount: order.Quantity.TruncateInt(), | |
| }) | |
| } | |
| return limitOrdersCoins, nil | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
passthrough/usecase/passthrough_usecase.go(15 hunks)
🔇 Additional comments (4)
passthrough/usecase/passthrough_usecase.go (4)
15-15: LGTM: Clean structural changes
The additions maintain consistency with the existing codebase patterns.
Also applies to: 24-24, 39-39
111-127: LGTM: Clean constructor update
The constructor properly handles the new dependency injection for orderBookUseCase.
146-148: LGTM: Clean limit orders channel setup
The limit orders channel is properly initialized and managed.
Also applies to: 194-201
618-651: 🛠️ Refactor suggestion
Multiple improvements needed for getLimitOrderCoins
- Error handling issue on line 638
- Unnecessary error variable declaration
- Missing function documentation
Apply these improvements:
+// getLimitOrderCoins returns the user's limit order coins.
+// For ask orders, it returns the base asset quantity.
+// For bid orders, it returns the quote asset quantity.
+// Returns error if fails to get active orders or if an unknown order direction is encountered.
func (p *passthroughUseCase) getLimitOrderCoins(ctx context.Context, address string) (sdk.Coins, error) {
- var err error
orders, _, err := p.orderbookUseCase.GetActiveOrders(ctx, address)
if err != nil {
return nil, err
}
var limitOrdersCoins sdk.Coins
for _, order := range orders {
denom, derr := func() (string, error) {
switch order.OrderDirection {
case orderbookdomain.DirectionAsk:
return order.BaseAsset.Symbol, nil
case orderbookdomain.DirectionBid:
return order.QuoteAsset.Symbol, nil
default:
return "", fmt.Errorf("unknown order direction: %s", order.OrderDirection)
}
}()
- if err != nil {
- err = derr
+ if derr != nil {
p.logger.Error("unable to get denom for limit order", zap.Error(err))
continue
}
limitOrdersCoins = limitOrdersCoins.Add(sdk.Coin{
Denom: denom,
Amount: order.Quantity.TruncateInt(),
})
}
- return limitOrdersCoins, err
+ return limitOrdersCoins, nil
}Likely invalid or redundant comment.
| _, totalAssetsCap, err := p.computeCapitalizationForCoins(ctx, job.coins) | ||
| p.logger.Info("total assets composition", zap.String("totalAssetsCategoryName", job.name), zap.Any("Capitalization", totalAssetsCap), zap.Error(err)) | ||
|
|
There was a problem hiding this comment.
Fix error variable shadowing in logging statement
The error from computeCapitalizationForCoins shadows the outer error variable, which could lead to incorrect error reporting.
Apply this diff:
- _, totalAssetsCap, err := p.computeCapitalizationForCoins(ctx, job.coins)
+ _, totalAssetsCap, capErr := p.computeCapitalizationForCoins(ctx, job.coins)
- p.logger.Info("total assets composition", zap.String("totalAssetsCategoryName", job.name), zap.Any("Capitalization", totalAssetsCap), zap.Error(err))
+ p.logger.Info("total assets composition", zap.String("totalAssetsCategoryName", job.name), zap.Any("Capitalization", totalAssetsCap), zap.Error(capErr))📝 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.
| _, totalAssetsCap, err := p.computeCapitalizationForCoins(ctx, job.coins) | |
| p.logger.Info("total assets composition", zap.String("totalAssetsCategoryName", job.name), zap.Any("Capitalization", totalAssetsCap), zap.Error(err)) | |
| _, totalAssetsCap, capErr := p.computeCapitalizationForCoins(ctx, job.coins) | |
| p.logger.Info("total assets composition", zap.String("totalAssetsCategoryName", job.name), zap.Any("Capitalization", totalAssetsCap), zap.Error(capErr)) |
This PR introduces changes to include limit order balances in total balance for
passthrough/portfolio-assetsendpoint. As a side effect changes includes cleaning up hardcoded order direction strings in some places.Consider we have ask order where we sell 0.1 OSMO and want to receive 1 USD. Order is 50% filled of which 25% is already claimed, that would result in following properties of the order:
1 USD * 50%=1 * 0.5=0.5 USD0.1 OSMO * 50%=0.0050.5 USD * 25%=0.5 * 0.25=0.125 USD0.5 USD - 0.125 USD=0.375 USDIntroduced changes adds
limit-orderssection that contains unclaimed balances, as per example above such section would look like:Second change is that unclaimed coins will appear under
total-assetssection inaccount_coins_result, for example:At the moment I am opening this PR I am not 100% sure I have addressed the problem correctly because due following questions:
user-balancesvsaccount_coins_result.Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation