Skip to content

chore: Epoch cleanup#106

Open
iverc wants to merge 1 commit intodevfrom
iverc/epoch-cleanup
Open

chore: Epoch cleanup#106
iverc wants to merge 1 commit intodevfrom
iverc/epoch-cleanup

Conversation

@iverc
Copy link
Copy Markdown
Contributor

@iverc iverc commented Apr 2, 2026

Description

This PR adds minor fixes and cleans up the epoch module.

@iverc iverc requested a review from Lodek April 2, 2026 06:05
@iverc iverc self-assigned this Apr 2, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 2, 2026

📝 Walkthrough

Walkthrough

The epochs module's ABCI BeginBlocker is refactored: the keeper's method signature now returns an error with pointer receiver, the module-level BeginBlocker delegates directly to the keeper, and cached context utility functions are removed from the osmoutils package.

Changes

Cohort / File(s) Summary
Keeper BeginBlocker Enhancement
x/epochs/keeper/abci.go
Updated method signature to use pointer receiver (*Keeper) and return error instead of void. Function body preserved; import ordering adjusted.
Module BeginBlocker Delegation
x/epochs/module/abci.go
Removed all in-module epoch iteration, state mutation, and event emission logic. BeginBlocker now delegates directly to keeper's BeginBlocker(ctx) in a single return statement.
Cache Context Utilities Removal
x/epochs/osmoutils/cache_ctx.go
Entire file deleted, removing exported helper functions: ApplyFuncIfNoError, ApplyFuncIfNoErrorLogToDebug, IsOutOfGasError, and PrintPanicRecoveryError that wrapped state-machine execution and panic recovery.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@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.

🧹 Nitpick comments (1)
x/epochs/keeper/abci.go (1)

19-21: Consider explicit return false for consistency.

Line 20 uses a bare return while lines 28 and 60 explicitly return false. Both are functionally equivalent (Go uses zero-value false for bare return), but explicit returns improve readability and consistency.

Suggested fix
 		if ctx.BlockTime().Before(epochInfo.StartTime) {
-			return
+			return false
 		}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@x/epochs/keeper/abci.go` around lines 19 - 21, Change the bare "return"
inside the epoch start-time check to an explicit "return false" for consistency
with the other explicit boolean returns in this function (see the existing
returns at the lines returning false later, e.g., the returns around the checks
at lines 28 and 60); update the return in the block that checks
ctx.BlockTime().Before(epochInfo.StartTime) so the function consistently returns
false explicitly when the epoch hasn't started.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@x/epochs/keeper/abci.go`:
- Around line 19-21: Change the bare "return" inside the epoch start-time check
to an explicit "return false" for consistency with the other explicit boolean
returns in this function (see the existing returns at the lines returning false
later, e.g., the returns around the checks at lines 28 and 60); update the
return in the block that checks ctx.BlockTime().Before(epochInfo.StartTime) so
the function consistently returns false explicitly when the epoch hasn't
started.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b903e90d-0160-4022-9f94-b82121619aed

📥 Commits

Reviewing files that changed from the base of the PR and between f0564ce and 995c14b.

📒 Files selected for processing (3)
  • x/epochs/keeper/abci.go
  • x/epochs/module/abci.go
  • x/epochs/osmoutils/cache_ctx.go
💤 Files with no reviewable changes (1)
  • x/epochs/osmoutils/cache_ctx.go
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: test
🔇 Additional comments (3)
x/epochs/module/abci.go (1)

9-12: Clean delegation pattern.

The refactored BeginBlocker now properly delegates to the keeper, which is the appropriate location for the epoch iteration logic. The pointer parameter *keeper.Keeper correctly matches how am.keeper is passed from AppModule.BeginBlock (as shown in x/epochs/module/module.go:144-150).

x/epochs/keeper/abci.go (2)

13-14: Signature change aligns with Cosmos SDK conventions.

The pointer receiver *Keeper and error return type properly follow Cosmos SDK's ABCI interface patterns. This works correctly with the value-receiver methods called within (IterateEpochInfo, AfterEpochEnd, BeforeEpochStart) since Go automatically dereferences the pointer when calling value receiver methods.


62-62: Return signature correctly follows ABCI pattern.

Returning nil is appropriate here since hook errors are intentionally absorbed (as documented in x/epochs/keeper/hooks.go - "Error is not handled as AfterEpochEnd Hooks use ApplyFuncIfNoError()"). The error return type provides future flexibility to propagate critical errors if needed.

@codecov-commenter
Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 48.03%. Comparing base (f0564ce) to head (995c14b).

Additional details and impacted files
@@            Coverage Diff             @@
##              dev     #106      +/-   ##
==========================================
+ Coverage   47.92%   48.03%   +0.11%     
==========================================
  Files         276      275       -1     
  Lines       16204    16119      -85     
==========================================
- Hits         7766     7743      -23     
+ Misses       7639     7579      -60     
+ Partials      799      797       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

2 participants