These two places look like they obviously rely on undefined behaviour to me. See e.g. http://blog.regehr.org/archives/213 for a primer. In short, the compiler is free to do pretty much whatever it wants if the sum of two signed integers overflows. This includes deleting your wallet file, stopping the process, or simply optimising out the check entirely.
Please review carefully.
While (a + b) is guaranteed to not overflow the 63 bits of int64_t if
both MoneyRange(a) and MoneyRange(b), we should still not compute it
before checking that these conditions hold, since that could lead to
undefined behaviour.
Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/c5392e6b9996ab46d6377091b9fae25801b25e37 for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.
I may be misunderstanding the link in connection with the changes. Could you define the scenario in which the original lines would trigger undefined behavior, and how your code fixes said situation?
On second look, it seems like the overflow is not possible after all and the values are already checked before reaching this point, in CheckTransaction(), anyway. Never mind, closing. Thanks.