RPC: Use integer satoshis instead BTC with decimals #9855

pull jtimon wants to merge 4 commits into bitcoin:master from jtimon:0.15-rpc-satoshis changing 6 files +31 −19
  1. jtimon commented at 12:40 AM on February 25, 2017: contributor

    There's been talks about using satoshis in RPC instead of BTC, which requires decimals and don't work well with json.

    On the other hand, such a change would break the RPC API as well as many tests. This is a draft on how we could move there (doing it only for 2 of the 8 RPC calls that would need to be changed). An useDecimalAmounts optional param is added. If true, the user can introduce BTC amounts as before. If false, the user needs to introduce the amounts in satoshis.

    Interestingly enough, for signrawtransaction, useDecimalAmounts can default to false without breaking any tests (which means the amounts in useDecimalAmounts are not being currently tested). In contrast, createrawtransaction would break many tests.

    We could slowly adapt the tests to use satoshis and once that's done, at some point in the future we could break the API, remove all useDecimalAmounts params and not allow to BTC amounts anymore.

    I didn't want go further before getting some feedback.

  2. RPC: Allow AmountFromValue() to process in units other than satoshis 7406336a58
  3. RPC: Allow int satoshis instead of BTC with decimals for amounts:
    (using the option useDecimalAmounts)
    
    - createrawtransaction
    - signrawtransaction
    d59e225148
  4. TODO: named params f799d8b01d
  5. TODO: Remaining RPC functions that use BTC instead of satoshis for amounts 35854c31c3
  6. fanquake added the label RPC/REST/ZMQ on Feb 25, 2017
  7. da2ce7 commented at 2:23 AM on February 25, 2017: none

    Concept ACK. I think that it is important to have an API Break at some point. Moving away from Decimal Amounts is very desirable.

  8. jgarzik commented at 2:53 AM on February 25, 2017: contributor

    General agreement, with a notable caveat:

    Older but widely deployed versions of the jansson library will truncate JSON numbers that are integers larger than unsigned 32-bit.

  9. fanquake added the label Brainstorming on Feb 25, 2017
  10. luke-jr commented at 3:21 AM on February 25, 2017: member

    Dislike making another numbered argument.

    If we care about jansson bugs, I suspect throwing a ".0" on the end would workaround them.

  11. jgarzik commented at 3:26 AM on February 25, 2017: contributor

    On the floating point side, you're limited by the 'double' C data type.

    Some people just throw up their hands and store numbers inside of strings :)

  12. JeremyRubin commented at 3:33 AM on February 25, 2017: contributor

    One idea:

    pass a "multiplier" argument to the RPCs such that users can select arbitrary number systems. Default it to Bitcoin or Satoshis. This future proofs the API design in the event of any popular system (or even prepping for fractional satoshis...)

    This could also be useful for people wishing to pay at an exchange rate :)

  13. TheBlueMatt commented at 4:17 AM on February 25, 2017: member

    This is the kind of thing I think should be an option in the named arguments list, but probably not added to every single RPC command's positional parameters.

    On 02/25/17 00:40, Jorge Timón wrote:

    There's been talks about using satoshis in RPC instead of BTC, which requires decimals and don't work well with json.

    On the other hand, such a change would break the RPC API as well as many tests. This is a draft on how we could move there (doing it only for 2 of the 8 RPC calls that would need to be changed). An useDecimalAmounts optional param is added. If true, the user can introduce BTC amounts as before. If false, the user needs to introduce the amounts in satoshis.

    Interestingly enough, for signrawtransaction, useDecimalAmounts can default to false without breaking any tests (which means the amounts in useDecimalAmounts are not being currently tested). In contrast, createrawtransaction would break many tests.

    We could slowly adapt the tests to use satoshis and once that's done, at some point in the future we could break the API, remove all useDecimalAmounts params and not allow to BTC amounts anymore.

    I didn't want go further before getting some feedback.


        You can view, comment on, or merge this pull request online at:

    #9855

        Commit Summary

    * RPC: Allow AmountFromValue() to process in units other than satoshis

    — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/bitcoin/bitcoin/pull/9855, or mute the thread https://github.com/notifications/unsubscribe-auth/AAnoHrz_SwveslAtCRJeFYOEXrqLbob2ks5rf3iZgaJpZM4ML1JD.

  14. luke-jr commented at 5:49 AM on February 25, 2017: member

    double is large enough for any satoshi amounts on most systems (and any exceptions are likely to not support Core at all right now).

  15. laanwj commented at 7:49 AM on February 25, 2017: member

    NACK doing it in this way.

    • I'd prefer "return amounts as strings". It would be more consistent. "Integers" don't exist in JSON. A JSON parser could still convert large numbers to float silently. Returning the values as strings means the program can use whatever internal format they want - it moves parsing away from the JSON parser, which is usually hard to override.

    We already added handling of these for input in AmountFromValue in #6380, with the idea of making that an option later.

    • If you really want to go ahead with this please see my implementation in #3759. It was much less invasive, effectively only two functions were touched AmountFromValue and ValueFromAmount, and it allowed the user to select between four variants with a command-line option.

    There is no need to change every single call site of those functions for this!

  16. jtimon commented at 12:13 AM on February 28, 2017: contributor

    Replaced by #9882

  17. jtimon closed this on Feb 28, 2017

  18. DrahtBot locked this on Sep 8, 2021

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-04-13 15:15 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me