rpc: Accept scientific notation for monetary amounts in JSON #6379

pull laanwj wants to merge 1 commits into bitcoin:master from laanwj:2015_07_fixedpoint_iterations changing 5 files +215 −1

laanwj commented at 8:54 AM on July 6, 2015: member

Add a function ParseFixedPoint that parses numbers according to the JSON number specification and returns a 64-bit integer.

Then use this in AmountFromValue, rather than ParseMoney.

Also add lots of tests (thanks to @jonasschnelli for some of them).

Fixes issue #6297.
laanwj added the label RPC on Jul 6, 2015

jonasschnelli commented at 12:32 PM on July 6, 2015: contributor

Tested ACK (reviewed code/tests, ran tests, successfully ran smartfees.py, some curl scientific notation tests).

jonasschnelli$ curl --user bitcoinrpc:DP6DvqZtqXarpeNWyN3LZTFchCCyCUuHwNF7E8pX99x1 --data-binary '{"jsonrpc": "1.0", "id" : "1", "method": "sendtoaddress", "params": ["myWgdwSvL2z2WxUbc7wKnRnWnbR925YDaH", 1e-4] }' http://127.0.0.1:18332
{"result":"2ebf06579a36aa8a452924a071460de05d5d87ffc9266d35190ebc42e8476bf9","error":null,"id":"1"}

jonasschnelli$ ./src/bitcoin-cli --regtest gettransaction 2ebf06579a36aa8a452924a071460de05d5d87ffc9266d35190ebc42e8476bf9
{
  "amount": 0.00000000,
  "fee": -0.00000226,
  "confirmations": 0,
  "txid": "2ebf06579a36aa8a452924a071460de05d5d87ffc9266d35190ebc42e8476bf9",
  "walletconflicts": [
  ],
  "time": 1436184739,
  "timereceived": 1436184739,
  "details": [
    {
      "account": "",
      "address": "myWgdwSvL2z2WxUbc7wKnRnWnbR925YDaH",
      "category": "send",
      "amount": -0.00010000,
...

in src/test/rpc_tests.cpp:None in 6739d3c05a outdated

 146 | +    BOOST_CHECK_EQUAL(AmountFromValue(ValueFromString("1e-8")), COIN/100000000);
 147 | +    BOOST_CHECK_EQUAL(AmountFromValue(ValueFromString("0.1e-7")), COIN/100000000);
 148 | +    BOOST_CHECK_EQUAL(AmountFromValue(ValueFromString("0.01e-6")), COIN/100000000);
 149 | +    BOOST_CHECK_EQUAL(AmountFromValue(ValueFromString("0.0000000000000000000000000000000000000000000000000000000000000000000000000001e+68")), COIN/100000000);
 150 | +    BOOST_CHECK_EQUAL(AmountFromValue(ValueFromString("10000000000000000000000000000000000000000000000000000000000000000e-64")), COIN);
 151 | +    BOOST_CHECK_EQUAL(AmountFromValue(ValueFromString("10000000000000000000000000000000000000000000000000000000000000000e-64")), COIN);

morcos commented at 3:38 PM on July 6, 2015:

Isn't this a duplicate of the above line?

laanwj commented at 3:46 PM on July 6, 2015:

Yes, will remove (or if you have another test to suggest, please do :-).

morcos commented at 4:25 PM on July 6, 2015:

maybe add some zeroes after the decimal point. you have an excessive number before and and excessive number after, try a test with an excessive number of zeroes both before and after....

laanwj commented at 7:47 PM on July 6, 2015:

Ok, done

jgarzik commented at 4:27 PM on July 6, 2015: contributor

ut ACK
morcos commented at 5:35 PM on July 6, 2015: member

ACK (code review and tested, but not exhaustively)
laanwj force-pushed on Jul 6, 2015
morcos cross-referenced this on Jul 7, 2015 from issue rpc: Accept strings in AmountFromValue by laanwj
in src/utilstrencodings.cpp:None in 768516e55d outdated
```
 537 | @@ -538,3 +538,118 @@ int atoi(const std::string& str)
 538 |  {
 539 |      return atoi(str.c_str());
 540 |  }
 541 | +
 542 | +/** Upper bound for mantissa.
 543 | + * 10^18-1, largest arbitrary decimal that will fit in 64 bit.
```
sipa commented at 8:25 PM on July 9, 2015:

Hypernit: this comment seems inaccurate. There are definitely larger integers that fit in 64 bit.
laanwj commented at 1:35 PM on July 10, 2015:

it's worded somewhat ackwardly: my point is that larger 64-bit signed integers cannot consist of arbitrary combinations of 0-9:

999999999999999999 1^18-1 9223372036854775807 (1<<63)-1 (max) 9999999999999999999 would not fit

Wouldn't be impossible to handle it, but I it would complicate bounds checking and don't see the point in our case. I'll update the comment.
sipa commented at 8:38 PM on July 9, 2015: member

ACK. Carefully reviewed the code, but didn't test. The included test cases are convincing.

rpc: Accept scientific notation for monetary amounts in JSON

Add a function `ParseFixedPoint` that parses numbers according
to the JSON number specification and returns a 64-bit integer.

Then this in `AmountFromValue`, rather than `ParseMoney`.

Also add lots of tests (thanks to @jonasschnelli for some of them).

Fixes issue #6297.

9cc91523db

laanwj force-pushed on Jul 10, 2015
laanwj merged this on Jul 10, 2015
laanwj closed this on Jul 10, 2015
laanwj referenced this in commit c52e8b300a on Jul 10, 2015
laanwj cross-referenced this on Feb 9, 2016 from issue RPC doesn't accept scientific notation for amounts by laanwj
str4d cross-referenced this on Feb 10, 2017 from issue Add UniValue as subtree by str4d
zkbot referenced this in commit eaaa5f625f on Feb 10, 2017
dagurval cross-referenced this on Mar 18, 2017 from issue rpc: Accept strings in AmountFromValue by dagurval
furszy cross-referenced this on Jun 6, 2020 from issue Base work for the Sapling signatureHash by furszy
furszy cross-referenced this on Jun 25, 2020 from issue RPC & Mempool back ports. by furszy
random-zebra referenced this in commit 6bc917e859 on Jul 6, 2020
random-zebra referenced this in commit 5a092159f6 on Aug 5, 2020
bitcoin locked this on Sep 8, 2021

Contributors

Labels

Linked (view graph)

#6297 RPC doesn't accept scientific notation for amounts #6380 rpc: Accept strings in AmountFromValue