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

NEP: URI Standard for Native Asset Transfer #25

Merged
merged 26 commits into from
Jun 9, 2018

Conversation

saltyskip
Copy link
Contributor

No description provided.

nep-7.mediawiki Outdated

===Native Asset Transfer URI===

A Native asset transfer would have the following URI. It describes the reciever address, assetID, and additional attributes that will be sent with the transaction. The attributes are presented in raw hex data.
Copy link
Member

Choose a reason for hiding this comment

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

Native asset or Native Asset or native asset?

receiver spelling error

nep-7.mediawiki Outdated

A Native asset transfer would have the following URI. It describes the reciever address, assetID, and additional attributes that will be sent with the transaction. The attributes are presented in raw hex data.

<code>NEO:<address>[?assetID=<assetID>][?amount=<amount>][?attributes=<key>=<value>]</code>
Copy link
Member

Choose a reason for hiding this comment

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

I would prefer that the key/value pair be separated by another symbol. I think its good to follow querystring conventions, not sure if the decimal point in the amount will screw things up

Copy link

@apisit apisit Jan 15, 2018

Choose a reason for hiding this comment

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

I believe this follows query string convention. the decimal point will not screw things up. Please see sample implementation.

nep-7.mediawiki Outdated

<code>NEO:AeNkbJdiMx49kBStQdDih7BzfDwyTNVRfb?assetID=c56f33fc6ecfcd0c225c4ab356fee59390af8560be0e930faebe74a6daff7c9b&amount=1.0</code>

Begin transaction to specified address of 1 NEO with transaction attribute Remark = "Hello"
Copy link
Member

Choose a reason for hiding this comment

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

Can we have another example with 2 attributes? It is currently not clear if i can attach 2 or more attributes and how to do it.

One way we can do it is to just attach the entire attribute serialized string in. So a tx with 3 Hello remarks will be:

attributes=f00548656c6c6ff00548656c6c6ff00548656c6c6f

Whether we want to include the array length byte would be up to the standard.

Copy link

Choose a reason for hiding this comment

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

Will do.
Here is the example of attaching more than one attributes. Implemented using standard URL package in Go.

https://play.golang.org/p/USCC87orRMS

nep-7.mediawiki Outdated
| Script
| Additional validation of transactions
|-
| 0x30
Copy link
Member

Choose a reason for hiding this comment

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

This would be pretty cool and useful to implement a vote for me button

nep-7.mediawiki Outdated
@@ -0,0 +1,117 @@
<pre>
NEP: 7.0
Copy link
Member

Choose a reason for hiding this comment

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

The number 7 has been alloced for NEP: Triggers for NeoContract.
After a proposal being accepted, I will alloc a number for it.

@erikzhang erikzhang changed the title NEP-7 URI Standard for Native Asset Transfer NEP: URI Standard for Native Asset Transfer Jan 15, 2018
@erikzhang
Copy link
Member

As a URI standard for transfer, do we really need complete support for transaction attributes? Or do we just support some of the attributes associated with the transfer?
Like this:

neo:AQc5mtFayAdoCK13BW1cGAzAHyo9SoUWe7?asset=602c79718b16e442de58778e148d0b1084e3b2dffd5de6b7b16cee7969282de7&amount=0.1&description=Hello

@apisit
Copy link

apisit commented Jan 17, 2018

Transaction attributes are optional.

I believe putting a list of supported attribute keys in the proposal can benefit developers who are reading it. It's just reflecting the NEO document right now.

An application might choose to support only one attribute key. e.g. 0x90 for description
A list of supported keys is there to help with validation when an application chooses to implement it.

I totally agree that the more human-readable URI, the better.

I was thinking about the case which an application can let user generate a QR code and pass one's public key for ECDH in 0x02 field.

e.g.
NEO:AQc5mtFayAdoCK13BW1cGAzAHyo9SoUWe7?assetID= 602c79718b16e442de58778e148d0b1084e3b2dffd5de6b7b16cee7969282de7&amount=0.1&attributes=0x02=02ed53ad58c838435d4dd7a4b25c1eba01384c814ca53a539405434807afbb04b4

what if we have a map table of transaction attribute and URI key?

e.g.

NEO transaction attribute URI Key
0x02 publicKey
0x09 description
0xf0 remark1

note: not a complete list.

then it would be something like

NEO:AQc5mtFayAdoCK13BW1cGAzAHyo9SoUWe7?assetID= 602c79718b16e442de58778e148d0b1084e3b2dffd5de6b7b16cee7969282de7&amount=0.1&attributes=publicKey=02ed53ad58c838435d4dd7a4b25c1eba01384c814ca53a539405434807afbb04b4

@erikzhang
Copy link
Member

If we have a map table of transaction attribute and URI key, can we use the keys in URI directly?
e.g.

neo:AQc5mtFayAdoCK13BW1cGAzAHyo9SoUWe7?assetID= 602c79718b16e442de58778e148d0b1084e3b2dffd5de6b7b16cee7969282de7&amount=0.1&publicKey=02ed53ad58c838435d4dd7a4b25c1eba01384c814ca53a539405434807afbb04b4&description=Hello

By the way, the scheme name of URI is usually lowercase.

Added URI key to NEO transaction attribute
@saltyskip
Copy link
Contributor Author

A thorough reference implementation can be found here

https://github.com/O3Labs/NEP9-go

@saltyskip
Copy link
Contributor Author

@erikzhang What Needs to be done to push this to Final?

@erikzhang
Copy link
Member

I think we can support NEP-5 assets in this standard as well.

neo:<address>[?asset=<AssetID>|<ScriptHash>][&amount=<amount>][&<TransactionAttributeKey>=<value>]

If we want to transfer native asset, we put the AssetID to the asset field, which is 32-bytes length hash;

If we want to transfer NEP-5 asset, we put the ScriptHash of the contract to the asset field, which is 20-bytes length hash.

@apisit
Copy link

apisit commented Mar 18, 2018

I have the implementation of NEP-5 asset transfer and smart contract invocation. I'll update the proposal tomorrow to get some feedback.

@saltyskip
Copy link
Contributor Author

Won't this interfere with generic smart contract execution URI's in the future? If we do this then we'll have a specific format for a small subset of invoke transactions (NEP-5 token transfer) and another format for everything else?

Or is NEP-5 token transfer URI important enough such that it deserves it's own URI scheme?

@erikzhang
Copy link
Member

If we have the generic smart contract execution URI's, we do not need a small subset of NEP-5 token transfer.

@erikzhang
Copy link
Member

erikzhang commented May 3, 2018

I wonder if the complete AssetID hash is really necessary... I mean, if it could be determined by the first initial hash values (such as 602c79), like github does for commit hashes, perhaps that would save some valuable space (maybe for qrcodes). The ambiguity can possibly become a problem here?

It is very easy to make two different hash values with same first few bytes.

@igormcoelho
Copy link

igormcoelho commented May 3, 2018

It is very easy to make two different hash values with same first few bytes.

But the asset must be registered in the main net, required to pay all fees... and if this happens, people could simply start using more bytes in uri. But I get your point, for security purposes it's safer to use the complete id.

In fact, that just gave me another idea, do you think it is useful to have some sort of checksum in the uri? That would help avoiding small fraudulent changes on uri, and it could consider the whole assetid in the count, even if only the first bytes are present... so a few extra space in the checksum (that would also check the whole uri) would be compensated by shorter assetid. Perhaps I'm overcomplicating things, but some thoughts may be useful :)

@erikzhang
Copy link
Member

So what if the checksum itself be replaced?

- Removed `from` field in NEP5 Transfer URI
- Added URI Keys for NEP5 Transfer 
- Added sample implementation
@igormcoelho
Copy link

igormcoelho commented May 8, 2018

So what if the checksum itself be replaced?

Then we will need a checksum of the checksum :D hahah just kidding. I mean Erik, the current URI has no checksum, so data can be manipulated anyway (but I agree that's not in the scope of the NEP). I'm just saying that perhaps those middle bytes of the assetid could be replaced by something that represents all transfer information (including the whole assetid), and an extra checksum, with fewer space. But theoretically speaking, I agree with you that there may exist another checksum that is coeherent with the manipulated assetid (not the original one, missing pieces), and also coherent with the rest of the data.. but I don't imagine how to generate it, really. In another way, perhaps it's just easier to allow "asset=NEO" or "asset=GAS", that will achieve the same effect with less complication than my proposal. The checksum can be optional anyway, as everything else I'm saying :D If anyone is interested I can create some practical examples to put here, but like I said, just a thought. ;)

@mwherman2000
Copy link
Contributor

mwherman2000 commented May 15, 2018

Is this a valid query parameter string? i.e. multiple equal signs without the embedded equals sign being escaped?

attributes=0x02=02ed53ad58c838435d4dd7a4b25c1eba01384c814ca53a539405434807afbb04b4

...I can't find definitive reference for this but there it is a common convention for the query component of URI to be well-formed name=value pairs. e.g. https://en.wikipedia.org/wiki/Uniform_Resource_Identifier#Generic_syntax

@apisit
Copy link

apisit commented May 17, 2018

@mwherman2000 You might saw the old one. It's already updated to use Transaction attribute name as a query key.

I have updated the proposal to include detail on endianness and optional fields for NEP5.

We have a simple implementation in JS of embedding a QR code generated with this NEP9 spec here

@igormcoelho
Copy link

igormcoelho commented May 17, 2018

This script for generating the QR Code is really nice, showing that my previous comments on compressing the URI (just for QR code usage) were actually not necessary :)
Congratulations for this nice NEP!

@erikzhang
Copy link
Member

erikzhang commented May 18, 2018

Can we handle the transfer of global assets and the transfer of NEP assets in a unified way?

neo:AeNkbJdiMx49kBStQdDih7BzfDwyTNVRfb?asset=602c79718b16e442de58778e148d0b1084e3b2dffd5de6b7b16cee7969282de7

For NEP-5 tokens:

neo:AeNkbJdiMx49kBStQdDih7BzfDwyTNVRfb?asset=ceab719b8baa2310f232ee0d277c061704541cfb

@apisit
Copy link

apisit commented May 21, 2018

yeah that looks better.
I'll update it.

@erikzhang updated. Please have a look.

Unified native asset transfer and NEP-5 transfer
@erikzhang
Copy link
Member

Two suggestions:

  1. Now the article structure is a bit messy, can you rearrange it?
  2. For the asset field, can we support the alias for neo and gas?

apisit and others added 4 commits May 22, 2018 17:08
- Reorganized the page.
- Allowing using `neo` and `gas` as an alias in native asset transfer.
@erikzhang
Copy link
Member

Now we need to update the implementations.

@apisit
Copy link

apisit commented May 23, 2018

@erikzhang
Copy link
Member

One last question, should we mark it as Final or Active?

@nickfujita
Copy link

I've updated the sample QR code creation page to conform to the latest updates:

  • update assetID to asset
  • update for neo and gas aliases for asset
  • updates for nep5 token transfer compatability

This can be used as an example for QR code creation in accordance with NEP9.

https://o3.network/nep9/
source

@apisit
Copy link

apisit commented Jun 4, 2018

@erikzhang I voted it to be Final

@saltyskip
Copy link
Contributor Author

👍 for final

@nickfujita
Copy link

+1 to finalize! 💯

@igormcoelho
Copy link

Congratulations, that's nice! Seems final to me as well :) +1

shargon
shargon previously approved these changes Jun 8, 2018
@erikzhang erikzhang merged commit 91aed6f into neo-project:master Jun 9, 2018
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.

9 participants