chore: reduce default wait interval

bcnmy / biconomy-client-sdk

Biconomy SDK is a plug & play toolkit for dApps to build transaction legos that enable a highly customised one-click experience for their users

MIT License

76 stars 78 forks source link

chore: reduce default wait interval #550

Closed joepegler closed 4 weeks ago

joepegler commented 3 months ago

PR-Codex overview

This PR focuses on updating the version of the package, adjusting the default polling interval, and refining error messages in the Bundler class for better clarity.

Detailed summary

Added a new version 4.6.4 to CHANGELOG.md and noted a patch change to shorten the default pollInterval.
Updated the version in package.json from 4.6.3 to 4.5.6.
Introduced a constant POLL_INTERVAL set to 2000 in src/bundler/Bundler.ts.
Replaced the hardcoded default polling interval with POLL_INTERVAL.
Improved error messages in the Bundler class for clarity during receipt retrieval failures.

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

github-actions[bot] commented 3 months ago

size-limit report 📦

Path	Size
core (esm)	55.9 KB (0%)
core (cjs)	61.08 KB (0%)
account (tree-shaking)	53.38 KB (0%)
bundler (tree-shaking)	2.57 KB (0%)
paymaster (tree-shaking)	2.21 KB (0%)
modules (tree-shaking)	40.98 KB (0%)

livingrockrises commented 3 months ago

Whats the benefit of reducing default polling to 2 ms from 5?

we removed the hard coded value but the constant variable can be changed by env var setting?

joepegler commented 2 months ago

Hey,

So this number we're picking is just a default. Devs can go and set this themselves but we don't really document it widely so they'd have to search it out so in all likelihood they probably don't customise it all that often.

This value we're discussing is, in effect, the 'max amount of unnecessary milliseconds after a tx has been mined that we make a user wait for the receipt'. We should set it to as small a number as possible (without causing 502 timeouts on the backend). It has a big impact on the effective latency that end users notice imo.

I think getting this number right is pretty important, and it might be different from chain to chain based on backend load, block-mining-times per chain etc etc.

joepegler commented 2 months ago

The problem is: 'clients don't know when the backend has mined a tx'. The current solution is for the client to send requests every 'n' milliseconds to check if the backend is done (polling). This PR reduces 'n'. A better (definitely longer term) solution might be to switch the backend to a websocket (which has pub/sub bi-directional flow of information), so the backend can notify the client when txs have been mined, instead of the client having to ask on an interval (which unnecssarily adds to the wait time)

joepegler commented 2 months ago

Also Chirag in this PR I'm not added an env var here. I've just added the constant directly in the file - there's no change in that regard.