I will not comment about the change itself...

But this will fail:

    BOOST_CHECK_EQUAL(AmountFromValue(ValueFromString("20999999.9999999")), 2099999999999990LL);

and as tests fail, this is sure NACK. The commit message contains typo (CAmout) etc.

Thanks for pointing out the typo.

The tx_valid.json raw txs are spending 2100000000000000 and will need to be corrected accordingly themselves, in addition to the ones you pointed out.

NACK. Too risky and controversial to change this.

I hope it is obvious the risk and controversy lie in the flawed accounting practice, not its correction.

@paveljanik: The related tests have now been updated accordingly: https://github.com/21E14/bitcoin/commit/6f1278e62312384c7e5966030eaa4f05fe68b63a

I think this reinforces a misunderstanding about what this constant does.

The total number of BTC will never exceed 20999839.74826098, due to provably unspendable outputs, overwritten coinbase transactions (see BIP30), unclaimed subsidies (see block 124724), and the unspendable genesis block output. So your number is still not accurate.

The point is, this constant doesn't do anything but prevent some integer wraparound error. I believe we could turn it into 2 billion, and nothing would change at all. Maybe we should :)

I would almost agree with the spirit of the concluding remark. In this instance, inaction is the least preferable option for the reasons cited. The answer to the often brought up theme of max supply ought to be unambiguos. Erroneously, the current constant fails at that, reinforcing the misunderstanding in absurd contexts.

The value ought to be either aligned with the supply i.e. there will never be more (but perhaps less spendable) than 20999999.97690000 mined major units (a truistic statement), or the suggestion inherent in the constant's name ought to be entirely abandoned (max spendable amount as is being implied). The PR advocates the former for practical reasons, but I wouldn't be opposed to the latter, once the references to the constant are disambiguated (unlike mined, spendable is not a constant). Perhaps both in this exact order.

Apart from philosophical reasons there seems to be no motivation for changing this constant. Even if there is little risk, there is no gain to be had. That's a good reason to leave it as is.

If your purpose is to avoid that people misinterpret it, adding a comment explaining the (non-) use of the constant would do just as well.

@laanwj: The sole purpose of two test cases in /src/test/data is to successfully spend more BTC than will ever be mined, and the raw tx data is carefully constructed to make sure the overspend goes smoothly. I am not sure how to explain or comment it away, but am open to suggestions.