Xahau / xahaud

Codebase for Xahaud - The consensus, RPC & blockchain app for the Xahau network.
https://xahau.network
ISC License
23 stars 11 forks source link

Potential rounding rrror on all `float_` functions #322

Closed dangell7 closed 4 months ago

dangell7 commented 4 months ago
const _float_root = float_root(6097866696204910592, 2)
trace('_float_root', _float_root, false)
// 6091866696204911000 = 3.000000000000408
tequdev commented 4 months ago

trace("float_one()", trace_one()) expect: 6089866696204910592 result : 6089866696204911000

dangell7 commented 4 months ago

The issue is that returnJS(float_one_internal); returns either an int32 or a float64. Float64 starts rounding at 15 digits so its not safe to use with XFL.

JS_NewBigInt64 does return the correct value

dangell7 commented 4 months ago

https://github.com/Xahau/xahaud/pull/318/commits/13719a1357b75595ffab3ecb59cf56e3f8a5e2c7

tequdev commented 4 months ago

After that commit, the following code resulted in an error (-7). trace('_', float_sum(float_one(), float_one()), false) before: 6090866696204911000 after: -7

tequdev commented 4 months ago

JS_IsNumber() and JS_IsBigInt() seem to be exclusive.

applyHook.js: FromJSInt()

    if (JS_IsNumber(v))
    {
        if (JS_IsBigInt(ctx, v))
        {
            int64_t out = 0;
            JS_ToBigInt64(ctx, &out, v);
            return out;
        }
        int64_t out = 0;
        JS_ToInt64(ctx, &out, v);
        return out;
    }
dangell7 commented 4 months ago

yeah still working out though all the apis

dangell7 commented 4 months ago

The problem is the return of float_one() and 6089866696204910592. Richard will fix it all next week I was just trying to get a head start

tequdev commented 4 months ago

In the latest commit after the fix(https://github.com/Xahau/xahaud/commit/13719a1357b75595ffab3ecb59cf56e3f8a5e2c7), I don't think the float_one() is the problem, but a bug in the BigInt check code you added.

It should be as follows

    if (JS_IsNumber(v))
    {
        int64_t out = 0;
        JS_ToInt64(ctx, &out, v);
        return out;
    }

    if (JS_IsBigInt(ctx, v))
    {
        int64_t out = 0;
        JS_ToBigInt64(ctx, &out, v);
        return out;
    }
dangell7 commented 4 months ago

Yepp you're right. I actually added the BIG_INT check to that function JS_IsNumber but removed it in the commit.

Thank you

dangell7 commented 4 months ago

@tequdev Can you check now on the newest build? Looking good?

tequdev commented 4 months ago

Just checked, and works fine!

Thank you!