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

JsonRpcApi input & output differs from C# #233

Closed
ixje opened this issue Feb 16, 2018 · 9 comments
Closed

JsonRpcApi input & output differs from C# #233

ixje opened this issue Feb 16, 2018 · 9 comments

Comments

@ixje
Copy link
Member

ixje commented Feb 16, 2018

issue 1
C# accepts single quotes

{'params': ["ecc6b20d3ccac1ee9ef109af5a7cdb85706b1df9", "symbol"], 'id': '2', 'jsonrpc': '2.0', 'method': 'invokefunction'}

JsonRpcApi does not and will have an error saying:

{
    "jsonrpc": "2.0",
    "error": {
        "message": "Parse error",
        "code": -32700
    },
    "id": null
}

The actual error message is dropped and says

Expecting property name enclosed in double quotes: line 1 column 2 (char 1)

issue 2
C# returns full names for the VMstate and Stack types after execution e.g.

http://seed3.cityofzion.io:8080  (2.7.0 node)

{'params': ["ecc6b20d3ccac1ee9ef109af5a7cdb85706b1df9", "symbol"], 'id': '2', 'jsonrpc': '2.0', 'method': 'invokefunction'}

{
    "jsonrpc": "2.0",
    "id": "2",
    "result": {
        "script": "00c10673796d626f6c67f91d6b7085db7c5aaf09f19eeec1ca3c0db2c6ec",
        "state": "HALT, BREAK",    <-----
        "gas_consumed": "0.141",
        "stack": [
            {
                "type": "ByteArray", <------
                "value": "525058"
            }
        ]
    }
}

JsonRpcApi gives VM state and stack type as (string) numbers to indicate the same. Note: the only difference in the calls is the scripthash I'm talking to.

{"id": "2", "method": "invokefunction", "params": ["584660c663f114d803928b77cb9abdd585a0fc17", "symbol"], "jsonrpc": "2.0"}

{
    "jsonrpc": "2.0",
    "result": {
        "script": "303063313036373337393664363236663663363731376663613038356435626439616362373738623932303364383134663136336336363034363538",
        "gas_consumed": "0.372",
        "stack": [
            {
                "value": "50434731",
                "type": "05" <--------
            }
        ],
        "state": 5 <-------
    },
    "id": "2"
}
@ixje
Copy link
Member Author

ixje commented Feb 16, 2018

I need to add one more difference in representation

C#

{'params': ["ecc6b20d3ccac1ee9ef109af5a7cdb85706b1df9", "decimals"], 'id': '2', 'jsonrpc': '2.0', 'method': 'invokefunction'}

            {
                "type": "Integer",
                "value": "8"  <---- string int
            }

JsonRpcApi

            {
                "value": 8,  <---- real int
                "type": "02"
            }

@metachris
Copy link
Contributor

Very nice, detective @ixje.

@ixje ixje changed the title JsonRpcApi output differs from C# JsonRpcApi input & output differs from C# Feb 16, 2018
@ixje
Copy link
Member Author

ixje commented Feb 16, 2018

Issue 3
C# accepts different input parameter types e.g. "Hash160" whereas JsonRpcApi would require "03" for the equivalent behaviour. ("Hash160") will throw exceptions

C#

{
	"jsonrpc": "2.0",
	"method": "invokefunction", 
	"params": [
    "ecc6b20d3ccac1ee9ef109af5a7cdb85706b1df9",
	"balanceOf",
	[
		{
        	"type": "Hash160", <-------
	        "value": "584660c663f114d803928b77cb9abdd585a0fc17"
    	}
	]
    ],
	"id": 2 
}

vs neo-python.
(note; again only script hashes swapped for privnet vs mainnet as i'm not synced up. Just focus on the arrows)

{
	"jsonrpc": "2.0",
	"method": "invokefunction", 
	"params": [
    "584660c663f114d803928b77cb9abdd585a0fc17",
	"balanceOf",
	[
		{
        	"type": "03", <------
	        "value": "ecc6b20d3ccac1ee9ef109af5a7cdb85706b1df9"
    	}
	]
    ],
	"id": 2 
}

@brianlenz
Copy link
Contributor

Issue 3 has been addressed in #267. I'm going to take a look at the other issues here in a bit.

@brianlenz
Copy link
Contributor

For issue 1, I would actually argue that's a bug in the C# implementation. JSON doesn't support strings with single quotes, so I think getting an error in that case is justifiable.

Thoughts?

brianlenz pushed a commit to brianlenz/neo-python that referenced this issue Feb 28, 2018
* Fixed the VMState output from JSON-RPC so it properly reflects the same string output as the C# implementation.

* Fixed integer value outputs so that they are represented as a String, which is consistent with the C# implementation.

* Fixed `ContractParameterType` outputs so that they use the String representation instead of a hex representation for consistency with the C# implementation.

CityOfZion#233
@brianlenz brianlenz mentioned this issue Feb 28, 2018
5 tasks
@brianlenz
Copy link
Contributor

issue 2 is fixed in #272, so the only outstanding question here is if we want to do anything for issue 1 or not.

@metachris
Copy link
Contributor

I think not, but I'm not religious about it. I think we can just stick with standard JSON.

I did a quick look, and we'd probably have to switch to a custom "flexible" JSON decoder, rather than the standard ones.

@metachris
Copy link
Contributor

Thanks so much @ixje and @brianlenz for documenting the efforts here. Closing this issue now, in favor of #273

@metachris
Copy link
Contributor

Just a note for future readers, these issues are fixed on the current dev branch (v0.4.11-dev). Will report back once we merge it into master

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

No branches or pull requests

3 participants