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

SPIKE: determine current state (token balances) retrieval strategy #91

Open
aristidesstaffieri opened this issue Dec 17, 2024 · 17 comments
Assignees

Comments

@aristidesstaffieri
Copy link
Contributor

aristidesstaffieri commented Dec 17, 2024

Wallets need a way to index token balances and potentially other contract data for custom tokens.
We recently did a spike on this for Freighter and came to the conclusion that a good solution would be to have balances be indexed and to be able to track the TTL in order to display the appropriate UI during/after a balance data entry has expired.

Should wallet backend index token balances?
Should it also index things like decimals and symbol?
Should it track the TTL for the entries that are being indexed?

@JakeUrban
Copy link
Contributor

Should wallet backend index token balances?

Given that the only alternative is querying the ledger state via RPC and the entry might be archived, I think the answer to this is yes.

However, something to note is that some tokens' balances are not the sum of all credits and debits of the token. Some tokens use a rebasing approach, where a user's balance increases or decreases based on a multiple stored and updated in the contract's state. In this case, the indexer would need to add support for ingesting changes to the multiplier, which would imply changes to all balances of the token. We can decide not to support this type of token in the wallet backend, at least initially, and default to reading ledger state within freighter on a special case basis.

Should it also index things like decimals and symbol?

This is tricky because the number of decimals and the token's symbol is a value returned from calling a function, and that function's implementation could change. Maybe we can account for this by monitoring for token contract's wasm being updated and calling these functions to fetch the potentially updated values.

Alternatives are assuming the number of decimals and the symbol is static, or working with the ecosystem to have these attributes in the contract's meta as static values.

Should it track the TTL for the entries that are being indexed?

I don't see why the indexer would need to track TTLs unless we wanted to create some kind of notification service that notified users that relevant entires are expiring soon. I'm not sure if its worth doing that vs. just letting them restore entries as needed.

@aristidesstaffieri
Copy link
Contributor Author

However, something to note is that some tokens' balances are not the sum of all credits and debits of the token. Some tokens use a rebasing approach, where a user's balance increases or decreases based on a multiple stored and updated in the contract's state. In this case, the indexer would need to add support for ingesting changes to the multiplier, which would imply changes to all balances of the token. We can decide not to support this type of token in the wallet backend, at least initially, and default to reading ledger state within freighter on a special case basis.

It seems like this won't be the only variant of a token that we could decide to support, but I think even tokens that behave like your typical SEP-41 asset could have very different storage layouts for users balances. How would we approach indexing the data generically?

This is tricky because the number of decimals and the token's symbol is a value returned from calling a function, and that function's implementation could change. Maybe we can account for this by monitoring for token contract's wasm being updated and calling these functions to fetch the potentially updated values.

Could valid tokens implement a decimals or asymbol that changes without a wasm upgrade? This seems possible but maybe so edge case that we would not want to support it or care about when it happens.

I don't see why the indexer would need to track TTLs unless we wanted to create some kind of notification service that notified users that relevant entires are expiring soon. I'm not sure if its worth doing that vs. just letting them restore entries as needed.

The only reason is to be able to show a UI state when an entry related to an asset has been archived, but yeah it sounds like this isn't a huge win for the work it would take.

@JakeUrban
Copy link
Contributor

JakeUrban commented Dec 17, 2024

How would we approach indexing the data generically?

Generally speaking off chain applications are expected to index a contract's state using the events the contract emits. SEP-41 defines when tokens should emit events and what their format should be. In fact, maybe the rebasing token could still emit transfer events when the multiplier is updated, which would remove the need for us to treat it as a special case.

Could valid tokens implement a decimals or a symbol that changes without a wasm upgrade?

Yes, the function could be implemented to return different values based on an arbitrary set of variables, its up to them. Thats partly why I think decimals and symbol should have been required meta attributes rather than functions.

The only reason is to be able to show a UI state when an entry related to an asset has been archived, but yeah it sounds like this isn't a huge win for the work it would take.

Yea I view this sort of functionality as an optimization, not a requirement. The default approach should be supporting restoration when necessary, and we can improve the experience opportunistically from there.

@JakeUrban
Copy link
Contributor

I just spoke with the core team, and they said that in Protocol 23, transactions that require archived entires will automatically restore them prior to executing the transaction.

That means we'd be able to simulate calls to a token contract's balance() function and get the result even if the balance is archived.

Given that, we may not need to index token balances, or any ledger state for that matter. The wallet backend could index state transitions (i.e. token transfers), but when Freighter needs a user's current balance, it could use RPC for this.

cc @gouthamp-stellar

@JakeUrban JakeUrban changed the title Index token balances SPIKE: determine current state (token balances) retrieval strategy Jan 7, 2025
@gouthamp-stellar
Copy link
Contributor

We can implement this feature in the wallet backend and have freighter get an account's token balances from the wallet backend instead of having to call rpc. This will have the added advantage that getting a custom token's balance will not require an expensive call to rpc, and it will be a nice feature for other wallets to have as well

A wallet (like Freighter) could tell the wallet backend which tokens it wants to track balances for. The waller backend could then fetch the current balances of these tokens via the rpc calls getLedgerEntries and calling custom tokens' balance() functions. These balances can then be updated every time the state of an account changes. This way, a wallet could query the wallet backend directly to get account balances. Note that Freighter will only have to tell the wallet backend which tokens to track for an account once, instead of having to do it every time a user re-installs Freighter

We may have to periodically call rpc to make sure that the balances in the wallet backend for an account are the exact same as the ones rpc returns.

I think the next step here is to write an RFC for this feature, which we could get to work on after the state change indexer is implemented. I will create a ticker in the backlog for this

@JakeUrban
Copy link
Contributor

We can implement this feature in the wallet backend and have freighter get an account's token balances from the wallet backend instead of having to call rpc. This will have the added advantage that getting a custom token's balance will not require an expensive call to rpc, and it will be a nice feature for other wallets to have as well

I agree getting the current state of an account balances as well as metadata for each token using the wallet backend, rather than making multiple queries to RPC to fetch the same information, would be a nice-to-have feature, although its not strictly required. Clients like Freighter could use RPC as they do today.

A wallet (like Freighter) could tell the wallet backend which tokens it wants to track balances for. The waller backend could then fetch the current balances of these tokens via the rpc calls getLedgerEntries and calling custom tokens' balance() functions. These balances can then be updated every time the state of an account changes.

This is true for classic assets or custom tokens that adhere to SEP41. However, this will not work for custom tokens that do not adhere to SEP41. These tokens may not emit transfer events every time the users' balance changes.

For example, we know USDL is one such token. It actually has all of the contract functions defined in SEP41, which means Freighter still supports it today because it only needs to be able to call balance, transfer, name, and symbol.

However, it doesn't emit transfer events when it rebases, and instead emits a custom event to publish the updated multiplier used to calculate balances. This means that the wallet backend would return incorrect values for USDL balances, unless it added support for USDL's custom events.

Its actually a coincidence that Freighter supports USDL at all. There could be other custom tokens that don't implement the SEP41 functions that Freighter uses.

So, I guess what I'm trying to point out is:

  • if we use the wallet backend to provide current balances (and other token metadata), and it relies on SEP41 events to do so, Freighter would currently not be able to support USDL or other tokens that don't use SEP41 transfer events
  • having the wallet backend call balance like Freighter does every time a client requests the balance doesn't make sense to do because the client could do that themselves. Calling balance periodically also wouldn't work because the next time the token rebases, it would become incorrect
  • the wallet backend could add support for USDL specifically, and it would have to do the same for any custom token we wanted to support. There may be new SEPs that define new standard events, and we could add support for those too
  • however, I think it makes more sense to add support for specific custom tokens in Freighter (or its backend) before adding support in the wallet backend, which should probably only add support for token specifications defined in finalized and adopted SEPs. Maybe USDL's rebasing mechanism will be standardized as a SEP in the future

@aristidesstaffieri
Copy link
Contributor Author

aristidesstaffieri commented Jan 29, 2025

however, I think it makes more sense to add support for specific custom tokens in Freighter (or its backend) before adding support in the wallet backend, which should probably only add support for token specifications defined in finalized and adopted SEPs. Maybe USDL's rebasing mechanism will be standardized as a SEP in the future

It seems like we can't really predict how many non-compliant tokens will need to be supported by Freighter. What we do know will always be true is that any contract that we want to represent as a token will need to have a value that maps to a balance(and a name/symbol).

So is there a way to push the responsibility of mapping a spec <-> balance down to clients?

My thought is that the possibilities are endless, we could potentially care about - NFTs, upgradeable contracts, multi contract tokens, tokens with special mechanics that affect the balance, etc.

No matter what though, Freighter will want to map a value to a balance and will want to have that balance not be stale. Can there be a flow something like -

  1. Freighter subscribes to a contract, including the method or methods from the spec that can affect balance(transfer/transfer_from/burn/mint from sep41, additionally increase/decrease supply in the case of usdl, etc)

  2. wallet-backend processes blocks and when the contract ID is invoked with one of the methods, the spec is fetched(maybe we can cache it and watch for wasm changes?), method is confirmed in spec and the balance is fetched from data entry and updated in balances table.

What I'm going after is a model where we have the flexibility to map arbitrary contracts/specs to a balance in our token balance schema.

Any down sides to this?
Is it better to have clients give you a ledger key for the balances entry at subscription time? Then the indexing instead could look for a match for that ledger key in a LedgerEntryChange in the tx meta.

We've talked about a plugin system for empowering protocols to add their own asset icons to Freighter's UI with our approval. If we gave clients of wallet-backend more control in this way, could we also use a plugin system to get protocols to give us their balance mappings in order to gain support in Freighter?

@JakeUrban
Copy link
Contributor

the balance is fetched from data entry and updated in balances table.

Contract specs define the function signatures for the contract (and maybe the schema for the events published?). They do not define the contract's storage schema, so the wallet backend wouldn't know how to which of the contract's data entries are relevant or how to parse them -- they're meant to be opaque to the client.

Clients are expected to reason about the contract's activity primarily through its events, and maybe secondarily through the functions called within the invocation.


That being said, I do think the idea of indexing the txmeta for contracts registered by clients is worth considering as a default approach to indexing contracts we don't have first-class support for. For example, Freighter can register the USDL contract with the wallet backend before the wallet backend knows how to parse its events for the purpose of tracking balances. Freighter can implement that in the short term, and then if/when these events are standardized, the wallet-backend can add first-class support for tokens that implement the SEP. What do you all think?

@aristidesstaffieri
Copy link
Contributor Author

That being said, I do think the idea of indexing the txmeta for contracts registered by clients is worth considering as a default approach to indexing contracts we don't have first-class support for. For example, Freighter can register the USDL contract with the wallet backend before the wallet backend knows how to parse its events for the purpose of tracking balances. Freighter can implement that in the short term, and then if/when these events are standardized, the wallet-backend can add first-class support for tokens that implement the SEP. What do you all think?

My only reservation is - why use an event based ingestion when a tx meta based ingestion could cover standard and non-standard use cases?

I don't want to die on that hill though so I think that using events to index standard contracts and falling back to a tx based ingestion for other cases would cover any use case I can think of today for Freighters future.

@JakeUrban
Copy link
Contributor

What information specifically within the txmeta you're referring to? Txmetas contain pretty much everything, including the transaction envelope, events, invocation tree, auths, data entry footprint, etc.

What I assumed the flow would look like is:

  • clients register accounts and contracts for the wallet backend to watch
  • the wallet backend fetches the ledger close meta for a ledger via RPC
  • the wallet backend iterates through the transaction metas contained with the ledger close meta
  • for every transaction meta, it iterates through the events. Lets say it sees a transfer to an account that is registered
  • to find out more about the reason for the transfer, the wallet backend inspects the operation that resulted in the event being emitted
  • if the operation is known, either because its a classic op or the top-level invocation is for a known contract that we have first-class support for, the wallet backend parses and indexes that information into a more query-able structure for clients.
  • otherwise, if the contract is unknown, it defaults to indexing the transfer and the txmeta blob for downstream clients to parse if they can

@aristidesstaffieri
Copy link
Contributor Author

What information specifically within the txmeta you're referring to? Txmetas contain pretty much everything, including the transaction envelope, events, invocation tree, auths, data entry footprint, etc.

Specifically questioning this piece -

for every transaction meta, it iterates through the events. Lets say it sees a transfer to an account that is registered

If we can also get this perspective from a combo of the tx envelope/auth entries, then we can use that same ingestion pattern for non standard tokens like USDL who don't emit standard events. Maybe I'm missing something here but seems like we can implement 1 way to ingest standard and non-standard contracts if we get away from events. Wdyt?

@JakeUrban
Copy link
Contributor

If we can also get this perspective from a combo of the tx envelope/auth entries, then we can use that same ingestion pattern for non standard tokens like USDL who don't emit standard events.

In USDL's case specifically, its events don't adhere to SEP41, but it does have the SEP41 function signatures. So to your point, we could inspect the invocation tree, see that a function for the USDL contract was invoked, see that it was the transfer function, and that one of the registered accounts was the from or to parameter value.

However, other custom tokens may not use SEP41's functions.

USDL also has non-SEP41 functions we'd need to monitor in order to know if the state of our registered accounts changed, such as the function thats called to update the rebasing multiplier. In this case, no accounts are provided as function parameters and no address authorizations are needed, which means we wouldn't know if this transaction is relevant or not unless we understood the nature of the USDL contract spec specifically.

All this is to say, the wallet backend should use both events as well as the other information available in a txmeta such as the invocation tree and auths to determine if the transaction is relevant to our set of registered accounts and what information to index.

But at the end of the day, if

  • neither the wallet backend or the client know how to interpret a contract's events and/or function interface
  • none of our registered accounts provide authorization
  • the transaction was not submitted to our TSS by an authenticated freighter user

The wallet backend won't index the transaction, and Freighter wouldn't know what to do with it even if it did.

So, I think we need to focus on defining:

  • what the wallet backend will "first-class".
    • What I mean by first class is "derive information that would be meaningful to the user". I think classic ops and SEP41 token activity is the answer to this. In time, we can add more standards or explicit support for specific protocols (ex. blend, soroswap)
  • what the wallet backend will not "first-class" but still index the transaction meta for downstream clients to interpret
    • I think if the transaction was submitted by the TSS, or if the transaction required authorization from one of our accounts, then it should be indexed
    • @aristidesstaffieri you seem to be advocating for also allowing clients to tell the wallet backend to index specific function calls on specific contracts, even if the transaction was not submitted by the TSS or our accounts didn't authorize it. I'm open to that too
  • Finally, anything else should either be indexed by the Freighter backend itself (ingesting via RPC directly) or not indexed at all

@gouthamp-stellar
Copy link
Contributor

gouthamp-stellar commented Jan 30, 2025

In USDL's case specifically, its events don't adhere to SEP41, but it does have the SEP41 function signatures. So to your point, we could inspect the invocation tree, see that a function for the USDL contract was invoked, see that it was the transfer function, and that one of the registered accounts was the from or to parameter value.

USDL also has non-SEP41 functions we'd need to monitor in order to know if the state of our registered accounts changed, such as the function thats called to update the rebasing multiplier. In this case, no accounts are provided as function parameters and no address authorizations are needed, which means we wouldn't know if this transaction is relevant or not unless we understood the nature of the USDL contract spec specifically.

It sounds like we can track USDL's transfer function by inspecting the invocation tree to know when a USDL was transferred between 2 accounts, but if the USDL balance of those accounts changed because of another function - like the function thats called to update the rebasing multiplier, then there's no easy easy to determine that - at least currently. And if the token doesn't even implement SEP 41's functions, then there's nothing we can do to track movement

If that's the case, the wallet backend can't really index state changes that have to do with tokens like USDL

However, it will index state changes that have to do with SEP-41 compliant tokens, and state changes for stellar assets. We can, if we wanted to, build a account current state using this, that stores the current balance of all these tokens like so:

  1. We get all the assets an account would like to track. These would be a combination of trustline assets and tokens
  2. For trustline assets we'd grab their current value from getLedgerEntries (NOTE: this assumes there is a way to convert a assetcode:issuergaddress pair to a trustline id for that account that we can pass to getLedgerEntries) to get the current values for that trustline
  3. For contract tokens, we'd grab the balance()
  4. We'd update the balances for the assets above for trustline assets and SEP-41 tokens as token transfer events came in, but not for non SEP-41 clients, for which...

we can either have the wallet backend proxy a call to RPC to get its balance() or have freighter deal with it. Maybe in the future if USDL does become SEP-41 compliant we won't have to proxy call RPC anymore if we went that route.

Would that be something worth building? Depending on how many non compliant tokens are out there in the wild - and they arent that many from what I gather, I think so. A wallet would still save the effort it would take it to fetch all those SEP-41 compliant and trust line tokens

@aristidesstaffieri
Copy link
Contributor Author

4. We'd update the balances for the assets above for trustline assets and SEP-41 tokens as token transfer events came in, but not for non SEP-41 clients, for which...

we can either have the wallet backend proxy a call to RPC to get its balance() or have freighter deal with it. Maybe in the future if USDL does become SEP-41 compliant we won't have to proxy call RPC anymore if we went that route.

What about special case-ing USDL's ingestion?

I think you're right that we won't see a ton of non-compliant tokens, there's not a huge set of reasons to break the standard but I think it would be nice for clients to not have to ingest these separately. It seems like special cases will be infrequent enough to either work towards a standard for those or just special case their ingestion in wallet backend to allow for their inclusion along with classic assets & sep41 tokens.

@gouthamp-stellar
Copy link
Contributor

What about special case-ing USDL's ingestion?

I think you're right that we won't see a ton of non-compliant tokens, there's not a huge set of reasons to break the standard but I think it would be nice for clients to not have to ingest these separately. It seems like special cases will be infrequent enough to either work towards a standard for those or just special case their ingestion in wallet backend to allow for their inclusion along with classic assets & sep41 tokens.

Hmmm that's a good question. We could special case tokens like USDL in the wallet backend, but I feel like we should be careful with this strategy. I'd ideally like to avoid having to do too much of this kind of special casing. Perhaps we can make exceptions for non compliant tokens that are really popular.

@gouthamp-stellar
Copy link
Contributor

Also, the state change indexer could potentially index tokens like USDL if USDL's contract id was registered with it. Like @JakeUrban mentioned in a previous comment, we could simply save the corresponding txmeta for these contract ids that we can't accurately reason about so that downstream clients (like Freighter) could parse that data themselves

@aristidesstaffieri
Copy link
Contributor Author

Also, the state change indexer could potentially index tokens like USDL if USDL's contract id was registered with it. Like @JakeUrban mentioned in a previous comment, we could simply save the corresponding txmeta for these contract ids that we can't accurately reason about so that downstream clients (like Freighter) could parse that data themselves

Yeah either approach seems like a fit. The scenario I want to avoid is one where clients of wallet-backend(or just Freighter in my case) have to also stream transactions and merge with the wallet-backend stream or events in order to have a complete view. It would be great to have the flexibility to have everything the client needs from one source, even if clients have to parse or process the tx meta themselves for non standard stuff.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants