[draft] Test vector specification

[ixje]

Last week I added support for running the JSON test vectors in neo-python. This was a very useful exercise in uncovering deviating behaviour or state and I think other language implementations could follow. The process of implementing a parser wasn't really smooth because there was no specification describing which members were optional, required and where applicable the possible values they could hold. This PR is a draft of such a specification that I'd like to start a discussion for.

[ixje]

@shargon as you designed the thing I'd like to get your input and also to discuss some improvements

• some files have a BOM header. I think they should all be converted to UTF-8 and we make that part of the specification. Currently I had to use utf-8-sig encoding which is pretty non-standard and didn't even work with the regular open function in Python. This might be the same for other languages.

• some members like script, message etc MAY start with 0x and are then followed by a hexadecimal sequence. I'd like to propose to make this prefix a MUST.

• Integer values of the StackItem object (see spec) return either "5" or 5. I think we need to fix them to return only numbers and never string values

• as the message and scripttable members have not been used in any tests I don't know what format they'll be in or what their exact member names are. Please provide input.

[shargon]

some files have a BOM header. I think they should all be converted to UTF-8 and we make that part of the specification. Currently I had to use utf-8-sig encoding which is pretty non-standard and didn't even work with the regular open function in Python. This might be the same for other languages.

We can remove all BOM headers

some members like script, message etc MAY start with 0x and are then followed by a hexadecimal sequence. I'd like to propose to make this prefix a MUST.

Agree

Integer values of the StackItem object (see spec) return either "5" or 5. I think we need to fix them to return only numbers and never string values

We can accept both, but test should be use the same, in the case of integer, i prefer without quotes

as the message and scripttable members have not been used in any tests I don't know what format they'll be in or what their exact member names are. Please provide input.

I need to do more unit test, so i will add some examples :)

[ixje]

Integer values of the StackItem object (see spec) return either "5" or 5. I think we need to fix them to return only numbers and never string values

We can accept both, but test should be use the same, in the case of integer, i prefer without quotes

I understand we can accept both. I just think that it makes more sense to have a fixed return type. For example Boolean returns true and false keywords, not both true/false keywords and "true"/"false" strings. So my proposal was to use integer/decimal values for the "Integer" type and don't accept string values to simplify parsing logic for some languages.

Don't worry, I can update the tests according to the spec once we've agreed on the final format.

[ixje]

as the message and scripttable members have not been used in any tests I don't know what format they'll be in or what their exact member names are. Please provide input.

I need to do more unit test, so i will add some examples :)

No worries. Can we at least agree on the member names to be message and scriptTable? At least that way I can finalise the headings/member naming, and we just have to update the description of how they look once used.

[erikzhang]

Can we move the JSON files to a git module so that every repo can import it?

[igormcoelho]

This is amazing @ixje @shargon!
@erikzhang I fully agree on having a standard repo for that. neo-project/neo-vm-tests ?

[erikzhang]

neo-project/test-vectors

[ixje]

A generic place for test-vectors sounds good. Perhaps have a VM subfolder, assuming we one day will also have test vectors for the ApplicationEngine, RpcServer and who knows what's yet to come.

[dauTT]

In general I believe we should a dedicated repository for all the neo tests following the example of ethreum:
https://github.com/ethereum/tests

[shargon]

Yes, we can do it. But if take the json from these repo, note that we will need to do two PR for update the VM, one in this repo, and other here.

[erikzhang]

Let's do it later. Now we need to focus on NEO 3.0.

[ixje]

@shargon I saw you added scriptTable support 🎉

The current format looks as follows:

            "scriptTable":
            [
                {
                    "script": "0x51"
                }
            ],

Do you have any other members besides script in mind for the object inside the list? If not, shall we change it to be just a list of scripts? e.g.

            "scriptTable":
            [
                  "0x51",
                  "0x554c4101"
            ],

That would be cleaner to implement but also cleaner to describe in the specification