test: add wait for block to tss migration test #2931
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kingpinXD
requested review from
brewmaster012,
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📝 Walkthrough📝 Walkthrough📝 WalkthroughWalkthroughThe pull request introduces enhancements to the end-to-end testing framework by adding functionality to wait for TSS (Threshold Signature Scheme) generation before executing migration logic. Specifically, it modifies the Changes
Possibly related PRs
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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (2)
e2e/e2etests/test_migrate_tss.go (1)
51-51: Approve: Addition of TSS generation wait improves test reliability.The introduction of
r.WaitForTssGeneration(2)before the Bitcoin fund migration is a prudent enhancement. It aligns with the PR objective to improve the success rate of the TSS-migration CI test by ensuring TSS generation is complete before proceeding.To improve code clarity, consider adding a brief comment explaining the purpose of the wait and the significance of the argument '2'. For example:
// Wait for TSS generation to complete (max 2 attempts) before migrating funds r.WaitForTssGeneration(2)e2e/runner/zeta.go (1)
44-45: Define backoff parameters as constantsFor better configurability and readability, consider defining the backoff duration and maximum retries as constants or configurable variables.
Apply this diff:
+ const backoffInterval = time.Second * 5 + const maxRetries = 10 - bo := backoff.NewConstantBackOff(time.Second * 5) - boWithMaxRetries := backoff.WithMaxRetries(bo, 10) + bo := backoff.NewConstantBackOff(backoffInterval) + boWithMaxRetries := backoff.WithMaxRetries(bo, maxRetries)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
- e2e/e2etests/test_migrate_tss.go (1 hunks)
- e2e/runner/zeta.go (3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
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.e2e/runner/zeta.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
🔇 Additional comments (1)
e2e/runner/zeta.go (1)
12-12: ImportobservertypespackageThe import of
observertypesis necessary for accessing TSS history and is appropriately added.
swift1337
approved these changes
Sep 27, 2024
lumtis
requested changes
Sep 27, 2024
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #2931 +/- ##
========================================
Coverage 66.39% 66.39%
========================================
Files 389 389
Lines 21758 21758
========================================
Hits 14447 14447
Misses 6584 6584
Partials 727 727 |
lumtis
reviewed
Sep 30, 2024
lumtis
approved these changes
Sep 30, 2024
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
morde08
pushed a commit
that referenced
this pull request
Oct 2, 2024
* add wait for block to tss migration test * add comments * refactor identifiers * rename checkNumberOfTssGenerated to checkNumberOfTSSGenerated
github-merge-queue bot
pushed a commit
that referenced
this pull request
Oct 3, 2024
…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 <[email protected]> * fix: changed coin denom --------- Co-authored-by: skosito <[email protected]> Co-authored-by: Lucas Bertrand <[email protected]> * 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 <[email protected]> * 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 <[email protected]> * use const for message --------- Co-authored-by: Francisco de Borja Aranda Castillejo <[email protected]> * 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 <[email protected]> Co-authored-by: Francisco de Borja Aranda Castillejo <[email protected]> Co-authored-by: Lucas Bertrand <[email protected]> Co-authored-by: Alex Gartner <[email protected]> Co-authored-by: jkan2 <[email protected]> Co-authored-by: jkan2 <[email protected]> Co-authored-by: Tanmay <[email protected]>
5 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
The change might improve the success frequency of the TSS-migration-test on the CI .
The pr adds a loop to make sure the new TSS is set before trying to migrate funds.
How Has This Been Tested?
Summary by CodeRabbit
New Features
Bug Fixes