test: add tests staking #1439
test: add tests staking #1439
Conversation
WalkthroughThe recent updates encompass enhancements to testing, configuration, and runtime transaction handling in the Cosmos codebase. New test cases and blocks have been added, constants for timeouts have been introduced, error messaging has been improved, and telemetry has been integrated to track events and metrics. The transaction pool logic has been refactored for better performance and monitoring, while the Ethereum compatibility layer has been adjusted to improve client interactions and error responses. Changes
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- cosmos/precompile/staking/staking_test.go (1 hunks)
Additional comments: 4
cosmos/precompile/staking/staking_test.go (4)
- 610-638: The tests added for
GetBondedValidatorsByPowercover scenarios with no active validators, one active validator, and all validators active. This is a good range of test cases to ensure the method works under different conditions.- 640-658: The tests for
GetValidatorsinclude scenarios without pagination and with pagination. This ensures that the pagination logic is correctly handled.- 661-670: The tests for
GetValidatorcheck both a valid and an invalid validator. It's important to ensure that the method behaves correctly when provided with an invalid address.- 673-683: The tests for
GetDelegatorValidatorsinclude a success case and a case with an invalid delegator. This ensures that the method can handle error scenarios gracefully.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- cosmos/precompile/staking/staking_test.go (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- cosmos/precompile/staking/staking_test.go
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- cosmos/precompile/staking/staking_test.go (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- cosmos/precompile/staking/staking_test.go
|
can you review this PR @itsdevbear |
|
Lgtm just fix linter, the e2e tests failing is on us, we will fix :) |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- cosmos/config/default.go (2 hunks)
Additional comments: 2
cosmos/config/default.go (2)
- 34-36: Introduced constants
TimeoutProposeSecondsandTimeoutDefault. Ensure that the naming convention is consistent with the project's standards and that the values are appropriate for the timeouts they represent.- 53-55: Replaced hard-coded timeout values with the new constants in the
RecommendedCometBFTConfigfunction. Verify that these changes do not affect other parts of the codebase where the previous hard-coded values were expected.Verification successful
The verification process has confirmed that the changes made to the timeout values in the
RecommendedCometBFTConfigfunction using the new constantsTimeoutProposeSecondsandTimeoutDefaultare localized and do not affect other parts of the codebase. The constants are not used outside of thecosmos/config/default.gofile, and other instances oftime.Secondin the codebase are unrelated to the consensus timeouts and serve different purposes.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for the old hard-coded timeout values to ensure they are not used elsewhere. rg --type go '3 \* time.Second' --context 5 rg --type go '1 \* time.Second' --context 5Length of output: 91
Script:
#!/bin/bash # Search for any occurrences of 'time.Second' to catch any other hard-coded timeout values. rg --type go 'time.Second' --context 5 # Check for the usage of the new constants `TimeoutProposeSeconds` and `TimeoutDefault`. rg --type go 'TimeoutProposeSeconds' --context 5 rg --type go 'TimeoutDefault' --context 5Length of output: 6964
thank you, fixed |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #1439 +/- ##
==========================================
+ Coverage 46.33% 47.38% +1.05%
==========================================
Files 89 89
Lines 5169 5149 -20
==========================================
+ Hits 2395 2440 +45
+ Misses 2598 2524 -74
- Partials 176 185 +9
|
|
@itsdevbear |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (18)
build/tools/go.modis excluded by:!**/*.modbuild/tools/go.sumis excluded by:!**/*.sumcontracts/go.modis excluded by:!**/*.modcontracts/go.sumis excluded by:!**/*.sumcosmos/go.modis excluded by:!**/*.modcosmos/go.sumis excluded by:!**/*.sume2e/localnet/go.modis excluded by:!**/*.mode2e/localnet/go.sumis excluded by:!**/*.sume2e/precompile/go.modis excluded by:!**/*.mode2e/precompile/go.sumis excluded by:!**/*.sume2e/testapp/go.modis excluded by:!**/*.mode2e/testapp/go.sumis excluded by:!**/*.sumeth/go.modis excluded by:!**/*.modeth/go.sumis excluded by:!**/*.sumgo.workis excluded by:!**/*.workgo.work.sumis excluded by:!**/*.sumlib/go.modis excluded by:!**/*.modlib/go.sumis excluded by:!**/*.sum
Files selected for processing (7)
- cosmos/config/default.go (2 hunks)
- cosmos/runtime/txpool/ante.go (4 hunks)
- cosmos/runtime/txpool/handler.go (6 hunks)
- cosmos/runtime/txpool/mempool.go (2 hunks)
- cosmos/runtime/txpool/mocks/tx_sub_provider.go (1 hunks)
- cosmos/runtime/txpool/telemetry.go (1 hunks)
- eth/polar/api_backend.go (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- cosmos/config/default.go
Additional comments: 11
cosmos/runtime/txpool/telemetry.go (1)
- 25-45: The introduction of new metric keys for telemetry is consistent with the summary. The prefix change to "polaris_cometbft_" aligns with the CometBFT consensus algorithm branding. This change is straightforward and does not appear to introduce any issues.
cosmos/runtime/txpool/mocks/tx_sub_provider.go (1)
- 57-59: The
Statsmethod has been added to theTxSubProvidermock as described in the summary. It returns two integers, presumably representing some statistics about transactions. Since this is a mock, the implementation is expected to be simplistic. However, it's important to ensure that the actual implementation ofStatsin the realTxSubProviderinterface is consistent with this mock.cosmos/runtime/txpool/ante.go (1)
- 70-114: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [42-113]
Telemetry increments have been added to the
AnteHandleandshouldEjectFromCometMempoolfunctions to track events such as ejections from the mempool. The logic withinshouldEjectFromCometMempoolhas been refactored to separate stateless and stateful checks, which is a good practice for clarity and maintainability. The added telemetry increments for expired transactions and transactions with gas price below the limit are consistent with the summary and seem to be correctly implemented.cosmos/runtime/txpool/mempool.go (2)
- 36-36: The import statement for
telemetryfrom"github.com/cosmos/cosmos-sdk/telemetry"has been added, which is necessary for the new telemetry features being implemented.- 134-134: A new telemetry counter is incremented under certain conditions within the
Insertfunction. This change is consistent with the summary and the overall goal of enhancing telemetry within the transaction pool.cosmos/runtime/txpool/handler.go (3)
- 45-45: The constants
statPeriodandretryDelayhave been added. These constants are used for scheduling statistics reporting and managing retry delays, respectively. The introduction of these constants is a good practice for code maintainability and readability.- 56-56: The
Statsmethod has been added to theTxSubProviderinterface, which is consistent with the changes in the mock file and the summary.- 181-201: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [126-197]
Three new methods,
statLoop,failedLoop, andStats, have been added to thehandlertype, and telemetry tracking has been incorporated into themainLoopandfailedLoopmethods. These changes are consistent with the summary and are implemented correctly to enhance the tracking and handling of transactions within the runtime transaction pool.eth/polar/api_backend.go (3)
- 208-213: The
HeaderByNumbermethod has been modified to return the latest state if the pending block is not available, which improves client compatibility. This change is consistent with the summary and is a sensible fallback.- 262-269: Similarly, the
BlockByNumbermethod has been updated to handle the case when the pending block is not available by returning the latest state. This change aligns with the summary and the modification in theHeaderByNumbermethod.- 208-213: Both
HeaderByNumberandBlockByNumbermethods have been modified to improve client compatibility by returning the latest state if the pending block is not available. This is a good approach to ensure that the system remains functional even when the pending block is not present.Also applies to: 262-269
hoank101
force-pushed
the
hoa/test
branch
3 times, most recently
from
67f21ee
to
4945f7b
Compare
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- cosmos/config/config.go (2 hunks)
Additional comments: 2
cosmos/config/config.go (2)
- 24-24: The addition of the "fmt" package import is appropriate for the use of
fmt.Errorfin the error handling.- 110-110: The use of
fmt.Errorfto format the error message with the invalid git commit length is correct and improves error reporting.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- cosmos/precompile/staking/staking_test.go (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- cosmos/precompile/staking/staking_test.go
|
hi @itsdevbear can you merge this PR? |
add tests staking and improve coverage
Summary by CodeRabbit
New Features
Improvements
Refactor
Documentation
Bug Fixes
Style
Tests
Chores
Revert