Skip to content
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

Refund stuck fees when ics29 is downgraded in an upgrade (edge case scenario) #5738

Open
3 tasks
colin-axner opened this issue Jan 25, 2024 · 1 comment
Open
3 tasks
Labels
29-fee channel-upgradability Channel upgradability feature help wanted Issues for which we would appreciate help/support from the community nice-to-have

Comments

@colin-axner
Copy link
Contributor

Summary

There is an edge case scenario where a user incentivizes the next packet to be sent, but before the packet is sent, the channel undergoes an upgrade which removes ics29 from the channel version. This will result in packet fees being left in escrow for a packet that can never be incentivized. We may optionally refund any packet fees leftover in state when downgrading. Because all in-flight packets are flushed, only the next packet to be sent should be capable of having fees in escrow.

It might be possible to reuse the logic in RefundFeeOnChannelClosure

For now we have decided to leave the behaviour as is.

Discussed during our internal security audit


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged/assigned
@crodriguezvega
Copy link
Contributor

The changes would basically be:

diff --git a/modules/apps/29-fee/ibc_middleware.go b/modules/apps/29-fee/ibc_middleware.go
index 4ac1e7dd9..13f92c644 100644
--- a/modules/apps/29-fee/ibc_middleware.go
+++ b/modules/apps/29-fee/ibc_middleware.go
@@ -429,6 +429,9 @@ func (im IBCMiddleware) OnChanUpgradeOpen(ctx sdk.Context, portID, channelID str
 
        versionMetadata, err := types.MetadataFromVersion(proposedVersion)
        if err != nil {
+               // refund any fees left in escrow that might be in state for the next packet
+               // to be sent and that can no longer be incentivized after the downgrade succeeds
+               im.keeper.RefundFeesOnChannelClosure(ctx, portID, channelID)
                // set fee disabled and passthrough to the next middleware or application in callstack.
                im.keeper.DeleteFeeEnabled(ctx, portID, channelID)
                cbs.OnChanUpgradeOpen(ctx, portID, channelID, proposedOrder, proposedConnectionHops, proposedVersion)

Plus a test here. Fees may be left in escrow if the next sequence is incentivised with MsgPayPacketFee and the packet for next sequence is not sent.

@crodriguezvega crodriguezvega added the help wanted Issues for which we would appreciate help/support from the community label Apr 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
29-fee channel-upgradability Channel upgradability feature help wanted Issues for which we would appreciate help/support from the community nice-to-have
Projects
Status: No status
Development

No branches or pull requests

2 participants