ci: add rpcimportable test #2817
Conversation
gartnera
requested review from
a team,
brewmaster012,
kingpinXD,
lumtis,
ws4charlie,
skosito,
swift1337 and
fbac
as code owners
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 Walkthrough📝 WalkthroughWalkthroughThe pull request introduces significant changes to the CI workflow, adds new modules and test files, updates dependencies, and modifies existing code by removing outdated functions and tests. Key updates include the introduction of a new Changes
Assessment against linked issues
Suggested labels
Suggested reviewers
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 using PR comments)
Other keywords and placeholders
Documentation and Community
|
github-actions
bot
added
ci
gartnera
force-pushed
the
rpc-importable-test
branch
2 times, most recently
from
4a56ae5
to
c556bc4
Compare
gartnera
force-pushed
the
rpc-importable-test
branch
4 times, most recently
from
e96a13e
to
f95066d
Compare
gartnera
force-pushed
the
rpc-importable-test
branch
from
9e3c783
to
ebd9d3e
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #2817 +/- ##
===========================================
- Coverage 66.76% 66.39% -0.37%
===========================================
Files 387 389 +2
Lines 21745 21758 +13
===========================================
- Hits 14517 14447 -70
- Misses 6500 6584 +84
+ Partials 728 727 -1
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
Outside diff range and nitpick comments (13)
contrib/rpcimportable/go.mod (1)
12-13: Enhance documentation for local development replace directive.The commented-out replace directive for local development is a useful feature. To improve clarity and ease of use, consider adding a brief note explaining when and how to use this directive. For example:
// For local development/testing/debugging: // 1. Uncomment the line below // 2. Ensure the relative path is correct // 3. Run `go mod tidy` after uncommenting // replace github.com/zeta-chain/node => ../..This additional context will help developers quickly understand how to leverage this feature when needed.
pkg/sdkconfig/sdkconfig.go (1)
7-17: Constants and variables are well-defined and consistent.The use of a constant
AccountAddressPrefixas the base for deriving other prefixes ensures consistency and makes future modifications easier. The naming convention is clear and follows Go standards.Consider grouping related prefixes together for improved readability:
var ( AccountPubKeyPrefix = AccountAddressPrefix + "pub" ValidatorAddressPrefix = AccountAddressPrefix + "valoper" ValidatorPubKeyPrefix = AccountAddressPrefix + "valoperpub" ConsNodeAddressPrefix = AccountAddressPrefix + "valcons" ConsNodePubKeyPrefix = AccountAddressPrefix + "valconspub" )zetaclient/zetacore/client_crosschain.go (1)
14-19: Enhance function documentation for clarity and completeness.While the function signature is well-structured, the documentation could be more comprehensive. Consider expanding the comment to include details about the parameters and return values.
Suggested improvement:
// GetAllOutboundTrackerByChain retrieves all outbound trackers for a specified chain. // It accepts a context, the chainID of the target chain, and an order parameter to // determine the sorting of the results. // // Parameters: // - ctx: The context for the operation // - chainID: The ID of the chain to query // - order: The sorting order for the results (Ascending or Descending) // // Returns: // - A slice of OutboundTracker objects sorted by Nonce // - An error if the operation fails func (c *Client) GetAllOutboundTrackerByChain( ctx context.Context, chainID int64, order interfaces.Order, ) ([]types.OutboundTracker, error) {x/crosschain/types/message_update_tss_address_test.go (1)
15-15: Approve configuration setup modification.The change from
keeper.SetConfig(false)tosdkconfig.SetDefault(false)reflects the new approach to configuration management introduced by thesdkconfigpackage. This modification aligns with the overall goal of improving the project's structure.Consider adding a brief comment explaining the purpose of
sdkconfig.SetDefault(false)to enhance code readability. For example:// Set default SDK configuration for testing sdkconfig.SetDefault(false)Additionally, ensure that the
sdkconfig.SetDefaultfunction is well-documented in its package, explaining its parameters and effects on the SDK configuration.x/observer/types/message_vote_blame_test.go (1)
16-16: Approve configuration setup change and suggest documentation.The modification from
keeper.SetConfig(false)tosdkconfig.SetDefault(false)is consistent with the import statement change and likely improves the modularity of the configuration management.To enhance code clarity and maintainability, consider adding a brief comment explaining the purpose of
sdkconfig.SetDefault(false)and its impact on the test environment. For example:// Set default SDK configuration for testing environment sdkconfig.SetDefault(false)Additionally, ensure that the
SetDefaultfunction in thesdkconfigpackage is well-documented, explaining its parameters and effects on the system configuration.x/crosschain/types/message_whitelist_erc20_test.go (1)
15-15: Approve function call change with documentation suggestion.The replacement of
keeper.SetConfig(false)withsdkconfig.SetDefault(false)is consistent with the import changes and reflects the shift in configuration management. This modification enhances code modularity and aligns with the PR objectives.Consider updating the documentation for the
sdkconfig.SetDefaultfunction to clarify its purpose and usage, especially if it differs from the previouskeeper.SetConfigfunction. This will ensure that developers understand the implications of this configuration setting in the context of these tests.x/crosschain/types/message_update_erc20_custody_pause_status_test.go (1)
16-16: Approve function call change and suggest documentation.The replacement of
keeper.SetConfig(false)withsdkconfig.SetDefault(false)is consistent with the import changes and likely improves the modularity of the configuration management.Consider adding a comment explaining the purpose of
sdkconfig.SetDefault(false)in this context, especially if it's setting a global configuration that affects multiple tests. For example:// Set default SDK configuration to ensure consistent behavior across tests sdkconfig.SetDefault(false).github/workflows/ci.yml (3)
1-1: Consider a more descriptive workflow name.The workflow name has been changed from "PR Testing" to "ci", which is concise but less descriptive. A more informative name could better convey the workflow's purpose.
Consider renaming the workflow to "Continuous Integration" or "CI Pipeline" to maintain clarity while keeping it concise:
-name: ci +name: Continuous IntegrationThe concurrency group identifier has been appropriately updated to align with the new workflow name.
Also applies to: 12-12
96-115: Approve newrpcimportablejob with a minor suggestion.The new
rpcimportablejob effectively addresses the PR objective of ensuring RPC package importability. It sets up the correct Go version, fetches the appropriate node module version, and runs tests in thecontrib/rpcimportabledirectory.Consider removing the
GOPROXY: directenvironment variable unless it's specifically required:- name: go get node working-directory: contrib/rpcimportable run: go get github.com/zeta-chain/node@${{github.event.pull_request.head.sha || github.sha}} - env: - GOPROXY: directThis allows the use of Go module proxies, which can improve build performance and reliability.
116-126: Approve newci-okjob with a suggestion for improved clarity.The new
ci-okjob effectively serves as a final check to ensure all CI steps have passed, aligning with the PR objective of simplifying required status checks.To improve clarity, consider adding a success message when all jobs pass:
steps: - if: ${{ contains(needs.*.result, 'failure') || contains(needs.*.result, 'cancelled') }} run: | echo "One of the jobs failed or was cancelled" exit 1 + - if: ${{ !contains(needs.*.result, 'failure') && !contains(needs.*.result, 'cancelled') }} + run: echo "All CI jobs completed successfully"This addition provides explicit confirmation when all checks pass, enhancing the workflow's readability.
testutil/keeper/emissions.go (1)
36-36: Approve the configuration update with a minor suggestion.The change from
SetConfig(false)tosdkconfig.SetDefault(false)appears to be a positive step towards more modular configuration management. This update aligns with best practices by utilizing a dedicated package for SDK configuration.To enhance code clarity and maintainability, consider extracting the boolean parameter into a named constant or variable with a descriptive name. This would improve readability and make the purpose of the
falsevalue more explicit.Consider refactoring the line as follows:
const useTestConfig = false sdkconfig.SetDefault(useTestConfig)This change would make the intent of the configuration setting more apparent to future readers of the code.
x/observer/types/message_vote_block_header_test.go (1)
Line range hint
1-214: Summary of changes and recommendations.The modifications to this test file are minimal and focused on improving the import structure and configuration management. These changes align well with the project's objectives of simplifying imports and reducing dependencies. The core test logic remains unchanged, which is appropriate.
Recommendations:
- Ensure that all tests pass with the new
sdkconfig.SetDefaultfunction.- Consider adding a comment explaining the purpose of
sdkconfig.SetDefault(false)to improve code readability.- If not already done, update any related documentation to reflect these changes in configuration management.
x/authority/types/policies_test.go (1)
Further Action Required:
setConfigFunction is Still in UseThe
setConfigfunction removal cannot be approved at this time as it is still referenced incmd/zetacored/parse_genesis_test.go. Please ensure that all usages ofsetConfigare addressed before proceeding with its removal.Analysis chain
Line range hint
1-258: Removal ofsetConfigfunction is appropriate.The elimination of the
setConfigfunction aligns with the PR objectives of simplifying imports and reducing internal logic in the types package. This change contributes to a more streamlined and maintainable codebase.To ensure that no other tests or files depended on the removed
setConfigfunction, please run the following verification script:If the script returns no results, it confirms that the
setConfigfunction was not used elsewhere in the codebase.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that setConfig is not used elsewhere in the codebase # Test: Search for any remaining usage of setConfig function rg --type go 'func setConfig\(' . rg --type go 'setConfig\(' .Length of output: 414
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
Files selected for processing (23)
- .github/workflows/ci.yml (3 hunks)
- contrib/rpcimportable/go.mod (1 hunks)
- contrib/rpcimportable/rpcimportable_test.go (1 hunks)
- go.mod (1 hunks)
- pkg/rpc/clients_crosschain.go (0 hunks)
- pkg/rpc/clients_test.go (0 hunks)
- pkg/sdkconfig/sdkconfig.go (1 hunks)
- testutil/keeper/config.go (0 hunks)
- testutil/keeper/emissions.go (2 hunks)
- x/authority/types/genesis_test.go (1 hunks)
- x/authority/types/policies_test.go (1 hunks)
- x/crosschain/keeper/cctx_test.go (2 hunks)
- x/crosschain/types/cctx_test.go (0 hunks)
- x/crosschain/types/message_migrate_erc20_custody_funds_test.go (1 hunks)
- x/crosschain/types/message_migrate_tss_funds_test.go (1 hunks)
- x/crosschain/types/message_update_erc20_custody_pause_status_test.go (1 hunks)
- x/crosschain/types/message_update_tss_address_test.go (1 hunks)
- x/crosschain/types/message_whitelist_erc20_test.go (1 hunks)
- x/emissions/abci_test.go (2 hunks)
- x/observer/types/message_vote_blame_test.go (1 hunks)
- x/observer/types/message_vote_block_header_test.go (1 hunks)
- zetaclient/zetacore/client_crosschain.go (1 hunks)
- zetaclient/zetacore/client_test.go (2 hunks)
Files not reviewed due to no reviewable changes (4)
- pkg/rpc/clients_crosschain.go
- pkg/rpc/clients_test.go
- testutil/keeper/config.go
- x/crosschain/types/cctx_test.go
Additional context used
Path-based instructions (16)
contrib/rpcimportable/rpcimportable_test.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.pkg/sdkconfig/sdkconfig.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.testutil/keeper/emissions.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.x/authority/types/genesis_test.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.x/authority/types/policies_test.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.x/crosschain/keeper/cctx_test.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.x/crosschain/types/message_migrate_erc20_custody_funds_test.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.x/crosschain/types/message_migrate_tss_funds_test.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.x/crosschain/types/message_update_erc20_custody_pause_status_test.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.x/crosschain/types/message_update_tss_address_test.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.x/crosschain/types/message_whitelist_erc20_test.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.x/emissions/abci_test.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.x/observer/types/message_vote_blame_test.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.x/observer/types/message_vote_block_header_test.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.zetaclient/zetacore/client_crosschain.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.zetaclient/zetacore/client_test.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
GitHub Check: codecov/patch
pkg/sdkconfig/sdkconfig.go
[warning] 25-25: pkg/sdkconfig/sdkconfig.go#L25
Added line #L25 was not covered by tests
[warning] 29-34: pkg/sdkconfig/sdkconfig.go#L29-L34
Added lines #L29 - L34 were not covered by testszetaclient/zetacore/client_crosschain.go
[warning] 33-33: zetaclient/zetacore/client_crosschain.go#L33
Added line #L33 was not covered by tests
[warning] 38-39: zetaclient/zetacore/client_crosschain.go#L38-L39
Added lines #L38 - L39 were not covered by tests
[warning] 42-43: zetaclient/zetacore/client_crosschain.go#L42-L43
Added lines #L42 - L43 were not covered by tests
Additional comments not posted (25)
contrib/rpcimportable/rpcimportable_test.go (1)
1-7: Package declaration and imports are appropriate.The package name
rpcimportablealigns with the directory structure, and the imports are correctly formatted and necessary for the test function.contrib/rpcimportable/go.mod (1)
8-10: Appropriate use of replace directive.The replace directive for
github.com/gogo/protobufis correctly implemented. This is a common practice in Cosmos SDK-based projects to ensure compatibility with the forked version maintained by Regen Network.pkg/sdkconfig/sdkconfig.go (1)
1-5: Package declaration and imports are appropriate.The package name
sdkconfigaccurately reflects its purpose, and the import of the Cosmos SDK types package is correctly aliased. This structure promotes clean and maintainable code.x/crosschain/types/message_update_tss_address_test.go (2)
Line range hint
1-124: Overall impact assessment: Minimal with potential for improvement.The changes to the import statement and configuration setup appear to be part of a larger refactoring effort to improve the project's structure and importability. The test cases themselves remain unchanged, suggesting that the underlying functionality being tested is not affected.
To ensure that these changes haven't introduced any regressions, please run the entire test suite and verify that all tests pass. You can use the following command:
#!/bin/bash # Description: Run the entire test suite for the crosschain package # Test: Run all tests in the crosschain package. Expect: All tests pass. go test ./x/crosschain/...Additionally, consider adding more comprehensive tests to cover the new
sdkconfigpackage and its integration with the existing codebase. This will help ensure the robustness of the new configuration management approach.
9-9: Approve import statement modification.The change from
keepertosdkconfigaligns with the PR objectives of improving the project's structure and importability. This modification suggests a more focused approach to managing SDK configurations.To ensure consistency across the codebase, please run the following script to check for any remaining references to the old import:
x/observer/types/message_vote_blame_test.go (2)
Line range hint
1-138: Verify test coverage and behavior consistency.While the changes to the configuration setup appear localized, it's crucial to ensure that the new
sdkconfig.SetDefault(false)provides the same testing environment as the previouskeeper.SetConfig(false).To maintain the integrity of the test suite, please:
- Run the entire test suite for the
observermodule to verify that all tests still pass with the new configuration setup.- Review the test coverage to ensure it hasn't been inadvertently affected by the configuration change.
You can use the following commands to assist with this verification:
#!/bin/bash # Description: Verify test coverage and consistency for the observer module # Run tests for the observer module go test ./x/observer/... -v # Generate and view test coverage report go test ./x/observer/... -coverprofile=coverage.out go tool cover -html=coverage.out -o coverage.html echo "Coverage report generated: coverage.html" # Check for any test files that might need updating due to the configuration change echo "Test files that might need review:" rg --type go 'keeper\.SetConfig' ./x/observer/...Ensure that the test results and coverage remain consistent with the previous implementation.
10-10: Approve import statement change and verifysdkconfigusage.The change from
keepertosdkconfigaligns with the PR objectives of improving the project's importability. This modification suggests a more modular approach to configuration management.To ensure consistency across the project, please run the following script to verify the usage of the
sdkconfigpackage:x/authority/types/genesis_test.go (2)
10-10: Import addition is appropriate.The addition of the
sdkconfigpackage import is correct and aligns with the subsequent usage in the test setup.
10-16: Changes align with PR objectives and improve code standardization.The modifications to the import statement and test setup are consistent with the pull request's objectives of enhancing importability and simplifying the codebase. By standardizing the configuration setup through the
sdkconfigpackage, these changes contribute to a more consistent and maintainable test suite. This approach should facilitate easier imports for external users and simplify the management of configuration across the project.x/crosschain/types/message_whitelist_erc20_test.go (1)
9-9: Approve import change with verification suggestion.The replacement of the
keeperpackage import withsdkconfigaligns with the PR objectives of improving code structure and importability. This change suggests a shift in configuration management, which is a positive step towards better modularity.To ensure this change is consistent across the codebase, please run the following verification script:
x/crosschain/types/message_update_erc20_custody_pause_status_test.go (1)
Line range hint
1-138: Summarize overall impact and suggest comprehensive testing.The changes to this file are part of a larger refactoring effort to improve the project's structure and importability. While the modifications are minimal and don't affect the test cases directly, it's crucial to ensure that these changes don't introduce any unintended side effects.
To validate the changes:
Run the entire test suite to confirm that all tests still pass:
go test ./...If you have a CI/CD pipeline, ensure that all stages complete successfully, including any integration or end-to-end tests that might be affected by configuration changes.
Manually verify that the behavior of the
MsgUpdateERC20CustodyPauseStatusmessage remains consistent with the expected functionality in a test environment.x/crosschain/types/message_migrate_tss_funds_test.go (2)
17-17: Approve function call change and suggest documentation update.The replacement of
keeper.SetConfig(false)withsdkconfig.SetDefault(false)is consistent with the import statement change and appears to maintain the same functionality. The new function name,SetDefault, is more descriptive and aligns with best practices for clear and self-explanatory method naming.To ensure clarity for other developers, consider updating any relevant documentation to reflect this change in configuration management approach. If there's a README or documentation file for this package, it would be beneficial to mention the transition from
keeper.SetConfigtosdkconfig.SetDefault.
Line range hint
1-138: Summary of changes and suggestion for comprehensive testing.The modifications to this test file, namely the import statement change and the function call update, appear to be part of a larger refactoring effort to improve the project's structure and importability. These changes align well with the PR objectives of addressing dependency issues and ensuring the project remains importable.
While the core functionality of the test cases remains unchanged, it's crucial to ensure that these modifications haven't introduced any unintended side effects.
To validate the changes:
Run the entire test suite to ensure no regressions:
Verify that the new
sdkconfig.SetDefault(false)function behaves identically to the previouskeeper.SetConfig(false):Review the results to ensure consistent usage and behavior across the project.
.github/workflows/ci.yml (1)
Line range hint
1-127: Approve overall workflow structure.The updated workflow structure effectively incorporates the new
rpcimportablejob and the finalci-okcheck while maintaining existing functionality. This aligns well with the PR objectives of adding an importability check and simplifying status checks.The dependency structure ensures that all checks are completed before the final status is determined, providing a comprehensive CI process.
The workflow now provides a robust and comprehensive CI pipeline that addresses the project's needs.
Tools
actionlint
95-95: shellcheck reported issue in this script: SC2035:info:1:8: Use ./glob or -- glob so names with dashes won't become options
(shellcheck)
x/observer/types/message_vote_block_header_test.go (1)
15-15: Import change approved.The replacement of the
keeperpackage withsdkconfigaligns with the project's goal of simplifying imports and reducing dependencies. This change contributes to a cleaner and more maintainable codebase.x/crosschain/types/message_migrate_erc20_custody_funds_test.go (3)
Line range hint
1-170: Comprehensive test coverage with room for enhancement.The test file provides thorough coverage for the
MsgMigrateERC20CustodyFundsmessage type, including various scenarios for message validation and signer verification. The recent changes in import statements and configuration setup appear to be isolated and don't directly affect the existing test cases.To further improve the test suite, consider the following suggestions:
- Add test cases that specifically verify the behavior affected by the new
sdkconfig.SetDefault(false)configuration.- Include edge cases for the
Amountfield, such as testing with the maximum allowed value.- Consider adding benchmarks for performance-critical operations, if applicable.
- Enhance documentation by adding comments explaining the purpose of each test case group.
17-17: Verify the equivalence of the new configuration setup.The change from
keeper.SetConfig(false)tosdkconfig.SetDefault(false)aligns with the import statement modification. However, it's crucial to ensure that this new setup provides equivalent functionality for the test cases.To confirm that the new configuration setup maintains the intended test behavior, please run the following verification:
#!/bin/bash # Description: Verify the usage and impact of the new sdkconfig.SetDefault function # Test 1: Check for any remaining usage of the old keeper.SetConfig function echo "Searching for any remaining usage of keeper.SetConfig:" rg --type go 'keeper\.SetConfig' # Test 2: Verify the usage of the new sdkconfig.SetDefault function echo "Verifying usage of sdkconfig.SetDefault:" rg --type go 'sdkconfig\.SetDefault' # Test 3: Check for any test failures that might be related to this change echo "Running tests to check for any failures:" go test ./x/crosschain/types/... -vPlease review the test results to ensure that the new configuration setup maintains the intended test behavior and doesn't introduce any regressions.
11-11: Evaluate the impact of the configuration management change.The replacement of the
keeperpackage withsdkconfigindicates a shift in the configuration management approach. This change may have implications for other parts of the codebase that rely on the previouskeeperpackage for configuration.To ensure this change doesn't introduce unintended side effects, please run the following verification:
Verification successful
Further Verification Required for Configuration Management Changes
The previous verification attempt encountered an error when executing
go test, which is unsupported in the current environment. To ensure that thekeeperpackage has been entirely removed and that thesdkconfigpackage is correctly implemented, please execute the following shell scripts:
Configuration Management Change Verified Successfully
The old
keeperpackage has been completely removed, and the newsdkconfigpackage is correctly imported and utilized across all relevant test files. No issues were found regarding the configuration management changes.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining references to the old keeper package and verify the usage of the new sdkconfig package. # Test 1: Search for any remaining imports of the old keeper package echo "Searching for remaining keeper package imports:" rg --type go 'import.*"github\.com/zeta-chain/node/x/crosschain/keeper"' # Test 2: Verify the usage of the new sdkconfig package echo "Verifying usage of the new sdkconfig package:" rg --type go 'import.*"github\.com/zeta-chain/node/pkg/sdkconfig"' rg --type go 'sdkconfig\.SetDefault'Length of output: 1310
Script:
#!/bin/bash # Description: Verify the removal of the old keeper package and the proper usage of the new sdkconfig package. # Test 1: Ensure no imports of the old keeper package remain echo "Checking for any remaining imports of 'github.com/zeta-chain/node/x/crosschain/keeper':" rg --type go 'import.*"github\.com/zeta-chain/node/x/crosschain/keeper"' # Test 2: Confirm the new sdkconfig package is correctly imported echo "Verifying imports of 'github.com/zeta-chain/node/pkg/sdkconfig':" rg --type go 'import.*"github\.com/zeta-chain/node/pkg/sdkconfig"' # Test 3: Check for usage of sdkconfig.SetDefault echo "Searching for usage of 'sdkconfig.SetDefault':" rg --type go 'sdkconfig\.SetDefault'Length of output: 1454
x/authority/types/policies_test.go (3)
8-8: Import change is appropriate.The addition of the
sdkconfigpackage import aligns with the modifications in the test setup, replacing the previoussetConfigfunction.
Line range hint
1-258: Summary of changes and their impact.The modifications in this file effectively simplify the test setup process by replacing the
setConfigfunction with a direct call tosdkconfig.SetDefault(false). These changes align with the PR objectives of streamlining imports and reducing internal logic in the types package. The existing test cases remain intact, maintaining the overall test coverage and functionality.The improvements contribute to a more maintainable and cleaner codebase, which should facilitate easier imports for external users. However, it is crucial to verify that the new configuration setup provides equivalent functionality to the previous approach and that the removal of
setConfigdoes not impact other parts of the codebase.
14-14: Test setup simplification is appropriate.The replacement of
setConfigwithsdkconfig.SetDefault(false)streamlines the test setup process. This change eliminates the need for error handling related to configuration sealing, resulting in a more concise and maintainable test setup.To ensure the new setup provides equivalent configuration, please verify that
sdkconfig.SetDefault(false)sets the same bech32 prefixes for Zeta Chain accounts as the previoussetConfigfunction. You can run the following script to check the configuration:Verification successful
Configuration Verification Successful.
The usage of
sdkconfig.SetDefault(false)in place ofsetConfighas been verified across the codebase. This change consistently sets the necessary Bech32 prefixes without sealing the configuration, ensuring a flexible and reliable test setup similar to the previous implementation.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the configuration set by sdkconfig.SetDefault(false) # Test: Search for the SetDefault function usage and its surrounding context rg --type go -A 10 -B 10 'SetDefault\(false\)' pkg/sdkconfigLength of output: 62
Script:
#!/bin/bash # Description: Search the entire codebase for all usages of the SetDefault function # Search for any occurrence of SetDefault in Go files with context lines rg --type go 'SetDefault\(' -A 5 -B 5Length of output: 9662
zetaclient/zetacore/client_test.go (1)
12-12: Import statements are correctly added and necessary.The new import statements for
query,chains, andinterfacesare appropriately placed and required for the new test function. They contribute to the enhanced functionality without introducing unnecessary dependencies.Also applies to: 19-20
go.mod (1)
60-60: Approve dependency update with verification recommendation.The update to
github.com/zeta-chain/ethermintappears to be a minor version change. This aligns with the PR objectives of addressing import issues and improving the project's importability.To ensure compatibility, please run the following verification script:
x/emissions/abci_test.go (2)
Line range hint
1-524: LGTM: Comprehensive test coverage for emissions module.The existing test cases in this file provide thorough coverage for the emissions module, particularly for observer rewards distribution. The various scenarios tested, including different voting patterns and reward calculations, contribute to a robust test suite.
315-315: Verify the impact of configuration change across the codebase.The change from
keepertest.SetConfig(false)tosdkconfig.SetDefault(false)appears to be a refactoring of the configuration setup. While this change is likely part of a larger effort to standardize configuration management, it's crucial to ensure consistency across the entire codebase.To assess the impact of this change, please run the following script:
This script will help identify any inconsistencies in configuration management across the project.
Verification successful
Configuration refactoring successfully verified across the codebase.
The replacement of
keepertest.SetConfig(false)withsdkconfig.SetDefault(false)has been consistently applied throughout the project. No instances ofkeepertest.SetConfigremain, andsdkconfig.SetDefault(false)is uniformly used in relevant test files, ensuring standardized configuration management.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining usage of keepertest.SetConfig and identify other occurrences of sdkconfig.SetDefault echo "Checking for remaining usage of keepertest.SetConfig:" rg --type go "keepertest\.SetConfig" echo "\nChecking for other occurrences of sdkconfig.SetDefault:" rg --type go "sdkconfig\.SetDefault" echo "\nChecking for any potential imports of keepertest that might be related to SetConfig:" rg --type go "import.*keepertest"Length of output: 1365
gartnera
force-pushed
the
rpc-importable-test
branch
from
50db18b
to
f11064d
Compare
* ci: add rpcimportable test * add ci * fmt * use github.com/btcsuite/btcd/btcutil in pkg/chains * remove app imports types tests * use standalone sdkconfig package * fix policies test * move crosschain keeper tests from types to keeper * never seal config in tests * use zeta-chain/ethermint#126 * add some comments * use merged ethermint hash * show resulting go.mod
…guration (#2953) * update artillery config * more fixes * feat: integrate authenticated calls smart contract functionality into protocol (#2904) * e2e tests and modifications for authenticated call * extend test with sender check and revert case * separate tests into separate files * cleanup * withdraw and call support and tests * bump protocol contracts * split tests into separate files * small cleanup * fmt * generate * lint * changelog * PR comments * fix case in proto * bump vote inbound gas limit in zetaclient * fix test * generate * fixing tests * call options non empty * generate * test fix * rename gateway caller * pr comments rename tests * PR comment * generate * tests * update tests fixes * tests fixes * fix * test fix * feat!: bank precompile (#2860) * feat: bank precompile * feat: add deposit * feat: extend deposit * PoC: spend amount on behalf of EOA * feat: expand deposit with transferFrom * use CallEVM instead on ZRC20 bindings * divide the contract into different files * initialize e2e testing * remove duplicated funding * add codecov * expand e2e * fix: wait for deposit tx to be mined * apply first round of reviews * cover al error types test * fixes using time.Since * Include CallContract interface * fix eth events in deposit precompile method * emit Deposit event * add withdraw function * finalize withdraw * pack event arguments generically * add high level event function * first round of review fixes * second round of reviews * create bank account when instantiating bank * e2e: add good and bad scenarios * modify fmt * chore: group input into eventData struct * docs: document bank's methods * chore: generate files with suffix .gen.go * chore: assert errors with errorIs * chore: reset e2e test by resetting allowance * test: add first batch of unit test * test: cover all cases * test: complete unit test cases * include review suggestions * include e2e through contract * test: add e2e through contract complete * test: revert balance between tests * Update precompiles/bank/const.go Co-authored-by: Lucas Bertrand <lucas.bertrand.22@gmail.com> * fix: changed coin denom --------- Co-authored-by: skosito <skostic9242@gmail.com> Co-authored-by: Lucas Bertrand <lucas.bertrand.22@gmail.com> * feat: add sender to revert context (#2919) * e2e tests and modifications for authenticated call * extend test with sender check and revert case * separate tests into separate files * cleanup * withdraw and call support and tests * bump protocol contracts * split tests into separate files * small cleanup * fmt * generate * lint * changelog * PR comments * fix case in proto * bump vote inbound gas limit in zetaclient * fix test * generate * fixing tests * call options non empty * generate * test fix * rename gateway caller * pr comments rename tests * PR comment * generate * tests * add sender in test contract * extend e2e tests * generate * changelog * PR comment * generate * update tests fixes * tests fixes * fix * test fix * gas limit fixes * PR comment fix * fix bad merge * ci: add option to enable monitoring stack (#2927) * ci: add option to enable monitoring stack * start prometheus faster * update * ci: add rpcimportable test (#2817) * ci: add rpcimportable test * add ci * fmt * use github.com/btcsuite/btcd/btcutil in pkg/chains * remove app imports types tests * use standalone sdkconfig package * fix policies test * move crosschain keeper tests from types to keeper * never seal config in tests * use zeta-chain/ethermint#126 * add some comments * use merged ethermint hash * show resulting go.mod * ci: Add SARIF upload to GitHub Security Dashboard (#2929) * add semgrep sarif upload to GHAS * added comment to clairfy the usage of the utility script * use ghcr.io instead * add tag to image * bad org name --------- Co-authored-by: jkan2 <5862123+jkan2@users.noreply.github.com> * fix: add recover to InitChainer to diplay informative message when starting a node from block 1 (#2925) * add recover to InitChainer * generate files * add docs link to error message * move InitChainErrorMessage to app.go * Update app/app.go Co-authored-by: Francisco de Borja Aranda Castillejo <borja@zetachain.com> * use const for message --------- Co-authored-by: Francisco de Borja Aranda Castillejo <borja@zetachain.com> * test: add wait for block to tss migration test (#2931) * add wait for block to tss migration test * add comments * refactor identifiers * rename checkNumberOfTssGenerated to checkNumberOfTSSGenerated * chore: allow full zetaclient config overlay (#2945) * test(e2e): add gateway upgrade in upgrade test (#2932) * add gateway upgrade * change reference * add v2 setup for all tests * test v2 in light upgrade * refactor setup to use custody v2 directly * chore: improve localnet build performance (#2928) * chore: improve localnet build performance * propagate NODE_VERSION and NODE_COMMIT * update hashes --------- Co-authored-by: skosito <skostic9242@gmail.com> Co-authored-by: Francisco de Borja Aranda Castillejo <borja@zetachain.com> Co-authored-by: Lucas Bertrand <lucas.bertrand.22@gmail.com> Co-authored-by: Alex Gartner <alexg@zetachain.com> Co-authored-by: jkan2 <jkan2@users.noreply.github.com> Co-authored-by: jkan2 <5862123+jkan2@users.noreply.github.com> Co-authored-by: Tanmay <tanmay@zetachain.com>
Add a CI check that the RPC package is importable. This check should eventually check the import tree to ensure that the required dependencies are minimal.
Will move required status check to
ci-okto make the required status checks easier too.Also fix issues causing import:
GetAllOutboundTrackerByChainback to zetaclient as it imports zetaclientUpdate: the btcd deps are just too complex. Blocked by #2728. Update: unblocked now.
Closes #2798
Currently failing with:
https://go.dev/ref/mod#go-mod-tidy
We should try ensure there is minimal internal logic in the types package as all that will be imported when a user tries to import the package.
Ok now we're hitting:
We definitely cannot force external users to use our ethereum/client-go fork. I will try to move that logic to keeper.
zeta-chain/ethermint#126
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores