-
Notifications
You must be signed in to change notification settings - Fork 217
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
[ADP-3479] Add deposit wallet payment page #4837
Conversation
608ad20
to
fb9289a
Compare
6b4010d
to
28ab6b0
Compare
5e4340b
to
bb3d325
Compare
599efb3
to
69c34dc
Compare
4ef48d6
to
0200b5a
Compare
6a21e81
to
bf9a4a7
Compare
2876118
to
925cd15
Compare
bf9a4a7
to
bb8e1d7
Compare
|
||
showLovelaceAsAda :: Integral a => a -> String | ||
showLovelaceAsAda c = | ||
showFFloatAlt @Double (Just 2) (fromIntegral c / 1_000_000) "" |
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.
It seems odd that I can enter 10.000001
as payment and not see the result:
showFFloatAlt @Double (Just 2) (fromIntegral c / 1_000_000) "" | |
showFFloatAlt @Double (Just 6) (fromIntegral c / 1_000_000) "" |
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.
Also, I wonder if the Double
precision could be problematic...
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.
The maximum amount of Lovelace one can define is bounded to be a Word64
if I am not mistaken, which should be representable as Double
with some loss of precision. Do we want an exact representation? If so then this code is incorrect. If we don't really care which I think we don't because this UI will probably not go into production as is, then this is fine.
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.
I am tempted to approve this PR even though I did not went through it in details (it's adding 2000 lines which is way too much IMO), for the sake of moving forward and given the fact we've actually demoed this feature last week.
5718b41
to
8f6fd06
Compare
boxes
to get a better readabilityADP-3479