-
Notifications
You must be signed in to change notification settings - Fork 78
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Display error when relay fee is greater than xdai amount #2219
base: master
Are you sure you want to change the base?
Changes from all commits
f996bf7
1601119
98b11d4
5a5ac0c
7e0c4f5
d56b3ad
db2bdae
22a51a9
5556aff
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,7 +12,7 @@ import { | |
MAX_MARKET_FEE, | ||
} from '../../../../../common/constants' | ||
import { useConnectedWeb3Context } from '../../../../../contexts' | ||
import { useCollateralBalance, useCpkAllowance, useCpkProxy } from '../../../../../hooks' | ||
import { useCollateralBalance, useCpkAllowance, useCpkProxy, useRelay } from '../../../../../hooks' | ||
import { useGraphMarketsFromQuestion } from '../../../../../hooks/graph/useGraphMarketsFromQuestion' | ||
import { BalanceState, fetchAccountBalance } from '../../../../../store/reducer' | ||
import { MarketCreationStatus } from '../../../../../util/market_creation_status_data' | ||
|
@@ -253,6 +253,8 @@ const FundingAndFeeStep: React.FC<Props> = (props: Props) => { | |
const { proxyIsUpToDate, updateProxy } = useCpkProxy(collateral.address === pseudoNativeAssetAddress) | ||
const isUpdated = RemoteData.hasData(proxyIsUpToDate) ? proxyIsUpToDate.data : true | ||
|
||
const { relayFeeGreaterThanAmount, relayFeeGreaterThanBalance } = useRelay(amount || new BigNumber(0), collateral) | ||
|
||
useEffect(() => { | ||
dispatch(fetchAccountBalance(account, provider, collateral)) | ||
}, [dispatch, account, provider, collateral]) | ||
|
@@ -285,10 +287,14 @@ const FundingAndFeeStep: React.FC<Props> = (props: Props) => { | |
? null | ||
: !maybeCollateralBalance.eq(collateralBalance) | ||
? null | ||
: relayFeeGreaterThanBalance | ||
? 'Insufficient Dai in your Omen Account' | ||
: maybeCollateralBalance.isZero() && funding.gt(maybeCollateralBalance) | ||
? `Insufficient balance` | ||
: funding.gt(maybeCollateralBalance) | ||
? `Value must be less than or equal to ${collateralBalanceFormatted} ${collateral.symbol}` | ||
: relayFeeGreaterThanAmount | ||
? 'Relay fee is greater than buy amount' | ||
Comment on lines
+290
to
+297
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @hexyls Also one more consideration. I was looking through the codebase now and seems that in this pr and throught the codebase in components(marketBuy,marketSell,fundingAndFee and MarketPoolLiqudity) we have almost exactly the same amount error evaluations. Wondering does it make sense to make separate component or service dedicated to it? So we don't mess up in future if we have to add some new error handling. What do you think? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if you consider this makes sense to do we can even move it also into separate issue... |
||
: null | ||
|
||
const isCreateMarketbuttonDisabled = | ||
|
@@ -472,6 +478,7 @@ const FundingAndFeeStep: React.FC<Props> = (props: Props) => { | |
/> | ||
</CurrenciesWrapper> | ||
<TextfieldCustomPlaceholder | ||
error={!!amountError} | ||
formField={ | ||
<BigNumberInput | ||
decimals={collateral.decimals} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,67 @@ | ||
import { BigNumber } from 'ethers/utils' | ||
import { useEffect, useState } from 'react' | ||
|
||
import { useConnectedWeb3Context } from '../contexts/connectedWeb3' | ||
import { RelayService } from '../services' | ||
import { getNativeAsset } from '../util/networks' | ||
import { Token } from '../util/types' | ||
|
||
interface Status { | ||
active: boolean | ||
} | ||
|
||
interface RelayServiceResponse { | ||
relayAddress: string | ||
relayFee: BigNumber | ||
relayFeeGreaterThanAmount: boolean | ||
relayFeeGreaterThanBalance: boolean | ||
} | ||
|
||
export const useRelay = (amount?: BigNumber, collateral?: Token): RelayServiceResponse => { | ||
const { account, balances, networkId, relay } = useConnectedWeb3Context() | ||
|
||
const [relayAddress, setRelayAddress] = useState('') | ||
const [relayFee, setRelayFee] = useState(new BigNumber('0')) | ||
const [relayFeeGreaterThanAmount, setRelayFeeGreaterThanAmount] = useState(false) | ||
const [relayFeeGreaterThanBalance, setRelayFeeGreaterThanBalance] = useState(false) | ||
|
||
const fetchRelayInfo = async (status?: Status) => { | ||
const relayService = new RelayService() | ||
const { address, fee } = await relayService.getInfo() | ||
if (!status || status.active) { | ||
setRelayAddress(address) | ||
|
||
const feeBN = new BigNumber(fee) | ||
setRelayFee(feeBN) | ||
setRelayFeeGreaterThanBalance(feeBN.gt(balances.xDaiBalance)) | ||
} | ||
} | ||
|
||
useEffect(() => { | ||
const status = { active: true } | ||
if (account && relay) { | ||
fetchRelayInfo(status) | ||
} else { | ||
setRelayAddress('') | ||
setRelayFee(new BigNumber('0')) | ||
setRelayFeeGreaterThanBalance(false) | ||
} | ||
return () => { | ||
status.active = false | ||
} | ||
Comment on lines
+49
to
+51
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wondering just out of learning purposes why are you returning this status.active=false. And what does it do? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this prevents against race conditions when requesting data, e.g. imagine we start requesting data while the user is using the relay and the user switches accounts mid request - it is possible that the state could be updated with data we don't want once the The line you commented will make sure status.active is set to false if the hook is updated, so in our scenario above once |
||
// eslint-disable-next-line | ||
}, [account, networkId, relay]) | ||
|
||
useEffect(() => { | ||
if (account && relay && collateral && amount) { | ||
const native = getNativeAsset(networkId, relay) | ||
const feeGreaterThanAmount = | ||
relay && amount && !amount.isZero() && relayFee.gt(amount) && collateral.address === native.address | ||
setRelayFeeGreaterThanAmount(feeGreaterThanAmount) | ||
} else { | ||
setRelayFeeGreaterThanAmount(false) | ||
} | ||
}, [amount, collateral, account, relay, networkId, relayFee]) | ||
|
||
return { relayAddress, relayFee, relayFeeGreaterThanAmount, relayFeeGreaterThanBalance } | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we make this into style(TYPE.BodyRegular) and remove couple of prperties like font-size,font-weight,line-height
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mmmm will leave this one, styled(TYPE.bodyRegular) doesn't appear to apply font styling in the same way using the
> input
selector does