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

NIP 0007 - QR Library Standard Definition #3

Open
AnthonyLaw opened this issue Aug 9, 2018 · 20 comments

Comments

Projects
None yet
7 participants
@AnthonyLaw
Copy link

commented Aug 9, 2018

NIP-? - QR Library Specification

Summary

    NIP: ?
    Title: QR Library Specification
    Author: Anthony Law <anthonylaw@nem.my>
    Co-Author: Aleix <aleix@nemeurope.eu>
    Comments-URI: https://github.com/nemtech/NIP/issues/3
    Status: Draft
    Type: Standards Track
    Layer: Library
    Created: 2018-7-15
    License: MIT

Introduction/Abstract

The purpose of this NIP is to create/define QR library specification standard. Which allows generating QRCode for NEM wallet backups, creating payment requests (invoices) and adding address to a contacts list.

The benefit for standardizing QR library specification is to help all SDKs in adopting a common QR schema.

Motivation

In NIS1 there was not a standard, each project used a variation of a QR code schema. It always confuses developer, In addition, it doesn't support mosaics for invoice.

In addition, user announce transactions without sharing the private key, using only trusted wallets. it will really improve user experience easy to use and increase the security because users will be able to send transactions without sharing the private key across the network.

Design Decisions

In Catapult version, it supports cross chain transaction and decentralized asset swap. I foresee in the near future that will be a lot of side chain running on enterprise or business, and they perform cross chain transaction with the NEM mainnet.
To standardize the QR code schema is easy for the developer to apply it. we have many different types of transaction in NEM blockchain.

Base on this QR scheme, we should have an independent library and it should be able to communicate with current existing SDK.

QR library has to generate transaction output and input for SDK to process the transaction.

Specification

The table below shows, all the required field using in QR code standard parameters.

field type detail
schema integer 1 = add contract | 2 = transaction | 3 = import/export account
chain object Chain id use for cross-chain interoperability
type object TransferTransaction | AggregateTransaction | CosignatureTransaction
data object dataset

Add wallet contact

{
  "schema": 1,
  "network": "MAIN_NET",
  "nem_version": "Catapult",
  "data": {
    "address": "NA...."
  }
}

Transaction (request Assets / Invoice)

{
  "schema": 2,
  "network": "MAIN_NET",
  "nem_version": "Catapult",
  "type": "TransferTransaction",
  "data": {
    "recipient": "NA....",
    "assets": [
      {
        "id": "nem:xem",
        "relativeQuantity": 1
      }
    ],
    "message": {
      "type": 1,
      "payload": "my message"
    }
  }
}

Transaction (AggregateCompleteTransaction / multiple transaction with different invoice)

{
  "schema": 2,
  "network": "MAIN_NET",
  "nem_version": "Catapult",
  "type": "AggregateTransaction",
  "data": {
    "AggregateCompleteTransaction": [
      {
        "recipient": "NA....",
        "assets": [
          {
            "id": "nem:xem",
            "relativeQuantity": 1
          }
        ],
        "message": {
          "type": 1,
          "payload": "my message"
        }
      }
    ]
  }
}

Transaction (Co-sign Transaction)

{
  "schema": 2,
  "network": "MAIN_NET",
  "nem_version": "Catapult",
  "type": "CosignatureTransaction",
  "data": {
    "AggregateBondedTransaction": [
      {
        "hash": "6714c22c89c81c656fb771a0492bda7bce6195f7d3ce10fff7cb1b343fdf7671"
      }
    ]
  }
}

Import/Export Account

{
  "schema": 3,
  "network": "MAIN_NET",
  "nem_version": "Catapult",
  "data": {
    "encryptedKey": "6714c22c89c81c656....", // vi (32) + encryptedKey (96)
    "salt":"6c9ba0c43cd75b4f323947273db9b6f95aef95695197c0b24a5e10116e3923d1"
  }
}

Note: We should not directly show the account's privateKey in QR content, the output of "encryptedKey" and "salt" is encrypted data from the account's privateKey + user wallet password, this data format is currently used in NEM wallet (import/Export).

PrivateKey will be encrypted with user wallet password, and decrypt by user wallet password.

Implementation

We should create an independent QR library Just like Apostille-library. it's easy to maintain and update if there is any new QR schema added in.

QR library generates the output and Catapult SDK to do sign transaction and announce.

Read
QR code --> QR library Scan --> QR library generate tx output -> SDK sign and announce

Write
Define transaction (tx) -> generate QR code

Backwards compatibility

The previous QR schemas used in NIS 1 was not well design and lack of support such as a document. In addition, that is a lot of different variables QR schema in different SDK.

The new QR schema design is not compatible with the previous schemas used in NIS 1, because Catapult and NIS 1 have big different structures design itself.

The idea of creating whole new QR schema is to provide a standardizing QR schema and less complexity. Everyone can use this QR schema to build the application or SDK.

Drawbacks

we should not involve to many inner transaction inside AggregateCompleteTransaction
because maximum character storage capacity for QRcode max is 4296 Alphanumeric

we may able to append many inner transactions, it depends on the transaction content. An example that is 0 message and each transaction are attached 1 asset.

Not all types of transaction are covered right now, but here is the list of the transaction type I thinking the most often used:

  • TransferTransaction : transfer asset from one to another one address.

  • AggregateTransaction

    • AggregateCompleteTransaction: transfer multiple transactions by signing used a single private key in one transaction.
    • AggregateBondedTransaction: transfer multiple transactions and involved multiple parties signing in one transaction.
  • CosignatureTransaction: sign transaction by transaction hash.

  • LockFundsTransaction: Announce a lock funds transaction before sending a signed aggregate bonded transaction. This mechanism is required to prevent network spamming.

  • SecretLockTransaction : Use a secret lock transaction to start the cross-chain swap

  • SecretProofTransaction : Use a secret proof transaction to unlock secret lock transactions.

In the future, if there is any new plugin for the catapult, we can just add the new QR schema.

Alternatives

For QR content, current JSON object format.
the alternatives will be using URI Query base, the benefit of URI it already has some URI protocol such as open default email or open application apps. Check on this NIP #6

For Add Wallet contact we actually can think of how to use vCard in our catapult wallet, the advantage of Vcard it can directly save into device contact list.

References

QR Service Library on NIS 1

NEM-SDK : QR object library
NEM-Library: QRService library
NEM Android : Android QR Library

Catapult SDK

Typescript/Javascript : nem2-sdk-typescript-javascript
Java : nem2-sdk-java
more : NEMTECH Doc

Contributors

Special thanks to @aleixmorgadas and @dgarcia360 for contributing actively to improve this NIP.

History

Date Version
July 15 2018 Initial Draft
Aug 6 2018 Second Draft
Dec 23 2018 Third Draft
@jorisadri

This comment has been minimized.

Copy link

commented Aug 27, 2018

Do you want to make in compatible with nis1 and catapult?
My concern is that the tag "version" has different meaning in different version numbers.
Version 1 and 2 are related to network type(testnet-mainnet) and version 3 is related to the nis version.

@dgarcia360

This comment has been minimized.

Copy link
Collaborator

commented Aug 27, 2018

To include the NIP in the repository and assign a number, the following must be achieved:

NIP must sound complete

I recommend moving the content under "Design Desitions" to "Specification". Then, complete "Design decisions" section:

* Reason about the correctness of the implementation.
* "Feel and fit" with existing core libraries.
* Performance and threading considerations.
* Potential conflicts with existing libraries, e.g. name clashes, IDE auto-import friction, etc.

Tite describes the content accurately:

✓, but I suggest to change it to QR Library or QR Library Specification to fit Apostille proposal.

NIP draft has been published as an issue

Motivation and backward compatibility must be addressed

Express in more detail the motivation. As a suggestion, you could propose the need to have an agreed standard to create QR codes in NEM that can be read with any app. Also, comment on the benefits of doing so. Put some examples of apps working with different QR standards.

Backward compatibility commented through the NIP, but not present in this section.

Header correctness

Add an author Layer should go after Type. Delete * symbols.

Licensing terms are acceptable

@dgarcia360

This comment has been minimized.

Copy link
Collaborator

commented Aug 27, 2018

From a technical perspective, I agree with Joris. In the proposed NIP version treat different things. In addition, I can't find the proposed QR codes for types 1 and 3.

@aleixmorgadas

This comment has been minimized.

Copy link

commented Nov 15, 2018

I would say the QR Data Schema isn't clear without checking this specification. I think it can be simplified enough to become more obvious.

{
  "schema": 1,
  "network": "MAIN_NET",
  "nem_version": "NIS",
  "type": "TransferTransaction",
  "data": {
    "recipient": "NA....",
    "assets": [
      {
        "id": "nem:xem",
        "relativeQuantity": 1
      }
    ],
    "message": {
      "type": 1,
      "payload": "my message"
    }
  }
}

IMO, a schema definition similar to this one is more clear and obvious of the meaning of each field. It also helps mapping the QR Data into model constructors

@aleixmorgadas

This comment has been minimized.

Copy link

commented Nov 15, 2018

Also, I would just drop NIS 1 schema and prepare the Schema just for Catapult in case the scope is too big.

@AnthonyLaw

This comment has been minimized.

Copy link
Author

commented Nov 21, 2018

@aleixmorgadas i got your idea. thank you for the advise.
I think should drop NIS 1, because it will add more complexity.

@aleixmorgadas

This comment has been minimized.

Copy link

commented Nov 21, 2018

I'm OK in dropping in the schema definition, but in the QR Library, both should be supported since NEM applications would like to migrate faster to Catapult.

@dgarcia360

This comment has been minimized.

Copy link
Collaborator

commented Nov 23, 2018

@AnthonyLaw The next step is to update the NIP addressing this review.

Consider updating the QR data schema with the structure proposed by @aleixmorgadas. Start a discussion if you think the new structure can be improved.

If you decide to use it, you could add him as a NIP co-author or contributor.

@dgarcia360

This comment has been minimized.

Copy link
Collaborator

commented Dec 10, 2018

Added a review #4

@dgarcia360

This comment has been minimized.

Copy link
Collaborator

commented Dec 20, 2018

Changes of the first review applied. In progress a second review #4

@AnthonyLaw Please keep updated the opening message with the latest NIP version.

@dgarcia360

This comment has been minimized.

Copy link
Collaborator

commented Dec 20, 2018

@AnthonyLaw @aleixmorgadas

We could achieve a better result if we follow similar practices in #6 and #4.

  1. #6 proposes to use a schema that allows JSON or binary transactions representation. The definition of the QR schema for preparing transactions can benefit from it:
  • Large transactions can be masked under the QR code using serialized data (e.g. large aggregates).
  • The QR code still human-readable if the user selects to format them as JSON. This could be still used for not complex transactions.
  • The JSON format for each transaction is already defined in the SDK. There is no need to define new schemas for each transaction for the QR code.
  1. I'm not a big fan of using cryptic variables names (example: msg, qty, addrss, v). How about using the complete name?
@aleixmorgadas

This comment has been minimized.

Copy link

commented Dec 20, 2018

We could achieve a better result if we follow similar practices in #6 and #4.

Totally agree, have cohesion between similar methods it's the way to go ^^

@AnthonyLaw

This comment has been minimized.

Copy link
Author

commented Jan 15, 2019

@AnthonyLaw @aleixmorgadas

Editor 🎩 off, entering in developer mode:

We could achieve a better result if we follow similar practices in #6 and #4.

  1. #6 proposes to use a schema that allows JSON or binary transactions representation. The definition of the QR schema for preparing transactions can benefit from it:
  • Large transactions can be masked under the QR code using serialized data (e.g. large aggregates).
  • The QR code still human-readable if the user selects to format them as JSON. This could be still used for not complex transactions.
  • The JSON format for each transaction is already defined in the SDK. There is no need to define new schemas for each transaction for the QR code.
  1. I'm not a big fan of using cryptic variables names (example: msg, qty, addrss, v). How about using the complete name?
  1. Agree, the concept will be define URI schemes, and disply in QR.
  2. Yes, i will agree to using complete name, easy to other people to read and understand the code.
@jabo38

This comment has been minimized.

Copy link

commented Jan 21, 2019

Can we have a "prunable payload": "message meant as metadata" added to the QR standard? This gives future app makers the ability to put in customizable data that most apps would just ignore in formatting a tx but could possibly have extra data that certain apps like to know before being pruned. Like I could include the name, address, and phone number of the person making the QR code, but of course, that doesn't need to be included in the transaction, but would still be nice to have in some circumstances and could be stored locally on the receiver's database. The prunable payload could reference previous txs on the blockchain, include secret encrypted messages, reference other invoices, really just about anything. It would also allow businesses to make its own specific standard defined in the prunable payload within the general QR standard and still be compatible with all apps.

This kind of reminds me of the OP_Return in Bitcoin, which wasn't really mean to be used for anything but then ended up being used for everything.

@9zero

This comment has been minimized.

Copy link

commented Feb 15, 2019

  1. There will be many private chains, a chainId is necessary.
  2. relativeQuantity is better.
  3. signature of data should be considered. maybe need deadline
@dgarcia360

This comment has been minimized.

Copy link
Collaborator

commented Apr 10, 2019

Have updated the opening thread with the latest pull-request submitted.

There are some open issues to make this NIP eligible to be merged into the repository, being the major problem specification table not matching the specification examples.

@AnthonyLaw Is there any plans to create a first implementation to test the NIP proposed?

@AnthonyLaw

This comment has been minimized.

Copy link
Author

commented Apr 17, 2019

Have updated the opening thread with the latest pull-request submitted.

There are some open issues to make this NIP eligible to be merged into the repository, being the major problem specification table not matching the specification examples.

@AnthonyLaw Is there any plans to create a first implementation to test the NIP proposed?

I'm sorry and overlook on this.
and Yes @dgarcia360 I will do it.

@dgarcia360 dgarcia360 changed the title QR Library - Definition NIP 0007 - QR Library Standard Definition Apr 29, 2019

@dgarcia360

This comment has been minimized.

Copy link
Collaborator

commented Apr 29, 2019

Thanks @evias and @AnthonyLaw for submitting a new draft. The NIP sounds complete and has assigned the number 0007.

@evias

This comment has been minimized.

Copy link

commented Apr 30, 2019

I will continue updating it during this week to meet standard requirements 👍

Also probably coordinating with Anthony to get this issue updated.

@evias

This comment has been minimized.

Copy link

commented May 1, 2019

Updated PR to new #20 . Latest status update include following discussion points:

  • "network_id": 0x98, : does anyone think storying “MIJIN_TEST” string here would make sense more than the hex representation ? (character limitation aware)
  • “chain_id” field is prone to change as we are awaiting more specification on the cross-network topic.
  • “addr” field is a left-over from v1 + v2 QRs, I would like to change it everywhere to address or publicKey.
  • CosignatureTransaction do not have a type and are specific with hash only, what about this ?

evias added a commit to evias/NIP that referenced this issue May 17, 2019

nemtech#3 : Fixed review comments
- fixed title
- added transaction object is compatible with nip-2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.