refactor(perp): use collections for state management #952
Conversation
testinginprod
changed the title
|
Why is failing for me on Go test in my local? |
go1.19 has a compiler bug |
|
In order to make this pr easier to review I will address #931 in another PR. |
collections/index_multi.go
Outdated
|
|
||
| // Insert inserts fetches the index key IK from the object v. | ||
| // And then maps the index key to the primary key. | ||
| func (i *MultiIndex[IK, PK, V]) Insert(ctx sdk.Context, pk PK, v V) { |
There was a problem hiding this comment.
Can you add unit tests for MultiIndex?
| // K1 returns the first part of the key, | ||
| // if present. If the key is not present | ||
| // the zero value is returned. | ||
| func (t Pair[K1, K2]) K1() K1 { |
There was a problem hiding this comment.
Are these used anywhere?
There was a problem hiding this comment.
Leftovers from multi index... but we can keep this since it's going to be needed
| @@ -35,7 +35,7 @@ func (m msgServer) RemoveMargin(ctx context.Context, msg *types.MsgRemoveMargin, | |||
| return &types.MsgRemoveMarginResponse{ | |||
| MarginOut: marginOut, | |||
| FundingPayment: fundingPayment, | |||
| Position: position, | |||
| Position: &position, | |||
There was a problem hiding this comment.
Is there a benefit to returning a pointer in the response?
There was a problem hiding this comment.
we avoid the copy but it's more because proto imposes pointers on every message
| @@ -62,7 +68,7 @@ func (q queryServer) QueryTraderPosition( | |||
| } | |||
|
|
|||
| return &types.QueryTraderPositionResponse{ | |||
| Position: position, | |||
| Position: &position, | |||
There was a problem hiding this comment.
Is there a benefit to returning a pointer in the response?
There was a problem hiding this comment.
we avoid the copy but it's more because proto imposes pointers on every message
There was a problem hiding this comment.
What if we set (gogoproto.nullable) = false?
There was a problem hiding this comment.
we break proto API, because then it's impossible to assert presence on the msg, there are some cases in which it's done in the sdk but im not a fan if i have to be honest.
Description
Moves perp positions to collections
Implements query positions by traders.
Purpose