web
web copied to clipboard
feat: improve useSendDetails loading, early bail and debounce
Description
Started as a simple fix for early bailing out of estimating fees when funds are insufficient (see this sentry issue, but noticed our debouncing in useSendFees is borked out of sanity i.e:
- Have a balance of 0.6 ETH
- Type 0.5 which is happy
- Now either clear the
0.
part, or add a 9 at the end which would result in insufficient balance. The insufficient balance will be displayed for a second or so, only for the previously debounced promise to settle, its side effects run, and the send modal being wrongly in a valid balance state.
The root cause is that while async functions may look like good ol' functions from a syntactic sugar point of view, they are promises, and promises are not cancellable.
Us calling .cancel()
on the debounced "function" doesn't really do anything as far as canceling promises goes. This could be solved by using bluebird or other fancy APIs like AbortController
which both come with their set of issues (package and API overhead for the former, boilerplaty and limited to the fetch API for the latter).
Fixed by going on a proper refactor of useSendFees which:
- decouples the network logic and the input change logic.
handleInputChange
's sole responsibility is now setting values in the form, and the newly introduced query handles fetching fees and error-handling - things are still debounced, though in a more simple fashion, by debouncing the
amountCryptoPrecision
input
And obviously fixes the issue this initially went for, which is early bailing out of estimating fees when there is insufficient balance.
Pull Request Type
- [ ] :bug: Bug fix (Non-breaking Change: Fixes an issue)
- [ ] :hammer_and_wrench: Chore (Non-breaking Change: Doc updates, pkg upgrades, typos, etc..)
- [x] :nail_care: New Feature (Breaking/Non-breaking Change)
Issue (if applicable)
closes #6877
Risk
High Risk PRs Require 2 approvals
What protocols, transaction types or contract interactions might be affected by this PR?
Testing
- Loading states are still happy i.e is still reflected immediately on user input
- Insufficient funds checks are still happy:
- Insufficient funds (either for token or native asset sends) are still surfaced with a "Insufficient funds" error
- Token sends without enough funds for gas are still surfaced with a "Not enough native token for gas" error
- Send max is still happy:
- Token sends set the absolute max
- Native asset still do fees deduction to set the actual max
- Debouncing is actually working, and previous requests completing do not end up in a wrong state i.e
- Having an initial input which settles to a happy state (enough funds), but a subsequent one that completes to a fail (not enough funds) correctly settles to a fail state visually, without a flash of wrong state
- Having an initial input which settles to a sad state (not enough funds), but a subsequent one that completes to a happy (enough funds) state correctly settles to a happy state visually, without a flash of wrong state
Engineering
- Input when not enough funds does not trigger an estimate request
- ☝🏽
Operations
Screenshots (if applicable)
- Having an initial input which settles to a happy state (enough funds), but a subsequent one that completes to a fail (not enough funds) correctly settles to a fail state visually, without a flash of wrong state
https://github.com/shapeshift/web/assets/17035424/863a9398-7db4-4642-9aaa-b796951b6624
- Having an initial input which settles to a sad state (not enough funds), but a subsequent one that completes to a happy (enough funds) state correctly settles to a happy state visually, without a flash of wrong state
https://github.com/shapeshift/web/assets/17035424/a946cea0-ef36-493d-9311-6984b773e441
- Input when not enough funds does not trigger an estimate request
https://github.com/shapeshift/web/assets/17035424/d49a057b-c312-4d52-acf4-99a3db4f3ffe
- Insufficient funds (either for token or native asset sends) are still reflected as "Insufficient funds"
- Token sends without enough funds for gas are still surfaced with a "Not enough native token for gas" error
- Token sends set the absolute max
- Native asset still do fees deduction to set the actual max
Testing notes
✅ Send non-native max ✅ Send not max
🚫 NaN
appearing when attempting to enter fiat amount (confirmed not in prod):
https://github.com/shapeshift/web/assets/125113430/72f0a8c9-07e1-40fe-b9df-f7cd7954a45a
🚫 Insufficient Funds
appears when clearing the fiat amount:
🚫 Allows user to send 0 amount:
https://github.com/shapeshift/web/assets/125113430/93a5688e-f337-4fa1-8e3e-53528aa6899c
🚫 Allows user to enter negative amount, displays Insufficient Funds
:
🚫 Unable to send max on native token:
Stellar testing @woodenfurniture, ty!
🚫 NaN appearing when attempting to enter fiat amount (confirmed not in prod) 🚫 Insufficient Funds appears when clearing the fiat amount
Both fixed in 271e1adb9e - https://jam.dev/c/d54a8ec9-29b1-46fc-808c-2adcc72d220c
🚫 Allows user to send 0 amount 🚫 Allows user to enter negative amount, displays Insufficient Funds
Both are also in develop/prod - quite wary about introducing more issues here so stacked out on top of this instead: https://github.com/shapeshift/web/pull/6930. See this Jam for the fix this PR introduces: https://jam.dev/c/542c2419-7c78-4339-a27e-77f63666e490 though I'll still need to test the send max negative amounts there. My main concern is that negative amounts can sometimes be present when clicking max, fee deduction is expected (i.e native asset) but fees are greater than amount, this is valid and expected and we could break the error messaging here).
🚫 Unable to send max on native token
That one is specific to Arb fees which are always derp and very volatile by nature and isn't specific to this PR, see jam against prod (note, I monkey patched the staleTime of fees estimation to 2s for ease of testing), though this PR made the behavior more obvious (and actually correct, vs. the infinite loading state on prod):
https://jam.dev/c/b01da49b-485d-4f43-b6b5-59982a609d92
If you test with another native asset, things should be consistently happy:
https://jam.dev/c/a8121673-3cf4-43df-8fa0-a9987b09a256