Skip to content
This repository has been archived by the owner on Mar 28, 2023. It is now read-only.

Compiler should convert binary string to byte array #77

Open
igormcoelho opened this issue May 29, 2018 · 9 comments
Open

Compiler should convert binary string to byte array #77

igormcoelho opened this issue May 29, 2018 · 9 comments

Comments

@igormcoelho
Copy link

As mentioned in CityOfZion/neo-python#438 I believe 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.

@igormcoelho
Copy link
Author

I believe @canesin will also support this, if possible to be implemented.

@canesin
Copy link

canesin commented May 30, 2018

@metachris or @localhuman .. the idea is to remove the extension from the VM and replace it for a helper function into the compiler, this way support can be kept but behavior of the generated AVM is the same.

@localhuman
Copy link
Collaborator

It doesn't make very much sense. a binary string, eg b'abmq' is not necessarily one that an be unhexlified ( especially a binary string with an odd amount of characters ). If we arbitrarily unhex binary strings that can be unhexxed to something like b'\x01\xaf etc, then we will be introducing a non-transparent api.

The main thing I have thought about in regards to this is that the VM should have opcodes ( or there should be interop methods) to hex and unhex a string or string of bytes.

@metachris
Copy link
Contributor

metachris commented May 30, 2018

Not sure this has a strong enough use-case to warrant a backwards incompatible change. Would break applications that used this part before.

What about a helper the other way around, to easily convert unhexlify a bytearray into the current format?

@igormcoelho
Copy link
Author

Thats true, not all bitstrings can be unhexlified, too bad, that would be beautiful :) anyway, keeping compatibility is also quite important, so a helper method HexToBytes will do the job. But still its better to accept a bitstring perhaps or both (string and bitstring) since its a helper it doesnt.matter on the avm. Thanks for the attention.

@canesin
Copy link

canesin commented May 31, 2018

on that line hex, unhex could be a compiler level tool while it is not opcodes or interop ? this way we can actually start something: 1st as helpers -> interop -> opcode .. depending on the usage/popularity.

@localhuman
Copy link
Collaborator

neo-project/neo#263

@igormcoelho
Copy link
Author

Guys, you know in C# there's already a helper to do it... something like: const byte[] b = "abcdabcd".HexToBytes();
The generated opcodes are just abcdabcd, and could solve at least the problem above... but the discussion is nice to have something in lower levels for Neo, it opens more possibilities to build a safer smart contract code.

@localhuman
Copy link
Collaborator

That would be a helper method for string literals, but it wouldn't work for actual variables. Because of that I think it would be confusing/ misleading.

So mybytes = "abadad".HexToBytes() would work but mybytes = myvar.HexToBytes() would not

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

4 participants