Skip to content
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

Neo3-Preview3 release checklist #1731

Closed
16 tasks done
superboyiii opened this issue Jun 24, 2020 · 30 comments
Closed
16 tasks done

Neo3-Preview3 release checklist #1731

superboyiii opened this issue Jun 24, 2020 · 30 comments

Comments

@superboyiii
Copy link
Member

superboyiii commented Jun 24, 2020

Neo3-preview3 release checklist
We're preparing checklist of tasks for neo3-preview3. The goal of this release is to fix major issues and make all repositories works compatibly after many new features were merged. We're already focused on these issues and PRs but we're also glad to collect other requirements. This checklist will be frozen on June 29th, then we'll work on these.
This release should be on 03/08/2020(Can be moved forward if completed ahead).

Mandatory issues

Mandatory PRs

@neo-project/core

@roman-khimov
Copy link
Contributor

Is there any particular reason for choosing 2.1 instead of 3? It looks like this release won't be compatible with preview2 and my (probably wrong) expectation for 2.1 is to be something like a compatible bugfix update to preview2. Maybe I'm just too picky with versioning, but I think there should be some meaning in these numbers.

@erikzhang
Copy link
Member

It looks like this release won't be compatible with preview2 and my (probably wrong) expectation for 2.1 is to be something like a compatible bugfix update to preview2.

Agree.

@superboyiii
Copy link
Member Author

superboyiii commented Jun 24, 2020

Is there any particular reason for choosing 2.1 instead of 3? It looks like this release won't be compatible with preview2 and my (probably wrong) expectation for 2.1 is to be something like a compatible bugfix update to preview2. Maybe I'm just too picky with versioning, but I think there should be some meaning in these numbers.

Yes, it should be incompatible with Preview2 but we plan to make a two-monthly release for developer requirements. It will be like v3.0.0-PreviewX.Y.Z. And Z is a hotfix version position. Y is the improvement of X and X should contain greater features or modules such like Oracle and neoFS(C#). In Y it could be incompatible since it's Preview version and will often be incompatible.
@erikzhang What do you think?

@superboyiii superboyiii changed the title Neo3-preview2.1 release checklist Neo3-Preview2.1 release checklist Jun 24, 2020
@devhawk
Copy link
Contributor

devhawk commented Jun 24, 2020

Yes, it should be incompatible with Preview2 but we plan to make a two-monthly release for developer requirements. It will be like v3.0.0-PreviewX.Y.Z. And Z is a hotfix version position. Y is the improvement of X and X should contain greater features or modules such like Oracle and neoFS(C#). In Y it could be incompatible since it's Preview version and will often be incompatible.

Major/minor/patch seems like overkill for preview versioning. If we're just going to ship a new preview every two months, there's little value in attempting to signal the level of backwards compatibility. Downstream projects that take a dependency on Neo have to move to the latest preview, regardless of what breaking changes or new features there might be.

If we are going to use preview major/minor/patch versioning, we do need to increment the preview major version for the next version. As @roman-khimov points out, a minor version bump is commonly used when you "add functionality in a backwards compatible manner" (as per Semantic Versioning). Since the next preview will not be backwards compatible with preview 2, it needs a full major version bump.

@devhawk
Copy link
Contributor

devhawk commented Jun 24, 2020

Regardless of what it's called, I'd like to make sure these two PRs are merged before the next preview

@superboyiii
Copy link
Member Author

Yes, it should be incompatible with Preview2 but we plan to make a two-monthly release for developer requirements. It will be like v3.0.0-PreviewX.Y.Z. And Z is a hotfix version position. Y is the improvement of X and X should contain greater features or modules such like Oracle and neoFS(C#). In Y it could be incompatible since it's Preview version and will often be incompatible.

Major/minor/patch seems like overkill for preview versioning. If we're just going to ship a new preview every two months, there's little value in attempting to signal the level of backwards compatibility. Downstream projects that take a dependency on Neo have to move to the latest preview, regardless of what breaking changes or new features there might be.

If we are going to use preview major/minor/patch versioning, we do need to increment the preview major version for the next version. As @roman-khimov points out, a minor version bump is commonly used when you "add functionality in a backwards compatible manner" (as per Semantic Versioning). Since the next preview will not be backwards compatible with preview 2, it needs a full major version bump.

Thanks for your advice. I'll make Preview3 happen.

@superboyiii superboyiii changed the title Neo3-Preview2.1 release checklist Neo3-Preview3 release checklist Jun 25, 2020
@superboyiii
Copy link
Member Author

Regardless of what it's called, I'd like to make sure these two PRs are merged before the next preview

Added

@devhawk
Copy link
Contributor

devhawk commented Jul 2, 2020

FYI, Debug Info support for NEON has been merged

@lock9
Copy link
Contributor

lock9 commented Jul 3, 2020

Hello.
I think that it is not possible to create a payable smart contract with the current master version. You can transfer funds, but I was unable to withdraw it.
Any chance we can add that to this list? The issue is probably related to this method:

internal static bool VerifyWitnesses(this IVerifiable verifiable, StoreView snapshot, long gas)

There are UT covering this feature, but they are not working (they pass, but there is an exception happening).
If this feature works, please let me know how to use it. There aren't examples available.
Thanks.

@Tommo-L
Copy link
Contributor

Tommo-L commented Jul 3, 2020

We'll have a test. If you want to withdraw from smart contract, it's better to add the Verify method in contract like the NEP5 demo.
https://github.com/neo-project/neo-devpack-dotnet/blob/44fabf6bd47ea8e9e4ea9efc394b33c980c1a52e/templates/Template.NEP5.CSharp/NEP5.cs#L35-L40

@lock9
Copy link
Contributor

lock9 commented Jul 3, 2020

I have the verify method, but how can I build a transaction that works? The UT is failing with an exception (but the test is passing)

@lock9
Copy link
Contributor

lock9 commented Jul 3, 2020

Maybe the question should be different. Is neo-cli adapted to withdraw funds from a smart contract?

@Tommo-L
Copy link
Contributor

Tommo-L commented Jul 3, 2020

Is neo-cli adapted to withdraw funds from a smart contract?

It dose not support, it's better to use sdk to build the transaction. As neo-cli may not know the argument list of verify method.

@lock9
Copy link
Contributor

lock9 commented Jul 3, 2020

@Tommo-L Is there any example on the SDK? Should I expect this feature to be working on this next release?

@erikzhang
Copy link
Member

As neo-cli may not know the argument list of verify method.

The verify method is included in the ABI file.

@lock9
Copy link
Contributor

lock9 commented Jul 3, 2020

I guess that the issue is related to the parameters. A verify without parameters should work, but I was unable to understand how can we pass parameters to the verify method.
For example, is this possible? (pseudocode)

public verify(accountHash, signature){
    account = storage.get(accountHash)
    balance = account.balance
    val tx = getTransferInformation() 
    if(tx.balance <= balance)
        return verifySignature(account.publicKey, signature)
    else
        return false
}

@shargon
Copy link
Member

shargon commented Jul 4, 2020

Yes, you have the verification script

ContractMethodDescriptor md = cs.Manifest.Abi.GetMethod("verify");
and the invocation script
engine.LoadScript(verifiable.Witnesses[i].InvocationScript, CallFlags.None);
in the invocation script you can push the arguments

@lock9
Copy link
Contributor

lock9 commented Jul 4, 2020

@shargon I will message you on discord, there is something I'm unable to understand. I imagine that the script I sent will be something like "appCall neo transfer from to", but I don't know how to push the arguments for the verification script. If I push the arguments after these, will it be passed to the verification method automatically?
Like "appCall neo transfer from to verificationParam1 verificationParam2" ?

@superboyiii
Copy link
Member Author

@devhawk I saw your new PR IApplicationEngineProvider, I think it should be included in Preview3 instead of neo #1724 , what's your opinion? If so, I will take it into checklist.

@devhawk
Copy link
Contributor

devhawk commented Jul 9, 2020

@devhawk I saw your new PR IApplicationEngineProvider, I think it should be included in Preview3 instead of neo #1724 , what's your opinion? If so, I will take it into checklist.

Yes I think we should swap #1758 for #1724, assuming @erikzhang likes the new design

@Tommo-L
Copy link
Contributor

Tommo-L commented Jul 9, 2020

It's better to add #1752 in checklist, as it's incompatible.

@superboyiii
Copy link
Member Author

It's better to add #1752 in checklist, as it's incompatible.

Done

@devhawk
Copy link
Contributor

devhawk commented Jul 13, 2020

FYI, #1758 is merged

@devhawk
Copy link
Contributor

devhawk commented Jul 14, 2020

FYI, neo-modules RpcClient and RpcServer don't currently compile against neo master branch.

@Tommo-L
Copy link
Contributor

Tommo-L commented Jul 15, 2020

@superboyiii This bug must be fixed before prievew3
neo-project/neo-devpack-dotnet#300

@devhawk
Copy link
Contributor

devhawk commented Jul 15, 2020

RpcServer was updated last night, so it's now compiling against neo master. I filed neo-modules 289 to track updating RpcClient. It needs to be fixed before preview 3

@superboyiii
Copy link
Member Author

RpcServer was updated last night, so it's now compiling against neo master. I filed neo-modules 289 to track updating RpcClient. It needs to be fixed before preview 3

Thanks, we've already worked on this, PR will be pushed soon.

@superboyiii
Copy link
Member Author

@devhawk Updated here. neo-project/neo-modules#296

@devhawk
Copy link
Contributor

devhawk commented Jul 21, 2020

@superboyiii I've started adapting neo-express and neo-debugger to master. I've found got two minor PRs so far that I'd like to see merged before preview 3: #1780 and #1783. As I make progress this week, there may be more minor PRs like these

@superboyiii
Copy link
Member Author

@devhawk Any urgent PR for release is recommanded to commit to preview3 branch. It's the release branch and we'll merge it to master after neo v3.0.0-preview3 released.

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

No branches or pull requests

7 participants