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.

Sign up for GitHub

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

Jump to bottom

Fix RefundAllFees #860

Closed
AdityaSripal opened this issue Feb 7, 2022 · 0 comments · Fixed by #1244
Closed

Fix RefundAllFees #860

AdityaSripal opened this issue Feb 7, 2022 · 0 comments · Fixed by #1244
Assignees
@colin-axner
Labels
29-fee
Milestone
Fee middleware beta

Comments

@AdityaSripal
Copy link
Member

Modify RefundFeesOnChannel in OnChanCloseInit to iterate over all incentivized packets, before refunding for each packet, it should check the escrow address for sufficient balance to refund the packet. If the escrow balance would be negative, this implies a severe bug and thus all fee channels should become disabled. If the refund fails for another reason (blocked address) we should continue iteration. RefundFeesOnChannel can be renamed. DisableAllChannels should be called from within the logic of detecting the negative escrow balanceWhen we refund packet upon channel closure we need to remove the incentivized packets from the mapping (since they have been refunded). More discussion is needed to determine the best approach since there exist potential edge case scenarios.

ibc-go/modules/apps/29-fee/keeper/escrow.go

Line 106 in 13f77de

func (k Keeper) RefundFeesOnChannel(ctx sdk.Context, portID, channelID string) error {

This should not be taken up until discussion in #821 is resolved.
@colin-axner colin-axner self-assigned this Feb 28, 2022
This was referenced Feb 28, 2022
Closed
Closed
@crodriguezvega crodriguezvega added this to the Fee middleware beta milestone Mar 29, 2022
This was referenced Apr 11, 2022
Merged
Merged
@mergify mergify bot closed this as completed in #1244 Apr 13, 2022
mergify bot pushed a commit that referenced this issue Apr 13, 2022
@colin-axner
refactor: Fix RefundFeesOnChannel (#1244)
e59a6d2 
## Description

read #1060 

closes: #860
closes: #780 

---

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

- [x] Targeted PR against correct branch (see [CONTRIBUTING.md](https://github.com/cosmos/ibc-go/blob/master/CONTRIBUTING.md#pr-targeting))
- [x] Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
- [x] Code follows the [module structure standards](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules/structure.md).
- [x] Wrote unit and integration [tests](https://github.com/cosmos/ibc-go/blob/master/CONTRIBUTING.md#testing)
- [x] Updated relevant documentation (`docs/`) or specification (`x/<module>/spec/`)
- [x] Added relevant `godoc` [comments](https://blog.golang.org/godoc-documenting-go-code).
- [x] Added a relevant changelog entry to the `Unreleased` section in `CHANGELOG.md`
- [x] Re-reviewed `Files changed` in the Github PR explorer
- [x] Review `Codecov Report` in the comment section below once CI passes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@colin-axner colin-axner

Labels
29-fee
Projects
ibc-go
Archived in project
Milestone
Fee middleware beta
Development

Successfully merging a pull request may close this issue.

refactor: Fix RefundFeesOnChannel cosmos/ibc-go
3 participants
@crodriguezvega @AdityaSripal @colin-axner