-
Notifications
You must be signed in to change notification settings - Fork 9
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
IOS-8050: Apply rounding to the Hedera fee value #850
Conversation
Signed-off-by: Andrey Fedorov <[email protected]>
let feeValue = exchangeRate.nextHBARPerUSD * feeBase * Constants.maxFeeMultiplier | ||
let feeAmount = Amount(with: walletManager.wallet.blockchain, value: feeValue) | ||
// Hedera fee calculation involves conversion from USD to HBar units, which ultimately results in a loss of precision. | ||
// Therefore, the fee value is always approximate and rounding of the fee value is mandatory. | ||
let feeRoundedValue = feeValue.rounded(blockchain: walletManager.wallet.blockchain, roundingMode: .up) | ||
let feeAmount = Amount(with: walletManager.wallet.blockchain, value: feeRoundedValue) |
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.
Вот тут я вообще не понял:
я изначально добавил .rounded(blockchain: walletManager.wallet.blockchain, roundingMode: .up)
только для exchangeRate.nextHBARPerUSD
- так как именно этот компонент привносил избыточную точность, он был чем-то вроде 17.9466625137819201082727172
Я округляю его до scale равного Hedera decimal count (это 8) - и получаю 17.94666251
Затем домножаю в 424 строке эту величину на feeBase
(0.05
в случае простого трансфера) и maxFeeMultiplier
(1.1
) и ожидаю, что scale результата будет меньше или равен наибольшему scale множителей (то есть 8)
В итоге получаю 0.98706643805
со scale равным 11, что естественно приводит к дробному числу во fromAmount
¯_(ツ)_/¯
Поэтому пришлось применить округление на финальное значение
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.
Логично, это же просто число, округленное, после последующих операций не гарантируется сохранение оригинального числа знаков после запятой
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.
Так может тогда имеет смысл оставить округление только на последнем этапе, чтобы не терять точность лишний раз?
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.
На последнем это каком? Непосредственно в экспрессе?
Вообще здесь и есть последний этап внутри воллет менеджера - перед отдачей посчитанного fee наружу в estimateFee
(что и вызывало ошибку в экспрессе)
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.
Да я это и имел в виду. Ты пишешь, округлил, потом помножил, потом надо опять округлять. Вот и предложил не округлять на промежуточных этапах, но раз уже так, то ок
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.
Эта логика специфична для Хедеры и может аффектить обычные транзы, а не только экспресс - поэтому округление точно нужно тут
Но в экспрессе по хорошему тоже сделать бы округление перед отправкой запроса (со scale == decimal count), раз у бэка такой строгий контракт - чтобы ситуация как с хедерой не повторилась с другими сетями
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.
Ты пишешь, округлил, потом помножил, потом надо опять округлять.
ну и это я же писал что пробовал и что не взлетело)
сейчас двойного округления нет
IOS-8050
В общем корневая причина проблемы - это то что у хедеры фии номинируются в USD.
И соответственно есть exchange rate usd/hbar
Использование этого exchange rate приводит к потери точности - поэтому добавил округление up