Skip to content
This repository has been archived by the owner on Nov 15, 2021. It is now read-only.

getaccountstate returns different schema #713

Closed
ediopia opened this issue Nov 8, 2018 · 7 comments
Closed

getaccountstate returns different schema #713

ediopia opened this issue Nov 8, 2018 · 7 comments

Comments

@ediopia
Copy link

ediopia commented Nov 8, 2018

Current behavior

Balances returns in object.

Expected behavior

It is supposed to return in array. Because neo-cli does.

From Neo python
balances:
{
'0x602c79718b16e442de58778e148d0b1084e3b2dffd5de6b7b16cee7969282de7': '14.00251105',
'0xc56f33fc6ecfcd0c225c4ab356fee59390af8560be0e930faebe74a6daff7c9b': '2.0'
}
From Neo cli
balances: [ [Object], [Object] ] } }

How to reproduce

Your environment

Let us know in what environment you're running into the issue:

  • OS: (Windows version/Linux distro/OSX version)
  • neo-python version: NEO-PYTHON:0.8.3-dev
  • Python version:
@jseagrave21
Copy link
Contributor

jseagrave21 commented Nov 8, 2018

@xxedocxx I think the neo-cli version is redundant. Why separate assets by list number? I think letting the asset be a dict key is much more user friendly and intuitive. See comparison below:

neo-cli:

"balances": [
            {
                "asset": "0xc56f33fc6ecfcd0c225c4ab356fee59390af8560be0e930faebe74a6daff7c9b",
                "value": "94"
            }
        ]

vs.
neo-python:

"balances": {
            "0xc56f33fc6ecfcd0c225c4ab356fee59390af8560be0e930faebe74a6daff7c9b": "50.0",
            "0x602c79718b16e442de58778e148d0b1084e3b2dffd5de6b7b16cee7969282de7": "13.9998"
        }

I prefer neo-python's version and think it is more efficient, and the functionality is the same. However, it would be a relatively easy change to make.
@ixje @localhuman your thoughts?

@dethos
Copy link
Contributor

dethos commented Nov 9, 2018

Since the objective of neo-python is supposed to be a full port of neo-cli and as stated on previous comments people should be able to "seamlessly switch" from one to another, it must receive the same inputs and return the same outputs, so I do think this is something that this should be fixed.

@jseagrave21
Copy link
Contributor

jseagrave21 commented Nov 9, 2018

@dethos the only problem with your argument is that your example is for a RPC method which is planned to not be included in the default JsonRpcApi like it is in neo-cli (see @ixje 's review here #633 (comment)). So, a default neo-python RPC server will not include this functionality.

In my opinion, neo-python is striving to be the best version of neo-cli. Just my opinion.

@dethos
Copy link
Contributor

dethos commented Nov 9, 2018

The comment is on a topic related to dumpprivkey, but the observation about maintaining a similar interface to the user is general. Nevertheless lets wait to see what other people think.

@ediopia
Copy link
Author

ediopia commented Nov 9, 2018

It already has broke some dapps who uses the rpc api and relying on nodes from http://monitor.cityofzion.io/

@jseagrave21
Copy link
Contributor

@xxedocxx I didn't know that. Sounds like this fix is definitely necessary then. I guess in my perfect world I would have suggested neo-cli update their RPC method to match neo-python instead of the other way around lol.

@ixje
Copy link
Member

ixje commented Nov 9, 2018

Since the objective of neo-python is supposed to be a full port of neo-cli and as stated on previous comments people should be able to "seamlessly switch" from one to another, it must receive the same inputs and return the same outputs, so I do think this is something that this should be fixed.

Yes at least for the RPC side the idea is that they can seamlessly switch from one to the other, so this needs a fix if it deviates.

The exclusion of dumpprivkey from the default interface is to prevent getting blamed for incompetence of the user. I'm not going to sit back while users mess up their setup and get wrecked due to including some stupid dangerous method in the default interface.

As for the full port; I don't think the vision of a full port of neo-cli is valid anymore as there's just many design decisions in there that don't make sense. We'll try to keep up for now, but we already had trouble keeping up (as they never document and rarely add tests), and they recently did a major refactoring that I don't think we're going to follow. I'd rather re-write than follow.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants