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

Make explicit decision on removing or keeping 0 int's in state for total escrow amount #3549

Closed
3 tasks
colin-axner opened this issue May 3, 2023 · 1 comment
Assignees
Labels
20-transfer needs discussion Issues that need discussion before they can be worked on
Milestone

Comments

@colin-axner
Copy link
Contributor

Summary

#3019 added a separate tracking amount in state for the total IBC'd out for a chain. One slight concern is handling of 0 amount escrow balances. Two issues arise:

  • GetTotalEscrowForDenom returns a zero coins slice for escrows which have never been referenced to ease caller logic (not ideal)
  • 0 int's exist in state while the bank keeper prunes zero balances (also not ideal to have inconsistency)

We should make an explicit decision on how we want to handle this situation, so we can ensure there are no pitfalls and that it is well documented

Given two problematic scenario's, I think I'm leaning towards adjusting the logic to:

  • require caller to handle initializing empty escrow accounts
  • prune zero ints (match bank behaviour)

I guess the idea is that the escrow function needs to handle empty escrows and unescrow needs to handle pruning zero amounts.


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged/assigned
@colin-axner colin-axner added the needs discussion Issues that need discussion before they can be worked on label May 3, 2023
@crodriguezvega crodriguezvega added this to the v7.1.0 milestone May 15, 2023
@crodriguezvega crodriguezvega self-assigned this May 15, 2023
@crodriguezvega
Copy link
Contributor

Closed by #3585

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
20-transfer needs discussion Issues that need discussion before they can be worked on
Projects
Archived in project
Development

When branches are created from issues, their pull requests are automatically linked.

2 participants