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

Update deploy command following NEO3 schema #463

Merged
merged 13 commits into from
Oct 18, 2019

Conversation

@superboyiii
Copy link
Member

Require neo-project/neo-devpack-dotnet#109
Related to neo-project/neo-devpack-dotnet#91

Should follow this logic
https://github.com/neo-project/neo/blob/e1fdde307153879f0a7ae64bd3e11d042284896e/neo/SmartContract/InteropService.NEO.cs#L248

Great! Let me have a look.

@superboyiii
Copy link
Member

superboyiii commented Sep 17, 2019

@shargon Hi, Shargon, I've made a test based on my simple sc. The content is:

using System;


namespace Neo.SmartContract
{
    public class ICO_Template : Framework.SmartContract
    {
        //Token Settings
        public static string Name() => "ZZH";
        public static string Symbol() => "ZZH";


        public static Object Main(string operation, params object[] args)
        {
            if (Runtime.Trigger == TriggerType.Application)
            {

                if (operation == "name") return Name();
                if (operation == "symbol") return Symbol();

            }
            return false;
        }
    }
}

It was compiled successfully within neon.exe
image
You could see there're manifest in the root path.
image
Then I deploy it on neo-cli-3.0.0-preview1, meet an issue on invoking ContractMethodDescriptor.
image
image
image

@shargon
Copy link
Member Author

shargon commented Sep 17, 2019

Thanks for the test, i will review it

@shargon
Copy link
Member Author

shargon commented Sep 17, 2019

You are right, we need some changes included in neo-project/neo-devpack-dotnet#93 , i will copy these fixes

@shargon shargon changed the title Update deploy command Update deploy command following NEO3 schema Sep 17, 2019
Copy link
Member

@vncoelho vncoelho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great to see how simpler is becomes!


if (!InteropService.SupportedMethods().ContainsKey(ci.TokenU32))
{
throw new FormatException($"Syscall not found {ci.TokenU32.ToString("x2")}. Are you using a NEO2 smartContract?");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this needed, @shargon?

The compiler is going to be different.
Maybe we can just say that the syscall is not valid. I think there is not need to really explicit that it is NEO2.

Copy link
Member

@superboyiii superboyiii Sep 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vncoelho Hey, my friend, since we've not deployed a new nuget for NEO3 SmartContract Framework, and the NEO3 one is also using the version number 2.9.3, it can make developers greatly confused, including me, hahaha. So I think this error message is necessary because if you're using the NEO2 nuget of Framework 2.9.3 to deploy a sc on NEO3, it's incompatible.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, If you use the old version of SmartContract.Framework, you can get a wrong NEF, so I think that this checks could be very useful. Is possible that some invalid contract pass, it is a basic filter for prevent some human errors.

@lock9
Copy link
Contributor

lock9 commented Sep 26, 2019

@shargon we just have to replace ".nef" with "manifest.json", or equivalent (I don't recall the exact file extension)

@shargon
Copy link
Member Author

shargon commented Sep 26, 2019

Is not the expected behavior , tell me one linux command that works as you said?

@lock9
Copy link
Contributor

lock9 commented Sep 26, 2019

@shargon all of them, you tell me a program that you have to point to all files 😂
What if it is optional? If you don't send any, it will just use the same file name with a different extension?

For example make, docker build and many others assume the filename. This is a very common pattern

@shargon
Copy link
Member Author

shargon commented Sep 27, 2019

Then i don't understand you very well, you are asking for a file (first parameter) and name (the second) ? give me an example please

neo-cli/Shell/MainService.cs Outdated Show resolved Hide resolved
@shargon
Copy link
Member Author

shargon commented Sep 27, 2019

@lock9 please review it again.

image

neo-cli/Shell/MainService.cs Outdated Show resolved Hide resolved
@superboyiii
Copy link
Member

I'll let @nicolegys to have a test.

@lock9
Copy link
Contributor

lock9 commented Sep 29, 2019

@shargon
image

Maybe the manifest is broken? I used the latest master and you can see that the file exists.

EDIT: It worked when I added 'static' to the methods, maybe it should have blocked it at compilation time?

lock9
lock9 previously approved these changes Sep 29, 2019
Copy link
Contributor

@lock9 lock9 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is working.
Thanks @shargon!

@nicolegys
Copy link
Contributor

Successfully deploy a smart contract without manifest.json
image

@nicolegys
Copy link
Contributor

In debug mode, it will throw an exception when I deploy a NEO2 smartContract.
image
But it will only return 'error' when I use a release version neo-cli.
image
Because of this:
image

image

Should we add 'Console.WriteLine' directly instead of throwing an exception?

@shargon
Copy link
Member Author

shargon commented Sep 29, 2019

we can remove the Debug directive

Copy link
Contributor

@lock9 lock9 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Shargon

Copy link
Member

@superboyiii superboyiii left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested. It works effectively.

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

Successfully merging this pull request may close these issues.

Neon compiler isn't ready for NEO3 contract deployment
5 participants