Skip to content

fix: enforce rate limit on outbound ibc transfers#116

Merged
AdriaCarrera merged 1 commit into
mainfrom
fix/ibc-rate-limiting
May 13, 2026
Merged

fix: enforce rate limit on outbound ibc transfers#116
AdriaCarrera merged 1 commit into
mainfrom
fix/ibc-rate-limiting

Conversation

@kpitapeersyst

@kpitapeersyst kpitapeersyst commented Apr 24, 2026

Copy link
Copy Markdown
Contributor

fix: enforce rate limit on outbound ibc transfers

Motivation 💡

TransferKeeper was constructed with app.IBCKeeper.ChannelKeeper as its ICS4Wrapper, so outbound transfers called transfer.SendPacket -> channel.SendPacket directly, completely bypassing the rate-limit middleware. Rate limiting only applied to inbound packets (via the IBCModule middleware chain), leaving the outbound path unprotected.

Changes 🛠

  • Moved RateLimitKeeper initialization to run after TransferKeeper creation
  • Wired the outbound stack with app.TransferKeeper.WithICS4Wrapper(&app.RateLimitKeeper) so outbound transfers flow through rate limiting before reaching the channel
  • Updated the stale transfer-stack doc comment to reflect the real layers (IBC Transfer, Rate Limit, ERC-20), removing references to recovery and claims middlewares

Considerations 🤔

  • Outbound chain is now transferKeeper.SendPacket -> rateLimitKeeper.SendPacket -> channel.SendPacket. Inbound chain remains channel.RecvPacket -> erc20.OnRecvPacket -> rateLimit.OnRecvPacket -> transfer.OnRecvPacket.
  • WithICS4Wrapper is called before transfer.NewAppModule and transfer.NewIBCModule, which take the keeper by value, so the module copies observe the correct ics4Wrapper. Erc20Keeper holds a pointer to TransferKeeper, so it also sees the updated wiring.
  • Passed &app.RateLimitKeeper (pointer) rather than a value copy to preserve identity and future-proof against later mutations of the keeper.

Summary by CodeRabbit

  • Bug Fixes

    • Improved rate limiting behavior for IBC transfers by reordering middleware initialization to ensure rate limits are properly enforced at the appropriate layer.
  • Documentation

    • Updated internal documentation to reflect the current middleware stack architecture and transfer processing flow.

@coderabbitai

coderabbitai Bot commented Apr 24, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6a34676b-9335-423f-aa27-8f15b9b6fdcc

📥 Commits

Reviewing files that changed from the base of the PR and between 43c1c36 and 1a5eddc.

📒 Files selected for processing (1)
  • app/app.go

📝 Walkthrough

Walkthrough

Rate limit keeper initialization is reordered to occur after the transfer keeper is created, then explicitly connected to it via WithICS4Wrapper. Documentation is updated to reflect the new middleware stack position between the transfer keeper and IBC channel.

Changes

Cohort / File(s) Summary
Initialization & Middleware Stack
app/app.go
Reordered rate limit keeper initialization to follow transfer keeper creation with explicit WithICS4Wrapper connection. Updated in-file documentation for transfer middleware stack and SendPacket/RecvPacket call flows to reflect the new middleware ordering and removed references to recovery/airdrop-claims stages.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A keeper reordered, connected with care,
The rate limiter finds its proper place there,
Through wrapper it clasps to the transfer it holds,
Documentation refreshed as the new story unfolds! 🔗

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix: enforce rate limit on outbound ibc transfers' is clear, specific, and directly describes the main change—addressing the issue where outbound IBC transfers were bypassing rate limiting.
Description check ✅ Passed The description follows the repository template with all required sections completed: Motivation clearly explains the problem, Changes itemizes the three key modifications, and Considerations addresses implementation details and trade-offs.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/ibc-rate-limiting

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.11.4)

Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions
The command is terminated due to an error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions


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.

@AdriaCarrera AdriaCarrera merged commit 63a1c3f into main May 13, 2026
6 checks passed
@AdriaCarrera AdriaCarrera deleted the fix/ibc-rate-limiting branch May 13, 2026 09:23
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