stellar / stellar-cli

CLI for Stellar developers
Apache License 2.0
74 stars 71 forks source link

`Stellar tx new payment` field of `amount` is orders of magnitude off. #1693

Open TomMD opened 3 weeks ago

TomMD commented 3 weeks ago

stellar tx new payment says:

      --asset <ASSET>
          Asset to send, default native, e.i. XLM [default: native]
      --amount <AMOUNT>
          Amount of the aforementioned asset to send

This seems incorrect. The amount is actually defaulting to stroop and the amount is thus orders of magnitude different.

willemneal commented 3 weeks ago

This was copy/pasted from the docs: https://developers.stellar.org/docs/learn/fundamentals/transactions/list-of-operations#payment

Parameters Type Description
Destination account ID Account address that receives the payment.
Asset asset Asset to send to the destination account.
Amount integer Amount of the aforementioned asset to send.

Would you prefer more detail? e.g. Amount of the aforementioned asset to send, in the case of XLM in units of stroop

leighmcculloch commented 3 weeks ago

The doc for the amount option should be improved and show what a value should look like.

In regards to the issue of whether the option should be a decimal or in stroops:

There's a UX argument for yes, it should be decimal 1.000000, or just 1, instead of a fixed point integer 1000000 because that's what users see in wallets and the consistency between the CLI and wallets would be less confusing.

But there's also a UX argument for consistency within the CLI, and other commands in the CLI already accept the integer form because in transactions and on the network amounts are always represented as integers. This is because the network actually only knows stroops, and any concept or conventions of decimal places is something added off chain. e.g.:

$ stellar contract invoke --id C... -- transfer --amount 1000000 ...

So I think we should opt for the latter, consistency within the CLI, and make the documentation much clearer.

janewang commented 2 weeks ago

Please also add to upstream docs because it was copy and pasted from there.