-
Notifications
You must be signed in to change notification settings - Fork 42
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
delete recent price from db when delisting #753
Conversation
538842d
to
d9e041a
Compare
d9e041a
to
f7bda47
Compare
plugins/dex/store/mapper.go
Outdated
} | ||
func removePrice(price []int64, i int) []int64 { | ||
return append(price[:i], price[i+1:]...) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest giving the RecentPrice
struct a method(like removePair) to remove both pair and price at the same time.
plugins/dex/store/mapper.go
Outdated
@@ -172,6 +173,36 @@ func (m mapper) GetRecentPrices(ctx sdk.Context, pricesStoreEvery, numPricesStor | |||
return recentPrices | |||
} | |||
|
|||
func (m mapper) DeleteRecentPrices(ctx sdk.Context, baseAsset, quoteAsset string) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just pass the trading symbol as the parameter
plugins/dex/order/keeper.go
Outdated
@@ -949,6 +949,7 @@ func (kp *DexKeeper) DelistTradingPair(ctx sdk.Context, symbol string, postAlloc | |||
if err != nil { | |||
kp.logger.Error("delete trading pair error", "err", err.Error()) | |||
} | |||
kp.PairMapper.DeleteRecentPrices(ctx, baseAsset, quoteAsset) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
better wrap a method to remove both from memory and db.
plugins/dex/store/mapper_test.go
Outdated
|
||
func TestMapper_DeleteRecentPrices(t *testing.T) { | ||
pairMapper, ctx := setup() | ||
for i := 0; i < 3000; i++ { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can be much smaller if it is not a benchmark
plugins/dex/store/mapper_test.go
Outdated
require.Equal(t, []interface{}{int64(10), int64(10), int64(10), int64(10), int64(10)}, allRecentPrices["ABC_BNB"].Elements()) | ||
|
||
pairMapper.DeleteRecentPrices(ctx, "ABC", "BNB") | ||
allRecentPrices = pairMapper.GetRecentPrices(ctx, 2, 5) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the parameters (ctx, 2, 5) are different from the above (ctx, 2, 10).
I suggest using a const instead.
plugins/dex/store/mapper_test.go
Outdated
pairMapper.AddTradingPair(ctx, tradingPair) | ||
} | ||
|
||
for i := 0; i < 2000; i++ { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest using some const instead to make it more readable.
064d9d4
to
db682e3
Compare
plugins/dex/store/types.go
Outdated
@@ -35,3 +35,15 @@ type RecentPrice struct { | |||
Pair []string | |||
Price []int64 | |||
} | |||
|
|||
func (prices *RecentPrice) removeRecentPrice(symbolToDelete string) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe a little confusing about the name. "recentPrice.removeRecentPrice"
pairMapper, ctx := setup() | ||
for i := 0; i < 30; i++ { | ||
lastPrices := make(map[string]int64, pairNum) | ||
lastPrices["ABC_BNB"] = 10 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make some changes of the prices
delete recent price from db when delisting
* delete recent price from db when delisting (#753) delete recent price from db when delisting * bump up version * update changelog * delete recent price key when recent price map is empty (#760) * Force match all BEP2 symbols on BEP8 upgrade height to update last ma… (#758) Force match all BEP2 symbols on BEP8 upgrade height to update last match height * Fix mini msg (#762) * fix unfreeze err msg; adjust mini issue/list fee; update changelog
delete recent price from db when delisting
* delete recent price from db when delisting (#753) delete recent price from db when delisting * bump up version * update changelog * delete recent price key when recent price map is empty (#760) * Force match all BEP2 symbols on BEP8 upgrade height to update last ma… (#758) Force match all BEP2 symbols on BEP8 upgrade height to update last match height * Fix mini msg (#762) * fix unfreeze err msg; adjust mini issue/list fee; update changelog
delete recent price from db when delisting
* delete recent price from db when delisting (#753) delete recent price from db when delisting * bump up version * update changelog * delete recent price key when recent price map is empty (#760) * Force match all BEP2 symbols on BEP8 upgrade height to update last ma… (#758) Force match all BEP2 symbols on BEP8 upgrade height to update last match height * Fix mini msg (#762) * fix unfreeze err msg; adjust mini issue/list fee; update changelog
delete recent price from db when delisting
* delete recent price from db when delisting (#753) delete recent price from db when delisting * bump up version * update changelog * delete recent price key when recent price map is empty (#760) * Force match all BEP2 symbols on BEP8 upgrade height to update last ma… (#758) Force match all BEP2 symbols on BEP8 upgrade height to update last match height * Fix mini msg (#762) * fix unfreeze err msg; adjust mini issue/list fee; update changelog
delete recent price from db when delisting
* delete recent price from db when delisting (#753) delete recent price from db when delisting * bump up version * update changelog * delete recent price key when recent price map is empty (#760) * Force match all BEP2 symbols on BEP8 upgrade height to update last ma… (#758) Force match all BEP2 symbols on BEP8 upgrade height to update last match height * Fix mini msg (#762) * fix unfreeze err msg; adjust mini issue/list fee; update changelog
delete recent price from db when delisting
* delete recent price from db when delisting (#753) delete recent price from db when delisting * bump up version * update changelog * delete recent price key when recent price map is empty (#760) * Force match all BEP2 symbols on BEP8 upgrade height to update last ma… (#758) Force match all BEP2 symbols on BEP8 upgrade height to update last match height * Fix mini msg (#762) * fix unfreeze err msg; adjust mini issue/list fee; update changelog
delete recent price from db when delisting
* delete recent price from db when delisting (#753) delete recent price from db when delisting * bump up version * update changelog * delete recent price key when recent price map is empty (#760) * Force match all BEP2 symbols on BEP8 upgrade height to update last ma… (#758) Force match all BEP2 symbols on BEP8 upgrade height to update last match height * Fix mini msg (#762) * fix unfreeze err msg; adjust mini issue/list fee; update changelog
delete recent price from db when delisting
* delete recent price from db when delisting (#753) delete recent price from db when delisting * bump up version * update changelog * delete recent price key when recent price map is empty (#760) * Force match all BEP2 symbols on BEP8 upgrade height to update last ma… (#758) Force match all BEP2 symbols on BEP8 upgrade height to update last match height * Fix mini msg (#762) * fix unfreeze err msg; adjust mini issue/list fee; update changelog
delete recent price from db when delisting
* delete recent price from db when delisting (#753) delete recent price from db when delisting * bump up version * update changelog * delete recent price key when recent price map is empty (#760) * Force match all BEP2 symbols on BEP8 upgrade height to update last ma… (#758) Force match all BEP2 symbols on BEP8 upgrade height to update last match height * Fix mini msg (#762) * fix unfreeze err msg; adjust mini issue/list fee; update changelog
delete recent price from db when delisting
* delete recent price from db when delisting (#753) delete recent price from db when delisting * bump up version * update changelog * delete recent price key when recent price map is empty (#760) * Force match all BEP2 symbols on BEP8 upgrade height to update last ma… (#758) Force match all BEP2 symbols on BEP8 upgrade height to update last match height * Fix mini msg (#762) * fix unfreeze err msg; adjust mini issue/list fee; update changelog
delete recent price from db when delisting
* delete recent price from db when delisting (#753) delete recent price from db when delisting * bump up version * update changelog * delete recent price key when recent price map is empty (#760) * Force match all BEP2 symbols on BEP8 upgrade height to update last ma… (#758) Force match all BEP2 symbols on BEP8 upgrade height to update last match height * Fix mini msg (#762) * fix unfreeze err msg; adjust mini issue/list fee; update changelog
delete recent price from db when delisting
Description
add a description of your changes here...
Rationale
tell us why we need these changes...
Example
add an example CLI or API response...
Changes
Notable changes:
Preflight checks
make build
)make test
)make integration_test
)Already reviewed by
...
Related issues
... reference related issue #'s here ...