removed flexible checkwitness for 66 or 40 bytes addresses

[igormcoelho]

What current issue(s) does this address, or what feature is it adding?
The Runtime_CheckWitness from neo-python currently accepts "addresses" (scripthash/pubkey) to have 66 or 40 bytes. This cannot be accepted, because the "original" network only accepts 20 or 33.
One problem this may cause (that we verified using Eco neocompiler.io) is that the same path may diverge from Python and C# implementations... The following code does this:

from boa.interop.Neo.Runtime import Notify, CheckWitness 
from boa.interop.Neo.Storage import Get, Put, Delete, GetContext
from boa.interop.Neo.Runtime import Notify

def Main():
    OWNER=b'031a6c6fbbdf02ca351745fa86b9ba5a9452d785ac4f7fc2b7548ca2a46c4fcf4a'
    result = CheckWitness(OWNER)
    if result:
        print("OWNER is caller")
        Put(GetContext(), "oi", "python")
        return True
    else:
        print("OWNER is not caller")
        Put(GetContext(), "oi", "C#")
        return True
        
    return False

Storage "oi" in python rpc will be "python", and in neo-cli C# node will be different... for the same execution.

How did you solve this problem?

I removed both problematic lines.

How did you make sure your solution works?

I tested this solution in neocompiler.io Eco locally.

Are there any special changes in the code that we should be aware of?

I believe these lines were put to simplify neo-boa work... but neo-boa needs some improvements, and there's an opportunity here.
Currently, both codes generate the same opcodes:

    OWNER=b'031a6c6fbbdf02ca351745fa86b9ba5a9452d785ac4f7fc2b7548ca2a46c4fcf4a'
    OWNER="031a6c6fbbdf02ca351745fa86b9ba5a9452d785ac4f7fc2b7548ca2a46c4fcf4a"

The opcode is: PUSHBYTES66 303331613663366662626466303263613335313734356661383662396261356139343532643738356163346637666332623735343863613261343663346663663461 ### "031a6c6fbbdf02ca351745fa86b9ba5a9452d785ac4f7fc2b7548ca2a46c4fcf4a"

In other words, it assumes the input is a string composed of ascii chars. It would be nice if the binary input was treated like: OWNER=b'\x03\x1alo\xbb\xdf\x02\xca5\x17E\xfa\x86\xb9\xbaZ\x94R\xd7\x85\xacO\x7f\xc2\xb7T\x8c\xa2\xa4lO\xcfJ'
(which is the output of binascii.unhexlify(OWNER), if OWNER is binary string)

I don't know if this information is available (binary string vs pure string) for byteplay, etc, but this is possibly a transparent solution for the user. If not possible, however, user must manually apply unhexilfy in order to make string real 20 or 33 bytes... that's life.

Please check the following, if applicable:

• Did you add any tests?
• Did you run make lint?
• Did you run make test?
• Are you making a PR to a feature branch or development rather than master?
• Did you add an entry to CHANGELOG.rst? (if not, please do)

[localhuman]

I think we will just have to require that when an owner is defined in a contract that it is defined in an unhexxed binary format such as OWNER=b'\x03\x1alo\xbb\xdf\x02\xca5\x17E\xfa\x86\xb9\xbaZ\x94R\xd7\x85\xacO\x7f\xc2\xb7T\x8c\xa2\xa4lO\xcfJ'

[localhuman]

Thanks for the contribution! Could you update the changelog?

[igormcoelho]

Ok, I'm doing the tests, then I'll update the changelog.

[igormcoelho]

Ok, I finished the tests. How do I submit them?
By the way, I think the docs part failed:
$ make docs
make -C docs html
make[1]: Entering directory '/home/imcoelho/git-reps/neo-python/docs'
/home/imcoelho/git-reps/neo-python/myvenv/bin/python: No module named sphinx
Makefile:20: recipe for target 'html' failed

[localhuman]

I wouldn't worry about the docs, you'll need to have a separate venv with python 3.5 for those to build correctly.

You could add the tests to one of the existing tests in neo/SmartContract/tests or create a new test file there.

[igormcoelho]

Ok, I didn't create new tests, only executed make lint, make test and make coverage. I believe that's all then.