[Claim] Approve flow - GNO approve #2130
Conversation
|
W3stside
force-pushed
the
claim-approve
branch
from
a6f3023 to
1a9c34a
Compare
| @@ -30,3 +31,16 @@ export function useApproveCallbackFromTrade( | |||
|
|
|||
| return useApproveCallback(openTransactionConfirmationModal, closeModals, amountToApprove, vaultRelayer) | |||
| } | |||
|
|
|||
| export function useApproveCallbackFromClaim( | |||
There was a problem hiding this comment.
Claim functionalty's useApproveCallback - opted NOT to hardcode MAX GNO uint256 here and instead allow passing in values.
There was a problem hiding this comment.
fine with me, we might even need this if users start to complaint for using MAX on their investments
| @@ -0,0 +1,38 @@ | |||
| import { useMemo } from 'react' | |||
There was a problem hiding this comment.
originally coded this because I thought we were approving only investing amount but it is no longer in use (as we use max unit256) - figured I'd leave it as it's useful for if we switch to this and/or can use in other parts of the app if we want to do exact approvals (it calcs current approved vs amount requested)
happy to remove.
| @@ -0,0 +1,40 @@ | |||
| import { useState, useCallback } from 'react' | |||
There was a problem hiding this comment.
simply extrapolated out of swapmod where it was being used for approvals.
it hasnt been used in SwapMod - only in Claiming - as i figured it's out of scope of this PR (this kinda is too but hey whatever it's way nicer this way)
W3stside
changed the title
W3stside
force-pushed
the
claim-approve
branch
from
b21f7af to
1e0bac9
Compare
| @@ -30,3 +31,16 @@ export function useApproveCallbackFromTrade( | |||
|
|
|||
| return useApproveCallback(openTransactionConfirmationModal, closeModals, amountToApprove, vaultRelayer) | |||
| } | |||
|
|
|||
| export function useApproveCallbackFromClaim( | |||
There was a problem hiding this comment.
fine with me, we might even need this if users start to complaint for using MAX on their investments
| @@ -30,3 +31,16 @@ export function useApproveCallbackFromTrade( | |||
|
|
|||
| return useApproveCallback(openTransactionConfirmationModal, closeModals, amountToApprove, vaultRelayer) | |||
| } | |||
|
|
|||
| export function useApproveCallbackFromClaim( | |||
There was a problem hiding this comment.
not sure if the two hooks belong in the same file, but just nitpicking
| * @description returns allowance of a token against a spender | ||
| * and the remaining allowance IF allowance > remaining. Else is null | ||
| */ | ||
| export default function useRemainingAllowanceToApprove({ amountToApprove, spender }: Params) { |
There was a problem hiding this comment.
would it be good to add a return type?
There was a problem hiding this comment.
I like you did this hook, I like sugar
|
|
||
| return { | ||
| allowance, | ||
| needsApproval: !allowance || amountToApprove?.greaterThan(allowance), |
There was a problem hiding this comment.
or remainingAllowanceToApprove greater than ZERO. but yours is good too
| import { useOpenModal, useCloseModals, useModalOpen } from 'state/application/hooks' | ||
| import { ApplicationModal } from 'state/application/reducer' | ||
|
|
||
| export default function useTransactionConfirmationModal( |
There was a problem hiding this comment.
So this is just to use this hook, and allow to open the confirmation with different operation types and messages for the same component?
im kind of ok, although i don't see completly the point since you only used it with one type, so probably using TransactionConfirmationModal would be not that bad
There was a problem hiding this comment.
yeah the code inside SwapMod, lines 150 - 167 + the modal component are identical in claim. and likely anywhere else with approve/transaction modal stuff
|
merging now to clean up, can re-visit this later |
| import Row from 'components/Row' | ||
|
|
||
| // Max approve amount | ||
| const MAX_GNO_UINT256 = CurrencyAmount.fromRawAmount(GNO[SupportedChainId.RINKEBY], MaxUint256) |
There was a problem hiding this comment.
Does it make sense to hard code for each token that'll be approved?
Also, you are relying on rinkeby's GNO. Won't that be a problem?
I know, I know, not likely a problem.
I just thought this would work like approval for any other token before trading
There was a problem hiding this comment.
good point, it will be an issue on another network..
| OperationType.APPROVE_TOKEN | ||
| ) | ||
|
|
||
| const [approveState, approveCallback] = useApproveCallbackFromClaim( |
There was a problem hiding this comment.
I don't fully get it. Why do you need a special approval callback for claims?
Isn't it like regular approvals (approve max always)?
There was a problem hiding this comment.
guess it doesn't really, @anxolin made a similar comment about it. I'll look into reverting youre right
|
I was able to approve GNO. |
Summary
Part of #2087 [approval]
Approve GNO for investing.
Screenshots
Approving
Approved
To Test
Incoming