search

neo-project / neo

NEO Smart Economy
MIT License
3.47k stars 1.03k forks source link

Neo3-Preview3 release checklist #1731

Closed superboyiii closed 4 years ago

superboyiii commented 4 years ago

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

@neo-project/core

roman-khimov commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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?

devhawk commented 4 years ago

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 commented 4 years ago

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

superboyiii commented 4 years ago

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 commented 4 years ago

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

Added

devhawk commented 4 years ago

FYI, Debug Info support for NEON has been merged

lock9 commented 4 years ago

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: https://github.com/neo-project/neo/blob/cd988e85222f4d9df614f969668d2a915094b2e2/src/neo/SmartContract/Helper.cs#L130

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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

Tommo-L commented 4 years ago

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 commented 4 years ago

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

erikzhang commented 4 years ago

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

The verify method is included in the ABI file.

lock9 commented 4 years ago

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 commented 4 years ago

Yes, you have the verification script https://github.com/neo-project/neo/blob/54095fc93991e90bfaf0adb8b807d3f403774f5c/src/neo/SmartContract/Helper.cs#L155 and the invocation script https://github.com/neo-project/neo/blob/54095fc93991e90bfaf0adb8b807d3f403774f5c/src/neo/SmartContract/Helper.cs#L168 in the invocation script you can push the arguments

lock9 commented 4 years ago

@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 commented 4 years ago

@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 commented 4 years ago

@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 commented 4 years ago

It's better to add https://github.com/neo-project/neo/pull/1752 in checklist, as it's incompatible.

superboyiii commented 4 years ago

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

Done

devhawk commented 4 years ago

FYI, #1758 is merged

devhawk commented 4 years ago

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

Tommo-L commented 4 years ago

@superboyiii This bug must be fixed before prievew3 https://github.com/neo-project/neo-devpack-dotnet/issues/300

devhawk commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

@devhawk Updated here. https://github.com/neo-project/neo-modules/pull/296

devhawk commented 4 years ago

@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 commented 4 years ago

@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.