Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Release/v1.6.3 #1713

Merged
merged 3 commits into from
Aug 16, 2024
Merged

Release/v1.6.3 #1713

merged 3 commits into from
Aug 16, 2024

Conversation

joe-bowman
Copy link
Contributor

@joe-bowman joe-bowman commented Aug 15, 2024

1. Summary

Fixes #1707

2.Type of change

Upgrade handler, that deletes and refunds failed unbonding ea0d86a3fb4b25fcb13a587e72542f99ebf8c7c3aa255a0922dfa7002a8ee861 back to the user that initiated the unbonding.

3. Implementation details

  • The upgrade handler simply moves the BurnAmount (escrowed qatoms) from the escrow account, back to the users' account.
  • The WithdrawalRecord is deleted.
  • Tests to demonstrate correctness.
  • Delete v1.5 upgrade handlers.

Summary by CodeRabbit

  • New Features

    • Introduced a new upgrade handler for version "v1.6.3" to manage specific upgrade logic and handle escrowed funds.
    • Updated the upgrade management strategy by adding a new entry for version "v1.6.3" and removing several outdated entries.
  • Tests

    • Streamlined the testing framework by removing outdated test cases and adding new tests for the latest upgrade handler.

Copy link

vercel bot commented Aug 15, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
quicksilver ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 15, 2024 7:53pm

Copy link
Contributor

coderabbitai bot commented Aug 15, 2024

Walkthrough

The recent changes introduce a new upgrade handler for version 1.6.3, enhancing the management of failed unbonding transactions within the Quicksilver application. The modifications streamline the upgrade process by removing outdated entries, refining testing strategies, and ensuring funds are returned correctly to users. This reflects an ongoing commitment to improving user experience and system stability.

Changes

File Path Change Summary
app/upgrades/types.go Added new constant V010603UpgradeName for version "v1.6.3".
app/upgrades/upgrades.go Updated Upgrades function by removing several older upgrade entries and adding V010603UpgradeHandler.
app/upgrades/v1_6.go Introduced V010603UpgradeHandler to manage escrowed funds for failed unbonding transactions.
app/upgrades_test.go Removed outdated upgrade handler tests and added TestV010603UpgradeHandler to verify new upgrade logic.

Assessment against linked issues

Objective Addressed Explanation
Add upgrade handler for v1.6.3 (#1707)
Cancel withdrawal for specific hash (#1707)
Negate need for upgrade handlers in future (#1703) Unclear if future deprecation of handlers is addressed.

🐰 "In a world where upgrades align,
The bunnies hop, their joy divine.
With funds returned and paths made clear,
We celebrate changes, give a cheer!
Hopping forward, no steps behind,
A brighter future for all, we find!" 🐇✨


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?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

app/upgrades/v1_6.go Dismissed Show dismissed Hide dismissed
app/upgrades_test.go Dismissed Show dismissed Hide dismissed
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Outside diff range, codebase verification and nitpick comments (1)
app/upgrades_test.go (1)

420-421: Format the code according to gofumpt standards.

Ensure that the file is formatted according to gofumpt standards to maintain consistency and readability.

gofumpt -w app/upgrades_test.go
Tools
golangci-lint

421-421: File is not gofumpt-ed

(gofumpt)

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between efda503 and ba6d841.

Files selected for processing (4)
  • app/upgrades/types.go (1 hunks)
  • app/upgrades/upgrades.go (2 hunks)
  • app/upgrades/v1_6.go (2 hunks)
  • app/upgrades_test.go (4 hunks)
Files skipped from review due to trivial changes (1)
  • app/upgrades/types.go
Additional context used
GitHub Check: devskim
app/upgrades/v1_6.go

[failure] 285-285: A token or key was found in source code. If this represents a secret, it should be moved somewhere else.
Do not store tokens or keys in source code.

app/upgrades_test.go

[failure] 393-393: A token or key was found in source code. If this represents a secret, it should be moved somewhere else.
Do not store tokens or keys in source code.

golangci-lint
app/upgrades_test.go

421-421: File is not gofumpt-ed

(gofumpt)

Additional comments not posted (2)
app/upgrades/upgrades.go (1)

25-25: Verify the integration of the new upgrade entry.

Ensure that V010603UpgradeHandler is correctly defined and integrated within the application. The pattern used for this entry should match the existing ones to maintain consistency.

Verification successful

Integration of V010603UpgradeHandler is verified.

The V010603UpgradeHandler function is correctly defined and integrated. It follows the expected pattern used for other upgrade handlers, ensuring consistency and proper functionality within the application.

  • The function is located in app/upgrades/v1_6.go.
  • It includes logic for returning escrowed funds and deleting records, as expected for an upgrade handler.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the definition and integration of V010603UpgradeHandler.

# Test: Search for the function definition. Expect: The function should be defined and correctly implemented.
ast-grep --lang go --pattern $'func V010603UpgradeHandler($_, $_, $_) $_ { $$$ }'

Length of output: 2601

app/upgrades_test.go (1)

393-393: Avoid hardcoding sensitive data in tests.

The transaction hash appears to be hardcoded. If it represents sensitive data, consider moving it to a configuration file or using a mock value.

Tools
GitHub Check: devskim

[failure] 393-393: A token or key was found in source code. If this represents a secret, it should be moved somewhere else.
Do not store tokens or keys in source code.

app/upgrades/v1_6.go Show resolved Hide resolved
app/upgrades/v1_6.go Show resolved Hide resolved
Copy link

codecov bot commented Aug 15, 2024

Codecov Report

Attention: Patch coverage is 77.77778% with 4 lines in your changes missing coverage. Please review.

Project coverage is 63.63%. Comparing base (efda503) to head (f9c3970).
Report is 1 commits behind head on main.

Files Patch % Lines
app/upgrades/v1_6.go 76.47% 2 Missing and 2 partials ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1713      +/-   ##
==========================================
- Coverage   64.04%   63.63%   -0.42%     
==========================================
  Files         196      194       -2     
  Lines       13961    13371     -590     
==========================================
- Hits         8942     8508     -434     
+ Misses       4164     4049     -115     
+ Partials      855      814      -41     
Flag Coverage Δ
unittests 63.63% <77.77%> (-0.42%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
app/upgrades/types.go 50.00% <ø> (ø)
app/upgrades/upgrades.go 86.66% <100.00%> (-4.25%) ⬇️
app/upgrades/v1_6.go 57.64% <76.47%> (+2.09%) ⬆️

faddat
faddat previously approved these changes Aug 15, 2024
@faddat
Copy link
Contributor

faddat commented Aug 15, 2024

I'm just going to lint this real quick

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Outside diff range, codebase verification and nitpick comments (1)
app/upgrades/account_migrations.go (1)

Line range hint 1-11: Consider documenting the new migration strategy.

The removal of migration functions suggests a change in how account migrations are handled. Ensure that the new strategy is well-documented to facilitate understanding and maintenance.

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between ba6d841 and f9c3970.

Files ignored due to path filters (1)
  • go.work.sum is excluded by !**/*.sum
Files selected for processing (2)
  • app/upgrades/account_migrations.go (1 hunks)
  • app/upgrades_test.go (4 hunks)
Additional context used
GitHub Check: devskim
app/upgrades_test.go

[failure] 393-393: A token or key was found in source code. If this represents a secret, it should be moved somewhere else.
Do not store tokens or keys in source code.

Additional comments not posted (1)
app/upgrades_test.go (1)

383-420: The new test function TestV010603UpgradeHandler looks good.

The test effectively verifies the upgrade handler's functionality, ensuring withdrawal records are managed correctly and balances are updated.

Tools
GitHub Check: devskim

[failure] 393-393: A token or key was found in source code. If this represents a secret, it should be moved somewhere else.
Do not store tokens or keys in source code.

app/upgrades_test.go Show resolved Hide resolved
@joe-bowman joe-bowman merged commit 965a6c8 into main Aug 16, 2024
21 of 22 checks passed
@joe-bowman joe-bowman deleted the release/v1.6.x branch October 24, 2024 19:57
@joe-bowman joe-bowman restored the release/v1.6.x branch October 24, 2024 19:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add upgrade handler for v1.6.3
2 participants