-
Notifications
You must be signed in to change notification settings - Fork 14
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: allow ignoring denoms #73
Conversation
WalkthroughThis update introduces the ability to ignore specific denominations ( Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant ConfigParser as Config Parser
participant DenomInfo as DenomInfo Struct
participant MetricsExporter as Metrics Exporter
User->>ConfigParser: Provide config with denoms
ConfigParser->>DenomInfo: Parse denoms with `ignore` field
DenomInfo->>MetricsExporter: Skip denoms with `ignore=true`
MetricsExporter-->>User: Export metrics excluding ignored denoms
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 (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #73 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 63 63
Lines 2567 2585 +18
=========================================
+ Hits 2567 2585 +18 ☔ View full report in Codecov by Sentry. |
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (16)
- config.example.toml (1 hunks)
- pkg/config/denom_info.go (2 hunks)
- pkg/generators/active_set_tokens.go (1 hunks)
- pkg/generators/active_set_tokens_test.go (2 hunks)
- pkg/generators/balance.go (2 hunks)
- pkg/generators/balance_test.go (4 hunks)
- pkg/generators/commission.go (1 hunks)
- pkg/generators/commission_test.go (4 hunks)
- pkg/generators/rewards.go (1 hunks)
- pkg/generators/rewards_test.go (4 hunks)
- pkg/generators/self_delegation.go (1 hunks)
- pkg/generators/self_delegations_test.go (4 hunks)
- pkg/generators/single_validator_info.go (1 hunks)
- pkg/generators/single_validator_info_test.go (2 hunks)
- pkg/generators/validators_info.go (1 hunks)
- pkg/generators/validators_info_test.go (2 hunks)
Files skipped from review due to trivial changes (1)
- pkg/generators/self_delegation.go
Additional context used
Learnings (2)
pkg/generators/active_set_tokens.go (1)
Learnt from: freak12techno PR: QuokkaStake/cosmos-validators-exporter#62 File: pkg/generators/active_set_tokens.go:61-61 Timestamp: 2024-06-22T22:57:51.216Z Learning: The indirect testing method used in `ActiveSetTokensGenerator`, which verifies the outcome by checking the metric's value against the expected stake of the last active validator, is considered sufficient by the user for reflecting the intended behavior of the sorting logic.
pkg/generators/active_set_tokens_test.go (1)
Learnt from: freak12techno PR: QuokkaStake/cosmos-validators-exporter#62 File: pkg/generators/active_set_tokens.go:61-61 Timestamp: 2024-06-22T22:57:51.216Z Learning: The indirect testing method used in `ActiveSetTokensGenerator`, which verifies the outcome by checking the metric's value against the expected stake of the last active validator, is considered sufficient by the user for reflecting the intended behavior of the sorting logic.
Additional comments not posted (24)
pkg/config/denom_info.go (3)
16-16
: EnsureIgnore
field default value is correctly set.The
Ignore
field is added to theDenomInfo
struct with a default value offalse
. Verify that thenull.Bool
type is correctly initialized with this default value.
24-24
: Update validation condition to handleIgnore
field correctly.The condition in the
Validate
method now checks ifDisplayDenom
is empty only ifIgnore
is not true. This logic seems correct.
61-63
: Correctly handle ignored denominations inConvert
method.The
Convert
method now skips processing ifIgnore
is true. Ensure that this logic is consistent with the intended behavior of ignoring certain denominations.pkg/generators/rewards.go (1)
50-52
: Correctly handlenil
values foramountConverted
.The check for
amountConverted
beingnil
ensures that the code skips processing when the conversion result isnil
. This logic is correct and prevents potential runtime errors.pkg/generators/commission.go (1)
50-52
: Correctly handlenil
values foramountConverted
.The check for
amountConverted
beingnil
ensures that the code skips processing when the conversion result isnil
. This logic is correct and prevents potential runtime errors.pkg/generators/rewards_test.go (3)
11-12
: Importnull
package to handlenull.Bool
type.The import of
github.com/guregu/null/v5
is necessary for handling thenull.Bool
type used in theIgnore
field.
50-50
: Test case for ignored denomination.The new test case correctly sets up a scenario where a denomination is ignored and verifies that the metrics are not generated for the ignored denomination.
64-64
: Verify the count of collected metrics.The assertion for the count of collected metrics ensures that the ignored denomination is not included in the results. This is a good practice to verify the correct behavior of the ignoring logic.
pkg/generators/commission_test.go (2)
11-12
: Import is appropriate.The
github.com/guregu/null/v5
package is necessary for handling thenull.Bool
type.
38-38
: Test updates are appropriate.The test function now includes a scenario where a denomination is ignored, ensuring the new functionality is covered.
Also applies to: 50-50, 64-64
pkg/generators/self_delegations_test.go (2)
11-12
: Import is appropriate.The
github.com/guregu/null/v5
package is necessary for handling thenull.Bool
type.
37-37
: Test updates are appropriate.The test function now includes a scenario where a denomination is ignored, ensuring the new functionality is covered.
Also applies to: 49-49, 53-53, 67-67
pkg/generators/balance.go (1)
51-53
: Conditional checks are appropriate.The
Generate
method now includes checks to skip processing ifamountConverted
isnil
, ensuring robustness.Also applies to: 77-79
pkg/generators/active_set_tokens.go (1)
70-72
: Conditional check is appropriate.The
Generate
method now includes a check to skip processing iflastValidatorAmount
isnil
, ensuring robustness.pkg/generators/validators_info.go (1)
79-81
: LGTM! Ensure proper verification of the new logic.The new conditional check ensures that processing is skipped if
totalBondedAmountConverted
isnil
. This change is approved.However, verify that the
totalBondedAmountConverted
can indeed benil
and that this condition is necessary.pkg/generators/balance_test.go (3)
11-12
: Import necessary for handling null values.The import of
github.com/guregu/null/v5
is necessary for handling thenull.Bool
type for theIgnore
field.
38-38
: Ensure ignored denominations are not counted in metrics.The test state includes an ignored denomination to verify that it is not counted in the metrics. This change is approved.
However, ensure that the logic for ignoring denominations is consistently applied across all relevant tests.
Also applies to: 45-45
81-81
: LGTM! Ensure proper verification of the new logic.The new assertion ensures that the ignored denomination is not counted in the collected metrics. This change is approved.
However, verify that the logic for ignoring denominations is correctly implemented in the generator.
pkg/generators/single_validator_info.go (1)
151-153
: LGTM! Ensure proper verification of the new logic.The new conditional check ensures that processing is skipped if
delegationsAmount
isnil
. This change is approved.However, verify that the
delegationsAmount
can indeed benil
and that this condition is necessary.pkg/generators/validators_info_test.go (2)
11-12
: Import necessary for handling null values.The import of
github.com/guregu/null/v5
is necessary for handling thenull.Bool
type for theIgnore
field.
90-136
: LGTM! Ensure proper verification of the new logic.The new test ensures that ignored base denominations are not counted in the collected metrics. This change is approved.
However, verify that the logic for ignoring base denominations is correctly implemented in the generator.
pkg/generators/active_set_tokens_test.go (1)
153-187
: LGTM!The test correctly verifies the behavior when a denomination is ignored.
config.example.toml (1)
39-53
: LGTM!The addition of the
ignore
field is well-documented and the default value is appropriately set tofalse
.pkg/generators/single_validator_info_test.go (1)
168-255
: LGTM!The test correctly verifies the behavior when a base denomination is ignored.
Summary by CodeRabbit
New Features
Bug Fixes
nil
, ensuring more robust and error-free metric generation.Tests
Chores