Skip to content

Conversation

@beer-1
Copy link
Member

@beer-1 beer-1 commented Nov 5, 2025

Description

Apply fix

Summary by CodeRabbit

  • Bug Fixes
    • Fixed fee grant allowance revocation to correctly remove associated expiry entries, ensuring proper cleanup of revoked allowances.

@coderabbitai
Copy link

coderabbitai bot commented Nov 5, 2025

Walkthrough

A bug fix in the revokeAllowance function where the expiry queue deletion now uses the consistent key format (key[1:]) instead of reconstructing the key from granter/grantee parameters, ensuring the stored entry is properly removed.

Changes

Cohort / File(s) Summary
Bug fix: FeeAllowance expiry queue deletion
x/feegrant/keeper/keeper.go
Modified revokeAllowance to use existing key variable when deleting from FeeAllowancePrefixQueue instead of constructing a new key

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

  • Single, localized fix in one function
  • Key consistency issue addressing an obvious bug
  • No signature changes or ripple effects indicated

Poem

🐰 A queue key lay scattered, oh what a plight,
Key mismatch muddle, not stored quite right,
Now keeper constructs it in harmony true,
Using key[1:] where it belonged through and through! ✨

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title clearly and specifically describes the main change: fixing the removal of expiration queue entries during allowance revocation in the feegrant module.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/feegrant-revoke

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between e7648fe and 1ec4684.

📒 Files selected for processing (1)
  • x/feegrant/keeper/keeper.go (1 hunks)
🔇 Additional comments (1)
x/feegrant/keeper/keeper.go (1)

177-179: Critical bug fix verified: queue deletion now uses consistent key format.

This fix ensures the expiration queue entry is deleted using the same key format (key[1:]) that is used when adding it in GrantAllowance (line 89). The flow is now consistent: both addToFeeAllowanceQueue (line 89) and revokeAllowance (line 177) pass key[1:] to FeeAllowancePrefixQueue, eliminating orphaned queue entries that would never be cleaned up.

Consider verifying with a test that:

  1. Grants an allowance with an expiration
  2. Revokes the allowance
  3. Confirms the queue entry is properly removed

Additionally, review existing deployments for orphaned queue entries that may require cleanup via a migration.


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

@beer-1 beer-1 merged commit ba9bc3c into release/v0.50.x Nov 5, 2025
1 check passed
@beer-1 beer-1 deleted the fix/feegrant-revoke branch November 5, 2025 08:48
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