search

JoinSEEDS / seeds_light_wallet

http://www.joinseeds.earth/
MIT License
42 stars 22 forks source link

Feature/precision optimization #1960

Open n13 opened 1 week ago

n13 commented 1 week ago

Description

Problem: Updating precision for all tokens is very expensive

Fix: Don't load precision from stats table - load it dynamically whenever the token balance is loaded

Why this works: We only need token precision for display and importantly for "send" - prior to display and send we must update the balances; therefore this will not cause any problems.

Tech Notes

In theory token balances are always loaded before we display, and certainly before we send. Also precision never dynamically changes so once we load it, it never changes - ever. That's guaranteed by the token contract in EOSIO.

Testing Tested "send" with a few different tokens, it works.

Also monitored that as token balances are received, we update the precision.

Future Improvements It would be nice to remove the tokens table and to package the side effect code a bit better.

I didn't remove the static table because I wasn't sure we have a comprehensive list of "all tokens" anymore since the token master contract change, so I don't know if all known tokens are always there. I believe they probably are - but not sure.

chuck-h commented 1 week ago

Good cleanup on some mess I left behind. Probably would benefit from a deeper clean too.

BTW I think (a) we want getTokenBalance to be called when a currency card slides into view; and (b) we don't want to go out and get all the balances at startup. Neither of these seems to be true.

When getTokenBalance is called, we either (1) get a balance back from the chain api, which tells us the precision, so we update the TokenModel.contractPrecisions table, or (2) get "no balance entry" back from the chain api; whereupon we should look in the contractPrecisions table and (2a) we find the token precision is already listed there -- we're done, or (2b) the token precision is not in the contractPrecisions table, we need to get it from a Stat table lookup.

I think this PR handles (1) perfectly but not (2a)/2(b).

I expect that with the upcoming new logic for manually enabling/disabling currency card visibility, a user can enable a card for a token he doesn't own, then click "receive" to generate a "please send me 10 tokens" QR code. That operation will require a Stat table lookup in order to create a valid transaction for the QR.

chuck-h commented 1 week ago

TL;DR The whole TokenModel structure deserves a review since it became more dynamic.

@n13 I think you were asking why we should keep a separate contractPrecisions static table instead of just holding that precision directly as a field in the TokenModel. My answer is "no good reason".

There already is a precision field in the TokenModel, which IIRC was being used as a hint to the UI display formatter. I believe that UI hinting function is probably worth keeping so the TokenModel needs two distinct precision fields.

Before I added the ability to download the token list from the tokensmaster contract, TokenModel was just a simple static list of the tokens. So I grafted on the dynamic loading to that. But I really did not address the lifecycle issues of new tokens appearing in the master list or old ones disappearing (e.g. being removed for bad behavior). I wrote a couple of functions that look related but they're dead code.