SD-102: Fix allowance fee not shown in pending transaction#92
Conversation
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
| const { data: needsApproval } = useQuery({ | ||
| queryKey: chainConfig && walletAddress && shouldCheckAllowance ? | ||
| getQueryKey.allowanceCheck(token.address, chainConfig.id.toString(), walletAddress, amount.toString()) : [], | ||
| queryFn: !publicClient || !chainConfig?.shielderConfig || !walletAddress || !shouldCheckAllowance ? | ||
| skipToken : | ||
| async (): Promise<boolean> => { | ||
| try { | ||
| const shielderConfig = chainConfig.shielderConfig; | ||
| if (!shielderConfig) return false; | ||
|
|
||
| const allowance = await publicClient.readContract({ | ||
| address: token.address, | ||
| abi: erc20Abi, | ||
| functionName: 'allowance', | ||
| args: [walletAddress, shielderConfig.shielderContractAddress], | ||
| }); | ||
| return allowance < amount; | ||
| } catch { | ||
| return false; | ||
| } | ||
| }, | ||
| enabled: shouldCheckAllowance && !!publicClient && !!chainConfig?.shielderConfig && !!walletAddress, | ||
| }); |
There was a problem hiding this comment.
Previously I was checking if the user actually needed allowance approval before calculating and showing the estimate. The issue with this approach is that if we want to display the estimate after the user already approved the allowance, this check returns false, so the fee estimate isn’t calculated or shown. This happened for example with pending transactions where the allowance fee wasn’t displayed.
I think it’s fine to always show the estimate, even in edge cases where the allowance check isn’t needed. We’re showing the maximum possible fees anyway, so if the allowance fee doesn’t apply, the actual fee will just be lower, which is acceptable in my opinion.
rAskVAL
force-pushed
the
sd-102-fees-improvements
branch
from
da5dc31 to
8ed66be
Compare
| useEffect(() => { | ||
| const handleEsc = (e: KeyboardEvent) => { | ||
| if (e.key === 'Escape' && !nonDismissible) { | ||
| // Only handle ESC key if this is the top modal and it's not non-dismissible |
There was a problem hiding this comment.
No need for this comment imo - the code explains that perfectly clearly.
src/domains/shielder/components/TokenList/Modals/SendModal/SendModal.tsx
Outdated
Show resolved
Hide resolved
rAskVAL
force-pushed
the
sd-102-fees-improvements
branch
from
88636b8 to
11d8898
Compare
2 participants
No description provided.