test: tss migration e2e test #2440
Conversation
WalkthroughThe recent updates introduce enhancements and new features, focusing on migration testing and TSS (Threshold Signature Scheme) generation. Key changes include updates to the Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Make as Makefile
participant ClientSupervisor as zetaclientd-supervisor
participant Zetaclientd as zetaclientd
participant E2E as End-to-End Tests
participant XC as Crosschain Keeper
Dev->>Make: Run install
Make->>ClientSupervisor: Install zetaclientd-supervisor
Make->>Zetaclientd: Install Zetaclientd
Dev->>Make: Run start-migrate-test
Make->>E2E: Start migration test
E2E->>XC: ValidateInbound with TSS migration check
XC->>E2E: Return validation status
E2E-->>Dev: Test results
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 Configuration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #2440 +/- ##
===========================================
- Coverage 47.19% 46.88% -0.32%
===========================================
Files 444 445 +1
Lines 29372 29636 +264
===========================================
+ Hits 13863 13894 +31
- Misses 14704 14936 +232
- Partials 805 806 +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: 1
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (6)
- cmd/zetae2e/config/localnet.yml (1 hunks)
- e2e/runner/bitcoin.go (4 hunks)
- x/crosschain/keeper/cctx_orchestrator_validate_inbound.go (3 hunks)
- x/crosschain/keeper/cctx_orchestrator_validate_inbound_test.go (1 hunks)
- zetaclient/chains/bitcoin/signer/signer.go (2 hunks)
- zetaclient/chains/evm/observer/outbound.go (3 hunks)
Files skipped from review as they are similar to previous changes (4)
- e2e/runner/bitcoin.go
- x/crosschain/keeper/cctx_orchestrator_validate_inbound.go
- x/crosschain/keeper/cctx_orchestrator_validate_inbound_test.go
- zetaclient/chains/evm/observer/outbound.go
Additional context used
Path-based instructions (1)
zetaclient/chains/bitcoin/signer/signer.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
Gitleaks
cmd/zetae2e/config/localnet.yml
38-38: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
Additional comments not posted (2)
zetaclient/chains/bitcoin/signer/signer.go (2)
36-36: LGTM!The constant
MaxNoOfInputsPerTxis appropriately set to 20.
Line range hint
199-203:
LGTM!Using the
MaxNoOfInputsPerTxconstant improves the readability and maintainability of the code.
There was a problem hiding this comment.
Actionable comments posted: 2
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (22)
- Makefile (2 hunks)
- changelog.md (1 hunks)
- cmd/zetaclientd/keygen_tss.go (5 hunks)
- cmd/zetaclientd/start.go (2 hunks)
- cmd/zetae2e/config/localnet.yml (1 hunks)
- cmd/zetae2e/local/local.go (12 hunks)
- cmd/zetae2e/local/post_migration.go (1 hunks)
- contrib/localnet/scripts/start-zetacored.sh (1 hunks)
- e2e/config/config.go (3 hunks)
- e2e/e2etests/test_migrate_tss.go (1 hunks)
- e2e/runner/accounting.go (3 hunks)
- e2e/txserver/zeta_tx_server.go (1 hunks)
- x/crosschain/keeper/cctx_orchestrator_validate_inbound.go (3 hunks)
- zetaclient/chains/bitcoin/observer/inbound.go (2 hunks)
- zetaclient/chains/bitcoin/observer/outbound.go (3 hunks)
- zetaclient/chains/bitcoin/signer/signer.go (2 hunks)
- zetaclient/chains/evm/observer/inbound.go (1 hunks)
- zetaclient/chains/evm/observer/outbound.go (4 hunks)
- zetaclient/chains/interfaces/interfaces.go (1 hunks)
- zetaclient/testutils/mocks/tss_signer.go (1 hunks)
- zetaclient/tss/tss_signer.go (7 hunks)
- zetaclient/zetacore/client_monitor.go (1 hunks)
Files skipped from review due to trivial changes (1)
- changelog.md
Files skipped from review as they are similar to previous changes (16)
- Makefile
- cmd/zetaclientd/keygen_tss.go
- cmd/zetaclientd/start.go
- cmd/zetae2e/local/local.go
- cmd/zetae2e/local/post_migration.go
- contrib/localnet/scripts/start-zetacored.sh
- e2e/runner/accounting.go
- e2e/txserver/zeta_tx_server.go
- x/crosschain/keeper/cctx_orchestrator_validate_inbound.go
- zetaclient/chains/bitcoin/observer/inbound.go
- zetaclient/chains/bitcoin/observer/outbound.go
- zetaclient/chains/bitcoin/signer/signer.go
- zetaclient/chains/evm/observer/inbound.go
- zetaclient/chains/evm/observer/outbound.go
- zetaclient/testutils/mocks/tss_signer.go
- zetaclient/tss/tss_signer.go
Additional context used
Path-based instructions (4)
zetaclient/zetacore/client_monitor.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.e2e/e2etests/test_migrate_tss.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.zetaclient/chains/interfaces/interfaces.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.e2e/config/config.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
Gitleaks
cmd/zetae2e/config/localnet.yml
38-38: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
GitHub Check: codecov/patch
e2e/e2etests/test_migrate_tss.go
[warning] 23-26: e2e/e2etests/test_migrate_tss.go#L23-L26
Added lines #L23 - L26 were not covered by tests
[warning] 29-35: e2e/e2etests/test_migrate_tss.go#L29-L35
Added lines #L29 - L35 were not covered by tests
[warning] 39-40: e2e/e2etests/test_migrate_tss.go#L39-L40
Added lines #L39 - L40 were not covered by tests
[warning] 42-44: e2e/e2etests/test_migrate_tss.go#L42-L44
Added lines #L42 - L44 were not covered by tests
[warning] 47-47: e2e/e2etests/test_migrate_tss.go#L47
Added line #L47 was not covered by tests
[warning] 49-50: e2e/e2etests/test_migrate_tss.go#L49-L50
Added lines #L49 - L50 were not covered by tests
[warning] 54-61: e2e/e2etests/test_migrate_tss.go#L54-L61
Added lines #L54 - L61 were not covered by tests
[warning] 64-67: e2e/e2etests/test_migrate_tss.go#L64-L67
Added lines #L64 - L67 were not covered by tests
[warning] 71-73: e2e/e2etests/test_migrate_tss.go#L71-L73
Added lines #L71 - L73 were not covered by tests
[warning] 75-77: e2e/e2etests/test_migrate_tss.go#L75-L77
Added lines #L75 - L77 were not covered by tests
[warning] 80-86: e2e/e2etests/test_migrate_tss.go#L80-L86
Added lines #L80 - L86 were not covered by tests
[warning] 89-94: e2e/e2etests/test_migrate_tss.go#L89-L94
Added lines #L89 - L94 were not covered by tests
[warning] 96-97: e2e/e2etests/test_migrate_tss.go#L96-L97
Added lines #L96 - L97 were not covered by tests
[warning] 99-100: e2e/e2etests/test_migrate_tss.go#L99-L100
Added lines #L99 - L100 were not covered by tests
[warning] 103-104: e2e/e2etests/test_migrate_tss.go#L103-L104
Added lines #L103 - L104 were not covered by tests
[warning] 106-106: e2e/e2etests/test_migrate_tss.go#L106
Added line #L106 was not covered by tests
[warning] 109-117: e2e/e2etests/test_migrate_tss.go#L109-L117
Added lines #L109 - L117 were not covered by tests
[warning] 120-120: e2e/e2etests/test_migrate_tss.go#L120
Added line #L120 was not covered by tests
[warning] 122-124: e2e/e2etests/test_migrate_tss.go#L122-L124
Added lines #L122 - L124 were not covered by tests
[warning] 126-127: e2e/e2etests/test_migrate_tss.go#L126-L127
Added lines #L126 - L127 were not covered by tests
[warning] 131-132: e2e/e2etests/test_migrate_tss.go#L131-L132
Added lines #L131 - L132 were not covered by tests
[warning] 134-135: e2e/e2etests/test_migrate_tss.go#L134-L135
Added lines #L134 - L135 were not covered by tests
[warning] 137-138: e2e/e2etests/test_migrate_tss.go#L137-L138
Added lines #L137 - L138 were not covered by tests
[warning] 140-141: e2e/e2etests/test_migrate_tss.go#L140-L141
Added lines #L140 - L141 were not covered by tests
[warning] 143-143: e2e/e2etests/test_migrate_tss.go#L143
Added line #L143 was not covered by tests
[warning] 145-146: e2e/e2etests/test_migrate_tss.go#L145-L146
Added lines #L145 - L146 were not covered by tests
[warning] 149-151: e2e/e2etests/test_migrate_tss.go#L149-L151
Added lines #L149 - L151 were not covered by tests
[warning] 155-160: e2e/e2etests/test_migrate_tss.go#L155-L160
Added lines #L155 - L160 were not covered by tests
[warning] 164-166: e2e/e2etests/test_migrate_tss.go#L164-L166
Added lines #L164 - L166 were not covered by testse2e/config/config.go
[warning] 289-289: e2e/config/config.go#L289
Added line #L289 was not covered by tests
Additional comments not posted (7)
cmd/zetae2e/config/localnet.yml (1)
38-38: Reiterate: Avoid exposing private keys in configuration files.The private key for
user_migrationis exposed in the configuration file. This poses a significant security risk. Consider using environment variables or a secure vault to manage sensitive information.- private_key: "0bcc2fa28b526f90e1d54648d612db901e860bf68248555593f91ea801c6b482" + private_key: "${USER_MIGRATION_PRIVATE_KEY}"Tools
Gitleaks
38-38: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
zetaclient/zetacore/client_monitor.go (1)
171-177: Simplified backoff strategy initialization approved.The changes to the
retryWithBackofffunction simplify the initialization of the backoff strategy, which enhances code readability and maintainability.e2e/e2etests/test_migrate_tss.go (2)
49-50: Reiterate: Parameterize the BTC Migration Fee.The hardcoded fee for BTC migration should be parameterized for flexibility.
- btcBalance = btcBalance - 0.01 + migrationFee := getMigrationFee() // Implement getMigrationFee to retrieve the fee from a config or parameter + btcBalance = btcBalance - migrationFeeTools
GitHub Check: codecov/patch
[warning] 49-50: e2e/e2etests/test_migrate_tss.go#L49-L50
Added lines #L49 - L50 were not covered by tests
120-120: Reiterate: Parameterize the Sleep Duration.The sleep duration for waiting for the TSS update should be parameterized for flexibility.
- time.Sleep(8 * time.Second) + sleepDuration := getTssUpdateWaitDuration() // Implement getTssUpdateWaitDuration to retrieve the duration from a config or parameter + time.Sleep(sleepDuration)Tools
GitHub Check: codecov/patch
[warning] 120-120: e2e/e2etests/test_migrate_tss.go#L120
Added line #L120 was not covered by testszetaclient/chains/interfaces/interfaces.go (1)
218-218: New methodEVMAddressList()added toTSSSignerinterface approved.The addition of
EVMAddressList()to theTSSSignerinterface likely enhances its functionality by allowing it to handle multiple Ethereum addresses, which could be crucial for certain operations.e2e/config/config.go (2)
68-68: Addition of UserMigration account in AdditionalAccounts struct.This change aligns with the PR's objective of adding a new end-to-end test for TSS migration, which presumably requires a special account configuration.
202-202: Ensure UserMigration account is included in account slices.The addition of
UserMigrationto theAsSlicemethod ensures that this new account is considered in operations that iterate over all accounts. This is crucial for consistency across the application where multiple accounts are processed together.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (5)
- e2e/e2etests/test_migrate_tss.go (1 hunks)
- e2e/runner/bitcoin.go (4 hunks)
- x/crosschain/keeper/cctx_orchestrator_validate_inbound_test.go (1 hunks)
- x/crosschain/types/errors.go (1 hunks)
- zetaclient/chains/evm/observer/outbound.go (4 hunks)
Files skipped from review as they are similar to previous changes (3)
- e2e/runner/bitcoin.go
- x/crosschain/keeper/cctx_orchestrator_validate_inbound_test.go
- zetaclient/chains/evm/observer/outbound.go
Additional context used
Path-based instructions (2)
x/crosschain/types/errors.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.e2e/e2etests/test_migrate_tss.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
GitHub Check: codecov/patch
e2e/e2etests/test_migrate_tss.go
[warning] 23-26: e2e/e2etests/test_migrate_tss.go#L23-L26
Added lines #L23 - L26 were not covered by tests
[warning] 29-35: e2e/e2etests/test_migrate_tss.go#L29-L35
Added lines #L29 - L35 were not covered by tests
[warning] 39-40: e2e/e2etests/test_migrate_tss.go#L39-L40
Added lines #L39 - L40 were not covered by tests
[warning] 42-44: e2e/e2etests/test_migrate_tss.go#L42-L44
Added lines #L42 - L44 were not covered by tests
[warning] 47-47: e2e/e2etests/test_migrate_tss.go#L47
Added line #L47 was not covered by tests
[warning] 49-50: e2e/e2etests/test_migrate_tss.go#L49-L50
Added lines #L49 - L50 were not covered by tests
[warning] 54-61: e2e/e2etests/test_migrate_tss.go#L54-L61
Added lines #L54 - L61 were not covered by tests
[warning] 64-67: e2e/e2etests/test_migrate_tss.go#L64-L67
Added lines #L64 - L67 were not covered by tests
[warning] 71-73: e2e/e2etests/test_migrate_tss.go#L71-L73
Added lines #L71 - L73 were not covered by tests
[warning] 75-77: e2e/e2etests/test_migrate_tss.go#L75-L77
Added lines #L75 - L77 were not covered by tests
[warning] 80-86: e2e/e2etests/test_migrate_tss.go#L80-L86
Added lines #L80 - L86 were not covered by tests
[warning] 89-94: e2e/e2etests/test_migrate_tss.go#L89-L94
Added lines #L89 - L94 were not covered by tests
[warning] 96-97: e2e/e2etests/test_migrate_tss.go#L96-L97
Added lines #L96 - L97 were not covered by tests
[warning] 99-100: e2e/e2etests/test_migrate_tss.go#L99-L100
Added lines #L99 - L100 were not covered by tests
[warning] 103-104: e2e/e2etests/test_migrate_tss.go#L103-L104
Added lines #L103 - L104 were not covered by tests
[warning] 106-106: e2e/e2etests/test_migrate_tss.go#L106
Added line #L106 was not covered by tests
[warning] 109-117: e2e/e2etests/test_migrate_tss.go#L109-L117
Added lines #L109 - L117 were not covered by tests
[warning] 120-120: e2e/e2etests/test_migrate_tss.go#L120
Added line #L120 was not covered by tests
[warning] 122-124: e2e/e2etests/test_migrate_tss.go#L122-L124
Added lines #L122 - L124 were not covered by tests
[warning] 126-127: e2e/e2etests/test_migrate_tss.go#L126-L127
Added lines #L126 - L127 were not covered by tests
[warning] 131-132: e2e/e2etests/test_migrate_tss.go#L131-L132
Added lines #L131 - L132 were not covered by tests
[warning] 134-135: e2e/e2etests/test_migrate_tss.go#L134-L135
Added lines #L134 - L135 were not covered by tests
[warning] 137-138: e2e/e2etests/test_migrate_tss.go#L137-L138
Added lines #L137 - L138 were not covered by tests
[warning] 140-141: e2e/e2etests/test_migrate_tss.go#L140-L141
Added lines #L140 - L141 were not covered by tests
[warning] 143-143: e2e/e2etests/test_migrate_tss.go#L143
Added line #L143 was not covered by tests
[warning] 145-146: e2e/e2etests/test_migrate_tss.go#L145-L146
Added lines #L145 - L146 were not covered by tests
[warning] 149-151: e2e/e2etests/test_migrate_tss.go#L149-L151
Added lines #L149 - L151 were not covered by tests
[warning] 155-160: e2e/e2etests/test_migrate_tss.go#L155-L160
Added lines #L155 - L160 were not covered by tests
[warning] 164-166: e2e/e2etests/test_migrate_tss.go#L164-L166
Added lines #L164 - L166 were not covered by tests
Additional comments not posted (3)
x/crosschain/types/errors.go (2)
53-57: New Error Definition: ErrMigrationFromOldTssThis error code
1156for "migration tx from an old tss address detected" seems appropriate and clearly describes the issue it represents.
58-58: New Error Definition: ErrValidatingInboundThe error code
1157for "unable to validate inbound" is consistent with the module's existing error definitions and clearly communicates the issue.e2e/e2etests/test_migrate_tss.go (1)
23-182: Review of TestMigrateTSS FunctionThis function appears to be well-structured and covers the migration process for both BTC and ETH using TSS. However, there are a few areas that could be improved for better readability and maintainability:
Parameterization of Values: As previously suggested, the hardcoded values such as BTC migration fee and sleep duration should be parameterized. This would make the tests more flexible and easier to maintain.
Error Handling: The function heavily relies on
require.NoErrorfor error handling, which is appropriate for stopping the test upon the first error. However, it might be beneficial to add more descriptive error messages to these assertions to aid in debugging.Coverage: According to the static analysis tools, many new lines in this test are not covered by tests. It's crucial to ensure that all parts of this critical functionality are thoroughly tested to prevent issues in production.
Use of Time.Sleep: The use of
time.Sleepfor waiting on blockchain operations is not ideal as it introduces delays and potential flakiness in tests. It would be better to implement a more robust waiting mechanism that checks for the condition to be met.Magic Numbers: The test uses several "magic numbers" and could benefit from defining these numbers as constants at the beginning of the file, which would improve readability and maintainability.
- btcBalance = btcBalance - 0.01 + const migrationFeeBTC = 0.01 + btcBalance -= migrationFeeBTC - time.Sleep(8 * time.Second) + const tssUpdateWait = 8 * time.Second + time.Sleep(tssUpdateWait)Tools
GitHub Check: codecov/patch
[warning] 23-26: e2e/e2etests/test_migrate_tss.go#L23-L26
Added lines #L23 - L26 were not covered by tests
[warning] 29-35: e2e/e2etests/test_migrate_tss.go#L29-L35
Added lines #L29 - L35 were not covered by tests
[warning] 39-40: e2e/e2etests/test_migrate_tss.go#L39-L40
Added lines #L39 - L40 were not covered by tests
[warning] 42-44: e2e/e2etests/test_migrate_tss.go#L42-L44
Added lines #L42 - L44 were not covered by tests
[warning] 47-47: e2e/e2etests/test_migrate_tss.go#L47
Added line #L47 was not covered by tests
[warning] 49-50: e2e/e2etests/test_migrate_tss.go#L49-L50
Added lines #L49 - L50 were not covered by tests
[warning] 54-61: e2e/e2etests/test_migrate_tss.go#L54-L61
Added lines #L54 - L61 were not covered by tests
[warning] 64-67: e2e/e2etests/test_migrate_tss.go#L64-L67
Added lines #L64 - L67 were not covered by tests
[warning] 71-73: e2e/e2etests/test_migrate_tss.go#L71-L73
Added lines #L71 - L73 were not covered by tests
[warning] 75-77: e2e/e2etests/test_migrate_tss.go#L75-L77
Added lines #L75 - L77 were not covered by tests
[warning] 80-86: e2e/e2etests/test_migrate_tss.go#L80-L86
Added lines #L80 - L86 were not covered by tests
[warning] 89-94: e2e/e2etests/test_migrate_tss.go#L89-L94
Added lines #L89 - L94 were not covered by tests
[warning] 96-97: e2e/e2etests/test_migrate_tss.go#L96-L97
Added lines #L96 - L97 were not covered by tests
[warning] 99-100: e2e/e2etests/test_migrate_tss.go#L99-L100
Added lines #L99 - L100 were not covered by tests
[warning] 103-104: e2e/e2etests/test_migrate_tss.go#L103-L104
Added lines #L103 - L104 were not covered by tests
[warning] 106-106: e2e/e2etests/test_migrate_tss.go#L106
Added line #L106 was not covered by tests
[warning] 109-117: e2e/e2etests/test_migrate_tss.go#L109-L117
Added lines #L109 - L117 were not covered by tests
[warning] 120-120: e2e/e2etests/test_migrate_tss.go#L120
Added line #L120 was not covered by tests
[warning] 122-124: e2e/e2etests/test_migrate_tss.go#L122-L124
Added lines #L122 - L124 were not covered by tests
[warning] 126-127: e2e/e2etests/test_migrate_tss.go#L126-L127
Added lines #L126 - L127 were not covered by tests
[warning] 131-132: e2e/e2etests/test_migrate_tss.go#L131-L132
Added lines #L131 - L132 were not covered by tests
[warning] 134-135: e2e/e2etests/test_migrate_tss.go#L134-L135
Added lines #L134 - L135 were not covered by tests
[warning] 137-138: e2e/e2etests/test_migrate_tss.go#L137-L138
Added lines #L137 - L138 were not covered by tests
[warning] 140-141: e2e/e2etests/test_migrate_tss.go#L140-L141
Added lines #L140 - L141 were not covered by tests
[warning] 143-143: e2e/e2etests/test_migrate_tss.go#L143
Added line #L143 was not covered by tests
[warning] 145-146: e2e/e2etests/test_migrate_tss.go#L145-L146
Added lines #L145 - L146 were not covered by tests
[warning] 149-151: e2e/e2etests/test_migrate_tss.go#L149-L151
Added lines #L149 - L151 were not covered by tests
[warning] 155-160: e2e/e2etests/test_migrate_tss.go#L155-L160
Added lines #L155 - L160 were not covered by tests
[warning] 164-166: e2e/e2etests/test_migrate_tss.go#L164-L166
Added lines #L164 - L166 were not covered by tests
There was a problem hiding this comment.
Actionable comments posted: 3
Outside diff range, codebase verification and nitpick comments (3)
cmd/zetaclientd/keygen_tss.go (3)
26-31: Clarify the documentation forGenerateTSS.The comments are verbose but slightly unclear, especially regarding the conditions under which the function returns immediately or continues to attempt TSS generation.
- // If a TSS was generated successfully in the past,and the keygen was successful, the function will return without doing anything. + // If a TSS was generated successfully in the past and the keygen was successful, the function will return without attempting another generation. - // If a keygen has been set the functions will wait for the correct block to arrive and generate a new TSS. + // If a keygen is pending, the function waits for the specified block to arrive before attempting to generate a new TSS. - // In case of a successful keygen a TSS success vote is broadcasted to zetacore and the newly generate TSS is tested. The generated keyshares are stored in the correct directory + // Upon a successful keygen, a TSS success vote is broadcasted to zetacore, the newly generated TSS is tested, and the keyshares are stored appropriately. - // In case of a failed keygen a TSS failed vote is broadcasted to zetacore. + // If the keygen fails, a TSS failure vote is broadcasted to zetacore.
125-127: Clarify the role ofkeygenTSS.The documentation for
keygenTSScould be enhanced to better explain its role and the conditions under which it operates.- // keygenTSS generates a new TSS using the keygen request and the TSS server. - // If the keygen is successful, the function returns the new TSS pubkey. - // If the keygen is unsuccessful, the function posts blame and returns an error. + // keygenTSS attempts to generate a new TSS using the provided keygen request and TSS server. + // It returns the new TSS pubkey on success or an error on failure, including posting blame data if applicable.
175-176: Enhance the documentation forTestTSS.The function
TestTSScould use more detailed documentation to explain its purpose and the conditions under which it operates.+ // TestTSS attempts to sign a sample message using the TSS key to verify its correctness. It returns an error if the test fails.
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (7)
- Makefile (2 hunks)
- changelog.md (1 hunks)
- cmd/zetaclientd-supervisor/lib.go (5 hunks)
- cmd/zetaclientd/keygen_tss.go (5 hunks)
- cmd/zetae2e/local/local.go (12 hunks)
- x/crosschain/keeper/cctx_orchestrator_validate_inbound.go (3 hunks)
- x/crosschain/keeper/cctx_orchestrator_validate_inbound_test.go (1 hunks)
Files skipped from review as they are similar to previous changes (6)
- Makefile
- changelog.md
- cmd/zetaclientd-supervisor/lib.go
- cmd/zetae2e/local/local.go
- x/crosschain/keeper/cctx_orchestrator_validate_inbound.go
- x/crosschain/keeper/cctx_orchestrator_validate_inbound_test.go
Additional context used
Path-based instructions (1)
cmd/zetaclientd/keygen_tss.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- .github/workflows/e2e.yml (6 hunks)
Additional comments not posted (4)
.github/workflows/e2e.yml (4)
46-49: Addition oftss-migration-testto workflow inputs.The addition of
tss-migration-testas a boolean input parameter is correctly implemented. It is optional and defaults to false, which aligns with typical usage patterns for feature toggles in CI configurations.
68-69: Addition ofTSS_MIGRATION_TESTSto job outputs.The output parameter
TSS_MIGRATION_TESTSis correctly set up to capture the output from thematrix-conditionalssteps. This is essential for conditional execution of jobs based on the presence of specific labels.
92-92: Conditional setting ofTSS_MIGRATION_TESTSbased on context.The script correctly sets the
TSS_MIGRATION_TESTSoutput based on the presence of theTSS_MIGRATION_TESTSlabel in pull requests and the corresponding input in workflow dispatch events. This ensures that the TSS migration tests are only run when explicitly requested, which is a good practice for managing test execution in CI environments.Also applies to: 121-121
151-153: Integration ofTSS_MIGRATION_TESTSinto the job matrix.The new test type
start-tss-migration-testis correctly added to the matrix strategy and will execute based on theTSS_MIGRATION_TESTScondition. This change is crucial for enabling the new end-to-end tests as part of the CI pipeline, ensuring that TSS migration functionality is tested automatically.
Co-authored-by: Francisco de Borja Aranda Castillejo <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- .github/workflows/e2e.yml (6 hunks)
Files skipped from review as they are similar to previous changes (1)
- .github/workflows/e2e.yml
* add new runner for migration tests * skip adding new keygen * skip adding new keygen * modify btc runner * tss fund migrate test for eth * add queries for tss funds migrator info * add changelog * fix lint messages * refactor keygen * refactor keygen * rearrange keygen creations * rearrange keygen creations * modify accounting * rebase develop * add retest after migration * add checks for error messages * add unit test for CheckMigration * fix unit tests for update msgUpdateTSS * fix some lint issues and rename all runners to r * fix some lint issues and rename all runners to r * add changelog * fix lint errors * fix lint errors * removed debug logs * add comments for zetasupervisor lib * add new user for migration tests * add new user for migration tests * change name to start-tss-migration-test * add changes for comments 1 * add changes for comments 1 * rename to CheckIfMigrationDeposit * rename to CheckIfMigrationDeposit * use cctx_Gateway to check cctx * add Tss migration to e2e.yml * move to a sub function * resolve comments 2 * resolve comments 3 * add todo * resolve comments 4 * Update .github/workflows/e2e.yml Co-authored-by: Francisco de Borja Aranda Castillejo <[email protected]> --------- Co-authored-by: Francisco de Borja Aranda Castillejo <[email protected]>
Description
This PR add a e2e test for TSS migration
Closes
#2370
#2441
#2442
How Has This Been Tested?
Summary by CodeRabbit
New Features
Bug Fixes
Tests