feat(bridge-ui-v2): Fixed input validations and catching additional errors #14331
Conversation
…tch additional errors
KorbinianK
requested review from
jscriptcoder,
adaki2004,
dantaik and
cyberhorsey
Codecov Report
@@ Coverage Diff @@
## alpha-4 #14331 +/- ##
===========================================
+ Coverage 67.75% 77.20% +9.45%
===========================================
Files 88 56 -32
Lines 3045 1452 -1593
Branches 112 112
===========================================
- Hits 2063 1121 -942
+ Misses 859 299 -560
+ Partials 123 32 -91
see 32 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
| // Todo: viem does not seem to detect UserRejectError | ||
| warningToast($t('bridge.errors.rejected')); | ||
| break; | ||
| case err instanceof TransactionExecutionError && err.shortMessage === 'User rejected the request.': |
There was a problem hiding this comment.
Not quite understand why this is needed? denying tx signature is handled in the bridge method which re-throws UserRejectedRequestError getting caught here already
There was a problem hiding this comment.
As discussed, we can move this logic to the ERC20Bridge but for some reason the wrong error is thrown and we need to catch it
| @@ -33,17 +37,19 @@ | |||
| let inputBox: InputBox; | |||
| let computingMaxAmount = false; | |||
|
|
|||
| onMount(() => { | |||
| clearAmount(); | |||
There was a problem hiding this comment.
Why do we want to clear on mount? If the reason is to clear the state, before we actually wanted to leave the amount on purpose when switching pages
There was a problem hiding this comment.
It is not persisting (visibly) right now. We either clear it here or we make sure it is also displayed.
| // Public API | ||
| export function clearAmount() { | ||
| inputBox.clear(); | ||
| $enteredAmount = BigInt(0); | ||
| } | ||
|
|
||
| export async function validateAmount(token = $selectedToken, fee = $processingFee) { | ||
| $insufficientBalance = false; | ||
| $insufficientAllowance = false; |
There was a problem hiding this comment.
Is this fixing a problem? We're gonna validate these two, we start assuming there is no issue and then validate
There was a problem hiding this comment.
yes, it helps with the flickering of the buttons. In the future we could introduce a loading or disabled status
| if (!tokenAddress) return; | ||
|
|
||
| // Check for allowance | ||
| $insufficientAllowance = await erc20Bridge.requireAllowance({ |
There was a problem hiding this comment.
Why do we need to do this here?, we already get the error when running the estimation, don't we?, is this not redundant?
There was a problem hiding this comment.
As discussed in the call, the gas estimation does not throw an error, so we need to call it.
Bridging ERC20 Tokens was not allowing to send all tokens, as it calculated the fees as if we were paying with the token
Also reordered some validations to stop other validations if we already hit an "error"