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.

Sign up for GitHub

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

BE-632 | Include limit order balances in passthrough/portfolio-assets #552

Closed
wants to merge 13 commits into from
+335 −79
Closed

BE-632 | Include limit order balances in passthrough/portfolio-assets #552

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
e79f080
chore: add mergify backport on tag action (#542)
deividaspetraitis Oct 31, 2024
1a4311b
chore: add required labels check for backporting (#543)
deividaspetraitis Oct 31, 2024
2a46eeb
BE-586 | Claimbot (#524)
deividaspetraitis Nov 3, 2024
662aac4
refactor: gas estimation APIs (#546)
p0mvn Nov 3, 2024
f2931d8
feat: simulate swap as part of quotes (#547)
p0mvn Nov 4, 2024
28f59d8
changelog
p0mvn Nov 4, 2024
0e45457
BE-632 | Include limit order balances in passthrough/portfolio-assets
deividaspetraitis Nov 6, 2024
136f13d
BE-632 | Rabbitai comments
deividaspetraitis Nov 21, 2024
3c784de
BE-632 | Do not use NewCoins constructor for performance reasons
deividaspetraitis Nov 21, 2024
a019f5a
Merge remote-tracking branch 'upstream/v27.x' into BE-632
deividaspetraitis Dec 9, 2024
b918fb3
BE-632 | Include limit order amounts into balances
deividaspetraitis Dec 9, 2024
e6d95de
BE-632 | Include limit order coins into user's balance
deividaspetraitis Dec 11, 2024
8f939b0
BE-632 | Filter low liquidity denoms
deividaspetraitis Dec 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 6 additions & 9 deletions app/sidecar_query_server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,11 +193,13 @@ func NewSideCarQueryServer(appCodec codec.Codec, config domain.Config, logger lo
return nil, err
}

wasmQueryClient := wasmtypes.NewQueryClient(passthroughGRPCClient.GetChainGRPCClient())
orderBookAPIClient := orderbookgrpcclientdomain.New(wasmQueryClient)
orderBookRepository := orderbookrepository.New()
orderBookUseCase := orderbookusecase.New(orderBookRepository, orderBookAPIClient, poolsUseCase, tokensUseCase, logger)

// Initialize passthrough query use case
passthroughUseCase := passthroughUseCase.NewPassThroughUsecase(passthroughGRPCClient, poolsUseCase, tokensUseCase, liquidityPricer, defaultQuoteDenom, logger)
if err != nil {
return nil, err
}
passthroughUseCase := passthroughUseCase.NewPassThroughUsecase(passthroughGRPCClient, poolsUseCase, tokensUseCase, orderBookUseCase, liquidityPricer, defaultQuoteDenom, logger)
deividaspetraitis marked this conversation as resolved.
Show resolved Hide resolved

// Use the same config to initialize coingecko pricing strategy
coingeckPricingConfig := *config.Pricing
Expand All @@ -211,11 +213,6 @@ func NewSideCarQueryServer(appCodec codec.Codec, config domain.Config, logger lo
tokensUseCase.RegisterPricingStrategy(domain.ChainPricingSourceType, chainPricingSource)
tokensUseCase.RegisterPricingStrategy(domain.CoinGeckoPricingSourceType, coingeckoPricingSource)

wasmQueryClient := wasmtypes.NewQueryClient(passthroughGRPCClient.GetChainGRPCClient())
orderBookAPIClient := orderbookgrpcclientdomain.New(wasmQueryClient)
orderBookRepository := orderbookrepository.New()
orderBookUseCase := orderbookusecase.New(orderBookRepository, orderBookAPIClient, poolsUseCase, tokensUseCase, logger)

// HTTP handlers
poolsHttpDelivery.NewPoolsHandler(e, poolsUseCase)
passthroughHttpDelivery.NewPassthroughHandler(e, passthroughUseCase, orderBookUseCase, logger)
Expand Down
1 change: 1 addition & 0 deletions domain/mvc/orderbook.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ type OrderBookUsecase interface {
GetAllTicks(poolID uint64) (map[int64]orderbookdomain.OrderbookTick, bool)

// GetOrder returns all active orderbook orders for a given address.
// If there was an error fetching the orders, the second return value will be true indicating best effort.
GetActiveOrders(ctx context.Context, address string) ([]orderbookdomain.LimitOrder, bool, error)

// GetActiveOrdersStream returns a channel for streaming limit orderbook orders for a given address.
Expand Down
103 changes: 76 additions & 27 deletions domain/orderbook/order.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,30 @@ import (
"github.com/osmosis-labs/osmosis/osmomath"
)

// NewDirection returns a new order direction.
func NewDirection(direction string) OrderDirection {
return OrderDirection(direction)
}
deividaspetraitis marked this conversation as resolved.
Show resolved Hide resolved

// Direction represents the direction of an order.
type OrderDirection string

// Is returns true if the order direction is equal to the given direction.
func (d OrderDirection) Is(direction OrderDirection) bool {
return d == direction
}

// String returns the string representation of the order direction.
func (d OrderDirection) String() string {
return string(d)
}

// Order direction types.
const (
DirectionBid OrderDirection = "bid"
DirectionAsk OrderDirection = "ask"
)

// OrderStatus represents the status of an order.
type OrderStatus string

Expand All @@ -20,13 +44,23 @@ const (

// Order represents an order in the orderbook returned by the orderbook contract.
type Order struct {
TickId int64 `json:"tick_id"`
OrderId int64 `json:"order_id"`
// The price for which to place the order.
// Can be calculated following the tick calculations:
// https://github.com/osmosis-labs/orderbook/edit/main/contracts/sumtree-orderbook/README.md#tick-calculations
TickId int64 `json:"tick_id"`
// The ID of the order
OrderId int64 `json:"order_id"`
// The direction for which to place the order, should be either an ask order or a bid order
OrderDirection string `json:"order_direction"`
Owner string `json:"owner"`
Quantity string `json:"quantity"`
Etas string `json:"etas"`
ClaimBounty string `json:"claim_bounty"`
// The address of the order owner
Owner string `json:"owner"`
// Quantity is the actual quantity for the order left to be filled
// It gets updated as the order is filled, claimed
Quantity string `json:"quantity"`
Etas string `json:"etas"`
// An optional percentage bounty to claim the order, capped at 1%
ClaimBounty string `json:"claim_bounty"`
// Immutable quantity of the order when placed
PlacedQuantity string `json:"placed_quantity"`
PlacedAt string `json:"placed_at"`
}
Expand Down Expand Up @@ -67,10 +101,10 @@ func (o Orders) TickID() []int64 {

// OrderByDirection filters orders by given direction and returns resulting slice.
// Original slice is not mutated.
func (o Orders) OrderByDirection(direction string) Orders {
func (o Orders) OrderByDirection(direction OrderDirection) Orders {
var result Orders
for _, v := range o {
if v.OrderDirection == direction {
if NewDirection(v.OrderDirection).Is(direction) {
result = append(result, v)
}
deividaspetraitis marked this conversation as resolved.
Show resolved Hide resolved
}
Expand All @@ -85,25 +119,25 @@ type Asset struct {

// LimitOrder represents a limit order in the orderbook.
type LimitOrder struct {
TickId int64 `json:"tick_id"`
OrderId int64 `json:"order_id"`
OrderDirection string `json:"order_direction"`
Owner string `json:"owner"`
Quantity osmomath.Dec `json:"quantity"`
Etas string `json:"etas"`
ClaimBounty string `json:"claim_bounty"`
PlacedQuantity osmomath.Dec `json:"placed_quantity"`
PlacedAt int64 `json:"placed_at"`
Price osmomath.Dec `json:"price"`
PercentClaimed osmomath.Dec `json:"percentClaimed"`
TotalFilled osmomath.Dec `json:"totalFilled"`
PercentFilled osmomath.Dec `json:"percentFilled"`
OrderbookAddress string `json:"orderbookAddress"`
Status OrderStatus `json:"status"`
Output osmomath.Dec `json:"output"`
QuoteAsset Asset `json:"quote_asset"`
BaseAsset Asset `json:"base_asset"`
PlacedTx *string `json:"placed_tx,omitempty"`
TickId int64 `json:"tick_id"`
OrderId int64 `json:"order_id"`
OrderDirection OrderDirection `json:"order_direction"`
Owner string `json:"owner"`
Quantity osmomath.Dec `json:"quantity"`
Etas string `json:"etas"`
ClaimBounty string `json:"claim_bounty"`
PlacedQuantity osmomath.Dec `json:"placed_quantity"`
PlacedAt int64 `json:"placed_at"`
Price osmomath.Dec `json:"price"`
PercentClaimed osmomath.Dec `json:"percentClaimed"`
TotalFilled osmomath.Dec `json:"totalFilled"`
PercentFilled osmomath.Dec `json:"percentFilled"`
OrderbookAddress string `json:"orderbookAddress"`
Status OrderStatus `json:"status"`
Output osmomath.Dec `json:"output"`
QuoteAsset Asset `json:"quote_asset"`
BaseAsset Asset `json:"base_asset"`
PlacedTx *string `json:"placed_tx,omitempty"`
}

// IsClaimable reports whether the limit order is filled above the given
Expand All @@ -112,6 +146,21 @@ func (o LimitOrder) IsClaimable(threshold osmomath.Dec) bool {
return o.PercentFilled.GT(threshold) && o.PercentFilled.LTE(osmomath.OneDec())
}

// 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)
}
Comment on lines +149 to +162
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 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 using PlacedQuantity.Sub(o.Output.Mul(o.PercentFilled))
  • BaseAssetFillableAmount: Calculates remaining base asset using Quantity.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


// OrderbookResult represents orderbook orders result.
type OrderbookResult struct {
LimitOrders []LimitOrder // The channel on which the orders are delivered.
Expand Down
109 changes: 91 additions & 18 deletions domain/orderbook/order_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,49 +82,49 @@ func TestOrdersByDirection(t *testing.T) {
testCases := []struct {
name string
orders orderbookdomain.Orders
direction string
direction orderbookdomain.OrderDirection
expectedOrders orderbookdomain.Orders
}{
{
name: "Filter buy orders",
orders: orderbookdomain.Orders{
{OrderDirection: "buy", OrderId: 1},
{OrderDirection: "sell", OrderId: 2},
{OrderDirection: "buy", OrderId: 3},
{OrderDirection: orderbookdomain.DirectionBid.String(), OrderId: 1},
{OrderDirection: orderbookdomain.DirectionAsk.String(), OrderId: 2},
{OrderDirection: orderbookdomain.DirectionBid.String(), OrderId: 3},
},
direction: "buy",
direction: orderbookdomain.DirectionBid,
expectedOrders: orderbookdomain.Orders{
{OrderDirection: "buy", OrderId: 1},
{OrderDirection: "buy", OrderId: 3},
{OrderDirection: orderbookdomain.DirectionBid.String(), OrderId: 1},
{OrderDirection: orderbookdomain.DirectionBid.String(), OrderId: 3},
},
},
{
name: "Filter sell orders",
orders: orderbookdomain.Orders{
{OrderDirection: "buy", OrderId: 1},
{OrderDirection: "sell", OrderId: 2},
{OrderDirection: "buy", OrderId: 3},
{OrderDirection: "sell", OrderId: 4},
{OrderDirection: orderbookdomain.DirectionBid.String(), OrderId: 1},
{OrderDirection: orderbookdomain.DirectionAsk.String(), OrderId: 2},
{OrderDirection: orderbookdomain.DirectionBid.String(), OrderId: 3},
{OrderDirection: orderbookdomain.DirectionAsk.String(), OrderId: 4},
},
direction: "sell",
direction: orderbookdomain.DirectionAsk,
expectedOrders: orderbookdomain.Orders{
{OrderDirection: "sell", OrderId: 2},
{OrderDirection: "sell", OrderId: 4},
{OrderDirection: orderbookdomain.DirectionAsk.String(), OrderId: 2},
{OrderDirection: orderbookdomain.DirectionAsk.String(), OrderId: 4},
},
},
{
name: "No matching orders",
orders: orderbookdomain.Orders{
{OrderDirection: "buy", OrderId: 1},
{OrderDirection: "buy", OrderId: 2},
{OrderDirection: orderbookdomain.DirectionBid.String(), OrderId: 1},
{OrderDirection: orderbookdomain.DirectionBid.String(), OrderId: 2},
},
direction: "sell",
direction: orderbookdomain.DirectionAsk,
expectedOrders: nil,
},
{
name: "Empty orders slice",
orders: orderbookdomain.Orders{},
direction: "buy",
direction: orderbookdomain.DirectionBid,
expectedOrders: nil,
},
}
Expand Down Expand Up @@ -185,3 +185,76 @@ func TestLimitOrder_IsClaimable(t *testing.T) {
})
}
}

func TestClaimableAmount(t *testing.T) {
tests := []struct {
name string
order orderbookdomain.LimitOrder
want osmomath.Dec
}{
{
name: "Buy 10 OSMO for 1 USD with 50% filled and 0.25% claimed",
order: orderbookdomain.LimitOrder{
OrderDirection: orderbookdomain.DirectionBid,
TotalFilled: osmomath.NewDec(5),
PercentClaimed: osmomath.MustNewDecFromStr("0.25"),
},
want: osmomath.MustNewDecFromStr("3.75"),
},
{
name: "Buy 10 OSMO with 0% filled and 1% claimed",
order: orderbookdomain.LimitOrder{
OrderDirection: orderbookdomain.DirectionBid,
TotalFilled: osmomath.NewDec(0),
PercentClaimed: osmomath.MustNewDecFromStr("1"),
},
want: osmomath.MustNewDecFromStr("0"),
},
{
name: "Buy 10 OSMO with 5% filled and 0% claimed",
order: orderbookdomain.LimitOrder{
OrderDirection: orderbookdomain.DirectionBid,
TotalFilled: osmomath.NewDec(5),
PercentClaimed: osmomath.MustNewDecFromStr("0"),
},
want: osmomath.MustNewDecFromStr("5"),
},
{
name: "Sell 0.1 OSMO for 1 USD, with 50% filled and 0.25% claimed",
order: orderbookdomain.LimitOrder{
OrderDirection: orderbookdomain.DirectionAsk,
Output: osmomath.NewDec(10),
TotalFilled: osmomath.MustNewDecFromStr("0.5"),
PercentClaimed: osmomath.MustNewDecFromStr("0.25"),
},
want: osmomath.MustNewDecFromStr("0.375"),
},
{
name: "Sell 0.1 OSMO for 1 USD, with 0% filled and 2% claimed",
order: orderbookdomain.LimitOrder{
OrderDirection: orderbookdomain.DirectionAsk,
Output: osmomath.NewDec(10),
TotalFilled: osmomath.NewDec(0),
PercentClaimed: osmomath.MustNewDecFromStr("0.2"),
},
want: osmomath.NewDec(0),
},
{
name: "Sell 0.1 OSMO for 1 USD, with 3% filled and 0% claimed",
order: orderbookdomain.LimitOrder{
OrderDirection: orderbookdomain.DirectionAsk,
Output: osmomath.NewDec(10),
TotalFilled: osmomath.MustNewDecFromStr("0.3"),
PercentClaimed: osmomath.MustNewDecFromStr("0"),
},
want: osmomath.MustNewDecFromStr("0.3"),
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := tt.order.ClaimableAmount()
assert.Equal(t, tt.want, got)
})
}
}
7 changes: 4 additions & 3 deletions orderbook/usecase/orderbook_usecase.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,8 @@ func (o *OrderbookUseCaseImpl) CreateFormattedLimitOrder(orderbook domain.Canoni

// Determine tick values and unrealized cancels based on order direction
var tickEtas, tickUnrealizedCancelled osmomath.Dec
if order.OrderDirection == "bid" {

if orderbookdomain.NewDirection(order.OrderDirection).Is(orderbookdomain.DirectionBid) {
tickEtas, err = osmomath.NewDecFromStr(tickState.BidValues.EffectiveTotalAmountSwapped)
if err != nil {
return orderbookdomain.LimitOrder{}, types.ParsingTickValuesError{
Expand Down Expand Up @@ -460,7 +461,7 @@ func (o *OrderbookUseCaseImpl) CreateFormattedLimitOrder(orderbook domain.Canoni

// Calculate output based on order direction
var output osmomath.Dec
if order.OrderDirection == "bid" {
if orderbookdomain.NewDirection(order.OrderDirection).Is(orderbookdomain.DirectionBid) {
output = placedQuantity.Quo(price.Dec())
} else {
output = placedQuantity.Mul(price.Dec())
Expand All @@ -483,7 +484,7 @@ func (o *OrderbookUseCaseImpl) CreateFormattedLimitOrder(orderbook domain.Canoni
return orderbookdomain.LimitOrder{
TickId: order.TickId,
OrderId: order.OrderId,
OrderDirection: order.OrderDirection,
OrderDirection: orderbookdomain.NewDirection(order.OrderDirection),
Owner: order.Owner,
Quantity: quantity,
Etas: order.Etas,
Expand Down
1 change: 1 addition & 0 deletions passthrough/usecase/export_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ const (
InLocksAssetsCategoryName = inLocksAssetsCategoryName
PooledAssetsCategoryName = pooledAssetsCategoryName
UnclaimedRewardsAssetsCategoryName = unclaimedRewardsAssetsCategoryName
LimitOrdersCategoryName = limitOrdersCategoryName
TotalAssetsCategoryName = totalAssetsCategoryName
)

Expand Down
Loading
Loading