cashscript icon indicating copy to clipboard operation
cashscript copied to clipboard

Rounding error when determining fee for Contract Transaction

Open jimtendo opened this issue 2 years ago • 1 comments

[In ](https://github.com/CashScript/cashscript/blob/123e45ae3456da2010b18c844ee6a2f705627b88/packages/cashscript/src/Transaction.ts#L325) 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.

jimtendo avatar Mar 10 '23 14:03 jimtendo

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);

mr-zwets avatar Nov 05 '24 14:11 mr-zwets

this old issue can be closed as the old transaction builder with automatic fee calculation has been deprecated and will be fully removed in the upcoming major release

see #337

mr-zwets avatar Aug 17 '25 06:08 mr-zwets