fix: add non-nil confirmation params in solana/ton E2E test to avoid chain param update error #3501
Conversation
📝 WalkthroughWalkthroughThis pull request updates the changelog and test runner configurations for Solana and TON. A new “Fixes” section is added to the changelog to document an E2E test failure caused by nil ConfirmationParams, and an existing entry has been reformatted for consistency. Additionally, both the Solana and TON test runners now include a new ConfirmationParams field in their ChainParams structure to configure confirmation counts for inbound and outbound transactions. The changes are limited to documentation and configuration enhancements without impacting existing control flow. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches
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
|
ws4charlie
added
UPGRADE_LIGHT_TESTS
![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
🧹 Nitpick comments (2)
e2e/runner/setup_ton.go (1)
100-100: Consider removing legacy ConfirmationCount field.The ConfirmationCount field is planned to be deprecated in favor of the new confirmation count system using SafeInboundCount and SafeOutboundCount fields.
e2e/runner/setup_solana.go (1)
109-109: Consider removing legacy ConfirmationCount field.The ConfirmationCount field is planned to be deprecated in favor of the new confirmation count system using SafeInboundCount and SafeOutboundCount fields.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
changelog.md(1 hunks)e2e/runner/setup_solana.go(1 hunks)e2e/runner/setup_ton.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.go`: Review the Go code, point out issues relative to ...
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
e2e/runner/setup_ton.goe2e/runner/setup_solana.go
📓 Learnings (2)
e2e/runner/setup_ton.go (1)
Learnt from: ws4charlie
PR: zeta-chain/node#3461
File: x/observer/types/chain_params.go:59-62
Timestamp: 2025-02-04T06:12:41.760Z
Learning: The legacy `ConfirmationCount` field in the ChainParams struct is planned to be deprecated after a full upgrade to the new confirmation count system that uses SafeInboundCount, FastInboundCount, SafeOutboundCount, and FastOutboundCount fields.
e2e/runner/setup_solana.go (1)
Learnt from: ws4charlie
PR: zeta-chain/node#3461
File: x/observer/types/chain_params.go:59-62
Timestamp: 2025-02-04T06:12:41.760Z
Learning: The legacy `ConfirmationCount` field in the ChainParams struct is planned to be deprecated after a full upgrade to the new confirmation count system that uses SafeInboundCount, FastInboundCount, SafeOutboundCount, and FastOutboundCount fields.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: start-e2e-performance-test / e2e
🔇 Additional comments (3)
e2e/runner/setup_ton.go (1)
113-116: LGTM! Addition of ConfirmationParams.The addition of non-nil ConfirmationParams with both SafeInboundCount and SafeOutboundCount set to 1 fixes the E2E test failure.
e2e/runner/setup_solana.go (1)
123-126: LGTM! Addition of ConfirmationParams.The addition of non-nil ConfirmationParams with both SafeInboundCount and SafeOutboundCount set to 32 fixes the E2E test failure.
changelog.md (1)
14-16: LGTM! Clear and well-formatted changelog entry.The changelog entry accurately describes the fix for the E2E test failure caused by nil ConfirmationParams.
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
Description
The Solana and TON E2E tests are failing due to nil
ConfirmationParamsfield when updating chain params.How Has This Been Tested?
Summary by CodeRabbit