witnet / my-wit-wallet

myWitWallet is a Witnet light wallet for mobile and desktop
https://mywitwallet.com
GNU General Public License v3.0
9 stars 7 forks source link

Assessment: Optimize request rate to Witnet explorer API #184

Open drcpu-github opened 1 year ago

drcpu-github commented 1 year ago

I noticed that a single wallet instance generates a lot of calls to the Witnet explorer API and wanted to get your opinion. While in the transaction history view, it creates a request for the UTXO's for 10 addresses every 30 seconds. I talked to @parodyBit and understand why, but I still think this is wasteful. I'm somewhat concerned that, if there are a lot of users, the explorer API will get flooded with "useless" requests. For example, even if there are only 10 simultaneous users, that's 100 UTXO requests to a node every 30 seconds.

I don't have the perfect solution, but I do think this can be optimized a bit without significantly hindering the user experience. Below is a list of suggestions in no particular order which can be combined.

  1. Ideally, a user can refresh an address manually by swiping down in the transaction history menu or hitting a refresh button or ...
  2. Reduce the global refresh rate to 1 minute. I don't think the wallet needs to see every balance change (almost) instantly after a block is created, a small delay (of at most 1 minute) seems fine to me.
  3. Only fetch the UTXO's for the single active address. For inactive addresses, either rely on a manual refresh or, potentially, fetch data for them at a reduced rate.
  4. Whenever a wallet stays on the transaction history panel, use an exponential backoff refresh rate (e.g., every 30s for 5 times, every 1 minute for 5 times, ..., up to only once every 5 minutes). I'm assuming that no one is staring at his wallet screen for 5 minutes on end and that for a wallet which they forgot it's open a reduced refresh rate is fine.
  5. Reduce the amount of addresses in the query by querying only the non-zero balance addresses plus one. People will generally not go for a gap in their wallet list and use addresses sequentially. The wallet will still see a new addresses being used, while also not querying addresses that are never used. I'd bet most users will not use more than two or three addresses, so querying ten all the time is unnecessary.
drcpu-github commented 1 year ago

I realized yesterday I could optimize this on my end by leveraging the already existing caching infrastructure for address data. This is essentially a server process which monitors API requests (to figure out for which addresses caching data is advisable) and which receives updates from the explorer process with all address involved in transaction in a block (to figure out if it needs to update cached data).

I implemented a PoC and did a small timing test. I made sure the UTXOs for 30 different addresses where cached and then launched three UTXO requests (querying UTXOs for 10 addresses). I repeated this several times to measure the response generation time:

Below are the actual response generation times (in seconds). As you can see, caching UTXOs and returned the cached version instead of querying a node yields a ~100x speedup.

Response time (s)
MetricNo cachingCachingSpeedup
AVG 0,53290,005597,5
STDEV 0,24700,0022114,1
MAX 1,31510,0124106,5

The biggest downside is that, if there is a bug in my caching algorithm (e.g., due to a corner case of failing inter-process communication), wallets can become unusable until the cached data expires. Hence, this implementation is very much hit or miss.

On the one hand, the fact that this is a beta-release of the wallet means this should be enabled and tested now. On the other hand, I'm a bit hesitant since this is effectively other people testing it with real money. What do you think?

aesedepece commented 1 year ago

Caching mechanisms of course don't come for free. I understand your concern about the situation in which UTXO misreporting precludes the creation or broadcasting of transactions.

But I completely agree that this is the right time to try such feature. It seems to me like a very beneficial optimization that also follows pretty standard patterns (e.g. resorting to memcached).

It's a green light for me.

drcpu-github commented 1 year ago

Time to revive this topic. In the past two days, the explorer got load tested a lot by UTXO requests launched by myWitnetWallet. Below you can find a screenshot showing the load during about three hours showing several peaks of more than 1000 requests per minute:

mww_sync_1

Based on my logs, the issue is that this load originates mostly from syncing what looks like a single wallet: between the shown timestamps it launched over 36k requests. At one point, that single wallet instance queried the explorer with 930 requests in a single minute. What is even more problematic is that of these 930 requests, only 22 were actually unique. In other words, this set of unique requests were launched almost 50 times in the same minute.

As you can see from the red line, around 6:15, the load actually resulted in degraded performance with a lot of 502 responses (over 3k in total). I'm not quite sure why the explorer sometimes manages to process the load and sometimes it can not, but that is largely irrelevant to the issue.

I have already highlighted a number of potential solutions in my initial post, but I'll add two important ones: (1) There needs to be some sort of detection mechanism to recognize 'inactive' addresses. One use case I can think of is a wallet which is heavily used but where (change) addresses are always emptied and never reused. The wallet should recognize that and not continously synchronize those addresses. One detection mechanism could be based on them being empty and having a lower keychain index than non-empty addresses. In above example wallet, it tries to synchronize 220 addresses every time and there is no way these are all in use. (2) Furthermore, an extra point that should be optimized is that it should not be possible for a wallet to launch the same set of requests 50 times in the same minute. I assume this is triggered by switching tabs in the wallet (or something similar), but regardless of how exactly it happens, the wallet should add a cooldown timer which prevents it from continuously sending the same set of requests.

If you want the wallet to continue delivering a good user experience, it is of the utmost important to solve this because obviously this type of synchronzing behavior can not possible scale to more users.

Tommytrg commented 1 year ago

@drcpu-github There was a bug in the synchronization process that caused the wallet to make the same call more than once. We have fixed this problem in https://github.com/witnet/my-wit-wallet/pull/348

drcpu-github commented 1 year ago

Great, thanks! If I remember correctly issue (2) from the post above was also solved in the latest release, right?