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(oracle): add support for get price indexed by assetID #77

Merged
merged 6 commits into from
Jul 12, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
18 changes: 16 additions & 2 deletions x/oracle/keeper/aggregator/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ type roundInfo struct {
// AggregatorContext keeps memory cache for state params, validatorset, and updatedthese values as they updated on chain. And it keeps the information to track all tokenFeeders' status and data collection
// nolint
type AggregatorContext struct {
params *common.Params
params *types.Params

// validator->power
validatorsPower map[string]*big.Int
Expand Down Expand Up @@ -270,7 +270,7 @@ func (agc *AggregatorContext) PrepareRound(ctx sdk.Context, block uint64) {
}
}

func (agc *AggregatorContext) SetParams(p *common.Params) {
func (agc *AggregatorContext) SetParams(p *types.Params) {
agc.params = p
}

Expand All @@ -288,6 +288,20 @@ func (agc *AggregatorContext) GetValidatorPowers() (vp map[string]*big.Int) {
return agc.validatorsPower
}

func (agc *AggregatorContext) GetTokenIDFromAssetID(assetID string) int {
agc.params.GetTokenIDFromAssetID(assetID)
cloud8little marked this conversation as resolved.
Show resolved Hide resolved
for id, token := range agc.params.Tokens {
if token.AssetID == assetID {
return id
}
}
return 0
}

func (agc *AggregatorContext) GetParams() types.Params {
return *agc.params
}

func NewAggregatorContext() *AggregatorContext {
return &AggregatorContext{
validatorsPower: make(map[string]*big.Int),
Expand Down
3 changes: 1 addition & 2 deletions x/oracle/keeper/aggregator/context_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,9 @@ func initAggregatorContext4Test() *AggregatorContext {
}

p := defaultParams
pWrapped := common.Params(p)

agc.SetValidatorPowers(validatorPowers)
agc.SetParams(&pWrapped)
agc.SetParams(&p)
return agc
}

Expand Down
16 changes: 8 additions & 8 deletions x/oracle/keeper/cache/caches.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ var zeroBig = big.NewInt(0)

type (
ItemV map[string]*big.Int
ItemP *common.Params
ItemP types.Params
ItemM types.MsgItem
)

Expand All @@ -22,7 +22,6 @@ type Cache struct {
params *cacheParams
}

// type cacheMsgs map[uint64][]*ItemM
type cacheMsgs []*ItemM

// used to track validator change
Expand All @@ -33,7 +32,8 @@ type cacheValidator struct {

// used to track params change
type cacheParams struct {
params *common.Params
// params types.Params
params *ItemP
update bool
}

Expand Down Expand Up @@ -102,10 +102,10 @@ func (c *cacheValidator) commit(ctx sdk.Context, k common.KeeperOracle) {
k.SetValidatorUpdateBlock(ctx, types.ValidatorUpdateBlock{Block: block})
}

func (c *cacheParams) add(p *common.Params) {
func (c *cacheParams) add(p ItemP) {
// params' update is triggered when params is actually updated, so no need to do comparison here, just udpate and mark the flag
// TODO: add comparison check, that's something should be done for validation
c.params = p
c.params = &p
Comment on lines +105 to +108
Copy link
Contributor

Choose a reason for hiding this comment

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

Add validation check for parameter updates.

The function updates the parameters without comparison, which might lead to unnecessary updates. Consider adding a validation check to ensure that the parameters have actually changed.

-  // TODO: add comparison check, that's something should be done for validation
-  c.params = &p
+  if !reflect.DeepEqual(c.params, &p) {
+    c.params = &p
+    c.update = true
+  }
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.

Suggested change
func (c *cacheParams) add(p ItemP) {
// params' update is triggered when params is actually updated, so no need to do comparison here, just udpate and mark the flag
// TODO: add comparison check, that's something should be done for validation
c.params = p
c.params = &p
func (c *cacheParams) add(p ItemP) {
// params' update is triggered when params is actually updated, so no need to do comparison here, just udpate and mark the flag
if !reflect.DeepEqual(c.params, &p) {
c.params = &p
c.update = true
}

c.update = true
}

Expand Down Expand Up @@ -164,11 +164,11 @@ func (c *Cache) GetCache(i any) bool {
item[addr] = power
}
return c.validators.update
case ItemP:
case *ItemP:
if item == nil {
return false
}
*item = *(c.params.params)
*item = *c.params.params
return c.params.update
case *([]*ItemM):
if item == nil {
Expand Down Expand Up @@ -218,7 +218,7 @@ func NewCache() *Cache {
validators: make(map[string]*big.Int),
},
params: &cacheParams{
params: &common.Params{},
params: &ItemP{},
},
}
}
9 changes: 4 additions & 5 deletions x/oracle/keeper/cache/caches_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"math/big"
"testing"

"github.com/ExocoreNetwork/exocore/x/oracle/keeper/common"
"github.com/ExocoreNetwork/exocore/x/oracle/types"
. "github.com/smartystreets/goconvey/convey"
// "go.uber.org/mock/gomock"
Expand All @@ -13,7 +12,7 @@ import (
func TestCache(t *testing.T) {
c := NewCache()
p := defaultParams
pWrapped := common.Params(p)
pWrapped := ItemP(p)

// ctrl := gomock.NewController(t)
// defer ctrl.Finish()
Expand All @@ -22,9 +21,9 @@ func TestCache(t *testing.T) {

Convey("test cache", t, func() {
Convey("add pramams item", func() {
c.AddCache(ItemP(&pWrapped))
pReturn := &common.Params{}
c.GetCache(ItemP(pReturn))
c.AddCache(pWrapped)
pReturn := &ItemP{}
c.GetCache(pReturn)
So(*pReturn, ShouldResemble, pWrapped)
})

Expand Down
11 changes: 9 additions & 2 deletions x/oracle/keeper/common/expected_keepers.go
Original file line number Diff line number Diff line change
@@ -1,14 +1,19 @@
package common

import (
"cosmossdk.io/math"
sdkmath "cosmossdk.io/math"
stakingkeeper "github.com/ExocoreNetwork/exocore/x/dogfood/keeper"
"github.com/ExocoreNetwork/exocore/x/oracle/types"
abci "github.com/cometbft/cometbft/abci/types"
sdk "github.com/cosmos/cosmos-sdk/types"
stakingTypes "github.com/cosmos/cosmos-sdk/x/staking/types"
)

type Price struct {
Value sdkmath.Int
Decimal uint8
}

type KeeperOracle interface {
KeeperStaking

Expand All @@ -32,12 +37,14 @@ type KeeperOracle interface {

RemoveRecentParams(sdk.Context, uint64)
RemoveRecentMsg(sdk.Context, uint64)

GetMultipleAssetsPrices(ctx sdk.Context, assets map[string]interface{}) (map[string]types.Price, error)
}

var _ KeeperStaking = stakingkeeper.Keeper{}

type KeeperStaking interface {
GetLastTotalPower(ctx sdk.Context) math.Int
GetLastTotalPower(ctx sdk.Context) sdkmath.Int
IterateBondedValidatorsByPower(ctx sdk.Context, fn func(index int64, validator stakingTypes.ValidatorI) (stop bool))
GetValidatorUpdates(ctx sdk.Context) []abci.ValidatorUpdate
GetValidatorByConsAddr(ctx sdk.Context, consAddr sdk.ConsAddress) (validator stakingTypes.Validator, found bool)
Expand Down
87 changes: 0 additions & 87 deletions x/oracle/keeper/common/types.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,8 @@
package common

import (
"errors"
"math/big"
"sort"

"github.com/ExocoreNetwork/exocore/x/oracle/types"
)

var (
Expand All @@ -24,90 +21,6 @@ var (
Mode int32 = 1
)

type Params types.Params

func (p Params) GetTokenFeeders() []*types.TokenFeeder {
return p.TokenFeeders
}

func (p Params) IsDeterministicSource(sourceID uint64) bool {
return p.Sources[sourceID].Deterministic
}

func (p Params) IsValidSource(sourceID uint64) bool {
if sourceID == 0 {
// custom defined source
return true
}
return p.Sources[sourceID].Valid
}

func (p Params) GetTokenFeeder(feederID uint64) *types.TokenFeeder {
for k, v := range p.TokenFeeders {
if uint64(k) == feederID {
return v
}
}
return nil
}

func (p Params) GetTokenInfo(feederID uint64) *types.Token {
for k, v := range p.TokenFeeders {
if uint64(k) == feederID {
return p.Tokens[v.TokenID]
}
}
return nil
}

func (p Params) CheckRules(feederID uint64, prices []*types.PriceSource) (bool, error) {
feeder := p.TokenFeeders[feederID]
rule := p.Rules[feeder.RuleID]
// specified sources set, v1 use this rule to set `chainlink` as official source
if rule.SourceIDs != nil && len(rule.SourceIDs) > 0 {
if len(rule.SourceIDs) != len(prices) {
return false, errors.New("count prices should match rule")
}
notFound := false
if rule.SourceIDs[0] == 0 {
// match all sources listed
for sID, source := range p.Sources {
if sID == 0 {
continue
}
if source.Valid {
notFound = true
for _, p := range prices {
if p.SourceID == uint64(sID) {
notFound = false
break
}
}

}
}
} else {
for _, source := range rule.SourceIDs {
notFound = true
for _, p := range prices {
if p.SourceID == source {
notFound = false
break
}
}
// return false, errors.New("price source not match with rule")
}
}
if notFound {
return false, errors.New("price source not match with rule")
}
}

// TODO: check NOM
// return true if no rule set, we will accept any source
return true, nil
}

type Set[T comparable] struct {
size int
slice []T
Expand Down
7 changes: 3 additions & 4 deletions x/oracle/keeper/msg_server_create_price_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
math "cosmossdk.io/math"
"github.com/ExocoreNetwork/exocore/x/oracle/keeper"
"github.com/ExocoreNetwork/exocore/x/oracle/keeper/cache"
"github.com/ExocoreNetwork/exocore/x/oracle/keeper/common"
"github.com/ExocoreNetwork/exocore/x/oracle/keeper/testdata"
"github.com/ExocoreNetwork/exocore/x/oracle/types"
. "github.com/agiledragon/gomonkey/v2"
Expand Down Expand Up @@ -81,11 +80,11 @@ var _ = Describe("MsgCreatePrice", func() {
})

c = keeper.GetCaches()
pRes := &common.Params{}
c.GetCache(cache.ItemP(pRes))
var pRes cache.ItemP
c.GetCache(&pRes)
p4Test := types.DefaultParams()
p4Test.TokenFeeders[1].StartBaseBlock = 1
Expect(*pRes).Should(BeEquivalentTo(p4Test))
Expect(pRes).Should(BeEquivalentTo(p4Test))
})

It("success on 3rd message", func() {
Expand Down
36 changes: 33 additions & 3 deletions x/oracle/keeper/prices.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import (
"encoding/binary"

sdkmath "cosmossdk.io/math"
"github.com/ExocoreNetwork/exocore/x/oracle/types"
"github.com/cosmos/cosmos-sdk/store/prefix"
sdk "github.com/cosmos/cosmos-sdk/types"
Expand Down Expand Up @@ -40,10 +41,37 @@
found = true
}
}

return
}

// return latest price for assets
func (k Keeper) GetMultipleAssetsPrices(ctx sdk.Context, assets map[string]interface{}) (map[string]types.Price, error) {
var p types.Params
// get params from cache if exists
if agc != nil {
p = agc.GetParams()
} else {
p = k.GetParams(ctx)
}
ret := make(map[string]types.Price)
for assetID := range assets {
tokenID := p.GetTokenIDFromAssetID(assetID)
if tokenID == 0 {
return nil, types.ErrGetPrices.Wrapf("assetID does not exist in oracle %s", assetID)
}
price, found := k.GetPriceTRLatest(ctx, uint64(tokenID))
if !found {
return nil, types.ErrGetPrices.Wrapf("no valid price for assetID=%s", assetID)
}
v, _ := sdkmath.NewIntFromString(price.Price)
ret[assetID] = types.Price{
Value: v,
Decimal: uint8(price.Decimal),
}
}

Check warning

Code scanning / CodeQL

Iteration over map Warning

Iteration over map may be a possible source of non-determinism
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider potential non-determinism in map iteration.

Iteration over a map may lead to non-deterministic behavior. Consider using a deterministic iteration method.

-  for assetID := range assets {
+  assetIDs := make([]string, 0, len(assets))
+  for assetID := range assets {
+    assetIDs = append(assetIDs, assetID)
+  }
+  sort.Strings(assetIDs)
+  for _, assetID := range assetIDs {
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.

Suggested change
for assetID := range assets {
tokenID := p.GetTokenIDFromAssetID(assetID)
if tokenID == 0 {
return nil, types.ErrGetPrices.Wrapf("assetID does not exist in oracle %s", assetID)
}
price, found := k.GetPriceTRLatest(ctx, uint64(tokenID))
if !found {
return nil, types.ErrGetPrices.Wrapf("no valid price for assetID=%s", assetID)
}
v, _ := sdkmath.NewIntFromString(price.Price)
ret[assetID] = types.Price{
Value: v,
Decimal: uint8(price.Decimal),
}
}
assetIDs := make([]string, 0, len(assets))
for assetID := range assets {
assetIDs = append(assetIDs, assetID)
}
sort.Strings(assetIDs)
for _, assetID := range assetIDs {
tokenID := p.GetTokenIDFromAssetID(assetID)
if tokenID == 0 {
return nil, types.ErrGetPrices.Wrapf("assetID does not exist in oracle %s", assetID)
}
price, found := k.GetPriceTRLatest(ctx, uint64(tokenID))
if !found {
return nil, types.ErrGetPrices.Wrapf("no valid price for assetID=%s", assetID)
}
v, _ := sdkmath.NewIntFromString(price.Price)
ret[assetID] = types.Price{
Value: v,
Decimal: uint8(price.Decimal),
}
}
Tools
GitHub Check: CodeQL

[warning] 57-71: Iteration over map
Iteration over map may be a possible source of non-determinism

return ret, nil
}

// RemovePrices removes a prices from the store
func (k Keeper) RemovePrices(
ctx sdk.Context,
Expand Down Expand Up @@ -118,14 +146,16 @@
}

func (k Keeper) GetPriceTRLatest(ctx sdk.Context, tokenID uint64) (price types.PriceTimeRound, found bool) {
// store := prefix.NewStore(ctx.KVStore(k.storeKey), types.KeyPrefix(types.PricesKeyPrefix))
// store = prefix.NewStore(store, types.PricesKey(tokenID))
store := k.getPriceTRStore(ctx, tokenID)
nextRoundIDB := store.Get(types.PricesNextRoundIDKey)
if nextRoundIDB == nil {
return
}
nextRoundID := binary.BigEndian.Uint64(nextRoundIDB)
// this token has no valid round yet
if nextRoundID <= 1 {
return
}
b := store.Get(types.PricesRoundKey(nextRoundID - 1))
if b != nil {
// should always be true
Expand Down
Loading
Loading