From dac37823d4799477b19434d4d53c74c4af455c76 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Mon, 7 Aug 2017 14:18:47 +0200 Subject: [PATCH 1/4] doc: Correct AmountFromValue/ValueFromAmount names --- doc/developer-notes.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/developer-notes.md b/doc/developer-notes.md index 458e7bcbf..d783a7a8a 100644 --- a/doc/developer-notes.md +++ b/doc/developer-notes.md @@ -561,10 +561,10 @@ A few guidelines for introducing and reviewing new RPC interfaces: which is error prone, and it is easy to get things such as escaping wrong. JSON already supports nested data structures, no need to re-invent the wheel. - - *Exception*: AmountToValue can parse amounts as string. This was introduced because many JSON + - *Exception*: AmountFromValue can parse amounts as string. This was introduced because many JSON parsers and formatters hard-code handling decimal numbers as floating point values, resulting in potential loss of precision. This is unacceptable for - monetary values. **Always** use `AmountToValue` and `ValueToAmount` when + monetary values. **Always** use `AmountFromValue` and `ValueFromAmount` when inputting or outputting monetary values. The only exceptions to this are `prioritisetransaction` and `getblocktemplate` because their interface is specified as-is in BIP22. From 46347add438d49a69f34a3f2ab755feda7daff10 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Mon, 7 Aug 2017 14:38:39 +0200 Subject: [PATCH 2/4] rpc: Move ValueFromAmount to core_write This is necessary because core_write has to write amounts in TxToUniv, and mistakingly uses FormatMoney for that (which is only for debugging). We don't move AmountFromValue at the same time, as this is more challenging due to the RPCError depencency there. --- src/core_io.h | 3 +++ src/core_write.cpp | 10 ++++++++++ src/rpc/misc.cpp | 1 + src/rpc/net.cpp | 1 + src/rpc/server.cpp | 10 ---------- src/rpc/server.h | 1 - src/test/rpc_tests.cpp | 1 + 7 files changed, 16 insertions(+), 11 deletions(-) diff --git a/src/core_io.h b/src/core_io.h index 2d63be5fc..3f25faf0e 100644 --- a/src/core_io.h +++ b/src/core_io.h @@ -5,6 +5,8 @@ #ifndef BITCOIN_CORE_IO_H #define BITCOIN_CORE_IO_H +#include "amount.h" + #include #include @@ -25,6 +27,7 @@ uint256 ParseHashStr(const std::string&, const std::string& strName); std::vector ParseHexUV(const UniValue& v, const std::string& strName); // core_write.cpp +UniValue ValueFromAmount(const CAmount& amount); std::string FormatScript(const CScript& script); std::string EncodeHexTx(const CTransaction& tx, const int serializeFlags = 0); void ScriptPubKeyToUniv(const CScript& scriptPubKey, UniValue& out, bool fIncludeHex); diff --git a/src/core_write.cpp b/src/core_write.cpp index a366ef933..2bf28a150 100644 --- a/src/core_write.cpp +++ b/src/core_write.cpp @@ -16,6 +16,16 @@ #include "utilmoneystr.h" #include "utilstrencodings.h" +UniValue ValueFromAmount(const CAmount& amount) +{ + bool sign = amount < 0; + int64_t n_abs = (sign ? -amount : amount); + int64_t quotient = n_abs / COIN; + int64_t remainder = n_abs % COIN; + return UniValue(UniValue::VNUM, + strprintf("%s%d.%08d", sign ? "-" : "", quotient, remainder)); +} + std::string FormatScript(const CScript& script) { std::string ret; diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp index f3c86038a..cd93919eb 100644 --- a/src/rpc/misc.cpp +++ b/src/rpc/misc.cpp @@ -6,6 +6,7 @@ #include "base58.h" #include "chain.h" #include "clientversion.h" +#include "core_io.h" #include "init.h" #include "validation.h" #include "httpserver.h" diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index b4d6795e6..6271e0cc8 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -6,6 +6,7 @@ #include "chainparams.h" #include "clientversion.h" +#include "core_io.h" #include "validation.h" #include "net.h" #include "net_processing.h" diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index 63e4e9c63..58640d69d 100644 --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -123,16 +123,6 @@ CAmount AmountFromValue(const UniValue& value) return amount; } -UniValue ValueFromAmount(const CAmount& amount) -{ - bool sign = amount < 0; - int64_t n_abs = (sign ? -amount : amount); - int64_t quotient = n_abs / COIN; - int64_t remainder = n_abs % COIN; - return UniValue(UniValue::VNUM, - strprintf("%s%d.%08d", sign ? "-" : "", quotient, remainder)); -} - uint256 ParseHashV(const UniValue& v, std::string strName) { std::string strHex; diff --git a/src/rpc/server.h b/src/rpc/server.h index b20c82772..dd6f76324 100644 --- a/src/rpc/server.h +++ b/src/rpc/server.h @@ -185,7 +185,6 @@ extern std::vector ParseHexV(const UniValue& v, std::string strNa extern std::vector ParseHexO(const UniValue& o, std::string strKey); extern CAmount AmountFromValue(const UniValue& value); -extern UniValue ValueFromAmount(const CAmount& amount); extern std::string HelpExampleCli(const std::string& methodname, const std::string& args); extern std::string HelpExampleRpc(const std::string& methodname, const std::string& args); diff --git a/src/test/rpc_tests.cpp b/src/test/rpc_tests.cpp index 134bd7c60..c6643be7a 100644 --- a/src/test/rpc_tests.cpp +++ b/src/test/rpc_tests.cpp @@ -6,6 +6,7 @@ #include "rpc/client.h" #include "base58.h" +#include "core_io.h" #include "netbase.h" #include "test/test_bitcoin.h" From ec05c508c681b01ca990f091f346f5171538ce8d Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Mon, 7 Aug 2017 14:41:29 +0200 Subject: [PATCH 3/4] rpc: Use ValueFromAmount instead of FormatMoney in TxToUniv With this, the amounts returned in `decoderawtransaction` will be padded to 8 digits like anywhwere else in the API. --- src/core_write.cpp | 3 +-- test/util/data/tt-delin1-out.json | 2 +- test/util/data/tt-delout1-out.json | 2 +- test/util/data/tt-locktime317000-out.json | 2 +- test/util/data/txcreate1.json | 4 ++-- test/util/data/txcreate2.json | 2 +- test/util/data/txcreatedata1.json | 4 ++-- test/util/data/txcreatedata2.json | 4 ++-- test/util/data/txcreatedata_seq0.json | 2 +- test/util/data/txcreatedata_seq1.json | 2 +- test/util/data/txcreatemultisig1.json | 2 +- test/util/data/txcreatemultisig2.json | 2 +- test/util/data/txcreatemultisig3.json | 2 +- test/util/data/txcreatemultisig4.json | 2 +- test/util/data/txcreateoutpubkey1.json | 2 +- test/util/data/txcreateoutpubkey2.json | 2 +- test/util/data/txcreateoutpubkey3.json | 2 +- test/util/data/txcreatescript1.json | 2 +- test/util/data/txcreatescript2.json | 2 +- test/util/data/txcreatescript3.json | 2 +- test/util/data/txcreatescript4.json | 2 +- test/util/data/txcreatesignv1.json | 2 +- 22 files changed, 25 insertions(+), 26 deletions(-) diff --git a/src/core_write.cpp b/src/core_write.cpp index 2bf28a150..217b491a0 100644 --- a/src/core_write.cpp +++ b/src/core_write.cpp @@ -194,8 +194,7 @@ void TxToUniv(const CTransaction& tx, const uint256& hashBlock, UniValue& entry) UniValue out(UniValue::VOBJ); - UniValue outValue(UniValue::VNUM, FormatMoney(txout.nValue)); - out.pushKV("value", outValue); + out.pushKV("value", ValueFromAmount(txout.nValue)); out.pushKV("n", (int64_t)i); UniValue o(UniValue::VOBJ); diff --git a/test/util/data/tt-delin1-out.json b/test/util/data/tt-delin1-out.json index f6dfbb51c..0b3235dd4 100644 --- a/test/util/data/tt-delin1-out.json +++ b/test/util/data/tt-delin1-out.json @@ -189,7 +189,7 @@ ], "vout": [ { - "value": 1.3782, + "value": 1.37820000, "n": 0, "scriptPubKey": { "asm": "OP_DUP OP_HASH160 8fd139bb39ced713f231c58a4d07bf6954d1c201 OP_EQUALVERIFY OP_CHECKSIG", diff --git a/test/util/data/tt-delout1-out.json b/test/util/data/tt-delout1-out.json index 6769ed79f..5b69d0cd8 100644 --- a/test/util/data/tt-delout1-out.json +++ b/test/util/data/tt-delout1-out.json @@ -198,7 +198,7 @@ ], "vout": [ { - "value": 1.3782, + "value": 1.37820000, "n": 0, "scriptPubKey": { "asm": "OP_DUP OP_HASH160 8fd139bb39ced713f231c58a4d07bf6954d1c201 OP_EQUALVERIFY OP_CHECKSIG", diff --git a/test/util/data/tt-locktime317000-out.json b/test/util/data/tt-locktime317000-out.json index 82b64df07..cf1ebcdf3 100644 --- a/test/util/data/tt-locktime317000-out.json +++ b/test/util/data/tt-locktime317000-out.json @@ -198,7 +198,7 @@ ], "vout": [ { - "value": 1.3782, + "value": 1.37820000, "n": 0, "scriptPubKey": { "asm": "OP_DUP OP_HASH160 8fd139bb39ced713f231c58a4d07bf6954d1c201 OP_EQUALVERIFY OP_CHECKSIG", diff --git a/test/util/data/txcreate1.json b/test/util/data/txcreate1.json index 36741044c..edb091f94 100644 --- a/test/util/data/txcreate1.json +++ b/test/util/data/txcreate1.json @@ -36,7 +36,7 @@ ], "vout": [ { - "value": 0.18, + "value": 0.18000000, "n": 0, "scriptPubKey": { "asm": "OP_DUP OP_HASH160 1fc11f39be1729bf973a7ab6a615ca4729d64574 OP_EQUALVERIFY OP_CHECKSIG", @@ -49,7 +49,7 @@ } }, { - "value": 4.00, + "value": 4.00000000, "n": 1, "scriptPubKey": { "asm": "OP_DUP OP_HASH160 f2d4db28cad6502226ee484ae24505c2885cb12d OP_EQUALVERIFY OP_CHECKSIG", diff --git a/test/util/data/txcreate2.json b/test/util/data/txcreate2.json index 23fe7ace6..cca00f752 100644 --- a/test/util/data/txcreate2.json +++ b/test/util/data/txcreate2.json @@ -9,7 +9,7 @@ ], "vout": [ { - "value": 0.00, + "value": 0.00000000, "n": 0, "scriptPubKey": { "asm": "", diff --git a/test/util/data/txcreatedata1.json b/test/util/data/txcreatedata1.json index e65a1859e..e66a6bb9a 100644 --- a/test/util/data/txcreatedata1.json +++ b/test/util/data/txcreatedata1.json @@ -18,7 +18,7 @@ ], "vout": [ { - "value": 0.18, + "value": 0.18000000, "n": 0, "scriptPubKey": { "asm": "OP_DUP OP_HASH160 1fc11f39be1729bf973a7ab6a615ca4729d64574 OP_EQUALVERIFY OP_CHECKSIG", @@ -31,7 +31,7 @@ } }, { - "value": 4.00, + "value": 4.00000000, "n": 1, "scriptPubKey": { "asm": "OP_RETURN 54686973204f505f52455455524e207472616e73616374696f6e206f7574707574207761732063726561746564206279206d6f646966696564206372656174657261777472616e73616374696f6e2e", diff --git a/test/util/data/txcreatedata2.json b/test/util/data/txcreatedata2.json index 8f1544e1c..0f8edcafd 100644 --- a/test/util/data/txcreatedata2.json +++ b/test/util/data/txcreatedata2.json @@ -18,7 +18,7 @@ ], "vout": [ { - "value": 0.18, + "value": 0.18000000, "n": 0, "scriptPubKey": { "asm": "OP_DUP OP_HASH160 1fc11f39be1729bf973a7ab6a615ca4729d64574 OP_EQUALVERIFY OP_CHECKSIG", @@ -31,7 +31,7 @@ } }, { - "value": 0.00, + "value": 0.00000000, "n": 1, "scriptPubKey": { "asm": "OP_RETURN 54686973204f505f52455455524e207472616e73616374696f6e206f7574707574207761732063726561746564206279206d6f646966696564206372656174657261777472616e73616374696f6e2e", diff --git a/test/util/data/txcreatedata_seq0.json b/test/util/data/txcreatedata_seq0.json index e52401f41..4b5a7cab4 100644 --- a/test/util/data/txcreatedata_seq0.json +++ b/test/util/data/txcreatedata_seq0.json @@ -18,7 +18,7 @@ ], "vout": [ { - "value": 0.18, + "value": 0.18000000, "n": 0, "scriptPubKey": { "asm": "OP_DUP OP_HASH160 1fc11f39be1729bf973a7ab6a615ca4729d64574 OP_EQUALVERIFY OP_CHECKSIG", diff --git a/test/util/data/txcreatedata_seq1.json b/test/util/data/txcreatedata_seq1.json index 093ff4a56..771ff1bb1 100644 --- a/test/util/data/txcreatedata_seq1.json +++ b/test/util/data/txcreatedata_seq1.json @@ -27,7 +27,7 @@ ], "vout": [ { - "value": 0.18, + "value": 0.18000000, "n": 0, "scriptPubKey": { "asm": "OP_DUP OP_HASH160 1fc11f39be1729bf973a7ab6a615ca4729d64574 OP_EQUALVERIFY OP_CHECKSIG", diff --git a/test/util/data/txcreatemultisig1.json b/test/util/data/txcreatemultisig1.json index 0cc530836..7c814dad8 100644 --- a/test/util/data/txcreatemultisig1.json +++ b/test/util/data/txcreatemultisig1.json @@ -9,7 +9,7 @@ ], "vout": [ { - "value": 1.00, + "value": 1.00000000, "n": 0, "scriptPubKey": { "asm": "2 02a5613bd857b7048924264d1e70e08fb2a7e6527d32b7ab1bb993ac59964ff397 021ac43c7ff740014c3b33737ede99c967e4764553d1b2b83db77c83b8715fa72d 02df2089105c77f266fa11a9d33f05c735234075f2e8780824c6b709415f9fb485 3 OP_CHECKMULTISIG", diff --git a/test/util/data/txcreatemultisig2.json b/test/util/data/txcreatemultisig2.json index 8ad2ffdc6..7d94ce739 100644 --- a/test/util/data/txcreatemultisig2.json +++ b/test/util/data/txcreatemultisig2.json @@ -9,7 +9,7 @@ ], "vout": [ { - "value": 1.00, + "value": 1.00000000, "n": 0, "scriptPubKey": { "asm": "OP_HASH160 1c6fbaf46d64221e80cbae182c33ddf81b9294ac OP_EQUAL", diff --git a/test/util/data/txcreatemultisig3.json b/test/util/data/txcreatemultisig3.json index 086bf44b8..06e093e22 100644 --- a/test/util/data/txcreatemultisig3.json +++ b/test/util/data/txcreatemultisig3.json @@ -9,7 +9,7 @@ ], "vout": [ { - "value": 1.00, + "value": 1.00000000, "n": 0, "scriptPubKey": { "asm": "0 e15a86a23178f433d514dbbce042e87d72662b8b5edcacfd2e37ab7a2d135f05", diff --git a/test/util/data/txcreatemultisig4.json b/test/util/data/txcreatemultisig4.json index d23ccc045..9a5d2f4a0 100644 --- a/test/util/data/txcreatemultisig4.json +++ b/test/util/data/txcreatemultisig4.json @@ -9,7 +9,7 @@ ], "vout": [ { - "value": 1.00, + "value": 1.00000000, "n": 0, "scriptPubKey": { "asm": "OP_HASH160 6edf12858999f0dae74f9c692e6694ee3621b2ac OP_EQUAL", diff --git a/test/util/data/txcreateoutpubkey1.json b/test/util/data/txcreateoutpubkey1.json index f10aaecf7..2704ed767 100644 --- a/test/util/data/txcreateoutpubkey1.json +++ b/test/util/data/txcreateoutpubkey1.json @@ -9,7 +9,7 @@ ], "vout": [ { - "value": 0.00, + "value": 0.00000000, "n": 0, "scriptPubKey": { "asm": "02a5613bd857b7048924264d1e70e08fb2a7e6527d32b7ab1bb993ac59964ff397 OP_CHECKSIG", diff --git a/test/util/data/txcreateoutpubkey2.json b/test/util/data/txcreateoutpubkey2.json index 5a473b76c..514472223 100644 --- a/test/util/data/txcreateoutpubkey2.json +++ b/test/util/data/txcreateoutpubkey2.json @@ -9,7 +9,7 @@ ], "vout": [ { - "value": 0.00, + "value": 0.00000000, "n": 0, "scriptPubKey": { "asm": "0 a2516e770582864a6a56ed21a102044e388c62e3", diff --git a/test/util/data/txcreateoutpubkey3.json b/test/util/data/txcreateoutpubkey3.json index b8389b8f7..0a5d489e1 100644 --- a/test/util/data/txcreateoutpubkey3.json +++ b/test/util/data/txcreateoutpubkey3.json @@ -9,7 +9,7 @@ ], "vout": [ { - "value": 0.00, + "value": 0.00000000, "n": 0, "scriptPubKey": { "asm": "OP_HASH160 a5ab14c9804d0d8bf02f1aea4e82780733ad0a83 OP_EQUAL", diff --git a/test/util/data/txcreatescript1.json b/test/util/data/txcreatescript1.json index 823168e9f..5072452fe 100644 --- a/test/util/data/txcreatescript1.json +++ b/test/util/data/txcreatescript1.json @@ -9,7 +9,7 @@ ], "vout": [ { - "value": 0.00, + "value": 0.00000000, "n": 0, "scriptPubKey": { "asm": "OP_DROP", diff --git a/test/util/data/txcreatescript2.json b/test/util/data/txcreatescript2.json index d4c7e10c7..94b669ffb 100644 --- a/test/util/data/txcreatescript2.json +++ b/test/util/data/txcreatescript2.json @@ -9,7 +9,7 @@ ], "vout": [ { - "value": 0.00, + "value": 0.00000000, "n": 0, "scriptPubKey": { "asm": "OP_HASH160 71ed53322d470bb96657deb786b94f97dd46fb15 OP_EQUAL", diff --git a/test/util/data/txcreatescript3.json b/test/util/data/txcreatescript3.json index 001e69511..980da2fb3 100644 --- a/test/util/data/txcreatescript3.json +++ b/test/util/data/txcreatescript3.json @@ -9,7 +9,7 @@ ], "vout": [ { - "value": 0.00, + "value": 0.00000000, "n": 0, "scriptPubKey": { "asm": "0 0bfe935e70c321c7ca3afc75ce0d0ca2f98b5422e008bb31c00c6d7f1f1c0ad6", diff --git a/test/util/data/txcreatescript4.json b/test/util/data/txcreatescript4.json index 20094bcd4..eecdf858b 100644 --- a/test/util/data/txcreatescript4.json +++ b/test/util/data/txcreatescript4.json @@ -9,7 +9,7 @@ ], "vout": [ { - "value": 0.00, + "value": 0.00000000, "n": 0, "scriptPubKey": { "asm": "OP_HASH160 6a2c482f4985f57e702f325816c90e3723ca81ae OP_EQUAL", diff --git a/test/util/data/txcreatesignv1.json b/test/util/data/txcreatesignv1.json index 519d3ab06..92a3f76a0 100644 --- a/test/util/data/txcreatesignv1.json +++ b/test/util/data/txcreatesignv1.json @@ -18,7 +18,7 @@ ], "vout": [ { - "value": 0.001, + "value": 0.00100000, "n": 0, "scriptPubKey": { "asm": "OP_DUP OP_HASH160 5834479edbbe0539b31ffd3a8f8ebadc2165ed01 OP_EQUALVERIFY OP_CHECKSIG", From ce076383a8578626a7eac37533cba26dece1c877 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Mon, 7 Aug 2017 17:10:42 +0200 Subject: [PATCH 4/4] doc: Add comment to use ValueFromAmount/AmountFromValue for JSON, not utilmoneystr --- src/utilmoneystr.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/utilmoneystr.h b/src/utilmoneystr.h index 5839b0734..bc885ee16 100644 --- a/src/utilmoneystr.h +++ b/src/utilmoneystr.h @@ -14,6 +14,9 @@ #include "amount.h" +/* Do not use these functions to represent or parse monetary amounts to or from + * JSON but use AmountFromValue and ValueFromAmount for that. + */ std::string FormatMoney(const CAmount& n); bool ParseMoney(const std::string& str, CAmount& nRet); bool ParseMoney(const char* pszIn, CAmount& nRet);