Merge pull request #6380

9127e97 doc: Mention RPC strings for monetary amounts in release notes (Wladimir J. van der Laan)
7d226b7 [QA] add testcases for parsing strings as values (Jonas Schnelli)
614601b rpc: Accept strings in AmountFromValue (Wladimir J. van der Laan)

3 files changed, 57 insertions, 22 deletions

diff --git a/doc/release-notes.md b/doc/release-notes.md
index d5ac70380f..7480a7cd21 100644
--- a/doc/release-notes.md
+++ b/doc/release-notes.md
@@ -19,10 +19,13 @@ https://www.torproject.org/docs/tor-manual.html.en
 
 This allows running bitcoind without having to do any manual configuration.
 
-Example header
-----------------------
+Low-level RPC API changes
+--------------------------
 
-Example content.
+- Monetary amounts can be provided as strings. This means that for example the
+  argument to sendtoaddress can be "0.0001" instead of 0.0001. This can be an
+  advantage if a JSON library insists on using a lossy floating point type for
+  numbers, which would be dangerous for monetary amounts.
 
 0.12.0 Change log
 =================
diff --git a/qa/rpc-tests/wallet.py b/qa/rpc-tests/wallet.py
index 46dc7765b6..f9ec6f429b 100755
--- a/qa/rpc-tests/wallet.py
+++ b/qa/rpc-tests/wallet.py
@@ -4,11 +4,11 @@
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
 #
-# Exercise the wallet.  Ported from wallet.sh.  
+# Exercise the wallet.  Ported from wallet.sh.
 # Does the following:
 #   a) creates 3 nodes, with an empty chain (no blocks).
 #   b) node0 mines a block
-#   c) node1 mines 101 blocks, so now nodes 0 and 1 have 50btc, node2 has none. 
+#   c) node1 mines 101 blocks, so now nodes 0 and 1 have 50btc, node2 has none.
 #   d) node0 sends 21 btc to node2, in two transactions (11 btc, then 10 btc).
 #   e) node0 mines a block, collects the fee on the second transaction
 #   f) node1 mines 100 blocks, to mature node0's just-mined block
@@ -75,14 +75,14 @@ class WalletTest (BitcoinTestFramework):
         assert_equal(self.nodes[2].getbalance(), 21)
 
         # Node0 should have two unspent outputs.
-        # Create a couple of transactions to send them to node2, submit them through 
-        # node1, and make sure both node0 and node2 pick them up properly: 
+        # Create a couple of transactions to send them to node2, submit them through
+        # node1, and make sure both node0 and node2 pick them up properly:
         node0utxos = self.nodes[0].listunspent(1)
         assert_equal(len(node0utxos), 2)
 
         # create both transactions
         txns_to_send = []
-        for utxo in node0utxos: 
+        for utxo in node0utxos:
             inputs = []
             outputs = {}
             inputs.append({ "txid" : utxo["txid"], "vout" : utxo["vout"]})
@@ -149,27 +149,27 @@ class WalletTest (BitcoinTestFramework):
         sync_mempools(self.nodes)
 
         assert(txid1 in self.nodes[3].getrawmempool())
-        
+
         #check if we can list zero value tx as available coins
         #1. create rawtx
-        #2. hex-changed one output to 0.0 
+        #2. hex-changed one output to 0.0
         #3. sign and send
         #4. check if recipient (node0) can list the zero value tx
         usp = self.nodes[1].listunspent()
         inputs = [{"txid":usp[0]['txid'], "vout":usp[0]['vout']}]
         outputs = {self.nodes[1].getnewaddress(): 49.998, self.nodes[0].getnewaddress(): 11.11}
-        
+
         rawTx = self.nodes[1].createrawtransaction(inputs, outputs).replace("c0833842", "00000000") #replace 11.11 with 0.0 (int32)
         decRawTx = self.nodes[1].decoderawtransaction(rawTx)
         signedRawTx = self.nodes[1].signrawtransaction(rawTx)
         decRawTx = self.nodes[1].decoderawtransaction(signedRawTx['hex'])
         zeroValueTxid= decRawTx['txid']
         sendResp = self.nodes[1].sendrawtransaction(signedRawTx['hex'])
-        
+
         self.sync_all()
         self.nodes[1].generate(1) #mine a block
         self.sync_all()
-        
+
         unspentTxs = self.nodes[0].listunspent() #zero value tx must be in listunspents output
         found = False
         for uTx in unspentTxs:
@@ -177,7 +177,7 @@ class WalletTest (BitcoinTestFramework):
                 found = True
                 assert_equal(uTx['amount'], Decimal('0.00000000'));
         assert(found)
-        
+
         #do some -walletbroadcast tests
         stop_nodes(self.nodes)
         wait_bitcoinds()
@@ -192,17 +192,17 @@ class WalletTest (BitcoinTestFramework):
         self.nodes[1].generate(1) #mine a block, tx should not be in there
         self.sync_all()
         assert_equal(self.nodes[2].getbalance(), Decimal('59.99800000')); #should not be changed because tx was not broadcasted
-        
+
         #now broadcast from another node, mine a block, sync, and check the balance
         self.nodes[1].sendrawtransaction(txObjNotBroadcasted['hex'])
         self.nodes[1].generate(1)
         self.sync_all()
         txObjNotBroadcasted = self.nodes[0].gettransaction(txIdNotBroadcasted)
         assert_equal(self.nodes[2].getbalance(), Decimal('61.99800000')); #should not be
-        
+
         #create another tx
         txIdNotBroadcasted  = self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), 2);
-        
+
         #restart the nodes with -walletbroadcast=1
         stop_nodes(self.nodes)
         wait_bitcoinds()
@@ -211,12 +211,44 @@ class WalletTest (BitcoinTestFramework):
         connect_nodes_bi(self.nodes,1,2)
         connect_nodes_bi(self.nodes,0,2)
         sync_blocks(self.nodes)
-        
+
         self.nodes[0].generate(1)
         sync_blocks(self.nodes)
-        
+
         #tx should be added to balance because after restarting the nodes tx should be broadcastet
         assert_equal(self.nodes[2].getbalance(), Decimal('63.99800000')); #should not be
-        
+
+        #send a tx with value in a string (PR#6380 +)
+        txId  = self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), "2")
+        txObj = self.nodes[0].gettransaction(txId)
+        assert_equal(txObj['amount'], Decimal('-2.00000000'))
+
+        txId  = self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), "0.0001")
+        txObj = self.nodes[0].gettransaction(txId)
+        assert_equal(txObj['amount'], Decimal('-0.00010000'))
+
+        #check if JSON parser can handle scientific notation in strings
+        txId  = self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), "1e-4")
+        txObj = self.nodes[0].gettransaction(txId)
+        assert_equal(txObj['amount'], Decimal('-0.00010000'))
+
+        #this should fail
+        errorString = ""
+        try:
+            txId  = self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), "1f-4")
+        except JSONRPCException,e:
+            errorString = e.error['message']
+
+        assert_equal("Invalid amount" in errorString, True);
+
+        errorString = ""
+        try:
+            self.nodes[0].generate("2") #use a string to as block amount parameter must fail because it's not interpreted as amount
+        except JSONRPCException,e:
+            errorString = e.error['message']
+
+        assert_equal("not an integer" in errorString, True);
+
+
 if __name__ == '__main__':
     WalletTest ().main ()
diff --git a/src/rpcserver.cpp b/src/rpcserver.cpp
index 201fc5eba8..03c123a361 100644
--- a/src/rpcserver.cpp
+++ b/src/rpcserver.cpp
@@ -120,8 +120,8 @@ void RPCTypeCheckObj(const UniValue& o,
 
 CAmount AmountFromValue(const UniValue& value)
 {
-    if (!value.isNum())
-        throw JSONRPCError(RPC_TYPE_ERROR, "Amount is not a number");
+    if (!value.isNum() && !value.isStr())
+        throw JSONRPCError(RPC_TYPE_ERROR, "Amount is not a number or string");
     CAmount amount;
     if (!ParseFixedPoint(value.getValStr(), 8, &amount))
         throw JSONRPCError(RPC_TYPE_ERROR, "Invalid amount");