fix(swap): Fix limit price precision / denomination #2771
Conversation
michaeljscript
requested review from
jorbuedo,
SorinC6,
banklesss and
stackchain
michaeljscript
changed the title
| fullDecValue = `${int}${decimalSeparator}${dec.slice(0, precision)}1` | ||
| value = `${int}${decimalSeparator}${dec.slice(0, precision)}` | ||
| } | ||
|
|
||
| const fullDecFormat = new BigNumber(fullDecValue.replace(decimalSeparator, '.')).toFormat() | ||
| const input = isDec ? fullDecFormat.slice(0, -1) : fullDecFormat |
There was a problem hiding this comment.
move inside the if, add a comment that the trailing is removed inside the hasDecimals logic
There was a problem hiding this comment.
You'll have to declare input as let before and move both lines inside the if
Which is why I preferred everything as const and the ternaries 😅
There was a problem hiding this comment.
BigNumber is object so const/let won't make that difference cuz you still can mutate it, but I also prefer and agree on not changing after declaring with initialization. To look even better I'd move everything dec / not decimal and return based on that, we will have some duplication, but readability is more important than duplication.
There was a problem hiding this comment.
Maybe it's because I wrote it, but it looked more clear to me before. Now with the lets, an if dividing everything, the early return and a couple lines duplicated because of it, looks harder to follow to me.
| @@ -129,6 +129,9 @@ describe('Quantities', () => { | |||
| expect(Quantities.parseFromText('55.10', 3, english)).toEqual(['55.10', '55100']) | |||
|
|
|||
| expect(Quantities.parseFromText('ab1.5c,6.5', 3, english)).toEqual(['1.56', '1560']) | |||
|
|
|||
| expect(Quantities.parseFromText('1.23456', 0, english, 3)).toEqual(['1.234', '1.234']) | |||
| expect(Quantities.parseFromText('1.23456', 2, english, 3)).toEqual(['1.234', '123.4']) | |||
There was a problem hiding this comment.
does it have the case for 1.0000 / 1.0 ?
There was a problem hiding this comment.
Yes, it has a test for expect(Quantities.parseFromText('55.0', 3, english)).toEqual(['55.0', '55000'])
|
|
||
| const value = isDec ? `${parts[0]}${decimalSeparator}${parts[1].slice(0, denomination)}` : sanitized | ||
| const quantity = new BigNumber(value.replace(decimalSeparator, '.')) |
There was a problem hiding this comment.
| const quantity = new BigNumber(value.replace(decimalSeparator, '.')) | |
| const quantity = asQuantity(new BigNumber(value.replace(decimalSeparator, '.')) |
| .shiftedBy(denomination) | ||
| .toString(10) | ||
| .toString(10) as Balance.Quantity |
There was a problem hiding this comment.
| .toString(10) as Balance.Quantity | |
| .toString(10)) |
| let fullDecFormat = new BigNumber(fullDecValue.replace(decimalSeparator, '.')).toFormat() | ||
| let input = fullDecFormat | ||
|
|
||
| if (parts.length >= 2) { |
There was a problem hiding this comment.
maybe if parts.length <= 1 return earlier and then process the decimals ?
There was a problem hiding this comment.
That's now updated
Resolves YOMO-855