test: update tss address on connectors and custody contracts#2661
test: update tss address on connectors and custody contracts#2661
Conversation
|
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 WalkthroughWalkthroughThis update introduces enhancements to the migration and testing functionalities within the codebase, particularly focusing on TSS (Threshold Signature Scheme) migrations. Key changes include improvements to environment variable management, restructuring of migration functions, updates to end-to-end (e2e) tests, and modifications to logging and error handling. Overall, the changes aim to refine the testing workflow, ensure accurate configuration, and bolster the robustness of the migration process. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant LocalNet
participant MigrationService
participant E2ETests
participant SmartContract
User->>LocalNet: Start e2e test in migrate mode
LocalNet->>MigrationService: Check for deployed.yml
alt Not Found
MigrationService->>LocalNet: Execute setup command
LocalNet-->>User: Setup successful
end
MigrationService->>E2ETests: Run pre-migration tests
E2ETests-->>MigrationService: Tests successful
MigrationService->>SmartContract: Update TSS addresses
SmartContract->>MigrationService: Transaction receipt
MigrationService->>E2ETests: Run post-migration tests
E2ETests-->>LocalNet: Verify TSS address functionality
LocalNet-->>User: E2E tests completed
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 Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #2661 +/- ##
========================================
Coverage 66.90% 66.91%
========================================
Files 364 364
Lines 20458 20461 +3
========================================
+ Hits 13688 13691 +3
Misses 6143 6143
Partials 627 627
|
kingpinXD
requested review from
brewmaster012,
fbac,
lumtis,
skosito,
swift1337 and
ws4charlie
as code owners
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (3)
e2e/e2etests/test_crosschain_swap.go (1)
28-28: Improve logging readability.The logging statement for the error message when creating a pair in the Uniswap factory has been reformatted for better readability. While this does not affect functionality, it enhances clarity in the logs.
- r.Logger.Print("ℹ️ create pair error") + r.Logger.Print("ℹ️ create pair error")contrib/localnet/orchestrator/start-zetae2e.sh (2)
98-127: Enhance error handling for migration mode.In the "migrate" mode, consider adding more detailed error messages or logs to help diagnose issues if the setup or tests fail. This will improve the maintainability and debuggability of the script.
# Example: Enhance error logging echo "e2e setup failed at $(date)" >&2
Line range hint
131-209:
Improve maintainability by consolidating repeated code.The script contains repeated logic for setting up and running tests in both "migrate" and "upgrade" modes. Consider refactoring these sections into functions to improve maintainability and reduce duplication.
# Example: Define a function for e2e setup function run_e2e_setup() { if [[ ! -f deployed.yml ]]; then zetae2e local $E2E_ARGS --setup-only --config config.yml --config-out deployed.yml --skip-header-proof if [ $? -ne 0 ]; then echo "e2e setup failed" exit 1 fi else echo "skipping e2e setup because it has already been completed" fi } # Use the function in both modes run_e2e_setup
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (13)
- Makefile (1 hunks)
- changelog.md (1 hunks)
- cmd/zetae2e/local/local.go (3 hunks)
- cmd/zetae2e/local/migration.go (1 hunks)
- contrib/localnet/orchestrator/start-zetae2e.sh (3 hunks)
- e2e/e2etests/test_crosschain_swap.go (4 hunks)
- e2e/e2etests/test_message_passing_external_chains_revert_fail.go (2 hunks)
- e2e/e2etests/test_migrate_tss.go (2 hunks)
- e2e/e2etests/test_zrc20_swap.go (1 hunks)
- e2e/runner/update_contracts.go (1 hunks)
- e2e/utils/zetacore.go (1 hunks)
- testutil/keeper/mocks/crosschain/observer.go (1 hunks)
- x/crosschain/keeper/msg_server_add_outbound_tracker.go (1 hunks)
Files skipped from review due to trivial changes (4)
- cmd/zetae2e/local/migration.go
- e2e/e2etests/test_migrate_tss.go
- e2e/e2etests/test_zrc20_swap.go
- x/crosschain/keeper/msg_server_add_outbound_tracker.go
Additional context used
Path-based instructions (7)
e2e/runner/update_contracts.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.e2e/e2etests/test_message_passing_external_chains_revert_fail.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.e2e/e2etests/test_crosschain_swap.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.e2e/utils/zetacore.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.contrib/localnet/orchestrator/start-zetae2e.sh (1)
Pattern
**/*.sh: Review the shell scripts, point out issues relative to security, performance, and maintainability.cmd/zetae2e/local/local.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.testutil/keeper/mocks/crosschain/observer.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
Additional comments not posted (11)
e2e/runner/update_contracts.go (2)
11-25: Well-structured function with robust error handling.The
UpdateTssAddressForConnectorfunction efficiently updates the TSS address, logs the transaction, and verifies the update. The use ofrequire.NoErrorensures that any errors are caught immediately, maintaining robustness.
27-40: Consistent and correct implementation.The
UpdateTssAddressForErc20custodyfunction follows a pattern similar toUpdateTssAddressForConnector, ensuring consistency across updates. The error handling and logging are appropriately implemented.e2e/e2etests/test_message_passing_external_chains_revert_fail.go (1)
43-43: Verify the increasedDestinationGasLimit.The
DestinationGasLimithas been increased from400,000to1,800,000. Ensure this change is intentional and aligns with the test's objectives, as it may affect transaction execution and cost.e2e/e2etests/test_crosschain_swap.go (2)
18-19: Enhance test setup with local Bitcoin mining.The addition of
r.MineBlocksIfLocalBitcoin()at the start of the function ensures that local Bitcoin blocks are mined during the test, enhancing the setup for cross-chain scenarios. This change improves the reliability of the test environment.
146-147: Dynamic UTXO handling enhances flexibility.The change to dynamically retrieve UTXOs using
r.ListDeployerUTXOs()ensures that the most current state is utilized, enhancing the flexibility and accuracy of the test.e2e/utils/zetacore.go (1)
23-23: Verify the impact of increased timeout duration.The
DefaultCctxTimeouthas been increased from4 * time.Minuteto6 * time.Minute. Ensure that this change aligns with the intended use cases and does not lead to unintended side effects, such as prolonged wait times in failure scenarios.cmd/zetae2e/local/local.go (1)
Line range hint
409-435:
Verify robustness and dependencies of TSS migration logic.The migration logic has been streamlined, focusing on the TSS address update process. Ensure that all dependencies are correctly handled and that the migration process is robust against failures or interruptions.
Verification successful
Ensure Robust Error Handling in
runTSSMigrationFunctionThe
runTSSMigrationfunction relies on external services for critical operations such as fetching the last Zeta height and updating the keygen. Ensure that all potential errors from these services are properly handled and logged to maintain robustness in the TSS migration process. Consider implementing retry mechanisms or fallback strategies if necessary.
- Location:
cmd/zetae2e/local/local.goat lines whereLastZetaHeightandUpdateKeygenare called.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the robustness and dependencies of the TSS migration logic. # Test: Search for all usages and dependencies of `runTSSMigration`. Expect: Ensure all dependencies are correctly handled. rg --type go 'runTSSMigration'Length of output: 255
Script:
#!/bin/bash # Description: Examine dependencies and error handling within the `runTSSMigration` function. # Look for the implementation or definition of `LastZetaHeight` to understand its behavior and error handling. ast-grep --lang go --pattern 'func LastZetaHeight($_, $_) $_ { $$$ }' # Look for the implementation or definition of `UpdateKeygen` to understand its behavior and error handling. ast-grep --lang go --pattern 'func UpdateKeygen($_) $_ { $$$ }'Length of output: 137
Script:
#!/bin/bash # Description: Search for the definitions or usages of `LastZetaHeight` and `UpdateKeygen` in the codebase. # Search for any references to `LastZetaHeight` to find its definition or usage. rg --type go 'LastZetaHeight' # Search for any references to `UpdateKeygen` to find its definition or usage. rg --type go 'UpdateKeygen'Length of output: 22274
Makefile (1)
266-266: Environment variable addition is appropriate.The addition of
export LOCALNET_MODE=migratein thestart-tss-migration-testtarget is a suitable enhancement for configuring the local network mode during migration tests.testutil/keeper/mocks/crosschain/observer.go (2)
607-625: Mock functionGetSupportedForeignChainsis well-implemented.The function enhances testing flexibility by allowing the simulation of interactions related to supported foreign chains. It adheres to the existing pattern for mock functions.
627-645: Mock functionGetSupportedForeignChainsByConsensusis well-implemented.This function extends the mocking capabilities by incorporating consensus-based filtering, which is useful for more specific testing scenarios. It maintains consistency with other mock functions.
changelog.md (1)
16-19: Changelog entry is clear and informative.The addition under the "Tests" section effectively documents the update of connector and ERC20 custody addresses in TSS migration e2e tests, enhancing the clarity of recent changes.
5 participants
Description
Closes: #2443
#2444
How Has This Been Tested?
Summary by CodeRabbit
New Features
Bug Fixes
TestMessagePassingRevertFailExternalChainsfunction, improving transaction execution success.Documentation
Style
Chores