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

If distribute fee fails, refund to original sender #862

Closed
AdityaSripal opened this issue Feb 7, 2022 · 1 comment · Fixed by #1245
Closed

If distribute fee fails, refund to original sender #862

AdityaSripal opened this issue Feb 7, 2022 · 1 comment · Fixed by #1245
Assignees
Labels

Comments

@AdityaSripal
Copy link
Member

func (k Keeper) distributeFee(ctx sdk.Context, receiver sdk.AccAddress, fee sdk.Coins) {

Should include a refund address argument which is original fee sender

If we are trying to refund to original sender and that fails, then we need to just no-op and continue.

@crodriguezvega crodriguezvega moved this to Backlog in ibc-go Feb 8, 2022
@crodriguezvega crodriguezvega added this to the Fee middleware beta milestone Mar 29, 2022
@crodriguezvega crodriguezvega moved this from Backlog to Todo in ibc-go Apr 4, 2022
@damiannolan
Copy link
Member

damiannolan commented Apr 7, 2022

What are the concerns of the fee being left behind in the ics29 module account if this fails?
For the same reasons a bank send fails to a forward/reverse relayer it may also fail to the refund account. Do you think this issue is still necessarily?

edit: resolved in #1245

@damiannolan damiannolan added the needs discussion Issues that need discussion before they can be worked on label Apr 7, 2022
@seantking seantking moved this from Todo to On hold in ibc-go Apr 7, 2022
@crodriguezvega crodriguezvega removed the needs discussion Issues that need discussion before they can be worked on label Apr 11, 2022
@crodriguezvega crodriguezvega moved this from On hold to Todo in ibc-go Apr 11, 2022
@damiannolan damiannolan moved this from Todo to In progress in ibc-go Apr 11, 2022
@damiannolan damiannolan moved this from In progress to In review in ibc-go Apr 11, 2022
Repository owner moved this from In review to Done in ibc-go Apr 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants