Rename MAX_MONEY to AMOUNT_OVERFLOW_PROTECTION_THRESHOLD #6197

Implemented as per sipa's suggestion. Was meant to be merged via #6191 had it not be closed out a little too expeditiously. It's not perfect, but it's good enough.

NAK, see #6198, less invasive.

Either is fine for me.

I also prefer Peter's comment over changing so many files.

The diff is only 12 lines over 7 files. Concept ACK, although it's currently failing Travis...

Keep in mind that this constant is already named "MAX_MONEY" in many other 3rd party libraries, including bitcoinj, libbitcoin, python-bitcoinlib, etc. etc.

Renaming it is needlessly confusing.

The #6198 comment will make a nice addition this PR (#6197), which has now been updated accordingly. Sipa's AMOUNT_OVERFLOW_PROTECTION_THRESHOLD is an elegant way to resolve the controversy. I see other implementations as no reason to perpetuate it.

re: other implementations, that includes a fair number of Bitcoin clients as well. For example: https://github.com/vinumeris/lighthouse/blob/63b35ca3abc6ec18e346d9eeb56462b5f35c6066/client/src/main/java/lighthouse/utils/BitcoinValue.java

Also, it's good practice when you do things like take others' commits and add them to your own pull-reqs to credit appropriately. git cherry-pick is useful for that purpose.

@petertodd The more reason to correct it, and disrupt the ripple effect — no pun intended. The credit for the comment is yours, of course, but it could not be adopted verbatim, due to the const reference. I expect the two to be squashed anyhow.

Either is fine with me too, although I have a slight preference for @petertodd 's solution due to lower impact. The constant is an upper bound on the possible coins, not the limit value of the cumulative reward function, so MAX_xxx is sort of appropriate, the new name is a bit awkward and pedantic (but correct). Getting rid of the word money on the other hand is nice...

The word "money" doesn't bother me at all, but I'd there was consensus to change it, the phrase MAX_VALUE is much less awkward. "Max upper bound on the value you will see in any one place; used as a (consensus critical) sanity check in various places, e.g. the total sum out of transaction txout"

Heh. We had that discussion once; MAX_VALUE and MAX_AMOUNT are too generic, could refer to everything, and could easily collide with other things. At least this long awkward constant name is impossible to mix up.

I prefer the less invasive approach of adding a comment as well. The name MAX_XYZ is appropriate given how the constant is used. Also, I find the name in this pr awkward and way long at 36 characters, which is completely contrary to elegant for me.

@davecgh I'd rather view invasiveness (12 lines over 7 files, as maaku pointed out) as no excuse for canonizing bugs, and that's what MAX_MONEY is. sipa's const and petertodd's comment are an imperfect step, but one in the right direction. As a matter of fact, sipa's choice of words was masterful here, and may survive the test of time as the const gets refactored as its taint inevitably will.

Tested ACK.

@21E14 BTW, MoneyRange() is OK for you?

Needs rebase

Rebased.

The new name really irks me. @petertodd's comment update is good enough, everyone that wonders what the constant does can now easily look it up, without having to embed it all into the identifier. Closing.