osmosis-labs / osmosis

The AMM Laboratory
https://app.osmosis.zone
Apache License 2.0
875 stars 563 forks source link

chore: set optimistic execution #8393

Open czarcas7ic opened 3 weeks ago

czarcas7ic commented 3 weeks ago

Closes: #XXX

What is the purpose of the change

As requested by @ValarDragon

coderabbitai[bot] commented 3 weeks ago

Walkthrough

The latest update introduces several significant enhancements to the osmosis platform, including support for superfluid staking of non-pool assets, unidirectional trading pair fee overrides, and fixing fee charging issues. Additionally, upgrades to the SDK v50 and Comet v0.38 have been incorporated, along with a new governance parameter for setting the minimum deposit ratio. Optimistic execution has also been enabled to improve transaction processing efficiency.

Changes

File/Path Change Summary
CMD changes Added SetOptimisticExecution() method to baseapp in newApp function
osmosis core Introduced support for non-pool assets in superfluid staking, uni-directional trading pair taker fee overrides, and enabled optimistic execution
SDK and Comet Integration Upgraded to SDK v50 and Comet v0.38
Governance Parameters Set MinDepositRatio governance parameter to 1% for SDK v50 upgrade
Fee Charging Fix Addressed fee payer charge on every message in the smartaccount module

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant OsmosisApp
    participant Governance
    participant TradingModule
    participant BaseApp

    User->>+OsmosisApp: Initiate Superfluid Staking
    OsmosisApp->>TradingModule: Validate Non-Pool Asset
    TradingModule-->>OsmosisApp: Non-Pool Asset Validated

    User->>+OsmosisApp: Initiate Trade
    OsmosisApp->>TradingModule: Check Taker Fee Override
    TradingModule-->>OsmosisApp: Taker Fee Applied

    User->>+OsmosisApp: Submit Governance Proposal
    OsmosisApp->>Governance: Set MinDepositRatio to 1%
    Governance-->>OsmosisApp: MinDepositRatio Set

    User->>+OsmosisApp: Execute Transaction
    OsmosisApp->>BaseApp: Execute with Optimism
    BaseApp-->>OsmosisApp: Optimistic Execution Completed

Poem

In the world of Osmosis, great changes take flight,
With assets non-pool staking in superfluid delight,
Fees adjusted, trading pairs made fair,
SDK and Comet upgrades, a future to share.
Optimistic execution speeds our quest,
In governance, deposit ratios set to the best.
Oh, the marvels of code, in the blockchain's nest! 🌌🐇


Recent review details **Configuration used: CodeRabbit UI** **Review profile: CHILL**
Commits Files that changed from the base of the PR and between 3b71637d3ea40787b84e9b932208a88706faf4ac and 671013fddb419f83abfe6cc2c20f1f3b9706169b.
Files selected for processing (2) * CHANGELOG.md (1 hunks) * cmd/osmosisd/cmd/root.go (1 hunks)
Additional context used
Markdownlint
CHANGELOG.md
1074-1074: Expected: h3; Actual: h4 (MD001, heading-increment) Heading levels should only increment by one level at a time --- 1086-1086: Expected: h3; Actual: h4 (MD001, heading-increment) Heading levels should only increment by one level at a time --- 1095-1095: Expected: h3; Actual: h4 (MD001, heading-increment) Heading levels should only increment by one level at a time --- 1100-1100: Expected: h3; Actual: h4 (MD001, heading-increment) Heading levels should only increment by one level at a time --- 1152-1152: Expected: h3; Actual: h4 (MD001, heading-increment) Heading levels should only increment by one level at a time --- 413-413: Punctuation: '.' (MD026, no-trailing-punctuation) Trailing punctuation in heading --- 676-676: null (MD034, no-bare-urls) Bare URL used --- 734-734: null (MD034, no-bare-urls) Bare URL used --- 1315-1315: null (MD042, no-empty-links) No empty links
Additional comments not posted (4)
cmd/osmosisd/cmd/root.go (1)
`932-932`: The integration of `SetOptimisticExecution()` in the `newApp` function is correctly placed to ensure that the base application runs with optimistic execution enabled. This setting is crucial for certain consensus optimizations and should be tested thoroughly to ensure it does not introduce any unintended side effects in transaction processing or state management.
CHANGELOG.md (3)
`47-52`: The changelog entries for state-breaking changes are well-documented, providing clear references to the associated pull requests and issue numbers. This is crucial for tracking changes and understanding their impact on the system. --- `52-52`: It's good to see the changelog entry for enabling optimistic execution, which aligns with the PR objectives. This entry helps in tracking the implementation and its effects on the system. --- `47-52`: The use of multiple issue references for a single changelog entry is effective in providing a comprehensive view of the changes. However, ensure that all referenced issues are directly related to the noted change to avoid confusion.
Verification successful --- The referenced issues in the changelog entry are directly related to the noted change about supporting non-pool assets in superfluid staking. - Issue #8169: Supports non-pool assets in superfluid staking. - Issue #8250: Adds risk factors to individual denoms for superfluid staking. - Issue #8276: Uses full routes instead of only one pool for pricing in superfluid staking. - Issue #8320: Updates queries to take non-pool assets into account for superfluid staking.
Scripts executed The following scripts were executed for the analysis: --- Script: ```shell #!/bin/bash # Verify that all referenced issues in changelog entry are related to the noted change. echo "Checking relevance of issues #8169, #8250, #8276, #8320 to superfluid staking support..." # Example command; adjust as necessary for actual validation. gh issue view 8169 --repo osmosis-labs/osmosis gh issue view 8250 --repo osmosis-labs/osmosis gh issue view 8276 --repo osmosis-labs/osmosis gh issue view 8320 --repo osmosis-labs/osmosis ``` Length of output: 5613
--- 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 - [X](https://twitter.com/intent/tweet?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A&url=https%3A//coderabbit.ai) - [Mastodon](https://mastodon.social/share?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A%20https%3A%2F%2Fcoderabbit.ai) - [Reddit](https://www.reddit.com/submit?title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&text=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code.%20Check%20it%20out%3A%20https%3A//coderabbit.ai) - [LinkedIn](https://www.linkedin.com/sharing/share-offsite/?url=https%3A%2F%2Fcoderabbit.ai&mini=true&title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&summary=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code)
Tips ### Chat There are 3 ways to chat with [CodeRabbit](https://coderabbit.ai): - Review comments: Directly reply to a review comment made by CodeRabbit. Example: - `I pushed a fix in commit .` - `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 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](https://docs.coderabbit.ai/guides/configure-coderabbit) 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](https://coderabbit.ai/docs) for detailed information on how to use CodeRabbit. - Join our [Discord Community](https://discord.com/invite/GsXnASn26c) to get help, request features, and share feedback. - Follow us on [X/Twitter](https://twitter.com/coderabbitai) for updates and announcements.
ValarDragon commented 2 weeks ago

Actually since this is state compat, maybe we should get v26.0.0 without this, and v26.1.0 with it?

czarcas7ic commented 2 weeks ago

@ValarDragon I was told by Facu from Binary Builders that the feat does nothing without 2/3 voting power on it.

We can add in minor but I don't think there is a high likelyhood of getting voting power on it without a manual push

(msg here https://osmosis-network.slack.com/archives/C047X1T6ERK/p1718615619678329?thread_ts=1718492283.495959&cid=C047X1T6ERK)

czarcas7ic commented 1 week ago

Lets keep this open until v26.0.0 is out and then merge, as per Dev's request.

github-actions[bot] commented 4 days ago

This pull request has been automatically marked as stale because it has not had any recent activity. It will be closed if no further activity occurs. Thank you!