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

refactor: remove unused oracle addresses #125

Merged
merged 3 commits into from
May 10, 2024
Merged

Conversation

manu0466
Copy link
Contributor

@manu0466 manu0466 commented May 9, 2024

Overview

This PR removes all references to the old oracles from the messages and the contract config.
Additionally, to properly remove the old oracle addresses from the config, the migration logic has been updated and
the MsgMigrate has been converted to an enum to support future migration logics.

To migrate the contract from the version v0.4.18 the following migrate message must be sent:

{
  "v0_4_18_to_v0_4_19": {}
}

closes: MILKTIA-32

What changes have been made in this PR?

  • Removed all the references to the old oracles
  • Added a migration logic to remove the old oracles address from the state of the contract
  • Refactored the migration logic to support future migrations

Checklist


  • Appropriate labels applied
  • Targeted PR against correct branch
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Wrote unit and integration
  • Updated relevant documentation

@manu0466 manu0466 self-assigned this May 9, 2024
Copy link
Contributor

coderabbitai bot commented May 9, 2024

Walkthrough

The update primarily involves a version increment and refactoring related to oracle integration across various components of a staking contract. Key changes include the removal of redundant oracle address fields, simplification of function parameters, and introduction of a new migration module to handle transitions between contract versions. The restructuring enhances modularity and clarity, focusing on streamlined configurations and error handling.

Changes

Files Change Summary
Cargo.toml Version updated from 0.4.18 to 0.4.19.
contracts/staking/src/contract.rs, .../execute.rs, .../lib.rs, .../migrations/mod.rs, .../migrations/v0_4_19.rs Introduced migrations module, updated function signatures, and removed specific oracle address fields. Added new migration logic.
contracts/staking/src/error.rs, .../msg.rs, .../state.rs Enhanced error handling with VersionError. Streamlined message structs by consolidating oracle address fields. Adjusted Config struct.
contracts/staking/src/tests/instantiate_tests.rs, .../tests/test_helper.rs Simplified test setups by removing unused oracle address declarations.

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.

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 a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

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

CodeRabbit Configration 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.

@@ -2,7 +2,7 @@
members = ["contracts/*", "packages/*"]

[workspace.package]
version = "0.4.18"
version = "0.4.19"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have bumped this just to have an higher version and test everithing. Let me know if I should change it.

@manu0466
Copy link
Contributor Author

manu0466 commented May 9, 2024

Here some screenshots of the contract state before and after the migration.

Contract state on version v0.4.18

0-4-18-version
old-contract-state

Contract state on version v0.4.19

states-post-migration

@manu0466 manu0466 marked this pull request as ready for review May 9, 2024 19:20
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: 0

Out of diff range and nitpick comments (1)
contracts/staking/src/execute.rs (1)

Line range hint 94-117: Modified function update_oracle_msgs to remove old oracle addresses and use new oracle_address.

-            contract: config.oracle_contract_address.clone().unwrap().to_string(),
+            contract: config.oracle_address.clone().unwrap().to_string(),

Ensure that the new oracle_address is correctly used and initialized elsewhere in the codebase to prevent runtime errors.

Review Details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits Files that changed from the base of the PR and between b0cc7ba and c926911.
Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
Files selected for processing (11)
  • Cargo.toml (1 hunks)
  • contracts/staking/src/contract.rs (9 hunks)
  • contracts/staking/src/error.rs (2 hunks)
  • contracts/staking/src/execute.rs (8 hunks)
  • contracts/staking/src/lib.rs (1 hunks)
  • contracts/staking/src/migrations/mod.rs (1 hunks)
  • contracts/staking/src/migrations/v0_4_19.rs (1 hunks)
  • contracts/staking/src/msg.rs (3 hunks)
  • contracts/staking/src/state.rs (2 hunks)
  • contracts/staking/src/tests/instantiate_tests.rs (5 hunks)
  • contracts/staking/src/tests/test_helper.rs (1 hunks)
Files skipped from review due to trivial changes (5)
  • Cargo.toml
  • contracts/staking/src/lib.rs
  • contracts/staking/src/migrations/mod.rs
  • contracts/staking/src/tests/instantiate_tests.rs
  • contracts/staking/src/tests/test_helper.rs
Additional comments not posted (10)
contracts/staking/src/migrations/v0_4_19.rs (2)

11-39: The definition of the v0_4_18_state module with the deprecated fields clearly marked and the use of Item for CONFIG is appropriate and follows CosmWasm conventions.


41-69: The migrate function correctly asserts the contract version, transforms the configuration from the old to the new format, and constructs a response that is informative of the migration action and versions involved.

contracts/staking/src/error.rs (1)

136-137: The addition of the Version error type is appropriate and aligns with the new migration logic that involves version checks. Using VersionError from cw2 ensures standardized error handling.

contracts/staking/src/state.rs (1)

Line range hint 1-14: The modifications to the Config struct, including the removal of deprecated oracle addresses and the adjustment of field order, align with the PR's objectives to simplify and update the contract's configuration.

contracts/staking/src/msg.rs (1)

195-197: The transformation of MigrateMsg from a struct to an enum to handle different migration scenarios, starting with V0_4_18ToV0_4_19, is a significant improvement that enhances the flexibility and clarity of migration handling.

contracts/staking/src/contract.rs (1)

38-39: Changing the visibility of CONTRACT_NAME and CONTRACT_VERSION to public is appropriate as it allows for consistent version handling across different modules of the contract.

contracts/staking/src/execute.rs (4)

7-7: Import of Oracle is added.

This change aligns with the PR's objective to refactor oracle usage.


15-16: Updated imports to include SubMsg, SubMsgResponse, and SubMsgResult.

These changes are necessary for the new sub-message handling logic introduced in other parts of the code.


Line range hint 94-117: Refactored update_oracle_msgs to use the new oracle_address from the config.

This change is consistent with the PR's objective to streamline oracle interactions and remove outdated addresses.


459-459: Added a call to add_messages to concatenate messages and oracle updates in execute_withdraw.

This change ensures that all necessary messages are sent together, improving the function's efficiency and clarity.

@@ -9,7 +9,6 @@ pub struct Config {
pub native_token_denom: String,
pub liquid_stake_token_denom: String,
pub treasury_address: Addr,
pub operators: Option<Vec<Addr>>, //depr
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have removed also this since was marked as deprecated

Copy link
Contributor

@rustcandy rustcandy left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@RiccardoM RiccardoM left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@RiccardoM RiccardoM merged commit c3b00e4 into main May 10, 2024
1 check passed
@RiccardoM RiccardoM deleted the manuel/remove-unused-fields branch May 10, 2024 14:20
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.

3 participants