novasamatech / parity-signer

Air-gapped crypto wallet.
https://www.parity.io/signer/
GNU General Public License v3.0
554 stars 164 forks source link

Transaction description support for Assets.transfer and Currencies.transfer #1050

Open stepanLav opened 2 years ago

stepanLav commented 2 years ago

We are faced with the problem related to showing an amount of operation when we are tried to sign transactions from the assets and currencies pallets.

Example for transfer 1 token with id 81 which has decimals 10 (WND has 12): https://westmint.statescan.io/event/2201385-3

Preocnditions: Added the Westmint network in ParitySigner https://pgolovkin.github.io/metadata-portal/#/westmint

Case:

  1. Generate QR for Assets.transfer(id=81, "5GFFVSRvsWvcM1vGyAHUPg1YPdWCtMjgfeU3Xrue97WqEGnf", 10_000_000_000)

    Screenshot 2022-05-23 at 16 20 19
  2. Scan it with Parity Signer

Actual result: It shows as WND token with wrong decimals

How it showing in the app: ![2022-05-23 15 21 38](https://user-images.githubusercontent.com/40560660/169818264-022e5065-86a8-4051-a739-2357472a55ef.jpg)

Do you have in plans to add a support for such transactions?

varovainen commented 2 years ago

Have been waiting for someone to use assets for a while, as I did not have anything to test on.

So, the transaction parser interprets certain values as balance based on type_name from the network metadata. And it worked so far because most popular apparently are the transfers in the pallet_balances.

Here it is also transfer, but in pallet_assets.

The call fields, from westmint900 metadata, in this case are:

So, in the last one the parser found type_name: Some("T::Balance") and interpreted it as a balance in westmint, not as a balance in asset units.

I see following possible options:

  1. If the metadata contains somewhere information on what exactly the asset variants are (i.e. what unit the asset 81 has), print the balance with correct unit and decimals.
  2. Whitelist. Keep proper balance display only for cases related to pallet_balances and to tips (btw, should the tip in your transaction be in westmint units?).
  3. Blacklist. Forbid display balance in cases related to pallet_assets and display the numbers as raw (i.e. 100000000 UNIT or something and let the user calculate the real value).

Pity I do not know of any standard defining strictly what to format as the balance and what not to.

stepanLav commented 2 years ago

Thank you for pay attention on my issue!

It's look like a not trivial problem, as in the metadata haven't data about token decimals 😞. Regarding tip, as I know, it should be paid as fee in the main asset of the network.

Slesarew commented 2 years ago

As far as I understand, those assets reside in chain state. From Signer's point of view, what happens on chain stays on chain - thus it will always be agnostic to those units. In best case scenario (probably the only "correct" thing to do), they should be rendered as unitless integers.

Blacklisting is attractive solution, but it will eventually result in same situation later - a pallet with code word Balance will be added meaning something completely different.

Whitelisting is not elegant at all and will probably break something, but should be robust. We'll have to make quite a list to cover all complex things that severely rely on balance formatting, like staking.

stepanLav commented 2 years ago

@varovainen Hello! want to remind of this problem 😅

Looks like display amount as unitless integers may be a balanced solution, isn't it?

varovainen commented 2 years ago

@stepanLav What happens in Signer is up to Signer team. I am completely reworking parser crate into standalone transaction parser currently (for my own purposes), and there the balance gets "formatted" in whitelisted set of pallets and displayed raw in all other pallets.