New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

importcontract fails with returntype >= 16 #217

Closed
igormcoelho opened this Issue Feb 11, 2018 · 5 comments

Comments

Projects
None yet
4 participants
@igormcoelho
Copy link
Contributor

igormcoelho commented Feb 11, 2018

Perhaps I misunderstood something, but I cannot import a contract with return type >= 16 (example, Void = ff = 255).
What happens is that "def GatherContractDetails(function_code, prompter)" will call "return generate_deploy_script(function_code.Script, name, version, author, email, description, function_code.ContractProperties, ord(function_code.ReturnType), function_code.ParameterList)" on line 199 of LoadSmartContract.py, and since returntype is passed in ord() function, it will be set as an int < 256.
Function "def generate_deploy_script" on line 204 of LoadSmartContract.py will receive return_type and push it "sb.push(return_type)" at line 220. Remember that return_type is always "int" in this case (because of ord() function called previously), but push function in ScriptBuilder.py will enter "if type(int) is int or ..." and will fail "else" command: return self.push(binascii.hexlify(data.ToByteArray())) because "int" cannot have ToByteArray.
I believe this code only works for return types < 16, such as ByteArray 0x05, Boolean 0x01, etc (because it wont enter "else" condition), and I think an easy solution is to force return value to be BigInteger (in line 220 of LoadSmartContract.py). It will become: "sb.push(BigInteger(return_type))".
I can make a pull request, but since it seems a rather complicated issue, I prefer to ask for opinions first :)

@igormcoelho igormcoelho changed the title testinvoke fails with returntype >= 16 importcontract fails with returntype >= 16 Feb 12, 2018

@localhuman localhuman added the HACKIT label Mar 16, 2018

@dethos

This comment has been minimized.

Copy link
Contributor

dethos commented May 15, 2018

@localhuman / @igormcoelho has this issue already been solved?

Was trying to reproduce it in the last version of neo-python, but no success until now.

(if yes perhaps would be better to close it)

@localhuman

This comment has been minimized.

Copy link
Collaborator

localhuman commented May 15, 2018

Yes, this has been fixed.

@localhuman localhuman closed this May 15, 2018

@dethos dethos referenced this issue May 18, 2018

Merged

Fix error with invalid return-types #422

3 of 5 tasks complete
@vncoelho

This comment has been minimized.

Copy link

vncoelho commented May 22, 2018

It is not in the master yet, right?
The error is still happening here.

Perhaps now with this last updated from dethos.

@igormcoelho

This comment has been minimized.

Copy link
Contributor

igormcoelho commented May 24, 2018

The problem was solved indeed, I can confirm it. The solution was the adoption of a bytearray( ff ) as default parameter in LoadSmartContract.py file (deploy function), which is quite similar to my older proposition using BigInteger(ff)... that's what we have used successfully in neocompiler.io for the past few months, but the result is the same (push one byte ff).
However, Vitor noticed that another issue indeed was happening, related to #422, but not exactly happening in deploy time... it seems database was being blocked when a wrong readbyte was performed in a further invocation (related to ff bytearray). That seems to be solved in 'development' branch now (not in current master 42e0bb4).

As a solution, we are using development branch now, even for production :)

@igormcoelho

This comment has been minimized.

Copy link
Contributor

igormcoelho commented May 29, 2018

In fact, I think a better solution would be this: neo-project/neo#253

@igormcoelho igormcoelho referenced this issue May 31, 2018

Closed

return type must be bigint #444

2 of 5 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment