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

Implement NEP-17 #2024

Merged
merged 41 commits into from
Nov 18, 2020
Merged

Implement NEP-17 #2024

merged 41 commits into from
Nov 18, 2020

Conversation

shargon
Copy link
Member

@shargon shargon commented Oct 26, 2020

Related to neo-project/proposals#108 (comment)
Close #2023 #2012
Require neo-project/proposals#124

Version with call flags

@shargon shargon changed the title Add IfExists to CallFlags NEP5 onPayment for NativeContracts Oct 27, 2020
@shargon
Copy link
Member Author

shargon commented Oct 29, 2020

using Neo.IO;
using Neo.Ledger;
using Neo.SmartContract.Manifest;
using Neo.VM.Types;
using System;
using System.Linq;
using System.Numerics;
using Array = Neo.VM.Types.Array;

namespace Neo.SmartContract
{
    partial class ApplicationEngine
    {
        public static readonly InteropDescriptor NEP5_EmitTransfer = Register("NEP5.EmitTransfer", nameof(NEP5EmitTransfer), 0, CallFlags.AllowModifyStates, false);

        protected internal void NEP5EmitTransfer(UInt160 from, UInt160 to, BigInteger amount)
        {
            // Send notification

            SendNotification(CurrentScriptHash, "Transfer",
                new Array { from == null ? StackItem.Null : from.ToArray(), to == null ? StackItem.Null : to.ToArray(), amount });

            if (to == null)
            {
                return;
            }

            // Call onPayment method if exists

            ContractState contract = Snapshot.Contracts.TryGet(to);
            if (contract is null)
            {
                return;
            }
            ContractMethodDescriptor md = contract.Manifest.Abi.GetMethod("onPayment");
            if (md is null)
            {
                return;
            }

            ContractManifest currentManifest = Snapshot.Contracts.TryGet(CurrentScriptHash)?.Manifest;
            if (currentManifest != null && !currentManifest.CanCall(contract.Manifest, md.Name))
                throw new InvalidOperationException($"Cannot Call Method {md.Name} Of Contract {contract.ScriptHash} From Contract {CurrentScriptHash}");

            CallContractInternal(contract, md, new Array(ReferenceCounter) { amount }, CallFlags.All, CheckReturnType.EnsureIsEmpty);
        }
    }
}

What do you think?

@erikzhang
Copy link
Member

I don't think it's a good idea. We may have a lot of NEPs in the future. Maybe NEP-5 will be obsolete in the future, what shall we do then?

@shargon
Copy link
Member Author

shargon commented Oct 29, 2020

@erikzhang Please take a look again, I added DropResult into CheckReturnType in order to fix the problem with native contracts. Regular smart contracts must call first System.Contract.Exists

engine.SendNotification(Hash, "Transfer",
new Array { from == null ? StackItem.Null : from.ToArray(), to == null ? StackItem.Null : to.ToArray(), amount });

if (!engine.ContractExists(to, "onPayment")) return;
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi Shargon, I noticed that now neo is doing some method checks using hardcoded strings for method names ('verify', for example). I believe this may cause some issues in the future. First 'verify', now 'onPayment'. Maybe the manifest file should have a section to map these methods into ABI methods. This way, instead of checking only if the method exists, you would get the name of it first, and if it exists, execute it.

Copy link
Member Author

Choose a reason for hiding this comment

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

instead of checking only if the method exists, you would get the name of it first, and if it exists, execute it.

Could you give me an example, i don't see the difference right now. 😕

Copy link
Contributor

Choose a reason for hiding this comment

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

erikzhang
erikzhang previously approved these changes Nov 15, 2020
@superboyiii superboyiii mentioned this pull request Nov 16, 2020
44 tasks
@ProDog
Copy link
Contributor

ProDog commented Nov 16, 2020

In neo-project/neo-devpack-dotnet#391, hash has removed from the abi.
So here need to sync

Hash = UInt160.Parse(json["hash"].AsString()),

in #2044.

erikzhang
erikzhang previously approved these changes Nov 17, 2020
Co-authored-by: Luchuan <luchuan@ngd.neo.org>
@erikzhang erikzhang added this to the NEO 3.0 milestone Nov 18, 2020
@shargon
Copy link
Member Author

shargon commented Nov 18, 2020

@erikzhang Merge before the proposal, or we should wait?

@erikzhang
Copy link
Member

We can merge it first. If the proposal changes, we can create another PR.

@shargon shargon merged commit 72eb3e6 into neo-project:master Nov 18, 2020
@shargon shargon deleted the allow-call-if-exists branch November 18, 2020 15:57
ShawnYun pushed a commit to ShawnYun/neo that referenced this pull request Jan 8, 2021
* If exists

* Call onPayment if to it's a smart contract

* Increase cost in transfer

* Remove Mint check

* return

* Remove extra args

* Drop result

* Clean code

* Method.Exists

* Rename

* protected

* Update ApplicationEngine.Contract.cs

* Fix merge

* Add Name in Extra

* Name in manifest

* Fix UT

* dotnet format

* Remove Method.Exists

* Clean code

* Move filed `Name`

* Rename

* Update null checks

* Fix CallFromNativeContract parameters

* Update AssetDescriptor.cs

* Fix UT

* format

* Shargon's suggestion

* Update src/neo/SmartContract/Native/Tokens/Nep17Token.cs

Co-authored-by: Luchuan <luchuan@ngd.neo.org>

* Fix

Co-authored-by: Erik Zhang <erik@neo.org>
Co-authored-by: Luchuan <luchuan@ngd.neo.org>
cloud8little pushed a commit to cloud8little/neo that referenced this pull request Jan 24, 2021
* If exists

* Call onPayment if to it's a smart contract

* Increase cost in transfer

* Remove Mint check

* return

* Remove extra args

* Drop result

* Clean code

* Method.Exists

* Rename

* protected

* Update ApplicationEngine.Contract.cs

* Fix merge

* Add Name in Extra

* Name in manifest

* Fix UT

* dotnet format

* Remove Method.Exists

* Clean code

* Move filed `Name`

* Rename

* Update null checks

* Fix CallFromNativeContract parameters

* Update AssetDescriptor.cs

* Fix UT

* format

* Shargon's suggestion

* Update src/neo/SmartContract/Native/Tokens/Nep17Token.cs

Co-authored-by: Luchuan <luchuan@ngd.neo.org>

* Fix

Co-authored-by: Erik Zhang <erik@neo.org>
Co-authored-by: Luchuan <luchuan@ngd.neo.org>
ixje added a commit to CityOfZion/neo-mamba that referenced this pull request Feb 2, 2021
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

Successfully merging this pull request may close these issues.

7 participants