Invalid integer negation in FormatMoney(CAmount n) and ValueFromAmount (CAmount n) when n = std::numeric_limits<CAmount>::min() #20402

FormatMoney(CAmount n) is not being properly defined for n = std::numeric_limits<CAmount>::min(). This is due to an invalid integer negation: the negation of -9223372036854775808 cannot be represented in type CAmount (which has a max value of 9223372036854775807):

https://github.com/bitcoin/bitcoin/blob/54f812d9d29893c690ae06b84aaeab128186aa36/src/util/moneystr.cpp#L12-L31

FWIW FormatMoney(-9223372036854775808) returns --92233720368.-54775808 when compiled with UBSan (LLVM/Clang, -fsanitize=undefined).

Context: #20383 (review)

It would be nice if FormatMoney(CAmount n) was properly defined for all CAmount n :)

These are the "expectations of least surprise":

BOOST_CHECK_EQUAL(FormatMoney(std::numeric_limits<CAmount>::max()), "92233720368.54775807");       //  9223372036854775807
BOOST_CHECK_EQUAL(FormatMoney(std::numeric_limits<CAmount>::max() - 1), "92233720368.54775806");   //  9223372036854775806
BOOST_CHECK_EQUAL(FormatMoney(std::numeric_limits<CAmount>::max() - 2), "92233720368.54775805");   //  9223372036854775805
BOOST_CHECK_EQUAL(FormatMoney(std::numeric_limits<CAmount>::max() - 3), "92233720368.54775804");   //  9223372036854775804
BOOST_CHECK_EQUAL(FormatMoney(std::numeric_limits<CAmount>::min() + 3), "-92233720368.54775805");  // -9223372036854775805
BOOST_CHECK_EQUAL(FormatMoney(std::numeric_limits<CAmount>::min() + 2), "-92233720368.54775806");  // -9223372036854775806
BOOST_CHECK_EQUAL(FormatMoney(std::numeric_limits<CAmount>::min() + 1), "-92233720368.54775807");  // -9223372036854775807
BOOST_CHECK_EQUAL(FormatMoney(std::numeric_limits<CAmount>::min()), "-92233720368.54775808");      // -9223372036854775808

These assertions are guaranteed to hold for all cases above except for the last one.

I think we have exactly the same small problem for UniValue ValueFromAmount(const CAmount& amount).

It needs the same fix :)

When fuzzing the RPC interface I stumbled upon this case again: decoderawtransaction ff0000000001000000000000008004ff0400fffe001f00 will trigger the problematic code path :)

$ git checkout master
$ CC=clang CXX=clang++ ./configure --with-sanitizers=address,undefined
$ make
$ src/bitcoind &
$ src/bitcoin-cli decoderawtransaction ff0000000001000000000000008004ff0400fffe001f00
core_write.cpp:21:29: runtime error: negation of -9223372036854775808 cannot be represented in type 'CAmount' (aka 'long'); cast to an unsigned type to negate this value to itself
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior core_write.cpp:21:29 in
error: couldn't parse reply from server

The JSON response by the Bitcoin Core RPC interface:

{
   "result" : {
      "size" : 23,
      "txid" : "cc0e5ecf6a06e4587bd011770c3ea4ceab58e86a234dd049ea3a27fab6fccfaa",
      "locktime" : 2031870,
      "hash" : "cc0e5ecf6a06e4587bd011770c3ea4ceab58e86a234dd049ea3a27fab6fccfaa",
      "vsize" : 23,
      "weight" : 92,
      "vout" : [
         {
            "scriptPubKey" : {
               "hex" : "ff0400ff",
               "asm" : "OP_INVALIDOPCODE [error]",
               "type" : "nonstandard"
            },
            "n" : 0,
            "value" : --92233720368.-54775808
         }
      ],
      "version" : 255,
      "vin" : []
   },
   "error" : null,
   "id" : 1
}

Notice the malformed result.vout[0].value field (--92233720368.-54775808) :)