rgb-std icon indicating copy to clipboard operation
rgb-std copied to clipboard

Use readable precision in invoice amount

Open dr-orlovsky opened this issue 1 year ago • 8 comments

The invoice for 100.00 of an asset with precision 2 or more should look like rgb:/RGB20/100.00+tb:utxob:... (independently from whether the precision is 2 or 8) and not like rgb:/RGB20/10000+tb:... (if the precision is 2) or like rgb:/RGB20/10000000000+tb:... (if the precision is 8).

This should help people to visualize how much they are actually paying.

dr-orlovsky avatar Dec 30 '23 12:12 dr-orlovsky

The invoice for 100.00 of an asset with precision 2 or more should look like rgb:/RGB20/100.00+tb:utxob:... (independently from whether the precision is 2 or 8) and not like rgb:/RGB20/10000+tb:... (if the precision is 2) or like rgb:/RGB20/10000000000+tb:... (if the precision is 8).

This should help people to visualize how much they are actually paying.

I think this works only when the contract is set in the invoice, right?

How do you imagine working when we don't define the contract?

crisdut avatar Jan 15 '24 20:01 crisdut

pub struct ContractAmount {
    pub int: u64,
    pub fract: u64,
    pub precision: u8,
}

let amount = ContractAmount::new(100, 2);
assert_eq!(amount.int, 100);
assert_eq!(amount.fract, 0);
assert_eq!(amount.clone().to_value(), 10000);
assert_eq!(amount.to_string(), "100.00");

shuimuliang avatar Jan 16 '24 03:01 shuimuliang

How do you imagine working when we don't define the contract?

When we do not specify the contract, the wallet will not know what to pay. You can't have invoice of "pay me 5 of anything". It can be either just "pay me 5 of this specific token" - or "pay me any amount of anything". Thus, it is good that with the proposed change we can't have invoices which pay a specific amount of unspecified something.

dr-orlovsky avatar Jan 18 '24 17:01 dr-orlovsky

How do you imagine working when we don't define the contract?

When we do not specify the contract, the wallet will not know what to pay. You can't have invoice of "pay me 5 of anything". It can be either just "pay me 5 of this specific token" - or "pay me any amount of anything". Thus, it is good that with the proposed change we can't have invoices which pay a specific amount of unspecified something.

Sure, even the rgb invoice allowed us to add iface or contract_Id as an optional parameter, when we pay for something, the wallet need know the contract_id and iface.

crisdut avatar Jan 18 '24 21:01 crisdut

For wallets working with integers is easier and less prone to errors, so I am not sure if making the invoice a bit easier to read for users that have difficulty to convert the amount between the base unit and the minimum one is worth it. Non technical users are anyway safer with just relaying on the data presented by the UI of their wallet rather than trying to decode the invoice manually.

fedsten avatar Mar 22 '24 14:03 fedsten

Why not both? It is good for the users to be able to check invoices with just eyes, not knowing the maximum precision of an asset.

dr-orlovsky avatar Mar 24 '24 18:03 dr-orlovsky

Why not both? It is good for the users to be able to check invoices with just eyes, not knowing the maximum precision of an asset.

IMO because adding precision makes wallets more prone to error. Handling integers is much simpler than handling floating numbers. Moreover, having precision in invoice introduces a possible error that would be impossible otherwise: specifying an amount with a precision different from the actual one of the requested contract. For example the invoice could request USDT (a contract with precision 2) and an amount of 100.123. Also, this mistake would arise only when trying to pay the invoice (unless the wallet is smart enough to parse the invoice, extract the contract metadata and make the required checks before trying to pay it). Moreover, as already pointed by @crisdut, an invoice with no embedded asset but an amount with precision could cause confusion and more mistakes (and as already said, currently specifying these invoices is possible and we would like this to remain a possibility, since we see a use case).

zoedberg avatar Mar 26 '24 15:03 zoedberg

There is no floating point: a precision makes two integers instead of floating point. Situations like 100.264 for precision 2 are hood since this is easily detectable error and the invoice must be invalidated.

dr-orlovsky avatar Mar 26 '24 19:03 dr-orlovsky

There is no floating point: a precision makes two integers instead of floating point.

You should handle the case in which you set both the integers to the max value of u64, which would result to an unsupported value.

Situations like 100.264 for precision 2 are hood since this is easily detectable error and the invoice must be invalidated.

IMO it's not an easy detectable error because it requires extracting the contract metadata, which requires an extra call and an available runtime (meaning a working internet connection).

I think some extra invoice readibility isn't worth the cost so I propose to close this issue.

zoedberg avatar May 21 '24 14:05 zoedberg

You should handle the case in which you set both the integers to the max value of u64, which would result to an unsupported value.

You have to handle overflows anyway, independently from the way you present amounts

IMO it's not an easy detectable error because it requires extracting the contract metadata, which requires an extra call and an available runtime (meaning a working internet connection).

It is easy detectable error since to pay invoice you have to interface the contract anyway

I think some extra invoice readibility isn't worth the cost so I propose to close this issue.

Strongly disagree. I do not hear a single valid argument why having percision in the invoice is bad.

dr-orlovsky avatar May 23 '24 10:05 dr-orlovsky

You have to handle overflows anyway, independently from the way you present amounts

Just 1 overflow instead of 3 (1 for merged number, 1 for left part and 1 for right part)

It is easy detectable error since to pay invoice you have to interface the contract anyway

You're assuming that immediately after parsing an invoice the user will pay it, that could not always be the case. For example a user with no internet connection could want to parse an invoice and then tell its wallet "try to pay it as soon as my internet connection is available"

Strongly disagree. I do not hear a single valid argument why having percision in the invoice is bad.

I think we should have a valid argument to implement this, rather than the opposite. The only reason you gave is "to make the invoice more user-readable", but this IMO is not a good argument. On the contrary, it's another reason to avoid this, because the more readable the invoice will be the higher the risk of users manually parsing the invoice will be, this could lead to more malicious users that try issuing contracts with similar IDs to other assets to trick the users into think they're paying something different. The user should be encouraged to always parse the invoice with an RGB-enabled wallet and never try to do that by themselves. Therefore, making the invoice more user-readable cannot be considered a valid argument to have this.

zoedberg avatar May 23 '24 14:05 zoedberg

The only reason you gave is "to make the invoice more user-readable", but this IMO is not a good argument.

this is the exact argument, and it is good for my opinion.

On the contrary, it's another reason to avoid this, because the more readable the invoice will be the higher the risk of users manually parsing the invoice will be

there is no reason to manually parse invoice. I know how my code works and I know that this can be done even if the user is offline.

dr-orlovsky avatar May 23 '24 15:05 dr-orlovsky

there is no reason to manually parse invoice.

If you think there are no reasons to manually parse the invoice then I don't understand why do you think we should make the invoice more readable.

I know how my code works and I know that this can be done even if the user is offline.

Correct, I forgot that we recently allowed the runtime to work offline.

Still I don't think we should add the precision to the invoice amount, for several reasons: a more readable invoice leads user to try to manually parse it (which is bad), ambiguos in case the invoice specifies an amount with precision but no asset and finally more checks and errors needed (therefore more code to write and maintain).

zoedberg avatar May 24 '24 12:05 zoedberg

If you think there are no reasons to manually parse the invoice then I don't understand why do you think we should make the invoice more readable.

Manual parsing is in no way equals to readability

Still I don't think we should add the precision to the invoice amount, for several reasons: a more readable invoice leads user to try to manually parse it (which is bad), ambiguos in case the invoice specifies an amount with precision but no asset and finally more checks and errors needed (therefore more code to write and maintain).

Sorry, this is just illogical: we either need to make amounts non-human-readable at all (if we are so afraid of "manual parsing", whatever that means) - or make them readable. Having them readable and misleading is the worst option of all.

dr-orlovsky avatar May 24 '24 18:05 dr-orlovsky

After giving a second though I agree that we'd better not give humans a chance of "manual parsing" and understanding what is there in the invoice; thus we need to hide the amount with some non-human encoding. Done in #205

dr-orlovsky avatar May 25 '24 11:05 dr-orlovsky

I'm happy you changed your mind on this and I think the proposed approach is better than the current one

zoedberg avatar May 28 '24 15:05 zoedberg

Always happy to make something human-unfriendly and harder to use :)

dr-orlovsky avatar May 28 '24 16:05 dr-orlovsky