JoshOrndorff / recipes

A Hands-On Cookbook for Aspiring Blockchain Chefs
GNU General Public License v3.0
378 stars 186 forks source link

Cannot input I16F16 in Apps UI #201

Open JoshOrndorff opened 4 years ago

JoshOrndorff commented 4 years ago

The Fixed Point recipe uses substrate-fixed's I16F16 signed fixed point type. The example works according to test cases, but users ca,not test it through the Apps UI because we don't know how to submit that data type. Apps does not like the input value even when 32 bits are supplied.

image

apopiak commented 4 years ago

I recently had a similar issue with Perbill and hacked it into the Apps UI in this PR. We should talk to Jaco about improving UI input. (AFAIK it's on his list of things that need fixing in the UI, but it seems hard to find the time.)

brenzi commented 4 years ago

Meanwhile we have solved this in our explorer. Would that be suitable to move upstream? https://github.com/encointer/explorer/blob/7541c77649252800761fb9a067b58b05b606c085/src/utils.js#L16-L38

Will not work with perbill out of the box because it assumes power of 2

Moreover, it converts the fixpoint to a float which is not the same as really supporting fixpoint in js

JoshOrndorff commented 4 years ago

Would that be suitable to move upstream?

That's up to @jacogr . It would be a PR against https://github.com/polkadot-js/apps

jacogr commented 4 years ago

The API doesn't know about I16F16 (it is not one of the types in the API), hence the UI doesn't know anything about I16F16, hence it doesn't allow input since it cannot be encoded at all.

brenzi commented 4 years ago

@jacogr I understand why its not possible now, but we're suggesting the following:

  1. encode and decode I16F16 as i32 and other types likewise
  2. display this type as float, converting it with our function linked above
  3. we'd need to adapt an inverse function to convert float to I16F16 (this is suboptimal because its a lossy function. But its better than nothing)
jacogr commented 4 years ago

I am not saying it is not possible - I am saying the API is not even aware of this type. So, trying to build UI support is not going to work since the API cannot even encode/decode it.And the UI will never have "custome encoding" that is not in the API types, that is a dead-end.

I don't have "all Rust types that may ever exist and be used someday in the API types", I have anything that is actually used by Substrate master and Polkadot. Since this type is NOT used there in any of the base modules, it never came up.

Having said that, there are a number of types NOT used in Substrate master/Polkadot that does have support ... it was implemented by people requiring it. (BTreeMap/BTreeSet comes to mind - that was available way before Substrate master actually used it since it was used by other parties in their clients)

So, if anybody wants it - then please log it at the correct place - even better, make a PR for it that implements SCALE for this type in the API types. (I don't take "API requests" in random repos since I can't track it, so it just drops off)

JoshOrndorff commented 4 years ago

Okay, I believe I understand where this needs to be logged and have logged it at https://github.com/polkadot-js/api/issues/2488