Skip to content

fix: Fix err ack in ibc v2 middleware#967

Merged
Eric-Warehime merged 6 commits intomainfrom
eric/return-value
Feb 6, 2026
Merged

fix: Fix err ack in ibc v2 middleware#967
Eric-Warehime merged 6 commits intomainfrom
eric/return-value

Conversation

@Eric-Warehime
Copy link
Copy Markdown
Contributor

Description

Fixes the err ack return value in the ibc v2 middleware for erc20s.

Previously we were swallowing the return value of the keeper invocation. After this change we properly return an error ack if the app's OnRecvPacket call passes, but the middleware keeper's OnRecvPacket call fails.

Adde test covers this case.


Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

  • tackled an existing issue or discussed with a team member
  • left instructions on how to review the changes
  • targeted the main branch

@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 30, 2026

Codecov Report

❌ Patch coverage is 57.14286% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.62%. Comparing base (83a3a6c) to head (9139176).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
x/erc20/v2/ibc_middleware.go 57.14% 2 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #967      +/-   ##
==========================================
+ Coverage   65.56%   65.62%   +0.06%     
==========================================
  Files         319      319              
  Lines       22220    22225       +5     
==========================================
+ Hits        14569    14586      +17     
+ Misses       6480     6476       -4     
+ Partials     1171     1163       -8     
Files with missing lines Coverage Δ
x/erc20/v2/ibc_middleware.go 69.41% <57.14%> (-1.84%) ⬇️

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread evmd/tests/ibc/v2_ibc_middleware_test.go
Copy link
Copy Markdown
Member

@srdtrk srdtrk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm. Left a hardening suggestion, I'll leave it up to you to implement it or not. I can verify that this fixes the original issue

Comment thread x/erc20/v2/ibc_middleware.go
@Eric-Warehime Eric-Warehime added this pull request to the merge queue Feb 6, 2026
Merged via the queue into main with commit 776462c Feb 6, 2026
28 of 29 checks passed
@Eric-Warehime Eric-Warehime deleted the eric/return-value branch February 6, 2026 17:28
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.

4 participants