Inconsistent data types in RPC JSON

[scotte]

I've noticed that some data in RPC JSON responses has inconsistent types. As an example, the Superblock Block Number from "execute superblockage" is a string in exponential notation. We can probably be assured that the block number will always be an integer, and there's no particular reason to use anything but the raw integer value as exponential notation doesn't have much purpose here.

[                                     
    {
        "Command" : "superblockage"
    },
    {
        "Superblock Age" : 118558,
        "Superblock Timestamp" : "08-24-2017 07:24:16",
        "Superblock Block Number" : "1.00253e+06",
        "Pending Superblock Height" : 0.00000000
    }
]

[denravonska]

This one is my bad and will be fixed in the next build.

[scotte]

Thanks! It's nice to have consistent data types for my little project (https://github.com/scotte/gridcoinweb).

[denravonska]

I love that project. Gj!

[scotte]

FYI, As of 3.6.1 it's still a floating point number in a string...

[iFoggz]

i'll have a look into this one as well

[iFoggz]

@scotte can u post the display of 'execute superblockage'

[scotte]

Sure @Foggyx420,

[
    {
        "Command" : "superblockage"
    },
    {
        "Superblock Age" : 47923,
        "Superblock Timestamp" : "09-18-2017 04:01:20",
        "Superblock Block Number" : "1024933.000000",
        "Pending Superblock Height" : 0.00000000
    }
]

This comes from two different places - the pending height, which you can see is returned as a floating point number comes from here: https://github.com/gridcoin/Gridcoin-Research/blob/master/src/rpcblockchain.cpp#L1813 :

        double height = cdbl(ReadCache("neuralsecurity","pending"),0);
        entry.push_back(Pair("Pending Superblock Height",height));

And the current height for the RPC command comes from mvApplicationCache, where it's a floating point number represented as a string. This comes from https://github.com/gridcoin/Gridcoin-Research/blob/master/src/main.cpp#L2749 :

bool LoadSuperblock(std::string data, int64_t nTime, double height)
[...]
        WriteCache("superblock","block_number",ToString(height),nTime);

Since the height can only ever be an integer value, it seems like this should always be kept as such, and there's no reason to cast into a string as well.

I'd send a pull request - and I would still be happy to do so - but I'm not familiar enough with the codebase that I couldn't guarantee breaking something else. If you'd like me to take a stab at it, I'm happy to do so - I'd simply do what I recommended above (straight integer type).

[iFoggz]

cache requires a string not int this problem doesn't occur in all my tests

[iFoggz]

also note i do have a pr to put up soon that addresses the uncessary floats to get rid of the .00000000

[scotte]

So it's fine that the cache requires a string, but the JSON should be typed appropriately. Currently, RPC is a mix of different types for the same sorts of value (we have height here as a double, a string encoded double, and as an integer (see "blocks" from the 'getinfo' RPC command)).

It's fine to store data internally with whatever data type is appropriate, but it's vital to be consistent with datatypes used in the API. Having three different encodings of the same value (block height) isn't ideal.

[iFoggz]

the doubles in rpc that dont need to be are being changed. and i believe i did a bunch already.

[iFoggz]

this one also addressed in my next pr

[tomasbrod]

The complete mvApplicationCache will be ideally removed and properly replaced with separate variables of correct types.