neo-project / proposals

NEO Enhancement Proposals
Creative Commons Attribution 4.0 International
136 stars 113 forks source link

NEP-3 Ammendment #84

Open igormcoelho opened 5 years ago

igormcoelho commented 5 years ago

I believe NeoContract ABI (https://github.com/neo-project/proposals/blob/master/nep-3.mediawiki) may have useful ammendments: 1) Officially support "Map" types, as proposed by Ricardo some time ago 2) Support "Any" type for return and parameters. This was unofficially supported on C# compiler, and further changed to ByteArray, but it would be really better to allow returning and receiving any type of stack item (BigInteger, ByteArray, Map, etc) officially in some dapp functions 3) Few wording changes, such as explaining Signature format as a 64-byte signature regarding Neo supported ECDSA standard (secp256r1), or even keeping this open to future possibilities on signature formats (with even different sizes if needed to)

erikzhang commented 5 years ago

Good suggestion.

igormcoelho commented 5 years ago

https://github.com/neo-project/neo/issues/504

Ricardo, please see this. @lock9

igormcoelho commented 5 years ago

Another important feature is a Witness type on NEP-3. I'll explain the details. It will work as the Address type (same as a 20-length byte[]), however with the requirement that this is a verified address (by means of the witness system). This strongly simplifies logic for users, and make code safer.

Using this, someone could write this code on NEP-5 (general language): transfer(Witness from, Address to, Integer value)

This means that parameter from is already validated by witness system (no explicit Runtime.CheckWitness will be needed on code).

Devpacks could adapt to it, for example, on C# we could provide a type conversion from Address type (or byte[]) to a Witness object. This function basically calls Runtime.CheckWitness over the parameter, verifies if it's correct, and return a Witness (same as a byte[], but validated from code perspective). On code, users could do (C# style):


object Main(string op, object[] args) {
...
    if (op == "transfer") {
        Witness from = ((byte[])args[0]).ToWitness(); // fails if it's not a verified witness (implicit call to Runtime.CheckWitness)
        Address to = ((byte[])args[1]).ToAddress(); // fails if size is not 20
        BigInteger value = ((BigInteger)args[2]).AsBigInteger(); // Shall we create a NonNegative type too? avoids negative values here already...
        return Transfer(from, to, value); // we could do in a single line too
    }
...
}

If this was standard since the beggining, we could have also avoided some validation problems on smart contracts.

What do you think @lightszero @erikzhang @shargon ?

igormcoelho commented 5 years ago

See https://github.com/neo-project/neo/issues/906 for other applications (automatically deducing that a witness is needed, on wallets via ABI).

A good example is also DomainRegister example, that requires authentication from both sides: transfer(Witness a, Witness b, String domain).

lock9 commented 5 years ago

@igormcoelho what if you divide this into 2 issues? it will be easier to implement, review and test.

Regarding the initial proposition, I think we should support JSON. Someone mentioned some json stuff these days, I just don't remember where. It was in the persistence layer I think 🤔

No Json = No Happy developers

igormcoelho commented 5 years ago

Ok, so we have many possibilities for ammendments here hahaha JSON is interop-layer element... but since NEP-3 covers all Neo, I don't see any problem in that. Thinking from this side, we would need too for enumerators (NFT), perhaps contracts, and all available (and useful) interop types. Let's think on how to proceed, without dumping all at once :joy: For Witness, I think it's absolutely necessary, in order to simplify wallet operations.

lock9 commented 5 years ago

@igormcoelho can you make a list? We can have other issues to add other features, I'm afraid we get stuck if we don't reduce the scope of this amendment, maybe "nep-3 amendment" is too broad? 😅 The smaller you make this issue, we have more chances of having it implemented in a short time frame.