-
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: add consumer commission #69
Conversation
WalkthroughThe update introduces functionalities to handle consumer commission data for validators. It adds new fetchers and generators, enhances configuration constants, and modifies Tendermint RPC interactions. Additionally, tests for both fetchers and generators ensure the new processes work correctly. A new JSON file sets a default commission rate. The update refines existing functionalities by removing unnecessary metrics and adjusts suitable test expectations accordingly. Changes
Sequence Diagram(s)Below is the sequence diagram representing the interaction between fetching and generating consumer commission rates: sequenceDiagram
participant App as App
participant Fetcher as ConsumerCommissionFetcher
participant Generator as ValidatorCommissionRateGenerator
participant RPC as Tendermint RPC
participant State as State
App->>Fetcher: NewConsumerCommissionFetcher(logger, appConfig.Chains, rpcs, tracer)
Fetcher->>RPC: fetchConsumerCommission(valcons, chainID)
RPC-->>Fetcher: ConsumerCommissionResponse
App->>Generator: NewValidatorCommissionRateGenerator(appConfig.Chains, logger)
Generator->>State: Generate(state)
State-->>Generator: Commission rates data
Generator-->>App: Prometheus metrics
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 ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #69 +/- ##
==========================================
+ Coverage 90.71% 91.01% +0.30%
==========================================
Files 61 63 +2
Lines 2455 2560 +105
==========================================
+ Hits 2227 2330 +103
- Misses 227 229 +2
Partials 1 1 ☔ 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: 5
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (11)
- assets/consumer-commission.json (1 hunks)
- pkg/app.go (2 hunks)
- pkg/constants/constants.go (1 hunks)
- pkg/fetchers/consumer_commission.go (1 hunks)
- pkg/fetchers/consumer_commission_test.go (1 hunks)
- pkg/generators/single_validator_info.go (3 hunks)
- pkg/generators/single_validator_info_test.go (3 hunks)
- pkg/generators/validator_commission_rate.go (1 hunks)
- pkg/generators/validator_commission_rate_test.go (1 hunks)
- pkg/tendermint/rpc.go (1 hunks)
- pkg/types/tendermint.go (1 hunks)
Files not reviewed due to errors (1)
- pkg/generators/validator_commission_rate_test.go (no review received)
Files skipped from review due to trivial changes (4)
- assets/consumer-commission.json
- pkg/constants/constants.go
- pkg/generators/single_validator_info.go
- pkg/generators/single_validator_info_test.go
Additional context used
Learnings (3)
pkg/fetchers/consumer_commission.go (2)
User: freak12techno PR: QuokkaStake/cosmos-validators-exporter#63 File: pkg/fetchers/consumer_info.go:34-40 Timestamp: 2024-06-23T11:24:17.378Z Learning: User freak12techno plans to add unit tests for the `NewConsumerInfoFetcher` constructor later.
User: freak12techno PR: QuokkaStake/cosmos-validators-exporter#63 File: pkg/fetchers/consumer_validators.go:40-40 Timestamp: 2024-06-23T16:11:23.143Z Learning: The `TestConsumerValidatorsFetcherBase` method in `pkg/fetchers/consumer_validators_test.go` tests the initialization of `ConsumerValidatorsFetcher` with the `Chains` parameter.
pkg/app.go (1)
User: freak12techno PR: QuokkaStake/cosmos-validators-exporter#57 File: pkg/app.go:86-86 Timestamp: 2024-06-16T10:28:15.556Z Learning: The `fetchers` array in `pkg/app.go` is iteratively processed, and each fetcher's `Fetch` method is invoked, confirming that all fetchers, including `ConsumerValidatorsFetcher`, are actively used to fetch data.
pkg/fetchers/consumer_commission_test.go (2)
User: freak12techno PR: QuokkaStake/cosmos-validators-exporter#63 File: pkg/fetchers/consumer_validators.go:40-40 Timestamp: 2024-06-23T16:11:23.143Z Learning: The `TestConsumerValidatorsFetcherBase` method in `pkg/fetchers/consumer_validators_test.go` tests the initialization of `ConsumerValidatorsFetcher` with the `Chains` parameter.
User: freak12techno PR: QuokkaStake/cosmos-validators-exporter#63 File: pkg/fetchers/consumer_info.go:34-40 Timestamp: 2024-06-23T11:24:17.378Z Learning: User freak12techno plans to add unit tests for the `NewConsumerInfoFetcher` constructor later.
GitHub Check: codecov/patch
pkg/app.go
[warning] 99-99: pkg/app.go#L99
Added line #L99 was not covered by tests
[warning] 124-124: pkg/app.go#L124
Added line #L124 was not covered by tests
Additional comments not posted (13)
pkg/fetchers/consumer_commission.go (3)
15-26
: Struct Definition: ConsumerCommissionFetcherThis struct is well-defined with appropriate fields for handling consumer commission data. However, consider documenting the purpose of each field for better maintainability.
32-44
: Constructor Function: NewConsumerCommissionFetcherThe constructor function is correctly implemented. It properly initializes the
ConsumerCommissionFetcher
struct with the provided arguments. Adding a comment describing the function could improve code readability.
78-80
: Method: NameThis simple getter method is correctly implemented and returns the appropriate constant for the fetcher name. No issues here.
pkg/generators/validator_commission_rate.go (2)
16-19
: Struct Definition: ValidatorCommissionRateGeneratorThe struct is minimal and appropriate for its intended use. However, adding documentation for the struct and its fields would improve code clarity and maintainability.
21-28
: Constructor Function: NewValidatorCommissionRateGeneratorThe constructor is straightforward and correctly initializes the struct. It uses fluent logging which is good for adding more context-specific information in logs. Consider adding a brief comment to describe the constructor's purpose.
pkg/types/tendermint.go (1)
198-201
: New Type: ConsumerCommissionResponseThe
ConsumerCommissionResponse
type is well-defined with appropriate JSON tags. It includes fields for code and rate, which are essential for handling commission data. No issues found here.pkg/fetchers/consumer_commission_test.go (6)
19-49
: Basic test setup and assertions look good.The test function
TestConsumerCommissionFetcherBase
properly initializes the fetcher and checks its integrity and correct naming, which aligns well with the expected functionality.
51-91
: Test logic and setup for missing consensus address are appropriate.The function
TestConsumerCommissionFetcherNoConsensusAddress
correctly simulates a scenario with no consensus address and validates that no data is returned, which is expected behavior.
93-135
: Test setup for disabled query scenario is correctly implemented.The function
TestConsumerCommissionFetcherQueryDisabled
accurately tests the behavior when the consumer commission query is disabled, ensuring no data is fetched or processed.
137-187
: Error handling in the test is robust and well-simulated.The function
TestConsumerCommissionFetcherQueryError
effectively useshttpmock
to simulate an API error and checks that the fetcher handles this scenario correctly by not processing any data.
189-239
: Node error simulation and handling are effectively tested.The function
TestConsumerCommissionFetcherNodeError
correctly simulates a node error using a predefined JSON response and validates that the fetcher handles this error scenario properly.
241-296
: Successful query handling is thoroughly tested.The function
TestConsumerCommissionFetcherQuerySuccess
effectively simulates a successful API response and ensures that the fetcher processes and stores the data correctly, adhering to the expected outcomes.pkg/tendermint/rpc.go (1)
593-622
: Implementation ofGetConsumerCommission
is consistent and well-structured.The method
GetConsumerCommission
is implemented consistently with other RPC methods. It properly handles the enabled state check, error handling, and response parsing.Verification successful
Integration of
GetConsumerCommission
is correctly handled.The method
GetConsumerCommission
is properly integrated within theprocessChain
method ofConsumerCommissionFetcher
. The results are handled correctly, with appropriate error logging and data storage.
pkg/fetchers/consumer_commission.go
:processChain
methodScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify integration of `GetConsumerCommission` with other components. # Test: Search for the method usage. Expect: Proper integration with other components. rg --type go $'GetConsumerCommission'Length of output: 238
Script:
#!/bin/bash # Description: Extract the context of `GetConsumerCommission` usage in `pkg/fetchers/consumer_commission.go`. # Display lines around the usage of GetConsumerCommission to understand its context. rg --type go --context 10 'GetConsumerCommission' pkg/fetchers/consumer_commission.goLength of output: 525
Script:
#!/bin/bash # Description: Extract the full implementation of the `processChain` method in `pkg/fetchers/consumer_commission.go`. # Display the full implementation of the processChain method to understand its context. ast-grep --lang go --pattern 'func (f *ConsumerCommissionFetcher) processChain($_, $_, $_, $_) { $$$ }' pkg/fetchers/consumer_commission.goLength of output: 1904
@@ -96,6 +96,7 @@ | |||
fetchersPkg.NewNodeInfoFetcher(logger, appConfig.Chains, rpcs, tracer), | |||
fetchersPkg.NewConsumerInfoFetcher(logger, appConfig.Chains, rpcs, tracer), | |||
fetchersPkg.NewValidatorConsumersFetcher(logger, appConfig.Chains, rpcs, tracer), | |||
fetchersPkg.NewConsumerCommissionFetcher(logger, appConfig.Chains, rpcs, tracer), |
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.
Addition of New Fetcher: ConsumerCommissionFetcher
The new fetcher for consumer commissions is correctly added to the list of fetchers. However, this line was not covered by tests according to the static analysis tool.
+ fetchersPkg.NewConsumerCommissionFetcher(logger, appConfig.Chains, rpcs, tracer),
Consider adding unit tests to ensure this fetcher is initialized correctly and functions as expected.
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.
fetchersPkg.NewConsumerCommissionFetcher(logger, appConfig.Chains, rpcs, tracer), | |
fetchersPkg.NewConsumerCommissionFetcher(logger, appConfig.Chains, rpcs, tracer), |
Tools
GitHub Check: codecov/patch
[warning] 99-99: pkg/app.go#L99
Added line #L99 was not covered by tests
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 will add tests for app.go later
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.
@freak12techno, understood! If you need any assistance when adding those tests later, feel free to reach out. Looking forward to seeing the updates!
Learnings used
User: freak12techno
PR: QuokkaStake/cosmos-validators-exporter#64
File: pkg/app.go:105-112
Timestamp: 2024-06-25T19:32:53.262Z
Learning: User freak12techno has clarified that while specific tests for new generator functions in `app.go` exist, there is an overall lack of tests for `app.go` itself, which is currently expected and accepted.
User: freak12techno
PR: QuokkaStake/cosmos-validators-exporter#63
File: pkg/fetchers/consumer_info.go:34-40
Timestamp: 2024-06-23T11:24:17.378Z
Learning: User freak12techno plans to add unit tests for the `NewConsumerInfoFetcher` constructor later.
User: freak12techno
PR: QuokkaStake/cosmos-validators-exporter#63
File: pkg/fetchers/validators.go:28-34
Timestamp: 2024-06-23T11:24:50.154Z
Learning: The constructor function `NewValidatorsFetcher` in `pkg/fetchers/validators.go` is now covered by tests in `validators_test.go`.
User: freak12techno
PR: QuokkaStake/cosmos-validators-exporter#63
File: pkg/fetchers/soft_opt_out_threshold.go:28-34
Timestamp: 2024-06-23T11:25:06.862Z
Learning: The `NewSoftOptOutThresholdFetcher` constructor function in `pkg/fetchers/soft_opt_out_threshold.go` is now covered by tests as verified by the presence of its usage in `pkg/fetchers/soft_opt_out_threshold_test.go`.
User: freak12techno
PR: QuokkaStake/cosmos-validators-exporter#57
File: pkg/app.go:86-86
Timestamp: 2024-06-16T10:28:15.556Z
Learning: The `fetchers` array in `pkg/app.go` is iteratively processed, and each fetcher's `Fetch` method is invoked, confirming that all fetchers, including `ConsumerValidatorsFetcher`, are actively used to fetch data.
User: freak12techno
PR: QuokkaStake/cosmos-validators-exporter#63
File: pkg/fetchers/consumer_validators.go:40-40
Timestamp: 2024-06-23T16:11:23.143Z
Learning: The `TestConsumerValidatorsFetcherBase` method in `pkg/fetchers/consumer_validators_test.go` tests the initialization of `ConsumerValidatorsFetcher` with the `Chains` parameter.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
commissionGauge
metric from validator info generator.Tests
Chores