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

Cleanup removed oracle feeds #1204

Merged
merged 6 commits into from
Jan 2, 2024
Merged

Cleanup removed oracle feeds #1204

merged 6 commits into from
Jan 2, 2024

Conversation

udpatil
Copy link
Collaborator

@udpatil udpatil commented Jan 2, 2024

Describe your changes and provide context

This ensure that oracle feeds are removed at the end of the oracle slashing window IF the asset is no longer whitelisted. Additionally, it prevents the TWAPs endpoint from serving twaps for denoms that are no longer in the whitelist.

Testing performed to validate your change

Unit tests

return false
})
// compare
activesToClear := []string{}
Copy link
Contributor

Choose a reason for hiding this comment

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

fwiw, idiomatic is defaulting to the nil: var activesToClear []string

@@ -681,3 +693,123 @@ func TestCalculateTwaps(t *testing.T) {
require.Error(t, err)
require.Equal(t, types.ErrInvalidTwapLookback, err)
}

func TestCalculateTwapsWithUnsupportedDenom(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️ yay test

@@ -162,5 +162,7 @@
// reset miss counters of all validators at the last block of slash window
if utils.IsPeriodLastBlock(ctx, params.SlashWindow) {
k.SlashAndResetCounters(ctx)
// Compare vote targets and actives and remove excess feeds
k.RemoveExcessFeeds(ctx)

Check warning

Code scanning / CodeQL

Panic in BeginBock or EndBlock consensus methods Warning

path flow from Begin/EndBlock to a panic call
path flow from Begin/EndBlock to a panic call
path flow from Begin/EndBlock to a panic call
Copy link

codecov bot commented Jan 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (62b3b9e) 63.40% compared to head (3804d0b) 63.35%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1204      +/-   ##
==========================================
- Coverage   63.40%   63.35%   -0.05%     
==========================================
  Files         257      257              
  Lines       16694    16726      +32     
==========================================
+ Hits        10585    10597      +12     
- Misses       5601     5626      +25     
+ Partials      508      503       -5     
Files Coverage Δ
x/oracle/abci.go 100.00% <100.00%> (ø)
x/oracle/keeper/keeper.go 95.01% <100.00%> (+0.42%) ⬆️

... and 4 files with indirect coverage changes

Comment on lines +137 to +140
for denom := range excessActives {
activesToClear[i] = denom
i++
}

Check warning

Code scanning / CodeQL

Iteration over map Warning

Iteration over map may be a possible source of non-determinism
@udpatil udpatil merged commit 3851b60 into main Jan 2, 2024
37 checks passed
@udpatil udpatil deleted the cleanup-removed-oracle-feeds branch January 2, 2024 17:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants