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

Feature/nep5 updates #18

Closed
wants to merge 15 commits into from

Conversation

lllwvlvwlll
Copy link
Member

Changes optional methods to required to enable contract interactions

@lllwvlvwlll
Copy link
Member Author

@erikzhang
Copy link
Member

It is better to create a new NEP for this update.

@lllwvlvwlll
Copy link
Member Author

We should continue to use NEP5 because people associate 'NEP5' as the token format and there has been limited implementation. If we create another NEP for these changes, it will be very confusing to developers about which standard to use. We should attempt to maintain a standard, singular message about what the token standard is unless there are substantial changes.

The changes proposed in the PR are not substantial. In fact, all of the functionality already exists in the current NEP5.

Another option is to use the notation NEP5.1 to include these changes. This would indicate that there have been changes made, but would make it obvious to developers that they should use the new version.

@localhuman
Copy link
Contributor

I agree with @lllwvlvwlll, it would be good to update the existing standard rather than use a new one.

@erikzhang
Copy link
Member

I have mark it as accepted. Now we need to finish the dynamic invocation proposal first.

@neotracker
Copy link

The C# sample implementation is missing the transfer event, can we update the sample to include it?

Copy link

@canesin canesin left a comment

Choose a reason for hiding this comment

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

👍 .. very good

@canesin
Copy link

canesin commented Dec 20, 2017

@erikzhang can we merge this ?

@erikzhang
Copy link
Member

@canesin I suggest that:

  1. Keep the methods optional.
  2. Point out that these optional methods should be implemented if the token want to be supported by the decentralized exchange.
  3. Update the code for the ICO template and add the implementations of these optional methods.

@localhuman
Copy link
Contributor

We can update the ICO Template to add these methods.

I don't understand the opposition to requiring these methods in the standard. Why would someone want to spend a lot of GAS to create a token that can't be traded very easily?

Could you explain your worry about this?

@erikzhang
Copy link
Member

We need to keep the token standard as simple as possible, and in fact not all tokens have the need to trade on the decentralized exchange. They only need to implement the most basic transfer functions.
We just need to make these points clear. There is no need to force all tokens to implement these optional features.

@canesin
Copy link

canesin commented Dec 22, 2017

@erikzhang there are many applications besides decentralized exchanges, anything that need some system to behave as a assignee will need this - platforms like lending , crowd sourcing, freelancing and IoT markets. Having this as optional will create confusion on the ecosystem on why some of the tokens cannot participate on those. These are very simple methods, only 3, that have already been added to the examples.

@erikzhang
Copy link
Member

erikzhang commented Dec 25, 2017

I made a demo for DEX without allowance, transferFrom and approve.

using Neo.SmartContract.Framework;
using Neo.SmartContract.Framework.Services.Neo;
using Neo.SmartContract.Framework.Services.System;
using System;
using System.Numerics;

namespace DEX_Demo
{
    public class DEX : SmartContract
    {
        public static bool Main(string operation, object[] args)
        {
            switch (Runtime.Trigger)
            {
                case TriggerType.Verification:
                    //Ensure that the current transaction is an invocation to this contract.
                    return true;
                case TriggerType.Application:
                    switch (operation)
                    {
                        case "deposit":
                            return deposit((byte[])args[0], (BigInteger)args[1], (byte[])args[2], (byte[])args[3]);
                        case "withdraw":
                            return withdraw((byte[])args[0], (BigInteger)args[1], (byte[])args[2], (byte[])args[3]);
                        //case "order"://Place orders.
                        //    break;
                        default:
                            return false;
                    }
                default:
                    //Handle direct deposit.
                    return false;
            }
        }

        public static bool deposit(byte[] asset_id, BigInteger amount, byte[] from, byte[] to)
        {
            var func = (Func<string, object[], bool>)asset_id.ToDelegate();
            if (!func("transfer", new object[] { from, ExecutionEngine.ExecutingScriptHash, amount }))
                return false;
            byte[] key = to.Concat(asset_id);
            BigInteger value = Storage.Get(Storage.CurrentContext, key).AsBigInteger();
            Storage.Put(Storage.CurrentContext, key, value + amount);
            return true;
        }

        public static bool withdraw(byte[] asset_id, BigInteger amount, byte[] from, byte[] to)
        {
            byte[] key = from.Concat(asset_id);
            BigInteger value = Storage.Get(Storage.CurrentContext, key).AsBigInteger();
            if (value < amount) return false;
            var func = (Func<string, object[], bool>)asset_id.ToDelegate();
            if (!func("transfer", new object[] { ExecutionEngine.ExecutingScriptHash, to, amount }))
                return false;
            Storage.Put(Storage.CurrentContext, key, value - amount);
            return true;
        }
    }
}

@localhuman
Copy link
Contributor

This does not provide a standardized interface for interoperable NEP5 tokens.

Also, the standard should provide an interface where approval is granted by the token user to an address, then from within that SC any other SC can work with that approval and perform a transfer.

In this example, only this SC would be able to delegate, since the storage of who did the approval is specific to this SC.

We should design for this scenario:

  • user A has 100 tokens
  • user A grants access to 50 tokens to user B
  • user B can transfer 50 tokens to any account from user A balance
  • this interaction is determined by the token SC, not the DEX SC
  • this can be invoked from anywhere, not just the DEX SC

@RavenXce
Copy link

RavenXce commented Jan 31, 2018

@erikzhang I noticed you mentioned //Ensure that the current transaction is an invocation to this contract.

But while we can know that the transaction is of Invocation type, there is no way to check the InvocationTransaction.Script within the SmartContract? (need to verify that it is a TAILCALL to the same contract ScriptHash, otherwise I think APPCALL can be done to any other arbitrary SC, and passing transfer verification directly)

Reference: https://github.com/neo-project/neo/blob/master/neo/SmartContract/StateReader.cs

@erikzhang
Copy link
Member

erikzhang commented Jan 31, 2018

We can use System.ExecutionEngine.GetScriptContainer to get the current transaction and use Neo.Transaction.GetType to check whether it is an InvocationTransaction.
Then I will add a new API Neo.InvocationTransaction.GetScript for checking the script of it.

RavenXce added a commit to ConjurTech/neo that referenced this pull request Feb 7, 2018
This allows smart contracts to check that the transaction is
an invocation to the same contract during verification phase.

Example use case: neo-project/proposals#18 (comment)
RavenXce added a commit to ConjurTech/neo-devpack-dotnet that referenced this pull request Feb 7, 2018
This allows checking that a transaction is an invocation to the
same smart contract during verification phase.

Example usage: neo-project/proposals#18 (comment)
RavenXce added a commit to ConjurTech/neo-devpack-dotnet that referenced this pull request Feb 7, 2018
This allows checking that a transaction is an invocation to the
same smart contract during verification phase.

Example usage: neo-project/proposals#18 (comment)
RavenXce added a commit to ConjurTech/neo-devpack-dotnet that referenced this pull request Feb 7, 2018
This allows checking that a transaction is an invocation to the
same smart contract during verification phase.

Example usage: neo-project/proposals#18 (comment)
erikzhang pushed a commit to neo-project/neo that referenced this pull request Feb 8, 2018
This allows smart contracts to check that the transaction is
an invocation to the same contract during verification phase.

Example use case: neo-project/proposals#18 (comment)
erikzhang pushed a commit to neo-project/neo-devpack-dotnet that referenced this pull request Feb 8, 2018
This allows checking that a transaction is an invocation to the
same smart contract during verification phase.

Example usage: neo-project/proposals#18 (comment)
@mwherman2000
Copy link
Contributor

mwherman2000 commented Apr 17, 2018

[deleted]

@mwherman2000
Copy link
Contributor

mwherman2000 commented Apr 18, 2018

Here's an alternative, concrete approach to the one I suggested previously: #40

This based on the work Joe Stewart has done with Non-Fungbile Token Extensions to NEP-5 and the work I've done to support NEP-5 extensions for Requisitions.

Looking forward to your feedback.

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

Successfully merging this pull request may close these issues.

None yet

7 participants