ton-blockchain / wallet-contract-v5

w5
MIT License
69 stars 16 forks source link

Type Conversion Issue: BigInt to Number #28

Closed frontusia closed 1 month ago

frontusia commented 1 month ago

When working with BigInt, problems arise when attempting to convert its value to type Number. This can lead to loss of precision or incorrect results when indentify users or comparing.

File: wallet-contract-v5/wrappers/wallet-v5.ts

Code steps:

  1. walletId: bigint
  2. subwalletNumber = Number(walletId)
  3. this.serialized = BigInt(this.subwalletNumber)

Extreme cases:

const bigIntNumber = BigInt(12345678901234567890n);
const regularNumber = Number(bigIntNumber);

console.log(bigIntNumber == regularNumber) // false

So, any number greater than max safe integer will be rounded to the nearest representable value.

Proposed changes:

Instead of converting BigInt to Number and then back to BigInt, use BigInt directly where needed.

Totemancer commented 1 month ago

For the use-case of wallet IDs within a safe range, see this commit for schematics:

https://github.com/tonkeeper/tonkeeper-ton/commit/d9aec6adfdb853eb37e0bba7453d83ae52e2a170#diff-c8ee60dec2f4e3ee55ad5e40f56fd9a104f21df78086a114d33d62e4fa0ffee6R149

nns2009 commented 1 month ago

walletId is in range from 0 to 2^32 - 1, so it fits in JavaScript's default number range.

(It might still simplify the code not to do any conversions, but I haven't looked deep into it)

tolya-yanot commented 1 month ago

yes, no problem with uint32.

but I hope v5 wrapper in ton-core used only Number or only BigInt, not both like the tests here.