Display error when relay fee is greater than xdai amount #2219

hexyls · 2021-08-23T06:57:37Z

Displays an error when relay fee is greater than xdai amount for both buys and deposit for xdai markets when connected to the relay.

Also added a warning for when a user is attempting to transaction through the relay with an xDai balance lower than the relay fee.

Testing (all using the relay/omen account):

When buying/depositing a tiny amount (0.0001) on a Dai market the following error message is displayed: "Relay fee is greater than buy amount."
Withdraw all Dai from your Omen account, when buying/depositing/selling/creating the following message is displayed: "Relay fee is greater than your DAI balance, please top up your Omen account."
Switch to Rinkeby and double check that these messages are not displayed in any markets

pimato · 2021-08-24T14:39:24Z

Testing (all using the relay/omen account):

When buying/depositing a tiny amount (0.0001) on a Dai market the following error message is displayed: "Relay fee is greater than buy amount."
Withdraw all Dai from your Omen account, when buying/depositing/selling/creating the following message is displayed: "Relay fee is greater than your DAI balance, please top up your Omen account."
Switch to Rinkeby and double check that these messages are not displayed in any markets

pimato · 2021-08-24T14:46:23Z

I would prefer the usage of our error message below the inputfield like this:

Figma:
Design Specification

hexyls · 2021-08-25T05:53:51Z

@pimato I moved the relay fee error under inputs but I'm unable to access that design spec atm 🤔

pimato · 2021-08-25T08:22:15Z

@pimato I moved the relay fee error under inputs but I'm unable to access that design spec atm 🤔

Sorry @hexyls , fixed!

pimato · 2021-08-25T08:54:14Z

I would add a new error message for xDai tight user who have not deposit Dai into the Omen account. Right now we are just saying Insufficient balance if the user has no balance for the specific asset. I think before that we should show the message Insufficient Dai in your Omen Account for deposit/withdraw/buy/sell if the Omen Account is not funded:

Priority of error message for xDai tight:

If Omen Account has no DAI show Insufficient Dai in your Omen Account
If Omen Account Dai balance > 0 we do the classic error message path like insufficient balance if the user inputs a higher amount he has available and so on.

Mi-Lan

@hexyls Great job with service! Code looks good added some stuff wondering what you think.

Mi-Lan · 2021-08-26T12:43:26Z

app/src/hooks/useRelay.tsx

+    return () => {
+      status.active = false
+    }


Wondering just out of learning purposes why are you returning this status.active=false. And what does it do?

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 relayService.getInfo() request finishes.

The line you commented will make sure status.active is set to false if the hook is updated, so in our scenario above once relayService.getInfo() is finished we check status.active, if it is true that means the hook has not updated since the request was made and it is safe to update the state.

Mi-Lan · 2021-08-26T13:01:48Z

app/src/components/market/common_sections/message_text/warning_message/index.tsx

@@ -11,7 +11,7 @@ const Wrapper = styled.div<{ margin?: boolean }>`
  border: ${({ theme }) => theme.borders.borderLineDisabled};
  align-content: center;
  padding: 4px 20px;
-  margin-bottom: ${props => (!props.margin ? '20px' : '0')};
+  margin-bottom: ${props => (props.margin ? '20px' : '0')};


I couldn't find figma designs but seems that because you flipped evaluation in some cases where it was previously set we have double margins...Like if value inputted in negative on market_buy component. Or this is how its supposed to be I couldn't find figma @pimato ?

@pimato Also seems that color on this error message when value is negative is different then the error message on form...

filled this back to !props.margin and changed up the alert color 👍

Mi-Lan · 2021-08-26T13:06:13Z

app/src/components/common/form/textfield_custom_symbol/index.tsx

  formField: any
  symbol: any
  shouldDisplayMaxButton?: boolean
  onClickMaxButton?: () => void
  style?: CSSProperties | undefined
 }

-const FieldWrapper = styled.div`
+const FieldWrapper = styled.div<{ error?: boolean }>`


Could we make this into style(TYPE.BodyRegular) and remove couple of prperties like font-size,font-weight,line-height

mmmm will leave this one, styled(TYPE.bodyRegular) doesn't appear to apply font styling in the same way using the> input selector does

Mi-Lan · 2021-08-26T13:23:37Z

app/src/components/market/market_create/steps/items/funding_and_fee_step.tsx

+      : 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'


@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?

if you consider this makes sense to do we can even move it also into separate issue...

Mi-Lan · 2021-08-26T14:03:34Z

Screen.Recording.2021-08-26.at.15.57.42.mov

@hexyls Also seems that message gets stuck and I can't even buy with proper amount inputted..After I triggered insufficient dai in your omen account

hexyls · 2021-09-02T06:05:27Z

@Mi-Lan for that last one where the error stays - do you know how to reproduce it? I managed to get it to show up once but haven't seen it since 🤔

fix: display error when relay is greater than xdai amount

f996bf7

hexyls marked this pull request as draft August 23, 2021 06:57

fix: add relay balanace warning to buys, sells and pooling

1601119

hexyls marked this pull request as ready for review August 24, 2021 06:05

hexyls requested review from Mi-Lan and pimato August 24, 2021 06:05

Merge branch 'master' into fix/2189

98b11d4

fix: move error message under text input

5a5ac0c

Merge branch 'master' into fix/2189

7e0c4f5

hexyls added 2 commits August 26, 2021 16:02

fix: update input styling

d56b3ad

fix: missing error field

db2bdae

Mi-Lan suggested changes Aug 26, 2021

View reviewed changes

hexyls added 2 commits September 2, 2021 16:05

fix: revert margin, change alert color

22a51a9

Merge branch 'master' into fix/2189

5556aff

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Display error when relay fee is greater than xdai amount #2219

Display error when relay fee is greater than xdai amount #2219

hexyls commented Aug 23, 2021 •

edited

Loading

pimato commented Aug 24, 2021 •

edited

Loading

pimato commented Aug 24, 2021

hexyls commented Aug 25, 2021

pimato commented Aug 25, 2021

pimato commented Aug 25, 2021 •

edited

Loading

Mi-Lan left a comment

Mi-Lan Aug 26, 2021

hexyls Sep 2, 2021 •

edited

Loading

Mi-Lan Aug 26, 2021

Mi-Lan Aug 26, 2021

hexyls Sep 2, 2021

Mi-Lan Aug 26, 2021

hexyls Sep 2, 2021

Mi-Lan Aug 26, 2021

Mi-Lan Aug 26, 2021 •

edited

Loading

Mi-Lan commented Aug 26, 2021

hexyls commented Sep 2, 2021

Display error when relay fee is greater than xdai amount #2219

Are you sure you want to change the base?

Display error when relay fee is greater than xdai amount #2219

Conversation

hexyls commented Aug 23, 2021 • edited Loading

pimato commented Aug 24, 2021 • edited Loading

pimato commented Aug 24, 2021

hexyls commented Aug 25, 2021

pimato commented Aug 25, 2021

pimato commented Aug 25, 2021 • edited Loading

Mi-Lan left a comment

Choose a reason for hiding this comment

Mi-Lan Aug 26, 2021

Choose a reason for hiding this comment

hexyls Sep 2, 2021 • edited Loading

Choose a reason for hiding this comment

Mi-Lan Aug 26, 2021

Choose a reason for hiding this comment

Mi-Lan Aug 26, 2021

Choose a reason for hiding this comment

hexyls Sep 2, 2021

Choose a reason for hiding this comment

Mi-Lan Aug 26, 2021

Choose a reason for hiding this comment

hexyls Sep 2, 2021

Choose a reason for hiding this comment

Mi-Lan Aug 26, 2021

Choose a reason for hiding this comment

Mi-Lan Aug 26, 2021 • edited Loading

Choose a reason for hiding this comment

Mi-Lan commented Aug 26, 2021

hexyls commented Sep 2, 2021

hexyls commented Aug 23, 2021 •

edited

Loading

pimato commented Aug 24, 2021 •

edited

Loading

pimato commented Aug 25, 2021 •

edited

Loading

hexyls Sep 2, 2021 •

edited

Loading

Mi-Lan Aug 26, 2021 •

edited

Loading