CashScript / cashscript

⚖️ Easily write and interact with Bitcoin Cash smart contracts
https://cashscript.org
MIT License
116 stars 80 forks source link

Rounding error when determining fee for Contract Transaction #146

Open jimtendo opened 1 year ago

jimtendo commented 1 year ago

[In ](bitcoincash:pr0qgla00u9rq34pael3ayjf7tuqm2p3kyyrvzcsak) the following line can sometimes lead to an incorrect fee as it does a Math.ceil on the value.

This means that if JS's Math engine, for example, gave a result of something like 620.00000001 in the preceding math, the Math.ceil() would round this up to 621, causing an off-by-one error.

Math.round() will probably resolve this as, even with JS Math's inconsistencies, it should always round back to the correct target integer.

mr-zwets commented 2 weeks ago

Hi there! Following up on this old issue, this code got rewritten 20 months ago as part of 'Enforce bigint for all script numbers and bch amounts'

https://github.com/CashScript/cashscript/commit/bac5967acafe9fd815a9cc20ca49359274630592#diff-5a9bd0f77f6dbbec45a9d9e9d42a52174ebd6701d18d1434ebae2a302f1e866b

Now the fee calculation code looks like this:

// High precision may not work with some 'number' inputs, so we set the default to 6 "decimal places"
const addPrecision = (amount: number | bigint, precision: number = 6): bigint => {
  if (typeof amount === 'number') {
    return BigInt(Math.ceil(amount * 10 ** precision));
  }

  return amount * BigInt(10 ** precision);
};

so it still includes a Math.ceil() but only for the number type but this is likely still problematic due to addPrecision(contractInputSize * this.feePerByte);