extension icon indicating copy to clipboard operation
extension copied to clipboard

Remove provider-wide pendingTransaction subscriptions.

Open 0xDaedalus opened this issue 2 years ago • 2 comments

Our current method of pending transaction subscription does not seem to be providing much benefit while resulting in either:

  • a) Large infrastructure costs when subscribing via alchemy (their pricing model seems to charge us for every pending transaction on a given chain - not just the transactions that match the filter we specify)
  • b) Large CPU load when subscribing via a generic provider (we need to poll for every transaction in the mempool using its transaction hash and then serialize it).

The main benefit we get out of subscribing to pending transactions in the mempool is we are able to detect and display transactions that relevant to a given account even if those transactions were not initiated by the Tally Ho wallet (e.g. a user has their account imported to both tally ho and rainbow and make a transaction using rainbow).

It seems reasonable to assume that this scenario is both somewhat rare and already taken care of by our periodic polling with alchemy_getAssetTransfers (although in that case the transactions can take up to 2 minutes to show up).

With the above points in mind and keeping in mind that we subscribe to transactions sent from the wallet via provider.once - we've chosen to remove our subscription to pending transactions until we find a use case that justifies the infra cost and/or cpu overhead generated by the subscription.

An open question here is what - if anything - do we need to extract from the handlePendingTransaction function to make sure our nonce is fresh when submitting a transaction. Specifically I'm wondering if recent nonce improvements by @greg-nagy make our nonce management inside of handlePendingTransaction no longer relevant.

To Test

  • [ ] Use the wallet - transactions initiated by you (send / swap) should immediately show up in wallet activity after being sent
  • [ ] Send / swap from metamask while having tally side-loaded - the transaction should eventually show up in activities (from testing seems like it takes less than 2 minutes)

Latest build: extension-builds-2728 (as of Wed, 07 Dec 2022 02:50:28 GMT).

0xDaedalus avatar Dec 07 '22 02:12 0xDaedalus

Hmmmn I only tested on Optimism and Arbitrum - wondering if different chains have different resolution times

0xDaedalus avatar Dec 07 '22 14:12 0xDaedalus

Moving to draft while I investigate 😭

0xDaedalus avatar Dec 12 '22 14:12 0xDaedalus

Going to go ahead and close this out for now—we've made improvements in a few other places in our resource usage, and the underlying functionality is useful. We can always come back to it if it becomes relevant again.

Shadowfiend avatar Apr 17 '23 14:04 Shadowfiend