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 a state entry to find total amount of native assets IBC'd out #2664

Closed
3 tasks
ValarDragon opened this issue Nov 2, 2022 · 6 comments · Fixed by #3019
Closed
3 tasks

Make a state entry to find total amount of native assets IBC'd out #2664

ValarDragon opened this issue Nov 2, 2022 · 6 comments · Fixed by #3019
Labels
20-transfer client-UX type: refactor Architecture, code or CI improvements that may or may not tackle technical debt.
Milestone

Comments

@ValarDragon
Copy link
Contributor

Summary

Currently every channel has its own escrow bank account, which is a great design! So its very easy to see the amount of native assets IBC'd in or out on any IBC channel.

However, there are use cases that would like us to be able to reason about the total amount of a native asset that has been IBC'd out.

So if there are say three channels from Osmosis to Juno:

channel-1: 10 Osmo -> Juno
channel-2: 20 Osmo -> Juno
channel-3: 30 Osmo -> Juno

I would like to be find that its been 60 osmo IBC'd out, with no iteration.

Problem Definition

One such use case is rate limiting, where we would like to limit the amount of osmo in total that can flow out of osmosis in a given time interval. For IBC"d in assets, this is quite easy as all incoming assets for it on are on the same channel, so we can easily find "total IBC'd to Osmosis".

For native assets though, there is no efficient way to get the total amount IBC'd out. We'd like to make our rate limits reason about this, rather than the total supply of osmo on Osmosis. (As much of the Osmo should be illiquid due to staking and LP'ing) We can't iterate over all channels, as there are thousands of these on Osmosis.

Proposal

  • Add a new state entry for total_native_ibc_out/{denom} which then stores the amount IBC'd out across all channels.
  • Add a query that allows us to find this value quickly
  • Add a safety check if this value is ever attempted to be serialized to a negative value

For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged/assigned
@colin-axner
Copy link
Contributor

I'm in favor. You technically could do this as a middleware if it was needed tomorrow, but I think transfer should do this anyways. I guess this will need to run a migration which calculates the native asset value in escrow when the functionality is added. This is something that could be done with #330 (though I wouldn't block on #330 being implemented)

@colin-axner
Copy link
Contributor

colin-axner commented Feb 22, 2023

Quick question

Just to clear up terminology:

  • native assets (tokens born on this chain)
  • IBC assets (tokens sent from another chain)
  • IBC'd in assets (tokens received from another chain)
  • IBC'd out assets (native or IBC assets sent to another chain)

The proposal here is to track the IBC'd out of all assets, not just native ones, correct?

For native assets though, there is no efficient way to get the total amount IBC'd out.

This is true for all types of tokens? Not just native ones? (using my definition of native)

@stackman27
Copy link
Contributor

stackman27 commented Feb 22, 2023

I believe we are only "tracking native assets that's outside of the chain" which is also total balance of the escrow accounts.

The reason being if there are say three channels from Osmosis to Juno:

channel-1: 10 Osmo -> Juno
channel-2: 20 Osmo -> Juno
channel-3: 30 Osmo -> Juno

We would like to be find that its been 60 osmo IBC'd out, with no iteration.

@stackman27
Copy link
Contributor

We discussed it here;

I originally thought we were only tracking total channel's IBC out instead of tracking channel's total escrow amount so it got me a little confused

This is a good question. We both had different interpretations of the requirements :) I assumed that we wanted to track the total amount of native assets that are outside of the chain (and hence the total balance of the escrow accounts would give us), but your interpretation might as well be correct (track how many tokens have flowed out of the chain regardless of whether they came back). I think it would be good to hear @ValarDragon thoughts on what he originally wanted to track when he wrote the issue. I am also curious to hear @womensrights opinion on this.

Hi @crodriguezvega had a chat with @nicolaslara about this and we are tracking "total amount of native assets that are outside of the chain (and hence the total balance of the escrow accounts would give us)". The new commit i pushed highlights those changes. Please lmk what you think about it

Link

@colin-axner
Copy link
Contributor

Is there a reason for explicitly not tracking non native tokens in this same situation? It seems like it would be useful, but that your use case is primarily concerned with having an understanding of the amount of osmo not on the chain? To me it seems other folks may be interested in tracking non-native tokens as well, and it would simplify the code logic?

@stackman27
Copy link
Contributor

stackman27 commented Feb 26, 2023

yup, we just wanted to track native tokens for this specific issue.

If you think its important i can make a follow up PR after we merge this that tracks non-native tokens as well

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
20-transfer client-UX type: refactor Architecture, code or CI improvements that may or may not tackle technical debt.
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants