feat(oracle): add support for get price indexed by assetID #77
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
x/oracle/keeper/prices.go
Outdated
| 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
|
Can you please add some information about this PR and how it's working? I see that you're looping through one of the identifiers to find the other (assetID and tokenID) whenever a query is received. Doing so may be too costly to do it as frequently as we need. |
There was a problem hiding this comment.
Maybe some states can be stored in the module through kvstore instead of in the params, such as the tokenID. Using assetID as the key and the tokenID as the value can remove the repeated information stored in the Oracle module and reduce the cost when querying the tokenID from the assetID.
Additionally, would it be better to allocate a number as the tokenID for all assets in the assets module? This way, the Oracle module can use these states directly instead of storing them.
The values all cached in memory, and only updated when the params changed(through gov module which is not frequently), does not rely on IO access |
The update of this state should be controlled by gov module through proposal/vote, so just put them in params. And for the frequently query, they basically accessed from memory cache |
I see, that seems fair. But can you please provide a small doc (2 sentences or so) about how this is working? It is not clear from the code. @coderabbitai review |
|
Tip For best results, initiate chat on the files or code changes. @MaxMustermann2, based on the provided code and the output from the script, here's a brief explanation of how the PR's functionality works: The PR introduces a mechanism where
This approach leverages caching to efficiently handle queries related to asset prices, ensuring that the system remains performant even as the frequency of queries increases. Analysis chainScripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Find and display the relevant parts of the code where assetIDs and tokenIDs are handled.
rg "assetID|tokenID" --context 5
Length of output: 208978 |
Updated. the unclear may be introduced by the code refactor. |
|
Tip For best results, initiate chat on the files or code changes. @leonz789, here is a concise documentation on how the functionality introduced by this PR works: This PR enhances the Oracle module by introducing a caching mechanism for
This approach ensures that the Oracle module remains efficient and responsive, even as the frequency of queries increases. |
leonz789
force-pushed
the
develop-oracle-assets
branch
from
957268a to
d452e62
Compare
WalkthroughThis update enhances the Oracle module in a Go application. It includes a version bump for Changes
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Outside diff range and nitpick comments (2)
x/oracle/keeper/aggregator/context.go (2)
Line range hint
222-272: Optimize thePrepareRoundmethod and fix typographical errors.The logic in the
PrepareRoundmethod can be simplified and optimized. Additionally, fix the typographical errors in the comments.// PrepareRound prepares the roundInfo for the next block, it will update the roundInfo based on the current block height func (agc *AggregatorContext) PrepareRound(ctx sdk.Context, block uint64) { // block > 0 means recache initialization, all roundInfo is empty if block == 0 { block = uint64(ctx.BlockHeight()) } for feederID, feeder := range agc.params.GetTokenFeeders() { if feederID == 0 || (feeder.EndBlock > 0 && feeder.EndBlock <= block) || feeder.StartBaseBlock > block { // this feeder is inactive continue } delta := block - feeder.StartBaseBlock left := delta % feeder.Interval count := delta / feeder.Interval latestBasedBlock := block - left latestNextRoundID := feeder.StartRoundID + count feederIDUint64 := uint64(feederID) round := agc.rounds[feederIDUint64] if round == nil { round = &roundInfo{ basedBlock: latestBasedBlock, nextRoundID: latestNextRoundID, status: 1, } if left >= uint64(common.MaxNonce) { round.status = 2 } agc.rounds[feederIDUint64] = round } else { // prepare a new round for existing roundInfo if left == 0 { round.basedBlock = latestBasedBlock round.nextRoundID = latestNextRoundID round.status = 1 // drop previous worker delete(agc.aggregators, feederIDUint64) } else if round.status == 1 && left >= uint64(common.MaxNonce) { // this shouldn't happen if sealRound is done properly before prepareRound, basically for test only round.status = 2 // TODO: just modify the status here, since sealRound should do all the related seal actions already when prepare is invoked } } } }
Line range hint
308-316: Improve readability by moving initialization to a separate function.Consider moving the initialization of
validatorsPowerandtotalPowerto a separate function to improve readability.func NewAggregatorContext() *AggregatorContext { return &AggregatorContext{ validatorsPower: make(map[string]*big.Int), totalPower: big.NewInt(0), rounds: make(map[uint64]*roundInfo), aggregators: make(map[uint64]*worker), } } func newValidatorPowerContext() map[string]*big.Int { return make(map[string]*big.Int) } func newTotalPower() *big.Int { return big.NewInt(0) }
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (15)
- go.mod (2 hunks)
- x/oracle/keeper/aggregator/context.go (4 hunks)
- x/oracle/keeper/aggregator/context_test.go (1 hunks)
- x/oracle/keeper/cache/caches.go (6 hunks)
- x/oracle/keeper/cache/caches_test.go (3 hunks)
- x/oracle/keeper/common/expected_keepers.go (2 hunks)
- x/oracle/keeper/common/types.go (2 hunks)
- x/oracle/keeper/msg_server_create_price_test.go (2 hunks)
- x/oracle/keeper/prices.go (3 hunks)
- x/oracle/keeper/prices_test.go (2 hunks)
- x/oracle/keeper/query_prices.go (1 hunks)
- x/oracle/keeper/single.go (4 hunks)
- x/oracle/types/errors.go (1 hunks)
- x/oracle/types/params.go (3 hunks)
- x/oracle/types/types.go (1 hunks)
Files skipped from review due to trivial changes (2)
- go.mod
- x/oracle/keeper/query_prices.go
Additional comments not posted (36)
x/oracle/types/types.go (2)
3-7: Imports look good.The new import for
cosmossdk.io/mathis correctly added.
9-12: Struct definition is correct.The
Pricestruct is well-defined with appropriate field types.x/oracle/types/errors.go (1)
14-14: New error addition looks good.The new error
ErrGetPricesis correctly registered with the appropriate module name and error message.x/oracle/keeper/common/expected_keepers.go (4)
4-4: Import alias renaming looks good.The import alias for
cosmossdk.io/mathhas been correctly renamed tosdkmath.
12-15: Struct definition is correct.The
Pricestruct is well-defined with appropriate field types.
41-41: New method addition looks good.The
GetMultipleAssetsPricesmethod is correctly added to theKeeperOracleinterface.
47-47: Method signature update is correct.The
GetLastTotalPowermethod in theKeeperStakinginterface is correctly updated to usesdkmath.Int.x/oracle/keeper/aggregator/context_test.go (1)
88-88: LGTM! The addition of theSetParamscall is correct.The call to
SetParamsensures that theAggregatorContextis initialized with the default parameters.x/oracle/keeper/prices_test.go (2)
7-7: LGTM! The new import forcosmossdk.io/mathis necessary.The import is used in the new test function
TestPricesGetMultiAssets.
50-68: The new test functionTestPricesGetMultiAssetsis well-implemented but has a commented-out line.The test function correctly tests the
GetMultipleAssetsPricesmethod. However, there is a commented-out line that should be removed or uncommented.- //v, _ := sdkmath.NewIntFromString(testdata.PTR5.Price)x/oracle/keeper/cache/caches_test.go (2)
15-15: LGTM! The wrapping ofdefaultParamsinItemPis correct.The wrapping ensures that the parameters are correctly added to the cache.
24-26: LGTM! The addition of theAddCacheandGetCachecalls is correct.The calls ensure that the parameters are correctly added to and retrieved from the cache.
x/oracle/keeper/msg_server_create_price_test.go (1)
83-87: LGTM! The addition of theGetCachecall and the parameter checks is correct.The calls ensure that the parameters are correctly retrieved from the cache and checked against the expected values.
x/oracle/keeper/cache/caches.go (6)
Line range hint
112-115: LGTM!The code changes are approved.
Line range hint
146-149: LGTM!The code changes are approved.
167-171: LGTM!The code changes are approved.
Line range hint
194-198: LGTM!The code changes are approved.
Line range hint
201-203: LGTM!The code changes are approved.
221-223: LGTM!The code changes are approved.
x/oracle/types/params.go (8)
45-45: LGTM!The code changes are approved.
109-116: LGTM!The code changes are approved.
118-120: LGTM!The code changes are approved.
122-128: LGTM!The code changes are approved.
130-137: LGTM!The code changes are approved.
139-146: LGTM!The code changes are approved.
148-193: LGTM!The code changes are approved.
108-108: LGTM!The code changes are approved.
x/oracle/keeper/single.go (5)
102-104: LGTM!The code changes are approved.
123-135: LGTM!The code changes are approved.
138-140: LGTM!The code changes are approved.
151-155: LGTM!The code changes are approved.
185-185: LGTM!The code changes are approved.
x/oracle/keeper/aggregator/context.go (4)
274-276: LGTM!The
SetParamsmethod is simple and straightforward.
Line range hint
279-288: Remove the commented-out line.The commented-out line
// t := big.NewInt(0)should be removed for cleanliness.func (agc *AggregatorContext) SetValidatorPowers(vp map[string]*big.Int) { agc.totalPower = big.NewInt(0) agc.validatorsPower = make(map[string]*big.Int) for addr, power := range vp { agc.validatorsPower[addr] = power agc.totalPower = new(big.Int).Add(agc.totalPower, power) } }
290-292: LGTM!The
GetValidatorPowersmethod is simple and straightforward.
303-306: LGTM!The
GetParamsmethod is simple and straightforward.
| // GetTokenIDFromAssetID returns tokenID for corresponding tokenID, it returns 0 if agc.params is nil or assetID not found in agc.params | ||
| func (agc *AggregatorContext) GetTokenIDFromAssetID(assetID string) int { | ||
| if agc.params == nil { | ||
| return 0 | ||
| } | ||
| return agc.params.GetTokenIDFromAssetID(assetID) | ||
| } |
There was a problem hiding this comment.
Improve error handling in GetTokenIDFromAssetID.
Instead of returning 0 when agc.params is nil or assetID is not found, consider returning an error to provide more context.
func (agc *AggregatorContext) GetTokenIDFromAssetID(assetID string) (int, error) {
if agc.params == nil {
return 0, errors.New("params are not set")
}
tokenID := agc.params.GetTokenIDFromAssetID(assetID)
if tokenID == 0 {
return 0, errors.New("assetID not found")
}
return tokenID, 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.
| // GetTokenIDFromAssetID returns tokenID for corresponding tokenID, it returns 0 if agc.params is nil or assetID not found in agc.params | |
| func (agc *AggregatorContext) GetTokenIDFromAssetID(assetID string) int { | |
| if agc.params == nil { | |
| return 0 | |
| } | |
| return agc.params.GetTokenIDFromAssetID(assetID) | |
| } | |
| func (agc *AggregatorContext) GetTokenIDFromAssetID(assetID string) (int, error) { | |
| if agc.params == nil { | |
| return 0, errors.New("params are not set") | |
| } | |
| tokenID := agc.params.GetTokenIDFromAssetID(assetID) | |
| if tokenID == 0 { | |
| return 0, errors.New("assetID not found") | |
| } | |
| return tokenID, nil | |
| } |
| 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 |
There was a problem hiding this comment.
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.
| 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 | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- x/oracle/keeper/aggregator/context.go (4 hunks)
- x/oracle/keeper/common/expected_keepers.go (3 hunks)
- x/oracle/keeper/msg_server_create_price_test.go (2 hunks)
- x/oracle/keeper/single.go (4 hunks)
- x/oracle/types/errors.go (1 hunks)
Files skipped from review as they are similar to previous changes (4)
- x/oracle/keeper/aggregator/context.go
- x/oracle/keeper/common/expected_keepers.go
- x/oracle/keeper/msg_server_create_price_test.go
- x/oracle/types/errors.go
Additional context used
GitHub Check: CodeQL
x/oracle/keeper/single.go
[warning] 125-130: Iteration over map
Iteration over map may be a possible source of non-determinism
Additional comments not posted (4)
x/oracle/keeper/single.go (4)
131-132: Ensure proper error handling for SetParams.The
SetParamsandsetCommonParamsmethods are invoked without error handling. Ensure that these methods handle errors appropriately to avoid silent failures.
148-151: Ensure proper error handling for SetParams and setCommonParams.The
SetParamsandsetCommonParamsmethods are invoked without error handling. Ensure that these methods handle errors appropriately to avoid silent failures.
184-184: Ensure proper error handling for setCommonParams.The
setCommonParamsmethod is invoked without error handling. Ensure that this method handles errors appropriately to avoid silent failures.
135-137: Ensure proper error handling for GetCache.The
GetCachemethod is invoked without error handling. Ensure that this method handles errors appropriately to avoid silent failures.
x/oracle/keeper/single.go
Outdated
| var p *types.Params | ||
| var b int64 | ||
| if from >= to { | ||
| // backwards compatible for that the validatorUpdateBlock updated every block | ||
| prev := int64(0) | ||
| for b, p := range recentParamsMap { | ||
| for b, p = range recentParamsMap { |
There was a problem hiding this comment.
Optimize map iteration for determinism.
Iteration over maps in Go is non-deterministic. Consider sorting the keys before iteration to ensure deterministic behavior.
- for b, p = range recentParamsMap {
+ keys := make([]int64, 0, len(recentParamsMap))
+ for k := range recentParamsMap {
+ keys = append(keys, k)
+ }
+ sort.Slice(keys, func(i, j int) bool { return keys[i] < keys[j] })
+ for _, b := range keys {
+ p = recentParamsMap[b]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.
| var p *types.Params | |
| var b int64 | |
| if from >= to { | |
| // backwards compatible for that the validatorUpdateBlock updated every block | |
| prev := int64(0) | |
| for b, p := range recentParamsMap { | |
| for b, p = range recentParamsMap { | |
| var p *types.Params | |
| var b int64 | |
| if from >= to { | |
| // backwards compatible for that the validatorUpdateBlock updated every block | |
| prev := int64(0) | |
| keys := make([]int64, 0, len(recentParamsMap)) | |
| for k := range recentParamsMap { | |
| keys = append(keys, k) | |
| } | |
| sort.Slice(keys, func(i, j int) bool { return keys[i] < keys[j] }) | |
| for _, b := range keys { | |
| p = recentParamsMap[b] |
leonz789
force-pushed
the
develop-oracle-assets
branch
from
3b3252d to
28806d9
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (13)
- x/oracle/keeper/aggregator/context.go (4 hunks)
- x/oracle/keeper/aggregator/context_test.go (1 hunks)
- x/oracle/keeper/cache/caches.go (6 hunks)
- x/oracle/keeper/cache/caches_test.go (3 hunks)
- x/oracle/keeper/common/expected_keepers.go (3 hunks)
- x/oracle/keeper/common/types.go (2 hunks)
- x/oracle/keeper/msg_server_create_price_test.go (2 hunks)
- x/oracle/keeper/prices.go (3 hunks)
- x/oracle/keeper/prices_test.go (2 hunks)
- x/oracle/keeper/single.go (4 hunks)
- x/oracle/types/errors.go (1 hunks)
- x/oracle/types/params.go (3 hunks)
- x/oracle/types/types.go (1 hunks)
Files skipped from review as they are similar to previous changes (10)
- x/oracle/keeper/aggregator/context.go
- x/oracle/keeper/cache/caches.go
- x/oracle/keeper/cache/caches_test.go
- x/oracle/keeper/common/expected_keepers.go
- x/oracle/keeper/common/types.go
- x/oracle/keeper/msg_server_create_price_test.go
- x/oracle/keeper/prices_test.go
- x/oracle/types/errors.go
- x/oracle/types/params.go
- x/oracle/types/types.go
Additional context used
GitHub Check: CodeQL
x/oracle/keeper/single.go
[warning] 125-130: Iteration over map
Iteration over map may be a possible source of non-determinismx/oracle/keeper/prices.go
[warning] 57-71: Iteration over map
Iteration over map may be a possible source of non-determinism
Additional comments not posted (5)
x/oracle/keeper/aggregator/context_test.go (1)
88-88: Change approved: Updated to usetypes.Params.The update to use
types.Paramsinstead ofcommon.Paramsin theSetParamsmethod aligns with the refactoring goal and appears correct.x/oracle/keeper/single.go (4)
148-148: Change approved: Updated to usetypes.Params.The update to use
types.Paramsinstead ofcommon.Paramsin theSetParamsmethod aligns with the refactoring goal and appears correct.
150-152: Change approved: Updated to usetypes.Params.The update to use
types.Paramsinstead ofcommon.Paramsin thesetCommonParamsmethod aligns with the refactoring goal and appears correct.
184-184: Change approved: Updated to usetypes.Params.The update to use
types.Paramsinstead ofcommon.Paramsin thesetCommonParamsmethod aligns with the refactoring goal and appears correct.
120-125: Optimize map iteration for determinism.Iteration over maps in Go is non-deterministic. Consider sorting the keys before iteration to ensure deterministic behavior.
- for b, p = range recentParamsMap { + keys := make([]int64, 0, len(recentParamsMap)) + for k := range recentParamsMap { + keys = append(keys, k) + } + sort.Slice(keys, func(i, j int) bool { return keys[i] < keys[j] }) + for _, b := range keys { + p = recentParamsMap[b]Likely invalid or redundant comment.
x/oracle/keeper/single.go
Outdated
| agc.SetParams(recentParams) | ||
| prev = b | ||
| setCommonParams(*recentParams) | ||
| setCommonParams(recentParams) |
There was a problem hiding this comment.
Tip
Codebase Verification
Ensure proper error handling for SetParams and setCommonParams.
The SetParams and setCommonParams methods are invoked without error handling. Ensure that these methods handle errors appropriately to avoid silent failures.
setCommonParamsinx/oracle/keeper/single.godoes not include error handling.SetParamsinx/oracle/keeper/aggregator/context.goandx/oracle/keeper/params.goshould be reviewed and updated to handle errors if not already done.
Analysis chain
Ensure proper error handling for SetParams.
The SetParams and setCommonParams methods are invoked without error handling. Ensure that these methods handle errors appropriately to avoid silent failures.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that SetParams and setCommonParams handle errors appropriately.
# Test: Search for the function definitions to verify error handling. Expect: Proper error handling in the function definitions.
rg --type go 'func SetParams|func setCommonParams'
Length of output: 118
Script:
#!/bin/bash
# Description: Verify error handling in SetParams and setCommonParams functions.
# Locate the definition of SetParams and extract its content.
rg --type go 'func SetParams' -A 10
# Extract the content of setCommonParams to verify error handling.
rg --type go 'func setCommonParams' -A 20
Length of output: 455
Script:
#!/bin/bash
# Description: Locate the definition of SetParams across the codebase and verify its implementation.
# Search for the definition of SetParams in all Go files.
rg --type go 'func SetParams' -A 20
Length of output: 37
Script:
#!/bin/bash
# Description: Locate the usage and declaration of SetParams to find its implementation.
# Search for the usage of SetParams in all Go files.
rg --type go 'SetParams'
# Search for the declaration of SetParams in all Go files.
rg --type go 'SetParams('
Length of output: 7050
x/oracle/keeper/prices.go
Outdated
| 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), | ||
| } | ||
| } |
There was a problem hiding this comment.
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.
| 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
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (8)
- app/app.go (1 hunks)
- testutil/utils.go (1 hunks)
- x/operator/keeper/abci.go (4 hunks)
- x/operator/types/expected_keepers.go (2 hunks)
- x/oracle/keeper/prices.go (3 hunks)
- x/oracle/keeper/prices_test.go (2 hunks)
- x/oracle/types/errors.go (1 hunks)
- x/oracle/types/types.go (1 hunks)
Files skipped from review as they are similar to previous changes (3)
- x/oracle/keeper/prices.go
- x/oracle/keeper/prices_test.go
- x/oracle/types/types.go
Additional comments not posted (13)
x/oracle/types/errors.go (2)
22-22: LGTM: New error definitionErrGetPriceAssetNotFoundThe error message is clear and follows the existing naming convention and structure.
23-23: LGTM: New error definitionErrGetPriceRoundNotFoundThe error message is clear and follows the existing naming convention and structure.
x/operator/types/expected_keepers.go (4)
68-69: LGTM: Added methodGetSpecifiedAssetsPriceinOracleKeeperinterfaceThe method signature looks correct and aligns with the intended functionality.
71-72: LGTM: Added methodGetMultipleAssetsPricesinOracleKeeperinterfaceThe method signature looks correct and aligns with the intended functionality.
76-79: LGTM: ImplementedGetSpecifiedAssetsPriceinMockOracleThe implementation returns a hardcoded price, which is appropriate for a mock implementation.
83-89: LGTM: ImplementedGetMultipleAssetsPricesinMockOracleThe implementation returns hardcoded prices for multiple assets, which is appropriate for a mock implementation.
x/operator/keeper/abci.go (2)
54-58: Verify: Error handling logicThe function now handles the new error
ErrGetPriceRoundNotFound. Ensure this error is appropriately handled and documented.Verification successful
Verify: Error handling logic
The error
ErrGetPriceRoundNotFoundis defined and used in multiple places. Ensure this error is appropriately handled in thex/operator/keeper/abci.gofile, particularly around its usage.
Verified: Error handling logic
The error
ErrGetPriceRoundNotFoundis appropriately handled in thex/operator/keeper/abci.gofile. The code checks for this error, and if it matches, it sets a default price to avoid a panic. This behavior is clearly documented with TODO comments.
x/operator/keeper/abci.go: Lines whereErrGetPriceRoundNotFoundis handled.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage and handling of the error `ErrGetPriceRoundNotFound`. # Test: Search for the error usage. Expect: Appropriate handling and documentation. rg --type go 'ErrGetPriceRoundNotFound'Length of output: 594
Script:
#!/bin/bash # Description: Verify the context and handling of `ErrGetPriceRoundNotFound` in `x/operator/keeper/abci.go`. # Fetch the lines around the usage of `ErrGetPriceRoundNotFound` in `x/operator/keeper/abci.go` for better context. rg --type go -A 10 -B 10 'ErrGetPriceRoundNotFound' x/operator/keeper/abci.goLength of output: 2026
107-113: Verify: Error handling logicThe function now handles the new error
ErrGetPriceRoundNotFound. Ensure this error is appropriately handled and documented.Verification successful
Verified: Error handling logic
The error
ErrGetPriceRoundNotFoundis correctly defined and used across the codebase. The handling inx/operator/keeper/abci.goincludes TODO comments indicating future improvements, showing that the current handling is intentional and documented.
x/oracle/keeper/prices.go: Error is returned when no valid price is found.x/operator/keeper/abci.go: Error is caught and ignored with TODO comments.x/oracle/types/errors.go: Error is defined and registered.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage and handling of the error `ErrGetPriceRoundNotFound`. # Test: Search for the error usage. Expect: Appropriate handling and documentation. rg --type go 'ErrGetPriceRoundNotFound'Length of output: 594
Script:
#!/bin/bash # Description: Capture the surrounding context of the error usage in the identified files. # Get the context around the usage of ErrGetPriceRoundNotFound in x/oracle/keeper/prices.go rg --type go --context 5 'ErrGetPriceRoundNotFound' x/oracle/keeper/prices.go # Get the context around the usage of ErrGetPriceRoundNotFound in x/operator/keeper/abci.go rg --type go --context 5 'ErrGetPriceRoundNotFound' x/operator/keeper/abci.go # Get the context around the definition of ErrGetPriceRoundNotFound in x/oracle/types/errors.go rg --type go --context 5 'ErrGetPriceRoundNotFound' x/oracle/types/errors.goLength of output: 2603
testutil/utils.go (4)
152-155: Verify: Correctness ofPricesListEnsure the
PricesListis correctly populated and consistent with the intended functionality.
144-150: Verify: Correctness of new token feederEnsure the new token feeder is correctly initialized and consistent with the intended functionality.
Verification successful
Verify: Context and usage of new token feeder
Check the context around the identified lines to confirm correct initialization and consistent usage of the new token feeder.
Verified: Correctness of new token feeder
The new token feeder with
TokenID: 2is correctly initialized and consistent with the intended functionality.
testutil/utils.go: The new token feeder is added tooracleDefaultParams.TokenFeedersand used inoracleGenesisinitialization.x/oracle/genesis_test.go: The new token feeder is part of the test case setup.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of the new token feeder addition in `oracleDefaultParams.TokenFeeders`. # Test: Search for the new token feeder usage. Expect: Correct initialization and consistency. rg --type go 'TokenID: 2'Length of output: 198
Script:
#!/bin/bash # Display lines around the identified matches to understand the context. rg --type go 'TokenID: 2' -A 5 -B 5Length of output: 1218
134-134: Verify: Correctness ofAssetIDEnsure the
AssetIDis correctly set and consistent with the intended functionality.Verification successful
Verified: Correctness of
AssetIDThe
AssetID"0xdac17f958d2ee523a2206206994597c13d831ec7_0x65" is correctly set and consistent with the intended functionality.
testutil/utils.go: Line 134 setsoracleDefaultParams.Tokens[1].AssetID.x/dogfood/types/params.go:DefaultAssetIDsis defined and used consistently.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of `AssetID` in `oracleDefaultParams.Tokens`. # Test: Search for the `AssetID` usage. Expect: Correct setting and consistency. rg --type go '0xdac17f958d2ee523a2206206994597c13d831ec7_0x65'Length of output: 266
Script:
#!/bin/bash # Description: Verify the usage and consistency of `DefaultAssetIDs`. # Search for the usage of `DefaultAssetIDs` to understand its context and ensure it aligns with the intended functionality. rg --type go 'DefaultAssetIDs'Length of output: 305
136-143: Verify: Correctness of new tokenEnsure the new token is correctly initialized and consistent with the intended functionality.
app/app.go (1)
545-545: Correct Initialization of OracleKeeperThe replacement of
operatorTypes.MockOracle{}with&app.OracleKeepercorrectly initializes the OracleKeeper with the actual implementation instead of a mock.
|
replace mock oracle with oracle keeper in operator keeper, and set default price to 1 when tokenFeeder has not reach consensus on 1st round. |
| for assetID := range assets { | ||
| tokenID := p.GetTokenIDFromAssetID(assetID) | ||
| if tokenID == 0 { | ||
| err = types.ErrGetPriceAssetNotFound.Wrapf("assetID does not exist in oracle %s", assetID) | ||
| prices = nil | ||
| break | ||
| } | ||
| price, found := k.GetPriceTRLatest(ctx, uint64(tokenID)) | ||
| if !found { | ||
| info = info + assetID + " " | ||
| prices[assetID] = types.Price{ | ||
| Value: sdkmath.NewInt(types.DefaultPriceValue), | ||
| Decimal: types.DefaultPriceDecimal, | ||
| } | ||
| } else { | ||
| v, _ := sdkmath.NewIntFromString(price.Price) | ||
| prices[assetID] = types.Price{ | ||
| Value: v, | ||
| Decimal: uint8(price.Decimal), | ||
| } | ||
| } | ||
| } |
Check warning
Code scanning / CodeQL
Iteration over map Warning
There was a problem hiding this comment.
Actionable comments posted: 3
Outside diff range, codebase verification and nitpick comments (3)
x/oracle/keeper/single.go (3)
188-188: Improve readability by adding comments.Consider adding comments to explain the purpose of
setCommonParams.+ // setCommonParams sets common parameters for the module. func setCommonParams(p *types.Params) {
108-109: Ensure proper error handling forSetParams.The
SetParamsmethod inx/oracle/keeper/aggregator/context.godoes not return an error, which means it cannot handle errors appropriately. You should update this function to return an error and handle it to avoid silent failures.
x/oracle/keeper/aggregator/context.go:func (agc *AggregatorContext) SetParams(p *types.Params)Analysis chain
Ensure proper error handling for
SetParams.The
SetParamsmethod should handle errors appropriately to avoid silent failures.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify error handling in SetParams function. # Locate the definition of SetParams and extract its content. rg --type go 'func SetParams' -A 10Length of output: 37
Script:
#!/bin/bash # Search for the term SetParams to locate its definition or usage. rg 'SetParams' # Display 20 lines around the target lines in single.go for more context. rg -A 10 -B 10 'SetParams' x/oracle/keeper/single.goLength of output: 8378
152-156: Ensure proper error handling forSetParams.The
SetParamsmethod inx/oracle/keeper/params.godoes not currently handle errors. It should be modified to include error handling for the marshalling process and validation checks to avoid silent failures.
- File:
x/oracle/keeper/params.go- Function:
func (k Keeper) SetParams(ctx sdk.Context, params types.Params) error { store := ctx.KVStore(k.storeKey) // Perform validation check if err := validateParams(params); err != nil { return err } bz, err := k.cdc.Marshal(¶ms) if err != nil { return err } store.Set(types.ParamsKey, bz) return nil }Ensure to update the function's usage throughout the codebase to handle the returned error.
Analysis chain
Ensure proper error handling for
SetParams.The
SetParamsmethod should handle errors appropriately to avoid silent failures.
To proceed with the verification, let's perform a more comprehensive search using
ast-grepto locate the definition ofSetParams.
Let's perform a comprehensive search using
rgto locate any occurrence ofSetParamsin the codebase.
To proceed, let's retrieve the full content of the
SetParamsfunction fromx/oracle/keeper/params.goto analyze how it can be modified to include error handling.
To ensure we capture the entire
SetParamsfunction, let's useast-grepto search for its complete definition inx/oracle/keeper/params.go.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify error handling in SetParams function. # Locate the definition of SetParams and extract its content. rg --type go 'func SetParams' -A 10Length of output: 37
Script:
#!/bin/bash # Description: Locate the definition of SetParams in Go codebase, including methods with receivers. # Search for function definitions named SetParams, including those with a receiver. ast-grep --lang go --pattern 'func ($_ SetParams) { $$$ }' ast-grep --lang go --pattern 'func SetParams($_) { $$$ }'Length of output: 120
Script:
#!/bin/bash # Description: Locate any occurrence of SetParams in the codebase. # Search for any occurrence of SetParams in the codebase. rg 'SetParams'Length of output: 6931
Script:
#!/bin/bash # Description: Retrieve the full content of the SetParams function from x/oracle/keeper/params.go. # Extract the content of the SetParams function from x/oracle/keeper/params.go. rg 'func (k Keeper) SetParams(ctx sdk.Context, params types.Params)' -A 20 x/oracle/keeper/params.goLength of output: 102
Script:
#!/bin/bash # Description: Retrieve the full content of the SetParams function from x/oracle/keeper/params.go using ast-grep. # Extract the content of the SetParams function from x/oracle/keeper/params.go. ast-grep --lang go --pattern 'func (k Keeper) SetParams(ctx sdk.Context, params types.Params) { $$$ }' x/oracle/keeper/params.goLength of output: 499
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (6)
- app/app.go (1 hunks)
- testutil/utils.go (1 hunks)
- x/oracle/keeper/aggregator/context.go (4 hunks)
- x/oracle/keeper/aggregator/context_test.go (1 hunks)
- x/oracle/keeper/prices.go (3 hunks)
- x/oracle/keeper/single.go (4 hunks)
Files skipped from review as they are similar to previous changes (2)
- app/app.go
- x/oracle/keeper/aggregator/context.go
Additional context used
GitHub Check: CodeQL
x/oracle/keeper/single.go
[warning] 102-106: Iteration over map
Iteration over map may be a possible source of non-determinismx/oracle/keeper/prices.go
[warning] 86-107: Iteration over map
Iteration over map may be a possible source of non-determinism
Additional comments not posted (7)
x/oracle/keeper/aggregator/context_test.go (1)
88-88: UpdateSetParamsto usetypes.Paramsdirectly.The change removes the conversion of
ptopWrappedbefore setting parameters inAggregatorContext, directly usingtypes.Params. This is consistent with the refactor to usetypes.Paramsinstead ofcommon.Params.x/oracle/keeper/single.go (2)
140-142: Ensure cache consistency.Verify that the cache is consistently updated to avoid stale data.
Verification successful
Cache Consistency Verified
The cache update and retrieval operations are used consistently to ensure that stale data is avoided.
x/oracle/module.goshows consistent use ofcs.AddCacheandcs.GetCache.x/oracle/keeper/single.gomanages parameters and validator powers withc.AddCacheandc.GetCache.- Test cases in
x/oracle/keeper/cache/caches_test.goreflect proper cache management.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify cache consistency. # Test: Search for cache update operations. Expect: Consistent cache updates. rg --type go 'AddCache|GetCache'Length of output: 2672
Script:
#!/bin/bash # Description: Gather context around cache update and retrieval operations to ensure proper cache management. # Search for AddCache operations and display surrounding lines for context rg --type go 'AddCache' -A 3 -B 3 # Search for GetCache operations and display surrounding lines for context rg --type go 'GetCache' -A 3 -B 3Length of output: 15918
97-98: Variablepshould be initialized properly.Ensure the variable
pis initialized correctly to avoid nil pointer dereference.x/oracle/keeper/prices.go (1)
47-72: Fetching specified asset price looks good.The
GetSpecifiedAssetsPricefunction correctly fetches the price for a specifiedassetID, handling errors appropriately.testutil/utils.go (3)
134-134: SetAssetIDfor the token.Ensure that the
AssetIDis correctly set for the token.
136-150: Add new tokens and token feeders.The addition of new tokens and token feeders is correctly implemented.
152-155: PopulatePricesList.The
PricesListis correctly populated with price entries.
| for b = range recentParamsMap { | ||
| if b > prev { | ||
| // pTmp be set at least once, since len(recentParamsMap)>0 | ||
| pTmp = common.Params(*p) | ||
| prev = b | ||
| } | ||
| } | ||
| agc.SetParams(&pTmp) | ||
| setCommonParams(types.Params(pTmp)) | ||
| p = recentParamsMap[prev] |
There was a problem hiding this comment.
Optimize map iteration for determinism.
Iteration over maps in Go is non-deterministic. Consider sorting the keys before iteration to ensure deterministic behavior.
- for b = range recentParamsMap {
+ keys := make([]int64, 0, len(recentParamsMap))
+ for k := range recentParamsMap {
+ keys = append(keys, k)
+ }
+ sort.Slice(keys, func(i, j int) bool { return keys[i] < keys[j] })
+ for _, b := range keys {
prev = bCommittable 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.
| for b = range recentParamsMap { | |
| if b > prev { | |
| // pTmp be set at least once, since len(recentParamsMap)>0 | |
| pTmp = common.Params(*p) | |
| prev = b | |
| } | |
| } | |
| agc.SetParams(&pTmp) | |
| setCommonParams(types.Params(pTmp)) | |
| p = recentParamsMap[prev] | |
| keys := make([]int64, 0, len(recentParamsMap)) | |
| for k := range recentParamsMap { | |
| keys = append(keys, k) | |
| } | |
| sort.Slice(keys, func(i, j int) bool { return keys[i] < keys[j] }) | |
| for _, b := range keys { | |
| if b > prev { | |
| prev = b | |
| } | |
| } | |
| p = recentParamsMap[prev] |
Tools
GitHub Check: CodeQL
[warning] 102-106: Iteration over map
Iteration over map may be a possible source of non-determinism
| for b, p = range recentParamsMap { | ||
| if b <= from && b > prev { | ||
| pTmp = common.Params(*recentParams) | ||
| agc.SetParams(&pTmp) | ||
| agc.SetParams(p) | ||
| prev = b | ||
| setCommonParams(*recentParams) | ||
| setCommonParams(p) |
There was a problem hiding this comment.
Optimize map iteration for determinism.
Iteration over maps in Go is non-deterministic. Consider sorting the keys before iteration to ensure deterministic behavior.
- for b, p = range recentParamsMap {
+ keys := make([]int64, 0, len(recentParamsMap))
+ for k := range recentParamsMap {
+ keys = append(keys, k)
+ }
+ sort.Slice(keys, func(i, j int) bool { return keys[i] < keys[j] })
+ for _, b := range keys {
p = recentParamsMap[b]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.
| for b, p = range recentParamsMap { | |
| if b <= from && b > prev { | |
| pTmp = common.Params(*recentParams) | |
| agc.SetParams(&pTmp) | |
| agc.SetParams(p) | |
| prev = b | |
| setCommonParams(*recentParams) | |
| setCommonParams(p) | |
| keys := make([]int64, 0, len(recentParamsMap)) | |
| for k := range recentParamsMap { | |
| keys = append(keys, k) | |
| } | |
| sort.Slice(keys, func(i, j int) bool { return keys[i] < keys[j] }) | |
| for _, b := range keys { | |
| p = recentParamsMap[b] | |
| if b <= from && b > prev { | |
| agc.SetParams(p) | |
| prev = b | |
| setCommonParams(p) |
Tools
GitHub Check: CodeQL
[warning] 114-120: Iteration over map
Iteration over map may be a possible source of non-determinism
| // return latest price for assets | ||
| func (k Keeper) GetMultipleAssetsPrices(ctx sdk.Context, assets map[string]interface{}) (prices map[string]types.Price, err 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) | ||
| prices = make(map[string]types.Price) | ||
| info := "" | ||
| for assetID := range assets { | ||
| tokenID := p.GetTokenIDFromAssetID(assetID) | ||
| if tokenID == 0 { | ||
| err = types.ErrGetPriceAssetNotFound.Wrapf("assetID does not exist in oracle %s", assetID) | ||
| prices = nil | ||
| break | ||
| } | ||
| price, found := k.GetPriceTRLatest(ctx, uint64(tokenID)) | ||
| if !found { | ||
| info = info + assetID + " " | ||
| prices[assetID] = types.Price{ | ||
| Value: sdkmath.NewInt(types.DefaultPriceValue), | ||
| Decimal: types.DefaultPriceDecimal, | ||
| } | ||
| } else { | ||
| v, _ := sdkmath.NewIntFromString(price.Price) | ||
| prices[assetID] = types.Price{ | ||
| Value: v, | ||
| Decimal: uint8(price.Decimal), | ||
| } | ||
| } | ||
| } | ||
| if len(info) > 0 { | ||
| err = types.ErrGetPriceRoundNotFound.Wrapf("no valid price for assetIDs=%s", info) | ||
| } | ||
| return prices, err | ||
| } |
There was a problem hiding this comment.
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 {
tokenID := p.GetTokenIDFromAssetID(assetID)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.
| // return latest price for assets | |
| func (k Keeper) GetMultipleAssetsPrices(ctx sdk.Context, assets map[string]interface{}) (prices map[string]types.Price, err 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) | |
| prices = make(map[string]types.Price) | |
| info := "" | |
| for assetID := range assets { | |
| tokenID := p.GetTokenIDFromAssetID(assetID) | |
| if tokenID == 0 { | |
| err = types.ErrGetPriceAssetNotFound.Wrapf("assetID does not exist in oracle %s", assetID) | |
| prices = nil | |
| break | |
| } | |
| price, found := k.GetPriceTRLatest(ctx, uint64(tokenID)) | |
| if !found { | |
| info = info + assetID + " " | |
| prices[assetID] = types.Price{ | |
| Value: sdkmath.NewInt(types.DefaultPriceValue), | |
| Decimal: types.DefaultPriceDecimal, | |
| } | |
| } else { | |
| v, _ := sdkmath.NewIntFromString(price.Price) | |
| prices[assetID] = types.Price{ | |
| Value: v, | |
| Decimal: uint8(price.Decimal), | |
| } | |
| } | |
| } | |
| if len(info) > 0 { | |
| err = types.ErrGetPriceRoundNotFound.Wrapf("no valid price for assetIDs=%s", info) | |
| } | |
| return prices, err | |
| } | |
| // return latest price for assets | |
| func (k Keeper) GetMultipleAssetsPrices(ctx sdk.Context, assets map[string]interface{}) (prices map[string]types.Price, err 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) | |
| prices = make(map[string]types.Price) | |
| info := "" | |
| 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 { | |
| err = types.ErrGetPriceAssetNotFound.Wrapf("assetID does not exist in oracle %s", assetID) | |
| prices = nil | |
| break | |
| } | |
| price, found := k.GetPriceTRLatest(ctx, uint64(tokenID)) | |
| if !found { | |
| info = info + assetID + " " | |
| prices[assetID] = types.Price{ | |
| Value: sdkmath.NewInt(types.DefaultPriceValue), | |
| Decimal: types.DefaultPriceDecimal, | |
| } | |
| } else { | |
| v, _ := sdkmath.NewIntFromString(price.Price) | |
| prices[assetID] = types.Price{ | |
| Value: v, | |
| Decimal: uint8(price.Decimal), | |
| } | |
| } | |
| } | |
| if len(info) > 0 { | |
| err = types.ErrGetPriceRoundNotFound.Wrapf("no valid price for assetIDs=%s", info) | |
| } | |
| return prices, err | |
| } |
Tools
GitHub Check: CodeQL
[warning] 86-107: Iteration over map
Iteration over map may be a possible source of non-determinism
Description
Add method to get prices from AssetIDs. And this PR includes some code refactor which remove the unnecessary alias struct
common.Paramsfortypes.ParamsChanges
Notable changes
common.Params, and move the corresponding method toParamsCloses #XXX
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
MockOraclewithOracleKeeperfor better Oracle functionality initialization.Tests