search

iotaledger / sandbox

14 stars 11 forks source link

Incorrect attribute names/types in `getBalances` response #7

Open todofixthis opened 6 years ago

todofixthis commented 6 years ago

From https://github.com/iotaledger/iota.lib.py/issues/55:

According to the getNodeInfo the IRI version of the sandbox is 1.4.0.

Calling getBalances on the sandbox returns

{
    "Duration": 0,
    "Balances": [0],
    "Milestone": "RXTDTARWZPYTWBFXDHGZDTCDYBWGMKAHDKIAYKKKGUTUNJAXIVIKYDVCTVE9WGJ9GAPFCEZOFYZPA9999",
    "MilestoneIndex": 233555
}

whereas running IRI v.1.4.0 locally (either via Java or Docker) returns

{
    "duration": 6,
    "balances": ["0"],
    "milestone": "999999999999999999999999999999999999999999999999999999999999999999999999999999999",
    "milestoneIndex": 217000
}

Note that the first letter of each JSON key is capitalised, and the balances are returned as integers instead of strings.

todofixthis commented 6 years ago

~I think the incorrect type for balances is due to https://github.com/iotaledger/iota.lib.go/issues/11~ I think the Go lib is doing the right thing; in my opinion, the sandbox application is the exception to the rule. For the majority of applications, I would expect that developers would prefer that balances are returned as integers rather than strings. The sandbox app is unique in that it acts as a proxy for IRI, so it has to return exactly what the IRI would in these situations.

todofixthis commented 6 years ago

The capitalised attribute names may be due to https://github.com/iotaledger/iri/issues/319

knarz commented 6 years ago

This was fixed here https://github.com/iotaledger/iota.lib.go/commit/63fcd33b000cd1511e126e8dc02b19100608e2de, seems like the sandbox was just not updated.

As far as string vs. int goes, I'm open to change that to comply with the actual IRI API.

todofixthis commented 6 years ago

Thanks @knarz! Who is responsible for updating the sandbox?

WRT string vs. int, I think most libraries will work fine either way. There might be an edge case where the JS lib is trying to process a getBalances response from the Sandbox that contains really really big numbers (see https://stackoverflow.com/a/9643650/5568265). That's the only issue I can think of.

todofixthis commented 6 years ago

btw did this fix ever get deployed to the Sandbox node?