-
Notifications
You must be signed in to change notification settings - Fork 607
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
fix: run proto disto on epoch #8106
Conversation
WalkthroughThe recent updates primarily focus on the ProtoRev module within the Osmosis blockchain framework, enhancing the distribution of profits at epoch ends. Key changes include the implementation of profit calculations, adjustments in the handling of epoch hooks, and the restructuring of test cases to accommodate these modifications. Changes
Assessment against linked issues
This whimsical celebration by a rabbit marks the successful enhancement of the ProtoRev module, ensuring timely and accurate profit distributions with each epoch, aligning perfectly with the cosmos of blockchain innovations. Recent Review DetailsConfiguration used: CodeRabbit UI Files selected for processing (1)
Files skipped from review as they are similar to previous changes (1)
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Debugged through the epoch to ensure this code runs as expected:
{"level":"info","module":"server","time":"2024-04-19T10:43:55Z","message":"distributeInternal CL for pool id 1671 finished"}
{"level":"info","module":"server","time":"2024-04-19T10:43:55Z","message":"x/incentives AfterEpochEnd finished distribution"}
{"level":"info","module":"server","mint":"mintedCoins","time":"2024-04-19T10:43:55Z","message":"AfterEpochEnd, minted coins"}
{"level":"info","module":"server","module":"mint","recepientModule":"fee_collector","distributionCoin":"91324200913uosmo","height":15040473,"time":"2024-04-19T10:43:55Z","message":"distributeToModule"}
{"level":"info","module":"server","module":"mint","recepientModule":"poolincentives","distributionCoin":"36529680365uosmo","height":15040473,"time":"2024-04-19T10:43:55Z","message":"distributeToModule"}
{"level":"info","module":"server","module":"poolincentives","totalMintedAmount":"36529680369","height":15040473,"time":"2024-04-19T10:43:55Z","message":"AllocateAsset minted amount"}
> github.com/osmosis-labs/osmosis/v24/x/protorev/keeper.EpochHooks.AfterEpochEnd() /root/github.com/osmosis-labs/osmosis/x/protorev/keeper/epoch_hook.go:43 (hits goroutine(52):1 total:1) (PC: 0x37f99b2)
38: func (h EpochHooks) AfterEpochEnd(ctx sdk.Context, epochIdentifier string, epochNumber int64) error {
39: if h.k.GetProtoRevEnabled(ctx) {
40: switch epochIdentifier {
41: case "day":
42: // Calculate and distribute protorev profits
=> 43: err := h.CalculateDistributeProfits(ctx)
44: if err != nil {
45: return err
46: }
47:
48: // Increment number of days since module genesis to properly calculate developer fees after cyclic arbitrage trades
There was a problem hiding this 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
Actionable comments outside the diff hunks (8)
CHANGELOG.md (8)
Line range hint
325-325
: Correct the punctuation in the heading.-## v6.4.0 +## v6.4.0.
Line range hint
588-588
: Replace the bare URL with a markdown link.-https://github.com/osmosis-labs/osmosis/releases/tag/v6.4.0 +[v6.4.0](https://github.com/osmosis-labs/osmosis/releases/tag/v6.4.0)
Line range hint
646-646
: Replace the bare URL with a markdown link.-https://github.com/osmosis-labs/osmosis/releases/tag/v6.3.1 +[v6.3.1](https://github.com/osmosis-labs/osmosis/releases/tag/v6.3.1)
Line range hint
986-986
: Ensure heading levels increment by one level at a time.-#### Golang API breaks +### Golang API breaks
Line range hint
998-998
: Ensure heading levels increment by one level at a time.-#### Bug Fixes +### Bug Fixes
Line range hint
1007-1007
: Ensure heading levels increment by one level at a time.-#### Features +### Features
Line range hint
1012-1012
: Ensure heading levels increment by one level at a time.-#### SDK fork updates +### SDK fork updates
Line range hint
1064-1064
: Ensure heading levels increment by one level at a time.-#### IAVL fork updates +### IAVL fork updates
Closes: #8089
Change from this PR: #8054
What is the purpose of the change
Needed to redo the PR from osmosis main as e2e won't work rn for external contributors
Fixes the registration of epoch hooks in protorev.
EpochHooks are set for protorev by the
EpochHooks()
method. Those are created as anEpochHooks
item, and not directly as part of the keeper.In #7564 we added hooks directly to the keeper which were never registered with the epoch hooks. Instead of having these live in separate places I've just combined the newly added logic into the existing AfterEpoch hook.
All logic remains the same.
Testing and Verifying
Existing unit test coverage still applies to all changes--I've simply copy/pasted some of those to organize all epoch-specific logic into one place within protorev.
Not sure if you have e2e tests somewhere that I can update to test protorev distributions?
Documentation and Release Note
Unreleased
section ofCHANGELOG.md
?Where is the change documented?
x/{module}/README.md
)Summary by CodeRabbit