icon-project / Nexus

7 stars 3 forks source link

Rounding error #702

Open theconnectooor opened 2 years ago

theconnectooor commented 2 years ago

Describe the bug There is a small rounding error on each bridging transaction.

To Reproduce Steps to reproduce the behavior:

  1. Log into your ICON wallet
  2. Go to Transfer
  3. Click on send ICX to Binance Smart Chain
  4. Transfer 100 ICX to BSC address and click "Transfer"
  5. Click "Approve"
  6. Navigate to transaction on BSC tracker

Expected behavior The value of ICX being bridged over should have been 99 ICX not 98.999999999999995

Screenshots

Screen Shot 2022-08-10 at 1 44 52 PM

Additional context 1) This is likely an error in the smart contracts using "float" math instead of "int" math

2) I believe this rounding is replicable the opposite way as well (BSC -> ICON)

CyrusVorwald commented 2 years ago

I believe this is how the value gets displayed on the tracker and is not an issue with Nexus. @robcxyz @duyphanz would like to get your thoughts

duyphanz commented 2 years ago

As my check on the Nexus FE side, the amount input's value will be passed to the contract request fully.

For example: 100 will be converted to `0xde0b6b3a7640000`, then passed to the contract request

Let me know if any further checks.

theconnectooor commented 2 years ago

I believe this is how the value gets displayed on the tracker and is not an issue with Nexus. @robcxyz @duyphanz would like to get your thoughts

The same rounding error shows on BscScan so don't think it has to do w/ how it's being displayed but rather within the smart contracts themselves.

https://testnet.bscscan.com/tx/0x60a74bf331b5357b44a9fbc14a579d8ef5f350438f9920def10fe82db040a057

CyrusVorwald commented 2 years ago

@pragyanshrestha-ibriz please check if this is a rounding issue with the contracts that can be truncated

izyak commented 2 years ago

For this testnet environment, the fee for ICX was set to:

{
  "feeNumerator": "100",
  "fixedFee": "5000"
}

when bridging 100 ICX feeNumerator cuts of 1% as fee, so 100 - 1% of 100 = 99 fixed fee cuts 5000 / (10 ** 18) = 5e-15 ICX so, the amount to transfer becomes 99 - 5e-15 which is equal to 98.999999999999995 as shown in the tracker.

theconnectooor commented 2 years ago

For this testnet environment, the fee for ICX was set to:

{
  "feeNumerator": "100",
  "fixedFee": "5000"
}

when bridging 100 ICX feeNumerator cuts of 1% as fee, so 100 - 1% of 100 = 99 fixed fee cuts 5000 / (10 ** 18) = 5e-15 ICX so, the amount to transfer becomes 99 - 5e-15 which is equal to 98.999999999999995 as shown in the tracker.

thank you for the clarification. what's the difference between the feeNumerator and the fixedFee?

seems as if feeNumerator is based on bps while fixed fee is just some set amount?

izyak commented 2 years ago

fee = amount * feeNumerator/denominator + fixedFee

During BTP txn, for eg, during ICX transfer to BSC, relayer has to catch a eventlog and call method in BSC. This costs gas fee, and has to be paid for by the relayer. So, generally, we set the txn fee that the relayer has to incur for a transaction as fixed fee. feeNumerator cuts off a certain percentage of amount to transfer during cross chain transfer.

theconnectooor commented 2 years ago

is the fixed fee included in the BTP transfer fee? or does it not show because it is so small?

seems as if that feature isn't working again so couldn't give a good example 😂

Screen Shot 2022-08-18 at 2 49 13 PM
izyak commented 2 years ago

I'm not exactly sure how the fee is displayed in the frontend. If the fee is too low, the UI probably just rounds it off. Maybe someone from Nexus team can answer this 😅

CyrusVorwald commented 2 years ago

Is there an update on this?